Skip to content
Open
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
18 changes: 14 additions & 4 deletions src/specify_cli/workflows/steps/command/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__}."
Expand Down
41 changes: 39 additions & 2 deletions tests/test_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down