feat(workflows): make shell step timeout configurable (#3327)

The `shell` step hardcoded a 300s subprocess timeout, so any command
that legitimately runs longer than five minutes (a full build, a linter
aggregator, an integration-test target) was killed with TimeoutExpired
and failed the whole run, with no YAML knob to raise the limit.

Add an optional `timeout` field (seconds) that defaults to 300 for
backward compatibility and is threaded through to `subprocess.run`. The
timeout failure message now reports the configured value instead of a
hardcoded 300. `validate` rejects a `timeout` that is not a positive
number (bool is rejected explicitly, since it is an int subclass but a
config error rather than a duration).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

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

@@ -26,6 +26,11 @@ class ShellStep(StepBase):
 
         cwd = context.project_root or "."
 
+        # Per-step execution timeout in seconds; defaults to 300 for backward
+        # compatibility. ``validate`` guarantees a positive number when the
+        # field is present, so ``execute`` can pass it straight through.
+        timeout = config.get("timeout", 300)
+
         # NOTE: shell=True is required to support pipes, redirects, and
         # multi-command expressions in workflow YAML.  Workflow authors
         # control commands; catalog-installed workflows should be reviewed
@@ -37,7 +42,7 @@ class ShellStep(StepBase):
                 capture_output=True,
                 text=True,
                 cwd=cwd,
-                timeout=300,
+                timeout=timeout,
             )
             output = {
                 "exit_code": proc.returncode,
@@ -74,7 +79,7 @@ class ShellStep(StepBase):
         except subprocess.TimeoutExpired:
             return StepResult(
                 status=StepStatus.FAILED,
-                error="Shell command timed out after 300 seconds.",
+                error=f"Shell command timed out after {timeout} seconds.",
                 output={"exit_code": -1, "stdout": "", "stderr": "timeout"},
             )
         except OSError as exc:
@@ -96,4 +101,17 @@ class ShellStep(StepBase):
                 f"Shell step {config.get('id', '?')!r}: 'output_format' must "
                 f"be 'json' when present, got {output_format!r}."
             )
+        if "timeout" in config:
+            timeout = config["timeout"]
+            # bool is a subclass of int, but ``timeout: true`` is a config
+            # error rather than a duration — reject it explicitly.
+            if (
+                isinstance(timeout, bool)
+                or not isinstance(timeout, (int, float))
+                or timeout <= 0
+            ):
+                errors.append(
+                    f"Shell step {config.get('id', '?')!r}: 'timeout' must be a "
+                    f"positive number of seconds, got {timeout!r}."
+                )
         return errors

tests/test_workflows.py

@@ -1240,6 +1240,95 @@ class TestShellStep:
         errors = step.validate({"id": "emit", "run": "exit 0", "output_format": "yaml"})
         assert any("'output_format' must be 'json'" in e for e in errors)
 
+    def test_configured_timeout_is_passed_to_subprocess(self, monkeypatch):
+        """A ``timeout:`` value on the step overrides the 300s default and is
+        threaded through to ``subprocess.run`` (issue #3327)."""
+        import subprocess
+
+        from specify_cli.workflows.steps.shell import ShellStep
+        from specify_cli.workflows.base import StepContext, StepStatus
+
+        captured: dict[str, object] = {}
+
+        def fake_run(*args, **kwargs):
+            captured["timeout"] = kwargs.get("timeout")
+            return subprocess.CompletedProcess(
+                args=args[0] if args else "", returncode=0, stdout="", stderr=""
+            )
+
+        monkeypatch.setattr(subprocess, "run", fake_run)
+        step = ShellStep()
+        result = step.execute(
+            {"id": "qa", "run": "echo hi", "timeout": 1800}, StepContext()
+        )
+        assert result.status == StepStatus.COMPLETED
+        assert captured["timeout"] == 1800
+
+    def test_default_timeout_preserved_when_omitted(self, monkeypatch):
+        """Omitting ``timeout:`` preserves the historical 300s default."""
+        import subprocess
+
+        from specify_cli.workflows.steps.shell import ShellStep
+        from specify_cli.workflows.base import StepContext
+
+        captured: dict[str, object] = {}
+
+        def fake_run(*args, **kwargs):
+            captured["timeout"] = kwargs.get("timeout")
+            return subprocess.CompletedProcess(
+                args=args[0] if args else "", returncode=0, stdout="", stderr=""
+            )
+
+        monkeypatch.setattr(subprocess, "run", fake_run)
+        step = ShellStep()
+        step.execute({"id": "qa", "run": "echo hi"}, StepContext())
+        assert captured["timeout"] == 300
+
+    def test_timeout_error_reports_configured_value(self, monkeypatch):
+        """The timeout failure message reflects the configured duration, not a
+        hardcoded 300."""
+        import subprocess
+
+        from specify_cli.workflows.steps.shell import ShellStep
+        from specify_cli.workflows.base import StepContext, StepStatus
+
+        def fake_run(*args, **kwargs):
+            raise subprocess.TimeoutExpired(cmd="echo hi", timeout=5)
+
+        monkeypatch.setattr(subprocess, "run", fake_run)
+        step = ShellStep()
+        result = step.execute(
+            {"id": "qa", "run": "echo hi", "timeout": 5}, StepContext()
+        )
+        assert result.status == StepStatus.FAILED
+        assert "5 seconds" in (result.error or "")
+
+    def test_validate_rejects_non_positive_timeout(self):
+        from specify_cli.workflows.steps.shell import ShellStep
+
+        step = ShellStep()
+        for bad in (0, -30):
+            errors = step.validate({"id": "qa", "run": "echo hi", "timeout": bad})
+            assert any("'timeout' must be a positive number" in e for e in errors)
+
+    def test_validate_rejects_non_numeric_timeout(self):
+        from specify_cli.workflows.steps.shell import ShellStep
+
+        step = ShellStep()
+        # A string and a bool are both invalid (bool is an int subclass but a
+        # config error, not a duration).
+        for bad in ("30", True):
+            errors = step.validate({"id": "qa", "run": "echo hi", "timeout": bad})
+            assert any("'timeout' must be a positive number" in e for e in errors)
+
+    def test_validate_accepts_positive_numeric_timeout(self):
+        from specify_cli.workflows.steps.shell import ShellStep
+
+        step = ShellStep()
+        for good in (1, 300, 1800, 12.5):
+            errors = step.validate({"id": "qa", "run": "echo hi", "timeout": good})
+            assert not any("'timeout'" in e for e in errors)
+
 class _StubStdin:
     """Stdin stub exposing only a fixed ``isatty`` result.