Feat/ts tools (#130)

* add base tool class with camelcase to snake case serialization boundary

* add interactive tool classes porting session management and command execution from python

* add report image tools using sharp for webp compression and path traversal validation

* add tools barrel export

* add tests for interactive shell and session management tools

* add tests for report image tools covering path traversal and platform validation

* verify process liveness with kill(0) so a missed exit event cannot leave the pty marked alive

* serialize session terminate and cleanup with a lifecycle lock to prevent double kill and stale exit codes

* drop redundant cleanup call in terminateSession so final output buffer is not discarded

* use function replacement in annotate and preserve modes so $& in escape sequences is not reinterpreted

* transition session to terminated from any non-terminal state so a startup failure cannot leave a creating zombie

* ignore non-positive history limit so a negative value cannot drop recent commands

* track session death time and use it for force-cleanup timing instead of last activity

* skip null session placeholders in terminateAllSessions so in-progress creates are not lost

* reject infinite and negative float settings so a timeout cannot be silently disabled

* reject negative integer settings so a negative session limit cannot block all creation

* backfill execution_time for all commands batched into a single readOutput

* preserve exit code across cleanup so late waitForExit callers get the real code

* use a live pid in the pty mock so the kill(0) liveness probe sees an active process

* reject whitespace-only path segments in validatePathSegment

* bound per-session command history so long-lived sessions do not leak memory

* block body-eval builtins and bracket substitution so the query tool cannot be bypassed via catch/if/foreach or set x [exec ls]

* terminate auto-created session when executeCommand throws so the session is not leaked

* derive wasAlive from info.isAlive and propagate it through error branches in TerminateSessionTool

* discard queued commands and warn on terminate so pending inputs are not silently dropped

* rename isAlive to checkAlive on InteractiveSession to surface its state-transition side effect

* wrap non-force cleanup in finally so a throwing cleanup still removes the dead session from the map

* snapshot session counts after the async metrics loop so the totals reflect post-loop state

* remove dead session loop in cleanupAll since terminateAllSessions already empties the map

* call terminateProcess with force=true in cleanup so a stuck process is not retried with SIGTERM

* guard against null image dimensions before resize so missing metadata does not produce a silent 256x256 output

* update integration_check to call checkAlive instead of the renamed isAlive

* extend bracket-scan regex to match [::exec ls] so namespace-qualified commands are not exempt

* code cleanup

* add interactive tool wire-format snapshots so CI snapshot tests pass

* clamp brace depth and gate quotes on word boundaries so an unbalanced } or mid-word " cannot hide a trailing statement from the verb check

* walk the report tree manually and skip symlinks so a linked dir or file cannot escape the contained run directory

* reject zero for env settings where it is degenerate (max sessions, queue/buffer/chunk sizes, command/idle timeouts) so a 0 cannot silently disable sessions or output

* scan bracket substitutions per statement and skip comment/blank lines so a bracket inside a # comment is not rejected as an exec

* treat EPERM from the liveness probe as alive so a pid we cannot signal is not misreported dead, only ESRCH counts as gone

* author the metrics and command-history payloads in camelCase like every other domain model so snake_case is produced only by toSnakeCase at the wire boundary, not hand-written

* resolve tcl backslash escapes in the verb and bracket scan so an obfuscated command like \socket or \x73ocket is matched as the command tcl actually runs, not the literal backslash form

* capture the bracket command word up to a delimiter so a dynamic substituted command ($x or nested [[...]]) is surfaced and rejected by the read-only tool instead of running exec via runtime variable substitution

* use function replacement in _detectErrors so $& or $1 in a captured error message is inserted literally instead of being reinterpreted as a replacement pattern

Author: kartikloops

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)

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");

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,14 +202,29 @@ 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<void>(() => {})); // 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", () => {
     it("inspectSession delegates to getDetailedMetrics", async () => {
       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);
     });
   });

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();
 

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", () => {