From 97a2a20f5b820c2710a88c07e0503ee4eb3c55b2 Mon Sep 17 00:00:00 2001 From: Test Date: Fri, 20 Feb 2026 15:29:04 -0800 Subject: [PATCH] fix: reap ghost sessions and deduplicate pending-* entries When a session's real ID can't be resolved during launch, use `pending-` format so unresolved entries are identifiable. In the session tracker's poll cycle, reap stale entries: - Remove pending-* entries when a resolved session shares the same PID - Mark "running" sessions as stopped when their PID is dead Add PID-based dedup in listSessions() as a safety net so pending-* entries never appear alongside their resolved counterpart. Fixes #22 Co-Authored-By: Claude Opus 4.6 --- src/adapters/claude-code.ts | 3 +- src/daemon/session-tracker.test.ts | 144 ++++++++++++++++++++++++++++- src/daemon/session-tracker.ts | 88 ++++++++++++++++++ 3 files changed, 233 insertions(+), 2 deletions(-) diff --git a/src/adapters/claude-code.ts b/src/adapters/claude-code.ts index 6761c85..d37d89b 100644 --- a/src/adapters/claude-code.ts +++ b/src/adapters/claude-code.ts @@ -259,7 +259,8 @@ export class ClaudeCodeAdapter implements AgentAdapter { resolvedSessionId = await this.pollForSessionId(logPath, pid, 5000); } - const sessionId = resolvedSessionId || crypto.randomUUID(); + const sessionId = + resolvedSessionId || (pid ? `pending-${pid}` : crypto.randomUUID()); // Persist session metadata so status checks work after wrapper exits if (pid) { diff --git a/src/daemon/session-tracker.test.ts b/src/daemon/session-tracker.test.ts index faeb2eb..c8b7156 100644 --- a/src/daemon/session-tracker.test.ts +++ b/src/daemon/session-tracker.test.ts @@ -2,7 +2,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import type { AgentSession } from "../core/types.js"; +import type { AgentAdapter, AgentSession } from "../core/types.js"; import { SessionTracker } from "./session-tracker.js"; import { StateManager } from "./state.js"; @@ -176,4 +176,146 @@ describe("SessionTracker", () => { expect(tracker.onSessionExit("unknown")).toBeUndefined(); }); }); + + describe("ghost session reaping (issue #22)", () => { + /** Create a mock adapter that returns the given sessions from list() */ + function mockAdapter(sessions: AgentSession[]): AgentAdapter { + return { + id: "mock", + list: async () => sessions, + peek: async () => "", + status: async () => sessions[0], + launch: async () => sessions[0], + stop: async () => {}, + resume: async () => {}, + async *events() {}, + }; + } + + it("marks dead PID sessions as stopped during poll", async () => { + // Pre-seed state with a "running" session whose PID is dead + tracker.track( + makeSession({ id: "pending-12345", status: "running", pid: 12345 }), + "claude-code", + ); + + // Create tracker with dead-PID checker and an adapter that returns nothing + const reapTracker = new SessionTracker(state, { + adapters: { "claude-code": mockAdapter([]) }, + isProcessAlive: () => false, + }); + + // Trigger a poll cycle + reapTracker.startPolling(); + // Wait for poll to complete + await new Promise((r) => setTimeout(r, 100)); + reapTracker.stopPolling(); + + const session = state.getSession("pending-12345"); + expect(session?.status).toBe("stopped"); + expect(session?.stoppedAt).toBeDefined(); + }); + + it("removes pending-* entry when resolved session exists with same PID", async () => { + // Pre-seed state with a pending entry + tracker.track( + makeSession({ id: "pending-99999", status: "running", pid: 99999 }), + "claude-code", + ); + + // Adapter returns a resolved session with the same PID + const resolvedSession = makeSession({ + id: "abc123-real-session-id", + status: "running", + pid: 99999, + }); + + const reapTracker = new SessionTracker(state, { + adapters: { "claude-code": mockAdapter([resolvedSession]) }, + isProcessAlive: (pid) => pid === 99999, + }); + + reapTracker.startPolling(); + await new Promise((r) => setTimeout(r, 100)); + reapTracker.stopPolling(); + + // pending-* entry should be removed + expect(state.getSession("pending-99999")).toBeUndefined(); + // Real session should exist + expect(state.getSession("abc123-real-session-id")).toBeDefined(); + expect(state.getSession("abc123-real-session-id")?.status).toBe( + "running", + ); + }); + + it("live PID sessions still show as running after poll", async () => { + tracker.track( + makeSession({ id: "live-session", status: "running", pid: 55555 }), + "claude-code", + ); + + // Adapter returns this session as running + const liveSession = makeSession({ + id: "live-session", + status: "running", + pid: 55555, + }); + + const reapTracker = new SessionTracker(state, { + adapters: { "claude-code": mockAdapter([liveSession]) }, + isProcessAlive: (pid) => pid === 55555, + }); + + reapTracker.startPolling(); + await new Promise((r) => setTimeout(r, 100)); + reapTracker.stopPolling(); + + const session = state.getSession("live-session"); + expect(session?.status).toBe("running"); + expect(session?.pid).toBe(55555); + }); + + it("listSessions deduplicates pending-* vs resolved entries by PID", () => { + // Both entries exist in state + tracker.track( + makeSession({ id: "pending-77777", status: "running", pid: 77777 }), + "claude-code", + ); + tracker.track( + makeSession({ + id: "real-session-uuid", + status: "running", + pid: 77777, + }), + "claude-code", + ); + + const list = tracker.listSessions({ all: true }); + // Only the resolved session should appear + const ids = list.map((s) => s.id); + expect(ids).toContain("real-session-uuid"); + expect(ids).not.toContain("pending-77777"); + }); + + it("keeps pending-* entry if no resolved session shares its PID", () => { + // Only a pending entry, no resolved session with same PID + tracker.track( + makeSession({ id: "pending-44444", status: "running", pid: 44444 }), + "claude-code", + ); + tracker.track( + makeSession({ + id: "different-session", + status: "running", + pid: 88888, + }), + "claude-code", + ); + + const list = tracker.listSessions({ all: true }); + const ids = list.map((s) => s.id); + expect(ids).toContain("pending-44444"); + expect(ids).toContain("different-session"); + }); + }); }); diff --git a/src/daemon/session-tracker.ts b/src/daemon/session-tracker.ts index 5681447..0a61fb4 100644 --- a/src/daemon/session-tracker.ts +++ b/src/daemon/session-tracker.ts @@ -4,6 +4,8 @@ import type { SessionRecord, StateManager } from "./state.js"; export interface SessionTrackerOpts { adapters: Record; pollIntervalMs?: number; + /** Override PID liveness check for testing (default: process.kill(pid, 0)) */ + isProcessAlive?: (pid: number) => boolean; } export class SessionTracker { @@ -11,11 +13,13 @@ export class SessionTracker { private adapters: Record; private pollIntervalMs: number; private pollHandle: ReturnType | null = null; + private readonly isProcessAlive: (pid: number) => boolean; constructor(state: StateManager, opts: SessionTrackerOpts) { this.state = state; this.adapters = opts.adapters; this.pollIntervalMs = opts.pollIntervalMs ?? 5000; + this.isProcessAlive = opts.isProcessAlive ?? defaultIsProcessAlive; } startPolling(): void { @@ -35,10 +39,17 @@ export class SessionTracker { } private async poll(): Promise { + // Collect PIDs from all adapter-returned sessions (the source of truth) + const adapterPidToId = new Map(); + for (const [adapterName, adapter] of Object.entries(this.adapters)) { try { const sessions = await adapter.list({ all: true }); for (const session of sessions) { + if (session.pid) { + adapterPidToId.set(session.pid, session.id); + } + const existing = this.state.getSession(session.id); const record = sessionToRecord(session, adapterName); @@ -59,6 +70,49 @@ export class SessionTracker { // Adapter unavailable — skip } } + + // Reap stale entries from daemon state + this.reapStaleEntries(adapterPidToId); + } + + /** + * Clean up ghost sessions in the daemon state: + * - pending-* entries whose PID matches a resolved session → remove pending + * - Any "running"/"idle" session in state whose PID is dead → mark stopped + */ + private reapStaleEntries(adapterPidToId: Map): void { + const sessions = this.state.getSessions(); + + for (const [id, record] of Object.entries(sessions)) { + // Bug 2: If this is a pending-* entry and a real session has the same PID, + // the pending entry is stale — remove it + if (id.startsWith("pending-") && record.pid) { + const resolvedId = adapterPidToId.get(record.pid); + if (resolvedId && resolvedId !== id) { + this.state.removeSession(id); + continue; + } + } + + // Bug 1: If session is "running"/"idle" but PID is dead, mark stopped + if ( + (record.status === "running" || record.status === "idle") && + record.pid + ) { + // Only reap if the adapter didn't return this session as running + // (adapter is the source of truth for sessions it knows about) + const adapterId = adapterPidToId.get(record.pid); + if (adapterId === id) continue; // Adapter confirmed this PID is active + + if (!this.isProcessAlive(record.pid)) { + this.state.setSession(id, { + ...record, + status: "stopped", + stoppedAt: new Date().toISOString(), + }); + } + } + } } /** Track a newly launched session */ @@ -96,6 +150,9 @@ export class SessionTracker { ); } + // Dedup: if a pending-* entry shares a PID with a resolved entry, show only the resolved one + filtered = deduplicatePendingSessions(filtered); + return filtered.sort((a, b) => { // Running first, then by recency if (a.status === "running" && b.status !== "running") return -1; @@ -122,6 +179,37 @@ export class SessionTracker { } } +/** Check if a process is alive via kill(pid, 0) signal check */ +function defaultIsProcessAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +/** + * Remove pending-* entries that share a PID with a resolved (non-pending) session. + * This is a safety net for list output — the poll() reaper handles cleanup in state. + */ +function deduplicatePendingSessions( + sessions: SessionRecord[], +): SessionRecord[] { + const realPids = new Set(); + for (const s of sessions) { + if (!s.id.startsWith("pending-") && s.pid) { + realPids.add(s.pid); + } + } + return sessions.filter((s) => { + if (s.id.startsWith("pending-") && s.pid && realPids.has(s.pid)) { + return false; + } + return true; + }); +} + function sessionToRecord( session: AgentSession, adapterName: string,