fix(workflows): an interrupted gate prompt must not approve the gate - #4529
Open
jawwad-ali wants to merge 1 commit into
Open
fix(workflows): an interrupted gate prompt must not approve the gate#4529jawwad-ali wants to merge 1 commit into
jawwad-ali wants to merge 1 commit into
Conversation
`GateStep._prompt` treated Ctrl+C / Ctrl+D as a choice:
except (EOFError, KeyboardInterrupt):
print()
return options[-1] # default to last (usually reject)
"usually reject" is an assumption `validate` never enforces. It requires only
that *some* option is 'reject'/'abort':
reject_choices = {"reject", "abort"}
if not any(o.lower() in reject_choices for o in options):
so a hand-written `options` list whose reject choice is not last validates
clean, and `options[-1]` is then an approving option. `execute` classifies the
result with `if choice.lower() in ("reject", "abort")`, so the gate reported
COMPLETED and the run walked straight past the human review:
options=['approve', 'reject'] -> failed choice='reject'
options=['reject', 'approve'] -> completed choice='approve'
options=['approve', 'reject', 'request-changes'] -> completed choice='request-changes'
All three validate with zero errors.
This also contradicted the engine's own interrupt contract: `WorkflowEngine`
catches KeyboardInterrupt and sets `RunStatus.PAUSED` with a
`workflow_interrupted` log event, so Ctrl+C anywhere else pauses the run for
`specify workflow resume`. Only at a gate did it silently make an approval
decision.
Now prefers the declared reject/abort option, falling back to the last option
when none is declared. For the documented default `[approve, reject]` this is
byte-for-byte the previous behaviour.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
GateStep._prompttreats Ctrl+C / Ctrl+D as a choice:"usually reject" is an assumption
validatenever enforces. It requires only that some option isreject/abort— never that it is last:So a hand-written
optionslist whose reject choice is not last validates clean, andoptions[-1]is then an approving option.executeclassifies the result withif choice.lower() in ("reject", "abort"), so the gate reports COMPLETED and the run continues past the human review the gate exists to enforce.Reproduction on current
main(c173bf1)Interrupting the prompt, with
validate()run on each config first:Identical for both
KeyboardInterruptandEOFError. All three validate with zero errors —specify workflow validategives the author no warning.The middle and last rows are the bug: the operator pressed Ctrl+C at an approval gate and the workflow recorded an approval.
It also contradicts the engine's own interrupt contract
WorkflowEnginecatchesKeyboardInterruptand setsRunStatus.PAUSEDwith aworkflow_interruptedlog event (engine.py:1059-1061), so Ctrl+C anywhere else in a run pauses it forspecify workflow resume. Only at a gate did it silently make a decision instead.Fix
Prefer the declared reject/abort option; fall back to the last option when none is declared.
After the fix, every config carrying a reject option rejects on interrupt:
Behaviour change — disclosed
The returned choice changes only when the prompt is interrupted and a reject/abort option is not last. The documented default
options: [approve, reject]is byte-for-byte unchanged, which thereject_lastparametrization pins by passing both before and after. A workflow that deliberately relied on Ctrl+C selecting a trailing non-reject option (saydefer) would now reject instead — that reliance is precisely the defect being fixed, but it is a real change and worth stating.Verification
upstream/main→ 55 passed with the fix.tests/test_workflows.py: 21 failed / 964 passed against a clean-mainbaseline of 21 failed / 956 passed — no new failures (the 21 are the known Windowsos.replaceflakiness in that file).uvx ruff@0.15.0 check src tests→ cleanWritten with assistance from Claude Code. Bug found, reproduced, and verified by me on current
main.🤖 Generated with Claude Code