Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/specify_cli/workflows/steps/fan_in/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,28 @@ def execute(self, config: dict[str, Any], context: StepContext) -> StepResult:
output={"results": []},
)

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

# Collect results from referenced steps
results = []
for step_id in wait_for:
Expand Down
28 changes: 28 additions & 0 deletions tests/test_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -2784,6 +2784,34 @@ def test_execute_non_list_wait_for_fails_loudly(self, bad_wait_for):
assert "'wait_for' must be a list" in (result.error or "")
assert result.output["results"] == []

@pytest.mark.parametrize("bad_entry", [["a", "b"], {"a": 1}, 123, None])
def test_execute_non_string_wait_for_entry_fails_loudly(self, bad_entry):
"""A ``wait_for`` list with a non-string entry must fail the step, not
crash the run or silently produce a bogus join.

The whole-list guard (``test_execute_non_list_wait_for_fails_loudly``)
and the engine's fan-in validation both already reject the list *shape*,
but neither the step's ``execute`` nor the engine's runtime path guarded
the list's *elements*. On an unvalidated run an unhashable entry
(a list/dict from a YAML indentation slip like ``wait_for: [[a, b]]``)
crashed ``context.steps.get(entry, ...)`` with a raw TypeError, while a
hashable-but-non-string entry (``wait_for: [123]``) silently joined an
empty ``{}`` and still reported COMPLETED — the same wiring bug the
list-shape guard exists to prevent. Mirrors the engine's
``test_non_string_wait_for_entry_is_rejected`` load-time check.
"""
from specify_cli.workflows.steps.fan_in import FanInStep
from specify_cli.workflows.base import StepContext, StepStatus

step = FanInStep()
ctx = StepContext(steps={"a": {"output": {"x": 1}}})
# A valid entry alongside the bad one proves it is the entry, not the
# list, that is rejected.
result = step.execute({"id": "collect", "wait_for": ["a", bad_entry]}, ctx)
assert result.status == StepStatus.FAILED
assert "'wait_for' entries must be step-id strings" in (result.error or "")
assert result.output["results"] == []

def test_validate_empty_wait_for(self):
from specify_cli.workflows.steps.fan_in import FanInStep

Expand Down