diff --git a/typescript/__tests__/config/command_whitelist.test.ts b/typescript/__tests__/config/command_whitelist.test.ts index 9645767..0b426c7 100644 --- a/typescript/__tests__/config/command_whitelist.test.ts +++ b/typescript/__tests__/config/command_whitelist.test.ts @@ -62,10 +62,16 @@ describe("pattern sets", () => { expect(READONLY_PATTERNS).toContain("check_*"); }); - it("READONLY contains Tcl builtins", () => { + it("READONLY contains safe Tcl builtins", () => { expect(READONLY_PATTERNS).toContain("puts"); - expect(READONLY_PATTERNS).toContain("foreach"); expect(READONLY_PATTERNS).toContain("set"); + expect(READONLY_PATTERNS).toContain("expr"); + }); + + it("READONLY excludes body-eval builtins (if/for/foreach/while/proc/catch/namespace)", () => { + for (const verb of ["if", "for", "foreach", "while", "proc", "catch", "namespace", "uplevel"]) { + expect(READONLY_PATTERNS).not.toContain(verb); + } }); it("EXEC_ONLY contains set_*/read_*/write_* globs", () => { @@ -195,6 +201,86 @@ describe("isQueryCommand", () => { it("splits on semicolons and rejects the offending verb", () => { expect(isQueryCommand("report_wns; global_placement")).toEqual([false, "global_placement"]); }); + + it("body-eval: blocks catch wrapping exec (finding 1)", () => { + expect(isQueryCommand("catch { exec ls }")).toEqual([false, "catch"]); + }); + + it("body-eval: blocks if wrapping exec (finding 1)", () => { + expect(isQueryCommand("if 1 { exec ls }")).toEqual([false, "if"]); + }); + + it("body-eval: blocks foreach wrapping exec (finding 1)", () => { + expect(isQueryCommand("foreach x {a} { exec ls }")).toEqual([false, "foreach"]); + }); + + it("bracket: blocks set x [exec ls] via bracket scan (finding 2)", () => { + expect(isQueryCommand("set x [exec ls]")).toEqual([false, "exec"]); + }); + + it("bracket: blocks set x [::exec ls] with namespace-qualified command", () => { + expect(isQueryCommand("set x [::exec ls]")).toEqual([false, "exec"]); + }); + + it("bracket: blocks expr {[exec ls]} via bracket scan (finding 2)", () => { + expect(isQueryCommand("expr {[exec ls]}")).toEqual([false, "exec"]); + }); + + it("bracket: allows puts [report_wns] when bracket verb is read-only (finding 2)", () => { + expect(isQueryCommand("puts [report_wns]")).toEqual([true, null]); + }); + + it("bracket: blocks puts [global_placement] (exec-only in bracket)", () => { + expect(isQueryCommand("puts [global_placement]")).toEqual([false, "global_placement"]); + }); + + it("semicolon in quoted string is not a statement separator (finding 3)", () => { + expect(isQueryCommand('puts "hello; world"')).toEqual([true, null]); + }); + + it("semicolon inside braces is not a statement separator (finding 3)", () => { + expect(isQueryCommand("report_checks {a; b}")).toEqual([true, null]); + }); + + it("allows a bracket inside a comment line (never executed)", () => { + expect(isQueryCommand("# harmless [exec ls]")).toEqual([true, null]); + }); + + it("allows a comment with a bracket after a readonly statement", () => { + expect(isQueryCommand("report_wns\n# harmless [exec ls]")).toEqual([true, null]); + }); + + it("still blocks a bracket when # is a mid-statement arg, not a comment", () => { + expect(isQueryCommand("report_checks # [exec ls]")).toEqual([false, "exec"]); + }); + + it("unbalanced close brace cannot hide a trailing exec (depth clamp)", () => { + expect(isQueryCommand("report_wns }; exec ls")).toEqual([false, "exec"]); + }); + + it("blocks a backslash-escaped exec-only verb (\\glob -> glob)", () => { + expect(isQueryCommand("\\glob *")).toEqual([false, "glob"]); + }); + + it("blocks a backslash-escaped command in a bracket substitution", () => { + expect(isQueryCommand("puts [\\exec ls]")).toEqual([false, "exec"]); + }); + + it("blocks a variable-substituted command in a bracket ([$x] -> exec at runtime)", () => { + expect(isQueryCommand("set x exec; puts [$x ls]")).toEqual([false, "$x"]); + }); + + it("blocks a nested command substitution as the bracket command ([[...] ...])", () => { + expect(isQueryCommand("puts [[set x exec] ls]")).toEqual([false, "[set"]); + }); + + it("still allows a substituted argument that is a variable ([get_cells $x])", () => { + expect(isQueryCommand("puts [get_property [get_cells $x] area]")).toEqual([true, null]); + }); + + it("mid-word quote is literal and cannot hide a trailing exec", () => { + expect(isQueryCommand('set x a"b ; exec ls')).toEqual([false, "exec"]); + }); }); // isExecCommand @@ -258,6 +344,21 @@ describe("isExecCommand", () => { it("blocks a multiline command with one blocked verb", () => { expect(isExecCommand("global_placement\nsocket tcp localhost")).toEqual([false, "socket"]); }); + + it("unbalanced close brace cannot hide a trailing quit (depth clamp)", () => { + expect(isExecCommand("set x } ; quit")).toEqual([false, "quit"]); + }); + + it("blocks a backslash-escaped verb (\\socket -> socket)", () => { + expect(isExecCommand("\\socket localhost 1")).toEqual([false, "socket"]); + expect(isExecCommand("\\quit")).toEqual([false, "quit"]); + expect(isExecCommand("\\load /tmp/x")).toEqual([false, "load"]); + }); + + it("blocks a hex/octal-escaped verb (\\x73ocket / \\163ocket -> socket)", () => { + expect(isExecCommand("\\x73ocket localhost 1")).toEqual([false, "socket"]); + expect(isExecCommand("\\163ocket localhost 1")).toEqual([false, "socket"]); + }); }); // isCommandAllowed (backward-compat alias) diff --git a/typescript/__tests__/config/settings.test.ts b/typescript/__tests__/config/settings.test.ts index 1594a74..c85069f 100644 --- a/typescript/__tests__/config/settings.test.ts +++ b/typescript/__tests__/config/settings.test.ts @@ -115,11 +115,52 @@ describe("Settings", () => { expect(() => Settings.fromEnv()).toThrow("OPENROAD_COMMAND_TIMEOUT"); }); + it("rejects Infinity for float fields", () => { + process.env["OPENROAD_COMMAND_TIMEOUT"] = "Infinity"; + expect(() => Settings.fromEnv()).toThrow("OPENROAD_COMMAND_TIMEOUT"); + }); + + it("rejects negative floats", () => { + process.env["OPENROAD_COMMAND_TIMEOUT"] = "-1"; + expect(() => Settings.fromEnv()).toThrow("OPENROAD_COMMAND_TIMEOUT"); + }); + it("throws on invalid int", () => { process.env["OPENROAD_MAX_SESSIONS"] = "3.7"; expect(() => Settings.fromEnv()).toThrow("OPENROAD_MAX_SESSIONS"); }); + it("rejects negative integers for int fields", () => { + process.env["OPENROAD_MAX_SESSIONS"] = "-1"; + expect(() => Settings.fromEnv()).toThrow("OPENROAD_MAX_SESSIONS"); + }); + + it("rejects zero for int fields that must be positive", () => { + for (const key of [ + "OPENROAD_MAX_SESSIONS", + "OPENROAD_SESSION_QUEUE_SIZE", + "OPENROAD_DEFAULT_BUFFER_SIZE", + "OPENROAD_READ_CHUNK_SIZE", + ]) { + process.env[key] = "0"; + expect(() => Settings.fromEnv()).toThrow(key); + delete process.env[key]; + } + }); + + it("rejects zero for float fields that must be positive", () => { + for (const key of ["OPENROAD_COMMAND_TIMEOUT", "OPENROAD_SESSION_IDLE_TIMEOUT"]) { + process.env[key] = "0"; + expect(() => Settings.fromEnv()).toThrow(key); + delete process.env[key]; + } + }); + + it("allows zero for COMMAND_COMPLETION_DELAY (no delay)", () => { + process.env["OPENROAD_COMMAND_COMPLETION_DELAY"] = "0"; + expect(Settings.fromEnv().COMMAND_COMPLETION_DELAY).toBe(0); + }); + it("rejects decimal strings for int fields", () => { process.env["OPENROAD_MAX_SESSIONS"] = "50.0"; expect(() => Settings.fromEnv()).toThrow("OPENROAD_MAX_SESSIONS"); diff --git a/typescript/__tests__/core/manager.test.ts b/typescript/__tests__/core/manager.test.ts index 07c7982..dd40abb 100644 --- a/typescript/__tests__/core/manager.test.ts +++ b/typescript/__tests__/core/manager.test.ts @@ -16,7 +16,7 @@ vi.mock("../../src/interactive/session.js", () => { interface MockSession { sessionId: string; lastActivity: Date; - isAlive: Mock; + checkAlive: Mock; start: Mock; sendCommand: Mock; readOutput: Mock; @@ -31,23 +31,23 @@ interface MockSession { function makeMockSession(sessionId: string, alive = true): MockSession { const metrics: SessionDetailedMetrics = { - session_id: sessionId, + sessionId, state: SessionState.ACTIVE, - is_alive: alive, - created_at: new Date().toISOString(), - last_activity: new Date().toISOString(), - uptime_seconds: 1, - idle_seconds: 0, - commands: { total_executed: 3, current_count: 3, history_length: 3 }, - performance: { total_cpu_time: 0.5, peak_memory_mb: 10, current_memory_mb: 8 }, - buffer: { current_size: 0, max_size: 1024, utilization_percent: 0 }, - timeout: { configured_seconds: null, is_timed_out: false }, + isAlive: alive, + createdAt: new Date().toISOString(), + lastActivity: new Date().toISOString(), + uptimeSeconds: 1, + idleSeconds: 0, + commands: { totalExecuted: 3, currentCount: 3, historyLength: 3 }, + performance: { totalCpuTime: 0.5, peakMemoryMb: 10, currentMemoryMb: 8 }, + buffer: { currentSize: 0, maxSize: 1024, utilizationPercent: 0 }, + timeout: { configuredSeconds: null, isTimedOut: false }, }; return { sessionId, lastActivity: new Date(), - isAlive: vi.fn().mockReturnValue(alive), + checkAlive: vi.fn().mockReturnValue(alive), start: vi.fn().mockResolvedValue(undefined), sendCommand: vi.fn().mockResolvedValue(undefined), readOutput: vi.fn().mockResolvedValue({ @@ -187,7 +187,9 @@ describe("OpenROADManager", () => { await manager.createSession({ sessionId: "s1" }); await manager.terminateSession("s1", true); expect(created[0]!.terminate).toHaveBeenCalledWith(true); - expect(created[0]!.cleanup).toHaveBeenCalledOnce(); + // terminate() handles teardown; cleanup() must not be called again here + // (it would clear the buffer and double-tear-down the PTY). + expect(created[0]!.cleanup).not.toHaveBeenCalled(); expect(manager.getSessionCount()).toBe(0); }); }); @@ -200,6 +202,21 @@ describe("OpenROADManager", () => { expect(count).toBe(2); expect(manager.getSessionCount()).toBe(0); }); + + it("skips in-progress placeholders instead of throwing", async () => { + // A session whose start() never resolves leaves a null placeholder in the + // map (createSession holds the lock). terminateAllSessions must not try to + // terminate it (which would throw "still being created"). + MockedSession.mockImplementationOnce(function (this: unknown, sessionId: string) { + const mock = makeMockSession(sessionId); + mock.start.mockReturnValue(new Promise(() => {})); // never resolves + created.push(mock); + return mock as unknown as InteractiveSession; + } as unknown as (sessionId: string) => InteractiveSession); + void manager.createSession({ sessionId: "pending" }); + + await expect(manager.terminateAllSessions()).resolves.toBe(0); + }); }); describe("inspectSession & getSessionHistory", () => { @@ -207,7 +224,7 @@ describe("OpenROADManager", () => { await manager.createSession({ sessionId: "s1" }); const metrics = await manager.inspectSession("s1"); expect(created[0]!.getDetailedMetrics).toHaveBeenCalledOnce(); - expect(metrics.session_id).toBe("s1"); + expect(metrics.sessionId).toBe("s1"); }); it("getSessionHistory forwards limit and search", async () => { @@ -222,9 +239,9 @@ describe("OpenROADManager", () => { await manager.createSession({ sessionId: "s1" }); await manager.createSession({ sessionId: "s2" }); const metrics = await manager.sessionMetrics(); - expect(metrics.manager.total_sessions).toBe(2); - expect(metrics.manager.active_sessions).toBe(2); - expect(metrics.aggregate.total_commands).toBe(6); // 3 per mock session + expect(metrics.manager.totalSessions).toBe(2); + expect(metrics.manager.activeSessions).toBe(2); + expect(metrics.aggregate.totalCommands).toBe(6); // 3 per mock session expect(metrics.sessions).toHaveLength(2); }); }); diff --git a/typescript/__tests__/interactive/buffer.test.ts b/typescript/__tests__/interactive/buffer.test.ts index b3ca8fa..ea0a4d6 100644 --- a/typescript/__tests__/interactive/buffer.test.ts +++ b/typescript/__tests__/interactive/buffer.test.ts @@ -143,9 +143,9 @@ describe("CircularBuffer", () => { // Start waitForData (fast-path sees _dataAvailable = false and enters the Promise) const waiter = buf.waitForData(5000); - // append() fires here — before runExclusive's callback has a chance to push - // wakeUp into _resolvers. Without the re-check inside runExclusive, wakeUp - // would be pushed after append() already drained an empty _resolvers, and + // append() fires before runExclusive's callback can push wakeUp into + // _resolvers. Without the re-check inside runExclusive, wakeUp would + // land in _resolvers after append() already drained an empty list, and // the caller would wait the full 5-second timeout. await buf.append("raced data"); @@ -168,7 +168,7 @@ describe("CircularBuffer", () => { it("wakes pending waitForData() immediately with false so callers do not hang", async () => { const buf = new CircularBuffer(100); - // waitForData with a large timeout — clear() must unblock it before the timeout fires + // Large timeout: clear() must unblock waitForData before it fires. const waiter = buf.waitForData(5000); await buf.clear(); diff --git a/typescript/__tests__/interactive/pty_handler.test.ts b/typescript/__tests__/interactive/pty_handler.test.ts index b01aabc..93f4aae 100644 --- a/typescript/__tests__/interactive/pty_handler.test.ts +++ b/typescript/__tests__/interactive/pty_handler.test.ts @@ -26,7 +26,9 @@ function makeMockPty(): MockPty { let capturedOnExit: ((e: { exitCode: number; signal?: number }) => void) | undefined; return { - pid: 12345, + // Use the test runner's own pid so isProcessAlive()'s `process.kill(pid, 0)` + // liveness probe sees a real, live process for an unterminated mock. + pid: process.pid, write: vi.fn(), kill: vi.fn(), resize: vi.fn(), @@ -161,6 +163,24 @@ describe("PtyHandler", () => { mockPty._exit(0); expect(handler.isProcessAlive()).toBe(false); }); + + it("returns false when the probe throws ESRCH (pid gone)", async () => { + await handler.createSession(["echo"]); + const killSpy = vi.spyOn(process, "kill").mockImplementation(() => { + throw Object.assign(new Error("kill ESRCH"), { code: "ESRCH" }); + }); + expect(handler.isProcessAlive()).toBe(false); + killSpy.mockRestore(); + }); + + it("returns true when the probe throws EPERM (pid exists, not signalable)", async () => { + await handler.createSession(["echo"]); + const killSpy = vi.spyOn(process, "kill").mockImplementation(() => { + throw Object.assign(new Error("kill EPERM"), { code: "EPERM" }); + }); + expect(handler.isProcessAlive()).toBe(true); + killSpy.mockRestore(); + }); }); describe("waitForExit", () => { diff --git a/typescript/__tests__/interactive/session.test.ts b/typescript/__tests__/interactive/session.test.ts index a9a16bc..162d041 100644 --- a/typescript/__tests__/interactive/session.test.ts +++ b/typescript/__tests__/interactive/session.test.ts @@ -3,6 +3,7 @@ import { InteractiveSession } from "../../src/interactive/session.js"; import { SessionState } from "../../src/core/models.js"; import { SessionError, SessionTerminatedError } from "../../src/interactive/models.js"; import { Settings } from "../../src/config/settings.js"; +import { MAX_COMMAND_HISTORY } from "../../src/constants.js"; import type { PtyHandler } from "../../src/interactive/pty_handler.js"; vi.mock("node-pty", () => ({ spawn: vi.fn() })); @@ -39,7 +40,7 @@ describe("InteractiveSession", () => { expect(session.sessionId).toBe("test-session-1"); expect(session.state).toBe(SessionState.CREATING); expect(session.commandCount).toBe(0); - expect(session.isAlive()).toBe(false); + expect(session.checkAlive()).toBe(false); expect(session.pty).not.toBeNull(); expect(session.outputBuffer).not.toBeNull(); }); @@ -216,7 +217,6 @@ describe("InteractiveSession", () => { await session.outputBuffer.append("% Exiting OpenROAD\r\n"); session.state = SessionState.TERMINATED; - // Must NOT throw even though the session is terminated const result = await session.readOutput(100); expect(result.output).toContain("Exiting OpenROAD"); @@ -226,8 +226,8 @@ describe("InteractiveSession", () => { it("signals shutdown when readOutput detects terminated session so writer task does not loop indefinitely", async () => { // Spy on the private method to verify readOutput() calls it directly. - // Scenario: _state was flipped to TERMINATED externally (e.g. via the setter) - // without calling _signalShutdown() — the exact gap @luarss identified. + // Scenario: _state was flipped to TERMINATED externally (e.g. via the + // setter) without calling _signalShutdown(). const signalShutdown = vi.spyOn(session as unknown as { _signalShutdown: () => void }, "_signalShutdown"); session.state = SessionState.TERMINATED; @@ -255,17 +255,17 @@ describe("InteractiveSession", () => { }); }); - describe("isAlive", () => { + describe("checkAlive", () => { it("returns false in CREATING state", () => { expect(session.state).toBe(SessionState.CREATING); - expect(session.isAlive()).toBe(false); + expect(session.checkAlive()).toBe(false); }); it("returns false in ACTIVE state when process is dead", () => { session.state = SessionState.ACTIVE; (mockPty.isProcessAlive as ReturnType).mockReturnValue(false); - expect(session.isAlive()).toBe(false); + expect(session.checkAlive()).toBe(false); expect(session.state).toBe(SessionState.TERMINATED); }); @@ -286,13 +286,13 @@ describe("InteractiveSession", () => { session.state = SessionState.ACTIVE; (mockPty.isProcessAlive as ReturnType).mockReturnValue(true); - expect(session.isAlive()).toBe(true); + expect(session.checkAlive()).toBe(true); expect(session.state).toBe(SessionState.ACTIVE); }); it("returns false in TERMINATED state", () => { session.state = SessionState.TERMINATED; - expect(session.isAlive()).toBe(false); + expect(session.checkAlive()).toBe(false); }); }); @@ -326,12 +326,11 @@ describe("InteractiveSession", () => { it("calls pty.cleanup() so listeners and pending resolvers are disposed without a subsequent session.cleanup()", async () => { session.state = SessionState.ACTIVE; - // terminate() without any follow-up cleanup() call await session.terminate(false); - // pty.cleanup() must have been called to dispose _dataDisposable, - // _exitDisposable, and drain _exitResolvers — otherwise post-kill - // data bursts keep appending and waitForExit() callers hang forever + // pty.cleanup() must run to dispose _dataDisposable, _exitDisposable, + // and drain _exitResolvers; otherwise post-kill data bursts keep + // appending and waitForExit() callers hang forever. expect(mockPty.cleanup).toHaveBeenCalledOnce(); }); }); @@ -419,7 +418,6 @@ describe("InteractiveSession", () => { await session.start(["echo"]); session.state = SessionState.TERMINATED; - // Should not throw or double-signal shutdown capturedOnExit?.(0); expect(session.state).toBe(SessionState.TERMINATED); }); @@ -436,7 +434,7 @@ describe("InteractiveSession", () => { await new Promise((r) => setTimeout(r, 5)); expect(session.state).toBe(SessionState.TERMINATED); - expect(session.isAlive()).toBe(false); + expect(session.checkAlive()).toBe(false); }); it("onData data exactly at READ_CHUNK_SIZE is a single append, not sliced", async () => { @@ -541,7 +539,7 @@ describe("InteractiveSession", () => { expect(mockPty.writeInput).toHaveBeenCalled(); expect(session.state).toBe(SessionState.TERMINATED); - expect(session.isAlive()).toBe(false); + expect(session.checkAlive()).toBe(false); }); it("subsequent sendCommand throws SessionTerminatedError after writer failure", async () => { @@ -585,6 +583,12 @@ describe("InteractiveSession", () => { expect(result.error).toMatch(/Invalid command/); }); + it("inserts captured text literally even when it contains $& replacement patterns", async () => { + await session.outputBuffer.append('invalid command name "foo$&bar"\n'); + const result = await session.readOutput(100); + expect(result.error).toBe("Invalid command: foo$&bar"); + }); + it("returns null error for clean output", async () => { await session.outputBuffer.append("openroad> \n"); const result = await session.readOutput(100); @@ -611,8 +615,8 @@ describe("InteractiveSession", () => { expect(session.commandHistory).toHaveLength(1); expect(session.commandHistory[0]!.command).toBe("report_wns"); - expect(session.commandHistory[0]!.command_number).toBe(1); - expect(typeof session.commandHistory[0]!.execution_start).toBe("number"); + expect(session.commandHistory[0]!.commandNumber).toBe(1); + expect(typeof session.commandHistory[0]!.executionStart).toBe("number"); expect(session.totalCommandsExecuted).toBe(1); expect(session.lastActivity.getTime()).toBeGreaterThanOrEqual(before); }); @@ -622,6 +626,30 @@ describe("InteractiveSession", () => { expect(session.commandHistory[0]!.command).toBe("puts hi"); }); + it("records execution_time for every command batched into one readOutput", async () => { + await session.sendCommand("cmd_a"); + await session.sendCommand("cmd_b"); + await session.readOutput(50); + + expect(session.commandHistory[0]!.executionTime).toBeDefined(); + expect(session.commandHistory[1]!.executionTime).toBeDefined(); + }); + + it("bounds commandHistory at MAX_COMMAND_HISTORY, dropping the oldest", async () => { + // Large queue so rapid sends never hit the input-queue-full guard. + const s = new InteractiveSession("hist-cap", 1024, new Settings({ SESSION_QUEUE_SIZE: 1_000_000 })); + s.pty = makeMockPty(); + await s.start(); + + const total = MAX_COMMAND_HISTORY + 5; + for (let i = 1; i <= total; i++) await s.sendCommand(`c${i}`); + + expect(s.commandHistory).toHaveLength(MAX_COMMAND_HISTORY); + // Oldest entries dropped: first retained command_number is total - MAX + 1. + expect(s.commandHistory[0]!.commandNumber).toBe(total - MAX_COMMAND_HISTORY + 1); + await s.cleanup(); + }); + it("getCommandHistory filters by search (case-insensitive)", async () => { await session.sendCommand("report_wns"); await session.sendCommand("get_nets foo"); @@ -643,17 +671,24 @@ describe("InteractiveSession", () => { expect(limited[0]!.command).toBe("cmd_b"); }); + it("getCommandHistory ignores a negative limit instead of dropping entries", async () => { + await session.sendCommand("cmd_a"); + await session.sendCommand("cmd_b"); + const all = session.getCommandHistory(-1); + expect(all).toHaveLength(2); + }); + it("getDetailedMetrics returns the full nested shape", async () => { await session.sendCommand("report_wns"); const m = await session.getDetailedMetrics(); - expect(m.session_id).toBe("test-session-1"); - expect(m.is_alive).toBe(true); - expect(m.commands.total_executed).toBe(1); - expect(m.commands.history_length).toBe(1); - expect(m.buffer.max_size).toBe(1024); - expect(m.timeout.configured_seconds).toBeNull(); - expect(m.timeout.is_timed_out).toBe(false); + expect(m.sessionId).toBe("test-session-1"); + expect(m.isAlive).toBe(true); + expect(m.commands.totalExecuted).toBe(1); + expect(m.commands.historyLength).toBe(1); + expect(m.buffer.maxSize).toBe(1024); + expect(m.timeout.configuredSeconds).toBeNull(); + expect(m.timeout.isTimedOut).toBe(false); }); it("isIdleTimeout is false right after activity, true past the threshold", async () => { @@ -673,8 +708,8 @@ describe("InteractiveSession", () => { session.createdAt.setTime(Date.now() - 10_000); const m = await session.getDetailedMetrics(); - expect(m.timeout.configured_seconds).toBe(1); - expect(m.timeout.is_timed_out).toBe(true); + expect(m.timeout.configuredSeconds).toBe(1); + expect(m.timeout.isTimedOut).toBe(true); }); it("readOutput backfills execution_time and output_length on the last entry", async () => { @@ -683,8 +718,8 @@ describe("InteractiveSession", () => { await session.readOutput(100); const entry = session.commandHistory[0]!; - expect(entry.execution_time).toBeGreaterThanOrEqual(0); - expect(entry.output_length).toBeGreaterThan(0); + expect(entry.executionTime).toBeGreaterThanOrEqual(0); + expect(entry.outputLength).toBeGreaterThan(0); }); it("filterOutput returns matching lines (regex, case-insensitive)", async () => { diff --git a/typescript/__tests__/tools/__snapshots__/interactive.test.ts.snap b/typescript/__tests__/tools/__snapshots__/interactive.test.ts.snap new file mode 100644 index 0000000..c6a9798 --- /dev/null +++ b/typescript/__tests__/tools/__snapshots__/interactive.test.ts.snap @@ -0,0 +1,5 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`Snapshots: wire format stability > ListSessionsTool with sessions 1`] = `"{"sessions":[{"session_id":"session-1","created_at":"2024-01-01T00:00:00.000Z","is_alive":true,"command_count":5,"buffer_size":1024,"uptime_seconds":100,"state":"active","error":null}],"total_count":1,"active_count":1,"error":null}"`; + +exports[`Snapshots: wire format stability > QueryShellTool success output 1`] = `"{"output":"test output","session_id":"session-1","timestamp":"2024-01-01T00:00:00.000Z","execution_time":0.1,"command_count":1,"buffer_size":0,"error":null}"`; diff --git a/typescript/__tests__/tools/interactive.test.ts b/typescript/__tests__/tools/interactive.test.ts new file mode 100644 index 0000000..c28b55b --- /dev/null +++ b/typescript/__tests__/tools/interactive.test.ts @@ -0,0 +1,453 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import type { Mock } from "vitest"; +import { QueryShellTool, ExecShellTool, ListSessionsTool, CreateSessionTool, TerminateSessionTool, InspectSessionTool, SessionHistoryTool, SessionMetricsTool, InteractiveShellTool } from "../../src/tools/interactive.js"; +import type { OpenROADManager } from "../../src/core/manager.js"; +import { SessionNotFoundError, SessionTerminatedError, SessionError } from "../../src/interactive/models.js"; +import { SessionState } from "../../src/core/models.js"; +import type { InteractiveExecResult, InteractiveSessionInfo, SessionDetailedMetrics, ManagerMetrics } from "../../src/core/models.js"; + +const NOW = "2024-01-01T00:00:00.000Z"; + +function makeExecResult(overrides: Partial = {}): InteractiveExecResult { + return { + output: "test output", + sessionId: "session-1", + timestamp: NOW, + executionTime: 0.1, + commandCount: 1, + bufferSize: 0, + error: null, + ...overrides, + }; +} + +function makeSessionInfo(overrides: Partial = {}): InteractiveSessionInfo { + return { + sessionId: "session-1", + createdAt: NOW, + isAlive: true, + commandCount: 5, + bufferSize: 1024, + uptimeSeconds: 100.0, + state: SessionState.ACTIVE, + error: null, + ...overrides, + }; +} + +function makeMetrics(sessionId = "session-1"): SessionDetailedMetrics { + return { + sessionId, + state: SessionState.ACTIVE, + isAlive: true, + createdAt: NOW, + lastActivity: NOW, + uptimeSeconds: 1, + idleSeconds: 0, + commands: { totalExecuted: 1, currentCount: 1, historyLength: 1 }, + performance: { totalCpuTime: 0.1, peakMemoryMb: 10, currentMemoryMb: 8 }, + buffer: { currentSize: 0, maxSize: 1024, utilizationPercent: 0 }, + timeout: { configuredSeconds: null, isTimedOut: false }, + }; +} + +function makeManagerMetrics(): ManagerMetrics { + return { + manager: { totalSessions: 1, activeSessions: 1, terminatedSessions: 0, maxSessions: 50, utilizationPercent: 2 }, + aggregate: { totalCommands: 5, totalCpuTime: 0.5, totalMemoryMb: 8, avgMemoryPerSession: 8 }, + sessions: [makeMetrics()], + }; +} + +interface MockManager extends Record { + createSession: Mock; + executeCommand: Mock; + listSessions: Mock; + getSessionInfo: Mock; + terminateSession: Mock; + inspectSession: Mock; + getSessionHistory: Mock; + sessionMetrics: Mock; +} + +function makeMockManager(): MockManager { + return { + createSession: vi.fn().mockResolvedValue("session-1"), + executeCommand: vi.fn().mockResolvedValue(makeExecResult()), + listSessions: vi.fn().mockResolvedValue([]), + getSessionInfo: vi.fn().mockResolvedValue(makeSessionInfo()), + terminateSession: vi.fn().mockResolvedValue(undefined), + inspectSession: vi.fn().mockResolvedValue(makeMetrics()), + getSessionHistory: vi.fn().mockResolvedValue([]), + sessionMetrics: vi.fn().mockResolvedValue(makeManagerMetrics()), + }; +} + +describe("QueryShellTool", () => { + let mgr: MockManager; + let tool: QueryShellTool; + + beforeEach(() => { + mgr = makeMockManager(); + tool = new QueryShellTool(mgr as unknown as OpenROADManager); + }); + + it("auto-creates a session when sessionId is null", async () => { + const raw = await tool.execute("help", null); + const result = JSON.parse(raw); + expect(mgr.createSession).toHaveBeenCalledOnce(); + expect(result.output).toBe("test output"); + expect(result.session_id).toBe("session-1"); + }); + + it("uses an existing session without creating a new one", async () => { + const raw = await tool.execute("help", "session-1"); + JSON.parse(raw); + expect(mgr.createSession).not.toHaveBeenCalled(); + expect(mgr.executeCommand).toHaveBeenCalledWith("session-1", "help", undefined); + }); + + it("returns snake_case keys in JSON output", async () => { + const raw = await tool.execute("help", "session-1"); + const result = JSON.parse(raw); + expect(Object.keys(result)).toContain("session_id"); + expect(Object.keys(result)).toContain("execution_time"); + expect(Object.keys(result)).toContain("command_count"); + expect(Object.keys(result)).toContain("buffer_size"); + }); + + it("handles SessionNotFoundError", async () => { + mgr.executeCommand.mockRejectedValue(new SessionNotFoundError("not found", "session-1")); + const raw = await tool.execute("help", "session-1"); + const result = JSON.parse(raw); + expect(result.output).toBe("Error: Session 'session-1' not found."); + expect(result.error).toContain("not found"); + }); + + it("handles SessionTerminatedError", async () => { + mgr.executeCommand.mockRejectedValue(new SessionTerminatedError("terminated", "session-1")); + const raw = await tool.execute("help", "session-1"); + const result = JSON.parse(raw); + expect(result.output).toBe(""); + expect(result.error).toContain("terminated"); + }); + + it("handles unexpected errors", async () => { + mgr.executeCommand.mockRejectedValue(new Error("boom")); + const raw = await tool.execute("help", "session-1"); + const result = JSON.parse(raw); + expect(result.error).toContain("Unexpected error"); + expect(result.error).toContain("boom"); + }); + + it("blocks dangerous commands when whitelist is enabled", async () => { + const raw = await tool.execute("quit"); + const result = JSON.parse(raw); + expect(result.error).toMatch(/CommandBlocked/); + expect(mgr.executeCommand).not.toHaveBeenCalled(); + }); + + it("InteractiveShellTool is an alias for QueryShellTool", () => { + expect(InteractiveShellTool).toBe(QueryShellTool); + }); +}); + +describe("ExecShellTool", () => { + let mgr: MockManager; + let tool: ExecShellTool; + + beforeEach(() => { + mgr = makeMockManager(); + tool = new ExecShellTool(mgr as unknown as OpenROADManager); + }); + + it("executes a state-modifying command", async () => { + const raw = await tool.execute("set_wire_rc -signal -layer metal3", "session-1"); + const result = JSON.parse(raw); + expect(result.output).toBe("test output"); + expect(mgr.createSession).not.toHaveBeenCalled(); + }); + + it("blocks quit via BLOCKED_COMMANDS", async () => { + const raw = await tool.execute("quit"); + const result = JSON.parse(raw); + expect(result.error).toMatch(/CommandBlocked/); + }); + + it("handles SessionNotFoundError", async () => { + mgr.executeCommand.mockRejectedValue(new SessionNotFoundError("missing", "session-1")); + const raw = await tool.execute("read_lef foo.lef", "session-1"); + const result = JSON.parse(raw); + expect(result.output).toContain("not found"); + }); +}); + +describe("ListSessionsTool", () => { + let mgr: MockManager; + let tool: ListSessionsTool; + + beforeEach(() => { + mgr = makeMockManager(); + tool = new ListSessionsTool(mgr as unknown as OpenROADManager); + }); + + it("returns empty list when no sessions exist", async () => { + const raw = await tool.execute(); + const result = JSON.parse(raw); + expect(result.sessions).toEqual([]); + expect(result.total_count).toBe(0); + expect(result.active_count).toBe(0); + expect(result.error).toBeNull(); + }); + + it("counts only alive sessions in active_count", async () => { + mgr.listSessions.mockResolvedValue([ + makeSessionInfo({ sessionId: "s1", isAlive: true }), + makeSessionInfo({ sessionId: "s2", isAlive: false }), + makeSessionInfo({ sessionId: "s3", isAlive: true }), + ]); + const raw = await tool.execute(); + const result = JSON.parse(raw); + expect(result.total_count).toBe(3); + expect(result.active_count).toBe(2); + }); + + it("returns error field on exception", async () => { + mgr.listSessions.mockRejectedValue(new Error("db error")); + const raw = await tool.execute(); + const result = JSON.parse(raw); + expect(result.error).toBeTruthy(); + expect(result.sessions).toEqual([]); + }); +}); + +describe("CreateSessionTool", () => { + let mgr: MockManager; + let tool: CreateSessionTool; + + beforeEach(() => { + mgr = makeMockManager(); + tool = new CreateSessionTool(mgr as unknown as OpenROADManager); + }); + + it("creates a session with default parameters", async () => { + const raw = await tool.execute(); + const result = JSON.parse(raw); + expect(mgr.createSession).toHaveBeenCalledOnce(); + expect(result.session_id).toBe("session-1"); + expect(result.is_alive).toBe(true); + }); + + it("passes custom parameters to createSession", async () => { + await tool.execute("my-id", ["openroad"], { KEY: "VAL" }, "/tmp"); + expect(mgr.createSession).toHaveBeenCalledWith({ + sessionId: "my-id", + command: ["openroad"], + env: { KEY: "VAL" }, + cwd: "/tmp", + }); + }); + + it("returns error info when creation fails", async () => { + mgr.createSession.mockRejectedValue(new SessionError("limit reached")); + const raw = await tool.execute("my-id"); + const result = JSON.parse(raw); + expect(result.is_alive).toBe(false); + expect(result.error).toContain("limit reached"); + }); +}); + +describe("TerminateSessionTool", () => { + let mgr: MockManager; + let tool: TerminateSessionTool; + + beforeEach(() => { + mgr = makeMockManager(); + tool = new TerminateSessionTool(mgr as unknown as OpenROADManager); + }); + + it("terminates a session normally", async () => { + const raw = await tool.execute("session-1"); + const result = JSON.parse(raw); + expect(result.terminated).toBe(true); + expect(result.was_alive).toBe(true); + expect(result.force).toBe(false); + expect(result.error).toBeNull(); + }); + + it("force-terminates a session", async () => { + const raw = await tool.execute("session-1", true); + const result = JSON.parse(raw); + expect(result.force).toBe(true); + expect(result.terminated).toBe(true); + }); + + it("handles terminating a non-existent session", async () => { + mgr.getSessionInfo.mockRejectedValue(new SessionNotFoundError("not found", "session-1")); + mgr.terminateSession.mockRejectedValue(new SessionNotFoundError("not found", "session-1")); + const raw = await tool.execute("session-1"); + const result = JSON.parse(raw); + expect(result.terminated).toBe(false); + expect(result.error).toBeTruthy(); + }); + + it("handles unexpected termination errors", async () => { + mgr.terminateSession.mockRejectedValue(new Error("PTY crash")); + const raw = await tool.execute("session-1"); + const result = JSON.parse(raw); + expect(result.terminated).toBe(false); + expect(result.error).toContain("Termination failed"); + }); +}); + +describe("InspectSessionTool", () => { + let mgr: MockManager; + let tool: InspectSessionTool; + + beforeEach(() => { + mgr = makeMockManager(); + tool = new InspectSessionTool(mgr as unknown as OpenROADManager); + }); + + it("returns session metrics", async () => { + const raw = await tool.execute("session-1"); + const result = JSON.parse(raw); + expect(result.session_id).toBe("session-1"); + expect(result.metrics).toBeTruthy(); + expect(result.metrics.session_id).toBe("session-1"); + expect(result.error).toBeNull(); + }); + + it("returns error when session not found", async () => { + mgr.inspectSession.mockRejectedValue(new SessionNotFoundError("missing", "session-1")); + const raw = await tool.execute("session-1"); + const result = JSON.parse(raw); + expect(result.metrics).toBeNull(); + expect(result.error).toBeTruthy(); + }); + + it("returns error on unexpected failure", async () => { + mgr.inspectSession.mockRejectedValue(new Error("cpu panic")); + const raw = await tool.execute("session-1"); + const result = JSON.parse(raw); + expect(result.error).toContain("Inspection failed"); + }); +}); + +describe("SessionHistoryTool", () => { + let mgr: MockManager; + let tool: SessionHistoryTool; + + beforeEach(() => { + mgr = makeMockManager(); + tool = new SessionHistoryTool(mgr as unknown as OpenROADManager); + }); + + it("returns session history", async () => { + mgr.getSessionHistory.mockResolvedValue([ + { command: "help", timestamp: NOW, commandNumber: 1, executionStart: 0 }, + ]); + const raw = await tool.execute("session-1"); + const result = JSON.parse(raw); + expect(result.session_id).toBe("session-1"); + expect(result.total_commands).toBe(1); + expect(result.history).toHaveLength(1); + expect(result.error).toBeNull(); + }); + + it("passes limit and search parameters", async () => { + await tool.execute("session-1", 10, "report"); + expect(mgr.getSessionHistory).toHaveBeenCalledWith("session-1", 10, "report"); + }); + + it("returns error when session not found", async () => { + mgr.getSessionHistory.mockRejectedValue(new SessionNotFoundError("gone", "session-1")); + const raw = await tool.execute("session-1"); + const result = JSON.parse(raw); + expect(result.history).toEqual([]); + expect(result.total_commands).toBe(0); + expect(result.error).toBeTruthy(); + }); + + it("returns error on unexpected failure", async () => { + mgr.getSessionHistory.mockRejectedValue(new Error("disk full")); + const raw = await tool.execute("session-1"); + const result = JSON.parse(raw); + expect(result.error).toContain("History retrieval failed"); + }); +}); + +describe("SessionMetricsTool", () => { + let mgr: MockManager; + let tool: SessionMetricsTool; + + beforeEach(() => { + mgr = makeMockManager(); + tool = new SessionMetricsTool(mgr as unknown as OpenROADManager); + }); + + it("returns manager-wide metrics", async () => { + const raw = await tool.execute(); + const result = JSON.parse(raw); + expect(result.metrics).toBeTruthy(); + expect(result.metrics.manager.total_sessions).toBe(1); + expect(result.error).toBeNull(); + }); + + it("returns error on unexpected failure", async () => { + mgr.sessionMetrics.mockRejectedValue(new Error("overload")); + const raw = await tool.execute(); + const result = JSON.parse(raw); + expect(result.metrics).toBeNull(); + expect(result.error).toContain("Metrics retrieval failed"); + }); +}); + +describe("Integration: session workflow", () => { + it("create, execute, list, terminate", async () => { + const mgr = makeMockManager(); + mgr.listSessions.mockResolvedValue([makeSessionInfo()]); + + const created = JSON.parse(await new CreateSessionTool(mgr as unknown as OpenROADManager).execute("test-id")); + expect(created.session_id).toBe("session-1"); + + const exec = JSON.parse(await new QueryShellTool(mgr as unknown as OpenROADManager).execute("help", "session-1")); + expect(exec.output).toBe("test output"); + + const list = JSON.parse(await new ListSessionsTool(mgr as unknown as OpenROADManager).execute()); + expect(list.total_count).toBe(1); + + const term = JSON.parse(await new TerminateSessionTool(mgr as unknown as OpenROADManager).execute("session-1")); + expect(term.terminated).toBe(true); + }); + + it("concurrent operations complete without interference", async () => { + const mgr = makeMockManager(); + const queryTool = new QueryShellTool(mgr as unknown as OpenROADManager); + const [r1, r2, r3] = await Promise.all([ + queryTool.execute("help", "session-1"), + queryTool.execute("version", "session-1"), + queryTool.execute("report_checks", "session-1"), + ]); + expect(JSON.parse(r1).output).toBe("test output"); + expect(JSON.parse(r2).output).toBe("test output"); + expect(JSON.parse(r3).output).toBe("test output"); + }); +}); + +// Snapshot: one representative output per tool + +describe("Snapshots: wire format stability", () => { + it("QueryShellTool success output", async () => { + const mgr = makeMockManager(); + const raw = await new QueryShellTool(mgr as unknown as OpenROADManager).execute("help", "session-1"); + expect(raw).toMatchSnapshot(); + }); + + it("ListSessionsTool with sessions", async () => { + const mgr = makeMockManager(); + mgr.listSessions.mockResolvedValue([makeSessionInfo()]); + const raw = await new ListSessionsTool(mgr as unknown as OpenROADManager).execute(); + expect(raw).toMatchSnapshot(); + }); +}); diff --git a/typescript/__tests__/tools/report_images.test.ts b/typescript/__tests__/tools/report_images.test.ts new file mode 100644 index 0000000..81c3d32 --- /dev/null +++ b/typescript/__tests__/tools/report_images.test.ts @@ -0,0 +1,431 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { + classifyImageType, + ListReportImagesTool, + ReadReportImageTool, + validatePlatformDesign, +} from "../../src/tools/report_images.js"; +import type { OpenROADManager } from "../../src/core/manager.js"; + +// Mock getSettings so tests do not depend on a filesystem ORFS install. +vi.mock("../../src/config/settings.js", () => { + let mockFlowPath = "/mock/flow"; + let mockPlatforms: string[] = []; + let mockDesigns: Record = {}; + return { + getSettings: vi.fn(() => ({ + get flowPath() { return mockFlowPath; }, + get platforms() { return mockPlatforms; }, + designs(platform: string) { return mockDesigns[platform] ?? []; }, + WHITELIST_ENABLED: false, + LOG_LEVEL: "INFO", + COMMAND_TIMEOUT: 30, + COMMAND_COMPLETION_DELAY: 0.1, + DEFAULT_BUFFER_SIZE: 131072, + MAX_SESSIONS: 50, + SESSION_QUEUE_SIZE: 128, + SESSION_IDLE_TIMEOUT: 300, + READ_CHUNK_SIZE: 8192, + LOG_FORMAT: "", + ALLOWED_COMMANDS: ["openroad"], + ENABLE_COMMAND_VALIDATION: true, + ORFS_FLOW_PATH: "/mock/flow", + })), + __setMock(fp: string, plats: string[], des: Record) { + mockFlowPath = fp; + mockPlatforms = plats; + mockDesigns = des; + }, + }; +}); + +import { getSettings } from "../../src/config/settings.js"; + +let tmpDir: string; + +function createFixture( + platform = "nangate45", + design = "gcd", + runSlug = "run-123", + imageFiles: string[] = ["cts_clk.webp", "final_all.webp"], +) { + const flowPath = tmpDir; + fs.mkdirSync(path.join(flowPath, "platforms", platform), { recursive: true }); + fs.mkdirSync(path.join(flowPath, "designs", platform, design), { recursive: true }); + const runPath = path.join(flowPath, "reports", platform, design, runSlug); + fs.mkdirSync(runPath, { recursive: true }); + for (const img of imageFiles) { + fs.writeFileSync(path.join(runPath, img), Buffer.from("RIFF\x00\x00\x00\x00WEBP")); + } + return { flowPath, runPath }; +} + +// Constructor requires a manager but the tools never invoke it. +const stubManager = {} as unknown as OpenROADManager; + +beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "openroad-test-")); +}); + +afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + vi.clearAllMocks(); +}); + +describe("classifyImageType", () => { + it("classifies CTS images correctly", () => { + expect(classifyImageType("cts_clk.webp")).toEqual(["cts", "clock_visualization"]); + expect(classifyImageType("cts_clk_layout.webp")).toEqual(["cts", "clock_layout"]); + expect(classifyImageType("cts_core_clock.webp")).toEqual(["cts", "core_clock_visualization"]); + }); + + it("classifies final stage images correctly", () => { + expect(classifyImageType("final_all.webp")).toEqual(["final", "complete_design"]); + expect(classifyImageType("final_congestion.webp")).toEqual(["final", "congestion_heatmap"]); + expect(classifyImageType("final_ir_drop.webp")).toEqual(["final", "ir_drop_analysis"]); + }); + + it("returns unknown for unrecognised filenames", () => { + expect(classifyImageType("unknown_image.webp")).toEqual(["unknown", "unknown"]); + expect(classifyImageType("foo.webp")).toEqual(["unknown", "unknown"]); + }); + + it("returns unknown stage when filename has no underscore", () => { + const [stage, _type] = classifyImageType("nounderscore.webp"); + expect(stage).toBe("unknown"); + }); +}); + +describe("validatePlatformDesign", () => { + it("throws on unknown platform", () => { + (getSettings as ReturnType).mockReturnValueOnce({ + platforms: ["nangate45"], + designs: () => ["gcd"], + flowPath: tmpDir, + }); + expect(() => validatePlatformDesign("bad_platform", "gcd")).toThrow(); + }); + + it("throws on unknown design", () => { + (getSettings as ReturnType).mockReturnValueOnce({ + platforms: ["nangate45"], + designs: (p: string) => (p === "nangate45" ? ["gcd"] : []), + flowPath: tmpDir, + }); + expect(() => validatePlatformDesign("nangate45", "bad_design")).toThrow(); + }); +}); + +describe("ListReportImagesTool", () => { + let tool: ListReportImagesTool; + + beforeEach(() => { + tool = new ListReportImagesTool(stubManager); + }); + + it("returns error when platform is invalid", async () => { + (getSettings as ReturnType).mockReturnValueOnce({ + platforms: [], + designs: () => [], + flowPath: tmpDir, + }); + const raw = await tool.execute("bad_platform", "gcd", "run-123"); + const result = JSON.parse(raw); + expect(result.error).toBeTruthy(); + }); + + it("returns RunNotFound error when run directory does not exist", async () => { + const { flowPath } = createFixture(); + (getSettings as ReturnType).mockReturnValue({ + platforms: ["nangate45"], + designs: (p: string) => (p === "nangate45" ? ["gcd"] : []), + flowPath, + WHITELIST_ENABLED: false, + }); + const raw = await tool.execute("nangate45", "gcd", "nonexistent"); + const result = JSON.parse(raw); + expect(result.error).toBe("RunNotFound"); + }); + + it("returns totalImages 0 when run directory has no .webp files", async () => { + const { flowPath } = createFixture("nangate45", "gcd", "run-empty", []); + (getSettings as ReturnType).mockReturnValue({ + platforms: ["nangate45"], + designs: (p: string) => (p === "nangate45" ? ["gcd"] : []), + flowPath, + WHITELIST_ENABLED: false, + }); + const raw = await tool.execute("nangate45", "gcd", "run-empty"); + const result = JSON.parse(raw); + expect(result.total_images).toBe(0); + expect(result.images_by_stage).toEqual({}); + }); + + it("lists all .webp files grouped by stage", async () => { + const { flowPath } = createFixture(); + (getSettings as ReturnType).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(2); + expect(result.images_by_stage).toBeTruthy(); + expect(result.images_by_stage).toHaveProperty("cts"); + expect(result.images_by_stage).toHaveProperty("final"); + }); + + it("does not descend symlinked directories or list symlinked files", async () => { + const { flowPath, runPath } = createFixture("nangate45", "gcd", "run-123", [ + "cts_clk.webp", + ]); + // A directory of images outside the run, reachable only via symlinks. + const outside = path.join(tmpDir, "outside"); + fs.mkdirSync(outside, { recursive: true }); + fs.writeFileSync(path.join(outside, "final_all.webp"), Buffer.from("RIFF\x00\x00\x00\x00WEBP")); + try { + fs.symlinkSync(outside, path.join(runPath, "linkdir")); + fs.symlinkSync(path.join(outside, "final_all.webp"), path.join(runPath, "linkfile.webp")); + } catch { + // symlink creation may fail in some environments; skip gracefully. + return; + } + (getSettings as ReturnType).mockReturnValue({ + platforms: ["nangate45"], + designs: (p: string) => (p === "nangate45" ? ["gcd"] : []), + flowPath, + WHITELIST_ENABLED: false, + }); + const result = JSON.parse(await tool.execute("nangate45", "gcd", "run-123")); + // Only the real cts_clk.webp is found; nothing reached through a symlink. + expect(result.total_images).toBe(1); + expect(result.images_by_stage).toHaveProperty("cts"); + expect(result.images_by_stage).not.toHaveProperty("final"); + }); + + it("filters images by stage", async () => { + const { flowPath } = createFixture("nangate45", "gcd", "run-123", [ + "cts_clk.webp", + "final_all.webp", + ]); + (getSettings as ReturnType).mockReturnValue({ + platforms: ["nangate45"], + designs: (p: string) => (p === "nangate45" ? ["gcd"] : []), + flowPath, + WHITELIST_ENABLED: false, + }); + const raw = await tool.execute("nangate45", "gcd", "run-123", "cts"); + const result = JSON.parse(raw); + expect(result.total_images).toBe(1); + expect(result.images_by_stage).toHaveProperty("cts"); + expect(result.images_by_stage).not.toHaveProperty("final"); + }); +}); + +describe("ReadReportImageTool", () => { + let tool: ReadReportImageTool; + + beforeEach(() => { + tool = new ReadReportImageTool(stubManager); + }); + + it("returns error when platform is invalid", async () => { + (getSettings as ReturnType).mockReturnValueOnce({ + platforms: [], + designs: () => [], + flowPath: tmpDir, + }); + const raw = await tool.execute("bad_platform", "gcd", "run-123", "cts_clk.webp"); + const result = JSON.parse(raw); + expect(result.error).toBeTruthy(); + expect(result.image_data).toBeNull(); + }); + + it("returns RunNotFound when run directory does not exist", async () => { + const { flowPath } = createFixture(); + (getSettings as ReturnType).mockReturnValue({ + platforms: ["nangate45"], + designs: (p: string) => (p === "nangate45" ? ["gcd"] : []), + flowPath, + WHITELIST_ENABLED: false, + }); + const raw = await tool.execute("nangate45", "gcd", "missing-run", "cts_clk.webp"); + const result = JSON.parse(raw); + expect(result.error).toBe("RunNotFound"); + }); + + it("returns ImageNotFound when image does not exist", async () => { + const { flowPath } = createFixture(); + (getSettings as ReturnType).mockReturnValue({ + platforms: ["nangate45"], + designs: (p: string) => (p === "nangate45" ? ["gcd"] : []), + flowPath, + WHITELIST_ENABLED: false, + }); + const raw = await tool.execute("nangate45", "gcd", "run-123", "missing.webp"); + const result = JSON.parse(raw); + expect(result.error).toBe("ImageNotFound"); + }); + + it("reads and base64-encodes a .webp image successfully", async () => { + const { flowPath } = createFixture("nangate45", "gcd", "run-123", ["cts_clk.webp"]); + (getSettings as ReturnType).mockReturnValue({ + platforms: ["nangate45"], + designs: (p: string) => (p === "nangate45" ? ["gcd"] : []), + flowPath, + WHITELIST_ENABLED: false, + }); + const raw = await tool.execute("nangate45", "gcd", "run-123", "cts_clk.webp"); + const result = JSON.parse(raw); + expect(typeof result.image_data).toBe("string"); + expect(result.image_data.length).toBeGreaterThan(0); + const decoded = Buffer.from(result.image_data, "base64"); + expect(decoded.length).toBeGreaterThan(0); + expect(result.metadata).toBeTruthy(); + expect(result.metadata.filename).toBe("cts_clk.webp"); + expect(result.metadata.stage).toBe("cts"); + expect(result.metadata.type).toBe("clock_visualization"); + }); + + it("returns FileTooLarge error when image exceeds 50 MB", async () => { + const { flowPath, runPath } = createFixture("nangate45", "gcd", "run-123", []); + const bigPath = path.join(runPath, "huge.webp"); + fs.writeFileSync(bigPath, Buffer.from("tiny content")); + (getSettings as ReturnType).mockReturnValue({ + platforms: ["nangate45"], + designs: (p: string) => (p === "nangate45" ? ["gcd"] : []), + flowPath, + WHITELIST_ENABLED: false, + }); + const originalStatSync = fs.statSync.bind(fs); + const statSpy = vi.spyOn(fs, "statSync").mockImplementation((p) => { + if (p === bigPath) return { size: 51 * 1024 * 1024, isFile: () => true, mtime: new Date() } as unknown as fs.Stats; + return originalStatSync(p) as fs.Stats; + }); + const raw = await tool.execute("nangate45", "gcd", "run-123", "huge.webp"); + const result = JSON.parse(raw); + expect(result.error).toBe("FileTooLarge"); + statSpy.mockRestore(); + }); + + it("rejects non-.webp extension", async () => { + const { flowPath } = createFixture(); + (getSettings as ReturnType).mockReturnValue({ + platforms: ["nangate45"], + designs: (p: string) => (p === "nangate45" ? ["gcd"] : []), + flowPath, + WHITELIST_ENABLED: false, + }); + const raw = await tool.execute("nangate45", "gcd", "run-123", "cts_clk.png"); + const result = JSON.parse(raw); + expect(result.error).toBe("InvalidImageName"); + }); +}); + +describe("TestPathTraversalSecurity", () => { + let tool: ListReportImagesTool; + let readTool: ReadReportImageTool; + let flowPath: string; + + beforeEach(() => { + const fixture = createFixture(); + flowPath = fixture.flowPath; + (getSettings as ReturnType).mockReturnValue({ + platforms: ["nangate45"], + designs: (p: string) => (p === "nangate45" ? ["gcd"] : []), + flowPath, + WHITELIST_ENABLED: false, + }); + tool = new ListReportImagesTool(stubManager); + readTool = new ReadReportImageTool(stubManager); + }); + + it("rejects path traversal in run_slug (../../etc/passwd)", async () => { + const raw = await tool.execute("nangate45", "gcd", "../../../etc/passwd"); + const result = JSON.parse(raw); + expect(result.error).toBeTruthy(); + expect(result.error).not.toBe("RunNotFound"); // must be a validation error + }); + + it("rejects bare '..' as run_slug", async () => { + const raw = await tool.execute("nangate45", "gcd", ".."); + const result = JSON.parse(raw); + expect(result.error).toBeTruthy(); + }); + + it("rejects glob characters in run_slug", async () => { + const raw = await tool.execute("nangate45", "gcd", "*"); + const result = JSON.parse(raw); + expect(result.error).toBeTruthy(); + }); + + it("rejects path traversal in image_name", async () => { + const raw = await readTool.execute("nangate45", "gcd", "run-123", "../../../etc/passwd"); + const result = JSON.parse(raw); + expect(result.error).toBeTruthy(); + }); + + it("rejects non-.webp extension in image_name", async () => { + const raw = await readTool.execute("nangate45", "gcd", "run-123", "file.sh"); + const result = JSON.parse(raw); + expect(result.error).toBe("InvalidImageName"); + }); + + it("rejects null byte in image_name", async () => { + const raw = await readTool.execute("nangate45", "gcd", "run-123", "evil\x00.webp"); + const result = JSON.parse(raw); + expect(result.error).toBeTruthy(); + }); + + it("blocks symlink escape from run directory", async () => { + const runPath = path.join(flowPath, "reports", "nangate45", "gcd", "run-123"); + const linkPath = path.join(runPath, "escape.webp"); + try { + fs.symlinkSync("/etc/passwd", linkPath); + } catch { + // symlink creation may fail in some environments; skip gracefully. + return; + } + const raw = await readTool.execute("nangate45", "gcd", "run-123", "escape.webp"); + const result = JSON.parse(raw); + // Should not find the image, reject path containment, or return an error + // and must NOT return valid image_data resolving to /etc/passwd content. + expect(result.image_data === null || result.error !== null).toBe(true); + }); +}); + +describe("TestPlatformDesignValidationInTools", () => { + beforeEach(() => { + (getSettings as ReturnType).mockReturnValue({ + platforms: ["nangate45"], + designs: (p: string) => (p === "nangate45" ? ["gcd"] : []), + flowPath: tmpDir, + WHITELIST_ENABLED: false, + }); + }); + + it("list tool returns error for invalid platform", async () => { + const raw = await new ListReportImagesTool(stubManager).execute("invalid_plat", "gcd", "run-123"); + expect(JSON.parse(raw).error).toBeTruthy(); + }); + + it("list tool returns error for invalid design", async () => { + const raw = await new ListReportImagesTool(stubManager).execute("nangate45", "bad_design", "run-123"); + expect(JSON.parse(raw).error).toBeTruthy(); + }); + + it("read tool returns error for invalid platform", async () => { + const raw = await new ReadReportImageTool(stubManager).execute("invalid_plat", "gcd", "run-123", "img.webp"); + expect(JSON.parse(raw).error).toBeTruthy(); + }); + + it("read tool returns error for invalid design", async () => { + const raw = await new ReadReportImageTool(stubManager).execute("nangate45", "bad_design", "run-123", "img.webp"); + expect(JSON.parse(raw).error).toBeTruthy(); + }); +}); diff --git a/typescript/__tests__/utils/ansi_decoder.test.ts b/typescript/__tests__/utils/ansi_decoder.test.ts index ba7fd2a..2731213 100644 --- a/typescript/__tests__/utils/ansi_decoder.test.ts +++ b/typescript/__tests__/utils/ansi_decoder.test.ts @@ -67,6 +67,22 @@ describe("ANSIDecoder", () => { }); }); + describe("translateOutput - $ replacement safety", () => { + it("does not reinterpret $& inside an OSC sequence body when annotating", () => { + // OSC title set with a literal "$&" in the body. With a string replacement + // the "$&" would expand to the matched sequence and corrupt the output. + const osc = "\x1b]0;$&title\x07"; + const result = ANSIDecoder.translateOutput(`before${osc}after`, "annotate"); + expect(result).toBe("before[Unknown escape sequence (\x1b]0;$&title\x07)]after"); + }); + + it("inserts the original sequence literally in preserve mode", () => { + const osc = "\x1b]0;$$x\x07"; + const result = ANSIDecoder.translateOutput(osc, "preserve"); + expect(result).toContain(osc); + }); + }); + describe("translateOutput - decode mode", () => { it("includes breakdown header", () => { const result = ANSIDecoder.translateOutput("\x1b[1mtext", "decode"); diff --git a/typescript/__tests__/utils/path_security.test.ts b/typescript/__tests__/utils/path_security.test.ts index a013faa..673cabf 100644 --- a/typescript/__tests__/utils/path_security.test.ts +++ b/typescript/__tests__/utils/path_security.test.ts @@ -18,6 +18,12 @@ describe("validatePathSegment", () => { ); }); + it("rejects whitespace-only segment", () => { + expect(() => validatePathSegment(" ", "test_segment")).toThrow( + new ValidationError("test_segment cannot be empty"), + ); + }); + it("rejects '.' segment", () => { expect(() => validatePathSegment(".", "test_segment")).toThrow( new ValidationError("test_segment cannot be '.' or '..'"), diff --git a/typescript/scripts/integration_check.ts b/typescript/scripts/integration_check.ts index ce61330..d162fb1 100644 --- a/typescript/scripts/integration_check.ts +++ b/typescript/scripts/integration_check.ts @@ -1,18 +1,13 @@ -/** - * Real OpenROAD REPL integration check. - * Run with: npx tsx scripts/integration_check.ts - */ - import { InteractiveSession } from "../src/interactive/session.js"; import { Settings } from "../src/config/settings.js"; -const PASS = "✓"; -const FAIL = "✗"; +const PASS = "PASS"; +const FAIL = "FAIL"; const results: { label: string; ok: boolean; detail?: string }[] = []; function check(label: string, ok: boolean, detail?: string) { results.push({ label, ok, detail }); - console.log(` ${ok ? PASS : FAIL} ${label}${detail ? ` → ${detail}` : ""}`); + console.log(` ${ok ? PASS : FAIL} ${label}${detail ? ` -> ${detail}` : ""}`); } async function waitForPrompt(session: InteractiveSession, timeoutMs = 5000): Promise { @@ -32,20 +27,18 @@ async function run() { const settings = new Settings({ ENABLE_COMMAND_VALIDATION: false }); const session = new InteractiveSession("integration-check", 256 * 1024, settings); - // ── 1. Spawn ──────────────────────────────────────────────────────────────── console.log("1. Session lifecycle"); try { await session.start(["openroad", "-no_init"]); check("start() succeeds", true); check("state is ACTIVE after start", session.state === "active", session.state); - check("isAlive() returns true", session.isAlive()); + check("checkAlive() returns true", session.checkAlive()); check("writer task running", session.isRunning()); } catch (e) { check("start() succeeds", false, String(e)); process.exit(1); } - // ── 2. Initial prompt ─────────────────────────────────────────────────────── console.log("\n2. Initial prompt"); const banner = await waitForPrompt(session, 6000); check("received output after spawn", banner.length > 0, `${banner.length} chars`); @@ -55,7 +48,6 @@ async function run() { banner.slice(0, 80).replace(/\n/g, " "), ); - // ── 3. puts echo ──────────────────────────────────────────────────────────── console.log("\n3. Command round-trip"); await session.sendCommand('puts "hello_integration"'); const echoResult = await session.readOutput(3000); @@ -67,7 +59,6 @@ async function run() { ); check("commandCount incremented", session.commandCount >= 1, String(session.commandCount)); - // ── 4. Error detection ────────────────────────────────────────────────────── console.log("\n4. Error detection"); await session.sendCommand("nonexistent_command_xyz"); const errResult = await session.readOutput(3000); @@ -77,7 +68,6 @@ async function run() { errResult.error ?? "(null)", ); - // ── 5. Multiple commands ──────────────────────────────────────────────────── console.log("\n5. Multiple sequential commands"); const before = session.commandCount; await session.sendCommand('puts "cmd1"'); @@ -86,24 +76,21 @@ async function run() { await session.readOutput(1000); check("commandCount advances correctly", session.commandCount === before + 2, String(session.commandCount)); - // ── 6. Buffer ─────────────────────────────────────────────────────────────── console.log("\n6. Output buffer"); const stats = await session.outputBuffer.getStats(); check("buffer maxSize is set", stats.maxSize > 0, `${stats.maxSize} chars`); - // ── 7. Graceful termination ───────────────────────────────────────────────── console.log("\n7. Termination"); await session.sendCommand("exit"); await new Promise((r) => setTimeout(r, 500)); await session.cleanup(); check("cleanup() does not throw", true); check("state is TERMINATED after cleanup", session.state === "terminated", session.state); - check("isAlive() returns false after cleanup", !session.isAlive()); + check("checkAlive() returns false after cleanup", !session.checkAlive()); - // ── Summary ───────────────────────────────────────────────────────────────── const passed = results.filter((r) => r.ok).length; const total = results.length; - console.log(`\n${"─".repeat(48)}`); + console.log(`\n${"-".repeat(48)}`); console.log(` ${passed}/${total} checks passed`); if (passed < total) { console.log(`\n Failed:`); diff --git a/typescript/src/config/command_whitelist.ts b/typescript/src/config/command_whitelist.ts index beb6042..3760b1f 100644 --- a/typescript/src/config/command_whitelist.ts +++ b/typescript/src/config/command_whitelist.ts @@ -76,17 +76,14 @@ export const EXEC_ONLY_PATTERNS: readonly string[] = [ ]; // Safe Tcl built-ins - usable in both tools. +// Intentionally excludes body-eval builtins (if, for, foreach, while, proc, +// catch, namespace, uplevel) because they accept a script body argument and +// can therefore wrap any dangerous command: `catch { exec ls }` would pass the +// verb check on `catch` alone. Those builtins belong in the exec tool. export const _TCL_BUILTINS: readonly string[] = [ "puts", "set", "expr", - "if", - "else", - "elseif", - "for", - "foreach", - "while", - "proc", "return", "break", "continue", @@ -105,9 +102,7 @@ export const _TCL_BUILTINS: readonly string[] = [ "scan", "array", "dict", - "catch", "error", - "namespace", "upvar", "global", "variable", @@ -151,34 +146,182 @@ function matchesAny(verb: string, matchers: readonly Minimatch[]): boolean { return matchers.some((m) => m.match(verb)); } +/** + * Resolve Tcl backslash substitution within a single command word so an + * obfuscated verb matches the command Tcl will actually run. Tcl drops a + * backslash before an ordinary character (`\socket` -> `socket`) and decodes + * numeric escapes (`\x73`/`\163`/`s` -> `s`), so without this a blocked + * verb could be smuggled past the allowlist as its literal-backslash form. A + * real command name never contains a backslash, so this only normalises + * obfuscation and never alters a legitimate verb. + */ +function tclUnescape(token: string): string { + if (!token.includes("\\")) return token; + let out = ""; + for (let i = 0; i < token.length; i++) { + if (token[i] !== "\\" || i + 1 >= token.length) { + out += token[i]; + continue; + } + const c = token[i + 1]!; + const simple: Record = { + a: "\x07", b: "\b", f: "\f", n: "\n", r: "\r", t: "\t", v: "\v", + }; + if (c in simple) { + out += simple[c]; + i++; + } else if (c === "x") { + const m = /^[0-9a-fA-F]+/.exec(token.slice(i + 2)); + if (m) { + out += String.fromCharCode(parseInt(m[0], 16) & 0xff); + i += 1 + m[0].length; + } else { + out += "x"; + i++; + } + } else if (c === "u" || c === "U") { + const m = new RegExp(`^[0-9a-fA-F]{1,${c === "u" ? 4 : 8}}`).exec(token.slice(i + 2)); + if (m) { + out += String.fromCodePoint(parseInt(m[0], 16)); + i += 1 + m[0].length; + } else { + out += c; + i++; + } + } else if (c >= "0" && c <= "7") { + const m = /^[0-7]{1,3}/.exec(token.slice(i + 1))!; + out += String.fromCharCode(parseInt(m[0], 8) & 0xff); + i += m[0].length; + } else { + // Backslash before an ordinary char: drop the backslash, keep the char. + out += c; + i++; + } + } + return out; +} + /** * Return the command verb (first token) of a single Tcl statement. * * Returns null only for blank lines and comment lines. Lines that start with a * substitution or grouping character (`$`, `[`, `]`, `{`, `}`) are returned - * as-is so the caller can reject them via the allowlist. + * as-is so the caller can reject them via the allowlist. Backslash escapes in + * the verb are resolved (see tclUnescape) so `\socket` is matched as `socket`. */ export function extractVerb(statement: string): string | null { const stripped = statement.trim(); if (stripped === "" || stripped.startsWith("#")) { return null; } - const firstToken = stripped.split(/\s+/)[0]!; - return firstToken.replace(/;+$/, ""); + const firstToken = stripped.split(/\s+/)[0]!.replace(/;+$/, ""); + return tclUnescape(firstToken); } -/** Iterate the verbs of a command, mirroring Python's naive `;`->newline split. */ +// Unicode line-boundary characters that Python's str.splitlines() recognises. +// Treating every one of these (not just \n) as a statement separator closes a +// \r-based bypass where a second command is hidden after a carriage return. +const STATEMENT_SEPARATORS: ReadonlySet = new Set([ + "\n", + "\r", + "\v", + "\f", + "\x1c", + "\x1d", + "\x1e", + "\x85", + "
", + "
", +]); + +/** + * Split a Tcl command string into individual statements, respecting quoted + * strings and brace groups so that separators inside them are not treated as + * statement boundaries (e.g. `puts "hello; world"` is one statement). Statement + * separators are `;` plus every Unicode line boundary in STATEMENT_SEPARATORS, + * so a bare \r cannot hide a second command from the verb check. + */ +function splitTclStatements(command: string): string[] { + const stmts: string[] = []; + let depth = 0; + let inQuote = false; + let current = ""; + // In Tcl a `"` only opens a quoted string at the start of a word; a quote in + // the middle of a word (e.g. `a"b`) is a literal. Track word boundaries so an + // unbalanced mid-word quote cannot flip us into quote mode and swallow the + // separator that follows it. + let atWordStart = true; + + for (let i = 0; i < command.length; i++) { + const ch = command[i]!; + if (ch === "\\" && i + 1 < command.length) { + current += ch + command[i + 1]!; + i++; + atWordStart = false; + } else if (ch === '"' && depth === 0 && (inQuote || atWordStart)) { + inQuote = !inQuote; + current += ch; + atWordStart = false; + } else if (!inQuote && ch === "{") { + depth++; + current += ch; + atWordStart = false; + } else if (!inQuote && ch === "}") { + // Clamp at zero: an unbalanced close brace must not drive depth negative, + // which would make the `depth === 0` separator test below permanently + // false and hide every subsequent statement from the verb check. + if (depth > 0) depth--; + current += ch; + atWordStart = false; + } else if (!inQuote && depth === 0 && (ch === ";" || STATEMENT_SEPARATORS.has(ch))) { + stmts.push(current); + current = ""; + atWordStart = true; + } else { + current += ch; + // Whitespace and `[` (which begins a command substitution) start a new + // word, so a `"` immediately after them is again quote-significant. + atWordStart = ch === " " || ch === "\t" || ch === "["; + } + } + if (current) stmts.push(current); + return stmts; +} + +/** + * Iterate the verbs of a Tcl command string. + * + * Per statement, in a Tcl-aware split (separators inside quotes/braces are + * ignored; `;` plus all Unicode line boundaries are separators so a \r cannot + * hide a command), two kinds of verb are yielded: + * 1. The statement verb — its leading token. + * 2. Bracket verbs — the command word following each `[` (bracket + * substitution). This catches `set x [exec ls]` where the outer verb `set` + * is safe but the substituted command `exec` is not. + * + * The bracket word is captured up to the first delimiter, so a substituted + * command whose name is itself dynamic is surfaced rather than skipped: a + * variable name (`[$x ls]`) yields `$x` and a nested substitution + * (`[[set x exec] ls]`) yields `[set`. Neither is in any allowlist, so the + * read-only tool rejects them instead of letting Tcl resolve `$x`->exec at + * runtime and run an arbitrary command. Backslash escapes are then resolved + * (see tclUnescape) so `[\exec ls]` is matched as `exec`. + * + * Comment and blank statements (extractVerb returns null) are skipped entirely, + * including their brackets: a `#` comment is discarded at Tcl parse time and is + * never executed, so `# harmless [exec ls]` must not be rejected on the `exec`. + * Brackets inside brace-quoted arguments are still scanned, because commands + * like `expr`/`eval` re-evaluate brace contents, so they cannot be assumed inert. + */ function* iterVerbs(command: string): Generator { - // Replace ';' with newline, then split on all Unicode line boundaries that - // Python's str.splitlines() recognises. \r\n must precede \r so the pair is - // consumed as one separator. Semicolons inside Tcl braces or quoted strings - // are not handled - this matches the Python implementation's known limitation. - for (const rawLine of command - .replace(/;/g, "\n") - .split(/\r\n|[\n\r\v\f\x1c\x1d\x1e\x85\u2028\u2029]/)) { - const verb = extractVerb(rawLine); - if (verb !== null) { - yield verb; + for (const stmt of splitTclStatements(command)) { + const verb = extractVerb(stmt); + if (verb === null) continue; + yield verb; + // `[` includes itself in the class so a nested `[[...]` substitution yields + // a `[`-led token; `$` is included so a `[$var]` indirection yields `$var`. + for (const match of stmt.matchAll(/\[\s*(?::+)?([^\s\]{};]+)/g)) { + yield tclUnescape(match[1]!).replace(/^:+/, ""); } } } diff --git a/typescript/src/config/settings.ts b/typescript/src/config/settings.ts index 97fbbc6..6368fc7 100644 --- a/typescript/src/config/settings.ts +++ b/typescript/src/config/settings.ts @@ -15,17 +15,25 @@ function parseBool(envKey: string, val: string): boolean { ); } -function parseFloat_(envKey: string, val: string): number { +function parseFloat_(envKey: string, val: string, allowZero: boolean): number { if (val.trim() === "") throw new Error(`Invalid value for ${envKey}: (empty string). Expected float.`); const n = Number(val); - if (isNaN(n)) throw new Error(`Invalid value for ${envKey}: ${val}. Expected float.`); + const bound = allowZero ? "non-negative" : "positive"; + if (!Number.isFinite(n) || n < 0 || (!allowZero && n === 0)) { + throw new Error(`Invalid value for ${envKey}: ${val}. Expected a ${bound} finite float.`); + } return n; } -function parseInt_(envKey: string, val: string): number { +function parseInt_(envKey: string, val: string, allowZero: boolean): number { if (val.trim() === "") throw new Error(`Invalid value for ${envKey}: (empty string). Expected int.`); if (!/^-?\d+$/.test(val.trim())) throw new Error(`Invalid value for ${envKey}: ${val}. Expected int.`); - return Number(val); + const n = Number(val); + const bound = allowZero ? "non-negative" : "positive"; + if (n < 0 || (!allowZero && n === 0)) { + throw new Error(`Invalid value for ${envKey}: ${val}. Expected a ${bound} integer.`); + } + return n; } export class Settings { @@ -86,19 +94,19 @@ export class Settings { } static fromEnv(): Settings { - // Mutable partial — strips readonly so we can build the object incrementally. const overrides: { -readonly [K in keyof Settings]?: Settings[K] } = {}; - const floatFields: Array<[keyof Settings, string]> = [ - ["COMMAND_TIMEOUT", "OPENROAD_COMMAND_TIMEOUT"], - ["COMMAND_COMPLETION_DELAY", "OPENROAD_COMMAND_COMPLETION_DELAY"], - ["SESSION_IDLE_TIMEOUT", "OPENROAD_SESSION_IDLE_TIMEOUT"], + // [field, envKey, allowZero] + const floatFields: Array<[keyof Settings, string, boolean]> = [ + ["COMMAND_TIMEOUT", "OPENROAD_COMMAND_TIMEOUT", false], + ["COMMAND_COMPLETION_DELAY", "OPENROAD_COMMAND_COMPLETION_DELAY", true], + ["SESSION_IDLE_TIMEOUT", "OPENROAD_SESSION_IDLE_TIMEOUT", false], ]; - const intFields: Array<[keyof Settings, string]> = [ - ["DEFAULT_BUFFER_SIZE", "OPENROAD_DEFAULT_BUFFER_SIZE"], - ["MAX_SESSIONS", "OPENROAD_MAX_SESSIONS"], - ["SESSION_QUEUE_SIZE", "OPENROAD_SESSION_QUEUE_SIZE"], - ["READ_CHUNK_SIZE", "OPENROAD_READ_CHUNK_SIZE"], + const intFields: Array<[keyof Settings, string, boolean]> = [ + ["DEFAULT_BUFFER_SIZE", "OPENROAD_DEFAULT_BUFFER_SIZE", false], + ["MAX_SESSIONS", "OPENROAD_MAX_SESSIONS", false], + ["SESSION_QUEUE_SIZE", "OPENROAD_SESSION_QUEUE_SIZE", false], + ["READ_CHUNK_SIZE", "OPENROAD_READ_CHUNK_SIZE", false], ]; const strFields: Array<[keyof Settings, string]> = [ ["LOG_LEVEL", "LOG_LEVEL"], @@ -106,13 +114,13 @@ export class Settings { ["ORFS_FLOW_PATH", "ORFS_FLOW_PATH"], ]; - for (const [field, envKey] of floatFields) { + for (const [field, envKey, allowZero] of floatFields) { const val = process.env[envKey]; - if (val !== undefined) (overrides as Record)[field] = parseFloat_(envKey, val); + if (val !== undefined) (overrides as Record)[field] = parseFloat_(envKey, val, allowZero); } - for (const [field, envKey] of intFields) { + for (const [field, envKey, allowZero] of intFields) { const val = process.env[envKey]; - if (val !== undefined) (overrides as Record)[field] = parseInt_(envKey, val); + if (val !== undefined) (overrides as Record)[field] = parseInt_(envKey, val, allowZero); } for (const [field, envKey] of strFields) { const val = process.env[envKey]; @@ -141,11 +149,6 @@ export class Settings { let _cachedSettings: Settings | null = null; -/** - * Build and cache settings from the environment. Wraps any parsing error with - * context so a misconfigured env var produces an actionable startup message - * instead of a raw error thrown from module initialisation. - */ export function initSettings(): Settings { try { _cachedSettings = Settings.fromEnv(); @@ -156,7 +159,6 @@ export function initSettings(): Settings { return _cachedSettings; } -/** Return the cached settings, initialising them lazily on first access. */ export function getSettings(): Settings { return _cachedSettings ?? initSettings(); } diff --git a/typescript/src/constants.ts b/typescript/src/constants.ts index 2f5e58e..8fb51eb 100644 --- a/typescript/src/constants.ts +++ b/typescript/src/constants.ts @@ -1,26 +1,22 @@ -// Command completion timing -export const MAX_COMMAND_COMPLETION_WINDOW = 0.1; // seconds +export const MAX_COMMAND_COMPLETION_WINDOW = 0.1; -// Process management export const PROCESS_SHUTDOWN_TIMEOUT = 2.0; export const FORCE_EXIT_DELAY_SECONDS = 2; -// Context display limits export const RECENT_OUTPUT_LINES = 20; export const LAST_COMMANDS_COUNT = 5; -// Memory conversion export const BYTES_TO_MB = 1024 * 1024; -// Buffer management export const UTILIZATION_PERCENTAGE_BASE = 100; export const LARGE_BUFFER_THRESHOLD = 10 * 1024 * 1024; export const SIGNIFICANT_LOG_THRESHOLD = 100_000; -// Performance optimization export const CHUNK_JOIN_THRESHOLD = 100; -// I/O logging thresholds export const LARGE_IO_THRESHOLD = 10_000; 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; diff --git a/typescript/src/core/manager.ts b/typescript/src/core/manager.ts index d2276b7..138fb17 100644 --- a/typescript/src/core/manager.ts +++ b/typescript/src/core/manager.ts @@ -27,13 +27,9 @@ export interface CreateSessionOptions { /** * Manages OpenROAD subprocess lifecycle and interactive sessions. * - * Node.js is single-threaded, so no reentrant lock is needed for plain state - * access (Python used asyncio.Lock + the GIL). The async-mutex `cleanupLock` - * serialises the multi-await cleanup/creation sections so concurrent callers - * cannot interleave session-map mutations across await points. - * - * The module exports a shared `manager` singleton; the class is exported too so - * tests can construct isolated instances. + * The async-mutex `cleanupLock` serialises the multi-await cleanup/creation + * sections so concurrent callers cannot interleave session-map mutations + * across await points. */ export class OpenROADManager { private readonly logger = getLogger("manager"); @@ -73,8 +69,8 @@ export class OpenROADManager { this.sessions.set(sessionId, null); try { - // Match Python's `buffer_size or default`: 0 (and undefined) fall back - // to the default so a zero-capacity buffer can't silently drop all output. + // 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); await session.start(opts.command, opts.env, opts.cwd); @@ -92,8 +88,8 @@ export class OpenROADManager { async executeCommand(sessionId: string, command: string, timeoutMs?: number): Promise { const session = this._getSession(sessionId); - // Match Python's `timeout_ms or default`: 0 (and undefined) fall back to the - // configured default rather than becoming an instant timeout. + // 0 (and undefined) fall back to the default rather than becoming an + // instant timeout. const actualTimeout = timeoutMs && timeoutMs > 0 ? timeoutMs : this.defaultTimeoutMs; await session.sendCommand(command); @@ -121,8 +117,10 @@ export class OpenROADManager { async terminateSession(sessionId: string, force = false): Promise { const session = this._getSession(sessionId); + // Do not call cleanup() here: cleanup() clears the output buffer, which + // would discard final output a concurrent reader may still need. The + // session is dropped from the map below, so its buffer is GC'd anyway. await session.terminate(force); - await session.cleanup(); this.logger.info(`Terminated session ${sessionId}`); await this.cleanupLock.runExclusive(() => { @@ -131,7 +129,10 @@ export class OpenROADManager { } async terminateAllSessions(force = false): Promise { - const sessionIds = [...this.sessions.keys()]; + // Skip null placeholders: they belong to an in-flight createSession + // (which resolves or removes them itself), so terminating them would + // throw "still being created" and be lost. + const sessionIds = this._initializedSessions().map(([sid]) => sid); if (sessionIds.length === 0) return 0; const results = await Promise.allSettled( @@ -166,10 +167,6 @@ export class OpenROADManager { async sessionMetrics(): Promise { await this._cleanupTerminatedSessionsWithLock(); - const totalSessions = this.sessions.size; - const activeSessions = this.getActiveSessionCount(); - const terminatedSessions = totalSessions - activeSessions; - const sessionDetails: SessionDetailedMetrics[] = []; let totalCommands = 0; let totalCpuTime = 0; @@ -179,27 +176,33 @@ export class OpenROADManager { try { const metrics = await session.getDetailedMetrics(); sessionDetails.push(metrics); - totalCommands += metrics.commands.total_executed; - totalCpuTime += metrics.performance.total_cpu_time; - totalMemoryMb += metrics.performance.current_memory_mb; + totalCommands += metrics.commands.totalExecuted; + totalCpuTime += metrics.performance.totalCpuTime; + totalMemoryMb += metrics.performance.currentMemoryMb; } catch (e) { this.logger.warn(`Failed to get metrics for session ${session.sessionId}: ${String(e)}`); } } + // Snapshot counts after the async loop so the result reflects the + // post-cleanup state. + const totalSessions = this.sessions.size; + const activeSessions = this.getActiveSessionCount(); + const terminatedSessions = totalSessions - activeSessions; + return { manager: { - total_sessions: totalSessions, - active_sessions: activeSessions, - terminated_sessions: terminatedSessions, - max_sessions: this.maxSessions, - utilization_percent: this.maxSessions > 0 ? (activeSessions / this.maxSessions) * 100 : 0, + totalSessions, + activeSessions, + terminatedSessions, + maxSessions: this.maxSessions, + utilizationPercent: this.maxSessions > 0 ? (activeSessions / this.maxSessions) * 100 : 0, }, aggregate: { - total_commands: totalCommands, - total_cpu_time: totalCpuTime, - total_memory_mb: totalMemoryMb, - avg_memory_per_session: activeSessions > 0 ? totalMemoryMb / activeSessions : 0, + totalCommands, + totalCpuTime, + totalMemoryMb, + avgMemoryPerSession: activeSessions > 0 ? totalMemoryMb / activeSessions : 0, }, sessions: sessionDetails, }; @@ -223,20 +226,7 @@ export class OpenROADManager { async cleanupAll(): Promise { this.logger.info("Starting OpenROAD cleanup"); - await this.terminateAllSessions(true); - - await this.cleanupLock.runExclusive(async () => { - for (const [, session] of this._initializedSessions()) { - try { - await session.cleanup(); - } catch (e) { - this.logger.warn(`Error during session cleanup: ${String(e)}`); - } - } - this.sessions.clear(); - }); - this.logger.info("OpenROAD cleanup completed"); } @@ -248,12 +238,10 @@ export class OpenROADManager { return this._countActive(); } - // internals - private _countActive(): number { let count = 0; for (const session of this.sessions.values()) { - if (session !== null && session.isAlive()) count++; + if (session !== null && session.checkAlive()) count++; } return count; } @@ -286,8 +274,12 @@ export class OpenROADManager { const terminated: Array<[string, InteractiveSession, boolean]> = []; for (const [sessionId, session] of this._initializedSessions()) { - if (!session.isAlive()) { - const timeSinceDeath = (now - session.lastActivity.getTime()) / 1000; + if (!session.checkAlive()) { + // Measure from death time, not lastActivity: a long-idle session + // dies far after its last command, which would trip force-cleanup + // immediately. + const deathTime = (session.terminatedAt ?? session.lastActivity).getTime(); + const timeSinceDeath = (now - deathTime) / 1000; terminated.push([sessionId, session, timeSinceDeath > FORCE_CLEANUP_AFTER_SECONDS]); } } @@ -306,9 +298,12 @@ export class OpenROADManager { cleaned++; } } else { - await session.cleanup(); - this.sessions.delete(sessionId); - cleaned++; + try { + await session.cleanup(); + } finally { + this.sessions.delete(sessionId); + cleaned++; + } } } catch (e) { this.logger.error(`Error during session ${sessionId} cleanup: ${String(e)}`); @@ -326,5 +321,4 @@ export class OpenROADManager { } } -/** Shared process-wide manager instance used by the MCP tools. */ export const manager = new OpenROADManager(); diff --git a/typescript/src/core/models.ts b/typescript/src/core/models.ts index 48d3420..378086e 100644 --- a/typescript/src/core/models.ts +++ b/typescript/src/core/models.ts @@ -39,64 +39,66 @@ export interface InteractiveExecResult { error?: string | null; } -// Opaque snake_case payloads -// These are passed straight through to the wire (no camel->snake conversion), -// matching Python's dict output byte-for-byte. +// Nested domain payloads (camelCase) +// Authored in camelCase like every other domain model and converted to the +// snake_case MCP wire format at the serialization boundary +// (BaseTool.formatResult -> toSnakeCase). Nothing here is hand-written in +// snake_case, so the wire convention is produced by a single rule. /** One entry in a session's command history. */ export interface CommandHistoryEntry { command: string; timestamp: string; - command_number: number; - execution_start: number; - execution_time?: number; - output_length?: number; + commandNumber: number; + executionStart: number; + executionTime?: number; + outputLength?: number; } /** Detailed per-session metrics returned by InteractiveSession.getDetailedMetrics. */ export interface SessionDetailedMetrics { - session_id: string; + sessionId: string; state: string; - is_alive: boolean; - created_at: string; - last_activity: string; - uptime_seconds: number; - idle_seconds: number; + isAlive: boolean; + createdAt: string; + lastActivity: string; + uptimeSeconds: number; + idleSeconds: number; commands: { - total_executed: number; - current_count: number; - history_length: number; + totalExecuted: number; + currentCount: number; + historyLength: number; }; performance: { - total_cpu_time: number; - peak_memory_mb: number; - current_memory_mb: number; + totalCpuTime: number; + peakMemoryMb: number; + currentMemoryMb: number; }; buffer: { - current_size: number; - max_size: number; - utilization_percent: number; + currentSize: number; + maxSize: number; + utilizationPercent: number; }; timeout: { - configured_seconds: number | null; - is_timed_out: boolean; + configuredSeconds: number | null; + isTimedOut: boolean; }; } /** Aggregate metrics across all sessions returned by OpenROADManager.sessionMetrics. */ export interface ManagerMetrics { manager: { - total_sessions: number; - active_sessions: number; - terminated_sessions: number; - max_sessions: number; - utilization_percent: number; + totalSessions: number; + activeSessions: number; + terminatedSessions: number; + maxSessions: number; + utilizationPercent: number; }; aggregate: { - total_commands: number; - total_cpu_time: number; - total_memory_mb: number; - avg_memory_per_session: number; + totalCommands: number; + totalCpuTime: number; + totalMemoryMb: number; + avgMemoryPerSession: number; }; sessions: SessionDetailedMetrics[]; } diff --git a/typescript/src/interactive/buffer.ts b/typescript/src/interactive/buffer.ts index bc33e60..80435d6 100644 --- a/typescript/src/interactive/buffer.ts +++ b/typescript/src/interactive/buffer.ts @@ -37,8 +37,8 @@ export class CircularBuffer { this._totalSize -= old.length; } - // A single chunk that still exceeds maxSize is truncated to the last - // maxSize bytes so the buffer never permanently exceeds its capacity. + // A single chunk larger than maxSize is truncated to its last maxSize + // bytes so capacity is never permanently exceeded. if (this._totalSize > this.maxSize) { const chunk = this._chunks[0]!; this._chunks[0] = chunk.slice(chunk.length - this.maxSize); @@ -96,9 +96,9 @@ export class CircularBuffer { }; // Re-check _dataAvailable under the mutex: runExclusive is async, so - // append() can fire between the fast-path check above and the push below, - // set _dataAvailable = true, drain an empty _resolvers, and release — - // leaving wakeUp unnoticed and the caller waiting the full timeout. + // append() can fire between the fast-path check above and the push + // below, drain an empty _resolvers, and release, leaving wakeUp + // unnoticed and the caller waiting the full timeout. this._mutex.runExclusive(() => { if (this._dataAvailable) { wakeUp(true); diff --git a/typescript/src/interactive/pty_handler.ts b/typescript/src/interactive/pty_handler.ts index 0061a1e..c3adb28 100644 --- a/typescript/src/interactive/pty_handler.ts +++ b/typescript/src/interactive/pty_handler.ts @@ -15,7 +15,6 @@ export class PtyHandler { constructor(private readonly _settings: Settings = getSettings()) {} - /** PID of the underlying PTY process, or null if no process is active. */ get pid(): number | null { return this._ptyProcess?.pid ?? null; } @@ -89,17 +88,21 @@ export class PtyHandler { this._alive = true; this._exitCode = null; - if (onData) { - this._dataDisposable = this._ptyProcess.onData(onData); - } - + // Register exit before onData so a fast-exiting process cannot slip + // its exit event through before we are listening. The guard keeps the + // handler idempotent against a double-delivered exit. this._exitDisposable = this._ptyProcess.onExit(({ exitCode }) => { + if (!this._alive && this._exitCode !== null) return; this._alive = false; this._exitCode = exitCode; const resolvers = this._exitResolvers.splice(0); for (const resolve of resolvers) resolve(exitCode); onExit?.(exitCode); }); + + if (onData) { + this._dataDisposable = this._ptyProcess.onData(onData); + } } catch (e) { if (e instanceof PTYError) throw e; throw new PTYError(`Failed to create PTY session: ${e}`); @@ -118,12 +121,25 @@ export class PtyHandler { } isProcessAlive(): boolean { - return this._alive; + if (!this._alive || !this._ptyProcess) return false; + // Defensive liveness probe in case the exit event was missed; signal 0 + // sends nothing, it only tests the pid. + try { + process.kill(this._ptyProcess.pid, 0); + return true; + } catch (e) { + // EPERM means the pid exists but we may not signal it (e.g. re-parented + // or owned by another user) — the process is still alive. Only ESRCH (no + // such pid) and other failures mean it is gone. + if ((e as NodeJS.ErrnoException).code === "EPERM") return true; + this._alive = false; + return false; + } } async waitForExit(timeoutMs?: number): Promise { - if (!this._ptyProcess) return null; if (this._exitCode !== null) return this._exitCode; + if (!this._ptyProcess) return null; return new Promise((resolve) => { let settled = false; @@ -174,9 +190,9 @@ export class PtyHandler { async cleanup(): Promise { if (this._alive) { try { - await this.terminateProcess(); + await this.terminateProcess(true); } catch { - // Best effort - don't let terminate errors prevent state reset + // best effort } } @@ -190,6 +206,7 @@ export class PtyHandler { this._alive = false; this._dataDisposable = null; this._exitDisposable = null; - this._exitCode = null; + // Preserve _exitCode so a late waitForExit() caller still sees the real + // exit code; createSession() resets it on reuse. } } diff --git a/typescript/src/interactive/session.ts b/typescript/src/interactive/session.ts index ef807df..62528fe 100644 --- a/typescript/src/interactive/session.ts +++ b/typescript/src/interactive/session.ts @@ -1,5 +1,7 @@ import pidusage from "pidusage"; +import { Mutex } from "async-mutex"; import { ANSIDecoder } from "../utils/ansi_decoder.js"; +import { getLogger } from "../utils/logging.js"; import { getSettings } from "../config/settings.js"; import type { Settings } from "../config/settings.js"; import { SessionState } from "../core/models.js"; @@ -9,7 +11,12 @@ import type { InteractiveSessionInfo, SessionDetailedMetrics, } from "../core/models.js"; -import { BYTES_TO_MB, MAX_COMMAND_COMPLETION_WINDOW, UTILIZATION_PERCENTAGE_BASE } from "../constants.js"; +import { + BYTES_TO_MB, + MAX_COMMAND_COMPLETION_WINDOW, + MAX_COMMAND_HISTORY, + UTILIZATION_PERCENTAGE_BASE, +} from "../constants.js"; import { CircularBuffer } from "./buffer.js"; import { SessionError, SessionTerminatedError } from "./models.js"; import { PtyHandler } from "./pty_handler.js"; @@ -48,6 +55,10 @@ export class InteractiveSession { sessionTimeoutSeconds: number | null = null; private _state: SessionState; + // Wall-clock time the process actually died, set on the first TERMINATED + // transition. Used by the manager's force-cleanup timer; lastActivity would + // be wrong because a long-idle session dies far after its last command. + private _terminatedAt: Date | null = null; pty: PtyHandler; readonly outputBuffer: CircularBuffer; @@ -55,6 +66,9 @@ export class InteractiveSession { private _inputWaiters: Array<() => void> = []; private _isShutdown = false; private _writerTask: Promise | null = null; + // Serialises terminate()/cleanup() so concurrent callers cannot double-kill + // the process or deliver a stale exit code to waiters. + private readonly _lifecycleLock = new Mutex(); constructor(sessionId: string, bufferSize?: number, private readonly _settings: Settings = getSettings()) { this.sessionId = sessionId; @@ -69,15 +83,29 @@ export class InteractiveSession { } set state(value: SessionState) { + if (value === SessionState.TERMINATED && this._terminatedAt === null) { + this._terminatedAt = new Date(); + } this._state = value; } - isAlive(): boolean { + /** Wall-clock time the session first became TERMINATED, or null if still alive. */ + get terminatedAt(): Date | null { + return this._terminatedAt; + } + + /** + * Check whether the session is alive, syncing state as a side effect. + * If the underlying PTY process has died since the last check, this + * transitions _state to TERMINATED and signals the writer to stop. + * Named checkAlive (not isAlive) to signal that it is not a pure predicate. + */ + checkAlive(): boolean { if (this._state === SessionState.TERMINATED) return false; const processAlive = this.pty.isProcessAlive(); if (!processAlive && this._state === SessionState.ACTIVE) { - this._state = SessionState.TERMINATED; + this.state = SessionState.TERMINATED; this._signalShutdown(); return false; } @@ -111,9 +139,7 @@ export class InteractiveSession { // circular buffer's eviction logic bounds memory correctly. const appendChunk = (chunk: string): void => { this.outputBuffer.append(chunk).catch(() => { - if (this._state === SessionState.ACTIVE) { - this._state = SessionState.TERMINATED; - } + this._markDead(); this._signalShutdown(); }); }; @@ -128,13 +154,18 @@ export class InteractiveSession { }, (_exitCode: number) => { if (this._state !== SessionState.TERMINATED) { - this._state = SessionState.TERMINATED; + this.state = SessionState.TERMINATED; this._signalShutdown(); } }, ); - this._state = SessionState.ACTIVE; + // Only promote to ACTIVE if the session is still creating. A fast process + // death during startup may already have flipped us to TERMINATED via the + // onData/onExit handlers; do not resurrect it into an undead ACTIVE state. + if (this._state === SessionState.CREATING) { + this._state = SessionState.ACTIVE; + } this._writerTask = this._writeInput(); } catch (e) { this._state = SessionState.ERROR; @@ -144,7 +175,7 @@ export class InteractiveSession { } async sendCommand(command: string): Promise { - if (!this.isAlive()) { + if (!this.checkAlive()) { throw new SessionTerminatedError(`Session ${this.sessionId} is not active`, this.sessionId); } @@ -160,9 +191,14 @@ export class InteractiveSession { this.commandHistory.push({ command: command.trim(), timestamp: new Date().toISOString(), - command_number: this.commandCount + 1, - execution_start: Date.now() / 1000, + commandNumber: this.commandCount + 1, + executionStart: Date.now() / 1000, }); + // Bound history so a long-lived session cannot grow it without limit. + // command_number keeps increasing, so dropping the oldest entry is safe. + if (this.commandHistory.length > MAX_COMMAND_HISTORY) { + this.commandHistory.shift(); + } const data = command.endsWith("\n") ? command : command + "\n"; this._inputQueue.push(data); @@ -177,7 +213,7 @@ export class InteractiveSession { async readOutput(timeoutMs = 1000): Promise { const startTime = Date.now(); - if (!this.isAlive()) { + if (!this.checkAlive()) { // Drain-before-reject: a fast-exiting command (e.g. "exit") can flip // _state to TERMINATED between sendCommand and readOutput because // sendCommand is synchronous and the event loop runs onExit at the @@ -252,7 +288,7 @@ export class InteractiveSession { return { sessionId: this.sessionId, createdAt: this.createdAt.toISOString(), - isAlive: this.isAlive(), + isAlive: this.checkAlive(), commandCount: this.commandCount, bufferSize: this.outputBuffer.size, uptimeSeconds: uptime, @@ -261,33 +297,43 @@ export class InteractiveSession { } async terminate(force = false): Promise { - if (this._state === SessionState.TERMINATED) return; - - this._state = SessionState.TERMINATED; - this._signalShutdown(); + await this._lifecycleLock.runExclusive(async () => { + if (this._state === SessionState.TERMINATED) return; + + if (this._inputQueue.length > 0) { + getLogger("session").warn( + `Session ${this.sessionId}: discarding ${this._inputQueue.length} pending command(s) on terminate`, + ); + this._inputQueue.length = 0; + } + this.state = SessionState.TERMINATED; + this._signalShutdown(); - await this.pty.terminateProcess(force); - await this.pty.cleanup(); + await this.pty.terminateProcess(force); + await this.pty.cleanup(); - if (this._writerTask !== null) { - await this._writerTask; - this._writerTask = null; - } + if (this._writerTask !== null) { + await this._writerTask; + this._writerTask = null; + } + }); } async cleanup(): Promise { - if (this._state !== SessionState.TERMINATED && this._state !== SessionState.ERROR) { - this._state = SessionState.TERMINATED; - } - this._signalShutdown(); + await this._lifecycleLock.runExclusive(async () => { + if (this._state !== SessionState.TERMINATED && this._state !== SessionState.ERROR) { + this.state = SessionState.TERMINATED; + } + this._signalShutdown(); - if (this._writerTask !== null) { - await this._writerTask; - this._writerTask = null; - } + if (this._writerTask !== null) { + await this._writerTask; + this._writerTask = null; + } - await this.pty.cleanup(); - await this.outputBuffer.clear(); + await this.pty.cleanup(); + await this.outputBuffer.clear(); + }); } private _signalShutdown(): void { @@ -296,6 +342,17 @@ export class InteractiveSession { for (const w of waiters) w(); } + /** + * Transition to TERMINATED from any non-terminal state (idempotent). Covers + * CREATING as well as ACTIVE so a session that dies mid-startup is never left + * stranded as an uncollectable CREATING zombie. + */ + private _markDead(): void { + if (this._state !== SessionState.TERMINATED && this._state !== SessionState.ERROR) { + this.state = SessionState.TERMINATED; + } + } + private async _writeInput(): Promise { while (!this._isShutdown) { const data = await this._dequeueInput(1000); @@ -304,9 +361,7 @@ export class InteractiveSession { try { this.pty.writeInput(data); } catch { - if (this._state === SessionState.ACTIVE) { - this._state = SessionState.TERMINATED; - } + this._markDead(); this._signalShutdown(); break; } @@ -348,19 +403,27 @@ export class InteractiveSession { const match = pattern.exec(output); if (match) { const capture = match[1]; - return capture ? template.replace("{0}", capture.trim()) : template; + // 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 undefined; } - /** Update lastActivity and backfill the last history entry after a read. */ + /** + * Update lastActivity and backfill history entries after a read. Walks back + * over every trailing entry still missing execution_time, stopping at the + * first already-recorded one, so commands batched into a single readOutput + * all get timing instead of only the most recent. + */ private _recordReadResult(outputLength: number, executionTime: number): void { - const last = this.commandHistory[this.commandHistory.length - 1]; - if (last && last.execution_time === undefined) { - last.execution_time = executionTime; - last.output_length = outputLength; + for (let i = this.commandHistory.length - 1; i >= 0; i--) { + const entry = this.commandHistory[i]; + if (!entry || entry.executionTime !== undefined) break; + entry.executionTime = executionTime; + entry.outputLength = outputLength; } this.lastActivity = new Date(); } @@ -399,31 +462,31 @@ export class InteractiveSession { const maxSize = this.outputBuffer.maxSize; return { - session_id: this.sessionId, + sessionId: this.sessionId, state: this._state, - is_alive: this.isAlive(), - created_at: this.createdAt.toISOString(), - last_activity: this.lastActivity.toISOString(), - uptime_seconds: uptimeSeconds, - idle_seconds: idleSeconds, + isAlive: this.checkAlive(), + createdAt: this.createdAt.toISOString(), + lastActivity: this.lastActivity.toISOString(), + uptimeSeconds, + idleSeconds, commands: { - total_executed: this.totalCommandsExecuted, - current_count: this.commandCount, - history_length: this.commandHistory.length, + totalExecuted: this.totalCommandsExecuted, + currentCount: this.commandCount, + historyLength: this.commandHistory.length, }, performance: { - total_cpu_time: this.totalCpuTime, - peak_memory_mb: this.peakMemoryMb, - current_memory_mb: currentMemoryMb, + totalCpuTime: this.totalCpuTime, + peakMemoryMb: this.peakMemoryMb, + currentMemoryMb, }, buffer: { - current_size: bufferSize, - max_size: maxSize, - utilization_percent: maxSize > 0 ? (bufferSize / maxSize) * UTILIZATION_PERCENTAGE_BASE : 0, + currentSize: bufferSize, + maxSize, + utilizationPercent: maxSize > 0 ? (bufferSize / maxSize) * UTILIZATION_PERCENTAGE_BASE : 0, }, timeout: { - configured_seconds: this.sessionTimeoutSeconds, - is_timed_out: this._checkSessionTimeout(), + configuredSeconds: this.sessionTimeoutSeconds, + isTimedOut: this._checkSessionTimeout(), }, }; } @@ -439,8 +502,9 @@ export class InteractiveSession { // Sort by timestamp, most recent first. history.sort((a, b) => (a.timestamp < b.timestamp ? 1 : a.timestamp > b.timestamp ? -1 : 0)); - // Match Python's truthy check: limit === 0 leaves the list unsliced. - if (limit) { + // Only a positive limit slices. A zero or negative limit leaves the list + // intact rather than letting `slice(0, -n)` silently drop recent entries. + if (limit !== undefined && limit > 0) { history = history.slice(0, limit); } @@ -449,7 +513,7 @@ export class InteractiveSession { async replayCommand(commandNumber: number): Promise { for (const cmd of this.commandHistory) { - if (cmd.command_number === commandNumber) { + if (cmd.commandNumber === commandNumber) { await this.sendCommand(cmd.command); return cmd.command; } diff --git a/typescript/src/main.ts b/typescript/src/main.ts index e228fb8..697dd4e 100644 --- a/typescript/src/main.ts +++ b/typescript/src/main.ts @@ -1,8 +1,7 @@ import { initSettings } from "./config/settings.js"; -// Eagerly initialise settings at startup so a misconfigured environment variable -// is reported with useful context and a non-zero exit code, rather than crashing -// later from inside module initialisation when settings are first accessed. +// Initialise settings up front so a misconfigured env var fails fast with +// context rather than crashing later from inside module initialisation. try { initSettings(); } catch (e) { diff --git a/typescript/src/tools/base.ts b/typescript/src/tools/base.ts new file mode 100644 index 0000000..83ccab0 --- /dev/null +++ b/typescript/src/tools/base.ts @@ -0,0 +1,36 @@ +import type { OpenROADManager } from "../core/manager.js"; + +function camelToSnakeKey(key: string): string { + return key.replace(/[A-Z]/g, (ch) => `_${ch.toLowerCase()}`); +} + +/** + * Recursively convert camelCase object keys to snake_case. Idempotent on + * already-snake_case keys, so opaque snake_case payloads pass through + * unchanged. + */ +export function toSnakeCase(value: unknown): unknown { + if (Array.isArray(value)) return value.map(toSnakeCase); + if (value !== null && typeof value === "object") { + return Object.fromEntries( + Object.entries(value as Record).map(([k, v]) => [ + camelToSnakeKey(k), + toSnakeCase(v), + ]), + ); + } + return value; +} + +/** + * Base class for MCP tool implementations. Provides the manager dependency + * and a serialization helper that converts the camelCase domain model to the + * snake_case wire format. + */ +export abstract class BaseTool { + protected constructor(protected readonly manager: OpenROADManager) {} + + protected formatResult(result: Record): string { + return JSON.stringify(toSnakeCase(result)); + } +} diff --git a/typescript/src/tools/index.ts b/typescript/src/tools/index.ts new file mode 100644 index 0000000..bdcee09 --- /dev/null +++ b/typescript/src/tools/index.ts @@ -0,0 +1,13 @@ +export { BaseTool, toSnakeCase } from "./base.js"; +export { + CreateSessionTool, + ExecShellTool, + InspectSessionTool, + InteractiveShellTool, + ListSessionsTool, + QueryShellTool, + SessionHistoryTool, + SessionMetricsTool, + TerminateSessionTool, +} from "./interactive.js"; +export { ListReportImagesTool, ReadReportImageTool, classifyImageType, validatePlatformDesign } from "./report_images.js"; diff --git a/typescript/src/tools/interactive.ts b/typescript/src/tools/interactive.ts new file mode 100644 index 0000000..c153fc6 --- /dev/null +++ b/typescript/src/tools/interactive.ts @@ -0,0 +1,426 @@ +import { getSettings } from "../config/settings.js"; +import { + isExecCommand, + isQueryCommand, +} from "../config/command_whitelist.js"; +import type { OpenROADManager } from "../core/manager.js"; +import { + InteractiveSessionListResult, + SessionHistoryResult, + SessionInspectionResult, + SessionMetricsResult, + SessionTerminationResult, +} from "../core/models.js"; +import type { + InteractiveExecResult, + InteractiveSessionInfo, +} from "../core/models.js"; +import { + SessionError, + SessionNotFoundError, + SessionTerminatedError, +} from "../interactive/models.js"; +import { getLogger } from "../utils/logging.js"; +import { BaseTool, toSnakeCase } from "./base.js"; + +const logger = getLogger("tools.interactive"); + +/** Single-quoted Python-style repr for embedding strings in error messages. */ +function pyRepr(s: string): string { + const escaped = s.replace(/\\/g, "\\\\").replace(/'/g, "\\'"); + return `'${escaped}'`; +} + +function blankExecResult( + sessionId: string | null, + error: string, +): InteractiveExecResult { + return { + output: "", + sessionId, + timestamp: new Date().toISOString(), + executionTime: 0.0, + commandCount: 0, + bufferSize: 0, + error, + }; +} + +function sessionNotFoundExecResult( + sessionId: string | null, + error: unknown, +): InteractiveExecResult { + return { + output: `Error: Session '${sessionId}' not found.`, + sessionId, + timestamp: new Date().toISOString(), + executionTime: 0.0, + commandCount: 0, + bufferSize: 0, + error: String(error), + }; +} + +function blockedError( + command: string, + blockedVerb: string, + sessionId: string | null, +): string { + const base: InteractiveExecResult = { + output: "", + sessionId, + timestamp: new Date().toISOString(), + executionTime: 0.0, + commandCount: 0, + bufferSize: 0, + error: `CommandBlocked: '${blockedVerb}'`, + }; + const message = `Command blocked: '${blockedVerb}' is not on the OpenROAD allowlist.\nFull command: ${pyRepr(command)}`; + return JSON.stringify(toSnakeCase({ ...base, message })); +} + +/** + * Returns a serialised blocked-error JSON string when the command is rejected + * by the Tcl whitelist, or null when it is allowed or the whitelist is off. + */ +function applyWhitelist( + command: string, + validator: (cmd: string) => [boolean, string | null], + sessionId: string | null, +): string | null { + const settings = getSettings(); + if (!settings.WHITELIST_ENABLED) return null; + const [allowed, blockedVerb] = validator(command); + if (!allowed && blockedVerb !== null) { + logger.warn( + `Command blocked: '${blockedVerb}' for session ${sessionId ?? "new"}`, + ); + return blockedError(command, blockedVerb, sessionId); + } + return null; +} + +/** Read-only query tool: report_*, get_*, check_*, sta, help, version, etc. */ +export class QueryShellTool extends BaseTool { + constructor(manager: OpenROADManager) { + super(manager); + } + + async execute( + command: string, + sessionId?: string | null, + timeoutMs?: number | null, + ): Promise { + const sid = sessionId ?? null; + + const blocked = applyWhitelist(command, isQueryCommand, sid); + if (blocked !== null) return blocked; + + let resolvedId = sid; + try { + if (resolvedId === null || resolvedId === undefined) { + resolvedId = await this.manager.createSession({}); + } + const result = await this.manager.executeCommand( + resolvedId, + command, + timeoutMs ?? undefined, + ); + return this.formatResult(result as unknown as Record); + } catch (e) { + // Tear down an auto-created session so executeCommand failures do not + // leak it. + if (sid === null && resolvedId !== null) { + this.manager.terminateSession(resolvedId, true).catch(() => { /* best effort */ }); + } + if (e instanceof SessionNotFoundError) { + return this.formatResult( + sessionNotFoundExecResult( + resolvedId, + e, + ) as unknown as Record, + ); + } + if (e instanceof SessionTerminatedError || e instanceof SessionError) { + return this.formatResult( + blankExecResult( + resolvedId, + (e as Error).message, + ) as unknown as Record, + ); + } + return this.formatResult( + blankExecResult( + resolvedId, + `Unexpected error: ${(e as Error).message ?? String(e)}`, + ) as unknown as Record, + ); + } + } +} + +/** State-modifying exec tool: set_*, create_*, read_*, write_*, flow/repair, etc. */ +export class ExecShellTool extends BaseTool { + constructor(manager: OpenROADManager) { + super(manager); + } + + async execute( + command: string, + sessionId?: string | null, + timeoutMs?: number | null, + ): Promise { + const sid = sessionId ?? null; + + const blocked = applyWhitelist(command, isExecCommand, sid); + if (blocked !== null) return blocked; + + let resolvedId = sid; + try { + if (resolvedId === null || resolvedId === undefined) { + resolvedId = await this.manager.createSession({}); + } + const result = await this.manager.executeCommand( + resolvedId, + command, + timeoutMs ?? undefined, + ); + return this.formatResult(result as unknown as Record); + } catch (e) { + if (sid === null && resolvedId !== null) { + this.manager.terminateSession(resolvedId, true).catch(() => { /* best effort */ }); + } + if (e instanceof SessionNotFoundError) { + return this.formatResult( + sessionNotFoundExecResult( + resolvedId, + e, + ) as unknown as Record, + ); + } + if (e instanceof SessionTerminatedError || e instanceof SessionError) { + return this.formatResult( + blankExecResult( + resolvedId, + (e as Error).message, + ) as unknown as Record, + ); + } + return this.formatResult( + blankExecResult( + resolvedId, + `Unexpected error: ${(e as Error).message ?? String(e)}`, + ) as unknown as Record, + ); + } + } +} + +export class ListSessionsTool extends BaseTool { + constructor(manager: OpenROADManager) { + super(manager); + } + + async execute(): Promise { + try { + const sessions = await this.manager.listSessions(); + const activeCount = sessions.filter((s) => s.isAlive).length; + return this.formatResult( + InteractiveSessionListResult.parse({ + sessions, + totalCount: sessions.length, + activeCount, + }) as unknown as Record, + ); + } catch (e) { + return this.formatResult( + InteractiveSessionListResult.parse({ + error: String(e), + }) as unknown as Record, + ); + } + } +} + +export class CreateSessionTool extends BaseTool { + constructor(manager: OpenROADManager) { + super(manager); + } + + async execute( + sessionId?: string, + command?: string[], + env?: Record, + cwd?: string, + ): Promise { + try { + const opts = { + ...(sessionId !== undefined && { sessionId }), + ...(command !== undefined && { command }), + ...(env !== undefined && { env }), + ...(cwd !== undefined && { cwd }), + }; + const id = await this.manager.createSession(opts); + const info = await this.manager.getSessionInfo(id); + return this.formatResult(info as unknown as Record); + } catch (e) { + const errInfo: InteractiveSessionInfo = { + sessionId: sessionId ?? "unknown", + createdAt: new Date().toISOString(), + isAlive: false, + commandCount: 0, + bufferSize: 0, + uptimeSeconds: null, + state: null, + error: String(e), + }; + return this.formatResult(errInfo as unknown as Record); + } + } +} + +export class TerminateSessionTool extends BaseTool { + constructor(manager: OpenROADManager) { + super(manager); + } + + async execute(sessionId: string, force = false): Promise { + let wasAlive = false; + try { + const info = await this.manager.getSessionInfo(sessionId); + wasAlive = info.isAlive; + } catch (e) { + if (!(e instanceof SessionNotFoundError)) throw e; + } + + try { + await this.manager.terminateSession(sessionId, force); + return this.formatResult( + SessionTerminationResult.parse({ + sessionId, + terminated: true, + wasAlive, + force, + }) as unknown as Record, + ); + } catch (e) { + if (e instanceof SessionNotFoundError) { + return this.formatResult( + SessionTerminationResult.parse({ + sessionId, + terminated: false, + wasAlive, + error: String(e), + }) as unknown as Record, + ); + } + return this.formatResult( + SessionTerminationResult.parse({ + sessionId, + terminated: false, + wasAlive, + error: `Termination failed: ${(e as Error).message ?? String(e)}`, + }) as unknown as Record, + ); + } + } +} + +export class InspectSessionTool extends BaseTool { + constructor(manager: OpenROADManager) { + super(manager); + } + + async execute(sessionId: string): Promise { + try { + const metrics = await this.manager.inspectSession(sessionId); + return this.formatResult( + SessionInspectionResult.parse({ + sessionId, + metrics, + }) as unknown as Record, + ); + } catch (e) { + if (e instanceof SessionNotFoundError) { + return this.formatResult( + SessionInspectionResult.parse({ + sessionId, + error: String(e), + }) as unknown as Record, + ); + } + return this.formatResult( + SessionInspectionResult.parse({ + sessionId, + error: `Inspection failed: ${(e as Error).message ?? String(e)}`, + }) as unknown as Record, + ); + } + } +} + +export class SessionHistoryTool extends BaseTool { + constructor(manager: OpenROADManager) { + super(manager); + } + + async execute( + sessionId: string, + limit?: number, + search?: string, + ): Promise { + try { + const history = await this.manager.getSessionHistory(sessionId, limit, search); + return this.formatResult( + SessionHistoryResult.parse({ + sessionId, + history, + totalCommands: history.length, + limit: limit ?? null, + search: search ?? null, + }) as unknown as Record, + ); + } catch (e) { + if (e instanceof SessionNotFoundError) { + return this.formatResult( + SessionHistoryResult.parse({ + sessionId, + error: String(e), + }) as unknown as Record, + ); + } + return this.formatResult( + SessionHistoryResult.parse({ + sessionId, + error: `History retrieval failed: ${(e as Error).message ?? String(e)}`, + }) as unknown as Record, + ); + } + } +} + +export class SessionMetricsTool extends BaseTool { + constructor(manager: OpenROADManager) { + super(manager); + } + + async execute(): Promise { + try { + const metrics = await this.manager.sessionMetrics(); + return this.formatResult( + SessionMetricsResult.parse({ metrics }) as unknown as Record< + string, + unknown + >, + ); + } catch (e) { + return this.formatResult( + SessionMetricsResult.parse({ + error: `Metrics retrieval failed: ${(e as Error).message ?? String(e)}`, + }) as unknown as Record, + ); + } + } +} + +export const InteractiveShellTool = QueryShellTool; diff --git a/typescript/src/tools/report_images.ts b/typescript/src/tools/report_images.ts new file mode 100644 index 0000000..f88ed2b --- /dev/null +++ b/typescript/src/tools/report_images.ts @@ -0,0 +1,475 @@ +import fs from "node:fs"; +import path from "node:path"; +import sharp from "sharp"; +import type { OpenROADManager } from "../core/manager.js"; +import { + ImageInfo, + ImageMetadata, + ListImagesResult, + ReadImageResult, +} from "../core/models.js"; +import { ValidationError } from "../exceptions.js"; +import { + validatePathSegment, + validateSafePathContainment, +} from "../utils/path_security.js"; +import { getSettings } from "../config/settings.js"; +import { getLogger } from "../utils/logging.js"; +import { BaseTool } from "./base.js"; + +const logger = getLogger("tools.report_images"); + +const MAX_BASE64_SIZE_KB = 15; +const MAX_IMAGE_SIZE_MB = 50; + +const IMAGE_TYPE_MAPPING: Record = { + cts_clk: "clock_visualization", + cts_clk_layout: "clock_layout", + cts_core_clock: "core_clock_visualization", + cts_core_clock_layout: "core_clock_layout", + final_all: "complete_design", + final_clocks: "clock_routing", + final_congestion: "congestion_heatmap", + final_ir_drop: "ir_drop_analysis", + final_placement: "cell_placement", + final_resizer: "resizer_results", + final_routing: "routing_visualization", +}; + +/** + * Derive the image stage and semantic type from a filename. Returns + * ["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 underscoreIdx = basename.indexOf("_"); + let stage: string; + let key: string; + if (underscoreIdx === -1) { + stage = "unknown"; + key = basename; + } else { + stage = basename.slice(0, underscoreIdx); + key = basename; + } + const type = IMAGE_TYPE_MAPPING[key] ?? "unknown"; + return [stage, type]; +} + +export function validatePlatformDesign(platform: string, design: string): void { + const settings = getSettings(); + const platforms = settings.platforms; + if (!platforms.includes(platform)) { + throw new ValidationError( + `Platform '${platform}' not found. Available platforms: ${platforms.join(", ") || "none"}`, + ); + } + const designs = settings.designs(platform); + if (!designs.includes(design)) { + throw new ValidationError( + `Design '${design}' not found for platform '${platform}'. Available designs: ${designs.join(", ") || "none"}`, + ); + } +} + +function resolveRunPath( + platform: string, + design: string, + runSlug: string, +): [string, string] { + validatePlatformDesign(platform, design); + validatePathSegment(runSlug, "run_slug"); + const settings = getSettings(); + const reportsBase = path.join(settings.flowPath, "reports", platform, design); + const runPath = path.join(reportsBase, runSlug); + validateSafePathContainment(runPath, reportsBase, "run directory"); + return [reportsBase, runPath]; +} + +function availableRuns(reportsBase: string): string[] { + try { + return fs + .readdirSync(reportsBase, { withFileTypes: true }) + .filter((e) => e.isDirectory()) + .map((e) => e.name); + } catch { + return []; + } +} + +function findWebpFiles(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(full); + } + } + return results; +} + +interface CompressResult { + imageBytes: Buffer; + compressionApplied: boolean; + originalSize: number; + compressedSize: number; + originalWidth: number | null; + originalHeight: number | null; + width: number | null; + height: number | null; +} + +/** + * Compress an image to fit within `maxSizeKb` of base64 output using sharp + * (lanczos3 resize, WebP quality 85). Falls back to raw bytes with null + * dimensions when sharp fails. + */ +async function loadAndCompressImage( + imagePath: string, + maxSizeKb: number = MAX_BASE64_SIZE_KB, +): Promise { + const originalSize = fs.statSync(imagePath).size; + const estimatedBase64 = Math.floor((originalSize * 4) / 3); + + if (estimatedBase64 / 1024 <= maxSizeKb) { + try { + const rawBytes = fs.readFileSync(imagePath); + const meta = await sharp(imagePath).metadata(); + return { + imageBytes: rawBytes, + compressionApplied: false, + originalSize, + compressedSize: originalSize, + originalWidth: meta.width ?? null, + originalHeight: meta.height ?? null, + width: meta.width ?? null, + height: meta.height ?? null, + }; + } catch (e) { + logger.warn({ err: e, imagePath }, "sharp.metadata() failed on small image; returning raw bytes with null dims"); + return { + imageBytes: fs.readFileSync(imagePath), + compressionApplied: false, + originalSize, + compressedSize: originalSize, + originalWidth: null, + originalHeight: null, + width: null, + height: null, + }; + } + } + + try { + const targetBytes = Math.floor((maxSizeKb * 1024 * 3) / 4); + const scale = Math.sqrt(targetBytes / originalSize); + const meta = await sharp(imagePath).metadata(); + if (!meta.width || !meta.height) { + throw new Error("Image dimensions unavailable"); + } + const origW = meta.width; + const origH = meta.height; + const newW = Math.max(Math.round(origW * scale), 256); + const newH = Math.max(Math.round(origH * scale), 256); + const compressed = await sharp(imagePath) + .resize(newW, newH, { kernel: "lanczos3" }) + .webp({ quality: 85 }) + .toBuffer(); + return { + imageBytes: compressed, + compressionApplied: true, + originalSize, + compressedSize: compressed.length, + originalWidth: meta.width ?? null, + originalHeight: meta.height ?? null, + width: newW, + height: newH, + }; + } catch (e) { + logger.warn({ err: e, imagePath }, "Image compression failed; returning raw bytes with null dims"); + return { + imageBytes: fs.readFileSync(imagePath), + compressionApplied: false, + originalSize, + compressedSize: originalSize, + originalWidth: null, + originalHeight: null, + width: null, + height: null, + }; + } +} + +/** Lists .webp report images for a specific platform/design/run. */ +export class ListReportImagesTool extends BaseTool { + constructor(manager: OpenROADManager) { + super(manager); + } + + async execute( + platform: string, + design: string, + runSlug: string, + stage = "all", + ): Promise { + let reportsBase: string; + let runPath: string; + + try { + [reportsBase, runPath] = resolveRunPath(platform, design, runSlug); + } catch (e) { + if (e instanceof ValidationError) { + return this.formatResult( + ListImagesResult.parse({ + error: e.constructor.name, + message: e.message, + }) as unknown as Record, + ); + } + return this.formatResult( + ListImagesResult.parse({ + error: "UnexpectedError", + message: (e as Error).message ?? String(e), + }) as unknown as Record, + ); + } + + if (!fs.existsSync(runPath)) { + const runs = availableRuns(reportsBase); + return this.formatResult( + ListImagesResult.parse({ + error: "RunNotFound", + message: `Run directory '${runSlug}' not found. Available runs: ${runs.join(", ") || "none"}`, + }) as unknown as Record, + ); + } + + try { + let files: string[]; + try { + files = findWebpFiles(runPath); + } catch { + files = []; + } + + if (files.length === 0) { + return this.formatResult( + ListImagesResult.parse({ + runPath, + totalImages: 0, + imagesByStage: {}, + }) as unknown as Record, + ); + } + + const imagesByStage: Record = {}; + let total = 0; + + for (const filePath of files) { + const filename = path.basename(filePath); + const [fileStage, type] = classifyImageType(filename); + if (stage !== "all" && stage !== fileStage) continue; + + const stat = fs.statSync(filePath); + const imageInfo = ImageInfo.parse({ + filename, + path: filePath, + sizeBytes: stat.size, + modifiedTime: stat.mtime.toISOString(), + type, + }); + + const bucket = imagesByStage[fileStage] ?? []; + bucket.push(imageInfo); + imagesByStage[fileStage] = bucket; + total++; + } + + for (const key of Object.keys(imagesByStage)) { + imagesByStage[key] = (imagesByStage[key] as Array<{ filename: string }>).sort((a, b) => + a.filename.localeCompare(b.filename), + ); + } + + return this.formatResult( + ListImagesResult.parse({ + runPath, + totalImages: total, + imagesByStage, + }) as unknown as Record, + ); + } catch (e) { + return this.formatResult( + ListImagesResult.parse({ + error: "UnexpectedError", + message: (e as Error).message ?? String(e), + }) as unknown as Record, + ); + } + } +} + +/** Reads, optionally compresses, and base64-encodes a single report image. */ +export class ReadReportImageTool extends BaseTool { + constructor(manager: OpenROADManager) { + super(manager); + } + + async execute( + platform: string, + design: string, + runSlug: string, + imageName: string, + ): Promise { + let reportsBase: string; + let runPath: string; + + try { + [reportsBase, runPath] = resolveRunPath(platform, design, runSlug); + } catch (e) { + if (e instanceof ValidationError) { + return this.formatResult( + ReadImageResult.parse({ + error: e.constructor.name, + message: e.message, + }) as unknown as Record, + ); + } + return this.formatResult( + ReadImageResult.parse({ + error: "UnexpectedError", + message: (e as Error).message ?? String(e), + }) as unknown as Record, + ); + } + + try { + validatePathSegment(imageName, "image_name"); + } catch (e) { + return this.formatResult( + ReadImageResult.parse({ + error: (e as ValidationError).constructor.name, + message: (e as Error).message, + }) as unknown as Record, + ); + } + + if (!imageName.endsWith(".webp")) { + return this.formatResult( + ReadImageResult.parse({ + error: "InvalidImageName", + message: `Image '${imageName}' must have a .webp extension`, + }) as unknown as Record, + ); + } + + if (!fs.existsSync(runPath)) { + const runs = availableRuns(reportsBase); + return this.formatResult( + ReadImageResult.parse({ + error: "RunNotFound", + message: `Run directory '${runSlug}' not found. Available runs: ${runs.join(", ") || "none"}`, + }) as unknown as Record, + ); + } + + const imagePath = path.join(runPath, imageName); + + try { + validateSafePathContainment(imagePath, runPath, "image file"); + } catch (e) { + return this.formatResult( + ReadImageResult.parse({ + error: (e as ValidationError).constructor.name, + message: (e as Error).message, + }) as unknown as Record, + ); + } + + if (!fs.existsSync(imagePath)) { + let available: string[] = []; + try { + available = findWebpFiles(runPath).map((f) => path.basename(f)); + } catch { + available = []; + } + return this.formatResult( + ReadImageResult.parse({ + error: "ImageNotFound", + message: `Image '${imageName}' not found. Available images: ${available.join(", ") || "none"}`, + }) as unknown as Record, + ); + } + + const stat = fs.statSync(imagePath); + if (!stat.isFile()) { + return this.formatResult( + ReadImageResult.parse({ + error: "NotAFile", + message: `'${imageName}' is not a regular file`, + }) as unknown as Record, + ); + } + + if (stat.size > MAX_IMAGE_SIZE_MB * 1024 * 1024) { + return this.formatResult( + ReadImageResult.parse({ + error: "FileTooLarge", + message: `Image '${imageName}' exceeds the ${MAX_IMAGE_SIZE_MB} MB size limit`, + }) as unknown as Record, + ); + } + + try { + const r = await loadAndCompressImage(imagePath); + const imageData = r.imageBytes.toString("base64"); + const [stage, type] = classifyImageType(imageName); + const compressionRatio = + r.compressionApplied && r.compressedSize > 0 + ? r.originalSize / r.compressedSize + : null; + + const metadata = ImageMetadata.parse({ + filename: imageName, + format: "webp", + sizeBytes: r.compressedSize, + width: r.width, + height: r.height, + modifiedTime: stat.mtime.toISOString(), + stage, + type, + compressionApplied: r.compressionApplied, + originalSizeBytes: r.compressionApplied ? r.originalSize : null, + originalWidth: r.originalWidth, + originalHeight: r.originalHeight, + compressionRatio, + }); + + return this.formatResult( + ReadImageResult.parse({ + imageData, + metadata, + }) as unknown as Record, + ); + } catch (e) { + if (e instanceof ValidationError) { + return this.formatResult( + ReadImageResult.parse({ + error: e.constructor.name, + message: e.message, + }) as unknown as Record, + ); + } + return this.formatResult( + ReadImageResult.parse({ + error: "UnexpectedError", + message: (e as Error).message ?? String(e), + }) as unknown as Record, + ); + } + } +} + diff --git a/typescript/src/utils/ansi_decoder.ts b/typescript/src/utils/ansi_decoder.ts index 9e16ce4..6610a1f 100644 --- a/typescript/src/utils/ansi_decoder.ts +++ b/typescript/src/utils/ansi_decoder.ts @@ -1,19 +1,17 @@ import stripAnsi from "strip-ansi"; +// Specific private-mode codes are listed before the generic private-mode +// catch-all so they match first. The `?` in the generic patterns is escaped, +// preventing single-char escapes like \x1bh from matching there. const ESCAPE_SEQUENCES: Record = { - // Private mode sequences (DECSET/DECRST). Specific codes are listed first so - // they match before the generic private-mode catch-all below. "\\x1b\\[\\?2004h": "Enable bracketed paste mode", "\\x1b\\[\\?2004l": "Disable bracketed paste mode", "\\x1b\\[\\?1049h": "Enable alternative screen buffer", "\\x1b\\[\\?1049l": "Disable alternative screen buffer", "\\x1b\\[\\?25h": "Show cursor", "\\x1b\\[\\?25l": "Hide cursor", - // Generic private-mode set/reset. The `?` is escaped so `[?` is mandatory, - // preventing single-char escapes like \x1bh / \x1bl from matching here. "\\x1b\\[\\?\\d*h": "Enable terminal mode", "\\x1b\\[\\?\\d*l": "Disable terminal mode", - // Cursor movement "\\x1b\\[\\d*A": "Move cursor up", "\\x1b\\[\\d*B": "Move cursor down", "\\x1b\\[\\d*C": "Move cursor right", @@ -26,7 +24,6 @@ const ESCAPE_SEQUENCES: Record = { "\\x1b\\[0K": "Clear line from cursor to end", "\\x1b\\[1K": "Clear line from start to cursor", "\\x1b\\[2K": "Clear entire line", - // Text formatting "\\x1b\\[0m": "Reset all formatting", "\\x1b\\[1m": "Bold text", "\\x1b\\[2m": "Dim text", @@ -36,7 +33,6 @@ const ESCAPE_SEQUENCES: Record = { "\\x1b\\[7m": "Reverse video", "\\x1b\\[8m": "Hidden text", "\\x1b\\[9m": "Strikethrough text", - // Colors (foreground) "\\x1b\\[30m": "Black text", "\\x1b\\[31m": "Red text", "\\x1b\\[32m": "Green text", @@ -45,7 +41,6 @@ const ESCAPE_SEQUENCES: Record = { "\\x1b\\[35m": "Magenta text", "\\x1b\\[36m": "Cyan text", "\\x1b\\[37m": "White text", - // Colors (background) "\\x1b\\[40m": "Black background", "\\x1b\\[41m": "Red background", "\\x1b\\[42m": "Green background", @@ -56,21 +51,17 @@ const ESCAPE_SEQUENCES: Record = { "\\x1b\\[47m": "White background", }; -// Matches the common ANSI escape sequence families so that non-CSI sequences -// (OSC, charset designation, single/two-char escapes) are detected in every -// mode rather than leaking through as raw bytes. Order matters: longer/anchored -// alternatives come first so they win at a given match position. -// 1. OSC: ESC ] ... (BEL | ST) -// 2. CSI: ESC [ params final -// 3. Charset/desig.: ESC ( | ) | # +// Order matters: longer/anchored alternatives come first so they win at a +// given match position. +// 1. OSC: ESC ] ... (BEL | ST) +// 2. CSI: ESC [ params final +// 3. Charset/desig.: ESC ( | ) | # // 4. Single/two-char: ESC const ESCAPE_PATTERN = /\x1b\][^\x07\x1b]*(?:\x07|\x1b\\)|\x1b\[[0-9;?]*[a-zA-Z]|\x1b[()#][0-9A-Za-z]|\x1b[=>NMcDEH78]/g; const VALID_MODES = ["remove", "annotate", "preserve", "decode"]; -// Pre-compile the escape-sequence patterns once at module load instead of -// allocating ~45 RegExp objects on every decodeEscapeSequence() call. const COMPILED_SEQUENCES: Array<[RegExp, string]> = Object.entries(ESCAPE_SEQUENCES).map( ([pattern, description]) => [new RegExp(`^${pattern}`), description], ); @@ -115,7 +106,10 @@ export class ANSIDecoder { if (mode === "annotate") { let result = text; for (const seq of new Set(sequences)) { - result = result.replaceAll(seq, `[${ANSIDecoder.decodeEscapeSequence(seq)}]`); + // Use a function replacement so `$&`/`$1` inside an OSC sequence + // body cannot be reinterpreted as a replacement pattern. + const annotation = `[${ANSIDecoder.decodeEscapeSequence(seq)}]`; + result = result.replaceAll(seq, () => annotation); } return result.replace(/\r\n/g, "\n").replace(/\r/g, "\n"); } @@ -123,7 +117,8 @@ export class ANSIDecoder { if (mode === "preserve") { let result = text; for (const seq of new Set(sequences)) { - result = result.replaceAll(seq, `${seq}[${ANSIDecoder.decodeEscapeSequence(seq)}]`); + const annotated = `${seq}[${ANSIDecoder.decodeEscapeSequence(seq)}]`; + result = result.replaceAll(seq, () => annotated); } return result; } diff --git a/typescript/src/utils/logging.ts b/typescript/src/utils/logging.ts index 3266ea3..d8f3f15 100644 --- a/typescript/src/utils/logging.ts +++ b/typescript/src/utils/logging.ts @@ -1,32 +1,21 @@ import pino from "pino"; import { getSettings } from "../config/settings.js"; -/** - * Pino-based logging for the MCP server. - * - * IMPORTANT: all log output goes to stderr (file descriptor 2). stdout is - * reserved exclusively for the MCP stdio transport - writing logs there would - * corrupt the JSON-RPC protocol stream. - * - * The root level is initialised from settings (env-driven) at module load so - * child loggers created eagerly (e.g. the module-level OpenROADManager - * singleton) honour the configured level without depending on setupLogging() - * being called first. setupLogging() mutates the root level for loggers created - * afterwards; note that pino child loggers capture their level at creation time - * and do not dynamically follow the parent. - */ +// All log output goes to stderr (fd 2). stdout is reserved for the MCP stdio +// transport and any log writes there would corrupt the JSON-RPC stream. +// pino child loggers capture their level at creation, so the root level is +// initialised from settings at module load to honour the configured level for +// eagerly created singletons. function createRoot(level: string): pino.Logger { return pino({ name: "openroad_mcp", level: level.toLowerCase() }, pino.destination(2)); } let rootLogger: pino.Logger = createRoot(getSettings().LOG_LEVEL); -/** Configure the root log level. Call once at startup before heavy logging. */ export function setupLogging(level: string): void { rootLogger.level = level.toLowerCase(); } -/** Return a child logger namespaced under `openroad_mcp.`. */ export function getLogger(name: string): pino.Logger { return rootLogger.child({ module: name }); } diff --git a/typescript/src/utils/path_security.ts b/typescript/src/utils/path_security.ts index 0c57409..10507e1 100644 --- a/typescript/src/utils/path_security.ts +++ b/typescript/src/utils/path_security.ts @@ -3,7 +3,7 @@ import fs from "node:fs"; import { ValidationError } from "../exceptions.js"; export function validatePathSegment(segment: string, segmentName: string): void { - if (!segment) throw new ValidationError(`${segmentName} cannot be empty`); + if (!segment || segment.trim() === "") throw new ValidationError(`${segmentName} cannot be empty`); if (segment === "." || segment === "..") throw new ValidationError(`${segmentName} cannot be '.' or '..'`); if (segment.includes("/") || segment.includes("\\")) throw new ValidationError(`${segmentName} cannot contain path separators`); if (segment.includes("\x00")) throw new ValidationError(`${segmentName} cannot contain null bytes`); @@ -26,10 +26,10 @@ export function validateSafePathContainment(targetPath: string, basePath: string if ((e as NodeJS.ErrnoException).code !== "ENOENT") { throw new ValidationError(`Failed to resolve ${context} path: ${e}`); } - // Walk up to find the longest existing prefix, resolve its symlinks, then - // re-append the non-existent suffix. A plain path.resolve() is unsafe here - // because it won't resolve symlinks in existing parent directories, allowing - // e.g. base/evil_link/nonexistent to escape containment at runtime. + // Walk up to the longest existing prefix, resolve its symlinks, then + // re-append the non-existent suffix. A plain path.resolve() would not + // resolve symlinks in existing parent directories, allowing e.g. + // base/evil_link/nonexistent to escape containment at runtime. const suffix: string[] = []; let current = path.resolve(targetPath); for (;;) {