fix(workflows): fail command-step before dispatch on non-mapping options

Address Copilot review on #3496. The prior guard silently skipped the
merge for a non-mapping ``options`` and still called ``_try_dispatch``,
so with an installed CLI a malformed step could return ``COMPLETED``
instead of the clean field-specific failure the PR claimed. The
regression test also masked this by forcing ``shutil.which`` to ``None``,
which made the observed ``FAILED`` unrelated to ``options``.

Changes:

- ``execute()`` returns a field-named ``FAILED`` result *before*
  dispatch when ``options`` is not a mapping. Workflow-level defaults
  are preserved in ``output.options`` and ``dispatched`` is ``False``.
- ``test_execute_non_mapping_options_fails_before_dispatch`` (renamed)
  now mocks ``_try_dispatch`` to a *success* result so an accidental
  dispatch would surface as ``COMPLETED``, and asserts the dispatch
  mock is never called and the error string names ``'options'``.
- ``test_validate_rejects_non_mapping_options`` covers ``None`` (a
  bare YAML ``options:``), matching the documented behavior.

Refs #3493.

Author: thejesh23

src/specify_cli/workflows/steps/command/__init__.py

@@ -61,15 +61,25 @@ def execute(self, config: dict[str, Any], context: StepContext) -> StepResult:
         if model and isinstance(model, str) and "{{" in model:
             model = evaluate_expression(model, context)
 
-        # Merge options (workflow defaults ← step overrides)
+        # Merge options (workflow defaults ← step overrides). Same rationale as
+        # 'input': a malformed options fails the step *before dispatch* rather
+        # than being silently ignored — a silent skip would let an installed
+        # CLI report ``COMPLETED`` for a malformed step, and ``dict.update``
+        # would still crash on a list.
         options = dict(context.default_options)
         step_options = config.get("options", {})
-        # Same rationale as 'input': a malformed options fails the step rather
-        # than being silently ignored (which would let an invalid step run and
-        # apparently complete).
         if not isinstance(step_options, dict):
             return StepResult(
                 status=StepStatus.FAILED,
+                output={
+                    "command": command,
+                    "integration": integration,
+                    "model": model,
+                    "options": options,
+                    "input": resolved_input,
+                    "dispatched": False,
+                    "exit_code": 1,
+                },
                 error=(
                     f"Command step {config.get('id', '?')!r}: 'options' must be a "
                     f"mapping, got {type(step_options).__name__}."

tests/test_workflows.py

@@ -960,11 +960,13 @@ def test_validate_rejects_non_mapping_input_and_options(self):
         step = CommandStep()
         # execute() does input.items() / options.update(); a non-mapping must be
         # reported by validate(), not crash at run time (like switch 'cases').
+        # ``None`` is included because a bare YAML ``options:`` parses as ``None``.
         for bad in (None, "args", ["a", "b"], 5):
             errs = step.validate({"id": "c", "command": "/x", "input": bad})
             assert any("'input' must be a mapping" in e for e in errs), bad
-        errs = step.validate({"id": "c", "command": "/x", "options": 42})
-        assert any("'options' must be a mapping" in e for e in errs)
+        for bad in (None, "foo", [1, 2], 42):
+            errs = step.validate({"id": "c", "command": "/x", "options": bad})
+            assert any("'options' must be a mapping" in e for e in errs), bad
         # a valid mapping config is still accepted
         assert step.validate({"id": "c", "command": "/x", "input": {"args": "y"}, "options": {"k": 1}}) == []
         # execute() has no auto-validation guarantee (the engine may skip
@@ -980,6 +982,41 @@ def test_validate_rejects_non_mapping_input_and_options(self):
         assert res_opt.status is StepStatus.FAILED
         assert "'options' must be a mapping" in (res_opt.error or "")
 
+    def test_execute_non_mapping_options_fails_before_dispatch(self):
+        """execute() must fail *before dispatch* when validate() is bypassed.
+
+        Silently discarding a non-mapping ``options`` and continuing would
+        let an installed CLI return exit code 0 and mark the malformed step
+        ``COMPLETED``. Force ``_try_dispatch`` to a success result so an
+        accidental dispatch would surface as ``COMPLETED`` rather than
+        being masked by an unrelated CLI-missing failure.
+        """
+        from unittest.mock import patch, MagicMock
+        from specify_cli.workflows.steps.command import CommandStep
+        from specify_cli.workflows.base import StepContext, StepStatus
+
+        step = CommandStep()
+        ctx = StepContext(
+            default_integration="claude",
+            default_options={"max-tokens": 8000},
+            project_root="/tmp",
+        )
+        dispatch_ok = MagicMock(
+            return_value={"exit_code": 0, "stdout": "", "stderr": ""}
+        )
+        with patch.object(CommandStep, "_try_dispatch", dispatch_ok):
+            result = step.execute(
+                {"id": "t", "command": "/speckit.plan", "options": [1, 2]},
+                ctx,
+            )
+        assert result.status == StepStatus.FAILED
+        assert "'options' must be a mapping" in (result.error or "")
+        # Dispatch must not be attempted for a malformed options value.
+        assert not dispatch_ok.called
+        # Workflow defaults preserved; malformed step-level override discarded.
+        assert result.output["options"] == {"max-tokens": 8000}
+        assert result.output["dispatched"] is False
+
     def test_step_override_integration(self):
         from unittest.mock import patch
         from specify_cli.workflows.steps.command import CommandStep