fix(upgrade): pin base image digest and rebuild stale sandboxes#1937
Conversation
After upgrading NemoClaw via curl | bash, Docker reuses the locally cached :latest sandbox base image instead of pulling the new one, leaving existing (and even newly created) sandboxes on a stale OpenClaw version. Three-layer fix: 1. Prevent — patchStagedDockerfile() rewrites ARG BASE_IMAGE to use the sha256 digest from blueprint.yaml so Docker must pull on cache miss. createSandbox() pre-pulls the pinned image for progress feedback and fail-fast on registry errors. 2. Detect + Remediate — new `nemoclaw upgrade-sandboxes` command lists all sandboxes, checks staleness via checkAgentVersion(), and offers to rebuild each stale one (--auto for non-interactive, --check for report-only). 3. Install integration — install.sh calls upgrade-sandboxes after onboarding completes so the upgrade path automatically catches stale sandboxes. Closes #1904 Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
📝 WalkthroughWalkthroughThis change implements sandbox upgrade functionality to address stale OpenClaw versions in existing sandboxes after NemoClaw upgrades. It introduces digest-pinning for base images during onboarding, a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Installer
participant Registry
participant Docker
participant CLI as NemoClaw CLI
participant OpenShell
participant SandboxAgent
User->>Installer: Run upgrade script
Installer->>Registry: Check for sandboxes
alt Sandboxes exist
Installer->>CLI: Invoke upgrade-sandboxes
CLI->>Registry: List registered sandboxes
CLI->>OpenShell: Query live sandboxes (sandbox list)
OpenShell-->>CLI: Return live sandbox states
CLI->>SandboxAgent: Check agent version for each sandbox
alt Stale sandboxes found
CLI->>User: Report staleness & prompt (unless --auto/--yes)
User->>CLI: Confirm rebuild
loop For each stale sandbox
CLI->>Docker: Rebuild sandbox (sandboxRebuild)
Docker-->>CLI: Success/Failure
end
CLI->>User: Print rebuild summary
else All up to date
CLI->>User: Print "up to date" message
end
else No sandboxes
Installer->>User: Complete install (no upgrade needed)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/nemoclaw.ts (1)
76-90:⚠️ Potential issue | 🟡 MinorReserve the new global command name.
promptValidatedSandboxName()insrc/lib/onboard.tsblocks sandbox names that collide with global commands, butupgrade-sandboxesis not added there. A sandbox with that name will route to the global command instead of the sandbox action.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/nemoclaw.ts` around lines 76 - 90, Add the reserved name "upgrade-sandboxes" to the sandbox-name collision check in promptValidatedSandboxName() (src/lib/onboard.ts) so sandbox names cannot match the global command set declared as GLOBAL_COMMANDS (src/nemoclaw.ts); either add "upgrade-sandboxes" to the local reserved list inside promptValidatedSandboxName() or refactor promptValidatedSandboxName() to reference GLOBAL_COMMANDS directly and ensure the validation rejects "upgrade-sandboxes".
🧹 Nitpick comments (2)
src/lib/onboard.ts (1)
453-473: Keep newsrc/libhelpers under TS checking.This adds another exported CLI helper inside a
//@ts-nocheck`` module with no explicit types, so the new surface still bypassestsconfig.cli.json. Please avoid growing the unchecked area here.As per coding guidelines, "Use TypeScript for CLI code in
src/lib/with type-checking viatsconfig.cli.json".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/onboard.ts` around lines 453 - 473, The getBlueprintBaseImageDigest helper is untyped inside a no-check module; convert it to TypeScript with explicit types and keep it under tsconfig.cli.json checking: change the implementation of getBlueprintBaseImageDigest(rootDir: string = ROOT): string | null to use typed imports (e.g., import YAML from "yaml"), replace require/any usage with proper types, annotate variables (parsed, value, trimmed) and the return type, and remove any // `@ts-nocheck` that causes the file to bypass TS checking so the helper is fully type-checked and exported from the module.test/onboard.test.ts (1)
4618-4621: Harden the pre-pull ordering assertion to reduce false positives.Lines [4618]-[4621] use broad
indexOf('"sandbox",')/indexOf('"create",')checks, which can match unrelated string literals. A single anchored regex over the create invocation would make this test more robust.Possible refinement
- const sandboxCreatePos = source.indexOf('"sandbox",', prePullPos); - assert.ok(sandboxCreatePos !== -1, "sandbox create args not found after pre-pull"); - const createArgPos = source.indexOf('"create",', sandboxCreatePos); - assert.ok(createArgPos !== -1, "pre-pull must appear before sandbox create command — regression `#1904`"); + const createAfterPull = /run\(\[\s*"openshell"[\s\S]*?"sandbox",\s*"create"/.test( + source.slice(prePullPos), + ); + assert.ok(createAfterPull, "pre-pull must appear before sandbox create command — regression `#1904`");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/onboard.test.ts` around lines 4618 - 4621, Current indexOf-based checks (sandboxCreatePos/createArgPos using source and prePullPos) can match unrelated literals; replace them with a single anchored regular-expression search that looks for the sandbox create invocation after prePullPos (e.g., a regex that matches the sequence '"sandbox",\s*"create",' or the actual sandbox create call pattern) and assert the regex finds a match after prePullPos. Update the code that computes sandboxCreatePos/createArgPos to use source.search(/.../) (or equivalent) and a single assert that the regex match index is > prePullPos to ensure the pre-pull ordering is robust.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/install.sh`:
- Around line 1277-1279: The installer currently calls `nemoclaw
upgrade-sandboxes` which can prompt during non-interactive runs; change the
invocation to include the non-interactive flag by calling `nemoclaw
upgrade-sandboxes --auto` (i.e., update the command in the block that checks
`_has_sandboxes` and `command_exists nemoclaw`) while keeping the existing
redirection and fallback `warn` behavior so failures remain non-fatal.
In `@src/lib/onboard.ts`:
- Around line 2747-2754: The pre-pull currently uses run(["docker", "pull",
pinnedRef], { ignoreError: true }) so failures are ignored; change this to
fail-fast in the onboarding flow by removing ignoreError and checking the result
(or catching the thrown error) from run in the same block where
getBlueprintBaseImageDigest and pinnedRef are used, then log a clear error
(including the digest/pinnedRef) and exit/throw (e.g., process.exit(1) or
rethrow) so onboarding does not proceed to build when the digest pull fails;
reference getBlueprintBaseImageDigest, pinnedRef and the
run(["docker","pull",...]) call to locate where to implement this change.
In `@src/nemoclaw.ts`:
- Around line 1943-1949: The batch loop's try/catch around sandboxRebuild never
runs because sandboxRebuild calls process.exit on errors; change sandboxRebuild
to propagate errors instead of terminating: replace any process.exit(...) (and
direct console.exit usage) inside the sandboxRebuild implementation with
throwing an Error (or rejecting the Promise) so callers can catch it, and keep
the existing try { await sandboxRebuild(...) } catch (err) { failed++; /* log */
} logic to allow the loop to continue after a single sandbox failure.
- Around line 1889-1890: captureOpenshell(["sandbox", "list"]) can fail and
return no output, causing parseLiveSandboxNames to produce an empty liveNames
which incorrectly marks all sandboxes as stopped; change the code around
captureOpenshell / parseLiveSandboxNames to detect failure (check
liveList.error, non-zero exitCode or missing output) before using liveNames, and
when it failed do not treat liveNames as authoritative—either abort the check,
log the error and skip classification, or keep previous state; update the block
referencing liveList and liveNames (functions captureOpenshell and
parseLiveSandboxNames) to handle the error path explicitly and avoid claiming
sandboxes are stopped when capture failed.
---
Outside diff comments:
In `@src/nemoclaw.ts`:
- Around line 76-90: Add the reserved name "upgrade-sandboxes" to the
sandbox-name collision check in promptValidatedSandboxName()
(src/lib/onboard.ts) so sandbox names cannot match the global command set
declared as GLOBAL_COMMANDS (src/nemoclaw.ts); either add "upgrade-sandboxes" to
the local reserved list inside promptValidatedSandboxName() or refactor
promptValidatedSandboxName() to reference GLOBAL_COMMANDS directly and ensure
the validation rejects "upgrade-sandboxes".
---
Nitpick comments:
In `@src/lib/onboard.ts`:
- Around line 453-473: The getBlueprintBaseImageDigest helper is untyped inside
a no-check module; convert it to TypeScript with explicit types and keep it
under tsconfig.cli.json checking: change the implementation of
getBlueprintBaseImageDigest(rootDir: string = ROOT): string | null to use typed
imports (e.g., import YAML from "yaml"), replace require/any usage with proper
types, annotate variables (parsed, value, trimmed) and the return type, and
remove any // `@ts-nocheck` that causes the file to bypass TS checking so the
helper is fully type-checked and exported from the module.
In `@test/onboard.test.ts`:
- Around line 4618-4621: Current indexOf-based checks
(sandboxCreatePos/createArgPos using source and prePullPos) can match unrelated
literals; replace them with a single anchored regular-expression search that
looks for the sandbox create invocation after prePullPos (e.g., a regex that
matches the sequence '"sandbox",\s*"create",' or the actual sandbox create call
pattern) and assert the regex finds a match after prePullPos. Update the code
that computes sandboxCreatePos/createArgPos to use source.search(/.../) (or
equivalent) and a single assert that the regex match index is > prePullPos to
ensure the pre-pull ordering is robust.
🪄 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: Pro Plus
Run ID: 7fbb489e-db94-464c-aeb4-8aa49480067a
📒 Files selected for processing (4)
scripts/install.shsrc/lib/onboard.tssrc/nemoclaw.tstest/onboard.test.ts
| if [ "${_has_sandboxes:-0}" -gt 0 ] 2>/dev/null && command_exists nemoclaw; then | ||
| info "Checking for sandboxes that need upgrading…" | ||
| nemoclaw upgrade-sandboxes 2>&1 || warn "Sandbox upgrade check failed (non-fatal). Run 'nemoclaw upgrade-sandboxes' manually." |
There was a problem hiding this comment.
Use --auto for non-interactive installer runs.
Line [1279] always invokes nemoclaw upgrade-sandboxes without flags. In non-interactive installs, this can prompt/fail instead of auto-remediating stale sandboxes.
Proposed fix
if [ "${_has_sandboxes:-0}" -gt 0 ] 2>/dev/null && command_exists nemoclaw; then
info "Checking for sandboxes that need upgrading…"
- nemoclaw upgrade-sandboxes 2>&1 || warn "Sandbox upgrade check failed (non-fatal). Run 'nemoclaw upgrade-sandboxes' manually."
+ local -a _upgrade_cmd=(upgrade-sandboxes)
+ if [ "${NON_INTERACTIVE:-}" = "1" ]; then
+ _upgrade_cmd+=(--auto)
+ fi
+ nemoclaw "${_upgrade_cmd[@]}" 2>&1 || warn "Sandbox upgrade check failed (non-fatal). Run 'nemoclaw upgrade-sandboxes' manually."
fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/install.sh` around lines 1277 - 1279, The installer currently calls
`nemoclaw upgrade-sandboxes` which can prompt during non-interactive runs;
change the invocation to include the non-interactive flag by calling `nemoclaw
upgrade-sandboxes --auto` (i.e., update the command in the block that checks
`_has_sandboxes` and `command_exists nemoclaw`) while keeping the existing
redirection and fallback `warn` behavior so failures remain non-fatal.
| // Pre-pull the digest-pinned base image so `docker build` does not silently | ||
| // reuse a stale :latest from the local layer cache. See #1904. | ||
| const blueprintDigest = getBlueprintBaseImageDigest(ROOT); | ||
| if (blueprintDigest) { | ||
| const pinnedRef = `ghcr.io/nvidia/nemoclaw/sandbox-base@${blueprintDigest}`; | ||
| console.log(` Pulling base image (${blueprintDigest.slice(0, 19)}...)...`); | ||
| run(["docker", "pull", pinnedRef], { ignoreError: true }); | ||
| } |
There was a problem hiding this comment.
Fail fast when the digest pull fails.
Right now the pre-pull is best-effort. If registry/auth fails, onboarding continues into the build step instead of stopping at the actual root cause.
Proposed fix
if (blueprintDigest) {
const pinnedRef = `ghcr.io/nvidia/nemoclaw/sandbox-base@${blueprintDigest}`;
console.log(` Pulling base image (${blueprintDigest.slice(0, 19)}...)...`);
- run(["docker", "pull", pinnedRef], { ignoreError: true });
+ const pullResult = run(`docker pull ${shellQuote(pinnedRef)}`, { ignoreError: true });
+ if (pullResult.status !== 0) {
+ console.error(` Failed to pull base image '${pinnedRef}'.`);
+ process.exit(pullResult.status || 1);
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/onboard.ts` around lines 2747 - 2754, The pre-pull currently uses
run(["docker", "pull", pinnedRef], { ignoreError: true }) so failures are
ignored; change this to fail-fast in the onboarding flow by removing ignoreError
and checking the result (or catching the thrown error) from run in the same
block where getBlueprintBaseImageDigest and pinnedRef are used, then log a clear
error (including the digest/pinnedRef) and exit/throw (e.g., process.exit(1) or
rethrow) so onboarding does not proceed to build when the digest pull fails;
reference getBlueprintBaseImageDigest, pinnedRef and the
run(["docker","pull",...]) call to locate where to implement this change.
| const liveList = captureOpenshell(["sandbox", "list"], { ignoreError: true }); | ||
| const liveNames = parseLiveSandboxNames(liveList.output || ""); |
There was a problem hiding this comment.
Handle sandbox list failures before classifying sandboxes as stopped.
When captureOpenshell(["sandbox", "list"]) fails, liveNames becomes empty and every stale sandbox is reported/skipped as not running. That makes upgrade-sandboxes --check misleading հենց when the gateway is unhealthy after an upgrade.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/nemoclaw.ts` around lines 1889 - 1890, captureOpenshell(["sandbox",
"list"]) can fail and return no output, causing parseLiveSandboxNames to produce
an empty liveNames which incorrectly marks all sandboxes as stopped; change the
code around captureOpenshell / parseLiveSandboxNames to detect failure (check
liveList.error, non-zero exitCode or missing output) before using liveNames, and
when it failed do not treat liveNames as authoritative—either abort the check,
log the error and skip classification, or keep previous state; update the block
referencing liveList and liveNames (functions captureOpenshell and
parseLiveSandboxNames) to handle the error path explicitly and avoid claiming
sandboxes are stopped when capture failed.
| try { | ||
| console.log(` Rebuilding '${sb.name}'...`); | ||
| await sandboxRebuild(sb.name, ["--yes"]); | ||
| rebuilt++; | ||
| } catch (err) { | ||
| console.error(` ${_RD}Failed to rebuild '${sb.name}': ${err.message || err}${R}`); | ||
| failed++; |
There was a problem hiding this comment.
Batch upgrade aborts on the first rebuild failure.
sandboxRebuild() exits the process on its error paths, so this try/catch never gets a chance to increment failed and continue. One broken sandbox will terminate the whole batch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/nemoclaw.ts` around lines 1943 - 1949, The batch loop's try/catch around
sandboxRebuild never runs because sandboxRebuild calls process.exit on errors;
change sandboxRebuild to propagate errors instead of terminating: replace any
process.exit(...) (and direct console.exit usage) inside the sandboxRebuild
implementation with throwing an Error (or rejecting the Promise) so callers can
catch it, and keep the existing try { await sandboxRebuild(...) } catch (err) {
failed++; /* log */ } logic to allow the loop to continue after a single sandbox
failure.
) The digest from blueprint.yaml (openshell-community registry) was applied to the nemoclaw/sandbox-base registry, causing "manifest unknown" on every sandbox creation. Reverts cf4941a until digest pinning can use the correct registry/digest pair. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
…sandboxes (#1904) Re-implements the digest pinning and upgrade-sandboxes features reverted in #1938. The original PR (#1937) broke all e2e tests because it read a digest from blueprint.yaml (belongs to ghcr.io/nvidia/openshell-community) and applied it to ghcr.io/nvidia/nemoclaw/sandbox-base — a different registry. Docker digests are repo-specific, so every pull returned "manifest unknown". This fix never reads blueprint.yaml for the base image digest. Instead: - pullAndResolveBaseImageDigest() pulls sandbox-base:latest from GHCR - docker inspect extracts the actual repo digest - patchStagedDockerfile() pins ARG BASE_IMAGE to the inspected digest The digest is self-consistent by construction — it always comes from the same registry we pin to. Falls back to unpinned :latest when GHCR is unreachable (offline/firewall users). Also re-adds the upgrade-sandboxes command with fixes from code review: - --auto flag for non-interactive installer contexts - sandbox list failure handling before classifying sandboxes - throwOnError option for sandboxRebuild to prevent process.exit in loops Closes #1904 Signed-off-by: Test User <test@example.com>
…sandboxes (#1904) Re-implements the digest pinning and upgrade-sandboxes features reverted in #1938. The original PR (#1937) broke all e2e tests because it read a digest from blueprint.yaml (belongs to ghcr.io/nvidia/openshell-community) and applied it to ghcr.io/nvidia/nemoclaw/sandbox-base — a different registry. Docker digests are repo-specific, so every pull returned "manifest unknown". This fix never reads blueprint.yaml for the base image digest. Instead: - pullAndResolveBaseImageDigest() pulls sandbox-base:latest from GHCR - docker inspect extracts the actual repo digest - patchStagedDockerfile() pins ARG BASE_IMAGE to the inspected digest The digest is self-consistent by construction — it always comes from the same registry we pin to. Falls back to unpinned :latest when GHCR is unreachable (offline/firewall users). Also re-adds the upgrade-sandboxes command with fixes from code review: - --auto flag for non-interactive installer contexts - sandbox list failure handling before classifying sandboxes - throwOnError option for sandboxRebuild to prevent process.exit in loops Closes #1904 Signed-off-by: Test User <test@example.com> Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
…sandboxes (#1904) Re-implements the digest pinning and upgrade-sandboxes features reverted in #1938. The original PR (#1937) broke all e2e tests because it read a digest from blueprint.yaml (belongs to ghcr.io/nvidia/openshell-community) and applied it to ghcr.io/nvidia/nemoclaw/sandbox-base — a different registry. Docker digests are repo-specific, so every pull returned "manifest unknown". This fix never reads blueprint.yaml for the base image digest. Instead: - pullAndResolveBaseImageDigest() pulls sandbox-base:latest from GHCR - docker inspect extracts the actual repo digest - patchStagedDockerfile() pins ARG BASE_IMAGE to the inspected digest The digest is self-consistent by construction — it always comes from the same registry we pin to. Falls back to unpinned :latest when GHCR is unreachable (offline/firewall users). Also re-adds the upgrade-sandboxes command with fixes from code review: - --auto flag for non-interactive installer contexts - sandbox list failure handling before classifying sandboxes - throwOnError option for sandboxRebuild to prevent process.exit in loops - RepoDigests filtered by SANDBOX_BASE_IMAGE prefix (ordering not guaranteed) - Exit non-zero from upgrade-sandboxes when sandbox list or rebuilds fail - Report sandboxes with unavailable version detection Closes #1904 Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
…sandboxes (#1943) ## Summary - Re-implements digest pinning and `upgrade-sandboxes` reverted in #1938 - Fixes the root cause: original PR read a digest from `blueprint.yaml` (belongs to `ghcr.io/nvidia/openshell-community`) and applied it to `ghcr.io/nvidia/nemoclaw/sandbox-base` — a different registry. Docker digests are repo-specific, so every sandbox creation failed with "manifest unknown" - New approach: `pullAndResolveBaseImageDigest()` pulls `sandbox-base:latest` from GHCR, then `docker inspect` extracts the actual repo digest. The digest is self-consistent by construction — it always comes from the same registry we pin to - Falls back to unpinned `:latest` when GHCR is unreachable (offline/firewall users) - Re-adds `nemoclaw upgrade-sandboxes` command with `--check`/`--auto`/`--yes` flags and fixes from CodeRabbit review (sandbox list error handling, `throwOnError` for batch rebuilds, `--auto` in install.sh) Closes #1904 ## Test plan - [x] 8 new unit tests (6 in onboard.test.ts, 2 CLI integration tests in cli.test.ts) - [x] Regression guard: verifies `ARG BASE_IMAGE` references `nemoclaw/sandbox-base`, NOT `openshell-community` - [x] Structural test: `pullAndResolveBaseImageDigest()` called before `patchStagedDockerfile()` - [x] CLI test: `upgrade-sandboxes --check` detects stale sandbox (original reporter scenario from #1904) - [x] CLI test: `upgrade-sandboxes --check` reports all-current when no sandboxes are stale - [x] Nightly cloud e2e: verify sandbox creation succeeds with digest pinning (the exact scenario that broke in #1937) - [ ] Manual: verify `nemoclaw upgrade-sandboxes --check` on a real environment with a stale sandbox <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a CLI command to detect and batch-upgrade stale sandboxes (report-only and auto/yes modes). * Installer now performs a post-onboarding scan and can trigger non-blocking sandbox upgrades. * **Improvements** * Sandbox base images are pinned to resolved digests when available for more reproducible builds. * Rebuild flow updated to continue processing multiple sandboxes and surface per-sandbox failures without aborting the whole run. * **Tests** * New unit/integration and end-to-end tests covering detection, reporting, upgrade, and rebuild workflows. * **Chores** * Nightly workflow extended with new E2E jobs; CI failure artifacts and notifications updated. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Summary
blueprint.yamlduring Dockerfile patching so Docker must pull on cache miss instead of reusing a stale:latesttagnemoclaw upgrade-sandboxescommand to detect and rebuild stale sandboxes after CLI upgrade (--autofor non-interactive,--checkfor report-only)upgrade-sandboxesintoinstall.shpost-onboard flow so the upgrade path automatically catches stale sandboxesTest plan
nemoclaw upgrade-sandboxesdetects stale sandbox--autoflag rebuilds without promptingCloses #1904
Summary by CodeRabbit
Release Notes
New Features
nemoclaw upgrade-sandboxescommand to upgrade existing sandboxes with version checking and progress reporting (supports--check,--auto,--yesoptions).Tests