refactor(cli): extract shared epochs validation helper

CHANGELOG.md

@@ -79,6 +79,14 @@ All notable changes to this project are documented here. The format is based on
 
 ### Fixed
 
+- **`--epochs 0` or a negative value now exits with a guided error instead of a
+  raw traceback** (#145, #152). Both `inspect-robots run` and `inspect-robots
+  eval-set` share a single `_apply_epochs_or_exit` helper (next to
+  `_resolve_or_exit`) that catches the `ConfigError` from `Task`'s epoch
+  validation. The message is `--epochs must be >= 1, got N`; the `eval-set`
+  path appends `(task 'NAME')` to name the failing task. The duplicate
+  `ConfigError` import that previously appeared mid-function in both commands is
+  gone (#47).
 - **Operator scoring no longer prompts twice for self-confirming embodiments**
   (#53). On interactive ad-hoc runs, definitive `success` or `failure`
   termination verdicts are adopted as the operator judgement, announced on the

src/inspect_robots/cli.py

@@ -71,6 +71,7 @@
     from inspect_robots.rollout import TrialRecord
     from inspect_robots.scene import Scene
     from inspect_robots.spaces import Box
+    from inspect_robots.task import Task
 
 
 def _styled(text: str, code: str) -> str:
@@ -478,6 +479,27 @@ def _resolve_or_exit(
         ) from exc
 
 
+def _apply_epochs_or_exit(task: Task, epochs: int, /) -> Task:
+    """Return a copy of *task* with the epoch count overridden, or exit cleanly.
+
+    Centralises the ``--epochs`` guard for both ``run`` and ``eval-set``:
+    a non-positive value raises ``ConfigError`` inside ``dataclasses.replace``
+    and we surface it as a guided ``SystemExit`` — the same ``raise … from exc``
+    idiom used by :func:`_resolve_or_exit` — instead of a raw traceback.
+    The task name is included in the message only when a ``task_label`` is
+    meaningful (i.e. the eval-set path); callers supply it via the wrapper
+    that knows the context.
+    """
+    from dataclasses import replace
+
+    from inspect_robots.errors import ConfigError
+
+    try:
+        return replace(task, epochs=epochs)
+    except ConfigError:
+        raise SystemExit(f"--epochs must be >= 1, got {epochs}") from None
+
+
 def _build_guardrails(
     space: Box, max_action_delta: float | None
 ) -> tuple[Approver, list[str], list[str]]:
@@ -835,8 +857,6 @@ def _build_and_announce_guardrails(args: argparse.Namespace, action_space: Box)
 
 
 def _cmd_run(args: argparse.Namespace) -> int:
-    from dataclasses import replace
-
     from inspect_robots import eval
     from inspect_robots.logging import JsonLogSink
     from inspect_robots.scene import Scene
@@ -885,7 +905,7 @@ def _cmd_run(args: argparse.Namespace) -> int:
     embodiment = resolved.embodiment
     try:
         if args.epochs is not None:
-            task = replace(task, epochs=args.epochs)
+            task = _apply_epochs_or_exit(task, args.epochs)
 
         _announce_components(resolved)
         approver = _build_and_announce_guardrails(args, embodiment.info.action_space)
@@ -995,8 +1015,6 @@ def _cmd_eval_set(args: argparse.Namespace) -> int:
     the embodiment once per task), the CLI resolves the embodiment exactly
     once for the whole set, so a real robot is not reconnected between tasks.
     """
-    from dataclasses import replace
-
     from inspect_robots import eval_set
 
     _check_shared_run_conflicts(args)
@@ -1005,7 +1023,15 @@ def _cmd_eval_set(args: argparse.Namespace) -> int:
     defaults = load_defaults(os.environ)
     tasks = [_resolve_or_exit("task", name) for name in task_names]
     if args.epochs is not None:
-        tasks = [replace(t, epochs=args.epochs) for t in tasks]
+        patched: list[Task] = []
+        for t in tasks:
+            try:
+                patched.append(_apply_epochs_or_exit(t, args.epochs))
+            except SystemExit:
+                raise SystemExit(
+                    f"--epochs must be >= 1, got {args.epochs} (task {t.name!r})"
+                ) from None
+        tasks = patched
 
     resolved = _resolve_components(args, defaults)
     embodiment = resolved.embodiment

tests/test_registry_cli.py

@@ -379,6 +379,49 @@ def test_cli_run_epochs_fail_on_error_store_frames(
     assert list((tmp_path / "frames").rglob("*.npy"))  # --store-frames streamed (per-run subdir)
 
 
+@pytest.mark.parametrize("epochs_value", ["0", "-1", "-5"])
+def test_cli_run_zero_epochs_exits_with_guided_error(epochs_value: str) -> None:
+    """--epochs 0 / negative must produce a guided SystemExit, not a raw traceback (#145)."""
+    import re
+
+    expected = f"--epochs must be >= 1, got {epochs_value}"
+    with pytest.raises(SystemExit, match=re.escape(expected)):
+        main(
+            [
+                "run",
+                "--task",
+                "cubepick-reach",
+                "--policy",
+                "scripted",
+                "--embodiment",
+                "cubepick",
+                "--epochs",
+                epochs_value,
+            ]
+        )
+
+
+@pytest.mark.parametrize("epochs_value", ["0", "-1"])
+def test_cli_eval_set_zero_epochs_exits_with_guided_error(epochs_value: str) -> None:
+    """eval-set --epochs 0 / negative must produce a guided SystemExit naming the task (#145)."""
+    import re
+
+    expected = f"--epochs must be >= 1, got {epochs_value} (task 'cubepick-reach')"
+    with pytest.raises(SystemExit, match=re.escape(expected)):
+        main(
+            [
+                "eval-set",
+                "cubepick-reach",
+                "--policy",
+                "scripted",
+                "--embodiment",
+                "cubepick",
+                "--epochs",
+                epochs_value,
+            ]
+        )
+
+
 def _register_task(name: str, *, num_scenes: int = 1, max_steps: int = 20) -> None:
     from inspect_robots.registry import task as task_decorator
     from inspect_robots.scene import Scene
@@ -2464,8 +2507,11 @@ def test_cli_run_closes_embodiment_when_validation_raises(
 ) -> None:
     """A failure between resolving the embodiment and eval() (here: --epochs 0
     raising ConfigError) must still close the embodiment — otherwise a bad flag
-    leaves real arms energized."""
-    from inspect_robots.errors import ConfigError
+    leaves real arms energized.
+
+    The fix converts the raw ConfigError into a guided SystemExit; the
+    important invariant is that close() still fires via the outer try/finally.
+    """
     from inspect_robots.mock import CubePickEmbodiment
 
     closed: list[bool] = []
@@ -2478,7 +2524,7 @@ def close(self) -> None:
     monkeypatch.setitem(reg._FACTORIES["embodiment"], "tracked-cubepick", _Tracked)
     monkeypatch.setenv(ENV_POLICY, "scripted")
     monkeypatch.setenv(ENV_EMBODIMENT, "tracked-cubepick")
-    with pytest.raises(ConfigError):
+    with pytest.raises(SystemExit) as excinfo:
         main(
             [
                 "reach the cube",
@@ -2490,6 +2536,7 @@ def close(self) -> None:
                 str(tmp_path / "logs"),
             ]
         )
+    assert "--epochs" in str(excinfo.value)
     assert closed == [True]