Skip to content

fix(onboard): restore not-ready check after compatibility decisions - #8373

Merged
jyaunches merged 3 commits into
NVIDIA:mainfrom
jyaunches:fix/8027-not-ready-resume-ordering
Aug 6, 2026
Merged

fix(onboard): restore not-ready check after compatibility decisions#8373
jyaunches merged 3 commits into
NVIDIA:mainfrom
jyaunches:fix/8027-not-ready-resume-ordering

Conversation

@jyaunches

@jyaunches jyaunches commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Restores the original ordering of the not_ready sandbox check in
decideSandboxResume, placing it after compatibility, configuration,
and tool-disclosure checks instead of before them.

Related Issue

Regression introduced by #8027 (commit 3312899).

Problem

PR #8027 moved the not_ready → repair-and-recreate decision to the
top of decideSandboxResume. This caused gateway upgrade/recreation
and named-rebuild resume flows to fail because:

  1. The sandbox appears not_ready on an upgraded gateway (expected
    transient state during upgrade)
  2. repair-and-recreate fires before compatibility checks can route
    the resume through their proven journal-capable paths
  3. selectedGatewayForSandboxRecreate returns null for old-version
    checkpoints without gateway authority → journal cannot begin
  4. replacesSameNameSandbox is true for repair-and-recreate → throws
    Cannot replace same-name sandbox: no recreate transaction proves ownership of the source sandbox and its registry row.

Fix

Restore the original ordering so that:

  • Gateway upgrades route through compatibility-driven recreate (no
    journal ownership required since removeRegistryEntry is false)
  • Named rebuilds route through recreateSandboxRequested (config check)
  • not_ready only triggers repair-and-recreate as a last resort for
    genuinely damaged sandboxes where no other trigger explains the state

Regressions Fixed

  • Gateway upgrade/recreation — 4/5 gateway-version E2E jobs failing
    with Recreate failed after sandbox was destroyed
  • Deep Agents named rebuild — rebuilding to disable thread opt-in
    left nemoclaw status failing (same recreate/resume surface)

Type of Change

  • Code change (feature, bug fix, or refactor)

Quality Gates

  • Tests added or updated for changed behavior
  • Docs not applicable — no user-facing behavior change
  • Sensitive paths changed (onboarding, sandbox)
  • Sensitive-path review completed — minimal ordering-only revert
    in a pure decision function; no new code paths introduced

Verification

  • npx vitest run src/lib/onboard/machine/handlers/sandbox-resume.test.ts — 30 passed
  • npx vitest run src/lib/onboard/machine/ — 602 passed
  • npm run typecheck:cli — clean
  • npm run build:cli — clean
  • All pre-commit hooks passed

Signed-off-by: Jessica Yaunches jyaunches@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved sandbox resume handling for environments that are not ready.
    • Compatible sandboxes can now resume without unnecessary recreation.
    • Added safer recovery when runtime settings, tools, inference routes, agent selection, approval settings, or reasoning capabilities change.
    • Ensured affected sandboxes are recreated with appropriate cleanup and explanatory notes.
    • Improved recovery when a recreation request is interrupted or resumed.

PR NVIDIA#8027 moved the not_ready → repair-and-recreate decision to the top
of decideSandboxResume, ahead of compatibility, configuration, and tool-
disclosure checks. This caused gateway upgrade/recreation and named-
rebuild resume flows to fail with 'Cannot replace same-name sandbox: no
recreate transaction proves ownership' because:

1. The sandbox appears not_ready on an upgraded gateway (expected)
2. repair-and-recreate fires before compatibility checks can route the
   resume through their proven journal-capable paths
3. selectedGatewayForSandboxRecreate returns null for old-version
   checkpoints without gateway authority → journal cannot begin
4. replacesSameNameSandbox is true for repair-and-recreate → throws

Restoring the original ordering (not_ready checked AFTER compatibility,
reuse, configuration, and tool-disclosure) ensures:
- Gateway upgrades route through compatibility-driven recreate (no
  journal ownership required since removeRegistryEntry is false)
- Named rebuilds route through recreateSandboxRequested (config check)
- not_ready only triggers repair-and-recreate as a last resort for
  genuinely damaged sandboxes where no other trigger explains the state

Fixes the gateway-version E2E regression (4/5 jobs failing) and the
Deep Agents named-rebuild regression introduced by 3312899.

Signed-off-by: Jessica Yaunches <jyaunches@nvidia.com>
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: fca16a20-9c99-41d1-b87b-a98f226ee7c3

📥 Commits

Reviewing files that changed from the base of the PR and between 3311296 and 9893d98.

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

📝 Walkthrough

Walkthrough

decideSandboxResume now continues journaled recreation and prioritizes repair-and-recreate for unowned not_ready requests. Tests cover recreation types, diagnostic notes, and registry cleanup for configuration drift.

Changes

Sandbox resume handling

Layer / File(s) Summary
Resume decision precedence
src/lib/onboard/machine/handlers/sandbox-resume.ts
The handler detects journaled recreation and unowned not_ready repair requests. It applies repair-and-recreate before compatibility, reuse, runtime-configuration, and tool-disclosure decisions.
Resume decision test coverage
src/lib/onboard/machine/handlers/sandbox-resume.test.ts
Tests preserve repair-and-recreate for DCode auto-approval and reasoning drift. Other not-ready drift cases assert compatibility or runtime-configuration recreation, diagnostic notes, and registry cleanup behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

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

Suggested reviewers: apurvvkumaria, cv

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes restoring the not-ready check after compatibility decisions, which matches the primary change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

🤖 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/sandbox-resume.test.ts`:
- Around line 136-138: Rename the test case around the DCode auto-approval
scenario to behavior-oriented wording that states it falls through to
repair-and-recreate when the sandbox is not_ready. Keep the test implementation
and assertions unchanged.
- Around line 163-178: Strengthen the compatibility-recreate test for
decideSandboxResume by using the table’s _noteFragment parameter to assert that
note contains the expected fragment, and require removeRegistryEntry to equal
false. Keep the existing recreate-kind assertion and verify these concrete
metadata values for every drift case.
🪄 Autofix

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: fbe2284b-915a-4443-9a61-cf75552e76a5

📥 Commits

Reviewing files that changed from the base of the PR and between c81f7fc and 18ef9e3.

📒 Files selected for processing (2)
  • src/lib/onboard/machine/handlers/sandbox-resume.test.ts
  • src/lib/onboard/machine/handlers/sandbox-resume.ts

Comment thread src/lib/onboard/machine/handlers/sandbox-resume.test.ts Outdated
Comment thread src/lib/onboard/machine/handlers/sandbox-resume.test.ts Outdated
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · medium confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 3 warnings · 0 suggestions
  • Model comparison: normalized findings differ; normalized terminology decisions differ; normalized E2E selections differ; Nemotron reported the same number of blockers, 3 more warnings, the same number of suggestions.
1 terminology difference from the second opinion

Advisory only. These are normalized differences from the primary terminology receipt.

  • requiresUnownedNotReadyRepair at src/lib/onboard/machine/handlers/sandbox-resume.ts:282: selected only by the second-opinion lane as define.
1 additional E2E selection from the second opinion

Advisory only. The primary lane did not select these E2E jobs or targets.

  • sandbox-operations: The completed second-opinion lane identified E2E coverage that the primary lane omitted.

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

Since last review: 0 prior items resolved · 0 still apply · 0 new items found

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite against this exact revision.

Recommended E2E: cloud-onboard, onboard-repair, onboard-resume, openshell-gateway-upgrade, ubuntu-repo-cloud-langchain-deepagents-code

2 optional E2E recommendations
  • rebuild-hermes
  • rebuild-openclaw

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

Address PR review advisor PRA-2 warning: add test coverage ensuring
runtime-configuration decisions (recreateSandboxRequested, web search,
GPU, messaging, Hermes gateway, observability) and tool-disclosure
decisions (migration, change) take precedence over the not-ready repair
fallback. Also rename the DCode auto-approval test to accurately reflect
the repair-and-recreate outcome (PRA-1 suggestion).

Signed-off-by: Jessica Yaunches <jyaunches@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/sandbox-resume.test.ts`:
- Around line 182-202: Update the parameterized cases in the decideSandboxResume
tests to include an expected diagnostic-note fragment for each drift scenario,
including the cases around the second assertion block. Replace each
expect.any(String) note matcher with
expect.stringContaining(expectedNoteFragment), while preserving the existing
kind and exact removeRegistryEntry assertions.
🪄 Autofix

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: 200fc238-e218-4a25-89ac-73b498592c8a

📥 Commits

Reviewing files that changed from the base of the PR and between 18ef9e3 and fd475a0.

📒 Files selected for processing (1)
  • src/lib/onboard/machine/handlers/sandbox-resume.test.ts

Comment thread src/lib/onboard/machine/handlers/sandbox-resume.test.ts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@jyaunches
jyaunches merged commit 8426e97 into NVIDIA:main Aug 6, 2026
69 checks passed
@cjagwani cjagwani mentioned this pull request Aug 7, 2026
23 tasks
cjagwani added a commit that referenced this pull request Aug 7, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Adds the canonical dated changelog entry required before cutting
`v0.0.104`.
The entry reconciles user-facing changes merged from `v0.0.103` through
`8d2b86aaf44968b4f7bc3b714222a73bd28e0403` while excluding hidden and
experimental product surfaces.

## Changes

- Added `docs/changelog/2026-08-06.mdx` with the exact `## v0.0.104`
heading and release themes for local inference, private endpoints,
network policy, state authority, lifecycle recovery, uninstall, Hermes,
MCP diagnostics, credential safety, and installation guidance.
- Source summary links:
- [#8399](#8399) ->
`docs/changelog/2026-08-06.mdx`: fixed DGX Spark local serving profiles.
- [#8418](#8418) ->
`docs/changelog/2026-08-06.mdx`: durable llama.cpp lifecycle management.
- [#8422](#8422) ->
`docs/changelog/2026-08-06.mdx`: recoverable llama.cpp receipt
publication.
- [#8402](#8402) ->
`docs/changelog/2026-08-06.mdx`: remediable DGX Spark storage admission.
- [#8391](#8391) ->
`docs/changelog/2026-08-06.mdx`: host-local serving recipe contracts.
- [#8401](#8401) ->
`docs/changelog/2026-08-06.mdx`: serving profile lifecycle provenance.
- [#8322](#8322) ->
`docs/changelog/2026-08-06.mdx`: guarded llama.cpp route compatibility.
- [#8272](#8272) ->
`docs/changelog/2026-08-06.mdx`: explicitly trusted private endpoints
with stable policy pins and CA trust.
- [#8431](#8431) ->
`docs/changelog/2026-08-06.mdx`: Personal onboarding policy tier and its
trust boundary.
- [#8143](#8143) ->
`docs/changelog/2026-08-06.mdx`: manifest-derived state authority.
- [#7859](#7859) ->
`docs/changelog/2026-08-06.mdx`: side-effect-free lifecycle lock
timeouts.
- [#8262](#8262) ->
`docs/changelog/2026-08-06.mdx`: managed gateway lease waiting.
- [#8339](#8339) ->
`docs/changelog/2026-08-06.mdx`: continued journaled rebuild recreation.
- [#8373](#8373) ->
`docs/changelog/2026-08-06.mdx`: restore readiness after compatibility
decisions.
- [#8443](#8443) ->
`docs/changelog/2026-08-06.mdx`: fail-closed malformed registry
handling.
- [#8419](#8419) ->
`docs/changelog/2026-08-06.mdx`: bounded recovery for a gateway that
never served.
- [#8486](#8486) ->
`docs/changelog/2026-08-06.mdx`: target-scoped registry recovery.
- [#8259](#8259) ->
`docs/changelog/2026-08-06.mdx`: scoped uninstall ordering and retry
safety.
- [#8457](#8457) ->
`docs/changelog/2026-08-06.mdx`: desktop metadata exclusion during
uninstall.
- [#8026](#8026) ->
`docs/changelog/2026-08-06.mdx`: typed Hermes configuration policy.
- [#8242](#8242) ->
`docs/changelog/2026-08-06.mdx`: Hermes WhatsApp session diagnostics.
- [#8344](#8344) ->
`docs/changelog/2026-08-06.mdx`: patched Hermes image and dependency
checks.
- [#8491](#8491) ->
`docs/changelog/2026-08-06.mdx`: bounded MCP discovery timeout.
- [#8490](#8490) ->
`docs/changelog/2026-08-06.mdx`: MCP shadow diagnostics.
- [#7619](#7619) ->
`docs/changelog/2026-08-06.mdx`: web-search credential isolation.
- [#8476](#8476) ->
`docs/changelog/2026-08-06.mdx`: stable preflight advisory identifiers.
- [#8452](#8452) ->
`docs/changelog/2026-08-06.mdx`: user-local CLI resolution.
- [#8481](#8481) ->
`docs/changelog/2026-08-06.mdx`: remote network-policy terminal
guidance.
- Product-scope exclusions:
[#8429](#8429) remains
experimental; [#8261](#8261)
remains feature-gated; and portable-profile changes
[#8408](#8408),
[#8415](#8415),
[#8446](#8446),
[#8458](#8458),
[#8462](#8462), and
[#8506](#8506) are not promoted
as supported product surfaces.

## Type of Change

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

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [x] Existing tests cover changed behavior — justification: `npx vitest
run test/changelog-docs.test.ts` passed 6/6 and validates dated
changelog structure and published links.
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Documentation Writer Review

- [x] Documentation writer subagent reviewed the completed changes
- Result: `docs-updated`
- Evidence: `docs/changelog/2026-08-06.mdx`; release-range scope,
writing rules, documentation style, skip terms, exact names,
threat-boundary wording, and published routes reviewed; changelog tests
and docs build passed.
- Agent: Codex Desktop
<!-- docs-review-head-sha: 02b51ae -->
<!-- docs-review-agents-blob-sha: c69aad4 -->

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit: Not applicable; no DGX Station host preparation script
changed.
- Station profile/scenario: Not applicable.
- Result: Not applicable.
- Supporting evidence: Not applicable.

## Verification

- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run validate:pr` passed after refreshing `origin/main` 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 test/changelog-docs.test.ts` passed 6/6.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result: Not applicable to a single
changelog entry.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [x] `npm run docs` builds without warnings (doc changes only)
- [x] 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)

The new dated changelog file includes the required parser-safe SPDX
header and intentionally has no frontmatter, matching the changelog
contract and existing entries.

---
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>


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

## Summary by CodeRabbit

* **Documentation**
  * Added release notes for v0.0.104.
* Documented fixes for local model runtimes, private endpoints, network
policies, state recovery, uninstall behavior, safety updates, MCP
diagnostics, credential isolation, and installation guidance.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@wscurran wscurran added the bug-fix PR fixes a bug or regression label Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants