diff --git a/src/lib/adapters/openshell/sandbox-identity.test.ts b/src/lib/adapters/openshell/sandbox-identity.test.ts index 422bf71ccd8..0b623d9e763 100644 --- a/src/lib/adapters/openshell/sandbox-identity.test.ts +++ b/src/lib/adapters/openshell/sandbox-identity.test.ts @@ -1,9 +1,9 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; -import { parseOpenShellSandboxId } from "./sandbox-identity"; +import { createOpenshellSandboxIdReader, parseOpenShellSandboxId } from "./sandbox-identity"; describe("OpenShell sandbox identity parsing", () => { it("accepts one exact durable ID with optional terminal color", () => { @@ -19,3 +19,29 @@ describe("OpenShell sandbox identity parsing", () => { expect(parseOpenShellSandboxId("id: sandbox-alpha\n")).toBeNull(); }); }); + +describe("OpenShell sandbox identity reading", () => { + it("reads each sandbox ID once per process (#9316)", () => { + const runCommand = vi.fn(() => ({ status: 0, stdout: "Name: alpha\nID: sandbox-alpha\n" })); + const readSandboxId = createOpenshellSandboxIdReader("/usr/bin/openshell", runCommand); + + expect(readSandboxId("alpha")).toBe("sandbox-alpha"); + expect(readSandboxId("alpha")).toBe("sandbox-alpha"); + expect(runCommand).toHaveBeenCalledExactlyOnceWith("/usr/bin/openshell", [ + "sandbox", + "get", + "alpha", + ]); + }); + + it("caches a failed sandbox ID lookup as unavailable (#9316)", () => { + const runCommand = vi.fn((): { status: number; stdout: string } => { + throw new Error("OpenShell unavailable"); + }); + const readSandboxId = createOpenshellSandboxIdReader("/usr/bin/openshell", runCommand); + + expect(readSandboxId("alpha")).toBeNull(); + expect(readSandboxId("alpha")).toBeNull(); + expect(runCommand).toHaveBeenCalledOnce(); + }); +}); diff --git a/src/lib/adapters/openshell/sandbox-identity.ts b/src/lib/adapters/openshell/sandbox-identity.ts index dbcc47696c4..b80724e3e7d 100644 --- a/src/lib/adapters/openshell/sandbox-identity.ts +++ b/src/lib/adapters/openshell/sandbox-identity.ts @@ -30,3 +30,33 @@ export function resolveOpenShellSandboxId( } return sandboxId; } + +/** + * Read sandbox IDs from the OpenShell CLI, memoized per process. + * + * Session detection needs the durable ID because newer OpenShell connects every + * sandbox through one fixed SSH alias and names the target only on its proxy + * command (#9316). Keeping the host-boundary call here leaves the state layer + * to parsing and classification. A sandbox whose ID cannot be read yields null, + * which leaves detection on SSH-host matching rather than failing the + * surrounding command. + */ +export function createOpenshellSandboxIdReader( + openshellBinary: string, + runCommand: (binary: string, args: string[]) => { status: number | null; stdout: string }, +): (sandboxName: string) => string | null { + const cache = new Map(); + return (sandboxName: string): string | null => { + const cached = cache.get(sandboxName); + if (cached !== undefined) return cached; + let resolved: string | null = null; + try { + const result = runCommand(openshellBinary, ["sandbox", "get", sandboxName]); + resolved = result.status === 0 ? parseOpenShellSandboxId(result.stdout || "") : null; + } catch { + resolved = null; + } + cache.set(sandboxName, resolved); + return resolved; + }; +} diff --git a/src/lib/list-command-deps.ts b/src/lib/list-command-deps.ts index b19cd7c0ffe..86f6ac8d147 100644 --- a/src/lib/list-command-deps.ts +++ b/src/lib/list-command-deps.ts @@ -50,6 +50,12 @@ export function buildListCommandDeps(): ListSandboxesCommandDeps { // Cache the SSH process probe once for all sandboxes — avoids spawning ps // per sandbox row. The getSshProcesses() call is the expensive part (5s timeout). let cachedSshOutput: string | null | undefined; + + // Resolving a sandbox ID costs one OpenShell call, so only pay it when the + // process list actually contains a proxied connection that needs one (#9316). + const resolveSandboxIdForSessions = (sshOutput: string, name: string): string | null => + sshOutput.includes("--sandbox-id") ? (sessionDeps?.resolveSandboxId?.(name) ?? null) : null; + const getCachedSshOutput = () => { if (cachedSshOutput === undefined && sessionDeps) { try { @@ -86,7 +92,8 @@ export function buildListCommandDeps(): ListSandboxesCommandDeps { try { const sshOutput = getCachedSshOutput(); if (sshOutput === null) return null; - return parseSshProcesses(sshOutput, name).length; + return parseSshProcesses(sshOutput, name, resolveSandboxIdForSessions(sshOutput, name)) + .length; } catch { return null; } diff --git a/src/lib/state/sandbox-session.test.ts b/src/lib/state/sandbox-session.test.ts index da211a9e9fc..93c76143f8d 100644 --- a/src/lib/state/sandbox-session.test.ts +++ b/src/lib/state/sandbox-session.test.ts @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { classifySessionState, type ForwardEntry, @@ -102,6 +102,48 @@ describe("parseSshProcesses", () => { }); }); + // Newer OpenShell routes every sandbox through one fixed `sandbox` alias and + // names the target only on its proxy command, so the SSH host carries no + // sandbox reference at all (#9316). + const PROXY = (id: string) => + `ssh -o ProxyCommand=/usr/local/bin/openshell ssh-proxy --gateway 'https://127.0.0.1:8080' --sandbox-id ${id} --token t --gateway-name nemoclaw -o StrictHostKeyChecking=no`; + const SANDBOX_ID = "de7eab7a-002f-41e9-acad-5fd4749e07bb"; + const interactiveLine = `12345 ${PROXY(SANDBOX_ID)} -tt -o RequestTTY=force -o SetEnv=TERM=xterm-256color sandbox`; + const forwardLine = `12300 ${PROXY(SANDBOX_ID)} -N -o ExitOnForwardFailure=yes -L 127.0.0.1:18789:127.0.0.1:18789 sandbox`; + + it("detects a proxied interactive session by sandbox ID (#9316)", () => { + expect(parseSshProcesses(interactiveLine, "my-sandbox", SANDBOX_ID)).toEqual([ + { + sandboxName: "my-sandbox", + pid: 12345, + sshHost: "openshell-my-sandbox.default", + }, + ]); + }); + + it("does not count the dashboard forward as a session (#9316)", () => { + // The forward runs through the same proxy and sandbox ID; only the + // interactive session requests a TTY. Counting it would report a session + // on every Ready sandbox. + expect(parseSshProcesses(forwardLine, "my-sandbox", SANDBOX_ID)).toEqual([]); + expect( + parseSshProcesses(`${forwardLine}\n${interactiveLine}`, "my-sandbox", SANDBOX_ID), + ).toHaveLength(1); + }); + + it("does not attribute a proxied session without a known sandbox ID (#9316)", () => { + // The command line carries no sandbox name, so guessing would attribute one + // sandbox's session to another. + expect(parseSshProcesses(interactiveLine, "my-sandbox")).toEqual([]); + expect(parseSshProcesses(interactiveLine, "my-sandbox", "")).toEqual([]); + }); + + it("does not match another sandbox's ID (#9316)", () => { + expect( + parseSshProcesses(interactiveLine, "other-sandbox", "aaaaaaaa-0000-0000-0000-000000000000"), + ).toEqual([]); + }); + it("detects a legacy SSH process during the upgrade window", () => { const output = `12345 ssh -F /tmp/config openshell-my-sandbox 67890 ssh -F /tmp/config openshell-other-sandbox`; @@ -287,6 +329,45 @@ describe("getActiveSandboxSessions", () => { expect(result.sessions[0].pid).toBe(12345); }); + it("resolves a durable ID for a proxied interactive session (#9316)", () => { + const sandboxId = "de7eab7a-002f-41e9-acad-5fd4749e07bb"; + const resolveSandboxId = vi.fn(() => sandboxId); + const deps: SessionDetectionDeps = { + getForwardList: () => "", + getSshProcesses: () => + `12345 ssh -o ProxyCommand=/usr/local/bin/openshell ssh-proxy --sandbox-id ${sandboxId} --token t -tt -o RequestTTY=force sandbox`, + resolveSandboxId, + }; + + const result = getActiveSandboxSessions("my-sandbox", deps); + + expect(resolveSandboxId).toHaveBeenCalledExactlyOnceWith("my-sandbox"); + expect(result).toEqual({ + detected: true, + sessions: [ + { + sandboxName: "my-sandbox", + pid: 12345, + sshHost: "openshell-my-sandbox.default", + }, + ], + }); + }); + + it("does not resolve a durable ID for a host-alias session (#9316)", () => { + const resolveSandboxId = vi.fn(() => "unused-id"); + const deps: SessionDetectionDeps = { + getForwardList: () => "", + getSshProcesses: () => "12345 ssh -F /tmp/cfg openshell-my-sandbox.default\n", + resolveSandboxId, + }; + + const result = getActiveSandboxSessions("my-sandbox", deps); + + expect(resolveSandboxId).not.toHaveBeenCalled(); + expect(result.sessions).toHaveLength(1); + }); + it("returns detected=false when pgrep unavailable (forward list alone insufficient)", () => { const deps: SessionDetectionDeps = { getForwardList: () => diff --git a/src/lib/state/sandbox-session.ts b/src/lib/state/sandbox-session.ts index 966af656bbd..6d5226de67d 100644 --- a/src/lib/state/sandbox-session.ts +++ b/src/lib/state/sandbox-session.ts @@ -14,6 +14,7 @@ */ import { spawnSync } from "node:child_process"; +import { createOpenshellSandboxIdReader } from "../adapters/openshell/sandbox-identity"; import { openshellSandboxSshHost } from "../adapters/openshell/sandbox-ssh-host"; // --------------------------------------------------------------------------- @@ -95,14 +96,31 @@ export function parseForwardList(output: string | null | undefined): ForwardEntr return entries; } +/** + * Does this command line belong to an interactive shell rather than a forward? + * + * OpenShell starts the dashboard port-forward through the same proxy and the + * same `sandbox` host alias as `connect`, so the sandbox reference alone cannot + * tell them apart. The interactive session is the one that asks for a TTY; the + * forward runs `-N` with no remote command. Counting the forward would report a + * session on every Ready sandbox. + */ +function isInteractiveSshCommand(command: string): boolean { + return /(?:^|\s)-tt(?:\s|$)/.test(command) || /RequestTTY=force/.test(command); +} + /** * Parse process list output to find SSH processes targeting a specific sandbox. * - * Current SSH connections use `openshell-.default`. During the - * supported v0.0.85 to v0.0.99 upgrade window, an already-running connection - * may still target the legacy `openshell-` alias. We recognize - * both as complete tokens to avoid false positives when one sandbox name is a - * prefix of another (e.g., `dev` vs `dev-staging`). + * Two shapes are recognized. OpenShell used to place the sandbox in the SSH + * host itself (`openshell-.default`, and the legacy + * `openshell-` from the v0.0.85 to v0.0.99 upgrade window); those + * are matched as complete tokens so one sandbox name cannot match another it is + * a prefix of (`dev` vs `dev-staging`). Newer OpenShell connects every sandbox + * through the fixed `sandbox` alias and identifies the target with + * `--sandbox-id ` on its proxy command instead, which left interactive + * sessions invisible to every session-reporting surface (#9316). When the + * caller knows the durable sandbox ID, that form is matched too. * * Input format: one line per process — ` ` * (compatible with both `pgrep -a` on Linux and `ps -axo pid,command`) @@ -110,6 +128,7 @@ export function parseForwardList(output: string | null | undefined): ForwardEntr export function parseSshProcesses( pgrepOutput: string | null | undefined, sandboxName: string, + sandboxId?: string | null, ): SandboxSession[] { if (!pgrepOutput || typeof pgrepOutput !== "string") return []; if (!sandboxName) return []; @@ -118,6 +137,10 @@ export function parseSshProcesses( const hostPatterns = sshHosts.map( (sshHost) => [sshHost, new RegExp(`(?:^|\\s)${escapeRegExp(sshHost)}(?:\\s|$)`)] as const, ); + const idPattern = + sandboxId && sandboxId.trim() + ? new RegExp(`--sandbox-id[=\\s]+${escapeRegExp(sandboxId.trim())}(?:\\s|$)`) + : null; const sessions: SandboxSession[] = []; const lines = pgrepOutput.split("\n").filter(Boolean); @@ -126,10 +149,17 @@ export function parseSshProcesses( if (!pidMatch) continue; const pid = Number.parseInt(pidMatch[1], 10); + const command = pidMatch[2]; - const sshHost = hostPatterns.find(([, pattern]) => pattern.test(pidMatch[2]))?.[0]; + const sshHost = hostPatterns.find(([, pattern]) => pattern.test(command))?.[0]; if (sshHost) { sessions.push({ sandboxName, pid, sshHost }); + continue; + } + // The proxied form carries no sandbox name, so it is only attributable + // when the caller resolved the sandbox's durable ID. + if (idPattern?.test(command) && isInteractiveSshCommand(command)) { + sessions.push({ sandboxName, pid, sshHost: openshellSandboxSshHost(sandboxName) }); } } @@ -216,6 +246,12 @@ export interface SessionDetectionDeps { getForwardList: () => string | null; /** Run `pgrep -a ssh` and return stdout. Null if unavailable. */ getSshProcesses: () => string | null; + /** + * Resolve the sandbox's durable OpenShell ID, or null when it cannot be + * determined. Only consulted when the process list contains a proxied + * connection, which is the only shape that needs it (#9316). + */ + resolveSandboxId?: (sandboxName: string) => string | null; } /** @@ -244,7 +280,12 @@ export function getActiveSandboxSessions( return { detected: false, sessions: [] }; } - const sshSessions = parseSshProcesses(pgrepOutput, sandboxName); + // Resolving the ID costs an OpenShell call, so only pay it for the proxied + // shape that cannot be attributed from the SSH host alone (#9316). + const sandboxId = pgrepOutput.includes("--sandbox-id") + ? (deps.resolveSandboxId?.(sandboxName) ?? null) + : null; + const sshSessions = parseSshProcesses(pgrepOutput, sandboxName, sandboxId); return { detected: true, @@ -298,5 +339,13 @@ export function createSystemDeps(openshellBinary: string): SessionDetectionDeps } }, getSshProcesses: querySshProcesses, + resolveSandboxId: createOpenshellSandboxIdReader(openshellBinary, (binary, args) => { + const result = spawnSync(binary, args, { + encoding: "utf-8", + stdio: ["ignore", "pipe", "pipe"], + timeout: 5000, + }); + return { status: result.status, stdout: result.stdout || "" }; + }), }; } diff --git a/src/lib/status-command-deps.ts b/src/lib/status-command-deps.ts index 46a53d37fa4..d47134288b2 100644 --- a/src/lib/status-command-deps.ts +++ b/src/lib/status-command-deps.ts @@ -248,6 +248,12 @@ export function buildStatusCommandDeps(rootDir: string): ShowStatusCommandDeps { // Cache the SSH process probe once per command invocation — avoids // spawning ps per sandbox row. #2604; mirrors buildListCommandDeps. let cachedSshOutput: string | null | undefined; + + // Resolving a sandbox ID costs one OpenShell call, so only pay it when the + // process list actually contains a proxied connection that needs one (#9316). + const resolveSandboxIdForSessions = (sshOutput: string, name: string): string | null => + sshOutput.includes("--sandbox-id") ? (sessionDeps?.resolveSandboxId?.(name) ?? null) : null; + const getCachedSshOutput = (): string | null => { if (cachedSshOutput === undefined && sessionDeps) { try { @@ -278,7 +284,8 @@ export function buildStatusCommandDeps(rootDir: string): ShowStatusCommandDeps { try { const sshOutput = getCachedSshOutput(); if (sshOutput === null) return null; - return parseSshProcesses(sshOutput, name).length; + return parseSshProcesses(sshOutput, name, resolveSandboxIdForSessions(sshOutput, name)) + .length; } catch { return null; }