fix(rebuild): remove obsolete sandbox images - #8039
Conversation
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
|
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:
📝 WalkthroughWalkthroughSandbox recreation now checkpoints the source workload, creates the replacement, and retires the replaced workload. Cleanup validates ownership, generation, identity, provider authority, and retention rules. ChangesSandbox recreation cleanup
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SandboxRecreateFlow
participant SandboxRegistry
participant RecreationJournal
participant RuntimeProvider
SandboxRecreateFlow->>SandboxRegistry: read source entry
SandboxRecreateFlow->>RecreationJournal: prepare recreation with source entry
RecreationJournal-->>SandboxRecreateFlow: preparation result
SandboxRecreateFlow->>SandboxRegistry: create replacement
SandboxRecreateFlow->>RuntimeProvider: retire replaced workload
RuntimeProvider-->>SandboxRecreateFlow: cleanup result
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
Sensitive-path security review — PASS at
Regression evidence covers obsolete owned-image removal, replacement image reuse, shared workloads, missing replacement identity, foreign replacement generation, exact lifecycle ordering, and active-launcher remediation. |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 7ab0927 in the TypeScript / code-coverage/cliThe overall coverage in commit 7ab0927 in the Show a code coverage summary of the most impacted files.
Updated |
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
senthilr-nv
added
bug-fix
|
Exact-head sensitive-path re-review — PASS at The only delta from the full nine-category PASS review at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/onboard/machine/handlers/sandbox.ts (1)
1283-1391: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve the original source workload across resume. If
createSandboxsucceeds before retirement and the process exits, the next--resumereads the replacement intosourceEntry. Retirement then receives the replacement as both source and replacement, returnsimage-reused, and leaks the original image. Persist the source workload in the journal and add interruption/resume coverage.🤖 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/machine/handlers/sandbox.ts` around lines 1283 - 1391, Preserve the original source workload across interruptions by storing it in the recreate journal created by beginSandboxRecreateJournal and restoring it during resume instead of deriving sourceEntry from the current registry. Ensure retireSandboxRecreateSourceWorkload receives the persisted original source and the newly created replacement, preventing image-reused from masking the leaked original image. Add interruption/resume coverage for creation succeeding before retirement.
🧹 Nitpick comments (2)
src/lib/onboard/sandbox-recreate-transaction.ts (1)
59-64: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNarrow the bare
catchto the expected error type.
requireRuntimeProviderDestructiveCleanupAuthoritythrowsRuntimeProviderSelectionErrorfor known unauthorized states. The currentcatch {}swallows every error, including unexpected bugs (for example aTypeErrorfrom a broken provider lookup). Cleanup then silently reports"authority-unproven"with no note to the caller, unlike the"failed"path, which does warn the user. Catch onlyRuntimeProviderSelectionErrorand let unexpected errors propagate, or log the swallowed error before returning "skipped".🛠️ Proposed narrower catch
let authority; try { authority = requireRuntimeProviderDestructiveCleanupAuthority(sandboxName, source, providers); - } catch { - return { status: "skipped", reason: "authority-unproven" }; + } catch (error) { + if (!(error instanceof RuntimeProviderSelectionError)) throw error; + return { status: "skipped", reason: "authority-unproven" }; }🤖 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/sandbox-recreate-transaction.ts` around lines 59 - 64, Update the catch around requireRuntimeProviderDestructiveCleanupAuthority in the sandbox recreation flow to handle only RuntimeProviderSelectionError and return the existing "skipped"/"authority-unproven" result for that expected case. Let unexpected errors, such as TypeError, propagate instead of silently swallowing them.src/lib/onboard/machine/core-flow-phases.test.ts (1)
190-288: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an explicit
retireReplacedSandboxWorkloadstub to this fixture.This
sandboxdeps object does not setretireReplacedSandboxWorkload. Because that dependency is optional,sandbox.tsfalls back to the realretireReplacedSandboxWorkloadDefault, which resolves the real Docker/Kubernetes runtime-provider bundles. Today no scenario in this file appears to reach that call, sincebeginSandboxRecreateJournalrequiresresume: trueor an existing transaction. Add an explicit stub now so a future test exercising the recreate/resume path cannot silently start executing real runtime-provider cleanup logic instead of a test double.🧪 Proposed fixture stub
note: vi.fn(), cliName: () => "nemoclaw", + retireReplacedSandboxWorkload: vi.fn(() => ({ + status: "skipped" as const, + reason: "replacement-unproven" as const, + })), updateSession: vi.fn((mutator) => mutator(createSession()) ?? createSession()),As per coding guidelines,
**/*.test.{js,ts}: "Mock external dependencies and do not call real NVIDIA APIs from unit tests."🤖 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/machine/core-flow-phases.test.ts` around lines 190 - 288, Add an explicit test-double implementation for retireReplacedSandboxWorkload in the sandbox deps fixture, alongside the other sandbox dependency stubs, so recreate/resume tests cannot fall through to retireReplacedSandboxWorkloadDefault or invoke real runtime-provider cleanup. Keep the stub inert and consistent with the fixture’s existing vi.fn-based mocks.Source: Coding guidelines
🤖 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/onboard/machine/handlers/sandbox.ts`:
- Around line 1283-1391: Preserve the original source workload across
interruptions by storing it in the recreate journal created by
beginSandboxRecreateJournal and restoring it during resume instead of deriving
sourceEntry from the current registry. Ensure
retireSandboxRecreateSourceWorkload receives the persisted original source and
the newly created replacement, preventing image-reused from masking the leaked
original image. Add interruption/resume coverage for creation succeeding before
retirement.
---
Nitpick comments:
In `@src/lib/onboard/machine/core-flow-phases.test.ts`:
- Around line 190-288: Add an explicit test-double implementation for
retireReplacedSandboxWorkload in the sandbox deps fixture, alongside the other
sandbox dependency stubs, so recreate/resume tests cannot fall through to
retireReplacedSandboxWorkloadDefault or invoke real runtime-provider cleanup.
Keep the stub inert and consistent with the fixture’s existing vi.fn-based
mocks.
In `@src/lib/onboard/sandbox-recreate-transaction.ts`:
- Around line 59-64: Update the catch around
requireRuntimeProviderDestructiveCleanupAuthority in the sandbox recreation flow
to handle only RuntimeProviderSelectionError and return the existing
"skipped"/"authority-unproven" result for that expected case. Let unexpected
errors, such as TypeError, propagate instead of silently swallowing them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1999d6c3-c65b-4933-8eb9-91fa75fb76b6
📒 Files selected for processing (7)
src/lib/onboard.tssrc/lib/onboard/machine/core-flow-phases.test.tssrc/lib/onboard/machine/handlers/sandbox-recreate-journal.test.tssrc/lib/onboard/machine/handlers/sandbox-test-fixtures.tssrc/lib/onboard/machine/handlers/sandbox.tssrc/lib/onboard/runtime-provider/replaced-workload.test.tssrc/lib/onboard/sandbox-recreate-transaction.ts
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
3 additional E2E selections from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
|
Exact-head sensitive-path security review — PASS at VerdictThe review found no security findings. The new recreate-journal payload is a minimal, secret-free ownership receipt. It remains subordinate to the existing provider, generation, replacement-identity, shared-image, and image-reuse checks before any image removal. Detailed analysis
Files reviewed: all 10 files changed by |
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Exact-head security review refreshVerdict: PASS at This refresh preserves the previously reviewed PR patches: the merge commit only incorporates merged
Validation at this exact tree: focused tests 95/95 passed; Prior full review: #8039 (comment) |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/lib/onboard/machine/handlers/sandbox-recreate-journal.test.ts (2)
244-262: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the retirement generation against a captured value, not the mutable fixture.
replacementEntryis reassigned inside thecreateSandboxmock. Line 254 and line 261 then read that same mutated object to build the expectation. The generation claim becomes partly self-referential: the expected value comes from the fixture the handler consumed.Capture the journaled
targetGenerationafter the first run and assert against it. The test then proves that retirement uses the journaled replacement generation.♻️ Suggested assertion change
await expect(handleSandboxState(options)).rejects.toThrow( /interrupted after replacement registration/u, ); expect(session.checkpoint?.sandboxRecreate?.sourceWorkload?.imageTag).toBe(sourceEntry.imageTag); + const journaledGeneration = session.checkpoint?.sandboxRecreate?.targetGeneration; + expect(journaledGeneration).toBeTruthy(); await handleSandboxState(options); expect(retireReplacedSandboxWorkload).toHaveBeenNthCalledWith( 2, "saved", - replacementEntry.lifecycleGeneration, + journaledGeneration, expect.objectContaining({As per path instructions: "Prefer observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions."
🤖 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/machine/handlers/sandbox-recreate-journal.test.ts` around lines 244 - 262, Update the test around handleSandboxState to capture the journaled replacement targetGeneration after the first rejected run, before the createSandbox mock mutates replacementEntry. Use that captured value for the subsequent retirement-generation assertion, including the expected replacement entry where needed, so the test validates the journaled generation rather than the mutable fixture.Source: Path instructions
143-223: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAssert the retry enters
createSandbox. Add assertions that the mock is called twice and that the second call carries the same transaction ID and target generation. The registered replacement must makecreateSandboxRecreateRuntimeaccept the target without repeating deletion or replacement registration.🤖 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/machine/handlers/sandbox-recreate-journal.test.ts` around lines 143 - 223, The test must verify that retrying after replacement registration re-enters createSandbox: assert createSandbox is called twice, and inspect the second call to confirm it uses the original recreate transaction ID and target generation. Ensure the registered replacement state allows createSandboxRecreateRuntime to accept the target on retry without invoking deletion or replacement registration again.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/state/onboard-checkpoint-types.ts`:
- Around line 84-87: Preserve the source workload’s sharing state across
checkpointing and sandbox recreation. In
src/lib/state/onboard-checkpoint-types.ts lines 84-87, add the shared field to
CheckpointSandboxRecreateSourceWorkload; in src/lib/state/onboard-checkpoint.ts
lines 285-296, validate and restore it from the journal; in
src/lib/onboard/sandbox-recreate-transaction.ts lines 104-127, serialize and
reconstruct the exact value instead of defaulting to false; and in
src/lib/onboard/sandbox-recreate-transaction.test.ts lines 120-125, add a
shared-workload case verifying reconstruction keeps shared: true and cleanup
retains the source image.
---
Nitpick comments:
In `@src/lib/onboard/machine/handlers/sandbox-recreate-journal.test.ts`:
- Around line 244-262: Update the test around handleSandboxState to capture the
journaled replacement targetGeneration after the first rejected run, before the
createSandbox mock mutates replacementEntry. Use that captured value for the
subsequent retirement-generation assertion, including the expected replacement
entry where needed, so the test validates the journaled generation rather than
the mutable fixture.
- Around line 143-223: The test must verify that retrying after replacement
registration re-enters createSandbox: assert createSandbox is called twice, and
inspect the second call to confirm it uses the original recreate transaction ID
and target generation. Ensure the registered replacement state allows
createSandboxRecreateRuntime to accept the target on retry without invoking
deletion or replacement registration again.
🪄 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: 89a42eae-8958-4e20-a78c-c553531bd061
📒 Files selected for processing (10)
src/lib/actions/sandbox/rebuild-recreate-observability.test.tssrc/lib/onboard/machine/core-flow-phases.test.tssrc/lib/onboard/machine/handlers/sandbox-recreate-journal.test.tssrc/lib/onboard/machine/handlers/sandbox.tssrc/lib/onboard/runtime-provider/replaced-workload.test.tssrc/lib/onboard/sandbox-recreate-transaction.test.tssrc/lib/onboard/sandbox-recreate-transaction.tssrc/lib/state/onboard-checkpoint-types.tssrc/lib/state/onboard-checkpoint.test.tssrc/lib/state/onboard-checkpoint.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/onboard/machine/core-flow-phases.test.ts
- src/lib/onboard/machine/handlers/sandbox.ts
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Exact-head security review refreshVerdict: PASS at The new commit preserves a source workload's sharing state through interrupted rebuild resume. A proven shared source now returns
Exact-head evidence: focused tests 97/97 passed; Prior full review: #8039 (comment) |
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/onboard/sandbox-recreate-transaction.ts (1)
60-109: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRequire an exact replacement identity match before deletion.
The guard only checks that
replacement.lifecycleLiveIdentityFingerprintis non-empty. It does not compare that value with the journaledtargetLiveIdentityFingerprint.A stale or foreign same-name registry row can have
targetGenerationand a different live identity. The function can then delete the source workload.Pass the expected target identity to this function. Skip cleanup unless it exactly matches
replacement.lifecycleLiveIdentityFingerprint. Update the handler call and add a mismatched-identity regression test.Proposed fix
export function retireReplacedSandboxWorkload( sandboxName: string, targetGeneration: string, + targetLiveIdentityFingerprint: string | null, source: ReplacedSandboxSourceEntry, replacement: SandboxEntry | null, deps: ReplacedSandboxWorkloadCleanupDeps = {}, ): ReplacedSandboxWorkloadCleanupResult { if ( source.name !== sandboxName || replacement?.name !== sandboxName || replacement.lifecycleGeneration !== targetGeneration || - !replacement.lifecycleLiveIdentityFingerprint + !targetLiveIdentityFingerprint || + replacement.lifecycleLiveIdentityFingerprint !== targetLiveIdentityFingerprint ) { return { status: "skipped", reason: "replacement-unproven" }; }Based on supplied checkpoint and handler context,
targetLiveIdentityFingerprintis persisted but is not passed to this guard.🤖 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/sandbox-recreate-transaction.ts` around lines 60 - 109, Require an exact live-identity match before cleanup in retireReplacedSandboxWorkload: add the expected targetLiveIdentityFingerprint parameter and skip with replacement-unproven unless it equals replacement.lifecycleLiveIdentityFingerprint. Update the handler call to pass the journaled target identity, and add a regression test covering a same-name replacement with a matching generation but mismatched identity.
🤖 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/onboard/sandbox-recreate-transaction.ts`:
- Around line 60-109: Require an exact live-identity match before cleanup in
retireReplacedSandboxWorkload: add the expected targetLiveIdentityFingerprint
parameter and skip with replacement-unproven unless it equals
replacement.lifecycleLiveIdentityFingerprint. Update the handler call to pass
the journaled target identity, and add a regression test covering a same-name
replacement with a matching generation but mismatched identity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c3dd430b-29ee-4523-be3a-21607db19b16
📒 Files selected for processing (7)
src/lib/onboard/machine/handlers/sandbox-recreate-journal.test.tssrc/lib/onboard/machine/handlers/sandbox.tssrc/lib/onboard/sandbox-recreate-transaction.test.tssrc/lib/onboard/sandbox-recreate-transaction.tssrc/lib/state/onboard-checkpoint-types.tssrc/lib/state/onboard-checkpoint.test.tssrc/lib/state/onboard-checkpoint.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/lib/state/onboard-checkpoint.ts
- src/lib/onboard/machine/handlers/sandbox-recreate-journal.test.ts
- src/lib/state/onboard-checkpoint.test.ts
- src/lib/onboard/sandbox-recreate-transaction.test.ts
- src/lib/onboard/machine/handlers/sandbox.ts
- src/lib/state/onboard-checkpoint-types.ts
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Exact-head sensitive-path reviewResult: PASS at
Additional exact-head evidence: |
|
Addressed the exact replacement-identity finding in
Focused E2E-support tests pass 98/98, CLI type-checking passes, and |
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 `@src/lib/onboard/machine/handlers/sandbox.ts`:
- Around line 1530-1533: Wrap the post-create sequence beginning with
reloadSandboxRecreateTransaction and including
retireSandboxRecreateSourceWorkload, recordSandboxRecreateRepairSuccess, and
recordSandboxRecreateRegistryCommit in its own try/catch. On any
transaction-reload or cleanup error, call recordSandboxRecreateRepairFailure,
then rethrow the original error so provider failures still propagate. Add a
regression test covering removeOwnedWorkload throwing after replacement
registration and verify the public boundary records repair failure while
preserving the thrown error.
🪄 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: c5142315-9fbd-4118-bbc0-b578e5a3a377
📒 Files selected for processing (5)
src/lib/onboard/machine/handlers/sandbox-recreate-journal.test.tssrc/lib/onboard/machine/handlers/sandbox.tssrc/lib/onboard/runtime-provider/replaced-workload.test.tssrc/lib/onboard/sandbox-recreate-transaction.test.tssrc/lib/onboard/sandbox-recreate-transaction.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lib/onboard/runtime-provider/replaced-workload.test.ts
- src/lib/onboard/sandbox-recreate-transaction.test.ts
- src/lib/onboard/machine/handlers/sandbox-recreate-journal.test.ts
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Exact-head sensitive-path reviewResult: PASS at
Additional exact-head evidence: |
<!-- markdownlint-disable MD041 --> ## Summary Completes the documentation follow-ups identified after the v0.0.100 tag. The durable guides and `docs/changelog/2026-07-31.mdx` now cover final inference-route timing, validation reuse boundaries, and replacement-image cleanup. ## Changes - [#8046](#8046) -> `docs/inference/verify-inference-route.mdx` and `docs/changelog/2026-07-31.mdx`: Documents the 2-second final `inference.local` response budget for OpenClaw and Hermes, including the OpenClaw client-overhead rationale. - [#8044](#8044) -> `docs/inference/understand-provider-validation.mdx`: Documents the exact one-shot Chat Completions validation reuse and forced-revalidation conditions. - [#8039](#8039) and [#8042](#8042) -> `docs/manage-sandboxes/recover-rebuild-sandboxes.mdx` and `docs/changelog/2026-07-31.mdx`: Documents obsolete owned source-image cleanup after durable replacement proof and the `gc` recovery action. - `docs/reference/system-readiness.mdx` and `docs/reference/troubleshooting.mdx`: Applies title case and removes code styling from headings while preserving literal identifiers in prose. ## 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: This documentation-only change does not modify executable behavior. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `docs/changelog/2026-07-31.mdx`, `docs/inference/understand-provider-validation.mdx`, `docs/inference/verify-inference-route.mdx`, `docs/manage-sandboxes/recover-rebuild-sandboxes.mdx`, `docs/reference/system-readiness.mdx`, and `docs/reference/troubleshooting.mdx`. The documentation-only diff was reviewed against the writing rules and documentation style. - Agent: Codex Desktop <!-- docs-review-head-sha: 6d2cd17 --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: Documentation-only; `npx vitest run test/changelog-docs.test.ts test/agent-variant-docs.test.ts` passed 23 tests. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [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` completed with 0 errors and the existing Fern warning. --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> --------- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
3 participants
Summary
Rebuild now removes the obsolete owned sandbox image only after the same-name replacement is registered and its exact journaled generation and live identity are proven. Interrupted resume preserves shared-image ownership state, so cleanup retains shared images. This restores the documented cleanup behavior without weakening the recreate journal added by #7788.
Changes
nemohermes gc.Product scope: this restores the existing supported and documented
rebuildimage-cleanup contract; it does not add a new integration, configuration, or product surface.Type of Change
Quality Gates
docs/reference/commands.mdxalready documents automatic rebuild image cleanup and thegcrecovery path; this change restores that contract.Documentation Writer Review
no-docs-neededdocs/manage-sandboxes/recover-rebuild-sandboxes.mdx:172-195already requires matching live identity and registry generation and fails closed on identity mismatch;docs/reference/commands.mdx:284-287routes interrupted replacements to that contract; exact-head writer review found no command, configuration, output, or workflow change; focused tests passed 98/98, andgit diff --checkpassed.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Not a broad runtime or harness change;npm run validate:prpassed repository checks, CLI type-checking, commitlint, secret scanning, and all applicable hooks.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Senthil Ravichandran senthilr@nvidia.com