diff --git a/bin/openpi.js b/bin/openpi.js index 62d9d72a..5c5eeaf8 100755 --- a/bin/openpi.js +++ b/bin/openpi.js @@ -95,6 +95,20 @@ try { : {}), }); await host.start(); + const onStopSignal = () => { + void stop().then( + () => process.exit(0), + (error) => { + console.error( + `Failed to stop OpenPI Web Workbench: ${error instanceof Error ? error.message : String(error)}`, + ); + process.exit(1); + }, + ); + }; + for (const signal of ["SIGINT", "SIGTERM"]) { + process.once(signal, onStopSignal); + } traceWeb("web_started", { ...(runtime.workspaceSelected === true ? { cwd: runtime.cwd } : {}), origin: host.origin, @@ -112,20 +126,6 @@ try { console.log(`OpenPI Web Workbench is running at ${host.origin}`); if (!opened) console.log(`Open this URL in a browser: ${host.url}`); } - - for (const signal of ["SIGINT", "SIGTERM"]) { - process.once(signal, () => { - void stop().then( - () => process.exit(0), - (error) => { - console.error( - `Failed to stop OpenPI Web Workbench: ${error instanceof Error ? error.message : String(error)}`, - ); - process.exit(1); - }, - ); - }); - } } catch (error) { let cleanupError; try { diff --git a/tests/web/cli.test.ts b/tests/web/cli.test.ts index 845067e3..5e0dae65 100644 --- a/tests/web/cli.test.ts +++ b/tests/web/cli.test.ts @@ -1,6 +1,7 @@ import assert from "node:assert/strict"; import { execFile, spawn } from "node:child_process"; import { once } from "node:events"; +import { watch } from "node:fs"; import { cp, mkdir, @@ -10,12 +11,76 @@ import { stat, writeFile, } from "node:fs/promises"; -import { join } from "node:path"; +import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { promisify } from "node:util"; import test from "node:test"; const execFileAsync = promisify(execFile); + +async function inspectPath(path: string) { + try { + return { exists: true, content: await readFile(path, "utf8") }; + } catch (error) { + return { + exists: false, + error: + error instanceof Error && "code" in error + ? String(error.code) + : String(error), + }; + } +} + +// Same-dir write-then-rename makes the final name appear only after +// contents are complete. Directory watch can fire first for the staging +// name and miss the rename; also wake from the writer's stderr. +function waitForPublishedMarker( + path: string, + expected: string, + timeoutMs: number, + diagnostics: () => string, + wake?: NodeJS.ReadableStream, +) { + return new Promise((resolve, reject) => { + let settled = false; + const finish = (error?: Error) => { + if (settled) return; + settled = true; + clearTimeout(timer); + watcher.close(); + wake?.off("data", tryRead); + if (error) reject(error); + else resolve(); + }; + const tryRead = () => { + void readFile(path, "utf8").then( + (content) => { + if (content === expected) finish(); + }, + () => {}, + ); + }; + const timer = setTimeout(() => { + void Promise.all([inspectPath(path), inspectPath(`${path}.tmp`)]).then( + ([marker, staging]) => { + finish( + new Error( + `timed out waiting for ${path} contents; marker=${JSON.stringify(marker)} staging=${JSON.stringify(staging)}; ${diagnostics()}`, + ), + ); + }, + ); + }, timeoutMs); + const watcher = watch(dirname(path), tryRead); + watcher.on("error", (error) => { + finish(new Error(`watch error for ${path}: ${error}; ${diagnostics()}`)); + }); + wake?.on("data", tryRead); + tryRead(); + }); +} + const entrypoint = new URL("../../bin/openpi.js", import.meta.url); const entrypointPath = fileURLToPath(entrypoint); const staticAssetsPath = fileURLToPath( @@ -36,6 +101,21 @@ test("openpi is an executable standalone Web entrypoint", async () => { assert.match(stdout, /never enter an interactive terminal Pi session/u); }); +test("CLI registers stop signals before printing readiness", async () => { + const source = await readFile(entrypointPath, "utf8"); + const handlers = source.indexOf("process.once(signal, onStopSignal)"); + const workspaceReady = source.indexOf( + "OpenPI Web Workbench is running at ${host.origin}", + ); + const unboundReady = source.indexOf("formatWebReadyScreen("); + assert.notEqual(handlers, -1); + assert.notEqual(workspaceReady, -1); + assert.notEqual(unboundReady, -1); + assert.ok(handlers < workspaceReady); + assert.ok(handlers < unboundReady); + assert.equal(source.includes("process.on(signal, onStopSignal)"), false); +}); + test("installed CLI loads TypeScript Web modules through its package loader", async () => { const temporaryRoot = await mkdtemp(join(process.cwd(), ".openpi-cli-test-")); const packageRoot = join(temporaryRoot, "node_modules", "@tt-a1i", "openpi"); @@ -195,34 +275,44 @@ export class PiWebRuntime { child.stderr.setEncoding("utf8"); let signalOutput = ""; let signalError = ""; - child.stdout.on("data", (chunk) => { - signalOutput += chunk; - }); + const closed = once(child, "close") as Promise< + [number | null, NodeJS.Signals | null] + >; child.stderr.on("data", (chunk) => { signalError += chunk; }); - await new Promise((resolve, reject) => { + const started = new Promise((resolve, reject) => { const timeout = setTimeout( () => reject(new Error("installed CLI did not start")), 5_000, ); - const waitForReady = () => { + let sentStop = false; + const onStdout = (chunk: string) => { + signalOutput += chunk; if ( - signalOutput.includes( + sentStop || + !signalOutput.includes( "OpenPI Web Workbench is running at http://127.0.0.1:12345", ) ) { - clearTimeout(timeout); - resolve(); return; } - setTimeout(waitForReady, 10); + sentStop = true; + child.stdout.off("data", onStdout); + clearTimeout(timeout); + child.kill("SIGTERM"); + resolve(); }; - waitForReady(); + child.stdout.on("data", onStdout); }); - child.kill("SIGTERM"); - const [exitCode] = (await once(child, "close")) as [number | null]; - assert.equal(exitCode, 1); + await started; + const [exitCode, signal] = await closed; + assert.equal( + exitCode, + 1, + `expected process.exit(1) after stop failure, got code=${String(exitCode)} signal=${String(signal)} stderr=${signalError}`, + ); + assert.equal(signal, null); assert.match( signalError, /Failed to stop OpenPI Web Workbench: stop failed/u, @@ -232,3 +322,167 @@ export class PiWebRuntime { await rm(temporaryRoot, { recursive: true, force: true }); } }); + +test("a second SIGTERM uses default termination while stop is in flight", async () => { + if (process.platform === "win32") return; + const temporaryRoot = await mkdtemp(join(process.cwd(), ".openpi-cli-test-")); + const packageRoot = join(temporaryRoot, "node_modules", "@tt-a1i", "openpi"); + const stopMarker = join(temporaryRoot, "stop-entered"); + let child: ReturnType | undefined; + let output = ""; + let errorOutput = ""; + try { + await mkdir(join(packageRoot, "bin"), { recursive: true }); + await mkdir(join(packageRoot, "web", "host"), { recursive: true }); + await mkdir(join(packageRoot, "web", "runtime"), { recursive: true }); + await cp(entrypointPath, join(packageRoot, "bin", "openpi.js")); + await writeFile( + join(packageRoot, "package.json"), + JSON.stringify({ type: "module" }), + ); + await writeFile( + join(packageRoot, "web", "host", "browser-launcher.ts"), + "export async function openBrowser(): Promise { return false; }\n", + ); + await writeFile( + join(packageRoot, "web", "host", "terminal-status.ts"), + "export function formatWebReadyScreen(options: { origin: string; url: string }): string { return `ready ${options.origin} ${options.url}`; }\n", + ); + await writeFile( + join(packageRoot, "web", "host", "web-host.ts"), + `import { rename, writeFile } from "node:fs/promises"; + +export class WebHost { + origin = "http://127.0.0.1:12348"; + url = "http://127.0.0.1:12348/"; + hang: ReturnType | undefined; + async start(): Promise { + this.hang = setInterval(() => undefined, 60_000); + } + async stop(): Promise { + const marker = process.env.OPENPI_CLI_STOP_ENTERED_MARKER; + if (marker) { + const staging = \`\${marker}.tmp\`; + await writeFile(staging, "entered"); + await rename(staging, marker); + process.stderr.write("stop-entered\\n"); + } + this.hang ??= setInterval(() => undefined, 60_000); + await new Promise(() => {}); + } +}\n`, + ); + await writeFile( + join(packageRoot, "web", "trace.ts"), + "export function traceWeb(): void {}\n", + ); + await writeFile( + join(packageRoot, "web", "runtime", "pi-runtime.ts"), + `export class PiWebRuntime { + static async create(cwd: string): Promise<{ cwd: string; dispose(): Promise }> { + return { cwd, async dispose(): Promise {} }; + } +}\n`, + ); + + child = spawn( + process.execPath, + [ + join(packageRoot, "bin", "openpi.js"), + "web", + temporaryRoot, + "--no-open", + ], + { + env: { + ...process.env, + OPENPI_CLI_STOP_ENTERED_MARKER: stopMarker, + }, + stdio: ["ignore", "pipe", "pipe"], + }, + ); + const stdout = child.stdout; + const stderr = child.stderr; + assert.ok(stdout); + assert.ok(stderr); + stdout.setEncoding("utf8"); + stderr.setEncoding("utf8"); + stdout.on("data", (chunk) => { + output += chunk; + }); + stderr.on("data", (chunk) => { + errorOutput += chunk; + }); + let firstKill: boolean | undefined; + const diagnostics = () => + `firstKill=${String(firstKill)} code=${String(child?.exitCode)} signal=${String(child?.signalCode)} stdout=${output} stderr=${errorOutput}`; + const closed = once(child, "close") as Promise< + [number | null, NodeJS.Signals | null] + >; + await new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + reject(new Error(`installed CLI did not start; ${diagnostics()}`)); + }, 5_000); + let sentFirst = false; + const onStdout = () => { + if ( + sentFirst || + !output.includes( + "OpenPI Web Workbench is running at http://127.0.0.1:12348", + ) + ) { + return; + } + sentFirst = true; + stdout.off("data", onStdout); + clearTimeout(timeout); + firstKill = child?.kill("SIGTERM"); + resolve(); + }; + stdout.on("data", onStdout); + }); + assert.equal( + firstKill, + true, + `first SIGTERM not delivered; ${diagnostics()}`, + ); + await waitForPublishedMarker( + stopMarker, + "entered", + 5_000, + diagnostics, + stderr, + ); + assert.equal(await readFile(stopMarker, "utf8"), "entered"); + child.kill("SIGTERM"); + const [exitCode, signal] = await Promise.race([ + closed, + new Promise<[number | null, NodeJS.Signals | null]>((_, reject) => { + setTimeout(() => { + reject( + new Error( + `child did not exit after second SIGTERM; ${diagnostics()}`, + ), + ); + }, 5_000); + }), + ]); + assert.equal( + exitCode, + null, + `expected default SIGTERM, got code=${String(exitCode)} signal=${String(signal)} stdout=${output} stderr=${errorOutput}`, + ); + assert.equal(signal, "SIGTERM"); + } finally { + if (child && child.exitCode === null && child.signalCode === null) { + child.kill("SIGKILL"); + await Promise.race([ + once(child, "close"), + new Promise((resolve) => { + setTimeout(resolve, 1_000); + }), + ]); + } + await rm(temporaryRoot, { recursive: true, force: true }); + } +});