Address eleventh review: command step fails without CLI, ID mismatch warning, state persistence

- Command step returns FAILED when CLI not installed (was silent COMPLETED)
- Catalog install warns on workflow ID vs catalog key mismatch
- Engine persists state.save() before returning on unknown step type
- Update tests to expect FAILED for command steps without CLI
- Integration tests use shell steps for CLI-independent execution

Author: mnriem

src/specify_cli/__init__.py

@@ -4488,6 +4488,13 @@ def _validate_and_install_local(yaml_path: Path, source_label: str) -> None:
             console.print(f"  \u2022 {err}")
         raise typer.Exit(1)
 
+    # Warn if the workflow's internal ID doesn't match the catalog key
+    if definition.id and definition.id != source:
+        console.print(
+            f"[yellow]Warning:[/yellow] Workflow ID in YAML ({definition.id!r}) "
+            f"differs from catalog key ({source!r}). Using catalog key."
+        )
+
     registry.add(source, {
         "name": definition.name or info.get("name", source),
         "version": definition.version or info.get("version", "0.0.0"),

src/specify_cli/workflows/engine.py

@@ -524,6 +524,7 @@ def _execute_steps(
                         "error": f"Unknown step type: {step_type!r}",
                     }
                 )
+                state.save()
                 return
 
             result: StepResult = step_impl.execute(step_config, context)

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

@@ -71,14 +71,22 @@ def execute(self, config: dict[str, Any], context: StepContext) -> StepResult:
                     output=output,
                     error=dispatch_result["stderr"] or f"Command exited with code {dispatch_result['exit_code']}",
                 )
+            return StepResult(
+                status=StepStatus.COMPLETED,
+                output=output,
+            )
         else:
-            output["exit_code"] = 0
+            output["exit_code"] = 1
             output["dispatched"] = False
-
-        return StepResult(
-            status=StepStatus.COMPLETED,
-            output=output,
-        )
+            return StepResult(
+                status=StepStatus.FAILED,
+                output=output,
+                error=(
+                    f"Cannot dispatch command {command!r}: "
+                    f"integration {integration!r} CLI not found or not installed. "
+                    f"Install the CLI tool or check 'specify integration list'."
+                ),
+            )
 
     @staticmethod
     def _try_dispatch(

tests/test_workflows.py

@@ -414,7 +414,7 @@ def test_execute_basic(self):
             "input": {"args": "{{ inputs.name }}"},
         }
         result = step.execute(config, ctx)
-        assert result.status == StepStatus.COMPLETED
+        assert result.status == StepStatus.FAILED
         assert result.output["command"] == "speckit.specify"
         assert result.output["integration"] == "claude"
         assert result.output["input"]["args"] == "login"
@@ -473,7 +473,7 @@ def test_options_merge(self):
         assert result.output["options"]["thinking-budget"] == 32768
 
     def test_dispatch_not_attempted_without_cli(self):
-        """When the CLI tool is not installed, dispatched should be False."""
+        """When the CLI tool is not installed, step should fail."""
         from specify_cli.workflows.steps.command import CommandStep
         from specify_cli.workflows.base import StepContext, StepStatus
 
@@ -489,9 +489,9 @@ def test_dispatch_not_attempted_without_cli(self):
             "input": {"args": "{{ inputs.name }}"},
         }
         result = step.execute(config, ctx)
-        assert result.status == StepStatus.COMPLETED
+        assert result.status == StepStatus.FAILED
         assert result.output["dispatched"] is False
-        assert result.output["exit_code"] == 0
+        assert result.error is not None
 
     def test_dispatch_with_mock_cli(self, tmp_path, monkeypatch):
         """When the CLI is installed, dispatch invokes the command by name."""
@@ -1328,7 +1328,7 @@ def test_execute_simple_workflow(self, project_dir):
         engine = WorkflowEngine(project_dir)
         state = engine.execute(definition, {"name": "login"})
 
-        assert state.status == RunStatus.COMPLETED
+        assert state.status == RunStatus.FAILED
         assert "step-one" in state.step_results
         assert state.step_results["step-one"]["output"]["command"] == "speckit.specify"
         assert state.step_results["step-one"]["output"]["input"]["args"] == "login"
@@ -1345,18 +1345,16 @@ def test_execute_with_gate_pauses(self, project_dir):
   version: "1.0.0"
 steps:
   - id: step-one
-    command: speckit.specify
-    input:
-      args: "test"
+    type: shell
+    run: "echo test"
   - id: gate
     type: gate
     message: "Review?"
     options: [approve, reject]
     on_reject: abort
   - id: step-two
-    command: speckit.plan
-    input:
-      args: "test"
+    type: shell
+    run: "echo done"
 """
         definition = WorkflowDefinition.from_string(yaml_str)
         engine = WorkflowEngine(project_dir)
@@ -1726,9 +1724,8 @@ def test_full_sequential_workflow(self, project_dir):
     default: "login"
 steps:
   - id: specify
-    command: speckit.specify
-    input:
-      args: "{{ inputs.feature }}"
+    type: shell
+    run: "echo speckit.specify {{ inputs.feature }}"
 
   - id: check-scope
     type: if
@@ -1743,9 +1740,8 @@ def test_full_sequential_workflow(self, project_dir):
         run: "echo partial scope"
 
   - id: plan
-    command: speckit.plan
-    input:
-      args: "{{ steps.specify.output.command }}"
+    type: shell
+    run: "echo speckit.plan"
 """
         definition = WorkflowDefinition.from_string(yaml_str)
         engine = WorkflowEngine(project_dir)