From dcfee04e1b2d1e500736c9d63a2720bd4ecb78db Mon Sep 17 00:00:00 2001 From: prateek Date: Wed, 18 Feb 2026 03:28:55 +0530 Subject: [PATCH 1/2] fix: terminal servers compatible with hash-based architecture (#87) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: terminal servers compatible with hash-based architecture The terminal WebSocket servers (direct-terminal-ws and terminal-websocket) used config.dataDir to validate sessions, which no longer exists in the hash-based architecture. Also fixed node-pty failing to find tmux via posix_spawnp. Changes: - Remove config.dataDir dependency, validate via `tmux has-session` instead - Add resolveTmuxSession() to map user-facing IDs (ao-15) to hash-prefixed tmux names (8474d6f29887-ao-15) - Use explicit tmux path discovery (findTmux) since node-pty's posix_spawnp doesn't reliably inherit PATH - Include /opt/homebrew/bin in fallback PATH for macOS ARM Co-Authored-By: Claude Opus 4.6 * fix: add session resolution to ttyd server and use exact tmux matching - Add findTmux() and resolveTmuxSession() to terminal-websocket.ts (previously only in direct-terminal-ws.ts), fixing hash-prefixed session lookup for the ttyd-based terminal server - Use tmux exact match prefix (=sessionId) in has-session checks to prevent ao-1 from matching ao-15 via prefix matching - Add server compatibility tests that verify both servers handle hash-based architecture correctly (14 tests) - Include server/ in tsconfig and vitest config for typecheck coverage Co-Authored-By: Claude Opus 4.6 * refactor: extract tmux-utils and add proper unit tests - Extract findTmux(), resolveTmuxSession(), validateSessionId() into shared server/tmux-utils.ts — eliminates duplication between direct-terminal-ws.ts and terminal-websocket.ts - Add 20 real unit tests with injected mocks that test actual behavior: - findTmux: candidate priority, fallback to bare name - resolveTmuxSession: exact match, hash-prefix resolution, = prefix for preventing tmux prefix matching (ao-1 vs ao-15), null when no session found, tmux not running - validateSessionId: path traversal, shell injection, whitespace - Slim down server-compatibility.test.ts to 10 structural checks (imports tmux-utils, no loadConfig, no config.dataDir, no existsSync) Total: 30 tests — all pass on fix branch, 8 fail on main Co-Authored-By: Claude Opus 4.6 * test: add real integration tests for direct-terminal-ws - Refactor direct-terminal-ws to export createDirectTerminalServer() factory so tests can control server lifecycle without side effects - Add 10 integration tests that create real tmux sessions, start the real server, connect via WebSocket, and verify the full flow: - Health endpoint returns 200 - Missing session parameter → close 1008 - Path traversal in session ID → close 1008 - Shell injection in session ID → close 1008 - Nonexistent tmux session → close 1008 - Real tmux session → connects and receives terminal output - Hash-prefixed session resolution works end-to-end - Can send input and receive echoed output - Resize messages don't crash the connection - Unknown HTTP path → 404 - Tests create/destroy tmux sessions in beforeAll/afterAll - Server runs on random port (port 0) to avoid conflicts - Total test suite: 40 tests (20 unit + 10 compatibility + 10 integration) Co-Authored-By: Claude Opus 4.6 * ci: add web server tests to CI pipeline The web package was explicitly excluded from CI test runs (pnpm -r --filter '!@composio/ao-web' test). Add a test-web job that installs tmux, starts the tmux server, and runs the web package tests (unit + integration). Co-Authored-By: Claude Opus 4.6 * fix: address CI failures and bugbot review comments - Fix lint: replace require() with ESM import in integration test - Fix base-path mismatch: ttyd now uses user-facing sessionId for --base-path/URL and actual tmux name for attach-session - Fix bare "tmux" in ttyd spawn args: use TMUX constant (full path) - Scope CI test-web job to server/__tests__/ to avoid pre-existing failures in src/__tests__/ Co-Authored-By: Claude Opus 4.6 * test: comprehensive unit and integration test coverage Unit tests (77): validateSessionId covers all injection vectors (shell, path traversal, command substitution, special chars, unicode, control chars), findTmux covers all candidate paths and error types, resolveTmuxSession covers exact match, hash-prefix resolution, suffix matching precision, edge cases (single char, long lists, multiple hyphens, different tmux paths). Integration tests (45): health endpoint lifecycle (active count tracks connections/disconnections), HTTP routing (404s for all non-health paths), WebSocket validation (11 injection/traversal vectors), terminal connection (resize, multi-resize, invalid JSON, non-resize JSON), hash-prefixed resolution (suffix match, command passthrough, session key tracking, cross-match prevention), terminal I/O (Ctrl-C, Tab, Enter, empty messages, rapid keystrokes, multi-line), connection lifecycle (cleanup, rapid connect/disconnect, error recovery), server creation (independent instances). Co-Authored-By: Claude Opus 4.6 * fix: validate 12-char hex prefix in hash-prefixed session resolution The previous endsWith("-{sessionId}") suffix match was ambiguous: "hash-my-app-1" would falsely match a lookup for "app-1". Now validates that the prefix matches the exact format generated by generateConfigHash (12-char lowercase hex) before comparing the remainder. Added unit tests for the ambiguity case and invalid prefix formats. Updated integration test session names to use proper 12-char hex prefixes. Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- .github/workflows/ci.yml | 19 + .../direct-terminal-ws.integration.test.ts | 760 +++++++++++++++++ .../__tests__/server-compatibility.test.ts | 81 ++ .../web/server/__tests__/tmux-utils.test.ts | 794 ++++++++++++++++++ packages/web/server/direct-terminal-ws.ts | 413 ++++----- packages/web/server/terminal-websocket.ts | 45 +- packages/web/server/tmux-utils.ts | 102 +++ packages/web/tsconfig.json | 2 +- packages/web/vitest.config.ts | 2 +- 9 files changed, 1994 insertions(+), 224 deletions(-) create mode 100644 packages/web/server/__tests__/direct-terminal-ws.integration.test.ts create mode 100644 packages/web/server/__tests__/server-compatibility.test.ts create mode 100644 packages/web/server/__tests__/tmux-utils.test.ts create mode 100644 packages/web/server/tmux-utils.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a7fff9cb6..efa827b84 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,3 +55,22 @@ jobs: - run: pnpm install --frozen-lockfile - run: pnpm -r --filter '!@composio/ao-web' build - run: pnpm test + + test-web: + name: Test (Web) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: pnpm/action-setup@v4 + - uses: actions/setup-node@v4 + with: + node-version: 20 + cache: pnpm + - name: Install tmux + run: sudo apt-get update && sudo apt-get install -y tmux + - name: Start tmux server + run: tmux start-server + - run: pnpm install --frozen-lockfile + - run: pnpm -r --filter '!@composio/ao-web' build + - name: Run web server tests (unit + integration) + run: pnpm --filter @composio/ao-web exec vitest run server/__tests__/ diff --git a/packages/web/server/__tests__/direct-terminal-ws.integration.test.ts b/packages/web/server/__tests__/direct-terminal-ws.integration.test.ts new file mode 100644 index 000000000..2cf132466 --- /dev/null +++ b/packages/web/server/__tests__/direct-terminal-ws.integration.test.ts @@ -0,0 +1,760 @@ +/** + * Integration tests for direct-terminal-ws. + * + * These start the real server, create real tmux sessions, connect via + * WebSocket, and verify the full flow works end-to-end — exactly what + * a user's browser does when opening a terminal on the dashboard. + * + * These tests would have caught the PR #58 breakage because: + * - The server would fail to start (loadConfig crash) + * - Session resolution would fail (config.dataDir doesn't exist) + * - WebSocket connections would be rejected (no tmux session match) + */ + +import { describe, it, expect, beforeAll, afterAll, afterEach } from "vitest"; +import { execFileSync } from "node:child_process"; +import { request, type IncomingMessage } from "node:http"; +import { WebSocket } from "ws"; +import { findTmux } from "../tmux-utils.js"; +import { createDirectTerminalServer, type DirectTerminalServer } from "../direct-terminal-ws.js"; + +const TMUX = findTmux(); +const TEST_SESSION = `ao-test-integration-${process.pid}`; +const TEST_HASH_SESSION = `abcdef123456-${TEST_SESSION}`; + +let terminal: DirectTerminalServer; +let port: number; + +// ============================================================================= +// Helpers +// ============================================================================= + +function httpGet(path: string): Promise<{ status: number; body: string }> { + return new Promise((resolve, reject) => { + const req = request( + { hostname: "localhost", port, path, method: "GET", timeout: 3000 }, + (res: IncomingMessage) => { + let body = ""; + res.on("data", (chunk: Buffer) => { body += chunk.toString(); }); + res.on("end", () => resolve({ status: res.statusCode ?? 0, body })); + }, + ); + req.on("error", reject); + req.end(); + }); +} + +function connectWs(sessionId: string): Promise { + return new Promise((resolve, reject) => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=${sessionId}`); + ws.on("open", () => resolve(ws)); + ws.on("error", reject); + setTimeout(() => reject(new Error("WebSocket connect timeout")), 5000); + }); +} + +function waitForWsClose(ws: WebSocket): Promise<{ code: number; reason: string }> { + return new Promise((resolve) => { + ws.on("close", (code, reason) => { + resolve({ code, reason: reason.toString() }); + }); + setTimeout(() => resolve({ code: -1, reason: "timeout" }), 5000); + }); +} + +function waitForWsData(ws: WebSocket, timeoutMs = 3000): Promise { + return new Promise((resolve, reject) => { + let buf = ""; + const handler = (data: Buffer | string) => { + buf += data.toString(); + if (buf.length > 0) { + ws.off("message", handler); + resolve(buf); + } + }; + ws.on("message", handler); + setTimeout(() => { + ws.off("message", handler); + if (buf.length > 0) resolve(buf); + else reject(new Error("No data received from terminal")); + }, timeoutMs); + }); +} + +/** Wait for output containing a specific marker string */ +function waitForMarker(ws: WebSocket, marker: string, timeoutMs = 3000): Promise { + return new Promise((resolve) => { + let buf = ""; + const handler = (data: Buffer | string) => { + buf += data.toString(); + if (buf.includes(marker)) { + ws.off("message", handler); + resolve(buf); + } + }; + ws.on("message", handler); + setTimeout(() => { ws.off("message", handler); resolve(buf); }, timeoutMs); + }); +} + +// ============================================================================= +// Lifecycle +// ============================================================================= + +beforeAll(() => { + // Create test tmux sessions + execFileSync(TMUX, ["new-session", "-d", "-s", TEST_SESSION, "-x", "80", "-y", "24"], { timeout: 5000 }); + execFileSync(TMUX, ["new-session", "-d", "-s", TEST_HASH_SESSION, "-x", "80", "-y", "24"], { timeout: 5000 }); + + // Start the server on a random port + terminal = createDirectTerminalServer(TMUX); + terminal.server.listen(0); + const addr = terminal.server.address(); + port = typeof addr === "object" && addr ? addr.port : 0; +}); + +afterEach(() => { + // Clean up any active sessions from tests + for (const [, session] of terminal.activeSessions) { + session.pty.kill(); + session.ws.close(); + } + terminal.activeSessions.clear(); +}); + +afterAll(() => { + terminal.shutdown(); + + // Kill test tmux sessions + try { execFileSync(TMUX, ["kill-session", "-t", TEST_SESSION], { timeout: 5000 }); } catch { /* already dead */ } + try { execFileSync(TMUX, ["kill-session", "-t", TEST_HASH_SESSION], { timeout: 5000 }); } catch { /* already dead */ } +}); + +// ============================================================================= +// Health endpoint +// ============================================================================= + +describe("health endpoint", () => { + it("GET /health returns 200 with JSON body", async () => { + const res = await httpGet("/health"); + + expect(res.status).toBe(200); + const data = JSON.parse(res.body); + expect(data).toHaveProperty("active"); + expect(data).toHaveProperty("sessions"); + }); + + it("health shows 0 active sessions initially", async () => { + const res = await httpGet("/health"); + const data = JSON.parse(res.body); + + expect(data.active).toBe(0); + expect(data.sessions).toEqual([]); + }); + + it("health reflects active sessions after WebSocket connection", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + const res = await httpGet("/health"); + const data = JSON.parse(res.body); + + expect(data.active).toBe(1); + expect(data.sessions).toContain(TEST_SESSION); + + ws.close(); + }); + + it("health active count matches number of connections", async () => { + // Create a second tmux session for this test + const secondSession = `ao-test-health-${process.pid}`; + execFileSync(TMUX, ["new-session", "-d", "-s", secondSession, "-x", "80", "-y", "24"], { timeout: 5000 }); + + try { + const ws1 = await connectWs(TEST_SESSION); + await waitForWsData(ws1); + const ws2 = await connectWs(secondSession); + await waitForWsData(ws2); + + const res = await httpGet("/health"); + const data = JSON.parse(res.body); + + expect(data.active).toBe(2); + expect(data.sessions).toContain(TEST_SESSION); + expect(data.sessions).toContain(secondSession); + + ws1.close(); + ws2.close(); + } finally { + try { execFileSync(TMUX, ["kill-session", "-t", secondSession], { timeout: 5000 }); } catch { /* */ } + } + }); + + it("health active count decreases after WebSocket close", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Verify connected + let res = await httpGet("/health"); + expect(JSON.parse(res.body).active).toBe(1); + + // Close and wait for cleanup + ws.close(); + await new Promise((r) => setTimeout(r, 200)); + + res = await httpGet("/health"); + expect(JSON.parse(res.body).active).toBe(0); + }); +}); + +// ============================================================================= +// HTTP routing +// ============================================================================= + +describe("HTTP routing", () => { + it("returns 404 for unknown HTTP path", async () => { + const res = await httpGet("/unknown-path"); + expect(res.status).toBe(404); + }); + + it("returns 404 for root path", async () => { + const res = await httpGet("/"); + expect(res.status).toBe(404); + }); + + it("returns 404 for /terminal (that's the ttyd server's endpoint)", async () => { + const res = await httpGet("/terminal"); + expect(res.status).toBe(404); + }); + + it("returns 404 for /ws via HTTP (not WebSocket upgrade)", async () => { + const res = await httpGet("/ws"); + expect(res.status).toBe(404); + }); +}); + +// ============================================================================= +// WebSocket connection validation +// ============================================================================= + +describe("WebSocket connection validation", () => { + it("rejects connection with no session parameter", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Missing session"); + }); + + it("rejects connection with empty session parameter", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=`); + const result = await waitForWsClose(ws); + + // URL searchParams.get("session") returns "" for ?session=, which is falsy + expect(result.code).toBe(1008); + expect(result.reason).toContain("Missing session"); + }); + + it("rejects connection with path traversal in session ID", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=../../../etc/passwd`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Invalid session ID"); + }); + + it("rejects connection with shell injection in session ID", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=test;rm%20-rf%20/`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Invalid session ID"); + }); + + it("rejects command substitution in session ID", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=test$(whoami)`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Invalid session ID"); + }); + + it("rejects backtick injection in session ID", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=test%60id%60`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Invalid session ID"); + }); + + it("rejects pipe in session ID", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=test|cat%20/etc/passwd`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Invalid session ID"); + }); + + it("rejects forward slash in session ID", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=ao/15`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Invalid session ID"); + }); + + it("rejects spaces in session ID", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=ao%2015`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Invalid session ID"); + }); + + it("rejects connection for nonexistent tmux session", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=ao-nonexistent-999`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Session not found"); + }); + + it("rejects connection for session that doesn't exist in tmux", async () => { + // Valid format but no such tmux session + const ws = new WebSocket(`ws://localhost:${port}/ws?session=definitely-not-real-${Date.now()}`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Session not found"); + }); +}); + +// ============================================================================= +// WebSocket terminal connection — basic +// ============================================================================= + +describe("WebSocket terminal connection", () => { + it("connects to a real tmux session and receives terminal output", async () => { + const ws = await connectWs(TEST_SESSION); + + // tmux sends terminal init sequences on attach + const data = await waitForWsData(ws); + expect(data.length).toBeGreaterThan(0); + + ws.close(); + }); + + it("can send input to the terminal and receive echo", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Send a command — "echo INTEGRATION_TEST_MARKER" + const marker = `MARKER_${Date.now()}`; + ws.send(`echo ${marker}\n`); + + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles resize messages without crashing", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Send resize message (same format xterm.js FitAddon sends) + ws.send(JSON.stringify({ type: "resize", cols: 120, rows: 40 })); + + // Verify connection still works after resize + const marker = `RESIZE_OK_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles multiple resize messages in sequence", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Rapid resize sequence (simulating window drag) + ws.send(JSON.stringify({ type: "resize", cols: 100, rows: 30 })); + ws.send(JSON.stringify({ type: "resize", cols: 110, rows: 35 })); + ws.send(JSON.stringify({ type: "resize", cols: 120, rows: 40 })); + + // Should still work + const marker = `MULTI_RESIZE_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("passes non-resize JSON as terminal input (not intercepted)", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // JSON that doesn't match resize format should pass through as terminal input + ws.send(JSON.stringify({ type: "not-resize", data: "hello" })); + + // Should not crash — the JSON string is written to the terminal + const marker = `JSON_PASSTHROUGH_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles incomplete resize JSON gracefully", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Resize with missing cols — should be treated as terminal input + ws.send(JSON.stringify({ type: "resize", rows: 40 })); + // Resize with missing rows + ws.send(JSON.stringify({ type: "resize", cols: 120 })); + + // Should still work + const marker = `INCOMPLETE_RESIZE_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles invalid JSON starting with { gracefully", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Looks like it might be JSON but isn't + ws.send("{not json at all"); + + // Should not crash — treated as terminal input + const marker = `BADJSON_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); +}); + +// ============================================================================= +// Hash-prefixed session resolution (integration) +// ============================================================================= + +describe("hash-prefixed session resolution", () => { + it("resolves hash-prefixed tmux session by suffix match", async () => { + // Create a session that only exists with a hash prefix (no exact match) + const hashOnlySession = `ao-hashtest-${process.pid}`; + const hashPrefixedName = `deadbeef0123-${hashOnlySession}`; + + execFileSync(TMUX, ["new-session", "-d", "-s", hashPrefixedName, "-x", "80", "-y", "24"], { timeout: 5000 }); + + try { + const ws = await connectWs(hashOnlySession); + + // Should have resolved via hash-prefix match and connected + const data = await waitForWsData(ws); + expect(data.length).toBeGreaterThan(0); + + ws.close(); + } finally { + try { execFileSync(TMUX, ["kill-session", "-t", hashPrefixedName], { timeout: 5000 }); } catch { /* */ } + } + }); + + it("can send input through hash-resolved session", async () => { + const hashOnlySession = `ao-hashcmd-${process.pid}`; + const hashPrefixedName = `cafebabe0123-${hashOnlySession}`; + + execFileSync(TMUX, ["new-session", "-d", "-s", hashPrefixedName, "-x", "80", "-y", "24"], { timeout: 5000 }); + + try { + const ws = await connectWs(hashOnlySession); + await waitForWsData(ws); + + const marker = `HASH_CMD_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + } finally { + try { execFileSync(TMUX, ["kill-session", "-t", hashPrefixedName], { timeout: 5000 }); } catch { /* */ } + } + }); + + it("uses user-facing ID (not hash name) as activeSessions key", async () => { + const hashOnlySession = `ao-hashkey-${process.pid}`; + const hashPrefixedName = `face12340000-${hashOnlySession}`; + + execFileSync(TMUX, ["new-session", "-d", "-s", hashPrefixedName, "-x", "80", "-y", "24"], { timeout: 5000 }); + + try { + const ws = await connectWs(hashOnlySession); + await waitForWsData(ws); + + // The activeSessions map should use the user-facing ID, not the hash-prefixed tmux name + expect(terminal.activeSessions.has(hashOnlySession)).toBe(true); + expect(terminal.activeSessions.has(hashPrefixedName)).toBe(false); + + ws.close(); + } finally { + try { execFileSync(TMUX, ["kill-session", "-t", hashPrefixedName], { timeout: 5000 }); } catch { /* */ } + } + }); + + it("does NOT cross-match ao-1 to hash-ao-15 via prefix", async () => { + // Create "deadbeef0123-ao-test-15-PID" but NOT "ao-test-1-PID" + // Connecting as "ao-test-1-PID" should fail (not match ao-test-15-PID) + const session15 = `ao-crosstest-15-${process.pid}`; + const hashSession15 = `deadbeef01ab-${session15}`; + const session1 = `ao-crosstest-1-${process.pid}`; + + execFileSync(TMUX, ["new-session", "-d", "-s", hashSession15, "-x", "80", "-y", "24"], { timeout: 5000 }); + + try { + // ao-crosstest-1-PID should NOT resolve to deadbeef01ab-ao-crosstest-15-PID + const ws = new WebSocket(`ws://localhost:${port}/ws?session=${session1}`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Session not found"); + } finally { + try { execFileSync(TMUX, ["kill-session", "-t", hashSession15], { timeout: 5000 }); } catch { /* */ } + } + }); +}); + +// ============================================================================= +// Terminal I/O +// ============================================================================= + +describe("terminal I/O", () => { + it("can run a command and get output", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + const marker = `PWD_TEST_${Date.now()}`; + ws.send(`echo ${marker}_$(pwd | wc -c)\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles special terminal characters (Ctrl-C as \\x03)", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Start a long-running process + ws.send("sleep 9999\n"); + await new Promise((r) => setTimeout(r, 200)); + + // Send Ctrl-C to interrupt it + ws.send("\x03"); + await new Promise((r) => setTimeout(r, 200)); + + // Should be back at the prompt — test by echoing a marker + const marker = `CTRLC_OK_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles Tab key (\\t) for completion", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Tab should not crash the connection + ws.send("ech\t"); + await new Promise((r) => setTimeout(r, 300)); + + // Clear with Ctrl-C and verify still working + ws.send("\x03"); + await new Promise((r) => setTimeout(r, 200)); + + const marker = `TAB_OK_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles Enter key (\\r or \\n)", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Enter via \r (what xterm.js typically sends) + const marker = `ENTER_TEST_${Date.now()}`; + ws.send(`echo ${marker}\r`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles empty messages without crashing", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Send empty string + ws.send(""); + + // Should still work + const marker = `EMPTY_MSG_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles rapid keystrokes", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Simulate rapid typing + const chars = "echo RAPID_TEST\n"; + for (const ch of chars) { + ws.send(ch); + } + + const output = await waitForMarker(ws, "RAPID_TEST"); + expect(output).toContain("RAPID_TEST"); + + ws.close(); + }); + + it("handles multi-line input", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + const marker = `MULTILINE_${Date.now()}`; + ws.send(`echo "line1" && echo "${marker}"\n`); + + const output = await waitForMarker(ws, marker); + expect(output).toContain("line1"); + expect(output).toContain(marker); + + ws.close(); + }); +}); + +// ============================================================================= +// Connection lifecycle +// ============================================================================= + +describe("connection lifecycle", () => { + it("cleans up activeSessions on WebSocket close", async () => { + // Use a dedicated session to avoid race conditions with afterEach cleanup + const cleanupSession = `ao-test-cleanup-${process.pid}`; + execFileSync(TMUX, ["new-session", "-d", "-s", cleanupSession, "-x", "80", "-y", "24"], { timeout: 5000 }); + + try { + const ws = await connectWs(cleanupSession); + await waitForWsData(ws); + + // Verify the session was registered + expect(terminal.activeSessions.has(cleanupSession)).toBe(true); + + ws.close(); + await new Promise((r) => setTimeout(r, 300)); + + // After close, the session should be cleaned up + expect(terminal.activeSessions.has(cleanupSession)).toBe(false); + } finally { + try { execFileSync(TMUX, ["kill-session", "-t", cleanupSession], { timeout: 5000 }); } catch { /* */ } + } + }); + + it("tracks session by user-facing ID in activeSessions", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + expect(terminal.activeSessions.has(TEST_SESSION)).toBe(true); + + const session = terminal.activeSessions.get(TEST_SESSION); + expect(session).toBeDefined(); + expect(session!.sessionId).toBe(TEST_SESSION); + + ws.close(); + }); + + it("handles rapid connect and disconnect", async () => { + // Connect and immediately close multiple times + for (let i = 0; i < 3; i++) { + const ws = await connectWs(TEST_SESSION); + ws.close(); + await new Promise((r) => setTimeout(r, 100)); + } + + // Server should still be healthy + const res = await httpGet("/health"); + expect(res.status).toBe(200); + }); + + it("server stays healthy after connection errors", async () => { + // Try invalid connection + const ws = new WebSocket(`ws://localhost:${port}/ws?session=nonexistent-${Date.now()}`); + await waitForWsClose(ws); + + // Server should still be healthy + const res = await httpGet("/health"); + expect(res.status).toBe(200); + expect(JSON.parse(res.body).active).toBe(0); + }); + + it("multiple health checks work consistently", async () => { + // Rapid health checks shouldn't break anything + const results = await Promise.all([ + httpGet("/health"), + httpGet("/health"), + httpGet("/health"), + ]); + + for (const res of results) { + expect(res.status).toBe(200); + const data = JSON.parse(res.body); + expect(typeof data.active).toBe("number"); + } + }); +}); + +// ============================================================================= +// Server creation +// ============================================================================= + +describe("server creation", () => { + it("createDirectTerminalServer returns all expected properties", () => { + expect(terminal).toHaveProperty("server"); + expect(terminal).toHaveProperty("wss"); + expect(terminal).toHaveProperty("activeSessions"); + expect(terminal).toHaveProperty("shutdown"); + expect(terminal.activeSessions).toBeInstanceOf(Map); + expect(typeof terminal.shutdown).toBe("function"); + }); + + it("can create multiple independent servers", () => { + const server2 = createDirectTerminalServer(TMUX); + server2.server.listen(0); + const addr = server2.server.address(); + const port2 = typeof addr === "object" && addr ? addr.port : 0; + + expect(port2).toBeGreaterThan(0); + expect(port2).not.toBe(port); + + // Independent activeSessions + expect(server2.activeSessions).not.toBe(terminal.activeSessions); + + server2.shutdown(); + }); +}); diff --git a/packages/web/server/__tests__/server-compatibility.test.ts b/packages/web/server/__tests__/server-compatibility.test.ts new file mode 100644 index 000000000..193f83b9a --- /dev/null +++ b/packages/web/server/__tests__/server-compatibility.test.ts @@ -0,0 +1,81 @@ +/** + * Server compatibility tests. + * + * These verify that the terminal server files import shared utilities + * from tmux-utils.ts and don't contain deprecated patterns from the + * pre-hash-based architecture (config.dataDir, loadConfig, existsSync). + * + * For actual behavioral tests of the shared utilities, see tmux-utils.test.ts. + */ + +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +const serverDir = join(__dirname, ".."); + +function readServerFile(name: string): string { + return readFileSync(join(serverDir, name), "utf-8"); +} + +describe("direct-terminal-ws.ts", () => { + const source = readServerFile("direct-terminal-ws.ts"); + + it("imports from shared tmux-utils", () => { + expect(source).toMatch(/from\s+["']\.\/tmux-utils/); + }); + + it("does not import loadConfig from @composio/ao-core", () => { + expect(source).not.toMatch(/import\s.*loadConfig.*from\s+["']@composio\/ao-core["']/); + }); + + it("does not reference config.dataDir", () => { + expect(source).not.toMatch(/config\.dataDir/); + }); + + it("does not use bare 'tmux' string for ptySpawn", () => { + expect(source).not.toMatch(/ptySpawn\(\s*["']tmux["']/); + }); + + it("does not check file existence for session validation", () => { + expect(source).not.toMatch(/existsSync.*session/i); + }); +}); + +describe("terminal-websocket.ts", () => { + const source = readServerFile("terminal-websocket.ts"); + + it("imports from shared tmux-utils", () => { + expect(source).toMatch(/from\s+["']\.\/tmux-utils/); + }); + + it("does not import loadConfig from @composio/ao-core", () => { + expect(source).not.toMatch(/import\s.*loadConfig.*from\s+["']@composio\/ao-core["']/); + }); + + it("does not reference config.dataDir", () => { + expect(source).not.toMatch(/config\.dataDir/); + }); + + it("does not check file existence for session validation", () => { + expect(source).not.toMatch(/existsSync.*session/i); + }); +}); + +describe("OrchestratorConfig compatibility", () => { + it("OrchestratorConfig does not have dataDir property", () => { + const typesSource = readFileSync( + join(__dirname, "..", "..", "..", "core", "src", "types.ts"), + "utf-8", + ); + + const configMatch = typesSource.match( + /export interface OrchestratorConfig \{[\s\S]*?\n\}/, + ); + expect(configMatch).toBeTruthy(); + const configBlock = configMatch![0]; + + expect(configBlock).not.toMatch(/dataDir/); + expect(configBlock).toMatch(/configPath/); + }); +}); diff --git a/packages/web/server/__tests__/tmux-utils.test.ts b/packages/web/server/__tests__/tmux-utils.test.ts new file mode 100644 index 000000000..a4d3581a8 --- /dev/null +++ b/packages/web/server/__tests__/tmux-utils.test.ts @@ -0,0 +1,794 @@ +/** + * Unit tests for tmux-utils. + * + * These test actual behavior by injecting mock execFileSync functions, + * verifying the logic handles all edge cases correctly. + */ + +import { describe, it, expect, vi } from "vitest"; +import { findTmux, resolveTmuxSession, validateSessionId, SESSION_ID_PATTERN } from "../tmux-utils.js"; + +// ============================================================================= +// validateSessionId +// ============================================================================= + +describe("validateSessionId", () => { + describe("accepts valid IDs", () => { + it("accepts simple alphanumeric IDs", () => { + expect(validateSessionId("ao-15")).toBe(true); + expect(validateSessionId("ao_orchestrator")).toBe(true); + expect(validateSessionId("session123")).toBe(true); + }); + + it("accepts hash-prefixed IDs", () => { + expect(validateSessionId("8474d6f29887-ao-15")).toBe(true); + expect(validateSessionId("abcdef123456-my-session")).toBe(true); + }); + + it("accepts single character IDs", () => { + expect(validateSessionId("a")).toBe(true); + expect(validateSessionId("1")).toBe(true); + expect(validateSessionId("-")).toBe(true); + expect(validateSessionId("_")).toBe(true); + }); + + it("accepts numbers-only IDs", () => { + expect(validateSessionId("12345")).toBe(true); + expect(validateSessionId("0")).toBe(true); + }); + + it("accepts hyphens and underscores only", () => { + expect(validateSessionId("---")).toBe(true); + expect(validateSessionId("___")).toBe(true); + expect(validateSessionId("-_-")).toBe(true); + }); + + it("accepts uppercase letters", () => { + expect(validateSessionId("AO-15")).toBe(true); + expect(validateSessionId("MySession")).toBe(true); + expect(validateSessionId("ALLCAPS")).toBe(true); + }); + + it("accepts long session IDs", () => { + const longId = "a".repeat(200); + expect(validateSessionId(longId)).toBe(true); + }); + + it("accepts realistic session names from the orchestrator", () => { + expect(validateSessionId("ao-orchestrator")).toBe(true); + expect(validateSessionId("ao-1")).toBe(true); + expect(validateSessionId("ao-99")).toBe(true); + expect(validateSessionId("integrator-44")).toBe(true); + expect(validateSessionId("splitly-3")).toBe(true); + expect(validateSessionId("8474d6f29887-ao-15")).toBe(true); + expect(validateSessionId("deadbeef1234-integrator-7")).toBe(true); + }); + }); + + describe("rejects empty and whitespace", () => { + it("rejects empty string", () => { + expect(validateSessionId("")).toBe(false); + }); + + it("rejects spaces", () => { + expect(validateSessionId("ao 15")).toBe(false); + expect(validateSessionId(" ao-15")).toBe(false); + expect(validateSessionId("ao-15 ")).toBe(false); + expect(validateSessionId(" ")).toBe(false); + }); + + it("rejects tabs and newlines", () => { + expect(validateSessionId("ao\t15")).toBe(false); + expect(validateSessionId("ao\n15")).toBe(false); + expect(validateSessionId("ao\r15")).toBe(false); + expect(validateSessionId("\t")).toBe(false); + }); + }); + + describe("rejects path traversal", () => { + it("rejects dot-dot-slash sequences", () => { + expect(validateSessionId("../etc/passwd")).toBe(false); + expect(validateSessionId("ao-15/../../secret")).toBe(false); + expect(validateSessionId("..")).toBe(false); + }); + + it("rejects forward slashes", () => { + expect(validateSessionId("ao/15")).toBe(false); + expect(validateSessionId("/etc/passwd")).toBe(false); + expect(validateSessionId("a/b")).toBe(false); + }); + + it("rejects backslashes", () => { + expect(validateSessionId("ao\\15")).toBe(false); + expect(validateSessionId("..\\..\\secret")).toBe(false); + }); + + it("rejects dots (current directory reference)", () => { + expect(validateSessionId(".")).toBe(false); + expect(validateSessionId("ao.15")).toBe(false); + }); + }); + + describe("rejects shell injection", () => { + it("rejects semicolons", () => { + expect(validateSessionId("ao-15; rm -rf /")).toBe(false); + expect(validateSessionId(";id")).toBe(false); + }); + + it("rejects command substitution", () => { + expect(validateSessionId("ao-15$(whoami)")).toBe(false); + expect(validateSessionId("$(cat /etc/passwd)")).toBe(false); + }); + + it("rejects backticks", () => { + expect(validateSessionId("ao-15`id`")).toBe(false); + expect(validateSessionId("`rm -rf /`")).toBe(false); + }); + + it("rejects pipes", () => { + expect(validateSessionId("ao-15|cat /etc/passwd")).toBe(false); + expect(validateSessionId("|id")).toBe(false); + }); + + it("rejects ampersands", () => { + expect(validateSessionId("ao-15&sleep 10")).toBe(false); + expect(validateSessionId("ao-15&&id")).toBe(false); + }); + + it("rejects angle brackets (redirection)", () => { + expect(validateSessionId("ao-15>output")).toBe(false); + expect(validateSessionId("ao-15 { + expect(validateSessionId("ao-15'")).toBe(false); + expect(validateSessionId('ao-15"')).toBe(false); + }); + + it("rejects dollar signs", () => { + expect(validateSessionId("$HOME")).toBe(false); + expect(validateSessionId("ao-${15}")).toBe(false); + }); + + it("rejects parentheses", () => { + expect(validateSessionId("ao(15)")).toBe(false); + expect(validateSessionId("(id)")).toBe(false); + }); + + it("rejects hash/pound sign", () => { + expect(validateSessionId("ao#15")).toBe(false); + expect(validateSessionId("#comment")).toBe(false); + }); + + it("rejects exclamation mark", () => { + expect(validateSessionId("ao!15")).toBe(false); + }); + + it("rejects asterisk and question mark (glob)", () => { + expect(validateSessionId("ao*")).toBe(false); + expect(validateSessionId("ao?15")).toBe(false); + }); + + it("rejects tilde (home directory)", () => { + expect(validateSessionId("~")).toBe(false); + expect(validateSessionId("~/something")).toBe(false); + }); + }); + + describe("rejects other dangerous characters", () => { + it("rejects null bytes", () => { + expect(validateSessionId("ao\x0015")).toBe(false); + }); + + it("rejects unicode", () => { + expect(validateSessionId("ao-\u00e9")).toBe(false); + expect(validateSessionId("ao-\u200b15")).toBe(false); // zero-width space + }); + + it("rejects control characters", () => { + expect(validateSessionId("ao\x01")).toBe(false); + expect(validateSessionId("ao\x7f")).toBe(false); // DEL + }); + + it("rejects percent (URL encoding attempts)", () => { + expect(validateSessionId("ao%2F15")).toBe(false); + expect(validateSessionId("%00")).toBe(false); + }); + + it("rejects at sign", () => { + expect(validateSessionId("user@host")).toBe(false); + }); + + it("rejects colons", () => { + expect(validateSessionId("ao:15")).toBe(false); + }); + + it("rejects equals sign", () => { + expect(validateSessionId("ao=15")).toBe(false); + }); + + it("rejects square brackets", () => { + expect(validateSessionId("ao[15]")).toBe(false); + }); + + it("rejects curly braces", () => { + expect(validateSessionId("ao{15}")).toBe(false); + }); + + it("rejects comma", () => { + expect(validateSessionId("ao,15")).toBe(false); + }); + }); + + describe("SESSION_ID_PATTERN export", () => { + it("is exported and matches the same pattern", () => { + expect(SESSION_ID_PATTERN).toBeInstanceOf(RegExp); + expect(SESSION_ID_PATTERN.test("ao-15")).toBe(true); + expect(SESSION_ID_PATTERN.test("../bad")).toBe(false); + }); + }); +}); + +// ============================================================================= +// findTmux +// ============================================================================= + +describe("findTmux", () => { + it("returns first candidate that succeeds", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { throw new Error("not found"); }) // /opt/homebrew/bin/tmux + .mockImplementationOnce(() => "tmux 3.4") // /usr/local/bin/tmux succeeds + .mockImplementationOnce(() => "tmux 3.4"); // /usr/bin/tmux (not reached) + + const result = findTmux(mockExec); + + expect(result).toBe("/usr/local/bin/tmux"); + expect(mockExec).toHaveBeenCalledTimes(2); + }); + + it("returns /opt/homebrew/bin/tmux on macOS ARM (first candidate)", () => { + const mockExec = vi.fn().mockReturnValue("tmux 3.4"); + + const result = findTmux(mockExec); + + expect(result).toBe("/opt/homebrew/bin/tmux"); + expect(mockExec).toHaveBeenCalledTimes(1); + }); + + it("returns /usr/bin/tmux on Linux (third candidate)", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { throw new Error("not found"); }) // /opt/homebrew/bin/tmux + .mockImplementationOnce(() => { throw new Error("not found"); }) // /usr/local/bin/tmux + .mockImplementationOnce(() => "tmux 3.3a"); // /usr/bin/tmux + + const result = findTmux(mockExec); + + expect(result).toBe("/usr/bin/tmux"); + expect(mockExec).toHaveBeenCalledTimes(3); + }); + + it("falls back to bare 'tmux' when no candidates found", () => { + const mockExec = vi.fn().mockImplementation(() => { + throw new Error("not found"); + }); + + const result = findTmux(mockExec); + + expect(result).toBe("tmux"); + expect(mockExec).toHaveBeenCalledTimes(3); + }); + + it("checks all three standard locations in order", () => { + const mockExec = vi.fn().mockImplementation(() => { + throw new Error("not found"); + }); + + findTmux(mockExec); + + expect(mockExec).toHaveBeenNthCalledWith(1, "/opt/homebrew/bin/tmux", ["-V"], { timeout: 5000 }); + expect(mockExec).toHaveBeenNthCalledWith(2, "/usr/local/bin/tmux", ["-V"], { timeout: 5000 }); + expect(mockExec).toHaveBeenNthCalledWith(3, "/usr/bin/tmux", ["-V"], { timeout: 5000 }); + }); + + it("handles timeout errors from execFileSync", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { throw Object.assign(new Error("ETIMEDOUT"), { code: "ETIMEDOUT" }); }) + .mockImplementationOnce(() => "tmux 3.4"); + + const result = findTmux(mockExec); + + expect(result).toBe("/usr/local/bin/tmux"); + }); + + it("handles permission denied errors", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { throw Object.assign(new Error("EACCES"), { code: "EACCES" }); }) + .mockImplementationOnce(() => { throw Object.assign(new Error("EACCES"), { code: "EACCES" }); }) + .mockImplementationOnce(() => "tmux 3.3a"); + + const result = findTmux(mockExec); + + expect(result).toBe("/usr/bin/tmux"); + }); + + it("handles ENOENT (file not found) errors", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { throw Object.assign(new Error("ENOENT"), { code: "ENOENT" }); }) + .mockImplementationOnce(() => "tmux 3.4"); + + const result = findTmux(mockExec); + + expect(result).toBe("/usr/local/bin/tmux"); + }); + + it("passes -V flag and 5000ms timeout to each candidate", () => { + const mockExec = vi.fn().mockReturnValue("tmux 3.4"); + + findTmux(mockExec); + + const [, args, options] = mockExec.mock.calls[0]; + expect(args).toEqual(["-V"]); + expect(options).toEqual({ timeout: 5000 }); + }); + + it("stops checking after first success (short-circuit)", () => { + const mockExec = vi.fn().mockReturnValue("tmux 3.4"); + + findTmux(mockExec); + + expect(mockExec).toHaveBeenCalledTimes(1); + }); +}); + +// ============================================================================= +// resolveTmuxSession +// ============================================================================= + +describe("resolveTmuxSession", () => { + const TMUX = "/opt/homebrew/bin/tmux"; + + // --------------------------------------------------------------------------- + // Exact match behavior + // --------------------------------------------------------------------------- + + describe("exact match", () => { + it("returns sessionId for exact match", () => { + const mockExec = vi.fn().mockReturnValue(""); + + const result = resolveTmuxSession("ao-orchestrator", TMUX, mockExec); + + expect(result).toBe("ao-orchestrator"); + }); + + it("uses = prefix to prevent tmux prefix matching", () => { + // This is the critical bugbot fix: without =, "ao-1" matches "ao-15" + const mockExec = vi.fn().mockReturnValue(""); + + resolveTmuxSession("ao-1", TMUX, mockExec); + + // Must pass "=ao-1" not "ao-1" to has-session + expect(mockExec).toHaveBeenCalledWith( + TMUX, + ["has-session", "-t", "=ao-1"], + { timeout: 5000 }, + ); + }); + + it("passes correct tmux path and timeout to has-session", () => { + const customTmux = "/usr/bin/tmux"; + const mockExec = vi.fn().mockReturnValue(""); + + resolveTmuxSession("my-session", customTmux, mockExec); + + expect(mockExec).toHaveBeenCalledWith( + customTmux, + ["has-session", "-t", "=my-session"], + { timeout: 5000 }, + ); + }); + + it("prefers exact match over hash-prefixed match", () => { + // If "ao-15" exists as both exact and hash-prefixed, return exact + const mockExec = vi.fn().mockReturnValue(""); + + const result = resolveTmuxSession("ao-15", TMUX, mockExec); + + expect(result).toBe("ao-15"); + // Should only call has-session, not list-sessions + expect(mockExec).toHaveBeenCalledTimes(1); + }); + + it("does not call list-sessions when exact match succeeds", () => { + const mockExec = vi.fn().mockReturnValue(""); + + resolveTmuxSession("ao-15", TMUX, mockExec); + + expect(mockExec).toHaveBeenCalledTimes(1); + expect(mockExec).toHaveBeenCalledWith( + TMUX, + ["has-session", "-t", "=ao-15"], + { timeout: 5000 }, + ); + }); + }); + + // --------------------------------------------------------------------------- + // Hash-prefix resolution + // --------------------------------------------------------------------------- + + describe("hash-prefix resolution", () => { + it("resolves hash-prefixed session when exact match fails", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "8474d6f29887-ao-15\na1b2c3d4e5f6-ao-16\nao-orchestrator\n"; + }); + + const result = resolveTmuxSession("ao-15", TMUX, mockExec); + + expect(result).toBe("8474d6f29887-ao-15"); + }); + + it("uses exact match after hash prefix (not endsWith)", () => { + // "ao-1" should NOT match "8474d6f29887-ao-15" because substring(13) is "ao-15" not "ao-1" + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "8474d6f29887-ao-15\n8474d6f29887-ao-16\n"; + }); + + const result = resolveTmuxSession("ao-1", TMUX, mockExec); + + expect(result).toBeNull(); + }); + + it("does NOT match ambiguous suffixes (bugbot: hash-my-app-1 vs app-1)", () => { + // This is the critical bugbot fix: "hash-my-app-1" ends with "-app-1" + // but the hash prefix is not a valid 12-char hex string, so it shouldn't match. + // A user looking up "app-1" should NOT be connected to "hash-my-app-1". + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "nonhexprefix-my-app-1\n8474d6f29887-app-1\n"; + }); + + // Should match the one with valid hash prefix, not the ambiguous one + expect(resolveTmuxSession("app-1", TMUX, mockExec)).toBe("8474d6f29887-app-1"); + }); + + it("rejects session names where hash prefix is not 12-char hex", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + // These look like hash-prefixed but aren't valid 12-char hex + return [ + "short-ao-15", // too short + "toolonghashprefix-ao-15", // too long + "ABCDEF123456-ao-15", // uppercase hex (not matching [a-f0-9]) + "zzzzzzzzzzzz-ao-15", // not hex chars + "8474d6f2988-ao-15", // 11 chars (one short) + "8474d6f29887a-ao-15", // 13 chars (one extra) + ].join("\n") + "\n"; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("only matches valid 12-char lowercase hex prefix", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "abcdef012345-ao-15\n"; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBe("abcdef012345-ao-15"); + }); + + it("matches the correct session among many", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return [ + "aabbccddeef0-ao-1", + "112233445566-ao-15", + "ffeeddccbbaa-ao-2", + "a0b1c2d3e4f5-ao-orchestrator", + ].join("\n") + "\n"; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBe("112233445566-ao-15"); + }); + + it("matches ao-1 to hash-ao-1 (not hash-ao-15)", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return [ + "aabbccddeef0-ao-1", + "112233445566-ao-15", + "ffeeddccbbaa-ao-2", + ].join("\n") + "\n"; + }); + + expect(resolveTmuxSession("ao-1", TMUX, mockExec)).toBe("aabbccddeef0-ao-1"); + }); + + it("matches session with multiple hyphens in name", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-my-long-session-name\n112233445566-other-session\n"; + }); + + expect(resolveTmuxSession("my-long-session-name", TMUX, mockExec)) + .toBe("aabbccddeef0-my-long-session-name"); + }); + + it("matches session with underscores", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-my_session\n112233445566-other_session\n"; + }); + + expect(resolveTmuxSession("my_session", TMUX, mockExec)).toBe("aabbccddeef0-my_session"); + }); + + it("passes list-sessions format flag correctly", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "some-session\n"; + }); + + resolveTmuxSession("ao-99", TMUX, mockExec); + + expect(mockExec).toHaveBeenNthCalledWith(2, + TMUX, + ["list-sessions", "-F", "#{session_name}"], + { timeout: 5000, encoding: "utf8" }, + ); + }); + + it("does NOT match if session name contains the ID but not after hash prefix", () => { + // e.g., "ao-15-extended" has no valid hash prefix + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "ao-15-extended\nao-15-backup\n"; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("does NOT match hash-prefixed session with extra suffix", () => { + // "aabbccddeef0-ao-15-backup" has valid hash but substring(13) is "ao-15-backup" not "ao-15" + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-ao-15-backup\n"; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("returns first match when multiple hash-prefixed sessions exist for same ID", () => { + // This shouldn't happen in practice, but test the behavior + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-ao-15\n112233445566-ao-15\n"; + }); + + // find() returns the first match + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBe("aabbccddeef0-ao-15"); + }); + }); + + // --------------------------------------------------------------------------- + // Not found scenarios + // --------------------------------------------------------------------------- + + describe("not found", () => { + it("returns null when no session matches", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "some-other-session\nanother-session\n"; + }); + + expect(resolveTmuxSession("ao-99", TMUX, mockExec)).toBeNull(); + }); + + it("returns null when tmux is not running", () => { + const mockExec = vi.fn().mockImplementation(() => { + throw new Error("no server running on /tmp/tmux-501/default"); + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("returns null when list-sessions returns empty string", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return ""; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("returns null when list-sessions returns only newlines", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "\n\n\n"; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("returns null when list-sessions throws (no sessions exist)", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); // has-session fails + }) + .mockImplementationOnce(() => { + throw new Error("no sessions"); // list-sessions fails + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("returns null when has-session times out and list-sessions is empty", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw Object.assign(new Error("ETIMEDOUT"), { code: "ETIMEDOUT" }); + }) + .mockImplementationOnce(() => { + return "\n"; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + }); + + // --------------------------------------------------------------------------- + // Edge cases + // --------------------------------------------------------------------------- + + describe("edge cases", () => { + it("handles session name that looks like a hash (all hex chars)", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-abcdef123456\n"; + }); + + expect(resolveTmuxSession("abcdef123456", TMUX, mockExec)).toBe("aabbccddeef0-abcdef123456"); + }); + + it("handles single-char session ID", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-a\n112233445566-b\n"; + }); + + expect(resolveTmuxSession("a", TMUX, mockExec)).toBe("aabbccddeef0-a"); + }); + + it("does not match session without valid hash prefix", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "xao-15\nnotahash-ao-15\n"; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("handles Windows-style line endings in list-sessions output", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-ao-15\r\n112233445566-ao-16\r\n"; + }); + + // \r will remain in the session name after split("\n") + // substring(13) of "aabbccddeef0-ao-15\r" is "ao-15\r" which !== "ao-15" + // This documents the current behavior — tmux shouldn't produce \r\n on unix + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("handles very long session list", () => { + // Generate 100 sessions with valid 12-char hex prefixes + const sessions = Array.from({ length: 100 }, (_, i) => { + const hex = i.toString(16).padStart(12, "0"); + return `${hex}-session-${i}`; + }).join("\n") + "\n"; + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => sessions); + + const hex50 = (50).toString(16).padStart(12, "0"); + expect(resolveTmuxSession("session-50", TMUX, mockExec)).toBe(`${hex50}-session-50`); + }); + + it("handles session list where target is last entry", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-ao-1\n112233445566-ao-2\nffeeddccbbaa-ao-3\na0b1c2d3e4f5-ao-target\n"; + }); + + expect(resolveTmuxSession("ao-target", TMUX, mockExec)).toBe("a0b1c2d3e4f5-ao-target"); + }); + + it("handles session list where target is first entry", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-ao-target\n112233445566-ao-2\nffeeddccbbaa-ao-3\n"; + }); + + expect(resolveTmuxSession("ao-target", TMUX, mockExec)).toBe("aabbccddeef0-ao-target"); + }); + + it("works with different tmux paths", () => { + const paths = ["/opt/homebrew/bin/tmux", "/usr/local/bin/tmux", "/usr/bin/tmux", "tmux"]; + + for (const tmuxPath of paths) { + const mockExec = vi.fn().mockReturnValue(""); + resolveTmuxSession("ao-15", tmuxPath, mockExec); + expect(mockExec).toHaveBeenCalledWith(tmuxPath, ["has-session", "-t", "=ao-15"], { timeout: 5000 }); + } + }); + }); +}); diff --git a/packages/web/server/direct-terminal-ws.ts b/packages/web/server/direct-terminal-ws.ts index 1f0f00595..05badae16 100644 --- a/packages/web/server/direct-terminal-ws.ts +++ b/packages/web/server/direct-terminal-ws.ts @@ -7,29 +7,12 @@ * that tmux requires for clipboard support. */ -import { createServer } from "node:http"; +import { createServer, type Server } from "node:http"; import { spawn } from "node:child_process"; import { WebSocketServer, WebSocket } from "ws"; import { spawn as ptySpawn, type IPty } from "node-pty"; -import { existsSync } from "node:fs"; -import { join } from "node:path"; import { homedir, userInfo } from "node:os"; -import { loadConfig } from "@composio/ao-core"; - -// Load config with fallback -let config: ReturnType; -try { - config = loadConfig(); -} catch (err) { - console.warn( - "[DirectTerminal] Could not load config, using defaults:", - err instanceof Error ? err.message : String(err), - ); - // Fallback to default config - config = { - dataDir: join(homedir(), ".ao/sessions"), - } as ReturnType; -} +import { findTmux, resolveTmuxSession, validateSessionId } from "./tmux-utils.js"; interface TerminalSession { sessionId: string; @@ -37,190 +20,212 @@ interface TerminalSession { ws: WebSocket; } -const activeSessions = new Map(); - -/** - * Create HTTP server with WebSocket upgrade handling - */ -const server = createServer((req, res) => { - // Health check endpoint - if (req.url === "/health") { - res.writeHead(200, { "Content-Type": "application/json" }); - res.end( - JSON.stringify({ - active: activeSessions.size, - sessions: Array.from(activeSessions.keys()), - }), - ); - return; - } - - res.writeHead(404); - res.end("Not found"); -}); - -/** - * WebSocket server for terminal connections - */ -const wss = new WebSocketServer({ - server, - path: "/ws", -}); - -wss.on("connection", (ws, req) => { - const url = new URL(req.url ?? "/", "ws://localhost"); - const sessionId = url.searchParams.get("session"); - - if (!sessionId) { - console.error("[DirectTerminal] Missing session parameter"); - ws.close(1008, "Missing session parameter"); - return; - } - - // Validate session ID - if (!/^[a-zA-Z0-9_-]+$/.test(sessionId)) { - console.error("[DirectTerminal] Invalid session ID:", sessionId); - ws.close(1008, "Invalid session ID"); - return; - } - - // Validate session exists - const sessionPath = join(config.dataDir, sessionId); - if (!existsSync(sessionPath)) { - console.error("[DirectTerminal] Session not found:", sessionId); - ws.close(1008, "Session not found"); - return; - } - - console.log(`[DirectTerminal] New connection for session: ${sessionId}`); - - // Enable mouse mode for scrollback support - const mouseProc = spawn("tmux", ["set-option", "-t", sessionId, "mouse", "on"]); - mouseProc.on("error", (err) => { - console.error(`[DirectTerminal] Failed to set mouse mode for ${sessionId}:`, err.message); - }); - - // Hide the green status bar for cleaner appearance - const statusProc = spawn("tmux", ["set-option", "-t", sessionId, "status", "off"]); - statusProc.on("error", (err) => { - console.error(`[DirectTerminal] Failed to hide status bar for ${sessionId}:`, err.message); - }); - - // Spawn PTY attached to tmux session - // Use tmux from PATH for cross-platform compatibility - const tmuxPath = "tmux"; - - // Build complete environment - node-pty requires proper env setup - const homeDir = process.env.HOME || homedir(); - const currentUser = process.env.USER || userInfo().username; - const env = { - HOME: homeDir, - SHELL: process.env.SHELL || "/bin/bash", - USER: currentUser, - PATH: process.env.PATH || "/usr/local/bin:/usr/bin:/bin", - TERM: "xterm-256color", - LANG: process.env.LANG || "en_US.UTF-8", - TMPDIR: process.env.TMPDIR || "/tmp", - }; - - let pty: IPty; - try { - console.log(`[DirectTerminal] Spawning PTY: ${tmuxPath} attach-session -t ${sessionId}`); - console.log(`[DirectTerminal] CWD: ${homeDir}, ENV keys:`, Object.keys(env).join(", ")); - - pty = ptySpawn(tmuxPath, ["attach-session", "-t", sessionId], { - name: "xterm-256color", - cols: 80, - rows: 24, - cwd: homeDir, - env, - }); - - console.log(`[DirectTerminal] PTY spawned successfully`); - } catch (err) { - console.error(`[DirectTerminal] Failed to spawn PTY:`, err); - ws.close(1011, `Failed to spawn terminal: ${err instanceof Error ? err.message : String(err)}`); - return; - } - - const session: TerminalSession = { sessionId, pty, ws }; - activeSessions.set(sessionId, session); - - // PTY -> WebSocket - pty.onData((data) => { - if (ws.readyState === WebSocket.OPEN) { - ws.send(data); - } - }); - - // PTY exit - pty.onExit(({ exitCode }) => { - console.log(`[DirectTerminal] PTY exited for ${sessionId} with code ${exitCode}`); - activeSessions.delete(sessionId); - if (ws.readyState === WebSocket.OPEN) { - ws.close(1000, "Terminal session ended"); - } - }); - - // WebSocket -> PTY - ws.on("message", (data) => { - const message = data.toString("utf8"); - - // Handle resize messages (sent by xterm.js FitAddon) - if (message.startsWith("{")) { - try { - const parsed = JSON.parse(message) as { type?: string; cols?: number; rows?: number }; - if (parsed.type === "resize" && parsed.cols && parsed.rows) { - pty.resize(parsed.cols, parsed.rows); - return; - } - } catch { - // Not JSON, treat as terminal input - } - } - - // Normal terminal input - pty.write(message); - }); - - // WebSocket close - ws.on("close", () => { - console.log(`[DirectTerminal] WebSocket closed for ${sessionId}`); - activeSessions.delete(sessionId); - pty.kill(); - }); - - // WebSocket error - ws.on("error", (err) => { - console.error(`[DirectTerminal] WebSocket error for ${sessionId}:`, err.message); - activeSessions.delete(sessionId); - pty.kill(); - }); -}); - -const PORT = parseInt(process.env.DIRECT_TERMINAL_PORT ?? "3003", 10); - -server.listen(PORT, () => { - console.log(`[DirectTerminal] WebSocket server listening on port ${PORT}`); -}); - -// Graceful shutdown -function shutdown(signal: string) { - console.log(`[DirectTerminal] Received ${signal}, shutting down...`); - for (const [, session] of activeSessions) { - session.pty.kill(); - session.ws.close(1001, "Server shutting down"); - } - server.close(() => { - console.log("[DirectTerminal] Server closed"); - process.exit(0); - }); - const forceExitTimer = setTimeout(() => { - console.error("[DirectTerminal] Forced shutdown after timeout"); - process.exit(1); - }, 5000); - forceExitTimer.unref(); +export interface DirectTerminalServer { + server: Server; + wss: WebSocketServer; + activeSessions: Map; + shutdown: () => void; } -process.on("SIGINT", () => shutdown("SIGINT")); -process.on("SIGTERM", () => shutdown("SIGTERM")); +/** + * Create the direct terminal WebSocket server. + * Separated from listen() so tests can control lifecycle. + */ +export function createDirectTerminalServer(tmuxPath?: string): DirectTerminalServer { + const TMUX = tmuxPath ?? findTmux(); + const activeSessions = new Map(); + + const server = createServer((req, res) => { + if (req.url === "/health") { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end( + JSON.stringify({ + active: activeSessions.size, + sessions: Array.from(activeSessions.keys()), + }), + ); + return; + } + + res.writeHead(404); + res.end("Not found"); + }); + + const wss = new WebSocketServer({ + server, + path: "/ws", + }); + + wss.on("connection", (ws, req) => { + const url = new URL(req.url ?? "/", "ws://localhost"); + const sessionId = url.searchParams.get("session"); + + if (!sessionId) { + console.error("[DirectTerminal] Missing session parameter"); + ws.close(1008, "Missing session parameter"); + return; + } + + // Validate session ID format + if (!validateSessionId(sessionId)) { + console.error("[DirectTerminal] Invalid session ID:", sessionId); + ws.close(1008, "Invalid session ID"); + return; + } + + // Resolve tmux session name: try exact match first, then suffix match + // (hash-prefixed sessions like "8474d6f29887-ao-15" are accessed by user-facing ID "ao-15") + const tmuxSessionId = resolveTmuxSession(sessionId, TMUX); + if (!tmuxSessionId) { + console.error("[DirectTerminal] tmux session not found:", sessionId); + ws.close(1008, "Session not found"); + return; + } + + console.log(`[DirectTerminal] New connection for session: ${tmuxSessionId}`); + + // Enable mouse mode for scrollback support + const mouseProc = spawn(TMUX, ["set-option", "-t", tmuxSessionId, "mouse", "on"]); + mouseProc.on("error", (err) => { + console.error( + `[DirectTerminal] Failed to set mouse mode for ${tmuxSessionId}:`, + err.message, + ); + }); + + // Hide the green status bar for cleaner appearance + const statusProc = spawn(TMUX, ["set-option", "-t", tmuxSessionId, "status", "off"]); + statusProc.on("error", (err) => { + console.error( + `[DirectTerminal] Failed to hide status bar for ${tmuxSessionId}:`, + err.message, + ); + }); + + // Build complete environment - node-pty requires proper env setup + const homeDir = process.env.HOME || homedir(); + const currentUser = process.env.USER || userInfo().username; + const env = { + HOME: homeDir, + SHELL: process.env.SHELL || "/bin/bash", + USER: currentUser, + PATH: process.env.PATH || "/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin", + TERM: "xterm-256color", + LANG: process.env.LANG || "en_US.UTF-8", + TMPDIR: process.env.TMPDIR || "/tmp", + }; + + let pty: IPty; + try { + console.log(`[DirectTerminal] Spawning PTY: tmux attach-session -t ${tmuxSessionId}`); + + pty = ptySpawn(TMUX, ["attach-session", "-t", tmuxSessionId], { + name: "xterm-256color", + cols: 80, + rows: 24, + cwd: homeDir, + env, + }); + + console.log(`[DirectTerminal] PTY spawned successfully`); + } catch (err) { + console.error(`[DirectTerminal] Failed to spawn PTY:`, err); + ws.close(1011, `Failed to spawn terminal: ${err instanceof Error ? err.message : String(err)}`); + return; + } + + const session: TerminalSession = { sessionId, pty, ws }; + activeSessions.set(sessionId, session); + + // PTY -> WebSocket + pty.onData((data) => { + if (ws.readyState === WebSocket.OPEN) { + ws.send(data); + } + }); + + // PTY exit + pty.onExit(({ exitCode }) => { + console.log(`[DirectTerminal] PTY exited for ${sessionId} with code ${exitCode}`); + activeSessions.delete(sessionId); + if (ws.readyState === WebSocket.OPEN) { + ws.close(1000, "Terminal session ended"); + } + }); + + // WebSocket -> PTY + ws.on("message", (data) => { + const message = data.toString("utf8"); + + // Handle resize messages (sent by xterm.js FitAddon) + if (message.startsWith("{")) { + try { + const parsed = JSON.parse(message) as { type?: string; cols?: number; rows?: number }; + if (parsed.type === "resize" && parsed.cols && parsed.rows) { + pty.resize(parsed.cols, parsed.rows); + return; + } + } catch { + // Not JSON, treat as terminal input + } + } + + // Normal terminal input + pty.write(message); + }); + + // WebSocket close + ws.on("close", () => { + console.log(`[DirectTerminal] WebSocket closed for ${sessionId}`); + activeSessions.delete(sessionId); + pty.kill(); + }); + + // WebSocket error + ws.on("error", (err) => { + console.error(`[DirectTerminal] WebSocket error for ${sessionId}:`, err.message); + activeSessions.delete(sessionId); + pty.kill(); + }); + }); + + function shutdown() { + for (const [, session] of activeSessions) { + session.pty.kill(); + session.ws.close(1001, "Server shutting down"); + } + server.close(); + } + + return { server, wss, activeSessions, shutdown }; +} + +// --- Run as standalone script --- +// Only start the server when executed directly (not imported by tests) +const isMainModule = process.argv[1]?.endsWith("direct-terminal-ws.ts") || + process.argv[1]?.endsWith("direct-terminal-ws.js"); + +if (isMainModule) { + const TMUX = findTmux(); + console.log(`[DirectTerminal] Using tmux: ${TMUX}`); + + const { server, shutdown } = createDirectTerminalServer(TMUX); + const PORT = parseInt(process.env.DIRECT_TERMINAL_PORT ?? "3003", 10); + + server.listen(PORT, () => { + console.log(`[DirectTerminal] WebSocket server listening on port ${PORT}`); + }); + + function handleShutdown(signal: string) { + console.log(`[DirectTerminal] Received ${signal}, shutting down...`); + shutdown(); + const forceExitTimer = setTimeout(() => { + console.error("[DirectTerminal] Forced shutdown after timeout"); + process.exit(1); + }, 5000); + forceExitTimer.unref(); + } + + process.on("SIGINT", () => handleShutdown("SIGINT")); + process.on("SIGTERM", () => handleShutdown("SIGTERM")); +} diff --git a/packages/web/server/terminal-websocket.ts b/packages/web/server/terminal-websocket.ts index 05369644e..b8840b7a9 100644 --- a/packages/web/server/terminal-websocket.ts +++ b/packages/web/server/terminal-websocket.ts @@ -15,9 +15,11 @@ import { spawn, type ChildProcess } from "node:child_process"; import { createServer, request } from "node:http"; -import { existsSync } from "node:fs"; -import { join } from "node:path"; -import { loadConfig } from "@composio/ao-core"; +import { findTmux, resolveTmuxSession, validateSessionId } from "./tmux-utils.js"; + +/** Cached full path to tmux binary */ +const TMUX = findTmux(); +console.log(`[Terminal] Using tmux: ${TMUX}`); interface TtydInstance { sessionId: string; @@ -30,8 +32,6 @@ const availablePorts = new Set(); // Pool of recycled ports let nextPort = 7800; // Start ttyd instances from port 7800 const MAX_PORT = 7900; // Prevent unbounded port allocation -// Load config once at startup -const config = loadConfig(); /** * Check if ttyd is ready to accept connections by making a test request. @@ -105,7 +105,13 @@ function waitForTtyd(port: number, sessionId: string, timeoutMs = 3000): Promise }); } -function getOrSpawnTtyd(sessionId: string): TtydInstance { +/** + * Spawn or reuse a ttyd instance for a tmux session. + * + * @param sessionId - User-facing session ID (used for base-path and URL) + * @param tmuxSessionName - Actual tmux session name (may be hash-prefixed) + */ +function getOrSpawnTtyd(sessionId: string, tmuxSessionName: string): TtydInstance { const existing = instances.get(sessionId); if (existing) return existing; @@ -123,20 +129,22 @@ function getOrSpawnTtyd(sessionId: string): TtydInstance { port = nextPort++; } - console.log(`[Terminal] Spawning ttyd for ${sessionId} on port ${port}`); + console.log(`[Terminal] Spawning ttyd for ${tmuxSessionName} on port ${port}`); // Enable mouse mode for scrollback support - const mouseProc = spawn("tmux", ["set-option", "-t", sessionId, "mouse", "on"]); + const mouseProc = spawn(TMUX, ["set-option", "-t", tmuxSessionName, "mouse", "on"]); mouseProc.on("error", (err) => { - console.error(`[Terminal] Failed to set mouse mode for ${sessionId}:`, err.message); + console.error(`[Terminal] Failed to set mouse mode for ${tmuxSessionName}:`, err.message); }); // Hide the green status bar for cleaner appearance - const statusProc = spawn("tmux", ["set-option", "-t", sessionId, "status", "off"]); + const statusProc = spawn(TMUX, ["set-option", "-t", tmuxSessionName, "status", "off"]); statusProc.on("error", (err) => { - console.error(`[Terminal] Failed to hide status bar for ${sessionId}:`, err.message); + console.error(`[Terminal] Failed to hide status bar for ${tmuxSessionName}:`, err.message); }); + // Use user-facing sessionId for base-path (matches URL the dashboard uses) + // Use tmuxSessionName for tmux attach (may be hash-prefixed) const proc = spawn( "ttyd", [ @@ -145,10 +153,10 @@ function getOrSpawnTtyd(sessionId: string): TtydInstance { String(port), "--base-path", `/${sessionId}`, - "tmux", + TMUX, "attach-session", "-t", - sessionId, + tmuxSessionName, ], { stdio: ["ignore", "pipe", "pipe"], @@ -241,15 +249,16 @@ const server = createServer(async (req, res) => { } // Validate session ID to prevent path traversal and injection - if (!/^[a-zA-Z0-9_-]+$/.test(sessionId)) { + if (!validateSessionId(sessionId)) { res.writeHead(400, { "Content-Type": "application/json" }); res.end(JSON.stringify({ error: "Invalid session ID" })); return; } - // Validate session exists before spawning ttyd using configured dataDir - const sessionPath = join(config.dataDir, sessionId); - if (!existsSync(sessionPath)) { + // Resolve tmux session name: try exact match first, then suffix match + // (hash-prefixed sessions like "8474d6f29887-ao-15" are accessed by user-facing ID "ao-15") + const tmuxSessionId = resolveTmuxSession(sessionId, TMUX); + if (!tmuxSessionId) { res.writeHead(404, { "Content-Type": "application/json" }); res.end(JSON.stringify({ error: "Session not found" })); return; @@ -257,7 +266,7 @@ const server = createServer(async (req, res) => { // Spawn ttyd and wait for it to be ready (catch port exhaustion and startup failures) try { - const instance = getOrSpawnTtyd(sessionId); + const instance = getOrSpawnTtyd(sessionId, tmuxSessionId); await waitForTtyd(instance.port, sessionId); // Use the request host to construct the terminal URL (supports remote access) diff --git a/packages/web/server/tmux-utils.ts b/packages/web/server/tmux-utils.ts new file mode 100644 index 000000000..6a476fbe1 --- /dev/null +++ b/packages/web/server/tmux-utils.ts @@ -0,0 +1,102 @@ +/** + * Shared tmux utilities for terminal servers. + * + * Extracted from direct-terminal-ws.ts and terminal-websocket.ts + * so the logic can be properly unit tested. + */ + +import { execFileSync } from "node:child_process"; + +/** Session ID validation regex — alphanumeric, hyphens, underscores only */ +export const SESSION_ID_PATTERN = /^[a-zA-Z0-9_-]+$/; + +/** Hash prefix pattern — 12-char lowercase hex, as generated by generateConfigHash */ +const HASH_PREFIX_PATTERN = /^[a-f0-9]{12}-/; + +/** + * Validate a session ID format. + * Prevents path traversal, shell injection, and other attacks. + */ +export function validateSessionId(sessionId: string): boolean { + return SESSION_ID_PATTERN.test(sessionId); +} + +/** + * Find full path to tmux binary. + * + * Checks common installation locations because node-pty's posix_spawnp + * doesn't reliably inherit PATH, and some Node.js environments (e.g., + * launched from GUI apps) have minimal PATH. + * + * @param execFn - Injectable execFileSync for testing. Defaults to child_process.execFileSync. + */ +export function findTmux( + execFn: typeof execFileSync = execFileSync, +): string { + const candidates = [ + "/opt/homebrew/bin/tmux", // macOS ARM (Homebrew) + "/usr/local/bin/tmux", // macOS Intel (Homebrew) + "/usr/bin/tmux", // Linux + ]; + for (const p of candidates) { + try { + execFn(p, ["-V"], { timeout: 5000 }); + return p; + } catch { + continue; + } + } + return "tmux"; // Fall back to bare name +} + +/** + * Resolve a user-facing session ID to its actual tmux session name. + * + * The hash-based architecture prefixes tmux session names with a config hash + * (e.g., "8474d6f29887-ao-15" for user-facing "ao-15"). This function: + * + * 1. Tries exact match first using tmux's `=` prefix syntax to prevent + * prefix matching (where "ao-1" would incorrectly match "ao-15"). + * 2. Falls back to listing all sessions and finding one with a 12-char hex + * prefix followed by the exact session ID (e.g., `{12-hex}-{sessionId}`). + * + * @param sessionId - User-facing session ID (e.g., "ao-15") + * @param tmuxPath - Full path to tmux binary + * @param execFn - Injectable execFileSync for testing. Defaults to child_process.execFileSync. + * @returns The actual tmux session name, or null if not found + */ +export function resolveTmuxSession( + sessionId: string, + tmuxPath: string, + execFn: typeof execFileSync = execFileSync, +): string | null { + // Try exact match first using = prefix for exact matching (e.g., "ao-orchestrator") + // Without =, tmux uses prefix matching: "ao-1" would match "ao-15" + try { + execFn(tmuxPath, ["has-session", "-t", `=${sessionId}`], { timeout: 5000 }); + return sessionId; + } catch { + // Not an exact match + } + + // Search for hash-prefixed tmux session (e.g., "8474d6f29887-ao-15" for "ao-15") + // Validate the 12-char hex prefix to avoid ambiguous suffix matches where + // "hash-my-app-1" could falsely match a lookup for "app-1". + try { + const output = execFn(tmuxPath, ["list-sessions", "-F", "#{session_name}"], { + timeout: 5000, + encoding: "utf8", + }) as string; + const sessions = output.split("\n").filter(Boolean); + const match = sessions.find((s) => + HASH_PREFIX_PATTERN.test(s) && s.substring(13) === sessionId, + ); + if (match) { + return match; + } + } catch { + // tmux not running or no sessions + } + + return null; +} diff --git a/packages/web/tsconfig.json b/packages/web/tsconfig.json index 58736ffd0..536398df7 100644 --- a/packages/web/tsconfig.json +++ b/packages/web/tsconfig.json @@ -22,6 +22,6 @@ "declarationMap": false, "verbatimModuleSyntax": false }, - "include": ["next-env.d.ts", "src", ".next/types/**/*.ts"], + "include": ["next-env.d.ts", "src", "server", ".next/types/**/*.ts"], "exclude": ["node_modules", "src/**/*.test.tsx", "src/**/__tests__"] } diff --git a/packages/web/vitest.config.ts b/packages/web/vitest.config.ts index 03c46261e..f3cbb2328 100644 --- a/packages/web/vitest.config.ts +++ b/packages/web/vitest.config.ts @@ -8,7 +8,7 @@ export default defineConfig({ globals: true, environment: "jsdom", setupFiles: ["./src/__tests__/setup.ts"], - include: ["src/**/*.test.{ts,tsx}"], + include: ["src/**/*.test.{ts,tsx}", "server/**/*.test.ts"], }, resolve: { alias: { From 73957182f7816cb85b97544bdc666494bc2e2e04 Mon Sep 17 00:00:00 2001 From: prateek Date: Wed, 18 Feb 2026 03:48:19 +0530 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20activity=20detection=20=E2=80=94=20f?= =?UTF-8?q?ix=20path=20encoding=20bug,=20add=20ready=20state=20(#71)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: activity detection — fix path encoding, use tail -1 for JSONL Two tightly coupled infrastructure fixes: - Fix toClaudeProjectPath(): leading `/` becomes `-` (not stripped), matching Claude Code's actual project directory naming convention. - Replace manual 4KB buffer read in readLastJsonlEntry() with `tail -1` + JSON.parse — handles any file size, any line length, and eliminates the truncated-line edge case entirely. Co-Authored-By: Claude Opus 4.6 * feat: add "ready" state, return null when unknown, remove dead code Behavioral changes to activity detection: - Add "ready" to ActivityState — separates "alive at prompt" from "idle/stale". Configurable via readyThresholdMs (default 5 min). - Agent plugins return null when they can't determine activity (no workspace, no JSONL, no per-session tracking). Session manager preserves existing activity instead of overwriting with a guess. - Remove isProcessing() from Agent interface — zero callers in production code, fully superseded by getActivityState(). - Remove extractLastMessageType() from claude-code — the field it populated (lastMessageType) was only consumed by the old inline CLI mapping, which is now replaced by plugin delegation. - CLI status delegates to agent.getActivityState() (single source of truth) with metadata fallback when plugin returns null. Co-Authored-By: Claude Opus 4.6 * test: comprehensive activity detection coverage - activity-detection.test.ts: 42+ tests covering path encoding, getActivityState edge cases (exited/null/fallback), real Claude Code JSONL types, agent interface spec types, staleness thresholds, JSONL file selection, and realistic session sequences. - status.test.ts: plugin delegation tests — verifies CLI uses agent.getActivityState() as single source of truth, passes readyThresholdMs from config, falls back to metadata on null/throw. - Integration tests: updated type expectations for null returns from codex, opencode, and aider; added "ready" to valid state lists. Co-Authored-By: Claude Opus 4.6 * fix: flaky Linear integration test + missing ready label in SessionDetail Linear API has eventual consistency — updateIssue state changes don't propagate instantly. Poll with retries instead of asserting immediately. Also adds "ready" entry to SessionDetail activityLabel map (was missing, causing fallback to dim/unstyled rendering). Co-Authored-By: Claude Opus 4.6 * fix: pure Node.js readLastJsonlEntry, use pollUntilEqual for Linear test Replace `tail -1` with pure Node.js implementation that reads backwards from end of file in 4KB chunks. No external binary dependency — works on any platform. Fix flaky Linear integration test by using the existing pollUntilEqual helper instead of an inline retry loop. Linear API has eventual consistency; pollUntilEqual retries for up to 5s with 500ms intervals. Co-Authored-By: Claude Opus 4.6 * fix: use tail -1 for readLastJsonlEntry, add real-data integration test Replace over-engineered pure Node.js backward-reading implementation with simple `tail -1` via execFile. The codebase already shells out to tmux, git, and ps everywhere — tail is no different. Add integration test that validates toClaudeProjectPath() and readLastJsonlEntry() against real ~/.claude/projects/ data on disk. No API key needed — just requires Claude to have been run once. Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- .../cli/__tests__/commands/status.test.ts | 101 ++++- packages/cli/src/commands/status.ts | 40 +- packages/cli/src/lib/format.ts | 4 +- packages/core/README.md | 2 +- .../src/__tests__/lifecycle-manager.test.ts | 2 +- .../src/__tests__/plugin-integration.test.ts | 1 - .../src/__tests__/session-manager.test.ts | 38 +- packages/core/src/config.ts | 1 + packages/core/src/session-manager.ts | 15 +- packages/core/src/types.ts | 20 +- packages/core/src/utils.ts | 61 +-- .../src/agent-aider.integration.test.ts | 11 +- .../src/agent-claude-code.integration.test.ts | 145 ++++++- .../src/agent-codex.integration.test.ts | 12 +- .../src/agent-opencode.integration.test.ts | 12 +- ...li-spawn-core-read-new.integration.test.ts | 2 + .../src/tracker-linear.integration.test.ts | 15 +- .../plugins/agent-aider/src/index.test.ts | 23 -- packages/plugins/agent-aider/src/index.ts | 29 +- .../src/__tests__/activity-detection.test.ts | 375 +++++++++++++++++- .../agent-claude-code/src/index.test.ts | 166 +------- .../plugins/agent-claude-code/src/index.ts | 86 ++-- .../plugins/agent-codex/src/index.test.ts | 23 -- packages/plugins/agent-codex/src/index.ts | 16 +- .../plugins/agent-opencode/src/index.test.ts | 23 -- packages/plugins/agent-opencode/src/index.ts | 14 +- .../app/api/sessions/[id]/restore/route.ts | 3 +- packages/web/src/components/SessionCard.tsx | 3 +- packages/web/src/components/SessionDetail.tsx | 5 +- packages/web/src/lib/types.ts | 6 +- 30 files changed, 788 insertions(+), 466 deletions(-) diff --git a/packages/cli/__tests__/commands/status.test.ts b/packages/cli/__tests__/commands/status.test.ts index 1dd2fe21c..e69b28978 100644 --- a/packages/cli/__tests__/commands/status.test.ts +++ b/packages/cli/__tests__/commands/status.test.ts @@ -8,6 +8,7 @@ const { mockGit, mockConfigRef, mockIntrospect, + mockGetActivityState, mockDetectPR, mockGetCISummary, mockGetReviewDecision, @@ -17,6 +18,7 @@ const { mockGit: vi.fn(), mockConfigRef: { current: null as Record | null }, mockIntrospect: vi.fn(), + mockGetActivityState: vi.fn(), mockDetectPR: vi.fn(), mockGetCISummary: vi.fn(), mockGetReviewDecision: vi.fn(), @@ -57,12 +59,14 @@ vi.mock("../../src/lib/plugins.js", () => ({ processName: "claude", detectActivity: () => "idle", getSessionInfo: mockIntrospect, + getActivityState: mockGetActivityState, }), getAgentByName: () => ({ name: "claude-code", processName: "claude", detectActivity: () => "idle", getSessionInfo: mockIntrospect, + getActivityState: mockGetActivityState, }), getSCM: () => ({ name: "github", @@ -105,6 +109,7 @@ beforeEach(() => { mockConfigRef.current = { configPath, port: 3000, + readyThresholdMs: 300_000, defaults: { runtime: "tmux", agent: "claude-code", @@ -142,6 +147,8 @@ beforeEach(() => { mockGit.mockReset(); mockIntrospect.mockReset(); mockIntrospect.mockResolvedValue(null); + mockGetActivityState.mockReset(); + mockGetActivityState.mockResolvedValue("active"); mockDetectPR.mockReset(); mockDetectPR.mockResolvedValue(null); mockGetCISummary.mockReset(); @@ -488,10 +495,10 @@ describe("status command", () => { expect(parsed[0].pendingThreads).toBeNull(); }); - it("falls back to metadata status for activity when introspection unavailable", async () => { + it("uses agent.getActivityState() for activity detection (plugin is single source of truth)", async () => { writeFileSync( join(sessionsDir, "app-1"), - "worktree=/tmp/wt\nbranch=feat/meta-act\nstatus=working\n", + "worktree=/tmp/wt\nbranch=feat/act\nstatus=working\n", ); mockTmux.mockImplementation(async (...args: string[]) => { @@ -499,23 +506,23 @@ describe("status command", () => { if (args[0] === "display-message") return String(Math.floor(Date.now() / 1000)); return null; }); - mockGit.mockResolvedValue("feat/meta-act"); + mockGit.mockResolvedValue("feat/act"); - // Introspection returns null (no session info) - mockIntrospect.mockResolvedValue(null); + // Plugin returns "ready" — CLI should use it directly, not recompute + mockGetActivityState.mockResolvedValue("ready"); await program.parseAsync(["node", "test", "status", "--json"]); const jsonCalls = consoleSpy.mock.calls.map((c) => c[0]).join(""); const parsed = JSON.parse(jsonCalls); - // status=working should fall back to activity=active - expect(parsed[0].activity).toBe("active"); + expect(parsed[0].activity).toBe("ready"); + expect(mockGetActivityState).toHaveBeenCalled(); }); - it("treats assistant lastMessageType as idle, not active", async () => { + it("passes readyThresholdMs from config to agent.getActivityState()", async () => { writeFileSync( join(sessionsDir, "app-1"), - "worktree=/tmp/wt\nbranch=feat/asst\nstatus=working\n", + "worktree=/tmp/wt\nbranch=feat/thr\nstatus=working\n", ); mockTmux.mockImplementation(async (...args: string[]) => { @@ -523,18 +530,82 @@ describe("status command", () => { if (args[0] === "display-message") return String(Math.floor(Date.now() / 1000)); return null; }); - mockGit.mockResolvedValue("feat/asst"); + mockGit.mockResolvedValue("feat/thr"); + mockGetActivityState.mockResolvedValue("idle"); - mockIntrospect.mockResolvedValue({ - summary: "Working on feature", - agentSessionId: "abc", - lastMessageType: "assistant", + await program.parseAsync(["node", "test", "status", "--json"]); + + // Verify the config threshold (300_000) was passed through + expect(mockGetActivityState).toHaveBeenCalledWith( + expect.objectContaining({ id: "app-1" }), + 300_000, + ); + }); + + it("shows null activity when getActivityState() throws", async () => { + writeFileSync( + join(sessionsDir, "app-1"), + "worktree=/tmp/wt\nbranch=feat/err\nstatus=working\n", + ); + + mockTmux.mockImplementation(async (...args: string[]) => { + if (args[0] === "list-sessions") return "app-1"; + if (args[0] === "display-message") return String(Math.floor(Date.now() / 1000)); + return null; }); + mockGit.mockResolvedValue("feat/err"); + + // Plugin throws — activity stays null (unknown) + mockGetActivityState.mockRejectedValue(new Error("detection failed")); await program.parseAsync(["node", "test", "status", "--json"]); const jsonCalls = consoleSpy.mock.calls.map((c) => c[0]).join(""); const parsed = JSON.parse(jsonCalls); - expect(parsed[0].activity).toBe("idle"); + expect(parsed[0].activity).toBeNull(); + }); + + it("shows null activity when getActivityState() returns null", async () => { + writeFileSync( + join(sessionsDir, "app-1"), + "worktree=/tmp/wt\nbranch=feat/null\nstatus=working\n", + ); + + mockTmux.mockImplementation(async (...args: string[]) => { + if (args[0] === "list-sessions") return "app-1"; + if (args[0] === "display-message") return String(Math.floor(Date.now() / 1000)); + return null; + }); + mockGit.mockResolvedValue("feat/null"); + + // Plugin returns null — activity stays null (unknown) + mockGetActivityState.mockResolvedValue(null); + + await program.parseAsync(["node", "test", "status", "--json"]); + + const jsonCalls = consoleSpy.mock.calls.map((c) => c[0]).join(""); + const parsed = JSON.parse(jsonCalls); + expect(parsed[0].activity).toBeNull(); + }); + + it("shows exited activity from getActivityState()", async () => { + writeFileSync( + join(sessionsDir, "app-1"), + "worktree=/tmp/wt\nbranch=feat/dead\nstatus=working\n", + ); + + mockTmux.mockImplementation(async (...args: string[]) => { + if (args[0] === "list-sessions") return "app-1"; + if (args[0] === "display-message") return null; + return null; + }); + mockGit.mockResolvedValue("feat/dead"); + mockGetActivityState.mockResolvedValue("exited"); + + await program.parseAsync(["node", "test", "status", "--json"]); + + const jsonCalls = consoleSpy.mock.calls.map((c) => c[0]).join(""); + const parsed = JSON.parse(jsonCalls); + expect(parsed[0].activity).toBe("exited"); }); }); diff --git a/packages/cli/src/commands/status.ts b/packages/cli/src/commands/status.ts index 06aefe71f..5d8b93092 100644 --- a/packages/cli/src/commands/status.ts +++ b/packages/cli/src/commands/status.ts @@ -63,7 +63,7 @@ function buildSessionForIntrospect( id: sessionName, projectId: "", status: "working", - activity: "idle", + activity: null, branch: branch ?? null, issueId: null, pr: null, @@ -82,6 +82,7 @@ async function gatherSessionInfo( agent: Agent, scm: SCM, projectConfig: ProjectConfig, + readyThresholdMs?: number, ): Promise { const metaFile = `${sessionDir}/${sessionName}`; const meta = readMetadata(metaFile); @@ -107,31 +108,19 @@ async function gatherSessionInfo( // Get agent's auto-generated summary and activity via introspection let claudeSummary: string | null = null; let activity: ActivityState | null = null; + const session = buildSessionForIntrospect(sessionName, worktree, branch); try { - const session = buildSessionForIntrospect(sessionName, worktree, branch); const introspection = await agent.getSessionInfo(session); claudeSummary = introspection?.summary ?? null; - - // Detect activity from agent's last message type - const msgType = introspection?.lastMessageType; - if (msgType === "summary" || msgType === "assistant" || msgType === "result") { - activity = "idle"; - } else if (msgType === "tool_use" || msgType === "user") { - activity = "active"; - } } catch { - // Introspection failed — not critical + // Summary extraction failed — not critical } - // Fall back to metadata status for activity when introspection is unavailable - if (activity === null && status) { - const statusToActivity: Record = { - working: "active", - idle: "idle", - stuck: "blocked", - errored: "blocked", - }; - activity = statusToActivity[status] ?? null; + // Detect activity via the agent plugin (single source of truth) + try { + activity = await agent.getActivityState(session, readyThresholdMs); + } catch { + // Activity detection failed — stays null (displayed as "unknown") } // Fetch PR, CI, and review data from SCM @@ -309,7 +298,16 @@ export function registerStatus(program: Command): void { // Gather all session info in parallel const infoPromises = projectSessions .sort() - .map((session) => gatherSessionInfo(session, sessionsDir, agent, scm, projectConfig)); + .map((session) => + gatherSessionInfo( + session, + sessionsDir, + agent, + scm, + projectConfig, + config.readyThresholdMs, + ), + ); const sessionInfos = await Promise.all(infoPromises); for (const info of sessionInfos) { diff --git a/packages/cli/src/lib/format.ts b/packages/cli/src/lib/format.ts index 95b28d9fd..b0e7e4160 100644 --- a/packages/cli/src/lib/format.ts +++ b/packages/cli/src/lib/format.ts @@ -89,6 +89,8 @@ export function activityIcon(activity: ActivityState | null): string { switch (activity) { case "active": return chalk.green("working"); + case "ready": + return chalk.cyan("ready"); case "idle": return chalk.yellow("idle"); case "waiting_input": @@ -98,7 +100,7 @@ export function activityIcon(activity: ActivityState | null): string { case "exited": return chalk.dim("exited"); case null: - return chalk.dim("-"); + return chalk.dim("unknown"); } } diff --git a/packages/core/README.md b/packages/core/README.md index 20f939790..cdbf8c058 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -74,7 +74,7 @@ spawning → working → pr_open → ci_failed/review_pending/approved → merge **Polling loop:** -1. For each session: check if agent is processing (`Agent.isProcessing()`) +1. For each session: check agent activity state (`Agent.getActivityState()`) 2. If PR exists: check CI status (`SCM.getCISummary()`), review state (`SCM.getReviewDecision()`) 3. Update session status based on state 4. Trigger reactions if state changed diff --git a/packages/core/src/__tests__/lifecycle-manager.test.ts b/packages/core/src/__tests__/lifecycle-manager.test.ts index f16efc965..2663ab4b2 100644 --- a/packages/core/src/__tests__/lifecycle-manager.test.ts +++ b/packages/core/src/__tests__/lifecycle-manager.test.ts @@ -86,7 +86,6 @@ beforeEach(() => { detectActivity: vi.fn().mockReturnValue("active" as ActivityState), getActivityState: vi.fn().mockResolvedValue("active" as ActivityState), isProcessRunning: vi.fn().mockResolvedValue(true), - isProcessing: vi.fn().mockResolvedValue(false), getSessionInfo: vi.fn().mockResolvedValue(null), }; @@ -138,6 +137,7 @@ beforeEach(() => { info: [], }, reactions: {}, + readyThresholdMs: 300_000, }; // Calculate sessions directory diff --git a/packages/core/src/__tests__/plugin-integration.test.ts b/packages/core/src/__tests__/plugin-integration.test.ts index 5c70c56e6..2e4483a5d 100644 --- a/packages/core/src/__tests__/plugin-integration.test.ts +++ b/packages/core/src/__tests__/plugin-integration.test.ts @@ -143,7 +143,6 @@ beforeEach(() => { getEnvironment: vi.fn().mockReturnValue({ AGENT_VAR: "1" }), detectActivity: vi.fn().mockResolvedValue("active"), isProcessRunning: vi.fn().mockResolvedValue(true), - isProcessing: vi.fn().mockResolvedValue(false), getSessionInfo: vi.fn().mockResolvedValue(null), }; diff --git a/packages/core/src/__tests__/session-manager.test.ts b/packages/core/src/__tests__/session-manager.test.ts index e6baeb470..4089bc7c3 100644 --- a/packages/core/src/__tests__/session-manager.test.ts +++ b/packages/core/src/__tests__/session-manager.test.ts @@ -55,7 +55,6 @@ beforeEach(() => { detectActivity: vi.fn().mockReturnValue("active"), getActivityState: vi.fn().mockResolvedValue("active"), isProcessRunning: vi.fn().mockResolvedValue(true), - isProcessing: vi.fn().mockResolvedValue(false), getSessionInfo: vi.fn().mockResolvedValue(null), }; @@ -112,6 +111,7 @@ beforeEach(() => { info: [], }, reactions: {}, + readyThresholdMs: 300_000, }; // Calculate sessions directory @@ -447,7 +447,7 @@ describe("list", () => { expect(sessions[0].activity).toBe("active"); }); - it("falls back to idle on getActivityState error", async () => { + it("keeps existing activity when getActivityState throws", async () => { const agentWithError: Agent = { ...mockAgent, getActivityState: vi.fn().mockRejectedValue(new Error("detection failed")), @@ -472,8 +472,38 @@ describe("list", () => { const sm = createSessionManager({ config, registry: registryWithError }); const sessions = await sm.list(); - // Should fall back to idle when getActivityState fails - expect(sessions[0].activity).toBe("idle"); + // Should keep null (absent) when getActivityState fails + expect(sessions[0].activity).toBeNull(); + }); + + it("keeps existing activity when getActivityState returns null", async () => { + const agentWithNull: Agent = { + ...mockAgent, + getActivityState: vi.fn().mockResolvedValue(null), + }; + const registryWithNull: PluginRegistry = { + ...mockRegistry, + get: vi.fn().mockImplementation((slot: string) => { + if (slot === "runtime") return mockRuntime; + if (slot === "agent") return agentWithNull; + return null; + }), + }; + + writeMetadata(sessionsDir, "app-1", { + worktree: "/tmp", + branch: "a", + status: "working", + project: "my-app", + runtimeHandle: JSON.stringify(makeHandle("rt-1")), + }); + + const sm = createSessionManager({ config, registry: registryWithNull }); + const sessions = await sm.list(); + + // null = "I don't know" — activity stays null (absent) + expect(agentWithNull.getActivityState).toHaveBeenCalled(); + expect(sessions[0].activity).toBeNull(); }); }); diff --git a/packages/core/src/config.ts b/packages/core/src/config.ts index 05081f09b..728752895 100644 --- a/packages/core/src/config.ts +++ b/packages/core/src/config.ts @@ -90,6 +90,7 @@ const DefaultPluginsSchema = z.object({ const OrchestratorConfigSchema = z.object({ port: z.number().default(3000), + readyThresholdMs: z.number().nonnegative().default(300_000), defaults: DefaultPluginsSchema.default({}), projects: z.record(ProjectConfigSchema), notifiers: z.record(NotifierConfigSchema).default({}), diff --git a/packages/core/src/session-manager.ts b/packages/core/src/session-manager.ts index fb1bb6d7b..701e11cb9 100644 --- a/packages/core/src/session-manager.ts +++ b/packages/core/src/session-manager.ts @@ -110,7 +110,7 @@ function metadataToSession( id: sessionId, projectId: meta["project"] ?? "", status: validateStatus(meta["status"]), - activity: "idle", + activity: null, branch: meta["branch"] || null, issueId: meta["issue"] || null, pr: meta["pr"] @@ -231,10 +231,17 @@ export function createSessionManager(deps: SessionManagerDeps): SessionManager { } else if (plugins.agent) { // Runtime is alive — detect activity using agent-native mechanism try { - session.activity = await plugins.agent.getActivityState(session); + const detected = await plugins.agent.getActivityState( + session, + config.readyThresholdMs, + ); + // Only overwrite if plugin returned a concrete state. + // null means "I don't have enough data" — keep existing value. + if (detected !== null) { + session.activity = detected; + } } catch { - // Can't detect activity — explicitly set to idle - session.activity = "idle"; + // Can't detect activity — keep existing value } } } catch { diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 24ee0dc02..8bf00cd49 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -44,7 +44,8 @@ export type SessionStatus = /** Activity state as detected by the agent plugin */ export type ActivityState = | "active" // agent is processing (thinking, writing code) - | "idle" // agent is at prompt, waiting for input + | "ready" // agent finished its turn, alive and waiting for input + | "idle" // agent has been inactive for a while (stale) | "waiting_input" // agent is asking a question / permission prompt | "blocked" // agent hit an error or is stuck | "exited"; // agent process is no longer running @@ -52,12 +53,16 @@ export type ActivityState = /** Activity state constants */ export const ACTIVITY_STATE = { ACTIVE: "active" as const, + READY: "ready" as const, IDLE: "idle" as const, WAITING_INPUT: "waiting_input" as const, BLOCKED: "blocked" as const, EXITED: "exited" as const, } satisfies Record; +/** Default threshold (ms) before a "ready" session becomes "idle". */ +export const DEFAULT_READY_THRESHOLD_MS = 300_000; // 5 minutes + /** Session status constants */ export const SESSION_STATUS = { SPAWNING: "spawning" as const, @@ -89,8 +94,8 @@ export interface Session { /** Current lifecycle status */ status: SessionStatus; - /** Activity state from agent plugin */ - activity: ActivityState; + /** Activity state from agent plugin (null = not yet determined) */ + activity: ActivityState | null; /** Git branch name */ branch: string | null; @@ -222,15 +227,13 @@ export interface Agent { /** * Get current activity state using agent-native mechanism (JSONL, SQLite, etc.). * This is the preferred method for activity detection. + * @param readyThresholdMs - ms before "ready" becomes "idle" (default: DEFAULT_READY_THRESHOLD_MS) */ - getActivityState(session: Session): Promise; + getActivityState(session: Session, readyThresholdMs?: number): Promise; /** Check if agent process is running (given runtime handle) */ isProcessRunning(handle: RuntimeHandle): Promise; - /** Fast check: is the agent actively processing? Uses agent-native mechanism (not runtime). */ - isProcessing(session: Session): Promise; - /** Extract information from agent's internal data (summary, cost, session ID) */ getSessionInfo(session: Session): Promise; @@ -715,6 +718,9 @@ export interface OrchestratorConfig { /** Web dashboard port */ port: number; + /** Milliseconds before a "ready" session becomes "idle" (default: 300000 = 5 min) */ + readyThresholdMs: number; + /** Default plugin selections */ defaults: DefaultPlugins; diff --git a/packages/core/src/utils.ts b/packages/core/src/utils.ts index 61899f46c..253cde379 100644 --- a/packages/core/src/utils.ts +++ b/packages/core/src/utils.ts @@ -2,7 +2,11 @@ * Shared utility functions for agent-orchestrator plugins. */ -import { open } from "node:fs/promises"; +import { execFile } from "node:child_process"; +import { stat } from "node:fs/promises"; +import { promisify } from "node:util"; + +const execFileAsync = promisify(execFile); /** * POSIX-safe shell escaping: wraps value in single quotes, @@ -33,60 +37,33 @@ export function validateUrl(url: string, label: string): void { } /** - * Read only the last 4KB of JSONL files to find the last entry. - * Avoids loading entire (potentially large) files into memory. - */ -const TAIL_READ_BYTES = 4096; - -/** - * Read the last entry from a JSONL file efficiently. - * Only reads the last TAIL_READ_BYTES (4KB) from the file to avoid loading - * entire potentially large files into memory. + * Read the last entry from a JSONL file. + * Uses `tail -1` to efficiently read the last line, then JSON.parse in Node. * * @param filePath - Path to the JSONL file - * @returns Object containing the last entry's type and file mtime, or null if file is empty/invalid + * @returns Object containing the last entry's type and file mtime, or null if empty/invalid */ export async function readLastJsonlEntry( filePath: string, ): Promise<{ lastType: string | null; modifiedAt: Date } | null> { - let fh; try { - fh = await open(filePath, "r"); - const fileStat = await fh.stat(); - const size = fileStat.size; - if (size === 0) return null; + const [{ stdout }, fileStat] = await Promise.all([ + execFileAsync("tail", ["-1", filePath], { timeout: 5_000 }), + stat(filePath), + ]); - // Read only the last TAIL_READ_BYTES (4KB) from the file - const readSize = Math.min(TAIL_READ_BYTES, size); - const buffer = Buffer.alloc(readSize); - const { bytesRead } = await fh.read(buffer, 0, readSize, size - readSize); + const line = stdout.trim(); + if (!line) return null; - const chunk = buffer.toString("utf-8", 0, bytesRead); - // Walk backwards through lines to find the last valid JSON object with a type - const lines = chunk.split("\n"); - for (let i = lines.length - 1; i >= 0; i--) { - const line = lines[i]; - if (!line) continue; - const trimmed = line.trim(); - if (!trimmed) continue; - try { - const parsed: unknown = JSON.parse(trimmed); - if (typeof parsed === "object" && parsed !== null && !Array.isArray(parsed)) { - const obj = parsed as Record; - if (typeof obj.type === "string") { - return { lastType: obj.type, modifiedAt: fileStat.mtime }; - } - } - } catch { - // Skip malformed lines (possibly truncated first line in our chunk) - } + const parsed: unknown = JSON.parse(line); + if (typeof parsed === "object" && parsed !== null && !Array.isArray(parsed)) { + const obj = parsed as Record; + const lastType = typeof obj.type === "string" ? obj.type : null; + return { lastType, modifiedAt: fileStat.mtime }; } - // No entry with a type field found return { lastType: null, modifiedAt: fileStat.mtime }; } catch { return null; - } finally { - await fh?.close(); } } diff --git a/packages/integration-tests/src/agent-aider.integration.test.ts b/packages/integration-tests/src/agent-aider.integration.test.ts index a20eaf86a..9dc6182a4 100644 --- a/packages/integration-tests/src/agent-aider.integration.test.ts +++ b/packages/integration-tests/src/agent-aider.integration.test.ts @@ -93,11 +93,11 @@ describe.skipIf(!canRun)("agent-aider (integration)", () => { // Observations captured while the agent is alive let aliveRunning = false; - let aliveActivityState: ActivityState | undefined; + let aliveActivityState: ActivityState | null | undefined; // Observations captured after the agent exits let exitedRunning: boolean; - let exitedActivityState: ActivityState; + let exitedActivityState: ActivityState | null; let sessionInfo: AgentSessionInfo | null; beforeAll(async () => { @@ -149,10 +149,13 @@ describe.skipIf(!canRun)("agent-aider (integration)", () => { }); it("getActivityState → returns valid state while agent is running", () => { - // Aider checks git commits and chat history mtime for activity detection + // Aider checks git commits and chat history mtime for activity detection. + // May return null if no chat history exists yet. if (aliveActivityState !== undefined) { expect(aliveActivityState).not.toBe("exited"); - expect(["active", "idle", "waiting_input", "blocked"]).toContain(aliveActivityState); + expect([null, "active", "ready", "idle", "waiting_input", "blocked"]).toContain( + aliveActivityState, + ); } }); diff --git a/packages/integration-tests/src/agent-claude-code.integration.test.ts b/packages/integration-tests/src/agent-claude-code.integration.test.ts index 2855e96ea..3ebecb339 100644 --- a/packages/integration-tests/src/agent-claude-code.integration.test.ts +++ b/packages/integration-tests/src/agent-claude-code.integration.test.ts @@ -1,22 +1,27 @@ /** * Integration tests for the Claude Code agent plugin. * - * Requires: - * - `claude` binary on PATH - * - tmux installed and running - * - ANTHROPIC_API_KEY set (Claude will make a real API call) + * Two test suites: + * + * 1. "path encoding & JSONL reading" — validates that toClaudeProjectPath() + * resolves to real ~/.claude/projects/ directories and readLastJsonlEntry() + * can parse real session files. Only requires Claude to have been run at + * least once on this machine (no API key, no tmux needed). + * + * 2. "agent-claude-code (integration)" — full lifecycle test spawning a real + * Claude process. Requires claude binary, tmux, and ANTHROPIC_API_KEY. * * Skipped automatically when prerequisites are missing. */ import { execFile } from "node:child_process"; -import { mkdtemp, realpath, rm } from "node:fs/promises"; -import { tmpdir } from "node:os"; +import { mkdtemp, readdir, realpath, rm } from "node:fs/promises"; +import { homedir, tmpdir } from "node:os"; import { join } from "node:path"; import { promisify } from "node:util"; -import type { ActivityState, AgentSessionInfo } from "@composio/ao-core"; -import { afterAll, beforeAll, describe, expect, it } from "vitest"; -import claudeCodePlugin from "@composio/ao-plugin-agent-claude-code"; +import { readLastJsonlEntry, type ActivityState, type AgentSessionInfo } from "@composio/ao-core"; +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; +import claudeCodePlugin, { toClaudeProjectPath } from "@composio/ao-plugin-agent-claude-code"; import { isTmuxAvailable, killSessionsByPrefix, @@ -52,7 +57,118 @@ const hasApiKey = Boolean(process.env.ANTHROPIC_API_KEY); const canRun = tmuxOk && claudeBin !== null && hasApiKey; // --------------------------------------------------------------------------- -// Tests +// Path encoding & JSONL reading — real ~/.claude/projects/ validation +// --------------------------------------------------------------------------- + +/** + * Find a workspace path on this machine that has a matching Claude project + * directory with at least one JSONL session file. Returns null if Claude + * has never been used (test will skip). + */ +async function findRealClaudeProject(): Promise<{ + workspacePath: string; + projectDir: string; + jsonlFile: string; +} | null> { + const claudeProjectsDir = join(homedir(), ".claude", "projects"); + let dirs: string[]; + try { + dirs = await readdir(claudeProjectsDir); + } catch { + return null; + } + + for (const dir of dirs) { + // Reverse the encoding: leading dash → leading slash, internal dashes → slashes + // This is a heuristic — we just need one workspace that exists on disk + const projectDir = join(claudeProjectsDir, dir); + let files: string[]; + try { + files = await readdir(projectDir); + } catch { + continue; + } + + const jsonlFiles = files.filter( + (f) => f.endsWith(".jsonl") && !f.startsWith("agent-"), + ); + if (jsonlFiles.length === 0) continue; + + // Try to reconstruct the workspace path from the encoded dir name + // Encoded format: /Users/dev/project → -Users-dev-project + // We can't perfectly reverse this (dashes are ambiguous), but we can + // verify the forward direction: encode known paths and check if they match + const candidatePath = "/" + dir.slice(1).replace(/-/g, "/"); + const reEncoded = toClaudeProjectPath(candidatePath); + if (reEncoded === dir) { + return { + workspacePath: candidatePath, + projectDir, + jsonlFile: join(projectDir, jsonlFiles[0]), + }; + } + } + + return null; +} + +const realProject = await findRealClaudeProject(); + +describe.skipIf(!realProject)("path encoding & JSONL reading (real Claude data)", () => { + it("toClaudeProjectPath resolves to a real ~/.claude/projects/ directory", () => { + const encoded = toClaudeProjectPath(realProject!.workspacePath); + const expectedDir = join(homedir(), ".claude", "projects", encoded); + expect(expectedDir).toBe(realProject!.projectDir); + }); + + it("readLastJsonlEntry parses a real session JSONL file", async () => { + const entry = await readLastJsonlEntry(realProject!.jsonlFile); + + expect(entry).not.toBeNull(); + expect(entry!.modifiedAt).toBeInstanceOf(Date); + // lastType should be a known Claude message type or null (not undefined) + expect(entry!.lastType === null || typeof entry!.lastType === "string").toBe(true); + }); + + it("readLastJsonlEntry returns a recognized message type", async () => { + const entry = await readLastJsonlEntry(realProject!.jsonlFile); + if (entry?.lastType) { + const knownTypes = [ + "user", + "assistant", + "system", + "tool_use", + "progress", + "permission_request", + "error", + "summary", + "result", + "file-history-snapshot", + "queue-operation", + "pr-link", + ]; + expect(knownTypes).toContain(entry.lastType); + } + }); + + it("getActivityState returns a valid state for a real workspace path", async () => { + const agent = claudeCodePlugin.create(); + // Mock isProcessRunning to return false — we're not testing process detection, + // we're testing path resolution and JSONL parsing + vi.spyOn(agent, "isProcessRunning").mockResolvedValue(false); + + const handle = makeTmuxHandle("fake-session"); + const session = makeSession("real-path-test", handle, realProject!.workspacePath); + const state = await agent.getActivityState(session); + + // Process is "not running" so should get "exited" — but the important thing + // is it didn't return null (which would mean the path didn't resolve) + expect(state).toBe("exited"); + }); +}); + +// --------------------------------------------------------------------------- +// Full lifecycle test (requires claude binary + API key + tmux) // --------------------------------------------------------------------------- describe.skipIf(!canRun)("agent-claude-code (integration)", () => { @@ -62,12 +178,12 @@ describe.skipIf(!canRun)("agent-claude-code (integration)", () => { // Observations captured while the agent is alive let aliveRunning = false; - let aliveActivityState: ActivityState | undefined; + let aliveActivityState: ActivityState | null | undefined; let aliveSessionInfo: AgentSessionInfo | null = null; // Observations captured after the agent exits let exitedRunning: boolean; - let exitedActivityState: ActivityState; + let exitedActivityState: ActivityState | null; let exitedSessionInfo: AgentSessionInfo | null; beforeAll(async () => { @@ -131,7 +247,10 @@ describe.skipIf(!canRun)("agent-claude-code (integration)", () => { it("getActivityState → returns valid non-exited state while agent is alive", () => { expect(aliveActivityState).toBeDefined(); expect(aliveActivityState).not.toBe("exited"); - expect(["active", "idle", "waiting_input", "blocked"]).toContain(aliveActivityState); + // May be null (no JSONL yet) or a concrete state + expect([null, "active", "ready", "idle", "waiting_input", "blocked"]).toContain( + aliveActivityState, + ); }); it("getSessionInfo → returns session data while agent is alive (or null if path mismatch)", () => { diff --git a/packages/integration-tests/src/agent-codex.integration.test.ts b/packages/integration-tests/src/agent-codex.integration.test.ts index 5c41190b6..01f57c9b3 100644 --- a/packages/integration-tests/src/agent-codex.integration.test.ts +++ b/packages/integration-tests/src/agent-codex.integration.test.ts @@ -62,11 +62,11 @@ describe.skipIf(!canRun)("agent-codex (integration)", () => { // Observations captured while the agent is alive let aliveRunning = false; - let aliveActivityState: ActivityState | undefined; + let aliveActivityState: ActivityState | null | undefined; // Observations captured after the agent exits let exitedRunning: boolean; - let exitedActivityState: ActivityState; + let exitedActivityState: ActivityState | null; let sessionInfo: AgentSessionInfo | null; beforeAll(async () => { @@ -115,11 +115,11 @@ describe.skipIf(!canRun)("agent-codex (integration)", () => { expect(aliveRunning).toBe(true); }); - it("getActivityState → returns active while agent is running", () => { - // Codex uses conservative fallback: returns "active" when process is running - // (due to global rollout file storage without per-session scoping) + it("getActivityState → returns null while agent is running (no per-session tracking)", () => { + // Codex uses global rollout file storage without per-session scoping, + // so getActivityState honestly returns null instead of guessing. if (aliveActivityState !== undefined) { - expect(aliveActivityState).toBe("active"); + expect(aliveActivityState).toBeNull(); } }); diff --git a/packages/integration-tests/src/agent-opencode.integration.test.ts b/packages/integration-tests/src/agent-opencode.integration.test.ts index 7fd6cabd8..7f07d4242 100644 --- a/packages/integration-tests/src/agent-opencode.integration.test.ts +++ b/packages/integration-tests/src/agent-opencode.integration.test.ts @@ -86,11 +86,11 @@ describe.skipIf(!canRun)("agent-opencode (integration)", () => { // Observations captured while the agent is alive let aliveRunning = false; - let aliveActivityState: ActivityState | undefined; + let aliveActivityState: ActivityState | null | undefined; // Observations captured after the agent exits let exitedRunning: boolean; - let exitedActivityState: ActivityState; + let exitedActivityState: ActivityState | null; let sessionInfo: AgentSessionInfo | null; beforeAll(async () => { @@ -139,11 +139,11 @@ describe.skipIf(!canRun)("agent-opencode (integration)", () => { expect(aliveRunning).toBe(true); }); - it("getActivityState → returns active while agent is running", () => { - // OpenCode uses conservative fallback: returns "active" when process is running - // (due to global SQLite database shared by all sessions) + it("getActivityState → returns null while agent is running (no per-session tracking)", () => { + // OpenCode uses a global SQLite database shared by all sessions, + // so getActivityState honestly returns null instead of guessing. if (aliveActivityState !== undefined) { - expect(aliveActivityState).toBe("active"); + expect(aliveActivityState).toBeNull(); } }); diff --git a/packages/integration-tests/src/cli-spawn-core-read-new.integration.test.ts b/packages/integration-tests/src/cli-spawn-core-read-new.integration.test.ts index 27392edca..6c1b3f791 100644 --- a/packages/integration-tests/src/cli-spawn-core-read-new.integration.test.ts +++ b/packages/integration-tests/src/cli-spawn-core-read-new.integration.test.ts @@ -156,6 +156,7 @@ describe.skipIf(!tmuxOk)("CLI-Core integration (hash-based architecture)", () => const config: OrchestratorConfig = { configPath, // This enables hash-based architecture port: 3000, + readyThresholdMs: 300_000, defaults: { runtime: "tmux", agent: "claude-code", @@ -215,6 +216,7 @@ describe.skipIf(!tmuxOk)("CLI-Core integration (hash-based architecture)", () => const config: OrchestratorConfig = { configPath, port: 3000, + readyThresholdMs: 300_000, defaults: { runtime: "tmux", agent: "claude-code", diff --git a/packages/integration-tests/src/tracker-linear.integration.test.ts b/packages/integration-tests/src/tracker-linear.integration.test.ts index 5b5f41dd5..59cf9bdc4 100644 --- a/packages/integration-tests/src/tracker-linear.integration.test.ts +++ b/packages/integration-tests/src/tracker-linear.integration.test.ts @@ -22,6 +22,7 @@ import { request } from "node:https"; import type { ProjectConfig } from "@composio/ao-core"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; import trackerLinear from "@composio/ao-plugin-tracker-linear"; +import { pollUntilEqual } from "./helpers/polling.js"; // --------------------------------------------------------------------------- // Prerequisites @@ -250,7 +251,12 @@ describe.skipIf(!canRun)("tracker-linear (integration)", () => { it("updateIssue closes the issue and isCompleted reflects it", async () => { await tracker.updateIssue!(issueIdentifier, { state: "closed" }, project); - const completed = await tracker.isCompleted(issueIdentifier, project); + // Linear API has eventual consistency — poll until the state propagates + const completed = await pollUntilEqual( + () => tracker.isCompleted(issueIdentifier, project), + true, + { timeoutMs: 5_000, intervalMs: 500 }, + ); expect(completed).toBe(true); const issue = await tracker.getIssue(issueIdentifier, project); @@ -260,7 +266,12 @@ describe.skipIf(!canRun)("tracker-linear (integration)", () => { it("updateIssue reopens the issue", async () => { await tracker.updateIssue!(issueIdentifier, { state: "open" }, project); - const completed = await tracker.isCompleted(issueIdentifier, project); + // Linear API has eventual consistency — poll until the state propagates + const completed = await pollUntilEqual( + () => tracker.isCompleted(issueIdentifier, project), + false, + { timeoutMs: 5_000, intervalMs: 500 }, + ); expect(completed).toBe(false); const issue = await tracker.getIssue(issueIdentifier, project); diff --git a/packages/plugins/agent-aider/src/index.test.ts b/packages/plugins/agent-aider/src/index.test.ts index 077116c9b..3a31a5f28 100644 --- a/packages/plugins/agent-aider/src/index.test.ts +++ b/packages/plugins/agent-aider/src/index.test.ts @@ -258,29 +258,6 @@ describe("detectActivity", () => { }); }); -// ========================================================================= -// isProcessing -// ========================================================================= -describe("isProcessing", () => { - const agent = create(); - - it("returns false when no runtime handle", async () => { - expect(await agent.isProcessing(makeSession())).toBe(false); - }); - - it("returns true when process is running", async () => { - mockTmuxWithProcess("aider"); - const session = makeSession({ runtimeHandle: makeTmuxHandle() }); - expect(await agent.isProcessing(session)).toBe(true); - }); - - it("returns false when process is not running", async () => { - mockTmuxWithProcess("aider", false); - const session = makeSession({ runtimeHandle: makeTmuxHandle() }); - expect(await agent.isProcessing(session)).toBe(false); - }); -}); - // ========================================================================= // getSessionInfo // ========================================================================= diff --git a/packages/plugins/agent-aider/src/index.ts b/packages/plugins/agent-aider/src/index.ts index 772c87b5d..2ccbd43ab 100644 --- a/packages/plugins/agent-aider/src/index.ts +++ b/packages/plugins/agent-aider/src/index.ts @@ -1,5 +1,6 @@ import { shellEscape, + DEFAULT_READY_THRESHOLD_MS, type Agent, type AgentSessionInfo, type AgentLaunchConfig, @@ -104,14 +105,19 @@ function createAiderAgent(): Agent { return "active"; }, - async getActivityState(session: Session): Promise { + async getActivityState( + session: Session, + readyThresholdMs?: number, + ): Promise { + const threshold = readyThresholdMs ?? DEFAULT_READY_THRESHOLD_MS; + // Check if process is running first if (!session.runtimeHandle) return "exited"; const running = await this.isProcessRunning(session.runtimeHandle); if (!running) return "exited"; // Process is running - check for activity signals - if (!session.workspacePath) return "active"; + if (!session.workspacePath) return null; // Check for recent git commits (Aider auto-commits changes) const hasCommits = await hasRecentCommits(session.workspacePath); @@ -120,15 +126,15 @@ function createAiderAgent(): Agent { // Check chat history file modification time const chatMtime = await getChatHistoryMtime(session.workspacePath); if (!chatMtime) { - // No chat history yet, but process is running - assume active - return "active"; + // No chat history — cannot determine activity + return null; } - // If chat file was modified within last 30 seconds, consider active + // Classify by age: <30s active, threshold idle const ageMs = Date.now() - chatMtime.getTime(); - if (ageMs < 30_000) return "active"; - - // No recent activity - idle at prompt + const activeWindowMs = Math.min(30_000, threshold); + if (ageMs < activeWindowMs) return "active"; + if (ageMs < threshold) return "ready"; return "idle"; }, @@ -183,13 +189,6 @@ function createAiderAgent(): Agent { } }, - // NOTE: Aider lacks introspection to distinguish "processing" from "idle at prompt". - // Falling back to process liveness until richer detection is implemented (see #18). - async isProcessing(session: Session): Promise { - if (!session.runtimeHandle) return false; - return this.isProcessRunning(session.runtimeHandle); - }, - async getSessionInfo(_session: Session): Promise { // Aider doesn't have JSONL session files for introspection yet return null; diff --git a/packages/plugins/agent-claude-code/src/__tests__/activity-detection.test.ts b/packages/plugins/agent-claude-code/src/__tests__/activity-detection.test.ts index e7bea0eb5..33a602808 100644 --- a/packages/plugins/agent-claude-code/src/__tests__/activity-detection.test.ts +++ b/packages/plugins/agent-claude-code/src/__tests__/activity-detection.test.ts @@ -1,27 +1,376 @@ -import { describe, it, expect } from "vitest"; -import { toClaudeProjectPath } from "../index.js"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, utimesSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { toClaudeProjectPath, create } from "../index.js"; +import type { Session, RuntimeHandle } from "@composio/ao-core"; + +// Mock homedir() so getActivityState looks in our temp dir +vi.mock("node:os", async (importOriginal) => { + const actual = (await importOriginal()) as Record; + return { + ...actual, + homedir: () => fakeHome, + }; +}); + +let fakeHome: string; +let workspacePath: string; +let projectDir: string; + +function makeSession(overrides: Partial = {}): Session { + const handle: RuntimeHandle = { id: "test-1", runtimeName: "tmux", data: {} }; + return { + id: "test-1", + projectId: "test", + status: "working", + activity: "idle", + branch: "main", + issueId: null, + pr: null, + workspacePath, + runtimeHandle: handle, + agentInfo: null, + createdAt: new Date(), + lastActivityAt: new Date(), + metadata: {}, + ...overrides, + }; +} + +function writeJsonl( + entries: Array<{ type: string; [key: string]: unknown }>, + ageMs = 0, + filename = "session-abc.jsonl", +): void { + const content = entries.map((e) => JSON.stringify(e)).join("\n") + "\n"; + const filePath = join(projectDir, filename); + writeFileSync(filePath, content); + if (ageMs > 0) { + const past = new Date(Date.now() - ageMs); + utimesSync(filePath, past, past); + } +} + +// ============================================================================= +// toClaudeProjectPath +// ============================================================================= describe("Claude Code Activity Detection", () => { describe("toClaudeProjectPath", () => { - it("encodes paths correctly", () => { - expect(toClaudeProjectPath("/Users/dev/.worktrees/ao")).toBe("Users-dev--worktrees-ao"); + it("encodes paths with leading dash", () => { + expect(toClaudeProjectPath("/Users/dev/.worktrees/ao")).toBe("-Users-dev--worktrees-ao"); }); - it("strips leading slash", () => { - expect(toClaudeProjectPath("/tmp/test")).toBe("tmp-test"); + it("preserves leading slash as leading dash", () => { + expect(toClaudeProjectPath("/tmp/test")).toBe("-tmp-test"); }); - it("replaces dots", () => { - expect(toClaudeProjectPath("/path/to/.hidden")).toBe("path-to--hidden"); + it("replaces dots with dashes", () => { + expect(toClaudeProjectPath("/path/to/.hidden")).toBe("-path-to--hidden"); }); - it("handles Windows paths", () => { + it("handles Windows paths (no leading slash)", () => { expect(toClaudeProjectPath("C:\\Users\\dev\\project")).toBe("C-Users-dev-project"); }); + + it("handles consecutive dots and slashes", () => { + // /a/../b/./c → -a- -- -b- - -c → -a----b---c + expect(toClaudeProjectPath("/a/../b/./c")).toBe("-a----b---c"); + }); + + it("handles paths with multiple dot-directories", () => { + expect(toClaudeProjectPath("/Users/dev/.config/.local/share")).toBe( + "-Users-dev--config--local-share", + ); + }); }); - // NOTE: Full integration tests for getActivityState() require mocking homedir() - // or using a real Claude Code installation with actual session files. - // For now, we test the path encoding logic which is the core transformation. - // End-to-end testing should be done manually or with a real Claude Code instance. + // ============================================================================= + // getActivityState — integration tests with real JSONL files on disk + // ============================================================================= + + describe("getActivityState", () => { + const agent = create(); + + beforeEach(() => { + fakeHome = mkdtempSync(join(tmpdir(), "ao-activity-test-")); + workspacePath = join(fakeHome, "workspace"); + mkdirSync(workspacePath, { recursive: true }); + + // Create the Claude project directory matching the workspace path + const encoded = toClaudeProjectPath(workspacePath); + projectDir = join(fakeHome, ".claude", "projects", encoded); + mkdirSync(projectDir, { recursive: true }); + + // Mock isProcessRunning to always return true (we test exited separately) + vi.spyOn(agent, "isProcessRunning").mockResolvedValue(true); + }); + + afterEach(() => { + rmSync(fakeHome, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + // ----------------------------------------------------------------------- + // Process / handle edge cases + // ----------------------------------------------------------------------- + + it("returns 'exited' when process is not running", async () => { + vi.spyOn(agent, "isProcessRunning").mockResolvedValue(false); + writeJsonl([{ type: "assistant" }]); + expect(await agent.getActivityState(makeSession())).toBe("exited"); + }); + + it("returns 'exited' when no runtimeHandle", async () => { + expect(await agent.getActivityState(makeSession({ runtimeHandle: undefined }))).toBe( + "exited", + ); + }); + + it("returns 'exited' when runtimeHandle is null", async () => { + expect(await agent.getActivityState(makeSession({ runtimeHandle: null }))).toBe("exited"); + }); + + // ----------------------------------------------------------------------- + // Fallback cases (no JSONL data available) + // ----------------------------------------------------------------------- + + it("returns null when no session file exists yet", async () => { + // projectDir exists but is empty — no .jsonl files + expect(await agent.getActivityState(makeSession())).toBeNull(); + }); + + it("returns null when no workspacePath", async () => { + expect(await agent.getActivityState(makeSession({ workspacePath: null }))).toBeNull(); + }); + + it("returns null when project directory does not exist", async () => { + // Point to a workspace whose project dir doesn't exist + const badPath = join(fakeHome, "nonexistent-workspace"); + expect(await agent.getActivityState(makeSession({ workspacePath: badPath }))).toBeNull(); + }); + + // ----------------------------------------------------------------------- + // Real Claude Code entry types (observed in production) + // ----------------------------------------------------------------------- + + describe("real Claude Code entry types", () => { + it("returns 'active' for recent 'progress' entry (streaming)", async () => { + writeJsonl([{ type: "progress", status: "running tool" }]); + expect(await agent.getActivityState(makeSession())).toBe("active"); + }); + + it("returns 'active' for recent 'user' entry", async () => { + writeJsonl([{ type: "user", message: { content: "fix the bug" } }]); + expect(await agent.getActivityState(makeSession())).toBe("active"); + }); + + it("returns 'ready' for recent 'assistant' entry", async () => { + writeJsonl([{ type: "assistant", message: { content: "Done!" } }]); + expect(await agent.getActivityState(makeSession())).toBe("ready"); + }); + + it("returns 'ready' for recent 'system' entry", async () => { + writeJsonl([{ type: "system", summary: "session started" }]); + expect(await agent.getActivityState(makeSession())).toBe("ready"); + }); + + it("returns 'active' for recent 'file-history-snapshot' (bookkeeping)", async () => { + writeJsonl([{ type: "file-history-snapshot" }]); + expect(await agent.getActivityState(makeSession())).toBe("active"); + }); + + it("returns 'active' for recent 'queue-operation' (bookkeeping)", async () => { + writeJsonl([{ type: "queue-operation" }]); + expect(await agent.getActivityState(makeSession())).toBe("active"); + }); + + it("returns 'active' for recent 'pr-link' (bookkeeping)", async () => { + writeJsonl([{ type: "pr-link" }]); + expect(await agent.getActivityState(makeSession())).toBe("active"); + }); + }); + + // ----------------------------------------------------------------------- + // Agent interface spec types (may appear in future versions) + // ----------------------------------------------------------------------- + + describe("agent interface spec types", () => { + it("returns 'active' for recent 'tool_use' entry", async () => { + writeJsonl([{ type: "tool_use" }]); + expect(await agent.getActivityState(makeSession())).toBe("active"); + }); + + it("returns 'waiting_input' for 'permission_request'", async () => { + writeJsonl([{ type: "permission_request" }]); + expect(await agent.getActivityState(makeSession())).toBe("waiting_input"); + }); + + it("returns 'blocked' for 'error'", async () => { + writeJsonl([{ type: "error" }]); + expect(await agent.getActivityState(makeSession())).toBe("blocked"); + }); + + it("returns 'ready' for recent 'summary' entry", async () => { + writeJsonl([{ type: "summary", summary: "Implemented login feature" }]); + expect(await agent.getActivityState(makeSession())).toBe("ready"); + }); + + it("returns 'ready' for recent 'result' entry", async () => { + writeJsonl([{ type: "result" }]); + expect(await agent.getActivityState(makeSession())).toBe("ready"); + }); + }); + + // ----------------------------------------------------------------------- + // Staleness / threshold behavior + // ----------------------------------------------------------------------- + + describe("staleness threshold", () => { + it("returns 'idle' for stale 'assistant' entry (> threshold)", async () => { + writeJsonl([{ type: "assistant" }], 400_000); // 6+ min old + expect(await agent.getActivityState(makeSession())).toBe("idle"); + }); + + it("returns 'idle' for stale 'user' entry (> threshold)", async () => { + writeJsonl([{ type: "user" }], 400_000); + expect(await agent.getActivityState(makeSession())).toBe("idle"); + }); + + it("returns 'idle' for stale 'progress' entry (> threshold)", async () => { + writeJsonl([{ type: "progress" }], 400_000); + expect(await agent.getActivityState(makeSession())).toBe("idle"); + }); + + it("returns 'idle' for stale bookkeeping entry (> threshold)", async () => { + writeJsonl([{ type: "file-history-snapshot" }], 400_000); + expect(await agent.getActivityState(makeSession())).toBe("idle"); + }); + + it("'permission_request' ignores staleness (always waiting_input)", async () => { + writeJsonl([{ type: "permission_request" }], 400_000); + expect(await agent.getActivityState(makeSession())).toBe("waiting_input"); + }); + + it("'error' ignores staleness (always blocked)", async () => { + writeJsonl([{ type: "error" }], 400_000); + expect(await agent.getActivityState(makeSession())).toBe("blocked"); + }); + + it("respects custom readyThresholdMs", async () => { + // 2 minutes old — stale with 60s threshold, ready with default 5min + writeJsonl([{ type: "assistant" }], 120_000); + + expect(await agent.getActivityState(makeSession(), 60_000)).toBe("idle"); + expect(await agent.getActivityState(makeSession(), 300_000)).toBe("ready"); + }); + + it("custom threshold applies to active types too", async () => { + // 2 minutes old + writeJsonl([{ type: "user" }], 120_000); + + expect(await agent.getActivityState(makeSession(), 60_000)).toBe("idle"); + expect(await agent.getActivityState(makeSession(), 300_000)).toBe("active"); + }); + }); + + // ----------------------------------------------------------------------- + // JSONL file selection + // ----------------------------------------------------------------------- + + describe("JSONL file selection", () => { + it("picks the most recently modified JSONL file", async () => { + // Write an older file with "assistant" and a newer file with "user" + writeJsonl([{ type: "assistant" }], 10_000, "old-session.jsonl"); + writeJsonl([{ type: "user" }], 0, "new-session.jsonl"); + + expect(await agent.getActivityState(makeSession())).toBe("active"); + }); + + it("ignores agent- prefixed JSONL files", async () => { + writeJsonl([{ type: "user" }], 0, "agent-toolkit.jsonl"); + // No real session file → returns null (cannot determine activity) + expect(await agent.getActivityState(makeSession())).toBeNull(); + }); + + it("reads last entry from multi-entry JSONL (not first)", async () => { + // First entry is user (active), last entry is assistant (ready) + writeJsonl([ + { type: "user", message: { content: "fix bug" } }, + { type: "progress", status: "thinking" }, + { type: "assistant", message: { content: "Done!" } }, + ]); + expect(await agent.getActivityState(makeSession())).toBe("ready"); + }); + + it("returns null for empty JSONL file", async () => { + writeFileSync(join(projectDir, "empty-session.jsonl"), ""); + expect(await agent.getActivityState(makeSession())).toBeNull(); + }); + + it("returns null for JSONL with only whitespace", async () => { + writeFileSync(join(projectDir, "whitespace-session.jsonl"), "\n\n \n"); + // All lines are whitespace — readLastJsonlEntry returns null + expect(await agent.getActivityState(makeSession())).toBeNull(); + }); + + it("ignores non-JSONL files in project directory", async () => { + // Write a non-JSONL file + writeFileSync(join(projectDir, "config.json"), '{"type": "user"}'); + writeFileSync(join(projectDir, "notes.txt"), "some notes"); + // Write actual JSONL + writeJsonl([{ type: "assistant" }]); + expect(await agent.getActivityState(makeSession())).toBe("ready"); + }); + }); + + // ----------------------------------------------------------------------- + // Realistic session sequences + // ----------------------------------------------------------------------- + + describe("realistic session sequences", () => { + it("detects agent mid-work (progress is last entry)", async () => { + writeJsonl([ + { type: "user", message: { content: "implement auth" } }, + { type: "assistant", message: { content: "I'll implement..." } }, + { type: "progress", status: "Reading file" }, + { type: "progress", status: "Writing file" }, + { type: "progress", status: "Running tool" }, + ]); + expect(await agent.getActivityState(makeSession())).toBe("active"); + }); + + it("detects agent done and waiting (assistant is last entry)", async () => { + writeJsonl([ + { type: "user", message: { content: "implement auth" } }, + { type: "progress", status: "thinking" }, + { type: "progress", status: "writing" }, + { type: "assistant", message: { content: "I've implemented the auth feature." } }, + ]); + expect(await agent.getActivityState(makeSession())).toBe("ready"); + }); + + it("detects agent done with system summary", async () => { + writeJsonl([ + { type: "user", message: { content: "fix tests" } }, + { type: "progress", status: "thinking" }, + { type: "assistant", message: { content: "Fixed!" } }, + { type: "system", summary: "Fixed failing tests" }, + ]); + expect(await agent.getActivityState(makeSession())).toBe("ready"); + }); + + it("detects stale finished session", async () => { + writeJsonl( + [ + { type: "user", message: { content: "implement auth" } }, + { type: "assistant", message: { content: "Done" } }, + ], + 600_000, // 10 min old + ); + expect(await agent.getActivityState(makeSession())).toBe("idle"); + }); + }); + }); }); diff --git a/packages/plugins/agent-claude-code/src/index.test.ts b/packages/plugins/agent-claude-code/src/index.test.ts index bb58fd26d..24c32e6c4 100644 --- a/packages/plugins/agent-claude-code/src/index.test.ts +++ b/packages/plugins/agent-claude-code/src/index.test.ts @@ -4,13 +4,12 @@ import type { Session, RuntimeHandle, AgentLaunchConfig } from "@composio/ao-cor // --------------------------------------------------------------------------- // Hoisted mocks — available inside vi.mock factories // --------------------------------------------------------------------------- -const { mockExecFileAsync, mockReaddir, mockReadFile, mockStat, mockOpen, mockHomedir } = +const { mockExecFileAsync, mockReaddir, mockReadFile, mockStat, mockHomedir } = vi.hoisted(() => ({ mockExecFileAsync: vi.fn(), mockReaddir: vi.fn(), mockReadFile: vi.fn(), mockStat: vi.fn(), - mockOpen: vi.fn(), mockHomedir: vi.fn(() => "/mock/home"), })); @@ -22,7 +21,6 @@ vi.mock("node:child_process", () => { }); vi.mock("node:fs/promises", () => ({ - open: mockOpen, readdir: mockReaddir, readFile: mockReadFile, stat: mockStat, @@ -437,7 +435,7 @@ describe("getSessionInfo", () => { mockJsonlFiles('{"type":"user","message":{"content":"hello"}}'); await agent.getSessionInfo(makeSession({ workspacePath: "/Users/dev/.worktrees/ao/ao-3" })); expect(mockReaddir).toHaveBeenCalledWith( - "/mock/home/.claude/projects/Users-dev--worktrees-ao-ao-3", + "/mock/home/.claude/projects/-Users-dev--worktrees-ao-ao-3", ); }); }); @@ -499,24 +497,6 @@ describe("getSessionInfo", () => { }); }); - describe("last message type", () => { - it("returns the type of the last JSONL line", async () => { - const jsonl = [ - '{"type":"user","message":{"content":"test"}}', - '{"type":"assistant","message":{"content":"response"}}', - ].join("\n"); - mockJsonlFiles(jsonl); - const result = await agent.getSessionInfo(makeSession()); - expect(result?.lastMessageType).toBe("assistant"); - }); - - it("returns undefined when no lines have type", async () => { - mockJsonlFiles('{"content":"no type field"}'); - const result = await agent.getSessionInfo(makeSession()); - expect(result?.lastMessageType).toBeUndefined(); - }); - }); - describe("cost estimation", () => { it("aggregates usage.input_tokens and usage.output_tokens", async () => { const jsonl = [ @@ -666,145 +646,3 @@ describe("getSessionInfo", () => { }); }); -// ========================================================================= -// isProcessing — JSONL tail-read -// ========================================================================= -describe("isProcessing", () => { - const agent = create(); - - /** Helper to create a mock file handle from `open()` */ - function mockOpenWithContent(content: string, mtime: Date = new Date()) { - const buf = Buffer.from(content, "utf-8"); - const fh = { - stat: vi.fn().mockResolvedValue({ size: buf.length, mtime }), - read: vi - .fn() - .mockImplementation((buffer: Buffer, offset: number, length: number, position: number) => { - buf.copy(buffer, offset, position, position + length); - return Promise.resolve({ bytesRead: length, buffer }); - }), - close: vi.fn().mockResolvedValue(undefined), - }; - mockOpen.mockResolvedValue(fh); - // findLatestSessionFile uses readdir + stat - mockReaddir.mockResolvedValue(["session-abc.jsonl"]); - mockStat.mockResolvedValue({ mtimeMs: mtime.getTime(), mtime }); - } - - it("returns false when workspacePath is null", async () => { - expect(await agent.isProcessing(makeSession({ workspacePath: null }))).toBe(false); - }); - - it("returns false when no JSONL files exist", async () => { - mockReaddir.mockResolvedValue([]); - expect(await agent.isProcessing(makeSession())).toBe(false); - }); - - it("returns false when project dir does not exist", async () => { - mockReaddir.mockRejectedValue(new Error("ENOENT")); - expect(await agent.isProcessing(makeSession())).toBe(false); - }); - - it("returns false when JSONL file is empty", async () => { - mockOpenWithContent(""); - // Override stat to return size 0 - const fh = { - stat: vi.fn().mockResolvedValue({ size: 0, mtime: new Date() }), - read: vi.fn(), - close: vi.fn().mockResolvedValue(undefined), - }; - mockOpen.mockResolvedValue(fh); - mockReaddir.mockResolvedValue(["session-abc.jsonl"]); - mockStat.mockResolvedValue({ mtimeMs: Date.now(), mtime: new Date() }); - expect(await agent.isProcessing(makeSession())).toBe(false); - }); - - it("returns false when file is older than 30 seconds", async () => { - const oldTime = new Date(Date.now() - 60_000); - const content = '{"type":"user","message":{"content":"hello"}}\n'; - mockOpenWithContent(content, oldTime); - expect(await agent.isProcessing(makeSession())).toBe(false); - }); - - it("returns false when last type is assistant", async () => { - const now = new Date(); - const content = - [ - '{"type":"user","message":{"content":"fix bug"}}', - '{"type":"assistant","message":{"content":"I fixed it"}}', - ].join("\n") + "\n"; - mockOpenWithContent(content, now); - expect(await agent.isProcessing(makeSession())).toBe(false); - }); - - it("returns false when last type is system", async () => { - const now = new Date(); - const content = - [ - '{"type":"user","message":{"content":"hello"}}', - '{"type":"system","summary":"session started"}', - ].join("\n") + "\n"; - mockOpenWithContent(content, now); - expect(await agent.isProcessing(makeSession())).toBe(false); - }); - - it("returns true when last type is user and file is recent", async () => { - const now = new Date(); - const content = '{"type":"user","message":{"content":"fix this"}}\n'; - mockOpenWithContent(content, now); - expect(await agent.isProcessing(makeSession())).toBe(true); - }); - - it("returns true when last type is progress and file is recent", async () => { - const now = new Date(); - const content = - [ - '{"type":"user","message":{"content":"do it"}}', - '{"type":"progress","status":"running tool"}', - ].join("\n") + "\n"; - mockOpenWithContent(content, now); - expect(await agent.isProcessing(makeSession())).toBe(true); - }); - - it("returns false when open() throws", async () => { - mockOpen.mockRejectedValue(new Error("EACCES")); - mockReaddir.mockResolvedValue(["session-abc.jsonl"]); - mockStat.mockResolvedValue({ mtimeMs: Date.now(), mtime: new Date() }); - expect(await agent.isProcessing(makeSession())).toBe(false); - }); - - it("returns false when JSONL has no lines with type field", async () => { - const now = new Date(); - const content = '{"content":"no type here"}\n'; - mockOpenWithContent(content, now); - // lastType is null, and null is neither "assistant" nor "system", - // but we still return true because the file is recent and the agent - // appears active. Actually, let's verify what happens: - // entry = { lastType: null, modifiedAt: now } - // ageMs < 30_000 → true - // lastType !== "assistant" && lastType !== "system" → true - // So it returns true. That's the conservative (safe) behavior. - expect(await agent.isProcessing(makeSession())).toBe(true); - }); - - it("handles file with only malformed JSON gracefully", async () => { - const now = new Date(); - const content = "not valid json at all\n"; - mockOpenWithContent(content, now); - // All parse attempts fail → lastType: null → returns true (conservative) - expect(await agent.isProcessing(makeSession())).toBe(true); - }); - - it("always closes file handle even on error", async () => { - const fh = { - stat: vi.fn().mockRejectedValue(new Error("stat failed")), - read: vi.fn(), - close: vi.fn().mockResolvedValue(undefined), - }; - mockOpen.mockResolvedValue(fh); - mockReaddir.mockResolvedValue(["session-abc.jsonl"]); - mockStat.mockResolvedValue({ mtimeMs: Date.now(), mtime: new Date() }); - await agent.isProcessing(makeSession()); - expect(fh.close).toHaveBeenCalled(); - }); -}); diff --git a/packages/plugins/agent-claude-code/src/index.ts b/packages/plugins/agent-claude-code/src/index.ts index b2d303865..4878fcd6a 100644 --- a/packages/plugins/agent-claude-code/src/index.ts +++ b/packages/plugins/agent-claude-code/src/index.ts @@ -1,6 +1,7 @@ import { shellEscape, readLastJsonlEntry, + DEFAULT_READY_THRESHOLD_MS, type Agent, type AgentSessionInfo, type AgentLaunchConfig, @@ -195,7 +196,8 @@ export const manifest = { export function toClaudeProjectPath(workspacePath: string): string { // Handle Windows drive letters (C:\Users\... → C-Users-...) const normalized = workspacePath.replace(/\\/g, "/"); - return normalized.replace(/^\//, "").replace(/:/g, "").replace(/[/.]/g, "-"); + // Claude Code replaces / and . with - (keeping the leading slash as a leading -) + return normalized.replace(/:/g, "").replace(/[/.]/g, "-"); } /** Find the most recently modified .jsonl session file in a directory */ @@ -299,15 +301,6 @@ function extractSummary(lines: JsonlLine[]): string | null { return null; } -/** Extract the last message type from JSONL */ -function extractLastMessageType(lines: JsonlLine[]): string | undefined { - for (let i = lines.length - 1; i >= 0; i--) { - const line = lines[i]; - if (line?.type) return line.type; - } - return undefined; -} - /** Aggregate cost estimate from JSONL usage events */ function extractCost(lines: JsonlLine[]): CostEstimate | undefined { let inputTokens = 0; @@ -607,28 +600,12 @@ function createClaudeCodeAgent(): Agent { return pid !== null; }, - async isProcessing(session: Session): Promise { - if (!session.workspacePath) return false; + async getActivityState( + session: Session, + readyThresholdMs?: number, + ): Promise { + const threshold = readyThresholdMs ?? DEFAULT_READY_THRESHOLD_MS; - const projectPath = toClaudeProjectPath(session.workspacePath); - const projectDir = join(homedir(), ".claude", "projects", projectPath); - - const sessionFile = await findLatestSessionFile(projectDir); - if (!sessionFile) return false; - - const entry = await readLastJsonlEntry(sessionFile); - if (!entry) return false; - - const ageMs = Date.now() - entry.modifiedAt.getTime(); - if (ageMs > 30_000) return false; - - // If the last entry is "assistant" or "system", Claude has finished its turn - if (entry.lastType === "assistant" || entry.lastType === "system") return false; - - return true; - }, - - async getActivityState(session: Session): Promise { // Check if process is running first if (!session.runtimeHandle) return "exited"; const running = await this.isProcessRunning(session.runtimeHandle); @@ -636,8 +613,8 @@ function createClaudeCodeAgent(): Agent { // Process is running - check JSONL session file for activity if (!session.workspacePath) { - // No workspace path but process is running - assume active - return "active"; + // No workspace path — cannot determine activity without it + return null; } const projectPath = toClaudeProjectPath(session.workspacePath); @@ -645,31 +622,46 @@ function createClaudeCodeAgent(): Agent { const sessionFile = await findLatestSessionFile(projectDir); if (!sessionFile) { - // No session file found yet, but process is running - assume active - return "active"; + // No session file found — cannot determine activity + return null; } const entry = await readLastJsonlEntry(sessionFile); if (!entry) { - // Empty file or read error, but process is running - assume active - return "active"; + // Empty file or read error — cannot determine activity + return null; } - // Check staleness - no activity in 30 seconds means idle const ageMs = Date.now() - entry.modifiedAt.getTime(); - if (ageMs > 30_000) return "idle"; - // Classify based on last JSONL entry type + // Classify based on last JSONL entry type. + // + // Real Claude Code JSONL types (observed in production): + // progress — streaming tokens (most frequent) + // assistant — completed assistant response + // user — user message submitted + // system — system messages + // file-history-snapshot — file change tracking + // queue-operation — internal queue ops + // pr-link — PR URL logged + // + // Types from the Agent interface spec (may appear in future versions): + // tool_use, permission_request, error, summary, result switch (entry.lastType) { case "user": case "tool_use": - // Agent is processing user request or running tools - return "active"; + case "progress": + // Agent is processing: user just sent input, tools running, or + // actively streaming. Stale past threshold. + return ageMs > threshold ? "idle" : "active"; case "assistant": case "system": - // Agent finished its turn, waiting for user input - return "idle"; + case "summary": + case "result": + // Agent finished its turn. If recent, the session is alive and + // ready for the next instruction. Past threshold it's stale. + return ageMs > threshold ? "idle" : "ready"; case "permission_request": // Agent needs user approval for an action @@ -680,8 +672,9 @@ function createClaudeCodeAgent(): Agent { return "blocked"; default: - // Unknown type, assume active - return "active"; + // Unknown/bookkeeping types (file-history-snapshot, queue-operation, + // pr-link, etc.) — if recent, assume active; otherwise idle. + return ageMs > threshold ? "idle" : "active"; } }, @@ -716,7 +709,6 @@ function createClaudeCodeAgent(): Agent { summary: extractSummary(lines), agentSessionId, cost: extractCost(lines), - lastMessageType: extractLastMessageType(lines), lastLogModified, }; }, diff --git a/packages/plugins/agent-codex/src/index.test.ts b/packages/plugins/agent-codex/src/index.test.ts index 8c04c42c8..71a7422d8 100644 --- a/packages/plugins/agent-codex/src/index.test.ts +++ b/packages/plugins/agent-codex/src/index.test.ts @@ -257,29 +257,6 @@ describe("detectActivity", () => { }); }); -// ========================================================================= -// isProcessing -// ========================================================================= -describe("isProcessing", () => { - const agent = create(); - - it("returns false when no runtime handle", async () => { - expect(await agent.isProcessing(makeSession())).toBe(false); - }); - - it("returns true when process is running", async () => { - mockTmuxWithProcess("codex"); - const session = makeSession({ runtimeHandle: makeTmuxHandle() }); - expect(await agent.isProcessing(session)).toBe(true); - }); - - it("returns false when process is not running", async () => { - mockTmuxWithProcess("codex", false); - const session = makeSession({ runtimeHandle: makeTmuxHandle() }); - expect(await agent.isProcessing(session)).toBe(false); - }); -}); - // ========================================================================= // getSessionInfo // ========================================================================= diff --git a/packages/plugins/agent-codex/src/index.ts b/packages/plugins/agent-codex/src/index.ts index ac421240d..650e2900f 100644 --- a/packages/plugins/agent-codex/src/index.ts +++ b/packages/plugins/agent-codex/src/index.ts @@ -69,7 +69,7 @@ function createCodexAgent(): Agent { return "active"; }, - async getActivityState(session: Session): Promise { + async getActivityState(session: Session, _readyThresholdMs?: number): Promise { // Check if process is running first if (!session.runtimeHandle) return "exited"; const running = await this.isProcessRunning(session.runtimeHandle); @@ -78,12 +78,11 @@ function createCodexAgent(): Agent { // NOTE: Codex stores rollout files in a global ~/.codex/sessions/ directory // without workspace-specific scoping. When multiple Codex sessions run in // parallel, we cannot reliably determine which rollout file belongs to which - // session. Until Codex provides per-workspace session tracking, we fall back - // to process-running check only. See issue #13 for details. + // session. Until Codex provides per-workspace session tracking, we return + // null (unknown) rather than guessing. See issue #13 for details. // // TODO: Implement proper per-session activity detection when Codex supports it. - // For now, return "active" if process is running (conservative approach). - return "active"; + return null; }, async isProcessRunning(handle: RuntimeHandle): Promise { @@ -137,13 +136,6 @@ function createCodexAgent(): Agent { } }, - // NOTE: Codex lacks introspection to distinguish "processing" from "idle at prompt". - // Falling back to process liveness until richer detection is implemented (see #17). - async isProcessing(session: Session): Promise { - if (!session.runtimeHandle) return false; - return this.isProcessRunning(session.runtimeHandle); - }, - async getSessionInfo(_session: Session): Promise { // Codex doesn't have JSONL session files for introspection yet return null; diff --git a/packages/plugins/agent-opencode/src/index.test.ts b/packages/plugins/agent-opencode/src/index.test.ts index dd5de4c97..fa24045f9 100644 --- a/packages/plugins/agent-opencode/src/index.test.ts +++ b/packages/plugins/agent-opencode/src/index.test.ts @@ -250,29 +250,6 @@ describe("detectActivity", () => { }); }); -// ========================================================================= -// isProcessing -// ========================================================================= -describe("isProcessing", () => { - const agent = create(); - - it("returns false when no runtime handle", async () => { - expect(await agent.isProcessing(makeSession())).toBe(false); - }); - - it("returns true when process is running", async () => { - mockTmuxWithProcess("opencode"); - const session = makeSession({ runtimeHandle: makeTmuxHandle() }); - expect(await agent.isProcessing(session)).toBe(true); - }); - - it("returns false when process is not running", async () => { - mockTmuxWithProcess("opencode", false); - const session = makeSession({ runtimeHandle: makeTmuxHandle() }); - expect(await agent.isProcessing(session)).toBe(false); - }); -}); - // ========================================================================= // getSessionInfo // ========================================================================= diff --git a/packages/plugins/agent-opencode/src/index.ts b/packages/plugins/agent-opencode/src/index.ts index 8a60969ac..954247585 100644 --- a/packages/plugins/agent-opencode/src/index.ts +++ b/packages/plugins/agent-opencode/src/index.ts @@ -63,7 +63,7 @@ function createOpenCodeAgent(): Agent { return "active"; }, - async getActivityState(session: Session): Promise { + async getActivityState(session: Session, _readyThresholdMs?: number): Promise { // Check if process is running first if (!session.runtimeHandle) return "exited"; const running = await this.isProcessRunning(session.runtimeHandle); @@ -73,11 +73,10 @@ function createOpenCodeAgent(): Agent { // at ~/.local/share/opencode/opencode.db without per-workspace scoping. When // multiple OpenCode sessions run in parallel, database modifications from any // session will cause all sessions to appear active. Until OpenCode provides - // per-workspace session tracking, we fall back to process-running check only. + // per-workspace session tracking, we return null (unknown) rather than guessing. // // TODO: Implement proper per-session activity detection when OpenCode supports it. - // For now, return "active" if process is running (conservative approach). - return "active"; + return null; }, async isProcessRunning(handle: RuntimeHandle): Promise { @@ -131,13 +130,6 @@ function createOpenCodeAgent(): Agent { } }, - // NOTE: OpenCode lacks introspection to distinguish "processing" from "idle at prompt". - // Falling back to process liveness until richer detection is implemented (see #19). - async isProcessing(session: Session): Promise { - if (!session.runtimeHandle) return false; - return this.isProcessRunning(session.runtimeHandle); - }, - async getSessionInfo(_session: Session): Promise { // OpenCode doesn't have JSONL session files for introspection yet return null; diff --git a/packages/web/src/app/api/sessions/[id]/restore/route.ts b/packages/web/src/app/api/sessions/[id]/restore/route.ts index 015b34d3e..4e5ac683f 100644 --- a/packages/web/src/app/api/sessions/[id]/restore/route.ts +++ b/packages/web/src/app/api/sessions/[id]/restore/route.ts @@ -30,7 +30,8 @@ export async function POST(_request: NextRequest, { params }: { params: Promise< } const isTerminal = - RESTORABLE_STATUSES.has(session.status) || RESTORABLE_ACTIVITIES.has(session.activity); + RESTORABLE_STATUSES.has(session.status) || + (session.activity !== null && RESTORABLE_ACTIVITIES.has(session.activity)); if (!isTerminal) { return NextResponse.json({ error: "Session is not in a terminal state" }, { status: 409 }); diff --git a/packages/web/src/components/SessionCard.tsx b/packages/web/src/components/SessionCard.tsx index 7ae46bae2..93d4d9b8d 100644 --- a/packages/web/src/components/SessionCard.tsx +++ b/packages/web/src/components/SessionCard.tsx @@ -17,6 +17,7 @@ interface SessionCardProps { const activityIcon: Record = { active: "\u26A1", + ready: "\uD83D\uDFE2", idle: "\uD83D\uDCA4", waiting_input: "\u2753", blocked: "\uD83D\uDEA7", @@ -86,7 +87,7 @@ export function SessionCard({ session, onSend, onKill, onMerge, onRestore }: Ses session.activity === "active" && "animate-[pulse_2s_ease-in-out_infinite]", )} > - {activityIcon[session.activity] ?? "\u2753"} + {(session.activity && activityIcon[session.activity]) ?? "\u2753"} {session.id} diff --git a/packages/web/src/components/SessionDetail.tsx b/packages/web/src/components/SessionDetail.tsx index bf74782a3..a5403d39f 100644 --- a/packages/web/src/components/SessionDetail.tsx +++ b/packages/web/src/components/SessionDetail.tsx @@ -16,6 +16,7 @@ interface SessionDetailProps { const activityLabel: Record = { active: { label: "Active", color: "var(--color-accent-green)" }, + ready: { label: "Ready", color: "var(--color-accent-blue)" }, idle: { label: "Idle", color: "var(--color-text-muted)" }, waiting_input: { label: "Waiting for input", color: "var(--color-accent-yellow)" }, blocked: { label: "Blocked", color: "var(--color-accent-red)" }, @@ -111,8 +112,8 @@ export function SessionDetail({ session }: SessionDetailProps) { const searchParams = useSearchParams(); const startFullscreen = searchParams.get("fullscreen") === "true"; const pr = session.pr; - const activity = activityLabel[session.activity] ?? { - label: session.activity, + const activity = (session.activity && activityLabel[session.activity]) ?? { + label: session.activity ?? "unknown", color: "var(--color-text-muted)", }; diff --git a/packages/web/src/lib/types.ts b/packages/web/src/lib/types.ts index fea4699c5..e0f5efe58 100644 --- a/packages/web/src/lib/types.ts +++ b/packages/web/src/lib/types.ts @@ -53,7 +53,7 @@ export interface DashboardSession { id: string; projectId: string; status: SessionStatus; - activity: ActivityState; + activity: ActivityState | null; branch: string | null; issueId: string | null; // Deprecated: use issueUrl instead issueUrl: string | null; // Full issue URL @@ -124,7 +124,7 @@ export interface SSESnapshotEvent { sessions: Array<{ id: string; status: SessionStatus; - activity: ActivityState; + activity: ActivityState | null; attentionLevel: AttentionLevel; lastActivityAt: string; }>; @@ -134,7 +134,7 @@ export interface SSESnapshotEvent { export interface SSEActivityEvent { type: "session.activity"; sessionId: string; - activity: ActivityState; + activity: ActivityState | null; status: SessionStatus; attentionLevel: AttentionLevel; timestamp: string;