test/e2e/test-openshell-gateway-upgrade.sh (1)

84-93: ⚡ Quick win

Restore sandboxes.json after test to avoid persistent local side effects.

At Line 213-237 the script mutates $HOME/.nemoclaw/sandboxes.json, but cleanup (Line 84-93) doesn’t revert it. This can leave stale local state after manual runs.

♻️ Proposed patch

@@
 REGISTRY_FILE="$HOME/.nemoclaw/sandboxes.json"
+REGISTRY_BACKUP=""
@@
 cleanup() {
   set +e
   cleanup_pid "$OLD_PID"
   cleanup_pid "$NEW_PID"
@@
   rm -f "$PID_FILE"
+  if [ "${REGISTRY_BACKUP:-}" = "__ABSENT__" ]; then
+    rm -f "$REGISTRY_FILE"
+  elif [ -n "${REGISTRY_BACKUP:-}" ] && [ -f "$REGISTRY_BACKUP" ]; then
+    mkdir -p "$(dirname "$REGISTRY_FILE")"
+    cp "$REGISTRY_BACKUP" "$REGISTRY_FILE"
+    rm -f "$REGISTRY_BACKUP"
+  fi
 }
@@
 create_survivor_sandbox() {
@@
   mkdir -p "$(dirname "$REGISTRY_FILE")"
+  if [ -z "${REGISTRY_BACKUP:-}" ]; then
+    if [ -f "$REGISTRY_FILE" ]; then
+      REGISTRY_BACKUP="$(mktemp)"
+      cp "$REGISTRY_FILE" "$REGISTRY_BACKUP"
+    else
+      REGISTRY_BACKUP="__ABSENT__"
+    fi
+  fi
   python3 - <<PY

Also applies to: 212-237

🤖 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 `@test/e2e/test-openshell-gateway-upgrade.sh` around lines 84 - 93, The test
mutates the local sandboxes JSON but never restores it; before any mutation (in
the setup section around where sandboxes.json is modified) save the original
contents to a temp backup (e.g., BACKUP_SANDBOXES or a tmp file) and after the
test ensure restoration in the cleanup() function; update cleanup() (which
already calls cleanup_pid, handles OLD_PID/NEW_PID, openshell sandbox
delete/SURVIVOR_SANDBOX and removes PID_FILE) to detect the backup and restore
the original sandboxes JSON (or remove the temporary copy) so the local state is
reverted even on early exits.

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-openshell-gateway-upgrade.sh (1)

406-410: 💤 Low value

Duplicate local declaration for marker.

Line 406 declares local marker pid counter, then line 410 declares local marker again. The second declaration is redundant.

♻️ Proposed fix

 assert_survivor_sandbox_after_upgrade() {
-  local marker pid counter
+  local pid counter marker
   info "Verifying survivor sandbox after OpenShell gateway upgrade"
   wait_for_survivor_ready || fail "survivor sandbox is not Ready after gateway upgrade"

-  local marker
   marker="$(

🤖 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 `@test/e2e/test-openshell-gateway-upgrade.sh` around lines 406 - 410, Remove
the redundant local declaration for marker: keep the original single declaration
"local marker pid counter" and delete the subsequent "local marker" line so the
variables are declared only once (look for the duplicate declaration of marker
in the test-openshell-gateway-upgrade.sh script near the survivor readiness
check).

src/lib/onboard.ts (1)

3927-3931: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Darwin enablement still routes through Linux-only /proc checks.

With this gate returning true on macOS, the reuse/drift path now reaches readProcessEnv / readProcessExe and the requireDockerDriverEnv: true checks below. Those are Linux-only, so healthy openshell-gateway processes on Darwin will be classified as stale/unowned and get restarted or fail adoption on later onboard/resume runs. This still needs the non-Linux guard/fallback that was already called out earlier.

🤖 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.ts` around lines 3927 - 3931, The function
isLinuxDockerDriverGatewayEnabled currently returns true for darwin which lets
macOS code flow into Linux-only checks (readProcessEnv, readProcessExe and
places using requireDockerDriverEnv: true) causing healthy openshell-gateway
processes to be misclassified; change the gate to only enable on Linux (return
platform === "linux") or, alternatively, keep the function but add an explicit
platform guard where callers invoke readProcessEnv/readProcessExe and check
requireDockerDriverEnv so those Linux-specific checks are skipped on darwin,
ensuring macOS follows the non-Linux fallback path.

test/e2e/test-openshell-gateway-upgrade.sh (1)

358-401: 💤 Low value

Redundant local marker declaration.

Line 359 already declares marker as local, so the local marker on line 363 is redundant.

♻️ Suggested cleanup

 assert_survivor_sandbox_after_upgrade() {
   local marker pid counter
   info "Verifying survivor sandbox after OpenShell gateway upgrade"
   wait_for_survivor_ready || fail "survivor sandbox is not Ready after gateway upgrade"

-  local marker
   marker="$(
     openshell sandbox exec --name "$SURVIVOR_SANDBOX" -- \
       cat /tmp/nemoclaw-gateway-upgrade-marker 2>/dev/null || true
   )"

🤖 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 `@test/e2e/test-openshell-gateway-upgrade.sh` around lines 358 - 401, In
function assert_survivor_sandbox_after_upgrade remove the duplicate local
declaration for marker (the second "local marker" before assigning
marker="$(openshell sandbox exec ... )"); keep the initial local
marker,pid,counter declaration at the top and delete the redundant declaration
so marker is only declared once.

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-openshell-gateway-upgrade.sh (1)

419-427: 💤 Low value

Remove duplicate local marker declaration.

marker is declared twice: at line 419 and again at line 423. This is redundant and likely a copy-paste artifact.

Proposed fix

 assert_survivor_sandbox_after_upgrade() {
   local marker pid counter
   info "Verifying survivor sandbox after OpenShell gateway upgrade"
   wait_for_survivor_ready || fail "survivor sandbox is not Ready after gateway upgrade"

-  local marker
   marker="$(
     openshell sandbox exec --name "$SURVIVOR_SANDBOX" -- \
       cat /tmp/nemoclaw-gateway-upgrade-marker 2>/dev/null || true

🤖 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 `@test/e2e/test-openshell-gateway-upgrade.sh` around lines 419 - 427, The file
declares the variable "marker" twice; remove the redundant declaration so
"marker" is declared only once before it's assigned. Specifically, delete the
earlier standalone "local marker" and keep the single "local marker" that
directly precedes the assignment using openshell sandbox exec (referencing
SURVIVOR_SANDBOX and the cat /tmp/nemoclaw-gateway-upgrade-marker command); no
other logic changes needed and wait_for_survivor_ready() and subsequent uses of
marker should remain unchanged.

Fix all unresolved CodeRabbit comments on this PR:

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