Repository navigation
Conversation
…sion With sandbox_provider set, litmus_agent starts a per-rollout sandbox keyed by the session cookie, and stopped it only in the wrapped /verify or at shutdown. Under single_agent_turn_legacy, an episode that never reached /verify kept its sandbox until the provider TTL (OpenSandbox) or server shutdown (Apptainer), because the inherited close_resources_session kept no state. _cleanup_session also dropped the session before stopping it and swallowed stop errors, so a failed stop leaked the sandbox. - seed_session binds a typed seed's resources_session_id to the cookie and records its episode and task; the legacy /run seed body keeps no state. - close_resources_session stops the sandbox by resources_session_id, fences late seeds with ResourcesSessionFence, is idempotent, and raises when the stop fails, so the caller can retry. - A sandbox whose stop fails stays registered so a later cleanup retries it; verify logs the failure and keeps its score, and shutdown goes on to the remaining sandboxes. - A tool call for a closed session gets ResourcesSessionLostError (404); a call that a close overtook stops the sandbox it started. - A /verify for a closed session returns lost_session_verification. - Drop the shared litmus-no-session fallback key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
yaoyu-33
added
area:environment
|
/ok to test a5e35cf |
yaoyu-33
added
the
needs-review
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
…ping it Verify's cleanup ran without the session's lock, so a verify that arrived while the first tool call was still creating its sandbox stopped a wrapper with no handle yet: that only released the client. Startup then created the container, which nothing tracked or stopped. A close could likewise stop a sandbox under a running cell, so the call failed with a 500 instead of finishing. _cleanup_session now holds the session's lock across the stop, and a tool call that waited behind a cleanup gets 404. The post-exec closed check could no longer fire, so it is gone; its test is replaced by tests that drive a real close during a cell and a real verify during startup, both of which fail on the previous code. The conformance import moves to nemo_gym.server_runtime.testing, and the README notes OpenShell's queued deletes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
|
/ok to test 5678ae6 |
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
github-actions
Bot
added
the
sla:triage-overdue
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>
…ycle - The repo contract test counts a server that implements the state hooks as closing through the inherited close_resources_session. newton_bench, workplace_assistant, tavily_search and indirect_prompt_injection override seed_session but close through the hooks, and failed it. - Export reject_partial_typed_seed from nemo_gym.server_runtime, so servers that override seed_session reuse the shared guard instead of a copy. - The MCP seed wrapper still recognises a request param annotated as Annotated[Request, ...]; keeping Annotated metadata had made it inject a second Request and fail with KeyError. - Name the 409 for a typed seed over a cookie session "cookie-seeded", not "legacy", and document that a served seed_session override declares request before body. - Shared tests now cover the MCP-wrapped seed route, close after verify (checked against the verified episode), and a cookie seed of a verified typed session. - The add-benchmark skill describes the three state hooks as the default for a stateful server. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
…nt cookie sessions - Stop the remaining sandboxes concurrently at shutdown. The server supervisor kills the process about a second after SIGINT, so stopping them one at a time left most of them to their ttl_s. A failed stop is still logged with its session ID and does not keep the others from stopping. - Test that shutdown stops sandboxes concurrently, and that a shutdown during a pending sandbox start waits for the start, stops the started sandbox, and fails the waiting tool call as a lost session. - README: shutdown stops what it can within the shutdown grace and leaves the rest to ttl_s; after a cookie session's /verify a tool call on that cookie gets 404 until the cookie is seeded again; a cookie seed of a closed session gets 409. - Tests import check_resources_session_contract and ResourcesParticipant from nemo_gym.server_runtime, which test_public_api_facades requires. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
…antics 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>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
github-actions
Bot
removed
the
sla:triage-overdue
svcnvidia-nemo-ci
left a comment
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
SHIP
One documentation NOTE: cleanup failures preserve the verification score, but the lifecycle description says they fail the request. I found no runtime correctness defects in the scoped review.
I inspected all three changed files and directly affected lifecycle, sandbox, checkpoint, and verification paths. This was a read-only source review; I didn't run tests. The verification/documentation child's report was unavailable, so I completed that inspection directly.
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.
| starting or running a cell before they stop it. A stop that fails makes the close | ||
| fail, and the sandbox stays registered, so the caller's retry stops it; a provider | ||
| that only queues the delete, such as OpenShell, logs its failures instead. A failed stop at `/verify` | ||
| also fails that request, keeping checkpoint ownership available for cleanup on retry. The retry |
There was a problem hiding this comment.
NOTE — Describe score-preserving cleanup failures (suggestion)
Could you update these sentences to match the implemented HTTP contract? After seeding and executing a cell, make the provider's close() raise and post a valid answer to /verify: the route still returns HTTP 200 with its computed reward, rather than failing the request. The shared verification context reports the cleanup fault separately and retains ownership. The current wording gives callers the wrong retry trigger.
Replace the two cleanup-at-verify sentences with:
A failed stop at
/verifypreserves the computed score and reports the cleanup failure separately. The sandbox stays registered and checkpoint ownership is retained until a later verify, close, or retirement successfully releases it. A verify retry recomputes the score.
Keep the neighboring distinctions: a failed /close_session still fails and remains retryable; a successful close still fences late seeds and masks late verification. No runtime change is needed.
Acceptance, based on existing assertions rather than executed tests: retain HTTP 200/reward 1.0 despite failed verify cleanup, HTTP 500 for the subsequent failed close, and one successful stop after recovery. Also retain checkpoint ownership after failed cleanup and its release after a successful verify retry. The revised paragraph should describe those outcomes.
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
… lifespan shutdown Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
yaoyu-33
changed the title
|
/ok to test 5f1cbc2 |
geekysuavo
left a comment
geekysuavo
left a comment
There was a problem hiding this comment.
No concerns from my end, as these changes don't impact the domain-specific logic of the litmus agent resources server. Would appreciate eyes from core maintainers, though.
The docs now describe only the typed Environment Server path. The cookie seed still works unchanged; the PR description records that compatibility. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
|
/ok to test 4022766 |
Release Litmus sandbox state through the shared resources lifecycle, preserving close without verification and cleanup retries. Verification now uses the public shared context, including the supported verifier-only configuration without a sandbox provider.
Depends on #5709 (which depends on #5708). The child diff contains only this environment and its related documentation/configuration; shared implementation is reviewed once in the dependency. No separate issue is needed for this focused migration of an existing resources server.
Request-scored: retains an empty marker (
retain(None)) because the request already carries the answer and grading data.Validation on
5f1cbc29237454fc23e7675b1bc3b044e6b3a4d3, Python 3.13.14, with this checkout first on PYTHONPATH and NEMO_GYM_EXTRA_ROOTS pointing to it:python -m pytest -o addopts='' -o log_cli=false resources_servers/litmus_agent/tests -q --tb=short: 111 passed.ruff check,ruff format --checkandgit diff --checkpassed on the changed files. The private guardwords catalog was unavailable, so that scan was skipped. New commits include DCO sign-off.Final-source real-model validation passed on
56c33f3705f5669705d432e1c8e53e885d07ac6a(shared cored273280a6841) usingnvidia/nemotron-3.5-lightning-30b-a3b, Python 3.13.14 and the real Enroot provider. Three model calls and two real tools produced Python output 385 and the final answer. Verification scored 1.0 unmasked, with a completed grading record bound to the actual response and reward; verify/close returned 200 and all session/identity maps were empty. All 9,181 source files and helper hashes matched before/after, sandbox data was empty and tracked backend/model processes ended.The preceding startup-fix revision
b8cf93dalso passed normal and skip-verification model paths: skip made zero verify calls and its acknowledged close released a live sandbox. That bounded job recovered from a temporary storage stall and completed without cancellation. The original older attempt's rootfs remains blocked on shared-storage deletion; no live container is claimed. Earlier preparation/forced-shutdown failures remain preserved. These synthetic lifecycle checks do not establish benchmark parity or a guarantee for forced global cancellation/provider failure before handle publication.These targeted checks do not replace the repository CI matrix or approval of the current head.