Skip to content

feat(sessions): centralize resources lifecycle and verification ownership - #5709

Open
yaoyu-33 wants to merge 9 commits into
mainfrom
yuya/shared-resources-lifecycle
Open

yaoyu-33 wants to merge 9 commits into
mainfrom
yuya/shared-resources-lifecycle

Conversation

@yaoyu-33

@yaoyu-33 yaoyu-33 commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Stateful resources servers currently repeat seed, close, verification cleanup and identity bookkeeping. Centralize that lifecycle in SimpleResourcesServer so failed initialization, cookie reseeding, verification errors and cleanup retries preserve resource ownership consistently.

The base owns identity checks, session locking, bounded verification receipts, close fencing and checkpoint retirement. Servers implement three state hooks and score within async with self.verifying(...); public identity/restore helpers let checkpoint implementations preserve state without editing base-private dictionaries. Hook completeness and async shape are validated at class definition. Stateless servers and custom seed handlers retain their existing behavior.

This is the common dependency for #5479, #5480, #5481, #5482, #5483, #5485, #5486, #5487 and #5488. It builds on the separate MCP annotation fix, #5708. Each environment PR contains only its own migration and documentation. No separate issue is needed for this consolidation of the existing session migrations.

Compatibility:

  • A cookie reseed releases the preceding run before initialization and retains the cookie ID. Failed initialization attempts cleanup; failed release retains ownership for retry. A previous verification receipt is discarded only after successful replacement.
  • Verification holds the session lock through scoring and release, including score exceptions. Explicit retain(None) permits request-only retry scoring; an omitted retention masks retries. Ordinary release failures preserve the completed score or original scoring exception and emit a separate cleanup observation. Pending cleanup retains checkpoint and worker ownership for close/retirement; cancellation still propagates. Retry payloads retain the original expiry and cannot resurrect an expired receipt after a slow judge call. Close invalidates retries; checkpoint retirement does not fence a later restoration.
  • The three-hook interface intentionally retains has_session_state to distinguish verifier-visible state and protect tool-created or restored resources. Tools that overlap verify/close must use the same session lock.
  • This does not add a general TTL for abandoned live sessions. External resources still require provider TTLs or an environment-specific expiry policy. Newton retains its existing TTL policy in its child PR.
  • A request cancelled by its caller's disconnect does not finish its release: the pending record keeps ownership, and the next verify retry, close or retirement releases the session (the provider's TTL does if none comes). A close that waits on the session lock holds nothing and is cancelled without side effects if its caller gives up.
  • State, locks, fences and receipts live in the worker process. With several workers, session routing keeps a session's requests on its owner, so a hook server needs no num_workers: 1; a failed initialization whose cleanup also fails is retried by the next seed or close of that session and is not tracked by checkpoint retirement.
  • check_resources_session_contract(keeps_state=True) now also requires that a close of the open session naming another episode answers 409 and releases nothing. The suites that call it and run in this checkout pass.
  • The example single-tool-call server's private episode table is renamed so it no longer shares a name with the base's.

Prior lifecycle validation on 56c611ea354e9e339407bebd25271af5297beda3, Python 3.13.14:

python -m pytest -o addopts='' -o log_cli=false --import-mode=importlib --tb=short -q \
  tests/unit_tests/server_runtime/test_resources_session_lifecycle.py \
  tests/unit_tests/server_runtime/test_resources_session_policy.py \
  tests/unit_tests/server_runtime/test_base_resources_server.py \
  tests/unit_tests/server_runtime/test_mcp_auto_exposure.py \
  tests/unit_tests/server_runtime/test_mcp_seed_validation.py \
  tests/unit_tests/server_runtime/test_session_routing.py \
  tests/unit_tests/server_runtime/test_verifier_observability.py \
  tests/unit_tests/server_runtime/test_checkpoint_resources.py \
  tests/unit_tests/server_runtime/test_checkpoint_resources_mcp.py \
  tests/unit_tests/test_session_contract_repo.py \
  tests/unit_tests/test_public_api_facades.py

1,251 passed at that revision in this standalone common checkout, with main merged. New regressions cover omitted versus explicit None/falsey evidence, score and judge-error preservation after failed cleanup, cancellation (including a cancellation that arrives during failed-initialization cleanup), a failed cookie reseed after a finished run, a retry that finishes after its window behind a later receipt, cookie-seed races and fences, typed takeover and restore refusals, exact response/reward grading bindings, and retained checkpoint/worker ownership. Negative controls fail on the prior behavior and when the checkpoint ownership guard is removed. All nine environment suites passed after this common follow-up: 904 tests in total (after trimming redundant weather tests and adding the new regressions); their child descriptions record exact commands and counts. The original suite and five earlier negative controls remain evidence for reseed/failed-init cleanup, hook validation, and retry expiry.

pre-commit on the changed files (including the import-linter layering check) and git diff --check passed. The private guardwords catalog was unavailable, so that scan was skipped. The commit includes DCO sign-off. Real-model validation of preceding shared revision 4e92d4891 used nvidia/nemotron-3.5-lightning-30b-a3b across all nine consumer environments, with source hashes verified. Typed normal paths exercised scoring, release and close; Workplace also exercised its cookie path. Math, NS and Newton additionally exercised skip-verification close with actual live workers/backend sessions. Weather used actual MCP initialize/list/tools/call, scored 1.0, retried verify at 1.0 and closed with empty state; that client ran on macOS Python 3.13.14 after cluster shared-storage stalls. Other completed cases ran on Linux Python 3.13.14.

Keep the limits distinct: Newton's strict rollout receipts contain invalid numeric tool arguments (HTTP 422), although its worker cleanup succeeded; Tavily scored 0 and Maze reached its step cap with reward -1.7. These are lifecycle checks, not benchmark baselines. Litmus's original skip case stalled in provider allocation; its child PR #5482 contains a separately reviewed startup-cancellation repair. Both normal and skip-verification real-model cases passed on child revision b8cf93d with this shared core: normal reward 1.0, zero verify calls on skip, successful close, and empty session/sandbox state. That job recovered from a temporary storage stall and completed without cancellation. The older attempt's disk-rootfs cleanup remains blocked in shared-storage I/O; no live container is claimed. Original failures and scoped cleanup evidence are retained.

The grading paths remain owned by each verifier and the registered observation wrapper. The new helper is opt-in: this standalone PR changes no environment verifier; only the nine children adopt it. Its bounded in-process receipts do not promise durable or offline regrading.

Representative validation on the preceding core also passed the real-model Weather MCP path: two model calls, one actual MCP tool call, reward 1.0, retry reward 1.0, successful close and empty live/ownership maps. All source files and symlink targets remained unchanged. Litmus validation on child 56c33f370 also passed: three real model calls, two Enroot tools, reward 1.0 with a grading record bound to the actual response/reward, verify/close 200 and empty session/sandbox state. All 9,181 source file hashes matched before and after; tracked processes ended. Other seven consumers retain their explicitly versioned preceding-head model evidence plus final-head regression suites, rather than claiming those older trajectories ran on the new head.

MCP annotation follow-up on e484e13b4c847060a54d7ee6120c62332d99d70f addresses the remaining annotation-reader review on #5708. Both readers now preserve Annotated metadata, resolve only deferred string annotations, honor concrete factory-set __signature__ annotations, and recognize Request subclasses (including annotated ones). The seed-wrapper comment now describes the general validation/dependency-injection rule.

Compatibility: direct MCP dispatch refuses unsupported annotated tool bodies and annotated response models at startup rather than bypassing validators or response filtering. A supported explicit plain response model still takes precedence. HTTP seed wrapping retains its validators and correctly reuses the live Request.

Validation of this follow-up, Python 3.13.14 with the pinned aiohttp 3.14.4: the complete eleven-file regression command above passed 1,266 tests, including 15 new annotation cases. The original implementation failed nine of the first thirteen new cases; removing the response guard fails both new response cases. pre-commit run --all-files (including guardwords and import-linter) and git diff --check passed; the commit has DCO sign-off. Independent source review found and checked the response-filtering guard. The initial sandboxed broad run could not bind routing-test sockets; the complete rerun with local socket access passed.

After merging main 379ae226d12a46644cf11419b73eac081f0f7432, the same eleven-file command passed 1,274 tests on final head 404fa4d8346f8072b5ba1b71e16aae28b1036de6, and all pre-commit hooks passed again. The conflicts came from the squash merge of #5708; the three conflicted files on main were byte-identical to this follow-up's parent, and the resolution preserves the reviewed fix exactly.

New model rollout: N/A for this bounded annotation-inspection fix; real HTTP seed routes and MCP initialize/tools/call exercise the changed behavior directly. This follow-up does not change prompts, scoring or tool implementations. The preceding model evidence remains tied to its recorded revisions, not this follow-up.

Full repository CI and reviewer approval remain required. The prerequisite #5708 is merged; integrate the environment children after this PR and their own validation and review.

Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
…ship

Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Nov 5, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@AdamRajfer AdamRajfer left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lock, fence and receipt logic is careful and the ownership tests are thorough. My concern is grading, inline. I would keep this as a draft until those are settled.

Migration risk: 45 servers define their own seed_session, 18 define close_resources_session, and all 154 SimpleResourcesServer subclasses get the wrapped seed route; resources_servers/example_single_tool_call/app.py already declares _session_episodes, the same name as the new base private attribute. The session lock is held through a slow judge call, so a close the Environment Server cancels for cleanup_timeout_seconds leaves the sandbox to its TTL; bound that wait. num_workers: 1 is not enforced and receipts are process-local.

Mergeable when the receipt, release-failure and lock cases are fixed and tested, and one stateful consumer shows a masked judge fault and a retained-answer re-judge end to end.

Comment thread .agents/skills/add-benchmark/SKILL.md Resolved
@yaoyu-33
yaoyu-33 marked this pull request as ready for review November 6, 2026 00:09
yaoyu-33 and others added 5 commits November 5, 2026 16:27
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
The wrapper strips Annotated only to recognize an injected Request. A
validator written directly on the body parameter, the form an author is
most likely to write, was covered only through a union arm, so a wrapper
that stripped every parameter still passed.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>

# Conflicts:
#	nemo_gym/server_runtime/base_resources_server.py
…ment the hook contract

- A cancellation that arrives while a failed initialization is being cleaned up now propagates instead of
  being swallowed by the cleanup guard.
- A cookie reseed that released the preceding run moves its finished receipt into the retry window, so a
  failed replacement no longer leaves a pending-cleanup record with nothing to release.
- Document that a disconnect leaves a verification release pending, that the release hook runs under the
  session lock, that state is per worker with session routing keeping a session on its owner, and the
  override rules for servers that keep their own seed and close.
- Rename the example server's private episode table so it no longer shares a name with the base's.
- Add regressions for the receipt deadline guard, worker ownership wiring, cookie seed races and fences,
  typed seed takeover, restore refusals and a cancelled close waiting on verification.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
@yaoyu-33

yaoyu-33 commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 56c611e

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Base automatically changed from yuya/fix-mcp-seed-validation to main November 6, 2026 19:03
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
@yaoyu-33
yaoyu-33 enabled auto-merge November 6, 2026 20:16

This branch was previously deployed

1 inactive (outdated) deployment
public — 56c611ea Deployed Nov 6, 2026 by copy-pr-bot[bot] via release / finalize / notify #10584
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants