Skip to content

Commit 010f4a6

Browse files
committed
fix(cli): detect proxied connect sessions in session reporting
Session detection identified a sandbox by its SSH host alias (`openshell-<name>.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 <yanyunl@nvidia.com>
1 parent 8cdc3c4 commit 010f4a6

4 files changed

Lines changed: 135 additions & 9 deletions

File tree

src/lib/list-command-deps.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ export function buildListCommandDeps(): ListSandboxesCommandDeps {
5050
// Cache the SSH process probe once for all sandboxes — avoids spawning ps
5151
// per sandbox row. The getSshProcesses() call is the expensive part (5s timeout).
5252
let cachedSshOutput: string | null | undefined;
53+
54+
// Resolving a sandbox ID costs one OpenShell call, so only pay it when the
55+
// process list actually contains a proxied connection that needs one (#9316).
56+
const resolveSandboxIdForSessions = (sshOutput: string, name: string): string | null =>
57+
sshOutput.includes("--sandbox-id") ? (sessionDeps?.resolveSandboxId?.(name) ?? null) : null;
58+
5359
const getCachedSshOutput = () => {
5460
if (cachedSshOutput === undefined && sessionDeps) {
5561
try {
@@ -86,7 +92,8 @@ export function buildListCommandDeps(): ListSandboxesCommandDeps {
8692
try {
8793
const sshOutput = getCachedSshOutput();
8894
if (sshOutput === null) return null;
89-
return parseSshProcesses(sshOutput, name).length;
95+
return parseSshProcesses(sshOutput, name, resolveSandboxIdForSessions(sshOutput, name))
96+
.length;
9097
} catch {
9198
return null;
9299
}

src/lib/state/sandbox-session.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,48 @@ describe("parseSshProcesses", () => {
102102
});
103103
});
104104

105+
// Newer OpenShell routes every sandbox through one fixed `sandbox` alias and
106+
// names the target only on its proxy command, so the SSH host carries no
107+
// sandbox reference at all (#9316).
108+
const PROXY = (id: string) =>
109+
`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`;
110+
const SANDBOX_ID = "de7eab7a-002f-41e9-acad-5fd4749e07bb";
111+
const interactiveLine = `12345 ${PROXY(SANDBOX_ID)} -tt -o RequestTTY=force -o SetEnv=TERM=xterm-256color sandbox`;
112+
const forwardLine = `12300 ${PROXY(SANDBOX_ID)} -N -o ExitOnForwardFailure=yes -L 127.0.0.1:18789:127.0.0.1:18789 sandbox`;
113+
114+
it("detects a proxied interactive session by sandbox ID (#9316)", () => {
115+
expect(parseSshProcesses(interactiveLine, "my-sandbox", SANDBOX_ID)).toEqual([
116+
{
117+
sandboxName: "my-sandbox",
118+
pid: 12345,
119+
sshHost: "openshell-my-sandbox.default",
120+
},
121+
]);
122+
});
123+
124+
it("does not count the dashboard forward as a session (#9316)", () => {
125+
// The forward runs through the same proxy and sandbox ID; only the
126+
// interactive session requests a TTY. Counting it would report a session
127+
// on every Ready sandbox.
128+
expect(parseSshProcesses(forwardLine, "my-sandbox", SANDBOX_ID)).toEqual([]);
129+
expect(
130+
parseSshProcesses(`${forwardLine}\n${interactiveLine}`, "my-sandbox", SANDBOX_ID),
131+
).toHaveLength(1);
132+
});
133+
134+
it("does not attribute a proxied session without a known sandbox ID (#9316)", () => {
135+
// The command line carries no sandbox name, so guessing would attribute one
136+
// sandbox's session to another.
137+
expect(parseSshProcesses(interactiveLine, "my-sandbox")).toEqual([]);
138+
expect(parseSshProcesses(interactiveLine, "my-sandbox", "")).toEqual([]);
139+
});
140+
141+
it("does not match another sandbox's ID (#9316)", () => {
142+
expect(
143+
parseSshProcesses(interactiveLine, "other-sandbox", "aaaaaaaa-0000-0000-0000-000000000000"),
144+
).toEqual([]);
145+
});
146+
105147
it("detects a legacy SSH process during the upgrade window", () => {
106148
const output = `12345 ssh -F /tmp/config openshell-my-sandbox
107149
67890 ssh -F /tmp/config openshell-other-sandbox`;

src/lib/state/sandbox-session.ts

Lines changed: 77 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
*/
1515

1616
import { spawnSync } from "node:child_process";
17+
import { parseOpenShellSandboxId } from "../adapters/openshell/sandbox-identity";
1718
import { openshellSandboxSshHost } from "../adapters/openshell/sandbox-ssh-host";
1819

1920
// ---------------------------------------------------------------------------
@@ -95,21 +96,39 @@ export function parseForwardList(output: string | null | undefined): ForwardEntr
9596
return entries;
9697
}
9798

99+
/**
100+
* Does this command line belong to an interactive shell rather than a forward?
101+
*
102+
* OpenShell starts the dashboard port-forward through the same proxy and the
103+
* same `sandbox` host alias as `connect`, so the sandbox reference alone cannot
104+
* tell them apart. The interactive session is the one that asks for a TTY; the
105+
* forward runs `-N` with no remote command. Counting the forward would report a
106+
* session on every Ready sandbox.
107+
*/
108+
function isInteractiveSshCommand(command: string): boolean {
109+
return /(?:^|\s)-tt(?:\s|$)/.test(command) || /RequestTTY=force/.test(command);
110+
}
111+
98112
/**
99113
* Parse process list output to find SSH processes targeting a specific sandbox.
100114
*
101-
* Current SSH connections use `openshell-<sandboxName>.default`. During the
102-
* supported v0.0.85 to v0.0.99 upgrade window, an already-running connection
103-
* may still target the legacy `openshell-<sandboxName>` alias. We recognize
104-
* both as complete tokens to avoid false positives when one sandbox name is a
105-
* prefix of another (e.g., `dev` vs `dev-staging`).
115+
* Two shapes are recognized. OpenShell used to place the sandbox in the SSH
116+
* host itself (`openshell-<sandboxName>.default`, and the legacy
117+
* `openshell-<sandboxName>` from the v0.0.85 to v0.0.99 upgrade window); those
118+
* are matched as complete tokens so one sandbox name cannot match another it is
119+
* a prefix of (`dev` vs `dev-staging`). Newer OpenShell connects every sandbox
120+
* through the fixed `sandbox` alias and identifies the target with
121+
* `--sandbox-id <id>` on its proxy command instead, which left interactive
122+
* sessions invisible to every session-reporting surface (#9316). When the
123+
* caller knows the durable sandbox ID, that form is matched too.
106124
*
107125
* Input format: one line per process — `<PID> <full command line>`
108126
* (compatible with both `pgrep -a` on Linux and `ps -axo pid,command`)
109127
*/
110128
export function parseSshProcesses(
111129
pgrepOutput: string | null | undefined,
112130
sandboxName: string,
131+
sandboxId?: string | null,
113132
): SandboxSession[] {
114133
if (!pgrepOutput || typeof pgrepOutput !== "string") return [];
115134
if (!sandboxName) return [];
@@ -118,6 +137,10 @@ export function parseSshProcesses(
118137
const hostPatterns = sshHosts.map(
119138
(sshHost) => [sshHost, new RegExp(`(?:^|\\s)${escapeRegExp(sshHost)}(?:\\s|$)`)] as const,
120139
);
140+
const idPattern =
141+
sandboxId && sandboxId.trim()
142+
? new RegExp(`--sandbox-id[=\\s]+${escapeRegExp(sandboxId.trim())}(?:\\s|$)`)
143+
: null;
121144
const sessions: SandboxSession[] = [];
122145
const lines = pgrepOutput.split("\n").filter(Boolean);
123146

@@ -126,10 +149,17 @@ export function parseSshProcesses(
126149
if (!pidMatch) continue;
127150

128151
const pid = Number.parseInt(pidMatch[1], 10);
152+
const command = pidMatch[2];
129153

130-
const sshHost = hostPatterns.find(([, pattern]) => pattern.test(pidMatch[2]))?.[0];
154+
const sshHost = hostPatterns.find(([, pattern]) => pattern.test(command))?.[0];
131155
if (sshHost) {
132156
sessions.push({ sandboxName, pid, sshHost });
157+
continue;
158+
}
159+
// The proxied form carries no sandbox name, so it is only attributable
160+
// when the caller resolved the sandbox's durable ID.
161+
if (idPattern?.test(command) && isInteractiveSshCommand(command)) {
162+
sessions.push({ sandboxName, pid, sshHost: openshellSandboxSshHost(sandboxName) });
133163
}
134164
}
135165

@@ -216,6 +246,12 @@ export interface SessionDetectionDeps {
216246
getForwardList: () => string | null;
217247
/** Run `pgrep -a ssh` and return stdout. Null if unavailable. */
218248
getSshProcesses: () => string | null;
249+
/**
250+
* Resolve the sandbox's durable OpenShell ID, or null when it cannot be
251+
* determined. Only consulted when the process list contains a proxied
252+
* connection, which is the only shape that needs it (#9316).
253+
*/
254+
resolveSandboxId?: (sandboxName: string) => string | null;
219255
}
220256

221257
/**
@@ -244,7 +280,12 @@ export function getActiveSandboxSessions(
244280
return { detected: false, sessions: [] };
245281
}
246282

247-
const sshSessions = parseSshProcesses(pgrepOutput, sandboxName);
283+
// Resolving the ID costs an OpenShell call, so only pay it for the proxied
284+
// shape that cannot be attributed from the SSH host alone (#9316).
285+
const sandboxId = pgrepOutput.includes("--sandbox-id")
286+
? (deps.resolveSandboxId?.(sandboxName) ?? null)
287+
: null;
288+
const sshSessions = parseSshProcesses(pgrepOutput, sandboxName, sandboxId);
248289

249290
return {
250291
detected: true,
@@ -298,5 +339,34 @@ export function createSystemDeps(openshellBinary: string): SessionDetectionDeps
298339
}
299340
},
300341
getSshProcesses: querySshProcesses,
342+
resolveSandboxId: createOpenshellSandboxIdResolver(openshellBinary),
343+
};
344+
}
345+
346+
/**
347+
* Read a sandbox's durable ID via `openshell sandbox get`, memoized per process
348+
* and failing soft. Detection stays on SSH-host matching when the lookup fails,
349+
* so an unavailable OpenShell client never breaks the surrounding command.
350+
*/
351+
function createOpenshellSandboxIdResolver(
352+
openshellBinary: string,
353+
): (sandboxName: string) => string | null {
354+
const cache = new Map<string, string | null>();
355+
return (sandboxName: string): string | null => {
356+
const cached = cache.get(sandboxName);
357+
if (cached !== undefined) return cached;
358+
let resolved: string | null = null;
359+
try {
360+
const result = spawnSync(openshellBinary, ["sandbox", "get", sandboxName], {
361+
encoding: "utf-8",
362+
stdio: ["ignore", "pipe", "pipe"],
363+
timeout: 5000,
364+
});
365+
resolved = result.status === 0 ? parseOpenShellSandboxId(result.stdout || "") : null;
366+
} catch {
367+
resolved = null;
368+
}
369+
cache.set(sandboxName, resolved);
370+
return resolved;
301371
};
302372
}

src/lib/status-command-deps.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,12 @@ export function buildStatusCommandDeps(rootDir: string): ShowStatusCommandDeps {
248248
// Cache the SSH process probe once per command invocation — avoids
249249
// spawning ps per sandbox row. #2604; mirrors buildListCommandDeps.
250250
let cachedSshOutput: string | null | undefined;
251+
252+
// Resolving a sandbox ID costs one OpenShell call, so only pay it when the
253+
// process list actually contains a proxied connection that needs one (#9316).
254+
const resolveSandboxIdForSessions = (sshOutput: string, name: string): string | null =>
255+
sshOutput.includes("--sandbox-id") ? (sessionDeps?.resolveSandboxId?.(name) ?? null) : null;
256+
251257
const getCachedSshOutput = (): string | null => {
252258
if (cachedSshOutput === undefined && sessionDeps) {
253259
try {
@@ -278,7 +284,8 @@ export function buildStatusCommandDeps(rootDir: string): ShowStatusCommandDeps {
278284
try {
279285
const sshOutput = getCachedSshOutput();
280286
if (sshOutput === null) return null;
281-
return parseSshProcesses(sshOutput, name).length;
287+
return parseSshProcesses(sshOutput, name, resolveSandboxIdForSessions(sshOutput, name))
288+
.length;
282289
} catch {
283290
return null;
284291
}

0 commit comments

Comments
 (0)