fix(cli): gc scans locally prebuilt sandbox image repo for orphans - #6306
Conversation
Signed-off-by: Rui Luo <ruluo@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR extends sandbox image garbage collection to scan both gateway-built and locally prebuilt image repositories, so orphaned local images are detected. It also centralizes sandbox image repository names into shared constants and updates related tag and prebuild code to use them. ChangesSandbox Image Repository Scanning
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/domain/sandbox/image-tag.ts (1)
21-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRegex still hardcodes the repo string the new constant was meant to centralize.
BUILT_SANDBOX_IMAGE_REduplicates"openshell/sandbox-from"as a literal even thoughSANDBOX_FROM_IMAGE_REPOwas just introduced for this exact purpose. Building the regex from the constant would avoid future drift between the two.♻️ Suggested fix
-const BUILT_SANDBOX_IMAGE_RE = /Built image (openshell\/sandbox-from:\d+)/; +const BUILT_SANDBOX_IMAGE_RE = new RegExp( + `Built image (${SANDBOX_FROM_IMAGE_REPO.replace(/\//g, "\\/")}:\\d+)`, +);🤖 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/domain/sandbox/image-tag.ts` at line 21, The BUILT_SANDBOX_IMAGE_RE regex in image-tag.ts still hardcodes the sandbox repo string instead of using SANDBOX_FROM_IMAGE_REPO. Update the regex construction to reference the new constant so the Built image pattern stays centralized and cannot drift from the repo value.
🤖 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/actions/maintenance.test.ts`:
- Line 47: The garbage-collection test in maintenance.test.ts is only asserting
the dockerListImagesFormat mock was called, so it never proves orphan detection
works for the local repo. Update the test around
garbageCollectImages/findOrphanedSandboxImages to have dockerListImagesFormat
return distinct rows for each repo, including an orphan from
nemoclaw-sandbox-local, and assert on an observable result such as the logged
orphan or the removal count rather than the mock call itself. This will verify
the real GC path instead of just the query setup.
---
Nitpick comments:
In `@src/lib/domain/sandbox/image-tag.ts`:
- Line 21: The BUILT_SANDBOX_IMAGE_RE regex in image-tag.ts still hardcodes the
sandbox repo string instead of using SANDBOX_FROM_IMAGE_REPO. Update the regex
construction to reference the new constant so the Built image pattern stays
centralized and cannot drift from the repo value.
🪄 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: aa9eb893-4b73-4c2a-ba38-934e5ee06ca1
📒 Files selected for processing (5)
src/lib/actions/maintenance.test.tssrc/lib/actions/maintenance.tssrc/lib/domain/maintenance/images.test.tssrc/lib/domain/sandbox/image-tag.tssrc/lib/onboard/sandbox-prebuild.ts
Signed-off-by: Rui Luo <ruluo@nvidia.com>
cv
left a comment
There was a problem hiding this comment.
Maintainer review: GC now enumerates and unions both local sandbox-image repositories while preserving registered images, with focused output and preservation tests. CI, DCO, commit verification, and review-thread checks are clean.
|
✨ Related open issues: |
<!-- markdownlint-disable MD041 --> ## Summary Prepares the user-facing documentation for NemoClaw v0.0.76 and closes the release-prep documentation gate. It adds the release highlights, documents the arm64 Local NIM warning and expanded image cleanup behavior, and fixes agent-specific command headings in generated guides. ## Changes - Add the v0.0.76 release-notes section and move the shared-gateway route containment entry out of the v0.0.74 history where it was incorrectly placed. - Document the advisory Linux arm64 Local NIM manifest warning in the canonical platform matrix and local-inference guidance. - Document that `gc` scans both gateway-built and locally prebuilt sandbox image repositories. - Keep OpenClaw and Hermes session headings out of the generated Deep Agents command guide. - Add a focused variant regression test for the agent-specific session headings. ### Source summary | Merged sources | Documentation coverage | | --- | --- | | [#6414](#6414), [#6418](#6418), [#6416](#6416), [#6344](#6344) | v0.0.76 release notes and the Deep Agents quickstart/inference routes | | [#6340](#6340) | v0.0.76 release notes and existing Deep Agents observability guidance | | [#6338](#6338), [#6378](#6378), [#6297](#6297) | v0.0.76 release notes and existing inference/troubleshooting guidance | | [#6362](#6362) | v0.0.76 release notes and existing lifecycle, command, and credential guidance | | [#6330](#6330), [#6307](#6307), [#6008](#6008) | v0.0.76 release notes and existing security, troubleshooting, and command guidance | | [#6382](#6382) | v0.0.76 release notes and existing MCP/command guidance | | [#6326](#6326), [#5868](#5868), [#5539](#5539) | v0.0.76 release notes, platform matrix, inference options, and local-inference guidance | | [#6396](#6396), [#6390](#6390), [#6007](#6007) | v0.0.76 release notes and existing messaging guidance | | [#5388](#5388), [#6249](#6249), [#6303](#6303), [#6306](#6306) | v0.0.76 release notes and command/lifecycle guidance | ## 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 - [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: - [ ] 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 — `npx vitest run --project integration test/generate-platform-docs.test.ts test/agent-variant-docs.test.ts test/sync-agent-variant-docs.test.ts` (3 files, 29 tests passed) - [ ] 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) — completed with 0 errors and 2 pre-existing Fern warnings - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added v0.0.76 release notes content, and removed an older conflicting bullet from the surrounding release history. * Expanded Local NVIDIA NIM guidance across inference/provider docs, including an advisory for Linux arm64 DGX Spark/DGX Station hosts when a matching `linux/arm64` image manifest is unavailable. * Updated the command reference for correct session-section rendering and clarified `gc` image cleanup sources. * **Tests** * Added coverage ensuring Deep Agents omits sessions headings while Hermes includes them. * **CI** * Refreshed Local NVIDIA NIM provider notes used in the platform matrix. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
…VIDIA#6306) <!-- markdownlint-disable MD041 --> ## Summary <!-- 1-3 sentences: what this PR does and why. --> `nemoclaw gc` never removed orphaned locally-prebuilt sandbox images because it scanned only the gateway-built image repository (`openshell/sandbox-from`). This makes gc enumerate every sandbox image repo, so orphans under `nemoclaw-sandbox-local` (the docker-driver-gateway prebuild path — Linux or macOS/Apple Silicon) are detected, and the "run 'nemoclaw gc' to clean up" remediation that `destroy --force` prints actually works. ## Related Issue <!-- Fixes #NNN or Closes #NNN. Remove this section if none. --> Fixes NVIDIA#6301 ## Changes <!-- Bullet list of key changes. --> - `src/lib/domain/sandbox/image-tag.ts`: add `SANDBOX_FROM_IMAGE_REPO`, `LOCAL_SANDBOX_IMAGE_REPO`, and `SANDBOX_IMAGE_REPOS` as the single source of truth for sandbox image repositories. - `src/lib/actions/maintenance.ts`: `garbageCollectImages()` now queries every repo in `SANDBOX_IMAGE_REPOS` and unions the results before orphan detection (was hardcoded to `openshell/sandbox-from` only). - `src/lib/onboard/sandbox-prebuild.ts`: reuse the shared `LOCAL_SANDBOX_IMAGE_REPO` constant instead of a private literal. - Tests: `images.test.ts` gains a mixed-repo case (local orphan detected, registered local image preserved); `maintenance.test.ts` asserts gc scans both repos. ## Type of Change - [√] 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) ## Quality Gates <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [√] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] 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 <!-- 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. --> - [√] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [√] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [√] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [√] Quality Gates section completed with required justifications or waivers - [√] No secrets, API keys, or credentials committed - [ ] `npm run 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) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Rui Luo <ruluo@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved sandbox image cleanup to scan all sandbox image repositories, rather than a single hard-coded source, to remove unused images more reliably. * Fixed local sandbox image reference handling so repository names and tags are generated consistently across the app. * Enhanced orphan image detection to correctly flag unregistered local images even when repository context differs. * **Tests** * Added coverage for orphan-detection and garbage-collection behavior across both sandbox image repositories. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Rui Luo <ruluo@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Prepares the user-facing documentation for NemoClaw v0.0.76 and closes the release-prep documentation gate. It adds the release highlights, documents the arm64 Local NIM warning and expanded image cleanup behavior, and fixes agent-specific command headings in generated guides. ## Changes - Add the v0.0.76 release-notes section and move the shared-gateway route containment entry out of the v0.0.74 history where it was incorrectly placed. - Document the advisory Linux arm64 Local NIM manifest warning in the canonical platform matrix and local-inference guidance. - Document that `gc` scans both gateway-built and locally prebuilt sandbox image repositories. - Keep OpenClaw and Hermes session headings out of the generated Deep Agents command guide. - Add a focused variant regression test for the agent-specific session headings. ### Source summary | Merged sources | Documentation coverage | | --- | --- | | [NVIDIA#6414](NVIDIA#6414), [NVIDIA#6418](NVIDIA#6418), [NVIDIA#6416](NVIDIA#6416), [NVIDIA#6344](NVIDIA#6344) | v0.0.76 release notes and the Deep Agents quickstart/inference routes | | [NVIDIA#6340](NVIDIA#6340) | v0.0.76 release notes and existing Deep Agents observability guidance | | [NVIDIA#6338](NVIDIA#6338), [NVIDIA#6378](NVIDIA#6378), [NVIDIA#6297](NVIDIA#6297) | v0.0.76 release notes and existing inference/troubleshooting guidance | | [NVIDIA#6362](NVIDIA#6362) | v0.0.76 release notes and existing lifecycle, command, and credential guidance | | [NVIDIA#6330](NVIDIA#6330), [NVIDIA#6307](NVIDIA#6307), [NVIDIA#6008](NVIDIA#6008) | v0.0.76 release notes and existing security, troubleshooting, and command guidance | | [NVIDIA#6382](NVIDIA#6382) | v0.0.76 release notes and existing MCP/command guidance | | [NVIDIA#6326](NVIDIA#6326), [NVIDIA#5868](NVIDIA#5868), [NVIDIA#5539](NVIDIA#5539) | v0.0.76 release notes, platform matrix, inference options, and local-inference guidance | | [NVIDIA#6396](NVIDIA#6396), [NVIDIA#6390](NVIDIA#6390), [NVIDIA#6007](NVIDIA#6007) | v0.0.76 release notes and existing messaging guidance | | [NVIDIA#5388](NVIDIA#5388), [NVIDIA#6249](NVIDIA#6249), [NVIDIA#6303](NVIDIA#6303), [NVIDIA#6306](NVIDIA#6306) | v0.0.76 release notes and command/lifecycle guidance | ## 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 - [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: - [ ] 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 — `npx vitest run --project integration test/generate-platform-docs.test.ts test/agent-variant-docs.test.ts test/sync-agent-variant-docs.test.ts` (3 files, 29 tests passed) - [ ] 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) — completed with 0 errors and 2 pre-existing Fern warnings - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added v0.0.76 release notes content, and removed an older conflicting bullet from the surrounding release history. * Expanded Local NVIDIA NIM guidance across inference/provider docs, including an advisory for Linux arm64 DGX Spark/DGX Station hosts when a matching `linux/arm64` image manifest is unavailable. * Updated the command reference for correct session-section rendering and clarified `gc` image cleanup sources. * **Tests** * Added coverage ensuring Deep Agents omits sessions headings while Hermes includes them. * **CI** * Refreshed Local NVIDIA NIM provider notes used in the platform matrix. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Summary
nemoclaw gcnever removed orphaned locally-prebuilt sandbox images because it scanned only the gateway-built image repository (openshell/sandbox-from). This makes gc enumerate every sandbox image repo, so orphans undernemoclaw-sandbox-local(the docker-driver-gateway prebuild path — Linux or macOS/Apple Silicon) are detected, and the "run 'nemoclaw gc' to clean up" remediation thatdestroy --forceprints actually works.Related Issue
Fixes #6301
Changes
src/lib/domain/sandbox/image-tag.ts: addSANDBOX_FROM_IMAGE_REPO,LOCAL_SANDBOX_IMAGE_REPO, andSANDBOX_IMAGE_REPOSas the single source of truth for sandbox image repositories.src/lib/actions/maintenance.ts:garbageCollectImages()now queries every repo inSANDBOX_IMAGE_REPOSand unions the results before orphan detection (was hardcoded toopenshell/sandbox-fromonly).src/lib/onboard/sandbox-prebuild.ts: reuse the sharedLOCAL_SANDBOX_IMAGE_REPOconstant instead of a private literal.images.test.tsgains a mixed-repo case (local orphan detected, registered local image preserved);maintenance.test.tsasserts gc scans both repos.Type of Change
Quality Gates
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:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Rui Luo ruluo@nvidia.com
Summary by CodeRabbit