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
30 changes: 28 additions & 2 deletions src/lib/adapters/openshell/sandbox-identity.test.ts
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand All @@ -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();
});
});
30 changes: 30 additions & 0 deletions src/lib/adapters/openshell/sandbox-identity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string | null>();
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;
};
}
9 changes: 8 additions & 1 deletion src/lib/list-command-deps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand Down
83 changes: 82 additions & 1 deletion src/lib/state/sandbox-session.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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`;
Expand Down Expand Up @@ -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: () =>
Expand Down
63 changes: 56 additions & 7 deletions src/lib/state/sandbox-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -95,21 +96,39 @@ 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-<sandboxName>.default`. During the
* supported v0.0.85 to v0.0.99 upgrade window, an already-running connection
* may still target the legacy `openshell-<sandboxName>` 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-<sandboxName>.default`, and the legacy
* `openshell-<sandboxName>` 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 <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 — `<PID> <full command line>`
* (compatible with both `pgrep -a` on Linux and `ps -axo pid,command`)
*/
export function parseSshProcesses(
pgrepOutput: string | null | undefined,
sandboxName: string,
sandboxId?: string | null,
): SandboxSession[] {
if (!pgrepOutput || typeof pgrepOutput !== "string") return [];
if (!sandboxName) return [];
Expand All @@ -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);

Expand All @@ -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) });
}
}

Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 || "" };
}),
};
}
9 changes: 8 additions & 1 deletion src/lib/status-command-deps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand Down
Loading