From 010f4a659719367c975d05c63331e38a135476b3 Mon Sep 17 00:00:00 2001 From: Yanyun Liao Date: Mon, 17 Aug 2026 21:02:33 +0800 Subject: [PATCH 1/3] fix(cli): detect proxied connect sessions in session reporting Session detection identified a sandbox by its SSH host alias (`openshell-.default`). Newer OpenShell connects every sandbox through one fixed `sandbox` alias and names the target only with `--sandbox-id` on its proxy command, so an attached `connect` session matched nothing: `list` drew no active-session dot, `status` reported "SSH sessions: none", and `list --json` reported activeSessionCount 0. Match the proxied shape by the sandbox's durable OpenShell ID as well. The dashboard forward runs through the same proxy and the same ID, so only a command requesting a TTY counts as a session; otherwise every Ready sandbox would report one. The ID is resolved at most once per sandbox and only when the process list actually contains a proxied connection, and a failed lookup leaves detection on SSH-host matching. Fixes #9316 Signed-off-by: Yanyun Liao --- src/lib/list-command-deps.ts | 9 ++- src/lib/state/sandbox-session.test.ts | 42 ++++++++++++++ src/lib/state/sandbox-session.ts | 84 ++++++++++++++++++++++++--- src/lib/status-command-deps.ts | 9 ++- 4 files changed, 135 insertions(+), 9 deletions(-) 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..469896b351f 100644 --- a/src/lib/state/sandbox-session.test.ts +++ b/src/lib/state/sandbox-session.test.ts @@ -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`; diff --git a/src/lib/state/sandbox-session.ts b/src/lib/state/sandbox-session.ts index 966af656bbd..e3710e4ceab 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 { parseOpenShellSandboxId } 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,34 @@ export function createSystemDeps(openshellBinary: string): SessionDetectionDeps } }, getSshProcesses: querySshProcesses, + resolveSandboxId: createOpenshellSandboxIdResolver(openshellBinary), + }; +} + +/** + * Read a sandbox's durable ID via `openshell sandbox get`, memoized per process + * and failing soft. Detection stays on SSH-host matching when the lookup fails, + * so an unavailable OpenShell client never breaks the surrounding command. + */ +function createOpenshellSandboxIdResolver( + openshellBinary: 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 = spawnSync(openshellBinary, ["sandbox", "get", sandboxName], { + encoding: "utf-8", + stdio: ["ignore", "pipe", "pipe"], + timeout: 5000, + }); + resolved = result.status === 0 ? parseOpenShellSandboxId(result.stdout || "") : null; + } catch { + resolved = null; + } + cache.set(sandboxName, resolved); + return resolved; }; } 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; } From 5b191b9d836634fe11ea39ec75e8ab637c00e4c3 Mon Sep 17 00:00:00 2001 From: Yanyun Liao Date: Mon, 17 Aug 2026 21:42:36 +0800 Subject: [PATCH 2/3] refactor(cli): read sandbox IDs through the OpenShell adapter Move the `openshell sandbox get` call behind the OpenShell identity adapter and inject it into session detection, so the state module keeps to parsing and classification. Behavior is unchanged: the lookup is still memoized per sandbox, still made only when the process list contains a proxied connection, and still fails soft to SSH-host matching. Signed-off-by: Yanyun Liao --- .../adapters/openshell/sandbox-identity.ts | 30 ++++++++++++++++++ src/lib/state/sandbox-session.ts | 31 +++---------------- 2 files changed, 35 insertions(+), 26 deletions(-) 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/state/sandbox-session.ts b/src/lib/state/sandbox-session.ts index e3710e4ceab..6d5226de67d 100644 --- a/src/lib/state/sandbox-session.ts +++ b/src/lib/state/sandbox-session.ts @@ -14,7 +14,7 @@ */ import { spawnSync } from "node:child_process"; -import { parseOpenShellSandboxId } from "../adapters/openshell/sandbox-identity"; +import { createOpenshellSandboxIdReader } from "../adapters/openshell/sandbox-identity"; import { openshellSandboxSshHost } from "../adapters/openshell/sandbox-ssh-host"; // --------------------------------------------------------------------------- @@ -339,34 +339,13 @@ export function createSystemDeps(openshellBinary: string): SessionDetectionDeps } }, getSshProcesses: querySshProcesses, - resolveSandboxId: createOpenshellSandboxIdResolver(openshellBinary), - }; -} - -/** - * Read a sandbox's durable ID via `openshell sandbox get`, memoized per process - * and failing soft. Detection stays on SSH-host matching when the lookup fails, - * so an unavailable OpenShell client never breaks the surrounding command. - */ -function createOpenshellSandboxIdResolver( - openshellBinary: 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 = spawnSync(openshellBinary, ["sandbox", "get", sandboxName], { + resolveSandboxId: createOpenshellSandboxIdReader(openshellBinary, (binary, args) => { + const result = spawnSync(binary, args, { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"], timeout: 5000, }); - resolved = result.status === 0 ? parseOpenShellSandboxId(result.stdout || "") : null; - } catch { - resolved = null; - } - cache.set(sandboxName, resolved); - return resolved; + return { status: result.status, stdout: result.stdout || "" }; + }), }; } From b6e52059bbea32152591c0cd3d9f169400926449 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Mon, 17 Aug 2026 09:53:18 -0700 Subject: [PATCH 3/3] test(cli): cover proxied session resolver wiring Signed-off-by: Apurv Kumaria --- .../openshell/sandbox-identity.test.ts | 30 +++++++++++++- src/lib/state/sandbox-session.test.ts | 41 ++++++++++++++++++- 2 files changed, 68 insertions(+), 3 deletions(-) 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/state/sandbox-session.test.ts b/src/lib/state/sandbox-session.test.ts index 469896b351f..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, @@ -329,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: () =>