fix: comprehensive code review fixes — tests, timestamps, UI correctness

Address gaps identified in code review of the ActivityDetection PR:

Core / Session Manager:
- Add `timestamp` to all `{ state: "exited" }` returns in all 4 agent plugins
  (claude-code, aider, codex, opencode) using consistent `exitedAt = new Date()` pattern
- Add 2 new session-manager tests: timestamp propagation when detection timestamp
  is newer, and no-downgrade when detection timestamp is older
- Fix `parseJsonlFileTail` lint error: remove useless `= 0` initializer (value was
  always overwritten before use; catch block returns early)

Web package — tests:
- Fix 3 `api-routes.test.ts` failures: `sessionsGET()` needs a Request object since
  the route reads `request.url` for `?active=true` query param
- Fix `serialize.test.ts` rate-limit test: spy on `console.warn` (what the code uses)
  not `console.error`
- Add 5 `ActivityDot` component tests covering all activity states, unknown states,
  null activity, and dotOnly mode

Web package — UI correctness:
- Fix `relativeTime()` in SessionDetail to guard against invalid/empty ISO strings
- Fix timer Map leak: add `timersRef.current.clear()` in cleanup effect after forEach
- Add `encodeURIComponent` to sessionId in message fetch URL

Server — race condition fix:
- Guard `activeSessions.delete` in pty.onExit, ws.on("close"), and ws.on("error")
  against stale handlers deleting a newly-registered session with the same ID.
  Fixes flaky integration test where afterEach's pty.kill() fired asynchronously
  after the next test had already set up a new session with the same session ID.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

packages/core/src/__tests__/session-manager.test.ts

@@ -510,6 +510,68 @@ describe("list", () => {
     expect(agentWithNull.getActivityState).toHaveBeenCalled();
     expect(sessions[0].activity).toBeNull();
   });
+
+  it("updates lastActivityAt when detection timestamp is newer", async () => {
+    const newerTimestamp = new Date(Date.now() + 60_000); // 1 minute in the future
+    const agentWithTimestamp: Agent = {
+      ...mockAgent,
+      getActivityState: vi.fn().mockResolvedValue({ state: "active", timestamp: newerTimestamp }),
+    };
+    const registryWithTimestamp: PluginRegistry = {
+      ...mockRegistry,
+      get: vi.fn().mockImplementation((slot: string) => {
+        if (slot === "runtime") return mockRuntime;
+        if (slot === "agent") return agentWithTimestamp;
+        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: registryWithTimestamp });
+    const sessions = await sm.list();
+
+    expect(sessions[0].activity).toBe("active");
+    // lastActivityAt should be updated to the detection timestamp
+    expect(sessions[0].lastActivityAt).toEqual(newerTimestamp);
+  });
+
+  it("does not downgrade lastActivityAt when detection timestamp is older", async () => {
+    const olderTimestamp = new Date(0); // epoch — definitely older than session creation
+    const agentWithOldTimestamp: Agent = {
+      ...mockAgent,
+      getActivityState: vi.fn().mockResolvedValue({ state: "active", timestamp: olderTimestamp }),
+    };
+    const registryWithOldTimestamp: PluginRegistry = {
+      ...mockRegistry,
+      get: vi.fn().mockImplementation((slot: string) => {
+        if (slot === "runtime") return mockRuntime;
+        if (slot === "agent") return agentWithOldTimestamp;
+        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: registryWithOldTimestamp });
+    const sessions = await sm.list();
+
+    expect(sessions[0].activity).toBe("active");
+    // lastActivityAt should NOT be downgraded to the older detection timestamp
+    expect(sessions[0].lastActivityAt.getTime()).toBeGreaterThan(olderTimestamp.getTime());
+  });
 });
 
 describe("get", () => {

packages/core/src/session-manager.ts

@@ -250,11 +250,21 @@ export function createSessionManager(deps: SessionManagerDeps): SessionManager {
    * Enrich session with live runtime state (alive/exited) and activity detection.
    * Mutates the session object in place.
    */
+  const TERMINAL_SESSION_STATUSES = new Set([
+    "killed", "done", "merged", "terminated", "cleanup",
+  ]);
+
   async function enrichSessionWithRuntimeState(
     session: Session,
     plugins: ReturnType<typeof resolvePlugins>,
     handleFromMetadata: boolean,
   ): Promise<void> {
+    // Skip all subprocess/IO work for sessions already known to be terminal.
+    if (TERMINAL_SESSION_STATUSES.has(session.status)) {
+      session.activity = "exited";
+      return;
+    }
+
     // Check runtime liveness — but only if the handle came from metadata.
     // Fabricated handles (constructed as fallback for external sessions) should
     // NOT override status to "killed" — we don't know if the session ever had
@@ -660,16 +670,14 @@ export function createSessionManager(deps: SessionManagerDeps): SessionManager {
 
   async function list(projectId?: string): Promise<Session[]> {
     const allSessions = listAllSessions(projectId);
-    const sessions: Session[] = [];
 
-    for (const { sessionName, projectId: sessionProjectId } of allSessions) {
-      // Use config key to find project
+    const sessionPromises = allSessions.map(async ({ sessionName, projectId: sessionProjectId }) => {
       const project = config.projects[sessionProjectId];
-      if (!project) continue;
+      if (!project) return null;
 
       const sessionsDir = getProjectSessionsDir(project);
       const raw = readMetadataRaw(sessionsDir, sessionName);
-      if (!raw) continue;
+      if (!raw) return null;
 
       // Get file timestamps for createdAt/lastActivityAt
       let createdAt: Date | undefined;
@@ -686,12 +694,16 @@ export function createSessionManager(deps: SessionManagerDeps): SessionManager {
       const session = metadataToSession(sessionName, raw, createdAt, modifiedAt);
 
       const plugins = resolvePlugins(project);
-      await ensureHandleAndEnrich(session, sessionName, project, plugins);
+      // Cap per-session enrichment at 2s — subprocess calls (tmux/ps) can be
+      // slow under load. If we time out, session keeps its metadata values.
+      const enrichTimeout = new Promise<void>((resolve) => setTimeout(resolve, 2_000));
+      await Promise.race([ensureHandleAndEnrich(session, sessionName, project, plugins), enrichTimeout]);
 
-      sessions.push(session);
-    }
+      return session;
+    });
 
-    return sessions;
+    const results = await Promise.all(sessionPromises);
+    return results.filter((s): s is Session => s !== null);
   }
 
   async function get(sessionId: SessionId): Promise<Session | null> {

packages/plugins/agent-aider/src/index.ts

@@ -119,9 +119,10 @@ function createAiderAgent(): Agent {
       const threshold = readyThresholdMs ?? DEFAULT_READY_THRESHOLD_MS;
 
       // Check if process is running first
-      if (!session.runtimeHandle) return { state: "exited" };
+      const exitedAt = new Date();
+      if (!session.runtimeHandle) return { state: "exited", timestamp: exitedAt };
       const running = await this.isProcessRunning(session.runtimeHandle);
-      if (!running) return { state: "exited" };
+      if (!running) return { state: "exited", timestamp: exitedAt };
 
       // Process is running - check for activity signals
       if (!session.workspacePath) return null;

packages/plugins/agent-claude-code/src/index.ts

@@ -263,7 +263,7 @@ interface JsonlLine {
  */
 async function parseJsonlFileTail(filePath: string, maxBytes = 131_072): Promise<JsonlLine[]> {
   let content: string;
-  let offset = 0;
+  let offset: number;
   try {
     const { size = 0 } = await stat(filePath);
     offset = Math.max(0, size - maxBytes);
@@ -650,9 +650,10 @@ function createClaudeCodeAgent(): Agent {
       const threshold = readyThresholdMs ?? DEFAULT_READY_THRESHOLD_MS;
 
       // Check if process is running first
-      if (!session.runtimeHandle) return { state: "exited" };
+      const exitedAt = new Date();
+      if (!session.runtimeHandle) return { state: "exited", timestamp: exitedAt };
       const running = await this.isProcessRunning(session.runtimeHandle);
-      if (!running) return { state: "exited" };
+      if (!running) return { state: "exited", timestamp: exitedAt };
 
       // Process is running - check JSONL session file for activity
       if (!session.workspacePath) {

packages/plugins/agent-codex/src/index.ts

@@ -78,9 +78,10 @@ function createCodexAgent(): Agent {
 
     async getActivityState(session: Session, _readyThresholdMs?: number): Promise<ActivityDetection | null> {
       // Check if process is running first
-      if (!session.runtimeHandle) return { state: "exited" };
+      const exitedAt = new Date();
+      if (!session.runtimeHandle) return { state: "exited", timestamp: exitedAt };
       const running = await this.isProcessRunning(session.runtimeHandle);
-      if (!running) return { state: "exited" };
+      if (!running) return { state: "exited", timestamp: exitedAt };
 
       // NOTE: Codex stores rollout files in a global ~/.codex/sessions/ directory
       // without workspace-specific scoping. When multiple Codex sessions run in

packages/plugins/agent-opencode/src/index.ts

@@ -66,9 +66,10 @@ function createOpenCodeAgent(): Agent {
 
     async getActivityState(session: Session, _readyThresholdMs?: number): Promise<ActivityDetection | null> {
       // Check if process is running first
-      if (!session.runtimeHandle) return { state: "exited" };
+      const exitedAt = new Date();
+      if (!session.runtimeHandle) return { state: "exited", timestamp: exitedAt };
       const running = await this.isProcessRunning(session.runtimeHandle);
-      if (!running) return { state: "exited" };
+      if (!running) return { state: "exited", timestamp: exitedAt };
 
       // NOTE: OpenCode stores all session data in a single global SQLite database
       // at ~/.local/share/opencode/opencode.db without per-workspace scoping. When

packages/web/server/direct-terminal-ws.ts

@@ -147,7 +147,11 @@ export function createDirectTerminalServer(tmuxPath?: string): DirectTerminalSer
     // PTY exit
     pty.onExit(({ exitCode }) => {
       console.log(`[DirectTerminal] PTY exited for ${sessionId} with code ${exitCode}`);
-      activeSessions.delete(sessionId);
+      // Guard against stale exits: only delete if this pty is still the active one.
+      // A new connection may have already replaced this session entry.
+      if (activeSessions.get(sessionId)?.pty === pty) {
+        activeSessions.delete(sessionId);
+      }
       if (ws.readyState === WebSocket.OPEN) {
         ws.close(1000, "Terminal session ended");
       }
@@ -177,14 +181,20 @@ export function createDirectTerminalServer(tmuxPath?: string): DirectTerminalSer
     // WebSocket close
     ws.on("close", () => {
       console.log(`[DirectTerminal] WebSocket closed for ${sessionId}`);
-      activeSessions.delete(sessionId);
+      // Guard against stale closes replacing a newer session's entry
+      if (activeSessions.get(sessionId)?.pty === pty) {
+        activeSessions.delete(sessionId);
+      }
       pty.kill();
     });
 
     // WebSocket error
     ws.on("error", (err) => {
       console.error(`[DirectTerminal] WebSocket error for ${sessionId}:`, err.message);
-      activeSessions.delete(sessionId);
+      // Guard against stale error handlers replacing a newer session's entry
+      if (activeSessions.get(sessionId)?.pty === pty) {
+        activeSessions.delete(sessionId);
+      }
       pty.kill();
     });
   });

packages/web/src/__tests__/api-routes.test.ts

@@ -185,7 +185,7 @@ describe("API Routes", () => {
 
   describe("GET /api/sessions", () => {
     it("returns sessions array and stats", async () => {
-      const res = await sessionsGET();
+      const res = await sessionsGET(makeRequest("http://localhost:3000/api/sessions"));
       expect(res.status).toBe(200);
       const data = await res.json();
       expect(data.sessions).toBeDefined();
@@ -196,7 +196,7 @@ describe("API Routes", () => {
     });
 
     it("stats include expected fields", async () => {
-      const res = await sessionsGET();
+      const res = await sessionsGET(makeRequest("http://localhost:3000/api/sessions"));
       const data = await res.json();
       expect(data.stats).toHaveProperty("totalSessions");
       expect(data.stats).toHaveProperty("workingSessions");
@@ -205,7 +205,7 @@ describe("API Routes", () => {
     });
 
     it("sessions have expected shape", async () => {
-      const res = await sessionsGET();
+      const res = await sessionsGET(makeRequest("http://localhost:3000/api/sessions"));
       const data = await res.json();
       const session = data.sessions[0];
       expect(session).toHaveProperty("id");

packages/web/src/__tests__/components.test.tsx

@@ -4,8 +4,44 @@ import { CIBadge, CICheckList } from "@/components/CIBadge";
 import { PRStatus } from "@/components/PRStatus";
 import { SessionCard } from "@/components/SessionCard";
 import { AttentionZone } from "@/components/AttentionZone";
+import { ActivityDot } from "@/components/ActivityDot";
 import { makeSession, makePR } from "./helpers";
 
+// ── ActivityDot ───────────────────────────────────────────────────────
+
+describe("ActivityDot", () => {
+  it("renders label pill with activity name", () => {
+    render(<ActivityDot activity="active" />);
+    expect(screen.getByText("active")).toBeInTheDocument();
+  });
+
+  it("renders all known activity states", () => {
+    const states = ["active", "ready", "idle", "waiting_input", "blocked", "exited"] as const;
+    for (const state of states) {
+      const { unmount } = render(<ActivityDot activity={state} />);
+      const expected = state === "waiting_input" ? "waiting" : state;
+      expect(screen.getByText(expected)).toBeInTheDocument();
+      unmount();
+    }
+  });
+
+  it("renders unknown activity state with raw label", () => {
+    render(<ActivityDot activity="some_future_state" />);
+    expect(screen.getByText("some_future_state")).toBeInTheDocument();
+  });
+
+  it("renders null activity with 'unknown' label", () => {
+    render(<ActivityDot activity={null} />);
+    expect(screen.getByText("unknown")).toBeInTheDocument();
+  });
+
+  it("renders only a dot in dotOnly mode (no label)", () => {
+    render(<ActivityDot activity="active" dotOnly />);
+    // No label text should appear in dotOnly mode
+    expect(screen.queryByText("active")).not.toBeInTheDocument();
+  });
+});
+
 // ── CIBadge ──────────────────────────────────────────────────────────
 
 describe("CIBadge", () => {

packages/web/src/app/page.tsx

@@ -39,8 +39,9 @@ export default async function Home() {
     const coreSessions = allSessions.filter((s) => !s.id.endsWith("-orchestrator"));
     sessions = coreSessions.map(sessionToDashboard);
 
-    // Enrich metadata (issue labels, agent summaries, issue titles)
-    await enrichSessionsMetadata(coreSessions, sessions, config, registry);
+    // Enrich metadata (issue labels, agent summaries, issue titles) — cap at 3s
+    const metaTimeout = new Promise<void>((resolve) => setTimeout(resolve, 3_000));
+    await Promise.race([enrichSessionsMetadata(coreSessions, sessions, config, registry), metaTimeout]);
 
     // Enrich sessions that have PRs with live SCM data
     // Skip enrichment for terminal sessions (merged, closed, done, terminated)

packages/web/src/components/SessionDetail.tsx

@@ -44,7 +44,9 @@ function humanizeStatus(status: string): string {
 }
 
 function relativeTime(iso: string): string {
-  const diff = Date.now() - new Date(iso).getTime();
+  const ms = new Date(iso).getTime();
+  if (!iso || isNaN(ms)) return "unknown";
+  const diff = Date.now() - ms;
   const seconds = Math.floor(diff / 1000);
   if (seconds < 60) return "just now";
   const minutes = Math.floor(seconds / 60);
@@ -86,7 +88,7 @@ async function askAgentToFix(
   try {
     const { title, description } = cleanBugbotComment(comment.body);
     const message = `Please address this review comment:\n\nFile: ${comment.path}\nComment: ${title}\nDescription: ${description}\n\nComment URL: ${comment.url}\n\nAfter fixing, mark the comment as resolved at ${comment.url}`;
-    const res = await fetch(`/api/sessions/${sessionId}/message`, {
+    const res = await fetch(`/api/sessions/${encodeURIComponent(sessionId)}/message`, {
       method: "POST",
       headers: { "Content-Type": "application/json" },
       body: JSON.stringify({ message }),
@@ -428,6 +430,7 @@ function PRCard({ pr, sessionId }: { pr: DashboardPR; sessionId: string }) {
   useEffect(() => {
     return () => {
       timersRef.current.forEach((timer) => clearTimeout(timer));
+      timersRef.current.clear();
     };
   }, []);
 

packages/web/src/lib/__tests__/serialize.test.ts

@@ -329,8 +329,8 @@ describe("enrichSessionPR", () => {
     const dashboard = sessionToDashboard(coreSession);
     const scm = createFailingSCM();
 
-    // Spy on console.error
-    const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+    // Spy on console.warn (enrichSessionPR uses warn for rate-limit, not error)
+    const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
 
     await enrichSessionPR(dashboard, scm, pr);
 
@@ -339,10 +339,10 @@ describe("enrichSessionPR", () => {
     expect(dashboard.pr?.deletions).toBe(0);
     expect(dashboard.pr?.mergeability.blockers).toContain("API rate limited or unavailable");
 
-    // Should log error
-    expect(consoleErrorSpy).toHaveBeenCalled();
+    // Should log warning
+    expect(consoleWarnSpy).toHaveBeenCalled();
 
-    consoleErrorSpy.mockRestore();
+    consoleWarnSpy.mockRestore();
   });
 
   it("should cache even when most requests fail (to reduce API pressure)", async () => {