Repository navigation
feat(nooa): restrict SWE task networking and prepare offline runtime - #4088
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
1aff24a to
c2de575
Compare
c2de575 to
f8c3c5b
Compare
f8c3c5b to
070c74a
Compare
eeaf374 to
db166b3
Compare
49a8ff0 to
f8fbeec
Compare
10a2a19 to
4c56c44
Compare
|
/ok to test e3645b6 |
|
🌿 Preview your docs: https://nvidia-preview-pr-4088-d70ea7e9d392.docs.buildwithfern.com/nemo/gym |
init-nikhil
left a comment
There was a problem hiding this comment.
CODEX SELF REVIEW
Reviewed head e3645b69cb70e3ad6781cb9b972ca9771c5a26ce against actual stacked base 53196dcf6bdb8c923060b0c517612d24aaa53803 using official Codex CLI 0.154.0 (gpt-6-astra, fresh read-only process).
- [P2] Preserve host ownership of Docker build artifacts —
responses_api_agents/nooa_agent/offline_runtime.py:165-169. On a non-root Linux controller using ordinary rootful Docker, the builder creates root-owned files in its bind-mounted temporary directory. Cleanup can raisePermissionErrorafter bundle publication, and failed builds can leave undeletable artifacts. Preserve controller ownership through success and failure cleanup.
This is the engine's static finding; independent reproduction and repair validation are tracked separately. Tests were not executed by the review process.
|
/ok to test 22dedb4 |
init-nikhil
left a comment
There was a problem hiding this comment.
CODEX SELF REVIEW
Reviewed head 22dedb4ab7477df679e994ed314bd568c2abff2f against actual stacked base 53196dcf6bdb8c923060b0c517612d24aaa53803 with official Codex CLI 0.154.0 (gpt-6-astra, fresh read-only review process).
No actionable findings were returned.
The engine performed static source inspection; it did not execute tests. Model, benchmark and CI validation are tracked separately.
|
/ok to test 125ce2d |
init-nikhil
left a comment
There was a problem hiding this comment.
CODEX SELF REVIEW
Reviewed head 125ce2d7d9991f7d585a0ff4d8fd0bd2ddf6935d against actual stacked base 53196dcf6bdb8c923060b0c517612d24aaa53803 with official Codex CLI 0.154.0 (gpt-6-astra, fresh read-only review process).
No actionable findings were returned.
The engine performed static source inspection; it did not execute tests. Model, benchmark and CI validation are tracked separately.
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
SHIP — no actionable reliability findings.
Completed the light static review of all 32 changed files and the directly affected networking, runtime-installation, and context-planning paths. The historical Docker artifact-ownership issue is addressed by the owner probe and user-scoped build in offline_runtime.py.
Tests, offline-container installation, and live OpenSandbox network enforcement were not executed during this review.
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.
furgalep
left a comment
There was a problem hiding this comment.
I reviewed this by reading the code and tracing callers; nothing was run. The two I'd look at first are the event-loop blocking in prepare_nooa_runtime and the breadth of the egress allowlist.
| model_url = sandbox_server_url(self.config.task_network_policy.model_server.name, require_reachable=True) | ||
| # Replace rather than extend any baseline allowlist. Grading keeps the original options. | ||
| provider_options = dict(provider_options or {}) | ||
| provider_options["network_policy"] = restricted_network_policy(next(iter(provider_config)), [model_url]) |
There was a problem hiding this comment.
The allowlist is the model server's whole host, on all ports. In the recipe, policy_model is a Gym server on the controller, and that host also runs the head server, the SWE Resources server and the agent server. Task code could reach all of them, including the head server, whose config may hold OPENSANDBOX_API_KEY / POLICY_API_KEY.
The docs acknowledge this, but nothing enforces it. Could the policy be limited to the model server's port, or could the model endpoint be exposed through a dedicated proxy?
There was a problem hiding this comment.
the open sandbox version we use right now does not support this, but there is a release candidate being tested. this version of open sandbox will provide support for specific ports.
init-nikhil
left a comment
There was a problem hiding this comment.
CODEX SELF REVIEW
Reviewed head 3057bb5ac3c2825799e7a2a9bdec9aaf334ddbb3 against actual base a7a6ba544f2592c1bb2712a765f48db5b8e8b168 using a fresh official codex exec review process.
No actionable regressions were identified. All 83 focused model-adapter, network-policy, and observability tests passed; live Docker builds and OpenSandbox enforcement were not validated.
This is a source-review result, not an approving review or a substitute for the separate validation and CI gates.
|
/ok to test 1a2491c |
|
/ok to test 09862c8 |
Signed-off-by: Nikhil Deshmudre <nikhil.deshmudre@gmail.com>
|
/ok to test d70ea7e |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/41766407359 |
Current stack revision — November 3, 2026
Head:
d70ea7e9d3926556cf14997ef5a3446dfe67a2b2. Stack refreshed onto main3880fa5024bf97199cba96ade2e0fa6ec9eb6fbfwith current import locations and merge conflicts resolved. Each PR remains a separate layer; landing uses native stack merge.#4088 now packages the launching Gym checkout using its unchanged root
pyproject.tomland full core dependencies. The separateresponses_api_agents/nooa_agent/runtime/pyproject.tomlwas removed. Tracked source and package assets are copied with their current contents; untracked files, Git metadata, and caches are excluded. This requires Git on the controller. Sandbox installation stays in its private Python, and source changes invalidate prepared offline wheelhouses. #4813 retains its workflow change and code-owner review requirement.Validation: 87 focused runtime, offline-bundle, and task-network-policy tests passed after the refresh; all-files pre-commit passed (private guardword catalog unavailable). A real Gym wheel built successfully from the new archive format, and its NOOA entrypoint, resource dispatcher, and core package asset were verified. No model rollout or Linux installation was run for this packaging change. Earlier evidence below is historical and does not establish fresh results for this head.
GitHub dismissed earlier reviews after this update; current-head reviews and required CI are still needed.
Stack refresh — 2026-11-02
Current head:
ce3f4dcc670265d076379588d021666515a0cef9. Stack refreshed onto main2b8efc987af03951a80f941f2a278317badfe085. This PR remains a separate layer in the intended stack merge: #4088 → #4820 → #4821 → #4351 → #4357. #4813 is excluded.Reconstructed each layer from its final reviewed patch instead of replaying foundation commits already merged into main. Resolved current package-import and legacy-environment renames while retaining upstream checkpoint handling and newer harness support. #4088 also exports its networking helpers through the current public API and mechanically updates existing callers; #4351 exports its newly used response types.
Fresh validation on the combined refreshed stack:
19caab169b018476ac433d040f6ae3f06aeff101.pre-commit run --all-files: passed independently at all six PR layers, including package layering, guardwords, lint and format.No fresh real-model benchmark was run for this conflict/import refresh. Prior live-rollout evidence below is historical. Fresh reviews and required GitHub CI remain outstanding; this is not a merge-readiness claim.
Earlier implementation and validation (before this refresh)
Tests and validation
Head
3057bb5ac3c2825799e7a2a9bdec9aaf334ddbb3; basea7a6ba544f2592c1bb2712a765f48db5b8e8b168. The existing stack tracks this work; no separate issue is needed.145 focused checks passed across offline packaging, runtime staging, native task adaptation, network configuration and token budgets. Independent concurrency/cancellation probes also passed. Production
prepare_runtime_wheelhousebuilt a fresh source-bound bundle; productionprepare_nooa_runtimeinstalled, imported and reused it inside an owned Docker sandbox withnetwork=none. Archive construction, validation and staging left the event loop responsive.A separate fresh five-call model rollout exercised the actual adapter/worker, native tools, a 2,048-token output cap and 65,536-token context setting. The synthetic SWE task earned canonical verifier reward 1 and completed cleanup.
The offline build used a custom manylinux 2.28 amd64 builder; default manylinux2014/glibc 2.17 binary compatibility was not demonstrated. The live rollout used local provisioning and a preselected runtime. Current OpenSandbox provider egress enforcement was not requalified; older network evidence remains historical. Host-wide model egress is deliberate, TCP-port enforcement remains deferred pending provider support, and that review thread remains unresolved. Explicit Docker use requires the documented unrestricted policy configuration. This is integration evidence, not an official benchmark score.
pre-commit run --all-filespassed. Fresh official Codex review returned no actionable findings; the independent bug hunt passed for this head and base.CI snapshot (2026-10-24 03:47 UTC) on this head: failing:
Harness conformance (non-blocking) / kilocode conformance probes;Harness conformance (non-blocking) / Publish check;Harness conformance (non-blocking). Current checks. The all-green readiness gate remains unmet; no final verification-success post or new review request is implied.