From 318fdc2d2ba3c6c6a908ed53910d1ef95001445d Mon Sep 17 00:00:00 2001
From: jawwad-ali <alijawwad001@gmail.com>
Date: Sun, 5 Jul 2026 19:35:45 +0500
Subject: [PATCH] fix(workflows): fan-in validate() rejects non-mapping output

FanInStep.validate() only checked wait_for, so a non-mapping 'output'
(a list or scalar) validated clean; execute() then silently coerces it
to {}, so the author's declared aggregation keys vanish with COMPLETED
status and no diagnostic. Reject a non-mapping output at validation,
mirroring the command-step (#3262) non-mapping fix. execute()'s
defensive coercion is left in place for unvalidated callers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 .../workflows/steps/fan_in/__init__.py        |  9 ++++++++
 tests/test_workflows.py                       | 21 +++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/src/specify_cli/workflows/steps/fan_in/__init__.py b/src/specify_cli/workflows/steps/fan_in/__init__.py
index dec3e3fd4d..7b82df50b3 100644
--- a/src/specify_cli/workflows/steps/fan_in/__init__.py
+++ b/src/specify_cli/workflows/steps/fan_in/__init__.py
@@ -58,4 +58,13 @@ def validate(self, config: dict[str, Any]) -> list[str]:
                 f"Fan-in step {config.get('id', '?')!r}: "
                 f"'wait_for' must be a non-empty list of step IDs."
             )
+        output = config.get("output")
+        if output is not None and not isinstance(output, dict):
+            # execute() silently coerces a non-mapping output to {}, so the
+            # author's declared aggregation keys would vanish with no error.
+            # Reject at validation, mirroring the command-step (#3262) fix.
+            errors.append(
+                f"Fan-in step {config.get('id', '?')!r}: 'output' must be a "
+                f"mapping of key -> expression, got {type(output).__name__}."
+            )
         return errors
diff --git a/tests/test_workflows.py b/tests/test_workflows.py
index 2fdbf887b3..88671232d6 100644
--- a/tests/test_workflows.py
+++ b/tests/test_workflows.py
@@ -2099,6 +2099,27 @@ def test_validate_wait_for_not_list(self):
         errors = step.validate({"id": "test", "wait_for": "not-a-list"})
         assert any("non-empty list" in e for e in errors)
 
+    @pytest.mark.parametrize("bad_output", [["{{ fan_in.results }}"], "{{ x }}", 42])
+    def test_validate_rejects_non_mapping_output(self, bad_output):
+        """A non-mapping 'output' must be rejected: execute() would otherwise
+        silently coerce it to {} and drop the declared aggregation keys."""
+        from specify_cli.workflows.steps.fan_in import FanInStep
+
+        step = FanInStep()
+        errors = step.validate(
+            {"id": "j", "wait_for": ["a"], "output": bad_output}
+        )
+        assert any("'output' must be a mapping" in e for e in errors)
+
+    def test_validate_accepts_mapping_or_absent_output(self):
+        from specify_cli.workflows.steps.fan_in import FanInStep
+
+        step = FanInStep()
+        assert step.validate(
+            {"id": "j", "wait_for": ["a"], "output": {"joined": "{{ x }}"}}
+        ) == []
+        assert step.validate({"id": "j", "wait_for": ["a"]}) == []
+
 
 class TestFanOutConcurrency:
     """Fan-out honors max_concurrency (WorkflowEngine._run_fan_out)."""