Skip to content

Commit 3d2901e

Browse files
fix(workflows): fail fan-in loudly on a non-string wait_for entry (#3579)
`FanInStep.execute` already guards a non-list `wait_for` (#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>
1 parent c1e5cfa commit 3d2901e

2 files changed

Lines changed: 50 additions & 0 deletions

File tree

‎src/specify_cli/workflows/steps/fan_in/__init__.py‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,28 @@ def execute(self, config: dict[str, Any], context: StepContext) -> StepResult:
4242
output={"results": []},
4343
)
4444

45+
# A non-string entry can never match a real step id. An unhashable one
46+
# (a list/dict from a YAML indentation slip like ``wait_for: [[a, b]]``)
47+
# crashes the whole run at ``context.steps.get(step_id, ...)`` below with
48+
# a raw TypeError; a hashable-but-non-string one (``wait_for: [123]``)
49+
# silently joins an empty ``{}`` and still reports COMPLETED — the exact
50+
# "silent empty result + COMPLETED" wiring bug the whole-list guard above
51+
# and the engine's fan-in validation (engine.py) both reject. The engine
52+
# does not auto-validate step config, so fail this step loudly on an
53+
# unvalidated run too, using the engine's phrasing.
54+
bad_entries = [w for w in wait_for if not isinstance(w, str)]
55+
if bad_entries:
56+
first = bad_entries[0]
57+
return StepResult(
58+
status=StepStatus.FAILED,
59+
error=(
60+
f"Fan-in step {config.get('id', '?')!r}: 'wait_for' entries "
61+
f"must be step-id strings, got {type(first).__name__} "
62+
f"({first!r})."
63+
),
64+
output={"results": []},
65+
)
66+
4567
# Collect results from referenced steps
4668
results = []
4769
for step_id in wait_for:

‎tests/test_workflows.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2843,6 +2843,34 @@ def test_execute_non_list_wait_for_fails_loudly(self, bad_wait_for):
28432843
assert "'wait_for' must be a list" in (result.error or "")
28442844
assert result.output["results"] == []
28452845

2846+
@pytest.mark.parametrize("bad_entry", [["a", "b"], {"a": 1}, 123, None])
2847+
def test_execute_non_string_wait_for_entry_fails_loudly(self, bad_entry):
2848+
"""A ``wait_for`` list with a non-string entry must fail the step, not
2849+
crash the run or silently produce a bogus join.
2850+
2851+
The whole-list guard (``test_execute_non_list_wait_for_fails_loudly``)
2852+
and the engine's fan-in validation both already reject the list *shape*,
2853+
but neither the step's ``execute`` nor the engine's runtime path guarded
2854+
the list's *elements*. On an unvalidated run an unhashable entry
2855+
(a list/dict from a YAML indentation slip like ``wait_for: [[a, b]]``)
2856+
crashed ``context.steps.get(entry, ...)`` with a raw TypeError, while a
2857+
hashable-but-non-string entry (``wait_for: [123]``) silently joined an
2858+
empty ``{}`` and still reported COMPLETED — the same wiring bug the
2859+
list-shape guard exists to prevent. Mirrors the engine's
2860+
``test_non_string_wait_for_entry_is_rejected`` load-time check.
2861+
"""
2862+
from specify_cli.workflows.steps.fan_in import FanInStep
2863+
from specify_cli.workflows.base import StepContext, StepStatus
2864+
2865+
step = FanInStep()
2866+
ctx = StepContext(steps={"a": {"output": {"x": 1}}})
2867+
# A valid entry alongside the bad one proves it is the entry, not the
2868+
# list, that is rejected.
2869+
result = step.execute({"id": "collect", "wait_for": ["a", bad_entry]}, ctx)
2870+
assert result.status == StepStatus.FAILED
2871+
assert "'wait_for' entries must be step-id strings" in (result.error or "")
2872+
assert result.output["results"] == []
2873+
28462874
def test_validate_empty_wait_for(self):
28472875
from specify_cli.workflows.steps.fan_in import FanInStep
28482876

0 commit comments

Comments
 (0)