Repository navigation
feat(browsecomp_advanced_harness): sandbox workspace tools and serve sessions - #5484
Merged
Merged
Conversation
An Environment Server seeds with ResourcesSeedSessionRequest and closes with /close_session; the harness accepted neither, and its per-rollout state (the per_session page workspace and the provider-call metrics) was released only by /verify, and the metrics not at all. - /seed_session takes the typed request, keyed by resources_session_id, and still takes an agent /run's flat row under the cookie session. A typed seed is refused with 422 when task_data lacks question or ground_truth, or when a workspace ID is not a plain directory name; a seed for another episode or after the close gets 409. - /close_session removes the workspace and the metrics, idempotently, through ResourcesSessionFence. A failed removal raises and keeps the session for a retried close. - A tool call for a closed session gets 404 (session_lost) instead of recreating its workspace. - /verify drops the session's metrics entry after reporting it, unless dump_session_id_to_metrics_on_exit keeps it for the shutdown dump. Refs GYMINT-161. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Contributor
Author
|
/ok to test 8dbe563 |
Every server under the resources session contract answers a verify for a closed session with lost_session_verification, including those whose reward reads only the response, so a closed session reads the same everywhere. single_agent_turn verifies before it closes, so the mask only catches an out-of-order verify. TavilySearchVerifyResponse now inherits BaseVerifyResponse so it can carry mask_sample and failure_kind. A session the fence never recorded as closed, such as an agent's /run or a caller that never seeds, is graded as before. Refs GYMINT-161. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Contributor
Author
|
/ok to test c78b6f4 |
This was referenced Nov 4, 2026
…from nemo_gym.server_runtime main removed nemo_gym.testing.session_conformance; the check now lives in the server_runtime facade. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
yaoyu-33
added a commit
that referenced
this pull request
Nov 4, 2026
…rowsecomp_agent sessions branch Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Contributor
Author
|
/ok to test d7a83fb |
…urces-sandbox Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
yaoyu-33
marked this pull request as ready for review
November 4, 2026 20:31
…keep the shell's error envelope - benchmarks/browsecomp/config.yaml kept workspace: per_session with no sandbox_provider, so every seed got 422 until #5489 lands. Give the benchmark's resources server the Docker provider, as #5489 does. - An agent's /run can end without /verify (browsecomp_agent does after a timeout), and only /verify releases that path's sandbox. Start the /run sandbox with the first bash_command instead of at the seed, so only a run that used the shell can hold one, and a failed start no longer strands the seed's workspace (the failed seed's cookie was dropped, so nothing could release it). Document the remaining gap. - A command killed at the supervisor deadline, or a provider exec failure, answered HTTP 500. Answer with the host interpreter's [exit_code=-1] / [exit_code=-2] envelopes, as the README promises, and log the detail. - Say that only Enroot is probed for isolation, and that providers such as Apptainer bind-mount the host's home and /tmp by default. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Contributor
Author
|
/ok to test ac48bc4 |
…res; refuse bad sandbox config - A /run shell sandbox that cannot start now answers the [exit_code=-2] envelope instead of HTTP 500 with the provider's error text, and a blocked command answers before any sandbox starts. - A session whose tool sandbox failed to start or run a command gets a masked sample at /verify (failure_kind provider_unavailable). The judged reward is kept. - A command the supervisor ends at its deadline answers [exit_code=-1], as the README says, instead of -2. - A missing provider block or an unknown sandbox_config key refuses the seed with 422 instead of a retryable 500. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Contributor
Author
|
/ok to test 33eeb92 |
hemildesai
approved these changes
Nov 5, 2026
yaoyu-33
enabled stacked auto-merge
November 5, 2026 05:23
kajalj22
approved these changes
Nov 5, 2026
Contributor
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/41974035836 |
Contributor
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/41975510114 |
yaoyu-33
added a commit
that referenced
this pull request
Nov 5, 2026
Main now carries #5484 as a squash commit, so the files this branch's #5484 fixes touch conflict in overlapping hunks; this branch's side is #5484 plus those fixes and is kept. The agent picks up main's rename of VLLMConverter to ChatCompletionsConverter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BrowseComp kept its workspace and metrics on the Resources Server, and
bash_commandinterpreted model-generated commands in that host process. This change adds typed resources sessions and moves the existing pocketshell interpreter into a Resources-owned sandbox.Behavior
workspace: per_sessionrequires an isolatingsandbox_provider. The sandbox receives the checked-out shell source, retrieved pages and manifest. Search credentials, ground truth and verification remain on the Resources Server.SandboxPythonRuntimeandSandboxSession. Guards, supported commands, output truncation and exit-code formatting remain unchanged. Tool, verification and close operations are serialized per session./procbefore any model-directed command. Missing isolation rejects the session and uses the retained cleanup path. Older Enroot versions that silently ignoreENROOT_UNSHARE_PIDare refused.allow_host_execution: trueis an explicit development compatibility option. There is no automatic host fallback.workspace: nonestill returns inline search/browse content without a tool sandbox.The agent and Resources Server own separate containers in the full pairing. This works with providers that cannot serialize/reconnect a live sandbox. #5489 supplies the sandboxed agent and migrates both shipped pairings; this PR alone does not qualify the complete pairing. No separate issue is needed for this focused correction to the existing BrowseComp integration.
Validation
resources_servers/browsecomp_advanced_harness/.venv/bin/pytest resources_servers/browsecomp_advanced_harness/tests -qon the merged stack: 226 passed, including the child PR's timeout-verification cases.wcpadding mismatches; assertions are unchanged.pre-commit run --all-files: passed.cat,grepand manifest access succeeded; installed source hashes matched the candidate, mount namespaces differed from the host, host credential files were inaccessible, and owned containers were removed.After merging main and two review passes, at
33eeb9222682(macOS, no Docker, so no sandbox smoke for these changes):pytest resources_servers/browsecomp_advanced_harness/tests -q: 232 passed. The tests added in these passes fail on the previous code:/runseed starts no sandbox until the firstbash_command, and a blocked command starts none at all.[exit_code=-2]envelope instead of HTTP 500, without the provider's error text./verifythen keeps the judged reward but masks the sample (failure_kind: provider_unavailable).[exit_code=-1], as the README says.sandbox_configkey refuses the seed with 422 instead of a retryable 500.scripts/component_locks.pywith uv 0.13.4: 269 projects, 0 failed. The earlier lock and component-dependency CI failures came fromopencode_search, which main fixed in build(opencode_search): add the missing component lock files #5629 and this branch now includes.pytest tests/unit_tests/tooling/test_component_dependencies.py tests/unit_tests/test_public_api_facades.py tests/unit_tests/config/test_global_config.pyon the merged stack: 1041 passed.gym env resolveonbenchmarks/browsecomp/config.yamlwith the vLLM policy config: resolves, with the resources server'ssandbox_provider: sandboxblock (previously every seed got 422 until feat(browsecomp_agent): run the session harness inside a sandbox #5489 landed). On this PR alone the benchmark still runsbrowsecomp_agent's/run, so an aborted run keeps its sandbox untilttl_s; feat(browsecomp_agent): run the session harness inside a sandbox #5489 moves the benchmark onto the Environment Server path, which always closes.pre-commit run --fileson the changed files: passed.Pairing evidence and remaining gate
The earlier functional cluster run completed five BrowseComp rows (0/5 correct) and two reasoning_gym rows (2/2 correct), including 20 real shell calls. System Enroot 3.4.1 did not provide PID isolation, so that run is functional evidence only.
Actual final-source diagnostics verified that both guards reject system Enroot 3.4.1 and accept a complete Enroot 4.2.1 runtime with separate PID namespaces and matching
/proc. The nonsecret host sentinel is absent/unreadable from the new container and signal 0 returnsProcessLookupError.The final merged source completed one unchanged BrowseComp row (reward 0) and one unchanged reasoning_gym row (reward 1) on Enroot 4.2.1, including 5 natural shell calls. #5489 records the model/search settings, old/new runtime evidence, Evaluation gaps and retained failed attempts. Usage/lifecycle accounting, owned-container cleanup and final exact-value credential scanning passed; temporary cluster keys were deleted. The final live audit covered the resource container and PID1, but did not catch the short-lived shell runner itself; all 5 tool outputs and exact source/exec-path namespace checks are retained. This PR is ready for maintainer CI/review.
The sandbox and session migration changes deployment requirements. It does not establish benchmark score parity; #5489 documents the failure-denominator and activation-time-limit changes.