Skip to content

fix(rebuild): name the sandbox in the printed resume recovery command (#5801) - #6835

Merged
cv merged 2 commits into
NVIDIA:mainfrom
Hokonoken:fix/rebuild-recovery-name-5801
Jul 15, 2026
Merged

fix(rebuild): name the sandbox in the printed resume recovery command (#5801)#6835
cv merged 2 commits into
NVIDIA:mainfrom
Hokonoken:fix/rebuild-recovery-name-5801

Conversation

@Hokonoken

@Hokonoken Hokonoken commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

When a rebuild's recreate step hits a transient fault after the old sandbox was deleted but before create recorded its name, the printed manual-recovery step 2 (nemoclaw onboard --resume) fails verbatim with no sandbox name was recorded. Re-run with --name, even though the next printed line asserts the command "will recreate sandbox ''". This carries --name <sandbox> on the resume retry form so the documented recovery runs as printed.

Related Issue

Addresses the residual-recovery gap discussed in #5801 (the non-atomic rebuild window from #2273 / #5787). This does not close #5801 — the atomic build-verify-swap it asks for still needs upstream work — it makes the existing preserved-backup mitigation actually followable.

Changes

  • src/lib/actions/sandbox/rebuild-mcp-phase.ts: the no-MCP onboard --resume fallback in printMcpRebuildRetryCommand now emits --name <sandbox>. The MCP-bearing form already carried the name via the <name> rebuild subcommand; only this fallback dropped it.
  • Tests updated/added in rebuild-mcp-phase.test.ts and the shared test/helpers/rebuild-flow-recovery-cases.ts to lock the sandbox name into the printed resume command.

No new abstraction, config, or fallback path — a one-argument fix to an existing printed command.

Type of Change

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

Quality Gates

  • Tests added or updated for changed behavior
  • Docs not applicable — justification: no user-facing docs describe this internal recovery print; the change makes an existing printed command runnable.
  • 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: requesting maintainer review; change is limited to recovery-guidance text plus its tests, no lifecycle/side-effect behavior.

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 — npm run check:diff exit 0
  • Targeted behavior tests pass for the current change set — npx vitest run src/lib/actions/sandbox/rebuild-mcp-phase.test.ts (9 passed) and every rebuild-flow / recovery-helper file consuming the shared assertion passes in isolation (rebuild-flow.test.ts plus the 8 rebuild-* files that use rebuild-flow-recovery-cases.ts). Note for honesty: running all of those files together under full parallel load intermittently trips pre-existing 5000 ms-timeout flakiness in unrelated DCode/provider tests (rebuild-dcode-*, rebuild-local-provider-recreate); those pass individually, live in files this change does not modify (it only edits a print function and its two test files), and fail with variable membership run-to-run — the signature of a load/timeout flake rather than a regression.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed

Reproduction / evidence

Verified by execution on main (3de1de6b1), docker driver, with a shim that fails only openshell sandbox create (a post-delete transient fault):

  • Before: rebuild prints 2. Run: nemoclaw onboard --resume --tool-disclosure progressive; running it verbatim fails with Cannot resume non-interactive onboard: ... no sandbox name was recorded. Re-run with --name <sandbox>.
  • After: the compiled printer emits nemoclaw onboard --resume --name rebuild-probe --tool-disclosure progressive; that exact command completed recovery live (sandbox returned Ready/healthy), followed by snapshot restore of the preserved backup (14 directories, 1 file).

Signed-off-by: Hokonoken 41166525+Hokonoken@users.noreply.github.com

…NVIDIA#5801)

When a rebuild's recreate step hits a transient fault after the old sandbox
was deleted but before create recorded its name, the resumable onboard
session has no sandbox name to resume. The printed manual-recovery step 2
(`nemoclaw onboard --resume`) then fails verbatim with "no sandbox name was
recorded. Re-run with --name", even though the very next printed line asserts
it "will recreate sandbox '<name>'".

Carry --name <sandbox> on the resume retry form so the documented recovery
command runs as printed. The name is already known (same message and the
retained registry row). The MCP-bearing retry form already carried the name
via the rebuild subcommand; only the onboard --resume fallback dropped it.

Signed-off-by: Hokonoken <41166525+Hokonoken@users.noreply.github.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 14, 2026

Copy link
Copy Markdown

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

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The MCP rebuild retry command now passes the sandbox name to onboard --resume. Unit and recovery tests were updated to verify the new command format while preserving existing disclosure and observability options.

Changes

Sandbox rebuild retry

Layer / File(s) Summary
Include sandbox name in resume retry command
src/lib/actions/sandbox/rebuild-mcp-phase.ts, src/lib/actions/sandbox/rebuild-mcp-phase.test.ts, test/helpers/rebuild-flow-recovery-cases.ts
The printed resume command includes --name ${sandboxName}, and tests validate the sandbox name alongside existing retry options.

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

Suggested labels: bug-fix, area: sandbox

Suggested reviewers: ericksoa, cv

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR only fixes the printed recovery command; it does not implement the atomic build-verify-swap health-before-delete flow requested in #5801. Implement the atomic build-verify-swap flow requested in #5801, or relink the issue if this PR is only a mitigation.
✅ Passed checks (4 passed)
Check name Status Explanation
Out of Scope Changes check ✅ Passed The changes stay focused on the recovery command and its tests, with no unrelated code paths or broad refactors.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding the sandbox name to the printed resume recovery command.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / high confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: onboard-repair, onboard-resume, state-backup-restore, upgrade-stale-sandbox

1 optional E2E recommendation
  • sandbox-rebuild

Workflow run details

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

@wscurran wscurran added area: cli Command line interface, flags, terminal UX, or output area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression labels Jul 14, 2026
@prekshivyas prekshivyas self-assigned this Jul 14, 2026

@prekshivyas prekshivyas 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 91a1c72a1bc44c7227653227cc15b33195b7fed1.

No findings. The recovery command now carries the validated sandbox name into the incomplete-create resume path, where the session cannot supply it, and the added assertions cover the resulting command. The broader atomic-rebuild work in #5801 remains correctly out of scope.

Validation:

  • GitHub CI, security scans, both PR Advisors, CodeRabbit, macOS E2E, and WSL E2E are green.
  • The credentialed E2E gate is green via the protected fork-skip approval from @cv; credentialed targets did not execute. The PR includes live Linux recreate/resume/restore evidence.
  • Local build:cli, typecheck:cli, check:diff, and 115 focused rebuild/onboard tests passed.

Approved as merge-ready.

@cv
cv merged commit 8599fc7 into NVIDIA:main Jul 15, 2026
43 checks passed
cv pushed a commit that referenced this pull request Jul 16, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Adds the canonical `docs/changelog/2026-07-15.mdx` entry with the exact
`## v0.0.84` heading for the release candidate range from `v0.0.83`
through `710d2b36b9eebcb6bca3c2b2f796a1bdb69c3a31`.
Fills two owner-page gaps for model-aware local inference health and
pre-write OpenClaw candidate validation.

## Changes

- Add the complete shared Fern changelog entry for `v0.0.84`, with
literal CLI names and root-absolute OpenClaw and Hermes routes.
- Document that sandbox status and doctor compare the configured Ollama
or vLLM model with provider inventory without issuing a completion.
- Document that host-side OpenClaw `config set` validates the complete
candidate before replacing live config or reaching gateway restart.
- Reconcile the `v0.0.84` release label with the commit range. PR #6773
is already contained in `v0.0.83` and remains documented there; CI,
test-harness, docs-infrastructure, and `.js` to `.mts` migration-only
changes require no additional user guidance.

### Source summary

- [#6882](#6882) ->
`docs/manage-sandboxes/backup-restore.mdx`,
`docs/changelog/2026-07-15.mdx`: Explain that OpenClaw runtime identity
and pairing state are excluded from snapshots and ignored during
restore.
- [#6873](#6873) ->
`docs/inference/set-up-ollama.mdx`, `docs/changelog/2026-07-15.mdx`:
Record the Ollama requested-model environment fallback and interactive
default.
- [#6835](#6835) ->
`docs/changelog/2026-07-15.mdx`: Include the sandbox name in the
documented rebuild resume-recovery behavior.
- [#6886](#6886) ->
`docs/inference/custom-endpoint-security.mdx`,
`docs/inference/set-up-openai-compatible-endpoint.mdx`,
`docs/changelog/2026-07-15.mdx`: Explain the exact-host trusted-private
endpoint opt-in and retained SSRF boundaries.
- [#6887](#6887) ->
`docs/reference/commands.mdx`, `docs/changelog/2026-07-15.mdx`: Document
Telegram channel health verdicts, summary behavior, and exit status.
- [#6863](#6863) ->
`docs/manage-sandboxes/lifecycle.mdx`, `docs/changelog/2026-07-15.mdx`:
Add the missing model-inventory behavior for local status and doctor
checks.
- [#6902](#6902) ->
`docs/manage-sandboxes/runtime-controls.mdx`,
`docs/changelog/2026-07-15.mdx`: Add the missing pre-write OpenClaw
candidate-validation contract.
- [#6916](#6916) ->
`docs/changelog/2026-07-15.mdx`: Preserve the failed-session
fresh-install recovery correction in the release entry.
- [#6934](#6934) ->
`docs/reference/commands.mdx`, `docs/reference/troubleshooting.mdx`,
`docs/security/credential-storage.mdx`, `docs/changelog/2026-07-15.mdx`:
Summarize completed-prompt checkpointing and validated credential reuse
during OpenClaw resume.
- [#6898](#6898) ->
`docs/inference/switch-models.mdx`,
`docs/inference/switch-providers.mdx`,
`docs/reference/troubleshooting.mdx`, `docs/changelog/2026-07-15.mdx`:
Explain Hermes dashboard convergence after in-place inference changes.
- [#6711](#6711) ->
`docs/manage-sandboxes/run-sandboxes.mdx`,
`docs/manage-sandboxes/uninstall-nemoclaw.mdx`,
`docs/reference/architecture.mdx`, `docs/reference/commands.mdx`,
`docs/changelog/2026-07-15.mdx`: Summarize port-scoped host state and
uninstall preservation.
- [#6767](#6767) ->
`docs/inference/configure-model-limits.mdx`,
`docs/inference/set-up-ollama.mdx`,
`docs/reference/troubleshooting.mdx`, `docs/changelog/2026-07-15.mdx`:
Record the Hermes `64000`-token Ollama floor and unchanged OpenClaw
floor.
- [#6862](#6862) ->
`docs/get-started/quickstart.mdx`,
`docs/inference/verify-inference-route.mdx`,
`docs/changelog/2026-07-15.mdx`: Explain retryable not-ready
finalization for unhealthy inference routes.
- [#6766](#6766) ->
`docs/security/tcb-boundary.mdx`, `docs/changelog/2026-07-15.mdx`:
Document definitive stale transition-lock recovery and fail-closed
ambiguous cases.
- [#6948](#6948) ->
`docs/manage-sandboxes/manage-mcp-servers.mdx`,
`docs/changelog/2026-07-15.mdx`: Include Hermes MCP apply-state race
recovery in the release entry without changing the established user
workflow.
- [#6964](#6964) ->
`docs/reference/troubleshooting.mdx`, `docs/changelog/2026-07-15.mdx`:
Record complete agent-specific fresh-install and resume recovery
commands.
- [#6883](#6883) ->
`docs/get-started/quickstart.mdx`, `docs/inference/set-up-vllm.mdx`,
`docs/reference/platform-support.mdx`, `docs/changelog/2026-07-15.mdx`:
Summarize the DGX Station Nemotron Ultra express path and pinned
managed-vLLM recipe.
- [#6985](#6985) ->
`docs/inference/set-up-vllm.mdx`, `docs/reference/commands.mdx`,
`docs/changelog/2026-07-15.mdx`: Capture the final automated and
interactive storage-warning behavior.

## 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 —
`test/changelog-docs.test.ts` validates the dated-entry structure, exact
version heading, and preserved history.
- [ ] 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:

## 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 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
test/changelog-docs.test.ts` (6 passed)
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — not run for this doc-only change.
- [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) —
completed with 0 errors; Fern reported the unchanged unauthenticated
redirect-check and light-theme contrast warnings.
- [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 native changelog entry uses the required parser-safe MDX SPDX
comment and intentionally has no frontmatter.

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


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

## Summary by CodeRabbit

* **Documentation**
* Added the v0.0.84 changelog entry covering setup, endpoint onboarding,
model handling, sandbox readiness, recovery, channel status, and
configuration safeguards.
* Clarified that sandbox health checks validate configured models
against local Ollama and vLLM provider inventories without generating
completions or consuming tokens.
* Documented that invalid runtime configuration changes are rejected
while preserving the existing working configuration.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli Command line interface, flags, terminal UX, or output area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rebuild/upgrade: atomic build-verify-swap before deleting old sandbox (health-before-delete)

4 participants