code cleanup

Author: kartikloops

typescript/__tests__/config/command_whitelist.test.ts

@@ -9,8 +9,6 @@ import {
   isQueryCommand,
 } from "../../src/config/command_whitelist.js";
 
-// extractVerb
-
 describe("extractVerb", () => {
   it("returns a simple command", () => {
     expect(extractVerb("report_checks")).toBe("report_checks");
@@ -53,8 +51,6 @@ describe("extractVerb", () => {
   });
 });
 
-// Pattern set membership
-
 describe("pattern sets", () => {
   it("READONLY contains report/get/check globs", () => {
     expect(READONLY_PATTERNS).toContain("report_*");
@@ -119,8 +115,6 @@ describe("pattern sets", () => {
   });
 });
 
-// isQueryCommand
-
 describe("isQueryCommand", () => {
   it("allows report_*", () => {
     expect(isQueryCommand("report_checks -path_delay max")).toEqual([true, null]);
@@ -243,8 +237,6 @@ describe("isQueryCommand", () => {
   });
 });
 
-// isExecCommand
-
 describe("isExecCommand", () => {
   it("allows set_clock_period", () => {
     expect(isExecCommand("set_clock_period -name clk 2.0")).toEqual([true, null]);
@@ -306,8 +298,6 @@ describe("isExecCommand", () => {
   });
 });
 
-// isCommandAllowed (backward-compat alias)
-
 describe("isCommandAllowed", () => {
   it("mirrors isExecCommand for allowed commands", () => {
     expect(isCommandAllowed("report_checks -path_delay max")).toEqual([true, null]);
@@ -328,18 +318,15 @@ describe("isCommandAllowed", () => {
   });
 });
 
-// minimatch vs fnmatch parity
 
 describe("glob parity (minimatch vs fnmatch)", () => {
   it("matches star-suffix against the empty remainder", () => {
-    // report_ / set_ / read_ match report_* / set_* / read_* (star matches empty)
     expect(isQueryCommand("report_")).toEqual([true, null]);
     expect(isExecCommand("set_")).toEqual([true, null]);
     expect(isExecCommand("read_")).toEqual([true, null]);
   });
 
   it("is case-sensitive like POSIX fnmatch on verbs", () => {
-    // Report_Checks (capitalized) does not match report_* and is unknown -> blocked in query
     expect(isQueryCommand("Report_Checks")).toEqual([false, "Report_Checks"]);
   });
 });

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/session.test.ts

@@ -217,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");
@@ -227,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;
@@ -327,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();
     });
   });
@@ -420,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);
     });

typescript/__tests__/tools/interactive.test.ts

@@ -6,10 +6,6 @@ import { SessionNotFoundError, SessionTerminatedError, SessionError } from "../.
 import { SessionState } from "../../src/core/models.js";
 import type { InteractiveExecResult, InteractiveSessionInfo, SessionDetailedMetrics, ManagerMetrics } from "../../src/core/models.js";
 
-// ---------------------------------------------------------------------------
-// Mock helpers
-// ---------------------------------------------------------------------------
-
 const NOW = "2024-01-01T00:00:00.000Z";
 
 function makeExecResult(overrides: Partial<InteractiveExecResult> = {}): InteractiveExecResult {
@@ -87,10 +83,6 @@ function makeMockManager(): MockManager {
   };
 }
 
-// ---------------------------------------------------------------------------
-// QueryShellTool
-// ---------------------------------------------------------------------------
-
 describe("QueryShellTool", () => {
   let mgr: MockManager;
   let tool: QueryShellTool;
@@ -149,7 +141,6 @@ describe("QueryShellTool", () => {
   });
 
   it("blocks dangerous commands when whitelist is enabled", async () => {
-    // `quit` is in BLOCKED_COMMANDS
     const raw = await tool.execute("quit");
     const result = JSON.parse(raw);
     expect(result.error).toMatch(/CommandBlocked/);
@@ -161,10 +152,6 @@ describe("QueryShellTool", () => {
   });
 });
 
-// ---------------------------------------------------------------------------
-// ExecShellTool
-// ---------------------------------------------------------------------------
-
 describe("ExecShellTool", () => {
   let mgr: MockManager;
   let tool: ExecShellTool;
@@ -195,10 +182,6 @@ describe("ExecShellTool", () => {
   });
 });
 
-// ---------------------------------------------------------------------------
-// ListSessionsTool
-// ---------------------------------------------------------------------------
-
 describe("ListSessionsTool", () => {
   let mgr: MockManager;
   let tool: ListSessionsTool;
@@ -238,10 +221,6 @@ describe("ListSessionsTool", () => {
   });
 });
 
-// ---------------------------------------------------------------------------
-// CreateSessionTool
-// ---------------------------------------------------------------------------
-
 describe("CreateSessionTool", () => {
   let mgr: MockManager;
   let tool: CreateSessionTool;
@@ -278,10 +257,6 @@ describe("CreateSessionTool", () => {
   });
 });
 
-// ---------------------------------------------------------------------------
-// TerminateSessionTool
-// ---------------------------------------------------------------------------
-
 describe("TerminateSessionTool", () => {
   let mgr: MockManager;
   let tool: TerminateSessionTool;
@@ -325,10 +300,6 @@ describe("TerminateSessionTool", () => {
   });
 });
 
-// ---------------------------------------------------------------------------
-// InspectSessionTool
-// ---------------------------------------------------------------------------
-
 describe("InspectSessionTool", () => {
   let mgr: MockManager;
   let tool: InspectSessionTool;
@@ -363,10 +334,6 @@ describe("InspectSessionTool", () => {
   });
 });
 
-// ---------------------------------------------------------------------------
-// SessionHistoryTool
-// ---------------------------------------------------------------------------
-
 describe("SessionHistoryTool", () => {
   let mgr: MockManager;
   let tool: SessionHistoryTool;
@@ -410,10 +377,6 @@ describe("SessionHistoryTool", () => {
   });
 });
 
-// ---------------------------------------------------------------------------
-// SessionMetricsTool
-// ---------------------------------------------------------------------------
-
 describe("SessionMetricsTool", () => {
   let mgr: MockManager;
   let tool: SessionMetricsTool;
@@ -440,12 +403,8 @@ describe("SessionMetricsTool", () => {
   });
 });
 
-// ---------------------------------------------------------------------------
-// Integration: full workflow
-// ---------------------------------------------------------------------------
-
 describe("Integration: session workflow", () => {
-  it("create → execute → list → terminate", async () => {
+  it("create, execute, list, terminate", async () => {
     const mgr = makeMockManager();
     mgr.listSessions.mockResolvedValue([makeSessionInfo()]);
 
@@ -476,9 +435,7 @@ describe("Integration: session workflow", () => {
   });
 });
 
-// ---------------------------------------------------------------------------
 // Snapshot: one representative output per tool
-// ---------------------------------------------------------------------------
 
 describe("Snapshots: wire format stability", () => {
   it("QueryShellTool success output", async () => {