Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions src/lib/adapters/openshell/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
48 changes: 41 additions & 7 deletions src/lib/adapters/openshell/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -238,16 +250,32 @@ export function captureOpenshellCommand(
};
}

/**
* Insert `-g <gateway>` 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);
Expand All @@ -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(
Expand Down
102 changes: 101 additions & 1 deletion src/lib/sandbox/version.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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-<port>`. `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 });

Expand Down
56 changes: 53 additions & 3 deletions src/lib/sandbox/version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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";
}

/**
Expand All @@ -79,19 +84,48 @@ function resolveAgentForSandbox(sandboxName: string): ReturnType<typeof loadAgen
return loadAgent(agentName);
}

/**
* Gateway to scope the probe to, or null when the persisted binding was
* rejected. An entry with no binding fields resolves to the canonical default
* gateway, so null here means the row is corrupted — never that scoping is
* unnecessary.
*/
function resolveProbeGatewayName(sandboxName: string): string | null {
try {
return resolveSandboxGatewayName(registry.getSandbox(sandboxName));
} catch {
return null;
}
}

/**
* Probe the live agent version inside a sandbox via SSH.
* Returns the parsed version string or null on failure.
*/
export function probeAgentVersion(sandboxName: string): string | null {
export function probeAgentVersion(sandboxName: string, gatewayName?: string): string | null {
const agent = resolveAgentForSandbox(sandboxName);

// Scope the lookup to the sandbox's own gateway. Without it OpenShell
// resolves `sandbox get`/`ssh-config` against its mutable current selection,
// so a sandbox bound to a sibling gateway is reported missing and its
// version renders as `v?` (#7429).
//
// Resolved before the binary lookup: a rejected binding must not fall back to
// the ambient gateway (the case `resolveSandboxGatewayName` fails closed on,
// because a same-named sandbox on another gateway would be probed and its
// version cached onto this row), and there is no reason to shell out to
// `command -v openshell` only to discard the result. A caller that already
// resolved the binding passes it in rather than re-reading the registry.
const probeGatewayName = gatewayName ?? resolveProbeGatewayName(sandboxName);
if (probeGatewayName === null) return null;

const openshellBinary = resolveOpenshell();
if (!openshellBinary) return null;

const sshConfigResult = captureSandboxSshConfigCommand(openshellBinary, sandboxName, {
ignoreError: true,
timeout: OPENSHELL_PROBE_TIMEOUT_MS,
gatewayName: probeGatewayName,
});
if (sshConfigResult.status !== 0) return null;
if (!sshConfigResult.output.trim()) return null;
Expand Down Expand Up @@ -186,8 +220,24 @@ export function checkAgentVersion(
};
}

// A rejected gateway binding means no probe is attempted at all, which the
// result contract distinguishes from a probe that ran and failed: report
// `unavailable` rather than `unknown`/`probe-failed` so an operator can tell
// a corrupted registry row from an unreachable sandbox.
const probeGatewayName = resolveProbeGatewayName(sandboxName);
if (probeGatewayName === null) {
return {
sandboxVersion: null,
expectedVersion,
isStale: false,
verificationFailed: true,
detectionMethod: "unavailable",
unavailableReason: "invalid-gateway-binding",
};
}

// Slow path: SSH exec into sandbox
const probed = probeAgentVersion(sandboxName);
const probed = probeAgentVersion(sandboxName, probeGatewayName);
if (probed && sb) {
// Cache for future fast-path lookups
registry.updateSandbox(sandboxName, { agentVersion: probed });
Expand Down
10 changes: 5 additions & 5 deletions test/cli/connect-readiness.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe("CLI connect readiness", () => {
`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'",
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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'",
Expand Down Expand Up @@ -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'",
Expand Down Expand Up @@ -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");
});
Expand Down
2 changes: 1 addition & 1 deletion test/cli/connect-recovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
Loading
Loading