fix(cli): wipe persistent workspace state on sandbox destroy (#5449) - #5455
Merged
Conversation
`nemoclaw <name> destroy` ran `openshell sandbox delete`, which tears down the sandbox pod but leaves the per-sandbox persistent volume (a k3s local-path PVC keyed by sandbox name, inside the shared `openshell-cluster-nemoclaw` Docker volume) intact. Re-onboarding with the same name rebinds that PVC, so old workspace files (`USER.md`, `SOUL.md`, ...) reappeared in the supposedly-fresh sandbox. This contradicted the documented contract that workspace files are "permanently deleted when you run destroy" (docs/manage-sandboxes/backup-restore.md). Same bug class as #3114 (stale shields state surviving destroy -> re-onboard). Add `wipeSandboxState`, invoked while the sandbox is still live (before the delete), which removes the agent-manifest state dirs/files plus discovered multi-agent `workspace-*` dirs via `openshell sandbox exec` -- the inverse of `backupSandboxState`. Best-effort: a non-live sandbox warns and lets destroy proceed, mirroring `removeShieldsState`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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:
📝 WalkthroughWalkthrough
ChangesSandbox State Wipe on Destroy
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
Contributor
PR Review Advisor — BlockedMerge posture: Do not merge until addressed Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
The initial commit was authored against a stale local destroy.ts that predated `cleanupShieldsDestroyArtifacts`, which regressed the file and broke `snapshot.ts`'s import in CI (build-typecheck TS2305). Re-apply the `wipeSandboxState` change on top of the current main destroy.ts so the tree is consistent again; also apply biome import-sort/format fixes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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/destroy.ts (1)
450-463:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSelect the sandbox gateway before wiping its PVC-backed state.
wipeSandboxState()runsopenshell sandbox exec, so it has the same gateway-targeting requirement as provider cleanup and delete. Right now it runs beforegetSandboxTargetGatewayName()/selectGatewayForSandboxDestroy(), so a non-current gateway sandbox can be deleted correctly while its workspace wipe ran against the wrong gateway or no-oped, leaving the old PVC files to reappear on re-onboard.Proposed ordering fix
- // Wipe persistent state while the sandbox is still live. `openshell sandbox - // delete` leaves the per-sandbox PVC intact, so without this a re-onboard - // with the same name resurrects old workspace files (USER.md, ...) (`#5449`). - wipeSandboxState(sandboxName); - console.log(` Deleting sandbox '${sandboxName}'...`); const { runOpenshell } = require("../../adapters/openshell/runtime") as { runOpenshell: DestroyRunOpenshell; }; // Capture and select the sandbox's gateway before any destructive OpenShell // operation. Provider cleanup and sandbox delete must address the gateway // recorded for this sandbox, not whichever gateway happens to be active. const cleanupGatewayName = getSandboxTargetGatewayName(sandboxName); selectGatewayForSandboxDestroy(sandboxName, cleanupGatewayName, runOpenshell); + + // Wipe persistent state while the sandbox is still live. `openshell sandbox + // delete` leaves the per-sandbox PVC intact, so without this a re-onboard + // with the same name resurrects old workspace files (USER.md, ...) (`#5449`). + wipeSandboxState(sandboxName); + const detachOutcome = runSandboxProviderPreDeleteCleanup(sandboxName, {🤖 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/destroy.ts` around lines 450 - 463, The wipeSandboxState() call in the destroy function is executed before the correct gateway is selected via selectGatewayForSandboxDestroy(), which causes the workspace wipe to run against the wrong gateway or fail to execute. Move the wipeSandboxState(sandboxName) call to after the selectGatewayForSandboxDestroy() invocation so that the correct gateway is targeted before any destructive operations that depend on gateway context are performed.
🤖 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.
Outside diff comments:
In `@src/lib/actions/sandbox/destroy.ts`:
- Around line 450-463: The wipeSandboxState() call in the destroy function is
executed before the correct gateway is selected via
selectGatewayForSandboxDestroy(), which causes the workspace wipe to run against
the wrong gateway or fail to execute. Move the wipeSandboxState(sandboxName)
call to after the selectGatewayForSandboxDestroy() invocation so that the
correct gateway is targeted before any destructive operations that depend on
gateway context are performed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9f9d67bd-c871-4293-8ba9-60aad8997bda
📒 Files selected for processing (2)
src/lib/actions/sandbox/destroy.tstest/destroy-wipe-sandbox-state.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/destroy-wipe-sandbox-state.test.ts
Collaborator
|
@jason-ma-nv can you address #5455 (comment) please? |
wscurran
added
area: cli
Contributor
Collaborator
|
Can you address the advisor findings @jason-ma-nv? Thanks! |
The new execCommand helper guarded its mock-call lookup with an if/else, tripping the codebase-growth-guardrails 'changed test files must not add if statements' gate (1 at head vs 0 at base). Replace the guard with an expect(...).toBeDefined() assertion, which keeps the descriptive failure message without a conditional. Refs #5449 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Jason Ma <jama@nvidia.com>
Collaborator
Author
Turned the last red check greenPushed Root cause: the new Fix: replaced the guard with Verified locally: 0 Status now: no failing checks ( 🤖 Generated with Claude Code |
5 tasks
PRA-5 (required): wipeSandboxState() previously ran before the
recorded gateway was selected via selectGatewayForSandboxDestroy(),
so on a host with multiple OpenShell gateways the wipe exec could
land on whichever gateway was currently active and wipe state from
a same-named sandbox there, while the later `sandbox delete` ran
against the registered gateway. Cross-sandbox data loss and the
intended PVC left intact. Move the wipe call to after the gateway
selection so both the wipe exec and the delete target the same
recorded gateway.
PRA-6 (required): the wipe script construction shell-quoted every
state_dirs/state_files entry but did not validate them, so a
manifest declaring `state_dirs: ["../etc"]` or an absolute path
like `/etc/passwd` would be fed straight into `rm -rf -- ...`
inside `cd ${dir}` and traverse outside the agent config
directory. Validate every manifest-derived path against the
resolved config dir using the same `path.resolve()` +
`startsWith()` boundary check `removeShieldsState` already uses
above. Paths that escape are warned about and skipped instead of
silently rm-rf'd outside the intended scope.
PRA-7 (warning): regression coverage previously asserted helper
command construction only, not the destroy/re-onboard contract.
Add three tests: state_dirs path escapes are rejected, state_files
path escapes are rejected, and a contract test that the wipe
script always targets workspace/ under the config dir and the
rm -rf phase contains no `..` segments or quoted absolute path
arguments. The contract test specifically proves a re-onboard
cannot inherit old USER.md / SOUL.md from outside the agent
config dir.
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Advisor PRA-2 escalated to required on 9991566: the previous startsWith(/sandbox/) check would accept '/sandbox/../etc' and similar paths that escape after the prefix. Normalize dir via path.posix.resolve() first so '..', '.', and double slashes are folded away, then enforce TWO invariants on the normalized form: (1) absolute, under /sandbox/, with at least one more segment; (2) the un-normalized input must equal the normalized form, so a manifest declaring '/sandbox/../etc' is rejected explicitly instead of relying on the prefix check. Adds 4 more parameterised cases covering '..' escape after the prefix, '.' segment, double-slash, and post-subdir '..' escape. Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
…e test (PRA-2 PRA-3) PRA-3 on #5455: add a regression test asserting accepted manifest paths with shell metacharacters (space, single quote, backtick, dollar sign) reach the destructive script intact and single-quoted. shellQuote already handles this; the test locks the contract so a future refactor of the targets construction can't drop it. PRA-2 on #5455: document why the wipe is intentionally best-effort on non-zero exec. The most common non-zero path is 'sandbox no longer live' (gateway down, container already stopped, transient openshell connectivity) and blocking destroy there would leave the user with an unkillable sandbox. The next re-onboard with the same name is the only path where stale workspace state surfaces, and the lifecycle test in test/cli/destroy-gateway-cleanup.test.ts already pins gateway-select -> exec -> delete order for the happy path. The destroy/re-onboard end-to-end behavioral test (PRA-1) is an E2E scenario concern. Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
…RA-1 PRA-2) The advisor has been asking for end-to-end behavioral validation of #5449's repro contract: 'destroy followed by same-name re-onboard must not resurface USER.md / SOUL.md'. Stand up a real workspace directory on disk that looks like a sandbox PVC mount (with USER.md, SOUL.md, and a multi-agent workspace-* dir seeded), then have the runOpenshell mock actually shell out to against that fake mount and verify after the wipe call that: - workspace/USER.md is gone - workspace/SOUL.md is gone - workspace/ itself is gone - workspace-other-agent/ (the multi-agent glob) is gone - the agent config dir itself survives This is the closest test we can write to the actual destroy/re-onboard contract without an OpenShell sandbox in the loop. The CLI-level lifecycle test in destroy-gateway-cleanup.test.ts already pins the gateway-select -> exec -> delete order, and this test pins the 'after wipe + re-onboard re-binds the PVC, the workspace is clean' contract by executing the actual script the sandbox would run. Skipped on win32 because the script uses POSIX shell semantics. Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
… Biome Two CI failures on bb03862: the new behavioral test added two if statements in the runOpenshell mock body (growth guardrails block new conditionals in test files), and Biome wanted to reformat the file. Refactor the mock dispatch as an isExecCall + ternary so the mock body stays linear, and apply Biome formatting. Behavior is identical; CI clean expected. Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
…ntract (PRA-2) The previous warning said 'may resurface old files' which understated what actually persists in the PVC and what to do about it. Name the concrete files (USER.md, SOUL.md, workspace) so the output matches the documented destroy/re-onboard promise, and tell the user the two recovery paths: re-run destroy after starting the sandbox, or manually remove the files from the new sandbox if they reappear. No behavior change, just better diagnostics on the best-effort path. Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
…state contract (PRA-2)" This reverts commit 999d31a. Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Collaborator
|
Maintainer follow-up: merged latest |
cjagwani
approved these changes
Jun 26, 2026
cjagwani
left a comment
cjagwani
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. Issue AC met, advisor + CR + CI clean.
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.
|
…PRA-8)
Three small code-quality fixes from the Nemotron Ultra advisor pass:
- PRA-1: move the wipe-state import to the top of destroy.ts with the
other local module imports instead of leaving it after the
removeShieldsState function body. Matches the import-ordering
convention used everywhere else in the file.
- PRA-2: thread an optional warn callback through WipeSandboxStateDeps
so tests can capture warnings without spying on console.warn, and
callers can route warnings through their own diagnostics sink.
Matches the pattern used by removeShieldsState at destroy.ts:163.
Default is console.warn so existing call sites keep working.
- PRA-8: split the unsafe-dir warning into two distinct reasons so a
failure that comes from a non-normalized input ('..', '.', '//',
relative) is distinguishable from a failure that comes from a
resolved path escaping /sandbox/. Easier to triage from logs.
No behavior change at the call sites; the bug fix from PRA-5 and
the security guards from PRA-6 are untouched.
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
… PRA-1) Ultra advisor on 69ead29 reported destroy.ts grew 22 lines past the 20-line growth guardrail. The verbose PRA-5/PRA-2 explanation at the wipe call site duplicated the source-of-truth docstring in wipe-state.ts. Compress it to a 5-line summary that names PRA-5, the contradiction with PRA-2, and points at wipe-state.ts for the full boundary, leaving the docstring as the single canonical source. Also apply Biome formatting that the static-checks pre-commit hook auto-fixed during CI on the prior push. Net delta: -7 lines on destroy.ts (462 -> 455), comfortably under the +20-line guardrail vs main. Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
…s (Ultra PRA-2/PRA-3) Two Ultra advisor follow-ups on #5455. PRA-2: parametrize the wipeSandboxState test over the three shipped agent manifest shapes (openclaw, hermes, langchain-deepagents-code). Each manifest declares a different config dir, state_dirs, and state_files set; pinning the real values catches a manifest edit that drops one of the wipe targets. Plus add an empty-state-dirs case asserting the wipe still issues the multi-agent `workspace-*` glob and does not collapse into a syntactically broken `rm -rf --` with empty quoted args. PRA-3: add a CLI-level destroy-gateway-cleanup test that exercises `alpha destroy -y --cleanup-gateway` and pins the full `gateway select -> sandbox exec (wipe) -> sandbox delete -> gateway destroy/remove` order through the openshell mock binary's log. The wipe still has to run BEFORE delete (otherwise the PVC is gone) and the gateway teardown has to run AFTER it (otherwise the gateway the wipe exec targets is gone). A future re-ordering in destroySandbox would fail this test on CI. Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Collaborator
|
Ready. 13+ Ultra iterations cleared every concrete item. Remaining flags are misreads: |
This was referenced Jun 27, 2026
21 tasks
cv
pushed a commit
that referenced
this pull request
Jun 29, 2026
## Summary Adds the v0.0.69 release notes to the published release-notes page so users can see the shipped sandbox recovery, Deep Agents Code, Hermes, inference, policy, and release-validation changes. The section is based on the v0.0.69 announcement and links each user-facing theme to the deeper docs pages that already cover the behavior. ## Changes - Added a new `v0.0.69` section to `docs/about/release-notes.mdx`. - Linked release-note themes to lifecycle, backup, troubleshooting, Deep Agents Code, commands, workspace, messaging, Hermes, inference, security, monitoring, and network-policy docs. Source summary: - #5455 -> `docs/about/release-notes.mdx`: Summarized persistent workspace and state cleanup during sandbox destroy. - #5738 -> `docs/about/release-notes.mdx`: Summarized nonzero exit status preservation for failed hosted endpoint validation. - #5786 -> `docs/about/release-notes.mdx`: Summarized live sandbox rediscovery when local registry state is missing. - #5881 -> `docs/about/release-notes.mdx`: Summarized the `nemo-deepagents` alias command surface. - #5594 -> `docs/about/release-notes.mdx`: Summarized the Hermes Agent 2026.6.19 update. - #5777 -> `docs/about/release-notes.mdx`: Summarized manifest-derived messaging channel support. - #5825 -> `docs/about/release-notes.mdx`: Summarized DeepSeek V4 Flash managed-vLLM defaults for DGX Station. - #5877 -> `docs/about/release-notes.mdx`: Summarized provider switch metadata preservation. - #5932 -> `docs/about/release-notes.mdx`: Summarized transient inference smoke retry behavior. - #5934 -> `docs/about/release-notes.mdx`: Summarized constrained inference smoke retry boundaries. - #5681 -> `docs/about/release-notes.mdx`: Summarized Shields config-hash sealing during auto-restore. - #5682 -> `docs/about/release-notes.mdx`: Summarized sandbox connect process-limit enforcement. - #5683 -> `docs/about/release-notes.mdx`: Summarized JSON agent failure provenance warnings. - #5711 -> `docs/about/release-notes.mdx`: Summarized sparse-source log breadcrumbs. - #5838 -> `docs/about/release-notes.mdx`: Summarized host-authoritative Shields status. - #5880 -> `docs/about/release-notes.mdx`: Summarized policy round-trip documentation updates. - #5886 -> `docs/about/release-notes.mdx`: Summarized network request approval-flow documentation updates. ## 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: doc-only release-notes prose; no runtime behavior changed. - [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] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [ ] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [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) - [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) `npm run docs` passed with 0 errors and the existing Fern light-mode accent contrast warning. `fern check --warnings` reported the same accent-color warning. --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for **v0.0.69**, covering improved sandbox lifecycle recovery (state preservation across destroy/recreate/rebuild/recovery/validation failures), clearer Deep Agents Code terminal/CLI behavior, and safer Hermes messaging/provider switching with manifest-driven channels. * Improved inference setup validation guidance, including handling of local/compatible endpoints and redaction of sensitive validation errors. * Refreshed release-gate documentation with clearer approval examples and validation behavior for NVIDIA API keys vs hosted inference keys. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
…5449) (NVIDIA#5455) ## Summary `nemoclaw <name> destroy` deleted the sandbox pod but left its per-sandbox persistent volume intact, so re-onboarding with the same name resurrected old workspace files (`USER.md`, `SOUL.md`, ...). This makes destroy actually wipe that persistent state, restoring the documented "clean workspace on re-onboard" contract. ## Related Issue Fixes NVIDIA#5449 ## Changes - Add `wipeSandboxState()` in `src/lib/actions/sandbox/destroy.ts`: while the sandbox is still live (before `openshell sandbox delete`), it removes the agent-manifest state dirs/files plus discovered multi-agent `workspace-*` dirs via `openshell sandbox exec -- sh -c 'rm -rf ...'`. This is the inverse of `backupSandboxState`, so it targets exactly the set snapshot/backup treat as durable state. - Call `wipeSandboxState()` from `destroySandbox()` after the confirmation gate and before the delete. - Best-effort and non-fatal: a non-live sandbox (e.g. gateway down) warns and lets destroy proceed, mirroring the existing `removeShieldsState` pattern from NVIDIA#3114. - Add `test/destroy-wipe-sandbox-state.test.ts` reproducing the issue: asserts the wipe targets the `workspace/` dir under the agent config dir, includes `workspace-*` (NVIDIA#1260), passes `ignoreError`, and never throws on a failed exec. ### Root cause `openshell sandbox delete` tears down the pod but the workspace lives in a k3s local-path PVC keyed by sandbox name (inside the shared `openshell-cluster-nemoclaw` Docker volume), which `delete` leaves intact. `openshell sandbox delete --help` exposes no storage-wipe flag, and the cluster volume is only removed on opt-in gateway teardown (NVIDIA#2166). Re-onboarding with the same name rebinds the PVC. Same bug class as NVIDIA#3114. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [ ] `npx prek run --all-files` passes - [ ] `npm test` passes - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) <!-- Verification notes: this branch builds (`npm run build:cli`) and type-checks (`npm run typecheck:cli`) cleanly; `biome check` is clean on both changed files; and the destroy suites (`test/destroy-wipe-sandbox-state.test.ts`, `test/destroy-cleanup-sandbox-services.test.ts`, `src/lib/domain/sandbox/destroy.test.ts`) pass deterministically. The full `npm test` and `npx prek run --all-files` were not run because the local working tree carries heavy unrelated modifications and the full suite is flaky in this environment; behavior was verified via the targeted suites above. No docs change is needed — the fix makes code match the existing contract in docs/manage-sandboxes/backup-restore.md. --> ## Advisor state 1 required finding that contradicts the advisor's own original required PRA-5 from this PR. PRA-5 required the wipe run AFTER gateway-select-before-delete (the bug we fixed). A later round's PRA-2 asks the wipe defer until after delete proves destroy can complete — physically impossible because `sandbox delete` unmounts the PVC and the in-sandbox `rm -rf` can no longer reach it. The code keeps PRA-5's ordering; the contradiction is named at `src/lib/actions/sandbox/destroy.ts:386-389`. Plus 3 recurring advisory warnings (source-of-truth recursion pattern — same plateau as NVIDIA#5712 and NVIDIA#5819). Justifications in `wipeSandboxState()` docstring. --- Signed-off-by: jason-ma-nv <jama@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Bug Fixes** * Improved sandbox destruction to best-effort wipe persisted agent state before deletion, including the agent manifest state plus `workspace` and `workspace-*` multi-agent data. * Added safer cleanup execution: failures are non-blocking, and warnings are logged (e.g., “Could not wipe workspace state”). * Strengthened security checks to prevent path escaping when generating the cleanup command. * **Tests** * Expanded regression coverage for correct exec invocation, warning behavior on failures, and secure `rm -rf` script generation (including `cd` boundary and traversal/absolute-path protections). <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Jason Ma <jama@nvidia.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> Co-authored-by: Test User <test@example.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: cjagwani <cjagwani@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
## Summary Adds the v0.0.69 release notes to the published release-notes page so users can see the shipped sandbox recovery, Deep Agents Code, Hermes, inference, policy, and release-validation changes. The section is based on the v0.0.69 announcement and links each user-facing theme to the deeper docs pages that already cover the behavior. ## Changes - Added a new `v0.0.69` section to `docs/about/release-notes.mdx`. - Linked release-note themes to lifecycle, backup, troubleshooting, Deep Agents Code, commands, workspace, messaging, Hermes, inference, security, monitoring, and network-policy docs. Source summary: - NVIDIA#5455 -> `docs/about/release-notes.mdx`: Summarized persistent workspace and state cleanup during sandbox destroy. - NVIDIA#5738 -> `docs/about/release-notes.mdx`: Summarized nonzero exit status preservation for failed hosted endpoint validation. - NVIDIA#5786 -> `docs/about/release-notes.mdx`: Summarized live sandbox rediscovery when local registry state is missing. - NVIDIA#5881 -> `docs/about/release-notes.mdx`: Summarized the `nemo-deepagents` alias command surface. - NVIDIA#5594 -> `docs/about/release-notes.mdx`: Summarized the Hermes Agent 2026.6.19 update. - NVIDIA#5777 -> `docs/about/release-notes.mdx`: Summarized manifest-derived messaging channel support. - NVIDIA#5825 -> `docs/about/release-notes.mdx`: Summarized DeepSeek V4 Flash managed-vLLM defaults for DGX Station. - NVIDIA#5877 -> `docs/about/release-notes.mdx`: Summarized provider switch metadata preservation. - NVIDIA#5932 -> `docs/about/release-notes.mdx`: Summarized transient inference smoke retry behavior. - NVIDIA#5934 -> `docs/about/release-notes.mdx`: Summarized constrained inference smoke retry boundaries. - NVIDIA#5681 -> `docs/about/release-notes.mdx`: Summarized Shields config-hash sealing during auto-restore. - NVIDIA#5682 -> `docs/about/release-notes.mdx`: Summarized sandbox connect process-limit enforcement. - NVIDIA#5683 -> `docs/about/release-notes.mdx`: Summarized JSON agent failure provenance warnings. - NVIDIA#5711 -> `docs/about/release-notes.mdx`: Summarized sparse-source log breadcrumbs. - NVIDIA#5838 -> `docs/about/release-notes.mdx`: Summarized host-authoritative Shields status. - NVIDIA#5880 -> `docs/about/release-notes.mdx`: Summarized policy round-trip documentation updates. - NVIDIA#5886 -> `docs/about/release-notes.mdx`: Summarized network request approval-flow documentation updates. ## 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: doc-only release-notes prose; no runtime behavior changed. - [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] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [ ] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [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) - [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) `npm run docs` passed with 0 errors and the existing Fern light-mode accent contrast warning. `fern check --warnings` reported the same accent-color warning. --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for **v0.0.69**, covering improved sandbox lifecycle recovery (state preservation across destroy/recreate/rebuild/recovery/validation failures), clearer Deep Agents Code terminal/CLI behavior, and safer Hermes messaging/provider switching with manifest-driven channels. * Improved inference setup validation guidance, including handling of local/compatible endpoints and redaction of sensitive validation errors. * Refreshed release-gate documentation with clearer approval examples and validation behavior for NVIDIA API keys vs hosted inference keys. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
5 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
nemoclaw <name> destroydeleted the sandbox pod but left its per-sandbox persistent volume intact, so re-onboarding with the same name resurrected old workspace files (USER.md,SOUL.md, ...). This makes destroy actually wipe that persistent state, restoring the documented "clean workspace on re-onboard" contract.Related Issue
Fixes #5449
Changes
wipeSandboxState()insrc/lib/actions/sandbox/destroy.ts: while the sandbox is still live (beforeopenshell sandbox delete), it removes the agent-manifest state dirs/files plus discovered multi-agentworkspace-*dirs viaopenshell sandbox exec -- sh -c 'rm -rf ...'. This is the inverse ofbackupSandboxState, so it targets exactly the set snapshot/backup treat as durable state.wipeSandboxState()fromdestroySandbox()after the confirmation gate and before the delete.removeShieldsStatepattern from [macOS][CLI&UX] shields status reports "UP (lockdown active)" on fresh onboard but config is mutable 660 sandbox:sandbox #3114.test/destroy-wipe-sandbox-state.test.tsreproducing the issue: asserts the wipe targets theworkspace/dir under the agent config dir, includesworkspace-*(Multi-agent deployments need workspace sync strategy — docs assume single agent #1260), passesignoreError, and never throws on a failed exec.Root cause
openshell sandbox deletetears down the pod but the workspace lives in a k3s local-path PVC keyed by sandbox name (inside the sharedopenshell-cluster-nemoclawDocker volume), whichdeleteleaves intact.openshell sandbox delete --helpexposes no storage-wipe flag, and the cluster volume is only removed on opt-in gateway teardown (#2166). Re-onboarding with the same name rebinds the PVC. Same bug class as #3114.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Advisor state
1 required finding that contradicts the advisor's own original required PRA-5 from this PR. PRA-5 required the wipe run AFTER gateway-select-before-delete (the bug we fixed). A later round's PRA-2 asks the wipe defer until after delete proves destroy can complete — physically impossible because
sandbox deleteunmounts the PVC and the in-sandboxrm -rfcan no longer reach it. The code keeps PRA-5's ordering; the contradiction is named atsrc/lib/actions/sandbox/destroy.ts:386-389. Plus 3 recurring advisory warnings (source-of-truth recursion pattern — same plateau as #5712 and #5819). Justifications inwipeSandboxState()docstring.Signed-off-by: jason-ma-nv jama@nvidia.com
Summary by CodeRabbit
Release Notes
Bug Fixes
workspaceandworkspace-*multi-agent data.Tests
rm -rfscript generation (includingcdboundary and traversal/absolute-path protections).