From 2e84333c4a78d9095e1d6bfbb64ce3e18d39c8f7 Mon Sep 17 00:00:00 2001 From: kartikloops Date: Wed, 24 Jun 2026 23:23:34 -0600 Subject: [PATCH 01/16] add EXIT_CODE_ERROR and EXIT_CODE_KEYBOARD_INTERRUPT so the entrypoint can map config and interrupt failures to distinct exit codes --- typescript/src/constants.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/typescript/src/constants.ts b/typescript/src/constants.ts index 8fb51eb..92d6fc1 100644 --- a/typescript/src/constants.ts +++ b/typescript/src/constants.ts @@ -1,3 +1,8 @@ +// Process exit codes for the CLI entrypoint. 130 follows the shell convention +// for a process terminated by SIGINT (128 + 2). +export const EXIT_CODE_ERROR = 1; +export const EXIT_CODE_KEYBOARD_INTERRUPT = 130; + export const MAX_COMMAND_COMPLETION_WINDOW = 0.1; export const PROCESS_SHUTDOWN_TIMEOUT = 2.0; From 0672acb9900d5b5e3777df59dd1dcd5976514bb0 Mon Sep 17 00:00:00 2001 From: kartikloops Date: Wed, 24 Jun 2026 23:24:16 -0600 Subject: [PATCH 02/16] add CleanupManager with SIGTERM/SIGINT handlers and an unref'd force-exit timer so a hung graceful shutdown still exits after FORCE_EXIT_DELAY_SECONDS --- typescript/src/utils/cleanup.ts | 77 +++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 typescript/src/utils/cleanup.ts diff --git a/typescript/src/utils/cleanup.ts b/typescript/src/utils/cleanup.ts new file mode 100644 index 0000000..aec7503 --- /dev/null +++ b/typescript/src/utils/cleanup.ts @@ -0,0 +1,77 @@ +import { FORCE_EXIT_DELAY_SECONDS } from "../constants.js"; +import { getLogger } from "./logging.js"; + +const logger = getLogger("cleanup"); + +type CleanupHandler = () => Promise | void; + +/** + * Coordinates graceful shutdown. Node is single-threaded and event-driven, so + * this collapses the Python atexit/signal/threading version into process signal + * handlers plus a single shutdown promise. + * + * `waitForShutdown()` blocks the server lifecycle until either a signal arrives + * or the transport closes (both call `triggerShutdown()`). A signal also arms an + * unref'd force-exit timer so a hung graceful shutdown still exits the process. + */ +export class CleanupManager { + private shutdownInitiated = false; + private readonly handlers: CleanupHandler[] = []; + + private resolveShutdown: (() => void) | null = null; + private readonly shutdownPromise: Promise = new Promise((resolve) => { + this.resolveShutdown = resolve; + }); + + /** Register a handler to run during graceful shutdown (sync or async). */ + registerAsyncCleanupHandler(handler: CleanupHandler): void { + this.handlers.push(handler); + } + + /** Install SIGTERM/SIGINT handlers that trigger shutdown and arm a force-exit. */ + setupSignalHandlers(): void { + const onSignal = (signal: NodeJS.Signals): void => { + if (this.shutdownInitiated) return; + logger.info( + `Received ${signal}, shutting down (forcing exit in ${FORCE_EXIT_DELAY_SECONDS}s if it hangs)`, + ); + // Force-exit safety net: if graceful shutdown stalls, leave anyway. The + // timer is unref'd so it never keeps the event loop alive on its own. + const timer = setTimeout(() => process.exit(0), FORCE_EXIT_DELAY_SECONDS * 1000); + timer.unref(); + this.triggerShutdown(); + }; + process.on("SIGTERM", onSignal); + process.on("SIGINT", onSignal); + } + + /** + * Unblock `waitForShutdown()`. Idempotent: a second signal or a transport + * close after the first is a no-op. Called by the signal handlers and by the + * stdio transport's onclose. + */ + triggerShutdown(): void { + if (this.shutdownInitiated) return; + this.shutdownInitiated = true; + this.resolveShutdown?.(); + } + + /** Resolves once shutdown is triggered. */ + async waitForShutdown(): Promise { + await this.shutdownPromise; + } + + /** Run every registered cleanup handler, isolating failures. */ + async runHandlers(): Promise { + for (const handler of this.handlers) { + try { + await handler(); + } catch (e) { + logger.error(`Error in cleanup handler: ${e instanceof Error ? e.message : String(e)}`); + } + } + } +} + +// Global cleanup manager instance (matches server.py's module-level singleton). +export const cleanupManager = new CleanupManager(); From b17e37130cafa95d6526844a2afbc79baa4c8eb4 Mon Sep 17 00:00:00 2001 From: kartikloops Date: Wed, 24 Jun 2026 23:25:41 -0600 Subject: [PATCH 03/16] add tests for CleanupManager covering handler dispatch, triggerShutdown idempotence, and the force-exit timer --- typescript/__tests__/utils/cleanup.test.ts | 97 ++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 typescript/__tests__/utils/cleanup.test.ts diff --git a/typescript/__tests__/utils/cleanup.test.ts b/typescript/__tests__/utils/cleanup.test.ts new file mode 100644 index 0000000..14be832 --- /dev/null +++ b/typescript/__tests__/utils/cleanup.test.ts @@ -0,0 +1,97 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { CleanupManager } from "../../src/utils/cleanup.js"; +import { FORCE_EXIT_DELAY_SECONDS } from "../../src/constants.js"; + +describe("CleanupManager", () => { + afterEach(() => { + vi.restoreAllMocks(); + vi.useRealTimers(); + }); + + it("runs registered handlers on runHandlers", async () => { + const mgr = new CleanupManager(); + const ran: string[] = []; + mgr.registerAsyncCleanupHandler(() => { + ran.push("a"); + }); + mgr.registerAsyncCleanupHandler(async () => { + await Promise.resolve(); + ran.push("b"); + }); + + await mgr.runHandlers(); + + expect(ran).toEqual(["a", "b"]); + }); + + it("isolates a throwing handler so later handlers still run", async () => { + const mgr = new CleanupManager(); + const ran: string[] = []; + mgr.registerAsyncCleanupHandler(() => { + throw new Error("boom"); + }); + mgr.registerAsyncCleanupHandler(() => { + ran.push("after"); + }); + + await expect(mgr.runHandlers()).resolves.toBeUndefined(); + expect(ran).toEqual(["after"]); + }); + + it("triggerShutdown resolves waitForShutdown", async () => { + const mgr = new CleanupManager(); + let resolved = false; + const wait = mgr.waitForShutdown().then(() => { + resolved = true; + }); + + mgr.triggerShutdown(); + await wait; + + expect(resolved).toBe(true); + }); + + it("triggerShutdown is idempotent", async () => { + const mgr = new CleanupManager(); + mgr.triggerShutdown(); + mgr.triggerShutdown(); // second call must be a no-op, not throw + await expect(mgr.waitForShutdown()).resolves.toBeUndefined(); + }); + + it("a signal triggers shutdown and arms the force-exit timer", async () => { + vi.useFakeTimers(); + const exitSpy = vi + .spyOn(process, "exit") + .mockImplementation((() => undefined) as never); + + const sigtermBefore = process.listeners("SIGTERM"); + const sigintBefore = process.listeners("SIGINT"); + + try { + const mgr = new CleanupManager(); + mgr.setupSignalHandlers(); + + let resolved = false; + const wait = mgr.waitForShutdown().then(() => { + resolved = true; + }); + + process.emit("SIGTERM", "SIGTERM"); + await wait; + expect(resolved).toBe(true); + + // Force-exit fires only after the configured delay. + expect(exitSpy).not.toHaveBeenCalled(); + vi.advanceTimersByTime(FORCE_EXIT_DELAY_SECONDS * 1000); + expect(exitSpy).toHaveBeenCalledWith(0); + } finally { + // Remove only the listeners this test added, leaving the runner's intact. + for (const l of process.listeners("SIGTERM")) { + if (!sigtermBefore.includes(l)) process.removeListener("SIGTERM", l); + } + for (const l of process.listeners("SIGINT")) { + if (!sigintBefore.includes(l)) process.removeListener("SIGINT", l); + } + } + }); +}); From b084ae881b9b4669af248b5e5a6bc0fa620c02e7 Mon Sep 17 00:00:00 2001 From: kartikloops Date: Wed, 24 Jun 2026 23:26:58 -0600 Subject: [PATCH 04/16] add commander-based CLI parsing that rejects --host and --port unless --transport is http --- typescript/src/config/cli.ts | 97 ++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 typescript/src/config/cli.ts diff --git a/typescript/src/config/cli.ts b/typescript/src/config/cli.ts new file mode 100644 index 0000000..2bc5a9e --- /dev/null +++ b/typescript/src/config/cli.ts @@ -0,0 +1,97 @@ +import { Command, Option } from "commander"; +import { ValidationError } from "../exceptions.js"; + +export interface TransportConfig { + mode: "stdio" | "http"; + host: string; + port: number; +} + +export interface CLIConfig { + transport: TransportConfig; + verbose: boolean; + logLevel: string; +} + +const DEFAULT_HOST = "localhost"; +const DEFAULT_PORT = 8000; + +function parsePort(value: string): number { + const port = Number.parseInt(value, 10); + if (Number.isNaN(port)) { + throw new ValidationError(`Invalid --port value: ${value}`); + } + return port; +} + +/** + * Parse argv into a CLIConfig. Pass an explicit argument array (without the + * node/script prefix) in tests; omit it to read process.argv. + * + * commander is configured with exitOverride so bad input throws a + * ValidationError instead of calling process.exit, which keeps parsing testable + * and lets main.ts map every config failure to a single exit code. + */ +export function parseCliArgs(argv?: string[]): CLIConfig { + const program = new Command(); + program + .name("openroad-mcp") + .description("OpenROAD Model Context Protocol (MCP) Server") + .addOption( + new Option("-t, --transport ", "Transport mode for the MCP server") + .choices(["stdio", "http"]) + .default("stdio"), + ) + .addOption( + new Option("--host ", "HTTP server host (http mode only)").default(DEFAULT_HOST), + ) + .addOption( + new Option("--port ", "HTTP server port (http mode only)") + .default(DEFAULT_PORT) + .argParser(parsePort), + ) + .option("-v, --verbose", "Enable verbose logging", false) + .addOption( + new Option("--log-level ", "Logging level") + .choices(["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"]) + .default("INFO"), + ) + .exitOverride((err) => { + // --help / --version already printed their output; exit cleanly rather + // than surfacing them as configuration errors. + if (err.code === "commander.helpDisplayed" || err.code === "commander.version") { + process.exit(0); + } + throw new ValidationError(err.message); + }); + + try { + if (argv === undefined) { + program.parse(); + } else { + program.parse(argv, { from: "user" }); + } + } catch (e) { + if (e instanceof ValidationError) throw e; + throw new ValidationError(e instanceof Error ? e.message : String(e)); + } + + const opts = program.opts(); + const mode = opts.transport as "stdio" | "http"; + const host = opts.host as string; + const port = opts.port as number; + + // HTTP host/port are meaningless for stdio; reject them so a misconfigured + // command fails loudly instead of silently ignoring the flags. + if (mode !== "http" && (host !== DEFAULT_HOST || port !== DEFAULT_PORT)) { + throw new ValidationError( + "--host and --port options are only valid with --transport http", + ); + } + + return { + transport: { mode, host, port }, + verbose: Boolean(opts.verbose), + logLevel: opts.logLevel as string, + }; +} From bf20dfbb70dc83155d49a0e5b74f4aa3511f7986 Mon Sep 17 00:00:00 2001 From: kartikloops Date: Wed, 24 Jun 2026 23:27:54 -0600 Subject: [PATCH 05/16] add tests for CLI parsing covering defaults, http overrides, and the host/port validation error --- typescript/__tests__/config/cli.test.ts | 60 +++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 typescript/__tests__/config/cli.test.ts diff --git a/typescript/__tests__/config/cli.test.ts b/typescript/__tests__/config/cli.test.ts new file mode 100644 index 0000000..ed1186e --- /dev/null +++ b/typescript/__tests__/config/cli.test.ts @@ -0,0 +1,60 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { parseCliArgs } from "../../src/config/cli.js"; +import { ValidationError } from "../../src/exceptions.js"; + +describe("parseCliArgs", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("returns stdio defaults with no args", () => { + expect(parseCliArgs([])).toEqual({ + transport: { mode: "stdio", host: "localhost", port: 8000 }, + verbose: false, + logLevel: "INFO", + }); + }); + + it("parses http transport with custom host and port", () => { + expect(parseCliArgs(["-t", "http", "--host", "0.0.0.0", "--port", "8080"])).toEqual({ + transport: { mode: "http", host: "0.0.0.0", port: 8080 }, + verbose: false, + logLevel: "INFO", + }); + }); + + it("parses verbose and log level", () => { + const config = parseCliArgs(["--verbose", "--log-level", "DEBUG"]); + expect(config.verbose).toBe(true); + expect(config.logLevel).toBe("DEBUG"); + }); + + it("rejects --host without http transport", () => { + expect(() => parseCliArgs(["--host", "0.0.0.0"])).toThrow(ValidationError); + expect(() => parseCliArgs(["--host", "0.0.0.0"])).toThrow( + "--host and --port options are only valid with --transport http", + ); + }); + + it("rejects --port without http transport", () => { + expect(() => parseCliArgs(["--port", "9000"])).toThrow( + "--host and --port options are only valid with --transport http", + ); + }); + + it("rejects an invalid transport choice", () => { + // commander prints the error to stderr before throwing; silence it. + vi.spyOn(process.stderr, "write").mockReturnValue(true); + expect(() => parseCliArgs(["--transport", "bogus"])).toThrow(ValidationError); + }); + + it("rejects an invalid log level choice", () => { + vi.spyOn(process.stderr, "write").mockReturnValue(true); + expect(() => parseCliArgs(["--log-level", "TRACE"])).toThrow(ValidationError); + }); + + it("rejects a non-numeric port", () => { + vi.spyOn(process.stderr, "write").mockReturnValue(true); + expect(() => parseCliArgs(["-t", "http", "--port", "abc"])).toThrow(ValidationError); + }); +}); From ea5e167aac183fe3de6570656346f04d557b78df Mon Sep 17 00:00:00 2001 From: kartikloops Date: Wed, 24 Jun 2026 23:31:25 -0600 Subject: [PATCH 06/16] add createMcpServer registering all 10 tools with zod schemas and annotations ported from server.py --- typescript/src/server.ts | 241 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 241 insertions(+) create mode 100644 typescript/src/server.ts diff --git a/typescript/src/server.ts b/typescript/src/server.ts new file mode 100644 index 0000000..073b415 --- /dev/null +++ b/typescript/src/server.ts @@ -0,0 +1,241 @@ +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { z } from "zod"; +import { manager as defaultManager } from "./core/manager.js"; +import type { OpenROADManager } from "./core/manager.js"; +import { + CreateSessionTool, + ExecShellTool, + InspectSessionTool, + ListSessionsTool, + QueryShellTool, + SessionHistoryTool, + SessionMetricsTool, + TerminateSessionTool, +} from "./tools/interactive.js"; +import { ListReportImagesTool, ReadReportImageTool } from "./tools/report_images.js"; + +const VERSION = "0.5.0"; + +/** Wrap a tool's JSON-string result in the MCP text-content envelope. */ +function text(value: string): { content: [{ type: "text"; text: string }] } { + return { content: [{ type: "text" as const, text: value }] }; +} + +/** + * Build an McpServer with all 10 tools registered. Accepts an optional manager + * so tests can inject an isolated/mocked one; defaults to the module singleton. + * + * Tool names, descriptions, input params, and annotations mirror the Python + * server.py verbatim so the wire contract is unchanged across the migration. + */ +export function createMcpServer(manager: OpenROADManager = defaultManager): McpServer { + const mcp = new McpServer({ name: "openroad-mcp", version: VERSION }); + + const queryTool = new QueryShellTool(manager); + const execTool = new ExecShellTool(manager); + const listSessionsTool = new ListSessionsTool(manager); + const createSessionTool = new CreateSessionTool(manager); + const terminateSessionTool = new TerminateSessionTool(manager); + const inspectSessionTool = new InspectSessionTool(manager); + const sessionHistoryTool = new SessionHistoryTool(manager); + const sessionMetricsTool = new SessionMetricsTool(manager); + const listReportImagesTool = new ListReportImagesTool(manager); + const readReportImageTool = new ReadReportImageTool(manager); + + mcp.registerTool( + "interactive_openroad_query", + { + description: + "Execute a read-only OpenROAD command (report_*, get_*, check_*, sta, help, etc.). " + + "Use this for querying design state, generating reports, and inspecting timing. " + + "Commands that modify design state are blocked — use interactive_openroad_exec instead.", + inputSchema: { + command: z.string(), + session_id: z.string().optional(), + timeout_ms: z.number().int().optional(), + }, + annotations: { + readOnlyHint: true, + destructiveHint: false, + idempotentHint: false, + openWorldHint: false, + }, + }, + async (args) => text(await queryTool.execute(args.command, args.session_id, args.timeout_ms)), + ); + + mcp.registerTool( + "interactive_openroad_exec", + { + description: + "Execute a state-modifying OpenROAD command (set_*, create_*, read_*, write_*, flow commands). " + + "Use this for loading designs, running placement/routing, applying constraints, and writing " + + "output files. Read-only commands are blocked — use interactive_openroad_query instead.", + inputSchema: { + command: z.string(), + session_id: z.string().optional(), + timeout_ms: z.number().int().optional(), + }, + annotations: { + readOnlyHint: false, + destructiveHint: true, + idempotentHint: false, + openWorldHint: false, + }, + }, + async (args) => text(await execTool.execute(args.command, args.session_id, args.timeout_ms)), + ); + + mcp.registerTool( + "list_interactive_sessions", + { + description: "List all active interactive OpenROAD sessions.", + annotations: { + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: false, + }, + }, + async () => text(await listSessionsTool.execute()), + ); + + mcp.registerTool( + "create_interactive_session", + { + description: "Create a new interactive OpenROAD session.", + inputSchema: { + session_id: z.string().optional(), + command: z.array(z.string()).optional(), + env: z.record(z.string(), z.string()).optional(), + cwd: z.string().optional(), + }, + annotations: { + readOnlyHint: false, + destructiveHint: false, + idempotentHint: false, + openWorldHint: false, + }, + }, + async (args) => + text(await createSessionTool.execute(args.session_id, args.command, args.env, args.cwd)), + ); + + mcp.registerTool( + "terminate_interactive_session", + { + description: "Terminate an interactive OpenROAD session.", + inputSchema: { + session_id: z.string(), + force: z.boolean().optional(), + }, + annotations: { + readOnlyHint: false, + destructiveHint: true, + idempotentHint: true, + openWorldHint: false, + }, + }, + async (args) => text(await terminateSessionTool.execute(args.session_id, args.force ?? false)), + ); + + mcp.registerTool( + "inspect_interactive_session", + { + description: "Get detailed inspection data for an interactive OpenROAD session.", + inputSchema: { session_id: z.string() }, + annotations: { + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: false, + }, + }, + async (args) => text(await inspectSessionTool.execute(args.session_id)), + ); + + mcp.registerTool( + "get_session_history", + { + description: "Get command history for an interactive OpenROAD session.", + inputSchema: { + session_id: z.string(), + limit: z.number().int().optional(), + search: z.string().optional(), + }, + annotations: { + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: false, + }, + }, + async (args) => + text(await sessionHistoryTool.execute(args.session_id, args.limit, args.search)), + ); + + mcp.registerTool( + "get_session_metrics", + { + description: "Get comprehensive metrics for all interactive OpenROAD sessions.", + annotations: { + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: false, + }, + }, + async () => text(await sessionMetricsTool.execute()), + ); + + mcp.registerTool( + "list_report_images", + { + description: "List available report images from ORFS runs organized by stage.", + inputSchema: { + platform: z.string(), + design: z.string(), + run_slug: z.string(), + stage: z.string().optional(), + }, + annotations: { + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: false, + }, + }, + async (args) => + text(await listReportImagesTool.execute(args.platform, args.design, args.run_slug, args.stage)), + ); + + mcp.registerTool( + "read_report_image", + { + description: "Read a report image and return base64-encoded data with metadata.", + inputSchema: { + platform: z.string(), + design: z.string(), + run_slug: z.string(), + image_name: z.string(), + }, + annotations: { + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: false, + }, + }, + async (args) => + text( + await readReportImageTool.execute( + args.platform, + args.design, + args.run_slug, + args.image_name, + ), + ), + ); + + return mcp; +} From fe47bf6837566f7882c44edbb01ad3d3b20835e5 Mon Sep 17 00:00:00 2001 From: kartikloops Date: Wed, 24 Jun 2026 23:38:20 -0600 Subject: [PATCH 07/16] add runServer with stdio and stateless http transports so the server shuts down on signal or transport close --- typescript/src/server.ts | 94 ++++++++++++++++++++++++++++++++++++++++ typescript/tsconfig.json | 6 ++- 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/typescript/src/server.ts b/typescript/src/server.ts index 073b415..535d157 100644 --- a/typescript/src/server.ts +++ b/typescript/src/server.ts @@ -1,7 +1,14 @@ +import { createServer } from "node:http"; +import type { IncomingMessage, ServerResponse } from "node:http"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; +import { StreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/streamableHttp.js"; import { z } from "zod"; +import type { CLIConfig } from "./config/cli.js"; import { manager as defaultManager } from "./core/manager.js"; import type { OpenROADManager } from "./core/manager.js"; +import { cleanupManager } from "./utils/cleanup.js"; +import { getLogger } from "./utils/logging.js"; import { CreateSessionTool, ExecShellTool, @@ -14,6 +21,8 @@ import { } from "./tools/interactive.js"; import { ListReportImagesTool, ReadReportImageTool } from "./tools/report_images.js"; +const logger = getLogger("server"); + const VERSION = "0.5.0"; /** Wrap a tool's JSON-string result in the MCP text-content envelope. */ @@ -239,3 +248,88 @@ export function createMcpServer(manager: OpenROADManager = defaultManager): McpS return mcp; } + +// Module-level server instance for the production entrypoint. Tests build their +// own isolated server via createMcpServer(). +export const mcp = createMcpServer(); + +/** Terminate every live session so shutdown does not leak OpenROAD processes. */ +export async function shutdownOpenroad(): Promise { + logger.info("Initiating graceful shutdown..."); + try { + await defaultManager.cleanupAll(); + logger.info("Graceful shutdown complete"); + } catch (e) { + logger.error(`Error during shutdown: ${e instanceof Error ? e.message : String(e)}`); + } +} + +/** Collect a request body and JSON-parse it, rejecting malformed payloads. */ +async function readJsonBody(req: IncomingMessage): Promise { + const chunks: Buffer[] = []; + for await (const chunk of req) { + chunks.push(chunk as Buffer); + } + const raw = Buffer.concat(chunks).toString("utf8"); + if (raw.length === 0) return undefined; + return JSON.parse(raw); +} + +/** + * Boot the MCP server for the configured transport and block until shutdown. + * + * stdio is the primary npx path. http uses a stateless streamable-HTTP + * transport — session continuity is provided by OpenROADManager keying on its + * own session_id, so no MCP-level session state is needed. Either way the + * lifecycle ends on a signal (SIGTERM/SIGINT) or transport close, after which + * every session is cleaned up. + */ +export async function runServer(config: CLIConfig): Promise { + cleanupManager.registerAsyncCleanupHandler(shutdownOpenroad); + cleanupManager.setupSignalHandlers(); + + try { + if (config.transport.mode === "stdio") { + // A client disconnect / stdin EOF closes the transport; treat that as a + // shutdown so the process does not hang waiting for a signal. + mcp.server.onclose = (): void => cleanupManager.triggerShutdown(); + const transport = new StdioServerTransport(); + await mcp.connect(transport); + logger.info("MCP server running on stdio transport"); + await cleanupManager.waitForShutdown(); + } else { + // Omitting sessionIdGenerator selects stateless mode (no MCP session + // tracking); OpenROADManager owns session continuity via its session_id. + const transport = new StreamableHTTPServerTransport(); + // The SDK's streamable-HTTP transport types its onclose as + // `(() => void) | undefined`, which trips exactOptionalPropertyTypes + // against the Transport interface; the runtime contract is unaffected. + await mcp.connect(transport as unknown as Parameters[0]); + + const httpServer = createServer((req: IncomingMessage, res: ServerResponse): void => { + void (async (): Promise => { + try { + const body = req.method === "POST" ? await readJsonBody(req) : undefined; + await transport.handleRequest(req, res, body); + } catch (e) { + logger.error(`HTTP request error: ${e instanceof Error ? e.message : String(e)}`); + if (!res.headersSent) { + res.writeHead(400, { "Content-Type": "application/json" }).end( + JSON.stringify({ error: "Invalid request body" }), + ); + } + } + })(); + }); + + httpServer.listen(config.transport.port, config.transport.host); + logger.info( + `MCP server running on http transport at ${config.transport.host}:${config.transport.port}`, + ); + await cleanupManager.waitForShutdown(); + await new Promise((resolve) => httpServer.close(() => { resolve(); })); + } + } finally { + await cleanupManager.runHandlers(); + } +} diff --git a/typescript/tsconfig.json b/typescript/tsconfig.json index 7dfd322..1f85c9b 100644 --- a/typescript/tsconfig.json +++ b/typescript/tsconfig.json @@ -37,7 +37,11 @@ "verbatimModuleSyntax": true, "isolatedModules": true, "moduleDetection": "force", - "skipLibCheck": false + // The @modelcontextprotocol/sdk streamable-HTTP transport's .d.ts does not + // satisfy exactOptionalPropertyTypes (its class onclose is typed as + // `(() => void) | undefined`), so lib checking must be skipped to consume + // it. Our own source is still fully type-checked. + "skipLibCheck": true }, "include": ["src/**/*"], "exclude": ["node_modules", "dist", "__tests__"] From 587dd30fd8289b66759def3f3bfb0320bda4bcc3 Mon Sep 17 00:00:00 2001 From: kartikloops Date: Wed, 24 Jun 2026 23:40:25 -0600 Subject: [PATCH 08/16] add a smoke test booting the stdio server in-memory so all 10 tools are asserted to enumerate with correct annotations --- typescript/__tests__/server.test.ts | 82 +++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 typescript/__tests__/server.test.ts diff --git a/typescript/__tests__/server.test.ts b/typescript/__tests__/server.test.ts new file mode 100644 index 0000000..9d80f5a --- /dev/null +++ b/typescript/__tests__/server.test.ts @@ -0,0 +1,82 @@ +import { describe, it, expect, vi } from "vitest"; +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; +import { createMcpServer } from "../src/server.js"; +import type { OpenROADManager } from "../src/core/manager.js"; + +// node-pty must never spawn during a boot/list-tools smoke test. +vi.mock("node-pty", () => ({ spawn: vi.fn() })); + +const EXPECTED_TOOLS = [ + "interactive_openroad_query", + "interactive_openroad_exec", + "list_interactive_sessions", + "create_interactive_session", + "terminate_interactive_session", + "inspect_interactive_session", + "get_session_history", + "get_session_metrics", + "list_report_images", + "read_report_image", +].sort(); + +/** Minimal manager stub: listing tools needs no calls; one round-trip uses listSessions. */ +function makeMockManager(): OpenROADManager { + return { + listSessions: vi.fn().mockResolvedValue([]), + } as unknown as OpenROADManager; +} + +async function connectClient(manager: OpenROADManager): Promise { + const server = createMcpServer(manager); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + const client = new Client({ name: "test-client", version: "0.0.0" }); + await client.connect(clientTransport); + return client; +} + +describe("createMcpServer over MCP", () => { + it("enumerates exactly the 10 expected tools", async () => { + const client = await connectClient(makeMockManager()); + const { tools } = await client.listTools(); + + expect(tools.map((t) => t.name).sort()).toEqual(EXPECTED_TOOLS); + await client.close(); + }); + + it("carries the correct behaviour annotations", async () => { + const client = await connectClient(makeMockManager()); + const { tools } = await client.listTools(); + const byName = new Map(tools.map((t) => [t.name, t])); + + expect(byName.get("interactive_openroad_query")?.annotations).toMatchObject({ + readOnlyHint: true, + destructiveHint: false, + }); + expect(byName.get("interactive_openroad_exec")?.annotations).toMatchObject({ + readOnlyHint: false, + destructiveHint: true, + }); + expect(byName.get("list_interactive_sessions")?.annotations).toMatchObject({ + readOnlyHint: true, + idempotentHint: true, + }); + await client.close(); + }); + + it("round-trips a tool call returning a JSON string in text content", async () => { + const manager = makeMockManager(); + const client = await connectClient(manager); + + const result = await client.callTool({ name: "list_interactive_sessions" }); + const content = result.content as Array<{ type: string; text: string }>; + + expect(content[0]?.type).toBe("text"); + const parsed = JSON.parse(content[0]!.text) as { sessions: unknown[]; total_count: number }; + expect(parsed.sessions).toEqual([]); + expect(parsed.total_count).toBe(0); + expect(manager.listSessions).toHaveBeenCalledOnce(); + await client.close(); + }); +}); From 829e42b51eeb780b27e9e6503d5e1540e8cb4314 Mon Sep 17 00:00:00 2001 From: kartikloops Date: Wed, 24 Jun 2026 23:44:56 -0600 Subject: [PATCH 09/16] replace the main.ts stub with the full entrypoint, dynamically importing logging and server so settings validate before any logger is built and the CLI log level applies first --- typescript/src/main.ts | 46 +++++++++++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/typescript/src/main.ts b/typescript/src/main.ts index 697dd4e..c49242b 100644 --- a/typescript/src/main.ts +++ b/typescript/src/main.ts @@ -1,10 +1,42 @@ +#!/usr/bin/env node +import { parseCliArgs } from "./config/cli.js"; import { initSettings } from "./config/settings.js"; +import { EXIT_CODE_ERROR } from "./constants.js"; +import { ValidationError } from "./exceptions.js"; -// Initialise settings up front so a misconfigured env var fails fast with -// context rather than crashing later from inside module initialisation. -try { - initSettings(); -} catch (e) { - console.error(e instanceof Error ? e.message : String(e)); - process.exit(1); +/** + * Entry point. The eager work (settings validation, CLI parsing) runs before + * any module that reads settings or builds a logger is imported. + * + * This ordering matters and is why `logging` and `server` are dynamic imports: + * `utils/logging` calls `getSettings()` at module load to seed the root logger, + * and `server` builds the manager (and its child logger) at load. Importing + * either statically would validate settings before main()'s try/catch (turning + * a bad env var into an uncaught stack trace) and create loggers before the CLI + * log level is applied. Only pure modules are imported statically here. + */ +async function main(): Promise { + // Fail fast on a misconfigured env var; surface it as a configuration error. + try { + initSettings(); + } catch (e) { + throw new ValidationError(e instanceof Error ? e.message : String(e)); + } + + const config = parseCliArgs(); + + const { setupLogging } = await import("./utils/logging.js"); + setupLogging(config.verbose ? "DEBUG" : config.logLevel); + + const { runServer } = await import("./server.js"); + await runServer(config); } + +main().catch((e: unknown) => { + if (e instanceof ValidationError) { + console.error(`Configuration error: ${e.message}`); + process.exit(EXIT_CODE_ERROR); + } + console.error(`Unexpected error: ${e instanceof Error ? e.message : String(e)}`); + process.exit(EXIT_CODE_ERROR); +}); From ea9888c97d3a8ac81520bad8129f2d0cf900dde6 Mon Sep 17 00:00:00 2001 From: kartikloops Date: Wed, 24 Jun 2026 23:58:56 -0600 Subject: [PATCH 10/16] create a fresh streamable-http transport and server per request so concurrent stateless clients do not collide on JSON-RPC request ids --- typescript/src/server.ts | 56 +++++++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/typescript/src/server.ts b/typescript/src/server.ts index 535d157..2715bcb 100644 --- a/typescript/src/server.ts +++ b/typescript/src/server.ts @@ -275,6 +275,40 @@ async function readJsonBody(req: IncomingMessage): Promise { return JSON.parse(raw); } +/** + * Handle one HTTP request in stateless mode. The SDK forbids reusing a + * streamable-HTTP transport across requests — a shared transport keys its + * request→stream map by JSON-RPC id, so two clients both numbering from 1 would + * collide. A fresh server + transport per request keeps clients isolated; both + * are torn down when the response closes. Session continuity is unaffected + * because OpenROADManager owns it via its own session_id, independent of MCP. + */ +async function handleHttpRequest(req: IncomingMessage, res: ServerResponse): Promise { + const requestServer = createMcpServer(); + const transport = new StreamableHTTPServerTransport(); + res.on("close", () => { + void transport.close(); + void requestServer.close(); + }); + try { + // The SDK's streamable-HTTP transport types its onclose as + // `(() => void) | undefined`, which trips exactOptionalPropertyTypes against + // the Transport interface; the runtime contract is unaffected. + await requestServer.connect( + transport as unknown as Parameters[0], + ); + const body = req.method === "POST" ? await readJsonBody(req) : undefined; + await transport.handleRequest(req, res, body); + } catch (e) { + logger.error(`HTTP request error: ${e instanceof Error ? e.message : String(e)}`); + if (!res.headersSent) { + res.writeHead(400, { "Content-Type": "application/json" }).end( + JSON.stringify({ error: "Invalid request body" }), + ); + } + } +} + /** * Boot the MCP server for the configured transport and block until shutdown. * @@ -298,28 +332,8 @@ export async function runServer(config: CLIConfig): Promise { logger.info("MCP server running on stdio transport"); await cleanupManager.waitForShutdown(); } else { - // Omitting sessionIdGenerator selects stateless mode (no MCP session - // tracking); OpenROADManager owns session continuity via its session_id. - const transport = new StreamableHTTPServerTransport(); - // The SDK's streamable-HTTP transport types its onclose as - // `(() => void) | undefined`, which trips exactOptionalPropertyTypes - // against the Transport interface; the runtime contract is unaffected. - await mcp.connect(transport as unknown as Parameters[0]); - const httpServer = createServer((req: IncomingMessage, res: ServerResponse): void => { - void (async (): Promise => { - try { - const body = req.method === "POST" ? await readJsonBody(req) : undefined; - await transport.handleRequest(req, res, body); - } catch (e) { - logger.error(`HTTP request error: ${e instanceof Error ? e.message : String(e)}`); - if (!res.headersSent) { - res.writeHead(400, { "Content-Type": "application/json" }).end( - JSON.stringify({ error: "Invalid request body" }), - ); - } - } - })(); + void handleHttpRequest(req, res); }); httpServer.listen(config.transport.port, config.transport.host); From 6a04c1f47eac6e11fa9874ba3c9e3d65d788549a Mon Sep 17 00:00:00 2001 From: kartikloops Date: Thu, 25 Jun 2026 00:08:20 -0600 Subject: [PATCH 11/16] cleanup --- typescript/src/main.ts | 1 - typescript/src/server.ts | 21 +++++++-------------- typescript/src/utils/cleanup.ts | 4 ---- 3 files changed, 7 insertions(+), 19 deletions(-) diff --git a/typescript/src/main.ts b/typescript/src/main.ts index c49242b..b8cc382 100644 --- a/typescript/src/main.ts +++ b/typescript/src/main.ts @@ -16,7 +16,6 @@ import { ValidationError } from "./exceptions.js"; * log level is applied. Only pure modules are imported statically here. */ async function main(): Promise { - // Fail fast on a misconfigured env var; surface it as a configuration error. try { initSettings(); } catch (e) { diff --git a/typescript/src/server.ts b/typescript/src/server.ts index 2715bcb..2330dee 100644 --- a/typescript/src/server.ts +++ b/typescript/src/server.ts @@ -25,7 +25,6 @@ const logger = getLogger("server"); const VERSION = "0.5.0"; -/** Wrap a tool's JSON-string result in the MCP text-content envelope. */ function text(value: string): { content: [{ type: "text"; text: string }] } { return { content: [{ type: "text" as const, text: value }] }; } @@ -253,7 +252,6 @@ export function createMcpServer(manager: OpenROADManager = defaultManager): McpS // own isolated server via createMcpServer(). export const mcp = createMcpServer(); -/** Terminate every live session so shutdown does not leak OpenROAD processes. */ export async function shutdownOpenroad(): Promise { logger.info("Initiating graceful shutdown..."); try { @@ -264,7 +262,6 @@ export async function shutdownOpenroad(): Promise { } } -/** Collect a request body and JSON-parse it, rejecting malformed payloads. */ async function readJsonBody(req: IncomingMessage): Promise { const chunks: Buffer[] = []; for await (const chunk of req) { @@ -277,11 +274,11 @@ async function readJsonBody(req: IncomingMessage): Promise { /** * Handle one HTTP request in stateless mode. The SDK forbids reusing a - * streamable-HTTP transport across requests — a shared transport keys its - * request→stream map by JSON-RPC id, so two clients both numbering from 1 would - * collide. A fresh server + transport per request keeps clients isolated; both - * are torn down when the response closes. Session continuity is unaffected - * because OpenROADManager owns it via its own session_id, independent of MCP. + * streamable-HTTP transport across requests: a shared transport keys its + * request-to-stream map by JSON-RPC id, so two clients both numbering from 1 + * would collide. A fresh server + transport per request keeps clients isolated; + * both are torn down when the response closes. OpenROADManager owns session + * continuity via its own session_id, independent of MCP. */ async function handleHttpRequest(req: IncomingMessage, res: ServerResponse): Promise { const requestServer = createMcpServer(); @@ -311,12 +308,8 @@ async function handleHttpRequest(req: IncomingMessage, res: ServerResponse): Pro /** * Boot the MCP server for the configured transport and block until shutdown. - * - * stdio is the primary npx path. http uses a stateless streamable-HTTP - * transport — session continuity is provided by OpenROADManager keying on its - * own session_id, so no MCP-level session state is needed. Either way the - * lifecycle ends on a signal (SIGTERM/SIGINT) or transport close, after which - * every session is cleaned up. + * Lifecycle ends on SIGTERM/SIGINT or transport close, then every session is + * cleaned up. */ export async function runServer(config: CLIConfig): Promise { cleanupManager.registerAsyncCleanupHandler(shutdownOpenroad); diff --git a/typescript/src/utils/cleanup.ts b/typescript/src/utils/cleanup.ts index aec7503..969d5f9 100644 --- a/typescript/src/utils/cleanup.ts +++ b/typescript/src/utils/cleanup.ts @@ -23,12 +23,10 @@ export class CleanupManager { this.resolveShutdown = resolve; }); - /** Register a handler to run during graceful shutdown (sync or async). */ registerAsyncCleanupHandler(handler: CleanupHandler): void { this.handlers.push(handler); } - /** Install SIGTERM/SIGINT handlers that trigger shutdown and arm a force-exit. */ setupSignalHandlers(): void { const onSignal = (signal: NodeJS.Signals): void => { if (this.shutdownInitiated) return; @@ -56,12 +54,10 @@ export class CleanupManager { this.resolveShutdown?.(); } - /** Resolves once shutdown is triggered. */ async waitForShutdown(): Promise { await this.shutdownPromise; } - /** Run every registered cleanup handler, isolating failures. */ async runHandlers(): Promise { for (const handler of this.handlers) { try { From eae22608cddbce73ecd2a96def1265a8008ee211 Mon Sep 17 00:00:00 2001 From: kartikloops Date: Thu, 25 Jun 2026 21:32:23 -0600 Subject: [PATCH 12/16] fixed pty failure --- .../__tests__/interactive/pty_handler.test.ts | 12 +++++++++ typescript/package.json | 1 + typescript/scripts/fix-node-pty.cjs | 27 +++++++++++++++++++ typescript/src/interactive/pty_handler.ts | 11 ++++++++ 4 files changed, 51 insertions(+) create mode 100644 typescript/scripts/fix-node-pty.cjs diff --git a/typescript/__tests__/interactive/pty_handler.test.ts b/typescript/__tests__/interactive/pty_handler.test.ts index 93f4aae..99b60b3 100644 --- a/typescript/__tests__/interactive/pty_handler.test.ts +++ b/typescript/__tests__/interactive/pty_handler.test.ts @@ -106,6 +106,18 @@ describe("PtyHandler", () => { await expect(handler.createSession(["echo"])).rejects.toThrow("Failed to create PTY session"); }); + it("includes PATH hint when spawn fails with posix_spawnp/ENOENT", async () => { + vi.mocked(spawn).mockImplementation(() => { + throw new Error("posix_spawnp failed"); + }); + + await expect(handler.createSession(["openroad"])).rejects.toThrow( + "could not be started", + ); + await expect(handler.createSession(["openroad"])).rejects.toThrow("PATH="); + await expect(handler.createSession(["openroad"])).rejects.toThrow("spawn-helper"); + }); + it("registers onExit and marks process dead when it fires", async () => { await handler.createSession(["echo"]); expect(handler.isProcessAlive()).toBe(true); diff --git a/typescript/package.json b/typescript/package.json index cccf28f..ea612bb 100644 --- a/typescript/package.json +++ b/typescript/package.json @@ -13,6 +13,7 @@ "dist/" ], "scripts": { + "postinstall": "node scripts/fix-node-pty.cjs", "build": "tsc --project tsconfig.json", "typecheck": "tsc --noEmit && tsc -p tsconfig.test.json", "lint": "eslint .", diff --git a/typescript/scripts/fix-node-pty.cjs b/typescript/scripts/fix-node-pty.cjs new file mode 100644 index 0000000..b510e1f --- /dev/null +++ b/typescript/scripts/fix-node-pty.cjs @@ -0,0 +1,27 @@ +/** + * node-pty ships platform prebuilds with a spawn-helper binary that must be + * executable. Some npm installs (sandboxed CI, strict umask, package managers) + * leave it mode 0644, which makes posix_spawnp fail for every PTY session. + */ +const fs = require("node:fs"); +const path = require("node:path"); + +const prebuildsDir = path.join(__dirname, "..", "node_modules", "node-pty", "prebuilds"); + +if (!fs.existsSync(prebuildsDir)) { + process.exit(0); +} + +for (const entry of fs.readdirSync(prebuildsDir, { withFileTypes: true })) { + if (!entry.isDirectory()) continue; + const helper = path.join(prebuildsDir, entry.name, "spawn-helper"); + if (!fs.existsSync(helper)) continue; + try { + const mode = fs.statSync(helper).mode & 0o777; + if ((mode & 0o111) === 0) { + fs.chmodSync(helper, mode | 0o755); + } + } catch { + // Best effort; PTY spawn will surface a clearer error at runtime. + } +} diff --git a/typescript/src/interactive/pty_handler.ts b/typescript/src/interactive/pty_handler.ts index c3adb28..c316873 100644 --- a/typescript/src/interactive/pty_handler.ts +++ b/typescript/src/interactive/pty_handler.ts @@ -64,6 +64,7 @@ export class PtyHandler { onData?: (data: string) => void, onExit?: (exitCode: number) => void, ): Promise { + const executable = command[0] ?? ""; try { this.validateCommand(command); @@ -105,6 +106,16 @@ export class PtyHandler { } } catch (e) { if (e instanceof PTYError) throw e; + const raw = e instanceof Error ? e.message : String(e); + const pathValue = process.env.PATH ?? ""; + if (/posix_spawnp failed|ENOENT|command not found/i.test(raw)) { + throw new PTYError( + `Failed to create PTY session: executable '${executable}' could not be started. ` + + `Common causes: (1) '${executable}' is missing from PATH or not executable ` + + `(PATH=${JSON.stringify(pathValue)}), or (2) node-pty's spawn-helper binary is not ` + + `executable — run: chmod +x node_modules/node-pty/prebuilds/*/spawn-helper`, + ); + } throw new PTYError(`Failed to create PTY session: ${e}`); } } From c6ddf6c946b7cd864730d6ba5e213a58c59fb3fd Mon Sep 17 00:00:00 2001 From: kartikloops Date: Thu, 2 Jul 2026 18:08:03 -0600 Subject: [PATCH 13/16] reject cleanly when httpServer.listen fails so a bind error (port in use, permission denied) surfaces as an error exit instead of an uncaught crash, and log post-bind runtime errors into graceful shutdown --- typescript/src/server.ts | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/typescript/src/server.ts b/typescript/src/server.ts index 2330dee..37f786f 100644 --- a/typescript/src/server.ts +++ b/typescript/src/server.ts @@ -329,10 +329,28 @@ export async function runServer(config: CLIConfig): Promise { void handleHttpRequest(req, res); }); - httpServer.listen(config.transport.port, config.transport.host); - logger.info( - `MCP server running on http transport at ${config.transport.host}:${config.transport.port}`, - ); + const { host, port } = config.transport; + // Bind can fail (port in use, permission denied); surface it as a clean + // rejection instead of an uncaught 'error' event that crashes the process. + await new Promise((resolve, reject) => { + const onListenError = (e: Error): void => { + reject(new Error(`Failed to start HTTP server on ${host}:${port}: ${e.message}`)); + }; + httpServer.once("error", onListenError); + httpServer.listen(port, host, (): void => { + httpServer.removeListener("error", onListenError); + resolve(); + }); + }); + + // After a successful bind, keep runtime errors from crashing the process: + // log and trigger graceful shutdown instead. + httpServer.on("error", (e: Error): void => { + logger.error(`HTTP server error: ${e.message}`); + cleanupManager.triggerShutdown(); + }); + + logger.info(`MCP server running on http transport at ${host}:${port}`); await cleanupManager.waitForShutdown(); await new Promise((resolve) => httpServer.close(() => { resolve(); })); } From fd195b7b61e6bc2e0b03420ec5ec4fd95ff97db0 Mon Sep 17 00:00:00 2001 From: kartikloops Date: Thu, 2 Jul 2026 18:36:14 -0600 Subject: [PATCH 14/16] reject out-of-range and malformed --port values (must be 1-65535) so a bad port fails at parse time with a clear message instead of crashing later at listen() --- typescript/__tests__/config/cli.test.ts | 12 ++++++++++++ typescript/src/config/cli.ts | 13 ++++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/typescript/__tests__/config/cli.test.ts b/typescript/__tests__/config/cli.test.ts index ed1186e..2461d80 100644 --- a/typescript/__tests__/config/cli.test.ts +++ b/typescript/__tests__/config/cli.test.ts @@ -57,4 +57,16 @@ describe("parseCliArgs", () => { vi.spyOn(process.stderr, "write").mockReturnValue(true); expect(() => parseCliArgs(["-t", "http", "--port", "abc"])).toThrow(ValidationError); }); + + it("rejects out-of-range and malformed ports", () => { + vi.spyOn(process.stderr, "write").mockReturnValue(true); + for (const bad of ["-1", "0", "65536", "99999", "80abc", "1.5"]) { + expect(() => parseCliArgs(["-t", "http", "--port", bad])).toThrow(ValidationError); + } + }); + + it("accepts boundary ports 1 and 65535", () => { + expect(parseCliArgs(["-t", "http", "--port", "1"]).transport.port).toBe(1); + expect(parseCliArgs(["-t", "http", "--port", "65535"]).transport.port).toBe(65535); + }); }); diff --git a/typescript/src/config/cli.ts b/typescript/src/config/cli.ts index 2bc5a9e..19f47e6 100644 --- a/typescript/src/config/cli.ts +++ b/typescript/src/config/cli.ts @@ -15,11 +15,18 @@ export interface CLIConfig { const DEFAULT_HOST = "localhost"; const DEFAULT_PORT = 8000; +const MIN_PORT = 1; +const MAX_PORT = 65535; function parsePort(value: string): number { - const port = Number.parseInt(value, 10); - if (Number.isNaN(port)) { - throw new ValidationError(`Invalid --port value: ${value}`); + const trimmed = value.trim(); + const port = Number.parseInt(trimmed, 10); + // Reject non-digits ("-1", "80abc", "1.5") and out-of-range values up front so + // a bad port fails here with a clear message instead of later at listen(). + if (!/^\d+$/.test(trimmed) || port < MIN_PORT || port > MAX_PORT) { + throw new ValidationError( + `Invalid --port value: '${value}'. Expected an integer between ${MIN_PORT} and ${MAX_PORT}.`, + ); } return port; } From 1b77ff65fdea191a7d481abcfa56b17fc21622c0 Mon Sep 17 00:00:00 2001 From: kartikloops Date: Sat, 4 Jul 2026 18:03:57 -0600 Subject: [PATCH 15/16] reframe the PTY spawn-failure message so a missing/non-executable binary on PATH is the primary cause and the spawn-helper chmod is the rare fallback normally handled by the postinstall script --- typescript/src/interactive/pty_handler.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/typescript/src/interactive/pty_handler.ts b/typescript/src/interactive/pty_handler.ts index c316873..cae3282 100644 --- a/typescript/src/interactive/pty_handler.ts +++ b/typescript/src/interactive/pty_handler.ts @@ -111,9 +111,10 @@ export class PtyHandler { if (/posix_spawnp failed|ENOENT|command not found/i.test(raw)) { throw new PTYError( `Failed to create PTY session: executable '${executable}' could not be started. ` + - `Common causes: (1) '${executable}' is missing from PATH or not executable ` + - `(PATH=${JSON.stringify(pathValue)}), or (2) node-pty's spawn-helper binary is not ` + - `executable — run: chmod +x node_modules/node-pty/prebuilds/*/spawn-helper`, + `Most likely '${executable}' is missing from PATH or not executable ` + + `(PATH=${JSON.stringify(pathValue)}). Rarely, node-pty's spawn-helper binary lost its ` + + `exec bit — the postinstall script normally restores it; if not (e.g. install ran ` + + `with --ignore-scripts), run: chmod +x node_modules/node-pty/prebuilds/*/spawn-helper`, ); } throw new PTYError(`Failed to create PTY session: ${e}`); From dc8a8430f1d8cbc14862bda749d84514d87ae1f4 Mon Sep 17 00:00:00 2001 From: kartikloops Date: Sat, 4 Jul 2026 18:13:57 -0600 Subject: [PATCH 16/16] exit non-zero (EXIT_CODE_ERROR) on forced shutdown so a stalled graceful shutdown that has to be force-killed reports as an abnormal exit instead of a clean one --- typescript/__tests__/utils/cleanup.test.ts | 7 ++++--- typescript/src/utils/cleanup.ts | 15 +++++++++++---- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/typescript/__tests__/utils/cleanup.test.ts b/typescript/__tests__/utils/cleanup.test.ts index 14be832..d5c1a7a 100644 --- a/typescript/__tests__/utils/cleanup.test.ts +++ b/typescript/__tests__/utils/cleanup.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, vi, afterEach } from "vitest"; import { CleanupManager } from "../../src/utils/cleanup.js"; -import { FORCE_EXIT_DELAY_SECONDS } from "../../src/constants.js"; +import { EXIT_CODE_ERROR, FORCE_EXIT_DELAY_SECONDS } from "../../src/constants.js"; describe("CleanupManager", () => { afterEach(() => { @@ -80,10 +80,11 @@ describe("CleanupManager", () => { await wait; expect(resolved).toBe(true); - // Force-exit fires only after the configured delay. + // Force-exit fires only after the configured delay, and exits non-zero + // because a forced exit means graceful shutdown did not complete. expect(exitSpy).not.toHaveBeenCalled(); vi.advanceTimersByTime(FORCE_EXIT_DELAY_SECONDS * 1000); - expect(exitSpy).toHaveBeenCalledWith(0); + expect(exitSpy).toHaveBeenCalledWith(EXIT_CODE_ERROR); } finally { // Remove only the listeners this test added, leaving the runner's intact. for (const l of process.listeners("SIGTERM")) { diff --git a/typescript/src/utils/cleanup.ts b/typescript/src/utils/cleanup.ts index 969d5f9..a72d702 100644 --- a/typescript/src/utils/cleanup.ts +++ b/typescript/src/utils/cleanup.ts @@ -1,4 +1,4 @@ -import { FORCE_EXIT_DELAY_SECONDS } from "../constants.js"; +import { EXIT_CODE_ERROR, FORCE_EXIT_DELAY_SECONDS } from "../constants.js"; import { getLogger } from "./logging.js"; const logger = getLogger("cleanup"); @@ -33,9 +33,16 @@ export class CleanupManager { logger.info( `Received ${signal}, shutting down (forcing exit in ${FORCE_EXIT_DELAY_SECONDS}s if it hangs)`, ); - // Force-exit safety net: if graceful shutdown stalls, leave anyway. The - // timer is unref'd so it never keeps the event loop alive on its own. - const timer = setTimeout(() => process.exit(0), FORCE_EXIT_DELAY_SECONDS * 1000); + // Force-exit safety net: if graceful shutdown stalls, leave anyway. This + // path only fires when cleanup failed to finish in time, so it exits + // non-zero — a forced exit is an abnormal outcome, not a clean shutdown. + // The timer is unref'd so it never keeps the event loop alive on its own. + const timer = setTimeout(() => { + logger.error( + `Graceful shutdown did not complete within ${FORCE_EXIT_DELAY_SECONDS}s; forcing exit`, + ); + process.exit(EXIT_CODE_ERROR); + }, FORCE_EXIT_DELAY_SECONDS * 1000); timer.unref(); this.triggerShutdown(); };