test/cli.test.ts (1)

2199-2313: Tighten stale/current assertions to prevent false positives.

Both new tests assert expected substrings, but neither asserts the opposite state is absent. Adding negative assertions will make regressions much harder to slip through.

Suggested assertion hardening

       expect(r.code).toBe(0);
       // Should report the stale sandbox with version info
       expect(r.out).toContain("my-agent");
       expect(r.out).toContain("2026.3.11");
       expect(r.out).toMatch(/stale|need upgrading/i);
+      expect(r.out).not.toMatch(/up to date/i);
@@
       const r = runWithEnv("upgrade-sandboxes --check 2>&1", {
         HOME: home,
         PATH: `${localBin}:${process.env.PATH || ""}`,
       });

       expect(r.code).toBe(0);
       expect(r.out).toContain("up to date");
+      expect(r.out).not.toMatch(/stale|need upgrading/i);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/cli.test.ts` around lines 2199 - 2313, The tests for "upgrade-sandboxes
--check" should add negative assertions to avoid false positives: in the first
test (the one expecting a stale sandbox) assert that r.out does NOT contain the
"up to date" message (or equivalent string), and in the second test (the one
expecting all-current) assert that r.out does NOT contain the stale sandbox
indicators (e.g., the sandbox name "my-agent", the old version "2026.3.11", or
the regex /stale|need upgrading/i). Locate the two specs by their it(...)
descriptions and add these complementary expect(r.out).not.toContain /
not.toMatch assertions using the existing r variable to harden the checks.

test/onboard.test.ts (1)

4703-4728: Add direct behavior tests for pullAndResolveBaseImageDigest().

Current checks are mostly structural/constant-based. Add runtime-path tests for digest resolution and GHCR-unreachable fallback to fully protect the regression fix.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/onboard.test.ts` around lines 4703 - 4728, The test suite only checks
structure/constants and misses exercising pullAndResolveBaseImageDigest
behavior; add new runtime tests that call pullAndResolveBaseImageDigest (and
indirectly createSandbox if needed) to assert it returns a resolved image digest
when GHCR is reachable and that it falls back to the sandbox-base registry or a
local default when GHCR is unreachable/unresolvable; ensure tests stub/mock
network/Docker registry calls (or inject a mock registry client) so you can
simulate a successful digest resolution and a GHCR failure, and add assertions
for the returned image string containing a digest and for the fallback branch,
referencing pullAndResolveBaseImageDigest, createSandbox (if used),
patchStagedDockerfile, and SANDBOX_BASE_IMAGE to locate relevant code.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

test/e2e/test-upgrade-stale-sandbox.sh (2)

169-175: Wait loop lacks explicit timeout failure.

Both wait loops (lines 105-110 and 170-175) iterate 30 times with 5-second sleeps but don't explicitly fail if the sandbox never becomes ready. The subsequent grep -q check (line 111-112) will catch the failure, but having an explicit timeout message would improve debuggability.

💡 Optional: Add explicit timeout handling

 # Wait for sandbox to be ready after rebuild
+ready=0
 for _i in $(seq 1 30); do
   if openshell sandbox list 2>/dev/null | grep -q "${SANDBOX_NAME}.*Ready\|${SANDBOX_NAME}.*Running"; then
+    ready=1
     break
   fi
   sleep 5
 done
+[ "$ready" = "1" ] || fail "Sandbox ${SANDBOX_NAME} did not become ready within 150s after rebuild"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/e2e/test-upgrade-stale-sandbox.sh` around lines 169 - 175, The wait
loops that run `for _i in $(seq 1 30); do ... openshell sandbox list ... | grep
-q "${SANDBOX_NAME}.*Ready\|${SANDBOX_NAME}.*Running"; done` lack explicit
timeout failure handling; after the loop completes without a break add an
explicit check for the sandbox state and emit a clear error message to stderr
(including ${SANDBOX_NAME}) and exit non‑zero (e.g., `echo "Timed out waiting
for ${SANDBOX_NAME} to be Ready/Running" >&2; exit 1`). Apply this to both
occurrences of the loop so the script fails fast with a descriptive message when
the sandbox never becomes ready.

---

119-127: Shell variable not expanded inside Python heredoc.

The ${REGISTRY_FILE} and ${SANDBOX_NAME} variables are inside single quotes of the Python string, but they're being expanded by the shell because the outer quotes are double quotes. This actually works correctly here because the shell expands them before Python sees them. However, this pattern can be confusing and error-prone.

Additionally, consider adding a timeout failure check after the wait loop (lines 105-110) to fail explicitly if the sandbox never becomes ready, rather than continuing silently.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/e2e/test-upgrade-stale-sandbox.sh` around lines 119 - 127, Replace the
inline Python invocation that embeds shell vars into the quoted Python string
(producing OLD_AGENT_VERSION) with a safer pattern that passes ${REGISTRY_FILE}
and ${SANDBOX_NAME} as explicit arguments or reads them from the environment
(e.g., call python3 with "$REGISTRY_FILE" "$SANDBOX_NAME" and use sys.argv or
os.environ inside Python) so shell expansion isn't implicitly relied upon; also
add an explicit timeout/failure check immediately after the sandbox readiness
wait loop (the loop that waits for the sandbox to become ready) so the script
exits non‑zero and logs a clear error if the sandbox never becomes ready instead
of continuing silently.

test/e2e/test-upgrade-stale-sandbox.sh (1)

119-127: Registry JSON lookup is brittle for sandboxes shape.

The Python snippet assumes sandboxes is a key-addressable object (.get(name)), but runtime code paths consume it as an iterable list of sandbox objects. This can degrade diagnostics to error: ... strings.

Proposed fix

 OLD_AGENT_VERSION=$(python3 -c "
 import json, sys
+name = '${SANDBOX_NAME}'
 try:
     d = json.load(open('${REGISTRY_FILE}'))
-    sb = d.get('sandboxes', {}).get('${SANDBOX_NAME}', {})
+    sandboxes = d.get('sandboxes', {})
+    if isinstance(sandboxes, list):
+        sb = next((s for s in sandboxes if s.get('name') == name), {})
+    elif isinstance(sandboxes, dict):
+        sb = sandboxes.get(name, {})
+    else:
+        sb = {}
     print(sb.get('agentVersion', 'unknown'))
 except Exception as e:
     print(f'error: {e}')
 " 2>/dev/null || echo "unknown")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/e2e/test-upgrade-stale-sandbox.sh` around lines 119 - 127, The Python
snippet that sets OLD_AGENT_VERSION treats the registry's "sandboxes" as a dict
and can emit "error: ..." into OLD_AGENT_VERSION; update the lookup to handle
"sandboxes" as a list by iterating d.get('sandboxes', []) and finding the object
whose "name" equals the SANDBOX_NAME, then print its "agentVersion" (or
"unknown" if missing); also ensure any exception prints nothing useful to STDOUT
(or print "unknown") so OLD_AGENT_VERSION never captures raw exception strings
from REGISTRY_FILE.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

test/e2e/test-upgrade-stale-sandbox.sh (2)

120-121: ⚠️ Potential issue | 🟠 Major

Fail fast when OpenClaw version probes fail.

Line 120 and Line 182 still use || true, which masks openshell sandbox exec failures and can make the version comparison unreliable.

Proposed fix

-OLD_OPENCLAW_VERSION=$(openshell sandbox exec --name "${SANDBOX_NAME}" -- openclaw --version 2>&1 || true)
+OLD_OPENCLAW_VERSION=$(openshell sandbox exec --name "${SANDBOX_NAME}" -- openclaw --version 2>&1) \
+  || fail "Failed to read old sandbox OpenClaw version: ${OLD_OPENCLAW_VERSION}"

@@
-NEW_OPENCLAW_VERSION=$(openshell sandbox exec --name "${SANDBOX_NAME}" -- openclaw --version 2>&1 || true)
+NEW_OPENCLAW_VERSION=$(openshell sandbox exec --name "${SANDBOX_NAME}" -- openclaw --version 2>&1) \
+  || fail "Failed to read new sandbox OpenClaw version after rebuild: ${NEW_OPENCLAW_VERSION}"

Also applies to: 182-183

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/e2e/test-upgrade-stale-sandbox.sh` around lines 120 - 121, The version
probe currently masks failures by appending "|| true" to the "openshell sandbox
exec --name \"${SANDBOX_NAME}\" -- openclaw --version" calls (used to populate
OLD_OPENCLAW_VERSION and the later NEW_OPENCLAW_VERSION), which can produce
unreliable comparisons; remove "|| true" and instead immediately fail fast when
the command exits non-zero: run the openshell command without "|| true", check
its exit status, and if it fails log an error including the SANDBOX_NAME and the
raw command output and exit the script with a non-zero status so the test never
compares invalid version strings.

---

198-202: ⚠️ Potential issue | 🟠 Major

Make Phase 6 a hard failure when stale state remains.

Line 201 treats a non-clean upgrade-sandboxes --check result as non-fatal, so this E2E can still report success incorrectly.

Proposed fix

 if echo "$RECHECK_OUTPUT" | grep -qi "up to date"; then
   pass "Phase 6: upgrade-sandboxes --check reports all up to date after rebuild"
 else
-  info "Phase 6: Sandbox may still appear stale (non-fatal)"
+  fail "Phase 6: upgrade-sandboxes --check did not report 'up to date' after rebuild. Output: ${RECHECK_OUTPUT}"
 fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/e2e/test-upgrade-stale-sandbox.sh` around lines 198 - 202, Currently the
Phase 6 check inspects RECHECK_OUTPUT with grep -qi "up to date" and calls
pass(...) on success but only logs info(...) on failure; change the failure
branch to make the test a hard failure by invoking the test-failure mechanism
(replace info "Phase 6: Sandbox may still appear stale (non-fatal)" with fail
"Phase 6: upgrade-sandboxes --check reports stale state after rebuild" or
otherwise exit with non-zero status), keeping the same condition that inspects
RECHECK_OUTPUT and the existing pass(...) call on success.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

test/e2e/test-upgrade-stale-sandbox.sh (3)

201-208: ⚠️ Potential issue | 🟠 Major

Phase 6 should fail when --check is not clean.

At Lines 201-208, the test can pass even if stale sandboxes remain because the non-clean result is logged as info instead of failing.

Suggested fix

 if echo "$RECHECK_OUTPUT" | grep -qi "up to date"; then
   pass "Phase 6: upgrade-sandboxes --check reports all up to date after rebuild"
 else
-  info "Phase 6: Sandbox may still appear stale (non-fatal)"
+  fail "Phase 6: upgrade-sandboxes --check did not report 'up to date' after rebuild. Output: ${RECHECK_OUTPUT}"
 fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/e2e/test-upgrade-stale-sandbox.sh` around lines 201 - 208, The test
currently treats a non-clean "nemoclaw upgrade-sandboxes --check" result as
non-fatal by calling info, so Phase 6 can pass incorrectly; change the else
branch that currently calls info to instead fail the test (use the existing fail
function or exit with non-zero) so that when RECHECK_OUTPUT does not contain "up
to date" the script fails (update the block referencing RECHECK_OUTPUT, the grep
conditional, and replace info "Phase 6: Sandbox may still appear stale
(non-fatal)" with a failing call such as fail "Phase 6: upgrade-sandboxes
--check reports stale sandboxes" or exit 1).

---

48-48: ⚠️ Potential issue | 🟡 Minor

Guard diagnostics pipeline from pipefail short-circuit.

At Line 48, grep can return non-zero when no matches exist, which can interrupt diagnostics in fail() under strict mode.

Suggested fix

-  echo -e "${YELLOW}[DIAG]${NC} Docker images: $(docker images --format '{{.Repository}}:{{.Tag}} {{.ID}}' | grep -i 'sandbox\|nemoclaw\|openclaw' | head -10)" >&2
+  echo -e "${YELLOW}[DIAG]${NC} Docker images: $(docker images --format '{{.Repository}}:{{.Tag}} {{.ID}}' | grep -Ei 'sandbox|nemoclaw|openclaw' | head -10 || true)" >&2

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/e2e/test-upgrade-stale-sandbox.sh` at line 48, The diagnostics pipeline
using "docker images ... | grep -i 'sandbox\|nemoclaw\|openclaw' | head -10" can
return non-zero and short-circuit under set -o pipefail (affecting the fail()
diagnostics); update that echo/substitution to guard the pipeline by making the
grep failure non-fatal (for example append "|| true" to the grep or to the whole
substitution) so the echo command always succeeds; locate the line containing
the docker images + grep pipeline in test/e2e/test-upgrade-stale-sandbox.sh and
add the "|| true" guard to the pipeline expression.

---

120-121: ⚠️ Potential issue | 🟠 Major

Do not mask OpenClaw version probe failures.

Lines 120 and 188 use || true, which can hide openshell sandbox exec failures and make the before/after comparison unreliable.

Suggested fix

-OLD_OPENCLAW_VERSION=$(openshell sandbox exec --name "${SANDBOX_NAME}" -- openclaw --version 2>&1 || true)
+OLD_OPENCLAW_VERSION=$(openshell sandbox exec --name "${SANDBOX_NAME}" -- openclaw --version 2>&1) \
+  || fail "Failed to read OpenClaw version from old sandbox"

@@
-NEW_OPENCLAW_VERSION=$(openshell sandbox exec --name "${SANDBOX_NAME}" -- openclaw --version 2>&1 || true)
+NEW_OPENCLAW_VERSION=$(openshell sandbox exec --name "${SANDBOX_NAME}" -- openclaw --version 2>&1) \
+  || fail "Failed to read OpenClaw version after rebuild"

Also applies to: 188-189

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/e2e/test-upgrade-stale-sandbox.sh` around lines 120 - 121, The test
currently masks failures by appending "|| true" to the OpenClaw probe calls (the
assignment to OLD_OPENCLAW_VERSION and the later probe around lines referenced),
which can hide command failures; remove the "|| true" from the openshell sandbox
exec invocations (the commands that set OLD_OPENCLAW_VERSION and the later
NEW_OPENCLAW_VERSION) and instead explicitly check the command exit status or
the captured variable (e.g. if [ -z "$OLD_OPENCLAW_VERSION" ] || [ $? -ne 0 ];
then error "openshell sandbox exec failed for SANDBOX_NAME"; exit 1; fi) so any
probe failure fails the test and the before/after comparison remains reliable.

src/nemoclaw.ts (2)

1612-1618: Preserve the exit code on the throw path.

code is accepted here but discarded when throwOnError is enabled, so batch callers only get a generic Error. Attaching it to the thrown error also removes the unused-parameter guideline violation.

Suggested change

   const bail = opts.throwOnError
     ? (msg, code = 1) => {
-        throw new Error(msg);
+        const error = new Error(msg);
+        error.exitCode = code;
+        throw error;
       }
     : (_msg, code = 1) => process.exit(code);

As per coding guidelines, "Prefix unused variables with underscore (_) in JavaScript/TypeScript".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/nemoclaw.ts` around lines 1612 - 1618, The bail function currently
discards the exit code when opts.throwOnError is true; modify the throw branch
(the function assigned to bail when opts.throwOnError is truthy) to include the
provided code on the thrown error so callers (e.g., upgradeSandboxes) can
inspect the exit code; specifically, in the throw path of bail, construct an
Error with msg and attach the code (e.g., set error.code = code or use
Object.assign) before throwing so the code parameter is used and preserved.

---

1832-1938: Split upgradeSandboxes() into smaller helpers.

This function now handles parsing, classification, reporting, prompting, and execution in one block. It is already at/over the repo complexity cap, so extracting helpers here would make the failure paths much easier to maintain.

As per coding guidelines, "Limit cyclomatic complexity to 20 in JavaScript/TypeScript files, with target of 15".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/nemoclaw.ts` around lines 1832 - 1938, The upgradeSandboxes function is
too large—extract smaller helpers to lower cyclomatic complexity and improve
maintainability: create (1) a gatherSandboxes helper that wraps
registry.listSandboxes, calls captureOpenshell(["sandbox","list"]) and
parseLiveSandboxNames, and returns { sandboxes, liveNames } and any error state;
(2) a classifySandboxes(sandboxes, liveNames) that uses
sandboxVersion.checkAgentVersion to build and return { stale, unknown }; (3) a
reportSandboxStatus(stale, unknown, checkOnly) that prints the status/summary
lines currently in the function; and (4) a rebuildStaleSandboxes(rebuildable,
stopped, skipConfirm) that encapsulates prompting via askPrompt and calling
sandboxRebuild, counting rebuilt/failed and returning the result (and throwing
or returning an exit code). Then make upgradeSandboxes orchestrate these
helpers, preserving existing calls/behavior to sandboxRebuild, askPrompt,
sandboxVersion.checkAgentVersion, captureOpenshell, parseLiveSandboxNames, and
registry.listSandboxes.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

test/e2e/test-upgrade-stale-sandbox.sh (1)

101-114: ⚠️ Potential issue | 🟡 Minor

Blueprint restore may be skipped if docker build fails under set -e.

With set -euo pipefail, a failing docker build at lines 106-110 will exit the script before BUILD_RC=$? (line 111) and before the mv restore (line 113). This leaves the repository with a modified blueprint.yaml.

Consider using a trap to ensure cleanup:

Suggested fix

 cp "${BLUEPRINT}" "${BLUEPRINT_BAK}"
+restore_blueprint() {
+  [ -f "${BLUEPRINT_BAK}" ] && mv "${BLUEPRINT_BAK}" "${BLUEPRINT}"
+}
+trap restore_blueprint EXIT
+
 sed "s/min_openclaw_version:.*/min_openclaw_version: \"${OLD_OPENCLAW_VERSION}\"/" "${BLUEPRINT}" >"${BLUEPRINT}.tmp"
 mv "${BLUEPRINT}.tmp" "${BLUEPRINT}"
 
-docker build \
+if ! docker build \
   --build-arg "OPENCLAW_VERSION=${OLD_OPENCLAW_VERSION}" \
   -f "${REPO_ROOT}/Dockerfile.base" \
   -t "${OLD_BASE_TAG}" \
-  "${REPO_ROOT}"
-BUILD_RC=$?
-
-mv "${BLUEPRINT_BAK}" "${BLUEPRINT}"
-[ "$BUILD_RC" -eq 0 ] || fail "Failed to build old base image"
+  "${REPO_ROOT}"; then
+  fail "Failed to build old base image"
+fi
+
+restore_blueprint
+trap - EXIT

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/e2e/test-upgrade-stale-sandbox.sh` around lines 101 - 114, The script
can exit before restoring BLUEPRINT if docker build fails under set -e; add a
cleanup trap that always restores the original file (move BLUEPRINT_BAK back to
BLUEPRINT) and captures the exit status so BUILD_RC is set correctly after the
docker build attempt; specifically wrap the docker build block so that either a
trap (using trap 'mv "${BLUEPRINT_BAK}" "${BLUEPRINT}"' EXIT) or a local cleanup
function is installed before running docker build, ensure BUILD_RC is assigned
from the docker build exit code (or from $? immediately after the build) and the
trap is cleared/reset after successful restore, referencing variables and steps
BLUEPRINT, BLUEPRINT_BAK, docker build, and BUILD_RC to locate where to add the
trap and restore logic.

test/onboard.test.ts (1)

4703-4714: Add a behavioral test for pullAndResolveBaseImageDigest, not just constant shape.

This check validates SANDBOX_BASE_IMAGE, but it doesn’t exercise the helper’s pull+inspect flow or its null fallback path. A focused unit test for pullAndResolveBaseImageDigest() would better protect the core fix path.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/onboard.test.ts` around lines 4703 - 4714, Add a focused behavioral unit
test that calls pullAndResolveBaseImageDigest (not just checking
SANDBOX_BASE_IMAGE string) and verifies both the successful pull+inspect path
and the null fallback; mock/stub the Docker interactions (the pull and image
inspect/resolve used inside pullAndResolveBaseImageDigest) to simulate a
resolved digest from the nemoclaw/sandbox-base registry and assert the returned
digest and that the registry host in the pulled image matches
SANDBOX_BASE_IMAGE, then add a second sub-case where the pull/inspect mocks
fail/throw and assert pullAndResolveBaseImageDigest returns null to cover the
fallback path.

src/nemoclaw.ts (1)

1894-1903: Minor UX improvement: Clarify rebuild guidance when unknown sandboxes are present.

Line 1901 says "Run nemoclaw upgrade-sandboxes to rebuild them" even when the only sandboxes reported are unknown (version could not be checked). Since unknown sandboxes are not included in the rebuild path (only stale are), this message could be misleading.

Consider conditionally showing this message only when stale.length > 0:

   if (checkOnly) {
     if (stale.length > 0) console.log(`  ${stale.length} sandbox(es) need upgrading.`);
     if (unknown.length > 0) {
       console.log(
         `  ${unknown.length} sandbox(es) could not be version-checked; start them and rerun, or rebuild manually.`,
       );
     }
-    console.log("  Run `nemoclaw upgrade-sandboxes` to rebuild them.");
+    if (stale.length > 0) {
+      console.log("  Run `nemoclaw upgrade-sandboxes` to rebuild them.");
+    }
     return;
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/nemoclaw.ts` around lines 1894 - 1903, The message telling users to run
`nemoclaw upgrade-sandboxes` is shown even when only unknown sandboxes exist;
update the checkOnly branch in the function containing variables checkOnly,
stale, and unknown so that the "Run `nemoclaw upgrade-sandboxes` to rebuild
them." line is only printed when stale.length > 0 (i.e., when there are actually
sandboxes eligible for upgrade); keep the existing unknown diagnostic and
overall early return behavior unchanged.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/lib/onboard.ts (1)

2825-2848: Consider extracting this fallback block to keep createSandbox() from growing further.

This is good logic, but adding more branch-heavy behavior inside createSandbox() makes future maintenance/testing harder.

As per coding guidelines: "Limit cyclomatic complexity to 20 in JavaScript/TypeScript files, with target of 15".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/onboard.ts` around lines 2825 - 2848, The fallback logic checking
pullAndResolveBaseImageDigest(), invoking runCapture(...) to inspect
SANDBOX_BASE_IMAGE and SANDBOX_BASE_TAG, and logging/errors should be extracted
out of createSandbox() into a new helper (e.g., ensureBaseImageAvailableOrExit
or resolveBaseImageWithFallback) that returns the resolved digest or null and
performs the local-check logging and process.exit(1) on hard failure; update
createSandbox() to call this helper and use its return value, keeping all
references to pullAndResolveBaseImageDigest, runCapture, SANDBOX_BASE_IMAGE and
SANDBOX_BASE_TAG inside the new function so the main createSandbox() body is
shorter and cyclomatic complexity is reduced.

test/onboard.test.ts (2)

4753-4764: Test name overstates what is being verified.

This case validates SANDBOX_BASE_IMAGE only; it does not exercise pullAndResolveBaseImageDigest directly. Consider renaming for precision.

✏️ Suggested rename

-it("regression `#1904`: pullAndResolveBaseImageDigest uses sandbox-base registry", () => {
+it("regression `#1904`: SANDBOX_BASE_IMAGE uses sandbox-base registry", () => {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/onboard.test.ts` around lines 4753 - 4764, The test title is misleading:
it claims to verify pullAndResolveBaseImageDigest but only asserts the
SANDBOX_BASE_IMAGE constant; update the test name/description to reflect that it
validates the SANDBOX_BASE_IMAGE value (e.g., "SANDBOX_BASE_IMAGE references
nemoclaw/sandbox-base and not openshell-community") so the behavior matches the
assertions, and leave the assertions on SANDBOX_BASE_IMAGE unchanged; reference
the test block containing the assertions and the SANDBOX_BASE_IMAGE symbol when
making the rename.

---

4766-4778: Order assertion is too broad and can false-pass.

The current indexOf scan is file-wide; it can match unrelated occurrences outside createSandbox. Scope the check to createSandbox source text first.

🎯 More robust scoped assertion

   const source = fs.readFileSync(
     path.join(import.meta.dirname, "..", "src", "lib", "onboard.ts"),
     "utf-8",
   );
-  const pullPos = source.indexOf("pullAndResolveBaseImageDigest()");
-  assert.ok(pullPos !== -1, "pullAndResolveBaseImageDigest() call not found in onboard.ts");
-  const patchPos = source.indexOf("patchStagedDockerfile(", pullPos);
+  const fnMatch = source.match(/async function createSandbox\([\s\S]*?\n\}/);
+  assert.ok(fnMatch, "createSandbox() definition not found in onboard.ts");
+  const fnSource = fnMatch[0];
+  const pullPos = fnSource.indexOf("pullAndResolveBaseImageDigest()");
+  assert.ok(pullPos !== -1, "pullAndResolveBaseImageDigest() call not found in createSandbox()");
+  const patchPos = fnSource.indexOf("patchStagedDockerfile(", pullPos);
   assert.ok(
     patchPos > pullPos,
     "pullAndResolveBaseImageDigest must be called BEFORE patchStagedDockerfile — 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 4766 - 4778, The current test searches the
whole file for pullAndResolveBaseImageDigest() and patchStagedDockerfile( which
can match unrelated code; instead extract the source of createSandbox and run
the ordering assertions inside that scope: locate the createSandbox start by
finding "createSandbox(" or its declaration token (e.g., "function
createSandbox" / "const createSandbox"), compute the function body bounds (e.g.,
find the matching closing brace or use a small regex to capture the function
block), then call indexOf("pullAndResolveBaseImageDigest()") and
indexOf("patchStagedDockerfile(", startAt=0) on that extracted createSandbox
source and assert that pullPos !== -1 and patchPos > pullPos to ensure
pullAndResolveBaseImageDigest is called before patchStagedDockerfile only within
createSandbox.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes