Skip to content

fix(onboard): scope live inference recovery to sandbox#6634

Merged
cv merged 17 commits into
mainfrom
fix/6630_dcode_agent_default_model
Jul 10, 2026
Merged

fix(onboard): scope live inference recovery to sandbox#6634
cv merged 17 commits into
mainfrom
fix/6630_dcode_agent_default_model

Conversation

@chengjiew

@chengjiew chengjiew commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Prevent a newly named sandbox from inheriting a stale inference provider and model left on an otherwise unowned OpenShell gateway. Live gateway recovery now requires matching registry or onboard-session identity, so Deep Agents Code can apply its Ultra agent default.

Related Issue

Fixes #6630

Changes

  • Gate live gateway provider/model recovery on a matching sandbox registry row or onboard session.
  • Add a regression test for an empty registry with a stale Super route and a brand-new dcode sandbox name.
  • Preserve and test live-route recovery for a real rebuild with a matching session identity.

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: This restores the documented dcode agent default without changing commands, flags, or user-facing configuration.
  • 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: Focused recovery-boundary review confirms live state is accepted only after durable sandbox identity is established; positive rebuild coverage protects the intended recovery path.
  • 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 or justification: npx vitest run src/lib/onboard/provider-recovery.test.ts src/lib/onboard/setup-nim-flow.test.ts src/lib/onboard/machine/handlers/provider-inference.test.ts --project cli — 61 passed.
  • 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)

Signed-off-by: Chengjie Wang chengjiew@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Tightened onboarding recorded-provider recovery with sandbox recovery authority checks, preventing recovery when authorization is missing/unauthorized or when session identity is incomplete or mismatched.
    • Refined pending route handling during recovery so pending-route readers only succeed for the owning current session; otherwise they return no result.
    • Fail-closed behavior: when recovery authority can’t be confirmed or ownership is lost during setup, the flow stops and emits a warning/error.
  • Tests
    • Added/expanded automated coverage for recovery gating, pending-route ownership, and retry/setup parameter correctness.

Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
@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 fcc121d 50efbbe +/-
src/lib/inferen...del-registry.ts 100% 77% -23%
src/lib/actions...ge-preflight.ts 94% 74% -20%
src/lib/actions...de-preflight.ts 61% 51% -10%
src/lib/inference/local.ts 74% 69% -5%
src/lib/inferen...board-probes.ts 86% 84% -2%
src/lib/credentials/store.ts 61% 59% -2%
src/lib/runner.ts 74% 83% +9%
src/lib/onboard...der-recovery.ts 80% 94% +14%
src/lib/onboard...er-discovery.ts 0% 94% +94%
src/lib/onboard...nce-recovery.ts 0% 100% +100%

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

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Recorded provider recovery now uses registry sandbox authority and caller session identity. Unauthorized recorded state and pending routes are rejected, while authorized sessions can recover persisted provider data. Onboarding propagates this context through provider inference, setupNim, and route setup.

Changes

Provider recovery

Layer / File(s) Summary
Define recovery authority
src/lib/onboard/provider-recovery.ts, src/lib/onboard/provider-recovery.test.ts
Classifies sandbox recovery authority, gates recorded-state readers, fails closed on registry errors, and tests reservation ownership.
Propagate recovery-session context
src/lib/onboard.ts, src/lib/onboard/setup-nim-flow.ts
Threads the caller’s recovery session identity through recorded provider, model, container, endpoint, and route lookups.
Gate provider inference and route setup
src/lib/onboard/machine/handlers/provider-inference.ts, src/lib/onboard/setup-inference.ts
Computes recovery eligibility, carries recovery state into inference setup, and aborts route mutation when ownership is lost.
Validate recovery behavior
src/lib/onboard/machine/handlers/*test*, src/lib/onboard/*test*
Covers fresh and resumed selection, pending reservations, persistence, retries, route containment, and updated dependency contracts.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Onboard
  participant ProviderInference
  participant ProviderRecovery
  participant SetupNim
  participant SetupInference
  Onboard->>ProviderInference: Pass recovery session identity
  ProviderInference->>ProviderRecovery: Get sandbox recovery authority
  ProviderRecovery-->>ProviderInference: Return recovery authority
  ProviderInference->>SetupNim: Pass recovery decision and session identity
  SetupNim->>SetupInference: Pass ownership recheck callback
  SetupInference->>SetupInference: Abort when reservation ownership is lost
Loading

Possibly related PRs

  • NVIDIA/NemoClaw#5173: Both changes modify the recorded-provider recovery and provider-selection path.

Suggested labels: bug-fix, area: onboarding, area: sandbox

Suggested reviewers: cv, laitingsheng, sandl99

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 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 change: limiting onboard recovery to the intended sandbox.
Linked Issues check ✅ Passed The code changes address #6630 by preventing a new sandbox from inheriting unrelated recorded model state and gating recovery by sandbox/session ownership.
Out of Scope Changes check ✅ Passed The changes are focused on sandbox-scoped recovery logic and matching tests, with no clear unrelated additions.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/6630_dcode_agent_default_model

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

@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: None

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 for platform-install coverage: provider inference machine changes can affect a clean hosted onboarding flow reaching a usable agent.
  • inference-routing (medium): Required by the deterministic risk plan for inference-policy coverage: changed provider recovery and setup paths can affect selected provider reachability through the route advertised to the agent.
  • network-policy (high): Required by the deterministic risk plan for inference-policy coverage: route recovery and inference setup changes must still permit intended inference traffic and deny unintended egress at the real boundary.
  • onboard-repair (high): Required by the deterministic risk plan and the onboarding resume rule: the PR changes live onboard machine provider recovery, session/reservation ownership, and repair-sensitive state transitions that must be validated by real repair onboarding.
  • onboard-resume (medium): Required by the deterministic risk plan and the onboarding resume rule: changes to provider recovery authorization, resume session IDs, and setupNim/setupInference handoff directly affect resumed onboarding and cannot be covered by unit tests alone.

Optional E2E

  • None.

New E2E recommendations

  • lifecycle-state (high): This PR adds explicit ownership gating for recovered recorded provider routes and pending route reservations. Existing onboard-resume/onboard-repair jobs cover resume and repair broadly, but a dedicated live scenario that seeds an owned versus foreign pending route reservation before resume would provide stronger regression coverage for this new security/state boundary.
    • Suggested test: Add a live E2E target that creates or seeds a sandbox route reservation with a reservationSessionId, verifies resume can recover only the current session's reservation, and verifies a foreign or ownerless reservation forces fresh provider setup instead of reusing the recorded route.

Dispatch hint

  • Workflow: .github/workflows/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/core-flow-phases.test.ts
  • src/lib/onboard/machine/handlers/provider-inference-recovery-gating.test.ts
  • src/lib/onboard/machine/handlers/provider-inference-recovery.ts
  • src/lib/onboard/machine/handlers/provider-inference-route-containment.test.ts
  • src/lib/onboard/machine/handlers/provider-inference.test-support.ts
  • src/lib/onboard/machine/handlers/provider-inference.test.ts
  • src/lib/onboard/machine/handlers/provider-inference.ts
  • src/lib/onboard/provider-recovery.test.ts
  • src/lib/onboard/provider-recovery.ts
  • src/lib/onboard/setup-inference-route-containment.test.ts
  • src/lib/onboard/setup-inference.ts
  • src/lib/onboard/setup-nim-flow.test.ts
  • src/lib/onboard/setup-nim-flow.ts
  • src/lib/onboard/setup-nim-provider-discovery.ts
  • test/onboard-resume-provider-recovery.test.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 file monolith growth exceeds guardrail; then add or justify PRA-T1.
Open items: 1 required · 3 warnings · 0 suggestions · 7 test follow-ups
Since last review: 0 prior items resolved · 0 still apply · 4 new items found

Action checklist

  • PRA-1 Fix: Test file monolith growth exceeds guardrail in src/lib/onboard/provider-recovery.test.ts:1
  • PRA-2 Resolve or justify: Wide trust-boundary propagation of recoverySessionId in src/lib/onboard/machine/handlers/provider-inference.ts:266
  • PRA-3 Resolve or justify: Missing concurrent recovery race regression test in src/lib/onboard/machine/handlers/provider-inference-recovery-gating.test.ts:1
  • PRA-4 Resolve or justify: Missing registry mutation during recovery regression test in src/lib/onboard/machine/handlers/provider-inference-recovery-gating.test.ts:1
  • 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: Missing concurrent recovery race regression test
  • PRA-T7 Add or justify test follow-up: Missing registry mutation during recovery regression test

Findings index

ID Severity Category Location Required action
PRA-1 Required architecture src/lib/onboard/provider-recovery.test.ts:1 Extract recovery authority tests, provider recovery helpers tests, and recorded route readers tests into separate focused test files (e.g., provider-recovery-authority.test.ts, provider-recovery-helpers.test.ts, recorded-route-readers.test.ts).
PRA-2 Resolve/justify architecture src/lib/onboard/machine/handlers/provider-inference.ts:266 Consider consolidating the authority recheck into a single shared helper. Ensure provider-inference-recovery.ts is the canonical source for recovery decisions and setup-inference.ts only calls the provided isRecordedProviderRecoveryAuthorized callback.
PRA-3 Resolve/justify tests src/lib/onboard/machine/handlers/provider-inference-recovery-gating.test.ts:1 Add a test that spawns two concurrent handleProviderInferenceState calls with different sessionIds for the same sandboxName — one with reservationSessionId matching, one without — and verify only the owner succeeds.
PRA-4 Resolve/justify tests src/lib/onboard/machine/handlers/provider-inference-recovery-gating.test.ts:1 Add a test where shouldRecover() returns true (no pending reservation), then a mock registry adds a foreign pendingRouteReservation before mutateGatewayRoute runs, and verify isRecordedProviderRecoveryAuthorized() returns false and setup aborts.

🚨 Required before merge

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

PRA-1 Required — Test file monolith growth exceeds guardrail

  • Location: src/lib/onboard/provider-recovery.test.ts:1
  • Category: architecture
  • Problem: provider-recovery.test.ts grew from 171 to 429 lines (+258), exceeding the 20-line growth threshold. The file is now a monolith that should be split or offset before merge.
  • Impact: Large test files are harder to maintain, review, and keep deterministic. Growth guardrail exists to prevent unbounded test accumulation.
  • Required action: Extract recovery authority tests, provider recovery helpers tests, and recorded route readers tests into separate focused test files (e.g., provider-recovery-authority.test.ts, provider-recovery-helpers.test.ts, recorded-route-readers.test.ts).
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check line count: wc -l src/lib/onboard/provider-recovery.test.ts. Verify new extracted files exist and original is reduced.
  • Missing regression test: No regression test needed — this is a structural guardrail violation. The fix is refactoring, not behavior change.
  • Done when: The required change is committed and verification passes: Check line count: wc -l src/lib/onboard/provider-recovery.test.ts. Verify new extracted files exist and original is reduced.
  • Evidence: diff stat shows +258 lines for provider-recovery.test.ts riskPlan.monolithDeltas[0] severity=blocker with delta=258
Review findings by urgency: 1 required fix, 3 items to resolve/justify, 0 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-2 Resolve/justify — Wide trust-boundary propagation of recoverySessionId

  • Location: src/lib/onboard/machine/handlers/provider-inference.ts:266
  • Category: architecture
  • Problem: recoverySessionId (caller session.sessionId) now threads through provider-recovery.ts helpers, setup-nim-provider-discovery.ts, setup-inference.ts, provider-inference.ts, and provider-inference-recovery.ts — 16 files total. While internally generated, the propagation surface increases coupling.
  • Impact: Future changes to session ID generation or recovery logic must touch many files. The recovery authority check is duplicated between provider-inference-recovery.ts (createRecovery.shouldRecover) and setup-inference.ts (mutateGatewayRoute callback).
  • Recommended action: Consider consolidating the authority recheck into a single shared helper. Ensure provider-inference-recovery.ts is the canonical source for recovery decisions and setup-inference.ts only calls the provided isRecordedProviderRecoveryAuthorized callback.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search for 'recoverySessionId' across changed files. Verify setup-inference.ts mutateGatewayRoute calls options.isRecordedProviderRecoveryAuthorized() rather than reimplementing authority logic.
  • Missing regression test: Add a test that verifies authority recheck is called exactly once inside both mutation locks (sandbox + gateway) and not duplicated elsewhere.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search for 'recoverySessionId' across changed files. Verify setup-inference.ts mutateGatewayRoute calls options.isRecordedProviderRecoveryAuthorized() rather than reimplementing authority logic.
  • Evidence: provider-inference-recovery.ts:27-44 createRecovery returns setupOptions with isRecordedProviderRecoveryAuthorized callback setup-inference.ts:170-177 mutateGatewayRoute calls options.isRecordedProviderRecoveryAuthorized() provider-inference.ts:266-273 createRecovery invoked, setupOptions passed to inferenceOptions

PRA-3 Resolve/justify — Missing concurrent recovery race regression test

  • Location: src/lib/onboard/machine/handlers/provider-inference-recovery-gating.test.ts:1
  • Category: tests
  • Problem: Two sessions attempting recovery for the same sandbox concurrently (one owns the pendingRouteReservation, one doesn't) is not covered. The TOCTOU recheck inside mutation locks should prevent credential leakage, but no test verifies this under actual concurrency.
  • Impact: A race could allow unauthorized recovery if the authority check passes but the registry row changes before the mutation lock is acquired.
  • Recommended action: Add a test that spawns two concurrent handleProviderInferenceState calls with different sessionIds for the same sandboxName — one with reservationSessionId matching, one without — and verify only the owner succeeds.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search for Promise.all or concurrent test patterns in provider-inference-recovery-gating.test.ts. Verify test fails without the mutex recheck.
  • Missing regression test: Concurrent recovery race test: two sessions, same sandbox, different sessionIds — only owner should recover credentials/route.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search for Promise.all or concurrent test patterns in provider-inference-recovery-gating.test.ts. Verify test fails without the mutex recheck.
  • Evidence: setup-inference-route-containment.test.ts:75-130 tests recheck order but single-threaded riskPlan.lifecycle-state invariant: 'partial failure and retry converge without ghost resources' provider-inference-recovery.ts:33-38 shouldRecover uses authority at selection time, recheck at mutation time

PRA-4 Resolve/justify — Missing registry mutation during recovery regression test

  • Location: src/lib/onboard/machine/handlers/provider-inference-recovery-gating.test.ts:1
  • Category: tests
  • Problem: A registry row's pendingRouteReservation could be added/removed between the authority check in shouldRecover() and the recheck inside mutation locks. The recheck should catch this, but no test verifies the fail-closed behavior under mutation.
  • Impact: If a pending reservation is created for another session after shouldRecover() passes but before mutation locks acquire, the recheck must deny recovery.
  • Recommended action: Add a test where shouldRecover() returns true (no pending reservation), then a mock registry adds a foreign pendingRouteReservation before mutateGatewayRoute runs, and verify isRecordedProviderRecoveryAuthorized() returns false and setup aborts.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Verify test uses vi.spyOn to mutate registry.getSandbox return value between shouldRecover() call and setupInference invocation.
  • Missing regression test: Registry mutation race: authority passes initially, then foreign reservation added before lock — recovery must be denied.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Verify test uses vi.spyOn to mutate registry.getSandbox return value between shouldRecover() call and setupInference invocation.
  • Evidence: provider-inference.ts:280-284 providerRecovery.shouldRecover() called before setupNim setup-inference.ts:170-177 isRecordedProviderRecoveryAuthorized() called inside gateway mutation lock riskPlan.lifecycle-state invariant: 'cleanup preserves unrelated sandboxes and removes only owned resources'

💡 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.

  • None.
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-recovery.ts`, `src/lib/onboard/machine/handlers/provider-inference.test-support.ts`, `src/lib/onboard/machine/handlers/provider-inference.ts`.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install families have invariants that need host-to-sandbox boundary verification. Required E2E jobs (cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume) are validation floor, not checked-in unit coverage.
  • PRA-T2 Runtime validation — Add concurrent recovery race test (F-003): two sessions, same sandbox, different sessionIds — only owner recovers.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install families have invariants that need host-to-sandbox boundary verification. Required E2E jobs (cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume) are validation floor, not checked-in unit coverage.
  • 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-recovery.ts`, `src/lib/onboard/machine/handlers/provider-inference.test-support.ts`, `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 require live validation: lifecycle-state, inference-policy, platform-install families have invariants that need host-to-sandbox boundary verification. Required E2E jobs (cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume) are validation floor, not checked-in unit coverage.
  • PRA-T4 Runtime validation — Add registry mutation race test (F-004): authority passes initially, foreign reservation added before lock — recovery denied.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install families have invariants that need host-to-sandbox boundary verification. Required E2E jobs (cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume) are validation floor, not checked-in unit coverage.
  • 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-recovery.ts`, `src/lib/onboard/machine/handlers/provider-inference.test-support.ts`, `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 require live validation: lifecycle-state, inference-policy, platform-install families have invariants that need host-to-sandbox boundary verification. Required E2E jobs (cloud-onboard, inference-routing, network-policy, onboard-repair, onboard-resume) are validation floor, not checked-in unit coverage.
  • PRA-T6 Missing concurrent recovery race regression test — Add a test that spawns two concurrent handleProviderInferenceState calls with different sessionIds for the same sandboxName — one with reservationSessionId matching, one without — and verify only the owner succeeds.
  • PRA-T7 Missing registry mutation during recovery regression test — Add a test where shouldRecover() returns true (no pending reservation), then a mock registry adds a foreign pendingRouteReservation before mutateGatewayRoute runs, and verify isRecordedProviderRecoveryAuthorized() returns false and setup aborts.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Required — Test file monolith growth exceeds guardrail

  • Location: src/lib/onboard/provider-recovery.test.ts:1
  • Category: architecture
  • Problem: provider-recovery.test.ts grew from 171 to 429 lines (+258), exceeding the 20-line growth threshold. The file is now a monolith that should be split or offset before merge.
  • Impact: Large test files are harder to maintain, review, and keep deterministic. Growth guardrail exists to prevent unbounded test accumulation.
  • Required action: Extract recovery authority tests, provider recovery helpers tests, and recorded route readers tests into separate focused test files (e.g., provider-recovery-authority.test.ts, provider-recovery-helpers.test.ts, recorded-route-readers.test.ts).
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check line count: wc -l src/lib/onboard/provider-recovery.test.ts. Verify new extracted files exist and original is reduced.
  • Missing regression test: No regression test needed — this is a structural guardrail violation. The fix is refactoring, not behavior change.
  • Done when: The required change is committed and verification passes: Check line count: wc -l src/lib/onboard/provider-recovery.test.ts. Verify new extracted files exist and original is reduced.
  • Evidence: diff stat shows +258 lines for provider-recovery.test.ts riskPlan.monolithDeltas[0] severity=blocker with delta=258

PRA-2 Resolve/justify — Wide trust-boundary propagation of recoverySessionId

  • Location: src/lib/onboard/machine/handlers/provider-inference.ts:266
  • Category: architecture
  • Problem: recoverySessionId (caller session.sessionId) now threads through provider-recovery.ts helpers, setup-nim-provider-discovery.ts, setup-inference.ts, provider-inference.ts, and provider-inference-recovery.ts — 16 files total. While internally generated, the propagation surface increases coupling.
  • Impact: Future changes to session ID generation or recovery logic must touch many files. The recovery authority check is duplicated between provider-inference-recovery.ts (createRecovery.shouldRecover) and setup-inference.ts (mutateGatewayRoute callback).
  • Recommended action: Consider consolidating the authority recheck into a single shared helper. Ensure provider-inference-recovery.ts is the canonical source for recovery decisions and setup-inference.ts only calls the provided isRecordedProviderRecoveryAuthorized callback.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search for 'recoverySessionId' across changed files. Verify setup-inference.ts mutateGatewayRoute calls options.isRecordedProviderRecoveryAuthorized() rather than reimplementing authority logic.
  • Missing regression test: Add a test that verifies authority recheck is called exactly once inside both mutation locks (sandbox + gateway) and not duplicated elsewhere.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search for 'recoverySessionId' across changed files. Verify setup-inference.ts mutateGatewayRoute calls options.isRecordedProviderRecoveryAuthorized() rather than reimplementing authority logic.
  • Evidence: provider-inference-recovery.ts:27-44 createRecovery returns setupOptions with isRecordedProviderRecoveryAuthorized callback setup-inference.ts:170-177 mutateGatewayRoute calls options.isRecordedProviderRecoveryAuthorized() provider-inference.ts:266-273 createRecovery invoked, setupOptions passed to inferenceOptions

PRA-3 Resolve/justify — Missing concurrent recovery race regression test

  • Location: src/lib/onboard/machine/handlers/provider-inference-recovery-gating.test.ts:1
  • Category: tests
  • Problem: Two sessions attempting recovery for the same sandbox concurrently (one owns the pendingRouteReservation, one doesn't) is not covered. The TOCTOU recheck inside mutation locks should prevent credential leakage, but no test verifies this under actual concurrency.
  • Impact: A race could allow unauthorized recovery if the authority check passes but the registry row changes before the mutation lock is acquired.
  • Recommended action: Add a test that spawns two concurrent handleProviderInferenceState calls with different sessionIds for the same sandboxName — one with reservationSessionId matching, one without — and verify only the owner succeeds.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search for Promise.all or concurrent test patterns in provider-inference-recovery-gating.test.ts. Verify test fails without the mutex recheck.
  • Missing regression test: Concurrent recovery race test: two sessions, same sandbox, different sessionIds — only owner should recover credentials/route.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search for Promise.all or concurrent test patterns in provider-inference-recovery-gating.test.ts. Verify test fails without the mutex recheck.
  • Evidence: setup-inference-route-containment.test.ts:75-130 tests recheck order but single-threaded riskPlan.lifecycle-state invariant: 'partial failure and retry converge without ghost resources' provider-inference-recovery.ts:33-38 shouldRecover uses authority at selection time, recheck at mutation time

PRA-4 Resolve/justify — Missing registry mutation during recovery regression test

  • Location: src/lib/onboard/machine/handlers/provider-inference-recovery-gating.test.ts:1
  • Category: tests
  • Problem: A registry row's pendingRouteReservation could be added/removed between the authority check in shouldRecover() and the recheck inside mutation locks. The recheck should catch this, but no test verifies the fail-closed behavior under mutation.
  • Impact: If a pending reservation is created for another session after shouldRecover() passes but before mutation locks acquire, the recheck must deny recovery.
  • Recommended action: Add a test where shouldRecover() returns true (no pending reservation), then a mock registry adds a foreign pendingRouteReservation before mutateGatewayRoute runs, and verify isRecordedProviderRecoveryAuthorized() returns false and setup aborts.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Verify test uses vi.spyOn to mutate registry.getSandbox return value between shouldRecover() call and setupInference invocation.
  • Missing regression test: Registry mutation race: authority passes initially, then foreign reservation added before lock — recovery must be denied.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Verify test uses vi.spyOn to mutate registry.getSandbox return value between shouldRecover() call and setupInference invocation.
  • Evidence: provider-inference.ts:280-284 providerRecovery.shouldRecover() called before setupNim setup-inference.ts:170-177 isRecordedProviderRecoveryAuthorized() called inside gateway mutation lock riskPlan.lifecycle-state invariant: 'cleanup preserves unrelated sandboxes and removes only owned resources'

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: Split oversized provider recovery test growth into focused files.
Open items: 0 required · 2 warnings · 0 suggestions · 8 test follow-ups
Since last review: 2 prior items resolved · 0 still apply · 2 new items found

Action checklist

  • PRA-1 Resolve or justify: Split oversized provider recovery test growth into focused files in src/lib/onboard/provider-recovery.test.ts:1
  • PRA-2 Resolve or justify: Add an integrated regression for stale dcode session falling back to Ultra in src/lib/onboard/setup-nim-flow.test.ts:145
  • 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: Add an integrated regression for stale dcode session falling back to Ultra
  • PRA-T7 Add or justify test follow-up: Acceptance clause
  • PRA-T8 Add or justify test follow-up: Acceptance clause

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture src/lib/onboard/provider-recovery.test.ts:1 Extract the new recovery authority and pending-reservation reader cases into one or more focused test files, or offset the growth with equivalent split/cleanup while preserving the current security coverage.
PRA-2 Resolve/justify tests src/lib/onboard/setup-nim-flow.test.ts:145 Add a behavior test that seeds a stale global session/model for a different or unowned sandbox, runs the dcode provider selection path for a brand-new sandbox with no NEMOCLAW_MODEL, and asserts the selected model is `nvidia/nemotron-3-ultra-550b-a55b` rather than the stale Super model.
Review findings by urgency: 0 required fixes, 2 items to resolve/justify, 0 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-1 Resolve/justify — Split oversized provider recovery test growth into focused files

  • Location: src/lib/onboard/provider-recovery.test.ts:1
  • Category: architecture
  • Problem: The provider recovery test file expanded by 258 lines in this PR, making a single recovery test module carry validation, ownership classification, persisted route fallback, and registry-error behavior together.
  • Impact: This concentrates security-critical recovery cases in a large test monolith, making future changes to route/session ownership behavior harder to review and increasing the chance that a regression is hidden among unrelated cases.
  • Recommended action: Extract the new recovery authority and pending-reservation reader cases into one or more focused test files, or offset the growth with equivalent split/cleanup while preserving the current security coverage.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/onboard/provider-recovery.test.ts` and compare the monolithDeltas line counts from the scope context; confirm whether the new ownership/recovery sections are split into focused test modules.
  • Missing regression test: Existing added tests should remain as the regression coverage after extraction: authority classification, fresh/registered/matching-session recovery decisions, ownerless/foreign/current pending reservation readers, caller-session identity, and registry-error fail-closed behavior.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/onboard/provider-recovery.test.ts` and compare the monolithDeltas line counts from the scope context; confirm whether the new ownership/recovery sections are split into focused test modules.
  • Evidence: Scope context monolithDeltas reports `src/lib/onboard/provider-recovery.test.ts` baseLines 171, headLines 429, delta 258, severity blocker. Diff adds extensive `shouldRecoverRecordedProvider`, `sandbox recovery authority`, pending route reader ownership, caller-session identity, and registry-error fail-closed cases to the existing provider recovery persisted routing state test file.

PRA-2 Resolve/justify — Add an integrated regression for stale dcode session falling back to Ultra

  • Location: src/lib/onboard/setup-nim-flow.test.ts:145
  • Category: tests
  • Problem: The checked-in tests cover the new sandbox recovery gate and the Deep Agents manifest default separately, but do not compose the exact [All Platforms][Onboard] dcode onboard inherits prior session model instead of DeepAgents Ultra agent-default #6630 regression path where a brand-new dcode onboard with a stale global Super session and no NEMOCLAW_MODEL must select the Deep Agents Ultra default.
  • Impact: A future change could keep the isolated recovery-gate and default-model unit tests passing while reintroducing the original stale-session override in the real provider-selection path for new dcode sandboxes.
  • Recommended action: Add a behavior test that seeds a stale global session/model for a different or unowned sandbox, runs the dcode provider selection path for a brand-new sandbox with no NEMOCLAW_MODEL, and asserts the selected model is `nvidia/nemotron-3-ultra-550b-a55b` rather than the stale Super model.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search the changed tests for a single case containing `langchain-deepagents-code`, a stale Super model (`nvidia/nemotron-3-super-120b-a12b`), no `NEMOCLAW_MODEL`, and an assertion for Ultra (`nvidia/nemotron-3-ultra-550b-a55b`).
  • Missing regression test: Add an integrated unit/subprocess regression for [All Platforms][Onboard] dcode onboard inherits prior session model instead of DeepAgents Ultra agent-default #6630: create or stub a prior onboard session containing Super for an unrelated/old sandbox, ensure no registry authority for the new `dc-after` sandbox and no `NEMOCLAW_MODEL`, invoke the dcode `setupNim`/provider-inference path, and assert the selected model/default is Ultra and no `recovered from sandbox` path is used.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search the changed tests for a single case containing `langchain-deepagents-code`, a stale Super model (`nvidia/nemotron-3-super-120b-a12b`), no `NEMOCLAW_MODEL`, and an assertion for Ultra (`nvidia/nemotron-3-ultra-550b-a55b`).
  • Evidence: `src/lib/onboard/machine/handlers/provider-inference-recovery-gating.test.ts` covers brand-new sandbox recovery disabled by asserting `setupNim(..., recover=false, ...)`, but it does not assert the resulting dcode model/default config. `src/lib/onboard/setup-nim-flow.test.ts` line 145 covers passing the Deep Agents manifest default to shared model selection, but it does not seed a stale prior onboard session or verify the no-env brand-new dcode path. `test/onboard-resume-provider-recovery.test.ts` covers stale recorded providers ignored when recovery is disabled, but the fixture agent is `null` and asserts `nvidia/test-model` from `NEMOCLAW_MODEL`, not dcode Ultra with no model env.

💡 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.

  • None.
Simplification opportunities: 1 possible cut

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

  • PRA-1 shrink (src/lib/onboard/provider-recovery.test.ts:1): Move the new recovery authority and pending-reservation reader tests into focused test files or smaller describe-scoped modules, leaving only directly related provider recovery helper tests in this file.
    • Replacement: Focused test modules for recovery authority decisions and recorded route reader authorization, reusing the same checked-in assertions.
    • Net: 0 lines
    • Safety boundary: Do not remove any trust-boundary validation or negative coverage for unauthorized, ownerless, caller-session, or registry-error recovery paths.
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-recovery.ts`, `src/lib/onboard/machine/handlers/provider-inference.test-support.ts`, `src/lib/onboard/machine/handlers/provider-inference.ts`.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Checked-in unit and composition tests cover many recovery and route-ownership branches, but the risk plan marks lifecycle-state, inference-policy, and platform-install as deterministic runtime-risk families. One concrete checked-in integrated regression gap remains for [All Platforms][Onboard] dcode onboard inherits prior session model instead of DeepAgents Ultra agent-default #6630.
  • PRA-T2 Runtime validation — Add the F-002 integrated regression: stale prior onboard session with `nvidia/nemotron-3-super-120b-a12b`, brand-new dcode sandbox `dc-after`, no `NEMOCLAW_MODEL`, and assert `nvidia/nemotron-3-ultra-550b-a55b` with no recovered-provider path.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Checked-in unit and composition tests cover many recovery and route-ownership branches, but the risk plan marks lifecycle-state, inference-policy, and platform-install as deterministic runtime-risk families. One concrete checked-in integrated regression gap remains for [All Platforms][Onboard] dcode onboard inherits prior session model instead of DeepAgents Ultra agent-default #6630.
  • 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-recovery.ts`, `src/lib/onboard/machine/handlers/provider-inference.test-support.ts`, `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 unit and composition tests cover many recovery and route-ownership branches, but the risk plan marks lifecycle-state, inference-policy, and platform-install as deterministic runtime-risk families. One concrete checked-in integrated regression gap remains for [All Platforms][Onboard] dcode onboard inherits prior session model instead of DeepAgents Ultra agent-default #6630.
  • PRA-T4 Runtime validation — Run the `cloud-onboard` E2E job for clean-host/platform-install validation of the changed provider-inference recovery path.. Deterministic regression risks require live validation: lifecycle-state, inference-policy, platform-install. Checked-in unit and composition tests cover many recovery and route-ownership branches, but the risk plan marks lifecycle-state, inference-policy, and platform-install as deterministic runtime-risk families. One concrete checked-in integrated regression gap remains for [All Platforms][Onboard] dcode onboard inherits prior session model instead of DeepAgents Ultra agent-default #6630.
  • 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-recovery.ts`, `src/lib/onboard/machine/handlers/provider-inference.test-support.ts`, `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 unit and composition tests cover many recovery and route-ownership branches, but the risk plan marks lifecycle-state, inference-policy, and platform-install as deterministic runtime-risk families. One concrete checked-in integrated regression gap remains for [All Platforms][Onboard] dcode onboard inherits prior session model instead of DeepAgents Ultra agent-default #6630.
  • PRA-T6 Add an integrated regression for stale dcode session falling back to Ultra — Add a behavior test that seeds a stale global session/model for a different or unowned sandbox, runs the dcode provider selection path for a brand-new sandbox with no NEMOCLAW_MODEL, and asserts the selected model is `nvidia/nemotron-3-ultra-550b-a55b` rather than the stale Super model.
  • PRA-T7 Acceptance clause — A fresh `nemoclaw onboard --agent dcode` with a brand-new sandbox name and no `NEMOCLAW_MODEL` inherits the model recorded in the global onboard session state (`~/.nemoclaw/onboard-session.json`) from a prior onboard, instead of applying the DeepAgents manifest default (Ultra = `nvidia/nemotron-3-ultra-550b-a55b`). — add test evidence or identify existing coverage. `createRecovery(...).shouldRecover()` and `provider-inference-recovery-gating.test.ts` cover disabling recorded recovery for a brand-new sandbox, and `setup-nim-flow.test.ts` covers passing the Deep Agents default to model selection. The exact composed dcode/no-env/stale-Super scenario lacks a single checked-in regression and is covered by F-002.
  • PRA-T8 Acceptance clause — If a prior session left Super (`nvidia/nemotron-3-super-120b-a12b`) — e.g. from an earlier openclaw onboard — the newly-named dcode sandbox silently comes up on Super. — add test evidence or identify existing coverage. The new authorization gate prevents recovery for missing/unowned sandbox identities, but the checked-in tests do not compose stale Super from a prior session with a brand-new dcode Ultra assertion; see F-002.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Split oversized provider recovery test growth into focused files

  • Location: src/lib/onboard/provider-recovery.test.ts:1
  • Category: architecture
  • Problem: The provider recovery test file expanded by 258 lines in this PR, making a single recovery test module carry validation, ownership classification, persisted route fallback, and registry-error behavior together.
  • Impact: This concentrates security-critical recovery cases in a large test monolith, making future changes to route/session ownership behavior harder to review and increasing the chance that a regression is hidden among unrelated cases.
  • Recommended action: Extract the new recovery authority and pending-reservation reader cases into one or more focused test files, or offset the growth with equivalent split/cleanup while preserving the current security coverage.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/onboard/provider-recovery.test.ts` and compare the monolithDeltas line counts from the scope context; confirm whether the new ownership/recovery sections are split into focused test modules.
  • Missing regression test: Existing added tests should remain as the regression coverage after extraction: authority classification, fresh/registered/matching-session recovery decisions, ownerless/foreign/current pending reservation readers, caller-session identity, and registry-error fail-closed behavior.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/onboard/provider-recovery.test.ts` and compare the monolithDeltas line counts from the scope context; confirm whether the new ownership/recovery sections are split into focused test modules.
  • Evidence: Scope context monolithDeltas reports `src/lib/onboard/provider-recovery.test.ts` baseLines 171, headLines 429, delta 258, severity blocker. Diff adds extensive `shouldRecoverRecordedProvider`, `sandbox recovery authority`, pending route reader ownership, caller-session identity, and registry-error fail-closed cases to the existing provider recovery persisted routing state test file.

PRA-2 Resolve/justify — Add an integrated regression for stale dcode session falling back to Ultra

  • Location: src/lib/onboard/setup-nim-flow.test.ts:145
  • Category: tests
  • Problem: The checked-in tests cover the new sandbox recovery gate and the Deep Agents manifest default separately, but do not compose the exact [All Platforms][Onboard] dcode onboard inherits prior session model instead of DeepAgents Ultra agent-default #6630 regression path where a brand-new dcode onboard with a stale global Super session and no NEMOCLAW_MODEL must select the Deep Agents Ultra default.
  • Impact: A future change could keep the isolated recovery-gate and default-model unit tests passing while reintroducing the original stale-session override in the real provider-selection path for new dcode sandboxes.
  • Recommended action: Add a behavior test that seeds a stale global session/model for a different or unowned sandbox, runs the dcode provider selection path for a brand-new sandbox with no NEMOCLAW_MODEL, and asserts the selected model is `nvidia/nemotron-3-ultra-550b-a55b` rather than the stale Super model.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search the changed tests for a single case containing `langchain-deepagents-code`, a stale Super model (`nvidia/nemotron-3-super-120b-a12b`), no `NEMOCLAW_MODEL`, and an assertion for Ultra (`nvidia/nemotron-3-ultra-550b-a55b`).
  • Missing regression test: Add an integrated unit/subprocess regression for [All Platforms][Onboard] dcode onboard inherits prior session model instead of DeepAgents Ultra agent-default #6630: create or stub a prior onboard session containing Super for an unrelated/old sandbox, ensure no registry authority for the new `dc-after` sandbox and no `NEMOCLAW_MODEL`, invoke the dcode `setupNim`/provider-inference path, and assert the selected model/default is Ultra and no `recovered from sandbox` path is used.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search the changed tests for a single case containing `langchain-deepagents-code`, a stale Super model (`nvidia/nemotron-3-super-120b-a12b`), no `NEMOCLAW_MODEL`, and an assertion for Ultra (`nvidia/nemotron-3-ultra-550b-a55b`).
  • Evidence: `src/lib/onboard/machine/handlers/provider-inference-recovery-gating.test.ts` covers brand-new sandbox recovery disabled by asserting `setupNim(..., recover=false, ...)`, but it does not assert the resulting dcode model/default config. `src/lib/onboard/setup-nim-flow.test.ts` line 145 covers passing the Deep Agents manifest default to shared model selection, but it does not seed a stale prior onboard session or verify the no-env brand-new dcode path. `test/onboard-resume-provider-recovery.test.ts` covers stale recorded providers ignored when recovery is disabled, but the fixture agent is `null` and asserts `nvidia/test-model` from `NEMOCLAW_MODEL`, not dcode Ultra with no model env.

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.

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

🧹 Nitpick comments (1)
src/lib/onboard/provider-recovery.ts (1)

134-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract shared identity-gating logic to eliminate duplication.

readRecordedProvider and readRecordedModel are now near-identical apart from the field name (provider vs model), including the new hasRecordedSandboxIdentity tracking, the try/catch structure, and the live-gateway fallback gate. This doubles the maintenance surface for a very security-sensitive check (the exact logic this PR is fixing) — any future edit risks fixing one copy and missing the other.

As per coding guidelines, **/*.{js,ts} should "Keep function complexity low in JavaScript and TypeScript code," and this duplication is a good candidate to consolidate into a single generic helper.

♻️ Proposed refactor: shared field-reader helper
+  function readRecordedField(
+    sandboxName: string,
+    field: "provider" | "model",
+  ): string | null {
+    let hasRecordedSandboxIdentity = false;
+    try {
+      const entry = registry.getSandbox(sandboxName);
+      hasRecordedSandboxIdentity = Boolean(entry);
+      const value = entry?.[field];
+      if (typeof value === "string" && value) return value;
+    } catch {
+      // fall through to session
+    }
+    try {
+      const session = onboardSession.loadSession();
+      if (session && session.sandboxName === sandboxName) {
+        hasRecordedSandboxIdentity = true;
+        const value = session[field];
+        if (typeof value === "string" && value) return value;
+      }
+    } catch {
+      // fall through
+    }
+    // An empty registry does not prove that the gateway's residual route
+    // belongs to this requested sandbox name. Require a matching registry or
+    // session identity before using that route as rebuild recovery state;
+    // otherwise a brand-new sandbox inherits the previous sandbox's model.
+    if (!hasRecordedSandboxIdentity) return null;
+    const live = readLiveInference(sandboxName);
+    const liveValue = live?.[field];
+    return typeof liveValue === "string" && liveValue ? liveValue : null;
+  }
+
   function readRecordedProvider(sandboxName: string | null | undefined): string | null {
     if (!sandboxName) return null;
-    let hasRecordedSandboxIdentity = false;
-    try {
-      const entry = registry.getSandbox(sandboxName);
-      hasRecordedSandboxIdentity = Boolean(entry);
-      if (entry && typeof entry.provider === "string" && entry.provider) {
-        return entry.provider;
-      }
-    } catch {
-      // fall through to session
-    }
-    try {
-      const session = onboardSession.loadSession();
-      if (session && session.sandboxName === sandboxName) {
-        hasRecordedSandboxIdentity = true;
-        if (typeof session.provider === "string" && session.provider) {
-          return session.provider;
-        }
-      }
-    } catch {
-      // fall through to live gateway
-    }
-    if (!hasRecordedSandboxIdentity) return null;
-    const live = readLiveInference(sandboxName);
-    if (live && typeof live.provider === "string" && live.provider) {
-      return live.provider;
-    }
-    return null;
+    return readRecordedField(sandboxName, "provider");
   }

Apply the analogous simplification to readRecordedModel. Note the diff is illustrative — verify the InferenceSelection/session types allow index access on field without excessive type assertions.

Also applies to: 195-224

🤖 Prompt for 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.

In `@src/lib/onboard/provider-recovery.ts` around lines 134 - 167, Extract the
duplicated identity-gating and fallback flow from readRecordedProvider and
readRecordedModel into one generic helper that accepts the requested sandbox
name and field key (provider or model), safely reads registry/session values,
requires a recorded identity before consulting readLiveInference, and returns
the selected string or null. Refactor both functions to delegate to this helper,
preserving their existing behavior and using minimal type assertions compatible
with the InferenceSelection and session types.

Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@src/lib/onboard/provider-recovery.ts`:
- Around line 134-167: Extract the duplicated identity-gating and fallback flow
from readRecordedProvider and readRecordedModel into one generic helper that
accepts the requested sandbox name and field key (provider or model), safely
reads registry/session values, requires a recorded identity before consulting
readLiveInference, and returns the selected string or null. Refactor both
functions to delegate to this helper, preserving their existing behavior and
using minimal type assertions compatible with the InferenceSelection and session
types.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f6c515dc-f709-4313-8814-04151b9cc21e

📥 Commits

Reviewing files that changed from the base of the PR and between 9dec1fb and e2f3388.

📒 Files selected for processing (2)
  • src/lib/onboard/provider-recovery.test.ts
  • src/lib/onboard/provider-recovery.ts

@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 29083126817
Workflow ref: fix/6630_dcode_agent_default_model
Requested targets: (default — all supported)
Requested jobs: cloud-onboard,onboard-repair,onboard-resume
Summary: 3 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
cloud-onboard ✅ success
onboard-repair ✅ success
onboard-resume ✅ success

Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 29083689065
Workflow ref: fix/6630_dcode_agent_default_model
Requested targets: (default — all supported)
Requested jobs: cloud-onboard,onboard-repair,onboard-resume
Summary: 3 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
cloud-onboard ✅ success
onboard-repair ✅ success
onboard-resume ✅ success

@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.ts`:
- Around line 4420-4423: Move the recorded-provider recovery eligibility
decision out of the onboard entrypoint and into handleProviderInferenceState.
Pass the raw inputs (fresh, resume, sandboxName, session, and the registry
lookup capability) through coreFlowContext/deps, then call
providerRecovery.shouldRecoverRecordedProvider within the handler and use its
result there. Remove allowRecordedProviderRecovery from the entrypoint and
intermediate wiring.
🪄 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: 932ed885-ac6c-4e1a-aa51-a5f041bff8f9

📥 Commits

Reviewing files that changed from the base of the PR and between e2f3388 and df8fc93.

📒 Files selected for processing (6)
  • src/lib/onboard.ts
  • src/lib/onboard/machine/core-flow-phases.ts
  • src/lib/onboard/machine/handlers/provider-inference.test.ts
  • src/lib/onboard/machine/handlers/provider-inference.ts
  • src/lib/onboard/provider-recovery.test.ts
  • src/lib/onboard/provider-recovery.ts

Comment thread src/lib/onboard.ts Outdated
Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 29084488770
Workflow ref: fix/6630_dcode_agent_default_model
Requested targets: (default — all supported)
Requested jobs: cloud-onboard,onboard-repair,onboard-resume
Summary: 3 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
cloud-onboard ✅ success
onboard-repair ✅ success
onboard-resume ✅ success

@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.ts`:
- Around line 324-330: Move the recoverRecordedProvider computation out of the
pre-loop initialization and recompute it immediately before each
deps.setupNim(...) call, using the current sandboxName, fresh, resume,
session?.sandboxName, and registered-sandbox status. Update all setupNim retry
paths, including selection retries, so the recovery decision reflects sandbox
names resolved during loop iterations.
🪄 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: 881b0550-770e-488a-af45-615819c87b18

📥 Commits

Reviewing files that changed from the base of the PR and between df8fc93 and cddddd2.

📒 Files selected for processing (8)
  • src/lib/onboard.ts
  • src/lib/onboard/machine/core-flow-phases.test.ts
  • src/lib/onboard/machine/handlers/provider-inference-route-containment.test.ts
  • src/lib/onboard/machine/handlers/provider-inference.test-support.ts
  • src/lib/onboard/machine/handlers/provider-inference.test.ts
  • src/lib/onboard/machine/handlers/provider-inference.ts
  • src/lib/onboard/provider-recovery.test.ts
  • src/lib/onboard/provider-recovery.ts
💤 Files with no reviewable changes (1)
  • src/lib/onboard/machine/handlers/provider-inference.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/onboard/provider-recovery.test.ts

Comment thread src/lib/onboard/machine/handlers/provider-inference.ts Outdated
Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 29085347620
Workflow ref: fix/6630_dcode_agent_default_model
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

Signed-off-by: Chengjie Wang <chengjiew@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-recovery-gating.test.ts`:
- Around line 55-56: Update the “allows recovery for a registered sandbox” test
to make hasRegisteredSandbox in createDeps identity-sensitive: return true only
when called with "dc-after", record the invocation, and assert it was called
with exactly "dc-after" so incorrect or null sandbox lookups cannot pass.
🪄 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: 03a8f399-faf6-4b22-9e22-149e49d66de6

📥 Commits

Reviewing files that changed from the base of the PR and between 81f37c9 and c86c42f.

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

Comment thread src/lib/onboard/machine/handlers/provider-inference-recovery-gating.test.ts Outdated
@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 29085927211
Workflow ref: fix/6630_dcode_agent_default_model
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

Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 29086523887
Workflow ref: fix/6630_dcode_agent_default_model
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 added the v0.0.80 Release target label 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 ce957f129. The stale CodeRabbit architecture finding is fixed: the recovery decision now lives in handleProviderInferenceState, and I resolved that outdated thread.

One correctness blocker remains. onboard.ts currently supplies:

hasRegisteredSandbox: (name) => Boolean(registry.getSandbox(name))

That treats a pendingRouteReservation placeholder as established sandbox identity. An abandoned reservation from an earlier onboarding session can therefore make shouldRecoverRecordedProvider() accept the old registry/live provider and model for a new attempt—the stale-identity case this PR is meant to prevent.

Please sequence this with #6626, which adds reservationSessionId / isPendingReservationForSession, then make recovery eligible only for either:

  • a fully registered sandbox row (pendingRouteReservation !== true), or
  • a pending reservation owned by the current onboard session.sessionId.

Add a negative regression for an orphaned/other-session pending reservation with a stale route (the new DCode attempt must retain its agent default), plus the positive same-session resume case. Rebase on current main and refresh the exact-head gates/live evidence after the overlap is resolved.

@cv

cv commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Status update — branch tip 7834b5f9c (GitHub PR synchronization is still pending)

#6626 is merged, and this branch was synced with main before the recovery hardening landed.

What is fixed:

  • Registry presence is authoritative: foreign-owned and ownerless pending route reservations are denied even when an older completed session matches the sandbox name.
  • Recovery is bound to the caller's snapshotted session ID throughout; ambient on-disk session identity is not used for authorization.
  • Registry read failures fail closed before session/live-route fallback.
  • A genuinely recovered selection forces route setup, and ownership is rechecked while both sandbox and gateway mutation locks are held before route mutation or credential reuse.
  • The CodeRabbit ownerless/no-active-session test gap was added in 288947ed8.
  • 7834b5f9c updates the stale integration expectation exposed by CI: an unreadable registry now returns null, does not consult stale session state, and emits a warning.

Verification:

  • 171/171 adjacent CLI tests passed for the production change; CLI typecheck and build passed.
  • test/onboard-resume-provider-recovery.test.ts: 28/28 passed after the CI expectation update.
  • The unrelated shard-4 timeout reproducer passes locally: 11/11 in 3.81s.
  • Normal pre-commit, commit-msg, and pre-push hooks passed; GitHub reports 7834b5f9c as Verified.
  • Live E2E run 29125206127 ran on production head 7e04b4fea and passed all five requested targets: cloud-onboard, inference-routing, network-policy, onboard-repair, and onboard-resume.
  • CodeRabbit completed its production-head review with no actionable comments.

Advisor disposition:

  • The current-head advisor confirms the seven earlier session/ownership findings are resolved.
  • Its broad partial-failure, multi-sandbox cleanup, live-health, network-policy, and clean-host follow-ups are runtime concerns covered by the five live jobs above, not unit regressions introduced here.
  • The architecture warnings are acknowledged; the repository growth guardrail passes, and extracting existing onboarding monoliths in this security fix would broaden the change after behavior is covered.
  • Registry-error warnings are observable in production: the sole production helper construction supplies console.warn, and the focused test asserts the fail-closed warning path. The optional callback remains useful for isolated test harnesses.

Human review state:

  • The latest security follow-up states the authorization blocker is resolved.
  • Earlier CHANGES_REQUESTED reviews remain recorded and have not been dismissed or self-approved.

Platform note: the protected branch ref and commit API both show verified tip 7834b5f9c, but the PR API still reports 7e04b4fea and has not emitted its asynchronous synchronize event. Consequently, the new exact-head PR checks have not started yet; no force-push, close/reopen, or other invasive workaround was attempted.

cv added 3 commits July 10, 2026 12:11
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 29123478046
Workflow ref: fix/6630_dcode_agent_default_model
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

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

🧹 Nitpick comments (1)
src/lib/onboard/provider-recovery.test.ts (1)

118-158: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add coverage for an orphaned reservation with no active session identity.

The classifySandboxRecoveryAuthority matrix covers orphaned vs. foreign vs. owned pending reservations, all against a concrete sessionId: "session-current". It never exercises the combination of an orphaned reservation (entry.reservationSessionId unset) with sessionId: null/undefined — the scenario where no active onboard session exists at all. Given this function is the core authorization gate this PR introduces to fix #6630, adding this case would close a residual coverage gap on a security-relevant path.

🧪 Suggested additional case
     {
       label: "another session's pending reservation",
       entry: pending("session-other"),
       sessionId: "session-current",
       expected: "unauthorized",
     },
+    {
+      label: "orphaned reservation with no active session",
+      entry: pending(),
+      sessionId: null,
+      expected: "unauthorized",
+    },
🤖 Prompt for 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.

In `@src/lib/onboard/provider-recovery.test.ts` around lines 118 - 158, Add a
parameterized test case to the “sandbox recovery authority” matrix for a pending
reservation without reservationSessionId and with no active session identity
(sessionId null or undefined), asserting classifySandboxRecoveryAuthority
returns “unauthorized”.
🤖 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.

Nitpick comments:
In `@src/lib/onboard/provider-recovery.test.ts`:
- Around line 118-158: Add a parameterized test case to the “sandbox recovery
authority” matrix for a pending reservation without reservationSessionId and
with no active session identity (sessionId null or undefined), asserting
classifySandboxRecoveryAuthority returns “unauthorized”.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: cefabc99-474c-4c04-b783-d4015a563a26

📥 Commits

Reviewing files that changed from the base of the PR and between 81f37c9 and 73dc06f.

📒 Files selected for processing (9)
  • src/lib/onboard.ts
  • src/lib/onboard/machine/core-flow-phases.test.ts
  • src/lib/onboard/machine/handlers/provider-inference-recovery-gating.test.ts
  • src/lib/onboard/machine/handlers/provider-inference-route-containment.test.ts
  • src/lib/onboard/machine/handlers/provider-inference.test-support.ts
  • src/lib/onboard/machine/handlers/provider-inference.test.ts
  • src/lib/onboard/machine/handlers/provider-inference.ts
  • src/lib/onboard/provider-recovery.test.ts
  • src/lib/onboard/provider-recovery.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/onboard/machine/handlers/provider-inference.test.ts

cv added 2 commits July 10, 2026 14:13
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv

cv commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 29125206127
Workflow ref: fix/6630_dcode_agent_default_model
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

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cjagwani

Copy link
Copy Markdown
Contributor

Exact-head security follow-up for 7e04b4feaaf509ed26225ac96b74bb2b8ae6a299: the authorization blocker is now resolved.

  • caller session identity is threaded through every recovery read; the ambient session reload is gone;
  • foreign/ownerless pending reservations still fail closed;
  • the recovered path forces setup and rechecks ownership inside the sandbox/gateway mutation locks before route mutation or credential reuse;
  • 6 focused files / 104 tests, build:cli, and typecheck:cli passed locally.

This head is not approval-ready yet because required CI is red:

  1. shard 2's test/onboard-resume-provider-recovery.test.ts:164-174 still expects session fallback after the registry read throws. The new fail-closed contract correctly returns null and warns, so that stale expectation needs updating;
  2. shard 4 timed out in the unchanged mcp-bridge-status-resolution.test.ts:150 case and can be rerun after the real shard-2 test fix lands.

The five required live jobs passed on 288947ed, but that evidence predates this production change. After CI is green, rerun exact-current-head live coverage and refresh final review. I have not approved or dispatched new live work on the failing head.

Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
@cv cv enabled auto-merge (squash) July 10, 2026 22:29
@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 29127739163
Workflow ref: fix/6630_dcode_agent_default_model
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

@github-actions

Copy link
Copy Markdown
Contributor

E2E Target Results — ✅ All requested jobs passed

Run: 29129518218
Workflow ref: fix/6630_dcode_agent_default_model
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

@cjagwani cjagwani 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.

lgtm

@cv cv merged commit 89e19f0 into main Jul 10, 2026
122 checks passed
@cv cv deleted the fix/6630_dcode_agent_default_model branch July 10, 2026 23:18
cv pushed a commit that referenced this pull request Jul 11, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Preserve the preflighted compatible-endpoint recovery authority while an
authoritative rebuild revalidates messaging inference after deleting the
old sandbox. This prevents the recreate flow from falling back to NVIDIA
Endpoints and demanding an unrelated host key.

## Related Issue
Regression follow-up to #6634.

## Changes
- Thread authoritative rebuild context into recorded-provider recovery.
- Keep ordinary incomplete sessions, fresh sandboxes, mismatched
identities, and foreign reservations fail-closed.
- Cover both the recovery gate and the two-pass messaging revalidation
flow.

## 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 rebuild recovery
correction; commands and documented behavior are unchanged
- [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: pending maintainer
review; the change retains #6634's fresh, mismatched, and unauthorized
recovery denials
- [ ] 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
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — `npx vitest run --project cli
src/lib/onboard/machine/handlers/provider-inference-recovery-gating.test.ts
src/lib/onboard/machine/handlers/provider-inference.test.ts` (44
passed); adjacent recovery/rebuild selection (126 passed)
- [ ] Applicable broad gate passed — full CLI project: 8,101 passed; six
unrelated local prerequisite/time-budget failures, with timeout cases
passing under a realistic budget; CI remains authoritative
- [ ] 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: Prekshi Vyas <prekshiv@nvidia.com>


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

* **Bug Fixes**
* Improved provider inference recovery for authoritative resume flows,
including incomplete onboarding sessions.
* Recovery behavior now correctly accounts for sandbox/session context
and foreign reservation flags.
* Added stronger validation for preflighted locked rebuild resumes, with
clearer blocking when required authoritative conditions are missing.

* **Tests**
* Added new test coverage for authoritative incomplete-session recovery
across matching/mismatched sandboxes and reservation scenarios.
* Expanded assertions to verify inference setup inputs and recovery
gating behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
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
<!-- markdownlint-disable MD041 -->
## Summary
Prevent a newly named sandbox from inheriting a stale inference provider
and model left on an otherwise unowned OpenShell gateway. Live gateway
recovery now requires matching registry or onboard-session identity, so
Deep Agents Code can apply its Ultra agent default.

## Related Issue
Fixes NVIDIA#6630

## Changes
- Gate live gateway provider/model recovery on a matching sandbox
registry row or onboard session.
- Add a regression test for an empty registry with a stale Super route
and a brand-new dcode sandbox name.
- Preserve and test live-route recovery for a real rebuild with a
matching session identity.

## 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
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [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: This restores the documented
dcode agent default without changing commands, flags, or user-facing
configuration.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: Focused
recovery-boundary review confirms live state is accepted only after
durable sandbox identity is established; positive rebuild coverage
protects the intended recovery path.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [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
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: `npx
vitest run src/lib/onboard/provider-recovery.test.ts
src/lib/onboard/setup-nim-flow.test.ts
src/lib/onboard/machine/handlers/provider-inference.test.ts --project
cli` — 61 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)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>


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

* **Bug Fixes**
* Tightened onboarding recorded-provider recovery with sandbox recovery
authority checks, preventing recovery when authorization is
missing/unauthorized or when session identity is incomplete or
mismatched.
* Refined pending route handling during recovery so pending-route
readers only succeed for the owning current session; otherwise they
return no result.
* Fail-closed behavior: when recovery authority can’t be confirmed or
ownership is lost during setup, the flow stops and emits a
warning/error.
* **Tests**
* Added/expanded automated coverage for recovery gating, pending-route
ownership, and retry/setup parameter correctness.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Co-authored-by: Carlos Villela <cvillela@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
<!-- markdownlint-disable MD041 -->
## Summary
Preserve the preflighted compatible-endpoint recovery authority while an
authoritative rebuild revalidates messaging inference after deleting the
old sandbox. This prevents the recreate flow from falling back to NVIDIA
Endpoints and demanding an unrelated host key.

## Related Issue
Regression follow-up to NVIDIA#6634.

## Changes
- Thread authoritative rebuild context into recorded-provider recovery.
- Keep ordinary incomplete sessions, fresh sandboxes, mismatched
identities, and foreign reservations fail-closed.
- Cover both the recovery gate and the two-pass messaging revalidation
flow.

## 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 rebuild recovery
correction; commands and documented behavior are unchanged
- [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: pending maintainer
review; the change retains NVIDIA#6634's fresh, mismatched, and unauthorized
recovery denials
- [ ] 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
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — `npx vitest run --project cli
src/lib/onboard/machine/handlers/provider-inference-recovery-gating.test.ts
src/lib/onboard/machine/handlers/provider-inference.test.ts` (44
passed); adjacent recovery/rebuild selection (126 passed)
- [ ] Applicable broad gate passed — full CLI project: 8,101 passed; six
unrelated local prerequisite/time-budget failures, with timeout cases
passing under a realistic budget; CI remains authoritative
- [ ] 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: Prekshi Vyas <prekshiv@nvidia.com>


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

* **Bug Fixes**
* Improved provider inference recovery for authoritative resume flows,
including incomplete onboarding sessions.
* Recovery behavior now correctly accounts for sandbox/session context
and foreign reservation flags.
* Added stronger validation for preflighted locked rebuild resumes, with
clearer blocking when required authoritative conditions are missing.

* **Tests**
* Added new test coverage for authoritative incomplete-session recovery
across matching/mismatched sandboxes and reservation scenarios.
* Expanded assertions to verify inference setup inputs and recovery
gating behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Prekshi Vyas <prekshiv@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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v0.0.80 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[All Platforms][Onboard] dcode onboard inherits prior session model instead of DeepAgents Ultra agent-default

3 participants