diff --git a/CHANGELOG.md b/CHANGELOG.md index 0622bb0..4deff07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,6 +101,16 @@ All notable changes to this project are documented here. The format is based on ### Fixed +- **An explicit invalid `--max-action-delta` now fails fast instead of silently + running with weaker guardrails** (#154). Non-finite or non-positive values + were previously caught by `_build_guardrails`'s degrade-per-component path + (meant for derived limits an embodiment's space can't support) and + downgraded to a stderr warning, so the run proceeded with clamp-only + guardrails despite the operator explicitly asking for a tighter limit. Both + `run` and `eval-set` now reject a malformed explicit value in the shared + conflict check, before anything resolves or energizes. Derived-limit + degradation (no explicit flag, an embodiment declaring no bounds) is + unaffected — it still warns and continues. - **`--epochs 0` or a negative value now exits with a guided error instead of a raw traceback** (#145). Both `inspect-robots run` and `inspect-robots eval-set` catch the `ConfigError` raised by `Task`'s epoch validation and surface it diff --git a/src/inspect_robots/cli.py b/src/inspect_robots/cli.py index 7c3bd59..44a6306 100644 --- a/src/inspect_robots/cli.py +++ b/src/inspect_robots/cli.py @@ -745,7 +745,7 @@ class _ResolvedComponents(NamedTuple): def _check_shared_run_conflicts(args: argparse.Namespace) -> None: - """Reject flag combinations invalid for both ``run`` and ``eval-set``.""" + """Reject flag combinations and malformed values invalid for both ``run`` and ``eval-set``.""" if args.sim and args.embodiment: raise SystemExit( "--sim selects your configured sim_embodiment; " @@ -755,6 +755,15 @@ def _check_shared_run_conflicts(args: argparse.Namespace) -> None: raise SystemExit( "--max-action-delta tunes the guardrails that --disable-guardrails turns off — drop one" ) + if args.max_action_delta is not None and not ( + math.isfinite(args.max_action_delta) and args.max_action_delta > 0 + ): + # A malformed *explicit* value is a CLI input error, not an embodiment + # limitation: fail fast here rather than let _build_guardrails's + # degrade-per-component path (meant for derived limits an embodiment's + # space can't support) downgrade it to a warning and run with weaker + # guardrails than the operator explicitly asked for. + raise SystemExit(f"--max-action-delta must be finite and > 0, got {args.max_action_delta}") def _resolve_components(args: argparse.Namespace, defaults: Defaults) -> _ResolvedComponents: diff --git a/tests/test_registry_cli.py b/tests/test_registry_cli.py index 0fba4d6..99666e7 100644 --- a/tests/test_registry_cli.py +++ b/tests/test_registry_cli.py @@ -2835,6 +2835,36 @@ def test_cli_max_action_delta_conflicts_with_disable(tmp_path: Path) -> None: ) +@pytest.mark.parametrize("value", ["-1", "0", "inf", "nan"]) +def test_cli_run_rejects_invalid_max_action_delta(value: str, tmp_path: Path) -> None: + """An explicit malformed --max-action-delta fails fast (#154), not a soft warning: + it is a CLI input error, distinct from a derived limit an embodiment's space + cannot support (test_cli_degraded_guardrails_warn_but_run stays a warning).""" + with pytest.raises(SystemExit, match="finite and > 0"): + main( + [ + "run", + "--task", + "cubepick-reach", + "--policy", + "scripted", + "--embodiment", + "cubepick", + "--log-dir", + str(tmp_path), + "--max-action-delta", + value, + ] + ) + + +@pytest.mark.parametrize("value", ["-1", "0", "inf", "nan"]) +def test_cli_eval_set_rejects_invalid_max_action_delta(value: str) -> None: + """Same guard as run, exercised through eval-set's shared conflict check (#154).""" + with pytest.raises(SystemExit, match="finite and > 0"): + main(["eval-set", "cubepick-reach", "--max-action-delta", value]) + + def test_cli_degraded_guardrails_warn_but_run( tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: