fix(sandbox): recover relaunches a restart-killed gateway supervisor - #6677
Merged
Conversation
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughManaged recovery now distinguishes a missing in-sandbox supervisor, reconstructs a credential-free startup environment, performs pinned transactional container recreation, validates managed gateway readiness, and documents rollback and fallback behavior. ChangesSupervisor Relaunch Recovery
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant checkAndRecoverSandboxProcesses
participant relaunchManagedSupervisorSession
participant Docker
participant ManagedGateway
checkAndRecoverSandboxProcesses->>relaunchManagedSupervisorSession: handle SUPERVISOR_NOT_RUNNING
relaunchManagedSupervisorSession->>Docker: recreate pinned legacy container
Docker-->>relaunchManagedSupervisorSession: replacement container
relaunchManagedSupervisorSession->>ManagedGateway: probe supervisor and gateway health
ManagedGateway-->>relaunchManagedSupervisorSession: readiness result
relaunchManagedSupervisorSession-->>checkAndRecoverSandboxProcesses: finalize commit or rollback
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-6677.docs.buildwithfern.com/nemoclaw |
laitingsheng
added
NV QA
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
Contributor
PR Review Advisor (Nemotron Ultra) — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
Contributor
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings This is an automated review. Required findings need action before merge. Warnings and optional suggestions do not require a response or follow-up. A human maintainer makes the final merge decision. |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/actions/sandbox/process-recovery.ts (1)
938-967: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRebuild hint is skipped when relaunch "succeeds" but the gateway stays unhealthy.
managedRecoveryFailureLayeris captured fromonFailureLayereven on the"relaunched"success path (it's set unconditionally before the relaunch attempt at lines 513-519). But the call toprintHostManagedGatewayRecoveryHintsreached when the post-relaunch health probe still fails (inside therecoveryKind !== nullbranch, around line 964) doesn't passmanagedRecoveryFailureLayer, so it falls back to the generic "Retry the managed restart from the host" hint instead of the rebuild/re-onboard guidance — even though we already know the supervisor had to be relaunched. Since relaunch intentionally omits credentials, it's plausible the daemon starts but the gateway never becomes healthy, landing users back in the circular-guidance situation issue#6635was meant to fix. This scenario is also not covered bytest/process-recovery-supervisor-relaunch.test.ts(the second test exercises exactly this "relaunched but still STOPPED" case but only asserts on the spawned command, not the printed hints).🐛 Proposed fix
if ( !waitForRecoveredSandboxGateway(sandboxName, { quiet, initialManagedHealthPassed: recoveryKind === "managed", timeoutSeconds: gatewayRecoveryTimeoutSeconds(recoveryAgent), managedProbeImpl: (name) => confirmRecoveredSandboxGatewayManaged(name, { requestGatewaySupervisorActionImpl: requestGatewaySupervisorAction, }), }) ) { if (!quiet) { console.error(" Gateway process started but is not responding."); printGatewayWedgeDiagnostics(sandboxName, executeSandboxExecCommand); console.error(" Check /tmp/gateway.log inside the sandbox for details."); - printHostManagedGatewayRecoveryHints(sandboxName, recoveryAgent); + printHostManagedGatewayRecoveryHints(sandboxName, recoveryAgent, managedRecoveryFailureLayer); }Note: the specialized hint's wording ("cannot relaunch it") may need a small tweak to also fit the "relaunch started but gateway never became healthy" case.
Also applies to: 1022-1025
🤖 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/actions/sandbox/process-recovery.ts` around lines 938 - 967, Pass the captured managedRecoveryFailureLayer into printHostManagedGatewayRecoveryHints in both unhealthy-gateway paths, including the post-relaunch health-check failure and the corresponding later branch around the recovery result handling. Update the specialized hint wording if needed so it accurately covers a relaunch that started but left the gateway unhealthy, while preserving the existing generic guidance for cases without a managed failure layer.
🤖 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/lifecycle.mdx`:
- Around line 233-234: Replace the em dashes in both lifecycle documentation
copies, including the sentences describing supervisor recovery and gateway
failure, with permitted punctuation such as commas or parentheses. Preserve the
existing meaning and command examples.
---
Outside diff comments:
In `@src/lib/actions/sandbox/process-recovery.ts`:
- Around line 938-967: Pass the captured managedRecoveryFailureLayer into
printHostManagedGatewayRecoveryHints in both unhealthy-gateway paths, including
the post-relaunch health-check failure and the corresponding later branch around
the recovery result handling. Update the specialized hint wording if needed so
it accurately covers a relaunch that started but left the gateway unhealthy,
while preserving the existing generic guidance for cases without a managed
failure layer.
🪄 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: 396b8197-d2cf-48c7-a83e-1511d9cdf1eb
📒 Files selected for processing (8)
docs/manage-sandboxes/lifecycle.mdxdocs/reference/commands.mdxsrc/lib/actions/sandbox/gateway-restart.test.tssrc/lib/actions/sandbox/gateway-restart.tssrc/lib/actions/sandbox/process-recovery.tssrc/lib/onboard/sandbox-create-launch.test.tssrc/lib/onboard/sandbox-create-launch.tstest/process-recovery-supervisor-relaunch.test.ts
…very-after-restart
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/actions/sandbox/supervisor-relaunch.test.ts (1)
35-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a direct unit test for the success path.
Both tests here only cover failure branches (missing sandbox, transport throw). There's no direct test asserting
relaunchManagedSupervisorSessionreturnstruewhenspawnSyncsucceeds with theNEMOCLAW_SUPERVISOR_RELAUNCHEDmarker, nor a direct test of theNEMOCLAW_DISABLE_SUPERVISOR_RELAUNCH=1short-circuit. These are exercised only indirectly through the heavier integration test intest/process-recovery-supervisor-relaunch.test.ts. A focused positive-path test here would pin down the core contract cheaply and independently of the integration flow.As per path instructions, sandbox lifecycle behavior review requires tests to "cover failure, recovery, rebuild, and resume behavior without bypassing the public action boundary."
🤖 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/actions/sandbox/supervisor-relaunch.test.ts` around lines 35 - 59, Add focused unit coverage to the relaunchManagedSupervisorSession suite for the successful spawnSync path, asserting it returns true when the output contains NEMOCLAW_SUPERVISOR_RELAUNCHED. Also add a direct test for NEMOCLAW_DISABLE_SUPERVISOR_RELAUNCH=1 confirming the action returns false without invoking spawnSync, while retaining the existing failure-path tests.Source: Path instructions
🤖 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 `@test/process-recovery-supervisor-relaunch.test.ts`:
- Around line 202-218: Update the spawnSync mock in the relaunch test to
distinguish calls by the documented observable contract or expected call order,
not the literal “nemoclaw-start” and “setsid” command text. Verify that the
HTTP_CODE=$(curl branch is exercised in this scenario and return output matching
the captured HTTP status expected by the code under test, removing or correcting
the STOPPED stub if unused. Keep isLocalForwardReachable and captureOpenshell
behavior intact while ensuring the test still exercises the supervisor recovery
path.
- Around line 185-189: Update the requireSource calls for openshellRuntime,
agentRuntime, registry, and forwardHealth to reference their .ts source modules
instead of .js paths; leave the unrelated childProcess import unchanged.
---
Nitpick comments:
In `@src/lib/actions/sandbox/supervisor-relaunch.test.ts`:
- Around line 35-59: Add focused unit coverage to the
relaunchManagedSupervisorSession suite for the successful spawnSync path,
asserting it returns true when the output contains
NEMOCLAW_SUPERVISOR_RELAUNCHED. Also add a direct test for
NEMOCLAW_DISABLE_SUPERVISOR_RELAUNCH=1 confirming the action returns false
without invoking spawnSync, while retaining the existing failure-path tests.
🪄 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: 38b39e8a-09d9-446c-9ff3-4e716a8f57a3
📒 Files selected for processing (3)
src/lib/actions/sandbox/supervisor-relaunch.test.tssrc/lib/actions/sandbox/supervisor-relaunch.tstest/process-recovery-supervisor-relaunch.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/actions/sandbox/supervisor-relaunch.ts
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Collaborator
|
Addressed the provenance blocker in Verification on this revision:
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/lib/onboard/docker-startup-command-patch.test.ts (1)
239-244: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake the rollback proof target the original container.
This mock returns the same inspect fixture for every
inspectcommand, regardless of the requested container ID. The test would therefore pass even if production inspected the backup or another container instead ofold-container-id. Dispatch on the requested ID so the test validates the rollback boundary.Suggested test adjustment
- : args[0] === "inspect" + : args[0] === "inspect" && args[args.length - 1] === "old-container-id" ? JSON.stringify([inspectFixture()]) : "",🤖 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/docker-startup-command-patch.test.ts` around lines 239 - 244, Update the dockerCapture mock in the relevant test to inspect args[1] and return inspectFixture() only when the requested container ID is old-container-id; return an empty or appropriate non-matching result for other IDs. Keep the existing ps behavior unchanged so the rollback assertion specifically proves production targets the original container.Source: Path instructions
🤖 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/reference/commands.mdx`:
- Line 3141: Reformat the documentation text near the sandbox remote-binding
instructions so each of its three sentences occupies a separate source line,
preserving the wording and Markdown/MDX formatting.
In `@src/lib/onboard/docker-gpu-patch.ts`:
- Around line 1090-1093: Update the error message in the container
identity-mismatch path to state that the operation is pinned but the observed
Docker container identity differs from the expected identity. Remove the
misleading “unpinned startup-command recreation” wording while preserving the
existing sandbox name context and throw behavior.
In `@src/lib/sandbox/privileged-exec.ts`:
- Around line 223-228: Update the identity check in the privileged execution
path to treat only undefined expectedContainerId as omitted, while rejecting an
empty expectedContainerId instead of bypassing validation. Preserve the existing
mismatch error for non-empty IDs and ensure malformed or missing upstream
identities fail closed.
---
Nitpick comments:
In `@src/lib/onboard/docker-startup-command-patch.test.ts`:
- Around line 239-244: Update the dockerCapture mock in the relevant test to
inspect args[1] and return inspectFixture() only when the requested container ID
is old-container-id; return an empty or appropriate non-matching result for
other IDs. Keep the existing ps behavior unchanged so the rollback assertion
specifically proves production targets the original container.
🪄 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: f755b071-d8ef-4f4f-bae8-5d38e6ff8306
📒 Files selected for processing (26)
docs/manage-sandboxes/lifecycle.mdxdocs/reference/commands.mdxdocs/reference/troubleshooting.mdxscripts/managed-gateway-control.pysrc/lib/actions/sandbox/gateway-restart.test.tssrc/lib/actions/sandbox/gateway-restart.tssrc/lib/actions/sandbox/process-recovery.tssrc/lib/actions/sandbox/supervisor-relaunch.test.tssrc/lib/actions/sandbox/supervisor-relaunch.tssrc/lib/onboard/docker-gpu-diagnostic-redaction.test.tssrc/lib/onboard/docker-gpu-patch-finalize.test.tssrc/lib/onboard/docker-gpu-patch-finalize.tssrc/lib/onboard/docker-gpu-patch-validation.test.tssrc/lib/onboard/docker-gpu-patch.test.tssrc/lib/onboard/docker-gpu-patch.tssrc/lib/onboard/docker-gpu-pre-rollback-diagnostics.test.tssrc/lib/onboard/docker-gpu-supervisor-reconnect.tssrc/lib/onboard/docker-startup-command-patch.test.tssrc/lib/onboard/docker-startup-command-patch.tssrc/lib/onboard/docker-startup-command-sandbox-create.test.tssrc/lib/onboard/sandbox-create-launch.tssrc/lib/sandbox/privileged-exec.test.tssrc/lib/sandbox/privileged-exec.tstest/e2e/live/gateway-guard-recovery.test.tstest/managed-gateway-control.test.tstest/process-recovery-supervisor-relaunch.test.ts
✅ Files skipped from review due to trivial changes (2)
- src/lib/onboard/docker-startup-command-sandbox-create.test.ts
- docs/manage-sandboxes/lifecycle.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/actions/sandbox/gateway-restart.ts
- src/lib/onboard/sandbox-create-launch.ts
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/lib/onboard/docker-startup-command-patch.test.ts (1)
239-244: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake the rollback proof target the original container.
This mock returns the same inspect fixture for every
inspectcommand, regardless of the requested container ID. The test would therefore pass even if production inspected the backup or another container instead ofold-container-id. Dispatch on the requested ID so the test validates the rollback boundary.Suggested test adjustment
- : args[0] === "inspect" + : args[0] === "inspect" && args[args.length - 1] === "old-container-id" ? JSON.stringify([inspectFixture()]) : "",🤖 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/docker-startup-command-patch.test.ts` around lines 239 - 244, Update the dockerCapture mock in the relevant test to inspect args[1] and return inspectFixture() only when the requested container ID is old-container-id; return an empty or appropriate non-matching result for other IDs. Keep the existing ps behavior unchanged so the rollback assertion specifically proves production targets the original container.Source: Path instructions
🤖 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/reference/commands.mdx`:
- Line 3141: Reformat the documentation text near the sandbox remote-binding
instructions so each of its three sentences occupies a separate source line,
preserving the wording and Markdown/MDX formatting.
In `@src/lib/onboard/docker-gpu-patch.ts`:
- Around line 1090-1093: Update the error message in the container
identity-mismatch path to state that the operation is pinned but the observed
Docker container identity differs from the expected identity. Remove the
misleading “unpinned startup-command recreation” wording while preserving the
existing sandbox name context and throw behavior.
In `@src/lib/sandbox/privileged-exec.ts`:
- Around line 223-228: Update the identity check in the privileged execution
path to treat only undefined expectedContainerId as omitted, while rejecting an
empty expectedContainerId instead of bypassing validation. Preserve the existing
mismatch error for non-empty IDs and ensure malformed or missing upstream
identities fail closed.
---
Nitpick comments:
In `@src/lib/onboard/docker-startup-command-patch.test.ts`:
- Around line 239-244: Update the dockerCapture mock in the relevant test to
inspect args[1] and return inspectFixture() only when the requested container ID
is old-container-id; return an empty or appropriate non-matching result for
other IDs. Keep the existing ps behavior unchanged so the rollback assertion
specifically proves production targets the original container.
🪄 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: f755b071-d8ef-4f4f-bae8-5d38e6ff8306
📒 Files selected for processing (26)
docs/manage-sandboxes/lifecycle.mdxdocs/reference/commands.mdxdocs/reference/troubleshooting.mdxscripts/managed-gateway-control.pysrc/lib/actions/sandbox/gateway-restart.test.tssrc/lib/actions/sandbox/gateway-restart.tssrc/lib/actions/sandbox/process-recovery.tssrc/lib/actions/sandbox/supervisor-relaunch.test.tssrc/lib/actions/sandbox/supervisor-relaunch.tssrc/lib/onboard/docker-gpu-diagnostic-redaction.test.tssrc/lib/onboard/docker-gpu-patch-finalize.test.tssrc/lib/onboard/docker-gpu-patch-finalize.tssrc/lib/onboard/docker-gpu-patch-validation.test.tssrc/lib/onboard/docker-gpu-patch.test.tssrc/lib/onboard/docker-gpu-patch.tssrc/lib/onboard/docker-gpu-pre-rollback-diagnostics.test.tssrc/lib/onboard/docker-gpu-supervisor-reconnect.tssrc/lib/onboard/docker-startup-command-patch.test.tssrc/lib/onboard/docker-startup-command-patch.tssrc/lib/onboard/docker-startup-command-sandbox-create.test.tssrc/lib/onboard/sandbox-create-launch.tssrc/lib/sandbox/privileged-exec.test.tssrc/lib/sandbox/privileged-exec.tstest/e2e/live/gateway-guard-recovery.test.tstest/managed-gateway-control.test.tstest/process-recovery-supervisor-relaunch.test.ts
✅ Files skipped from review due to trivial changes (2)
- src/lib/onboard/docker-startup-command-sandbox-create.test.ts
- docs/manage-sandboxes/lifecycle.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/actions/sandbox/gateway-restart.ts
- src/lib/onboard/sandbox-create-launch.ts
🛑 Comments failed to post (3)
docs/reference/commands.mdx (1)
3141-3141: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Split multi-sentence line into one sentence per line.
Line 3141 contains three sentences on a single line.
✏️ Proposed fix
-Set `NEMOCLAW_DASHBOARD_BIND=0.0.0.0` before `$$nemoclaw onboard` to prepare the sandbox for remote exposure and bind the forward on all interfaces. Use the same setting for later `$$nemoclaw <sandbox> connect` calls. A sandbox created without this opt-in must be recreated with `NEMOCLAW_DASHBOARD_BIND=0.0.0.0 $$nemoclaw onboard --recreate-sandbox` before a remote-bind connect is allowed. +Set `NEMOCLAW_DASHBOARD_BIND=0.0.0.0` before `$$nemoclaw onboard` to prepare the sandbox for remote exposure and bind the forward on all interfaces. +Use the same setting for later `$$nemoclaw <sandbox> connect` calls. +A sandbox created without this opt-in must be recreated with `NEMOCLAW_DASHBOARD_BIND=0.0.0.0 $$nemoclaw onboard --recreate-sandbox` before a remote-bind connect is allowed.As per coding guidelines: "Keep one sentence per line in Markdown and MDX source files."
📝 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.Set `NEMOCLAW_DASHBOARD_BIND=0.0.0.0` before `$$nemoclaw onboard` to prepare the sandbox for remote exposure and bind the forward on all interfaces. Use the same setting for later `$$nemoclaw <sandbox> connect` calls. A sandbox created without this opt-in must be recreated with `NEMOCLAW_DASHBOARD_BIND=0.0.0.0 $$nemoclaw onboard --recreate-sandbox` before a remote-bind connect is allowed.🤖 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/reference/commands.mdx` at line 3141, Reformat the documentation text near the sandbox remote-binding instructions so each of its three sentences occupies a separate source line, preserving the wording and Markdown/MDX formatting.Source: Coding guidelines
src/lib/onboard/docker-gpu-patch.ts (1)
1090-1093: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify the identity-mismatch diagnostic.
The operation is pinned; it is being rejected because the observed container identity differs from the expected one. “Refusing an unpinned startup-command recreation” is misleading and can send recovery troubleshooting in the wrong direction.
Suggested wording
- "refusing an unpinned startup-command recreation.", + "refusing startup-command recreation because the container identity changed.",📝 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.throw new Error( `OpenShell Docker container identity changed for sandbox '${options.sandboxName}'; ` + "refusing startup-command recreation because the container identity changed.", );🤖 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/docker-gpu-patch.ts` around lines 1090 - 1093, Update the error message in the container identity-mismatch path to state that the operation is pinned but the observed Docker container identity differs from the expected identity. Remove the misleading “unpinned startup-command recreation” wording while preserving the existing sandbox name context and throw behavior.src/lib/sandbox/privileged-exec.ts (1)
223-228: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Make the identity pin fail closed for empty IDs.
expectedContainerId &&treats""as “not supplied,” so a malformed or missing upstream identity can silently disable the container check at this privileged boundary. Treat onlyundefinedas omitted and reject an empty expected ID.Proposed fix
- if (expectedContainerId && container !== expectedContainerId) { + if (expectedContainerId !== undefined && container !== expectedContainerId) {📝 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.if (expectedContainerId !== undefined && container !== expectedContainerId) { throw new Error( `OpenShell container identity changed for sandbox '${sandboxName}'; ` + "refusing privileged execution against a different container.", ); }🤖 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/sandbox/privileged-exec.ts` around lines 223 - 228, Update the identity check in the privileged execution path to treat only undefined expectedContainerId as omitted, while rejecting an empty expectedContainerId instead of bypassing validation. Preserve the existing mismatch error for non-empty IDs and ensure malformed or missing upstream identities fail closed.Source: Path instructions
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Collaborator
|
@coderabbitai review Addressed all findings from review 4679197751 in
The focused 95-test matrix, CLI type-check, docs build, test-conditional scan, diff-scoped gate, and normal pre-push checks pass on this revision. |
Contributor
|
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
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 `@docs/reference/commands.mdx`:
- Line 3121: Update the NEMOCLAW_DASHBOARD_BIND documentation to explicitly
scope the 0.0.0.0 remote-bind opt-in to non-WSL or other SSH-deployed hosts,
while preserving the stated WSL all-interface behavior when unset. Use direct,
active wording consistently across the referenced dashboard-binding
documentation entries.
- Around line 1036-1038: Update the missing-supervisor recovery description near
the managed controller flow to state that the two zero-supervisor scans are
unchanged and root-controller-attested, in addition to requiring a stable PID 1.
Keep the existing transactional recreation, credential-free startup, identity
pinning, rollback retention, health, settle-check, and writable-layer caveats
unchanged.
🪄 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: 6a5f2cec-9c27-4d71-b67a-f2095f895834
📒 Files selected for processing (27)
docs/manage-sandboxes/lifecycle.mdxdocs/reference/commands.mdxdocs/reference/troubleshooting.mdxscripts/managed-gateway-control.pysrc/lib/actions/sandbox/gateway-restart.test.tssrc/lib/actions/sandbox/gateway-restart.tssrc/lib/actions/sandbox/process-recovery.tssrc/lib/actions/sandbox/supervisor-relaunch.test.tssrc/lib/actions/sandbox/supervisor-relaunch.tssrc/lib/onboard/docker-gpu-diagnostic-redaction.test.tssrc/lib/onboard/docker-gpu-patch-finalize.test.tssrc/lib/onboard/docker-gpu-patch-finalize.tssrc/lib/onboard/docker-gpu-patch-validation.test.tssrc/lib/onboard/docker-gpu-patch.test.tssrc/lib/onboard/docker-gpu-patch.tssrc/lib/onboard/docker-gpu-pre-rollback-diagnostics.test.tssrc/lib/onboard/docker-gpu-supervisor-reconnect.tssrc/lib/onboard/docker-startup-command-patch.test.tssrc/lib/onboard/docker-startup-command-patch.tssrc/lib/onboard/docker-startup-command-sandbox-create.test.tssrc/lib/onboard/sandbox-create-launch.tssrc/lib/sandbox/privileged-exec.test.tssrc/lib/sandbox/privileged-exec.tstest/e2e/live/gateway-guard-recovery.test.tstest/e2e/mock-parity.jsontest/managed-gateway-control.test.tstest/process-recovery-supervisor-relaunch.test.ts
✅ Files skipped from review due to trivial changes (2)
- src/lib/onboard/docker-gpu-patch-validation.test.ts
- docs/manage-sandboxes/lifecycle.mdx
🚧 Files skipped from review as they are similar to previous changes (22)
- src/lib/onboard/docker-gpu-supervisor-reconnect.ts
- src/lib/sandbox/privileged-exec.test.ts
- src/lib/onboard/docker-gpu-diagnostic-redaction.test.ts
- src/lib/onboard/docker-gpu-patch-finalize.ts
- src/lib/actions/sandbox/gateway-restart.ts
- src/lib/actions/sandbox/supervisor-relaunch.test.ts
- src/lib/onboard/docker-startup-command-sandbox-create.test.ts
- src/lib/onboard/docker-startup-command-patch.ts
- docs/reference/troubleshooting.mdx
- src/lib/onboard/docker-gpu-pre-rollback-diagnostics.test.ts
- src/lib/sandbox/privileged-exec.ts
- src/lib/onboard/docker-gpu-patch-finalize.test.ts
- src/lib/onboard/sandbox-create-launch.ts
- src/lib/onboard/docker-gpu-patch.test.ts
- test/process-recovery-supervisor-relaunch.test.ts
- src/lib/actions/sandbox/gateway-restart.test.ts
- src/lib/onboard/docker-startup-command-patch.test.ts
- scripts/managed-gateway-control.py
- src/lib/actions/sandbox/supervisor-relaunch.ts
- test/managed-gateway-control.test.ts
- src/lib/onboard/docker-gpu-patch.ts
- src/lib/actions/sandbox/process-recovery.ts
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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 `@docs/reference/commands.mdx`:
- Line 3121: Update the NEMOCLAW_DASHBOARD_BIND documentation to explicitly
scope the 0.0.0.0 remote-bind opt-in to non-WSL or other SSH-deployed hosts,
while preserving the stated WSL all-interface behavior when unset. Use direct,
active wording consistently across the referenced dashboard-binding
documentation entries.
- Around line 1036-1038: Update the missing-supervisor recovery description near
the managed controller flow to state that the two zero-supervisor scans are
unchanged and root-controller-attested, in addition to requiring a stable PID 1.
Keep the existing transactional recreation, credential-free startup, identity
pinning, rollback retention, health, settle-check, and writable-layer caveats
unchanged.
🪄 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: 6a5f2cec-9c27-4d71-b67a-f2095f895834
📒 Files selected for processing (27)
docs/manage-sandboxes/lifecycle.mdxdocs/reference/commands.mdxdocs/reference/troubleshooting.mdxscripts/managed-gateway-control.pysrc/lib/actions/sandbox/gateway-restart.test.tssrc/lib/actions/sandbox/gateway-restart.tssrc/lib/actions/sandbox/process-recovery.tssrc/lib/actions/sandbox/supervisor-relaunch.test.tssrc/lib/actions/sandbox/supervisor-relaunch.tssrc/lib/onboard/docker-gpu-diagnostic-redaction.test.tssrc/lib/onboard/docker-gpu-patch-finalize.test.tssrc/lib/onboard/docker-gpu-patch-finalize.tssrc/lib/onboard/docker-gpu-patch-validation.test.tssrc/lib/onboard/docker-gpu-patch.test.tssrc/lib/onboard/docker-gpu-patch.tssrc/lib/onboard/docker-gpu-pre-rollback-diagnostics.test.tssrc/lib/onboard/docker-gpu-supervisor-reconnect.tssrc/lib/onboard/docker-startup-command-patch.test.tssrc/lib/onboard/docker-startup-command-patch.tssrc/lib/onboard/docker-startup-command-sandbox-create.test.tssrc/lib/onboard/sandbox-create-launch.tssrc/lib/sandbox/privileged-exec.test.tssrc/lib/sandbox/privileged-exec.tstest/e2e/live/gateway-guard-recovery.test.tstest/e2e/mock-parity.jsontest/managed-gateway-control.test.tstest/process-recovery-supervisor-relaunch.test.ts
✅ Files skipped from review due to trivial changes (2)
- src/lib/onboard/docker-gpu-patch-validation.test.ts
- docs/manage-sandboxes/lifecycle.mdx
🚧 Files skipped from review as they are similar to previous changes (22)
- src/lib/onboard/docker-gpu-supervisor-reconnect.ts
- src/lib/sandbox/privileged-exec.test.ts
- src/lib/onboard/docker-gpu-diagnostic-redaction.test.ts
- src/lib/onboard/docker-gpu-patch-finalize.ts
- src/lib/actions/sandbox/gateway-restart.ts
- src/lib/actions/sandbox/supervisor-relaunch.test.ts
- src/lib/onboard/docker-startup-command-sandbox-create.test.ts
- src/lib/onboard/docker-startup-command-patch.ts
- docs/reference/troubleshooting.mdx
- src/lib/onboard/docker-gpu-pre-rollback-diagnostics.test.ts
- src/lib/sandbox/privileged-exec.ts
- src/lib/onboard/docker-gpu-patch-finalize.test.ts
- src/lib/onboard/sandbox-create-launch.ts
- src/lib/onboard/docker-gpu-patch.test.ts
- test/process-recovery-supervisor-relaunch.test.ts
- src/lib/actions/sandbox/gateway-restart.test.ts
- src/lib/onboard/docker-startup-command-patch.test.ts
- scripts/managed-gateway-control.py
- src/lib/actions/sandbox/supervisor-relaunch.ts
- test/managed-gateway-control.test.ts
- src/lib/onboard/docker-gpu-patch.ts
- src/lib/actions/sandbox/process-recovery.ts
🛑 Comments failed to post (2)
docs/reference/commands.mdx (2)
1036-1038: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Document the complete missing-supervisor proof.
This wording omits that the two scans must be unchanged and root-controller-attested, making the documented recovery gate appear broader than the PR’s safety contract.
Suggested wording
-When the managed controller reports `SUPERVISOR_NOT_RUNNING` after two zero-supervisor process scans with a stable PID 1, a local Docker-driver sandbox with the legacy keepalive startup can enter a transactional container recreation. +When the managed controller reports `SUPERVISOR_NOT_RUNNING` after two unchanged, root-controller-attested scans confirm no supervisor while PID 1 remains stable, a local Docker-driver sandbox with the legacy keepalive startup can enter a transactional container recreation.📝 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.When the managed controller reports `SUPERVISOR_NOT_RUNNING` after two unchanged, root-controller-attested scans confirm no supervisor while PID 1 remains stable, a local Docker-driver sandbox with the legacy keepalive startup can enter a transactional container recreation. The recreation uses a credential-free managed startup command, pins the registered container identity, retains the previous container for rollback, and commits only after managed gateway health and the settle check pass. The recreation preserves mounted sandbox state, but a committed swap does not retain changes stored only in the previous container's writable layer.🤖 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/reference/commands.mdx` around lines 1036 - 1038, Update the missing-supervisor recovery description near the managed controller flow to state that the two zero-supervisor scans are unchanged and root-controller-attested, in addition to requiring a stable PID 1. Keep the existing transactional recreation, credential-free startup, identity pinning, rollback retention, health, settle-check, and writable-layer caveats unchanged.
3121-3121: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Clarify the WSL exception for dashboard binding.
The documentation says WSL binds all interfaces when unset, then says only
0.0.0.0enables remote binding. Scope the latter to the explicit non-WSL opt-in so users do not misinterpret the exposure behavior. The rewrite also follows the required direct, active documentation style.Suggested wording
-Set `NEMOCLAW_DASHBOARD_BIND=0.0.0.0` before `$$nemoclaw onboard` to prepare the sandbox for remote exposure and bind the forward on all interfaces. +On non-WSL hosts, set `NEMOCLAW_DASHBOARD_BIND=0.0.0.0` before `$$nemoclaw onboard` to opt into remote exposure and bind the forward on all interfaces. Use the same setting for later `$$nemoclaw <sandbox> connect` calls. -A sandbox created without this opt-in must be recreated with `NEMOCLAW_DASHBOARD_BIND=0.0.0.0 $$nemoclaw onboard --recreate-sandbox` before a remote-bind connect is allowed. -Only `0.0.0.0` enables the remote bind; onboarding rejects any other non-empty value. +If you created a sandbox without this opt-in, recreate it with `NEMOCLAW_DASHBOARD_BIND=0.0.0.0 $$nemoclaw onboard --recreate-sandbox` before you use a remote-bind connect. +Outside WSL, only `0.0.0.0` enables the explicit remote bind; onboarding rejects any other non-empty value.Also applies to: 3139-3144
🤖 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/reference/commands.mdx` at line 3121, Update the NEMOCLAW_DASHBOARD_BIND documentation to explicitly scope the 0.0.0.0 remote-bind opt-in to non-WSL or other SSH-deployed hosts, while preserving the stated WSL all-interface behavior when unset. Use direct, active wording consistently across the referenced dashboard-binding documentation entries.Source: Coding guidelines
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Collaborator
|
@coderabbitai review Addressed the final review-body findings in
Targeted integration, CLI type-check, docs, condition-budget scanning, |
Contributor
|
✅ Action performedReview finished.
|
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 `@test/process-recovery-supervisor-relaunch.test.ts`:
- Around line 98-104: Update the test around
relaunchManagedSupervisorSessionImpl to assert that the mock was invoked before
checking that no mutation occurred. Keep the existing no-side-effect assertions,
ensuring the test fails when the relaunch branch is skipped for any unrelated
reason.
🪄 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: ea6fac52-ebfc-4e0f-aef4-dde54327275c
📒 Files selected for processing (3)
docs/reference/commands.mdxdocs/reference/troubleshooting.mdxtest/process-recovery-supervisor-relaunch.test.ts
✅ Files skipped from review due to trivial changes (1)
- docs/reference/troubleshooting.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/reference/commands.mdx
Collaborator
|
Reviewed the remaining CodeRabbit item from review 4679266923; no code change is needed. The full-flow kill-switch test already asserts |
Contributor
E2E Target Results — ❌ Some tests failedRun: 29177181325
|
Contributor
E2E Target Results — ❌ Some tests failedRun: 29177072715
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
E2E Target Results — ❌ Some tests failedRun: 29177741579
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
E2E Target Results — ❌ Some tests failedRun: 29178149876
|
Contributor
E2E Target Results — ✅ All requested tests passedRun: 29178405818
|
21 tasks
cv
added a commit
that referenced
this pull request
Jul 12, 2026
<!-- markdownlint-disable MD041 --> ## Summary Release-prep documentation for v0.0.81 now summarizes user-facing changes merged since v0.0.80. It also closes the Hermes dashboard-profile backup gap and distinguishes direct blueprint-runner actions from public host CLI commands. ## Changes - Add the `v0.0.81` section to `docs/about/release-notes.mdx` with links to the detailed user guides. - Document that Hermes rebuilds preserve `.hermes/dashboard-home/`, including Dashboard `MEMORY.md` and `USER.md`. - Update Hermes manual backup and restore examples to transfer those two profile files without copying generated configuration or the secret-bearing dashboard `.env`. - Explain the new per-item backup failure causes. - Clarify that migration snapshot retention fragments are direct-runner arguments and are not exposed by the host `nemoclaw` CLI. ### Source summary - #6445 -> `docs/about/release-notes.mdx`, `docs/manage-sandboxes/backup-restore.mdx`, and `docs/manage-sandboxes/workspace-files.mdx`: Summarize manifest-owned key-level restore and current-config authority. - #6617 -> `docs/about/release-notes.mdx` and `docs/manage-sandboxes/backup-restore.mdx`: Record the fail-closed `/proc` fallback used to verify an idle Deep Agents runtime before snapshot creation. - #6685 -> `docs/about/release-notes.mdx`, `docs/manage-sandboxes/backup-restore.mdx`, and `docs/manage-sandboxes/workspace-files.mdx`: Document Hermes Web Dashboard profile persistence and safe manual transfer. - #6649 -> `docs/about/release-notes.mdx`: Summarize host-validated loopback compatible-endpoint routing through the sandbox gateway. - #6643 -> `docs/about/release-notes.mdx`: Summarize automatic `max_completion_tokens` handling for GPT-5 and o-series models. - #6661 -> `docs/about/release-notes.mdx`: Summarize bounded connection reuse for eligible provider-validation probes. - #6704 -> `docs/about/release-notes.mdx`: Record that direct blueprint apply stops instead of persisting incomplete state after provider or inference setup fails. - #6677 -> `docs/about/release-notes.mdx`: Summarize transactional recovery for legacy Docker containers whose managed supervisor disappeared after restart. - #6625 -> `docs/about/release-notes.mdx`: Record Hermes managed-startup persistence across direct Docker restarts. - #6597 -> `docs/about/release-notes.mdx`: Record final-sandbox gateway cleanup on macOS. - #6680 -> `docs/about/release-notes.mdx`: Summarize managed Deep Agents first-run and process-tree cleanup improvements. - #6647 -> `docs/about/release-notes.mdx`: Record fail-closed validation for the managed Deep Agents fetch CA bundle. - #6645 -> `docs/about/release-notes.mdx`: Summarize WhatsApp loopback pairing and trusted npm plugin provenance. - #6673 -> `docs/about/release-notes.mdx` and `docs/manage-sandboxes/backup-restore.mdx`: Document stopped-sandbox backup remediation. - #6631 -> `docs/about/release-notes.mdx` and `docs/manage-sandboxes/backup-restore.mdx`: Document per-item backup failure causes. - #6620 -> `docs/about/release-notes.mdx`: Record the created-but-not-ready sandbox lifecycle receipt. - #6664 -> `docs/about/release-notes.mdx`: Record prompt-aware onboarding progress output. - #6598 -> `docs/about/release-notes.mdx`: Summarize stale replay-result invalidation during resumed onboarding. - #6593 -> `docs/about/release-notes.mdx`: Summarize contextual OpenClaw audit findings for managed dashboard compatibility settings. - #6650 -> `docs/about/release-notes.mdx`: Record redaction of token-shaped URL query values. - #6638 -> `docs/about/release-notes.mdx`: Record the exact-path MCP `DELETE` policy recipe for session termination. - #5453 -> `docs/reference/host-files-and-state.mdx`: Clarify that snapshot retention actions belong to direct runner integrations and are not standalone host CLI commands. ### Skipped from docs-skip - #6633 matched the `openclaw-sandbox-permissive.yaml` path in `docs/.docs-skip` and produced no documentation in this update. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] 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: This is a documentation-only release-prep update; behavior is protected by the merged source PRs, and the documentation build validates the changed examples and routes. - [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 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 — tests are not applicable for this documentation-only change; `npm run docs` completed successfully. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: not run for this documentation-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) — 0 errors; two existing Fern warnings remain. - [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) — no new pages. --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Added release notes for v0.0.81 covering state preservation, inference setup, sandbox recovery, session setup, pairing, diagnostics, and security policy updates. - Expanded backup and restore guidance to include dashboard profile files and clarify files that must not be copied. - Added dashboard profile persistence details to workspace and rebuild documentation. - Clarified snapshot retention guidance and the distinction between host CLI capabilities and direct runner actions. - Added more detailed backup failure reporting information. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
…VIDIA#6677) <!-- markdownlint-disable MD041 --> ## Summary After a local Docker-driver sandbox container restart, legacy containers whose persisted startup command was `sleep infinity` lost the `nemoclaw-start` workload session. `recover` now handles only a root-controller-attested, stable no-supervisor state by transactionally recreating the registered container with a credential-free managed startup command. NemoClaw retains the previous container until the replacement passes container-pinned managed-control health and settle checks; otherwise it attempts rollback and returns rebuild or re-onboard guidance. ## Related Issue Fixes NVIDIA#6635 ## Changes - Added the dedicated `SUPERVISOR_NOT_RUNNING` controller result. It is emitted only after two complete zero-supervisor scans with unchanged PID 1 and no unreadable, ambiguous, or changing process state. Those cases remain `SUPERVISOR_UNAVAILABLE` and cannot authorize container mutation. - Replaced the ordinary sandbox-exec background launch with a host-side container transaction. Recovery pins the registered container identity, verifies the legacy `sleep infinity` startup, uses the immutable image identity, and persists `env ... nemoclaw-start` as the managed startup command. - Reconstructs only non-credential runtime environment values. Credential placeholders, credential-bearing proxy URLs, and secret-shaped diagnostics are omitted or redacted. - Retains the previous container for rollback. The replacement is probed through root managed control pinned to its container ID; recovery commits only after managed gateway health and the settle check pass. - Keeps `SUPERVISOR_NOT_RUNNING` distinct from privileged-control failures and prints non-circular rebuild or re-onboard guidance when trusted recovery cannot complete. - Added unit and integration coverage for stable absence, ambiguity and race refusal, container identity pinning, immutable-image recreation, invalid startup-command refusal, credential exclusion, rollback, commit-after-health, and sanitized diagnostics. - Extended `gateway-guard-recovery` with a real Docker container restart. It verifies container replacement, managed supervisor topology, forwarded health, credential exclusion, and a successful agent inference request after recovery. - Updated lifecycle, command-reference, and troubleshooting documentation for the transactional recovery boundary and writable-layer limitation. ## Verification Evidence - Final-head PR CI: [run 29178402069](https://github.com/NVIDIA/NemoClaw/actions/runs/29178402069) passed all eight CLI shards, type checking, static checks, plugin and installer tests, E2E support, and the remaining required non-live jobs. - Focused container/recovery unit matrix: 17 files and 187 tests passed. - Focused managed-controller/recovery integration matrix: 2 files and 8 tests passed. - Additional recovery/controller integration matrix: 122 tests passed during the implementation review. - E2E support project: 116 files and 946 tests passed; live/fast parity passed. - `npm run check:diff`: all applicable pre-commit, commit-message, secret-scan, repository, and pre-push checks passed. - Normal pre-push hooks: plugin TypeScript, checked-JavaScript build, CLI TypeScript, and version synchronization passed. - `npm run docs`: passed with 0 errors; two pre-existing hidden-page warnings remain. - Local broad integration was attempted but is not claimed as a pass: the shared host has an occupied port required by an unchanged onboarding test, and the isolated worktree's temporary dependency symlink is intentionally rejected by an unchanged trusted-path guard. The affected tests pass when those environmental constraints are removed. PR CI remains authoritative. - Final-head live `gateway-guard-recovery`: [run 29178405818](https://github.com/NVIDIA/NemoClaw/actions/runs/29178405818) passed 1/1. Recovery exited 0 without timing out, proved one managed supervisor under PID 1, restored forward health with HTTP 200, and completed live inference with the expected answer. [Artifact](https://github.com/NVIDIA/NemoClaw/actions/runs/29178405818/artifacts/8255686220). - Final-head automatic release gate: [run 29178569987](https://github.com/NVIDIA/NemoClaw/actions/runs/29178569987) passed both `onboard-repair` and `onboard-resume`. - Adjacent live targets on production commit `a89859bb5` passed: [gpu-e2e](https://github.com/NVIDIA/NemoClaw/actions/runs/29178149876/job/86611005998), [credential-sanitization](https://github.com/NVIDIA/NemoClaw/actions/runs/29178149876/job/86611006184), [rebuild-openclaw](https://github.com/NVIDIA/NemoClaw/actions/runs/29178149876/job/86611006091), [sandbox-operations](https://github.com/NVIDIA/NemoClaw/actions/runs/29178149876/job/86611006238), and [cloud-onboard](https://github.com/NVIDIA/NemoClaw/actions/runs/29178149876/job/86611006233). Final commit `2c57e7ea2` changes only the live topology test UID assertion. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] 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: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [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: maintainer security review found no remaining code blocker; [Terra](https://github.com/NVIDIA/NemoClaw/actions/runs/29178402046) reports `merge_as_is` with zero findings, and the final-head live recovery proof passed. - [ ] 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: focused recovery matrix 149 passed after the readiness and forward changes; focused container/recovery 187 passed; focused controller/recovery integration 8 passed; E2E support 946 passed; final-head live gateway recovery 1/1 passed. - [x] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: final-head [PR CI](https://github.com/NVIDIA/NemoClaw/actions/runs/29178402069) and [E2E / PR Gate](https://github.com/NVIDIA/NemoClaw/actions/runs/29178559443) passed; local broad-run limitations remain documented above. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [x] `npm run docs` builds without new warnings (doc changes only) — 0 errors; two pre-existing hidden-page warnings unchanged. - [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: Tinson Lai <tinsonl@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a trusted legacy recovery path for supported local Docker sandboxes when the managed supervisor isn’t running. * Added a new lifecycle behavior flag to disable automatic supervisor relaunch during recovery. * **Bug Fixes** * Improved gateway restart failure classification (including a distinct “supervisor not running” outcome) and corresponding recovery hints. * Strengthened safeguards to prevent recovery when container identity changes, and improved Docker rollback/restore behavior when results are incomplete. * Added validation to prevent privileged execution when the expected container identity doesn’t match. * **Documentation** * Expanded recovery and troubleshooting guidance for supervisor-not-running scenarios and the disable-relaunch flag. * **Tests** * Added/updated coverage for supervisor relaunch recovery decisions and Docker redaction/patch edge cases. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
<!-- markdownlint-disable MD041 --> ## Summary Release-prep documentation for v0.0.81 now summarizes user-facing changes merged since v0.0.80. It also closes the Hermes dashboard-profile backup gap and distinguishes direct blueprint-runner actions from public host CLI commands. ## Changes - Add the `v0.0.81` section to `docs/about/release-notes.mdx` with links to the detailed user guides. - Document that Hermes rebuilds preserve `.hermes/dashboard-home/`, including Dashboard `MEMORY.md` and `USER.md`. - Update Hermes manual backup and restore examples to transfer those two profile files without copying generated configuration or the secret-bearing dashboard `.env`. - Explain the new per-item backup failure causes. - Clarify that migration snapshot retention fragments are direct-runner arguments and are not exposed by the host `nemoclaw` CLI. ### Source summary - NVIDIA#6445 -> `docs/about/release-notes.mdx`, `docs/manage-sandboxes/backup-restore.mdx`, and `docs/manage-sandboxes/workspace-files.mdx`: Summarize manifest-owned key-level restore and current-config authority. - NVIDIA#6617 -> `docs/about/release-notes.mdx` and `docs/manage-sandboxes/backup-restore.mdx`: Record the fail-closed `/proc` fallback used to verify an idle Deep Agents runtime before snapshot creation. - NVIDIA#6685 -> `docs/about/release-notes.mdx`, `docs/manage-sandboxes/backup-restore.mdx`, and `docs/manage-sandboxes/workspace-files.mdx`: Document Hermes Web Dashboard profile persistence and safe manual transfer. - NVIDIA#6649 -> `docs/about/release-notes.mdx`: Summarize host-validated loopback compatible-endpoint routing through the sandbox gateway. - NVIDIA#6643 -> `docs/about/release-notes.mdx`: Summarize automatic `max_completion_tokens` handling for GPT-5 and o-series models. - NVIDIA#6661 -> `docs/about/release-notes.mdx`: Summarize bounded connection reuse for eligible provider-validation probes. - NVIDIA#6704 -> `docs/about/release-notes.mdx`: Record that direct blueprint apply stops instead of persisting incomplete state after provider or inference setup fails. - NVIDIA#6677 -> `docs/about/release-notes.mdx`: Summarize transactional recovery for legacy Docker containers whose managed supervisor disappeared after restart. - NVIDIA#6625 -> `docs/about/release-notes.mdx`: Record Hermes managed-startup persistence across direct Docker restarts. - NVIDIA#6597 -> `docs/about/release-notes.mdx`: Record final-sandbox gateway cleanup on macOS. - NVIDIA#6680 -> `docs/about/release-notes.mdx`: Summarize managed Deep Agents first-run and process-tree cleanup improvements. - NVIDIA#6647 -> `docs/about/release-notes.mdx`: Record fail-closed validation for the managed Deep Agents fetch CA bundle. - NVIDIA#6645 -> `docs/about/release-notes.mdx`: Summarize WhatsApp loopback pairing and trusted npm plugin provenance. - NVIDIA#6673 -> `docs/about/release-notes.mdx` and `docs/manage-sandboxes/backup-restore.mdx`: Document stopped-sandbox backup remediation. - NVIDIA#6631 -> `docs/about/release-notes.mdx` and `docs/manage-sandboxes/backup-restore.mdx`: Document per-item backup failure causes. - NVIDIA#6620 -> `docs/about/release-notes.mdx`: Record the created-but-not-ready sandbox lifecycle receipt. - NVIDIA#6664 -> `docs/about/release-notes.mdx`: Record prompt-aware onboarding progress output. - NVIDIA#6598 -> `docs/about/release-notes.mdx`: Summarize stale replay-result invalidation during resumed onboarding. - NVIDIA#6593 -> `docs/about/release-notes.mdx`: Summarize contextual OpenClaw audit findings for managed dashboard compatibility settings. - NVIDIA#6650 -> `docs/about/release-notes.mdx`: Record redaction of token-shaped URL query values. - NVIDIA#6638 -> `docs/about/release-notes.mdx`: Record the exact-path MCP `DELETE` policy recipe for session termination. - NVIDIA#5453 -> `docs/reference/host-files-and-state.mdx`: Clarify that snapshot retention actions belong to direct runner integrations and are not standalone host CLI commands. ### Skipped from docs-skip - NVIDIA#6633 matched the `openclaw-sandbox-permissive.yaml` path in `docs/.docs-skip` and produced no documentation in this update. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] 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: This is a documentation-only release-prep update; behavior is protected by the merged source PRs, and the documentation build validates the changed examples and routes. - [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 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 — tests are not applicable for this documentation-only change; `npm run docs` completed successfully. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: not run for this documentation-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) — 0 errors; two existing Fern warnings remain. - [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) — no new pages. --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Added release notes for v0.0.81 covering state preservation, inference setup, sandbox recovery, session setup, pairing, diagnostics, and security policy updates. - Expanded backup and restore guidance to include dashboard profile files and clarify files that must not be copied. - Added dashboard profile persistence details to workspace and rebuild documentation. - Clarified snapshot retention guidance and the distinction between host CLI capabilities and direct runner actions. - Added more detailed backup failure reporting information. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Carlos Villela <cvillela@nvidia.com>
This was referenced Jul 20, 2026
23 tasks
23 tasks
Open
2 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
After a local Docker-driver sandbox container restart, legacy containers whose persisted startup command was
sleep infinitylost thenemoclaw-startworkload session.recovernow handles only a root-controller-attested, stable no-supervisor state by transactionally recreating the registered container with a credential-free managed startup command. NemoClaw retains the previous container until the replacement passes container-pinned managed-control health and settle checks; otherwise it attempts rollback and returns rebuild or re-onboard guidance.Related Issue
Fixes #6635
Changes
SUPERVISOR_NOT_RUNNINGcontroller result. It is emitted only after two complete zero-supervisor scans with unchanged PID 1 and no unreadable, ambiguous, or changing process state. Those cases remainSUPERVISOR_UNAVAILABLEand cannot authorize container mutation.sleep infinitystartup, uses the immutable image identity, and persistsenv ... nemoclaw-startas the managed startup command.SUPERVISOR_NOT_RUNNINGdistinct from privileged-control failures and prints non-circular rebuild or re-onboard guidance when trusted recovery cannot complete.gateway-guard-recoverywith a real Docker container restart. It verifies container replacement, managed supervisor topology, forwarded health, credential exclusion, and a successful agent inference request after recovery.Verification Evidence
npm run check:diff: all applicable pre-commit, commit-message, secret-scan, repository, and pre-push checks passed.npm run docs: passed with 0 errors; two pre-existing hidden-page warnings remain.gateway-guard-recovery: run 29178405818 passed 1/1. Recovery exited 0 without timing out, proved one managed supervisor under PID 1, restored forward health with HTTP 200, and completed live inference with the expected answer. Artifact.onboard-repairandonboard-resume.a89859bb5passed: gpu-e2e, credential-sanitization, rebuild-openclaw, sandbox-operations, and cloud-onboard. Final commit2c57e7ea2changes only the live topology test UID assertion.Type of Change
Quality Gates
merge_as_iswith zero findings, and the final-head live recovery proof passed.Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: final-head PR CI and E2E / PR Gate passed; local broad-run limitations remain documented above.npm run docsbuilds without new warnings (doc changes only) — 0 errors; two pre-existing hidden-page warnings unchanged.Signed-off-by: Tinson Lai tinsonl@nvidia.com
Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit