fix(workflows): strip the resolved value before matching switch cases - #4143
Open
jawwad-ali wants to merge 1 commit into
Open
fix(workflows): strip the resolved value before matching switch cases#4143jawwad-ali wants to merge 1 commit into
jawwad-ali wants to merge 1 commit into
Conversation
`SwitchStep.execute` matched with `str(value)` and no strip. The values a
switch dispatches on are overwhelmingly captured command output, and
`ShellStep` stores `proc.stdout` verbatim, so `run: echo approve` resolves
to "approve\n" — which matches no `approve:` case:
stdout stored : 'approve\n'
matched_case : '__default__' <-- silently wrong
next steps : ['fallback']
The switch falls through to `default:` (or dispatches nothing at all) while
still reporting COMPLETED. A workflow author cannot fix it themselves: the
registered filters are default/join/map/contains/from_json — there is no
`trim`.
spec-kit already treats exactly this as a bug wherever else it matches a
resolved string against declared literals — `evaluate_condition` strips for
this same shell-newline reason, and `InitStep._resolve_bool` does
`resolved.strip().lower()`. Switch case keys are such literals, and this was
the only site not stripping.
`expression_value` still reports the raw value, so nothing downstream loses
information, and a genuine mismatch ("approve-later") still falls through.
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.executematches with:No strip. But the values a switch dispatches on are overwhelmingly captured command output, and
ShellStepstoresproc.stdoutverbatim (shell/__init__.py:67). Sorun: echo approveresolves to"approve\n", which matches noapprove:case key.Reproduction on current
main(bf88c9f)The switch silently falls through to
default:— or, with no default, completes having dispatched nothing — while still reporting COMPLETED.A workflow author cannot work around it: the registered filters are
default/join/map/contains/from_json. There is notrim.This is already treated as a bug everywhere else
spec-kit strips before matching a resolved string against declared literals in every comparable site:
evaluate_conditionresult.strip().lower()— fixed for this exact shell-newline reasonInitStep._resolve_boolresolved.strip().lower()workflows/catalog.pyraw_install.strip().lower() in (...)Switch case keys are such declared literals, and this was the only site not stripping.
Fix
One line, plus the class docstring which said "exact match, string-coerced" and is now accurate.
Narrow.
expression_valuestill reports the raw value, so nothing downstream loses information — pinned by an assertion. A genuine mismatch still falls through: a second test checks"approve-later\n"still routes todefault.The only inputs whose behaviour changes are ones that previously took the wrong branch.
Verification
"approve\n","approve\r\n"," approve "and"approve".TestSwitchStepclass — several open PRs touch this file, and a duplicate class name silently shadows.mainbaseline captured onbf88c9f9(21 pre-existing in scope).uvx ruff@0.15.0 check src tests→ cleanOne note on the run: an unrelated
test_resume_rejects_malformed_run_state_origin_fieldscase failed once during verification. I checked rather than assumed — it fails on a different parametrization each time and passes 4/4 in isolation both with and without this change, so it is a pre-existing Windows flake (os.replace PermissionError). A clean re-run of the full gate passed.Written with assistance from Claude Code. Bug found, reproduced, and verified by me on current
main.