fix(sandbox): repair legacy gateway upgrade recovery - #8735
Merged
Conversation
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Contributor
📝 WalkthroughWalkthroughThe installer now reuses validated pre-upgrade backups for stale or non-Ready sandbox recovery. Recovery tests cover stale, missing-backup, and disabled-restore cases. Gateway upgrade fixtures now select legacy Docker networks by gateway reference. ChangesSandbox recovery
Legacy gateway network selection
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Installer
participant RecoveryLogic
participant BackupStore
participant ReplacementGateway
participant Sandbox
Installer->>RecoveryLogic: assess stale or non-Ready sandbox
RecoveryLogic->>BackupStore: load validated pre-upgrade backup
BackupStore-->>RecoveryLogic: return matching backup
RecoveryLogic->>ReplacementGateway: recreate sandbox with recovery manifest
ReplacementGateway-->>Sandbox: report Ready or Running state
RecoveryLogic-->>Installer: report recovery result
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
apurvvkumaria
enabled auto-merge (squash)
August 10, 2026 11:47
Contributor
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-8735.docs.buildwithfern.com/nemoclaw |
sandl99
approved these changes
Aug 10, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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 `@docs/manage-sandboxes/update-sandboxes.mdx`:
- Around line 81-85: Update the recovery requirements in the paragraph beginning
“During installer-driven recovery” to state that registry evidence of
NemoClaw-managed image ownership is required unless the exact-name
pre-fingerprint OpenClaw or Hermes confirmation exception applies. Align the
wording with the recovery behavior described in the surrounding lines so this
supported fallback remains explicitly available.
🪄 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: fe22a6b9-a389-4623-8ef8-d49b9c3f0f6e
📒 Files selected for processing (7)
docs/manage-sandboxes/update-sandboxes.mdxscripts/install.shsrc/lib/actions/upgrade-sandboxes-recovery.test.tssrc/lib/actions/upgrade-sandboxes.tstest/e2e/live/openshell-gateway-upgrade-helpers.tstest/e2e/live/openshell-gateway-upgrade.test.tstest/e2e/support/openshell-gateway-upgrade-workflow-boundary.test.ts
Comment on lines
+81
to
+85
| During installer-driven recovery, each stale or non-Ready sandbox requires a validated latest backup. | ||
| The backup's sandbox and agent identities must match the registry. | ||
| The registry must also contain positive evidence that NemoClaw managed the sandbox image. | ||
| If the replacement gateway reports a stale sandbox as Ready or Running, the installer reuses the validated pre-upgrade backup. | ||
| It does not attempt another backup from the replaced legacy runtime. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
State the managed-image confirmation exception.
Line 83 says that positive registry evidence is always required. The recovery code also permits an exact-name pre-fingerprint OpenClaw or Hermes confirmation when that registry evidence is absent. This conflicts with lines 87-92 and can make the supported recovery path appear unavailable.
Proposed fix
- The registry must also contain positive evidence that NemoClaw managed the sandbox image.
+ The registry must contain positive evidence that NemoClaw managed the sandbox image, unless a listed pre-fingerprint OpenClaw or Hermes entry has explicit managed-image confirmation.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| During installer-driven recovery, each stale or non-Ready sandbox requires a validated latest backup. | |
| The backup's sandbox and agent identities must match the registry. | |
| The registry must also contain positive evidence that NemoClaw managed the sandbox image. | |
| If the replacement gateway reports a stale sandbox as Ready or Running, the installer reuses the validated pre-upgrade backup. | |
| It does not attempt another backup from the replaced legacy runtime. | |
| During installer-driven recovery, each stale or non-Ready sandbox requires a validated latest backup. | |
| The backup's sandbox and agent identities must match the registry. | |
| The registry must contain positive evidence that NemoClaw managed the sandbox image, unless a listed pre-fingerprint OpenClaw or Hermes entry has explicit managed-image confirmation. | |
| If the replacement gateway reports a stale sandbox as Ready or Running, the installer reuses the validated pre-upgrade backup. | |
| It does not attempt another backup from the replaced legacy runtime. |
🤖 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 `@docs/manage-sandboxes/update-sandboxes.mdx` around lines 81 - 85, Update the
recovery requirements in the paragraph beginning “During installer-driven
recovery” to state that registry evidence of NemoClaw-managed image ownership is
required unless the exact-name pre-fingerprint OpenClaw or Hermes confirmation
exception applies. Align the wording with the recovery behavior described in the
surrounding lines so this supported fallback remains explicitly available.
Contributor
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 1 semantic terminology decisionTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
This was referenced Aug 10, 2026
23 tasks
cv
pushed a commit
that referenced
this pull request
Aug 10, 2026
<!-- markdownlint-disable MD041 --> ## Summary This docs-only follow-up to #8735 clarifies the managed-image evidence required during installer-driven legacy gateway recovery. OpenClaw and Hermes retain the exact-name pre-fingerprint confirmation exception, while Deep Agents still requires positive registry evidence. ## Changes - Render the pre-fingerprint confirmation exception only in the OpenClaw and Hermes guide variants. - State that explicit managed-image confirmation is the only exception to the registry evidence requirement. ## 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 - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: Docs-only clarification of behavior already tested and merged in #8735. - [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/manage-sandboxes/update-sandboxes.mdx` - Agent: Codex Desktop <!-- docs-review-head-sha: a3a5654 --> <!-- docs-review-agents-blob-sha: c4923a3 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## 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: `npm run docs` and `npx prek run --files docs/manage-sandboxes/update-sandboxes.mdx` passed; generated variants preserve the agent-specific requirements. - [ ] 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 this one-page docs-only follow-up. - [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) — passed with Fern's two existing hidden 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) --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
wscurran
added
bug-fix
23 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This change repairs two legacy OpenShell gateway upgrade failures from the live E2E workflow. The v0.0.36 fixture now probes its historical Docker network, and installer-driven recovery reuses the validated pre-upgrade backup for stale Ready or Running sandboxes instead of attempting a second backup from the replaced runtime.
Changes
openshell-cluster-nemoclaw. That cluster-era gateway creates a gateway-named Docker network, while newer fixtures still require the sharedopenshell-dockerdefault. The OpenShell gateway upgrade boundary test protects this compatibility path.Type of Change
Quality Gates
Documentation Writer Review
docs-updateddocs/manage-sandboxes/update-sandboxes.mdxDGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpm run typecheck:cli,npm run check:installer-hash, and semantic E2E checks also passed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Not run because the change is confined to one upgrade recovery path and one live fixture boundary; targeted tests and normal hooks passed.npm run docsbuilds without warnings (doc changes only) — the docs build passed with Fern's two existing hidden warnings.Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests