Skip to content

feat: add workflow condition expressions#60

Merged
mbelinky merged 2 commits intomainfrom
mariano/slice-conditions
Apr 6, 2026
Merged

feat: add workflow condition expressions#60
mbelinky merged 2 commits intomainfrom
mariano/slice-conditions

Conversation

@mbelinky
Copy link
Copy Markdown
Contributor

@mbelinky mbelinky commented Apr 6, 2026

Summary

Scope

Included:

  • compound workflow conditions
  • nested step refs in comparisons like $review.response.decision == approve
  • dry-run coverage for compound conditions

Explicitly excluded:

Hardening

  • bare identifiers still fail as standalone conditions, so typos do not silently pass
  • unresolved non-step shell vars remain untouched outside condition parsing

Verification

  • pnpm build
  • pnpm test

Ref: #39

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +1157 to +1159
if (token.type === 'step_ref') {
return getStepRefValue(token.value, results, false);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment on lines +1199 to +1201
try {
return JSON.stringify(left) === JSON.stringify(right);
} catch {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@mbelinky mbelinky merged commit e1519f4 into main Apr 6, 2026
@mbelinky mbelinky deleted the mariano/slice-conditions branch April 6, 2026 13:44
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +1158 to +1160
if (token.type === 'step_ref') {
return getStepRefValue(token.value, results, true);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment on lines +1161 to +1163
if (token.type === 'string' || token.type === 'number' || token.type === 'boolean' || token.type === 'null') {
return token.value;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant