fix(workflows): require a cases block on switch steps - #4144
Open
jawwad-ali wants to merge 1 commit into
Open
Conversation
`SwitchStep.validate` requires `expression` and type-checks `cases`, but
never checks that `cases` is PRESENT. It is the only control-flow step whose
branch payload is optional:
if -> requires 'then'
fan-out -> requires 'items' and 'step'
fan-in -> requires a non-empty 'wait_for'
gate -> requires 'message'
switch -> cases optional
So a switch whose branch table is absent or mistyped — `case:` for `cases:`
is the obvious slip — passes validation with zero errors:
if missing then : ["If step 'x' is missing 'then' field."]
fanout missing all: ["Fan-out step 'y' is missing 'items' field.", ...]
switch typo case: : []
switch no cases : []
and then at run time reports COMPLETED with
`matched_case: "__default__"` — a default it does not even declare — having
dispatched nothing, so the whole run "succeeds". That is the "silent empty
result + COMPLETED" wiring bug the fan-in guard exists to prevent.
An explicitly declared but empty `cases: {}` is still a declaration and
stays valid, pinned by a test.
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
SwitchStep.validaterequiresexpressionand type-checkscases/default, but never checks thatcasesis present. It is the only control-flow step whose branch payload is optional:ifthenfan-outitemsandstepfan-inwait_forgatemessagewhile/do-whileconditionswitchReproduction on current
main(bf88c9f)So a
case:/cases:typo passesspecify workflow validatewith zero errors, then at run time reports COMPLETED withmatched_case: "__default__"— a default it does not even declare — having dispatched nothing. The whole run reports success while the branch never executed.That is precisely the "silent empty result + COMPLETED" wiring bug the fan-in
wait_forguard and the engine's fan-in check exist to prevent.Fix
One presence check, mirroring the sibling steps' wording.
Narrow. An explicitly declared but empty
cases: {}is still a declaration and stays valid — pinned bytest_validate_accepts_an_empty_cases_mapping. Only configs that never declared the block at all become errors, and those already do nothing at run time.Verification
-k "Switch or validate or Validat"): 1 failed → 202 passed.tests/workflows→ 171 passed, 10 failed, matching the clean-mainbaseline exactly.uvx ruff@0.15.0 check src tests→ cleanTestSwitchStepclass, next totest_validate_missing_expression.A note on the full-file run.
tests/test_workflows.pyis intermittently flaky on Windows — a rotating handful of tests (TestFanOutConcurrency,TestContextWorkflowDir,TestContinueOnError,TestWorkflowRunExitCodes) fail on different runs and pass in isolation, fromos.replace PermissionError. I confirmed this is unrelated to this change:test_sequential_and_concurrent_agreefails 2 of 4 runs on cleanmainand passes 4/4 with this patch applied, and the two other flagged tests pass in isolation. Hence the targeted verification above.Written with assistance from Claude Code. Bug found, reproduced, and verified by me on current
main.