Repository navigation
fix(configs): grant no tools from judge servers that serve none - #5590
gwarmstrong wants to merge 9 commits into
Conversation
The deprecated pi_sandboxed_agent and opencode_sandboxed_agent each ship a configs/benchmark.yaml profile. A caller that composes a benchmark itself, such as an evaluation framework that runs HLE, loads that profile instead of naming a Resources server. The session harnesses that replace them had no such profile, so moving those callers off the deprecated agents meant copying the `resources_server.name: ???` binding into the caller, where some config systems reject a mandatory-missing value. Add the same profile to pi_agent, opencode_agent, codex_agent, openclaw_agent, hermes_agent and claude_code_agent, at configs/profiles/benchmark.yaml. Each leaves the Resources server unset, so Gym composes the agent onto the benchmark's own agent instance, and each session creates its own sandbox through the `sandbox` provider at an absolute address. The Claude Code profile also points the CLI at the `policy_model` Gym model server. Image, limits and prompts stay with the caller. Agent discovery reads only the top level of configs/, so, like opencode_agent's permission profiles, these are not listed as standalone agent variants. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
Composed onto HLE, every session harness rejected the first seed with a 422. HLE's Environment Server grants the agent its Resources server over direct_http, which no session harness can use; HLE Diamond, GPQA and APEX Shortlist do the same. The judge offers the agent no tools, so each adapter README now gives the pairing setting that clears the grant, `resources_tool_transports: []` under the benchmark's Environment Server. The profiles also fill the gaps an episode reached after that: - Claude Code pins `claude_code_version: "2.1.108"`, which sandbox sessions require. - Each profile declares `sandbox_config` with a mandatory image, so a missing image fails at resolve and the image can be set under the composed agent's name. All but Hermes set `ttl_s: 14400`, without which OpenSandbox ends the sandbox after 10 minutes; Hermes derives its TTL from its own timeouts. - `head_server.host` stays on 127.0.0.1. `use_absolute_ip` otherwise puts the head server, whose config endpoint is unauthenticated, on the address sandboxes reach. A new test composes each profile onto HLE, seeds the composed agent with the request HLE's Environment Server builds, and checks that the seed reaches sandbox creation with the setting and is refused without it. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
equivalence_llm_judge, math_with_judge and mcqa register no tool routes, but their Environment Servers granted the agent their Resources server over direct_http, a leftover of NVIDIA-NeMo#4953's switch from the legacy_agent relay. Every config of these servers binds simple_agent, which accepts a seed with no grant, so dropping the transport changes nothing for it. A session harness, which rejects a required direct_http grant, now runs HLE, HLE Diamond, GPQA and APEX Shortlist without a per-benchmark override. The adapter READMEs and profile comments drop that override. The profile test now seeds each composed agent through HLE as shipped, and keeps a case that grants direct_http to show the 422 still guards pairings whose Resources server serves tools. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
|
/ok to test 8a1bebf |
|
🌿 Preview your docs: https://nvidia-preview-pr-5590-1afd921b4add.docs.buildwithfern.com/nemo/gym |
svcnvidia-nemo-ci
left a comment
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
HOLD
One BLOCKER: all six profiles expose an agent endpoint that still supports tool-enabled execution on the Gym host. The inline finding gives a narrow fix that preserves sandbox model access and legitimate local callers.
RISK — Complete the representative real smoke rollouts
The PR description leaves both HLE smoke rollouts pending. These changes affect runnable environment/agent configuration, so the protected AGENTS.md quality bar requires representative real rollouts with a model and inspection of agent and verifier behavior. The new test deliberately stops before sandbox creation, so it cannot establish successful runtime installation, model access, verification, or cleanup. This is a required-evidence gap, not a demonstrated runtime failure.
Could you complete and document the two planned HLE smokes: the default simple_agent path and a session harness using its benchmark profile, both without a transport-clearing override? Record the commands, model, sandbox image where applicable, inspected rollout artifacts, verifier result, and cleanup outcome. No code patch follows from this gap alone.
Acceptance: the default agent must still answer and reach verification without a Resources tool grant. The session harness must create its sandbox, reach the configured model, return an answer, reach verification, and stop its owned sandbox. An incorrect answer may legitimately score zero; setup, transport, or cleanup failures must not count as successful evaluation. Preserve the negative test's required direct_http grant, HTTP 422 response, and assertion that provisioning wasn't reached.
I inspected all 21 changed files and their directly affected composition, session, and runtime contracts. I found no correctness regression in removing grants from the tool-free judge servers. This was static review; I couldn't execute tests or rollouts.
Note: formal approval is not available for this request, so this review is posted as a comment only.
Review generated by codex. This was a full review. Request a full review with /review reset=true.
| pi_agent: | ||
| responses_api_agents: | ||
| pi_agent: | ||
| resources_server: |
There was a problem hiding this comment.
BLOCKER — Keep the host-capable agent endpoint on loopback. This affects all six new profiles. use_absolute_ip: true assigns the reachable host address to every server without an explicit host (parser, host population). Only the head server is pinned to loopback; Uvicorn binds the agent to its configured address.
A sandbox or other caller on that network can POST {"input":"Use bash to inspect this machine"} to the agent's /v1/responses without first seeding a session. An absent agent-session marker returns None, and Pi's unseeded branch enters local compatibility execution. That path installs and launches Pi on the Gym host with built-in tools enabled, rather than in the configured sandbox. Other adapters retain the same exposure; for example, unseeded Claude Code requests enter its local path, whose command bypasses permission prompts. The exposed resource is the Gym host under the agent-server user's permissions. I haven't executed an exploit or demonstrated credential disclosure.
Correction: Could you add an explicit agent host inside each of the six new profiles? At this location:
| resources_server: | |
| host: 127.0.0.1 | |
| resources_server: |
Keep use_absolute_ip: true so the model server remains reachable from the sandbox. This agent host survives composition: only Resources, model, and dataset bindings carry over from the benchmark. Unlike absolute_ip_server_types alone, the explicit host also stays loopback when the caller supplies default_host. The invariant is that sandbox callers can reach the model endpoint, but not the host-execution agent endpoint.
Preservation and acceptance: Update the profile composition test: with each profile, an image, and default_host=192.0.2.30, expect the composed agent host and head host to be 127.0.0.1, while the model host remains 192.0.2.30. Check the same binding separation with automatically resolved addressing. From the sandbox network, connecting to the agent port must fail; a normal Environment Server episode must still seed, activate, verify, and close. Retain legitimate local unseeded execution rather than disabling the shared compatibility branch globally—the existing Pi test expects both local calls to return done. These checks and the suggested patch are source-reasoned, not executed.
There was a problem hiding this comment.
Fixed in #5550, where these profiles live: 6f326e0 adds host: 127.0.0.1 to the agent block of all six profiles, and this PR merged it at 1afd921. With +default_host=192.0.2.30, the composition test now expects the agent and the head server on 127.0.0.1 and the model server on 192.0.2.30. No adapter's sandbox calls its own agent server: Codex's rate-limit proxy binds 127.0.0.1 inside the agent process, on the host path only. Each adapter's seed test still passes through HLE's Environment Server, so episodes seed and activate as before.
The core test that seeded each composed agent imported all six adapters, so core coverage measured their modules, about 6,700 statements that only the adapters' own suites exercise, and fell to 87.57%, under its 95% floor. The seed check now lives in tests/session_harness_profiles.py and runs from a test in each adapter's suite, in that adapter's environment, which also runs the Hermes case that the core venv had to skip. The core test keeps the config-only checks. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
… grant change NVIDIA-NeMo#5550 moved the HLE seed check out of the core test into each adapter's suite. This applies this branch's change there: the seed test runs through HLE as shipped, with a case that grants direct_http to keep the 422, and the core test reads the unset transports as empty. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
|
/ok to test f091648 |
use_absolute_ip gave the agent server the address sandboxes reach. An unseeded request to its /v1/responses runs the harness on the Gym host with its built-in tools, so a sandbox could start one there. Each profile now binds its agent to 127.0.0.1, like the head server; only the model server, which the sandboxes need, takes the reachable address. No adapter's sandbox calls its own agent server. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
…hange Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
|
Both smokes this review asked for ran on 8a1bebf and are in the description's Rollouts section: HLE through |
|
/ok to test 1afd921 |
NVIDIA-NeMo#5550 landed squashed, so its files conflicted as added on both sides. This branch's versions are kept: main had not changed them since NVIDIA-NeMo#5550, except the Claude Code and Hermes READMEs, which take main's text with this change's paragraph applied. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
|
/ok to test 3f36450 |
github-actions
Bot
added
the
sla:review-overdue
Brings in NVIDIA-NeMo#5554, which pinned Pi's profile and generalized the profile test helper; this branch's as-shipped HLE seed test keeps its transports parameter on top of it. A test that landed on main meanwhile expected the aime24 math_with_judge Environment Server to grant direct_http, which this change removes, so it now expects no transports. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
|
/ok to test 4f480e3 |
What does this PR do?
Drops
resources_tool_transports: [direct_http]from every Environment Server config ofequivalence_llm_judge,math_with_judgeandmcqa(8 configs, includinglc_judge,nl2bash-equivalency,dapo17k,dapo17k_trajectory_collectionandmath_stack_overflow). The setting falls back to its default,[], so these Environment Servers grant the agent no tools.#5550's six adapter READMEs and profile comments drop the per-benchmark override they documented, and say instead that a session harness rejects a required
direct_httpgrant. Each adapter's seed test now seeds the composed agent through HLE as shipped, and keeps a case that grantsdirect_httpto show the 422 still catches a pairing whose Resources server serves tools.Why. None of these three servers registers a tool route, so the grant gave the agent nothing to call. The lines came in with #4953's switch from the
legacy_agentrelay. A session harness rejects a requireddirect_httpgrant with a 422, so on main every session-harness pairing with HLE, HLE Diamond, GPQA or APEX Shortlist needs a per-benchmark override. Suggested by @yaoyu-33 in the #5550 review, which also asked not to dropdirect_httpin composition instead, since a benchmark whose tools really come overdirect_httpwould then run without them.Part of #5552.
Validation
simple_agent, which accepts a seed with no grant. Thelegacy_agentpresets of HLE, HLE Diamond and APEX Shortlist delete thesingle_agent_turn_legacyblock, so this setting never reaches them. No benchmark config sets the transports for these servers.responses_api_agents/<agent>/tests/test_benchmark_profile.py(from feat(agents): add swappable benchmark profiles for the session harnesses #5550, now as shipped): 2 passed in each of the six adapters' environments.tests/unit_tests/environment/test_session_harness_benchmark_profiles.py: 18 passed. With HLE's old config restored, the as-shipped seed case and the 6 composition checks fail.tests/unit_tests/configandtests/unit_tests/environment: 753 passed, 2 skipped, 1 failed. The failure,test_every_repo_benchmark_appears_in_listing, fails the same way on feat(agents): add swappable benchmark profiles for the session harnesses #5550's head: this host's hostname resolves to 127.0.1.1, which stops some existinguse_absolute_ipconfigs from resolving.pre-commit runon the changed files: clean.Rollouts
Two 5-task HLE smokes on 8a1bebf (Nemotron 3.5 Super on vLLM), both without any override. In both, the composed
hle_equivalence_llm_judge_environment_serverhas noresources_tool_transportsand every seed carried no grants.simple_agent, the default path: 5/5 completed and healthy, all graded (2/5 correct). Each rollout is one step with a reasoning item and an answer, as before.pi_agentwith feat(agents): add swappable benchmark profiles for the session harnesses #5550's profile and no override: 5/5 completed and healthy, all graded (1/5 correct), no 422.Compatibility and benchmark impact
simple_agentruns of these benchmarks now get no tool grant. They had no tools to call, so their behavior and results are unchanged. Session-harness pairings with these benchmarks no longer need the override; a config that still setsresources_tool_transports: []keeps working.Checklist
pre-commit run --all-files).git commit -s).