diff --git a/CHANGELOG.md b/CHANGELOG.md index be1ec5b5..8c21d9ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -626,6 +626,11 @@ All notable changes to this project are documented here. The format is based on regression (`eef_delta_pose` + `rot6d` already reached the displacement clamp path before #143/#144). `euler_xyz` and `axis_angle` deltas have no such problem and remain guardrail-conformant. +- **Agent policy configuration parameters now strictly reject non-strings** + in `LLMAgentPolicy` constructor, raising a guided `ConfigError` (#169). This + prevents unquoted CLI values (e.g., `-P model=42` or `-P api_key_env=false`) + from causing downstream errors or incorrect fallback logic, prompting the + user to pass quoted strings instead. - **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 diff --git a/plugins/inspect-robots-agent/src/inspect_robots_agent/policy.py b/plugins/inspect-robots-agent/src/inspect_robots_agent/policy.py index 4c7436a2..444c650b 100644 --- a/plugins/inspect-robots-agent/src/inspect_robots_agent/policy.py +++ b/plugins/inspect-robots-agent/src/inspect_robots_agent/policy.py @@ -340,6 +340,21 @@ def __init__( env: dict[str, str] | None = None, pre_check: PreCheck | None = None, ) -> None: + # Reject non-strings with a guided ConfigError to prevent unquoted CLI + # values (e.g. -P model=42) from causing downstream errors or silent bypasses. + for name, val in [ + ("model", model), + ("base_url", base_url), + ("api_key_env", api_key_env), + ("speed", speed), + ]: + if val is not None and not isinstance(val, str): + raise ConfigError( + f"{name} must be a string, got {val!r}.\n" + f"fix: the -P parser coerces unquoted values; pass " + f"-P '{name}=\"value\"'" + ) + prior_learnings_path: str | None = None prior_learnings_text: str | None = None prior_learnings_sha256: str | None = None diff --git a/plugins/inspect-robots-agent/tests/test_anthropic.py b/plugins/inspect-robots-agent/tests/test_anthropic.py index f37d227c..868601f3 100644 --- a/plugins/inspect-robots-agent/tests/test_anthropic.py +++ b/plugins/inspect-robots-agent/tests/test_anthropic.py @@ -1288,20 +1288,19 @@ def test_variant_strip_keeps_fine_tune_colons() -> None: ) -@pytest.mark.parametrize("api_key_env", [None, "", False, 0, 0.0]) +@pytest.mark.parametrize("api_key_env", [None, ""]) def test_falsy_api_key_env_does_not_send_the_openrouter_key_to_a_gateway( - api_key_env: object, + api_key_env: str | None, ) -> None: - # '-P api_key_env=' parses to '', and 'false'/'0' to other falsy values, - # all of which resolve_provider treats as unset and answers with - # $OPENROUTER_API_KEY. An `is None` test would hand a third-party gateway - # the OpenRouter secret. + # '-P api_key_env=' parses to '', which resolve_provider treats as unset + # and answers with $OPENROUTER_API_KEY. An `is None` test would hand a + # third-party gateway the OpenRouter secret. seen, handler = _capture(_anthropic_response(_text("ok"), stop_reason="end_turn")) policy = LLMAgentPolicy( model="claude-opus-5", wire="messages", base_url="https://gw.example/v1", - api_key_env=api_key_env, # type: ignore[arg-type] + api_key_env=api_key_env, transport=httpx.MockTransport(handler), env={"ANTHROPIC_API_KEY": "sk-ant", "OPENROUTER_API_KEY": "sk-or"}, ) diff --git a/plugins/inspect-robots-agent/tests/test_policy_e2e.py b/plugins/inspect-robots-agent/tests/test_policy_e2e.py index bd080fbf..5a0a3c39 100644 --- a/plugins/inspect-robots-agent/tests/test_policy_e2e.py +++ b/plugins/inspect-robots-agent/tests/test_policy_e2e.py @@ -2791,3 +2791,38 @@ def test_chat_wire_usage_metadata_counts_calls_only(tmp_path: Path) -> None: ) assert sink.records[0].metadata["llm_usage"] == {"llm_calls": 2} + + +@pytest.mark.parametrize( + "param, val", + [ + ("model", 42), + ("model", 0), + ("base_url", True), + ("base_url", False), + ("api_key_env", True), + ("api_key_env", False), + ("speed", False), + ], +) +def test_non_string_params_rejected(param: str, val: Any) -> None: + kwargs: dict[str, Any] = { + "model": "test-model", + "base_url": "http://localhost:8000", + "api_key_env": "TEST_KEY", + "effort": "low", + "speed": None, + "wire": "chat", + } + kwargs[param] = val + + # speed requires wire='anthropic' + if param == "speed": + kwargs["wire"] = "anthropic" + + with pytest.raises(ConfigError) as exc_info: + LLMAgentPolicy(**kwargs) + + assert f"{param} must be a string, got {val!r}." in str(exc_info.value) + expected_fix = f"fix: the -P parser coerces unquoted values; pass -P '{param}=\"value\"'" + assert expected_fix in str(exc_info.value) diff --git a/tests/test_rerun_sink.py b/tests/test_rerun_sink.py index c8c916df..662e3882 100644 --- a/tests/test_rerun_sink.py +++ b/tests/test_rerun_sink.py @@ -1861,6 +1861,7 @@ def test_real_rerun_accepts_the_blueprint(tmp_path: Path) -> None: def test_real_rerun_process_exits_when_tcp_peer_never_reads() -> None: """The real SDK atexit path is bounded after a connected peer stops reading.""" rr = pytest.importorskip("rerun") + if not hasattr(rr, "connect_grpc"): pytest.skip("pre-gRPC rerun-sdk cannot run the connect-mode wedge scenario") server = socket.socket(socket.AF_INET, socket.SOCK_STREAM)