From 96be2ac2cd88935e7cac95ebdcf7cbea78069489 Mon Sep 17 00:00:00 2001
From: kartikloops <kartik55463@gmail.com>
Date: Sun, 16 Aug 2026 00:17:07 -0600
Subject: [PATCH 01/12] fix(pty): use a plain wide terminal and respect
 caller-supplied env

OpenROAD runs a readline-style line editor when driven through a PTY. At the
hardcoded 80 columns that editor horizontally scrolls any command longer than
the window, redrawing the whole visible line once per character: a single
75-character read_liberty came back as a cascade of near-identical lines that
buried the real output.

Widen the terminal well past any realistic command and ask for a plain TERM so
the editor skips the cursor-addressing layer we cannot interpret anyway.

The terminal variables were also applied after the caller's env, silently
overriding it, so create_interactive_session's env parameter could never set
TERM, COLUMNS or LINES. Apply the defaults first and let the caller win.

Also adds PTY_LINE_TERMINATOR, used by the session layer in the next commit.
---
 .../__tests__/interactive/pty_handler.test.ts | 17 ++++++++++++--
 typescript/src/constants.ts                   | 23 +++++++++++++++++++
 typescript/src/interactive/pty_handler.ts     | 15 +++++++-----
 3 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/typescript/__tests__/interactive/pty_handler.test.ts b/typescript/__tests__/interactive/pty_handler.test.ts
index a2be8f4..5c874e8 100644
--- a/typescript/__tests__/interactive/pty_handler.test.ts
+++ b/typescript/__tests__/interactive/pty_handler.test.ts
@@ -79,9 +79,22 @@ describe("PtyHandler", () => {
       const opts = call[2]!;
       expect(opts.cwd).toBe("/tmp");
       expect((opts.env as Record<string, string>)["TEST"]).toBe("value");
-      expect((opts.env as Record<string, string>)["TERM"]).toBe("xterm-256color");
-      expect((opts.env as Record<string, string>)["COLUMNS"]).toBe("80");
+      // A plain terminal and a width beyond any realistic command keep
+      // OpenROAD's line editor from horizontally scrolling long input, which
+      // would otherwise redraw the whole line once per character.
+      expect((opts.env as Record<string, string>)["TERM"]).toBe("dumb");
+      expect((opts.env as Record<string, string>)["COLUMNS"]).toBe("4096");
       expect((opts.env as Record<string, string>)["LINES"]).toBe("24");
+      expect(opts.cols).toBe(4096);
+    });
+
+    it("lets an explicit caller env override the terminal defaults", async () => {
+      await handler.createSession(["echo", "hello"], { TERM: "vt100", COLUMNS: "120" });
+
+      const opts = vi.mocked(spawn).mock.calls[0]![2]!;
+      expect((opts.env as Record<string, string>)["TERM"]).toBe("vt100");
+      expect(opts.name).toBe("vt100");
+      expect(opts.cols).toBe(120);
     });
 
     it("marks process as alive after createSession", async () => {
diff --git a/typescript/src/constants.ts b/typescript/src/constants.ts
index 92d6fc1..7648e5a 100644
--- a/typescript/src/constants.ts
+++ b/typescript/src/constants.ts
@@ -25,3 +25,26 @@ export const SLOW_OPERATION_THRESHOLD = 1.0;
 // Bounds memory on long-lived sessions; oldest entries are dropped when
 // exceeded.
 export const MAX_COMMAND_HISTORY = 1000;
+
+/**
+ * PTY geometry and terminal type.
+ *
+ * OpenROAD runs a readline-style line editor when stdin is a TTY. At a narrow
+ * width that editor horizontally scrolls long commands, redrawing the whole
+ * visible window once per character -- a single 75-character read_liberty turns
+ * into a cascade of near-identical lines that swamps the real output. A width
+ * far beyond any realistic command avoids the redraw entirely.
+ *
+ * TERM=dumb tells readline to skip the fancy editing layer (bracketed paste,
+ * horizontal scroll, cursor addressing) that we neither need nor can interpret.
+ */
+export const PTY_COLS = 4096;
+export const PTY_ROWS = 24;
+export const PTY_TERM = "dumb";
+
+/**
+ * Line terminator written after each command. A real Enter key sends carriage
+ * return, and a line editor in raw mode binds CR -- not LF -- to accept-line.
+ * Sending LF can leave the command sitting unsubmitted in the edit buffer.
+ */
+export const PTY_LINE_TERMINATOR = "\r";
diff --git a/typescript/src/interactive/pty_handler.ts b/typescript/src/interactive/pty_handler.ts
index ecd93b6..f499f4b 100644
--- a/typescript/src/interactive/pty_handler.ts
+++ b/typescript/src/interactive/pty_handler.ts
@@ -3,6 +3,7 @@ import { spawn } from "node-pty";
 import type { IPty, IDisposable } from "node-pty";
 import { getSettings } from "../config/settings.js";
 import type { Settings } from "../config/settings.js";
+import { PTY_COLS, PTY_ROWS, PTY_TERM } from "../constants.js";
 import { PTYError } from "./models.js";
 
 export class PtyHandler {
@@ -68,20 +69,22 @@ export class PtyHandler {
     try {
       this.validateCommand(command);
 
+      // Terminal settings come first so an explicit caller env can override
+      // them; previously they were applied last and silently won.
       const processEnv: Record<string, string> = {
         ...Object.fromEntries(
           Object.entries(process.env).filter((e): e is [string, string] => e[1] !== undefined),
         ),
+        TERM: PTY_TERM,
+        COLUMNS: String(PTY_COLS),
+        LINES: String(PTY_ROWS),
         ...env,
-        TERM: "xterm-256color",
-        COLUMNS: "80",
-        LINES: "24",
       };
 
       this._ptyProcess = spawn(command[0]!, command.slice(1), {
-        name: "xterm-256color",
-        cols: 80,
-        rows: 24,
+        name: processEnv.TERM ?? PTY_TERM,
+        cols: Number(processEnv.COLUMNS) || PTY_COLS,
+        rows: Number(processEnv.LINES) || PTY_ROWS,
         cwd: cwd ?? process.cwd(),
         env: processEnv,
       });

From 44e91f76380157fbcc35d8a1675b85bab70b11f6 Mon Sep 17 00:00:00 2001
From: kartikloops <kartik55463@gmail.com>
Date: Sun, 16 Aug 2026 00:18:04 -0600
Subject: [PATCH 02/12] fix(session): detect command completion with a
 sentinel, not a silence window

readOutput decided a command had finished once output went quiet for 100ms.
The PTY echoes the command back instantly, so that window closed while
OpenROAD was still working: read_db on a 15MB checkpoint and a 56s
repair_timing both returned the bare echo with error:null, indistinguishable
from success. timeout_ms was inert too, because the inner break abandoned the
whole loop on the first quiet moment rather than running to the deadline.

Write a nonce marker after each command and read until it appears. The Tcl
assembles the marker at runtime via join, so the literal never occurs in the
echoed input line and a match can only come from real stdout.

Consequences: timeout_ms now bounds the wait and reports CommandTimeout
instead of empty success, a process that dies mid-command reports
SessionTerminated, and execution_time is the command's real duration rather
than a flat 0.101s for everything.

Commands are also terminated with CR instead of LF. A line editor in raw mode
binds CR to accept-line; sending LF can leave the command sitting unsubmitted
in the edit buffer, which is how ten commands piled up without one executing.

readOutput is kept for buffer sampling and documented as unable to detect
completion, with a characterisation test pinning that behaviour so nobody
routes commands back through it. Output accumulation is capped at the buffer
size, keeping the tail, so a verbose repair_timing cannot grow the heap.
---
 typescript/__tests__/core/manager.test.ts     |  24 ++-
 .../__tests__/interactive/session.test.ts     | 147 +++++++++++++-
 typescript/src/core/manager.ts                |   3 +-
 typescript/src/interactive/session.ts         | 183 ++++++++++++++++--
 4 files changed, 325 insertions(+), 32 deletions(-)

diff --git a/typescript/__tests__/core/manager.test.ts b/typescript/__tests__/core/manager.test.ts
index fe8cac3..9f8717d 100644
--- a/typescript/__tests__/core/manager.test.ts
+++ b/typescript/__tests__/core/manager.test.ts
@@ -21,6 +21,7 @@ interface MockSession {
   start: Mock;
   sendCommand: Mock;
   readOutput: Mock;
+  runCommand: Mock;
   getInfo: Mock;
   getDetailedMetrics: Mock;
   getCommandHistory: Mock;
@@ -63,6 +64,15 @@ function makeMockSession(sessionId: string, alive = true): MockSession {
       bufferSize: 0,
       error: null,
     }),
+    runCommand: vi.fn().mockResolvedValue({
+      output: "ok",
+      sessionId,
+      timestamp: new Date().toISOString(),
+      executionTime: 0.01,
+      commandCount: 1,
+      bufferSize: 0,
+      error: null,
+    }),
     getInfo: vi.fn().mockResolvedValue({
       sessionId,
       createdAt: new Date().toISOString(),
@@ -155,11 +165,13 @@ describe("OpenROADManager", () => {
   });
 
   describe("executeCommand", () => {
-    it("delegates to sendCommand then readOutput", async () => {
+    it("delegates to runCommand so completion is actually detected", async () => {
       await manager.createSession({ sessionId: "s1" });
       const result = await manager.executeCommand("s1", "report_wns");
-      expect(created[0]!.sendCommand).toHaveBeenCalledWith("report_wns");
-      expect(created[0]!.readOutput).toHaveBeenCalledOnce();
+      expect(created[0]!.runCommand).toHaveBeenCalledWith("report_wns", expect.any(Number));
+      // readOutput cannot tell a finished command from a quiet one, so the
+      // command path must never fall back to it.
+      expect(created[0]!.readOutput).not.toHaveBeenCalled();
       expect(result.output).toBe("ok");
     });
 
@@ -172,8 +184,8 @@ describe("OpenROADManager", () => {
     it("falls back to the default timeout when timeoutMs is 0", async () => {
       await manager.createSession({ sessionId: "s1" });
       await manager.executeCommand("s1", "report_wns", 0);
-      // 0 must not be forwarded as an instant timeout; readOutput gets the default.
-      const timeoutArg = created[0]!.readOutput.mock.calls[0]![0] as number;
+      // 0 must not be forwarded as an instant timeout; runCommand gets the default.
+      const timeoutArg = created[0]!.runCommand.mock.calls[0]![1] as number;
       expect(timeoutArg).toBeGreaterThan(0);
     });
   });
@@ -323,7 +335,7 @@ describe("OpenROADManager", () => {
     it("forwards an explicit positive timeoutMs as-is", async () => {
       await manager.createSession({ sessionId: "s1" });
       await manager.executeCommand("s1", "report_wns", 5000);
-      expect(created[0]!.readOutput).toHaveBeenCalledWith(5000);
+      expect(created[0]!.runCommand).toHaveBeenCalledWith("report_wns", 5000);
     });
   });
 
diff --git a/typescript/__tests__/interactive/session.test.ts b/typescript/__tests__/interactive/session.test.ts
index 162d041..d8bf16f 100644
--- a/typescript/__tests__/interactive/session.test.ts
+++ b/typescript/__tests__/interactive/session.test.ts
@@ -124,15 +124,22 @@ describe("InteractiveSession", () => {
       expect(session.inputQueueSize()).toBe(1);
     });
 
-    it("appends newline to command if missing", async () => {
-      session.state = SessionState.ACTIVE;
+    it("terminates each command with a carriage return, normalising any trailing newline", async () => {
+      await session.start(["openroad", "-no_init"]);
       (mockPty.isProcessAlive as ReturnType<typeof vi.fn>).mockReturnValue(true);
 
       await session.sendCommand("test command");
-      expect(session.inputQueueSize()).toBe(1);
-
       await session.sendCommand("with newline\n");
       expect(session.commandCount).toBe(2);
+
+      // A line editor in raw mode accepts CR, not LF; sending LF can leave the
+      // command unsubmitted in the edit buffer.
+      await vi.waitFor(() => {
+        expect(mockPty.writeInput).toHaveBeenCalledTimes(2);
+      });
+      const written = (mockPty.writeInput as ReturnType<typeof vi.fn>).mock.calls.map((c) => c[0]);
+      expect(written[0]).toBe("test command\r");
+      expect(written[1]).toBe("with newline\r");
     });
 
     it("throws SessionTerminatedError on terminated session", async () => {
@@ -253,6 +260,138 @@ describe("InteractiveSession", () => {
       expect(result.output).toContain("delayed output");
       expect(result.executionTime).toBeGreaterThan(0);
     });
+
+    it("cannot detect completion: returns the echo as success while the command is still running", async () => {
+      // Characterisation test, not aspirational. readOutput stops after
+      // MAX_COMMAND_COMPLETION_WINDOW of silence, so a command that echoes
+      // immediately and then thinks for 400ms looks finished and successful.
+      // This is why executeCommand routes through runCommand instead; if you
+      // are tempted to use readOutput for a command, read this first.
+      await session.outputBuffer.append("read_db /tmp/big.odb\r\n"); // PTY echo
+      setTimeout(() => {
+        void session.outputBuffer.append("real result arrives late\r\n");
+      }, 400);
+
+      const result = await session.readOutput(5000);
+
+      expect(result.output).not.toContain("real result arrives late");
+      expect(result.error).toBeNull();
+      expect(result.executionTime).toBeLessThan(0.3);
+    });
+  });
+
+  describe("runCommand (sentinel-based completion)", () => {
+    const NONCE_RE = /join \{ORMCP DONE (\S+)\}/;
+
+    /**
+     * Drive the mock PTY like a real one: echo every written line back, then
+     * after `thinkMs` of silence emit the command's output followed by the
+     * sentinel. `thinkMs` is the whole point -- it reproduces a command that
+     * stays quiet longer than the old 100ms completion window.
+     */
+    function wirePty(opts: { thinkMs: number; output: string; emitSentinel?: boolean }): void {
+      const { thinkMs, output, emitSentinel = true } = opts;
+      (mockPty.writeInput as ReturnType<typeof vi.fn>).mockImplementation((data: string) => {
+        // A terminal echoes the submitted line back as CRLF.
+        void session.outputBuffer.append(data.replace(/\r$/, "\r\n"));
+        const match = NONCE_RE.exec(data);
+        if (!match) return;
+        const nonce = match[1]!;
+        setTimeout(() => {
+          void session.outputBuffer.append(output);
+          if (emitSentinel) void session.outputBuffer.append(`ORMCP-DONE-${nonce}\r\n`);
+        }, thinkMs);
+      });
+    }
+
+    beforeEach(async () => {
+      await session.start(["openroad", "-no_init"]);
+    });
+
+    it("waits for a command that stays silent well past the 100ms completion window", async () => {
+      // The regression that shipped: readOutput() saw the PTY echo, waited
+      // MAX_COMMAND_COMPLETION_WINDOW (100ms), saw nothing more, and returned
+      // the echo as a successful empty result while OpenROAD was still working.
+      wirePty({ thinkMs: 400, output: "wns max -0.485\r\n" });
+
+      const result = await session.runCommand("report_wns", 5000);
+
+      expect(result.output).toContain("wns max -0.485");
+      expect(result.error).toBeNull();
+      expect(result.executionTime).toBeGreaterThan(0.3);
+    });
+
+    it("strips the sentinel echo and marker from the returned output", async () => {
+      wirePty({ thinkMs: 10, output: "real output\r\n" });
+
+      const result = await session.runCommand("report_wns", 2000);
+
+      expect(result.output).toContain("real output");
+      expect(result.output).not.toContain("ORMCP");
+      expect(result.output).not.toContain("join {");
+    });
+
+    it("reports CommandTimeout instead of silent success when the command never finishes", async () => {
+      wirePty({ thinkMs: 10, output: "partial work\r\n", emitSentinel: false });
+
+      const result = await session.runCommand("repair_timing", 400);
+
+      expect(result.error).toMatch(/CommandTimeout/);
+      expect(result.output).toContain("partial work");
+    });
+
+    it("does not report success when the process dies mid-command", async () => {
+      (mockPty.writeInput as ReturnType<typeof vi.fn>).mockImplementation((data: string) => {
+        void session.outputBuffer.append(data.replace(/\r$/, "\r\n"));
+        if (NONCE_RE.test(data)) {
+          setTimeout(() => {
+            (mockPty.isProcessAlive as ReturnType<typeof vi.fn>).mockReturnValue(false);
+          }, 50);
+        }
+      });
+
+      const result = await session.runCommand("read_db /tmp/huge.odb", 3000);
+
+      expect(result.error).toMatch(/SessionTerminated/);
+    });
+
+    it("keeps the sentinel out of the audit trail", async () => {
+      wirePty({ thinkMs: 10, output: "ok\r\n" });
+
+      await session.runCommand("report_tns", 2000);
+
+      const history = session.getCommandHistory();
+      expect(history).toHaveLength(1);
+      expect(history[0]!.command).toBe("report_tns");
+      expect(session.commandCount).toBe(1);
+    });
+
+    it("records a real per-command execution time rather than a fixed window", async () => {
+      wirePty({ thinkMs: 350, output: "done\r\n" });
+
+      await session.runCommand("place_design", 5000);
+
+      const entry = session.getCommandHistory()[0]!;
+      expect(entry.executionTime).toBeGreaterThan(0.3);
+    });
+
+    it("suppresses a stale marker left behind by an earlier timed-out command", async () => {
+      await session.outputBuffer.append("ORMCP-DONE-staleabc123\r\n");
+      wirePty({ thinkMs: 10, output: "fresh output\r\n" });
+
+      const result = await session.runCommand("report_checks", 2000);
+
+      expect(result.output).toContain("fresh output");
+      expect(result.output).not.toContain("staleabc123");
+    });
+
+    it("still surfaces OpenROAD errors detected in the output", async () => {
+      wirePty({ thinkMs: 10, output: 'invalid command name "bogus_cmd"\r\n' });
+
+      const result = await session.runCommand("bogus_cmd", 2000);
+
+      expect(result.error).toContain("Invalid command");
+    });
   });
 
   describe("checkAlive", () => {
diff --git a/typescript/src/core/manager.ts b/typescript/src/core/manager.ts
index 138fb17..73ae4f3 100644
--- a/typescript/src/core/manager.ts
+++ b/typescript/src/core/manager.ts
@@ -92,8 +92,7 @@ export class OpenROADManager {
     // instant timeout.
     const actualTimeout = timeoutMs && timeoutMs > 0 ? timeoutMs : this.defaultTimeoutMs;
 
-    await session.sendCommand(command);
-    return session.readOutput(actualTimeout);
+    return session.runCommand(command, actualTimeout);
   }
 
   async getSessionInfo(sessionId: string): Promise<InteractiveSessionInfo> {
diff --git a/typescript/src/interactive/session.ts b/typescript/src/interactive/session.ts
index 92646d8..7aaee8c 100644
--- a/typescript/src/interactive/session.ts
+++ b/typescript/src/interactive/session.ts
@@ -15,12 +15,35 @@ import {
   BYTES_TO_MB,
   MAX_COMMAND_COMPLETION_WINDOW,
   MAX_COMMAND_HISTORY,
+  PTY_LINE_TERMINATOR,
   UTILIZATION_PERCENTAGE_BASE,
 } from "../constants.js";
 import { CircularBuffer } from "./buffer.js";
 import { SessionError, SessionTerminatedError } from "./models.js";
 import { PtyHandler } from "./pty_handler.js";
 
+/**
+ * Completion sentinel. After each user command we emit a marker line and read
+ * until it appears, which is the only reliable signal that OpenROAD finished:
+ * output alone cannot distinguish "still working" from "done", and a quiet
+ * period is not a completion signal for commands that think silently for
+ * minutes (read_db, repair_timing, detailed route).
+ *
+ * The Tcl is written so the assembled marker never appears in the PTY's echo of
+ * the input line -- `join` builds "ORMCP-DONE-<nonce>" at runtime, so a search
+ * for that literal can only match real stdout, never the echoed source.
+ */
+const SENTINEL_MARKER = (nonce: string): string => `ORMCP-DONE-${nonce}`;
+const SENTINEL_COMMAND = (nonce: string): string => `puts "[join {ORMCP DONE ${nonce}} -]"`;
+
+/** Matches any sentinel, including a stale one left by a timed-out command. */
+const ANY_SENTINEL_LINE = /ORMCP[-\s]DONE[-\s][0-9a-z]+/i;
+
+/** Liveness re-check cadence while blocked waiting for the sentinel. Bounded so
+ * a process that dies without emitting output is noticed promptly rather than
+ * at the full command timeout. */
+const SENTINEL_POLL_MS = 200;
+
 const ERROR_PATTERNS: Array<[RegExp, string]> = [
   [/invalid command name "([^"]+)"/i, "Invalid command: {0}"],
   [/wrong # args: should be "([^"]+)"/i, "Wrong arguments for command: {0}"],
@@ -65,6 +88,7 @@ export class InteractiveSession {
   private _inputQueue: string[] = [];
   private _inputWaiters: Array<() => void> = [];
   private _isShutdown = false;
+  private _sentinelCounter = 0;
   private _writerTask: Promise<void> | null = null;
   // Serialises terminate()/cleanup() so concurrent callers cannot double-kill
   // the process or deliver a stale exit code to waiters.
@@ -200,16 +224,152 @@ export class InteractiveSession {
       this.commandHistory.shift();
     }
 
-    const data = command.endsWith("\n") ? command : command + "\n";
-    this._inputQueue.push(data);
+    this._enqueueRaw(command);
     this.commandCount++;
     this.totalCommandsExecuted++;
     this.lastActivity = new Date();
+  }
 
+  /** Queue a line for the writer task without touching history or counters.
+   * Used for the internal completion sentinel, which is bookkeeping rather
+   * than a user command and must not appear in the audit trail. */
+  private _enqueueRaw(command: string): void {
+    this._inputQueue.push(command.replace(/[\r\n]+$/, "") + PTY_LINE_TERMINATOR);
     const waiters = this._inputWaiters.splice(0);
     for (const w of waiters) w();
   }
 
+  /**
+   * Send a command and read until it actually completes.
+   *
+   * Prefer this over sendCommand + readOutput: readOutput cannot tell a
+   * finished command from one that has merely gone quiet, so it returns the
+   * PTY's echo as a successful result for anything slower than a few
+   * milliseconds. This waits for an explicit sentinel and reports a real error
+   * when the command does not finish inside timeoutMs.
+   */
+  async runCommand(command: string, timeoutMs = 30000): Promise<InteractiveExecResult> {
+    const startTime = Date.now();
+
+    if (!this.checkAlive()) {
+      return this._drainTerminatedSession(startTime);
+    }
+
+    const nonce = `${Date.now().toString(36)}${(this._sentinelCounter++).toString(36)}${Math.floor(
+      Math.random() * 0xffffff,
+    ).toString(36)}`;
+    const marker = SENTINEL_MARKER(nonce);
+
+    await this.sendCommand(command);
+    this._enqueueRaw(SENTINEL_COMMAND(nonce));
+
+    const { text, timedOut, died } = await this._readUntilMarker(marker, startTime, timeoutMs);
+
+    const executionTime = (Date.now() - startTime) / 1000;
+    const output = ANSIDecoder.cleanOpenroadOutput(this._stripSentinelLines(text));
+
+    await this._updatePerformanceMetrics();
+    this._recordReadResult(output.length, executionTime);
+
+    let error = this._detectErrors(output) ?? null;
+    if (error === null && timedOut) {
+      error = `CommandTimeout: command did not complete within ${timeoutMs}ms`;
+    } else if (error === null && died) {
+      error = "SessionTerminated: process exited before the command completed";
+    }
+
+    return {
+      output,
+      sessionId: this.sessionId,
+      timestamp: new Date().toISOString(),
+      executionTime,
+      commandCount: this.commandCount,
+      bufferSize: this.outputBuffer.size,
+      error,
+    };
+  }
+
+  /**
+   * Accumulate output until the sentinel arrives, the deadline passes, or the
+   * process dies. Accumulation is capped at the buffer's configured size,
+   * keeping the tail, so a verbose multi-megabyte command cannot grow the heap
+   * without bound. The sentinel is last, so trimming the front never loses it.
+   */
+  private async _readUntilMarker(
+    marker: string,
+    startTime: number,
+    timeoutMs: number,
+  ): Promise<{ text: string; timedOut: boolean; died: boolean }> {
+    const cap = Math.max(this.outputBuffer.maxSize, 1);
+    let text = "";
+
+    const absorb = (chunks: string[]): void => {
+      if (chunks.length === 0) return;
+      text += chunks.join("");
+      if (text.length > cap) text = text.slice(text.length - cap);
+    };
+
+    for (;;) {
+      absorb(await this.outputBuffer.drainAll());
+      if (text.includes(marker)) return { text, timedOut: false, died: false };
+
+      if (!this.checkAlive()) {
+        absorb(await this.outputBuffer.drainAll());
+        return { text, timedOut: false, died: !text.includes(marker) };
+      }
+
+      const remaining = timeoutMs - (Date.now() - startTime);
+      if (remaining <= 0) return { text, timedOut: true, died: false };
+
+      // Capped so process death (which appends nothing) is still noticed.
+      await this.outputBuffer.waitForData(Math.min(remaining, SENTINEL_POLL_MS));
+    }
+  }
+
+  /** Remove sentinel echo and marker lines so callers never see the plumbing.
+   * Matches any sentinel, not just the current one, so a marker left behind by
+   * a previously timed-out command cannot leak into a later result. */
+  private _stripSentinelLines(text: string): string {
+    if (!ANY_SENTINEL_LINE.test(text)) return text;
+    // Terminals delimit with CR, LF or CRLF, and a line editor emits bare CRs
+    // when it redraws. Splitting on LF alone would leave the sentinel sharing a
+    // segment with real output and discard both.
+    return text
+      .split(/\r\n|[\r\n]/)
+      .filter((line) => !ANY_SENTINEL_LINE.test(line))
+      .join("\n");
+  }
+
+  /** Shared drain-before-reject path for a session that is already dead. */
+  private async _drainTerminatedSession(startTime: number): Promise<InteractiveExecResult> {
+    this._signalShutdown();
+    const chunks = await this.outputBuffer.drainAll();
+    if (chunks.length === 0) {
+      throw new SessionTerminatedError(`Session ${this.sessionId} is not active`, this.sessionId);
+    }
+    const output = ANSIDecoder.cleanOpenroadOutput(this._stripSentinelLines(chunks.join("")));
+    const executionTime = (Date.now() - startTime) / 1000;
+    this._recordReadResult(output.length, executionTime);
+    return {
+      output,
+      sessionId: this.sessionId,
+      timestamp: new Date().toISOString(),
+      executionTime,
+      commandCount: this.commandCount,
+      bufferSize: this.outputBuffer.size,
+      error: this._detectErrors(output) ?? null,
+    };
+  }
+
+  /**
+   * Drain whatever output has arrived, using a quiet period as a stop signal.
+   *
+   * This cannot detect command completion -- a command that thinks silently
+   * for longer than MAX_COMMAND_COMPLETION_WINDOW returns here as an empty
+   * success. Use runCommand() for anything that needs to know the command
+   * actually finished; this remains for callers that just want to sample the
+   * buffer.
+   */
   async readOutput(timeoutMs = 1000): Promise<InteractiveExecResult> {
     const startTime = Date.now();
 
@@ -222,24 +382,7 @@ export class InteractiveSession {
       // Return whatever is buffered rather than discarding it.
       // Also signal shutdown here so the writer task is guaranteed to stop
       // even when readOutput() is the first caller to observe the dead state.
-      this._signalShutdown();
-      const chunks = await this.outputBuffer.drainAll();
-      if (chunks.length === 0) {
-        throw new SessionTerminatedError(`Session ${this.sessionId} is not active`, this.sessionId);
-      }
-      const rawOutput = chunks.join("");
-      const output = ANSIDecoder.cleanOpenroadOutput(rawOutput);
-      const executionTime = (Date.now() - startTime) / 1000;
-      this._recordReadResult(output.length, executionTime);
-      return {
-        output,
-        sessionId: this.sessionId,
-        timestamp: new Date().toISOString(),
-        executionTime,
-        commandCount: this.commandCount,
-        bufferSize: this.outputBuffer.size,
-        error: this._detectErrors(output) ?? null,
-      };
+      return this._drainTerminatedSession(startTime);
     }
 
     const collected: string[] = [];

From 56cbe03c53a91336eb9bb162fabc56fb93c3403e Mon Sep 17 00:00:00 2001
From: kartikloops <kartik55463@gmail.com>
Date: Sun, 16 Aug 2026 00:19:02 -0600
Subject: [PATCH 03/12] feat(session): refuse to hand out a session that cannot
 execute commands

An OpenROAD process can come up looking perfectly healthy -- alive, prompt
printed, responding to inspect -- while its line editor never accepts the
lines we write. Every command then silently does nothing. In one 23-minute
run both sessions reported ten successful commands with 00:00:00 of CPU and
RSS pinned at the startup footprint; nothing had been loaded or computed.

createSession now runs a probe command through the sentinel path and fails if
it does not come back, so this turns into an obvious error within seconds of
session creation instead of a run's worth of results that were never real.
The probe is stripped from the audit trail afterwards.

The failure path also terminates the session. start() may already have spawned
a process, and previously a rejected creation dropped it from the registry
while leaving the OpenROAD process orphaned.
---
 typescript/__tests__/core/manager.test.ts     |  2 ++
 .../__tests__/interactive/session.test.ts     | 19 ++++++++++++++
 .../__tests__/performance/benchmarks.test.ts  |  2 ++
 .../performance/memory_monitoring.test.ts     |  1 +
 .../performance/response_sizes.test.ts        |  1 +
 typescript/src/core/manager.ts                | 11 +++++++-
 typescript/src/interactive/session.ts         | 26 +++++++++++++++++++
 7 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/typescript/__tests__/core/manager.test.ts b/typescript/__tests__/core/manager.test.ts
index 9f8717d..027acd2 100644
--- a/typescript/__tests__/core/manager.test.ts
+++ b/typescript/__tests__/core/manager.test.ts
@@ -19,6 +19,7 @@ interface MockSession {
   terminatedAt: Date | null;
   checkAlive: Mock;
   start: Mock;
+  verifyResponsive: Mock;
   sendCommand: Mock;
   readOutput: Mock;
   runCommand: Mock;
@@ -54,6 +55,7 @@ function makeMockSession(sessionId: string, alive = true): MockSession {
     terminatedAt: null,
     checkAlive: vi.fn().mockReturnValue(alive),
     start: vi.fn().mockResolvedValue(undefined),
+    verifyResponsive: vi.fn().mockResolvedValue(undefined),
     sendCommand: vi.fn().mockResolvedValue(undefined),
     readOutput: vi.fn().mockResolvedValue({
       output: "ok",
diff --git a/typescript/__tests__/interactive/session.test.ts b/typescript/__tests__/interactive/session.test.ts
index d8bf16f..1929111 100644
--- a/typescript/__tests__/interactive/session.test.ts
+++ b/typescript/__tests__/interactive/session.test.ts
@@ -385,6 +385,25 @@ describe("InteractiveSession", () => {
       expect(result.output).not.toContain("staleabc123");
     });
 
+    it("verifyResponsive rejects a session whose line editor never accepts input", async () => {
+      // The VM failure: the process is alive and echoes every keystroke, but
+      // the line is never submitted, so nothing ever executes.
+      (mockPty.writeInput as ReturnType<typeof vi.fn>).mockImplementation((data: string) => {
+        void session.outputBuffer.append(data.replace(/\r$/, ""));
+      });
+
+      await expect(session.verifyResponsive(500)).rejects.toThrow(/not executing commands/);
+    });
+
+    it("verifyResponsive accepts a healthy session without polluting the audit trail", async () => {
+      wirePty({ thinkMs: 10, output: "ORMCP_READY\r\n" });
+
+      await session.verifyResponsive(2000);
+
+      expect(session.getCommandHistory()).toHaveLength(0);
+      expect(session.commandCount).toBe(0);
+    });
+
     it("still surfaces OpenROAD errors detected in the output", async () => {
       wirePty({ thinkMs: 10, output: 'invalid command name "bogus_cmd"\r\n' });
 
diff --git a/typescript/__tests__/performance/benchmarks.test.ts b/typescript/__tests__/performance/benchmarks.test.ts
index ece730b..17fa222 100644
--- a/typescript/__tests__/performance/benchmarks.test.ts
+++ b/typescript/__tests__/performance/benchmarks.test.ts
@@ -13,6 +13,7 @@ interface MockSession {
   lastActivity: Date;
   checkAlive: Mock;
   start: Mock;
+  verifyResponsive: Mock;
   sendCommand: Mock;
   readOutput: Mock;
   getInfo: Mock;
@@ -43,6 +44,7 @@ function makeMockSession(sessionId: string): MockSession {
     lastActivity: new Date(),
     checkAlive: vi.fn().mockReturnValue(true),
     start: vi.fn().mockResolvedValue(undefined),
+    verifyResponsive: vi.fn().mockResolvedValue(undefined),
     sendCommand: vi.fn().mockResolvedValue(undefined),
     readOutput: vi.fn().mockResolvedValue({
       output: "ok",
diff --git a/typescript/__tests__/performance/memory_monitoring.test.ts b/typescript/__tests__/performance/memory_monitoring.test.ts
index 266a162..41ec4b5 100644
--- a/typescript/__tests__/performance/memory_monitoring.test.ts
+++ b/typescript/__tests__/performance/memory_monitoring.test.ts
@@ -12,6 +12,7 @@ function makeMockSession(sessionId: string) {
     lastActivity: new Date(),
     checkAlive: vi.fn().mockReturnValue(true),
     start: vi.fn().mockResolvedValue(undefined),
+    verifyResponsive: vi.fn().mockResolvedValue(undefined),
     sendCommand: vi.fn().mockResolvedValue(undefined),
     readOutput: vi.fn().mockResolvedValue({
       output: "ok",
diff --git a/typescript/__tests__/performance/response_sizes.test.ts b/typescript/__tests__/performance/response_sizes.test.ts
index efe6c95..0b9d34c 100644
--- a/typescript/__tests__/performance/response_sizes.test.ts
+++ b/typescript/__tests__/performance/response_sizes.test.ts
@@ -114,6 +114,7 @@ describe("Live Tool Compactness", () => {
         lastActivity: new Date(),
         checkAlive: vi.fn().mockReturnValue(true),
         start: vi.fn().mockResolvedValue(undefined),
+        verifyResponsive: vi.fn().mockResolvedValue(undefined),
         getInfo: vi.fn().mockResolvedValue({
           sessionId,
           createdAt: new Date().toISOString(),
diff --git a/typescript/src/core/manager.ts b/typescript/src/core/manager.ts
index 73ae4f3..217d5c9 100644
--- a/typescript/src/core/manager.ts
+++ b/typescript/src/core/manager.ts
@@ -73,7 +73,16 @@ export class OpenROADManager {
         // can't silently drop all output.
         const bufferSize = opts.bufferSize && opts.bufferSize > 0 ? opts.bufferSize : this.defaultBufferSize;
         const session = new InteractiveSession(sessionId, bufferSize);
-        await session.start(opts.command, opts.env, opts.cwd);
+        try {
+          await session.start(opts.command, opts.env, opts.cwd);
+          // Never hand out a session that cannot run commands.
+          await session.verifyResponsive();
+        } catch (e) {
+          // start() may have spawned a process before the failure; without this
+          // an unresponsive session leaves an orphaned OpenROAD behind.
+          await session.terminate(true).catch(() => { /* best effort */ });
+          throw e;
+        }
 
         this.sessions.set(sessionId, session);
         this.logger.info(`Created session ${sessionId}, total sessions: ${this.sessions.size}`);
diff --git a/typescript/src/interactive/session.ts b/typescript/src/interactive/session.ts
index 7aaee8c..fac3d9e 100644
--- a/typescript/src/interactive/session.ts
+++ b/typescript/src/interactive/session.ts
@@ -239,6 +239,32 @@ export class InteractiveSession {
     for (const w of waiters) w();
   }
 
+  /**
+   * Prove the session can actually execute a command before it is handed out.
+   *
+   * A PTY-attached OpenROAD can come up healthy-looking -- process alive,
+   * prompt printed -- while its line editor never accepts the lines we write,
+   * so every command silently does nothing. Failing here turns that into an
+   * immediate, obvious session-creation error instead of a run's worth of
+   * results that were never computed.
+   */
+  async verifyResponsive(timeoutMs = 15000): Promise<void> {
+    const result = await this.runCommand("puts ORMCP_READY", timeoutMs);
+    if (!result.output.includes("ORMCP_READY")) {
+      throw new SessionError(
+        `Session ${this.sessionId} started but is not executing commands: the OpenROAD ` +
+          `process is alive yet never ran a probe command within ${timeoutMs}ms. This usually ` +
+          `means its interactive line editor is not accepting input. ` +
+          `Probe returned: ${JSON.stringify(result.output.slice(-200))}`,
+        this.sessionId,
+      );
+    }
+    // The probe is plumbing, not user activity; keep it out of the audit trail.
+    this.commandHistory.length = 0;
+    this.commandCount = 0;
+    this.totalCommandsExecuted = 0;
+  }
+
   /**
    * Send a command and read until it actually completes.
    *

From e76cc082ef858e5767fb540f9098c7b83975f0a0 Mon Sep 17 00:00:00 2001
From: kartikloops <kartik55463@gmail.com>
Date: Sun, 16 Aug 2026 00:19:13 -0600
Subject: [PATCH 04/12] fix(report-images): match the .webp.png names some ORFS
 builds emit

list_report_images returned total_images:0 for a directory holding five real
images. save_images does not name them consistently: this build wrote
final_all.webp.png and final_routing.webp.png, PNGs carrying a doubled
extension, and the filter only matched a trailing ".webp".

Accept .webp.png, .webp and .png, and strip the doubled form before
classification so final_all.webp.png still resolves to complete_design rather
than falling through to unknown. read_report_image accepts the same set.

An empty listing now says which case it is -- no images generated, or images
present under names that did not match -- because a bare zero with error:null
leaves the caller unable to tell the run from the tool. That ambiguity is what
sent the last run off to read the files straight off disk instead.
---
 .../__tests__/tools/report_images.test.ts     | 50 ++++++++++++++++-
 typescript/src/tools/report_images.ts         | 56 ++++++++++++++++---
 2 files changed, 96 insertions(+), 10 deletions(-)

diff --git a/typescript/__tests__/tools/report_images.test.ts b/typescript/__tests__/tools/report_images.test.ts
index 431c6ca..985fd0a 100644
--- a/typescript/__tests__/tools/report_images.test.ts
+++ b/typescript/__tests__/tools/report_images.test.ts
@@ -108,6 +108,12 @@ describe("classifyImageType", () => {
     const [stage, _type] = classifyImageType("nounderscore.webp");
     expect(stage).toBe("unknown");
   });
+
+  it("classifies the doubled .webp.png extension some ORFS builds emit", () => {
+    expect(classifyImageType("final_all.webp.png")).toEqual(["final", "complete_design"]);
+    expect(classifyImageType("final_routing.webp.png")).toEqual(["final", "routing_visualization"]);
+    expect(classifyImageType("cts_clk.png")).toEqual(["cts", "clock_visualization"]);
+  });
 });
 
 describe("validatePlatformDesign", () => {
@@ -175,6 +181,46 @@ describe("ListReportImagesTool", () => {
     expect(result.images_by_stage).toEqual({});
   });
 
+  it("finds images written with the doubled .webp.png extension", async () => {
+    // The naming a real ORFS build produced; matching only ".webp" reported
+    // zero images for a directory that was full of them.
+    const { flowPath } = createFixture("nangate45", "gcd", "run-123", [
+      "final_all.webp.png",
+      "final_routing.webp.png",
+      "cts_clk.webp",
+    ]);
+    (getSettings as ReturnType<typeof vi.fn>).mockReturnValue({
+      platforms: ["nangate45"],
+      designs: (p: string) => (p === "nangate45" ? ["gcd"] : []),
+      flowPath,
+      WHITELIST_ENABLED: false,
+    });
+    const raw = await tool.execute("nangate45", "gcd", "run-123");
+    const result = JSON.parse(raw);
+    expect(result.total_images).toBe(3);
+    expect(result.images_by_stage["final"]).toHaveLength(2);
+    expect(result.images_by_stage["cts"]).toHaveLength(1);
+    expect(result.images_by_stage["final"][0].type).toBe("complete_design");
+  });
+
+  it("explains an empty result when the directory holds unrecognised image files", async () => {
+    const { flowPath, runPath } = createFixture("nangate45", "gcd", "run-odd", []);
+    fs.writeFileSync(path.join(runPath, "layout.tiff"), Buffer.from("x"));
+    fs.writeFileSync(path.join(runPath, "shot.jpeg"), Buffer.from("x"));
+    (getSettings as ReturnType<typeof vi.fn>).mockReturnValue({
+      platforms: ["nangate45"],
+      designs: (p: string) => (p === "nangate45" ? ["gcd"] : []),
+      flowPath,
+      WHITELIST_ENABLED: false,
+    });
+    const raw = await tool.execute("nangate45", "gcd", "run-odd");
+    const result = JSON.parse(raw);
+    expect(result.total_images).toBe(0);
+    // A bare zero cannot distinguish "no images" from "tool did not match them".
+    expect(result.message).toMatch(/do not match the expected extensions/);
+    expect(result.message).toContain("shot.jpeg");
+  });
+
   it("lists all .webp files grouped by stage", async () => {
     const { flowPath } = createFixture();
     (getSettings as ReturnType<typeof vi.fn>).mockReturnValue({
@@ -324,7 +370,7 @@ describe("ReadReportImageTool", () => {
     statSpy.mockRestore();
   });
 
-  it("rejects non-.webp extension", async () => {
+  it("rejects a non-image extension", async () => {
     const { flowPath } = createFixture();
     (getSettings as ReturnType<typeof vi.fn>).mockReturnValue({
       platforms: ["nangate45"],
@@ -332,7 +378,7 @@ describe("ReadReportImageTool", () => {
       flowPath,
       WHITELIST_ENABLED: false,
     });
-    const raw = await tool.execute("nangate45", "gcd", "run-123", "cts_clk.png");
+    const raw = await tool.execute("nangate45", "gcd", "run-123", "cts_clk.txt");
     const result = JSON.parse(raw);
     expect(result.error).toBe("InvalidImageName");
   });
diff --git a/typescript/src/tools/report_images.ts b/typescript/src/tools/report_images.ts
index 9e0f846..a62ff7b 100644
--- a/typescript/src/tools/report_images.ts
+++ b/typescript/src/tools/report_images.ts
@@ -41,7 +41,7 @@ const IMAGE_TYPE_MAPPING: Record<string, string> = {
  * ["unknown", "unknown"] for files with no underscore or unrecognised keys.
  */
 export function classifyImageType(filename: string): [string, string] {
-  const basename = path.basename(filename, path.extname(filename));
+  const basename = stripImageExtension(path.basename(filename));
   const underscoreIdx = basename.indexOf("_");
   let stage: string;
   let key: string;
@@ -97,14 +97,37 @@ function availableRuns(reportsBase: string): string[] {
   }
 }
 
-function findWebpFiles(dir: string): string[] {
+/**
+ * Report image extensions.
+ *
+ * ORFS does not consistently name these: depending on the build, save_images
+ * writes `final_all.webp` or `final_all.webp.png` (a PNG carrying a doubled
+ * extension). Matching only `.webp` silently finds nothing on the latter, so
+ * accept both and let the decoder sort out the actual format.
+ */
+const IMAGE_EXTENSIONS = [".webp.png", ".webp", ".png"];
+
+export function isReportImage(filename: string): boolean {
+  return IMAGE_EXTENSIONS.some((ext) => filename.toLowerCase().endsWith(ext));
+}
+
+/** Strip the report-image extension, including the doubled `.webp.png` form. */
+function stripImageExtension(filename: string): string {
+  const lower = filename.toLowerCase();
+  for (const ext of IMAGE_EXTENSIONS) {
+    if (lower.endsWith(ext)) return filename.slice(0, filename.length - ext.length);
+  }
+  return filename;
+}
+
+function findImageFiles(dir: string): string[] {
   const results: string[] = [];
   for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
     if (entry.isSymbolicLink()) continue;
     const full = path.join(dir, entry.name);
     if (entry.isDirectory()) {
-      results.push(...findWebpFiles(full));
-    } else if (entry.isFile() && entry.name.endsWith(".webp")) {
+      results.push(...findImageFiles(full));
+    } else if (entry.isFile() && isReportImage(entry.name)) {
       results.push(full);
     }
   }
@@ -250,17 +273,34 @@ export class ListReportImagesTool extends BaseTool {
     try {
       let files: string[];
       try {
-        files = findWebpFiles(runPath);
+        files = findImageFiles(runPath);
       } catch {
         files = [];
       }
 
       if (files.length === 0) {
+        // An empty result is ambiguous: no images generated, or images present
+        // under a name we do not recognise. Say which, so a caller is never
+        // left guessing whether the run or the tool is at fault.
+        let hint = "No report images found in this run directory.";
+        try {
+          const others = fs
+            .readdirSync(runPath, { withFileTypes: true })
+            .filter((e) => e.isFile() && /\.(png|jpe?g|gif|svg|webp)/i.test(e.name))
+            .map((e) => e.name);
+          if (others.length > 0) {
+            hint =
+              `Found ${others.length} image-like file(s) that do not match the expected ` +
+              `extensions (${IMAGE_EXTENSIONS.join(", ")}): ${others.slice(0, 10).join(", ")}`;
+          }
+        } catch { /* directory listing is best effort */ }
+
         return this.formatResult(
           ListImagesResult.parse({
             runPath,
             totalImages: 0,
             imagesByStage: {},
+            message: hint,
           }) as unknown as Record<string, unknown>,
         );
       }
@@ -357,11 +397,11 @@ export class ReadReportImageTool extends BaseTool {
       );
     }
 
-    if (!imageName.endsWith(".webp")) {
+    if (!isReportImage(imageName)) {
       return this.formatResult(
         ReadImageResult.parse({
           error: "InvalidImageName",
-          message: `Image '${imageName}' must have a .webp extension`,
+          message: `Image '${imageName}' must end in one of: ${IMAGE_EXTENSIONS.join(", ")}`,
         }) as unknown as Record<string, unknown>,
       );
     }
@@ -392,7 +432,7 @@ export class ReadReportImageTool extends BaseTool {
     if (!fs.existsSync(imagePath)) {
       let available: string[] = [];
       try {
-        available = findWebpFiles(runPath).map((f) => path.basename(f));
+        available = findImageFiles(runPath).map((f: string) => path.basename(f));
       } catch {
         available = [];
       }

From b7649f11082df4c9014c3f9706ec5cb03f539f42 Mon Sep 17 00:00:00 2001
From: kartikloops <kartik55463@gmail.com>
Date: Sun, 16 Aug 2026 00:29:28 -0600
Subject: [PATCH 05/12] fix(session): close three gaps found while reviewing
 the sentinel work

The startup probe could be satisfied by the PTY's echo of its own source
line, so a session that echoes every line but executes nothing -- the exact
failure the handshake exists to catch -- was accepted as healthy. Assemble
the probe token at runtime, as the completion sentinel already does, so only
real stdout can match it.

Also normalise embedded newlines, not just the trailing one, to CR: an inner
LF strands everything after it in the line editor's buffer for the same
reason a trailing LF did. And let a timeout outrank an error matched in the
partial output, so a command that printed "Error:" and then kept running is
not reported as finished.
---
 .../__tests__/interactive/session.test.ts     | 36 ++++++++++++++++---
 typescript/src/interactive/session.ts         | 29 +++++++++++----
 2 files changed, 54 insertions(+), 11 deletions(-)

diff --git a/typescript/__tests__/interactive/session.test.ts b/typescript/__tests__/interactive/session.test.ts
index 1929111..1f7a298 100644
--- a/typescript/__tests__/interactive/session.test.ts
+++ b/typescript/__tests__/interactive/session.test.ts
@@ -142,6 +142,22 @@ describe("InteractiveSession", () => {
       expect(written[1]).toBe("with newline\r");
     });
 
+    it("converts embedded newlines to carriage returns in a multi-line command", async () => {
+      await session.start(["openroad", "-no_init"]);
+      (mockPty.isProcessAlive as ReturnType<typeof vi.fn>).mockReturnValue(true);
+
+      // Each inner line has to be accepted by the editor too, so an LF left in
+      // the middle would strand everything after it in the edit buffer.
+      await session.sendCommand("set a 1\nset b 2");
+
+      await vi.waitFor(() => {
+        expect(mockPty.writeInput).toHaveBeenCalledTimes(1);
+      });
+      expect((mockPty.writeInput as ReturnType<typeof vi.fn>).mock.calls[0]![0]).toBe(
+        "set a 1\rset b 2\r",
+      );
+    });
+
     it("throws SessionTerminatedError on terminated session", async () => {
       session.state = SessionState.TERMINATED;
       await expect(session.sendCommand("test")).rejects.toThrow(SessionTerminatedError);
@@ -386,17 +402,18 @@ describe("InteractiveSession", () => {
     });
 
     it("verifyResponsive rejects a session whose line editor never accepts input", async () => {
-      // The VM failure: the process is alive and echoes every keystroke, but
-      // the line is never submitted, so nothing ever executes.
+      // The VM failure: the process is alive and echoes every submitted line
+      // back cleanly, but never runs it. The echo alone must not satisfy the
+      // probe, which is why the probe token is assembled at runtime.
       (mockPty.writeInput as ReturnType<typeof vi.fn>).mockImplementation((data: string) => {
-        void session.outputBuffer.append(data.replace(/\r$/, ""));
+        void session.outputBuffer.append(data.replace(/\r$/, "\r\n"));
       });
 
-      await expect(session.verifyResponsive(500)).rejects.toThrow(/not executing commands/);
+      await expect(session.verifyResponsive(600)).rejects.toThrow(/not executing commands/);
     });
 
     it("verifyResponsive accepts a healthy session without polluting the audit trail", async () => {
-      wirePty({ thinkMs: 10, output: "ORMCP_READY\r\n" });
+      wirePty({ thinkMs: 10, output: "ORMCP-READY-OK\r\n" });
 
       await session.verifyResponsive(2000);
 
@@ -404,6 +421,15 @@ describe("InteractiveSession", () => {
       expect(session.commandCount).toBe(0);
     });
 
+    it("reports a timeout even when the partial output contains an error pattern", async () => {
+      wirePty({ thinkMs: 10, output: "Error: something went wrong\r\n", emitSentinel: false });
+
+      const result = await session.runCommand("long_running_thing", 400);
+
+      // A truncated result must not be presented as a completed, failed one.
+      expect(result.error).toMatch(/CommandTimeout/);
+    });
+
     it("still surfaces OpenROAD errors detected in the output", async () => {
       wirePty({ thinkMs: 10, output: 'invalid command name "bogus_cmd"\r\n' });
 
diff --git a/typescript/src/interactive/session.ts b/typescript/src/interactive/session.ts
index fac3d9e..2d622e4 100644
--- a/typescript/src/interactive/session.ts
+++ b/typescript/src/interactive/session.ts
@@ -39,6 +39,15 @@ const SENTINEL_COMMAND = (nonce: string): string => `puts "[join {ORMCP DONE ${n
 /** Matches any sentinel, including a stale one left by a timed-out command. */
 const ANY_SENTINEL_LINE = /ORMCP[-\s]DONE[-\s][0-9a-z]+/i;
 
+/**
+ * Startup probe. Uses the same runtime-assembly trick as the sentinel for the
+ * same reason: a probe whose token appears verbatim in its own source would be
+ * satisfied by the PTY's echo, so a session that echoes every keystroke but
+ * executes nothing -- precisely the failure this guards against -- would pass.
+ */
+const PROBE_COMMAND = `puts "[join {ORMCP READY OK} -]"`;
+const PROBE_TOKEN = "ORMCP-READY-OK";
+
 /** Liveness re-check cadence while blocked waiting for the sentinel. Bounded so
  * a process that dies without emitting output is noticed promptly rather than
  * at the full command timeout. */
@@ -234,7 +243,10 @@ export class InteractiveSession {
    * Used for the internal completion sentinel, which is bookkeeping rather
    * than a user command and must not appear in the audit trail. */
   private _enqueueRaw(command: string): void {
-    this._inputQueue.push(command.replace(/[\r\n]+$/, "") + PTY_LINE_TERMINATOR);
+    // Every embedded newline is a line the editor has to accept, not just the
+    // final one, so normalise all of them rather than only the terminator.
+    const line = command.replace(/\r?\n/g, PTY_LINE_TERMINATOR).replace(/\r+$/, "");
+    this._inputQueue.push(line + PTY_LINE_TERMINATOR);
     const waiters = this._inputWaiters.splice(0);
     for (const w of waiters) w();
   }
@@ -249,12 +261,13 @@ export class InteractiveSession {
    * results that were never computed.
    */
   async verifyResponsive(timeoutMs = 15000): Promise<void> {
-    const result = await this.runCommand("puts ORMCP_READY", timeoutMs);
-    if (!result.output.includes("ORMCP_READY")) {
+    const result = await this.runCommand(PROBE_COMMAND, timeoutMs);
+    if (!result.output.includes(PROBE_TOKEN)) {
       throw new SessionError(
         `Session ${this.sessionId} started but is not executing commands: the OpenROAD ` +
           `process is alive yet never ran a probe command within ${timeoutMs}ms. This usually ` +
           `means its interactive line editor is not accepting input. ` +
+          `Probe error: ${result.error ?? "none"}. ` +
           `Probe returned: ${JSON.stringify(result.output.slice(-200))}`,
         this.sessionId,
       );
@@ -297,11 +310,15 @@ export class InteractiveSession {
     await this._updatePerformanceMetrics();
     this._recordReadResult(output.length, executionTime);
 
-    let error = this._detectErrors(output) ?? null;
-    if (error === null && timedOut) {
+    // An incomplete result outranks anything matched inside it: a command that
+    // printed "Error:" and then kept running must not look like it finished.
+    let error: string | null;
+    if (timedOut) {
       error = `CommandTimeout: command did not complete within ${timeoutMs}ms`;
-    } else if (error === null && died) {
+    } else if (died) {
       error = "SessionTerminated: process exited before the command completed";
+    } else {
+      error = this._detectErrors(output) ?? null;
     }
 
     return {

From df1324d416cde4fa60fa40577c3e5f1fb398f47b Mon Sep 17 00:00:00 2001
From: kartikloops <kartik55463@gmail.com>
Date: Sun, 16 Aug 2026 00:54:02 -0600
Subject: [PATCH 06/12] fix(report-images): report actual image format and
 anchor extension match

metadata.format was hardcoded to webp even for plain PNG/webp.png files,
and the unrecognized-file hint regex matched extension substrings anywhere
in the filename instead of at the end.
---
 docs/SECURITY.md                              |  5 +--
 .../__tests__/tools/report_images.test.ts     | 31 +++++++++++++++++++
 typescript/src/tools/report_images.ts         | 17 ++++++++--
 3 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/docs/SECURITY.md b/docs/SECURITY.md
index a4c5c71..a4142ed 100644
--- a/docs/SECURITY.md
+++ b/docs/SECURITY.md
@@ -199,8 +199,9 @@ under the base directory. A symlink that points outside the base is caught here.
 
 **Additional constraints:**
 
-- Only `.webp` files are served — the listing skips all other extensions and `read_report_image`
-  rejects any `image_name` that does not end in `.webp`.
+- Only report-image extensions are served — `.webp`, `.png`, and the doubled `.webp.png` form
+  some ORFS builds emit. The listing skips everything else and `read_report_image` rejects any
+  `image_name` that does not end in one of them.
 - Symlinks are skipped during directory listing.
 - On-disk file size is capped at 50 MB before any decoding.
 - The base64-encoded payload is targeted at 15 KB; larger images are downscaled using sharp
diff --git a/typescript/__tests__/tools/report_images.test.ts b/typescript/__tests__/tools/report_images.test.ts
index 985fd0a..70e2965 100644
--- a/typescript/__tests__/tools/report_images.test.ts
+++ b/typescript/__tests__/tools/report_images.test.ts
@@ -77,6 +77,16 @@ async function writeRealWebp(filePath: string, width: number, height: number): P
   fs.writeFileSync(filePath, buffer);
 }
 
+/** Writes a real PNG, for the `.webp.png` files some ORFS builds emit. */
+async function writeRealPng(filePath: string, width: number, height: number): Promise<void> {
+  const buffer = await sharp({
+    create: { width, height, channels: 3, background: { r: 10, g: 20, b: 30 } },
+  })
+    .png()
+    .toBuffer();
+  fs.writeFileSync(filePath, buffer);
+}
+
 beforeEach(() => {
   tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "openroad-test-"));
 });
@@ -347,6 +357,27 @@ describe("ReadReportImageTool", () => {
     expect(result.metadata.filename).toBe("cts_clk.webp");
     expect(result.metadata.stage).toBe("cts");
     expect(result.metadata.type).toBe("clock_visualization");
+    expect(result.metadata.format).toBe("webp");
+  });
+
+  it("reports the real format of a .webp.png, which is returned as PNG bytes", async () => {
+    const { flowPath, runPath } = createFixture("nangate45", "gcd", "run-123", []);
+    // Small enough to skip compression, so the bytes are the file's own.
+    await writeRealPng(path.join(runPath, "final_all.webp.png"), 4, 4);
+    (getSettings as ReturnType<typeof vi.fn>).mockReturnValue({
+      platforms: ["nangate45"],
+      designs: (p: string) => (p === "nangate45" ? ["gcd"] : []),
+      flowPath,
+      WHITELIST_ENABLED: false,
+    });
+
+    const result = JSON.parse(await tool.execute("nangate45", "gcd", "run-123", "final_all.webp.png"));
+
+    // The declared format has to match the bytes: a consumer that trusts it
+    // instead of sniffing the header would otherwise fail to decode.
+    const decoded = Buffer.from(result.image_data, "base64");
+    expect(decoded.subarray(0, 4)).toEqual(Buffer.from([0x89, 0x50, 0x4e, 0x47]));
+    expect(result.metadata.format).toBe("png");
   });
 
   it("returns FileTooLarge error when image exceeds 50 MB", async () => {
diff --git a/typescript/src/tools/report_images.ts b/typescript/src/tools/report_images.ts
index a62ff7b..cd9fd56 100644
--- a/typescript/src/tools/report_images.ts
+++ b/typescript/src/tools/report_images.ts
@@ -111,6 +111,19 @@ export function isReportImage(filename: string): boolean {
   return IMAGE_EXTENSIONS.some((ext) => filename.toLowerCase().endsWith(ext));
 }
 
+/**
+ * Format of the bytes actually returned, which is not always WebP.
+ *
+ * Only the compression path re-encodes; images small enough to send as-is are
+ * returned byte-for-byte from disk. Since `.webp.png` files really are PNG,
+ * reporting a blanket "webp" would misdescribe the payload to any consumer that
+ * trusts this field instead of sniffing the header.
+ */
+function imageFormat(filename: string, compressionApplied: boolean): string {
+  if (compressionApplied) return "webp";
+  return filename.toLowerCase().endsWith(".webp") ? "webp" : "png";
+}
+
 /** Strip the report-image extension, including the doubled `.webp.png` form. */
 function stripImageExtension(filename: string): string {
   const lower = filename.toLowerCase();
@@ -286,7 +299,7 @@ export class ListReportImagesTool extends BaseTool {
         try {
           const others = fs
             .readdirSync(runPath, { withFileTypes: true })
-            .filter((e) => e.isFile() && /\.(png|jpe?g|gif|svg|webp)/i.test(e.name))
+            .filter((e) => e.isFile() && /\.(png|jpe?g|gif|svg|webp)$/i.test(e.name))
             .map((e) => e.name);
           if (others.length > 0) {
             hint =
@@ -474,7 +487,7 @@ export class ReadReportImageTool extends BaseTool {
 
       const metadata = ImageMetadata.parse({
         filename: imageName,
-        format: "webp",
+        format: imageFormat(imageName, r.compressionApplied),
         sizeBytes: r.compressedSize,
         width: r.width,
         height: r.height,

From c650640577b66ec055c6099dfd13ab44f6ac507f Mon Sep 17 00:00:00 2001
From: kartikloops <kartik55463@gmail.com>
Date: Sun, 16 Aug 2026 00:54:49 -0600
Subject: [PATCH 07/12] fix(manager): stop holding the lock through session
 startup and handshake

createSession ran session.start() and verifyResponsive() -- which can take up
to a 15s (now 60s) timeout on a slow cold start -- entirely inside
cleanupLock, so one slow or stuck session stalled every other create/list/
terminate call sharing the lock.

Reserve the session id under the lock, release it, spawn and verify outside
the lock, then take the lock again to publish the result. Reservations
(sessions still mapped to null) now count towards maxSessions alongside
active ones, since concurrent creates would otherwise all see the same
pre-spawn total and admit past the cap together.
---
 typescript/__tests__/core/manager.test.ts | 53 +++++++++++++++++
 typescript/src/constants.ts               |  9 +++
 typescript/src/core/manager.ts            | 71 +++++++++++++++--------
 3 files changed, 110 insertions(+), 23 deletions(-)

diff --git a/typescript/__tests__/core/manager.test.ts b/typescript/__tests__/core/manager.test.ts
index 027acd2..fea0fe6 100644
--- a/typescript/__tests__/core/manager.test.ts
+++ b/typescript/__tests__/core/manager.test.ts
@@ -97,6 +97,25 @@ function makeMockSession(sessionId: string, alive = true): MockSession {
 
 const MockedSession = vi.mocked(InteractiveSession);
 
+/** Park the next session's handshake so a create stays in flight on demand.
+ * Must be a plain function, not an arrow, so `new` still works. */
+function stallNextHandshake(): { handshakeStarted: Promise<void>; release: () => void } {
+  let release!: () => void;
+  let signalStarted!: () => void;
+  const handshakeStarted = new Promise<void>((r) => (signalStarted = r));
+
+  MockedSession.mockImplementationOnce(function (this: unknown, sessionId: string) {
+    const mock = makeMockSession(sessionId);
+    mock.verifyResponsive = vi.fn().mockImplementation(async () => {
+      signalStarted();
+      await new Promise<void>((r) => (release = r));
+    });
+    return mock as unknown as InteractiveSession;
+  } as unknown as (sessionId: string) => InteractiveSession);
+
+  return { handshakeStarted, release: () => release() };
+}
+
 describe("OpenROADManager", () => {
   let manager: OpenROADManager;
   let created: MockSession[];
@@ -145,6 +164,40 @@ describe("OpenROADManager", () => {
       await expect(limited.createSession({ sessionId: "s2" })).rejects.toThrow(/Maximum session limit/);
     });
 
+    it("does not hold the manager lock across a slow startup handshake", async () => {
+      const { handshakeStarted, release } = stallNextHandshake();
+
+      const slow = manager.createSession({ sessionId: "slow" });
+      await handshakeStarted;
+
+      // A second session stuck in its handshake must not stall unrelated
+      // lifecycle work; before, this awaited the full handshake timeout.
+      await expect(
+        Promise.race([
+          manager.listSessions().then(() => "listed"),
+          new Promise((r) => setTimeout(() => r("blocked"), 1000)),
+        ]),
+      ).resolves.toBe("listed");
+
+      release();
+      await slow;
+    });
+
+    it("counts in-flight reservations towards the session cap", async () => {
+      const limited = new OpenROADManager(1);
+      const { handshakeStarted, release } = stallNextHandshake();
+
+      const first = limited.createSession({ sessionId: "s1" });
+      await handshakeStarted;
+
+      // Startup happens outside the lock now, so the reserved-but-unspawned
+      // slot is the only thing keeping concurrent creates under the cap.
+      await expect(limited.createSession({ sessionId: "s2" })).rejects.toThrow(/Maximum session limit/);
+
+      release();
+      await first;
+    });
+
     it("falls back to the default buffer size when bufferSize is 0", async () => {
       await manager.createSession({ sessionId: "zero", bufferSize: 0 });
       // InteractiveSession is constructed with (sessionId, bufferSize); a 0 must
diff --git a/typescript/src/constants.ts b/typescript/src/constants.ts
index 7648e5a..5abcc77 100644
--- a/typescript/src/constants.ts
+++ b/typescript/src/constants.ts
@@ -48,3 +48,12 @@ export const PTY_TERM = "dumb";
  * Sending LF can leave the command sitting unsubmitted in the edit buffer.
  */
 export const PTY_LINE_TERMINATOR = "\r";
+
+/**
+ * Budget for the startup handshake. This has to cover the whole cold start --
+ * loading a large OpenROAD binary and reaching its prompt -- not just the probe
+ * round-trip, because spawning does not wait for readiness. Generous on
+ * purpose: the handshake exists to catch a session that will never respond, and
+ * failing a slow-but-healthy start would be worse than the bug it prevents.
+ */
+export const SESSION_HANDSHAKE_TIMEOUT_MS = 60_000;
diff --git a/typescript/src/core/manager.ts b/typescript/src/core/manager.ts
index 217d5c9..5d42d8e 100644
--- a/typescript/src/core/manager.ts
+++ b/typescript/src/core/manager.ts
@@ -1,5 +1,6 @@
 import { Mutex } from "async-mutex";
 import { randomUUID } from "node:crypto";
+import { SESSION_HANDSHAKE_TIMEOUT_MS } from "../constants.js";
 import { getSettings } from "../config/settings.js";
 import type { Settings } from "../config/settings.js";
 import { getLogger } from "../utils/logging.js";
@@ -39,6 +40,11 @@ export class OpenROADManager {
   private readonly maxSessions: number;
   private readonly defaultTimeoutMs: number;
   private readonly defaultBufferSize: number;
+  // Covers the whole cold start, not just the probe round-trip: start() only
+  // spawns, so OpenROAD still has to load and reach its prompt inside this
+  // budget. A container on a cold filesystem is comfortably slower than the
+  // handshake's own default.
+  private readonly handshakeTimeoutMs: number = SESSION_HANDSHAKE_TIMEOUT_MS;
 
   constructor(maxSessions?: number) {
     this.maxSessions = maxSessions ?? this.settings.MAX_SESSIONS;
@@ -50,14 +56,20 @@ export class OpenROADManager {
   async createSession(opts: CreateSessionOptions = {}): Promise<string> {
     const sessionId = opts.sessionId ?? randomUUID().slice(0, 8);
 
-    return this.cleanupLock.runExclusive(async () => {
+    // Reserve the id under the lock, then release it: spawning OpenROAD and
+    // waiting out its handshake takes seconds, and holding the manager-wide
+    // lock for that long stalls every other create/terminate/list.
+    await this.cleanupLock.runExclusive(async () => {
       await this._cleanupTerminatedSessions();
 
       if (this.sessions.has(sessionId)) {
         throw new SessionError(`Session ${sessionId} already exists`, sessionId);
       }
 
-      const activeCount = this._countActive();
+      // Reservations count towards the cap: startup now happens outside the
+      // lock, so without this concurrent creates would all see the same
+      // pre-spawn total and admit past maxSessions together.
+      const activeCount = this._countActive() + this._countReserved();
       if (activeCount >= this.maxSessions) {
         throw new SessionError(
           `Maximum session limit reached (${this.maxSessions}). Currently ${activeCount} active sessions.`,
@@ -67,32 +79,36 @@ export class OpenROADManager {
 
       // Placeholder distinguishes "creating" (null) from "not found" (absent).
       this.sessions.set(sessionId, null);
+    });
 
+    try {
+      // 0 (and undefined) fall back to the default so a zero-capacity buffer
+      // can't silently drop all output.
+      const bufferSize = opts.bufferSize && opts.bufferSize > 0 ? opts.bufferSize : this.defaultBufferSize;
+      const session = new InteractiveSession(sessionId, bufferSize);
       try {
-        // 0 (and undefined) fall back to the default so a zero-capacity buffer
-        // can't silently drop all output.
-        const bufferSize = opts.bufferSize && opts.bufferSize > 0 ? opts.bufferSize : this.defaultBufferSize;
-        const session = new InteractiveSession(sessionId, bufferSize);
-        try {
-          await session.start(opts.command, opts.env, opts.cwd);
-          // Never hand out a session that cannot run commands.
-          await session.verifyResponsive();
-        } catch (e) {
-          // start() may have spawned a process before the failure; without this
-          // an unresponsive session leaves an orphaned OpenROAD behind.
-          await session.terminate(true).catch(() => { /* best effort */ });
-          throw e;
-        }
+        await session.start(opts.command, opts.env, opts.cwd);
+        // Never hand out a session that cannot run commands.
+        await session.verifyResponsive(this.handshakeTimeoutMs);
+      } catch (e) {
+        // start() may have spawned a process before the failure; without this
+        // an unresponsive session leaves an orphaned OpenROAD behind.
+        await session.terminate(true).catch(() => { /* best effort */ });
+        throw e;
+      }
 
+      await this.cleanupLock.runExclusive(() => {
         this.sessions.set(sessionId, session);
-        this.logger.info(`Created session ${sessionId}, total sessions: ${this.sessions.size}`);
-        return sessionId;
-      } catch (e) {
+      });
+      this.logger.info(`Created session ${sessionId}, total sessions: ${this.sessions.size}`);
+      return sessionId;
+    } catch (e) {
+      await this.cleanupLock.runExclusive(() => {
         this.sessions.delete(sessionId);
-        this.logger.error(`Failed to create session ${sessionId}: ${String(e)}`);
-        throw new SessionError(`Failed to create session: ${String(e)}`, sessionId);
-      }
-    });
+      });
+      this.logger.error(`Failed to create session ${sessionId}: ${String(e)}`);
+      throw new SessionError(`Failed to create session: ${String(e)}`, sessionId);
+    }
   }
 
   async executeCommand(sessionId: string, command: string, timeoutMs?: number): Promise<InteractiveExecResult> {
@@ -254,6 +270,15 @@ export class OpenROADManager {
     return count;
   }
 
+  /** Ids claimed by an in-flight createSession that has not spawned yet. */
+  private _countReserved(): number {
+    let count = 0;
+    for (const session of this.sessions.values()) {
+      if (session === null) count++;
+    }
+    return count;
+  }
+
   private _initializedSessions(): Array<[string, InteractiveSession]> {
     const result: Array<[string, InteractiveSession]> = [];
     for (const [sid, session] of this.sessions) {

From 02bf6a732247962123a6a6fa8b8cc154e0c58a03 Mon Sep 17 00:00:00 2001
From: kartikloops <kartik55463@gmail.com>
Date: Sun, 16 Aug 2026 00:54:52 -0600
Subject: [PATCH 08/12] fix(pty): reject non-positive COLUMNS/LINES instead of
 silently discarding them

cols/rows used Number(x) || default, so an explicit caller override of "0"
(or any falsy Number() result) fell through to the hardcoded default instead
of being honored, even though the surrounding code exists specifically to let
caller-supplied env win. Validate for a positive integer instead of relying
on JS falsiness.
---
 .../__tests__/interactive/pty_handler.test.ts      | 14 ++++++++++++++
 typescript/src/interactive/pty_handler.ts          | 12 ++++++++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/typescript/__tests__/interactive/pty_handler.test.ts b/typescript/__tests__/interactive/pty_handler.test.ts
index 5c874e8..c8f9bb3 100644
--- a/typescript/__tests__/interactive/pty_handler.test.ts
+++ b/typescript/__tests__/interactive/pty_handler.test.ts
@@ -97,6 +97,20 @@ describe("PtyHandler", () => {
       expect(opts.cols).toBe(120);
     });
 
+    it.each([
+      ["zero", "0"],
+      ["negative", "-5"],
+      ["fractional", "80.5"],
+      ["non-numeric", "wide"],
+      ["blank", ""],
+    ])("falls back to the default geometry for a %s COLUMNS", async (_label, columns) => {
+      // node-pty cannot honour any of these as a cell count, so a bad value
+      // must not reach it -- least of all a negative, which is truthy.
+      await handler.createSession(["echo", "hello"], { COLUMNS: columns });
+
+      expect(vi.mocked(spawn).mock.calls[0]![2]!.cols).toBe(4096);
+    });
+
     it("marks process as alive after createSession", async () => {
       await handler.createSession(["echo"]);
       expect(handler.isProcessAlive()).toBe(true);
diff --git a/typescript/src/interactive/pty_handler.ts b/typescript/src/interactive/pty_handler.ts
index f499f4b..f76deec 100644
--- a/typescript/src/interactive/pty_handler.ts
+++ b/typescript/src/interactive/pty_handler.ts
@@ -6,6 +6,14 @@ import type { Settings } from "../config/settings.js";
 import { PTY_COLS, PTY_ROWS, PTY_TERM } from "../constants.js";
 import { PTYError } from "./models.js";
 
+/** Terminal geometry must be a positive whole number of cells. Anything else --
+ * blank, non-numeric, zero or negative -- is not a usable size, so fall back
+ * rather than hand node-pty a dimension it cannot honour. */
+function positiveIntOr(value: string | undefined, fallback: number): number {
+  const n = Number(value);
+  return Number.isInteger(n) && n > 0 ? n : fallback;
+}
+
 export class PtyHandler {
   private _ptyProcess: IPty | null = null;
   private _alive = false;
@@ -83,8 +91,8 @@ export class PtyHandler {
 
       this._ptyProcess = spawn(command[0]!, command.slice(1), {
         name: processEnv.TERM ?? PTY_TERM,
-        cols: Number(processEnv.COLUMNS) || PTY_COLS,
-        rows: Number(processEnv.LINES) || PTY_ROWS,
+        cols: positiveIntOr(processEnv.COLUMNS, PTY_COLS),
+        rows: positiveIntOr(processEnv.LINES, PTY_ROWS),
         cwd: cwd ?? process.cwd(),
         env: processEnv,
       });

From 066a4da0fcc7d94f17f0c2b5823528710d2f1ab6 Mon Sep 17 00:00:00 2001
From: kartikloops <kartik55463@gmail.com>
Date: Sun, 16 Aug 2026 00:55:06 -0600
Subject: [PATCH 09/12] feat(config): auto-detect PATH and ORFS_FLOW_PATH at
 startup

Sessions failed for anyone launching an MCP client whose PATH does not
already include openroad (common with GUI-launched clients), since the
server only ever spawned by name and had no fallback. applyInheritedEnv now
runs before settings/logging init: if openroad is not already reachable, it
merges in the login-shell PATH and common install locations
(/opt/homebrew/bin, conda, local OpenROAD builds), without overriding an
explicit PATH that already resolves it.

Updates README, CROSS_PLATFORM, and SECURITY docs to match, and drops the
old "find your paths and edit .env" setup section since the common case no
longer needs it.
---
 .env.example                                 |   6 +-
 README.md                                    |  49 +++--
 docs/CROSS_PLATFORM.md                       |   2 +-
 docs/SECURITY.md                             |   4 +-
 typescript/README.md                         |   2 +-
 typescript/__tests__/config/path_env.test.ts | 184 ++++++++++++++++++
 typescript/src/config/path_env.ts            | 186 +++++++++++++++++++
 typescript/src/main.ts                       |   7 +-
 8 files changed, 406 insertions(+), 34 deletions(-)
 create mode 100644 typescript/__tests__/config/path_env.test.ts
 create mode 100644 typescript/src/config/path_env.ts

diff --git a/.env.example b/.env.example
index da45c5c..85b58d7 100644
--- a/.env.example
+++ b/.env.example
@@ -1,6 +1,8 @@
-# Copy to .env and fill in your paths.
+# Local-development reference only. The published `npx openroad-mcp` package
+# does not load this file. PATH and ORFS_FLOW_PATH are inherited/auto-detected
+# at startup; set these only to override an unusual install prefix.
 
-# Directory containing the openroad binary (find with: dirname $(which openroad))
+# Directory containing the openroad binary (find with: dirname "$(command -v openroad)")
 PATH=/path/to/openroad/bin:$PATH
 
 # flow/ directory inside your ORFS checkout (only needed for report image tools)
diff --git a/README.md b/README.md
index d4e6bf4..a7f54aa 100644
--- a/README.md
+++ b/README.md
@@ -43,27 +43,9 @@ To use this MCP server, you need the server runtime, plus the underlying OpenROA
 
 For platform-specific Node.js and C++ toolchain setup instructions, see the **[Cross-Platform Build Guide](docs/CROSS_PLATFORM.md)**.
 
-Before configuring your MCP client, you must provide the server with your local paths to OpenROAD and ORFS.
+You do **not** need to clone this repo or pass path environment variables in the common case. The published `npx` package does not read a `.env` file.
 
-### 1. Find Your Paths
-Run these commands in your terminal to locate the necessary directories:
-
-```bash
-# Get your OpenROAD binary directory:
-dirname $(which openroad)
-
-# Check for the default ORFS flow directory:
-ls ~/OpenROAD-flow-scripts/flow
-# (If not found, search with: find ~ -maxdepth 4 -type d -name flow -path "*/OpenROAD-flow-scripts/*" 2>/dev/null)
-```
-
-### 2. Set Up Environment Variables
-Copy the example environment file and fill in the paths you just found:
-
-```bash
-cp .env.example .env
-```
-Edit `.env` to ensure `PATH` includes your OpenROAD binary directory, and `ORFS_FLOW_PATH` points to your ORFS checkout. Pass these values into your MCP client's configuration via the `env` block.
+On startup the server inherits the MCP client's environment, then fills `PATH` the same way `which openroad` would: current `PATH`, then your login-shell `PATH`, then common install locations (`/opt/homebrew/bin`, conda, local OpenROAD builds). `ORFS_FLOW_PATH` defaults to `~/OpenROAD-flow-scripts/flow`, and is also detected when ORFS sits next to the `openroad` binary.
 
 ## Supported MCP Clients
 
@@ -76,16 +58,27 @@ Here is the standard base configuration used across most clients:
 }
 ```
 
-Find your specific client below for the exact configuration snippet and file location. 
-
-*(Note: If your client supports environment variables in the config, inject your `PATH` and `ORFS_FLOW_PATH` directly in the snippet).*
+Find your specific client below for the exact configuration snippet and file location.
 
 <details><summary><b>Claude Code</b></summary>
 
 ```bash
 claude mcp add --transport stdio openroad-mcp -- npx -y openroad-mcp
 ```
-Or add the standard config to `.claude/settings.json`.
+
+Or add the standard config to `.mcp.json` / `.claude/settings.json`.
+
+If a GUI-launched client still cannot find `openroad`, pass an override. Use `command -v` so you do not hard-code paths:
+
+```bash
+claude mcp add \
+  --env PATH="$(dirname "$(command -v openroad)"):${PATH}" \
+  --env ORFS_FLOW_PATH="${HOME}/OpenROAD-flow-scripts/flow" \
+  --transport stdio openroad-mcp \
+  -- npx -y openroad-mcp
+```
+
+Put `--transport` between `--env` and the server name so the CLI does not treat the name as another `KEY=value` pair.
 </details>
 
 <details><summary><b>Claude Desktop</b></summary>
@@ -184,15 +177,17 @@ Once configured, your AI assistant will have access to the following tools. For
 ## Troubleshooting
 
 - **The server fails to start**: Ensure you have Node.js 22+. Older versions will fail.
-- **Session creation fails**: Confirm `openroad` is on your `PATH`. The server spawns it by name; if it's missing, session creation (not startup) will fail.
+- **Session creation fails**: Confirm `command -v openroad` works in a terminal. The server inherits PATH and searches common install locations; if your prefix is unusual, pass `PATH` with `--env` as shown in the Claude Code section.
 - **Commands rejected with CommandBlocked**: You sent a state-modifying command to `interactive_openroad_query`. Use `interactive_openroad_exec` instead.
-- **Report images not found**: Make sure `ORFS_FLOW_PATH` points to your ORFS `flow/` directory.
+- **Report images not found**: The server defaults to `~/OpenROAD-flow-scripts/flow`. If ORFS lives elsewhere, set `ORFS_FLOW_PATH` in the MCP client's `env` block (not a `.env` file).
 
 To get more detail, set `LOG_LEVEL=DEBUG` in the server's environment.
 
 ## Development
 
-Clone the repository and run:
+Clone the repository. `.env.example` is a local-dev reference only; copy it to `.env` if you use direnv or similar. The server still reads `process.env` (the MCP client's `env` block), not the file.
+
+Then run:
 ```bash
 cd typescript
 npm install
diff --git a/docs/CROSS_PLATFORM.md b/docs/CROSS_PLATFORM.md
index 39ca330..2b31a2f 100644
--- a/docs/CROSS_PLATFORM.md
+++ b/docs/CROSS_PLATFORM.md
@@ -67,7 +67,7 @@ xcode-select --install
 
 | Issue | Workaround |
 |-------|------------|
-| OpenROAD is not on `PATH` after Homebrew install | Add `/opt/homebrew/bin` to your shell's `PATH`, or set `OPENROAD_ALLOWED_COMMANDS=openroad` and use the full path in your session config. |
+| OpenROAD is not on `PATH` after Homebrew install | The server prepends `/opt/homebrew/bin` automatically. If session creation still fails, pass `PATH="$(dirname "$(command -v openroad)"):${PATH}"` in the MCP client `env` block. |
 | `node-pty` rebuild fails after a Node upgrade | `cd typescript && npm rebuild` |
 | `sharp` fails with "dyld: Library not loaded" | `cd typescript && npm rebuild --update-binary` |
 
diff --git a/docs/SECURITY.md b/docs/SECURITY.md
index a4142ed..8a457b0 100644
--- a/docs/SECURITY.md
+++ b/docs/SECURITY.md
@@ -163,13 +163,15 @@ All variables are read at startup by [`typescript/src/config/settings.ts`](../ty
 | `OPENROAD_ALLOWED_COMMANDS` | string (comma-separated) | `openroad` | PTY spawn executable allowlist |
 | `OPENROAD_ENABLE_COMMAND_VALIDATION` | bool | `true` | Enables/disables `PtyHandler.validateCommand` |
 | `OPENROAD_WHITELIST_ENABLED` | bool | `true` | Enables/disables the Tcl command whitelist |
-| `ORFS_FLOW_PATH` | path | `~/OpenROAD-flow-scripts/flow` | Root for ORFS reports; tilde-expanded at runtime |
+| `ORFS_FLOW_PATH` | path | `~/OpenROAD-flow-scripts/flow` (auto-detected if unset) | Root for ORFS reports; tilde-expanded at runtime |
 | `LOG_LEVEL` | string | `INFO` | Root pino logger level (`DEBUG`, `INFO`, `WARNING`, `ERROR`, `CRITICAL`) |
 | `LOG_FORMAT` | string | (N/A) | Unused; logging uses pino with a fixed JSON format |
 
 CLI flags `--verbose` and `--log-level` override `LOG_LEVEL` after the settings are initialised.
 No other CLI flag overrides a Settings field.
 
+`PATH` is not a Settings field. At startup the server inherits the client's `PATH`, then (only if `openroad` is not already found) merges the login-shell `PATH` and common install locations. An explicit `PATH` in the MCP client `env` block still wins when it already contains `openroad`.
+
 ---
 
 ## Report Image Path Containment
diff --git a/typescript/README.md b/typescript/README.md
index 232d86a..3d01d53 100644
--- a/typescript/README.md
+++ b/typescript/README.md
@@ -82,7 +82,7 @@ The server reads configuration from environment variables. Key variables:
 |---|---|---|
 | `OPENROAD_MAX_SESSIONS` | `50` | Maximum concurrent sessions |
 | `OPENROAD_COMMAND_TIMEOUT` | `30.0` | Default per-command timeout in seconds |
-| `ORFS_FLOW_PATH` | `~/OpenROAD-flow-scripts/flow` | Path to ORFS flow directory |
+| `ORFS_FLOW_PATH` | `~/OpenROAD-flow-scripts/flow` (auto-detected if unset) | Path to ORFS flow directory |
 | `OPENROAD_WHITELIST_ENABLED` | `true` | Enable Tcl command whitelist |
 
 Full list: [docs/SECURITY.md#environment-variable-reference](https://github.com/The-OpenROAD-Project/openroad-mcp/blob/main/docs/SECURITY.md#environment-variable-reference)
diff --git a/typescript/__tests__/config/path_env.test.ts b/typescript/__tests__/config/path_env.test.ts
new file mode 100644
index 0000000..ab54101
--- /dev/null
+++ b/typescript/__tests__/config/path_env.test.ts
@@ -0,0 +1,184 @@
+import { describe, it, expect, beforeEach, afterEach } from "vitest";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+import {
+  mergePathDirs,
+  findExecutable,
+  parseLoginShellPath,
+  enrichInheritedEnv,
+  defaultExtraBinDirs,
+  defaultOrfsFlowCandidates,
+  type PathEnvDeps,
+} from "../../src/config/path_env.js";
+
+function makeExecutable(dir: string, name: string): string {
+  fs.mkdirSync(dir, { recursive: true });
+  const filePath = path.join(dir, name);
+  fs.writeFileSync(filePath, "#!/bin/sh\n");
+  fs.chmodSync(filePath, 0o755);
+  return filePath;
+}
+
+function makeOrfsFlow(root: string): string {
+  const flow = path.join(root, "flow");
+  fs.mkdirSync(path.join(flow, "designs"), { recursive: true });
+  fs.mkdirSync(path.join(flow, "platforms"), { recursive: true });
+  return flow;
+}
+
+describe("path_env", () => {
+  let tmpDir: string;
+
+  beforeEach(() => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "openroad-path-env-"));
+  });
+
+  afterEach(() => {
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+  });
+
+  describe("mergePathDirs", () => {
+    it("deduplicates and skips empty segments while preserving first-seen order", () => {
+      expect(mergePathDirs(
+        ["/a", "/b"].join(path.delimiter),
+        ["/b", "/c"],
+        "",
+        `  ${path.delimiter}/a`,
+      )).toBe(["/a", "/b", "/c"].join(path.delimiter));
+    });
+  });
+
+  describe("findExecutable", () => {
+    it("returns the first executable named on PATH", () => {
+      const bin = path.join(tmpDir, "bin");
+      const openroad = makeExecutable(bin, "openroad");
+      expect(findExecutable("openroad", ["/usr/bin", bin].join(path.delimiter))).toBe(openroad);
+    });
+
+    it("returns undefined when the binary is missing", () => {
+      expect(findExecutable("openroad", "/usr/bin")).toBeUndefined();
+    });
+  });
+
+  describe("parseLoginShellPath", () => {
+    it("reads the marker even when the shell printed a banner first", () => {
+      expect(parseLoginShellPath("Welcome!\n__OPENROAD_MCP_PATH__=/opt/homebrew/bin:/usr/bin\n")).toBe(
+        "/opt/homebrew/bin:/usr/bin",
+      );
+    });
+
+    it("returns undefined when the marker is absent", () => {
+      expect(parseLoginShellPath("/opt/homebrew/bin")).toBeUndefined();
+    });
+  });
+
+  describe("default location lists", () => {
+    it("puts Homebrew first on macOS", () => {
+      expect(defaultExtraBinDirs("/Users/me", "darwin")[0]).toBe("/opt/homebrew/bin");
+    });
+
+    it("includes the default ORFS checkout", () => {
+      expect(defaultOrfsFlowCandidates("/home/me")[0]).toBe(
+        path.join("/home/me", "OpenROAD-flow-scripts", "flow"),
+      );
+    });
+  });
+
+  describe("enrichInheritedEnv", () => {
+    function deps(overrides: Partial<PathEnvDeps> & { env: NodeJS.ProcessEnv }): PathEnvDeps {
+      return {
+        isExecutable: (p) => {
+          try {
+            fs.accessSync(p, fs.constants.X_OK);
+            return fs.statSync(p).isFile();
+          } catch {
+            return false;
+          }
+        },
+        dirExists: (p) => {
+          try {
+            return fs.statSync(p).isDirectory();
+          } catch {
+            return false;
+          }
+        },
+        extraBinDirs: () => [],
+        extraOrfsDirs: () => [],
+        readLoginShellPath: () => undefined,
+        ...overrides,
+      };
+    }
+
+    it("leaves PATH unchanged when openroad is already on it", () => {
+      const bin = path.join(tmpDir, "already");
+      makeExecutable(bin, "openroad");
+      const current = [bin, "/usr/bin"].join(path.delimiter);
+      const env: NodeJS.ProcessEnv = { PATH: current };
+      let loginCalled = false;
+      const result = enrichInheritedEnv(
+        deps({
+          env,
+          extraBinDirs: () => ["/opt/homebrew/bin"],
+          readLoginShellPath: () => {
+            loginCalled = true;
+            return "/opt/homebrew/bin";
+          },
+        }),
+      );
+      expect(env.PATH).toBe(current);
+      expect(result.openroad).toBe(path.join(bin, "openroad"));
+      expect(loginCalled).toBe(false);
+    });
+
+    it("prepends the login-shell directory that contains openroad", () => {
+      const loginBin = path.join(tmpDir, "login-bin");
+      makeExecutable(loginBin, "openroad");
+      const env: NodeJS.ProcessEnv = { PATH: "/usr/bin:/bin" };
+      const result = enrichInheritedEnv(
+        deps({
+          env,
+          readLoginShellPath: () => [loginBin, "/usr/bin"].join(path.delimiter),
+        }),
+      );
+      expect(result.openroad).toBe(path.join(loginBin, "openroad"));
+      expect(env.PATH?.split(path.delimiter)[0]).toBe(loginBin);
+    });
+
+    it("finds openroad in extra bin dirs used by Homebrew/conda/local builds", () => {
+      const brewBin = path.join(tmpDir, "homebrew");
+      makeExecutable(brewBin, "openroad");
+      const env: NodeJS.ProcessEnv = { PATH: "/usr/bin" };
+      const result = enrichInheritedEnv(deps({ env, extraBinDirs: () => [brewBin] }));
+      expect(result.openroad).toBe(path.join(brewBin, "openroad"));
+      expect(env.PATH?.split(path.delimiter)[0]).toBe(brewBin);
+    });
+
+    it("does not overwrite an explicit ORFS_FLOW_PATH", () => {
+      const env: NodeJS.ProcessEnv = { PATH: "/usr/bin", ORFS_FLOW_PATH: "/custom/flow" };
+      const flow = makeOrfsFlow(path.join(tmpDir, "OpenROAD-flow-scripts"));
+      const result = enrichInheritedEnv(deps({ env, extraOrfsDirs: () => [flow] }));
+      expect(result.orfsFlowPath).toBe("/custom/flow");
+      expect(env.ORFS_FLOW_PATH).toBe("/custom/flow");
+    });
+
+    it("sets ORFS_FLOW_PATH when a candidate flow tree exists", () => {
+      const flow = makeOrfsFlow(path.join(tmpDir, "OpenROAD-flow-scripts"));
+      const env: NodeJS.ProcessEnv = { PATH: "/usr/bin" };
+      const result = enrichInheritedEnv(deps({ env, extraOrfsDirs: () => [flow] }));
+      expect(result.orfsFlowPath).toBe(flow);
+      expect(env.ORFS_FLOW_PATH).toBe(flow);
+    });
+
+    it("detects ORFS next to a local openroad install", () => {
+      const root = path.join(tmpDir, "OpenROAD-flow-scripts");
+      const flow = makeOrfsFlow(root);
+      const bin = path.join(root, "tools", "install", "OpenROAD", "bin");
+      makeExecutable(bin, "openroad");
+      const env: NodeJS.ProcessEnv = { PATH: bin };
+      const result = enrichInheritedEnv(deps({ env, extraOrfsDirs: () => [] }));
+      expect(result.orfsFlowPath).toBe(flow);
+      expect(env.ORFS_FLOW_PATH).toBe(flow);
+    });
+  });
+});
diff --git a/typescript/src/config/path_env.ts b/typescript/src/config/path_env.ts
new file mode 100644
index 0000000..2abf9dd
--- /dev/null
+++ b/typescript/src/config/path_env.ts
@@ -0,0 +1,186 @@
+import { spawnSync } from "node:child_process";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+const LOGIN_SHELL_TIMEOUT_MS = 2500;
+const LOGIN_SHELL_PATH_MARKER = "__OPENROAD_MCP_PATH__=";
+
+export type PathEnvDeps = {
+  env: NodeJS.ProcessEnv;
+  isExecutable: (filePath: string) => boolean;
+  dirExists: (dirPath: string) => boolean;
+  extraBinDirs: () => string[];
+  extraOrfsDirs: () => string[];
+  readLoginShellPath: () => string | undefined;
+};
+
+export function isExecutableFile(filePath: string): boolean {
+  try {
+    fs.accessSync(filePath, fs.constants.X_OK);
+    return fs.statSync(filePath).isFile();
+  } catch {
+    return false;
+  }
+}
+
+export function dirExists(dirPath: string): boolean {
+  try {
+    return fs.statSync(dirPath).isDirectory();
+  } catch {
+    return false;
+  }
+}
+
+export function defaultExtraBinDirs(home: string, platform: NodeJS.Platform): string[] {
+  const dirs = [
+    path.join(home, ".local", "bin"),
+    path.join(home, "OpenROAD", "build", "src"),
+    path.join(home, "OpenROAD-flow-scripts", "tools", "install", "OpenROAD", "bin"),
+    path.join(home, "miniconda3", "bin"),
+    path.join(home, "anaconda3", "bin"),
+    path.join(home, "miniforge3", "bin"),
+    path.join(home, "mambaforge", "bin"),
+    "/opt/conda/bin",
+  ];
+  if (platform === "darwin") {
+    dirs.unshift("/opt/homebrew/bin", "/usr/local/bin");
+  } else if (platform !== "win32") {
+    dirs.unshift("/usr/local/bin");
+  }
+  return dirs;
+}
+
+export function defaultOrfsFlowCandidates(home: string): string[] {
+  return [
+    path.join(home, "OpenROAD-flow-scripts", "flow"),
+    path.join(home, "src", "OpenROAD-flow-scripts", "flow"),
+    path.join(home, "tools", "OpenROAD-flow-scripts", "flow"),
+  ];
+}
+
+export function mergePathDirs(...groups: Array<string | string[] | undefined>): string {
+  const seen = new Set<string>();
+  const out: string[] = [];
+  for (const group of groups) {
+    if (group === undefined) continue;
+    const dirs = Array.isArray(group) ? group : group.split(path.delimiter);
+    for (const dir of dirs) {
+      const trimmed = dir.trim();
+      if (!trimmed || seen.has(trimmed)) continue;
+      seen.add(trimmed);
+      out.push(trimmed);
+    }
+  }
+  return out.join(path.delimiter);
+}
+
+export function findExecutable(
+  name: string,
+  pathEnv: string,
+  isExecutable: (filePath: string) => boolean = isExecutableFile,
+): string | undefined {
+  for (const dir of pathEnv.split(path.delimiter)) {
+    const trimmed = dir.trim();
+    if (!trimmed) continue;
+    const candidate = path.join(trimmed, name);
+    if (isExecutable(candidate)) return candidate;
+  }
+  return undefined;
+}
+
+export function parseLoginShellPath(stdout: string, marker = LOGIN_SHELL_PATH_MARKER): string | undefined {
+  const idx = stdout.lastIndexOf(marker);
+  if (idx === -1) return undefined;
+  const rest = stdout.slice(idx + marker.length);
+  const end = rest.search(/[\r\n]/);
+  const value = (end === -1 ? rest : rest.slice(0, end)).trim();
+  return value.length > 0 ? value : undefined;
+}
+
+export function readLoginShellPath(): string | undefined {
+  if (process.platform === "win32") return undefined;
+  const shell = process.env["SHELL"] || (process.platform === "darwin" ? "/bin/zsh" : "/bin/bash");
+  try {
+    const result = spawnSync(shell, ["-lc", `printf '%s' '${LOGIN_SHELL_PATH_MARKER}'"$PATH"`], {
+      encoding: "utf8",
+      timeout: LOGIN_SHELL_TIMEOUT_MS,
+      env: { ...process.env, TERM: "dumb", SHELL: shell },
+      stdio: ["ignore", "pipe", "pipe"],
+    });
+    if (!result.stdout) return undefined;
+    return parseLoginShellPath(result.stdout);
+  } catch {
+    return undefined;
+  }
+}
+
+function orfsNearOpenroad(
+  openroadBin: string,
+  dirExistsFn: (dirPath: string) => boolean,
+): string | undefined {
+  let dir = path.dirname(openroadBin);
+  for (let i = 0; i < 8; i++) {
+    const flow = path.join(dir, "flow");
+    if (dirExistsFn(path.join(flow, "designs")) && dirExistsFn(path.join(flow, "platforms"))) {
+      return flow;
+    }
+    const parent = path.dirname(dir);
+    if (parent === dir) break;
+    dir = parent;
+  }
+  return undefined;
+}
+
+function looksLikeOrfsFlow(flowPath: string, dirExistsFn: (dirPath: string) => boolean): boolean {
+  return dirExistsFn(path.join(flowPath, "designs")) && dirExistsFn(path.join(flowPath, "platforms"));
+}
+
+export function defaultPathEnvDeps(env: NodeJS.ProcessEnv = process.env): PathEnvDeps {
+  const homedir = (): string => os.homedir();
+  const platform = process.platform;
+  return {
+    env,
+    isExecutable: isExecutableFile,
+    dirExists,
+    extraBinDirs: () => defaultExtraBinDirs(homedir(), platform),
+    extraOrfsDirs: () => defaultOrfsFlowCandidates(homedir()),
+    readLoginShellPath,
+  };
+}
+
+export function enrichInheritedEnv(deps: PathEnvDeps): {
+  path: string;
+  openroad: string | undefined;
+  orfsFlowPath: string | undefined;
+} {
+  const currentPath = deps.env["PATH"] ?? "";
+  let openroad = findExecutable("openroad", currentPath, deps.isExecutable);
+  let nextPath = currentPath;
+
+  if (!openroad) {
+    nextPath = mergePathDirs(deps.readLoginShellPath(), currentPath, deps.extraBinDirs());
+    openroad = findExecutable("openroad", nextPath, deps.isExecutable);
+    if (openroad) {
+      nextPath = mergePathDirs([path.dirname(openroad)], nextPath);
+    }
+    deps.env["PATH"] = nextPath;
+  }
+
+  const existingOrfs = deps.env["ORFS_FLOW_PATH"]?.trim();
+  let orfsFlowPath = existingOrfs && existingOrfs.length > 0 ? existingOrfs : undefined;
+  if (!orfsFlowPath) {
+    const near = openroad ? orfsNearOpenroad(openroad, deps.dirExists) : undefined;
+    const found = near ?? deps.extraOrfsDirs().find((p) => looksLikeOrfsFlow(p, deps.dirExists));
+    if (found) {
+      orfsFlowPath = found;
+      deps.env["ORFS_FLOW_PATH"] = found;
+    }
+  }
+
+  return { path: deps.env["PATH"] ?? nextPath, openroad, orfsFlowPath };
+}
+
+export function applyInheritedEnv(env: NodeJS.ProcessEnv = process.env): void {
+  enrichInheritedEnv(defaultPathEnvDeps(env));
+}
diff --git a/typescript/src/main.ts b/typescript/src/main.ts
index b8cc382..7c6f02a 100644
--- a/typescript/src/main.ts
+++ b/typescript/src/main.ts
@@ -1,12 +1,14 @@
 #!/usr/bin/env node
 import { parseCliArgs } from "./config/cli.js";
+import { applyInheritedEnv } from "./config/path_env.js";
 import { initSettings } from "./config/settings.js";
 import { EXIT_CODE_ERROR } from "./constants.js";
 import { ValidationError } from "./exceptions.js";
 
 /**
- * Entry point. The eager work (settings validation, CLI parsing) runs before
- * any module that reads settings or builds a logger is imported.
+ * Entry point. The eager work (PATH inheritance, 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,
@@ -17,6 +19,7 @@ import { ValidationError } from "./exceptions.js";
  */
 async function main(): Promise<void> {
   try {
+    applyInheritedEnv();
     initSettings();
   } catch (e) {
     throw new ValidationError(e instanceof Error ? e.message : String(e));

From f6bef9573aebb38cc14ce0f0cb0d8c3ab8df8c6f Mon Sep 17 00:00:00 2001
From: kartikloops <kartik55463@gmail.com>
Date: Sun, 16 Aug 2026 01:03:24 -0600
Subject: [PATCH 10/12] fix(tests): update performance-suite session mocks to
 match runCommand

The sentinel-based session rewrite replaced manager.ts's sendCommand +
readOutput call pair with a single session.runCommand(), but the
benchmarks/memory_monitoring mock sessions still only implemented the old
methods, so every executeCommand call in those suites threw "runCommand is
not a function" in CI's Docker test stage.
---
 typescript/__tests__/performance/benchmarks.test.ts       | 8 +++-----
 .../__tests__/performance/memory_monitoring.test.ts       | 3 +--
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/typescript/__tests__/performance/benchmarks.test.ts b/typescript/__tests__/performance/benchmarks.test.ts
index 17fa222..6232901 100644
--- a/typescript/__tests__/performance/benchmarks.test.ts
+++ b/typescript/__tests__/performance/benchmarks.test.ts
@@ -14,8 +14,7 @@ interface MockSession {
   checkAlive: Mock;
   start: Mock;
   verifyResponsive: Mock;
-  sendCommand: Mock;
-  readOutput: Mock;
+  runCommand: Mock;
   getInfo: Mock;
   getDetailedMetrics: Mock;
   getCommandHistory: Mock;
@@ -45,8 +44,7 @@ function makeMockSession(sessionId: string): MockSession {
     checkAlive: vi.fn().mockReturnValue(true),
     start: vi.fn().mockResolvedValue(undefined),
     verifyResponsive: vi.fn().mockResolvedValue(undefined),
-    sendCommand: vi.fn().mockResolvedValue(undefined),
-    readOutput: vi.fn().mockResolvedValue({
+    runCommand: vi.fn().mockResolvedValue({
       output: "ok",
       sessionId,
       timestamp: new Date().toISOString(),
@@ -247,7 +245,7 @@ describe("Stress Tests", () => {
     }
 
     const session = created[0]!;
-    expect(session.readOutput.mock.calls.length).toBe(TOTAL);
+    expect(session.runCommand.mock.calls.length).toBe(TOTAL);
     expect(session.checkAlive()).toBe(true);
   });
 
diff --git a/typescript/__tests__/performance/memory_monitoring.test.ts b/typescript/__tests__/performance/memory_monitoring.test.ts
index 41ec4b5..3e73267 100644
--- a/typescript/__tests__/performance/memory_monitoring.test.ts
+++ b/typescript/__tests__/performance/memory_monitoring.test.ts
@@ -13,8 +13,7 @@ function makeMockSession(sessionId: string) {
     checkAlive: vi.fn().mockReturnValue(true),
     start: vi.fn().mockResolvedValue(undefined),
     verifyResponsive: vi.fn().mockResolvedValue(undefined),
-    sendCommand: vi.fn().mockResolvedValue(undefined),
-    readOutput: vi.fn().mockResolvedValue({
+    runCommand: vi.fn().mockResolvedValue({
       output: "ok",
       sessionId,
       timestamp: new Date().toISOString(),

From 37e6233e5b07a44e2a2f5556dbc99a313fea5284 Mon Sep 17 00:00:00 2001
From: kartikloops <kartik55463@gmail.com>
Date: Sun, 16 Aug 2026 19:54:45 -0600
Subject: [PATCH 11/12] fix(session): treat OpenROAD [ERROR CODE] lines as
 command failures
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

repair_timing returned [ERROR RSZ-0089] with error: null because the
detector only recognised "Error:" / "ERROR:" with a colon. OpenROAD's
native log format has no colon, so a hard tool failure was reported as
success. Match [ERROR …] and [FATAL …] before the generic rules, and
leave [WARNING …] as a non-error.
---
 .../__tests__/interactive/session.test.ts     | 21 +++++++++++++++++++
 typescript/src/interactive/session.ts         | 10 +++++++--
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/typescript/__tests__/interactive/session.test.ts b/typescript/__tests__/interactive/session.test.ts
index 1f7a298..523fe37 100644
--- a/typescript/__tests__/interactive/session.test.ts
+++ b/typescript/__tests__/interactive/session.test.ts
@@ -761,6 +761,27 @@ describe("InteractiveSession", () => {
       expect(result.error).toMatch(/Fatal error/);
     });
 
+    it("detects OpenROAD [ERROR CODE-NNNN] lines, which have no colon", async () => {
+      // The dry-run failure: repair_timing printed [ERROR RSZ-0089] and the
+      // tool still returned error: null because only "Error:" / "ERROR:" were
+      // recognised. Warnings must stay non-errors.
+      await session.outputBuffer.append(
+        "[WARNING EST-0027] no estimated parasitics. Using wire load models.\n" +
+          "[ERROR RSZ-0089] Could not find a resistance value for any corner. Cannot evaluate max wire length for buffer.\n",
+      );
+      const result = await session.readOutput(100);
+      expect(result.error).toMatch(/OpenROAD RSZ-0089/);
+      expect(result.error).toMatch(/resistance value/);
+    });
+
+    it("does not treat an OpenROAD [WARNING] line as a command failure", async () => {
+      await session.outputBuffer.append(
+        "[WARNING EST-0027] no estimated parasitics. Using wire load models.\n",
+      );
+      const result = await session.readOutput(100);
+      expect(result.error).toBeNull();
+    });
+
     it("detects invalid command name pattern", async () => {
       await session.outputBuffer.append('invalid command name "foo_bar"\n');
       const result = await session.readOutput(100);
diff --git a/typescript/src/interactive/session.ts b/typescript/src/interactive/session.ts
index 2d622e4..84ad0c2 100644
--- a/typescript/src/interactive/session.ts
+++ b/typescript/src/interactive/session.ts
@@ -67,6 +67,11 @@ const ERROR_PATTERNS: Array<[RegExp, string]> = [
   [/Error: net ([^\s]+) not found/i, "Net not found: {0}"],
   [/Error: clock ([^\s]+) not found/i, "Clock not found: {0}"],
   [/Error: no clocks defined/i, "No clocks defined"],
+  // OpenROAD's native log format: [ERROR RSZ-0089] message. Must precede the
+  // generic "Error:" rules — those look for a colon, so this form was being
+  // returned as success with the failure sitting in the output text.
+  [/\[ERROR\s+([A-Z]+-\d+)\]\s*([^\r\n]+)/, "OpenROAD {0}: {1}"],
+  [/\[FATAL\s+([A-Z]+-\d+)\]\s*([^\r\n]+)/, "OpenROAD fatal {0}: {1}"],
   [/Error: (.+?)(?:\r?\n|$)/im, "Error: {0}"],
   [/ERROR: (.+?)(?:\r?\n|$)/m, "Error: {0}"],
   [/FATAL: (.+?)(?:\r?\n|$)/m, "Fatal error: {0}"],
@@ -592,10 +597,11 @@ export class InteractiveSession {
     for (const [pattern, template] of ERROR_PATTERNS) {
       const match = pattern.exec(output);
       if (match) {
-        const capture = match[1];
         // Function replacement so `$&`/`$1`/`$$` inside the captured error text
         // are inserted literally, not reinterpreted as replacement patterns.
-        return capture ? template.replace("{0}", () => capture.trim()) : template;
+        return template
+          .replace("{0}", () => (match[1] ?? "").trim())
+          .replace("{1}", () => (match[2] ?? "").trim());
       }
     }
 

From 0b9919c6998b12618aa60bad49c11ac1deef9ecc Mon Sep 17 00:00:00 2001
From: kartikloops <kartik55463@gmail.com>
Date: Thu, 20 Aug 2026 23:03:12 -0600
Subject: [PATCH 12/12] fix(session): lower startup handshake timeout to 10s

60s was inherited from the pre-fix CR/LF bug's worst case; with that bug
fixed, a healthy session responds sub-second and this only needs to bound
how long to wait before giving up on a broken one.
---
 typescript/src/constants.ts    | 11 ++++++-----
 typescript/src/core/manager.ts |  4 ++--
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/typescript/src/constants.ts b/typescript/src/constants.ts
index 5abcc77..7d3503b 100644
--- a/typescript/src/constants.ts
+++ b/typescript/src/constants.ts
@@ -51,9 +51,10 @@ export const PTY_LINE_TERMINATOR = "\r";
 
 /**
  * Budget for the startup handshake. This has to cover the whole cold start --
- * loading a large OpenROAD binary and reaching its prompt -- not just the probe
- * round-trip, because spawning does not wait for readiness. Generous on
- * purpose: the handshake exists to catch a session that will never respond, and
- * failing a slow-but-healthy start would be worse than the bug it prevents.
+ * loading OpenROAD and reaching its prompt -- not just the probe round-trip,
+ * because spawning does not wait for readiness. OpenROAD is native C++ and a
+ * healthy start is sub-second locally, so this is a few seconds of margin for
+ * a slow disk/container, not an expected duration: the handshake exists to
+ * catch a session that will never respond, not to tolerate a slow one.
  */
-export const SESSION_HANDSHAKE_TIMEOUT_MS = 60_000;
+export const SESSION_HANDSHAKE_TIMEOUT_MS = 10_000;
diff --git a/typescript/src/core/manager.ts b/typescript/src/core/manager.ts
index 5d42d8e..b14a657 100644
--- a/typescript/src/core/manager.ts
+++ b/typescript/src/core/manager.ts
@@ -42,8 +42,8 @@ export class OpenROADManager {
   private readonly defaultBufferSize: number;
   // Covers the whole cold start, not just the probe round-trip: start() only
   // spawns, so OpenROAD still has to load and reach its prompt inside this
-  // budget. A container on a cold filesystem is comfortably slower than the
-  // handshake's own default.
+  // budget. See SESSION_HANDSHAKE_TIMEOUT_MS for why this is a few seconds of
+  // margin, not a generous allowance.
   private readonly handshakeTimeoutMs: number = SESSION_HANDSHAKE_TIMEOUT_MS;
 
   constructor(maxSessions?: number) {