Skip to content

fix(onboard): keep pending route reservation through not-ready recreate#6626

Merged
cv merged 5 commits into
mainfrom
fix/onboard-pending-reservation-survives-create
Jul 10, 2026
Merged

fix(onboard): keep pending route reservation through not-ready recreate#6626
cv merged 5 commits into
mainfrom
fix/onboard-pending-reservation-survives-create

Conversation

@laitingsheng

@laitingsheng laitingsheng commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

onboard --resume still failed with sandbox route reservation '<name>' disappeared while onboarding was in progress after PR #6572, but only when an interrupted first run left a partial sandbox live on the gateway (reproduced on DGX Spark). This preserves the current onboard session's pending route reservation through the not-ready recreate so the resumed run can recover.

Related Issue

Fixes #6562

Same root cause as #6563 (closed, co-fixed by #6572). Release-safe groundwork for the #6228 checkpoint refactor and #6492 atomic registry/gateway work; it does not close their acceptance criteria.

Changes

  • state/registry.ts: add reservationSessionId to the sandbox row, stamp it in reserveSandboxInferenceRoute, and add isPendingReservationForSession to distinguish the active session's reservation from an abandoned one.
  • onboard/machine/handlers/provider-inference.ts: stamp the reservation with the current onboard sessionId (persisted, so it survives --resume).
  • onboard.ts: in the not-ready recreate path, skip the pre-rebuild registry removal for the current session's pending reservation. PR fix(onboard): preserve pending route reservation across resume #6572 guarded only the !liveExists tool-disclosure prune; this covers the liveExists (partial-sandbox) path that reproduced on slower hosts. Abandoned reservations from other sessions still prune, and the not-ready gateway sandbox is still deleted before the rebuild.
  • Tests: registry ownership/stamping coverage and a subprocess fault-injection test asserting the reservation row survives a not-ready recreate.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: internal onboarding lifecycle repair; no user-facing behavior, CLI surface, or configuration change.
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification:
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result: vitest runregistry-route-reservation 5/5, onboard-reservation-recreate 1/1, full onboard.test.ts 66/66, and tool-disclosure-flow + sandbox-lifecycle + provider-inference-route-containment 15/15.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Note on hooks: pre-commit and commit-msg passed. pre-push (and npm run check:diff) fail only on pre-existing environment gaps in the local worktree — an unbuilt dist/ (package-contract and e2e-live imports) and a missing typebox dependency (tools/pr-review-advisor) — neither of which touches this diff. npm run typecheck:cli reports zero errors in the changed files; CI runs the authoritative full-environment checks.


Signed-off-by: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

  • Bug Fixes

    • Preserved pending sandbox route reservations during sandbox recreation when owned by the active onboarding session.
    • Prevented cleanup from removing a sandbox when its pending reservation is reserved for the current session.
    • Added session-aware pending reservation ownership tracking, including support for stamping reservation ownership IDs onto reservations.
  • Tests

    • Added integration coverage for reservation-safe sandbox recreation across multiple reservation ownership scenarios.
    • Expanded unit/integration tests to verify reservation session stamping and correct ownership transfer behavior.

When an interrupted onboard left a partial sandbox live on the gateway,
the not-ready recreate removed the pending inference-route row before the
long rebuild, so a second interruption stranded `--resume` with "route
reservation disappeared". The create path finalises that row in place via
updateSandbox and never re-registers it, so the reservation must survive
the recreate.

Stamp the reservation with the owning onboard session id and skip the
pre-rebuild registry removal for the current session's pending
reservation; abandoned reservations from other sessions still prune.

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 481ccb3c-44db-4ea2-ae3b-a6ff2d7a2ab2

📥 Commits

Reviewing files that changed from the base of the PR and between 0642e00 and fd0249d.

📒 Files selected for processing (5)
  • src/lib/onboard/machine/core-flow-phases.test.ts
  • src/lib/onboard/machine/handlers/provider-inference.test.ts
  • src/lib/onboard/sandbox-lifecycle.test.ts
  • src/lib/state/registry-route-reservation.test.ts
  • test/onboard-reservation-recreate.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/lib/state/registry-route-reservation.test.ts
  • test/onboard-reservation-recreate.test.ts
  • src/lib/onboard/machine/handlers/provider-inference.test.ts

📝 Walkthrough

Walkthrough

Sandbox route reservations now track their owning onboarding session. Resume flows propagate that session ID, and sandbox recreation preserves matching pending reservations while continuing to delete non-ready sandbox resources.

Changes

Reservation ownership flow

Layer / File(s) Summary
Registry reservation ownership
src/lib/state/registry.ts, src/lib/state/registry-route-reservation.test.ts
Sandbox entries persist reservation session IDs, route reservations propagate them, and ownership matching is tested for pending, registered, and missing entries.
Inference reservation propagation
src/lib/onboard/machine/handlers/provider-inference.ts, src/lib/onboard/setup-inference.ts, src/lib/onboard/machine/handlers/provider-inference.test.ts, src/lib/onboard/setup-inference-route-containment.test.ts, src/lib/onboard/machine/core-flow-phases.test.ts
Inference setup and routed or authoritative resume paths pass the current session ID into route reservations, with updated expectations and containment coverage.
Sandbox recreation preservation
src/lib/onboard/sandbox-lifecycle.ts, src/lib/onboard/sandbox-lifecycle.test.ts, src/lib/onboard.ts, test/onboard-reservation-recreate.test.ts
Sandbox recreation retains entries owned by the active session and tests preservation alongside deletion of foreign or unstamped reservations.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ProviderInference
  participant SetupInference
  participant Registry
  participant Onboard
  participant SandboxLifecycle
  ProviderInference->>SetupInference: Pass session ID
  SetupInference->>Registry: Reserve route with session ID
  Registry-->>SetupInference: Persist pending reservation ownership
  Onboard->>SandboxLifecycle: Remove sandbox unless session reservation
  SandboxLifecycle->>Registry: Check reservation ownership
  Registry-->>SandboxLifecycle: Return ownership match
  SandboxLifecycle-->>Onboard: Preserve matching reservation
Loading

Suggested labels: area: onboarding

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main fix: preserving pending route reservations during not-ready sandbox recreation.
Linked Issues check ✅ Passed The changes persist reservationSessionId and preserve the current session's pending reservation across recreate/resume, matching #6562.
Out of Scope Changes check ✅ Passed The added code and tests are all directly tied to reservation persistence and recreate safety, with no clear unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/onboard-pending-reservation-survives-create

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the branch.

TypeScript / code-coverage/cli

The overall coverage in the branch remains at 77%, unchanged from the branch.

Show a code coverage summary of the most impacted files.
File 2b84a04 fd0249d +/-
src/lib/onboard...eway-cleanup.ts 58% 55% -3%
src/lib/state/config-io.ts 96% 95% -1%
src/lib/onboard...up-inference.ts 93% 92% -1%
src/lib/state/registry.ts 78% 79% +1%
src/lib/onboard...ox-lifecycle.ts 42% 43% +1%
src/lib/inference/config.ts 93% 95% +2%
src/lib/sandbox...vileged-exec.ts 82% 89% +7%

Updated July 10, 2026 17:26 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume
Optional E2E: model-router-provider-routed-inference, sandbox-operations

Dispatch hint: cloud-onboard,inference-routing,network-policy,onboard-repair,onboard-resume

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • cloud-onboard (high): Required by the deterministic risk plan and because provider-inference/session reservation changes can affect the full hosted onboarding flow on a clean runner.
  • inference-routing (medium): Required by the deterministic risk plan because route reservation ownership and setupInference changes can affect the real gateway route advertised to the agent.
  • network-policy (medium): Required by the deterministic risk plan because inference route changes must still agree with live network-policy allow/deny behavior at the host-to-sandbox boundary.
  • onboard-repair (medium): Required by the deterministic risk plan and the onboarding resume rule: the PR changes provider-inference resume/repair state handling and reservation persistence used during recovery.
  • onboard-resume (medium): Required by the deterministic risk plan and the onboarding resume rule: the PR changes live onboarding state transitions, session bootstrap propagation, and resume reservation preservation.

Optional E2E

  • model-router-provider-routed-inference (medium): Adjacent confidence for the routed-provider branch touched in provider-inference.ts, where authoritative resume now stamps reservationSessionId while re-upserting routed providers under the gateway lock.
  • sandbox-operations (medium): Useful adjacent coverage for registry and lifecycle behavior after the change that preserves session-owned pending route reservations during sandbox recreation.

New E2E recommendations

  • lifecycle-state (medium): The PR adds session-owned pending route reservations, but existing live E2E does not appear to directly exercise a crash/resume sequence where sandbox recreation must preserve the active session's reservation while removing a stale or foreign-session reservation.
    • Suggested test: Add a live onboarding reservation ownership scenario that interrupts after route reservation, resumes with the same session through recreate, and verifies a foreign/abandoned reservation is pruned.

Dispatch hint

  • Workflow: e2e.yaml
  • jobs input: cloud-onboard,inference-routing,network-policy,onboard-repair,onboard-resume

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume
Optional E2E targets: None

Dispatch required E2E targets:

  • gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=cloud-onboard
  • gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=inference-routing
  • gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=network-policy
  • gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=onboard-repair
  • gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=onboard-resume

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

  • cloud-onboard: Installer and platform changes must work on a clean supported host with the pinned runtime dependencies.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=cloud-onboard
  • inference-routing: Inference selection, reachability, and network policy must agree at the real host-to-sandbox boundary.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=inference-routing
  • network-policy: Inference selection, reachability, and network policy must agree at the real host-to-sandbox boundary.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=network-policy
  • onboard-repair: Onboarding and sandbox state must converge across persisted metadata, reported status, and the live runtime.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=onboard-repair
  • onboard-resume: Onboarding and sandbox state must converge across persisted metadata, reported status, and the live runtime.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=onboard-resume

Optional E2E targets

  • None.

Relevant changed files

  • src/lib/onboard.ts
  • src/lib/onboard/machine/handlers/provider-inference.ts
  • src/lib/onboard/sandbox-lifecycle.ts
  • src/lib/onboard/setup-inference.ts
  • src/lib/state/registry.ts

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Fix PRA-1: Test monolith growth exceeds guardrail threshold; then add or justify PRA-T1.
Open items: 1 required · 2 warnings · 2 suggestions · 8 test follow-ups
Since last review: 0 prior items resolved · 4 still apply · 1 new item found

Action checklist

  • PRA-1 Fix: Test monolith growth exceeds guardrail threshold in src/lib/onboard/machine/handlers/provider-inference.test.ts:1218
  • PRA-3 Resolve or justify: Reservation ownership not sticky to original session on retarget in src/lib/state/registry.ts:511
  • PRA-4 Resolve or justify: Missing unit test: removeSandboxUnlessSessionReservation handles null entry in src/lib/onboard/sandbox-lifecycle.ts:15
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Runtime validation
  • PRA-T6 Add or justify test follow-up: Acceptance clause
  • PRA-T7 Add or justify test follow-up: test/onboard-reservation-recreate.test.ts:125 silent catch handler
  • PRA-T8 Add or justify test follow-up: registry.ts:511 reservationSessionId overwrite on retarget
  • PRA-2 In-scope improvement: E2E test uses heavy subprocess mocking where unit test pattern suffices in test/onboard-reservation-recreate.test.ts:107
  • PRA-5 In-scope improvement: Test scaffold uses silent error handler masking failures in test/onboard-reservation-recreate.test.ts:125

Findings index

ID Severity Category Location Required action
PRA-1 Required architecture src/lib/onboard/machine/handlers/provider-inference.test.ts:1218 Extract shared test fixtures/helpers (e.g., createDeps variants, session builders, common assertions) into a separate support module, or split the describe blocks into focused test files by behavior (fresh, resume, rebuild, routed, compatible-endpoint).
PRA-2 Improvement architecture test/onboard-reservation-recreate.test.ts:107 Refactor to a direct Vitest unit test: import registry, onboard/sandbox-lifecycle, and onboard-session modules directly; mock only the gateway calls (runOpenshell) and filesystem. Keep subprocess form only if it catches integration gaps unit tests miss.
PRA-3 Resolve/justify correctness src/lib/state/registry.ts:511 Change the logic to preserve existing.reservationSessionId when entry.pendingRouteReservation === true, only stamping a new sessionId on fresh reservations. Or add a guard: if (existing?.pendingRouteReservation && existing.reservationSessionId) keep existing.
PRA-4 Resolve/justify correctness src/lib/onboard/sandbox-lifecycle.ts:15 Add a test case in sandbox-lifecycle.test.ts calling removeSandboxUnlessSessionReservation(null, 'sandbox-name') and verifying registry.removeSandbox is called.
PRA-5 Improvement architecture test/onboard-reservation-recreate.test.ts:125 Either let the promise reject naturally (vitest will catch it) or use a proper test assertion pattern. Better: convert to unit test per F-002.

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

PRA-1 Required — Test monolith growth exceeds guardrail threshold

  • Location: src/lib/onboard/machine/handlers/provider-inference.test.ts:1218
  • Category: architecture
  • Problem: provider-inference.test.ts grew from 1177 to 1218 lines (+41) due to new reservationSessionId assertions across test cases. The file is now a 1200+ line monolith; extract or offset growth before merge per project guardrail.
  • Impact: Continued accretion in this test file impairs readability, slows test execution, and makes future changes harder to review. The growth is legitimate (new reservationSessionId plumbing) but unoffset.
  • Required action: Extract shared test fixtures/helpers (e.g., createDeps variants, session builders, common assertions) into a separate support module, or split the describe blocks into focused test files by behavior (fresh, resume, rebuild, routed, compatible-endpoint).
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: wc -l src/lib/onboard/machine/handlers/provider-inference.test.ts shows 1218 lines; grep -c 'reservationSessionId' shows new assertions.
  • Missing regression test: No regression test needed — this is a structural quality gate. The existing test suite passes; the finding is about maintainability.
  • Done when: The required change is committed and verification passes: wc -l src/lib/onboard/machine/handlers/provider-inference.test.ts shows 1218 lines; grep -c 'reservationSessionId' shows new assertions.
  • Evidence: monolithDeltas: baseLines=1177, headLines=1218, delta=41, severity=blocker diff shows +55 lines in provider-inference.test.ts with reservationSessionId additions across 14+ test cases
Review findings by urgency: 1 required fix, 2 items to resolve/justify, 2 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-3 Resolve/justify — Reservation ownership not sticky to original session on retarget

  • Location: src/lib/state/registry.ts:511
  • Category: correctness
  • Problem: reserveSandboxInferenceRoute uses `reservationSessionId: route.reservationSessionId ?? existing?.reservationSessionId` which overwrites the original owner's sessionId when a caller explicitly passes a different sessionId. The acceptance criteria and fix intent imply ownership should be sticky to the original reserving session — a retarget by a different session should preserve the original owner.
  • Impact: If a retry or race retargets the reservation with a different sessionId, the original session loses ownership and fails to resume, reintroducing the 'reservation disappeared' bug that nemoclaw onboard --resume fails with route reservation disappeared after interrupted onboard #6562/fix(onboard): keep pending route reservation through not-ready recreate #6626 fixed.
  • Recommended action: Change the logic to preserve existing.reservationSessionId when entry.pendingRouteReservation === true, only stamping a new sessionId on fresh reservations. Or add a guard: if (existing?.pendingRouteReservation && existing.reservationSessionId) keep existing.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check registry.ts:504-520 reserveSandboxInferenceRoute — the nullish coalescing allows overwrite. No test covers retarget with different sessionId preserving original.
  • Missing regression test: Add test: 'preserves original reservationSessionId when retargeted by different session' in registry-route-reservation.test.ts
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check registry.ts:504-520 reserveSandboxInferenceRoute — the nullish coalescing allows overwrite. No test covers retarget with different sessionId preserving original.
  • Evidence: registry.ts:511 reservationSessionId: route.reservationSessionId ?? existing?.reservationSessionId registry-route-reservation.test.ts has 'transfers reservation ownership when a new session retargets' but not the negative case Issue nemoclaw onboard --resume fails with route reservation disappeared after interrupted onboard #6562: 'preserves a current-session pending route reservation across a not-ready recreate' implies ownership stickiness

PRA-4 Resolve/justify — Missing unit test: removeSandboxUnlessSessionReservation handles null entry

  • Location: src/lib/onboard/sandbox-lifecycle.ts:15
  • Category: correctness
  • Problem: removeSandboxUnlessSessionReservation is called from onboard.ts with previousEntry that may be null (no registry row). The function uses optional chaining (entry?.pendingRouteReservation) to handle null gracefully, but no test verifies this null-entry path.
  • Impact: If a recreate occurs for a sandbox with no registry entry, the null path must correctly fall through to registry.removeSandbox. Without a test, a future refactor could break this fallback.
  • Recommended action: Add a test case in sandbox-lifecycle.test.ts calling removeSandboxUnlessSessionReservation(null, 'sandbox-name') and verifying registry.removeSandbox is called.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search sandbox-lifecycle.test.ts for a test covering the null entry path; currently absent.
  • Missing regression test: Add test: 'removeSandboxUnlessSessionReservation with null entry calls removeSandbox' in sandbox-lifecycle.test.ts
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search sandbox-lifecycle.test.ts for a test covering the null entry path; currently absent.
  • Evidence: src/lib/onboard/sandbox-lifecycle.ts:15 removeSandboxUnlessSessionReservation(entry, sandboxName) uses entry?.pendingRouteReservation src/lib/onboard.ts:2645 calls with previousEntry which may be null sandbox-lifecycle.test.ts has no test for null entry

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-2 Improvement — E2E test uses heavy subprocess mocking where unit test pattern suffices

  • Location: test/onboard-reservation-recreate.test.ts:107
  • Category: architecture
  • Problem: The new test spawns a Node subprocess with 70+ lines of mocks for registry, runner, childProcess, onboardSession, and preflight to exercise the recreate path. The same behavior (reservation survives recreate) could be tested with direct Vitest unit mocks like registry-route-reservation.test.ts does.
  • Impact: Heavy subprocess mocking increases maintenance burden, test execution time, and flakiness risk. The test only validates one integration point (registry guard during recreate) which unit mocks can cover.
  • Suggested action: Refactor to a direct Vitest unit test: import registry, onboard/sandbox-lifecycle, and onboard-session modules directly; mock only the gateway calls (runOpenshell) and filesystem. Keep subprocess form only if it catches integration gaps unit tests miss.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Compare test/onboard-reservation-recreate.test.ts (170 lines, spawnSync) vs src/lib/state/registry-route-reservation.test.ts (200 lines, direct imports + vi.fn pattern).
  • Missing regression test: No regression test needed — this is a test architecture improvement. Existing test passes and covers the behavior.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/onboard-reservation-recreate.test.ts:26-107 heavy mock scaffolding with spawnSync src/lib/state/registry-route-reservation.test.ts:8-95 direct import + vi.fn pattern

PRA-5 Improvement — Test scaffold uses silent error handler masking failures

  • Location: test/onboard-reservation-recreate.test.ts:125
  • Category: architecture
  • Problem: The E2E test uses `}).catch((error) => { console.error(error); process.exit(1); })` which catches all errors but provides no test assertion — failures are logged and exit non-zero without vitest awareness. This masks the actual failure mode and prevents proper test reporting.
  • Impact: Test failures in the subprocess are not visible to vitest as test failures; they appear as process exits. Debugging requires parsing stdout/stderr manually.
  • Suggested action: Either let the promise reject naturally (vitest will catch it) or use a proper test assertion pattern. Better: convert to unit test per F-002.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check test/onboard-reservation-recreate.test.ts:125 — the catch handler swallows the error context.
  • Missing regression test: No regression test needed — this is a test scaffold quality issue. Converting to unit test (F-002) would resolve.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/onboard-reservation-recreate.test.ts:125 })().catch((error) => { console.error(error); process.exit(1); }) No try/catch with expect().rejects or similar vitest pattern
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Run the `cloud-onboard` E2E job for Installer and platform changes must work on a clean supported host with the pinned runtime dependencies. Matched files: `src/lib/onboard/machine/handlers/provider-inference.ts`.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Deterministic regression risks in lifecycle-state, inference-policy, platform-install require live E2E validation (cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume). Unit/mocked tests cover logic but cannot validate cross-boundary invariants (gateway probing, network policy, clean-host install).
  • PRA-T2 Runtime validation — Run the `inference-routing` E2E job for Inference selection, reachability, and network policy must agree at the real host-to-sandbox boundary. Matched files: `src/lib/onboard/machine/handlers/provider-inference.ts`, `src/lib/onboard/setup-inference.ts`.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Deterministic regression risks in lifecycle-state, inference-policy, platform-install require live E2E validation (cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume). Unit/mocked tests cover logic but cannot validate cross-boundary invariants (gateway probing, network policy, clean-host install).
  • PRA-T3 Runtime validation — Run the `network-policy` E2E job for Inference selection, reachability, and network policy must agree at the real host-to-sandbox boundary. Matched files: `src/lib/onboard/machine/handlers/provider-inference.ts`, `src/lib/onboard/setup-inference.ts`.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Deterministic regression risks in lifecycle-state, inference-policy, platform-install require live E2E validation (cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume). Unit/mocked tests cover logic but cannot validate cross-boundary invariants (gateway probing, network policy, clean-host install).
  • PRA-T4 Runtime validation — Run the `onboard-repair` E2E job for Onboarding and sandbox state must converge across persisted metadata, reported status, and the live runtime. Matched files: `src/lib/onboard.ts`, `src/lib/onboard/machine/handlers/provider-inference.ts`, `src/lib/onboard/sandbox-lifecycle.ts`, `src/lib/onboard/setup-inference.ts`, `src/lib/state/registry.ts`.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Deterministic regression risks in lifecycle-state, inference-policy, platform-install require live E2E validation (cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume). Unit/mocked tests cover logic but cannot validate cross-boundary invariants (gateway probing, network policy, clean-host install).
  • PRA-T5 Runtime validation — Run the `onboard-resume` E2E job for Onboarding and sandbox state must converge across persisted metadata, reported status, and the live runtime. Matched files: `src/lib/onboard.ts`, `src/lib/onboard/machine/handlers/provider-inference.ts`, `src/lib/onboard/sandbox-lifecycle.ts`, `src/lib/onboard/setup-inference.ts`, `src/lib/state/registry.ts`.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Deterministic regression risks in lifecycle-state, inference-policy, platform-install require live E2E validation (cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume). Unit/mocked tests cover logic but cannot validate cross-boundary invariants (gateway probing, network policy, clean-host install).
  • PRA-T6 Acceptance clause — ownership sticky to original session on retarget by different session (implied) — add test evidence or identify existing coverage. registry-route-reservation.test.ts has 'transfers reservation ownership when a new session retargets the route' but no negative test preserving original owner; F-003 tracks missing test
  • PRA-T7 test/onboard-reservation-recreate.test.ts:125 silent catch handler — Convert to unit test per F-002; unit test failures are naturally caught by vitest. test/onboard-reservation-recreate.test.ts:125 }).catch((error) => { console.error(error); process.exit(1); })
  • PRA-T8 registry.ts:511 reservationSessionId overwrite on retarget — Add test: 'preserves original reservationSessionId when retargeted by different session' in registry-route-reservation.test.ts. registry.ts:511 reservationSessionId: route.reservationSessionId ?? existing?.reservationSessionId
Since last review details

Current findings, using the urgency labels above:

PRA-1 Required — Test monolith growth exceeds guardrail threshold

  • Location: src/lib/onboard/machine/handlers/provider-inference.test.ts:1218
  • Category: architecture
  • Problem: provider-inference.test.ts grew from 1177 to 1218 lines (+41) due to new reservationSessionId assertions across test cases. The file is now a 1200+ line monolith; extract or offset growth before merge per project guardrail.
  • Impact: Continued accretion in this test file impairs readability, slows test execution, and makes future changes harder to review. The growth is legitimate (new reservationSessionId plumbing) but unoffset.
  • Required action: Extract shared test fixtures/helpers (e.g., createDeps variants, session builders, common assertions) into a separate support module, or split the describe blocks into focused test files by behavior (fresh, resume, rebuild, routed, compatible-endpoint).
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: wc -l src/lib/onboard/machine/handlers/provider-inference.test.ts shows 1218 lines; grep -c 'reservationSessionId' shows new assertions.
  • Missing regression test: No regression test needed — this is a structural quality gate. The existing test suite passes; the finding is about maintainability.
  • Done when: The required change is committed and verification passes: wc -l src/lib/onboard/machine/handlers/provider-inference.test.ts shows 1218 lines; grep -c 'reservationSessionId' shows new assertions.
  • Evidence: monolithDeltas: baseLines=1177, headLines=1218, delta=41, severity=blocker diff shows +55 lines in provider-inference.test.ts with reservationSessionId additions across 14+ test cases

PRA-2 Improvement — E2E test uses heavy subprocess mocking where unit test pattern suffices

  • Location: test/onboard-reservation-recreate.test.ts:107
  • Category: architecture
  • Problem: The new test spawns a Node subprocess with 70+ lines of mocks for registry, runner, childProcess, onboardSession, and preflight to exercise the recreate path. The same behavior (reservation survives recreate) could be tested with direct Vitest unit mocks like registry-route-reservation.test.ts does.
  • Impact: Heavy subprocess mocking increases maintenance burden, test execution time, and flakiness risk. The test only validates one integration point (registry guard during recreate) which unit mocks can cover.
  • Suggested action: Refactor to a direct Vitest unit test: import registry, onboard/sandbox-lifecycle, and onboard-session modules directly; mock only the gateway calls (runOpenshell) and filesystem. Keep subprocess form only if it catches integration gaps unit tests miss.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Compare test/onboard-reservation-recreate.test.ts (170 lines, spawnSync) vs src/lib/state/registry-route-reservation.test.ts (200 lines, direct imports + vi.fn pattern).
  • Missing regression test: No regression test needed — this is a test architecture improvement. Existing test passes and covers the behavior.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/onboard-reservation-recreate.test.ts:26-107 heavy mock scaffolding with spawnSync src/lib/state/registry-route-reservation.test.ts:8-95 direct import + vi.fn pattern

PRA-3 Resolve/justify — Reservation ownership not sticky to original session on retarget

  • Location: src/lib/state/registry.ts:511
  • Category: correctness
  • Problem: reserveSandboxInferenceRoute uses `reservationSessionId: route.reservationSessionId ?? existing?.reservationSessionId` which overwrites the original owner's sessionId when a caller explicitly passes a different sessionId. The acceptance criteria and fix intent imply ownership should be sticky to the original reserving session — a retarget by a different session should preserve the original owner.
  • Impact: If a retry or race retargets the reservation with a different sessionId, the original session loses ownership and fails to resume, reintroducing the 'reservation disappeared' bug that nemoclaw onboard --resume fails with route reservation disappeared after interrupted onboard #6562/fix(onboard): keep pending route reservation through not-ready recreate #6626 fixed.
  • Recommended action: Change the logic to preserve existing.reservationSessionId when entry.pendingRouteReservation === true, only stamping a new sessionId on fresh reservations. Or add a guard: if (existing?.pendingRouteReservation && existing.reservationSessionId) keep existing.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check registry.ts:504-520 reserveSandboxInferenceRoute — the nullish coalescing allows overwrite. No test covers retarget with different sessionId preserving original.
  • Missing regression test: Add test: 'preserves original reservationSessionId when retargeted by different session' in registry-route-reservation.test.ts
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check registry.ts:504-520 reserveSandboxInferenceRoute — the nullish coalescing allows overwrite. No test covers retarget with different sessionId preserving original.
  • Evidence: registry.ts:511 reservationSessionId: route.reservationSessionId ?? existing?.reservationSessionId registry-route-reservation.test.ts has 'transfers reservation ownership when a new session retargets' but not the negative case Issue nemoclaw onboard --resume fails with route reservation disappeared after interrupted onboard #6562: 'preserves a current-session pending route reservation across a not-ready recreate' implies ownership stickiness

PRA-4 Resolve/justify — Missing unit test: removeSandboxUnlessSessionReservation handles null entry

  • Location: src/lib/onboard/sandbox-lifecycle.ts:15
  • Category: correctness
  • Problem: removeSandboxUnlessSessionReservation is called from onboard.ts with previousEntry that may be null (no registry row). The function uses optional chaining (entry?.pendingRouteReservation) to handle null gracefully, but no test verifies this null-entry path.
  • Impact: If a recreate occurs for a sandbox with no registry entry, the null path must correctly fall through to registry.removeSandbox. Without a test, a future refactor could break this fallback.
  • Recommended action: Add a test case in sandbox-lifecycle.test.ts calling removeSandboxUnlessSessionReservation(null, 'sandbox-name') and verifying registry.removeSandbox is called.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search sandbox-lifecycle.test.ts for a test covering the null entry path; currently absent.
  • Missing regression test: Add test: 'removeSandboxUnlessSessionReservation with null entry calls removeSandbox' in sandbox-lifecycle.test.ts
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search sandbox-lifecycle.test.ts for a test covering the null entry path; currently absent.
  • Evidence: src/lib/onboard/sandbox-lifecycle.ts:15 removeSandboxUnlessSessionReservation(entry, sandboxName) uses entry?.pendingRouteReservation src/lib/onboard.ts:2645 calls with previousEntry which may be null sandbox-lifecycle.test.ts has no test for null entry

PRA-5 Improvement — Test scaffold uses silent error handler masking failures

  • Location: test/onboard-reservation-recreate.test.ts:125
  • Category: architecture
  • Problem: The E2E test uses `}).catch((error) => { console.error(error); process.exit(1); })` which catches all errors but provides no test assertion — failures are logged and exit non-zero without vitest awareness. This masks the actual failure mode and prevents proper test reporting.
  • Impact: Test failures in the subprocess are not visible to vitest as test failures; they appear as process exits. Debugging requires parsing stdout/stderr manually.
  • Suggested action: Either let the promise reject naturally (vitest will catch it) or use a proper test assertion pattern. Better: convert to unit test per F-002.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check test/onboard-reservation-recreate.test.ts:125 — the catch handler swallows the error context.
  • Missing regression test: No regression test needed — this is a test scaffold quality issue. Converting to unit test (F-002) would resolve.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/onboard-reservation-recreate.test.ts:125 })().catch((error) => { console.error(error); process.exit(1); }) No try/catch with expect().rejects or similar vitest pattern

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Offset repeated reservation-session assertions in provider inference test monolith.
Open items: 0 required · 1 warning · 1 suggestion · 8 test follow-ups
Since last review: 1 prior item resolved · 0 still apply · 2 new items found

Action checklist

  • PRA-1 Resolve or justify: Offset repeated reservation-session assertions in provider inference test monolith in src/lib/onboard/machine/handlers/provider-inference.test.ts:31
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Runtime validation
  • PRA-T6 Add or justify test follow-up: Acceptance clause
  • PRA-T7 Add or justify test follow-up: Acceptance clause
  • PRA-T8 Add or justify test follow-up: Acceptance clause
  • PRA-2 In-scope improvement: Clean up temp directory in onboard reservation recreate test in test/onboard-reservation-recreate.test.ts:38

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture src/lib/onboard/machine/handlers/provider-inference.test.ts:31 Extract a small local helper/expectation builder for the repeated session ownership expectation, or otherwise offset the file growth, while keeping explicit coverage that `reservationSessionId` reaches setupInference/reserveRoute paths.
PRA-2 Improvement correctness test/onboard-reservation-recreate.test.ts:38 Wrap the body after `tmpDir` creation in `try/finally` and call `fs.rmSync(tmpDir, { recursive: true, force: true })` (or async equivalent) in the `finally`.
Review findings by urgency: 0 required fixes, 1 item to resolve/justify, 1 in-scope improvement

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Offset repeated reservation-session assertions in provider inference test monolith

  • Location: src/lib/onboard/machine/handlers/provider-inference.test.ts:31
  • Category: architecture
  • Problem: `provider-inference.test.ts` is already a large hotspot and this PR repeats the same session/`reservationSessionId` expectation pattern in many tests instead of using a narrow helper or shared expectation shape.
  • Impact: Continued monolith growth makes the provider inference test suite harder to maintain and increases the chance future route-reservation contract changes are updated inconsistently across repeated assertions.
  • Recommended action: Extract a small local helper/expectation builder for the repeated session ownership expectation, or otherwise offset the file growth, while keeping explicit coverage that `reservationSessionId` reaches setupInference/reserveRoute paths.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/onboard/machine/handlers/provider-inference.test.ts` and count repeated `reservationSessionId: session.sessionId` expectation blocks added by this PR.
  • Missing regression test: Existing changed tests assert the behavior; no additional regression test is required for this architecture finding. The improvement is to keep those assertions while extracting a small helper/expectation builder or otherwise offsetting growth.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/onboard/machine/handlers/provider-inference.test.ts` and count repeated `reservationSessionId: session.sessionId` expectation blocks added by this PR.
  • Evidence: Scope/risk context reports `src/lib/onboard/machine/handlers/provider-inference.test.ts` grew from 1177 to 1218 lines (delta +41) with monolith severity blocker. Diff shows repeated additions of `const session = createSession(); calls.complete.mockResolvedValue(session);` and repeated `reservationSessionId: session.sessionId` fields across setupInference expectations in `src/lib/onboard/machine/handlers/provider-inference.test.ts`.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-2 Improvement — Clean up temp directory in onboard reservation recreate test

  • Location: test/onboard-reservation-recreate.test.ts:38
  • Category: correctness
  • Problem: `test/onboard-reservation-recreate.test.ts` leaks the per-case temporary directory because it creates `tmpDir` and writes a fake OpenShell/script into it without a cleanup `finally`.
  • Impact: Repeated runs can leave `nemoclaw-onboard-reservation-survives-*` directories and generated scripts in the system temp directory, making local/CI environments noisier and potentially affecting disk usage over time.
  • Suggested action: Wrap the body after `tmpDir` creation in `try/finally` and call `fs.rmSync(tmpDir, { recursive: true, force: true })` (or async equivalent) in the `finally`.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read `test/onboard-reservation-recreate.test.ts` and confirm `tmpDir` from `fs.mkdtempSync` is removed on both success and assertion failure.
  • Missing regression test: No behavioral regression test is needed; the existing test should wrap its setup/spawn/assertion body in `try/finally` and clean `tmpDir`, matching the cleanup pattern used by `src/lib/state/registry-route-reservation.test.ts`.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: `test/onboard-reservation-recreate.test.ts:38-50` creates `tmpDir`, `fakeBin`, and `scriptPath` and writes files under that directory. `test/onboard-reservation-recreate.test.ts:134-166` performs spawn/assertions but has no `fs.rmSync(tmpDir, { recursive: true, force: true })` cleanup, unlike the registry tests that remove their temp HOME in `finally` blocks.
Simplification opportunities: 1 possible cut, net -20 lines possible

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-1 shrink (src/lib/onboard/machine/handlers/provider-inference.test.ts:31): Repeated per-test session mock and `reservationSessionId` expectation boilerplate in provider-inference.test.ts
    • Replacement: Use a local helper that creates a session, wires `calls.complete`, and returns an expectation fragment for `{ reservationSessionId: session.sessionId }`.
    • Net: -20 lines
    • Safety boundary: Do not abstract away which setupInference/reserveRoute path is under test or remove explicit trust-boundary route ownership assertions.
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Run the `cloud-onboard` E2E job for Installer and platform changes must work on a clean supported host with the pinned runtime dependencies. Matched files: `src/lib/onboard/machine/handlers/provider-inference.ts`.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Checked-in tests cover the changed ownership contract with unit/helper, setup-inference, and subprocess caller-level regressions. The risk plan still includes lifecycle-state, inference-policy, and platform-install invariants that require live host/sandbox validation as a floor.
  • PRA-T2 Runtime validation — Run the `cloud-onboard` E2E job for installer/platform validation on a clean supported host because `provider-inference.ts` is in the platform-install risk family.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Checked-in tests cover the changed ownership contract with unit/helper, setup-inference, and subprocess caller-level regressions. The risk plan still includes lifecycle-state, inference-policy, and platform-install invariants that require live host/sandbox validation as a floor.
  • PRA-T3 Runtime validation — Run the `inference-routing` E2E job for Inference selection, reachability, and network policy must agree at the real host-to-sandbox boundary. Matched files: `src/lib/onboard/machine/handlers/provider-inference.ts`, `src/lib/onboard/setup-inference.ts`.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Checked-in tests cover the changed ownership contract with unit/helper, setup-inference, and subprocess caller-level regressions. The risk plan still includes lifecycle-state, inference-policy, and platform-install invariants that require live host/sandbox validation as a floor.
  • PRA-T4 Runtime validation — Run the `inference-routing` E2E job to verify the selected provider is reachable through the route advertised to the agent at the real host-to-sandbox boundary.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Checked-in tests cover the changed ownership contract with unit/helper, setup-inference, and subprocess caller-level regressions. The risk plan still includes lifecycle-state, inference-policy, and platform-install invariants that require live host/sandbox validation as a floor.
  • PRA-T5 Runtime validation — Run the `network-policy` E2E job for Inference selection, reachability, and network policy must agree at the real host-to-sandbox boundary. Matched files: `src/lib/onboard/machine/handlers/provider-inference.ts`, `src/lib/onboard/setup-inference.ts`.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Checked-in tests cover the changed ownership contract with unit/helper, setup-inference, and subprocess caller-level regressions. The risk plan still includes lifecycle-state, inference-policy, and platform-install invariants that require live host/sandbox validation as a floor.
  • PRA-T6 Acceptance clause — Platform scope: Reproduced on ubuntu22, wsl-x86, ubuntu24-gpu, dgx-station, macos (5 of 8 CI runners). Passed on ubuntu24, dgxspark, ubuntu26-gpu (non-interrupted path). — add test evidence or identify existing coverage. Checked-in tests cover the deterministic ownership behavior with mocks/subprocesses, but this review does not claim live platform E2E jobs ran on those platforms.
  • PRA-T7 Acceptance clause — Regression: Unknown — earlier versions not tested with this interrupt/resume flow. — add test evidence or identify existing coverage. The PR adds current regression tests for the fixed behavior, but this review did not compare earlier released versions.
  • PRA-T8 Acceptance clause — 1. A first onboard run fails **during sandbox image creation** (I used a custom Dockerfile whose build fails after validation). With fix(onboard): preserve pending route reservation across resume #6572 the registry row now survives the failed run: `PRESENT pending=true`. So far so good — fix(onboard): preserve pending route reservation across resume #6572 does its job. — add test evidence or identify existing coverage. The PR does not re-test the exact custom-Dockerfile failure chain, but it preserves and stamps pending route reservations before sandbox creation in the changed setup path.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Offset repeated reservation-session assertions in provider inference test monolith

  • Location: src/lib/onboard/machine/handlers/provider-inference.test.ts:31
  • Category: architecture
  • Problem: `provider-inference.test.ts` is already a large hotspot and this PR repeats the same session/`reservationSessionId` expectation pattern in many tests instead of using a narrow helper or shared expectation shape.
  • Impact: Continued monolith growth makes the provider inference test suite harder to maintain and increases the chance future route-reservation contract changes are updated inconsistently across repeated assertions.
  • Recommended action: Extract a small local helper/expectation builder for the repeated session ownership expectation, or otherwise offset the file growth, while keeping explicit coverage that `reservationSessionId` reaches setupInference/reserveRoute paths.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/onboard/machine/handlers/provider-inference.test.ts` and count repeated `reservationSessionId: session.sessionId` expectation blocks added by this PR.
  • Missing regression test: Existing changed tests assert the behavior; no additional regression test is required for this architecture finding. The improvement is to keep those assertions while extracting a small helper/expectation builder or otherwise offsetting growth.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/onboard/machine/handlers/provider-inference.test.ts` and count repeated `reservationSessionId: session.sessionId` expectation blocks added by this PR.
  • Evidence: Scope/risk context reports `src/lib/onboard/machine/handlers/provider-inference.test.ts` grew from 1177 to 1218 lines (delta +41) with monolith severity blocker. Diff shows repeated additions of `const session = createSession(); calls.complete.mockResolvedValue(session);` and repeated `reservationSessionId: session.sessionId` fields across setupInference expectations in `src/lib/onboard/machine/handlers/provider-inference.test.ts`.

PRA-2 Improvement — Clean up temp directory in onboard reservation recreate test

  • Location: test/onboard-reservation-recreate.test.ts:38
  • Category: correctness
  • Problem: `test/onboard-reservation-recreate.test.ts` leaks the per-case temporary directory because it creates `tmpDir` and writes a fake OpenShell/script into it without a cleanup `finally`.
  • Impact: Repeated runs can leave `nemoclaw-onboard-reservation-survives-*` directories and generated scripts in the system temp directory, making local/CI environments noisier and potentially affecting disk usage over time.
  • Suggested action: Wrap the body after `tmpDir` creation in `try/finally` and call `fs.rmSync(tmpDir, { recursive: true, force: true })` (or async equivalent) in the `finally`.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read `test/onboard-reservation-recreate.test.ts` and confirm `tmpDir` from `fs.mkdtempSync` is removed on both success and assertion failure.
  • Missing regression test: No behavioral regression test is needed; the existing test should wrap its setup/spawn/assertion body in `try/finally` and clean `tmpDir`, matching the cleanup pattern used by `src/lib/state/registry-route-reservation.test.ts`.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: `test/onboard-reservation-recreate.test.ts:38-50` creates `tmpDir`, `fakeBin`, and `scriptPath` and writes files under that directory. `test/onboard-reservation-recreate.test.ts:134-166` performs spawn/assertions but has no `fs.rmSync(tmpDir, { recursive: true, force: true })` cleanup, unlike the registry tests that remove their temp HOME in `finally` blocks.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

… entrypoint

Move the pending-reservation removal guard out of the onboard.ts
entrypoint into sandbox-lifecycle.removeSandboxUnlessSessionReservation
so the codebase-growth guardrail stays satisfied, and assert the new
reservationSessionId stamp in the provider-inference route reservation
tests.

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/onboard/machine/handlers/provider-inference.test.ts`:
- Line 386: Update the affected tests in the provider-inference scenarios to
capture the active session ID used by each scenario and assert that exact value
in the reserveRoute payload, replacing expect.any(String) at both assertions.
Use the existing session setup or fixture symbol that represents the active
session so stale or mismatched IDs fail the test.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e124bae7-71a3-40be-8cd2-54b1a240cb03

📥 Commits

Reviewing files that changed from the base of the PR and between 4e5c7b7 and f8798f2.

📒 Files selected for processing (3)
  • src/lib/onboard.ts
  • src/lib/onboard/machine/handlers/provider-inference.test.ts
  • src/lib/onboard/sandbox-lifecycle.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/onboard.ts

Comment thread src/lib/onboard/machine/handlers/provider-inference.test.ts Outdated
@laitingsheng laitingsheng added NV QA Bugs found by the NVIDIA QA Team bug-fix PR fixes a bug or regression area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery labels Jul 10, 2026
…tion

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/onboard/machine/handlers/provider-inference.test.ts`:
- Line 60: Replace the remaining weak expect.any(String) assertions for
reservationSessionId in the provider inference tests with assertions against the
concrete expected session ID, following the already-fixed rebuiltSession
scenarios around the calls.complete mocks. Update every listed scenario so the
expected value is the correct session ID returned or threaded through that test,
preserving the ownership contract.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8855dcb2-2b1d-4691-bfef-e8993d6171ac

📥 Commits

Reviewing files that changed from the base of the PR and between f8798f2 and 0642e00.

📒 Files selected for processing (4)
  • src/lib/onboard/machine/handlers/provider-inference.test.ts
  • src/lib/onboard/machine/handlers/provider-inference.ts
  • src/lib/onboard/setup-inference-route-containment.test.ts
  • src/lib/onboard/setup-inference.ts

Comment thread src/lib/onboard/machine/handlers/provider-inference.test.ts Outdated
cv
cv previously requested changes Jul 10, 2026

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed exact head 0642e00dd. The session-owned reservation design is sound and supplies the primitives #6634 needs, but this head is not merge-ready:

  1. Fix the source-related shard failure. Run 29086449236 shard 3 fails src/lib/onboard/machine/core-flow-phases.test.ts:355 because the expected setupInference options omit the newly correct reservationSessionId. Update the assertion with the exact session ID. (Shard 4's MCP status timeout is unrelated and can be rerun after the source fix.)

  2. Pin ownership behavior precisely. Replace the remaining expect.any(String) session-ID assertions with the concrete expected ID. Add caller-level negative coverage proving foreign-session and unstamped pending reservations are removed during recreate, plus a positive ownership-transfer test when a new session successfully retargets the reservation.

  3. Sequence #6634 explicitly. Landing these helpers alone is insufficient: #6634 must later accept either a fully registered row or a pending row owned by the active session, rather than Boolean(registry.getSandbox(name)).

After the test changes, sync current main, refresh the advisors, and run the required exact-head targets from E2E advisor run 29086449243: cloud-onboard, inference-routing, network-policy, onboard-repair, and onboard-resume.

No production security defect was found; this hold is correctness coverage, a real failing assertion, and exact-head evidence.

cjagwani added 2 commits July 10, 2026 10:03
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 29110826642
Workflow ref: fix/onboard-pending-reservation-survives-create
Requested targets: (default — all supported)
Requested jobs: cloud-onboard,inference-routing,network-policy,onboard-repair,onboard-resume
Summary: 5 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
cloud-onboard ✅ success
inference-routing ✅ success
network-policy ✅ success
onboard-repair ✅ success
onboard-resume ✅ success

@cv cv dismissed their stale review July 10, 2026 17:32

Superseded at exact head fd0249d: the stale assertion, concrete session IDs, foreign and unstamped reservation cleanup, ownership transfer, current-main sync, ordinary CI, and all five required exact-head live jobs are now addressed. Trusted advisor posture remains a separate gate.

@cv cv merged commit a3a2e4f into main Jul 10, 2026
136 of 140 checks passed
@cv cv deleted the fix/onboard-pending-reservation-survives-create branch July 10, 2026 17:40
cv pushed a commit that referenced this pull request Jul 11, 2026
## Summary

Release-prep documentation for **v0.0.80**. Adds the `## v0.0.80`
section to `docs/about/release-notes.mdx` summarizing user-facing
changes since v0.0.79, each bullet linking to the relevant deeper page.

Produced via `nemoclaw-contributor-update-docs` (pre-tag path): scanned
`v0.0.79..HEAD`, applied the docs skip list (no violations), and
confirmed the 8 commits that already shipped in-PR docs are complete. No
new pages needed.

## Source summary

- #6507 -> `docs/about/release-notes.mdx`: Hermes v0.18 + Slack Block
Kit (rich rendering, digest-pinned base image).
- #6584 / #6616 -> `docs/about/release-notes.mdx`: host-local OpenRouter
runtime attribution adapter (port `11437`,
`NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT`) and native Deep Agents
`openrouter` provider.
- #6210 / #6292 -> `docs/about/release-notes.mdx`: host corporate proxy
CA import into sandbox trust (`NEMOCLAW_CORPORATE_CA_BUNDLE`,
`NEMOCLAW_CORPORATE_CA_IMPORT`).
- #6624 / #6623 / #6656 -> `docs/about/release-notes.mdx`:
release-matched base-image selection, surfaced cluster-image build
diagnostics, preserved Nemotron profile registration.
- #6629 / #6637 -> `docs/about/release-notes.mdx`: bare `connect`
default-sandbox behavior and route-probe hardening.
- #6634 / #6626 / #6596 / #5569 / #6610 / #6655 ->
`docs/about/release-notes.mdx`: onboarding/recovery preservation,
stale-gateway-PID fix, installer backup message, vLLM label on managed
platforms.
- #6578 / #5670 -> `docs/about/release-notes.mdx`: automatic Hermes
light terminal skin and non-interactive `npx` MCP server startup.

## Verification

`npm run docs`: 0 errors, all internal links resolve (2 pre-existing
hidden-page warnings). `_build/` variants for OpenClaw, Hermes, and Deep
Agents all regenerate with the v0.0.80 section.

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>

🤖 Generated with [Claude Code](https://claude.com/claude-code)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Documentation**
  * Added release notes for v0.0.80.
  * Documented Hermes upgrades, including Slack Block Kit rendering.
  * Added details on OpenRouter traffic routing and attribution headers.
* Documented improved proxy certificate handling and sandbox
reliability.
* Highlighted enhanced connection defaults, route-probing safeguards,
onboarding recovery, and terminal/MCP startup behavior.
  * Added references to relevant user-guide documentation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
…te (NVIDIA#6626)

<!-- markdownlint-disable MD041 -->
## Summary

`onboard --resume` still failed with `sandbox route reservation '<name>'
disappeared while onboarding was in progress` after PR NVIDIA#6572, but only
when an interrupted first run left a partial sandbox live on the gateway
(reproduced on DGX Spark). This preserves the current onboard session's
pending route reservation through the not-ready recreate so the resumed
run can recover.

## Related Issue

Fixes NVIDIA#6562

Same root cause as NVIDIA#6563 (closed, co-fixed by NVIDIA#6572). Release-safe
groundwork for the NVIDIA#6228 checkpoint refactor and NVIDIA#6492 atomic
registry/gateway work; it does not close their acceptance criteria.

## Changes

- `state/registry.ts`: add `reservationSessionId` to the sandbox row,
stamp it in `reserveSandboxInferenceRoute`, and add
`isPendingReservationForSession` to distinguish the active session's
reservation from an abandoned one.
- `onboard/machine/handlers/provider-inference.ts`: stamp the
reservation with the current onboard `sessionId` (persisted, so it
survives `--resume`).
- `onboard.ts`: in the not-ready recreate path, skip the pre-rebuild
registry removal for the current session's pending reservation. PR NVIDIA#6572
guarded only the `!liveExists` tool-disclosure prune; this covers the
`liveExists` (partial-sandbox) path that reproduced on slower hosts.
Abandoned reservations from other sessions still prune, and the
not-ready gateway sandbox is still deleted before the rebuild.
- Tests: registry ownership/stamping coverage and a subprocess
fault-injection test asserting the reservation row survives a not-ready
recreate.

## Type of Change

- [x] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: internal onboarding lifecycle
repair; no user-facing behavior, CLI surface, or configuration change.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [ ] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result: `vitest run` —
`registry-route-reservation` 5/5, `onboard-reservation-recreate` 1/1,
full `onboard.test.ts` 66/66, and `tool-disclosure-flow` +
`sandbox-lifecycle` + `provider-inference-route-containment` 15/15.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only)
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

Note on hooks: `pre-commit` and `commit-msg` passed. `pre-push` (and
`npm run check:diff`) fail only on pre-existing environment gaps in the
local worktree — an unbuilt `dist/` (package-contract and e2e-live
imports) and a missing `typebox` dependency (`tools/pr-review-advisor`)
— neither of which touches this diff. `npm run typecheck:cli` reports
zero errors in the changed files; CI runs the authoritative
full-environment checks.

---
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Preserved pending sandbox route reservations during sandbox recreation
when owned by the active onboarding session.
* Prevented cleanup from removing a sandbox when its pending reservation
is reserved for the current session.
* Added session-aware pending reservation ownership tracking, including
support for stamping reservation ownership IDs onto reservations.

* **Tests**
* Added integration coverage for reservation-safe sandbox recreation
across multiple reservation ownership scenarios.
* Expanded unit/integration tests to verify reservation session stamping
and correct ownership transfer behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Co-authored-by: Charan Jagwani <cjagwani@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary

Release-prep documentation for **v0.0.80**. Adds the `## v0.0.80`
section to `docs/about/release-notes.mdx` summarizing user-facing
changes since v0.0.79, each bullet linking to the relevant deeper page.

Produced via `nemoclaw-contributor-update-docs` (pre-tag path): scanned
`v0.0.79..HEAD`, applied the docs skip list (no violations), and
confirmed the 8 commits that already shipped in-PR docs are complete. No
new pages needed.

## Source summary

- NVIDIA#6507 -> `docs/about/release-notes.mdx`: Hermes v0.18 + Slack Block
Kit (rich rendering, digest-pinned base image).
- NVIDIA#6584 / NVIDIA#6616 -> `docs/about/release-notes.mdx`: host-local OpenRouter
runtime attribution adapter (port `11437`,
`NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT`) and native Deep Agents
`openrouter` provider.
- NVIDIA#6210 / NVIDIA#6292 -> `docs/about/release-notes.mdx`: host corporate proxy
CA import into sandbox trust (`NEMOCLAW_CORPORATE_CA_BUNDLE`,
`NEMOCLAW_CORPORATE_CA_IMPORT`).
- NVIDIA#6624 / NVIDIA#6623 / NVIDIA#6656 -> `docs/about/release-notes.mdx`:
release-matched base-image selection, surfaced cluster-image build
diagnostics, preserved Nemotron profile registration.
- NVIDIA#6629 / NVIDIA#6637 -> `docs/about/release-notes.mdx`: bare `connect`
default-sandbox behavior and route-probe hardening.
- NVIDIA#6634 / NVIDIA#6626 / NVIDIA#6596 / NVIDIA#5569 / NVIDIA#6610 / NVIDIA#6655 ->
`docs/about/release-notes.mdx`: onboarding/recovery preservation,
stale-gateway-PID fix, installer backup message, vLLM label on managed
platforms.
- NVIDIA#6578 / NVIDIA#5670 -> `docs/about/release-notes.mdx`: automatic Hermes
light terminal skin and non-interactive `npx` MCP server startup.

## Verification

`npm run docs`: 0 errors, all internal links resolve (2 pre-existing
hidden-page warnings). `_build/` variants for OpenClaw, Hermes, and Deep
Agents all regenerate with the v0.0.80 section.

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>

🤖 Generated with [Claude Code](https://claude.com/claude-code)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Documentation**
  * Added release notes for v0.0.80.
  * Documented Hermes upgrades, including Slack Block Kit rendering.
  * Added details on OpenRouter traffic routing and attribution headers.
* Documented improved proxy certificate handling and sandbox
reliability.
* Highlighted enhanced connection defaults, route-probing safeguards,
onboarding recovery, and terminal/MCP startup behavior.
  * Added references to relevant user-guide documentation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cv pushed a commit that referenced this pull request Jul 13, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Resume onboarding can skip provider setup after the provider/model are
already recorded but before sandbox setup is complete. This change
reserves or repairs the sandbox inference route during that resume path
so the later sandbox profile step does not fail with `sandbox route
reservation '<name>' disappeared while onboarding was in progress`.

## Related Issue
Fixes #6562

## Changes
- Reserve the sandbox inference route when `nemoclaw onboard --resume`
skips already-ready inference while sandbox setup is still incomplete.
- Apply the repair to both routed and direct-compatible provider
branches while preserving complete-sandbox resume behavior.
- Add regression coverage for the reported
cancellation-after-sandbox-name resume path and the already-ready
inference resume path.

What each PR/fix does:

- #6572: protects an existing `pendingRouteReservation` from being
deleted by stale-entry cleanup when no live sandbox exists. It handles
"reservation existed, cleanup would remove it."
- #6626: protects an existing session-owned pending reservation during a
NotReady sandbox recreate. It handles "reservation existed, recreate
would remove it."
- This fix: creates/repairs the reservation when resume skips inference
before sandbox setup has a complete sandbox. It handles your repro:
"reservation was missing because resume skipped the step that normally
creates it."

## Type of Change

- [x] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: The public `--resume`
contract is unchanged; this repairs internal route reservation state. A
docs review found no documentation updates needed.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
  - `git push -u origin HEAD` ran pre-push TypeScript checks and passed.
- `npm run check:diff` passed in a temporary clean worktree after `npm
run build:cli` generated the required `dist/` artifacts. The main
checkout has ignored local `worktrees/` directories that otherwise make
the source-shape scanner inspect unrelated files.
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification:
- `npx vitest run --project cli
src/lib/onboard/machine/handlers/provider-inference.test.ts` — passed
(35 tests)
- `npx vitest run --project cli
src/lib/onboard/machine/handlers/provider-inference-recovery-gating.test.ts
src/lib/onboard/machine/handlers/provider-inference-route-containment.test.ts
src/lib/state/registry-route-reservation.test.ts
src/lib/onboard/setup-inference-route-containment.test.ts
src/lib/onboard/sandbox-lifecycle.test.ts` — passed (38 tests)
  - `npm run typecheck:cli` — passed
  - `npm run build:cli` — passed
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only)
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
Signed-off-by: San Dang <sdang@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved resume reliability during provider inference by correcting
which sandbox route is reserved, even when inference is already ready or
provider reselection is required.
* Ensured no sandbox route is reserved when resuming after the sandbox
step is already completed.
* Updated routed-inference reservation behavior to match the expected
host-alias endpoint details.
* **Tests**
* Added coverage to validate the resumed provider inference reservation
flow and updated related router reconciliation expectations.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: San Dang <sdang@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression NV QA Bugs found by the NVIDIA QA Team v0.0.80 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nemoclaw onboard --resume fails with route reservation disappeared after interrupted onboard

3 participants