From feb5b922cc8c4277043843b719e18d26330a60bb Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Thu, 21 May 2026 02:47:33 +0000 Subject: [PATCH 1/2] fix(share): target sandbox with -n in remote-path probe (#3889) `buildShareCommandDeps().checkSandboxPathExists` was invoking `openshell sandbox exec -- test -e ` with the sandbox name as a bare positional, so OpenShell parsed it as the command to run and the probe always reported failure. The default `nemoclaw share mount` workflow therefore refused to mount `/sandbox` even when the directory existed and was readable. Add `-n` before the sandbox name to match every other `openshell sandbox exec` call site in the repo, and add a regression test that asserts the exact probe argv. Signed-off-by: Tinson Lai --- src/lib/share-command-deps.ts | 2 +- test/share-command-deps-probe-argv.test.ts | 77 ++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 test/share-command-deps-probe-argv.test.ts diff --git a/src/lib/share-command-deps.ts b/src/lib/share-command-deps.ts index 9c7286474c..f6dd492adf 100644 --- a/src/lib/share-command-deps.ts +++ b/src/lib/share-command-deps.ts @@ -49,7 +49,7 @@ export function buildShareCommandDeps(): ShareCommandDeps { }, checkSandboxPathExists: (sandboxName: string, remotePath: string) => { const result = captureOpenshell( - ["sandbox", "exec", sandboxName, "--", "test", "-e", remotePath], + ["sandbox", "exec", "-n", sandboxName, "--", "test", "-e", remotePath], { ignoreError: true, timeout: OPENSHELL_PROBE_TIMEOUT_MS }, ); return result.status === 0; diff --git a/test/share-command-deps-probe-argv.test.ts b/test/share-command-deps-probe-argv.test.ts new file mode 100644 index 0000000000..0730955e3b --- /dev/null +++ b/test/share-command-deps-probe-argv.test.ts @@ -0,0 +1,77 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { createRequire } from "node:module"; +import { afterEach, describe, expect, it } from "vitest"; + +const require = createRequire(import.meta.url); +const requireCache: Record = require.cache as any; + +// Regression: `nemoclaw share mount` was passing the sandbox name as a bare +// positional to `openshell sandbox exec`, so OpenShell treated it as the +// command to run and the probe always returned a non-zero exit code even +// when `/sandbox` existed. Every other `openshell sandbox exec` call site +// in the repo selects the target sandbox with `-n` (or `--name`). +// See #3889 and #3954. +describe("buildShareCommandDeps().checkSandboxPathExists probe argv", () => { + afterEach(() => { + const openshellRuntimePath = require.resolve("../dist/lib/adapters/openshell/runtime"); + const shareDepsPath = require.resolve("../dist/lib/share-command-deps"); + delete require.cache[openshellRuntimePath]; + delete require.cache[shareDepsPath]; + }); + + it("targets the sandbox with `-n ` so it is not parsed as the command", () => { + const openshellRuntimePath = require.resolve("../dist/lib/adapters/openshell/runtime"); + const shareDepsPath = require.resolve("../dist/lib/share-command-deps"); + + let recordedArgs: readonly string[] | undefined; + requireCache[openshellRuntimePath] = { + id: openshellRuntimePath, + filename: openshellRuntimePath, + loaded: true, + exports: { + captureOpenshell: (args: readonly string[]) => { + recordedArgs = args; + return { status: 0, output: "" }; + }, + }, + } as any; + delete require.cache[shareDepsPath]; + + const { buildShareCommandDeps } = require("../dist/lib/share-command-deps"); + const deps = buildShareCommandDeps(); + const exists = deps.checkSandboxPathExists("prachi-sbox", "/sandbox"); + + expect(exists).toBe(true); + expect(recordedArgs).toEqual([ + "sandbox", + "exec", + "-n", + "prachi-sbox", + "--", + "test", + "-e", + "/sandbox", + ]); + }); + + it("reports the path as missing when the probe exits non-zero", () => { + const openshellRuntimePath = require.resolve("../dist/lib/adapters/openshell/runtime"); + const shareDepsPath = require.resolve("../dist/lib/share-command-deps"); + + requireCache[openshellRuntimePath] = { + id: openshellRuntimePath, + filename: openshellRuntimePath, + loaded: true, + exports: { + captureOpenshell: () => ({ status: 1, output: "" }), + }, + } as any; + delete require.cache[shareDepsPath]; + + const { buildShareCommandDeps } = require("../dist/lib/share-command-deps"); + const deps = buildShareCommandDeps(); + expect(deps.checkSandboxPathExists("alpha", "/sandbox/missing")).toBe(false); + }); +}); From a3ff8a76c53b87d109653bbf833a7d1097bb0551 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Thu, 21 May 2026 03:50:43 +0000 Subject: [PATCH 2/2] fix(e2e): use -n in openshell sandbox exec helpers for rebuild/lifecycle The sandbox-lifecycle and rebuild-upgrade validation suites also passed the sandbox name as a bare positional to `openshell sandbox exec`, mirroring the same bug fixed in `share-command-deps.ts`. Replace 9 call sites across the two helper libraries with `-n ` so the E2E helpers actually target the intended sandbox. Also clean up the doc comment on `ShareCommandDeps.checkSandboxPathExists` and the regression-test rationale, both of which still referenced the old form. Signed-off-by: Tinson Lai --- src/lib/share-command-deps.ts | 2 +- test/e2e/validation_suites/lib/rebuild_upgrade.sh | 10 +++++----- test/e2e/validation_suites/lib/sandbox_lifecycle.sh | 8 ++++---- test/share-command-deps-probe-argv.test.ts | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/lib/share-command-deps.ts b/src/lib/share-command-deps.ts index f6dd492adf..40fd03e5a4 100644 --- a/src/lib/share-command-deps.ts +++ b/src/lib/share-command-deps.ts @@ -12,7 +12,7 @@ export interface ShareCommandDeps { ensureLive: (sandboxName: string) => Promise; /** * Check whether `remotePath` exists inside the sandbox via - * `openshell sandbox exec -- test -e `. Returns true when + * `openshell sandbox exec -n -- test -e `. Returns true when * the path exists; false when it is missing, when the sandbox is unreachable, * or when the exec itself fails. Used by `share mount` as a pre-flight * before invoking `sshfs`, which exits non-zero with empty stderr on a diff --git a/test/e2e/validation_suites/lib/rebuild_upgrade.sh b/test/e2e/validation_suites/lib/rebuild_upgrade.sh index 973fbf394d..96b82917ba 100755 --- a/test/e2e/validation_suites/lib/rebuild_upgrade.sh +++ b/test/e2e/validation_suites/lib/rebuild_upgrade.sh @@ -38,7 +38,7 @@ rebuild_upgrade_assert_sandbox_reachable() { fi local sandbox sandbox="$(_rebuild_upgrade_ctx E2E_SANDBOX_NAME)" - if _rebuild_upgrade_run REBUILD_UPGRADE_SANDBOX_CMD openshell sandbox exec "${sandbox}" -- true; then + if _rebuild_upgrade_run REBUILD_UPGRADE_SANDBOX_CMD openshell sandbox exec -n "${sandbox}" -- true; then e2e_pass "suite.upgrade.survivor_agent_reachable" else e2e_fail "suite.upgrade.survivor_agent_reachable" @@ -55,7 +55,7 @@ rebuild_upgrade_assert_marker_preserved() { sandbox="$(_rebuild_upgrade_ctx E2E_SANDBOX_NAME)" marker_path="${E2E_REBUILD_MARKER_PATH:-/workspace/.nemoclaw-rebuild-marker}" expected="${E2E_REBUILD_MARKER_EXPECTED:-${E2E_STATE_MARKER_EXPECTED:-}}" - actual="$(_rebuild_upgrade_run REBUILD_UPGRADE_SANDBOX_CMD openshell sandbox exec "${sandbox}" -- cat "${marker_path}" 2>/dev/null || true)" + actual="$(_rebuild_upgrade_run REBUILD_UPGRADE_SANDBOX_CMD openshell sandbox exec -n "${sandbox}" -- cat "${marker_path}" 2>/dev/null || true)" if [[ -n "${actual}" && (-z "${expected}" || "${actual}" == "${expected}") ]]; then e2e_pass "suite.rebuild.workspace_state_preserved" else @@ -74,7 +74,7 @@ rebuild_upgrade_assert_agent_version_upgraded() { old="${E2E_OLD_AGENT_VERSION:-}" expected="${E2E_EXPECTED_AGENT_VERSION:-}" cmd="${E2E_AGENT_VERSION_COMMAND:-openclaw --version}" - actual="$(_rebuild_upgrade_run REBUILD_UPGRADE_SANDBOX_CMD openshell sandbox exec "${sandbox}" -- bash -lc "${cmd}" 2>/dev/null || true)" + actual="$(_rebuild_upgrade_run REBUILD_UPGRADE_SANDBOX_CMD openshell sandbox exec -n "${sandbox}" -- bash -lc "${cmd}" 2>/dev/null || true)" if [[ -n "${actual}" && (-z "${old}" || "${actual}" != *"${old}"*) && (-z "${expected}" || "${actual}" == *"${expected}"*) ]]; then e2e_pass "suite.rebuild.agent_version_upgraded" else @@ -91,7 +91,7 @@ rebuild_upgrade_assert_inference_works() { local sandbox cmd output sandbox="$(_rebuild_upgrade_ctx E2E_SANDBOX_NAME)" cmd="${E2E_INFERENCE_CHECK_COMMAND:-curl -fsS http://inference.local/v1/models}" - output="$(_rebuild_upgrade_run REBUILD_UPGRADE_SANDBOX_CMD openshell sandbox exec "${sandbox}" -- bash -lc "${cmd}" 2>/dev/null || true)" + output="$(_rebuild_upgrade_run REBUILD_UPGRADE_SANDBOX_CMD openshell sandbox exec -n "${sandbox}" -- bash -lc "${cmd}" 2>/dev/null || true)" if [[ -n "${output}" ]]; then e2e_pass "suite.rebuild.inference_still_works" else @@ -129,7 +129,7 @@ rebuild_upgrade_assert_hermes_config_preserved() { fi local sandbox output sandbox="$(_rebuild_upgrade_ctx E2E_SANDBOX_NAME)" - output="$(_rebuild_upgrade_run REBUILD_UPGRADE_SANDBOX_CMD openshell sandbox exec "${sandbox}" -- bash -lc "grep -R 'platforms.discord\|DISCORD' ~/.hermes . 2>/dev/null" || true)" + output="$(_rebuild_upgrade_run REBUILD_UPGRADE_SANDBOX_CMD openshell sandbox exec -n "${sandbox}" -- bash -lc "grep -R 'platforms.discord\|DISCORD' ~/.hermes . 2>/dev/null" || true)" if [[ "${output}" == *"discord"* || "${output}" == *"DISCORD"* ]]; then e2e_pass "suite.rebuild.hermes_config_preserved" else diff --git a/test/e2e/validation_suites/lib/sandbox_lifecycle.sh b/test/e2e/validation_suites/lib/sandbox_lifecycle.sh index 36cbd457ea..df942487e7 100755 --- a/test/e2e/validation_suites/lib/sandbox_lifecycle.sh +++ b/test/e2e/validation_suites/lib/sandbox_lifecycle.sh @@ -105,7 +105,7 @@ sandbox_lifecycle_assert_logs_available() { sandbox_lifecycle_assert_openshell_exec_ok() { local id="validation.sandbox_operations.openshell_exec_ok" - sandbox_lifecycle_run_with_timeout 20 openshell sandbox exec "${E2E_SANDBOX_NAME}" -- sh -lc 'echo lifecycle-ok' >/dev/null || { + sandbox_lifecycle_run_with_timeout 20 openshell sandbox exec -n "${E2E_SANDBOX_NAME}" -- sh -lc 'echo lifecycle-ok' >/dev/null || { sandbox_lifecycle_fail "${id}" "openshell exec failed" return 1 } @@ -139,7 +139,7 @@ sandbox_lifecycle_assert_gateway_recovers_after_probe() { } sandbox_lifecycle_assert_snapshot_create_list_restore_marker() { - sandbox_lifecycle_run_with_timeout 30 openshell sandbox exec "${E2E_SANDBOX_NAME}" -- sh -lc 'echo lifecycle-marker-before-snapshot > /tmp/nemoclaw-lifecycle-marker' >/dev/null || { + sandbox_lifecycle_run_with_timeout 30 openshell sandbox exec -n "${E2E_SANDBOX_NAME}" -- sh -lc 'echo lifecycle-marker-before-snapshot > /tmp/nemoclaw-lifecycle-marker' >/dev/null || { sandbox_lifecycle_fail validation.sandbox_snapshot.marker_written "failed to write marker" return 1 } @@ -149,7 +149,7 @@ sandbox_lifecycle_assert_snapshot_create_list_restore_marker() { return 1 } sandbox_lifecycle_pass validation.sandbox_snapshot.create_succeeds "snapshot create succeeded" - sandbox_lifecycle_run_with_timeout 30 openshell sandbox exec "${E2E_SANDBOX_NAME}" -- sh -lc 'echo lifecycle-marker-after-snapshot > /tmp/nemoclaw-lifecycle-marker' >/dev/null || { + sandbox_lifecycle_run_with_timeout 30 openshell sandbox exec -n "${E2E_SANDBOX_NAME}" -- sh -lc 'echo lifecycle-marker-after-snapshot > /tmp/nemoclaw-lifecycle-marker' >/dev/null || { sandbox_lifecycle_fail validation.sandbox_snapshot.restore_rolls_back_marker "failed to mutate marker" return 1 } @@ -162,7 +162,7 @@ sandbox_lifecycle_assert_snapshot_create_list_restore_marker() { sandbox_lifecycle_fail validation.sandbox_snapshot.restore_rolls_back_marker "snapshot restore failed" return 1 } - sandbox_lifecycle_run_with_timeout 30 openshell sandbox exec "${E2E_SANDBOX_NAME}" -- sh -lc 'test -f /tmp/nemoclaw-lifecycle-marker && grep -Fxq lifecycle-marker-before-snapshot /tmp/nemoclaw-lifecycle-marker' >/dev/null || { + sandbox_lifecycle_run_with_timeout 30 openshell sandbox exec -n "${E2E_SANDBOX_NAME}" -- sh -lc 'test -f /tmp/nemoclaw-lifecycle-marker && grep -Fxq lifecycle-marker-before-snapshot /tmp/nemoclaw-lifecycle-marker' >/dev/null || { sandbox_lifecycle_fail validation.sandbox_snapshot.restore_rolls_back_marker "marker did not roll back" return 1 } diff --git a/test/share-command-deps-probe-argv.test.ts b/test/share-command-deps-probe-argv.test.ts index 0730955e3b..20bdde44a0 100644 --- a/test/share-command-deps-probe-argv.test.ts +++ b/test/share-command-deps-probe-argv.test.ts @@ -10,8 +10,8 @@ const requireCache: Record = require.cache as any; // Regression: `nemoclaw share mount` was passing the sandbox name as a bare // positional to `openshell sandbox exec`, so OpenShell treated it as the // command to run and the probe always returned a non-zero exit code even -// when `/sandbox` existed. Every other `openshell sandbox exec` call site -// in the repo selects the target sandbox with `-n` (or `--name`). +// when `/sandbox` existed. The convention in this repo is to select the +// target sandbox with `-n` (or `--name`). // See #3889 and #3954. describe("buildShareCommandDeps().checkSandboxPathExists probe argv", () => { afterEach(() => {