Skip to content

fix(workflows): fail fan-in loudly on a non-string wait_for entry#3579

Open
Noor-ul-ain001 wants to merge 1 commit into
github:mainfrom
Noor-ul-ain001:fix/fan-in-wait-for-element-crash
Open

fix(workflows): fail fan-in loudly on a non-string wait_for entry#3579
Noor-ul-ain001 wants to merge 1 commit into
github:mainfrom
Noor-ul-ain001:fix/fan-in-wait-for-element-crash

Conversation

@Noor-ul-ain001

Copy link
Copy Markdown
Contributor

Summary

FanInStep.execute guards a non-list wait_for (added in #3482), and the engine's load-time validation (engine.py) rejects non-string wait_for entries. 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 — so execute iterated the list's elements raw and hit two failure modes:

  • Unhashable entry — a list/dict from a natural 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 (unhashable type: 'list').
  • Hashable non-string entrywait_for: [123] silently joined an empty {} and still reported COMPLETED — 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-string wait_for entry, 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)

from specify_cli.workflows.steps.fan_in import FanInStep
from specify_cli.workflows.base import StepContext

step = FanInStep()
ctx = StepContext(steps={"a": {"output": {"x": 1}}})

# 1) unhashable entry -> raw TypeError takes down the run
step.execute({"id": "j", "wait_for": [["a", "b"]]}, ctx)
# TypeError: cannot use 'list' as a dict key (unhashable type: 'list')

# 2) non-string entry -> silent empty join, reported COMPLETED
r = step.execute({"id": "j", "wait_for": ["a", 123]}, ctx)
# r.status == COMPLETED, r.output["results"] == [{'x': 1}, {}]   <-- bogus {}

Changes

  • src/specify_cli/workflows/steps/fan_in/__init__.py — reject any non-string wait_for entry in execute(), failing the step cleanly with output={"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

  • New test fails on the pre-fix source (raw TypeError for unhashable, wrong COMPLETED for non-string) and passes with the guard ("test-the-test" verified via git stash).
  • All fan-in tests pass (23 selected).
  • Pre-existing unrelated failures: TestWorkflowCliAlignment symlink/cross-project tests fail on Windows without Developer Mode/elevation — verified identical on a clean upstream/main tree with these changes stashed.

Related

Same "unvalidated execute() crash" class as the sibling fan-in non-list guard (#3482), fan-out non-list items/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

`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>
@Noor-ul-ain001
Noor-ul-ain001 requested a review from mnriem as a code owner July 17, 2026 06:50
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