fix(workflows): fail fan-in loudly on a non-string wait_for entry#3579
Open
Noor-ul-ain001 wants to merge 1 commit into
Open
fix(workflows): fail fan-in loudly on a non-string wait_for entry#3579Noor-ul-ain001 wants to merge 1 commit into
Noor-ul-ain001 wants to merge 1 commit into
Conversation
`FanInStep.execute` already guards a non-list `wait_for` (github#3482), and the engine's load-time validation rejects non-string entries. But the engine does not auto-validate step config, so on an unvalidated run `execute` iterated the list's *elements* raw: - An unhashable entry (a list/dict from a YAML indentation slip like `wait_for: [[a, b]]`) crashed the whole run at `context.steps.get(entry, ...)` with a raw `TypeError: cannot use 'list' as a dict key`. - A hashable-but-non-string entry (`wait_for: [123]`) silently joined an empty `{}` and still reported COMPLETED — the exact "silent empty result + COMPLETED" wiring bug the whole-list guard and the engine's fan-in validation both exist to prevent. Extend the execute() guard to reject any non-string entry with the engine's "entries must be step-id strings" phrasing, mirroring the sibling non-list guard right above it. Adds regression coverage for unhashable and hashable-non-string entries. Co-Authored-By: Claude Opus 4.8 (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.
Summary
FanInStep.executeguards a non-listwait_for(added in #3482), and the engine's load-time validation (engine.py) rejects non-stringwait_forentries. But the engine does not auto-validate step config before running it — the same "unvalidated run" scenario the existing non-list guard is written for — soexecuteiterated the list's elements raw and hit two failure modes:wait_for: [[a, b]]crashed the whole run atcontext.steps.get(entry, ...)with a rawTypeError: cannot use 'list' as a dict key (unhashable type: 'list').wait_for: [123]silently joined an empty{}and still reportedCOMPLETED— the exact "silent empty result + COMPLETED" wiring bug that the whole-list guard and the engine's fan-in validation both exist to prevent.This extends the
execute()guard to reject any non-stringwait_forentry, mirroring the sibling non-list guard directly above it and reusing the engine's own "entries must be step-id strings" phrasing for a consistent error.Reproduction (against current
main)Changes
src/specify_cli/workflows/steps/fan_in/__init__.py— reject any non-stringwait_forentry inexecute(), failing the step cleanly withoutput={"results": []}.tests/test_workflows.py— parametrized regression test (test_execute_non_string_wait_for_entry_fails_loudly) covering unhashable (list/dict) and hashable-non-string (int/None) entries.Testing
TypeErrorfor unhashable, wrongCOMPLETEDfor non-string) and passes with the guard ("test-the-test" verified viagit stash).TestWorkflowCliAlignmentsymlink/cross-project tests fail on Windows without Developer Mode/elevation — verified identical on a cleanupstream/maintree with these changes stashed.Related
Same "unvalidated
execute()crash" class as the sibling fan-in non-list guard (#3482), fan-out non-listitems/step-template guards, the if/switch non-list branch guards (#3515), and the while/do-while non-list steps guards (#3519).🤖 Generated with Claude Code