diff --git a/packages/coding-agent/.changes/empty-session-idle-status.md b/packages/coding-agent/.changes/empty-session-idle-status.md new file mode 100644 index 0000000000..f93584a897 --- /dev/null +++ b/packages/coding-agent/.changes/empty-session-idle-status.md @@ -0,0 +1 @@ +- Fixed `prime-agent list` pinning an abandoned empty session at "working" forever; an empty session with nothing in flight now reports "idle". diff --git a/packages/coding-agent/.changes/evict-empty-session-on-detach.md b/packages/coding-agent/.changes/evict-empty-session-on-detach.md new file mode 100644 index 0000000000..dc59645629 --- /dev/null +++ b/packages/coding-agent/.changes/evict-empty-session-on-detach.md @@ -0,0 +1 @@ +- Evict an empty, unnamed session's worker as soon as its last client disconnects, instead of parking it for the idle sweep; the on-disk draft session is preserved. diff --git a/packages/coding-agent/src/modes/daemon/daemon-session-list.ts b/packages/coding-agent/src/modes/daemon/daemon-session-list.ts index d41e83a67a..8d6eeff5cc 100644 --- a/packages/coding-agent/src/modes/daemon/daemon-session-list.ts +++ b/packages/coding-agent/src/modes/daemon/daemon-session-list.ts @@ -110,6 +110,17 @@ export function isSessionSummaryBusy(summary: SessionSummary): boolean { return summary.isSessionActive || summary.hasRunningRlmChildren === true; } +/** Naming signals intent to return, so named sessions are exempt even when empty. */ +export function isEvictableEmptySessionSummary(summary: SessionSummary): boolean { + return ( + summary.messageCount === 0 && + !summary.sessionName && + !isSessionSummaryBusy(summary) && + summary.hasRegisteredHeartbeat !== true && + summary.hasRegisteredCronJob !== true + ); +} + export function buildSessionList( activeSessions: readonly ActiveSessionState[], savedSessions: readonly SessionInfo[], @@ -395,6 +406,10 @@ export function activeActivityForSession(activeSession: ActiveSessionState): Ses if (activeSession.runtime.metadata?.kind === "subagent") { return "idle"; } + // An empty session never gets a summarizer verdict; don't hold it at "working" forever. + if (activeSession.runtime.session.messages.length === 0) { + return "idle"; + } // Hold at "working" until the idle verdict is current, so the view never // buckets an unlabeled idle session. return isSummaryCurrent(activeSession) ? "idle" : "working"; diff --git a/packages/coding-agent/src/modes/daemon/daemon-supervisor.ts b/packages/coding-agent/src/modes/daemon/daemon-supervisor.ts index 1db1f8eb72..5ba0b3e67c 100644 --- a/packages/coding-agent/src/modes/daemon/daemon-supervisor.ts +++ b/packages/coding-agent/src/modes/daemon/daemon-supervisor.ts @@ -89,6 +89,7 @@ import { getDaemonRuntimeIdentity } from "./daemon-runtime-identity.js"; import { matchesSessionIdSuffix } from "./daemon-session-id.js"; import { classifySessionRosterStatus, + isEvictableEmptySessionSummary, isSessionSummaryBusy, type SessionSummary, summaryForInactiveSession, @@ -869,17 +870,7 @@ export class DaemonSupervisor { ); if (this.shuttingDown || this.updateRestartPhase !== undefined) return; - let releaseFence: () => void = () => {}; - const fence = new Promise((resolveFence) => { - releaseFence = resolveFence; - }); - this.idleEvictionFence = fence; - try { - await this.mutationDrain.waitForDrain( - 0, - AbortSignal.timeout(IDLE_EVICTION_DRAIN_TIMEOUT_MS), - "Timed out draining daemon mutations for idle eviction", - ); + await this.withEvictionFence("Timed out draining daemon mutations for idle eviction", async () => { if (this.shuttingDown || this.updateRestartPhase !== undefined) return; await Promise.all( candidates.map((worker) => this.refreshWorkerSummaries(worker).catch(() => refreshed.delete(worker))), @@ -908,12 +899,96 @@ export class DaemonSupervisor { ); }), ); - } finally { + }); + } + + /** Waits for any held eviction fence, then takes the slot; release clears it only if still ours. */ + private async acquireIdleEvictionFence(): Promise<() => void> { + while (this.idleEvictionFence) await this.idleEvictionFence; + let releaseFence: () => void = () => {}; + const fence = new Promise((resolveFence) => { + releaseFence = resolveFence; + }); + this.idleEvictionFence = fence; + return () => { if (this.idleEvictionFence === fence) this.idleEvictionFence = undefined; releaseFence(); + }; + } + + /** Runs a passivation decision under the eviction fence after draining admitted mutations. */ + private async withEvictionFence(drainMessage: string, action: () => Promise): Promise { + const releaseFence = await this.acquireIdleEvictionFence(); + try { + await this.mutationDrain.waitForDrain(0, AbortSignal.timeout(IDLE_EVICTION_DRAIN_TIMEOUT_MS), drainMessage); + await action(); + } finally { + releaseFence(); } } + /** Re-validates one worker on fresh summaries under the caller's fence, then passivates it. */ + private async passivateWorkerIfStillEligible( + worker: ResidentWorker, + isStillEligible: () => boolean, + describeEvicted: () => string, + ): Promise { + await this.refreshWorkerSummaries(worker); + if (!isStillEligible()) return; + await this.stopWorker(worker, true); + this.log(describeEvicted()); + } + + private async evictEmptySessionOnLastDetach(activeSessionId: string): Promise { + if (this.shuttingDown || this.updateRestartPhase !== undefined) return; + const worker = this.matchWorkers(activeSessionId)[0]?.worker; + if ( + !worker || + worker.descriptor.lifecycle !== "ready" || + !worker.client || + worker.descriptor.ownerClientId !== undefined || // owned workers have their own cleanup path + this.isWorkerStopping(worker) + ) { + return; + } + try { + await this.refreshWorkerSummaries(worker); + } catch { + return; + } + if (!this.isEmptyDetachEvictionCandidate(worker)) return; + try { + // Idle-sweep coordination: fence new mutations, drain admitted ones, re-read before deciding. + await this.withEvictionFence("Timed out draining daemon mutations for empty-session eviction", () => + this.passivateWorkerIfStillEligible( + worker, + () => this.isEmptyDetachEvictionCandidate(worker), + () => + `Evicted empty session worker ${worker.descriptor.workerId} root=${worker.descriptor.rootSessionId ?? worker.descriptor.rootActiveSessionId} on last client detach`, + ), + ); + } catch (error) { + this.log(`Empty-session eviction failed for worker ${worker.descriptor.workerId}: ${String(error)}`); + } + } + + private isEmptyDetachEvictionCandidate(worker: ResidentWorker): boolean { + if ( + this.shuttingDown || + this.updateRestartPhase !== undefined || + this.workers.get(worker.descriptor.workerId) !== worker || + this.isWorkerStopping(worker) + ) { + return false; + } + const summaries = [...worker.summaries.values()]; + const hasAttachedClient = summaries.some((summary) => { + const summaryActiveSessionId = summary.activeSessionId ?? summary.id; + return [...this.clients].some((client) => client.attachedActiveSessionIds.has(summaryActiveSessionId)); + }); + return summaries.length > 0 && !hasAttachedClient && summaries.every(isEvictableEmptySessionSummary); + } + private async assertCurrentOwnership(): Promise { const ownership = this.ownership; if (!ownership) { @@ -1106,6 +1181,7 @@ export class DaemonSupervisor { for (const activeSessionId of [...client.attachedActiveSessionIds]) { client.attachedActiveSessionIds.delete(activeSessionId); void this.syncWorkerExtensionUi(activeSessionId); + void this.evictEmptySessionOnLastDetach(activeSessionId); } this.scheduleOwnedWorkerCleanupForClient(this.protocolClientId(client)); }; @@ -4174,6 +4250,7 @@ export class DaemonSupervisor { client.catchupPurposes?.delete(resolvedId); this.write(client, { type: "session_detached", activeSessionId: resolvedId }); void this.syncWorkerExtensionUi(resolvedId); + void this.evictEmptySessionOnLastDetach(resolvedId); } } diff --git a/packages/coding-agent/test/daemon-session-list.test.ts b/packages/coding-agent/test/daemon-session-list.test.ts index 6cb772e27f..a6b52745af 100644 --- a/packages/coding-agent/test/daemon-session-list.test.ts +++ b/packages/coding-agent/test/daemon-session-list.test.ts @@ -230,6 +230,12 @@ describe("buildSessionList", () => { expect(summary.sessionActions).toMatchObject({ queuedCount: 0, active: { kind: "turn" } }); expect(summary.unfinishedActionCount).toBe(3); + expect(summary.activity).toBe("working"); + }); + + it("marks an empty resident session idle instead of holding it at working", () => { + const summary = summaryForActiveSession(makeState({ activeSessionId: "empty" })); + expect(summary.activity).toBe("idle"); }); it("marks a finished subagent idle instead of holding it at working", () => { diff --git a/packages/coding-agent/test/daemon-supervisor-eviction.test.ts b/packages/coding-agent/test/daemon-supervisor-eviction.test.ts index 6611372dda..0941a3e552 100644 --- a/packages/coding-agent/test/daemon-supervisor-eviction.test.ts +++ b/packages/coding-agent/test/daemon-supervisor-eviction.test.ts @@ -31,6 +31,7 @@ interface SupervisorInternals { workers: Map; clients: Set<{ id: string; attachedActiveSessionIds: Set }>; idleEvictionFence?: Promise; + mutationDrain: { begin(): void; end(): void }; catalog: { resolve: ReturnType; stop: ReturnType }; createOrReuseWorker: ReturnType; stopWorker: ReturnType; @@ -442,3 +443,184 @@ describe("daemon supervisor whole-tree eviction", () => { expect(supervisor.createOrReuseWorker).not.toHaveBeenCalled(); }); }); + +describe("daemon supervisor empty-session eviction on detach", () => { + function makeDetachClient(id: string, attached: string[]) { + return { id, attachedActiveSessionIds: new Set(attached), socket: { destroyed: true } }; + } + + async function settle(): Promise { + await new Promise((resolve) => setTimeout(resolve, 25)); + } + + it("evicts only abandoned empty unnamed sessions, and only on the last detach", async () => { + const now = Date.parse("2026-08-01T12:00:00.000Z"); + const supervisor = makeSupervisor(); + const empty = makeWorker("empty", [makeSummary("empty-root", now, { messageCount: 0 })]); + const exempt = [ + makeWorker("named", [makeSummary("named-root", now, { messageCount: 0, sessionName: "keep me" })]), + makeWorker("busy", [makeSummary("busy-root", now, { messageCount: 0, isSessionActive: true })]), + makeWorker("one-message", [makeSummary("one-message-root", now)]), + makeWorker("heartbeat", [ + makeSummary("heartbeat-root", now, { messageCount: 0, hasRegisteredHeartbeat: true }), + ]), + makeWorker("cron", [makeSummary("cron-root", now, { messageCount: 0, hasRegisteredCronJob: true })]), + makeWorker("owned", [makeSummary("owned-root", now, { messageCount: 0 })]), + ]; + exempt[5]!.descriptor.ownerClientId = "owner"; + for (const worker of [empty, ...exempt]) { + supervisor.workers.set(worker.descriptor.workerId, worker); + } + const first = makeDetachClient("first", ["empty-root"]); + const viewer = makeDetachClient("viewer", [ + "empty-root", + "named-root", + "busy-root", + "one-message-root", + "heartbeat-root", + "cron-root", + "owned-root", + ]); + supervisor.clients.add(first); + supervisor.clients.add(viewer); + + await supervisor.handleCommand(first, { id: "detach-1", type: "detach", activeSessionId: "empty-root" }); + await settle(); + expect(supervisor.stopWorker).not.toHaveBeenCalled(); + + await supervisor.handleCommand(viewer, { id: "detach-all", type: "detach" }); + await vi.waitFor(() => expect(supervisor.stopWorker).toHaveBeenCalledWith(empty, true)); + await settle(); + expect(supervisor.stopWorker).toHaveBeenCalledTimes(1); + expect([...supervisor.workers.keys()].sort()).toEqual([ + "busy", + "cron", + "heartbeat", + "named", + "one-message", + "owned", + ]); + expect(supervisor.log).toHaveBeenCalledWith(expect.stringContaining("Evicted empty session worker empty")); + }); + + it("does not stop a worker that was replaced while its summary refresh was in flight", async () => { + const now = Date.parse("2026-08-01T12:00:00.000Z"); + const supervisor = makeSupervisor(); + const worker = makeWorker("swap", [makeSummary("swap-root", now, { messageCount: 0 })]); + let releaseList!: () => void; + worker.client!.request.mockImplementation( + () => + new Promise((resolve) => { + releaseList = () => resolve(success(undefined, "list", { sessions: [...worker.summaries.values()] })); + }), + ); + supervisor.workers.set("swap", worker); + const client = makeDetachClient("viewer", ["swap-root"]); + supervisor.clients.add(client); + + await supervisor.handleCommand(client, { id: "detach", type: "detach", activeSessionId: "swap-root" }); + await vi.waitFor(() => expect(worker.client!.request).toHaveBeenCalled()); + const successor = makeWorker("swap", [makeSummary("swap-root", now, { messageCount: 0 })]); + supervisor.workers.set("swap", successor); + releaseList(); + await settle(); + + expect(supervisor.stopWorker).not.toHaveBeenCalled(); + expect(supervisor.workers.get("swap")).toBe(successor); + }); + + it("does not evict when a mutation admitted during the refresh registers a schedule", async () => { + const now = Date.parse("2026-08-01T12:00:00.000Z"); + const supervisor = makeSupervisor(); + const worker = makeWorker("gap", [makeSummary("gap-root", now, { messageCount: 0 })]); + let releaseList!: () => void; + worker.client!.request.mockImplementationOnce( + () => + new Promise((resolve) => { + releaseList = () => + resolve( + success(undefined, "list", { sessions: [makeSummary("gap-root", now, { messageCount: 0 })] }), + ); + }), + ); + supervisor.workers.set("gap", worker); + const client = makeDetachClient("viewer", ["gap-root"]); + supervisor.clients.add(client); + + await supervisor.handleCommand(client, { id: "detach", type: "detach", activeSessionId: "gap-root" }); + await vi.waitFor(() => expect(worker.client!.request).toHaveBeenCalled()); + // A heartbeat_set admitted mid-refresh registers a schedule before the eviction decision. + supervisor.mutationDrain.begin(); + worker.client!.request.mockImplementation(async () => + success(undefined, "list", { + sessions: [makeSummary("gap-root", now, { messageCount: 0, hasRegisteredHeartbeat: true })], + }), + ); + releaseList(); + await settle(); + supervisor.mutationDrain.end(); + await settle(); + + expect(supervisor.stopWorker).not.toHaveBeenCalled(); + expect(supervisor.workers.get("gap")).toBe(worker); + }); + + it("evicts every empty draft when one client detaches from several at once", async () => { + const now = Date.parse("2026-08-01T12:00:00.000Z"); + const supervisor = makeSupervisor(); + const draftA = makeWorker("draft-a", [makeSummary("draft-a-root", now, { messageCount: 0 })]); + const draftB = makeWorker("draft-b", [makeSummary("draft-b-root", now, { messageCount: 0 })]); + supervisor.workers.set("draft-a", draftA); + supervisor.workers.set("draft-b", draftB); + const client = makeDetachClient("viewer", ["draft-a-root", "draft-b-root"]); + supervisor.clients.add(client); + + await supervisor.handleCommand(client, { id: "detach-all", type: "detach" }); + + await vi.waitFor(() => expect(supervisor.stopWorker).toHaveBeenCalledTimes(2)); + expect(supervisor.workers.size).toBe(0); + }); + + it("makes a starting sweep wait for the detach fence instead of overwriting it", async () => { + const now = Date.parse("2026-08-01T12:00:00.000Z"); + const supervisor = makeSupervisor(); + // Recent activity keeps the worker out of the sweep's own idle candidates. + const emptySessions = () => [ + makeSummary("gap-root", now, { messageCount: 0, lastActivityAt: new Date(now).toISOString() }), + ]; + const worker = makeWorker("gap", emptySessions()); + let releaseSweepList!: () => void; + let releaseHookList!: () => void; + worker + .client!.request.mockImplementationOnce( + () => + new Promise((resolve) => { + releaseSweepList = () => resolve(success(undefined, "list", { sessions: emptySessions() })); + }), + ) + .mockImplementationOnce(async () => success(undefined, "list", { sessions: emptySessions() })) + .mockImplementationOnce( + () => + new Promise((resolve) => { + releaseHookList = () => resolve(success(undefined, "list", { sessions: emptySessions() })); + }), + ); + supervisor.workers.set("gap", worker); + const client = makeDetachClient("viewer", ["gap-root"]); + supervisor.clients.add(client); + + const sweep = supervisor.runIdleEvictionSweep(now); + await vi.waitFor(() => expect(worker.client!.request).toHaveBeenCalledTimes(1)); + await supervisor.handleCommand(client, { id: "detach", type: "detach", activeSessionId: "gap-root" }); + await vi.waitFor(() => expect(worker.client!.request).toHaveBeenCalledTimes(3)); + releaseSweepList(); + await settle(); + + // The hook is still mid-decision, so its fence must still be in the slot. + expect(supervisor.idleEvictionFence).toBeDefined(); + releaseHookList(); + await sweep; + expect(supervisor.stopWorker).toHaveBeenCalledTimes(1); + expect(supervisor.stopWorker).toHaveBeenCalledWith(worker, true); + }); +});