Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16d756dddf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (token.type === 'step_ref') { | ||
| return getStepRefValue(token.value, results, false); | ||
| } |
There was a problem hiding this comment.
Reject unknown step refs in condition evaluation
Condition step references are resolved with strict=false, so a typo like !$aprove.approved or $aprove.approved != true yields undefined and then evaluates to true, which can unexpectedly run guarded steps (e.g., deploy gates) instead of failing fast. This behavior is newly reachable because !/!= were added in this change, so unknown step IDs should be treated as errors (or at least as a non-negatable sentinel) during condition evaluation.
Useful? React with 👍 / 👎.
src/workflows/file.ts
Outdated
| try { | ||
| return JSON.stringify(left) === JSON.stringify(right); | ||
| } catch { |
There was a problem hiding this comment.
Compare object operands without key-order sensitivity
Structured comparisons rely on JSON.stringify(left) === JSON.stringify(right), which makes object equality depend on insertion order. Two semantically equivalent objects from different steps can compare unequal when keys are emitted in different orders, causing condition branches to be skipped incorrectly. A real deep-equality check should ignore object key order.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a778f6df3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (token.type === 'step_ref') { | ||
| return getStepRefValue(token.value, results, true); | ||
| } |
There was a problem hiding this comment.
Reject unknown result-field paths in condition refs
Condition refs now use strict lookup, but strictness only checks the step id; a misspelled field path still resolves to undefined. With the new !/!= operators, typos like !$gate.aproved or $gate.aproved != true evaluate to true, which can execute steps that were meant to stay gated (for example deploy/approval-protected steps). Please make condition-time ref resolution fail for unknown top-level result fields (or otherwise prevent undefined from being used as a truthy guard bypass).
Useful? React with 👍 / 👎.
| if (token.type === 'string' || token.type === 'number' || token.type === 'boolean' || token.type === 'null') { | ||
| return token.value; | ||
| } |
There was a problem hiding this comment.
Disallow standalone non-boolean literals in conditions
The parser accepts string/number/null literals as primary expressions and then coerces the final value with Boolean(...), so conditions like 'approve' or 1 now pass as truthy instead of being rejected. This makes partially written or typoed expressions silently become always-on conditions, which is a regression from the earlier “supported condition” validation behavior.
Useful? React with 👍 / 👎.
Summary
!,==,!=,&&,||, and parenthesesScope
Included:
$review.response.decision == approveExplicitly excluded:
retry/retry_delay/on_errornext/ loops /max_iterationsHardening
Verification
pnpm buildpnpm testRef: #39