From 35a986c1f833e49b49a164ef9ea05e3243502500 Mon Sep 17 00:00:00 2001 From: San Dang Date: Mon, 7 Sep 2026 14:59:26 +0700 Subject: [PATCH 1/8] fix(openshell): wait for owned forward service Signed-off-by: San Dang --- .../openshell/forward-service.test.ts | 294 +++++++++- src/lib/adapters/openshell/forward-service.ts | 512 +++++++++++++++++- 2 files changed, 776 insertions(+), 30 deletions(-) diff --git a/src/lib/adapters/openshell/forward-service.test.ts b/src/lib/adapters/openshell/forward-service.test.ts index 2de4e61ca73..f84c7dab426 100644 --- a/src/lib/adapters/openshell/forward-service.test.ts +++ b/src/lib/adapters/openshell/forward-service.test.ts @@ -1,10 +1,13 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { createServer, type AddressInfo } from "node:net"; + import { describe, expect, it, vi } from "vitest"; import { buildForwardServiceArgs, + getForwardListenerOwnership, launchForwardService, type ForwardServiceTarget, } from "./forward-service"; @@ -19,6 +22,7 @@ const target: ForwardServiceTarget = { targetHost: "127.0.0.1", targetPort: 18_789, }; +const stableProcessIdentity = (pid: number): string => `start-${String(pid)}`; describe("OpenShell forward service", () => { it("builds the direct ForwardTcp command with explicit gateway authority", () => { @@ -45,13 +49,16 @@ describe("OpenShell forward service", () => { ); }); - it("detaches the OpenShell child and waits for its local port", () => { + it("detaches the OpenShell child and waits for its owned local listener (#11084)", () => { const unref = vi.fn(); - const spawnDetached = vi.fn(() => ({ unref })); - let probes = 0; + const spawnDetached = vi.fn(() => ({ pid: 41, unref })); + const isReachable = vi.fn(() => false); launchForwardService(target, { - isReachable: () => ++probes >= 3, + getProcessIdentity: stableProcessIdentity, + isListenerOwned: () => true, + isProcessRunning: () => true, + isReachable, sleep: () => {}, spawnDetached, timeoutMs: 1_000, @@ -63,6 +70,7 @@ describe("OpenShell forward service", () => { expect.any(Object), ); expect(unref).toHaveBeenCalledOnce(); + expect(isReachable).toHaveBeenCalledOnce(); }); it("refuses an occupied port without launching or adopting its listener", () => { @@ -74,14 +82,288 @@ describe("OpenShell forward service", () => { expect(spawnDetached).not.toHaveBeenCalled(); }); + it("does not accept a listener without proving the launched service owns it (#11084)", () => { + let running = true; + const stopProcess = vi.fn(() => { + running = false; + }); + + expect(() => + launchForwardService(target, { + getProcessIdentity: stableProcessIdentity, + isListenerOwned: () => false, + isProcessRunning: () => running, + isReachable: () => false, + sleep: () => {}, + spawnDetached: () => ({ pid: 42, unref: () => {} }), + stopProcess, + timeoutMs: 0, + }), + ).toThrow(/did not become ready|owned|identity|adopt/u); + expect(stopProcess).toHaveBeenCalledWith(42, "SIGTERM"); + }); + + it("accepts delayed ownership only after it remains stable (#11084)", () => { + let ownershipChecks = 0; + + launchForwardService(target, { + getProcessIdentity: stableProcessIdentity, + isListenerOwned: () => ++ownershipChecks >= 3, + isProcessRunning: () => true, + isReachable: () => false, + sleep: () => {}, + spawnDetached: () => ({ pid: 43, unref: vi.fn() }), + timeoutMs: 10_000, + }); + + expect(ownershipChecks).toBe(23); + }); + + it("does not signal a process whose launch identity changed (#11084)", () => { + let identityChecks = 0; + const stopProcess = vi.fn(); + + expect(() => + launchForwardService(target, { + getProcessIdentity: () => (++identityChecks === 1 ? "original" : "replacement"), + isListenerOwned: () => false, + isProcessRunning: () => true, + isReachable: () => false, + sleep: () => {}, + spawnDetached: () => ({ pid: 44, unref: vi.fn() }), + stopProcess, + }), + ).toThrow(/changed identity.*refusing to signal or retry/u); + expect(stopProcess).not.toHaveBeenCalled(); + }); + + it("does not retry when an owned unready process cannot be stopped (#11084)", () => { + const spawnDetached = vi.fn(() => ({ pid: 45, unref: vi.fn() })); + const stopProcess = vi.fn(); + + expect(() => + launchForwardService(target, { + getProcessIdentity: stableProcessIdentity, + isListenerOwned: () => false, + isProcessRunning: () => true, + isReachable: () => false, + sleep: () => {}, + spawnDetached, + stopProcess, + stopTimeoutMs: 0, + timeoutMs: 0, + }), + ).toThrow(/could not be stopped.*refusing to retry/u); + expect(stopProcess.mock.calls).toEqual([ + [45, "SIGTERM"], + [45, "SIGKILL"], + ]); + expect(spawnDetached).toHaveBeenCalledOnce(); + }); + + it("fails closed when OpenShell returns no process identity (#11084)", () => { + const spawnDetached = vi.fn(() => ({ unref: vi.fn() })); + + expect(() => + launchForwardService(target, { + isReachable: () => false, + spawnDetached, + }), + ).toThrow(/no process identity.*refusing to start a duplicate service/u); + expect(spawnDetached).toHaveBeenCalledOnce(); + }); + + it("retries an exited service only for the exact sandbox creating handoff (#11084)", () => { + const diagnostic = + "Error:\n × sandbox 'demo' is no longer ready (phase: creating); stopping service\n ╰─▶ forward"; + const spawnDetached = vi + .fn() + .mockReturnValueOnce({ + pid: 51, + readOutput: () => diagnostic, + removeOutput: vi.fn(), + unref: vi.fn(), + }) + .mockReturnValueOnce({ pid: 52, removeOutput: vi.fn(), unref: vi.fn() }); + const onSandboxCreatingRetry = vi.fn(); + const sleep = vi.fn(); + + launchForwardService(target, { + getProcessIdentity: stableProcessIdentity, + isListenerOwned: (pid) => pid === 52, + isProcessRunning: (pid) => pid === 52, + isReachable: () => false, + maxSandboxCreatingRetries: 1, + onSandboxCreatingRetry, + sleep, + spawnDetached, + timeoutMs: 10_000, + }); + + expect(spawnDetached).toHaveBeenCalledTimes(2); + expect(onSandboxCreatingRetry).toHaveBeenCalledWith({ + attempt: 1, + delayMs: 2_000, + processId: 51, + remainingMs: expect.any(Number), + }); + expect(sleep).toHaveBeenCalledWith(2_000); + }); + + it.each([ + [ + "terminal phase", + "sandbox 'demo' is no longer ready (phase: error); stopping service forward", + ], + [ + "different sandbox", + "sandbox 'another' is no longer ready (phase: creating); stopping service forward", + ], + ["missing sandbox", "sandbox 'demo' no longer exists; stopping service forward"], + ])("does not retry a terminal or unrelated start result [%s] (#11084)", (_case, diagnostic) => { + const spawnDetached = vi.fn(() => ({ + pid: 61, + readOutput: () => diagnostic, + removeOutput: vi.fn(), + unref: vi.fn(), + })); + + expect(() => + launchForwardService(target, { + getProcessIdentity: stableProcessIdentity, + isProcessRunning: () => false, + isReachable: () => false, + sleep: () => {}, + spawnDetached, + }), + ).toThrow(/non-readiness-diagnostic/u); + expect(spawnDetached).toHaveBeenCalledOnce(); + }); + + it("refuses an unknown listener that appears before a safe retry (#11084)", () => { + let probes = 0; + const spawnDetached = vi.fn(() => ({ + pid: 71, + readOutput: () => + "sandbox 'demo' is no longer ready (phase: creating); stopping service forward", + removeOutput: vi.fn(), + unref: vi.fn(), + })); + + expect(() => + launchForwardService(target, { + getProcessIdentity: stableProcessIdentity, + isProcessRunning: () => false, + isReachable: () => ++probes >= 2, + maxSandboxCreatingRetries: 1, + sleep: () => {}, + spawnDetached, + }), + ).toThrow(/became occupied.*refusing to adopt/u); + expect(spawnDetached).toHaveBeenCalledOnce(); + }); + + it("bounds repeated sandbox creating handoffs and records every attempt (#11084)", () => { + const spawnDetached = vi.fn(() => ({ + pid: 72, + readOutput: () => + "sandbox 'demo' is no longer ready (phase: creating); stopping service forward", + removeOutput: vi.fn(), + unref: vi.fn(), + })); + + expect(() => + launchForwardService(target, { + getProcessIdentity: stableProcessIdentity, + isProcessRunning: () => false, + isReachable: () => false, + maxSandboxCreatingRetries: 2, + onSandboxCreatingRetry: () => {}, + sleep: () => {}, + spawnDetached, + timeoutMs: 10_000, + }), + ).toThrow( + /attempts: 1=pid-72:sandbox-creating, 2=pid-72:sandbox-creating, 3=pid-72:sandbox-creating/u, + ); + expect(spawnDetached).toHaveBeenCalledTimes(3); + }); + it("fails when the detached service does not bind before the deadline", () => { + let running = true; expect(() => launchForwardService(target, { + getProcessIdentity: stableProcessIdentity, + isListenerOwned: () => false, + isProcessRunning: () => running, isReachable: () => false, sleep: () => {}, - spawnDetached: () => ({ unref: () => {} }), + spawnDetached: () => ({ pid: 81, unref: () => {} }), + stopProcess: () => { + running = false; + }, timeoutMs: 0, }), - ).toThrow(/did not bind/u); + ).toThrow(/did not become ready/u); + }); + + it("classifies captured start output without exposing its contents (#11084)", () => { + let error: unknown; + const removeOutput = vi.fn(); + try { + launchForwardService(target, { + getProcessIdentity: stableProcessIdentity, + isProcessRunning: () => false, + isReachable: () => false, + sleep: () => {}, + spawnDetached: () => ({ + pid: 82, + readOutput: () => "terminal failure API_KEY=secret-value", + removeOutput, + unref: vi.fn(), + }), + }); + } catch (caught) { + error = caught; + } + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toContain("non-readiness-diagnostic"); + expect((error as Error).message).not.toContain("secret-value"); + expect(removeOutput).toHaveBeenCalledOnce(); }); + + it.runIf(process.platform === "linux")( + "proves listener ownership from Linux procfs without connecting (#11084)", + async () => { + const server = createServer(); + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", resolve); + }); + try { + const address = server.address() as AddressInfo; + expect(getForwardListenerOwnership(process.pid, address.port)).toBe(true); + } finally { + await new Promise((resolve) => server.close(() => resolve())); + } + }, + ); + + it.runIf(process.platform === "darwin")( + "proves listener ownership from macOS lsof without connecting (#11084)", + async () => { + const server = createServer(); + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", resolve); + }); + try { + const address = server.address() as AddressInfo; + expect(getForwardListenerOwnership(process.pid, address.port)).toBe(true); + } finally { + await new Promise((resolve) => server.close(() => resolve())); + } + }, + ); }); diff --git a/src/lib/adapters/openshell/forward-service.ts b/src/lib/adapters/openshell/forward-service.ts index 8f78fe81983..83abfed9b51 100644 --- a/src/lib/adapters/openshell/forward-service.ts +++ b/src/lib/adapters/openshell/forward-service.ts @@ -1,17 +1,35 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { spawn } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; import path from "node:path"; +import { stripVTControlCharacters } from "node:util"; import { isValidName } from "../../name-validation"; import { buildOpenShellSubprocessEnv } from "./resolve-shared"; import { probeLocalForwardListener } from "./local-forward-listener"; const START_TIMEOUT_MS = 30_000; +const SANDBOX_CREATING_RETRY_INTERVAL_MS = 2_000; +const SANDBOX_CREATING_MAX_RETRIES = START_TIMEOUT_MS / SANDBOX_CREATING_RETRY_INTERVAL_MS; +const STOP_TIMEOUT_MS = 5_000; const POLL_INTERVAL_MS = 100; +// OpenShell 0.0.106 rechecks sandbox readiness every two seconds after it +// binds. Retain exact listener ownership through the next complete check. +const STABLE_LISTENER_OBSERVATIONS = SANDBOX_CREATING_RETRY_INTERVAL_MS / POLL_INTERVAL_MS + 1; +const FORWARD_INSTANCE_ENV = "NEMOCLAW_FORWARD_INSTANCE_ID"; const sleepBuffer = new Int32Array(new SharedArrayBuffer(4)); +type ForwardServiceChild = { + readonly pid?: number; + readonly readOutput?: () => string; + readonly removeOutput?: () => void; + unref(): void; +}; + export interface ForwardServiceTarget { readonly executable: string; readonly gatewayName: string; @@ -24,19 +42,318 @@ export interface ForwardServiceTarget { } export interface ForwardServiceLaunchOptions { + readonly getProcessIdentity?: (pid: number) => string | null | undefined; + readonly isListenerOwned?: (pid: number, port: number) => boolean | null; + readonly isProcessRunning?: (pid: number) => boolean; readonly isReachable?: (port: number) => boolean; + readonly maxSandboxCreatingRetries?: number; + readonly onSandboxCreatingRetry?: (evidence: { + readonly attempt: number; + readonly delayMs: number; + readonly processId: number; + readonly remainingMs: number; + }) => void; readonly sleep?: (milliseconds: number) => void; readonly sourceEnvironment?: NodeJS.ProcessEnv; readonly spawnDetached?: ( executable: string, args: readonly string[], environment: NodeJS.ProcessEnv, - ) => { - unref(): void; - }; + ) => ForwardServiceChild; + readonly stopProcess?: (pid: number, signal: NodeJS.Signals) => void; + readonly stopTimeoutMs?: number; readonly timeoutMs?: number; } +function readLinuxProcessStat(pid: number): string[] | null | undefined { + if (process.platform !== "linux") return undefined; + try { + const stat = fs.readFileSync(`/proc/${String(pid)}/stat`, "utf8"); + return stat.slice(stat.lastIndexOf(")") + 2).split(" "); + } catch (error) { + const code = + typeof error === "object" && error !== null && "code" in error ? error.code : undefined; + return code === "ENOENT" || code === "ESRCH" ? null : undefined; + } +} + +function parseForwardInstanceIdentity(output: string): string | undefined { + const match = new RegExp( + `(?:^|\\s)${FORWARD_INSTANCE_ENV}=([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})(?:\\s|$)`, + "iu", + ).exec(output); + return match?.[1] ? `${process.platform}:${match[1].toLowerCase()}` : undefined; +} + +function getProcessIdentity(pid: number): string | null | undefined { + const stat = readLinuxProcessStat(pid); + const startTime = stat?.[19]; + if (startTime && /^\d+$/u.test(startTime)) return `linux:${startTime}`; + if (stat === null) return null; + if (process.platform === "linux") return undefined; + const result = spawnSync("ps", ["eww", "-p", String(pid), "-o", "command="], { + encoding: "utf8", + env: buildOpenShellSubprocessEnv(process.env), + stdio: ["ignore", "pipe", "ignore"], + timeout: 1_000, + }); + if (result.error) return undefined; + const identity = result.status === 0 ? parseForwardInstanceIdentity(result.stdout) : undefined; + if (identity) return identity; + return result.status === 1 ? null : undefined; +} + +function readLinuxListeningSocketInodes(port: number): Set | null { + const expectedPort = port.toString(16).toUpperCase().padStart(4, "0"); + const inodes = new Set(); + let readTable = false; + for (const tablePath of ["/proc/net/tcp", "/proc/net/tcp6"]) { + let table: string; + try { + table = fs.readFileSync(tablePath, "utf8"); + readTable = true; + } catch (error) { + const code = + typeof error === "object" && error !== null && "code" in error ? error.code : undefined; + if (code === "ENOENT") continue; + return null; + } + for (const line of table.split(/\r?\n/u).slice(1)) { + const fields = line.trim().split(/\s+/u); + const localAddress = fields[1]; + const state = fields[3]; + const inode = fields[9]; + if ( + localAddress?.endsWith(`:${expectedPort}`) && + state === "0A" && + inode !== undefined && + /^\d+$/u.test(inode) + ) { + inodes.add(inode); + } + } + } + return readTable ? inodes : null; +} + +function linuxProcessOwnsListener( + pid: number, + listenerInodes: ReadonlySet, +): boolean | null { + let descriptors: string[]; + try { + descriptors = fs.readdirSync(`/proc/${String(pid)}/fd`); + } catch (error) { + const code = + typeof error === "object" && error !== null && "code" in error ? error.code : undefined; + return code === "ENOENT" || code === "ESRCH" ? false : null; + } + let unreadableDescriptor = false; + for (const descriptor of descriptors) { + try { + const target = fs.readlinkSync(`/proc/${String(pid)}/fd/${descriptor}`); + const match = /^socket:\[(\d+)\]$/u.exec(target); + if (match?.[1] && listenerInodes.has(match[1])) return true; + } catch (error) { + const code = + typeof error === "object" && error !== null && "code" in error ? error.code : undefined; + if (code !== "ENOENT" && code !== "ESRCH") unreadableDescriptor = true; + } + } + return unreadableDescriptor ? null : false; +} + +/** Prove that a listener belongs to the exact child requested by this launch. */ +export function getForwardListenerOwnership(pid: number, port: number): boolean | null { + if (process.platform === "linux") { + const listenerInodes = readLinuxListeningSocketInodes(port); + return listenerInodes ? linuxProcessOwnsListener(pid, listenerInodes) : null; + } + const result = spawnSync( + "lsof", + ["-nP", "-a", "-p", String(pid), `-iTCP:${String(port)}`, "-sTCP:LISTEN", "-t"], + { + encoding: "utf8", + env: buildOpenShellSubprocessEnv(process.env), + stdio: ["ignore", "pipe", "ignore"], + timeout: 1_000, + }, + ); + if (result.error) return null; + const listenerPids = result.stdout + .split(/\r?\n/u) + .map((value) => value.trim()) + .filter(Boolean); + if (result.status === 0) return listenerPids.includes(String(pid)); + return result.status === 1 && listenerPids.length === 0 ? false : null; +} + +function isProcessRunning(pid: number): boolean { + try { + if (process.platform === "linux") { + const stat = readLinuxProcessStat(pid); + if (stat === null || stat?.[0] === "Z") return false; + } + process.kill(pid, 0); + return true; + } catch (error) { + const code = + typeof error === "object" && error !== null && "code" in error ? error.code : undefined; + return code !== "ENOENT" && code !== "ESRCH"; + } +} + +function spawnForwardService( + executable: string, + args: readonly string[], + environment: NodeJS.ProcessEnv, +): ForwardServiceChild { + const outputDirectory = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-forward-service-")); + fs.chmodSync(outputDirectory, 0o700); + const outputPath = path.join(outputDirectory, "start.log"); + const outputDescriptor = fs.openSync(outputPath, "wx", 0o600); + let child: ReturnType; + try { + child = spawn(executable, [...args], { + detached: true, + env: environment, + stdio: ["ignore", outputDescriptor, outputDescriptor], + }); + } catch (error) { + try { + fs.closeSync(outputDescriptor); + } catch { + // Preserve the spawn failure when closing the diagnostic file also fails. + } + try { + fs.rmSync(outputDirectory, { force: true, recursive: true }); + } catch { + // Preserve the spawn failure when removing the diagnostic file also fails. + } + throw error; + } + try { + fs.closeSync(outputDescriptor); + } catch { + // The child owns its inherited descriptor; the parent no longer needs it. + } + return { + pid: child.pid, + unref: () => child.unref(), + readOutput: () => { + try { + return fs.readFileSync(outputPath, "utf8"); + } catch { + return ""; + } + }, + removeOutput: () => { + try { + fs.rmSync(outputDirectory, { force: true, recursive: true }); + } catch { + // The detached child may retain its inherited descriptor briefly. + } + }, + }; +} + +function compactOpenShellDiagnostic(output: string): string { + return stripVTControlCharacters(output) + .replace(/[^\p{L}\p{N}\s'"():;._-]+/gu, " ") + .replace(/\s+/gu, " ") + .trim(); +} + +function isSandboxCreatingHandoff(output: string, sandboxName: string): boolean { + const escapedName = sandboxName.replace(/[.*+?^${}()|[\]\\]/gu, "\\$&"); + return new RegExp( + `sandbox ["']${escapedName}["'] is no longer ready \\(phase: creating\\); stopping service forward`, + "iu", + ).test(compactOpenShellDiagnostic(output)); +} + +function classifyStartOutput( + child: ForwardServiceChild, + sandboxName: string, +): { readonly category: string; readonly sandboxCreating: boolean } { + const output = child.readOutput?.() ?? ""; + const sandboxCreating = isSandboxCreatingHandoff(output, sandboxName); + return { + category: sandboxCreating + ? "sandbox-creating" + : output.trim() + ? "non-readiness-diagnostic" + : "empty-diagnostic", + sandboxCreating, + }; +} + +function isProcessId(pid: number | undefined): pid is number { + return Number.isSafeInteger(pid) && Number(pid) > 0; +} + +function processIdentityStatus( + pid: number, + expectedIdentity: string | null | undefined, + readIdentity: (pid: number) => string | null | undefined, +): "owned" | "exited" | "unverified" { + if (!expectedIdentity) return "unverified"; + const observedIdentity = readIdentity(pid); + if (observedIdentity === null) return "exited"; + return observedIdentity === expectedIdentity ? "owned" : "unverified"; +} + +function isMissingProcessError(error: unknown): boolean { + const code = + typeof error === "object" && error !== null && "code" in error ? error.code : undefined; + return code === "ENOENT" || code === "ESRCH"; +} + +function stopOwnedProcess(input: { + readonly expectedIdentity: string | null | undefined; + readonly isRunning: (pid: number) => boolean; + readonly pid: number; + readonly readIdentity: (pid: number) => string | null | undefined; + readonly sleep: (milliseconds: number) => void; + readonly stop: (pid: number, signal: NodeJS.Signals) => void; + readonly timeoutMs: number; +}): "stopped" | "running" | "unverified" { + if (!input.expectedIdentity) return "unverified"; + const waitForExit = (): "stopped" | "running" | "unverified" => { + const deadline = Date.now() + input.timeoutMs; + while (true) { + const identity = processIdentityStatus(input.pid, input.expectedIdentity, input.readIdentity); + if (identity === "exited" || !input.isRunning(input.pid)) return "stopped"; + if (identity === "unverified") return "unverified"; + if (Date.now() >= deadline) return "running"; + input.sleep(POLL_INTERVAL_MS); + } + }; + const initialIdentity = processIdentityStatus( + input.pid, + input.expectedIdentity, + input.readIdentity, + ); + if (initialIdentity === "exited") return "stopped"; + if (initialIdentity === "unverified") return "unverified"; + try { + input.stop(input.pid, "SIGTERM"); + } catch (error) { + return isMissingProcessError(error) ? "stopped" : "running"; + } + const terminated = waitForExit(); + if (terminated !== "running") return terminated; + if (processIdentityStatus(input.pid, input.expectedIdentity, input.readIdentity) !== "owned") { + return "unverified"; + } + try { + input.stop(input.pid, "SIGKILL"); + } catch (error) { + return isMissingProcessError(error) ? "stopped" : "running"; + } + return waitForExit(); +} + function isPort(value: unknown): value is number { return Number.isSafeInteger(value) && Number(value) >= 1 && Number(value) <= 65_535; } @@ -94,35 +411,182 @@ export function buildForwardServiceArgs(target: ForwardServiceTarget): string[] ]; } +type ForwardAttemptResult = { + readonly category: string; + readonly processId: number; + readonly sandboxCreating: boolean; +}; + +function startForwardServiceAttempt(input: { + readonly args: readonly string[]; + readonly environment: NodeJS.ProcessEnv; + readonly options: ForwardServiceLaunchOptions; + readonly readyDeadline: number; + readonly target: ForwardServiceTarget; +}): ForwardAttemptResult | null { + const readIdentity = input.options.getProcessIdentity ?? getProcessIdentity; + const listenerOwned = input.options.isListenerOwned ?? getForwardListenerOwnership; + const running = input.options.isProcessRunning ?? isProcessRunning; + const sleep = + input.options.sleep ?? + ((milliseconds: number) => Atomics.wait(sleepBuffer, 0, 0, milliseconds)); + const spawnDetached = input.options.spawnDetached ?? spawnForwardService; + const stop = + input.options.stopProcess ?? + ((pid: number, signal: NodeJS.Signals) => process.kill(pid, signal)); + const instanceId = randomUUID(); + const child = spawnDetached(input.target.executable, input.args, { + ...input.environment, + [FORWARD_INSTANCE_ENV]: instanceId, + }); + if (!isProcessId(child.pid)) { + const start = classifyStartOutput(child, input.target.sandboxName); + child.removeOutput?.(); + throw new Error( + `OpenShell forward service returned no process identity for ${input.target.localHost}:${String(input.target.localPort)}; refusing to start a duplicate service; forward start: ${start.category}`, + ); + } + const pid = child.pid; + const expectedIdentity = input.options.getProcessIdentity + ? readIdentity(pid) + : process.platform === "linux" + ? readIdentity(pid) + : `${process.platform}:${instanceId}`; + child.unref(); + let stableObservations = 0; + let stabilityDeadline: number | undefined; + + while (true) { + const identity = processIdentityStatus(pid, expectedIdentity, readIdentity); + if (identity === "exited" || !running(pid)) { + const start = classifyStartOutput(child, input.target.sandboxName); + child.removeOutput?.(); + return { ...start, processId: pid }; + } + if (identity === "unverified") { + const start = classifyStartOutput(child, input.target.sandboxName); + child.removeOutput?.(); + throw new Error( + `OpenShell forward service process ${String(pid)} changed identity before binding ${input.target.localHost}:${String(input.target.localPort)}; refusing to signal or retry; forward start: ${start.category}`, + ); + } + if (listenerOwned(pid, input.target.localPort) === true) { + stabilityDeadline ??= Date.now() + SANDBOX_CREATING_RETRY_INTERVAL_MS + POLL_INTERVAL_MS; + stableObservations += 1; + if (stableObservations >= STABLE_LISTENER_OBSERVATIONS) { + child.removeOutput?.(); + return null; + } + } else { + stableObservations = 0; + } + const now = Date.now(); + if (stabilityDeadline !== undefined ? now >= stabilityDeadline : now >= input.readyDeadline) { + break; + } + sleep(POLL_INTERVAL_MS); + } + + const stopped = stopOwnedProcess({ + expectedIdentity, + isRunning: running, + pid, + readIdentity, + sleep, + stop, + timeoutMs: input.options.stopTimeoutMs ?? STOP_TIMEOUT_MS, + }); + const start = classifyStartOutput(child, input.target.sandboxName); + child.removeOutput?.(); + if (stopped !== "stopped") { + throw new Error( + `OpenShell forward service process ${String(pid)} did not become ready and ${stopped === "unverified" ? "could not be verified as owned" : "could not be stopped"}; refusing to retry; forward start: ${start.category}`, + ); + } + const reachable = input.options.isReachable ?? probeLocalForwardListener; + if (reachable(input.target.localPort)) { + throw new Error( + `Host port ${String(input.target.localPort)} remained reachable after the launched process stopped; refusing to adopt its listener or retry; forward start: ${start.category}`, + ); + } + throw new Error( + `OpenShell forward service did not become ready at ${input.target.localHost}:${String(input.target.localPort)}; forward start: ${start.category}`, + ); +} + /** Launch one foreground OpenShell service forward as a detached host child. */ export function launchForwardService( target: ForwardServiceTarget, options: ForwardServiceLaunchOptions = {}, ): void { validateForwardServiceTarget(target); - const isReachable = options.isReachable ?? probeLocalForwardListener; - if (isReachable(target.localPort)) { + const reachable = options.isReachable ?? probeLocalForwardListener; + if (reachable(target.localPort)) { throw new Error(`Host port ${String(target.localPort)} is already occupied`); } - const spawnDetached = - options.spawnDetached ?? - ((executable, args, environment) => - spawn(executable, [...args], { detached: true, env: environment, stdio: "ignore" })); - const child = spawnDetached( - target.executable, - buildForwardServiceArgs(target), - buildOpenShellSubprocessEnv(options.sourceEnvironment ?? process.env), - ); - child.unref(); - const sleep = options.sleep ?? ((milliseconds: number) => Atomics.wait(sleepBuffer, 0, 0, milliseconds)); - const deadline = Date.now() + (options.timeoutMs ?? START_TIMEOUT_MS); - while (Date.now() < deadline) { - if (isReachable(target.localPort)) return; - sleep(POLL_INTERVAL_MS); + const readyDeadline = Date.now() + (options.timeoutMs ?? START_TIMEOUT_MS); + const maxRetries = options.maxSandboxCreatingRetries ?? SANDBOX_CREATING_MAX_RETRIES; + if ( + !Number.isSafeInteger(maxRetries) || + maxRetries < 0 || + maxRetries > SANDBOX_CREATING_MAX_RETRIES + ) { + throw new Error( + `OpenShell sandbox creating retries must be between 0 and ${String(SANDBOX_CREATING_MAX_RETRIES)}`, + ); + } + const args = buildForwardServiceArgs(target); + const environment = buildOpenShellSubprocessEnv(options.sourceEnvironment ?? process.env); + const evidence: string[] = []; + let retries = 0; + + while (true) { + if (retries > 0 && Date.now() >= readyDeadline) { + throw new Error( + `OpenShell forward service readiness budget expired before retry; attempts: ${evidence.join(", ")}`, + ); + } + if (retries > 0 && reachable(target.localPort)) { + throw new Error( + `Host port ${String(target.localPort)} became occupied before forward retry; refusing to adopt its listener; attempts: ${evidence.join(", ")}`, + ); + } + const result = startForwardServiceAttempt({ + args, + environment, + options, + readyDeadline, + target, + }); + if (result === null) return; + const attempt = retries + 1; + evidence.push(`${String(attempt)}=pid-${String(result.processId)}:${result.category}`); + const remainingMs = Math.max(0, readyDeadline - Date.now()); + if ( + !result.sandboxCreating || + retries >= maxRetries || + remainingMs < SANDBOX_CREATING_RETRY_INTERVAL_MS + ) { + throw new Error( + `OpenShell forward service exited before binding ${target.localHost}:${String(target.localPort)}; attempts: ${evidence.join(", ")}`, + ); + } + retries += 1; + const retryEvidence = { + attempt, + delayMs: SANDBOX_CREATING_RETRY_INTERVAL_MS, + processId: result.processId, + remainingMs, + }; + if (options.onSandboxCreatingRetry) { + options.onSandboxCreatingRetry(retryEvidence); + } else { + console.warn( + `OpenShell ForwardTcp ${String(target.localPort)} start attempt ${String(attempt)} (pid ${String(result.processId)}) observed sandbox '${target.sandboxName}' in phase creating; retrying in ${String(SANDBOX_CREATING_RETRY_INTERVAL_MS)}ms with ${String(remainingMs)}ms remaining.`, + ); + } + sleep(SANDBOX_CREATING_RETRY_INTERVAL_MS); } - throw new Error( - `OpenShell forward service did not bind ${target.localHost}:${String(target.localPort)}`, - ); } From 9df7810200f607feff2272ab38e73525e55f020f Mon Sep 17 00:00:00 2001 From: San Dang Date: Mon, 7 Sep 2026 15:34:44 +0700 Subject: [PATCH 2/8] fix(openshell): preserve forward config authority Signed-off-by: San Dang --- .../openshell/forward-service.test.ts | 32 +++++++++++++++++++ src/lib/adapters/openshell/resolve-shared.ts | 2 +- .../adapters/openshell/sanitized-capture.ts | 2 +- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/lib/adapters/openshell/forward-service.test.ts b/src/lib/adapters/openshell/forward-service.test.ts index f84c7dab426..be5ad461ef0 100644 --- a/src/lib/adapters/openshell/forward-service.test.ts +++ b/src/lib/adapters/openshell/forward-service.test.ts @@ -73,6 +73,38 @@ describe("OpenShell forward service", () => { expect(isReachable).toHaveBeenCalledOnce(); }); + it("uses the selected OpenShell configuration without exposing credentials (#11084)", () => { + let launchedEnvironment: NodeJS.ProcessEnv | undefined; + const spawnDetached = vi.fn( + (_executable: string, _args: readonly string[], environment: NodeJS.ProcessEnv) => { + launchedEnvironment = environment; + return { pid: 42, unref: vi.fn() }; + }, + ); + + launchForwardService(target, { + getProcessIdentity: stableProcessIdentity, + isListenerOwned: () => true, + isProcessRunning: () => true, + isReachable: () => false, + sleep: () => {}, + sourceEnvironment: { + HOME: "/tmp/isolated-home", + NVIDIA_INFERENCE_API_KEY: "secret-value", + PATH: "/usr/bin", + XDG_CONFIG_HOME: "/tmp/selected-openshell-config", + }, + spawnDetached, + }); + + expect(launchedEnvironment).toMatchObject({ + HOME: "/tmp/isolated-home", + PATH: "/usr/bin", + XDG_CONFIG_HOME: "/tmp/selected-openshell-config", + }); + expect(launchedEnvironment).not.toHaveProperty("NVIDIA_INFERENCE_API_KEY"); + }); + it("refuses an occupied port without launching or adopting its listener", () => { const spawnDetached = vi.fn(); diff --git a/src/lib/adapters/openshell/resolve-shared.ts b/src/lib/adapters/openshell/resolve-shared.ts index 317d63c0148..1f3df6bb8f2 100644 --- a/src/lib/adapters/openshell/resolve-shared.ts +++ b/src/lib/adapters/openshell/resolve-shared.ts @@ -6,7 +6,6 @@ import { spawnSync, type SpawnSyncReturns } from "node:child_process"; import { HERMES_LIFECYCLE_DEFINITION } from "../../domain/lifecycle/hermes-definition"; import { assertPodmanExecutableAuthority, - capturePodmanExecutableAuthority, type PodmanExecutableAuthority, type PodmanExecutableAuthorityDeps, @@ -190,6 +189,7 @@ export function buildOpenShellSubprocessEnv( "SSL_CERT_DIR", "NODE_EXTRA_CA_CERTS", "CURL_CA_BUNDLE", + "XDG_CONFIG_HOME", ]); const environment = Object.fromEntries( Object.entries(source).filter( diff --git a/src/lib/adapters/openshell/sanitized-capture.ts b/src/lib/adapters/openshell/sanitized-capture.ts index 633ca2bd89a..d3d5c9075cc 100644 --- a/src/lib/adapters/openshell/sanitized-capture.ts +++ b/src/lib/adapters/openshell/sanitized-capture.ts @@ -23,7 +23,7 @@ export function captureSanitizedResolvedOpenshell( opts: SanitizedCaptureOptions, ): CapturedOpenShellCommandResult { const env = buildOpenShellSubprocessEnv(); - for (const name of ["XDG_CONFIG_HOME", "OPENSHELL_WORKSPACE"] as const) { + for (const name of ["OPENSHELL_WORKSPACE"] as const) { const value = process.env[name]; if (value !== undefined) env[name] = value; } From ffac8c8c5b2e99f574ceb5ff4abbaed94b7b632d Mon Sep 17 00:00:00 2001 From: San Dang Date: Mon, 7 Sep 2026 15:58:13 +0700 Subject: [PATCH 3/8] test(openshell): classify forward startup state Signed-off-by: San Dang --- .../openshell/forward-service.test.ts | 18 ++++++ src/lib/adapters/openshell/forward-service.ts | 64 ++++++++++++++----- 2 files changed, 66 insertions(+), 16 deletions(-) diff --git a/src/lib/adapters/openshell/forward-service.test.ts b/src/lib/adapters/openshell/forward-service.test.ts index be5ad461ef0..a4b8c333f49 100644 --- a/src/lib/adapters/openshell/forward-service.test.ts +++ b/src/lib/adapters/openshell/forward-service.test.ts @@ -365,6 +365,24 @@ describe("OpenShell forward service", () => { expect(removeOutput).toHaveBeenCalledOnce(); }); + it("records an exact OpenShell bind announcement without exposing raw output (#11084)", () => { + expect(() => + launchForwardService(target, { + getProcessIdentity: stableProcessIdentity, + isProcessRunning: () => false, + isReachable: () => false, + sleep: () => {}, + spawnDetached: () => ({ + pid: 83, + readOutput: () => + "✓ Forwarding 127.0.0.1:18789 -> 127.0.0.1:18789 in sandbox demo via gRPC", + removeOutput: vi.fn(), + unref: vi.fn(), + }), + }), + ).toThrow(/forwarding-announced/u); + }); + it.runIf(process.platform === "linux")( "proves listener ownership from Linux procfs without connecting (#11084)", async () => { diff --git a/src/lib/adapters/openshell/forward-service.ts b/src/lib/adapters/openshell/forward-service.ts index 83abfed9b51..62c81dbed1c 100644 --- a/src/lib/adapters/openshell/forward-service.ts +++ b/src/lib/adapters/openshell/forward-service.ts @@ -163,11 +163,22 @@ function linuxProcessOwnsListener( return unreadableDescriptor ? null : false; } +type ForwardListenerObservation = "owned" | "absent" | "foreign" | "unavailable"; + +function observeLinuxForwardListener(pid: number, port: number): ForwardListenerObservation { + const listenerInodes = readLinuxListeningSocketInodes(port); + if (listenerInodes === null) return "unavailable"; + if (listenerInodes.size === 0) return "absent"; + const owned = linuxProcessOwnsListener(pid, listenerInodes); + if (owned === null) return "unavailable"; + return owned ? "owned" : "foreign"; +} + /** Prove that a listener belongs to the exact child requested by this launch. */ export function getForwardListenerOwnership(pid: number, port: number): boolean | null { if (process.platform === "linux") { - const listenerInodes = readLinuxListeningSocketInodes(port); - return listenerInodes ? linuxProcessOwnsListener(pid, listenerInodes) : null; + const observation = observeLinuxForwardListener(pid, port); + return observation === "owned" ? true : observation === "unavailable" ? null : false; } const result = spawnSync( "lsof", @@ -272,18 +283,27 @@ function isSandboxCreatingHandoff(output: string, sandboxName: string): boolean ).test(compactOpenShellDiagnostic(output)); } +function isForwardingAnnounced(output: string, target: ForwardServiceTarget): boolean { + const expected = + `Forwarding ${target.localHost}:${String(target.localPort)} - ` + + `${target.targetHost}:${String(target.targetPort)} in sandbox ${target.sandboxName} via gRPC`; + return compactOpenShellDiagnostic(output).includes(expected); +} + function classifyStartOutput( child: ForwardServiceChild, - sandboxName: string, + target: ForwardServiceTarget, ): { readonly category: string; readonly sandboxCreating: boolean } { const output = child.readOutput?.() ?? ""; - const sandboxCreating = isSandboxCreatingHandoff(output, sandboxName); + const sandboxCreating = isSandboxCreatingHandoff(output, target.sandboxName); return { category: sandboxCreating ? "sandbox-creating" - : output.trim() - ? "non-readiness-diagnostic" - : "empty-diagnostic", + : isForwardingAnnounced(output, target) + ? "forwarding-announced" + : output.trim() + ? "non-readiness-diagnostic" + : "empty-diagnostic", sandboxCreating, }; } @@ -425,7 +445,17 @@ function startForwardServiceAttempt(input: { readonly target: ForwardServiceTarget; }): ForwardAttemptResult | null { const readIdentity = input.options.getProcessIdentity ?? getProcessIdentity; - const listenerOwned = input.options.isListenerOwned ?? getForwardListenerOwnership; + const observeListener = input.options.isListenerOwned + ? (pid: number, port: number): ForwardListenerObservation => { + const owned = input.options.isListenerOwned?.(pid, port); + return owned === true ? "owned" : owned === false ? "absent" : "unavailable"; + } + : process.platform === "linux" + ? observeLinuxForwardListener + : (pid: number, port: number): ForwardListenerObservation => { + const owned = getForwardListenerOwnership(pid, port); + return owned === true ? "owned" : owned === false ? "absent" : "unavailable"; + }; const running = input.options.isProcessRunning ?? isProcessRunning; const sleep = input.options.sleep ?? @@ -440,7 +470,7 @@ function startForwardServiceAttempt(input: { [FORWARD_INSTANCE_ENV]: instanceId, }); if (!isProcessId(child.pid)) { - const start = classifyStartOutput(child, input.target.sandboxName); + const start = classifyStartOutput(child, input.target); child.removeOutput?.(); throw new Error( `OpenShell forward service returned no process identity for ${input.target.localHost}:${String(input.target.localPort)}; refusing to start a duplicate service; forward start: ${start.category}`, @@ -455,22 +485,24 @@ function startForwardServiceAttempt(input: { child.unref(); let stableObservations = 0; let stabilityDeadline: number | undefined; + let listenerObservation: ForwardListenerObservation = "absent"; while (true) { const identity = processIdentityStatus(pid, expectedIdentity, readIdentity); if (identity === "exited" || !running(pid)) { - const start = classifyStartOutput(child, input.target.sandboxName); + const start = classifyStartOutput(child, input.target); child.removeOutput?.(); return { ...start, processId: pid }; } if (identity === "unverified") { - const start = classifyStartOutput(child, input.target.sandboxName); + const start = classifyStartOutput(child, input.target); child.removeOutput?.(); throw new Error( `OpenShell forward service process ${String(pid)} changed identity before binding ${input.target.localHost}:${String(input.target.localPort)}; refusing to signal or retry; forward start: ${start.category}`, ); } - if (listenerOwned(pid, input.target.localPort) === true) { + listenerObservation = observeListener(pid, input.target.localPort); + if (listenerObservation === "owned") { stabilityDeadline ??= Date.now() + SANDBOX_CREATING_RETRY_INTERVAL_MS + POLL_INTERVAL_MS; stableObservations += 1; if (stableObservations >= STABLE_LISTENER_OBSERVATIONS) { @@ -496,21 +528,21 @@ function startForwardServiceAttempt(input: { stop, timeoutMs: input.options.stopTimeoutMs ?? STOP_TIMEOUT_MS, }); - const start = classifyStartOutput(child, input.target.sandboxName); + const start = classifyStartOutput(child, input.target); child.removeOutput?.(); if (stopped !== "stopped") { throw new Error( - `OpenShell forward service process ${String(pid)} did not become ready and ${stopped === "unverified" ? "could not be verified as owned" : "could not be stopped"}; refusing to retry; forward start: ${start.category}`, + `OpenShell forward service process ${String(pid)} did not become ready and ${stopped === "unverified" ? "could not be verified as owned" : "could not be stopped"}; refusing to retry; listener: ${listenerObservation}; forward start: ${start.category}`, ); } const reachable = input.options.isReachable ?? probeLocalForwardListener; if (reachable(input.target.localPort)) { throw new Error( - `Host port ${String(input.target.localPort)} remained reachable after the launched process stopped; refusing to adopt its listener or retry; forward start: ${start.category}`, + `Host port ${String(input.target.localPort)} remained reachable after the launched process stopped; refusing to adopt its listener or retry; listener: ${listenerObservation}; forward start: ${start.category}`, ); } throw new Error( - `OpenShell forward service did not become ready at ${input.target.localHost}:${String(input.target.localPort)}; forward start: ${start.category}`, + `OpenShell forward service did not become ready at ${input.target.localHost}:${String(input.target.localPort)}; listener: ${listenerObservation}; forward start: ${start.category}`, ); } From c7853d662d24777ae9c5cec6c984fcb85b17b281 Mon Sep 17 00:00:00 2001 From: San Dang Date: Mon, 7 Sep 2026 16:23:51 +0700 Subject: [PATCH 4/8] fix(openshell): recheck owned forward after health window Signed-off-by: San Dang --- .../openshell/forward-service.test.ts | 32 +++++++++++++++++-- src/lib/adapters/openshell/forward-service.ts | 22 ++++++------- 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/src/lib/adapters/openshell/forward-service.test.ts b/src/lib/adapters/openshell/forward-service.test.ts index a4b8c333f49..a3636c89d8f 100644 --- a/src/lib/adapters/openshell/forward-service.test.ts +++ b/src/lib/adapters/openshell/forward-service.test.ts @@ -135,20 +135,46 @@ describe("OpenShell forward service", () => { expect(stopProcess).toHaveBeenCalledWith(42, "SIGTERM"); }); - it("accepts delayed ownership only after it remains stable (#11084)", () => { + it("accepts delayed ownership only after the exact child survives a health-check window (#11084)", () => { + let now = 0; let ownershipChecks = 0; + vi.spyOn(Date, "now").mockImplementation(() => now); launchForwardService(target, { getProcessIdentity: stableProcessIdentity, isListenerOwned: () => ++ownershipChecks >= 3, isProcessRunning: () => true, isReachable: () => false, - sleep: () => {}, + sleep: (milliseconds) => { + now += milliseconds; + }, spawnDetached: () => ({ pid: 43, unref: vi.fn() }), timeoutMs: 10_000, }); - expect(ownershipChecks).toBe(23); + expect(ownershipChecks).toBe(24); + expect(now).toBe(2_300); + }); + + it("does not require every intermediate listener inspection to observe the owned socket (#11084)", () => { + let now = 0; + let ownershipChecks = 0; + vi.spyOn(Date, "now").mockImplementation(() => now); + + launchForwardService(target, { + getProcessIdentity: stableProcessIdentity, + isListenerOwned: () => ++ownershipChecks % 10 === 1, + isProcessRunning: () => true, + isReachable: () => false, + sleep: (milliseconds) => { + now += milliseconds; + }, + spawnDetached: () => ({ pid: 44, unref: vi.fn() }), + timeoutMs: 10_000, + }); + + expect(ownershipChecks).toBe(31); + expect(now).toBe(3_000); }); it("does not signal a process whose launch identity changed (#11084)", () => { diff --git a/src/lib/adapters/openshell/forward-service.ts b/src/lib/adapters/openshell/forward-service.ts index 62c81dbed1c..f92f1c26876 100644 --- a/src/lib/adapters/openshell/forward-service.ts +++ b/src/lib/adapters/openshell/forward-service.ts @@ -18,8 +18,8 @@ const SANDBOX_CREATING_MAX_RETRIES = START_TIMEOUT_MS / SANDBOX_CREATING_RETRY_I const STOP_TIMEOUT_MS = 5_000; const POLL_INTERVAL_MS = 100; // OpenShell 0.0.106 rechecks sandbox readiness every two seconds after it -// binds. Retain exact listener ownership through the next complete check. -const STABLE_LISTENER_OBSERVATIONS = SANDBOX_CREATING_RETRY_INTERVAL_MS / POLL_INTERVAL_MS + 1; +// binds. Re-prove exact listener ownership after the next complete check. +const LISTENER_RECHECK_DELAY_MS = SANDBOX_CREATING_RETRY_INTERVAL_MS + POLL_INTERVAL_MS; const FORWARD_INSTANCE_ENV = "NEMOCLAW_FORWARD_INSTANCE_ID"; const sleepBuffer = new Int32Array(new SharedArrayBuffer(4)); @@ -483,8 +483,7 @@ function startForwardServiceAttempt(input: { ? readIdentity(pid) : `${process.platform}:${instanceId}`; child.unref(); - let stableObservations = 0; - let stabilityDeadline: number | undefined; + let firstOwnedListenerAt: number | undefined; let listenerObservation: ForwardListenerObservation = "absent"; while (true) { @@ -502,18 +501,19 @@ function startForwardServiceAttempt(input: { ); } listenerObservation = observeListener(pid, input.target.localPort); + const now = Date.now(); if (listenerObservation === "owned") { - stabilityDeadline ??= Date.now() + SANDBOX_CREATING_RETRY_INTERVAL_MS + POLL_INTERVAL_MS; - stableObservations += 1; - if (stableObservations >= STABLE_LISTENER_OBSERVATIONS) { + firstOwnedListenerAt ??= now; + if (now - firstOwnedListenerAt >= LISTENER_RECHECK_DELAY_MS) { child.removeOutput?.(); return null; } - } else { - stableObservations = 0; } - const now = Date.now(); - if (stabilityDeadline !== undefined ? now >= stabilityDeadline : now >= input.readyDeadline) { + const observationDeadline = + firstOwnedListenerAt === undefined + ? input.readyDeadline + : Math.max(input.readyDeadline, firstOwnedListenerAt + LISTENER_RECHECK_DELAY_MS); + if (now >= observationDeadline) { break; } sleep(POLL_INTERVAL_MS); From 99fba2b1112d9e793d07be3f3d031bd39264a67e Mon Sep 17 00:00:00 2001 From: San Dang Date: Mon, 7 Sep 2026 16:48:36 +0700 Subject: [PATCH 5/8] fix(openshell): verify forward reachability Signed-off-by: San Dang --- .../openshell/forward-service.test.ts | 73 ++++++++++++++++--- src/lib/adapters/openshell/forward-service.ts | 20 +++-- 2 files changed, 78 insertions(+), 15 deletions(-) diff --git a/src/lib/adapters/openshell/forward-service.test.ts b/src/lib/adapters/openshell/forward-service.test.ts index a3636c89d8f..97569e1c93c 100644 --- a/src/lib/adapters/openshell/forward-service.test.ts +++ b/src/lib/adapters/openshell/forward-service.test.ts @@ -52,7 +52,7 @@ describe("OpenShell forward service", () => { it("detaches the OpenShell child and waits for its owned local listener (#11084)", () => { const unref = vi.fn(); const spawnDetached = vi.fn(() => ({ pid: 41, unref })); - const isReachable = vi.fn(() => false); + const isReachable = vi.fn().mockReturnValueOnce(false).mockReturnValueOnce(true); launchForwardService(target, { getProcessIdentity: stableProcessIdentity, @@ -70,7 +70,7 @@ describe("OpenShell forward service", () => { expect.any(Object), ); expect(unref).toHaveBeenCalledOnce(); - expect(isReachable).toHaveBeenCalledOnce(); + expect(isReachable).toHaveBeenCalledTimes(2); }); it("uses the selected OpenShell configuration without exposing credentials (#11084)", () => { @@ -86,7 +86,7 @@ describe("OpenShell forward service", () => { getProcessIdentity: stableProcessIdentity, isListenerOwned: () => true, isProcessRunning: () => true, - isReachable: () => false, + isReachable: vi.fn().mockReturnValueOnce(false).mockReturnValueOnce(true), sleep: () => {}, sourceEnvironment: { HOME: "/tmp/isolated-home", @@ -144,7 +144,7 @@ describe("OpenShell forward service", () => { getProcessIdentity: stableProcessIdentity, isListenerOwned: () => ++ownershipChecks >= 3, isProcessRunning: () => true, - isReachable: () => false, + isReachable: vi.fn().mockReturnValueOnce(false).mockReturnValueOnce(true), sleep: (milliseconds) => { now += milliseconds; }, @@ -165,7 +165,7 @@ describe("OpenShell forward service", () => { getProcessIdentity: stableProcessIdentity, isListenerOwned: () => ++ownershipChecks % 10 === 1, isProcessRunning: () => true, - isReachable: () => false, + isReachable: vi.fn().mockReturnValueOnce(false).mockReturnValueOnce(true), sleep: (milliseconds) => { now += milliseconds; }, @@ -177,6 +177,59 @@ describe("OpenShell forward service", () => { expect(now).toBe(3_000); }); + it("does not accept an unowned listener at the completion observation (#11084)", () => { + let now = 0; + let running = true; + let ownershipChecks = 0; + const stopProcess = vi.fn(() => { + running = false; + }); + vi.spyOn(Date, "now").mockImplementation(() => now); + + expect(() => + launchForwardService(target, { + getProcessIdentity: stableProcessIdentity, + isListenerOwned: () => ++ownershipChecks === 1, + isProcessRunning: () => running, + isReachable: () => false, + sleep: (milliseconds) => { + now += milliseconds; + }, + spawnDetached: () => ({ pid: 45, unref: vi.fn() }), + stopProcess, + timeoutMs: 2_100, + }), + ).toThrow(/listener: absent/u); + expect(stopProcess).toHaveBeenCalledWith(45, "SIGTERM"); + }); + + it("does not accept an owned listener that still refuses connections (#11084)", () => { + let now = 0; + let running = true; + const isReachable = vi.fn(() => false); + const stopProcess = vi.fn(() => { + running = false; + }); + vi.spyOn(Date, "now").mockImplementation(() => now); + + expect(() => + launchForwardService(target, { + getProcessIdentity: stableProcessIdentity, + isListenerOwned: () => true, + isProcessRunning: () => running, + isReachable, + sleep: (milliseconds) => { + now += milliseconds; + }, + spawnDetached: () => ({ pid: 46, unref: vi.fn() }), + stopProcess, + timeoutMs: 2_100, + }), + ).toThrow(/listener: owned; reachability: refused/u); + expect(isReachable).toHaveBeenCalledTimes(3); + expect(stopProcess).toHaveBeenCalledWith(46, "SIGTERM"); + }); + it("does not signal a process whose launch identity changed (#11084)", () => { let identityChecks = 0; const stopProcess = vi.fn(); @@ -250,7 +303,11 @@ describe("OpenShell forward service", () => { getProcessIdentity: stableProcessIdentity, isListenerOwned: (pid) => pid === 52, isProcessRunning: (pid) => pid === 52, - isReachable: () => false, + isReachable: vi + .fn() + .mockReturnValueOnce(false) + .mockReturnValueOnce(false) + .mockReturnValueOnce(true), maxSandboxCreatingRetries: 1, onSandboxCreatingRetry, sleep, @@ -341,9 +398,7 @@ describe("OpenShell forward service", () => { spawnDetached, timeoutMs: 10_000, }), - ).toThrow( - /attempts: 1=pid-72:sandbox-creating, 2=pid-72:sandbox-creating, 3=pid-72:sandbox-creating/u, - ); + ).toThrow(/attempts: 1=pid-72:sandbox-creating:listener-absent:reachability-not-checked/u); expect(spawnDetached).toHaveBeenCalledTimes(3); }); diff --git a/src/lib/adapters/openshell/forward-service.ts b/src/lib/adapters/openshell/forward-service.ts index f92f1c26876..e6d93a1eb28 100644 --- a/src/lib/adapters/openshell/forward-service.ts +++ b/src/lib/adapters/openshell/forward-service.ts @@ -433,7 +433,9 @@ export function buildForwardServiceArgs(target: ForwardServiceTarget): string[] type ForwardAttemptResult = { readonly category: string; + readonly listenerObservation: ForwardListenerObservation; readonly processId: number; + readonly reachabilityObservation: "not-checked" | "refused"; readonly sandboxCreating: boolean; }; @@ -464,6 +466,7 @@ function startForwardServiceAttempt(input: { const stop = input.options.stopProcess ?? ((pid: number, signal: NodeJS.Signals) => process.kill(pid, signal)); + const reachable = input.options.isReachable ?? probeLocalForwardListener; const instanceId = randomUUID(); const child = spawnDetached(input.target.executable, input.args, { ...input.environment, @@ -485,13 +488,14 @@ function startForwardServiceAttempt(input: { child.unref(); let firstOwnedListenerAt: number | undefined; let listenerObservation: ForwardListenerObservation = "absent"; + let reachabilityObservation: "not-checked" | "refused" = "not-checked"; while (true) { const identity = processIdentityStatus(pid, expectedIdentity, readIdentity); if (identity === "exited" || !running(pid)) { const start = classifyStartOutput(child, input.target); child.removeOutput?.(); - return { ...start, processId: pid }; + return { ...start, listenerObservation, processId: pid, reachabilityObservation }; } if (identity === "unverified") { const start = classifyStartOutput(child, input.target); @@ -505,8 +509,11 @@ function startForwardServiceAttempt(input: { if (listenerObservation === "owned") { firstOwnedListenerAt ??= now; if (now - firstOwnedListenerAt >= LISTENER_RECHECK_DELAY_MS) { - child.removeOutput?.(); - return null; + if (reachable(input.target.localPort)) { + child.removeOutput?.(); + return null; + } + reachabilityObservation = "refused"; } } const observationDeadline = @@ -535,14 +542,13 @@ function startForwardServiceAttempt(input: { `OpenShell forward service process ${String(pid)} did not become ready and ${stopped === "unverified" ? "could not be verified as owned" : "could not be stopped"}; refusing to retry; listener: ${listenerObservation}; forward start: ${start.category}`, ); } - const reachable = input.options.isReachable ?? probeLocalForwardListener; if (reachable(input.target.localPort)) { throw new Error( `Host port ${String(input.target.localPort)} remained reachable after the launched process stopped; refusing to adopt its listener or retry; listener: ${listenerObservation}; forward start: ${start.category}`, ); } throw new Error( - `OpenShell forward service did not become ready at ${input.target.localHost}:${String(input.target.localPort)}; listener: ${listenerObservation}; forward start: ${start.category}`, + `OpenShell forward service did not become ready at ${input.target.localHost}:${String(input.target.localPort)}; listener: ${listenerObservation}; reachability: ${reachabilityObservation}; forward start: ${start.category}`, ); } @@ -594,7 +600,9 @@ export function launchForwardService( }); if (result === null) return; const attempt = retries + 1; - evidence.push(`${String(attempt)}=pid-${String(result.processId)}:${result.category}`); + evidence.push( + `${String(attempt)}=pid-${String(result.processId)}:${result.category}:listener-${result.listenerObservation}:reachability-${result.reachabilityObservation}`, + ); const remainingMs = Math.max(0, readyDeadline - Date.now()); if ( !result.sandboxCreating || From 14c7f2113a2ab27da8c32afd5b19224fa56799fe Mon Sep 17 00:00:00 2001 From: San Dang Date: Mon, 7 Sep 2026 17:19:18 +0700 Subject: [PATCH 6/8] fix(openshell): bound forward diagnostics Signed-off-by: San Dang --- .../openshell/forward-service.test.ts | 68 +++++++++++++++++++ src/lib/adapters/openshell/forward-service.ts | 29 ++++++-- 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/src/lib/adapters/openshell/forward-service.test.ts b/src/lib/adapters/openshell/forward-service.test.ts index 97569e1c93c..43579855a46 100644 --- a/src/lib/adapters/openshell/forward-service.test.ts +++ b/src/lib/adapters/openshell/forward-service.test.ts @@ -7,6 +7,7 @@ import { describe, expect, it, vi } from "vitest"; import { buildForwardServiceArgs, + forwardServiceInternals, getForwardListenerOwnership, launchForwardService, type ForwardServiceTarget, @@ -325,6 +326,38 @@ describe("OpenShell forward service", () => { expect(sleep).toHaveBeenCalledWith(2_000); }); + it("classifies a creating handoff that exits before its identity can be read (#11084)", () => { + const diagnostic = + "sandbox 'demo' is no longer ready (phase: creating); stopping service forward"; + const spawnDetached = vi + .fn() + .mockReturnValueOnce({ + pid: 53, + readOutput: () => diagnostic, + removeOutput: vi.fn(), + unref: vi.fn(), + }) + .mockReturnValueOnce({ pid: 54, removeOutput: vi.fn(), unref: vi.fn() }); + + launchForwardService(target, { + getProcessIdentity: (pid) => (pid === 53 ? null : stableProcessIdentity(pid)), + isListenerOwned: (pid) => pid === 54, + isProcessRunning: (pid) => pid === 54, + isReachable: vi + .fn() + .mockReturnValueOnce(false) + .mockReturnValueOnce(false) + .mockReturnValueOnce(true), + maxSandboxCreatingRetries: 1, + onSandboxCreatingRetry: () => {}, + sleep: () => {}, + spawnDetached, + timeoutMs: 10_000, + }); + + expect(spawnDetached).toHaveBeenCalledTimes(2); + }); + it.each([ [ "terminal phase", @@ -464,6 +497,41 @@ describe("OpenShell forward service", () => { ).toThrow(/forwarding-announced/u); }); + it.runIf(process.platform === "linux" || process.platform === "darwin")( + "bounds output retained by a long-running forward child (#11084)", + async () => { + const child = forwardServiceInternals.spawnForwardService( + process.execPath, + [ + "-e", + 'process.stdout.write("x".repeat(65536)); setInterval(() => process.stdout.write("later\\n"), 10);', + ], + process.env, + ); + try { + await vi.waitFor( + () => + expect(child.readOutput?.()).toHaveLength( + forwardServiceInternals.startOutputLimitBytes, + ), + { interval: 25, timeout: 2_000 }, + ); + await new Promise((resolve) => setTimeout(resolve, 100)); + expect(child.readOutput?.()).toHaveLength(forwardServiceInternals.startOutputLimitBytes); + child.removeOutput?.(); + expect(child.readOutput?.()).toBe(""); + } finally { + child.removeOutput?.(); + expect(child.pid).toBeTypeOf("number"); + try { + process.kill(child.pid as number, "SIGTERM"); + } catch { + // The fixture may already have stopped after a failed assertion. + } + } + }, + ); + it.runIf(process.platform === "linux")( "proves listener ownership from Linux procfs without connecting (#11084)", async () => { diff --git a/src/lib/adapters/openshell/forward-service.ts b/src/lib/adapters/openshell/forward-service.ts index e6d93a1eb28..d932fe1c1a3 100644 --- a/src/lib/adapters/openshell/forward-service.ts +++ b/src/lib/adapters/openshell/forward-service.ts @@ -17,11 +17,17 @@ const SANDBOX_CREATING_RETRY_INTERVAL_MS = 2_000; const SANDBOX_CREATING_MAX_RETRIES = START_TIMEOUT_MS / SANDBOX_CREATING_RETRY_INTERVAL_MS; const STOP_TIMEOUT_MS = 5_000; const POLL_INTERVAL_MS = 100; +const START_OUTPUT_LIMIT_BYTES = 16 * 1_024; // OpenShell 0.0.106 rechecks sandbox readiness every two seconds after it // binds. Re-prove exact listener ownership after the next complete check. const LISTENER_RECHECK_DELAY_MS = SANDBOX_CREATING_RETRY_INTERVAL_MS + POLL_INTERVAL_MS; const FORWARD_INSTANCE_ENV = "NEMOCLAW_FORWARD_INSTANCE_ID"; const sleepBuffer = new Int32Array(new SharedArrayBuffer(4)); +const boundedOutputWrapper = [ + "capture_path=$1", + "shift", + `exec \"$@\" > >({ /usr/bin/head -c ${String(START_OUTPUT_LIMIT_BYTES)} > \"$capture_path\"; /bin/cat >/dev/null; }) 2>&1`, +].join("\n"); type ForwardServiceChild = { readonly pid?: number; @@ -225,11 +231,17 @@ function spawnForwardService( const outputDescriptor = fs.openSync(outputPath, "wx", 0o600); let child: ReturnType; try { - child = spawn(executable, [...args], { - detached: true, - env: environment, - stdio: ["ignore", outputDescriptor, outputDescriptor], - }); + // Bash exec preserves the launch PID while the process substitution keeps + // only a bounded startup prefix and drains all later output to /dev/null. + child = spawn( + "/bin/bash", + ["-c", boundedOutputWrapper, "nemoclaw-forward-service", outputPath, executable, ...args], + { + detached: true, + env: environment, + stdio: ["ignore", "ignore", "ignore"], + }, + ); } catch (error) { try { fs.closeSync(outputDescriptor); @@ -246,7 +258,7 @@ function spawnForwardService( try { fs.closeSync(outputDescriptor); } catch { - // The child owns its inherited descriptor; the parent no longer needs it. + // The drain wrapper reopens the path; the parent no longer needs this descriptor. } return { pid: child.pid, @@ -268,6 +280,11 @@ function spawnForwardService( }; } +export const forwardServiceInternals = Object.freeze({ + spawnForwardService, + startOutputLimitBytes: START_OUTPUT_LIMIT_BYTES, +}); + function compactOpenShellDiagnostic(output: string): string { return stripVTControlCharacters(output) .replace(/[^\p{L}\p{N}\s'"():;._-]+/gu, " ") From e09c4ba873f471906a04d750d3a434113dcaf3d9 Mon Sep 17 00:00:00 2001 From: San Dang Date: Mon, 7 Sep 2026 17:46:37 +0700 Subject: [PATCH 7/8] fix(openshell): synchronize forward diagnostics Signed-off-by: San Dang --- .../openshell/forward-service.test.ts | 70 ++++++++++++++++++- src/lib/adapters/openshell/forward-service.ts | 52 ++++++++++---- 2 files changed, 107 insertions(+), 15 deletions(-) diff --git a/src/lib/adapters/openshell/forward-service.test.ts b/src/lib/adapters/openshell/forward-service.test.ts index 43579855a46..e1c32bbed2a 100644 --- a/src/lib/adapters/openshell/forward-service.test.ts +++ b/src/lib/adapters/openshell/forward-service.test.ts @@ -358,6 +358,68 @@ describe("OpenShell forward service", () => { expect(spawnDetached).toHaveBeenCalledTimes(2); }); + it("waits for the bounded output drain before classifying an exited service (#11084)", () => { + const diagnostic = + "sandbox 'demo' is no longer ready (phase: creating); stopping service forward"; + let capturedOutput = ""; + const waitForOutput = vi.fn(() => { + capturedOutput = diagnostic; + return true; + }); + const spawnDetached = vi + .fn() + .mockReturnValueOnce({ + pid: 55, + readOutput: () => capturedOutput, + removeOutput: vi.fn(), + unref: vi.fn(), + waitForOutput, + }) + .mockReturnValueOnce({ pid: 56, removeOutput: vi.fn(), unref: vi.fn() }); + + launchForwardService(target, { + getProcessIdentity: stableProcessIdentity, + isListenerOwned: (pid) => pid === 56, + isProcessRunning: (pid) => pid === 56, + isReachable: vi + .fn() + .mockReturnValueOnce(false) + .mockReturnValueOnce(false) + .mockReturnValueOnce(true), + maxSandboxCreatingRetries: 1, + onSandboxCreatingRetry: () => {}, + sleep: () => {}, + spawnDetached, + timeoutMs: 10_000, + }); + + expect(waitForOutput).toHaveBeenCalledOnce(); + expect(spawnDetached).toHaveBeenCalledTimes(2); + }); + + it("does not retry when an exited service diagnostic cannot be completed (#11084)", () => { + const spawnDetached = vi.fn(() => ({ + pid: 57, + readOutput: () => + "sandbox 'demo' is no longer ready (phase: creating); stopping service forward", + removeOutput: vi.fn(), + unref: vi.fn(), + waitForOutput: () => false, + })); + + expect(() => + launchForwardService(target, { + getProcessIdentity: stableProcessIdentity, + isProcessRunning: () => false, + isReachable: () => false, + maxSandboxCreatingRetries: 1, + sleep: () => {}, + spawnDetached, + }), + ).toThrow(/diagnostic-incomplete/u); + expect(spawnDetached).toHaveBeenCalledOnce(); + }); + it.each([ [ "terminal phase", @@ -518,16 +580,16 @@ describe("OpenShell forward service", () => { ); await new Promise((resolve) => setTimeout(resolve, 100)); expect(child.readOutput?.()).toHaveLength(forwardServiceInternals.startOutputLimitBytes); - child.removeOutput?.(); - expect(child.readOutput?.()).toBe(""); } finally { - child.removeOutput?.(); expect(child.pid).toBeTypeOf("number"); try { process.kill(child.pid as number, "SIGTERM"); } catch { // The fixture may already have stopped after a failed assertion. } + expect(child.waitForOutput?.()).toBe(true); + child.removeOutput?.(); + expect(child.readOutput?.()).toBe(""); } }, ); @@ -543,6 +605,7 @@ describe("OpenShell forward service", () => { try { const address = server.address() as AddressInfo; expect(getForwardListenerOwnership(process.pid, address.port)).toBe(true); + expect(getForwardListenerOwnership(process.ppid, address.port)).toBe(false); } finally { await new Promise((resolve) => server.close(() => resolve())); } @@ -560,6 +623,7 @@ describe("OpenShell forward service", () => { try { const address = server.address() as AddressInfo; expect(getForwardListenerOwnership(process.pid, address.port)).toBe(true); + expect(getForwardListenerOwnership(process.ppid, address.port)).toBe(false); } finally { await new Promise((resolve) => server.close(() => resolve())); } diff --git a/src/lib/adapters/openshell/forward-service.ts b/src/lib/adapters/openshell/forward-service.ts index d932fe1c1a3..d504f44d7bf 100644 --- a/src/lib/adapters/openshell/forward-service.ts +++ b/src/lib/adapters/openshell/forward-service.ts @@ -18,6 +18,9 @@ const SANDBOX_CREATING_MAX_RETRIES = START_TIMEOUT_MS / SANDBOX_CREATING_RETRY_I const STOP_TIMEOUT_MS = 5_000; const POLL_INTERVAL_MS = 100; const START_OUTPUT_LIMIT_BYTES = 16 * 1_024; +// After the service PID exits, allow the local pipe drainer at most one second +// to publish its completion receipt. Missing completion is terminal, not retryable. +const START_OUTPUT_DRAIN_TIMEOUT_MS = 1_000; // OpenShell 0.0.106 rechecks sandbox readiness every two seconds after it // binds. Re-prove exact listener ownership after the next complete check. const LISTENER_RECHECK_DELAY_MS = SANDBOX_CREATING_RETRY_INTERVAL_MS + POLL_INTERVAL_MS; @@ -25,14 +28,16 @@ const FORWARD_INSTANCE_ENV = "NEMOCLAW_FORWARD_INSTANCE_ID"; const sleepBuffer = new Int32Array(new SharedArrayBuffer(4)); const boundedOutputWrapper = [ "capture_path=$1", - "shift", - `exec \"$@\" > >({ /usr/bin/head -c ${String(START_OUTPUT_LIMIT_BYTES)} > \"$capture_path\"; /bin/cat >/dev/null; }) 2>&1`, + "capture_done_path=$2", + "shift 2", + `exec \"$@\" > >({ /usr/bin/head -c ${String(START_OUTPUT_LIMIT_BYTES)} > \"$capture_path\"; /bin/cat >/dev/null; : > \"$capture_done_path\"; }) 2>&1`, ].join("\n"); type ForwardServiceChild = { readonly pid?: number; readonly readOutput?: () => string; readonly removeOutput?: () => void; + readonly waitForOutput?: () => boolean; unref(): void; }; @@ -228,14 +233,25 @@ function spawnForwardService( const outputDirectory = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-forward-service-")); fs.chmodSync(outputDirectory, 0o700); const outputPath = path.join(outputDirectory, "start.log"); + const outputDonePath = path.join(outputDirectory, "start.done"); const outputDescriptor = fs.openSync(outputPath, "wx", 0o600); let child: ReturnType; try { // Bash exec preserves the launch PID while the process substitution keeps - // only a bounded startup prefix and drains all later output to /dev/null. + // only a bounded startup prefix, drains all later output to /dev/null, and + // records when the pipe has closed so an exited service is classified only + // after its complete bounded diagnostic is available. child = spawn( "/bin/bash", - ["-c", boundedOutputWrapper, "nemoclaw-forward-service", outputPath, executable, ...args], + [ + "-c", + boundedOutputWrapper, + "nemoclaw-forward-service", + outputPath, + outputDonePath, + executable, + ...args, + ], { detached: true, env: environment, @@ -270,6 +286,15 @@ function spawnForwardService( return ""; } }, + waitForOutput: () => { + const deadline = Date.now() + START_OUTPUT_DRAIN_TIMEOUT_MS; + while (!fs.existsSync(outputDonePath)) { + const remainingMs = deadline - Date.now(); + if (remainingMs <= 0) return false; + Atomics.wait(sleepBuffer, 0, 0, Math.min(POLL_INTERVAL_MS, remainingMs)); + } + return true; + }, removeOutput: () => { try { fs.rmSync(outputDirectory, { force: true, recursive: true }); @@ -311,16 +336,19 @@ function classifyStartOutput( child: ForwardServiceChild, target: ForwardServiceTarget, ): { readonly category: string; readonly sandboxCreating: boolean } { + const outputComplete = child.waitForOutput?.() ?? true; const output = child.readOutput?.() ?? ""; - const sandboxCreating = isSandboxCreatingHandoff(output, target.sandboxName); + const sandboxCreating = outputComplete && isSandboxCreatingHandoff(output, target.sandboxName); return { - category: sandboxCreating - ? "sandbox-creating" - : isForwardingAnnounced(output, target) - ? "forwarding-announced" - : output.trim() - ? "non-readiness-diagnostic" - : "empty-diagnostic", + category: !outputComplete + ? "diagnostic-incomplete" + : sandboxCreating + ? "sandbox-creating" + : isForwardingAnnounced(output, target) + ? "forwarding-announced" + : output.trim() + ? "non-readiness-diagnostic" + : "empty-diagnostic", sandboxCreating, }; } From efc00cb6badab8804a45045dd92a1f36a0bbf6b1 Mon Sep 17 00:00:00 2001 From: San Dang Date: Mon, 7 Sep 2026 19:09:24 +0700 Subject: [PATCH 8/8] fix(openshell): keep forward launch process-agnostic Signed-off-by: San Dang --- .../openshell/forward-service.test.ts | 539 +--------------- src/lib/adapters/openshell/forward-service.ts | 597 +----------------- .../adapters/openshell/sanitized-capture.ts | 2 +- 3 files changed, 33 insertions(+), 1105 deletions(-) diff --git a/src/lib/adapters/openshell/forward-service.test.ts b/src/lib/adapters/openshell/forward-service.test.ts index e1c32bbed2a..540c40f1aca 100644 --- a/src/lib/adapters/openshell/forward-service.test.ts +++ b/src/lib/adapters/openshell/forward-service.test.ts @@ -1,14 +1,10 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { createServer, type AddressInfo } from "node:net"; - import { describe, expect, it, vi } from "vitest"; import { buildForwardServiceArgs, - forwardServiceInternals, - getForwardListenerOwnership, launchForwardService, type ForwardServiceTarget, } from "./forward-service"; @@ -23,7 +19,6 @@ const target: ForwardServiceTarget = { targetHost: "127.0.0.1", targetPort: 18_789, }; -const stableProcessIdentity = (pid: number): string => `start-${String(pid)}`; describe("OpenShell forward service", () => { it("builds the direct ForwardTcp command with explicit gateway authority", () => { @@ -50,16 +45,13 @@ describe("OpenShell forward service", () => { ); }); - it("detaches the OpenShell child and waits for its owned local listener (#11084)", () => { + it("detaches the OpenShell child and waits for its local port", () => { const unref = vi.fn(); - const spawnDetached = vi.fn(() => ({ pid: 41, unref })); - const isReachable = vi.fn().mockReturnValueOnce(false).mockReturnValueOnce(true); + const spawnDetached = vi.fn(() => ({ unref })); + let probes = 0; launchForwardService(target, { - getProcessIdentity: stableProcessIdentity, - isListenerOwned: () => true, - isProcessRunning: () => true, - isReachable, + isReachable: () => ++probes >= 3, sleep: () => {}, spawnDetached, timeoutMs: 1_000, @@ -71,22 +63,12 @@ describe("OpenShell forward service", () => { expect.any(Object), ); expect(unref).toHaveBeenCalledOnce(); - expect(isReachable).toHaveBeenCalledTimes(2); }); it("uses the selected OpenShell configuration without exposing credentials (#11084)", () => { - let launchedEnvironment: NodeJS.ProcessEnv | undefined; - const spawnDetached = vi.fn( - (_executable: string, _args: readonly string[], environment: NodeJS.ProcessEnv) => { - launchedEnvironment = environment; - return { pid: 42, unref: vi.fn() }; - }, - ); + const spawnDetached = vi.fn(() => ({ unref: vi.fn() })); launchForwardService(target, { - getProcessIdentity: stableProcessIdentity, - isListenerOwned: () => true, - isProcessRunning: () => true, isReachable: vi.fn().mockReturnValueOnce(false).mockReturnValueOnce(true), sleep: () => {}, sourceEnvironment: { @@ -98,12 +80,11 @@ describe("OpenShell forward service", () => { spawnDetached, }); - expect(launchedEnvironment).toMatchObject({ + expect(spawnDetached).toHaveBeenCalledWith(target.executable, buildForwardServiceArgs(target), { HOME: "/tmp/isolated-home", PATH: "/usr/bin", XDG_CONFIG_HOME: "/tmp/selected-openshell-config", }); - expect(launchedEnvironment).not.toHaveProperty("NVIDIA_INFERENCE_API_KEY"); }); it("refuses an occupied port without launching or adopting its listener", () => { @@ -115,518 +96,14 @@ describe("OpenShell forward service", () => { expect(spawnDetached).not.toHaveBeenCalled(); }); - it("does not accept a listener without proving the launched service owns it (#11084)", () => { - let running = true; - const stopProcess = vi.fn(() => { - running = false; - }); - - expect(() => - launchForwardService(target, { - getProcessIdentity: stableProcessIdentity, - isListenerOwned: () => false, - isProcessRunning: () => running, - isReachable: () => false, - sleep: () => {}, - spawnDetached: () => ({ pid: 42, unref: () => {} }), - stopProcess, - timeoutMs: 0, - }), - ).toThrow(/did not become ready|owned|identity|adopt/u); - expect(stopProcess).toHaveBeenCalledWith(42, "SIGTERM"); - }); - - it("accepts delayed ownership only after the exact child survives a health-check window (#11084)", () => { - let now = 0; - let ownershipChecks = 0; - vi.spyOn(Date, "now").mockImplementation(() => now); - - launchForwardService(target, { - getProcessIdentity: stableProcessIdentity, - isListenerOwned: () => ++ownershipChecks >= 3, - isProcessRunning: () => true, - isReachable: vi.fn().mockReturnValueOnce(false).mockReturnValueOnce(true), - sleep: (milliseconds) => { - now += milliseconds; - }, - spawnDetached: () => ({ pid: 43, unref: vi.fn() }), - timeoutMs: 10_000, - }); - - expect(ownershipChecks).toBe(24); - expect(now).toBe(2_300); - }); - - it("does not require every intermediate listener inspection to observe the owned socket (#11084)", () => { - let now = 0; - let ownershipChecks = 0; - vi.spyOn(Date, "now").mockImplementation(() => now); - - launchForwardService(target, { - getProcessIdentity: stableProcessIdentity, - isListenerOwned: () => ++ownershipChecks % 10 === 1, - isProcessRunning: () => true, - isReachable: vi.fn().mockReturnValueOnce(false).mockReturnValueOnce(true), - sleep: (milliseconds) => { - now += milliseconds; - }, - spawnDetached: () => ({ pid: 44, unref: vi.fn() }), - timeoutMs: 10_000, - }); - - expect(ownershipChecks).toBe(31); - expect(now).toBe(3_000); - }); - - it("does not accept an unowned listener at the completion observation (#11084)", () => { - let now = 0; - let running = true; - let ownershipChecks = 0; - const stopProcess = vi.fn(() => { - running = false; - }); - vi.spyOn(Date, "now").mockImplementation(() => now); - - expect(() => - launchForwardService(target, { - getProcessIdentity: stableProcessIdentity, - isListenerOwned: () => ++ownershipChecks === 1, - isProcessRunning: () => running, - isReachable: () => false, - sleep: (milliseconds) => { - now += milliseconds; - }, - spawnDetached: () => ({ pid: 45, unref: vi.fn() }), - stopProcess, - timeoutMs: 2_100, - }), - ).toThrow(/listener: absent/u); - expect(stopProcess).toHaveBeenCalledWith(45, "SIGTERM"); - }); - - it("does not accept an owned listener that still refuses connections (#11084)", () => { - let now = 0; - let running = true; - const isReachable = vi.fn(() => false); - const stopProcess = vi.fn(() => { - running = false; - }); - vi.spyOn(Date, "now").mockImplementation(() => now); - - expect(() => - launchForwardService(target, { - getProcessIdentity: stableProcessIdentity, - isListenerOwned: () => true, - isProcessRunning: () => running, - isReachable, - sleep: (milliseconds) => { - now += milliseconds; - }, - spawnDetached: () => ({ pid: 46, unref: vi.fn() }), - stopProcess, - timeoutMs: 2_100, - }), - ).toThrow(/listener: owned; reachability: refused/u); - expect(isReachable).toHaveBeenCalledTimes(3); - expect(stopProcess).toHaveBeenCalledWith(46, "SIGTERM"); - }); - - it("does not signal a process whose launch identity changed (#11084)", () => { - let identityChecks = 0; - const stopProcess = vi.fn(); - - expect(() => - launchForwardService(target, { - getProcessIdentity: () => (++identityChecks === 1 ? "original" : "replacement"), - isListenerOwned: () => false, - isProcessRunning: () => true, - isReachable: () => false, - sleep: () => {}, - spawnDetached: () => ({ pid: 44, unref: vi.fn() }), - stopProcess, - }), - ).toThrow(/changed identity.*refusing to signal or retry/u); - expect(stopProcess).not.toHaveBeenCalled(); - }); - - it("does not retry when an owned unready process cannot be stopped (#11084)", () => { - const spawnDetached = vi.fn(() => ({ pid: 45, unref: vi.fn() })); - const stopProcess = vi.fn(); - - expect(() => - launchForwardService(target, { - getProcessIdentity: stableProcessIdentity, - isListenerOwned: () => false, - isProcessRunning: () => true, - isReachable: () => false, - sleep: () => {}, - spawnDetached, - stopProcess, - stopTimeoutMs: 0, - timeoutMs: 0, - }), - ).toThrow(/could not be stopped.*refusing to retry/u); - expect(stopProcess.mock.calls).toEqual([ - [45, "SIGTERM"], - [45, "SIGKILL"], - ]); - expect(spawnDetached).toHaveBeenCalledOnce(); - }); - - it("fails closed when OpenShell returns no process identity (#11084)", () => { - const spawnDetached = vi.fn(() => ({ unref: vi.fn() })); - - expect(() => - launchForwardService(target, { - isReachable: () => false, - spawnDetached, - }), - ).toThrow(/no process identity.*refusing to start a duplicate service/u); - expect(spawnDetached).toHaveBeenCalledOnce(); - }); - - it("retries an exited service only for the exact sandbox creating handoff (#11084)", () => { - const diagnostic = - "Error:\n × sandbox 'demo' is no longer ready (phase: creating); stopping service\n ╰─▶ forward"; - const spawnDetached = vi - .fn() - .mockReturnValueOnce({ - pid: 51, - readOutput: () => diagnostic, - removeOutput: vi.fn(), - unref: vi.fn(), - }) - .mockReturnValueOnce({ pid: 52, removeOutput: vi.fn(), unref: vi.fn() }); - const onSandboxCreatingRetry = vi.fn(); - const sleep = vi.fn(); - - launchForwardService(target, { - getProcessIdentity: stableProcessIdentity, - isListenerOwned: (pid) => pid === 52, - isProcessRunning: (pid) => pid === 52, - isReachable: vi - .fn() - .mockReturnValueOnce(false) - .mockReturnValueOnce(false) - .mockReturnValueOnce(true), - maxSandboxCreatingRetries: 1, - onSandboxCreatingRetry, - sleep, - spawnDetached, - timeoutMs: 10_000, - }); - - expect(spawnDetached).toHaveBeenCalledTimes(2); - expect(onSandboxCreatingRetry).toHaveBeenCalledWith({ - attempt: 1, - delayMs: 2_000, - processId: 51, - remainingMs: expect.any(Number), - }); - expect(sleep).toHaveBeenCalledWith(2_000); - }); - - it("classifies a creating handoff that exits before its identity can be read (#11084)", () => { - const diagnostic = - "sandbox 'demo' is no longer ready (phase: creating); stopping service forward"; - const spawnDetached = vi - .fn() - .mockReturnValueOnce({ - pid: 53, - readOutput: () => diagnostic, - removeOutput: vi.fn(), - unref: vi.fn(), - }) - .mockReturnValueOnce({ pid: 54, removeOutput: vi.fn(), unref: vi.fn() }); - - launchForwardService(target, { - getProcessIdentity: (pid) => (pid === 53 ? null : stableProcessIdentity(pid)), - isListenerOwned: (pid) => pid === 54, - isProcessRunning: (pid) => pid === 54, - isReachable: vi - .fn() - .mockReturnValueOnce(false) - .mockReturnValueOnce(false) - .mockReturnValueOnce(true), - maxSandboxCreatingRetries: 1, - onSandboxCreatingRetry: () => {}, - sleep: () => {}, - spawnDetached, - timeoutMs: 10_000, - }); - - expect(spawnDetached).toHaveBeenCalledTimes(2); - }); - - it("waits for the bounded output drain before classifying an exited service (#11084)", () => { - const diagnostic = - "sandbox 'demo' is no longer ready (phase: creating); stopping service forward"; - let capturedOutput = ""; - const waitForOutput = vi.fn(() => { - capturedOutput = diagnostic; - return true; - }); - const spawnDetached = vi - .fn() - .mockReturnValueOnce({ - pid: 55, - readOutput: () => capturedOutput, - removeOutput: vi.fn(), - unref: vi.fn(), - waitForOutput, - }) - .mockReturnValueOnce({ pid: 56, removeOutput: vi.fn(), unref: vi.fn() }); - - launchForwardService(target, { - getProcessIdentity: stableProcessIdentity, - isListenerOwned: (pid) => pid === 56, - isProcessRunning: (pid) => pid === 56, - isReachable: vi - .fn() - .mockReturnValueOnce(false) - .mockReturnValueOnce(false) - .mockReturnValueOnce(true), - maxSandboxCreatingRetries: 1, - onSandboxCreatingRetry: () => {}, - sleep: () => {}, - spawnDetached, - timeoutMs: 10_000, - }); - - expect(waitForOutput).toHaveBeenCalledOnce(); - expect(spawnDetached).toHaveBeenCalledTimes(2); - }); - - it("does not retry when an exited service diagnostic cannot be completed (#11084)", () => { - const spawnDetached = vi.fn(() => ({ - pid: 57, - readOutput: () => - "sandbox 'demo' is no longer ready (phase: creating); stopping service forward", - removeOutput: vi.fn(), - unref: vi.fn(), - waitForOutput: () => false, - })); - - expect(() => - launchForwardService(target, { - getProcessIdentity: stableProcessIdentity, - isProcessRunning: () => false, - isReachable: () => false, - maxSandboxCreatingRetries: 1, - sleep: () => {}, - spawnDetached, - }), - ).toThrow(/diagnostic-incomplete/u); - expect(spawnDetached).toHaveBeenCalledOnce(); - }); - - it.each([ - [ - "terminal phase", - "sandbox 'demo' is no longer ready (phase: error); stopping service forward", - ], - [ - "different sandbox", - "sandbox 'another' is no longer ready (phase: creating); stopping service forward", - ], - ["missing sandbox", "sandbox 'demo' no longer exists; stopping service forward"], - ])("does not retry a terminal or unrelated start result [%s] (#11084)", (_case, diagnostic) => { - const spawnDetached = vi.fn(() => ({ - pid: 61, - readOutput: () => diagnostic, - removeOutput: vi.fn(), - unref: vi.fn(), - })); - - expect(() => - launchForwardService(target, { - getProcessIdentity: stableProcessIdentity, - isProcessRunning: () => false, - isReachable: () => false, - sleep: () => {}, - spawnDetached, - }), - ).toThrow(/non-readiness-diagnostic/u); - expect(spawnDetached).toHaveBeenCalledOnce(); - }); - - it("refuses an unknown listener that appears before a safe retry (#11084)", () => { - let probes = 0; - const spawnDetached = vi.fn(() => ({ - pid: 71, - readOutput: () => - "sandbox 'demo' is no longer ready (phase: creating); stopping service forward", - removeOutput: vi.fn(), - unref: vi.fn(), - })); - - expect(() => - launchForwardService(target, { - getProcessIdentity: stableProcessIdentity, - isProcessRunning: () => false, - isReachable: () => ++probes >= 2, - maxSandboxCreatingRetries: 1, - sleep: () => {}, - spawnDetached, - }), - ).toThrow(/became occupied.*refusing to adopt/u); - expect(spawnDetached).toHaveBeenCalledOnce(); - }); - - it("bounds repeated sandbox creating handoffs and records every attempt (#11084)", () => { - const spawnDetached = vi.fn(() => ({ - pid: 72, - readOutput: () => - "sandbox 'demo' is no longer ready (phase: creating); stopping service forward", - removeOutput: vi.fn(), - unref: vi.fn(), - })); - - expect(() => - launchForwardService(target, { - getProcessIdentity: stableProcessIdentity, - isProcessRunning: () => false, - isReachable: () => false, - maxSandboxCreatingRetries: 2, - onSandboxCreatingRetry: () => {}, - sleep: () => {}, - spawnDetached, - timeoutMs: 10_000, - }), - ).toThrow(/attempts: 1=pid-72:sandbox-creating:listener-absent:reachability-not-checked/u); - expect(spawnDetached).toHaveBeenCalledTimes(3); - }); - it("fails when the detached service does not bind before the deadline", () => { - let running = true; expect(() => launchForwardService(target, { - getProcessIdentity: stableProcessIdentity, - isListenerOwned: () => false, - isProcessRunning: () => running, isReachable: () => false, sleep: () => {}, - spawnDetached: () => ({ pid: 81, unref: () => {} }), - stopProcess: () => { - running = false; - }, + spawnDetached: () => ({ unref: () => {} }), timeoutMs: 0, }), - ).toThrow(/did not become ready/u); - }); - - it("classifies captured start output without exposing its contents (#11084)", () => { - let error: unknown; - const removeOutput = vi.fn(); - try { - launchForwardService(target, { - getProcessIdentity: stableProcessIdentity, - isProcessRunning: () => false, - isReachable: () => false, - sleep: () => {}, - spawnDetached: () => ({ - pid: 82, - readOutput: () => "terminal failure API_KEY=secret-value", - removeOutput, - unref: vi.fn(), - }), - }); - } catch (caught) { - error = caught; - } - - expect(error).toBeInstanceOf(Error); - expect((error as Error).message).toContain("non-readiness-diagnostic"); - expect((error as Error).message).not.toContain("secret-value"); - expect(removeOutput).toHaveBeenCalledOnce(); - }); - - it("records an exact OpenShell bind announcement without exposing raw output (#11084)", () => { - expect(() => - launchForwardService(target, { - getProcessIdentity: stableProcessIdentity, - isProcessRunning: () => false, - isReachable: () => false, - sleep: () => {}, - spawnDetached: () => ({ - pid: 83, - readOutput: () => - "✓ Forwarding 127.0.0.1:18789 -> 127.0.0.1:18789 in sandbox demo via gRPC", - removeOutput: vi.fn(), - unref: vi.fn(), - }), - }), - ).toThrow(/forwarding-announced/u); + ).toThrow(/did not bind/u); }); - - it.runIf(process.platform === "linux" || process.platform === "darwin")( - "bounds output retained by a long-running forward child (#11084)", - async () => { - const child = forwardServiceInternals.spawnForwardService( - process.execPath, - [ - "-e", - 'process.stdout.write("x".repeat(65536)); setInterval(() => process.stdout.write("later\\n"), 10);', - ], - process.env, - ); - try { - await vi.waitFor( - () => - expect(child.readOutput?.()).toHaveLength( - forwardServiceInternals.startOutputLimitBytes, - ), - { interval: 25, timeout: 2_000 }, - ); - await new Promise((resolve) => setTimeout(resolve, 100)); - expect(child.readOutput?.()).toHaveLength(forwardServiceInternals.startOutputLimitBytes); - } finally { - expect(child.pid).toBeTypeOf("number"); - try { - process.kill(child.pid as number, "SIGTERM"); - } catch { - // The fixture may already have stopped after a failed assertion. - } - expect(child.waitForOutput?.()).toBe(true); - child.removeOutput?.(); - expect(child.readOutput?.()).toBe(""); - } - }, - ); - - it.runIf(process.platform === "linux")( - "proves listener ownership from Linux procfs without connecting (#11084)", - async () => { - const server = createServer(); - await new Promise((resolve, reject) => { - server.once("error", reject); - server.listen(0, "127.0.0.1", resolve); - }); - try { - const address = server.address() as AddressInfo; - expect(getForwardListenerOwnership(process.pid, address.port)).toBe(true); - expect(getForwardListenerOwnership(process.ppid, address.port)).toBe(false); - } finally { - await new Promise((resolve) => server.close(() => resolve())); - } - }, - ); - - it.runIf(process.platform === "darwin")( - "proves listener ownership from macOS lsof without connecting (#11084)", - async () => { - const server = createServer(); - await new Promise((resolve, reject) => { - server.once("error", reject); - server.listen(0, "127.0.0.1", resolve); - }); - try { - const address = server.address() as AddressInfo; - expect(getForwardListenerOwnership(process.pid, address.port)).toBe(true); - expect(getForwardListenerOwnership(process.ppid, address.port)).toBe(false); - } finally { - await new Promise((resolve) => server.close(() => resolve())); - } - }, - ); }); diff --git a/src/lib/adapters/openshell/forward-service.ts b/src/lib/adapters/openshell/forward-service.ts index d504f44d7bf..8f78fe81983 100644 --- a/src/lib/adapters/openshell/forward-service.ts +++ b/src/lib/adapters/openshell/forward-service.ts @@ -1,45 +1,16 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { spawn, spawnSync } from "node:child_process"; -import { randomUUID } from "node:crypto"; -import fs from "node:fs"; -import os from "node:os"; +import { spawn } from "node:child_process"; import path from "node:path"; -import { stripVTControlCharacters } from "node:util"; import { isValidName } from "../../name-validation"; import { buildOpenShellSubprocessEnv } from "./resolve-shared"; import { probeLocalForwardListener } from "./local-forward-listener"; const START_TIMEOUT_MS = 30_000; -const SANDBOX_CREATING_RETRY_INTERVAL_MS = 2_000; -const SANDBOX_CREATING_MAX_RETRIES = START_TIMEOUT_MS / SANDBOX_CREATING_RETRY_INTERVAL_MS; -const STOP_TIMEOUT_MS = 5_000; const POLL_INTERVAL_MS = 100; -const START_OUTPUT_LIMIT_BYTES = 16 * 1_024; -// After the service PID exits, allow the local pipe drainer at most one second -// to publish its completion receipt. Missing completion is terminal, not retryable. -const START_OUTPUT_DRAIN_TIMEOUT_MS = 1_000; -// OpenShell 0.0.106 rechecks sandbox readiness every two seconds after it -// binds. Re-prove exact listener ownership after the next complete check. -const LISTENER_RECHECK_DELAY_MS = SANDBOX_CREATING_RETRY_INTERVAL_MS + POLL_INTERVAL_MS; -const FORWARD_INSTANCE_ENV = "NEMOCLAW_FORWARD_INSTANCE_ID"; const sleepBuffer = new Int32Array(new SharedArrayBuffer(4)); -const boundedOutputWrapper = [ - "capture_path=$1", - "capture_done_path=$2", - "shift 2", - `exec \"$@\" > >({ /usr/bin/head -c ${String(START_OUTPUT_LIMIT_BYTES)} > \"$capture_path\"; /bin/cat >/dev/null; : > \"$capture_done_path\"; }) 2>&1`, -].join("\n"); - -type ForwardServiceChild = { - readonly pid?: number; - readonly readOutput?: () => string; - readonly removeOutput?: () => void; - readonly waitForOutput?: () => boolean; - unref(): void; -}; export interface ForwardServiceTarget { readonly executable: string; @@ -53,370 +24,17 @@ export interface ForwardServiceTarget { } export interface ForwardServiceLaunchOptions { - readonly getProcessIdentity?: (pid: number) => string | null | undefined; - readonly isListenerOwned?: (pid: number, port: number) => boolean | null; - readonly isProcessRunning?: (pid: number) => boolean; readonly isReachable?: (port: number) => boolean; - readonly maxSandboxCreatingRetries?: number; - readonly onSandboxCreatingRetry?: (evidence: { - readonly attempt: number; - readonly delayMs: number; - readonly processId: number; - readonly remainingMs: number; - }) => void; readonly sleep?: (milliseconds: number) => void; readonly sourceEnvironment?: NodeJS.ProcessEnv; readonly spawnDetached?: ( executable: string, args: readonly string[], environment: NodeJS.ProcessEnv, - ) => ForwardServiceChild; - readonly stopProcess?: (pid: number, signal: NodeJS.Signals) => void; - readonly stopTimeoutMs?: number; - readonly timeoutMs?: number; -} - -function readLinuxProcessStat(pid: number): string[] | null | undefined { - if (process.platform !== "linux") return undefined; - try { - const stat = fs.readFileSync(`/proc/${String(pid)}/stat`, "utf8"); - return stat.slice(stat.lastIndexOf(")") + 2).split(" "); - } catch (error) { - const code = - typeof error === "object" && error !== null && "code" in error ? error.code : undefined; - return code === "ENOENT" || code === "ESRCH" ? null : undefined; - } -} - -function parseForwardInstanceIdentity(output: string): string | undefined { - const match = new RegExp( - `(?:^|\\s)${FORWARD_INSTANCE_ENV}=([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})(?:\\s|$)`, - "iu", - ).exec(output); - return match?.[1] ? `${process.platform}:${match[1].toLowerCase()}` : undefined; -} - -function getProcessIdentity(pid: number): string | null | undefined { - const stat = readLinuxProcessStat(pid); - const startTime = stat?.[19]; - if (startTime && /^\d+$/u.test(startTime)) return `linux:${startTime}`; - if (stat === null) return null; - if (process.platform === "linux") return undefined; - const result = spawnSync("ps", ["eww", "-p", String(pid), "-o", "command="], { - encoding: "utf8", - env: buildOpenShellSubprocessEnv(process.env), - stdio: ["ignore", "pipe", "ignore"], - timeout: 1_000, - }); - if (result.error) return undefined; - const identity = result.status === 0 ? parseForwardInstanceIdentity(result.stdout) : undefined; - if (identity) return identity; - return result.status === 1 ? null : undefined; -} - -function readLinuxListeningSocketInodes(port: number): Set | null { - const expectedPort = port.toString(16).toUpperCase().padStart(4, "0"); - const inodes = new Set(); - let readTable = false; - for (const tablePath of ["/proc/net/tcp", "/proc/net/tcp6"]) { - let table: string; - try { - table = fs.readFileSync(tablePath, "utf8"); - readTable = true; - } catch (error) { - const code = - typeof error === "object" && error !== null && "code" in error ? error.code : undefined; - if (code === "ENOENT") continue; - return null; - } - for (const line of table.split(/\r?\n/u).slice(1)) { - const fields = line.trim().split(/\s+/u); - const localAddress = fields[1]; - const state = fields[3]; - const inode = fields[9]; - if ( - localAddress?.endsWith(`:${expectedPort}`) && - state === "0A" && - inode !== undefined && - /^\d+$/u.test(inode) - ) { - inodes.add(inode); - } - } - } - return readTable ? inodes : null; -} - -function linuxProcessOwnsListener( - pid: number, - listenerInodes: ReadonlySet, -): boolean | null { - let descriptors: string[]; - try { - descriptors = fs.readdirSync(`/proc/${String(pid)}/fd`); - } catch (error) { - const code = - typeof error === "object" && error !== null && "code" in error ? error.code : undefined; - return code === "ENOENT" || code === "ESRCH" ? false : null; - } - let unreadableDescriptor = false; - for (const descriptor of descriptors) { - try { - const target = fs.readlinkSync(`/proc/${String(pid)}/fd/${descriptor}`); - const match = /^socket:\[(\d+)\]$/u.exec(target); - if (match?.[1] && listenerInodes.has(match[1])) return true; - } catch (error) { - const code = - typeof error === "object" && error !== null && "code" in error ? error.code : undefined; - if (code !== "ENOENT" && code !== "ESRCH") unreadableDescriptor = true; - } - } - return unreadableDescriptor ? null : false; -} - -type ForwardListenerObservation = "owned" | "absent" | "foreign" | "unavailable"; - -function observeLinuxForwardListener(pid: number, port: number): ForwardListenerObservation { - const listenerInodes = readLinuxListeningSocketInodes(port); - if (listenerInodes === null) return "unavailable"; - if (listenerInodes.size === 0) return "absent"; - const owned = linuxProcessOwnsListener(pid, listenerInodes); - if (owned === null) return "unavailable"; - return owned ? "owned" : "foreign"; -} - -/** Prove that a listener belongs to the exact child requested by this launch. */ -export function getForwardListenerOwnership(pid: number, port: number): boolean | null { - if (process.platform === "linux") { - const observation = observeLinuxForwardListener(pid, port); - return observation === "owned" ? true : observation === "unavailable" ? null : false; - } - const result = spawnSync( - "lsof", - ["-nP", "-a", "-p", String(pid), `-iTCP:${String(port)}`, "-sTCP:LISTEN", "-t"], - { - encoding: "utf8", - env: buildOpenShellSubprocessEnv(process.env), - stdio: ["ignore", "pipe", "ignore"], - timeout: 1_000, - }, - ); - if (result.error) return null; - const listenerPids = result.stdout - .split(/\r?\n/u) - .map((value) => value.trim()) - .filter(Boolean); - if (result.status === 0) return listenerPids.includes(String(pid)); - return result.status === 1 && listenerPids.length === 0 ? false : null; -} - -function isProcessRunning(pid: number): boolean { - try { - if (process.platform === "linux") { - const stat = readLinuxProcessStat(pid); - if (stat === null || stat?.[0] === "Z") return false; - } - process.kill(pid, 0); - return true; - } catch (error) { - const code = - typeof error === "object" && error !== null && "code" in error ? error.code : undefined; - return code !== "ENOENT" && code !== "ESRCH"; - } -} - -function spawnForwardService( - executable: string, - args: readonly string[], - environment: NodeJS.ProcessEnv, -): ForwardServiceChild { - const outputDirectory = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-forward-service-")); - fs.chmodSync(outputDirectory, 0o700); - const outputPath = path.join(outputDirectory, "start.log"); - const outputDonePath = path.join(outputDirectory, "start.done"); - const outputDescriptor = fs.openSync(outputPath, "wx", 0o600); - let child: ReturnType; - try { - // Bash exec preserves the launch PID while the process substitution keeps - // only a bounded startup prefix, drains all later output to /dev/null, and - // records when the pipe has closed so an exited service is classified only - // after its complete bounded diagnostic is available. - child = spawn( - "/bin/bash", - [ - "-c", - boundedOutputWrapper, - "nemoclaw-forward-service", - outputPath, - outputDonePath, - executable, - ...args, - ], - { - detached: true, - env: environment, - stdio: ["ignore", "ignore", "ignore"], - }, - ); - } catch (error) { - try { - fs.closeSync(outputDescriptor); - } catch { - // Preserve the spawn failure when closing the diagnostic file also fails. - } - try { - fs.rmSync(outputDirectory, { force: true, recursive: true }); - } catch { - // Preserve the spawn failure when removing the diagnostic file also fails. - } - throw error; - } - try { - fs.closeSync(outputDescriptor); - } catch { - // The drain wrapper reopens the path; the parent no longer needs this descriptor. - } - return { - pid: child.pid, - unref: () => child.unref(), - readOutput: () => { - try { - return fs.readFileSync(outputPath, "utf8"); - } catch { - return ""; - } - }, - waitForOutput: () => { - const deadline = Date.now() + START_OUTPUT_DRAIN_TIMEOUT_MS; - while (!fs.existsSync(outputDonePath)) { - const remainingMs = deadline - Date.now(); - if (remainingMs <= 0) return false; - Atomics.wait(sleepBuffer, 0, 0, Math.min(POLL_INTERVAL_MS, remainingMs)); - } - return true; - }, - removeOutput: () => { - try { - fs.rmSync(outputDirectory, { force: true, recursive: true }); - } catch { - // The detached child may retain its inherited descriptor briefly. - } - }, + ) => { + unref(): void; }; -} - -export const forwardServiceInternals = Object.freeze({ - spawnForwardService, - startOutputLimitBytes: START_OUTPUT_LIMIT_BYTES, -}); - -function compactOpenShellDiagnostic(output: string): string { - return stripVTControlCharacters(output) - .replace(/[^\p{L}\p{N}\s'"():;._-]+/gu, " ") - .replace(/\s+/gu, " ") - .trim(); -} - -function isSandboxCreatingHandoff(output: string, sandboxName: string): boolean { - const escapedName = sandboxName.replace(/[.*+?^${}()|[\]\\]/gu, "\\$&"); - return new RegExp( - `sandbox ["']${escapedName}["'] is no longer ready \\(phase: creating\\); stopping service forward`, - "iu", - ).test(compactOpenShellDiagnostic(output)); -} - -function isForwardingAnnounced(output: string, target: ForwardServiceTarget): boolean { - const expected = - `Forwarding ${target.localHost}:${String(target.localPort)} - ` + - `${target.targetHost}:${String(target.targetPort)} in sandbox ${target.sandboxName} via gRPC`; - return compactOpenShellDiagnostic(output).includes(expected); -} - -function classifyStartOutput( - child: ForwardServiceChild, - target: ForwardServiceTarget, -): { readonly category: string; readonly sandboxCreating: boolean } { - const outputComplete = child.waitForOutput?.() ?? true; - const output = child.readOutput?.() ?? ""; - const sandboxCreating = outputComplete && isSandboxCreatingHandoff(output, target.sandboxName); - return { - category: !outputComplete - ? "diagnostic-incomplete" - : sandboxCreating - ? "sandbox-creating" - : isForwardingAnnounced(output, target) - ? "forwarding-announced" - : output.trim() - ? "non-readiness-diagnostic" - : "empty-diagnostic", - sandboxCreating, - }; -} - -function isProcessId(pid: number | undefined): pid is number { - return Number.isSafeInteger(pid) && Number(pid) > 0; -} - -function processIdentityStatus( - pid: number, - expectedIdentity: string | null | undefined, - readIdentity: (pid: number) => string | null | undefined, -): "owned" | "exited" | "unverified" { - if (!expectedIdentity) return "unverified"; - const observedIdentity = readIdentity(pid); - if (observedIdentity === null) return "exited"; - return observedIdentity === expectedIdentity ? "owned" : "unverified"; -} - -function isMissingProcessError(error: unknown): boolean { - const code = - typeof error === "object" && error !== null && "code" in error ? error.code : undefined; - return code === "ENOENT" || code === "ESRCH"; -} - -function stopOwnedProcess(input: { - readonly expectedIdentity: string | null | undefined; - readonly isRunning: (pid: number) => boolean; - readonly pid: number; - readonly readIdentity: (pid: number) => string | null | undefined; - readonly sleep: (milliseconds: number) => void; - readonly stop: (pid: number, signal: NodeJS.Signals) => void; - readonly timeoutMs: number; -}): "stopped" | "running" | "unverified" { - if (!input.expectedIdentity) return "unverified"; - const waitForExit = (): "stopped" | "running" | "unverified" => { - const deadline = Date.now() + input.timeoutMs; - while (true) { - const identity = processIdentityStatus(input.pid, input.expectedIdentity, input.readIdentity); - if (identity === "exited" || !input.isRunning(input.pid)) return "stopped"; - if (identity === "unverified") return "unverified"; - if (Date.now() >= deadline) return "running"; - input.sleep(POLL_INTERVAL_MS); - } - }; - const initialIdentity = processIdentityStatus( - input.pid, - input.expectedIdentity, - input.readIdentity, - ); - if (initialIdentity === "exited") return "stopped"; - if (initialIdentity === "unverified") return "unverified"; - try { - input.stop(input.pid, "SIGTERM"); - } catch (error) { - return isMissingProcessError(error) ? "stopped" : "running"; - } - const terminated = waitForExit(); - if (terminated !== "running") return terminated; - if (processIdentityStatus(input.pid, input.expectedIdentity, input.readIdentity) !== "owned") { - return "unverified"; - } - try { - input.stop(input.pid, "SIGKILL"); - } catch (error) { - return isMissingProcessError(error) ? "stopped" : "running"; - } - return waitForExit(); + readonly timeoutMs?: number; } function isPort(value: unknown): value is number { @@ -476,202 +94,35 @@ export function buildForwardServiceArgs(target: ForwardServiceTarget): string[] ]; } -type ForwardAttemptResult = { - readonly category: string; - readonly listenerObservation: ForwardListenerObservation; - readonly processId: number; - readonly reachabilityObservation: "not-checked" | "refused"; - readonly sandboxCreating: boolean; -}; - -function startForwardServiceAttempt(input: { - readonly args: readonly string[]; - readonly environment: NodeJS.ProcessEnv; - readonly options: ForwardServiceLaunchOptions; - readonly readyDeadline: number; - readonly target: ForwardServiceTarget; -}): ForwardAttemptResult | null { - const readIdentity = input.options.getProcessIdentity ?? getProcessIdentity; - const observeListener = input.options.isListenerOwned - ? (pid: number, port: number): ForwardListenerObservation => { - const owned = input.options.isListenerOwned?.(pid, port); - return owned === true ? "owned" : owned === false ? "absent" : "unavailable"; - } - : process.platform === "linux" - ? observeLinuxForwardListener - : (pid: number, port: number): ForwardListenerObservation => { - const owned = getForwardListenerOwnership(pid, port); - return owned === true ? "owned" : owned === false ? "absent" : "unavailable"; - }; - const running = input.options.isProcessRunning ?? isProcessRunning; - const sleep = - input.options.sleep ?? - ((milliseconds: number) => Atomics.wait(sleepBuffer, 0, 0, milliseconds)); - const spawnDetached = input.options.spawnDetached ?? spawnForwardService; - const stop = - input.options.stopProcess ?? - ((pid: number, signal: NodeJS.Signals) => process.kill(pid, signal)); - const reachable = input.options.isReachable ?? probeLocalForwardListener; - const instanceId = randomUUID(); - const child = spawnDetached(input.target.executable, input.args, { - ...input.environment, - [FORWARD_INSTANCE_ENV]: instanceId, - }); - if (!isProcessId(child.pid)) { - const start = classifyStartOutput(child, input.target); - child.removeOutput?.(); - throw new Error( - `OpenShell forward service returned no process identity for ${input.target.localHost}:${String(input.target.localPort)}; refusing to start a duplicate service; forward start: ${start.category}`, - ); - } - const pid = child.pid; - const expectedIdentity = input.options.getProcessIdentity - ? readIdentity(pid) - : process.platform === "linux" - ? readIdentity(pid) - : `${process.platform}:${instanceId}`; - child.unref(); - let firstOwnedListenerAt: number | undefined; - let listenerObservation: ForwardListenerObservation = "absent"; - let reachabilityObservation: "not-checked" | "refused" = "not-checked"; - - while (true) { - const identity = processIdentityStatus(pid, expectedIdentity, readIdentity); - if (identity === "exited" || !running(pid)) { - const start = classifyStartOutput(child, input.target); - child.removeOutput?.(); - return { ...start, listenerObservation, processId: pid, reachabilityObservation }; - } - if (identity === "unverified") { - const start = classifyStartOutput(child, input.target); - child.removeOutput?.(); - throw new Error( - `OpenShell forward service process ${String(pid)} changed identity before binding ${input.target.localHost}:${String(input.target.localPort)}; refusing to signal or retry; forward start: ${start.category}`, - ); - } - listenerObservation = observeListener(pid, input.target.localPort); - const now = Date.now(); - if (listenerObservation === "owned") { - firstOwnedListenerAt ??= now; - if (now - firstOwnedListenerAt >= LISTENER_RECHECK_DELAY_MS) { - if (reachable(input.target.localPort)) { - child.removeOutput?.(); - return null; - } - reachabilityObservation = "refused"; - } - } - const observationDeadline = - firstOwnedListenerAt === undefined - ? input.readyDeadline - : Math.max(input.readyDeadline, firstOwnedListenerAt + LISTENER_RECHECK_DELAY_MS); - if (now >= observationDeadline) { - break; - } - sleep(POLL_INTERVAL_MS); - } - - const stopped = stopOwnedProcess({ - expectedIdentity, - isRunning: running, - pid, - readIdentity, - sleep, - stop, - timeoutMs: input.options.stopTimeoutMs ?? STOP_TIMEOUT_MS, - }); - const start = classifyStartOutput(child, input.target); - child.removeOutput?.(); - if (stopped !== "stopped") { - throw new Error( - `OpenShell forward service process ${String(pid)} did not become ready and ${stopped === "unverified" ? "could not be verified as owned" : "could not be stopped"}; refusing to retry; listener: ${listenerObservation}; forward start: ${start.category}`, - ); - } - if (reachable(input.target.localPort)) { - throw new Error( - `Host port ${String(input.target.localPort)} remained reachable after the launched process stopped; refusing to adopt its listener or retry; listener: ${listenerObservation}; forward start: ${start.category}`, - ); - } - throw new Error( - `OpenShell forward service did not become ready at ${input.target.localHost}:${String(input.target.localPort)}; listener: ${listenerObservation}; reachability: ${reachabilityObservation}; forward start: ${start.category}`, - ); -} - /** Launch one foreground OpenShell service forward as a detached host child. */ export function launchForwardService( target: ForwardServiceTarget, options: ForwardServiceLaunchOptions = {}, ): void { validateForwardServiceTarget(target); - const reachable = options.isReachable ?? probeLocalForwardListener; - if (reachable(target.localPort)) { + const isReachable = options.isReachable ?? probeLocalForwardListener; + if (isReachable(target.localPort)) { throw new Error(`Host port ${String(target.localPort)} is already occupied`); } + const spawnDetached = + options.spawnDetached ?? + ((executable, args, environment) => + spawn(executable, [...args], { detached: true, env: environment, stdio: "ignore" })); + const child = spawnDetached( + target.executable, + buildForwardServiceArgs(target), + buildOpenShellSubprocessEnv(options.sourceEnvironment ?? process.env), + ); + child.unref(); + const sleep = options.sleep ?? ((milliseconds: number) => Atomics.wait(sleepBuffer, 0, 0, milliseconds)); - const readyDeadline = Date.now() + (options.timeoutMs ?? START_TIMEOUT_MS); - const maxRetries = options.maxSandboxCreatingRetries ?? SANDBOX_CREATING_MAX_RETRIES; - if ( - !Number.isSafeInteger(maxRetries) || - maxRetries < 0 || - maxRetries > SANDBOX_CREATING_MAX_RETRIES - ) { - throw new Error( - `OpenShell sandbox creating retries must be between 0 and ${String(SANDBOX_CREATING_MAX_RETRIES)}`, - ); - } - const args = buildForwardServiceArgs(target); - const environment = buildOpenShellSubprocessEnv(options.sourceEnvironment ?? process.env); - const evidence: string[] = []; - let retries = 0; - - while (true) { - if (retries > 0 && Date.now() >= readyDeadline) { - throw new Error( - `OpenShell forward service readiness budget expired before retry; attempts: ${evidence.join(", ")}`, - ); - } - if (retries > 0 && reachable(target.localPort)) { - throw new Error( - `Host port ${String(target.localPort)} became occupied before forward retry; refusing to adopt its listener; attempts: ${evidence.join(", ")}`, - ); - } - const result = startForwardServiceAttempt({ - args, - environment, - options, - readyDeadline, - target, - }); - if (result === null) return; - const attempt = retries + 1; - evidence.push( - `${String(attempt)}=pid-${String(result.processId)}:${result.category}:listener-${result.listenerObservation}:reachability-${result.reachabilityObservation}`, - ); - const remainingMs = Math.max(0, readyDeadline - Date.now()); - if ( - !result.sandboxCreating || - retries >= maxRetries || - remainingMs < SANDBOX_CREATING_RETRY_INTERVAL_MS - ) { - throw new Error( - `OpenShell forward service exited before binding ${target.localHost}:${String(target.localPort)}; attempts: ${evidence.join(", ")}`, - ); - } - retries += 1; - const retryEvidence = { - attempt, - delayMs: SANDBOX_CREATING_RETRY_INTERVAL_MS, - processId: result.processId, - remainingMs, - }; - if (options.onSandboxCreatingRetry) { - options.onSandboxCreatingRetry(retryEvidence); - } else { - console.warn( - `OpenShell ForwardTcp ${String(target.localPort)} start attempt ${String(attempt)} (pid ${String(result.processId)}) observed sandbox '${target.sandboxName}' in phase creating; retrying in ${String(SANDBOX_CREATING_RETRY_INTERVAL_MS)}ms with ${String(remainingMs)}ms remaining.`, - ); - } - sleep(SANDBOX_CREATING_RETRY_INTERVAL_MS); + const deadline = Date.now() + (options.timeoutMs ?? START_TIMEOUT_MS); + while (Date.now() < deadline) { + if (isReachable(target.localPort)) return; + sleep(POLL_INTERVAL_MS); } + throw new Error( + `OpenShell forward service did not bind ${target.localHost}:${String(target.localPort)}`, + ); } diff --git a/src/lib/adapters/openshell/sanitized-capture.ts b/src/lib/adapters/openshell/sanitized-capture.ts index d3d5c9075cc..633ca2bd89a 100644 --- a/src/lib/adapters/openshell/sanitized-capture.ts +++ b/src/lib/adapters/openshell/sanitized-capture.ts @@ -23,7 +23,7 @@ export function captureSanitizedResolvedOpenshell( opts: SanitizedCaptureOptions, ): CapturedOpenShellCommandResult { const env = buildOpenShellSubprocessEnv(); - for (const name of ["OPENSHELL_WORKSPACE"] as const) { + for (const name of ["XDG_CONFIG_HOME", "OPENSHELL_WORKSPACE"] as const) { const value = process.env[name]; if (value !== undefined) env[name] = value; }