diff --git a/src/lib/adapters/openshell/client.test.ts b/src/lib/adapters/openshell/client.test.ts index 37fec1735ac..29b0f829900 100644 --- a/src/lib/adapters/openshell/client.test.ts +++ b/src/lib/adapters/openshell/client.test.ts @@ -291,6 +291,31 @@ describe("openshell helpers", () => { ]); }); + it("scopes both sandbox lookups to the requested gateway (#7429)", () => { + const calls: string[][] = []; + const spawnSyncImpl: OpenshellSpawnSync = (_command, args) => { + calls.push([...args]); + return makeSpawnResult({ + status: 0, + stdout: args.includes("get") ? "alpha Ready\n" : "Host openshell-alpha\n", + stderr: "", + }); + }; + + captureSandboxSshConfigCommand("openshell", "alpha", { + spawnSyncImpl, + gatewayName: "nemoclaw-18080", + }); + + // Both hops must carry the gateway: `get` succeeding on one gateway while + // `ssh-config` resolves against another would emit a config for the wrong + // sandbox. + expect(calls).toEqual([ + ["sandbox", "get", "-g", "nemoclaw-18080", "alpha"], + ["sandbox", "ssh-config", "-g", "nemoclaw-18080", "alpha"], + ]); + }); + it("does not request SSH config when the sandbox is missing", () => { const calls: string[][] = []; const spawnSyncImpl: OpenshellSpawnSync = (_command, args) => { diff --git a/src/lib/adapters/openshell/client.ts b/src/lib/adapters/openshell/client.ts index 031e193992a..c635911433f 100644 --- a/src/lib/adapters/openshell/client.ts +++ b/src/lib/adapters/openshell/client.ts @@ -57,6 +57,18 @@ export interface CaptureOpenshellAsyncOptions extends CaptureOpenshellOptions { spawnImpl?: OpenshellSpawn; } +export interface CaptureSandboxSshConfigOptions extends CaptureOpenshellOptions { + /** + * Gateway the sandbox is recorded against (`resolveSandboxGatewayName`). + * `sandbox get` and `sandbox ssh-config` resolve against OpenShell's mutable + * current selection when no gateway is given, so a caller that knows the + * sandbox's own binding must pass it — otherwise the lookup can land on a + * sibling gateway and report the sandbox as missing (#7429). Omitted keeps + * the ambient-selection behavior for callers that have no binding to supply. + */ + gatewayName?: string; +} + export interface CaptureOpenshellResult { status: number | null; output: string; @@ -238,16 +250,32 @@ export function captureOpenshellCommand( }; } +/** + * Insert `-g ` after the subcommand pair, matching the placement + * `gatewayScopedArgs` already uses in `actions/sandbox/gateway-state.ts`. + * Duplicated rather than imported: an adapter must not depend on the actions + * layer. + */ +function gatewayScopedArgs(args: string[], gatewayName?: string): string[] { + if (!gatewayName) return args; + return [...args.slice(0, 2), "-g", gatewayName, ...args.slice(2)]; +} + export function captureSandboxSshConfigCommand( binary: string, sandboxName: string, - opts: CaptureOpenshellOptions = {}, + opts: CaptureSandboxSshConfigOptions = {}, ): CaptureOpenshellResult { - const sandboxGet = captureOpenshellCommand(binary, ["sandbox", "get", sandboxName], { - ...opts, - ignoreError: true, - includeStderr: true, - }); + const { gatewayName, ...spawnOpts } = opts; + const sandboxGet = captureOpenshellCommand( + binary, + gatewayScopedArgs(["sandbox", "get", sandboxName], gatewayName), + { + ...spawnOpts, + ignoreError: true, + includeStderr: true, + }, + ); if (sandboxGet.status !== 0) { const output = sandboxGet.output || `failed to query sandbox '${sandboxName}'`; const sandboxMissing = /\bnot[- ]?found\b/i.test(output); @@ -256,7 +284,13 @@ export function captureSandboxSshConfigCommand( output: sandboxMissing ? `sandbox '${sandboxName}' not found` : output, }; } - return captureOpenshellCommand(binary, ["sandbox", "ssh-config", sandboxName], opts); + // Pin every hop to the same gateway so `get` and `ssh-config` cannot + // disagree about which one owns the sandbox. + return captureOpenshellCommand( + binary, + gatewayScopedArgs(["sandbox", "ssh-config", sandboxName], gatewayName), + spawnOpts, + ); } export function captureOpenshellCommandAsync( diff --git a/src/lib/sandbox/version.test.ts b/src/lib/sandbox/version.test.ts index 3211f4b7494..d3f179b9b4f 100644 --- a/src/lib/sandbox/version.test.ts +++ b/src/lib/sandbox/version.test.ts @@ -162,10 +162,13 @@ describe("checkAgentVersion", () => { expect(result.detectionMethod).toBe("ssh-exec"); expect(result.sandboxVersion).toBe("2026.5.27"); expect(result.isStale).toBe(false); + // A row that pre-dates the per-port migration resolves to the canonical + // default gateway, and the probe pins to it explicitly rather than + // inheriting OpenShell's current selection (#7429). expect(captureSandboxSshConfigCommand).toHaveBeenCalledWith( "/usr/local/bin/openshell", "test-sb", - { ignoreError: true, timeout: OPENSHELL_PROBE_TIMEOUT_MS }, + { ignoreError: true, timeout: OPENSHELL_PROBE_TIMEOUT_MS, gatewayName: "nemoclaw" }, ); const sshArgs = vi.mocked(spawnSync).mock.calls[0]?.[1] as string[]; const configFile = sshArgs[sshArgs.indexOf("-F") + 1]; @@ -180,6 +183,103 @@ describe("checkAgentVersion", () => { expect(updated?.agentVersion).toBe("2026.5.27"); }); + it("probes the sandbox's own recorded gateway, not OpenShell's ambient selection (#7429)", () => { + // A sandbox onboarded under a non-default NEMOCLAW_GATEWAY_PORT is + // registered against `nemoclaw-`. `openshell sandbox get` / + // `ssh-config` fall back to OpenShell's mutable current selection when no + // gateway is given, so an unscoped probe queries the wrong gateway, + // returns "not found", and the sandbox is reported as `v?` even though the + // gateway-scoped sandbox listing observed it as live. + registry.registerSandbox({ name: "test-sb", agent: null, gatewayPort: 18080 }); + + vi.mocked(captureSandboxSshConfigCommand).mockReturnValue({ + status: 0, + output: "Host openshell-test-sb\n HostName 127.0.0.1\n", + }); + vi.mocked(spawnSync).mockReturnValue({ + status: 0, + stdout: "OpenClaw 2026.5.27 (abc123)\n", + stderr: "", + pid: 1234, + output: [], + signal: null, + }); + + const result = checkAgentVersion("test-sb", { forceProbe: true }); + + expect(result.detectionMethod).toBe("ssh-exec"); + expect(captureSandboxSshConfigCommand).toHaveBeenCalledWith( + "/usr/local/bin/openshell", + "test-sb", + { + ignoreError: true, + timeout: OPENSHELL_PROBE_TIMEOUT_MS, + gatewayName: "nemoclaw-18080", + }, + ); + }); + + it("scopes a Hermes sandbox on a non-default gateway to its own gateway (#7429)", () => { + // The exact reported topology: a Hermes sandbox onboarded under a + // non-default NEMOCLAW_GATEWAY_PORT. Before the fix the probe queried + // OpenShell's ambient selection, came back "not found", and + // `upgrade-sandboxes --check` printed `v? → v0.18.0`. + registry.registerSandbox({ name: "hermes-sb", agent: "hermes", gatewayPort: 18080 }); + + vi.mocked(captureSandboxSshConfigCommand).mockReturnValue({ + status: 0, + output: "Host openshell-hermes-sb\n HostName 127.0.0.1\n", + }); + vi.mocked(spawnSync).mockReturnValue({ + status: 0, + stdout: "Hermes Agent 0.17.0\n", + stderr: "", + pid: 1234, + output: [], + signal: null, + }); + + const result = checkAgentVersion("hermes-sb", { forceProbe: true }); + + expect(captureSandboxSshConfigCommand).toHaveBeenCalledWith( + "/usr/local/bin/openshell", + "hermes-sb", + { + ignoreError: true, + timeout: OPENSHELL_PROBE_TIMEOUT_MS, + gatewayName: "nemoclaw-18080", + }, + ); + // The version resolves instead of landing in the "Unknown version" bucket. + expect(result.detectionMethod).toBe("ssh-exec"); + expect(result.sandboxVersion).toBe("0.17.0"); + expect(result.verificationFailed).toBe(false); + // The Hermes agent definition drives the probe, not the openclaw default. + const sshArgs = vi.mocked(spawnSync).mock.calls[0]?.[1] as string[]; + expect(sshArgs).toContain("hermes --version"); + }); + + it("does not probe at all when the persisted gateway binding is corrupted (#7429)", () => { + // resolveSandboxGatewayName fails closed on an invalid binding. Falling + // back to an unscoped probe would defeat that: OpenShell would resolve the + // name against its ambient selection, so a same-named sandbox on another + // gateway could be probed and its version cached onto this row. A row with + // no binding fields resolves to the canonical default instead of throwing, + // so this path is reached only by genuinely corrupted state. + registry.registerSandbox({ name: "test-sb", agent: null, gatewayName: "not-a-nemoclaw-gw" }); + + const result = checkAgentVersion("test-sb", { forceProbe: true }); + + expect(captureSandboxSshConfigCommand).not.toHaveBeenCalled(); + expect(spawnSync).not.toHaveBeenCalled(); + // No probe was attempted, so the contract's `unavailable` applies — + // `unknown`/`probe-failed` would claim a probe ran and failed. + expect(result.detectionMethod).toBe("unavailable"); + expect(result.unavailableReason).toBe("invalid-gateway-binding"); + expect(result.verificationFailed).toBe(true); + expect(result.sandboxVersion).toBeNull(); + }); + it("returns an unknown verdict when SSH config fails so callers do not read isStale as verified current", () => { registry.registerSandbox({ name: "test-sb", agent: null }); diff --git a/src/lib/sandbox/version.ts b/src/lib/sandbox/version.ts index 59e535c6229..7fcfe6a004e 100644 --- a/src/lib/sandbox/version.ts +++ b/src/lib/sandbox/version.ts @@ -17,6 +17,7 @@ import { import { resolveOpenshell } from "../adapters/openshell/resolve.js"; import { OPENSHELL_PROBE_TIMEOUT_MS } from "../adapters/openshell/timeouts.js"; import { loadAgent } from "../agent/defs.js"; +import { resolveSandboxGatewayName } from "../onboard/gateway-binding.js"; import * as registry from "../state/registry.js"; import { createTempSshConfig } from "./temp-ssh-config.js"; import { evaluateStaleness } from "./version-scheme.js"; @@ -58,7 +59,11 @@ export interface VersionCheckResult { */ schemeMismatch?: boolean; /** Categorises why the result could not be computed, so callers can surface a distinct state. */ - unavailableReason?: "no-expected-version" | "skip-probe" | "probe-failed"; + unavailableReason?: + | "no-expected-version" + | "skip-probe" + | "probe-failed" + | "invalid-gateway-binding"; } /** @@ -79,19 +84,48 @@ function resolveAgentForSandbox(sandboxName: string): ReturnType { `marker_file=${JSON.stringify(markerFile)}`, `state_file=${JSON.stringify(stateFile)}`, 'printf \'%s\\n\' "$*" >> "$marker_file"', - 'if [ "$1" = "sandbox" ] && [ "$2" = "get" ] && [ "$3" = "alpha" ]; then', + 'if [ "$1" = "sandbox" ] && [ "$2" = "get" ] && [ "$3" = "-g" ] && [ "$4" = "nemoclaw" ] && [ "$5" = "alpha" ]; then', " echo 'Sandbox:'", " echo", " echo ' Id: abc'", @@ -97,7 +97,7 @@ describe("CLI connect readiness", () => { expect(r.out.includes("Waiting for sandbox 'alpha' to be ready")).toBeTruthy(); expect(r.out.includes("Sandbox is ready. Connecting")).toBeTruthy(); const calls = fs.readFileSync(markerFile, "utf8").trim().split("\n").filter(Boolean); - expect(calls).toContain("sandbox get alpha"); + expect(calls).toContain("sandbox get -g nemoclaw alpha"); expect( calls.filter((call) => call === "sandbox list -g nemoclaw").length, ).toBeGreaterThanOrEqual(2); @@ -136,7 +136,7 @@ describe("CLI connect readiness", () => { "#!/usr/bin/env bash", `marker_file=${JSON.stringify(markerFile)}`, 'printf \'%s\\n\' "$*" >> "$marker_file"', - 'if [ "$1" = "sandbox" ] && [ "$2" = "get" ] && [ "$3" = "alpha" ]; then', + 'if [ "$1" = "sandbox" ] && [ "$2" = "get" ] && [ "$3" = "-g" ] && [ "$4" = "nemoclaw" ] && [ "$5" = "alpha" ]; then', " echo 'Sandbox:'", " echo", " echo ' Id: abc'", @@ -221,7 +221,7 @@ describe("CLI connect readiness", () => { "#!/usr/bin/env bash", `marker_file=${JSON.stringify(markerFile)}`, 'printf \'%s\\n\' "$*" >> "$marker_file"', - 'if [ "$1" = "sandbox" ] && [ "$2" = "get" ] && [ "$3" = "alpha" ]; then', + 'if [ "$1" = "sandbox" ] && [ "$2" = "get" ] && [ "$3" = "-g" ] && [ "$4" = "nemoclaw" ] && [ "$5" = "alpha" ]; then', " echo 'Sandbox:'", " echo", " echo ' Id: abc'", @@ -253,7 +253,7 @@ describe("CLI connect readiness", () => { expect(r.out.includes("nemoclaw alpha logs --follow")).toBeTruthy(); expect(r.out.includes("nemoclaw alpha status")).toBeTruthy(); const calls = fs.readFileSync(markerFile, "utf8").trim().split("\n").filter(Boolean); - expect(calls).toContain("sandbox get alpha"); + expect(calls).toContain("sandbox get -g nemoclaw alpha"); expect(calls).toContain("sandbox list -g nemoclaw"); expect(calls).not.toContain("should-not-connect"); }); diff --git a/test/cli/connect-recovery.test.ts b/test/cli/connect-recovery.test.ts index 09094c2c390..dfa06e7f176 100644 --- a/test/cli/connect-recovery.test.ts +++ b/test/cli/connect-recovery.test.ts @@ -479,7 +479,7 @@ describe("CLI connect recovery process contracts", () => { expect(result.code).toBe(0); const calls = fs.readFileSync(markerFile, "utf8"); expect(calls).toContain("sandbox list"); - expect(calls).toContain("sandbox get alpha"); + expect(calls).toContain("sandbox get -g nemoclaw alpha"); expect(calls).toContain("sandbox connect alpha"); const recoveredRegistry = JSON.parse( fs.readFileSync(path.join(nemoclawDir, "sandboxes.json"), "utf8"), diff --git a/test/cli/list-share-live-inference.test.ts b/test/cli/list-share-live-inference.test.ts index 7ddac87d89a..843d6a0ad4b 100644 --- a/test/cli/list-share-live-inference.test.ts +++ b/test/cli/list-share-live-inference.test.ts @@ -253,11 +253,11 @@ describe("list shows live gateway inference", () => { ' echo "my-agent Running openclaw"', " exit 0", "fi", - 'if [ "$1" = "sandbox" ] && [ "$2" = "get" ] && [ "$3" = "my-agent" ]; then', + 'if [ "$1" = "sandbox" ] && [ "$2" = "get" ] && [ "$3" = "-g" ] && [ "$4" = "nemoclaw" ] && [ "$5" = "my-agent" ]; then', " echo 'Sandbox: my-agent'", " exit 0", "fi", - 'if [ "$1" = "sandbox" ] && [ "$2" = "ssh-config" ] && [ "$3" = "my-agent" ]; then', + 'if [ "$1" = "sandbox" ] && [ "$2" = "ssh-config" ] && [ "$3" = "-g" ] && [ "$4" = "nemoclaw" ] && [ "$5" = "my-agent" ]; then', " echo 'Host openshell-my-agent'", " echo ' HostName 127.0.0.1'", " exit 0", @@ -327,11 +327,11 @@ describe("list shows live gateway inference", () => { ' echo "my-agent Running openclaw"', " exit 0", "fi", - 'if [ "$1" = "sandbox" ] && [ "$2" = "get" ] && [ "$3" = "my-agent" ]; then', + 'if [ "$1" = "sandbox" ] && [ "$2" = "get" ] && [ "$3" = "-g" ] && [ "$4" = "nemoclaw" ] && [ "$5" = "my-agent" ]; then', " echo 'Sandbox: my-agent'", " exit 0", "fi", - 'if [ "$1" = "sandbox" ] && [ "$2" = "ssh-config" ] && [ "$3" = "my-agent" ]; then', + 'if [ "$1" = "sandbox" ] && [ "$2" = "ssh-config" ] && [ "$3" = "-g" ] && [ "$4" = "nemoclaw" ] && [ "$5" = "my-agent" ]; then', " echo 'Host openshell-my-agent'", " echo ' HostName 127.0.0.1'", " exit 0", @@ -405,7 +405,7 @@ describe("list shows live gateway inference", () => { ' echo "my-agent Running openclaw"', " exit 0", "fi", - 'if [ "$1" = "sandbox" ] && [ "$2" = "ssh-config" ] && [ "$3" = "my-agent" ]; then', + 'if [ "$1" = "sandbox" ] && [ "$2" = "ssh-config" ] && [ "$3" = "-g" ] && [ "$4" = "nemoclaw" ] && [ "$5" = "my-agent" ]; then', " echo 'Host openshell-my-agent'", " echo ' HostName 127.0.0.1'", " exit 0", @@ -478,11 +478,11 @@ describe("list shows live gateway inference", () => { ' echo "my-agent Running openclaw"', " exit 0", "fi", - 'if [ "$1" = "sandbox" ] && [ "$2" = "get" ] && [ "$3" = "my-agent" ]; then', + 'if [ "$1" = "sandbox" ] && [ "$2" = "get" ] && [ "$3" = "-g" ] && [ "$4" = "nemoclaw" ] && [ "$5" = "my-agent" ]; then', " echo 'Sandbox: my-agent'", " exit 0", "fi", - 'if [ "$1" = "sandbox" ] && [ "$2" = "ssh-config" ] && [ "$3" = "my-agent" ]; then', + 'if [ "$1" = "sandbox" ] && [ "$2" = "ssh-config" ] && [ "$3" = "-g" ] && [ "$4" = "nemoclaw" ] && [ "$5" = "my-agent" ]; then', " echo 'Host openshell-my-agent'", " echo ' HostName 127.0.0.1'", " exit 0",