From e1e13ab02710d7ab20a7dc782cc481c3a90b0556 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 9 Aug 2026 20:12:12 -0400 Subject: [PATCH 01/14] feat(sessions): promote live background work into canonical phase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A session whose foreground turn ended while its background jobs kept going read as idle everywhere a user glances: the Work-tab dot, the TopBar rollup, the dock badge, and the Lanes agent list all showed nothing while agents were mid-run. The "Background work xN" label existed, but only as a label — it never reached the canonical phase those surfaces derive from. canonicalSessionState now promotes a resting session with live background work back to `running`, and reports WHY via a new `liveness` field (turn / background / monitoring). Every existing consumer of the phase inherits the truth without a special case. - Two-state vocabulary: `monitoring` only when watch loops are the SOLE live work, so "still building" and "just watching CI" read differently. - Classification is a denylist (MONITOR_TASK_TYPES / INERT_TASK_TYPES). Unknown task types count as WORKING — an allowlist silently drops a real subagent the first time an SDK renames a type. - Generalized past Claude: codex background subagents and cursor cloud runs now count too. runtimeBackgroundWork() documents what escapes (detached nohup/setsid spawns, user-owned terminals, opencode/droid/pi). - Liveness stays in-memory and empty after restart: orphaned background work is not live work. - A failed, stopped, settled, or hand-raised session still outranks lingering liveness, so a stale "Working" can never mask a failure. - Subagent toolbar badge counts RUNNING subagents, not total tracked — a finished fleet no longer wears a number that only ever grew. - TerminalAttentionSummary.byLaneId removed deliberately: it had no consumer, and laneListSnapshotService already owns the per-lane rollup the Lanes tab and mobile both read. Co-Authored-By: Claude Opus 5 --- .../main/services/chat/agentChatService.ts | 81 +++++++- .../sessions/chatSessionProjection.ts | 1 + .../components/app/AppShell.aiStatus.test.tsx | 1 - .../renderer/components/app/TabNav.test.tsx | 1 - .../renderer/components/app/TopBar.test.tsx | 1 - .../components/chat/AgentChatPane.tsx | 21 +- .../components/lanes/LaneAgentList.tsx | 22 +- .../renderer/components/lanes/laneAgents.ts | 71 ++++++- .../components/review/ReviewPage.test.tsx | 1 - .../hooks/useAppWideSessionAttention.test.tsx | 2 - .../hooks/useAppWideSessionAttention.ts | 1 - .../renderer/lib/terminalAttention.test.ts | 80 +++++++ .../src/renderer/lib/terminalAttention.ts | 69 +++--- .../src/renderer/state/appStore.test.ts | 1 - apps/desktop/src/renderer/state/appStore.ts | 7 - .../src/shared/sessionCanonicalState.test.ts | 128 ++++++++++++ .../src/shared/sessionCanonicalState.ts | 196 ++++++++++++++++-- .../src/shared/sessionStatusPresentation.ts | 45 +++- apps/desktop/src/shared/types/chat.ts | 3 + apps/desktop/src/shared/types/sessions.ts | 7 + 20 files changed, 648 insertions(+), 91 deletions(-) diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index b5f2eae8b..5592c217c 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -472,6 +472,11 @@ import { type CodexSkillsListResponse, } from "../skills/agentSkillRuntimeService"; import { parseAgentChatTranscript } from "../../../shared/chatTranscript"; +import { + summarizeBackgroundWork, + totalBackgroundWork, + type SessionBackgroundWork, +} from "../../../shared/sessionCanonicalState"; import { isBackgroundShellCommand, isNonAgentTaskRun, @@ -1509,6 +1514,13 @@ type ClaudeRuntime = { * protect the long-lived query from idle cleanup and runtime eviction. */ liveBackgroundTaskIds: Set; + /** + * Raw SDK `task_type` per live background task, kept beside the level set so + * `runtimeBackgroundWork` can split working from monitoring without going + * through `activeSubagents` — which does not hold an entry for every plain + * background shell, and would silently classify those as unknown. + */ + backgroundTaskTypeById: Map; /** True after this CLI process has emitted its first authoritative level. */ backgroundTasksLevelObserved: boolean; seenBackgroundTaskIds: Set; @@ -1568,6 +1580,7 @@ type ClaudeRuntime = { function resetClaudeProcessBackgroundLevel(runtime: ClaudeRuntime): void { runtime.liveBackgroundTaskIds.clear(); + runtime.backgroundTaskTypeById.clear(); runtime.backgroundTasksLevelObserved = false; } @@ -2131,6 +2144,59 @@ function hasLivePendingInput(managed: ManagedChatSession | null | undefined): bo return false; } +const NO_BACKGROUND_WORK: SessionBackgroundWork = { workingCount: 0, monitoringCount: 0 }; + +/** + * Live work a chat session still owns after its foreground turn bookends, + * classified working vs monitoring for `canonicalSessionState`. + * + * ── Scope, stated honestly ────────────────────────────────────────────────── + * + * This reads RESIDENT runtime state only. It is therefore in-memory and empty + * after a restart, which is the intended contract: orphaned background work is + * not live work, and a persisted count would resurrect a "Working" pill over a + * process that died with the app. + * + * What it does NOT see, and cannot without new tracking: + * • processes an agent detached with `nohup`/`setsid`/`disown` — they leave + * ADE's process tree entirely, + * • long-lived processes started inside a user-owned terminal pane, which are + * the user's to manage and deliberately out of scope, + * • opencode / droid / pi background work — those runtimes report no + * background-task or subagent level to track. They contribute zero here + * rather than a guess. + */ +function runtimeBackgroundWork(runtime: ChatRuntime | null): SessionBackgroundWork { + if (!runtime) return NO_BACKGROUND_WORK; + switch (runtime.kind) { + case "claude": { + // The level set is authoritative for what is still live; the per-task + // types recorded alongside it decide which column each lands in. + return summarizeBackgroundWork( + [...runtime.liveBackgroundTaskIds].map( + (taskId) => runtime.backgroundTaskTypeById.get(taskId) ?? null, + ), + ); + } + case "codex": { + // Codex reports no task_type, so a backgrounded subagent is a real agent + // doing real work — the unknown-is-working default is also the right one. + const backgroundTypes: Array = []; + for (const subagent of runtime.activeSubagents.values()) { + if (subagent.background) backgroundTypes.push(null); + } + return summarizeBackgroundWork(backgroundTypes); + } + case "cursor": { + // A cloud run keeps executing on Cursor's infrastructure after the local + // turn ends — the clearest case of work outliving its turn ADE has. + return summarizeBackgroundWork(new Array(runtime.cloudRuns.size).fill(null)); + } + default: + return NO_BACKGROUND_WORK; + } +} + function hasRuntimeActiveWorkload(runtime: ChatRuntime | null): boolean { if (!runtime) return false; switch (runtime.kind) { @@ -14333,6 +14399,7 @@ export function createAgentChatService(args: { if (terminal) { runtime.seenBackgroundTaskIds.delete(args.taskId); runtime.liveBackgroundTaskIds.delete(args.taskId); + runtime.backgroundTaskTypeById.delete(args.taskId); runtime.backgroundTaskTitleById.delete(args.taskId); } else { runtime.seenBackgroundTaskIds.add(args.taskId); @@ -14354,11 +14421,14 @@ export function createAgentChatService(args: { tasks: unknown, ): void => { const nextIds = new Set(); + const nextTaskTypes = new Map(); for (const rawTask of Array.isArray(tasks) ? tasks : []) { const task = asRecord(rawTask); const taskId = compactString(task?.task_id); if (!task || !taskId) continue; nextIds.add(taskId); + const rawLevelTaskType = compactString(task.task_type); + if (rawLevelTaskType) nextTaskTypes.set(taskId, rawLevelTaskType); const description = compactString(task.description) ?? "Background work"; if (isClaudeAgentBackgroundTaskType(task.task_type)) { @@ -14424,6 +14494,10 @@ export function createAgentChatService(args: { runtime.liveBackgroundTaskIds.clear(); for (const taskId of nextIds) runtime.liveBackgroundTaskIds.add(taskId); + runtime.backgroundTaskTypeById.clear(); + for (const [taskId, taskType] of nextTaskTypes) { + runtime.backgroundTaskTypeById.set(taskId, taskType); + } runtime.backgroundTasksLevelObserved = true; managed.lastActivityTimestamp = Date.now(); }; @@ -29120,6 +29194,7 @@ export function createAgentChatService(args: { taskTodos: { seeded: false, byId: new Map() }, emittedTextByAssistantMessage: new Map(), liveBackgroundTaskIds: new Set(), + backgroundTaskTypeById: new Map(), backgroundTasksLevelObserved: false, seenBackgroundTaskIds: new Set(), stoppingBackgroundTaskIds: new Map(), @@ -38683,9 +38758,8 @@ export function createAgentChatService(args: { const claudeTag = provider === "claude" ? getClaudeSessionPointerForChat(row.id)?.tags[0] ?? null : undefined; - const activeBackgroundTaskCount = liveManaged?.runtime?.kind === "claude" - ? liveManaged.runtime.liveBackgroundTaskIds.size - : 0; + const backgroundWork = runtimeBackgroundWork(liveManaged?.runtime ?? null); + const activeBackgroundTaskCount = totalBackgroundWork(backgroundWork); let nextWakeAt: string | null = null; let scheduledWorkPaused = false; let scheduledWork: AgentChatScheduledWorkItem[] = []; @@ -38802,6 +38876,7 @@ export function createAgentChatService(args: { ...(provider === "claude" ? { claudeTag } : {}), nextWakeAt, activeBackgroundTaskCount, + backgroundWork, scheduledWorkPaused, scheduledWork, ...(sessionHasPendingInput ? { awaitingInput: true } : {}), diff --git a/apps/desktop/src/main/services/sessions/chatSessionProjection.ts b/apps/desktop/src/main/services/sessions/chatSessionProjection.ts index 03d2e4add..bca547e1e 100644 --- a/apps/desktop/src/main/services/sessions/chatSessionProjection.ts +++ b/apps/desktop/src/main/services/sessions/chatSessionProjection.ts @@ -77,6 +77,7 @@ export function projectChatOntoSession( nextWakeAt: chat.nextWakeAt, chatActivityMode: chat.interactionMode === "plan" ? "planning" : null, activeBackgroundTaskCount: chat.activeBackgroundTaskCount ?? 0, + ...(chat.backgroundWork ? { backgroundWork: chat.backgroundWork } : {}), ...(chat.claudeTag !== undefined ? { claudeTag: chat.claudeTag } : {}), ...(chat.orchestrationRunId ? { diff --git a/apps/desktop/src/renderer/components/app/AppShell.aiStatus.test.tsx b/apps/desktop/src/renderer/components/app/AppShell.aiStatus.test.tsx index bf8a8e976..43bafc4c7 100644 --- a/apps/desktop/src/renderer/components/app/AppShell.aiStatus.test.tsx +++ b/apps/desktop/src/renderer/components/app/AppShell.aiStatus.test.tsx @@ -92,7 +92,6 @@ function resetStore() { activeCount: 0, needsAttentionCount: 0, indicator: "none", - byLaneId: {}, }, } as any); } diff --git a/apps/desktop/src/renderer/components/app/TabNav.test.tsx b/apps/desktop/src/renderer/components/app/TabNav.test.tsx index 83ff16a59..e7934757f 100644 --- a/apps/desktop/src/renderer/components/app/TabNav.test.tsx +++ b/apps/desktop/src/renderer/components/app/TabNav.test.tsx @@ -22,7 +22,6 @@ function resetStore() { activeCount: 0, needsAttentionCount: 0, indicator: "none", - byLaneId: {}, }, workViewByProject: {}, laneWorkViewByScope: {}, diff --git a/apps/desktop/src/renderer/components/app/TopBar.test.tsx b/apps/desktop/src/renderer/components/app/TopBar.test.tsx index c8f0c0df2..462ef60cc 100644 --- a/apps/desktop/src/renderer/components/app/TopBar.test.tsx +++ b/apps/desktop/src/renderer/components/app/TopBar.test.tsx @@ -176,7 +176,6 @@ function resetStore() { activeCount: 0, needsAttentionCount: 0, indicator: "none", - byLaneId: {}, }, closeProject: vi.fn(async () => undefined), openRepo: vi.fn(async () => ({ rootPath: "/Users/arul/ADE", name: "ADE" })), diff --git a/apps/desktop/src/renderer/components/chat/AgentChatPane.tsx b/apps/desktop/src/renderer/components/chat/AgentChatPane.tsx index 60105a788..aa3e3b523 100644 --- a/apps/desktop/src/renderer/components/chat/AgentChatPane.tsx +++ b/apps/desktop/src/renderer/components/chat/AgentChatPane.tsx @@ -4471,6 +4471,19 @@ export function AgentChatPane({ () => selectedSubagentSnapshots.some((s) => s.background === true && s.status === "running"), [selectedSubagentSnapshots], ); + /** + * Subagents still RUNNING, which is what the toolbar badge counts. + * + * It used to count every tracked subagent, so a fleet that had finished an + * hour ago still wore a "7" — a number that only ever grew and told you + * nothing about now. A badge on a live-activity toggle has to answer "how + * much is happening", and once nothing is running the honest answer is to + * show no badge at all. + */ + const runningSubagentCount = useMemo( + () => selectedSubagentSnapshots.filter((s) => s.status === "running").length, + [selectedSubagentSnapshots], + ); // Auto-clear the subagent view when the underlying snapshot disappears // (e.g. session switch). Updating status is fine and stays in view. useEffect(() => { @@ -11695,8 +11708,8 @@ export function AgentChatPane({ : "Open agents, proof artifacts, and handoff controls for this chat.", effect: [ proofArtifactCount > 0 ? `${proofArtifactCount} artifact${proofArtifactCount === 1 ? "" : "s"}` : null, - selectedSubagentSnapshots.length > 0 - ? `${selectedSubagentSnapshots.length} subagent${selectedSubagentSnapshots.length === 1 ? "" : "s"}` + runningSubagentCount > 0 + ? `${runningSubagentCount} subagent${runningSubagentCount === 1 ? "" : "s"} running` : null, selectedScheduledWorkSnapshots.length > 0 ? `${selectedScheduledWorkSnapshots.length} scheduled` @@ -11737,9 +11750,9 @@ export function AgentChatPane({ {proofArtifactCount} - ) : selectedSubagentSnapshots.length > 0 ? ( + ) : runningSubagentCount > 0 ? ( - {selectedSubagentSnapshots.length} + {runningSubagentCount} ) : selectedScheduledWorkSnapshots.length > 0 ? ( diff --git a/apps/desktop/src/renderer/components/lanes/LaneAgentList.tsx b/apps/desktop/src/renderer/components/lanes/LaneAgentList.tsx index 76c6b83ac..0a6301606 100644 --- a/apps/desktop/src/renderer/components/lanes/LaneAgentList.tsx +++ b/apps/desktop/src/renderer/components/lanes/LaneAgentList.tsx @@ -9,17 +9,26 @@ import type { LaneAgent, LaneAgentActivity } from "./laneAgents"; function activityColor(activity: LaneAgentActivity): string { switch (activity) { - case "working": return COLORS.success; + case "working": + case "monitoring": return COLORS.success; case "awaiting-input": return COLORS.warning; case "ended": return COLORS.danger; default: return COLORS.textDim; } } -/** Tiny live pulse — spins while working/awaiting, static dot otherwise. */ +const ACTIVITY_TITLE: Record = { + working: "Working", + monitoring: "Monitoring", + "awaiting-input": "Awaiting input", + idle: "Idle", + ended: "Ended", +}; + +/** Tiny live pulse — spins while live, static dot otherwise. */ function ActivityPulse({ activity }: { activity: LaneAgentActivity }): React.ReactElement { const color = activityColor(activity); - if (activity === "working" || activity === "awaiting-input") { + if (activity === "working" || activity === "monitoring" || activity === "awaiting-input") { return ( ); } @@ -38,7 +50,7 @@ function ActivityPulse({ activity }: { activity: LaneAgentActivity }): React.Rea ); } diff --git a/apps/desktop/src/renderer/components/lanes/laneAgents.ts b/apps/desktop/src/renderer/components/lanes/laneAgents.ts index 8bc2c65b6..59a80ee10 100644 --- a/apps/desktop/src/renderer/components/lanes/laneAgents.ts +++ b/apps/desktop/src/renderer/components/lanes/laneAgents.ts @@ -4,11 +4,22 @@ import type { TerminalSessionSummary, TerminalToolType, } from "../../../shared/types"; +import { + backgroundWorkFromSummary, + totalBackgroundWork, + type SessionBackgroundWork, +} from "../../../shared/sessionCanonicalState"; import { listSessionsCached } from "../../lib/sessionListCache"; import { selectActiveProjectRoot, useAppStore } from "../../state/appStore"; -/** Unified live state for an agent row, glanceable at a list level. */ -export type LaneAgentActivity = "working" | "awaiting-input" | "idle" | "ended"; +/** + * Unified live state for an agent row, glanceable at a list level. + * + * `monitoring` is a live state, not a calm one — it sorts and pulses with + * `working`. It exists so a lane whose agents are only watching CI reads + * differently from one where three agents are mid-build. + */ +export type LaneAgentActivity = "working" | "monitoring" | "awaiting-input" | "idle" | "ended"; export type LaneAgent = { /** Session id (chat or terminal) — used to open the agent in the Work tab. */ @@ -30,11 +41,25 @@ export type LaneAgent = { /** CLI tool types that are agents (not plain shells). */ const SHELL_TOOL_TYPES = new Set(["shell"]); +/** + * A session whose turn is over but whose background jobs are not is still a + * live agent. Without this the Lanes list showed nothing at all while a + * background fleet ran — the row read "idle" for the whole of it. + */ +function backgroundActivity(summary: { + backgroundWork?: SessionBackgroundWork; + activeBackgroundTaskCount?: number; +}): LaneAgentActivity | null { + const work = backgroundWorkFromSummary(summary); + if (totalBackgroundWork(work) <= 0) return null; + return (work?.workingCount ?? 0) > 0 ? "working" : "monitoring"; +} + function chatActivity(summary: AgentChatSessionSummary): LaneAgentActivity { if (summary.status === "ended") return "ended"; if (summary.awaitingInput) return "awaiting-input"; if (summary.status === "active") return "working"; - return "idle"; + return backgroundActivity(summary) ?? "idle"; } function cliActivity(summary: TerminalSessionSummary): LaneAgentActivity { @@ -45,10 +70,10 @@ function cliActivity(summary: TerminalSessionSummary): LaneAgentActivity { ) return "awaiting-input"; switch (summary.runtimeState) { case "running": return "working"; - case "waiting-input": return "idle"; + case "waiting-input": return backgroundActivity(summary) ?? "idle"; case "exited": case "killed": return "ended"; - default: return "idle"; + default: return backgroundActivity(summary) ?? "idle"; } } @@ -77,11 +102,33 @@ function chatAgentFrom(summary: AgentChatSessionSummary): LaneAgent { activity: chatActivity(summary), lastHint: summary.awaitingInput ? "Awaiting your input" - : summary.summary?.trim() || summary.lastOutputPreview?.trim() || null, + : backgroundHint(summary) + ?? summary.summary?.trim() + ?? summary.lastOutputPreview?.trim() + ?? null, lastActivityAt: summary.lastActivityAt ?? summary.startedAt, }; } +/** + * Background work is the more useful hint than a stale last-output preview: + * the preview describes the turn that already ended, the count describes what + * is still running. Only shown once the turn is over — a live turn's own + * output is the better story. + */ +function backgroundHint(summary: { + status?: string; + backgroundWork?: SessionBackgroundWork; + activeBackgroundTaskCount?: number; +}): string | null { + if (summary.status === "active") return null; + const work = backgroundWorkFromSummary(summary); + const total = totalBackgroundWork(work); + if (total <= 0) return null; + const noun = (work?.workingCount ?? 0) > 0 ? "background job" : "monitor"; + return `${total} ${noun}${total === 1 ? "" : "s"} still running`; +} + function cliAgentFrom(summary: TerminalSessionSummary): LaneAgent { return { sessionId: summary.id, @@ -96,7 +143,10 @@ function cliAgentFrom(summary: TerminalSessionSummary): LaneAgent { || summary.attentionRequestedAt || summary.attentionSource === "provider_structured" ? "Awaiting your input" - : summary.summary?.trim() || summary.lastOutputPreview?.trim() || null, + : backgroundHint(summary) + ?? summary.summary?.trim() + ?? summary.lastOutputPreview?.trim() + ?? null, lastActivityAt: summary.endedAt ?? summary.startedAt, }; } @@ -132,9 +182,10 @@ export function buildLaneAgents( // most recently active first. const rank: Record = { working: 0, - "awaiting-input": 1, - idle: 2, - ended: 3, + monitoring: 1, + "awaiting-input": 2, + idle: 3, + ended: 4, }; return agents.sort((a, b) => { if (rank[a.activity] !== rank[b.activity]) return rank[a.activity] - rank[b.activity]; diff --git a/apps/desktop/src/renderer/components/review/ReviewPage.test.tsx b/apps/desktop/src/renderer/components/review/ReviewPage.test.tsx index a23b4e850..3a8303edf 100644 --- a/apps/desktop/src/renderer/components/review/ReviewPage.test.tsx +++ b/apps/desktop/src/renderer/components/review/ReviewPage.test.tsx @@ -53,7 +53,6 @@ function resetStore() { activeCount: 0, needsAttentionCount: 0, indicator: "none", - byLaneId: {}, }, workViewByProject: {}, laneWorkViewByScope: {}, diff --git a/apps/desktop/src/renderer/hooks/useAppWideSessionAttention.test.tsx b/apps/desktop/src/renderer/hooks/useAppWideSessionAttention.test.tsx index 3c638569f..4b0ccea1f 100644 --- a/apps/desktop/src/renderer/hooks/useAppWideSessionAttention.test.tsx +++ b/apps/desktop/src/renderer/hooks/useAppWideSessionAttention.test.tsx @@ -19,7 +19,6 @@ const summarizeTerminalAttention = vi.fn(() => ({ activeCount: 0, needsAttentionCount: 0, indicator: "none" as const, - byLaneId: {}, })); vi.mock("../lib/sessionListCache", () => ({ @@ -96,7 +95,6 @@ beforeEach(() => { activeCount: 0, needsAttentionCount: 2, indicator: "none" as const, - byLaneId: {}, }); Object.defineProperty(window, "ade", { configurable: true, diff --git a/apps/desktop/src/renderer/hooks/useAppWideSessionAttention.ts b/apps/desktop/src/renderer/hooks/useAppWideSessionAttention.ts index b41e0d0f8..e32e4ebd2 100644 --- a/apps/desktop/src/renderer/hooks/useAppWideSessionAttention.ts +++ b/apps/desktop/src/renderer/hooks/useAppWideSessionAttention.ts @@ -19,7 +19,6 @@ const EMPTY_TERMINAL_ATTENTION = { activeCount: 0, needsAttentionCount: 0, indicator: "none" as const, - byLaneId: {}, }; /** diff --git a/apps/desktop/src/renderer/lib/terminalAttention.test.ts b/apps/desktop/src/renderer/lib/terminalAttention.test.ts index f1bcafb3f..61245860c 100644 --- a/apps/desktop/src/renderer/lib/terminalAttention.test.ts +++ b/apps/desktop/src/renderer/lib/terminalAttention.test.ts @@ -6,6 +6,7 @@ import { sessionStatusBucket, sessionStatusDisplay, sessionStatusDot, + summarizeTerminalAttention, } from "./terminalAttention"; describe("terminalAttention", () => { @@ -301,6 +302,48 @@ describe("terminalAttention", () => { expect(presentation?.showsElapsed).toBe(true); }); + it("says Monitoring when watch loops are the only live work", () => { + const presentation = sessionStatusDisplay({ + status: "running", + runtimeState: "idle", + toolType: "claude-chat", + lastOutputPreview: "Foreground turn complete", + activeBackgroundTaskCount: 2, + backgroundWork: { workingCount: 0, monitoringCount: 2 }, + }); + + // "Monitoring" answers a different question than "Background work": + // a watch loop will not finish on its own, so the row is telling you it + // is safe to walk away. + expect(presentation).toMatchObject({ + label: "Monitoring ×2", + tone: "blue", + glyph: "monitoring", + }); + }); + + it("keeps a live turn's Planning label off a background-promoted row", () => { + const planningTurn = sessionStatusDisplay({ + status: "running", + runtimeState: "running", + toolType: "claude-chat", + lastOutputPreview: "thinking", + chatActivityMode: "planning", + }); + expect(planningTurn?.label).toBe("Planning"); + + // The turn is over; plan mode was how it ran, not what is happening now. + const backgroundOnly = sessionStatusDisplay({ + status: "running", + runtimeState: "idle", + toolType: "claude-chat", + lastOutputPreview: "Plan delivered", + chatActivityMode: "planning", + backgroundWork: { workingCount: 1, monitoringCount: 0 }, + }); + expect(backgroundOnly?.label).toBe("Background work"); + }); + it("shows Waiting only for an idle chat with a valid future wake", () => { const base = { status: "running" as const, @@ -353,3 +396,40 @@ describe("terminalAttention", () => { }); }); }); + +describe("summarizeTerminalAttention", () => { + const base = { + id: "s-1", + laneId: "lane-1", + status: "running" as const, + runtimeState: "idle" as const, + toolType: "claude-chat" as const, + lastOutputPreview: "Foreground turn complete", + startedAt: "2026-08-01T10:00:00.000Z", + }; + + it("counts a session whose only live work is in the background", () => { + // The Work-tab dot, the TopBar rollup and the dock badge all read this + // rollup. Before background work reached the canonical phase they showed + // nothing at all while agents were mid-run. + const quiet = summarizeTerminalAttention([base as never]); + expect(quiet.runningCount).toBe(0); + expect(quiet.indicator).toBe("none"); + + const busy = summarizeTerminalAttention([ + { ...base, backgroundWork: { workingCount: 2, monitoringCount: 0 }, activeBackgroundTaskCount: 2 } as never, + ]); + expect(busy.runningCount).toBe(1); + expect(busy.activeCount).toBe(1); + expect(busy.needsAttentionCount).toBe(0); + expect(busy.indicator).toBe("running-active"); + }); + + it("counts a monitoring-only session as running, not as needing you", () => { + const summary = summarizeTerminalAttention([ + { ...base, backgroundWork: { workingCount: 0, monitoringCount: 1 }, activeBackgroundTaskCount: 1 } as never, + ]); + expect(summary.runningCount).toBe(1); + expect(summary.needsAttentionCount).toBe(0); + }); +}); diff --git a/apps/desktop/src/renderer/lib/terminalAttention.ts b/apps/desktop/src/renderer/lib/terminalAttention.ts index 12669231a..5ba68766d 100644 --- a/apps/desktop/src/renderer/lib/terminalAttention.ts +++ b/apps/desktop/src/renderer/lib/terminalAttention.ts @@ -1,9 +1,11 @@ import type { SessionSettleOverride, TerminalRuntimeState, TerminalSessionStatus, TerminalSessionSummary, TerminalToolType } from "../../shared/types"; import { + backgroundWorkFromSummary, canonicalSessionState, canonicalStatusBucket, isSessionFiledAsSnoozed, type CanonicalSessionState, + type SessionBackgroundWork, type SessionBadge, } from "../../shared/sessionCanonicalState"; import { @@ -20,19 +22,22 @@ export type SessionStatusFilter = "all" | "running" | "awaiting-input" | "ended" export type SessionStatusBucket = Exclude; export type SessionFilingBucket = SessionStatusBucket | "snoozed"; -export type LaneTerminalAttentionSummary = { - runningCount: number; - activeCount: number; - needsAttentionCount: number; - indicator: TerminalRunIndicatorState; -}; - +/** + * App-wide rollup for the Work tab indicator and the dock badge. + * + * Deliberately has NO per-lane breakdown. It used to carry a `byLaneId` map + * that nothing ever read: the Lanes tab gets its per-lane rollup from + * `laneListSnapshotService.summarizeLaneRuntime` in the main process, which is + * also what the synced lane list and mobile use. Keeping a second, unread + * derivation here meant two answers to "is this lane busy" that could disagree + * the moment either moved — so the unread one is gone rather than wired up, and + * the main-process rollup stays the single per-lane source. + */ export type TerminalAttentionSummary = { runningCount: number; activeCount: number; needsAttentionCount: number; indicator: TerminalRunIndicatorState; - byLaneId: Record; }; const OSC_REGEX = /\u001b\][^\u0007]*(?:\u0007|\u001b\\)/g; @@ -143,6 +148,7 @@ type SessionCanonicalUiInput = { nextWakeAt?: string | null; chatActivityMode?: TerminalSessionSummary["chatActivityMode"]; activeBackgroundTaskCount?: number; + backgroundWork?: SessionBackgroundWork | null; nowMs?: number; }; @@ -168,6 +174,7 @@ export function canonicalInputFromSummary(session: TerminalSessionSummary): Sess nextWakeAt: session.nextWakeAt, chatActivityMode: session.chatActivityMode, activeBackgroundTaskCount: session.activeBackgroundTaskCount, + backgroundWork: backgroundWorkFromSummary(session), }; } @@ -185,6 +192,7 @@ export function sessionCanonicalUiState(session: SessionCanonicalUiInput): Canon settleOverride: session.settleOverride ?? null, attentionRequestedAt: session.attentionRequestedAt ?? null, lastTurnFailedAt: session.lastTurnFailedAt ?? null, + backgroundWork: backgroundWorkFromSummary(session), nowMs: session.nowMs, isChatTool: isChatToolType, }); @@ -220,10 +228,11 @@ export function sessionStatusDisplay( session: SessionCanonicalUiInput, overlay: SessionStatusOverlay = {}, ): SessionStatusPresentation | null { - const phase = sessionCanonicalUiState(session).phase; - return sessionStatusPresentation(phase, overlay, { + const state = sessionCanonicalUiState(session); + return sessionStatusPresentation(state.phase, overlay, { chatActivityMode: session.chatActivityMode, - activeBackgroundTaskCount: session.activeBackgroundTaskCount, + liveness: state.liveness, + backgroundWork: backgroundWorkFromSummary(session), nextWakeAt: session.nextWakeAt, nowMs: session.nowMs, }); @@ -298,7 +307,8 @@ export function sessionStatusDot( session: SessionCanonicalUiInput, overlay: SessionStatusOverlay = {}, ): SessionStatusDot { - const phase = sessionCanonicalUiState(session).phase; + const state = sessionCanonicalUiState(session); + const phase = state.phase; if (phase === "settled") { return { cls: "rounded-full border border-white/35 bg-transparent", @@ -306,7 +316,13 @@ export function sessionStatusDot( label: "Settled", }; } - const presentation = sessionStatusPresentation(phase, overlay); + // The dot's label is its tooltip, so it takes the same activity context as + // the full status slot — otherwise a monitoring row's dot reads "Working". + const presentation = sessionStatusPresentation(phase, overlay, { + chatActivityMode: session.chatActivityMode, + liveness: state.liveness, + backgroundWork: backgroundWorkFromSummary(session), + }); if (presentation) { return { cls: `rounded-full ${SESSION_TONE_DOT_CLASS[presentation.tone]}`, @@ -343,37 +359,25 @@ function legacySessionStatusDot(phase: CanonicalSessionPhase): SessionStatusDot * Rollup that feeds the Work tab indicator and dock badge. needsAttention is * the LOUD tier only (canonical needs_you) — a merely resting chat no longer * lights the tab amber; only a deterministic ask does. + * + * A session whose turn ended but whose background work is still live projects + * to `running` (see `canonicalSessionState`), so it counts here without any + * special case — which is the whole point of promoting background work into + * the phase rather than only into the row's label. */ export function summarizeTerminalAttention(sessions: TerminalSessionSummary[]): TerminalAttentionSummary { let runningCount = 0; let activeCount = 0; let needsAttentionCount = 0; - const byLane: Record = {}; for (const session of sessions) { const phase = sessionCanonicalUiState(canonicalInputFromSummary(session)).phase; const isLoud = phase === "needs_you"; const isWorking = phase === "starting" || phase === "running" || phase === "stale"; if (!isLoud && !isWorking) continue; - const lane = byLane[session.laneId] ?? { runningCount: 0, activeCount: 0, needsAttentionCount: 0 }; - lane.runningCount += 1; runningCount += 1; - if (isLoud) { - lane.needsAttentionCount += 1; - needsAttentionCount += 1; - } else { - lane.activeCount += 1; - activeCount += 1; - } - byLane[session.laneId] = lane; - } - - const byLaneId: Record = {}; - for (const [laneId, lane] of Object.entries(byLane)) { - byLaneId[laneId] = { - ...lane, - indicator: indicatorFromCounts(lane.runningCount, lane.needsAttentionCount) - }; + if (isLoud) needsAttentionCount += 1; + else activeCount += 1; } return { @@ -381,6 +385,5 @@ export function summarizeTerminalAttention(sessions: TerminalSessionSummary[]): activeCount, needsAttentionCount, indicator: indicatorFromCounts(runningCount, needsAttentionCount), - byLaneId }; } diff --git a/apps/desktop/src/renderer/state/appStore.test.ts b/apps/desktop/src/renderer/state/appStore.test.ts index 2f0013896..98c01e4c7 100644 --- a/apps/desktop/src/renderer/state/appStore.test.ts +++ b/apps/desktop/src/renderer/state/appStore.test.ts @@ -1001,7 +1001,6 @@ describe("appStore", () => { activeCount: 1, needsAttentionCount: 2, indicator: "running-needs-attention" as const, - byLaneId: {}, }; useAppStore.getState().setTerminalAttention(snapshot); expect(useAppStore.getState().terminalAttention).toEqual(snapshot); diff --git a/apps/desktop/src/renderer/state/appStore.ts b/apps/desktop/src/renderer/state/appStore.ts index 54fe86745..12bc6bfb0 100644 --- a/apps/desktop/src/renderer/state/appStore.ts +++ b/apps/desktop/src/renderer/state/appStore.ts @@ -216,12 +216,6 @@ export type TerminalAttentionSnapshot = { activeCount: number; needsAttentionCount: number; indicator: TerminalAttentionIndicator; - byLaneId: Record; }; const EMPTY_TERMINAL_ATTENTION: TerminalAttentionSnapshot = { @@ -229,7 +223,6 @@ const EMPTY_TERMINAL_ATTENTION: TerminalAttentionSnapshot = { activeCount: 0, needsAttentionCount: 0, indicator: "none", - byLaneId: {} }; /** diff --git a/apps/desktop/src/shared/sessionCanonicalState.test.ts b/apps/desktop/src/shared/sessionCanonicalState.test.ts index 710f06faa..a694f88c8 100644 --- a/apps/desktop/src/shared/sessionCanonicalState.test.ts +++ b/apps/desktop/src/shared/sessionCanonicalState.test.ts @@ -1,12 +1,15 @@ import { describe, expect, it } from "vitest"; import { parseSessionSettleOverride } from "./types/sessions"; import { + backgroundWorkFromSummary, canonicalSessionState, + classifyBackgroundWorkKind, isSessionFiledAsSnoozed, isSessionSnoozed, isSessionSnoozeExpired, isWakingSessionError, resolveSessionWakeReason, + summarizeBackgroundWork, SESSION_STALE_AFTER_MS, type CanonicalSessionInputs, } from "./sessionCanonicalState"; @@ -290,3 +293,128 @@ describe("parseSessionSettleOverride: a typo must not silently clear a pin", () expect(parseSessionSettleOverride("Clear")).toBeNull(); }); }); + +describe("background work liveness", () => { + const working = { workingCount: 1, monitoringCount: 0 }; + const monitoring = { workingCount: 0, monitoringCount: 2 }; + + it("promotes a resting chat with live background work back to running", () => { + // Before this, a Claude chat whose turn ended while its background agents + // kept going read `ready`, so the Work dot, TopBar rollup and dock badge + // all showed nothing while the agents were mid-run. + const resting = state({ toolType: "claude-chat", runtimeState: "idle" }); + expect(resting.phase).toBe("ready"); + expect(resting.liveness).toBeNull(); + + const promoted = state({ toolType: "claude-chat", runtimeState: "idle", backgroundWork: working }); + expect(promoted.phase).toBe("running"); + expect(promoted.liveness).toBe("background"); + }); + + it("promotes a resting CLI session too, not just chats", () => { + const promoted = state({ toolType: "codex", runtimeState: "idle", backgroundWork: working }); + expect(promoted.phase).toBe("running"); + expect(promoted.liveness).toBe("background"); + }); + + it("reads monitoring only when watch loops are the sole live work", () => { + expect(state({ runtimeState: "idle", backgroundWork: monitoring }).liveness).toBe("monitoring"); + // One real job alongside monitors is still "working" — the loudest live + // commitment wins, never the quietest. + expect( + state({ runtimeState: "idle", backgroundWork: { workingCount: 1, monitoringCount: 3 } }).liveness, + ).toBe("background"); + }); + + it("marks a genuinely live turn as turn liveness", () => { + expect(state({ runtimeState: "running" }).liveness).toBe("turn"); + }); + + it("never lets lingering background work mask a failure", () => { + // A failed turn with an orphaned monitor still ticking must read Failed. + expect( + state({ toolType: "claude-chat", runtimeState: "idle", lastTurnFailedAt: new Date(NOW).toISOString(), backgroundWork: working }).phase, + ).toBe("failed"); + expect( + state({ status: "completed", exitCode: 1, backgroundWork: working }).phase, + ).toBe("failed"); + expect( + state({ status: "completed", runtimeState: "killed", backgroundWork: working }).phase, + ).toBe("failed"); + }); + + it("never lets background work mask a raised hand or a declared settle", () => { + expect(state({ pendingInputItemId: "i-1", runtimeState: "idle", backgroundWork: working }).phase).toBe("needs_you"); + expect( + state({ runtimeState: "idle", settledAt: new Date(NOW).toISOString(), backgroundWork: working }).phase, + ).toBe("settled"); + }); + + it("leaves a silent session stale rather than claiming it is working", () => { + const silentSince = new Date(NOW - SESSION_STALE_AFTER_MS - 1_000).toISOString(); + expect(state({ lastActivityAt: silentSince, backgroundWork: working }).phase).toBe("stale"); + }); + + it("ignores an empty or absent background-work record", () => { + expect(state({ runtimeState: "idle", backgroundWork: null }).phase).toBe("idle"); + expect( + state({ runtimeState: "idle", backgroundWork: { workingCount: 0, monitoringCount: 0 } }).phase, + ).toBe("idle"); + }); +}); + +describe("classifyBackgroundWorkKind", () => { + it("treats every unrecognised task type as working", () => { + // The load-bearing property: an allowlist would silently drop a real + // subagent the first time an SDK renamed a task type. + expect(classifyBackgroundWorkKind("some_future_sdk_type")).toBe("working"); + expect(classifyBackgroundWorkKind(undefined)).toBe("working"); + expect(classifyBackgroundWorkKind(null)).toBe("working"); + expect(classifyBackgroundWorkKind(" ")).toBe("working"); + expect(classifyBackgroundWorkKind("subagent")).toBe("working"); + expect(classifyBackgroundWorkKind("local_workflow")).toBe("working"); + }); + + it("classifies only the known-passive types as monitoring", () => { + for (const taskType of ["monitor", "monitor_mcp", "local_bash", "shell", "background", "bash"]) { + expect(classifyBackgroundWorkKind(taskType)).toBe("monitoring"); + } + expect(classifyBackgroundWorkKind("MONITOR")).toBe("monitoring"); + expect(classifyBackgroundWorkKind(" local_bash ")).toBe("monitoring"); + }); + + it("drops inert types entirely", () => { + expect(classifyBackgroundWorkKind("plan")).toBe("inert"); + expect(classifyBackgroundWorkKind("dream")).toBe("inert"); + expect(summarizeBackgroundWork(["plan", "dream"])).toEqual({ workingCount: 0, monitoringCount: 0 }); + }); + + it("folds a mixed list into the two-state count", () => { + expect(summarizeBackgroundWork(["subagent", "monitor", "local_bash", "plan", null])).toEqual({ + workingCount: 2, + monitoringCount: 2, + }); + }); +}); + +describe("backgroundWorkFromSummary", () => { + it("prefers the explicit split when present", () => { + expect( + backgroundWorkFromSummary({ backgroundWork: { workingCount: 0, monitoringCount: 3 }, activeBackgroundTaskCount: 9 }), + ).toEqual({ workingCount: 0, monitoringCount: 3 }); + }); + + it("counts a split-less payload as working, never as passive", () => { + // An older peer or a remote runtime mid-upgrade sends only the total. + // Assuming those are monitors would under-report live work. + expect(backgroundWorkFromSummary({ activeBackgroundTaskCount: 2 })).toEqual({ + workingCount: 2, + monitoringCount: 0, + }); + }); + + it("returns null when nothing is live", () => { + expect(backgroundWorkFromSummary({})).toBeNull(); + expect(backgroundWorkFromSummary({ activeBackgroundTaskCount: 0 })).toBeNull(); + }); +}); diff --git a/apps/desktop/src/shared/sessionCanonicalState.ts b/apps/desktop/src/shared/sessionCanonicalState.ts index a29a78aa0..856e04ddb 100644 --- a/apps/desktop/src/shared/sessionCanonicalState.ts +++ b/apps/desktop/src/shared/sessionCanonicalState.ts @@ -40,12 +40,69 @@ export type SessionBadge = { label: string; }; +/** + * WHY a `running` phase is running. The phase alone cannot answer it, and the + * answer changes the copy: "Working 14s" is a live turn, "Background work ×2" + * is a turn that ended while its jobs kept going, and "Monitoring" is a watch + * loop that will never finish on its own. + * + * Deliberately not a phase. Filing, filtering, buckets, and the push wire all + * key off the phase, and every one of them wants these three treated + * identically: work is happening, nothing is asked of you. Splitting the phase + * would have forced a matching split into iOS's `AgentRunPhase`, the roster + * status, and `canonicalStatusBucket` for a distinction only the label cares + * about. + */ +export type SessionLiveness = "turn" | "background" | "monitoring"; + export type CanonicalSessionState = { phase: CanonicalSessionPhase; /** Non-null ONLY for attention states — capsules never render for calm ones. */ badge: SessionBadge | null; + /** Non-null ONLY for `running`. See `SessionLiveness`. */ + liveness: SessionLiveness | null; +}; + +/** + * Live work a session still owns after its foreground turn bookends — background + * shells, monitors, and subagent fleets that outlive the turn that spawned them. + * + * Split two ways because users need to tell "still building" apart from "just + * watching CI": a `monitoring` row is safe to walk away from, a `working` row + * is not. See `classifyBackgroundWorkKind` for how the split is decided. + */ +export type SessionBackgroundWork = { + /** Jobs doing real work. Anything unrecognised lands here — see the classifier. */ + workingCount: number; + /** Jobs that only watch: monitors, tails, polling loops. */ + monitoringCount: number; }; +export function totalBackgroundWork(work: SessionBackgroundWork | null | undefined): number { + if (!work) return 0; + return Math.max(0, work.workingCount) + Math.max(0, work.monitoringCount); +} + +/** + * Read a session summary's background work, tolerating a payload that carries + * only the older total. + * + * `activeBackgroundTaskCount` predates the working/monitoring split and is still + * the field the mobile roster and the push publisher read, so it stays the wire + * total. A summary that arrives with the total but no split (an older peer, a + * remote runtime mid-upgrade) is counted as WORKING — same denylist principle as + * `classifyBackgroundWorkKind`: unclassified is never assumed passive. + */ +export function backgroundWorkFromSummary(summary: { + backgroundWork?: SessionBackgroundWork | null; + activeBackgroundTaskCount?: number | null; +}): SessionBackgroundWork | null { + if (summary.backgroundWork) return summary.backgroundWork; + const total = summary.activeBackgroundTaskCount ?? 0; + if (!Number.isFinite(total) || total <= 0) return null; + return { workingCount: Math.trunc(total), monitoringCount: 0 }; +} + /** * A session still marked running that has produced no output for this long is * "stale" — running but silent. Distinct from the push relay's APNs TTLs @@ -60,6 +117,67 @@ const BADGE_BY_KIND: Record = { stale: { kind: "stale", label: "Stale" }, }; +/** + * Task types that only WATCH. A session whose ONLY live work is on this list + * reads "Monitoring" rather than "Working". + * + * ── Why a denylist, and why it must stay one ──────────────────────────────── + * + * The obvious shape is an allowlist of "these are real subagents". It is also + * the wrong one: provider agent-type names drift with every SDK release, so the + * first unfamiliar name silently drops a genuinely working agent out of the + * count and the row goes quiet while the agent is mid-run — the exact failure + * this whole state exists to prevent. + * + * The rule, therefore: classify only what we KNOW is passive, and treat every + * unrecognised type as working. A new SDK task type shows up as "Working", + * which is at worst slightly over-loud and at best exactly right. Adding a name + * here is a deliberate act with a known job behind it. + */ +const MONITOR_TASK_TYPES: ReadonlySet = new Set([ + "monitor", + "monitor_mcp", + "local_bash", + "shell", + "background", + "bash", +]); + +/** + * Types that hold no live work at all — they describe a turn's *thinking*, not + * a process. Counting them would put a "Working" pill on a session whose only + * outstanding item is a plan document. + */ +const INERT_TASK_TYPES: ReadonlySet = new Set(["plan", "dream"]); + +export type BackgroundWorkKind = "working" | "monitoring" | "inert"; + +/** + * The ONE classifier for live background work, shared by every runtime adapter + * so Claude's `local_bash` and Codex's monitor land in the same column. + */ +export function classifyBackgroundWorkKind(taskType: string | null | undefined): BackgroundWorkKind { + const normalized = typeof taskType === "string" ? taskType.trim().toLowerCase() : ""; + if (!normalized) return "working"; + if (INERT_TASK_TYPES.has(normalized)) return "inert"; + if (MONITOR_TASK_TYPES.has(normalized)) return "monitoring"; + return "working"; +} + +/** Fold a list of live task types into the two-state count. */ +export function summarizeBackgroundWork( + taskTypes: Iterable, +): SessionBackgroundWork { + let workingCount = 0; + let monitoringCount = 0; + for (const taskType of taskTypes) { + const kind = classifyBackgroundWorkKind(taskType); + if (kind === "working") workingCount += 1; + else if (kind === "monitoring") monitoringCount += 1; + } + return { workingCount, monitoringCount }; +} + export type CanonicalSessionInputs = { status: TerminalSessionStatus; runtimeState?: TerminalRuntimeState | null; @@ -89,6 +207,16 @@ export type CanonicalSessionInputs = { * so exitCode can't carry this). Cleared when the next turn starts. */ lastTurnFailedAt?: string | null; + /** + * Live background work owned by the session (monitors, background shells, + * subagent fleets still running after the turn bookends). + * + * In-memory and runtime-derived by design: after a restart nothing is live, + * because orphaned background work is not live work. There is deliberately no + * persisted column behind this — a resurrected "Working" pill on a session + * whose process died with the app is worse than no pill at all. + */ + backgroundWork?: SessionBackgroundWork | null; nowMs?: number; /** Chat sessions idle between turns are "ready", not running/ended. */ isChatTool?: (toolType: TerminalToolType | null | undefined) => boolean; @@ -113,12 +241,41 @@ function isSilentPast(lastActivityAt: string | null | undefined, nowMs: number, * 4. failed — non-zero exit / killed / chat turn death, * 5. stale — status running but silent ≥ SESSION_STALE_AFTER_MS, * 6. running, - * 7. resting states — ready (idle chat, quiet "your move"), idle, ended. + * 7. resting states — ready (idle chat, quiet "your move"), idle, ended, + * EXCEPT when the session still owns live background work, which promotes + * the row back to `running` (see `restingPhaseWithBackgroundWork`). + * + * Note where the background-work promotion sits: below failure, below stale. + * A stale "Working" pill must never mask a failed session — so a chat whose + * turn died reads Failed even while its orphaned monitor is still ticking, and + * a session silent past the stale threshold still reads Stale, because "nothing + * has happened in three hours" is the fact worth surfacing regardless of what + * claims to be alive. */ export function canonicalSessionState(args: CanonicalSessionInputs): CanonicalSessionState { const nowMs = args.nowMs ?? Date.now(); const chat = args.isChatTool?.(args.toolType) ?? false; + /** + * A resting session that still owns live background work is not resting. + * Returns the promoted `running` state, or the caller's resting state when + * nothing is live. + */ + const restingPhaseWithBackgroundWork = ( + resting: CanonicalSessionState, + ): CanonicalSessionState => { + const work = args.backgroundWork; + if (totalBackgroundWork(work) <= 0) return resting; + return { + phase: "running", + badge: null, + // Monitoring only when watch loops are the SOLE live work. One real job + // alongside three monitors is still "Working" — the honest summary of a + // session is its loudest live commitment, not its quietest. + liveness: (work?.workingCount ?? 0) > 0 ? "background" : "monitoring", + }; + }; + // 1. Deterministic attention beats everything — including the failure and // stale checks below (an agent explicitly asking is actionable regardless). if ( @@ -126,7 +283,7 @@ export function canonicalSessionState(args: CanonicalSessionInputs): CanonicalSe || args.attentionRequestedAt || args.attentionSource === "provider_structured" ) { - return { phase: "needs_you", badge: BADGE_BY_KIND.needs_you }; + return { phase: "needs_you", badge: BADGE_BY_KIND.needs_you, liveness: null }; } // 2. Declared settle (or a "settled" override). No timestamp math: activity @@ -139,7 +296,12 @@ export function canonicalSessionState(args: CanonicalSessionInputs): CanonicalSe const pinnedActive = args.settleOverride === "active"; const atRest = args.status !== "running" || args.runtimeState === "idle"; if (!pinnedActive && atRest && (args.settleOverride === "settled" || args.settledAt)) { - return { phase: "settled", badge: null }; + // Deliberately NOT promoted by background work. Settle now tears the + // session's machinery down (`stopSessionBackgroundWork`), so a settled row + // with live work is a teardown that has not finished draining, not a state + // worth advertising — and re-lighting it would let a stubborn monitor + // out-vote the user's explicit "this is done". + return { phase: "settled", badge: null, liveness: null }; } const ended = args.status !== "running"; @@ -148,20 +310,20 @@ export function canonicalSessionState(args: CanonicalSessionInputs): CanonicalSe // failure. Keep it badge-free and let the session row's red dot carry the // ended state. if (args.status === "disposed") { - return { phase: "stopped", badge: null }; + return { phase: "stopped", badge: null, liveness: null }; } // 4. Failure: a non-clean exit, an explicit "failed" persisted status // (spawn/setup failures that die before an exit code), or a killed // runtime — all deterministic "failed" signals a terminal-backed session // reports. if (typeof args.exitCode === "number" && args.exitCode !== 0) { - return { phase: "failed", badge: BADGE_BY_KIND.failed }; + return { phase: "failed", badge: BADGE_BY_KIND.failed, liveness: null }; } if (args.status === "failed") { - return { phase: "failed", badge: BADGE_BY_KIND.failed }; + return { phase: "failed", badge: BADGE_BY_KIND.failed, liveness: null }; } if (args.runtimeState === "killed") { - return { phase: "failed", badge: BADGE_BY_KIND.failed }; + return { phase: "failed", badge: BADGE_BY_KIND.failed, liveness: null }; } // Chats never "end" like PTYs — they rest between turns. A turn that died // on a runtime/API error is a real failure the row must carry (chats have @@ -170,39 +332,43 @@ export function canonicalSessionState(args: CanonicalSessionInputs): CanonicalSe // is genuinely over — ended, not perpetually "your move". if (chat) { if (args.lastTurnFailedAt) { - return { phase: "failed", badge: BADGE_BY_KIND.failed }; + return { phase: "failed", badge: BADGE_BY_KIND.failed, liveness: null }; } if (args.status === "detached") { - return { phase: "ended", badge: null }; + return { phase: "ended", badge: null, liveness: null }; } - return { phase: "ready", badge: null }; + return restingPhaseWithBackgroundWork({ phase: "ready", badge: null, liveness: null }); } // A clean process exit only says the CLI ended. Settlement is a lifecycle // declaration made by the user (or the lane PR-merge policy), never // inferred from process mechanics. There is no "derived clean-exit settle" // anywhere in ADE — if you find a comment claiming otherwise, it is stale. - return { phase: "ended", badge: null }; + return { phase: "ended", badge: null, liveness: null }; } // Chat rows keep status "running" even when a turn dies — surface the // persisted failure marker ahead of the calm running/ready states. if (chat && args.lastTurnFailedAt) { - return { phase: "failed", badge: BADGE_BY_KIND.failed }; + return { phase: "failed", badge: BADGE_BY_KIND.failed, liveness: null }; } // 6. Stale: running but silent past the threshold. if (isSilentPast(args.lastActivityAt, nowMs, SESSION_STALE_AFTER_MS)) { - return { phase: "stale", badge: BADGE_BY_KIND.stale }; + return { phase: "stale", badge: BADGE_BY_KIND.stale, liveness: null }; } // Idle chats between turns are ready (calm); idle agent CLIs at an // undetected prompt stay actionable via the caller's existing idle rules — // canonical keeps them "idle" (calm) because there is no deterministic ask. if (args.runtimeState === "idle") { - return chat ? { phase: "ready", badge: null } : { phase: "idle", badge: null }; + return restingPhaseWithBackgroundWork( + chat + ? { phase: "ready", badge: null, liveness: null } + : { phase: "idle", badge: null, liveness: null }, + ); } - return { phase: "running", badge: null }; + return { phase: "running", badge: null, liveness: "turn" }; } /** diff --git a/apps/desktop/src/shared/sessionStatusPresentation.ts b/apps/desktop/src/shared/sessionStatusPresentation.ts index a9f386bab..87fa782bb 100644 --- a/apps/desktop/src/shared/sessionStatusPresentation.ts +++ b/apps/desktop/src/shared/sessionStatusPresentation.ts @@ -1,4 +1,9 @@ -import type { CanonicalSessionPhase } from "./sessionCanonicalState"; +import type { + CanonicalSessionPhase, + SessionBackgroundWork, + SessionLiveness, +} from "./sessionCanonicalState"; +import { totalBackgroundWork } from "./sessionCanonicalState"; /** * The ONE presentation vocabulary for "how does a session's state look and @@ -50,6 +55,7 @@ export type SessionStatusTone = "blue" | "violet" | "amber" | "emerald" | "red" */ export type SessionStatusGlyph = | "working" + | "monitoring" | "planning" | "waiting" | "needs-you" @@ -129,11 +135,21 @@ export type SessionStatusOverlay = { export type SessionStatusActivityContext = { chatActivityMode?: "planning" | null; - activeBackgroundTaskCount?: number; + /** + * Why the session is running, from `canonicalSessionState`. Absent (or + * `"turn"`) means a live foreground turn and the plain "Working" copy. + */ + liveness?: SessionLiveness | null; + /** Live background work, for the "×N" suffix. */ + backgroundWork?: SessionBackgroundWork | null; nextWakeAt?: string | null; nowMs?: number; }; +function countSuffix(count: number): string { + return count > 1 ? ` ×${count}` : ""; +} + export function sessionStatusPresentation( phase: CanonicalSessionPhase, overlay: SessionStatusOverlay = {}, @@ -159,7 +175,11 @@ export function sessionStatusPresentation( return { label: "Woke", tone: "amber", glyph: "woke", showsElapsed: false, prominent: true }; } - if (phase === "running" && activity.chatActivityMode === "planning") { + // Planning is a property of a LIVE TURN. A resting session promoted back to + // `running` by its background work is not planning anything — its plan-mode + // flag is just the mode the finished turn ran in. + const liveness = activity.liveness ?? "turn"; + if (phase === "running" && liveness === "turn" && activity.chatActivityMode === "planning") { return { label: "Planning", tone: "violet", @@ -180,10 +200,23 @@ export function sessionStatusPresentation( // not in the session summary. It is a proxy: a job launched early in a long // turn reads ~0s at turn end. `showsElapsed` also re-enables the breathing // animation, which is intended — background work genuinely is a live state. - const backgroundJobCount = activity.activeBackgroundTaskCount ?? 0; - if ((phase === "ready" || phase === "idle") && backgroundJobCount > 0) { + if (phase === "running" && liveness !== "turn") { + const work = activity.backgroundWork; + // "Monitoring" earns its own word because it answers a different question. + // Background work might finish on its own; a watch loop will not, so the + // row is telling you it is safe to walk away — and, once the CI run it is + // watching lands, that it is yours to close. + if (liveness === "monitoring") { + return { + label: `Monitoring${countSuffix(work?.monitoringCount ?? 0)}`, + tone: "blue", + glyph: "monitoring", + showsElapsed: true, + prominent: false, + }; + } return { - label: backgroundJobCount > 1 ? `Background work ×${backgroundJobCount}` : "Background work", + label: `Background work${countSuffix(totalBackgroundWork(work))}`, tone: "blue", glyph: "working", showsElapsed: true, diff --git a/apps/desktop/src/shared/types/chat.ts b/apps/desktop/src/shared/types/chat.ts index db15915a0..e396360be 100644 --- a/apps/desktop/src/shared/types/chat.ts +++ b/apps/desktop/src/shared/types/chat.ts @@ -9,6 +9,7 @@ import type { FileDiff } from "./git"; import type { LaneLinearIssue, SessionLinearIssueLink } from "./lanes"; import type { OrchestrationContextItem, OrchestrationRole } from "./orchestration"; import type { AdeRecoveryErrorCode } from "./recovery"; +import type { SessionBackgroundWork } from "../sessionCanonicalState"; import type { SubagentCapability } from "../subagentCapabilities"; export type AgentChatProvider = "codex" | "claude" | "cursor" | "droid" | "opencode" | "pi" | (string & {}); @@ -1602,6 +1603,8 @@ export type AgentChatSessionSummary = { nextWakeAt: string | null; /** Authoritative provider-reported background tasks still running after the foreground turn. */ activeBackgroundTaskCount?: number; + /** The same live work split into working vs monitoring (`classifyBackgroundWorkKind`). */ + backgroundWork?: SessionBackgroundWork; /** True when this chat's durable schedules are paused. */ scheduledWorkPaused?: boolean; /** KV-backed durable schedules. This is the management source of truth. */ diff --git a/apps/desktop/src/shared/types/sessions.ts b/apps/desktop/src/shared/types/sessions.ts index 37ebccc96..b8aed2277 100644 --- a/apps/desktop/src/shared/types/sessions.ts +++ b/apps/desktop/src/shared/types/sessions.ts @@ -13,6 +13,7 @@ import type { } from "./chat"; import type { LaneLinearIssue } from "./lanes"; import type { OrchestrationRole } from "./orchestration"; +import type { SessionBackgroundWork } from "../sessionCanonicalState"; export type TerminalSessionStatus = "running" | "completed" | "failed" | "disposed" | "detached"; @@ -253,6 +254,12 @@ export type TerminalSessionSummary = { chatActivityMode?: "planning" | null; /** Authoritative provider-reported background tasks still running after the foreground turn. */ activeBackgroundTaskCount?: number; + /** + * The same live work split into working vs monitoring + * (`classifyBackgroundWorkKind`). `activeBackgroundTaskCount` stays the total + * so the mobile roster and push publisher keep reading one number. + */ + backgroundWork?: SessionBackgroundWork; /** First tag mirrored from the backing Claude SDK session pointer. */ claudeTag?: string | null; /** Owner session id for attached terminals, historically a parent chat id and now also a tracked CLI session id. */ From 689a6adea9c01fbb136162d5ceace1bd03a57901 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 9 Aug 2026 20:34:33 -0400 Subject: [PATCH 02/14] feat(sessions): settle stops the machinery it claims to conclude MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settle was a pure column write. The row went quiet and everything the session had started kept going — background shells held ports, subagent fleets kept spending tokens, and scheduled work woke the thread hours after the user had declared it done. Archive had the mirror problem: it released the lane's port lease and proxy route while the lane's processes were still bound to those ports, and an archived lane is filtered out of every surface that could have shown the user what to stop. Settle now runs a shared teardown (sessionMachineryTeardown.ts) before the lifecycle write, so a settle can never report success while its monitors are still armed: - pauses the session's scheduled work — pauses, not cancels, so an unsettle brings hand-made schedules back rather than having silently deleted them, - calls the new agentChatService.stopBackgroundWork, which stops every live child BEFORE the parent (stopping only the parent leaves the fleet running and untracked, which is how a "stopped" agent keeps spending), - keeps TERMINAL PANES OPEN. An agent's background shell is thread background work; a pane the user opened is theirs, and closing it on settle would destroy scrollback nobody asked to lose, - leaves an ACTIVE foreground turn alone — its subagents are work the user can see happening, and the row un-settles on its own activity anyway, - is best-effort throughout: a provider that cannot be reached delays nothing and blocks nothing. Wired into every settle entry point: the single/bulk ADE actions, the sessions.settle / settleMany IPC handlers, the session.settle* sync commands, and PR-merge auto-settle — which bypasses settlement blockers and is therefore the path most likely to file a session that is still running something. It composes with that service's session targeting rather than replacing it. laneService.archive is now async and stops the lane's chats, PTYs, watchers and auto-rebase through a shared stopLaneRuntimeWork before the status write, so the port lease its callers release immediately afterwards is released after the processes are gone. archiveAndReclaim uses the same helper; delete keeps its runStep version because the delete dialog reports each step. What escapes is documented rather than pretended away: processes an agent detached with nohup/setsid/disown leave ADE's tree entirely, and Codex background subagents are reported but expose no stop control. No new kill logic is introduced — teardown delegates to ptyService/agentChatService disposal, which already route through the Windows-correct tree-kill helpers. Co-Authored-By: Claude Opus 5 --- apps/ade-cli/src/bootstrap.ts | 2 + .../src/services/sync/rosterBuilder.ts | 18 ++- .../services/sync/syncRemoteCommandService.ts | 10 ++ apps/desktop/src/main/main.ts | 2 + .../src/main/services/adeActions/registry.ts | 14 +- .../main/services/chat/agentChatService.ts | 100 ++++++++++++ .../src/main/services/ipc/registerIpc.ts | 13 +- .../main/services/lanes/laneService.test.ts | 51 ++++++ .../src/main/services/lanes/laneService.ts | 68 +++++++- .../src/main/services/prs/prAsync.test.ts | 55 +++++++ .../prs/prMergeAutoSettlementService.ts | 25 +++ .../src/main/services/prs/prService.ts | 2 +- .../sessions/sessionMachineryTeardown.test.ts | 146 ++++++++++++++++++ .../sessions/sessionMachineryTeardown.ts | 146 ++++++++++++++++++ .../sessions/settleTerminalSession.ts | 19 +++ .../storage/storageInsightsService.test.ts | 8 +- .../storage/storageInsightsService.ts | 6 +- docs/features/lanes/README.md | 18 ++- .../features/terminals-and-sessions/README.md | 50 +++++- 19 files changed, 719 insertions(+), 34 deletions(-) create mode 100644 apps/desktop/src/main/services/sessions/sessionMachineryTeardown.test.ts create mode 100644 apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts diff --git a/apps/ade-cli/src/bootstrap.ts b/apps/ade-cli/src/bootstrap.ts index d09b68a91..723e9218b 100644 --- a/apps/ade-cli/src/bootstrap.ts +++ b/apps/ade-cli/src/bootstrap.ts @@ -1434,6 +1434,8 @@ export async function createAdeRuntime(args: { const prMergeAutoSettlementService = createPrMergeAutoSettlementService({ db, sessionService, + agentChatService, + logger, emitEvent: emitPrEvent, }); diff --git a/apps/ade-cli/src/services/sync/rosterBuilder.ts b/apps/ade-cli/src/services/sync/rosterBuilder.ts index dbff00661..f8b23425d 100644 --- a/apps/ade-cli/src/services/sync/rosterBuilder.ts +++ b/apps/ade-cli/src/services/sync/rosterBuilder.ts @@ -366,12 +366,18 @@ function diskChatStatus(row: TerminalSessionRow, sidecarAwaiting: boolean): Sync function liveChatStatus(live: RosterLiveSession): SyncRosterChatStatus { if (live.awaitingInput) return "awaiting"; if (live.status === "active") return "running"; - // Claude's background subagents keep working after the foreground turn ends, - // and agentChatService reports the chat `idle` for the whole of it. The - // desktop sidebar overrides that to Working - // (`sessionStatusPresentation.ts` — `activeBackgroundTaskCount > 0`); the - // roster has to agree, or Activity maps the session idle → stale → Done and - // reports a live agent as finished. + // Background work keeps running after the foreground turn ends, and + // agentChatService reports the chat `idle` for the whole of it. Desktop + // promotes that back to the `running` phase + // (`sessionCanonicalState.ts` — `backgroundWork`); the roster has to agree, + // or Activity maps the session idle → stale → Done and reports a live agent + // as finished. + // + // The count is cross-runtime, not Claude-only: it now also covers Codex + // background subagents and Cursor cloud runs, so this branch fires for them + // too without any change here. The phone deliberately reads the TOTAL rather + // than the working/monitoring split — a roster row has one status, and + // "something is still running" is the fact it needs. if ((live.activeBackgroundTaskCount ?? 0) > 0) return "running"; if (live.status === "idle") return "idle"; return "ended"; diff --git a/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts b/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts index c27fd6d5c..5ec1d4418 100644 --- a/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts +++ b/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts @@ -278,6 +278,7 @@ import type { ProductAnalyticsService } from "../../../../desktop/src/main/servi import { parseProductAnalyticsCapture } from "../../../../desktop/src/shared/types/productAnalytics"; import { deleteTerminalSessionWithRuntimeCleanup } from "../../../../desktop/src/main/services/sessions/deleteTerminalSession"; import { dismissPendingInputBeforeSettle, settleTerminalSession } from "../../../../desktop/src/main/services/sessions/settleTerminalSession"; +import { stopSettledSessionMachinery } from "../../../../desktop/src/main/services/sessions/sessionMachineryTeardown"; import type { createSessionDeltaService } from "../../../../desktop/src/main/services/sessions/sessionDeltaService"; import type { createSessionService } from "../../../../desktop/src/main/services/sessions/sessionService"; import { getSharedModelPickerStore, type ModelPickerStore } from "../modelPickerStore"; @@ -4092,6 +4093,7 @@ function registerWorkRemoteCommands({ args, register }: RemoteCommandRegistratio sessionService: args.sessionService, agentChatService: args.agentChatService ?? null, ptyService: args.ptyService, + logger: args.logger, }); if (!settled) throw new Error(`Session '${sessionId}' was not found.`); return { ok: true, sessionId }; @@ -4129,6 +4131,14 @@ function registerWorkRemoteCommands({ args, register }: RemoteCommandRegistratio ptyService: args.ptyService, }); } + await stopSettledSessionMachinery( + { + sessionService: args.sessionService, + agentChatService: args.agentChatService ?? null, + logger: args.logger, + }, + sessionIds, + ); return args.sessionService.settleSessions(sessionIds); }); register("session.unsettleSessions", { viewerAllowed: true, queueable: true }, async (payload) => { diff --git a/apps/desktop/src/main/main.ts b/apps/desktop/src/main/main.ts index 48f9917b9..f07806c2c 100644 --- a/apps/desktop/src/main/main.ts +++ b/apps/desktop/src/main/main.ts @@ -3588,6 +3588,8 @@ app.whenReady().then(async () => { prMergeAutoSettlementServiceRef = createPrMergeAutoSettlementService({ db, sessionService, + agentChatService, + logger, emitEvent: emitPrEvent, }); laneTeardownDeps.agentChatService = { diff --git a/apps/desktop/src/main/services/adeActions/registry.ts b/apps/desktop/src/main/services/adeActions/registry.ts index 72d087812..f70aa4729 100644 --- a/apps/desktop/src/main/services/adeActions/registry.ts +++ b/apps/desktop/src/main/services/adeActions/registry.ts @@ -126,6 +126,7 @@ import { parseWakeReason, } from "../sessions/sessionRequestValidation"; import { settleTerminalSession } from "../sessions/settleTerminalSession"; +import { stopSettledSessionMachinery } from "../sessions/sessionMachineryTeardown"; import { getSessionLifecycleSettings, setSessionLifecycleSettings, @@ -2126,6 +2127,7 @@ function buildSessionDomainService(runtime: AdeRuntime): OpaqueService | null { sessionService, agentChatService: runtime.agentChatService, ptyService: runtime.ptyService, + logger: runtime.logger, })) { throw new Error(`Session '${sessionId}' was not found.`); } @@ -2146,7 +2148,7 @@ function buildSessionDomainService(runtime: AdeRuntime): OpaqueService | null { }, // Bulk settle/unsettle for renderer surfaces on remote-bound projects // (mirrors deleteSession's generic trust posture). - settleSessions: (args?: unknown) => { + settleSessions: async (args?: unknown) => { const record = readObjectActionArg(args, "session.settleSessions"); const sessionIds = Array.isArray(record.sessionIds) ? record.sessionIds.filter((id): id is string => typeof id === "string") @@ -2168,6 +2170,14 @@ function buildSessionDomainService(runtime: AdeRuntime): OpaqueService | null { "session.settleSessions does not dismiss pending input; use session.settleSession for a single session.", ); } + await stopSettledSessionMachinery( + { + sessionService, + agentChatService: runtime.agentChatService, + logger: runtime.logger, + }, + sessionIds, + ); return sessionService.settleSessions(sessionIds); }, unsettleSessions: (args?: unknown) => { @@ -2506,7 +2516,7 @@ function buildLaneDomainService(runtime: AdeRuntime): OpaqueService { archive: async (args?: { laneId?: string }): Promise => { const laneId = requireNonEmptyString(args?.laneId, "laneId"); const lane = await findLaneForArchive(laneId); - runtime.laneService.archive({ laneId }); + await runtime.laneService.archive({ laneId }); try { releaseLaneRuntimeResources(runtime, laneId); } finally { diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index 5592c217c..4d66433ed 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -39269,6 +39269,105 @@ export function createAgentChatService(args: { await scheduledWorkScheduler?.refreshGlobalPause(); }; + /** + * Stop the background work a session still owns, WITHOUT closing the session + * or interrupting a turn the user is watching. + * + * This is the runtime half of settle teardown. Settle used to be a pure + * column write: the row went quiet and every monitor, background shell and + * subagent it had spawned kept running — burning tokens, holding ports, and + * (via scheduled work) waking the thread hours later. + * + * Two rules shape what it touches: + * + * • An ACTIVE foreground turn is left alone. Its subagents belong to work + * the user can see happening, and killing them because the row was filed + * would be worse than the leak. A settled session that is still streaming + * un-settles on its own activity anyway. + * • Children stop before parents. Stopping only the parent leaves the fleet + * running and untracked, which is how a "stopped" agent keeps spending. + * + * Returns how much live work was found, so callers can report honestly rather + * than claiming a teardown that did nothing. + */ + const stopBackgroundWork = async ( + { sessionId }: { sessionId: string }, + ): Promise<{ stopped: number; skippedActiveTurn: boolean }> => { + const managed = managedSessions.get(sessionId.trim()); + if (!managed || managed.closed || managed.deleted) return { stopped: 0, skippedActiveTurn: false }; + const runtime = managed.runtime; + if (!runtime) return { stopped: 0, skippedActiveTurn: false }; + + const turnActive = managed.session.status === "active" || Boolean(runtime.activeTurnId); + const stopped = totalBackgroundWork(runtimeBackgroundWork(runtime)); + if (turnActive) return { stopped: 0, skippedActiveTurn: true }; + if (stopped === 0) return { stopped: 0, skippedActiveTurn: false }; + + try { + switch (runtime.kind) { + case "claude": { + // Children first: this drains workflow agents, then subagents, then + // the background shells each of them owns. + await stopActiveClaudeSubagents( + managed, + runtime, + runtime.activeTurnId ?? undefined, + "Stopped when the session was settled", + ); + // Anything still on the authoritative level had no `activeSubagents` + // entry to be reached through — a plain backgrounded shell, usually. + // Those are exactly the ones that survived the old teardown. + const control = getClaudeQueryControl(runtime.query); + for (const taskId of [...runtime.liveBackgroundTaskIds]) { + if (typeof control.stopTask === "function") { + try { + await awaitClaudeControlCall( + `Stopping Claude background task '${taskId}'`, + CLAUDE_STOP_TASK_TIMEOUT_MS, + () => control.stopTask!(taskId), + ); + } catch (error) { + logger.warn("agent_chat.settle_background_stop_failed", { + sessionId: managed.session.id, + taskId, + error: error instanceof Error ? error.message : String(error), + }); + } + } + emitClaudeBackgroundTaskUpdate(managed, runtime, { taskId, status: "stopped" }); + } + break; + } + case "cursor": { + const agentId = managed.session.cursorCloudAgentId; + if (!agentId) break; + for (const runId of [...runtime.cloudRuns.keys()]) { + await cancelCursorCloudRun({ agentId, runId }).catch((error: unknown) => { + logger.warn("agent_chat.settle_cloud_cancel_failed", { + sessionId: managed.session.id, + runId, + error: error instanceof Error ? error.message : String(error), + }); + }); + } + break; + } + default: + // Codex reports background subagents but exposes no per-subagent stop + // control, and opencode/droid/pi report no background work at all. + // Clearing ADE's tracking without actually stopping anything would + // make the row lie, so this is deliberately a no-op for them. + break; + } + } catch (error) { + logger.warn("agent_chat.settle_background_teardown_failed", { + sessionId: managed.session.id, + error: error instanceof Error ? error.message : String(error), + }); + } + return { stopped, skippedActiveTurn: false }; + }; + const hasActiveWorkloads = (): boolean => { for (const managed of managedSessions.values()) { if (managed.closed || managed.deleted) continue; @@ -44150,6 +44249,7 @@ export function createAgentChatService(args: { getSessionSummary, ensureSessionSurface, hasActiveWorkloads, + stopBackgroundWork, hasRetainableSessions, countActiveForLane, disposeForLane, diff --git a/apps/desktop/src/main/services/ipc/registerIpc.ts b/apps/desktop/src/main/services/ipc/registerIpc.ts index e94945099..fc537a273 100644 --- a/apps/desktop/src/main/services/ipc/registerIpc.ts +++ b/apps/desktop/src/main/services/ipc/registerIpc.ts @@ -51,6 +51,7 @@ import { parseWakeReason, } from "../sessions/sessionRequestValidation"; import { settleTerminalSession } from "../sessions/settleTerminalSession"; +import { stopSettledSessionMachinery } from "../sessions/sessionMachineryTeardown"; import { getSessionLifecycleSettings, setSessionLifecycleSettings, @@ -6030,7 +6031,7 @@ export function registerIpc({ .list({ includeArchived: true, includeStatus: false }) .then((lanes) => lanes.find((entry) => entry.id === arg.laneId) ?? null) .catch(() => null); - ctx.laneService.archive(arg); + await ctx.laneService.archive(arg); try { releaseLaneRuntimeResources(ctx, arg.laneId); } finally { @@ -6966,6 +6967,7 @@ export function registerIpc({ sessionService: ctx.sessionService, agentChatService: ctx.agentChatService, ptyService: ctx.ptyService, + logger: ctx.logger, }); if (!settled) throw new Error(`Session '${sessionId}' was not found.`); }, @@ -6986,9 +6988,14 @@ export function registerIpc({ async (_event, arg: { sessionIds?: unknown }): Promise => { const ctx = ensureSessionContext(); if (!Array.isArray(arg?.sessionIds)) throw new Error("Session ids are required."); - return ctx.sessionService.settleSessions( - arg.sessionIds.filter((sessionId): sessionId is string => typeof sessionId === "string"), + const sessionIds = arg.sessionIds.filter( + (sessionId): sessionId is string => typeof sessionId === "string", + ); + await stopSettledSessionMachinery( + { sessionService: ctx.sessionService, agentChatService: ctx.agentChatService, logger: ctx.logger }, + sessionIds, ); + return ctx.sessionService.settleSessions(sessionIds); }, ); diff --git a/apps/desktop/src/main/services/lanes/laneService.test.ts b/apps/desktop/src/main/services/lanes/laneService.test.ts index e2ca65999..e71a58b72 100644 --- a/apps/desktop/src/main/services/lanes/laneService.test.ts +++ b/apps/desktop/src/main/services/lanes/laneService.test.ts @@ -4371,6 +4371,57 @@ describe("laneService delete teardown + cancellation + streaming", () => { return { db, service, repoRoot, worktreesDir, childPath }; } + it("stops the lane's runtime work before archiving it", async () => { + // Archive used to be a bare status write. Every caller releases the lane's + // port lease and proxy route the moment it returns, so the lane's dev + // servers and agents were still bound to those ports when the lease was + // handed back — and, filtered out of every surface, went on holding them + // indefinitely. + const events: any[] = []; + const fake = makeFakeServices(); + const { db, service } = await setupWithLane({ teardown: fake, events }); + + await service.archive({ laneId: "lane-child" }); + + expect(fake.calls).toContain("stop_chats"); + expect(fake.calls).toContain("stop_ptys"); + expect(fake.calls).toContain("stop_watchers"); + expect( + db.get<{ status: string }>("select status from lanes where id = ?", ["lane-child"])?.status, + ).toBe("archived"); + }); + + it("still archives when one teardown step throws, after attempting it", async () => { + // Each step is best-effort on purpose — a lane must not become + // un-archivable because one watcher refuses to stop. What must hold is + // that every stop is ATTEMPTED before the status write, and that a failure + // is logged rather than swallowed silently. + const events: any[] = []; + const fake = makeFakeServices(); + fake.ptyService.disposeForLane.mockImplementation(() => { + throw new Error("pty teardown exploded"); + }); + const { db, service } = await setupWithLane({ teardown: fake, events }); + + // Individual steps are best-effort, so the archive still completes — what + // must hold is that the stop was ATTEMPTED before the status write. + await service.archive({ laneId: "lane-child" }); + expect(fake.ptyService.disposeForLane).toHaveBeenCalledWith("lane-child"); + expect( + db.get<{ status: string }>("select status from lanes where id = ?", ["lane-child"])?.status, + ).toBe("archived"); + }); + + it("skips teardown for an already-archived lane", async () => { + const events: any[] = []; + const fake = makeFakeServices(); + const { service } = await setupWithLane({ teardown: fake, events }); + await service.archive({ laneId: "lane-child" }); + const callsAfterFirst = [...fake.calls]; + await service.archive({ laneId: "lane-child" }); + expect(fake.calls).toEqual(callsAfterFirst); + }); + it("transfers shared proof ownership so the final owning lane can delete it", async () => { const events: any[] = []; const fake = makeFakeServices(); diff --git a/apps/desktop/src/main/services/lanes/laneService.ts b/apps/desktop/src/main/services/lanes/laneService.ts index d15efc898..2ab793df8 100644 --- a/apps/desktop/src/main/services/lanes/laneService.ts +++ b/apps/desktop/src/main/services/lanes/laneService.ts @@ -3814,6 +3814,44 @@ export function createLaneService({ throw args.cause instanceof Error ? args.cause : new Error(originalMessage); } + /** + * Stop everything a lane is still running: chat sessions, PTY sessions, file + * watchers, and the rebase machinery watching it. + * + * Shared by `archive` and `archiveAndReclaim` so a step added for one cannot + * be forgotten by the other. `delete` runs the same steps through its own + * `runStep` progress reporting (the delete dialog shows each one), so it + * stays separate on purpose — keep the two in sync when adding a step. + * + * Best-effort per step: a lane must still archive when one watcher refuses + * to stop, and the caller's port-lease release is what actually needs the + * processes gone. + */ + const stopLaneRuntimeWork = async (laneId: string): Promise => { + const warn = (step: string, error: unknown): void => { + logger.warn("lane_runtime_teardown.step_failed", { + laneId, + step, + error: error instanceof Error ? error.message : String(error), + }); + }; + try { + teardownDeps?.autoRebaseService?.cancelForLane(laneId); + } catch (error) { warn("cancel_auto_rebase", error); } + try { + await teardownDeps?.rebaseSuggestionService?.dismiss({ laneId }); + } catch (error) { warn("dismiss_rebase_suggestion", error); } + try { + await teardownDeps?.agentChatService?.disposeForLane(laneId); + } catch (error) { warn("stop_chats", error); } + try { + teardownDeps?.ptyService?.disposeForLane(laneId); + } catch (error) { warn("stop_ptys", error); } + try { + teardownDeps?.fileWatcherService?.stopAllForWorkspace(laneId); + } catch (error) { warn("stop_watchers", error); } + }; + // Named so a few methods (branch-drift resolution) can delegate to sibling // methods instead of duplicating their transaction/rollback handling. const laneServiceApi = { @@ -6268,7 +6306,7 @@ export function createLaneService({ if (await hasSymlinkInManagedPath(packAdeDir, lanePackDir)) { throw new Error("ADE will not reclaim generated data through a symbolic link."); } - laneServiceApi.archive({ laneId: args.laneId }); + await laneServiceApi.archive({ laneId: args.laneId }); runtimeOpts?.onArchived?.(); db.run( `insert into local_lane_storage_state( @@ -6282,11 +6320,7 @@ export function createLaneService({ updated_at = excluded.updated_at`, [args.laneId, projectId, row.worktree_path, risk.reclaimableBytes, now], ); - teardownDeps?.autoRebaseService?.cancelForLane(args.laneId); - await teardownDeps?.rebaseSuggestionService?.dismiss({ laneId: args.laneId }); - await teardownDeps?.agentChatService?.disposeForLane(args.laneId); - teardownDeps?.ptyService?.disposeForLane(args.laneId); - teardownDeps?.fileWatcherService?.stopAllForWorkspace(args.laneId); + await stopLaneRuntimeWork(args.laneId); if (runtimeOpts?.teardownEnv) { try { await runtimeOpts.teardownEnv(); @@ -6417,7 +6451,23 @@ export function createLaneService({ } }, - archive({ laneId }: { laneId: string }): void { + /** + * Archive a lane, stopping its runtime work first. + * + * The stop is not cosmetic and its ORDER is the point. Every caller + * releases the lane's port lease and proxy route immediately after this + * resolves (`releaseLaneRuntimeResources`). Archive used to be a bare + * status write, so those resources were handed back while the lane's dev + * servers and agent processes were still bound to the ports — the lease + * could then be reassigned to another lane that could not bind, and the + * abandoned processes went on holding memory and ports indefinitely + * because the archived lane is filtered out of every surface that could + * have shown them to the user. + * + * Async for that reason, and the same teardown steps `delete` and + * `archiveAndReclaim` already run — one list, not three. + */ + async archive({ laneId }: { laneId: string }): Promise { const row = getLaneRow(laneId); if (!row) throw new Error(`Lane not found: ${laneId}`); if (row.lane_type === "primary") { @@ -6437,6 +6487,10 @@ export function createLaneService({ throw new Error("Cannot archive a lane that is part of a PR group. Remove from the group first."); } + // Before the status write, so a teardown failure leaves the lane visible + // and still owned rather than hidden with live processes behind it. + await stopLaneRuntimeWork(laneId); + const now = new Date().toISOString(); db.run("update lanes set status = 'archived', archived_at = ? where id = ? and project_id = ?", [now, laneId, projectId]); invalidateLanePathCaches(); diff --git a/apps/desktop/src/main/services/prs/prAsync.test.ts b/apps/desktop/src/main/services/prs/prAsync.test.ts index b251299b9..f0fe8bb23 100644 --- a/apps/desktop/src/main/services/prs/prAsync.test.ts +++ b/apps/desktop/src/main/services/prs/prAsync.test.ts @@ -864,6 +864,7 @@ describe("prMergeAutoSettlementService", () => { settledAt: settledSessionIds.has("raw-shell") ? "2026-03-24T12:01:05.000Z" : null, }, ]), + get: vi.fn(() => null), settleSessionsWithOutcome, }) as any, emitEvent, @@ -911,6 +912,55 @@ describe("prMergeAutoSettlementService", () => { expect(settleSessionsWithOutcome).toHaveBeenCalledTimes(2); }); + it("stops the merged session's machinery before filing it", async () => { + // This path deliberately bypasses the settlement blockers, so it is the + // one most likely to file a session that is still running something. Before + // teardown reached it, the merged lane's monitors kept polling and woke the + // thread hours after the PR had landed. + const db = createMemoryDb(); + const order: string[] = []; + const settleSessionsWithOutcome = vi.fn((ids: string[]) => { + order.push("settle"); + return ids; + }); + const stopBackgroundWork = vi.fn(async () => { + order.push("stop"); + return { stopped: 1, skippedActiveTurn: false }; + }); + const setScheduledWorkPaused = vi.fn(async ({ sessionId }: { sessionId: string }) => { + order.push("pause"); + return { sessionId, paused: true, nextWakeAt: null }; + }); + const rows = [{ id: "chat-live", toolType: "claude-chat", archivedAt: null, settledAt: null }]; + const service = createPrMergeAutoSettlementService({ + db: db as any, + sessionService: { + list: vi.fn(() => rows), + get: vi.fn((id: string) => rows.find((row) => row.id === id) ?? null), + settleSessionsWithOutcome, + } as any, + agentChatService: { + stopBackgroundWork, + setScheduledWorkPaused, + listScheduledWork: vi.fn(async () => [{ id: "sched-1", status: "scheduled" }]), + } as any, + emitEvent: vi.fn(), + }); + + await service.processSnapshot({ + prs: [createSummary({ state: "open" })], + polledAt: "2026-03-24T12:00:00.000Z", + }); + await service.processSnapshot({ + prs: [createSummary({ state: "merged", mergedAt: "2026-03-24T12:01:00.000Z" })], + polledAt: "2026-03-24T12:01:30.000Z", + }); + + expect(stopBackgroundWork).toHaveBeenCalledWith({ sessionId: "chat-live" }); + expect(setScheduledWorkPaused).toHaveBeenCalledWith({ sessionId: "chat-live", paused: true }); + expect(order).toEqual(["pause", "stop", "settle"]); + }); + it("does not re-settle after reactivation, but settles for a later PR", async () => { const db = createMemoryDb(); let settled = false; @@ -928,6 +978,7 @@ describe("prMergeAutoSettlementService", () => { archivedAt: null, settledAt: settled ? "2026-03-24T12:01:05.000Z" : null, }]), + get: vi.fn(() => null), settleSessionsWithOutcome, }) as any, emitEvent: vi.fn(), @@ -1009,6 +1060,7 @@ describe("prMergeAutoSettlementService", () => { archivedAt: null, settledAt: null, }]), + get: vi.fn(() => null), settleSessionsWithOutcome, }) as any, emitEvent: vi.fn(), @@ -1081,6 +1133,7 @@ describe("prMergeAutoSettlementService", () => { { laneId: "lane-1", id: "chat-owned", toolType: "codex-chat", archivedAt: null, settledAt: null }, { laneId: "lane-1", id: "chat-other", toolType: "codex-chat", archivedAt: null, settledAt: null }, ]), + get: vi.fn(() => null), settleSessionsWithOutcome, }) as any, emitEvent: vi.fn(), @@ -1124,6 +1177,7 @@ describe("prMergeAutoSettlementService", () => { archivedAt: null, settledAt: null, }]), + get: vi.fn(() => null), settleSessionsWithOutcome, }) as any, emitEvent, @@ -1187,6 +1241,7 @@ describe("prMergeAutoSettlementService", () => { archivedAt: null, settledAt: null, }]), + get: vi.fn(() => null), settleSessionsWithOutcome, }) as any, emitEvent: vi.fn(), diff --git a/apps/desktop/src/main/services/prs/prMergeAutoSettlementService.ts b/apps/desktop/src/main/services/prs/prMergeAutoSettlementService.ts index cd6ebf8c3..da5eb9355 100644 --- a/apps/desktop/src/main/services/prs/prMergeAutoSettlementService.ts +++ b/apps/desktop/src/main/services/prs/prMergeAutoSettlementService.ts @@ -11,6 +11,10 @@ import { isTrackedAgentCliToolType, } from "../../../shared/types"; import { isChatToolType } from "../sessions/chatSessionProjection"; +import { + stopSettledSessionMachinery, + type SessionMachineryTeardownDeps, +} from "../sessions/sessionMachineryTeardown"; function isMergeAtOrAfter(mergedAt: string | null | undefined, enabledSince: string): boolean { const mergedMs = Date.parse(mergedAt ?? ""); @@ -63,6 +67,13 @@ function resolveMergeSettlementScope(pr: PrSummary, snapshot: PrSummary[]): Merg export function createPrMergeAutoSettlementService(args: { db: Pick; sessionService: Pick, "get" | "list" | "settleSessionsWithOutcome">; + /** + * Optional so the service stays constructible in tests and headless hosts + * without a chat runtime. Absent, the settle still files the row — it just + * cannot stop what the row owns, which is the pre-teardown behaviour. + */ + agentChatService?: SessionMachineryTeardownDeps["agentChatService"]; + logger?: SessionMachineryTeardownDeps["logger"]; emitEvent: (event: PrEventPayload) => void; }) { /** @@ -185,6 +196,20 @@ export function createPrMergeAutoSettlementService(args: { // session even when it still owns scheduled work, a background task, // or another normal settlement blocker. Real activity can unsettle it // again, while handledPrIds prevents this PR from filing it twice. + // + // Because this path deliberately bypasses the settlement blockers, it + // is the one most likely to file a session that IS still running + // something — which is exactly why the teardown has to run here too. + // Without it, the merged lane's monitors kept polling and woke the + // thread hours after the PR landed. + await stopSettledSessionMachinery( + { + sessionService: args.sessionService, + agentChatService: args.agentChatService ?? null, + logger: args.logger ?? null, + }, + [session.id], + ); settledSessionIds.push(...args.sessionService.settleSessionsWithOutcome( [session.id], `PR #${pr.githubPrNumber} merged`, diff --git a/apps/desktop/src/main/services/prs/prService.ts b/apps/desktop/src/main/services/prs/prService.ts index 4268c1167..1374ae34f 100644 --- a/apps/desktop/src/main/services/prs/prService.ts +++ b/apps/desktop/src/main/services/prs/prService.ts @@ -10443,7 +10443,7 @@ export function createPrService({ continue; } try { - laneService.archive({ laneId }); + await laneService.archive({ laneId }); archivedLaneIds.push(laneId); } catch { skippedLaneIds.push(laneId); diff --git a/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.test.ts b/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.test.ts new file mode 100644 index 000000000..3e0793965 --- /dev/null +++ b/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.test.ts @@ -0,0 +1,146 @@ +import { describe, expect, it, vi } from "vitest"; +import { stopSettledSessionMachinery } from "./sessionMachineryTeardown"; +import { settleTerminalSession } from "./settleTerminalSession"; + +type Row = { id: string; toolType: string }; + +function deps(rows: Row[], overrides: Record = {}) { + const stopBackgroundWork = vi.fn(async () => ({ stopped: 2, skippedActiveTurn: false })); + const setScheduledWorkPaused = vi.fn(async ({ sessionId }: { sessionId: string }) => ({ + sessionId, + paused: true, + nextWakeAt: null, + })); + const listScheduledWork = vi.fn(async () => [{ id: "sched-1", status: "scheduled" }]); + return { + sessionService: { + get: (id: string) => rows.find((row) => row.id === id) ?? null, + } as never, + agentChatService: { + stopBackgroundWork, + setScheduledWorkPaused, + listScheduledWork, + ...overrides, + } as never, + logger: { warn: vi.fn() }, + stopBackgroundWork, + setScheduledWorkPaused, + listScheduledWork, + }; +} + +describe("stopSettledSessionMachinery", () => { + it("stops background work and pauses scheduled work for a chat session", async () => { + // Settle used to be a pure column write: the row went quiet while its + // monitors kept polling and its background shells kept holding ports. + const d = deps([{ id: "chat-1", toolType: "claude-chat" }]); + const result = await stopSettledSessionMachinery(d, ["chat-1"]); + + expect(d.stopBackgroundWork).toHaveBeenCalledWith({ sessionId: "chat-1" }); + expect(d.setScheduledWorkPaused).toHaveBeenCalledWith({ sessionId: "chat-1", paused: true }); + expect(result).toMatchObject({ + sessionIds: ["chat-1"], + stoppedBackgroundWork: 2, + pausedScheduledWork: 1, + skippedActiveTurns: 0, + }); + }); + + it("pauses rather than cancels, so an unsettle can bring the schedules back", async () => { + const d = deps([{ id: "chat-1", toolType: "claude-chat" }], { + cancelScheduledWork: vi.fn(), + }); + await stopSettledSessionMachinery(d, ["chat-1"]); + expect(d.setScheduledWorkPaused).toHaveBeenCalledTimes(1); + expect((d.agentChatService as unknown as { cancelScheduledWork: ReturnType }) + .cancelScheduledWork).not.toHaveBeenCalled(); + }); + + it("leaves terminal sessions alone — a terminal pane is user-owned", async () => { + // The whole carve-out of settle teardown: an agent's background shell is + // thread background work, but the pane the user opened to watch a build is + // theirs and must survive the settle with its scrollback. + const d = deps([ + { id: "term-1", toolType: "shell" }, + { id: "cli-1", toolType: "claude" }, + ]); + const result = await stopSettledSessionMachinery(d, ["term-1", "cli-1"]); + + expect(result.sessionIds).toEqual([]); + expect(d.stopBackgroundWork).not.toHaveBeenCalled(); + expect(d.setScheduledWorkPaused).not.toHaveBeenCalled(); + }); + + it("reports a session skipped because its foreground turn is still streaming", async () => { + const d = deps([{ id: "chat-1", toolType: "claude-chat" }], { + stopBackgroundWork: vi.fn(async () => ({ stopped: 0, skippedActiveTurn: true })), + }); + const result = await stopSettledSessionMachinery(d, ["chat-1"]); + expect(result.skippedActiveTurns).toBe(1); + expect(result.stoppedBackgroundWork).toBe(0); + }); + + it("never lets a provider failure block the settle", async () => { + const d = deps([{ id: "chat-1", toolType: "claude-chat" }], { + stopBackgroundWork: vi.fn(async () => { + throw new Error("provider unreachable"); + }), + }); + await expect(stopSettledSessionMachinery(d, ["chat-1"])).resolves.toMatchObject({ + sessionIds: ["chat-1"], + stoppedBackgroundWork: 0, + }); + expect(d.logger.warn).toHaveBeenCalled(); + }); + + it("skips unknown ids and de-duplicates repeats", async () => { + const d = deps([{ id: "chat-1", toolType: "claude-chat" }]); + const result = await stopSettledSessionMachinery(d, ["chat-1", "chat-1", " ", "missing"]); + expect(result.sessionIds).toEqual(["chat-1"]); + expect(d.stopBackgroundWork).toHaveBeenCalledTimes(1); + }); + + it("does nothing at all without a chat service, rather than throwing", async () => { + const result = await stopSettledSessionMachinery( + { + sessionService: { get: () => ({ id: "chat-1", toolType: "claude-chat" }) } as never, + agentChatService: null, + }, + ["chat-1"], + ); + expect(result.sessionIds).toEqual(["chat-1"]); + expect(result.stoppedBackgroundWork).toBe(0); + }); +}); + +describe("settleTerminalSession", () => { + it("tears the machinery down before writing the settled column", async () => { + // Ordering matters: a settle must never report success while the monitors + // it claims to have concluded are still armed. + const order: string[] = []; + const d = deps([{ id: "chat-1", toolType: "claude-chat" }], { + stopBackgroundWork: vi.fn(async () => { + order.push("stop"); + return { stopped: 1, skippedActiveTurn: false }; + }), + }); + const sessionService = { + get: (id: string) => (id === "chat-1" ? { id, toolType: "claude-chat" } : null), + settleSession: vi.fn(() => { + order.push("settle"); + return true; + }), + }; + + await expect(settleTerminalSession({ + sessionId: "chat-1", + opts: { source: "user" }, + sessionService: sessionService as never, + agentChatService: d.agentChatService, + logger: d.logger, + })).resolves.toBe(true); + + expect(order).toEqual(["stop", "settle"]); + expect(sessionService.settleSession).toHaveBeenCalledWith("chat-1", { source: "user" }); + }); +}); diff --git a/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts b/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts new file mode 100644 index 000000000..be66dda89 --- /dev/null +++ b/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts @@ -0,0 +1,146 @@ +import type { createAgentChatService } from "../chat/agentChatService"; +import type { createSessionService } from "./sessionService"; +import { isChatToolType } from "./chatSessionProjection"; + +/** + * Stop the machinery a session owns when its lifecycle ends. + * + * ── Why this exists ───────────────────────────────────────────────────────── + * + * Settle used to be a pure column write. The row went quiet and everything the + * session had started kept going: background shells held ports, subagent fleets + * kept spending tokens, and scheduled work woke the thread hours after the user + * had declared it done. "Settled" claimed a conclusion the process tree had not + * reached. + * + * Archive had the mirror problem from the other end — it released the lane's + * port lease and proxy route while the processes were still holding those + * ports, so the lease could be handed to another lane that then could not bind. + * + * Both now converge here so the step list cannot drift into two versions. + * + * ── What settle stops, and what it deliberately does not ──────────────────── + * + * stops scheduled work (monitors, crons, loops) for the session + * stops live background work — background shells, subagent fleets, + * cursor cloud runs — via `agentChatService.stopBackgroundWork` + * keeps the session itself, and its runtime, alive and resumable + * keeps terminal panes open — a terminal is USER-owned. An agent's + * background shell is thread background work; the pane the user + * opened to watch a build is theirs, and closing it on settle would + * destroy scrollback they never asked to lose. + * keeps an ACTIVE foreground turn running (see `stopBackgroundWork`). + * + * ── What escapes, stated plainly ──────────────────────────────────────────── + * + * A process an agent detached with `nohup`, `setsid`, or `disown` leaves ADE's + * tree entirely and nothing here can reach it. Codex background subagents are + * reported but expose no stop control. Neither is silently pretended away: + * `stopBackgroundWork` returns what it actually acted on. + */ +export type SessionMachineryTeardownDeps = { + sessionService: Pick, "get">; + agentChatService?: Pick< + ReturnType, + "stopBackgroundWork" | "setScheduledWorkPaused" | "listScheduledWork" + > | null; + logger?: { warn: (message: string, meta?: Record) => void } | null; +}; + +export type SessionMachineryTeardownResult = { + /** Sessions whose machinery this pass touched. */ + sessionIds: string[]; + /** Live background jobs stopped across those sessions. */ + stoppedBackgroundWork: number; + /** Sessions whose scheduled work was paused. */ + pausedScheduledWork: number; + /** Sessions skipped because a foreground turn was still streaming. */ + skippedActiveTurns: number; +}; + +const EMPTY_RESULT: SessionMachineryTeardownResult = { + sessionIds: [], + stoppedBackgroundWork: 0, + pausedScheduledWork: 0, + skippedActiveTurns: 0, +}; + +/** + * Pause rather than cancel. + * + * Cancelling would be destructive and irreversible: a settle that turns out to + * be premature (the user unsettles, or new activity un-settles the row) would + * have silently deleted schedules the user set up by hand. Pausing stops the + * 3am wake and survives being undone, which is the whole difference between + * "this is filed" and "this is deleted". + */ +async function pauseScheduledWork( + deps: SessionMachineryTeardownDeps, + sessionId: string, +): Promise { + const service = deps.agentChatService; + if (!service) return false; + try { + const schedules = await service.listScheduledWork({ sessionId }); + const armed = schedules.some( + (schedule) => schedule.status !== "completed" && schedule.status !== "cancelled", + ); + if (!armed) return false; + await service.setScheduledWorkPaused({ sessionId, paused: true }); + return true; + } catch (error) { + deps.logger?.warn("session_teardown.pause_scheduled_work_failed", { + sessionId, + error: error instanceof Error ? error.message : String(error), + }); + return false; + } +} + +/** + * Stop the background machinery for a set of sessions being settled. + * + * Best-effort by construction: a settle must not fail because a provider could + * not be reached, so every step swallows its own error and the result reports + * what actually happened. + */ +export async function stopSettledSessionMachinery( + deps: SessionMachineryTeardownDeps, + sessionIds: readonly string[], +): Promise { + const unique = [...new Set(sessionIds.map((id) => id.trim()).filter(Boolean))]; + if (unique.length === 0) return EMPTY_RESULT; + + const result: SessionMachineryTeardownResult = { + sessionIds: [], + stoppedBackgroundWork: 0, + pausedScheduledWork: 0, + skippedActiveTurns: 0, + }; + + for (const sessionId of unique) { + const row = deps.sessionService.get(sessionId); + // Only chat-backed sessions own the machinery this tears down. A plain + // terminal's process is the user's, and a tracked agent CLI's work lives in + // its PTY — which settle keeps open on purpose. + if (!row || !isChatToolType(row.toolType)) continue; + result.sessionIds.push(sessionId); + + if (await pauseScheduledWork(deps, sessionId)) result.pausedScheduledWork += 1; + + const service = deps.agentChatService; + if (!service) continue; + try { + const stop = await service.stopBackgroundWork({ sessionId }); + result.stoppedBackgroundWork += stop.stopped; + if (stop.skippedActiveTurn) result.skippedActiveTurns += 1; + } catch (error) { + deps.logger?.warn("session_teardown.stop_background_work_failed", { + sessionId, + error: error instanceof Error ? error.message : String(error), + }); + } + } + + return result; +} diff --git a/apps/desktop/src/main/services/sessions/settleTerminalSession.ts b/apps/desktop/src/main/services/sessions/settleTerminalSession.ts index d1b28df05..12b1d7438 100644 --- a/apps/desktop/src/main/services/sessions/settleTerminalSession.ts +++ b/apps/desktop/src/main/services/sessions/settleTerminalSession.ts @@ -3,6 +3,7 @@ import type { createPtyService } from "../pty/ptyService"; import type { createSessionService } from "./sessionService"; import type { SessionSettleSource } from "../../../shared/types"; import { isChatToolType } from "./chatSessionProjection"; +import { stopSettledSessionMachinery } from "./sessionMachineryTeardown"; export type SettleTerminalSessionOptions = { outcome?: string; @@ -45,12 +46,21 @@ export async function dismissPendingInputBeforeSettle(args: { return true; } +/** + * Settle a session AND stop the machinery it owns. + * + * The teardown runs before the column write so a settle can never report + * success while its monitors are still armed. It is best-effort — see + * `stopSettledSessionMachinery` — so a provider that cannot be reached delays + * nothing and blocks nothing. + */ export async function settleTerminalSession(args: { sessionId: string; opts?: SettleTerminalSessionOptions; sessionService: ReturnType; agentChatService?: ReturnType | null; ptyService?: ReturnType | null; + logger?: { warn: (message: string, meta?: Record) => void } | null; }): Promise { if (args.opts?.dismissPendingInput === true) { const dismissed = await dismissPendingInputBeforeSettle({ @@ -62,6 +72,15 @@ export async function settleTerminalSession(args: { if (!dismissed) return false; } + await stopSettledSessionMachinery( + { + sessionService: args.sessionService, + agentChatService: args.agentChatService ?? null, + logger: args.logger ?? null, + }, + [args.sessionId], + ); + return args.sessionService.settleSession( args.sessionId, { diff --git a/apps/desktop/src/main/services/storage/storageInsightsService.test.ts b/apps/desktop/src/main/services/storage/storageInsightsService.test.ts index 2039ac4e4..5c936586b 100644 --- a/apps/desktop/src/main/services/storage/storageInsightsService.test.ts +++ b/apps/desktop/src/main/services/storage/storageInsightsService.test.ts @@ -1007,7 +1007,7 @@ describe("storageMaintenanceJournal", () => { ); }; insertLock("lane-locked", lockedPath, "lock-before-scan"); - const archive = vi.fn(); + const archive = vi.fn(async () => {}); const getReclaimRisk = vi.fn(async (laneId: string) => { if (laneId === "lane-raced") insertLock(laneId, racedPath, "lock-during-scan"); return { @@ -1070,7 +1070,7 @@ describe("storageMaintenanceJournal", () => { const worktreePath = path.join(projectRoot, ".ade", "worktrees", "lane-invalid-activity"); seedLane(db, { id: "lane-invalid-activity", name: "Invalid activity", worktreePath }); db.run("update lanes set created_at = 'not-a-date' where id = ?", ["lane-invalid-activity"]); - const archive = vi.fn(); + const archive = vi.fn(async () => {}); const service = createStorageInsightsService({ projectRoot, adeHome, @@ -1127,7 +1127,7 @@ describe("storageMaintenanceJournal", () => { throw new Error("lane scan failed"); }), getReclaimRisk: vi.fn(), - archive: vi.fn(), + archive: vi.fn(async () => {}), }, projectConfigService: { get: () => ({ @@ -1170,7 +1170,7 @@ describe("storageMaintenanceJournal", () => { worktreePath: retainedPath, archivedAt: "2026-07-01T00:00:00.000Z", }); - const archive = vi.fn(({ laneId }: { laneId: string }) => { + const archive = vi.fn(async ({ laneId }: { laneId: string }) => { db.run( "update lanes set status = 'archived', archived_at = ? where id = ?", [new Date().toISOString(), laneId], diff --git a/apps/desktop/src/main/services/storage/storageInsightsService.ts b/apps/desktop/src/main/services/storage/storageInsightsService.ts index 09fbffdc1..f426ced3d 100644 --- a/apps/desktop/src/main/services/storage/storageInsightsService.ts +++ b/apps/desktop/src/main/services/storage/storageInsightsService.ts @@ -78,7 +78,7 @@ type LaneLifecycleBackend = { activeWatcherCount: number; blockedReasons: Array<{ code: string }>; }>; - archive: (args: { laneId: string }) => void; + archive: (args: { laneId: string }) => Promise; }; type LaneCleanupConfigReader = { @@ -439,7 +439,9 @@ export function createStorageInsightsService(options: StorageInsightsServiceOpti if (!candidate.dueByAge && stillNeedsArchive <= 0) continue; try { if (hasActiveLaneWorktreeLock(candidate.laneId)) continue; - laneService.archive({ laneId: candidate.laneId }); + // Await before the lease release: the lane's processes have to be + // gone before their ports are handed back. + await laneService.archive({ laneId: candidate.laneId }); await options.releaseLaneRuntimeResources?.(candidate.laneId); archivedAutomatically += 1; if (stillNeedsArchive > 0) stillNeedsArchive -= 1; diff --git a/docs/features/lanes/README.md b/docs/features/lanes/README.md index 728592409..57e362067 100644 --- a/docs/features/lanes/README.md +++ b/docs/features/lanes/README.md @@ -546,9 +546,21 @@ a lane parented to primary would always show zero behind. current branch. When both the parent link and the resolved base ref are unchanged, reparent short-circuits without touching git so a redundant apply is a no-op rather than a stack rebase. -7. **Archive / reclaim / restore** — `archive` sets `archived_at` and - `status = 'archived'` but keeps the worktree and generated files on disk, - then emits a `lane-archived` lifecycle event. `archiveAndReclaim` is the +7. **Archive / reclaim / restore** — `archive` stops the lane's runtime work + (chats, PTYs, file watchers, auto-rebase) through the shared + `stopLaneRuntimeWork` helper, then sets `archived_at` and + `status = 'archived'` while keeping the worktree and generated files on + disk, then emits a `lane-archived` lifecycle event. The stop is `await`ed + and its ORDER is load-bearing: every caller releases the lane's port lease + and proxy route (`releaseLaneRuntimeResources`) the moment archive resolves, + and archive used to be a bare status write — so the lease and route went + back to the pool while the lane's dev servers and agents were still bound to + those ports, and the abandoned processes went on holding them indefinitely + because an archived lane is filtered out of every surface that could have + shown them. Individual stop steps are best-effort (one stuck watcher must not + make a lane un-archivable) and failures are logged as + `lane_runtime_teardown.step_failed`. `delete` runs the same steps through its + own `runStep` progress reporting, since the delete dialog shows each one. `archiveAndReclaim` is the separate, typed-confirmation path: it preserves the lane row, branch, chats, and metadata while stopping lane-owned processes and removing only the ADE-managed worktree and lane pack data. Before sizing or removing a diff --git a/docs/features/terminals-and-sessions/README.md b/docs/features/terminals-and-sessions/README.md index ef1d4837d..daa7f628e 100644 --- a/docs/features/terminals-and-sessions/README.md +++ b/docs/features/terminals-and-sessions/README.md @@ -205,7 +205,25 @@ and in tests. presenting a false live/green agent. - `apps/desktop/src/main/services/sessions/settleTerminalSession.ts` — single settlement transaction shared by direct IPC and the ADE action - registry. Plain settle writes lifecycle state. `dismissPendingInput: true` + registry. Settle stops the machinery the session owns before it writes the + lifecycle column — see + `apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts`. It + pauses the session's scheduled work (pauses rather than cancels, so an + unsettle can bring hand-made schedules back) and calls + `agentChatService.stopBackgroundWork`, which stops every live child before + the parent. **Terminal panes stay open**: an agent's background shell is + thread background work, but a pane the user opened is theirs, and closing it + on settle would destroy scrollback nobody asked to lose. An ACTIVE foreground + turn is also left alone — its subagents are work the user can see happening, + and the row un-settles on its own activity anyway. What escapes is stated + rather than pretended away: processes an agent detached with + `nohup`/`setsid`/`disown` leave ADE's tree entirely, and Codex background + subagents are reported but expose no stop control. Every settle entry point + runs it — the single/bulk ADE actions, the `sessions.settle`/`settleMany` + IPC handlers, the `session.settle*` sync commands, and the PR-merge + auto-settle (which bypasses settlement blockers and is therefore the path + most likely to file a session that is still running something). + `dismissPendingInput: true` first quiets an SDK chat through `agentChatService`, or clears a tracked CLI's explicit `ade chat ask` marker through `ptyService`; arbitrary native terminal prompts are rejected because ADE cannot answer them truthfully. @@ -278,6 +296,24 @@ Shared types and IPC: explicit/structured attention → declared settle at rest → stopped/failure/ clean exit → stale/running/resting. It is the source of the one-word row capsule, Work grouping, and the loud-vs-quiet attention split. + **Live background work promotes a resting session back to `running`.** A + session whose foreground turn ended while its background shells, monitors, or + subagent fleet kept going used to project to `ready`/`idle`, so the Work-tab + dot, the TopBar rollup, the dock badge and the Lanes agent list all showed + nothing while agents were mid-run — the "Background work" copy existed but + never reached the phase those surfaces derive from. The promotion also + returns `liveness` (`"turn" | "background" | "monitoring"`), which is what the + label reads; it is not a phase, because filing, buckets, the roster status and + iOS's `AgentRunPhase` all want the three treated identically. + `monitoring` is set only when watch loops are the SOLE live work — one real + job among three monitors still reads as working. + Classification is a **denylist** (`MONITOR_TASK_TYPES` / `INERT_TASK_TYPES`, + via `classifyBackgroundWorkKind`): unknown task types count as WORKING, + because an allowlist silently drops a real subagent the first time a provider + SDK renames a type. Liveness is in-memory and deliberately empty after a + restart — orphaned background work is not live work — and it sits BELOW + failure, stopped, settled and stale in the precedence order, so a lingering + "Working" can never mask a failed session. The **settle override** (`terminal_sessions.settle_override`, `null | "settled" | "active"`) is consulted at the declared-settle tier, i.e. `"settled"` behaves like a declared settle, and `"active"` is an explicit @@ -317,11 +353,13 @@ Shared types and IPC: and Activity surfaces and mirrored by iOS widgets/Activity drawer. Blue means work in flight, amber is reserved exclusively for `Needs you`, emerald is a clean unseen outcome, red is failure, and neutral is true but non-actionable. - An idle or ready session that still has live background jobs is named for what - it is — **Background work**, or **Background work ×N** — rather than reusing - the bare **Working** of a live turn, and it sets `showsElapsed`, so the row - carries a duration instead of an unfalsifiable claim that the model is still - thinking. + A session running only because of background work is named for what it is — + **Background work** / **Background work ×N**, or **Monitoring** / **Monitoring + ×N** when watch loops are all that is left — rather than reusing the bare + **Working** of a live turn, and it sets `showsElapsed`, so the row carries a + duration instead of an unfalsifiable claim that the model is still thinking. + Plan mode is a property of a live turn only: a background-promoted row never + reads **Planning**. It also owns the short working-duration formatter; renderer icon components map its dependency-free glyph ids to platform symbols. - `apps/desktop/src/renderer/lib/sessionSnooze.ts` — the desktop half of snooze From 8bf5e063a2b41b569ccb3c6a2cdaaf07a60eb712 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 9 Aug 2026 20:49:21 -0400 Subject: [PATCH 03/14] =?UTF-8?q?fix(sessions):=20quality=20pass=20?= =?UTF-8?q?=E2=80=94=20honest=20teardown=20counts,=20no=20zero-record=20ch?= =?UTF-8?q?urn?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Findings from the /quality dual-review on this branch, all verified against the real code paths before applying: - stopBackgroundWork reported the live work it FOUND as the work it stopped, so a Codex session (no per-subagent stop control) or a Cursor session with no cloud agent id claimed a teardown that never happened. It now reports the measured DROP in live work across the call, which is 0 for those cases by construction and can never over-report. - A Claude background task ADE could not stop was closed as "stopped". It now settles as failed with the reason, matching closeOpenClaudeBackgroundTasks — both close the row, only one claims ADE did the stopping. - getSessionSummary emitted backgroundWork: {0,0} on every chat summary. Now omitted when nothing is live, like every other optional field there. - NO_BACKGROUND_WORK was a shared mutable object handed out by reference; frozen. - laneAgents' background hint guarded on a stringly-typed status that chat and CLI summaries spell differently. Callers now pass turnActive explicitly. Co-Authored-By: Claude Opus 5 --- .../main/services/chat/agentChatService.ts | 52 +++++++++++++++---- .../renderer/components/lanes/laneAgents.ts | 27 ++++++---- 2 files changed, 59 insertions(+), 20 deletions(-) diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index 4d66433ed..0cb55366d 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -2144,7 +2144,12 @@ function hasLivePendingInput(managed: ManagedChatSession | null | undefined): bo return false; } -const NO_BACKGROUND_WORK: SessionBackgroundWork = { workingCount: 0, monitoringCount: 0 }; +// Frozen because it is returned by reference to every caller with no live work; +// a single mutation would otherwise follow every session in the process. +const NO_BACKGROUND_WORK: SessionBackgroundWork = Object.freeze({ + workingCount: 0, + monitoringCount: 0, +}); /** * Live work a chat session still owns after its foreground turn bookends, @@ -38876,7 +38881,10 @@ export function createAgentChatService(args: { ...(provider === "claude" ? { claudeTag } : {}), nextWakeAt, activeBackgroundTaskCount, - backgroundWork, + // Omitted when nothing is live, like every other optional field here: a + // zero record carries no information and would ride along on every + // summary read for every session. + ...(activeBackgroundTaskCount > 0 ? { backgroundWork } : {}), scheduledWorkPaused, scheduledWork, ...(sessionHasPendingInput ? { awaitingInput: true } : {}), @@ -39287,8 +39295,9 @@ export function createAgentChatService(args: { * • Children stop before parents. Stopping only the parent leaves the fleet * running and untracked, which is how a "stopped" agent keeps spending. * - * Returns how much live work was found, so callers can report honestly rather - * than claiming a teardown that did nothing. + * Returns how much live work actually STOPPED — the drop in the session's + * live-work count across the call, not what it found. A runtime with no stop + * control reports 0 rather than claiming a teardown it never performed. */ const stopBackgroundWork = async ( { sessionId }: { sessionId: string }, @@ -39299,9 +39308,9 @@ export function createAgentChatService(args: { if (!runtime) return { stopped: 0, skippedActiveTurn: false }; const turnActive = managed.session.status === "active" || Boolean(runtime.activeTurnId); - const stopped = totalBackgroundWork(runtimeBackgroundWork(runtime)); if (turnActive) return { stopped: 0, skippedActiveTurn: true }; - if (stopped === 0) return { stopped: 0, skippedActiveTurn: false }; + const before = totalBackgroundWork(runtimeBackgroundWork(runtime)); + if (before === 0) return { stopped: 0, skippedActiveTurn: false }; try { switch (runtime.kind) { @@ -39319,7 +39328,17 @@ export function createAgentChatService(args: { // Those are exactly the ones that survived the old teardown. const control = getClaudeQueryControl(runtime.query); for (const taskId of [...runtime.liveBackgroundTaskIds]) { - if (typeof control.stopTask === "function") { + // Same convention as `closeOpenClaudeBackgroundTasks`: a task ADE + // could not actually stop settles as FAILED with the reason, not as + // "stopped". Both close the row — leaving it open would keep the + // session claiming work forever — but only one of them claims ADE + // did the stopping. + let terminalStatus: ScheduledWorkEvent["status"] = "stopped"; + let terminalSummary: string | undefined; + if (typeof control.stopTask !== "function") { + terminalStatus = "failed"; + terminalSummary = "The Claude query did not expose a task stop control."; + } else { try { await awaitClaudeControlCall( `Stopping Claude background task '${taskId}'`, @@ -39327,14 +39346,21 @@ export function createAgentChatService(args: { () => control.stopTask!(taskId), ); } catch (error) { + const message = error instanceof Error ? error.message : String(error); + terminalStatus = "failed"; + terminalSummary = `Failed to stop background task: ${message}`; logger.warn("agent_chat.settle_background_stop_failed", { sessionId: managed.session.id, taskId, - error: error instanceof Error ? error.message : String(error), + error: message, }); } } - emitClaudeBackgroundTaskUpdate(managed, runtime, { taskId, status: "stopped" }); + emitClaudeBackgroundTaskUpdate(managed, runtime, { + taskId, + status: terminalStatus, + ...(terminalSummary ? { summary: terminalSummary } : {}), + }); } break; } @@ -39365,7 +39391,13 @@ export function createAgentChatService(args: { error: error instanceof Error ? error.message : String(error), }); } - return { stopped, skippedActiveTurn: false }; + // Measured as the DROP in live work rather than what was found, so a + // runtime with no stop control (Codex) or a cursor session with no cloud + // agent id reports 0 instead of claiming a teardown it never performed. + // Under-reporting is the safe direction: a caller may never be told more + // was stopped than actually was. + const after = totalBackgroundWork(runtimeBackgroundWork(runtime)); + return { stopped: Math.max(0, before - after), skippedActiveTurn: false }; }; const hasActiveWorkloads = (): boolean => { diff --git a/apps/desktop/src/renderer/components/lanes/laneAgents.ts b/apps/desktop/src/renderer/components/lanes/laneAgents.ts index 59a80ee10..e9304e23c 100644 --- a/apps/desktop/src/renderer/components/lanes/laneAgents.ts +++ b/apps/desktop/src/renderer/components/lanes/laneAgents.ts @@ -102,7 +102,7 @@ function chatAgentFrom(summary: AgentChatSessionSummary): LaneAgent { activity: chatActivity(summary), lastHint: summary.awaitingInput ? "Awaiting your input" - : backgroundHint(summary) + : backgroundHint(summary, summary.status === "active") ?? summary.summary?.trim() ?? summary.lastOutputPreview?.trim() ?? null, @@ -113,15 +113,22 @@ function chatAgentFrom(summary: AgentChatSessionSummary): LaneAgent { /** * Background work is the more useful hint than a stale last-output preview: * the preview describes the turn that already ended, the count describes what - * is still running. Only shown once the turn is over — a live turn's own - * output is the better story. + * is still running. + * + * Callers pass `turnActive` explicitly rather than having this re-read a status + * field — chat and CLI summaries spell "a turn is running" differently + * (`status: "active"` vs `runtimeState: "running"`), and a single stringly-typed + * guard here would silently match neither for one of them. */ -function backgroundHint(summary: { - status?: string; - backgroundWork?: SessionBackgroundWork; - activeBackgroundTaskCount?: number; -}): string | null { - if (summary.status === "active") return null; +function backgroundHint( + summary: { + backgroundWork?: SessionBackgroundWork; + activeBackgroundTaskCount?: number; + }, + turnActive: boolean, +): string | null { + // A live turn's own output is the better story than a job count. + if (turnActive) return null; const work = backgroundWorkFromSummary(summary); const total = totalBackgroundWork(work); if (total <= 0) return null; @@ -143,7 +150,7 @@ function cliAgentFrom(summary: TerminalSessionSummary): LaneAgent { || summary.attentionRequestedAt || summary.attentionSource === "provider_structured" ? "Awaiting your input" - : backgroundHint(summary) + : backgroundHint(summary, summary.runtimeState === "running") ?? summary.summary?.trim() ?? summary.lastOutputPreview?.trim() ?? null, From feaf8566203092b35518b65a8e65f8e54c871808 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:17:05 -0400 Subject: [PATCH 04/14] fix(sessions): keep bulk-settle validation throwing synchronously + Pi in the matrix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebased onto origin/main (P0 #1056, Pi #1054/#1055). - registry's `session.settleSessions` had become an `async` function, which silently converted its argument-validation guard from a synchronous throw into a rejected promise — a contract change for any caller that does not await, caught by registry.test.ts. Only the success path is async now, returned as an explicit promise from a sync body. The success assertion is awaited, because that path genuinely did become asynchronous: the session's monitors have to stop before the settle is written. - runtimeBackgroundWork's switch is now exhaustive over ChatRuntime["kind"] with a `never` check, so a newly landed harness fails to compile until someone classifies it. Pi is listed explicitly: its runtime carries only turn-scoped state (activeTurnId, busy, pendingSteers, activeCompactionId, lease) with no subagent, background-task, or remote-run tracking, and it is absent from SUBAGENT_CAPABILITIES so resolveSubagentCapability already degrades it to the no-op descriptor. Zero is a verified fact about Pi, not an unexamined default. - Settle teardown now runs inside the merged candidateSessionsFor loop, so it targets exactly what the corrected targeting settles: nothing at all when the scope is ambiguous, only declared in-lane sessions when linked, and the bounded sweep minus other PRs' claims otherwise. Co-Authored-By: Claude Opus 5 --- .../main/services/adeActions/registry.test.ts | 14 ++++++--- .../src/main/services/adeActions/registry.ts | 29 +++++++++++------ .../main/services/chat/agentChatService.ts | 31 ++++++++++++++++--- 3 files changed, 56 insertions(+), 18 deletions(-) diff --git a/apps/desktop/src/main/services/adeActions/registry.test.ts b/apps/desktop/src/main/services/adeActions/registry.test.ts index df480f6b4..83db53104 100644 --- a/apps/desktop/src/main/services/adeActions/registry.test.ts +++ b/apps/desktop/src/main/services/adeActions/registry.test.ts @@ -1595,7 +1595,7 @@ describe("runtime session actions", () => { // session. This bulk action never has, and used to drop the key silently — so // the same argument meant "dismiss the prompt" over sync and nothing at all // here. Settling while quietly ignoring half the request is the failure mode. - it("refuses a bulk settle that asks to dismiss pending input", () => { + it("refuses a bulk settle that asks to dismiss pending input", async () => { const settleSessions = vi.fn(() => ["session-1"]); const runtime = { sessionService: { @@ -1608,15 +1608,21 @@ describe("runtime session actions", () => { settleSessions: (args: unknown) => unknown; } & Record; + // SYNCHRONOUS throw, even though the success path is now async: settle grew + // a teardown step, and marking the whole action `async` would have quietly + // turned this guard into a rejected promise that a non-awaiting caller + // drops on the floor. expect(() => sessionService.settleSessions({ sessionIds: ["session-1"], dismissPendingInput: true, })).toThrow(/does not dismiss pending input/); expect(settleSessions).not.toHaveBeenCalled(); - // Without the flag the bulk path is untouched. - expect(sessionService.settleSessions({ sessionIds: ["session-1", "session-2"] })) - .toEqual(["session-1"]); + // Without the flag the bulk path is untouched — but it is awaited now, + // because the session's monitors and background shells have to be stopped + // before the settle is written. + await expect(sessionService.settleSessions({ sessionIds: ["session-1", "session-2"] })) + .resolves.toEqual(["session-1"]); expect(settleSessions).toHaveBeenCalledWith(["session-1", "session-2"]); }); diff --git a/apps/desktop/src/main/services/adeActions/registry.ts b/apps/desktop/src/main/services/adeActions/registry.ts index f70aa4729..360b87259 100644 --- a/apps/desktop/src/main/services/adeActions/registry.ts +++ b/apps/desktop/src/main/services/adeActions/registry.ts @@ -2148,7 +2148,14 @@ function buildSessionDomainService(runtime: AdeRuntime): OpaqueService | null { }, // Bulk settle/unsettle for renderer surfaces on remote-bound projects // (mirrors deleteSession's generic trust posture). - settleSessions: async (args?: unknown) => { + // + // Deliberately NOT an `async` function: argument validation below must keep + // throwing SYNCHRONOUSLY, the way it did before settle grew a teardown + // step. Marking the whole action `async` silently converts every one of + // those guards into a rejected promise, which changes the contract for any + // caller that does not await — so only the success path is async, returned + // as an explicit promise from a sync body. + settleSessions: (args?: unknown) => { const record = readObjectActionArg(args, "session.settleSessions"); const sessionIds = Array.isArray(record.sessionIds) ? record.sessionIds.filter((id): id is string => typeof id === "string") @@ -2170,15 +2177,17 @@ function buildSessionDomainService(runtime: AdeRuntime): OpaqueService | null { "session.settleSessions does not dismiss pending input; use session.settleSession for a single session.", ); } - await stopSettledSessionMachinery( - { - sessionService, - agentChatService: runtime.agentChatService, - logger: runtime.logger, - }, - sessionIds, - ); - return sessionService.settleSessions(sessionIds); + return (async () => { + await stopSettledSessionMachinery( + { + sessionService, + agentChatService: runtime.agentChatService, + logger: runtime.logger, + }, + sessionIds, + ); + return sessionService.settleSessions(sessionIds); + })(); }, unsettleSessions: (args?: unknown) => { const record = readObjectActionArg(args, "session.unsettleSessions"); diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index 0cb55366d..386340dcd 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -2167,9 +2167,11 @@ const NO_BACKGROUND_WORK: SessionBackgroundWork = Object.freeze({ * ADE's process tree entirely, * • long-lived processes started inside a user-owned terminal pane, which are * the user's to manage and deliberately out of scope, - * • opencode / droid / pi background work — those runtimes report no - * background-task or subagent level to track. They contribute zero here - * rather than a guess. + * • opencode / droid / pi work — those harnesses expose no background-task, + * subagent, or remote-run level to track at all, so they contribute zero + * here. That is a checked fact per harness, not a default: the switch below + * is exhaustive over `ChatRuntime["kind"]`, so a newly landed harness fails + * to compile until someone decides which column it belongs in. */ function runtimeBackgroundWork(runtime: ChatRuntime | null): SessionBackgroundWork { if (!runtime) return NO_BACKGROUND_WORK; @@ -2197,8 +2199,29 @@ function runtimeBackgroundWork(runtime: ChatRuntime | null): SessionBackgroundWo // turn ends — the clearest case of work outliving its turn ADE has. return summarizeBackgroundWork(new Array(runtime.cloudRuns.size).fill(null)); } - default: + // ── Harnesses with no background-work surface ─────────────────────────── + // + // Listed individually rather than swept up by a `default`, so the + // exhaustiveness check below turns "a new harness landed" into a compile + // error instead of a silent zero. Pi (#1054/#1055) is the case that proved + // the point: its runtime carries only turn-scoped state — activeTurnId, + // busy, pendingSteers, activeCompactionId, lease — with no subagent, + // background-task, or remote-run tracking of any kind, and it is absent + // from `SUBAGENT_CAPABILITIES` so `resolveSubagentCapability` already + // degrades it to the no-op descriptor. Zero here is a verified fact about + // Pi, not an unexamined default. + case "opencode": + case "droid": + case "pi": + return NO_BACKGROUND_WORK; + default: { + // A new harness must state whether it owns work that outlives a turn. + // Getting this wrong in the silent direction is the exact bug this whole + // module exists to fix, so the decision is compulsory. + const exhaustive: never = runtime; + void exhaustive; return NO_BACKGROUND_WORK; + } } } From 4e6707392e7521292cc90daab6ceb90a70d68f7c Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:18:19 -0400 Subject: [PATCH 05/14] docs(sessions): drop reference to the settlement-blocker helper main removed Co-Authored-By: Claude Opus 5 --- docs/features/terminals-and-sessions/README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/features/terminals-and-sessions/README.md b/docs/features/terminals-and-sessions/README.md index daa7f628e..c59153794 100644 --- a/docs/features/terminals-and-sessions/README.md +++ b/docs/features/terminals-and-sessions/README.md @@ -221,8 +221,9 @@ and in tests. subagents are reported but expose no stop control. Every settle entry point runs it — the single/bulk ADE actions, the `sessions.settle`/`settleMany` IPC handlers, the `session.settle*` sync commands, and the PR-merge - auto-settle (which bypasses settlement blockers and is therefore the path - most likely to file a session that is still running something). + auto-settle (which files a session even when it still owns scheduled work or + a live background task, and is therefore the path most likely to file one + that is still running something). `dismissPendingInput: true` first quiets an SDK chat through `agentChatService`, or clears a tracked CLI's explicit `ade chat ask` marker through `ptyService`; arbitrary native From 8901c13ece6c95ddcac09ce9f66ed0b6e45c3011 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:32:28 -0400 Subject: [PATCH 06/14] test(sessions): consolidate end-of-life teardown coverage, pin the liveness contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pruned/consolidated: the sessions folder had 5 test files against a 3-file budget after this branch added one. deleteTerminalSession.test.ts and sessionMachineryTeardown.test.ts covered the same contract — what happens to a session's machinery at end of life — split across files for dependency reasons, not behavioral ones. Merged into sessionTeardown.test.ts (12 tests), returning the folder to the 4 files it had before this branch. No tests lost. Added, where the failure mode is actually reachable: - agentChatService.test.ts: drives a real background_tasks_changed level and asserts the summary splits it working/monitoring by denylist (local_bash -> monitoring, local_agent AND an unrecognised type -> working), that a live turn makes stopBackgroundWork decline rather than kill it, and that the record is omitted once the level drains rather than riding along as a zero. - laneAgents.test.ts: a resting agent stays live while its background work is, sorts working ahead of monitoring ahead of idle, reports what is still running instead of the finished turn's stale preview, and counts a split-less (older-peer) summary as working rather than passive. Parity: corrected stale prose in attentionItemBuilder that still described the promotion as a sessionStatusPresentation label override rather than a sessionCanonicalState phase promotion. Co-Authored-By: Claude Opus 5 --- .../src/services/push/attentionItemBuilder.ts | 19 ++-- .../services/chat/agentChatService.test.ts | 70 ++++++++++++ .../sessions/deleteTerminalSession.test.ts | 93 ---------------- ...ardown.test.ts => sessionTeardown.test.ts} | 102 ++++++++++++++++++ .../components/lanes/laneAgents.test.ts | 55 ++++++++++ 5 files changed, 237 insertions(+), 102 deletions(-) delete mode 100644 apps/desktop/src/main/services/sessions/deleteTerminalSession.test.ts rename apps/desktop/src/main/services/sessions/{sessionMachineryTeardown.test.ts => sessionTeardown.test.ts} (61%) diff --git a/apps/ade-cli/src/services/push/attentionItemBuilder.ts b/apps/ade-cli/src/services/push/attentionItemBuilder.ts index 23e04a935..0d7b86075 100644 --- a/apps/ade-cli/src/services/push/attentionItemBuilder.ts +++ b/apps/ade-cli/src/services/push/attentionItemBuilder.ts @@ -54,13 +54,13 @@ export type AgentRunState = { metaResolved: boolean; /** * Live background-task ids for this run, tracked from the `background_task` - * flavour of `scheduled_work_update`. Claude spawns background subagents that - * keep working after the foreground turn bookends, and the desktop sidebar - * already treats that as Working (`sessionStatusPresentation.ts` overrides - * ready/idle when `activeBackgroundTaskCount > 0`). This set is the - * publisher's copy of the same fact — both derive from agentChatService's - * live background-task level — so Activity cannot publish "is done" over a - * session that is demonstrably still working. + * flavour of `scheduled_work_update`. Agents spawn background work that keeps + * running after the foreground turn bookends, and desktop already treats that + * as Working (`sessionCanonicalState.ts` promotes a resting session with live + * background work back to the `running` phase). This set is the publisher's + * copy of the same fact — both derive from agentChatService's live + * background-task level — so Activity cannot publish "is done" over a session + * that is demonstrably still working. */ backgroundTaskIds: Set; /** @@ -160,8 +160,9 @@ export function agentAttentionPhase(run: AgentRunState): AttentionPhase { if (run.phase === "waiting_for_approval" || run.phase === "waiting_for_input") return "needs_you"; // Belt and braces over the `deferredTerminalPhase` state machine in // `onChatEvent`: whatever route left the run at a quiet phase, a session with - // live background subagents is Working. This mirrors the sidebar override in - // apps/desktop/src/shared/sessionStatusPresentation.ts — the two surfaces + // live background subagents is Working. This mirrors the phase promotion in + // apps/desktop/src/shared/sessionCanonicalState.ts, where live background + // work lifts a resting session back to `running` — the two surfaces // disagreeing about the same session is exactly the bug this guards. // `failed` is deliberately not overridden: a failure needs the user now, and // burying it under "working" would cost them the signal. diff --git a/apps/desktop/src/main/services/chat/agentChatService.test.ts b/apps/desktop/src/main/services/chat/agentChatService.test.ts index 8a283aed6..5b2fd041f 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.test.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.test.ts @@ -11777,6 +11777,76 @@ describe("createAgentChatService", () => { await expect(sendPromise).resolves.toBeUndefined(); }); + it("splits the background level into working and monitoring counts on the session summary", async () => { + // The classifier is a DENYLIST: `local_bash` is known-passive and reads + // as monitoring, while `local_agent` — and anything unrecognised — counts + // as working. An allowlist here would silently drop a real subagent the + // first time the SDK renamed a task type, which is the exact failure the + // whole liveness state exists to prevent. + const events: AgentChatEventEnvelope[] = []; + let streamCall = 0; + let warmupComplete = false; + let turnDone: (() => void) | null = null; + const turnDonePromise = new Promise((resolve) => { turnDone = resolve; }); + const send = vi.fn().mockResolvedValue(undefined); + const setPermissionMode = vi.fn().mockResolvedValue(undefined); + const stream = vi.fn(() => (async function* () { + streamCall += 1; + if (streamCall === 1) { + yield { type: "system", subtype: "init", session_id: "sdk-bgsplit-1", slash_commands: [] }; + warmupComplete = true; + yield { type: "result", usage: { input_tokens: 1, output_tokens: 1 } }; + return; + } + yield { + type: "system", + subtype: "background_tasks_changed", + tasks: [ + { task_id: "watch-ci", task_type: "local_bash", description: "Watch CI" }, + { task_id: "build-it", task_type: "local_agent", description: "Implement the feature" }, + { task_id: "who-knows", task_type: "some_future_sdk_type", description: "Unrecognised" }, + ], + }; + await turnDonePromise; + // The jobs finish on their own; the level is the authoritative drain. + yield { type: "system", subtype: "background_tasks_changed", tasks: [] }; + yield { type: "result", usage: { input_tokens: 1, output_tokens: 1 } }; + })()); + vi.mocked(claudeSdkCreateSessionCompat).mockReturnValue({ + send, stream, close: vi.fn(), sessionId: "sdk-bgsplit-1", setPermissionMode, + } as any); + const { service } = createService({ onEvent: (event: AgentChatEventEnvelope) => events.push(event) }); + const session = await service.createSession({ laneId: "lane-1", provider: "claude", model: "sonnet" }); + await vi.waitFor(() => { expect(warmupComplete).toBe(true); }); + const sendPromise = service.sendMessage({ sessionId: session.id, text: "kick off background work" }); + + await waitForEvent(events, (e): e is AgentChatEventEnvelope => + e.event.type === "scheduled_work_update" + && (e.event as any).id === "background:watch-ci" + && (e.event as any).status === "running"); + + const live = await service.getSessionSummary(session.id); + // Total stays the single number the mobile roster and push publisher read. + expect(live?.activeBackgroundTaskCount).toBe(3); + // Unknown types land in `working`, never in the quiet column. + expect(live?.backgroundWork).toEqual({ workingCount: 2, monitoringCount: 1 }); + + // A live turn is not ours to kill: its work is what the user can see + // happening, so settle teardown declines rather than stopping it. + await expect(service.stopBackgroundWork({ sessionId: session.id })) + .resolves.toEqual({ stopped: 0, skippedActiveTurn: true }); + + turnDone!(); + await expect(sendPromise).resolves.toBeUndefined(); + + // Background work outlives the turn by design, so the turn ending does not + // drain it — only the SDK's own empty level does. Once drained, the record + // is omitted entirely rather than riding along as a zero on every read. + const drained = await service.getSessionSummary(session.id); + expect(drained?.activeBackgroundTaskCount).toBe(0); + expect(drained?.backgroundWork).toBeUndefined(); + }); + it("uses the SDK background level to distinguish background and foreground local_bash tasks", async () => { // `local_bash` is the implementation kind for both foreground and // background Bash. Only the SDK's authoritative membership level makes diff --git a/apps/desktop/src/main/services/sessions/deleteTerminalSession.test.ts b/apps/desktop/src/main/services/sessions/deleteTerminalSession.test.ts deleted file mode 100644 index 62631ef02..000000000 --- a/apps/desktop/src/main/services/sessions/deleteTerminalSession.test.ts +++ /dev/null @@ -1,93 +0,0 @@ -import { describe, expect, it, vi } from "vitest"; -import type { TerminalSessionSummary } from "../../../shared/types"; -import { deleteTerminalSessionWithRuntimeCleanup } from "./deleteTerminalSession"; -import type { createPtyService } from "../pty/ptyService"; -import type { createSessionService } from "./sessionService"; - -function makeSession(overrides: Partial = {}): TerminalSessionSummary { - return { - id: "session-1", - laneId: "lane-1", - laneName: "Primary", - ptyId: null, - tracked: true, - pinned: false, - goal: null, - toolType: "shell", - title: "Shell", - status: "completed", - startedAt: "2026-08-01T00:00:00.000Z", - endedAt: "2026-08-01T00:01:00.000Z", - exitCode: 0, - transcriptPath: "/tmp/transcript", - headShaStart: null, - headShaEnd: null, - lastOutputPreview: null, - summary: null, - runtimeState: "exited", - resumeCommand: null, - ...overrides, - }; -} - -function makeServices(session: TerminalSessionSummary | null) { - const deleteSession = vi.fn().mockReturnValue(true); - const sessionService = { - get: vi.fn().mockReturnValue(session), - deleteSession, - } as unknown as ReturnType; - const ptyService = { - enrichSessions: vi.fn((sessions: TerminalSessionSummary[]) => sessions), - isSessionOwnedByLivePeerRuntime: vi.fn().mockReturnValue(false), - dispose: vi.fn(), - } as unknown as ReturnType; - return { deleteSession, ptyService, sessionService }; -} - -describe("deleteTerminalSessionWithRuntimeCleanup", () => { - it("deletes a session this runtime owns", () => { - const { deleteSession, ptyService, sessionService } = makeServices(makeSession()); - - expect(deleteTerminalSessionWithRuntimeCleanup({ - sessionId: "session-1", - sessionService, - ptyService, - })).toBe(true); - expect(deleteSession).toHaveBeenCalledWith("session-1"); - }); - - it("treats a session this runtime does not have as already deleted", () => { - // Delete is idempotent: the goal state is "not here", and it already holds. - // Throwing surfaced a red "Delete failed" banner over a list that was - // correct — the renderer routinely asks a runtime to delete a row that - // never persisted there, or that another window already removed. - const { deleteSession, ptyService, sessionService } = makeServices(null); - - expect(deleteTerminalSessionWithRuntimeCleanup({ - sessionId: "missing-session", - sessionService, - ptyService, - })).toBe(false); - expect(deleteSession).not.toHaveBeenCalled(); - }); - - it("still rejects an empty session id", () => { - const { ptyService, sessionService } = makeServices(makeSession()); - - expect(() => deleteTerminalSessionWithRuntimeCleanup({ - sessionId: " ", - sessionService, - ptyService, - })).toThrow("Session id is required."); - }); - - it("still refuses a chat session", () => { - const { ptyService, sessionService } = makeServices(makeSession({ toolType: "codex-chat" })); - - expect(() => deleteTerminalSessionWithRuntimeCleanup({ - sessionId: "session-1", - sessionService, - ptyService, - })).toThrow("Use the chat delete flow instead."); - }); -}); diff --git a/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.test.ts b/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts similarity index 61% rename from apps/desktop/src/main/services/sessions/sessionMachineryTeardown.test.ts rename to apps/desktop/src/main/services/sessions/sessionTeardown.test.ts index 3e0793965..54826c5a3 100644 --- a/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.test.ts +++ b/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts @@ -1,6 +1,20 @@ import { describe, expect, it, vi } from "vitest"; +import type { TerminalSessionSummary } from "../../../shared/types"; +import { deleteTerminalSessionWithRuntimeCleanup } from "./deleteTerminalSession"; import { stopSettledSessionMachinery } from "./sessionMachineryTeardown"; import { settleTerminalSession } from "./settleTerminalSession"; +import type { createPtyService } from "../pty/ptyService"; +import type { createSessionService } from "./sessionService"; + +/** + * End-of-life teardown for a session, in one place: what settle stops, what + * delete removes, and — the distinction the whole thing turns on — what each + * deliberately leaves alone. + * + * These three modules (`settleTerminalSession`, `sessionMachineryTeardown`, + * `deleteTerminalSession`) are one contract split across files for dependency + * reasons, not behavioral ones, so they are tested together. + */ type Row = { id: string; toolType: string }; @@ -144,3 +158,91 @@ describe("settleTerminalSession", () => { expect(sessionService.settleSession).toHaveBeenCalledWith("chat-1", { source: "user" }); }); }); + +function makeSession(overrides: Partial = {}): TerminalSessionSummary { + return { + id: "session-1", + laneId: "lane-1", + laneName: "Primary", + ptyId: null, + tracked: true, + pinned: false, + goal: null, + toolType: "shell", + title: "Shell", + status: "completed", + startedAt: "2026-08-01T00:00:00.000Z", + endedAt: "2026-08-01T00:01:00.000Z", + exitCode: 0, + transcriptPath: "/tmp/transcript", + headShaStart: null, + headShaEnd: null, + lastOutputPreview: null, + summary: null, + runtimeState: "exited", + resumeCommand: null, + ...overrides, + }; +} + +function makeServices(session: TerminalSessionSummary | null) { + const deleteSession = vi.fn().mockReturnValue(true); + const sessionService = { + get: vi.fn().mockReturnValue(session), + deleteSession, + } as unknown as ReturnType; + const ptyService = { + enrichSessions: vi.fn((sessions: TerminalSessionSummary[]) => sessions), + isSessionOwnedByLivePeerRuntime: vi.fn().mockReturnValue(false), + dispose: vi.fn(), + } as unknown as ReturnType; + return { deleteSession, ptyService, sessionService }; +} + +describe("deleteTerminalSessionWithRuntimeCleanup", () => { + it("deletes a session this runtime owns", () => { + const { deleteSession, ptyService, sessionService } = makeServices(makeSession()); + + expect(deleteTerminalSessionWithRuntimeCleanup({ + sessionId: "session-1", + sessionService, + ptyService, + })).toBe(true); + expect(deleteSession).toHaveBeenCalledWith("session-1"); + }); + + it("treats a session this runtime does not have as already deleted", () => { + // Delete is idempotent: the goal state is "not here", and it already holds. + // Throwing surfaced a red "Delete failed" banner over a list that was + // correct — the renderer routinely asks a runtime to delete a row that + // never persisted there, or that another window already removed. + const { deleteSession, ptyService, sessionService } = makeServices(null); + + expect(deleteTerminalSessionWithRuntimeCleanup({ + sessionId: "missing-session", + sessionService, + ptyService, + })).toBe(false); + expect(deleteSession).not.toHaveBeenCalled(); + }); + + it("still rejects an empty session id", () => { + const { ptyService, sessionService } = makeServices(makeSession()); + + expect(() => deleteTerminalSessionWithRuntimeCleanup({ + sessionId: " ", + sessionService, + ptyService, + })).toThrow("Session id is required."); + }); + + it("still refuses a chat session", () => { + const { ptyService, sessionService } = makeServices(makeSession({ toolType: "codex-chat" })); + + expect(() => deleteTerminalSessionWithRuntimeCleanup({ + sessionId: "session-1", + sessionService, + ptyService, + })).toThrow("Use the chat delete flow instead."); + }); +}); diff --git a/apps/desktop/src/renderer/components/lanes/laneAgents.test.ts b/apps/desktop/src/renderer/components/lanes/laneAgents.test.ts index db2031b84..b1df2a408 100644 --- a/apps/desktop/src/renderer/components/lanes/laneAgents.test.ts +++ b/apps/desktop/src/renderer/components/lanes/laneAgents.test.ts @@ -45,6 +45,61 @@ function cli(overrides: Partial): TerminalSessionSummary } describe("buildLaneAgents", () => { + it("keeps a resting agent live while its background work is", () => { + // The Lanes agent list read "idle" for the whole of a background fleet's + // run, because the turn had ended and nothing downstream looked at what the + // session still owned. + const [working, monitoring, quiet] = buildLaneAgents( + [ + chat({ + sessionId: "c-working", + status: "idle", + lastOutputPreview: "Turn finished", + activeBackgroundTaskCount: 2, + backgroundWork: { workingCount: 1, monitoringCount: 1 }, + }), + chat({ + sessionId: "c-monitoring", + status: "idle", + lastOutputPreview: "Turn finished", + activeBackgroundTaskCount: 1, + backgroundWork: { workingCount: 0, monitoringCount: 1 }, + }), + chat({ sessionId: "c-quiet", status: "idle", lastOutputPreview: "Turn finished" }), + ], + [], + ); + + // Live rows sort ahead of the genuinely idle one, working ahead of watching. + expect(working.sessionId).toBe("c-working"); + expect(working.activity).toBe("working"); + expect(monitoring.sessionId).toBe("c-monitoring"); + expect(monitoring.activity).toBe("monitoring"); + expect(quiet.activity).toBe("idle"); + + // The hint reports what is still running rather than the finished turn's + // stale preview. + expect(working.lastHint).toBe("2 background jobs still running"); + expect(monitoring.lastHint).toBe("1 monitor still running"); + expect(quiet.lastHint).toBe("Turn finished"); + }); + + it("counts a split-less CLI summary as working, and never over a live turn", () => { + // An older peer or a remote runtime mid-upgrade sends only the total. It + // must not be assumed passive, and a live turn's own output still wins. + const [resting] = buildLaneAgents([], [ + cli({ id: "t-resting", runtimeState: "waiting-input", activeBackgroundTaskCount: 3 }), + ]); + expect(resting.activity).toBe("working"); + expect(resting.lastHint).toBe("3 background jobs still running"); + + const [live] = buildLaneAgents([], [ + cli({ id: "t-live", runtimeState: "running", lastOutputPreview: "compiling", activeBackgroundTaskCount: 3 }), + ]); + expect(live.activity).toBe("working"); + expect(live.lastHint).toBe("compiling"); + }); + it("excludes plain shells", () => { const agents = buildLaneAgents( [], From 51f4d28fef825dbb6b18d66b5c4008a6f91164ce Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 9 Aug 2026 22:02:45 -0400 Subject: [PATCH 07/14] fix(sessions): give the settle pause an exact undo, and stop calling builds monitors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both P1s from Greptile on #1059, verified against the code before fixing. 1. Unsettle left schedules paused forever. The scheduler's session pause is PERSISTED, and settle took one while every unsettle path only cleared lifecycle columns — so a settled-then-unsettled chat kept its monitors, crons, and scheduled turns disabled indefinitely. A durable pause with no undo is just a slower deletion, and the docs already promised the undo. The scheduler now records which sessions settle paused (`settlePausedSessionIds`, persisted beside the pause it annotates). `setSessionPausedForSettle` claims a pause only when the user had not already taken one; `resumeSessionPausedForSettle` puts back exactly that and nothing else; an explicit user toggle drops settle's claim in either direction so a later unsettle cannot override their choice. Every unsettle entry point — registry single/bulk, both IPC handlers, both sync commands — now runs `resumeSettledSessionMachinery`, mirroring the settle wiring. Background work is deliberately not restarted: ADE cannot re-spawn a shell it stopped, and pretending otherwise is worse than leaving it quiet. 2. Generic backgrounded shells were classified as monitors. `local_bash` / `shell` / `background` / `bash` are how a provider says "the agent backgrounded a command" — a `tail -f` and a 20-minute `npm run build` arrive under the same type. Listing them labelled every background build "Monitoring", telling the user nothing was being produced while it was. Mixed is unknown, and this classifier's own stated rule is that unknown is working; including them contradicted it. MONITOR_TASK_TYPES is now only `monitor` / `monitor_mcp` — types whose whole job is to watch. Co-Authored-By: Claude Opus 5 --- .../services/sync/syncRemoteCommandService.ts | 27 +++++++- .../main/services/adeActions/registry.test.ts | 6 +- .../src/main/services/adeActions/registry.ts | 21 ++++++- .../services/chat/agentChatService.test.ts | 20 +++--- .../main/services/chat/agentChatService.ts | 22 +++++++ .../chat/chatScheduledWorkScheduler.ts | 48 ++++++++++++++ .../src/main/services/ipc/registerIpc.ts | 18 +++++- .../sessions/sessionMachineryTeardown.ts | 63 ++++++++++++++----- .../services/sessions/sessionTeardown.test.ts | 51 +++++++++++---- .../src/shared/sessionCanonicalState.test.ts | 18 ++++-- .../src/shared/sessionCanonicalState.ts | 13 ++-- .../features/terminals-and-sessions/README.md | 9 ++- 12 files changed, 259 insertions(+), 57 deletions(-) diff --git a/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts b/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts index 5ec1d4418..a4ee8b84c 100644 --- a/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts +++ b/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts @@ -278,7 +278,10 @@ import type { ProductAnalyticsService } from "../../../../desktop/src/main/servi import { parseProductAnalyticsCapture } from "../../../../desktop/src/shared/types/productAnalytics"; import { deleteTerminalSessionWithRuntimeCleanup } from "../../../../desktop/src/main/services/sessions/deleteTerminalSession"; import { dismissPendingInputBeforeSettle, settleTerminalSession } from "../../../../desktop/src/main/services/sessions/settleTerminalSession"; -import { stopSettledSessionMachinery } from "../../../../desktop/src/main/services/sessions/sessionMachineryTeardown"; +import { + resumeSettledSessionMachinery, + stopSettledSessionMachinery, +} from "../../../../desktop/src/main/services/sessions/sessionMachineryTeardown"; import type { createSessionDeltaService } from "../../../../desktop/src/main/services/sessions/sessionDeltaService"; import type { createSessionService } from "../../../../desktop/src/main/services/sessions/sessionService"; import { getSharedModelPickerStore, type ModelPickerStore } from "../modelPickerStore"; @@ -4100,7 +4103,16 @@ function registerWorkRemoteCommands({ args, register }: RemoteCommandRegistratio }); register("session.unsettleSession", { viewerAllowed: true, queueable: true }, async (payload) => { const sessionId = requireString(payload.sessionId, "session.unsettleSession requires sessionId."); - return { ok: args.sessionService.unsettleSession(sessionId), sessionId }; + const ok = args.sessionService.unsettleSession(sessionId); + await resumeSettledSessionMachinery( + { + sessionService: args.sessionService, + agentChatService: args.agentChatService ?? null, + logger: args.logger, + }, + [sessionId], + ); + return { ok, sessionId }; }); // Bulk settle. `dismissPendingInput` is OPTIONAL and additive: mobile sends // it for the "Dismiss & settle" row action (the same thing desktop passes to @@ -4142,7 +4154,16 @@ function registerWorkRemoteCommands({ args, register }: RemoteCommandRegistratio return args.sessionService.settleSessions(sessionIds); }); register("session.unsettleSessions", { viewerAllowed: true, queueable: true }, async (payload) => { - args.sessionService.unsettleSessions(parseRemoteSessionIds(payload, "session.unsettleSessions")); + const unsettleIds = parseRemoteSessionIds(payload, "session.unsettleSessions"); + args.sessionService.unsettleSessions(unsettleIds); + await resumeSettledSessionMachinery( + { + sessionService: args.sessionService, + agentChatService: args.agentChatService ?? null, + logger: args.logger, + }, + unsettleIds, + ); return { ok: true }; }); register("session.snoozeSession", { viewerAllowed: true, queueable: true }, async (payload) => { diff --git a/apps/desktop/src/main/services/adeActions/registry.test.ts b/apps/desktop/src/main/services/adeActions/registry.test.ts index 83db53104..97f58b664 100644 --- a/apps/desktop/src/main/services/adeActions/registry.test.ts +++ b/apps/desktop/src/main/services/adeActions/registry.test.ts @@ -1702,8 +1702,10 @@ describe("runtime session actions", () => { // The user-driven single-row unsettle (desktop row menu on a remote-bound // project, `ade code`'s /session unsettle) survives under a cto-gated name. - expect(sessionActions.unsettleSession({ sessionId: "session-1" })) - .toEqual({ ok: true, sessionId: "session-1" }); + // Awaited because unsettle now resumes exactly the scheduled work settle + // paused; the lifecycle write itself still happens synchronously first. + await expect(sessionActions.unsettleSession({ sessionId: "session-1" })) + .resolves.toEqual({ ok: true, sessionId: "session-1" }); expect(unsettleSession).toHaveBeenCalledWith("session-1"); }); diff --git a/apps/desktop/src/main/services/adeActions/registry.ts b/apps/desktop/src/main/services/adeActions/registry.ts index 360b87259..b0e59b733 100644 --- a/apps/desktop/src/main/services/adeActions/registry.ts +++ b/apps/desktop/src/main/services/adeActions/registry.ts @@ -126,7 +126,10 @@ import { parseWakeReason, } from "../sessions/sessionRequestValidation"; import { settleTerminalSession } from "../sessions/settleTerminalSession"; -import { stopSettledSessionMachinery } from "../sessions/sessionMachineryTeardown"; +import { + resumeSettledSessionMachinery, + stopSettledSessionMachinery, +} from "../sessions/sessionMachineryTeardown"; import { getSessionLifecycleSettings, setSessionLifecycleSettings, @@ -2144,7 +2147,13 @@ function buildSessionDomainService(runtime: AdeRuntime): OpaqueService | null { if (!sessionService.unsettleSession(sessionId)) { throw new Error(`Session '${sessionId}' was not found.`); } - return { ok: true, sessionId }; + return (async () => { + await resumeSettledSessionMachinery( + { sessionService, agentChatService: runtime.agentChatService, logger: runtime.logger }, + [sessionId], + ); + return { ok: true, sessionId }; + })(); }, // Bulk settle/unsettle for renderer surfaces on remote-bound projects // (mirrors deleteSession's generic trust posture). @@ -2195,7 +2204,13 @@ function buildSessionDomainService(runtime: AdeRuntime): OpaqueService | null { ? record.sessionIds.filter((id): id is string => typeof id === "string") : []; sessionService.unsettleSessions(sessionIds); - return { ok: true }; + return (async () => { + await resumeSettledSessionMachinery( + { sessionService, agentChatService: runtime.agentChatService, logger: runtime.logger }, + sessionIds, + ); + return { ok: true }; + })(); }, // ----------------------------------------------------------------------- // Snooze / wake / settle-override. Snooze is a synced VISIBILITY overlay: diff --git a/apps/desktop/src/main/services/chat/agentChatService.test.ts b/apps/desktop/src/main/services/chat/agentChatService.test.ts index 5b2fd041f..0ce8fc156 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.test.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.test.ts @@ -11778,11 +11778,11 @@ describe("createAgentChatService", () => { }); it("splits the background level into working and monitoring counts on the session summary", async () => { - // The classifier is a DENYLIST: `local_bash` is known-passive and reads - // as monitoring, while `local_agent` — and anything unrecognised — counts - // as working. An allowlist here would silently drop a real subagent the - // first time the SDK renamed a task type, which is the exact failure the - // whole liveness state exists to prevent. + // The classifier is a DENYLIST: only a type whose whole job is to watch + // (`monitor`) reads as monitoring. A generic backgrounded shell, a real + // subagent, and anything unrecognised all count as working — an allowlist + // would silently drop a real subagent the first time the SDK renamed a + // task type, which is the exact failure this state exists to prevent. const events: AgentChatEventEnvelope[] = []; let streamCall = 0; let warmupComplete = false; @@ -11802,7 +11802,8 @@ describe("createAgentChatService", () => { type: "system", subtype: "background_tasks_changed", tasks: [ - { task_id: "watch-ci", task_type: "local_bash", description: "Watch CI" }, + { task_id: "watch-ci", task_type: "monitor", description: "Watch CI" }, + { task_id: "run-build", task_type: "local_bash", description: "npm run build" }, { task_id: "build-it", task_type: "local_agent", description: "Implement the feature" }, { task_id: "who-knows", task_type: "some_future_sdk_type", description: "Unrecognised" }, ], @@ -11827,9 +11828,10 @@ describe("createAgentChatService", () => { const live = await service.getSessionSummary(session.id); // Total stays the single number the mobile roster and push publisher read. - expect(live?.activeBackgroundTaskCount).toBe(3); - // Unknown types land in `working`, never in the quiet column. - expect(live?.backgroundWork).toEqual({ workingCount: 2, monitoringCount: 1 }); + expect(live?.activeBackgroundTaskCount).toBe(4); + // Unknown types — and a generic backgrounded build — land in `working`, + // never in the quiet column. + expect(live?.backgroundWork).toEqual({ workingCount: 3, monitoringCount: 1 }); // A live turn is not ours to kill: its work is what the user can see // happening, so settle teardown declines rather than stopping it. diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index 386340dcd..09ec1415a 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -39423,6 +39423,27 @@ export function createAgentChatService(args: { return { stopped: Math.max(0, before - after), skippedActiveTurn: false }; }; + /** + * Pause / resume a session's scheduled work as part of settle teardown. + * + * Separate from `setScheduledWorkPaused` (the user-facing toggle) because the + * two must not fight: settle claims a pause only when the user had not + * already taken one, and unsettle puts back exactly what settle took. Without + * the resume half, a settled-then-unsettled chat kept its monitors, crons, + * and scheduled turns disabled forever. + */ + const setScheduledWorkPausedForSettle = async ( + { sessionId, paused }: { sessionId: string; paused: boolean }, + ): Promise => { + const normalizedSessionId = sessionId.trim(); + if (!normalizedSessionId) return false; + await scheduledWorkReady; + if (!scheduledWorkScheduler) return false; + return paused + ? scheduledWorkScheduler.setSessionPausedForSettle(normalizedSessionId) + : scheduledWorkScheduler.resumeSessionPausedForSettle(normalizedSessionId); + }; + const hasActiveWorkloads = (): boolean => { for (const managed of managedSessions.values()) { if (managed.closed || managed.deleted) continue; @@ -44305,6 +44326,7 @@ export function createAgentChatService(args: { ensureSessionSurface, hasActiveWorkloads, stopBackgroundWork, + setScheduledWorkPausedForSettle, hasRetainableSessions, countActiveForLane, disposeForLane, diff --git a/apps/desktop/src/main/services/chat/chatScheduledWorkScheduler.ts b/apps/desktop/src/main/services/chat/chatScheduledWorkScheduler.ts index f478af302..5073adcf2 100644 --- a/apps/desktop/src/main/services/chat/chatScheduledWorkScheduler.ts +++ b/apps/desktop/src/main/services/chat/chatScheduledWorkScheduler.ts @@ -46,6 +46,16 @@ export type ChatScheduledWorkState = { version: 1; schedules: ChatScheduledWorkRecord[]; pausedSessionIds: string[]; + /** + * Sessions whose pause was taken by SETTLE TEARDOWN rather than by the user. + * + * Persisted beside the pause itself because it is the exact undo record: + * settle pauses a session's schedules so a monitor cannot wake a thread the + * user has declared done, and unsettle has to put back precisely what settle + * took — never a pause the user set deliberately, and never nothing at all, + * which would leave the schedules disabled forever. + */ + settlePausedSessionIds?: string[]; }; export type ChatScheduledWorkUpsert = Omit< @@ -88,6 +98,10 @@ export type ChatScheduledWorkScheduler = { cancel(scheduleId: string): Promise; setSchedulePaused(scheduleId: string, paused: boolean): Promise; setSessionPaused(sessionId: string, paused: boolean): Promise; + /** Pause on settle, claiming the pause only if the user had not already taken one. Returns whether it paused. */ + setSessionPausedForSettle(sessionId: string): Promise; + /** Undo exactly what `setSessionPausedForSettle` did, and nothing else. Returns whether it resumed. */ + resumeSessionPausedForSettle(sessionId: string): Promise; refreshGlobalPause(): Promise; list(sessionId?: string): ChatScheduledWorkRecord[]; isSessionPaused(sessionId: string): boolean; @@ -188,10 +202,18 @@ function normalizeState(value: unknown): ChatScheduledWorkState { const pausedSessionIds = Array.isArray(record?.pausedSessionIds) ? record.pausedSessionIds.filter((item): item is string => typeof item === "string" && item.length > 0) : []; + const settlePausedSessionIds = Array.isArray(record?.settlePausedSessionIds) + ? record.settlePausedSessionIds.filter((item): item is string => typeof item === "string" && item.length > 0) + : []; return { version: 1, schedules, pausedSessionIds: [...new Set(pausedSessionIds)], + // Only meaningful for sessions that are actually paused; a stale marker + // for an already-resumed session would resume nothing but would linger. + settlePausedSessionIds: [...new Set(settlePausedSessionIds)].filter( + (sessionId) => pausedSessionIds.includes(sessionId), + ), }; } @@ -206,6 +228,7 @@ export function createChatScheduledWorkScheduler( const timers = options.timers ?? defaultTimers; const schedules = new Map(); const pausedSessionIds = new Set(); + const settlePausedSessionIds = new Set(); const timerHandles = new Map(); const inFlight = new Set(); let started = false; @@ -219,6 +242,7 @@ export function createChatScheduledWorkScheduler( .sort((left, right) => left.createdAt - right.createdAt || left.id.localeCompare(right.id)) .map(cloneSchedule), pausedSessionIds: [...pausedSessionIds].sort(), + settlePausedSessionIds: [...settlePausedSessionIds].sort(), }); const persist = async (): Promise => { @@ -495,6 +519,7 @@ export function createChatScheduledWorkScheduler( const state = normalizeState(await options.loadState()); schedules.clear(); pausedSessionIds.clear(); + settlePausedSessionIds.clear(); let migrated = false; for (const schedule of state.schedules) { // Pre-1.2.27 builds persisted cron-tool placeholders before Claude @@ -526,6 +551,7 @@ export function createChatScheduledWorkScheduler( schedules.set(schedule.id, schedule); } for (const sessionId of state.pausedSessionIds) pausedSessionIds.add(sessionId); + for (const sessionId of state.settlePausedSessionIds ?? []) settlePausedSessionIds.add(sessionId); migrated = pruneTerminalHistory() || migrated; started = true; for (const schedule of schedules.values()) await reconcileSchedule(schedule); @@ -626,7 +652,29 @@ export function createChatScheduledWorkScheduler( await start(); if (paused) pausedSessionIds.add(sessionId); else pausedSessionIds.delete(sessionId); + // An explicit choice by the user replaces settle's claim on this pause, + // in either direction: a later unsettle must not undo what they just did. + settlePausedSessionIds.delete(sessionId); + await updatePauseStatuses(sessionId); + }, + + async setSessionPausedForSettle(sessionId): Promise { + await start(); + // Already paused by the user — leave it, and take no claim on it, so + // unsettle cannot resume schedules they deliberately stopped. + if (pausedSessionIds.has(sessionId)) return false; + pausedSessionIds.add(sessionId); + settlePausedSessionIds.add(sessionId); + await updatePauseStatuses(sessionId); + return true; + }, + + async resumeSessionPausedForSettle(sessionId): Promise { + await start(); + if (!settlePausedSessionIds.delete(sessionId)) return false; + pausedSessionIds.delete(sessionId); await updatePauseStatuses(sessionId); + return true; }, async refreshGlobalPause(): Promise { diff --git a/apps/desktop/src/main/services/ipc/registerIpc.ts b/apps/desktop/src/main/services/ipc/registerIpc.ts index fc537a273..015c75c45 100644 --- a/apps/desktop/src/main/services/ipc/registerIpc.ts +++ b/apps/desktop/src/main/services/ipc/registerIpc.ts @@ -51,7 +51,10 @@ import { parseWakeReason, } from "../sessions/sessionRequestValidation"; import { settleTerminalSession } from "../sessions/settleTerminalSession"; -import { stopSettledSessionMachinery } from "../sessions/sessionMachineryTeardown"; +import { + resumeSettledSessionMachinery, + stopSettledSessionMachinery, +} from "../sessions/sessionMachineryTeardown"; import { getSessionLifecycleSettings, setSessionLifecycleSettings, @@ -6980,6 +6983,10 @@ export function registerIpc({ const sessionId = typeof arg?.sessionId === "string" ? arg.sessionId.trim() : ""; if (!sessionId) throw new Error("Session id is required."); ctx.sessionService.unsettleSession(sessionId); + await resumeSettledSessionMachinery( + { sessionService: ctx.sessionService, agentChatService: ctx.agentChatService, logger: ctx.logger }, + [sessionId], + ); }, ); @@ -7004,8 +7011,13 @@ export function registerIpc({ async (_event, arg: { sessionIds?: unknown }): Promise => { const ctx = ensureSessionContext(); if (!Array.isArray(arg?.sessionIds)) throw new Error("Session ids are required."); - ctx.sessionService.unsettleSessions( - arg.sessionIds.filter((sessionId): sessionId is string => typeof sessionId === "string"), + const unsettleIds = arg.sessionIds.filter( + (sessionId): sessionId is string => typeof sessionId === "string", + ); + ctx.sessionService.unsettleSessions(unsettleIds); + await resumeSettledSessionMachinery( + { sessionService: ctx.sessionService, agentChatService: ctx.agentChatService, logger: ctx.logger }, + unsettleIds, ); }, ); diff --git a/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts b/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts index be66dda89..5a29d4e48 100644 --- a/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts +++ b/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts @@ -42,7 +42,7 @@ export type SessionMachineryTeardownDeps = { sessionService: Pick, "get">; agentChatService?: Pick< ReturnType, - "stopBackgroundWork" | "setScheduledWorkPaused" | "listScheduledWork" + "stopBackgroundWork" | "setScheduledWorkPausedForSettle" | "listScheduledWork" > | null; logger?: { warn: (message: string, meta?: Record) => void } | null; }; @@ -66,31 +66,38 @@ const EMPTY_RESULT: SessionMachineryTeardownResult = { }; /** - * Pause rather than cancel. + * Pause rather than cancel, and pause REVERSIBLY. * * Cancelling would be destructive and irreversible: a settle that turns out to - * be premature (the user unsettles, or new activity un-settles the row) would - * have silently deleted schedules the user set up by hand. Pausing stops the - * 3am wake and survives being undone, which is the whole difference between - * "this is filed" and "this is deleted". + * be premature would have silently deleted schedules the user set up by hand. + * + * The pause is durable, so it needs an exact undo or it is just a slower + * deletion — a settled-then-unsettled chat would keep its monitors, crons, and + * scheduled turns disabled forever. `setScheduledWorkPausedForSettle` claims + * the pause only when the user had not already taken one, and + * `resumeSettledSessionMachinery` puts back precisely what it claimed. See + * `chatScheduledWorkScheduler`'s `settlePausedSessionIds`. */ -async function pauseScheduledWork( +async function setScheduledWorkPaused( deps: SessionMachineryTeardownDeps, sessionId: string, + paused: boolean, ): Promise { const service = deps.agentChatService; if (!service) return false; try { - const schedules = await service.listScheduledWork({ sessionId }); - const armed = schedules.some( - (schedule) => schedule.status !== "completed" && schedule.status !== "cancelled", - ); - if (!armed) return false; - await service.setScheduledWorkPaused({ sessionId, paused: true }); - return true; + if (paused) { + const schedules = await service.listScheduledWork({ sessionId }); + const armed = schedules.some( + (schedule) => schedule.status !== "completed" && schedule.status !== "cancelled", + ); + if (!armed) return false; + } + return await service.setScheduledWorkPausedForSettle({ sessionId, paused }); } catch (error) { - deps.logger?.warn("session_teardown.pause_scheduled_work_failed", { + deps.logger?.warn("session_teardown.scheduled_work_pause_failed", { sessionId, + paused, error: error instanceof Error ? error.message : String(error), }); return false; @@ -126,7 +133,7 @@ export async function stopSettledSessionMachinery( if (!row || !isChatToolType(row.toolType)) continue; result.sessionIds.push(sessionId); - if (await pauseScheduledWork(deps, sessionId)) result.pausedScheduledWork += 1; + if (await setScheduledWorkPaused(deps, sessionId, true)) result.pausedScheduledWork += 1; const service = deps.agentChatService; if (!service) continue; @@ -144,3 +151,27 @@ export async function stopSettledSessionMachinery( return result; } + +/** + * The undo half of settle teardown, run by every unsettle path. + * + * Only resumes schedules that settle itself paused — a pause the user took + * deliberately survives an unsettle untouched. Background work is deliberately + * NOT restarted: ADE cannot re-spawn a shell or a subagent fleet it stopped, + * and pretending otherwise would be worse than leaving the session quiet. + */ +export async function resumeSettledSessionMachinery( + deps: SessionMachineryTeardownDeps, + sessionIds: readonly string[], +): Promise<{ sessionIds: string[]; resumedScheduledWork: number }> { + const unique = [...new Set(sessionIds.map((id) => id.trim()).filter(Boolean))]; + const touched: string[] = []; + let resumedScheduledWork = 0; + for (const sessionId of unique) { + const row = deps.sessionService.get(sessionId); + if (!row || !isChatToolType(row.toolType)) continue; + touched.push(sessionId); + if (await setScheduledWorkPaused(deps, sessionId, false)) resumedScheduledWork += 1; + } + return { sessionIds: touched, resumedScheduledWork }; +} diff --git a/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts b/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts index 54826c5a3..1b9e133fb 100644 --- a/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts +++ b/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts @@ -1,7 +1,10 @@ import { describe, expect, it, vi } from "vitest"; import type { TerminalSessionSummary } from "../../../shared/types"; import { deleteTerminalSessionWithRuntimeCleanup } from "./deleteTerminalSession"; -import { stopSettledSessionMachinery } from "./sessionMachineryTeardown"; +import { + resumeSettledSessionMachinery, + stopSettledSessionMachinery, +} from "./sessionMachineryTeardown"; import { settleTerminalSession } from "./settleTerminalSession"; import type { createPtyService } from "../pty/ptyService"; import type { createSessionService } from "./sessionService"; @@ -20,11 +23,7 @@ type Row = { id: string; toolType: string }; function deps(rows: Row[], overrides: Record = {}) { const stopBackgroundWork = vi.fn(async () => ({ stopped: 2, skippedActiveTurn: false })); - const setScheduledWorkPaused = vi.fn(async ({ sessionId }: { sessionId: string }) => ({ - sessionId, - paused: true, - nextWakeAt: null, - })); + const setScheduledWorkPausedForSettle = vi.fn(async () => true); const listScheduledWork = vi.fn(async () => [{ id: "sched-1", status: "scheduled" }]); return { sessionService: { @@ -32,13 +31,13 @@ function deps(rows: Row[], overrides: Record = {}) { } as never, agentChatService: { stopBackgroundWork, - setScheduledWorkPaused, + setScheduledWorkPausedForSettle, listScheduledWork, ...overrides, } as never, logger: { warn: vi.fn() }, stopBackgroundWork, - setScheduledWorkPaused, + setScheduledWorkPausedForSettle, listScheduledWork, }; } @@ -51,7 +50,7 @@ describe("stopSettledSessionMachinery", () => { const result = await stopSettledSessionMachinery(d, ["chat-1"]); expect(d.stopBackgroundWork).toHaveBeenCalledWith({ sessionId: "chat-1" }); - expect(d.setScheduledWorkPaused).toHaveBeenCalledWith({ sessionId: "chat-1", paused: true }); + expect(d.setScheduledWorkPausedForSettle).toHaveBeenCalledWith({ sessionId: "chat-1", paused: true }); expect(result).toMatchObject({ sessionIds: ["chat-1"], stoppedBackgroundWork: 2, @@ -65,11 +64,41 @@ describe("stopSettledSessionMachinery", () => { cancelScheduledWork: vi.fn(), }); await stopSettledSessionMachinery(d, ["chat-1"]); - expect(d.setScheduledWorkPaused).toHaveBeenCalledTimes(1); + expect(d.setScheduledWorkPausedForSettle).toHaveBeenCalledTimes(1); expect((d.agentChatService as unknown as { cancelScheduledWork: ReturnType }) .cancelScheduledWork).not.toHaveBeenCalled(); }); + it("resumes on unsettle what settle paused — a durable pause needs an exact undo", async () => { + // A pause with no undo is just a slower deletion: before this, a settled + // then unsettled chat kept its monitors, crons, and scheduled turns + // disabled forever, because unsettle only cleared lifecycle columns. + const d = deps([{ id: "chat-1", toolType: "claude-chat" }]); + await stopSettledSessionMachinery(d, ["chat-1"]); + const resumed = await resumeSettledSessionMachinery(d, ["chat-1"]); + + expect(d.setScheduledWorkPausedForSettle).toHaveBeenNthCalledWith(1, { sessionId: "chat-1", paused: true }); + expect(d.setScheduledWorkPausedForSettle).toHaveBeenNthCalledWith(2, { sessionId: "chat-1", paused: false }); + expect(resumed).toEqual({ sessionIds: ["chat-1"], resumedScheduledWork: 1 }); + }); + + it("does not resume a pause settle never took", async () => { + // The scheduler refuses the claim when the user had already paused, and + // reports false — an unsettle must not restart schedules they stopped. + const d = deps([{ id: "chat-1", toolType: "claude-chat" }], { + setScheduledWorkPausedForSettle: vi.fn(async () => false), + }); + const resumed = await resumeSettledSessionMachinery(d, ["chat-1"]); + expect(resumed.resumedScheduledWork).toBe(0); + }); + + it("leaves terminals alone on the resume path too", async () => { + const d = deps([{ id: "term-1", toolType: "shell" }]); + const resumed = await resumeSettledSessionMachinery(d, ["term-1"]); + expect(resumed.sessionIds).toEqual([]); + expect(d.setScheduledWorkPausedForSettle).not.toHaveBeenCalled(); + }); + it("leaves terminal sessions alone — a terminal pane is user-owned", async () => { // The whole carve-out of settle teardown: an agent's background shell is // thread background work, but the pane the user opened to watch a build is @@ -82,7 +111,7 @@ describe("stopSettledSessionMachinery", () => { expect(result.sessionIds).toEqual([]); expect(d.stopBackgroundWork).not.toHaveBeenCalled(); - expect(d.setScheduledWorkPaused).not.toHaveBeenCalled(); + expect(d.setScheduledWorkPausedForSettle).not.toHaveBeenCalled(); }); it("reports a session skipped because its foreground turn is still streaming", async () => { diff --git a/apps/desktop/src/shared/sessionCanonicalState.test.ts b/apps/desktop/src/shared/sessionCanonicalState.test.ts index a694f88c8..af2fc8131 100644 --- a/apps/desktop/src/shared/sessionCanonicalState.test.ts +++ b/apps/desktop/src/shared/sessionCanonicalState.test.ts @@ -376,11 +376,21 @@ describe("classifyBackgroundWorkKind", () => { }); it("classifies only the known-passive types as monitoring", () => { - for (const taskType of ["monitor", "monitor_mcp", "local_bash", "shell", "background", "bash"]) { + for (const taskType of ["monitor", "monitor_mcp"]) { expect(classifyBackgroundWorkKind(taskType)).toBe("monitoring"); } expect(classifyBackgroundWorkKind("MONITOR")).toBe("monitoring"); - expect(classifyBackgroundWorkKind(" local_bash ")).toBe("monitoring"); + expect(classifyBackgroundWorkKind(" monitor_mcp ")).toBe("monitoring"); + }); + + it("counts a generic backgrounded shell as working, not monitoring", () => { + // These types mean "the agent backgrounded a command" — a `tail -f` and a + // 20-minute build arrive under the same one. Mixed is unknown, and unknown + // is working; labelling them monitoring told the user nothing was being + // produced while a build was running. + for (const taskType of ["local_bash", "shell", "background", "bash"]) { + expect(classifyBackgroundWorkKind(taskType)).toBe("working"); + } }); it("drops inert types entirely", () => { @@ -391,8 +401,8 @@ describe("classifyBackgroundWorkKind", () => { it("folds a mixed list into the two-state count", () => { expect(summarizeBackgroundWork(["subagent", "monitor", "local_bash", "plan", null])).toEqual({ - workingCount: 2, - monitoringCount: 2, + workingCount: 3, + monitoringCount: 1, }); }); }); diff --git a/apps/desktop/src/shared/sessionCanonicalState.ts b/apps/desktop/src/shared/sessionCanonicalState.ts index 856e04ddb..943f56f32 100644 --- a/apps/desktop/src/shared/sessionCanonicalState.ts +++ b/apps/desktop/src/shared/sessionCanonicalState.ts @@ -133,14 +133,19 @@ const BADGE_BY_KIND: Record = { * unrecognised type as working. A new SDK task type shows up as "Working", * which is at worst slightly over-loud and at best exactly right. Adding a name * here is a deliberate act with a known job behind it. + * + * ── Why the generic shell types are NOT here ──────────────────────────────── + * + * `local_bash` / `shell` / `background` / `bash` are how a provider reports + * "the agent backgrounded a command". That is a MIXED bag, not a passive one: a + * `tail -f` and a 20-minute `npm run build` arrive under the same type. Listing + * them here labelled every background build "Monitoring" — telling the user + * nothing was being produced while it was. Mixed is unknown, and by the rule + * above unknown is working. Only types whose whole job is to watch belong here. */ const MONITOR_TASK_TYPES: ReadonlySet = new Set([ "monitor", "monitor_mcp", - "local_bash", - "shell", - "background", - "bash", ]); /** diff --git a/docs/features/terminals-and-sessions/README.md b/docs/features/terminals-and-sessions/README.md index c59153794..dc27ac120 100644 --- a/docs/features/terminals-and-sessions/README.md +++ b/docs/features/terminals-and-sessions/README.md @@ -208,8 +208,13 @@ and in tests. registry. Settle stops the machinery the session owns before it writes the lifecycle column — see `apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts`. It - pauses the session's scheduled work (pauses rather than cancels, so an - unsettle can bring hand-made schedules back) and calls + pauses the session's scheduled work — pauses rather than cancels, and the + pause is durable, so it carries an exact undo: the scheduler records which + sessions **settle** paused (`settlePausedSessionIds`) and every unsettle path + runs `resumeSettledSessionMachinery`, which resumes only those. A pause the + user took deliberately is never claimed and never resumed, and background + work is never restarted — ADE cannot re-spawn a shell it stopped. It also + calls `agentChatService.stopBackgroundWork`, which stops every live child before the parent. **Terminal panes stay open**: an agent's background shell is thread background work, but a pane the user opened is theirs, and closing it From da00d7afb5d1e0d7f44bcdaa99a5dedfe470c4ca Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 9 Aug 2026 22:16:07 -0400 Subject: [PATCH 08/14] test(prs): update the PR-merge teardown mock for the settle-scoped pause API Missed when setScheduledWorkPaused was split into the settle-scoped setScheduledWorkPausedForSettle; the user-facing toggle keeps its old name and its own callers, which is why only the teardown mocks move. Co-Authored-By: Claude Opus 5 --- apps/desktop/src/main/services/prs/prAsync.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/desktop/src/main/services/prs/prAsync.test.ts b/apps/desktop/src/main/services/prs/prAsync.test.ts index f0fe8bb23..9f273fc2b 100644 --- a/apps/desktop/src/main/services/prs/prAsync.test.ts +++ b/apps/desktop/src/main/services/prs/prAsync.test.ts @@ -927,9 +927,9 @@ describe("prMergeAutoSettlementService", () => { order.push("stop"); return { stopped: 1, skippedActiveTurn: false }; }); - const setScheduledWorkPaused = vi.fn(async ({ sessionId }: { sessionId: string }) => { + const setScheduledWorkPausedForSettle = vi.fn(async () => { order.push("pause"); - return { sessionId, paused: true, nextWakeAt: null }; + return true; }); const rows = [{ id: "chat-live", toolType: "claude-chat", archivedAt: null, settledAt: null }]; const service = createPrMergeAutoSettlementService({ @@ -941,7 +941,7 @@ describe("prMergeAutoSettlementService", () => { } as any, agentChatService: { stopBackgroundWork, - setScheduledWorkPaused, + setScheduledWorkPausedForSettle, listScheduledWork: vi.fn(async () => [{ id: "sched-1", status: "scheduled" }]), } as any, emitEvent: vi.fn(), @@ -957,7 +957,7 @@ describe("prMergeAutoSettlementService", () => { }); expect(stopBackgroundWork).toHaveBeenCalledWith({ sessionId: "chat-live" }); - expect(setScheduledWorkPaused).toHaveBeenCalledWith({ sessionId: "chat-live", paused: true }); + expect(setScheduledWorkPausedForSettle).toHaveBeenCalledWith({ sessionId: "chat-live", paused: true }); expect(order).toEqual(["pause", "stop", "settle"]); }); From 53fee04fb45c96ad3c6b47aeeba0a9e13d3310a6 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 9 Aug 2026 22:48:31 -0400 Subject: [PATCH 09/14] fix(sessions): resume at the settle-clearing write, stop bypasses and false stops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three P1s from the #1059 re-review — Greptile and Codex independently found the first, which is the highest-signal one. 1. Activity-driven unsettle kept schedules paused (Greptile + Codex). Wiring the resume into each unsettle caller missed the most common unsettle of all: a user sending the next message, which clears settled_at through clearTurnStartMarkers. The chat went active while its monitors, crons, and scheduled turns stayed paused across restarts. The resume now hangs off sessionService's new onSettleCleared hook, fired by every route that clears the column — unsettleSession, unsettleSessions, and clearTurnStartMarkers. Per-caller wiring in the registry, both IPC handlers, and both sync commands is deleted as redundant, so a future caller cannot reintroduce the gap. Settle itself stays explicit per entry point because its teardown has to finish before the write. 2. CTO operator settle bypassed teardown entirely (Codex). createCtoOperatorTools called sessionService.settleSession directly, so a CTO-filed chat kept its schedules armed and its background fleet spending. It now runs the shared teardown first, like every other settle entry point. 3. A failed stop still closed the task row (Codex). When stopTask was missing, timed out, or rejected, the emitted terminal row dropped the task from liveBackgroundTaskIds — which is exactly what the caller measures the stop against, so a stop that did not happen was counted as one that did. The task now stays LIVE on failure; the SDK's next authoritative level drains it if it really ended. Under-reporting is the only safe direction here. Co-Authored-By: Claude Opus 5 --- apps/ade-cli/src/bootstrap.ts | 24 ++++++- .../services/sync/syncRemoteCommandService.ts | 27 +------- apps/desktop/src/main/main.ts | 17 ++++- .../main/services/adeActions/registry.test.ts | 9 +-- .../src/main/services/adeActions/registry.ts | 21 +------ .../services/ai/tools/ctoOperatorTools.ts | 22 +++++++ .../main/services/chat/agentChatService.ts | 63 ++++++++++--------- .../src/main/services/ipc/registerIpc.ts | 13 +--- .../services/sessions/sessionService.test.ts | 43 +++++++++++++ .../main/services/sessions/sessionService.ts | 41 +++++++++++- .../features/terminals-and-sessions/README.md | 15 +++-- 11 files changed, 196 insertions(+), 99 deletions(-) diff --git a/apps/ade-cli/src/bootstrap.ts b/apps/ade-cli/src/bootstrap.ts index 723e9218b..97920764c 100644 --- a/apps/ade-cli/src/bootstrap.ts +++ b/apps/ade-cli/src/bootstrap.ts @@ -48,6 +48,7 @@ import { createLaneTemplateService } from "../../desktop/src/main/services/lanes import { createPortAllocationService } from "../../desktop/src/main/services/lanes/portAllocationService"; import { createLaneProxyService } from "../../desktop/src/main/services/lanes/laneProxyService"; import { releaseLaneRuntimeResources } from "../../desktop/src/main/services/lanes/laneRuntimeLifecycle"; +import { resumeSettledSessionMachinery } from "../../desktop/src/main/services/sessions/sessionMachineryTeardown"; import { createOAuthRedirectService } from "../../desktop/src/main/services/lanes/oauthRedirectService"; import { createRuntimeDiagnosticsService } from "../../desktop/src/main/services/lanes/runtimeDiagnosticsService"; import { createRebaseSuggestionService } from "../../desktop/src/main/services/lanes/rebaseSuggestionService"; @@ -755,7 +756,27 @@ export async function createAdeRuntime(args: { // services. Session changes still use it once publishing is attached. let pushPublisherForPtySignals: PushPublisherService | null = null; let ptyServiceForSessionChanges: ReturnType | null = null; - const sessionService = createSessionService({ db }); + let sessionServiceRef: ReturnType | null = null; + // Late-bound for the same reason as the push publisher above: the chat + // service is constructed well after the session service that calls back into + // it when a settle is cleared. + let agentChatServiceForSettleResume: + | ReturnType + | null = null; + const sessionService = createSessionService({ + db, + // Resume exactly the scheduled work settle paused, on every route that + // clears a settle — explicit unsettle and turn-start activity alike. + onSettleCleared: (sessionId) => { + const chat = agentChatServiceForSettleResume; + if (!chat) return; + void resumeSettledSessionMachinery( + { sessionService: sessionServiceRef!, agentChatService: chat, logger }, + [sessionId], + ).catch(() => {}); + }, + }); + sessionServiceRef = sessionService; sessionService.onChanged((event) => { pushEvent("runtime", { type: "terminal_session_changed", event }); const session = sessionService.get(event.sessionId); @@ -1431,6 +1452,7 @@ export async function createAdeRuntime(args: { prService: headlessLinearServices.prService, aiIntegrationService, }); + agentChatServiceForSettleResume = agentChatService; const prMergeAutoSettlementService = createPrMergeAutoSettlementService({ db, sessionService, diff --git a/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts b/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts index a4ee8b84c..5ec1d4418 100644 --- a/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts +++ b/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts @@ -278,10 +278,7 @@ import type { ProductAnalyticsService } from "../../../../desktop/src/main/servi import { parseProductAnalyticsCapture } from "../../../../desktop/src/shared/types/productAnalytics"; import { deleteTerminalSessionWithRuntimeCleanup } from "../../../../desktop/src/main/services/sessions/deleteTerminalSession"; import { dismissPendingInputBeforeSettle, settleTerminalSession } from "../../../../desktop/src/main/services/sessions/settleTerminalSession"; -import { - resumeSettledSessionMachinery, - stopSettledSessionMachinery, -} from "../../../../desktop/src/main/services/sessions/sessionMachineryTeardown"; +import { stopSettledSessionMachinery } from "../../../../desktop/src/main/services/sessions/sessionMachineryTeardown"; import type { createSessionDeltaService } from "../../../../desktop/src/main/services/sessions/sessionDeltaService"; import type { createSessionService } from "../../../../desktop/src/main/services/sessions/sessionService"; import { getSharedModelPickerStore, type ModelPickerStore } from "../modelPickerStore"; @@ -4103,16 +4100,7 @@ function registerWorkRemoteCommands({ args, register }: RemoteCommandRegistratio }); register("session.unsettleSession", { viewerAllowed: true, queueable: true }, async (payload) => { const sessionId = requireString(payload.sessionId, "session.unsettleSession requires sessionId."); - const ok = args.sessionService.unsettleSession(sessionId); - await resumeSettledSessionMachinery( - { - sessionService: args.sessionService, - agentChatService: args.agentChatService ?? null, - logger: args.logger, - }, - [sessionId], - ); - return { ok, sessionId }; + return { ok: args.sessionService.unsettleSession(sessionId), sessionId }; }); // Bulk settle. `dismissPendingInput` is OPTIONAL and additive: mobile sends // it for the "Dismiss & settle" row action (the same thing desktop passes to @@ -4154,16 +4142,7 @@ function registerWorkRemoteCommands({ args, register }: RemoteCommandRegistratio return args.sessionService.settleSessions(sessionIds); }); register("session.unsettleSessions", { viewerAllowed: true, queueable: true }, async (payload) => { - const unsettleIds = parseRemoteSessionIds(payload, "session.unsettleSessions"); - args.sessionService.unsettleSessions(unsettleIds); - await resumeSettledSessionMachinery( - { - sessionService: args.sessionService, - agentChatService: args.agentChatService ?? null, - logger: args.logger, - }, - unsettleIds, - ); + args.sessionService.unsettleSessions(parseRemoteSessionIds(payload, "session.unsettleSessions")); return { ok: true }; }); register("session.snoozeSession", { viewerAllowed: true, queueable: true }, async (payload) => { diff --git a/apps/desktop/src/main/main.ts b/apps/desktop/src/main/main.ts index f07806c2c..6a1b9061e 100644 --- a/apps/desktop/src/main/main.ts +++ b/apps/desktop/src/main/main.ts @@ -82,6 +82,7 @@ import { createLaneWorktreeLockService } from "./services/lanes/laneWorktreeLock import { createPortAllocationService } from "./services/lanes/portAllocationService"; import { createLaneProxyService } from "./services/lanes/laneProxyService"; import { releaseLaneRuntimeResources } from "./services/lanes/laneRuntimeLifecycle"; +import { resumeSettledSessionMachinery } from "./services/sessions/sessionMachineryTeardown"; import { createOAuthRedirectService } from "./services/lanes/oauthRedirectService"; import { createRuntimeDiagnosticsService } from "./services/lanes/runtimeDiagnosticsService"; import { createSessionService } from "./services/sessions/sessionService"; @@ -2870,7 +2871,21 @@ app.whenReady().then(async () => { emitProjectEvent(projectRoot, IPC.lanesEnvEvent, ev), }); - const sessionService = createSessionService({ db }); + let sessionServiceRef: ReturnType | null = null; + const sessionService = createSessionService({ + db, + // Resume exactly the scheduled work settle paused, on every route that + // clears a settle — explicit unsettle and turn-start activity alike. + onSettleCleared: (sessionId) => { + const chat = agentChatServiceRef; + if (!chat) return; + void resumeSettledSessionMachinery( + { sessionService: sessionServiceRef!, agentChatService: chat, logger }, + [sessionId], + ).catch(() => {}); + }, + }); + sessionServiceRef = sessionService; sessionService.onChanged((event) => { emitProjectEvent(projectRoot, IPC.sessionsChanged, event); }); diff --git a/apps/desktop/src/main/services/adeActions/registry.test.ts b/apps/desktop/src/main/services/adeActions/registry.test.ts index 97f58b664..e13d93452 100644 --- a/apps/desktop/src/main/services/adeActions/registry.test.ts +++ b/apps/desktop/src/main/services/adeActions/registry.test.ts @@ -1702,10 +1702,11 @@ describe("runtime session actions", () => { // The user-driven single-row unsettle (desktop row menu on a remote-bound // project, `ade code`'s /session unsettle) survives under a cto-gated name. - // Awaited because unsettle now resumes exactly the scheduled work settle - // paused; the lifecycle write itself still happens synchronously first. - await expect(sessionActions.unsettleSession({ sessionId: "session-1" })) - .resolves.toEqual({ ok: true, sessionId: "session-1" }); + // Still synchronous: resuming the scheduled work settle paused is driven by + // sessionService's onSettleCleared hook at the column write, not by each + // caller — so the action itself stays a plain lifecycle call. + expect(sessionActions.unsettleSession({ sessionId: "session-1" })) + .toEqual({ ok: true, sessionId: "session-1" }); expect(unsettleSession).toHaveBeenCalledWith("session-1"); }); diff --git a/apps/desktop/src/main/services/adeActions/registry.ts b/apps/desktop/src/main/services/adeActions/registry.ts index b0e59b733..360b87259 100644 --- a/apps/desktop/src/main/services/adeActions/registry.ts +++ b/apps/desktop/src/main/services/adeActions/registry.ts @@ -126,10 +126,7 @@ import { parseWakeReason, } from "../sessions/sessionRequestValidation"; import { settleTerminalSession } from "../sessions/settleTerminalSession"; -import { - resumeSettledSessionMachinery, - stopSettledSessionMachinery, -} from "../sessions/sessionMachineryTeardown"; +import { stopSettledSessionMachinery } from "../sessions/sessionMachineryTeardown"; import { getSessionLifecycleSettings, setSessionLifecycleSettings, @@ -2147,13 +2144,7 @@ function buildSessionDomainService(runtime: AdeRuntime): OpaqueService | null { if (!sessionService.unsettleSession(sessionId)) { throw new Error(`Session '${sessionId}' was not found.`); } - return (async () => { - await resumeSettledSessionMachinery( - { sessionService, agentChatService: runtime.agentChatService, logger: runtime.logger }, - [sessionId], - ); - return { ok: true, sessionId }; - })(); + return { ok: true, sessionId }; }, // Bulk settle/unsettle for renderer surfaces on remote-bound projects // (mirrors deleteSession's generic trust posture). @@ -2204,13 +2195,7 @@ function buildSessionDomainService(runtime: AdeRuntime): OpaqueService | null { ? record.sessionIds.filter((id): id is string => typeof id === "string") : []; sessionService.unsettleSessions(sessionIds); - return (async () => { - await resumeSettledSessionMachinery( - { sessionService, agentChatService: runtime.agentChatService, logger: runtime.logger }, - sessionIds, - ); - return { ok: true }; - })(); + return { ok: true }; }, // ----------------------------------------------------------------------- // Snooze / wake / settle-override. Snooze is a synced VISIBILITY overlay: diff --git a/apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts b/apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts index 416d69c88..19bb8fae3 100644 --- a/apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts +++ b/apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts @@ -24,6 +24,10 @@ import type { createFileService } from "../../files/fileService"; import type { createLaneService } from "../../lanes/laneService"; import type { createPrService } from "../../prs/prService"; import type { createSessionService } from "../../sessions/sessionService"; +import { + stopSettledSessionMachinery, + type SessionMachineryTeardownDeps, +} from "../../sessions/sessionMachineryTeardown"; import { parseSnoozeDeadline } from "../../sessions/sessionRequestValidation"; import type { createCtoStateService } from "../../cto/ctoStateService"; import type { CtoMemoryService } from "../../cto/ctoMemoryService"; @@ -55,6 +59,12 @@ export interface CtoOperatorToolDeps { | "wakeSession" | "clearWokeMarker" >; + /** + * Only used to tear down what a settled session owns. Optional so the tools + * stay constructible without a chat runtime; absent, the settle still files + * the row, it just cannot stop the row's machinery. + */ + agentChatService?: SessionMachineryTeardownDeps["agentChatService"]; testService?: { listSuites: () => TestSuiteDefinition[]; run: (args: { laneId: string; suiteId: string }) => Promise; @@ -552,6 +562,18 @@ export function createCtoOperatorTools(deps: CtoOperatorToolDeps): Record { try { + // The operator settle is a real settle, so it stops the session's + // machinery like every other one. Without this the CTO could file a + // chat whose monitors kept polling and whose background fleet kept + // spending — the exact bug settle teardown exists to close, preserved + // in the one path that bypassed the shared entry points. + await stopSettledSessionMachinery( + { + sessionService: deps.sessionService, + agentChatService: deps.agentChatService ?? null, + }, + [sessionId], + ); const ok = deps.sessionService.settleSession(sessionId, { ...(outcome ? { outcome } : {}), source: "operator", diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index 09ec1415a..fae3d0b67 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -8374,6 +8374,9 @@ export function createAgentChatService(args: { defaultModelId: modelId, defaultReasoningEffort: reasoningEffort, resolveExecutionLane: resolveCtoExecutionLane, + // So the operator settle tears down the session's machinery like every + // other settle entry point rather than being the one path that skips it. + agentChatService: { stopBackgroundWork, setScheduledWorkPausedForSettle, listScheduledWork }, laneService, prService: prService ?? null, fileService: fileService ?? null, @@ -39351,39 +39354,37 @@ export function createAgentChatService(args: { // Those are exactly the ones that survived the old teardown. const control = getClaudeQueryControl(runtime.query); for (const taskId of [...runtime.liveBackgroundTaskIds]) { - // Same convention as `closeOpenClaudeBackgroundTasks`: a task ADE - // could not actually stop settles as FAILED with the reason, not as - // "stopped". Both close the row — leaving it open would keep the - // session claiming work forever — but only one of them claims ADE - // did the stopping. - let terminalStatus: ScheduledWorkEvent["status"] = "stopped"; - let terminalSummary: string | undefined; + // A task ADE could not actually stop stays LIVE. + // + // Emitting a terminal row here would drop it from + // `liveBackgroundTaskIds`, which is what the caller measures the + // stop against — so a stop that failed, timed out, or found no stop + // control would be counted as a stop that worked, and the settled + // row would hide a process still burning tokens. Leaving it live is + // self-correcting: the SDK's next authoritative level drains it if + // it really did end. if (typeof control.stopTask !== "function") { - terminalStatus = "failed"; - terminalSummary = "The Claude query did not expose a task stop control."; - } else { - try { - await awaitClaudeControlCall( - `Stopping Claude background task '${taskId}'`, - CLAUDE_STOP_TASK_TIMEOUT_MS, - () => control.stopTask!(taskId), - ); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - terminalStatus = "failed"; - terminalSummary = `Failed to stop background task: ${message}`; - logger.warn("agent_chat.settle_background_stop_failed", { - sessionId: managed.session.id, - taskId, - error: message, - }); - } + logger.warn("agent_chat.settle_background_stop_unavailable", { + sessionId: managed.session.id, + taskId, + }); + continue; } - emitClaudeBackgroundTaskUpdate(managed, runtime, { - taskId, - status: terminalStatus, - ...(terminalSummary ? { summary: terminalSummary } : {}), - }); + try { + await awaitClaudeControlCall( + `Stopping Claude background task '${taskId}'`, + CLAUDE_STOP_TASK_TIMEOUT_MS, + () => control.stopTask!(taskId), + ); + } catch (error) { + logger.warn("agent_chat.settle_background_stop_failed", { + sessionId: managed.session.id, + taskId, + error: error instanceof Error ? error.message : String(error), + }); + continue; + } + emitClaudeBackgroundTaskUpdate(managed, runtime, { taskId, status: "stopped" }); } break; } diff --git a/apps/desktop/src/main/services/ipc/registerIpc.ts b/apps/desktop/src/main/services/ipc/registerIpc.ts index 015c75c45..2d17d6cab 100644 --- a/apps/desktop/src/main/services/ipc/registerIpc.ts +++ b/apps/desktop/src/main/services/ipc/registerIpc.ts @@ -51,10 +51,7 @@ import { parseWakeReason, } from "../sessions/sessionRequestValidation"; import { settleTerminalSession } from "../sessions/settleTerminalSession"; -import { - resumeSettledSessionMachinery, - stopSettledSessionMachinery, -} from "../sessions/sessionMachineryTeardown"; +import { stopSettledSessionMachinery } from "../sessions/sessionMachineryTeardown"; import { getSessionLifecycleSettings, setSessionLifecycleSettings, @@ -6983,10 +6980,6 @@ export function registerIpc({ const sessionId = typeof arg?.sessionId === "string" ? arg.sessionId.trim() : ""; if (!sessionId) throw new Error("Session id is required."); ctx.sessionService.unsettleSession(sessionId); - await resumeSettledSessionMachinery( - { sessionService: ctx.sessionService, agentChatService: ctx.agentChatService, logger: ctx.logger }, - [sessionId], - ); }, ); @@ -7015,10 +7008,6 @@ export function registerIpc({ (sessionId): sessionId is string => typeof sessionId === "string", ); ctx.sessionService.unsettleSessions(unsettleIds); - await resumeSettledSessionMachinery( - { sessionService: ctx.sessionService, agentChatService: ctx.agentChatService, logger: ctx.logger }, - unsettleIds, - ); }, ); diff --git a/apps/desktop/src/main/services/sessions/sessionService.test.ts b/apps/desktop/src/main/services/sessions/sessionService.test.ts index 0e8a978c7..be1987345 100644 --- a/apps/desktop/src/main/services/sessions/sessionService.test.ts +++ b/apps/desktop/src/main/services/sessions/sessionService.test.ts @@ -66,6 +66,49 @@ afterEach(async () => { }); describe("sessionService resume metadata", () => { + it("reports every route that clears a settle, including turn-start activity", async () => { + // Settle teardown pauses the session's scheduled work durably, so every + // route that un-settles has to resume it. Wiring that into each caller left + // `clearTurnStartMarkers` — a user simply sending the next message, the + // most common unsettle there is — silently bypassing the resume, so the + // hook lives at the column write instead. + const projectRoot = makeProjectRoot("ade-session-service-settle-cleared-"); + const db = await openKvDb(path.join(projectRoot, ".ade", "ade.db"), createLogger() as any); + activeDisposers.push(async () => db.close()); + insertProjectGraph(db); + const cleared: string[] = []; + const svc = createSessionService({ db, onSettleCleared: (id) => cleared.push(id) }); + for (const id of ["s-1", "s-2"]) { + svc.create({ + sessionId: id, + laneId: "lane-1", + ptyId: null, + tracked: true, + title: id, + startedAt: "2026-03-17T00:10:00.000Z", + transcriptPath: `/tmp/${id}.log`, + toolType: "codex-chat", + }); + } + + svc.settleSession("s-1"); + svc.unsettleSession("s-1"); + expect(cleared).toEqual(["s-1"]); + + svc.settleSession("s-1"); + svc.clearTurnStartMarkers("s-1"); + expect(cleared).toEqual(["s-1", "s-1"]); + + svc.settleSession("s-2"); + svc.unsettleSessions(["s-1", "s-2"]); + expect(cleared.slice(2)).toEqual(["s-1", "s-2"]); + + // A missing row changes nothing, so it must not announce a clear. + svc.unsettleSession("nope"); + expect(cleared).toHaveLength(4); + }); + + it("reads terminal scrollback transparently from a compressed log", async () => { const projectRoot = makeProjectRoot("ade-session-service-gzip-"); const db = await openKvDb(path.join(projectRoot, ".ade", "ade.db"), createLogger() as any); diff --git a/apps/desktop/src/main/services/sessions/sessionService.ts b/apps/desktop/src/main/services/sessions/sessionService.ts index 93ea8d2f5..cdbcb873f 100644 --- a/apps/desktop/src/main/services/sessions/sessionService.ts +++ b/apps/desktop/src/main/services/sessions/sessionService.ts @@ -364,9 +364,37 @@ function normalizeSessionIds(sessionIds: string[]): string[] { )); } -export function createSessionService({ db }: { db: AdeDb }) { +export function createSessionService({ db, onSettleCleared }: { + db: AdeDb; + /** + * Called for every session whose declared settle this service just cleared — + * the explicit unsettle paths AND the implicit one where new activity clears + * `settled_at` at turn start. + * + * The hook lives HERE, at the authoritative column write, because settle + * teardown pauses the session's scheduled work and that pause is durable: an + * unsettle route that skips the resume leaves monitors, crons, and scheduled + * turns disabled forever. Wiring the resume into each caller instead left + * `clearTurnStartMarkers` — the most common unsettle of all, a user simply + * sending the next message — silently bypassing it. One hook at the mutation + * means a new caller cannot reintroduce that gap. + * + * Best-effort and non-blocking by contract: a lifecycle write must never fail + * because a resume did. + */ + onSettleCleared?: (sessionId: string) => void; +}) { const changeListeners = new Set<(event: TerminalSessionChangedEvent) => void>(); + const notifySettleCleared = (sessionId: string): void => { + if (!onSettleCleared) return; + try { + onSettleCleared(sessionId); + } catch { + // The column write already happened and is what callers depend on. + } + }; + /** * Shared skeleton for the single-session lifecycle mutators: trim, existence * probe, run the update, broadcast. Keeps every SQL literal at its call site. @@ -1457,7 +1485,7 @@ export function createSessionService({ db }: { db: AdeDb }) { /** Clears a declared settle plus any `'settled'` override. */ unsettleSession(sessionId: string): boolean { - return mutateSessionMeta(sessionId, (id) => { + const changed = mutateSessionMeta(sessionId, (id) => { db.run( ` update terminal_sessions @@ -1469,6 +1497,8 @@ export function createSessionService({ db }: { db: AdeDb }) { [id], ); }); + if (changed) notifySettleCleared(sessionId.trim()); + return changed; }, /** Explicit settle override, cleared with `settled_at` on real activity. */ @@ -1555,6 +1585,7 @@ export function createSessionService({ db }: { db: AdeDb }) { ); for (const id of ids) { emitChanged({ sessionId: id, reason: "meta-updated" }); + notifySettleCleared(id); } }, @@ -1754,7 +1785,7 @@ export function createSessionService({ db }: { db: AdeDb }) { }, clearTurnStartMarkers(sessionId: string): boolean { - return mutateSessionMeta(sessionId, (id) => { + const changed = mutateSessionMeta(sessionId, (id) => { db.run( ` update terminal_sessions @@ -1770,6 +1801,10 @@ export function createSessionService({ db }: { db: AdeDb }) { [id], ); }); + // The implicit unsettle: a user sending the next message into a settled + // chat. It must resume what settle paused exactly like an explicit one. + if (changed) notifySettleCleared(sessionId.trim()); + return changed; }, deleteSession(sessionId: string): boolean { diff --git a/docs/features/terminals-and-sessions/README.md b/docs/features/terminals-and-sessions/README.md index dc27ac120..a92ffd41f 100644 --- a/docs/features/terminals-and-sessions/README.md +++ b/docs/features/terminals-and-sessions/README.md @@ -210,11 +210,16 @@ and in tests. `apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts`. It pauses the session's scheduled work — pauses rather than cancels, and the pause is durable, so it carries an exact undo: the scheduler records which - sessions **settle** paused (`settlePausedSessionIds`) and every unsettle path - runs `resumeSettledSessionMachinery`, which resumes only those. A pause the - user took deliberately is never claimed and never resumed, and background - work is never restarted — ADE cannot re-spawn a shell it stopped. It also - calls + sessions **settle** paused (`settlePausedSessionIds`), and + `sessionService`'s `onSettleCleared` hook runs `resumeSettledSessionMachinery` + for every route that clears a settle. The hook sits at the column write rather + than in each caller because the most common unsettle is implicit — + `clearTurnStartMarkers`, i.e. a user sending the next message — and + per-caller wiring silently skipped it. A pause the user took deliberately is + never claimed and never resumed, and background work is never restarted: ADE + cannot re-spawn a shell it stopped. Settle itself is still explicit per entry + point because teardown must finish *before* the write; that includes the CTO + operator's `settleSession` tool. It also calls `agentChatService.stopBackgroundWork`, which stops every live child before the parent. **Terminal panes stay open**: an agent's background shell is thread background work, but a pane the user opened is theirs, and closing it From 20eccbfb25d99639c3768e7ffcd686430bb2d48e Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 00:49:48 -0400 Subject: [PATCH 10/14] revert(sessions): settle no longer pauses scheduled work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cutting a slice of this PR rather than patching it a fourth time. Greptile's latest round found that `settled_at` is cleared from SEVEN places in sessionService, not the three the onSettleCleared hook covered — including `setLastOutputPreview`, the hot PTY-output path. It also found a TOCTOU where a fire-and-forget resume overlapping a later settle releases the newer pause. That is the third consecutive review round to find a defect in the scheduled-work pause specifically, each in a route the previous fix had not traced. The pause is persisted, so it needs a COMPLETE undo or it silently deletes the user's own monitors and crons. Covering the remaining routes means either a pre-read or a split statement on a per-output-chunk path, plus serializing pause/resume per session — real cost and more machinery, for the part of this change that keeps producing bugs. So settle now stops background work only: background shells, subagent fleets, cursor cloud runs. That was the unmanaged, invisible thing the change was actually about, and it has been stable since the second iteration. Scheduled work in ADE is already visible and user-manageable (scheduledWork / nextWakeAt on the summary, a per-session pause toggle), and canonicalSessionState already handles a settled chat woken by a schedule: green while the turn streams, then re-settled. Leaving it running is the pre-existing, deliberate behavior. Removed: settlePausedSessionIds and the two scheduler methods, setScheduledWorkPausedForSettle, sessionService's onSettleCleared hook and its wiring in main/bootstrap, and resumeSettledSessionMachinery. Kept: the CTO operator settle now routing through shared teardown, and unstoppable Claude tasks staying live rather than being reported as stopped. Stopping scheduled work on settle remains a reasonable feature; it needs its own change with the full clear-path inventory up front, not a bolt-on to this one. Co-Authored-By: Claude Opus 5 --- apps/ade-cli/src/bootstrap.ts | 24 +---- apps/desktop/src/main/main.ts | 17 +--- .../main/services/chat/agentChatService.ts | 23 +---- .../chat/chatScheduledWorkScheduler.ts | 47 ---------- .../src/main/services/prs/prAsync.test.ts | 9 +- .../sessions/sessionMachineryTeardown.ts | 89 ++++--------------- .../services/sessions/sessionService.test.ts | 41 --------- .../main/services/sessions/sessionService.ts | 34 +------ .../services/sessions/sessionTeardown.test.ts | 50 +---------- .../features/terminals-and-sessions/README.md | 25 +++--- 10 files changed, 36 insertions(+), 323 deletions(-) diff --git a/apps/ade-cli/src/bootstrap.ts b/apps/ade-cli/src/bootstrap.ts index 97920764c..723e9218b 100644 --- a/apps/ade-cli/src/bootstrap.ts +++ b/apps/ade-cli/src/bootstrap.ts @@ -48,7 +48,6 @@ import { createLaneTemplateService } from "../../desktop/src/main/services/lanes import { createPortAllocationService } from "../../desktop/src/main/services/lanes/portAllocationService"; import { createLaneProxyService } from "../../desktop/src/main/services/lanes/laneProxyService"; import { releaseLaneRuntimeResources } from "../../desktop/src/main/services/lanes/laneRuntimeLifecycle"; -import { resumeSettledSessionMachinery } from "../../desktop/src/main/services/sessions/sessionMachineryTeardown"; import { createOAuthRedirectService } from "../../desktop/src/main/services/lanes/oauthRedirectService"; import { createRuntimeDiagnosticsService } from "../../desktop/src/main/services/lanes/runtimeDiagnosticsService"; import { createRebaseSuggestionService } from "../../desktop/src/main/services/lanes/rebaseSuggestionService"; @@ -756,27 +755,7 @@ export async function createAdeRuntime(args: { // services. Session changes still use it once publishing is attached. let pushPublisherForPtySignals: PushPublisherService | null = null; let ptyServiceForSessionChanges: ReturnType | null = null; - let sessionServiceRef: ReturnType | null = null; - // Late-bound for the same reason as the push publisher above: the chat - // service is constructed well after the session service that calls back into - // it when a settle is cleared. - let agentChatServiceForSettleResume: - | ReturnType - | null = null; - const sessionService = createSessionService({ - db, - // Resume exactly the scheduled work settle paused, on every route that - // clears a settle — explicit unsettle and turn-start activity alike. - onSettleCleared: (sessionId) => { - const chat = agentChatServiceForSettleResume; - if (!chat) return; - void resumeSettledSessionMachinery( - { sessionService: sessionServiceRef!, agentChatService: chat, logger }, - [sessionId], - ).catch(() => {}); - }, - }); - sessionServiceRef = sessionService; + const sessionService = createSessionService({ db }); sessionService.onChanged((event) => { pushEvent("runtime", { type: "terminal_session_changed", event }); const session = sessionService.get(event.sessionId); @@ -1452,7 +1431,6 @@ export async function createAdeRuntime(args: { prService: headlessLinearServices.prService, aiIntegrationService, }); - agentChatServiceForSettleResume = agentChatService; const prMergeAutoSettlementService = createPrMergeAutoSettlementService({ db, sessionService, diff --git a/apps/desktop/src/main/main.ts b/apps/desktop/src/main/main.ts index 6a1b9061e..f07806c2c 100644 --- a/apps/desktop/src/main/main.ts +++ b/apps/desktop/src/main/main.ts @@ -82,7 +82,6 @@ import { createLaneWorktreeLockService } from "./services/lanes/laneWorktreeLock import { createPortAllocationService } from "./services/lanes/portAllocationService"; import { createLaneProxyService } from "./services/lanes/laneProxyService"; import { releaseLaneRuntimeResources } from "./services/lanes/laneRuntimeLifecycle"; -import { resumeSettledSessionMachinery } from "./services/sessions/sessionMachineryTeardown"; import { createOAuthRedirectService } from "./services/lanes/oauthRedirectService"; import { createRuntimeDiagnosticsService } from "./services/lanes/runtimeDiagnosticsService"; import { createSessionService } from "./services/sessions/sessionService"; @@ -2871,21 +2870,7 @@ app.whenReady().then(async () => { emitProjectEvent(projectRoot, IPC.lanesEnvEvent, ev), }); - let sessionServiceRef: ReturnType | null = null; - const sessionService = createSessionService({ - db, - // Resume exactly the scheduled work settle paused, on every route that - // clears a settle — explicit unsettle and turn-start activity alike. - onSettleCleared: (sessionId) => { - const chat = agentChatServiceRef; - if (!chat) return; - void resumeSettledSessionMachinery( - { sessionService: sessionServiceRef!, agentChatService: chat, logger }, - [sessionId], - ).catch(() => {}); - }, - }); - sessionServiceRef = sessionService; + const sessionService = createSessionService({ db }); sessionService.onChanged((event) => { emitProjectEvent(projectRoot, IPC.sessionsChanged, event); }); diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index fae3d0b67..86660cf98 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -8376,7 +8376,7 @@ export function createAgentChatService(args: { resolveExecutionLane: resolveCtoExecutionLane, // So the operator settle tears down the session's machinery like every // other settle entry point rather than being the one path that skips it. - agentChatService: { stopBackgroundWork, setScheduledWorkPausedForSettle, listScheduledWork }, + agentChatService: { stopBackgroundWork }, laneService, prService: prService ?? null, fileService: fileService ?? null, @@ -39424,26 +39424,6 @@ export function createAgentChatService(args: { return { stopped: Math.max(0, before - after), skippedActiveTurn: false }; }; - /** - * Pause / resume a session's scheduled work as part of settle teardown. - * - * Separate from `setScheduledWorkPaused` (the user-facing toggle) because the - * two must not fight: settle claims a pause only when the user had not - * already taken one, and unsettle puts back exactly what settle took. Without - * the resume half, a settled-then-unsettled chat kept its monitors, crons, - * and scheduled turns disabled forever. - */ - const setScheduledWorkPausedForSettle = async ( - { sessionId, paused }: { sessionId: string; paused: boolean }, - ): Promise => { - const normalizedSessionId = sessionId.trim(); - if (!normalizedSessionId) return false; - await scheduledWorkReady; - if (!scheduledWorkScheduler) return false; - return paused - ? scheduledWorkScheduler.setSessionPausedForSettle(normalizedSessionId) - : scheduledWorkScheduler.resumeSessionPausedForSettle(normalizedSessionId); - }; const hasActiveWorkloads = (): boolean => { for (const managed of managedSessions.values()) { @@ -44327,7 +44307,6 @@ export function createAgentChatService(args: { ensureSessionSurface, hasActiveWorkloads, stopBackgroundWork, - setScheduledWorkPausedForSettle, hasRetainableSessions, countActiveForLane, disposeForLane, diff --git a/apps/desktop/src/main/services/chat/chatScheduledWorkScheduler.ts b/apps/desktop/src/main/services/chat/chatScheduledWorkScheduler.ts index 5073adcf2..d4f494afa 100644 --- a/apps/desktop/src/main/services/chat/chatScheduledWorkScheduler.ts +++ b/apps/desktop/src/main/services/chat/chatScheduledWorkScheduler.ts @@ -46,16 +46,6 @@ export type ChatScheduledWorkState = { version: 1; schedules: ChatScheduledWorkRecord[]; pausedSessionIds: string[]; - /** - * Sessions whose pause was taken by SETTLE TEARDOWN rather than by the user. - * - * Persisted beside the pause itself because it is the exact undo record: - * settle pauses a session's schedules so a monitor cannot wake a thread the - * user has declared done, and unsettle has to put back precisely what settle - * took — never a pause the user set deliberately, and never nothing at all, - * which would leave the schedules disabled forever. - */ - settlePausedSessionIds?: string[]; }; export type ChatScheduledWorkUpsert = Omit< @@ -98,10 +88,6 @@ export type ChatScheduledWorkScheduler = { cancel(scheduleId: string): Promise; setSchedulePaused(scheduleId: string, paused: boolean): Promise; setSessionPaused(sessionId: string, paused: boolean): Promise; - /** Pause on settle, claiming the pause only if the user had not already taken one. Returns whether it paused. */ - setSessionPausedForSettle(sessionId: string): Promise; - /** Undo exactly what `setSessionPausedForSettle` did, and nothing else. Returns whether it resumed. */ - resumeSessionPausedForSettle(sessionId: string): Promise; refreshGlobalPause(): Promise; list(sessionId?: string): ChatScheduledWorkRecord[]; isSessionPaused(sessionId: string): boolean; @@ -202,18 +188,10 @@ function normalizeState(value: unknown): ChatScheduledWorkState { const pausedSessionIds = Array.isArray(record?.pausedSessionIds) ? record.pausedSessionIds.filter((item): item is string => typeof item === "string" && item.length > 0) : []; - const settlePausedSessionIds = Array.isArray(record?.settlePausedSessionIds) - ? record.settlePausedSessionIds.filter((item): item is string => typeof item === "string" && item.length > 0) - : []; return { version: 1, schedules, pausedSessionIds: [...new Set(pausedSessionIds)], - // Only meaningful for sessions that are actually paused; a stale marker - // for an already-resumed session would resume nothing but would linger. - settlePausedSessionIds: [...new Set(settlePausedSessionIds)].filter( - (sessionId) => pausedSessionIds.includes(sessionId), - ), }; } @@ -228,7 +206,6 @@ export function createChatScheduledWorkScheduler( const timers = options.timers ?? defaultTimers; const schedules = new Map(); const pausedSessionIds = new Set(); - const settlePausedSessionIds = new Set(); const timerHandles = new Map(); const inFlight = new Set(); let started = false; @@ -242,7 +219,6 @@ export function createChatScheduledWorkScheduler( .sort((left, right) => left.createdAt - right.createdAt || left.id.localeCompare(right.id)) .map(cloneSchedule), pausedSessionIds: [...pausedSessionIds].sort(), - settlePausedSessionIds: [...settlePausedSessionIds].sort(), }); const persist = async (): Promise => { @@ -519,7 +495,6 @@ export function createChatScheduledWorkScheduler( const state = normalizeState(await options.loadState()); schedules.clear(); pausedSessionIds.clear(); - settlePausedSessionIds.clear(); let migrated = false; for (const schedule of state.schedules) { // Pre-1.2.27 builds persisted cron-tool placeholders before Claude @@ -551,7 +526,6 @@ export function createChatScheduledWorkScheduler( schedules.set(schedule.id, schedule); } for (const sessionId of state.pausedSessionIds) pausedSessionIds.add(sessionId); - for (const sessionId of state.settlePausedSessionIds ?? []) settlePausedSessionIds.add(sessionId); migrated = pruneTerminalHistory() || migrated; started = true; for (const schedule of schedules.values()) await reconcileSchedule(schedule); @@ -652,30 +626,9 @@ export function createChatScheduledWorkScheduler( await start(); if (paused) pausedSessionIds.add(sessionId); else pausedSessionIds.delete(sessionId); - // An explicit choice by the user replaces settle's claim on this pause, - // in either direction: a later unsettle must not undo what they just did. - settlePausedSessionIds.delete(sessionId); - await updatePauseStatuses(sessionId); - }, - - async setSessionPausedForSettle(sessionId): Promise { - await start(); - // Already paused by the user — leave it, and take no claim on it, so - // unsettle cannot resume schedules they deliberately stopped. - if (pausedSessionIds.has(sessionId)) return false; - pausedSessionIds.add(sessionId); - settlePausedSessionIds.add(sessionId); await updatePauseStatuses(sessionId); - return true; }, - async resumeSessionPausedForSettle(sessionId): Promise { - await start(); - if (!settlePausedSessionIds.delete(sessionId)) return false; - pausedSessionIds.delete(sessionId); - await updatePauseStatuses(sessionId); - return true; - }, async refreshGlobalPause(): Promise { await start(); diff --git a/apps/desktop/src/main/services/prs/prAsync.test.ts b/apps/desktop/src/main/services/prs/prAsync.test.ts index 9f273fc2b..7d26b43bf 100644 --- a/apps/desktop/src/main/services/prs/prAsync.test.ts +++ b/apps/desktop/src/main/services/prs/prAsync.test.ts @@ -927,10 +927,6 @@ describe("prMergeAutoSettlementService", () => { order.push("stop"); return { stopped: 1, skippedActiveTurn: false }; }); - const setScheduledWorkPausedForSettle = vi.fn(async () => { - order.push("pause"); - return true; - }); const rows = [{ id: "chat-live", toolType: "claude-chat", archivedAt: null, settledAt: null }]; const service = createPrMergeAutoSettlementService({ db: db as any, @@ -941,8 +937,6 @@ describe("prMergeAutoSettlementService", () => { } as any, agentChatService: { stopBackgroundWork, - setScheduledWorkPausedForSettle, - listScheduledWork: vi.fn(async () => [{ id: "sched-1", status: "scheduled" }]), } as any, emitEvent: vi.fn(), }); @@ -957,8 +951,7 @@ describe("prMergeAutoSettlementService", () => { }); expect(stopBackgroundWork).toHaveBeenCalledWith({ sessionId: "chat-live" }); - expect(setScheduledWorkPausedForSettle).toHaveBeenCalledWith({ sessionId: "chat-live", paused: true }); - expect(order).toEqual(["pause", "stop", "settle"]); + expect(order).toEqual(["stop", "settle"]); }); it("does not re-settle after reactivation, but settles for a later PR", async () => { diff --git a/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts b/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts index 5a29d4e48..6d4ea7067 100644 --- a/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts +++ b/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts @@ -21,7 +21,6 @@ import { isChatToolType } from "./chatSessionProjection"; * * ── What settle stops, and what it deliberately does not ──────────────────── * - * stops scheduled work (monitors, crons, loops) for the session * stops live background work — background shells, subagent fleets, * cursor cloud runs — via `agentChatService.stopBackgroundWork` * keeps the session itself, and its runtime, alive and resumable @@ -31,6 +30,23 @@ import { isChatToolType } from "./chatSessionProjection"; * destroy scrollback they never asked to lose. * keeps an ACTIVE foreground turn running (see `stopBackgroundWork`). * + * ── Scheduled work is deliberately NOT stopped ────────────────────────────── + * + * An earlier version of this paused the session's durable schedules. It was + * removed, and the reason is worth keeping: the pause is persisted, so it needs + * an exact undo on every route that clears a settle — and `settled_at` is + * cleared from seven places, including `setLastOutputPreview` on the hot PTY + * output path. Three review rounds each found another route that skipped the + * resume and left a chat's monitors and crons disabled forever. A pause without + * a complete undo is a slower deletion of the user's own schedules. + * + * It is also the smaller loss than it looks. ADE's scheduled work is already + * visible and user-manageable (`scheduledWork` and `nextWakeAt` on the summary, + * a per-session pause toggle), and `canonicalSessionState` already handles a + * settled chat woken by scheduled work: it shows green while the turn streams, + * then re-settles. The unmanaged, invisible thing settle needed to stop was + * background work, and that is what it stops. + * * ── What escapes, stated plainly ──────────────────────────────────────────── * * A process an agent detached with `nohup`, `setsid`, or `disown` leaves ADE's @@ -42,7 +58,7 @@ export type SessionMachineryTeardownDeps = { sessionService: Pick, "get">; agentChatService?: Pick< ReturnType, - "stopBackgroundWork" | "setScheduledWorkPausedForSettle" | "listScheduledWork" + "stopBackgroundWork" > | null; logger?: { warn: (message: string, meta?: Record) => void } | null; }; @@ -52,8 +68,6 @@ export type SessionMachineryTeardownResult = { sessionIds: string[]; /** Live background jobs stopped across those sessions. */ stoppedBackgroundWork: number; - /** Sessions whose scheduled work was paused. */ - pausedScheduledWork: number; /** Sessions skipped because a foreground turn was still streaming. */ skippedActiveTurns: number; }; @@ -61,48 +75,9 @@ export type SessionMachineryTeardownResult = { const EMPTY_RESULT: SessionMachineryTeardownResult = { sessionIds: [], stoppedBackgroundWork: 0, - pausedScheduledWork: 0, skippedActiveTurns: 0, }; -/** - * Pause rather than cancel, and pause REVERSIBLY. - * - * Cancelling would be destructive and irreversible: a settle that turns out to - * be premature would have silently deleted schedules the user set up by hand. - * - * The pause is durable, so it needs an exact undo or it is just a slower - * deletion — a settled-then-unsettled chat would keep its monitors, crons, and - * scheduled turns disabled forever. `setScheduledWorkPausedForSettle` claims - * the pause only when the user had not already taken one, and - * `resumeSettledSessionMachinery` puts back precisely what it claimed. See - * `chatScheduledWorkScheduler`'s `settlePausedSessionIds`. - */ -async function setScheduledWorkPaused( - deps: SessionMachineryTeardownDeps, - sessionId: string, - paused: boolean, -): Promise { - const service = deps.agentChatService; - if (!service) return false; - try { - if (paused) { - const schedules = await service.listScheduledWork({ sessionId }); - const armed = schedules.some( - (schedule) => schedule.status !== "completed" && schedule.status !== "cancelled", - ); - if (!armed) return false; - } - return await service.setScheduledWorkPausedForSettle({ sessionId, paused }); - } catch (error) { - deps.logger?.warn("session_teardown.scheduled_work_pause_failed", { - sessionId, - paused, - error: error instanceof Error ? error.message : String(error), - }); - return false; - } -} /** * Stop the background machinery for a set of sessions being settled. @@ -121,8 +96,7 @@ export async function stopSettledSessionMachinery( const result: SessionMachineryTeardownResult = { sessionIds: [], stoppedBackgroundWork: 0, - pausedScheduledWork: 0, - skippedActiveTurns: 0, + skippedActiveTurns: 0, }; for (const sessionId of unique) { @@ -133,8 +107,6 @@ export async function stopSettledSessionMachinery( if (!row || !isChatToolType(row.toolType)) continue; result.sessionIds.push(sessionId); - if (await setScheduledWorkPaused(deps, sessionId, true)) result.pausedScheduledWork += 1; - const service = deps.agentChatService; if (!service) continue; try { @@ -152,26 +124,3 @@ export async function stopSettledSessionMachinery( return result; } -/** - * The undo half of settle teardown, run by every unsettle path. - * - * Only resumes schedules that settle itself paused — a pause the user took - * deliberately survives an unsettle untouched. Background work is deliberately - * NOT restarted: ADE cannot re-spawn a shell or a subagent fleet it stopped, - * and pretending otherwise would be worse than leaving the session quiet. - */ -export async function resumeSettledSessionMachinery( - deps: SessionMachineryTeardownDeps, - sessionIds: readonly string[], -): Promise<{ sessionIds: string[]; resumedScheduledWork: number }> { - const unique = [...new Set(sessionIds.map((id) => id.trim()).filter(Boolean))]; - const touched: string[] = []; - let resumedScheduledWork = 0; - for (const sessionId of unique) { - const row = deps.sessionService.get(sessionId); - if (!row || !isChatToolType(row.toolType)) continue; - touched.push(sessionId); - if (await setScheduledWorkPaused(deps, sessionId, false)) resumedScheduledWork += 1; - } - return { sessionIds: touched, resumedScheduledWork }; -} diff --git a/apps/desktop/src/main/services/sessions/sessionService.test.ts b/apps/desktop/src/main/services/sessions/sessionService.test.ts index be1987345..0e759306d 100644 --- a/apps/desktop/src/main/services/sessions/sessionService.test.ts +++ b/apps/desktop/src/main/services/sessions/sessionService.test.ts @@ -66,47 +66,6 @@ afterEach(async () => { }); describe("sessionService resume metadata", () => { - it("reports every route that clears a settle, including turn-start activity", async () => { - // Settle teardown pauses the session's scheduled work durably, so every - // route that un-settles has to resume it. Wiring that into each caller left - // `clearTurnStartMarkers` — a user simply sending the next message, the - // most common unsettle there is — silently bypassing the resume, so the - // hook lives at the column write instead. - const projectRoot = makeProjectRoot("ade-session-service-settle-cleared-"); - const db = await openKvDb(path.join(projectRoot, ".ade", "ade.db"), createLogger() as any); - activeDisposers.push(async () => db.close()); - insertProjectGraph(db); - const cleared: string[] = []; - const svc = createSessionService({ db, onSettleCleared: (id) => cleared.push(id) }); - for (const id of ["s-1", "s-2"]) { - svc.create({ - sessionId: id, - laneId: "lane-1", - ptyId: null, - tracked: true, - title: id, - startedAt: "2026-03-17T00:10:00.000Z", - transcriptPath: `/tmp/${id}.log`, - toolType: "codex-chat", - }); - } - - svc.settleSession("s-1"); - svc.unsettleSession("s-1"); - expect(cleared).toEqual(["s-1"]); - - svc.settleSession("s-1"); - svc.clearTurnStartMarkers("s-1"); - expect(cleared).toEqual(["s-1", "s-1"]); - - svc.settleSession("s-2"); - svc.unsettleSessions(["s-1", "s-2"]); - expect(cleared.slice(2)).toEqual(["s-1", "s-2"]); - - // A missing row changes nothing, so it must not announce a clear. - svc.unsettleSession("nope"); - expect(cleared).toHaveLength(4); - }); it("reads terminal scrollback transparently from a compressed log", async () => { diff --git a/apps/desktop/src/main/services/sessions/sessionService.ts b/apps/desktop/src/main/services/sessions/sessionService.ts index cdbcb873f..edec4d682 100644 --- a/apps/desktop/src/main/services/sessions/sessionService.ts +++ b/apps/desktop/src/main/services/sessions/sessionService.ts @@ -364,36 +364,9 @@ function normalizeSessionIds(sessionIds: string[]): string[] { )); } -export function createSessionService({ db, onSettleCleared }: { - db: AdeDb; - /** - * Called for every session whose declared settle this service just cleared — - * the explicit unsettle paths AND the implicit one where new activity clears - * `settled_at` at turn start. - * - * The hook lives HERE, at the authoritative column write, because settle - * teardown pauses the session's scheduled work and that pause is durable: an - * unsettle route that skips the resume leaves monitors, crons, and scheduled - * turns disabled forever. Wiring the resume into each caller instead left - * `clearTurnStartMarkers` — the most common unsettle of all, a user simply - * sending the next message — silently bypassing it. One hook at the mutation - * means a new caller cannot reintroduce that gap. - * - * Best-effort and non-blocking by contract: a lifecycle write must never fail - * because a resume did. - */ - onSettleCleared?: (sessionId: string) => void; -}) { +export function createSessionService({ db }: { db: AdeDb }) { const changeListeners = new Set<(event: TerminalSessionChangedEvent) => void>(); - const notifySettleCleared = (sessionId: string): void => { - if (!onSettleCleared) return; - try { - onSettleCleared(sessionId); - } catch { - // The column write already happened and is what callers depend on. - } - }; /** * Shared skeleton for the single-session lifecycle mutators: trim, existence @@ -1497,7 +1470,6 @@ export function createSessionService({ db, onSettleCleared }: { [id], ); }); - if (changed) notifySettleCleared(sessionId.trim()); return changed; }, @@ -1585,7 +1557,6 @@ export function createSessionService({ db, onSettleCleared }: { ); for (const id of ids) { emitChanged({ sessionId: id, reason: "meta-updated" }); - notifySettleCleared(id); } }, @@ -1801,9 +1772,6 @@ export function createSessionService({ db, onSettleCleared }: { [id], ); }); - // The implicit unsettle: a user sending the next message into a settled - // chat. It must resume what settle paused exactly like an explicit one. - if (changed) notifySettleCleared(sessionId.trim()); return changed; }, diff --git a/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts b/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts index 1b9e133fb..b8edaf34b 100644 --- a/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts +++ b/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts @@ -1,10 +1,7 @@ import { describe, expect, it, vi } from "vitest"; import type { TerminalSessionSummary } from "../../../shared/types"; import { deleteTerminalSessionWithRuntimeCleanup } from "./deleteTerminalSession"; -import { - resumeSettledSessionMachinery, - stopSettledSessionMachinery, -} from "./sessionMachineryTeardown"; +import { stopSettledSessionMachinery } from "./sessionMachineryTeardown"; import { settleTerminalSession } from "./settleTerminalSession"; import type { createPtyService } from "../pty/ptyService"; import type { createSessionService } from "./sessionService"; @@ -23,22 +20,16 @@ type Row = { id: string; toolType: string }; function deps(rows: Row[], overrides: Record = {}) { const stopBackgroundWork = vi.fn(async () => ({ stopped: 2, skippedActiveTurn: false })); - const setScheduledWorkPausedForSettle = vi.fn(async () => true); - const listScheduledWork = vi.fn(async () => [{ id: "sched-1", status: "scheduled" }]); return { sessionService: { get: (id: string) => rows.find((row) => row.id === id) ?? null, } as never, agentChatService: { stopBackgroundWork, - setScheduledWorkPausedForSettle, - listScheduledWork, ...overrides, } as never, logger: { warn: vi.fn() }, stopBackgroundWork, - setScheduledWorkPausedForSettle, - listScheduledWork, }; } @@ -50,54 +41,16 @@ describe("stopSettledSessionMachinery", () => { const result = await stopSettledSessionMachinery(d, ["chat-1"]); expect(d.stopBackgroundWork).toHaveBeenCalledWith({ sessionId: "chat-1" }); - expect(d.setScheduledWorkPausedForSettle).toHaveBeenCalledWith({ sessionId: "chat-1", paused: true }); expect(result).toMatchObject({ sessionIds: ["chat-1"], stoppedBackgroundWork: 2, - pausedScheduledWork: 1, skippedActiveTurns: 0, }); }); - it("pauses rather than cancels, so an unsettle can bring the schedules back", async () => { - const d = deps([{ id: "chat-1", toolType: "claude-chat" }], { - cancelScheduledWork: vi.fn(), - }); - await stopSettledSessionMachinery(d, ["chat-1"]); - expect(d.setScheduledWorkPausedForSettle).toHaveBeenCalledTimes(1); - expect((d.agentChatService as unknown as { cancelScheduledWork: ReturnType }) - .cancelScheduledWork).not.toHaveBeenCalled(); - }); - - it("resumes on unsettle what settle paused — a durable pause needs an exact undo", async () => { - // A pause with no undo is just a slower deletion: before this, a settled - // then unsettled chat kept its monitors, crons, and scheduled turns - // disabled forever, because unsettle only cleared lifecycle columns. - const d = deps([{ id: "chat-1", toolType: "claude-chat" }]); - await stopSettledSessionMachinery(d, ["chat-1"]); - const resumed = await resumeSettledSessionMachinery(d, ["chat-1"]); - expect(d.setScheduledWorkPausedForSettle).toHaveBeenNthCalledWith(1, { sessionId: "chat-1", paused: true }); - expect(d.setScheduledWorkPausedForSettle).toHaveBeenNthCalledWith(2, { sessionId: "chat-1", paused: false }); - expect(resumed).toEqual({ sessionIds: ["chat-1"], resumedScheduledWork: 1 }); - }); - it("does not resume a pause settle never took", async () => { - // The scheduler refuses the claim when the user had already paused, and - // reports false — an unsettle must not restart schedules they stopped. - const d = deps([{ id: "chat-1", toolType: "claude-chat" }], { - setScheduledWorkPausedForSettle: vi.fn(async () => false), - }); - const resumed = await resumeSettledSessionMachinery(d, ["chat-1"]); - expect(resumed.resumedScheduledWork).toBe(0); - }); - it("leaves terminals alone on the resume path too", async () => { - const d = deps([{ id: "term-1", toolType: "shell" }]); - const resumed = await resumeSettledSessionMachinery(d, ["term-1"]); - expect(resumed.sessionIds).toEqual([]); - expect(d.setScheduledWorkPausedForSettle).not.toHaveBeenCalled(); - }); it("leaves terminal sessions alone — a terminal pane is user-owned", async () => { // The whole carve-out of settle teardown: an agent's background shell is @@ -111,7 +64,6 @@ describe("stopSettledSessionMachinery", () => { expect(result.sessionIds).toEqual([]); expect(d.stopBackgroundWork).not.toHaveBeenCalled(); - expect(d.setScheduledWorkPausedForSettle).not.toHaveBeenCalled(); }); it("reports a session skipped because its foreground turn is still streaming", async () => { diff --git a/docs/features/terminals-and-sessions/README.md b/docs/features/terminals-and-sessions/README.md index a92ffd41f..b74103c4a 100644 --- a/docs/features/terminals-and-sessions/README.md +++ b/docs/features/terminals-and-sessions/README.md @@ -208,20 +208,17 @@ and in tests. registry. Settle stops the machinery the session owns before it writes the lifecycle column — see `apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts`. It - pauses the session's scheduled work — pauses rather than cancels, and the - pause is durable, so it carries an exact undo: the scheduler records which - sessions **settle** paused (`settlePausedSessionIds`), and - `sessionService`'s `onSettleCleared` hook runs `resumeSettledSessionMachinery` - for every route that clears a settle. The hook sits at the column write rather - than in each caller because the most common unsettle is implicit — - `clearTurnStartMarkers`, i.e. a user sending the next message — and - per-caller wiring silently skipped it. A pause the user took deliberately is - never claimed and never resumed, and background work is never restarted: ADE - cannot re-spawn a shell it stopped. Settle itself is still explicit per entry - point because teardown must finish *before* the write; that includes the CTO - operator's `settleSession` tool. It also calls - `agentChatService.stopBackgroundWork`, which stops every live child before - the parent. **Terminal panes stay open**: an agent's background shell is + calls `agentChatService.stopBackgroundWork`, which stops every live child + before the parent. Every settle entry point runs it, including the CTO + operator's `settleSession` tool, because the teardown has to finish *before* + the lifecycle write. **Scheduled work is deliberately left running**: pausing + it would be durable, and `settled_at` is cleared from seven places (including + the hot `setLastOutputPreview` path), so a pause without a complete undo would + silently disable a user's own monitors and crons forever. ADE's scheduled work + is already visible and user-manageable (`scheduledWork` / `nextWakeAt` on the + summary, a per-session pause toggle), and `canonicalSessionState` already + handles a settled chat woken by a schedule — green while the turn streams, + then re-settled. **Terminal panes stay open**: an agent's background shell is thread background work, but a pane the user opened is theirs, and closing it on settle would destroy scrollback nobody asked to lose. An ACTIVE foreground turn is also left alone — its subagents are work the user can see happening, From 6d49a24ae52f8afc4bd6e7e848f9449dc81bb03b Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 01:07:38 -0400 Subject: [PATCH 11/14] fix(sessions): stop detached work mid-turn, wire RPC CTO teardown, drop the unhonest count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three P1s from Codex on 66e7dba6d. 1. Settling during an active turn tore down nothing. stopBackgroundWork returned early on a live turn while the caller still wrote settled_at, so PR auto-settlement, the CTO tool, and RPC callers left background shells running under a row that went quiet when the turn ended. The carve-out was too wide: a turn's own SUBAGENTS are work the user can see and are still spared, but its DETACHED background work outlives the turn by construction and an explicit settle is the user saying they are done with it. That now stops mid-turn; only stopActiveClaudeSubagents and cursor cloud-run cancellation are skipped while a turn runs. 2. The ADE RPC operator bridge never received the teardown control. adeRpcServer's createCtoOperatorTools construction had agentChatService in scope but did not pass it, so the CTO settle tool over the desktop socket filed rows without stopping their background work — the in-process path was fixed and the daemon path was not. Same bug class as every other 'wired in-process, missing from the daemon' regression. 3. The stopped count could not be kept honest. stopActiveClaudeSubagents routes through closeOpenClaudeBackgroundTasks, which closes a shell it FAILED to stop, so any before/after measurement silently counted unstoppable work as stopped. This is the third round to land on that number. It had no consumer anywhere, so it is gone rather than approximated; skippedActiveTurn is the remaining, checkable signal. Co-Authored-By: Claude Opus 5 --- apps/ade-cli/src/adeRpcServer.ts | 5 ++ .../services/chat/agentChatService.test.ts | 8 ++- .../main/services/chat/agentChatService.ts | 56 +++++++++++-------- .../src/main/services/prs/prAsync.test.ts | 2 +- .../sessions/sessionMachineryTeardown.ts | 12 ++-- .../services/sessions/sessionTeardown.test.ts | 11 ++-- 6 files changed, 52 insertions(+), 42 deletions(-) diff --git a/apps/ade-cli/src/adeRpcServer.ts b/apps/ade-cli/src/adeRpcServer.ts index 4f831ba71..d0f4a1084 100644 --- a/apps/ade-cli/src/adeRpcServer.ts +++ b/apps/ade-cli/src/adeRpcServer.ts @@ -2953,6 +2953,11 @@ async function runCtoOperatorBridgeTool( : null) ?? fallbackModelId; const tools = createCtoOperatorTools({ + // Without this the CTO settle tool reached this construction with a null + // chat service and filed rows without stopping their background work — the + // desktop socket-backed RPC path silently skipping the teardown the + // in-process path runs. + agentChatService: { stopBackgroundWork: agentChatService.stopBackgroundWork }, currentSessionId: session.identity.callerId || "ade-cli-cto", defaultLaneId, defaultModelId, diff --git a/apps/desktop/src/main/services/chat/agentChatService.test.ts b/apps/desktop/src/main/services/chat/agentChatService.test.ts index 0ce8fc156..ed1221456 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.test.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.test.ts @@ -11833,10 +11833,12 @@ describe("createAgentChatService", () => { // never in the quiet column. expect(live?.backgroundWork).toEqual({ workingCount: 3, monitoringCount: 1 }); - // A live turn is not ours to kill: its work is what the user can see - // happening, so settle teardown declines rather than stopping it. + // A live turn's own subagents are spared — they are work the user can see + // happening. Its DETACHED background work is not: the agent already + // backgrounded it, so an explicit settle stops it even mid-turn. Skipping + // everything mid-turn meant a settle during a turn tore down nothing. await expect(service.stopBackgroundWork({ sessionId: session.id })) - .resolves.toEqual({ stopped: 0, skippedActiveTurn: true }); + .resolves.toEqual({ skippedActiveTurn: true }); turnDone!(); await expect(sendPromise).resolves.toBeUndefined(); diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index 86660cf98..797e942a6 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -39321,34 +39321,45 @@ export function createAgentChatService(args: { * • Children stop before parents. Stopping only the parent leaves the fleet * running and untracked, which is how a "stopped" agent keeps spending. * - * Returns how much live work actually STOPPED — the drop in the session's - * live-work count across the call, not what it found. A runtime with no stop - * control reports 0 rather than claiming a teardown it never performed. + * Deliberately returns no count. An earlier version reported "how much was + * stopped" and could not keep the number honest: `closeOpenClaudeBackgroundTasks` + * (reached through `stopActiveClaudeSubagents`) closes a shell it failed to + * stop, which silently inflated any before/after measurement. The number had + * no consumer, so it is gone rather than approximated. */ const stopBackgroundWork = async ( { sessionId }: { sessionId: string }, - ): Promise<{ stopped: number; skippedActiveTurn: boolean }> => { + ): Promise<{ skippedActiveTurn: boolean }> => { const managed = managedSessions.get(sessionId.trim()); - if (!managed || managed.closed || managed.deleted) return { stopped: 0, skippedActiveTurn: false }; + if (!managed || managed.closed || managed.deleted) return { skippedActiveTurn: false }; const runtime = managed.runtime; - if (!runtime) return { stopped: 0, skippedActiveTurn: false }; - + if (!runtime) return { skippedActiveTurn: false }; + + // A live turn's own subagents are work the user can see happening, so they + // are never killed here. Its BACKGROUND work is different: the agent already + // detached it, it outlives the turn by construction, and an explicit settle + // is the user saying they are done with it. Skipping everything while a turn + // ran meant a settle during a turn tore down nothing at all, and the row + // then went quiet over shells that were still running. const turnActive = managed.session.status === "active" || Boolean(runtime.activeTurnId); - if (turnActive) return { stopped: 0, skippedActiveTurn: true }; - const before = totalBackgroundWork(runtimeBackgroundWork(runtime)); - if (before === 0) return { stopped: 0, skippedActiveTurn: false }; + if (totalBackgroundWork(runtimeBackgroundWork(runtime)) === 0) { + return { skippedActiveTurn: turnActive }; + } try { switch (runtime.kind) { case "claude": { // Children first: this drains workflow agents, then subagents, then - // the background shells each of them owns. - await stopActiveClaudeSubagents( - managed, - runtime, - runtime.activeTurnId ?? undefined, - "Stopped when the session was settled", - ); + // the background shells each of them owns. Skipped mid-turn — those + // subagents belong to the turn the user is watching. + if (!turnActive) { + await stopActiveClaudeSubagents( + managed, + runtime, + runtime.activeTurnId ?? undefined, + "Stopped when the session was settled", + ); + } // Anything still on the authoritative level had no `activeSubagents` // entry to be reached through — a plain backgrounded shell, usually. // Those are exactly the ones that survived the old teardown. @@ -39389,6 +39400,9 @@ export function createAgentChatService(args: { break; } case "cursor": { + // A cloud run is the turn's own execution, not detached background + // work, so a live turn keeps it. + if (turnActive) break; const agentId = managed.session.cursorCloudAgentId; if (!agentId) break; for (const runId of [...runtime.cloudRuns.keys()]) { @@ -39415,13 +39429,7 @@ export function createAgentChatService(args: { error: error instanceof Error ? error.message : String(error), }); } - // Measured as the DROP in live work rather than what was found, so a - // runtime with no stop control (Codex) or a cursor session with no cloud - // agent id reports 0 instead of claiming a teardown it never performed. - // Under-reporting is the safe direction: a caller may never be told more - // was stopped than actually was. - const after = totalBackgroundWork(runtimeBackgroundWork(runtime)); - return { stopped: Math.max(0, before - after), skippedActiveTurn: false }; + return { skippedActiveTurn: turnActive }; }; diff --git a/apps/desktop/src/main/services/prs/prAsync.test.ts b/apps/desktop/src/main/services/prs/prAsync.test.ts index 7d26b43bf..816896811 100644 --- a/apps/desktop/src/main/services/prs/prAsync.test.ts +++ b/apps/desktop/src/main/services/prs/prAsync.test.ts @@ -925,7 +925,7 @@ describe("prMergeAutoSettlementService", () => { }); const stopBackgroundWork = vi.fn(async () => { order.push("stop"); - return { stopped: 1, skippedActiveTurn: false }; + return { skippedActiveTurn: false }; }); const rows = [{ id: "chat-live", toolType: "claude-chat", archivedAt: null, settledAt: null }]; const service = createPrMergeAutoSettlementService({ diff --git a/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts b/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts index 6d4ea7067..e114b3d68 100644 --- a/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts +++ b/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts @@ -66,15 +66,15 @@ export type SessionMachineryTeardownDeps = { export type SessionMachineryTeardownResult = { /** Sessions whose machinery this pass touched. */ sessionIds: string[]; - /** Live background jobs stopped across those sessions. */ - stoppedBackgroundWork: number; - /** Sessions skipped because a foreground turn was still streaming. */ + /** + * Sessions whose foreground turn was still streaming. Their detached + * background work is still stopped; only the turn's own subagents are spared. + */ skippedActiveTurns: number; }; const EMPTY_RESULT: SessionMachineryTeardownResult = { sessionIds: [], - stoppedBackgroundWork: 0, skippedActiveTurns: 0, }; @@ -95,8 +95,7 @@ export async function stopSettledSessionMachinery( const result: SessionMachineryTeardownResult = { sessionIds: [], - stoppedBackgroundWork: 0, - skippedActiveTurns: 0, + skippedActiveTurns: 0, }; for (const sessionId of unique) { @@ -111,7 +110,6 @@ export async function stopSettledSessionMachinery( if (!service) continue; try { const stop = await service.stopBackgroundWork({ sessionId }); - result.stoppedBackgroundWork += stop.stopped; if (stop.skippedActiveTurn) result.skippedActiveTurns += 1; } catch (error) { deps.logger?.warn("session_teardown.stop_background_work_failed", { diff --git a/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts b/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts index b8edaf34b..3604cb58b 100644 --- a/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts +++ b/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts @@ -19,7 +19,7 @@ import type { createSessionService } from "./sessionService"; type Row = { id: string; toolType: string }; function deps(rows: Row[], overrides: Record = {}) { - const stopBackgroundWork = vi.fn(async () => ({ stopped: 2, skippedActiveTurn: false })); + const stopBackgroundWork = vi.fn(async () => ({ skippedActiveTurn: false })); return { sessionService: { get: (id: string) => rows.find((row) => row.id === id) ?? null, @@ -43,7 +43,6 @@ describe("stopSettledSessionMachinery", () => { expect(d.stopBackgroundWork).toHaveBeenCalledWith({ sessionId: "chat-1" }); expect(result).toMatchObject({ sessionIds: ["chat-1"], - stoppedBackgroundWork: 2, skippedActiveTurns: 0, }); }); @@ -68,11 +67,10 @@ describe("stopSettledSessionMachinery", () => { it("reports a session skipped because its foreground turn is still streaming", async () => { const d = deps([{ id: "chat-1", toolType: "claude-chat" }], { - stopBackgroundWork: vi.fn(async () => ({ stopped: 0, skippedActiveTurn: true })), + stopBackgroundWork: vi.fn(async () => ({ skippedActiveTurn: true })), }); const result = await stopSettledSessionMachinery(d, ["chat-1"]); expect(result.skippedActiveTurns).toBe(1); - expect(result.stoppedBackgroundWork).toBe(0); }); it("never lets a provider failure block the settle", async () => { @@ -83,7 +81,7 @@ describe("stopSettledSessionMachinery", () => { }); await expect(stopSettledSessionMachinery(d, ["chat-1"])).resolves.toMatchObject({ sessionIds: ["chat-1"], - stoppedBackgroundWork: 0, + skippedActiveTurns: 0, }); expect(d.logger.warn).toHaveBeenCalled(); }); @@ -104,7 +102,6 @@ describe("stopSettledSessionMachinery", () => { ["chat-1"], ); expect(result.sessionIds).toEqual(["chat-1"]); - expect(result.stoppedBackgroundWork).toBe(0); }); }); @@ -116,7 +113,7 @@ describe("settleTerminalSession", () => { const d = deps([{ id: "chat-1", toolType: "claude-chat" }], { stopBackgroundWork: vi.fn(async () => { order.push("stop"); - return { stopped: 1, skippedActiveTurn: false }; + return { skippedActiveTurn: false }; }), }); const sessionService = { From 83af875d7f948ebc1e6b8d62f308d5ed7d316528 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 01:29:43 -0400 Subject: [PATCH 12/14] fix(sessions): never close a background task whose stop was not confirmed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two P1s from Codex on cd53e3412. 1. An unconfirmed stop still closed the task row. closeOpenClaudeBackgroundTasks emits a terminal 'failed' update when stopTask is absent, times out, or rejects, and that removes the task from liveBackgroundTaskIds — which is exactly what runtimeBackgroundWork derives the row's user-visible liveness from. The session therefore went quiet over a shell that may still be running: the precise lie this whole change exists to remove. A stop we attempted and could not confirm now leaves the task LIVE and logs claude_background_stop_unconfirmed; the SDK's next authoritative level drains it if it really ended. Scoped to failed stop ATTEMPTS, so the turn-end close path ('completed', which attempts nothing) is unchanged. 2. Teardown raced the lifecycle write. Provider stop calls take seconds, and a user starting a turn inside that window runs clearTurnStartMarkers against a settle marker that does not exist yet — after which settleTerminalSession wrote settled_at over the freshly-active session, filing a live turn as settled. The settle now snapshots lastActivityAt before teardown and refuses the write if it moved. Real activity outranks a settle request that predates it. It reports true rather than false: the row exists and the request was handled, it simply woke, and false would surface a spurious 'not found'. Co-Authored-By: Claude Opus 5 --- .../main/services/chat/agentChatService.ts | 20 ++++++++ .../services/sessions/sessionTeardown.test.ts | 50 ++++++++++++++++++- .../sessions/settleTerminalSession.ts | 19 +++++++ 3 files changed, 88 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index 797e942a6..875c6e9e7 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -14627,6 +14627,26 @@ export function createAgentChatService(args: { if (stopState && !emittedByProvider) { stopState.emitted = true; } + // A stop we ATTEMPTED and could not confirm leaves the task live. + // + // Emitting a terminal row drops the task from `liveBackgroundTaskIds`, + // which is what `runtimeBackgroundWork` derives the row's user-visible + // liveness from — so an unconfirmed stop would make the session go quiet + // over a shell that is still running, which is the exact lie this whole + // feature exists to remove. Self-correcting: the SDK's next authoritative + // `background_tasks_changed` level drains it if it really did end. + // + // Only applies to a failed STOP. A turn-end close (`status: "completed"`) + // attempts nothing, so it still settles the row as before. + const stopAttemptFailed = canStopProviderTask && terminalStatus === "failed"; + if (stopAttemptFailed) { + logger.warn("agent_chat.claude_background_stop_unconfirmed", { + sessionId: managed.session.id, + taskId, + summary: terminalSummary, + }); + return; + } if (!emittedByProvider) { emitClaudeBackgroundTaskUpdate(managed, runtime, { taskId, diff --git a/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts b/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts index 3604cb58b..73d917f53 100644 --- a/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts +++ b/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts @@ -117,7 +117,7 @@ describe("settleTerminalSession", () => { }), }); const sessionService = { - get: (id: string) => (id === "chat-1" ? { id, toolType: "claude-chat" } : null), + get: (id: string) => (id === "chat-1" ? { id, toolType: "claude-chat", lastActivityAt: "t0" } : null), settleSession: vi.fn(() => { order.push("settle"); return true; @@ -224,3 +224,51 @@ describe("deleteTerminalSessionWithRuntimeCleanup", () => { })).toThrow("Use the chat delete flow instead."); }); }); + +describe("settleTerminalSession activity guard", () => { + it("refuses to settle over a turn that started while teardown was awaiting", () => { + // Provider stop calls take seconds. A user sending a message inside that + // window clears a settle marker that does not exist yet, and the write + // would then file the freshly-active session as settled. + let activity = "t0"; + const settleSession = vi.fn(() => true); + const sessionService = { + get: (id: string) => (id === "chat-1" ? { id, toolType: "claude-chat", lastActivityAt: activity } : null), + settleSession, + }; + const agentChatService = { + stopBackgroundWork: vi.fn(async () => { + activity = "t1"; // the user starts a turn mid-teardown + return { skippedActiveTurn: false }; + }), + }; + + return settleTerminalSession({ + sessionId: "chat-1", + sessionService: sessionService as never, + agentChatService: agentChatService as never, + }).then((settled) => { + // Reported as handled — the row exists and the request was honoured — but + // deliberately NOT settled. Returning false would surface a spurious + // "session not found" to the caller. + expect(settled).toBe(true); + expect(settleSession).not.toHaveBeenCalled(); + }); + }); + + it("settles normally when nothing happened during teardown", () => { + const settleSession = vi.fn(() => true); + const sessionService = { + get: () => ({ id: "chat-1", toolType: "claude-chat", lastActivityAt: "t0" }), + settleSession, + }; + return settleTerminalSession({ + sessionId: "chat-1", + sessionService: sessionService as never, + agentChatService: { stopBackgroundWork: vi.fn(async () => ({ skippedActiveTurn: false })) } as never, + }).then((settled) => { + expect(settled).toBe(true); + expect(settleSession).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/apps/desktop/src/main/services/sessions/settleTerminalSession.ts b/apps/desktop/src/main/services/sessions/settleTerminalSession.ts index 12b1d7438..7c0349293 100644 --- a/apps/desktop/src/main/services/sessions/settleTerminalSession.ts +++ b/apps/desktop/src/main/services/sessions/settleTerminalSession.ts @@ -72,6 +72,14 @@ export async function settleTerminalSession(args: { if (!dismissed) return false; } + // Teardown awaits provider stop calls that can take seconds, and a user can + // start a new turn inside that window. `clearTurnStartMarkers` would clear a + // settle marker that does not exist yet, and this write would then file the + // freshly-active session as settled. Snapshot the activity stamp first and + // refuse to settle over work that arrived while we were stopping things — + // real activity outranks a settle request that predates it. + const activityBeforeTeardown = args.sessionService.get(args.sessionId)?.lastActivityAt ?? null; + await stopSettledSessionMachinery( { sessionService: args.sessionService, @@ -81,6 +89,17 @@ export async function settleTerminalSession(args: { [args.sessionId], ); + const after = args.sessionService.get(args.sessionId); + if (!after) return false; + if ((after.lastActivityAt ?? null) !== activityBeforeTeardown) { + args.logger?.warn("session_teardown.settle_skipped_new_activity", { + sessionId: args.sessionId, + }); + // The row exists and the request was honoured; it simply woke, so it is not + // settled. Reporting false here would surface a spurious "not found". + return true; + } + return args.sessionService.settleSession( args.sessionId, { From 7eff3683b708d5676c364e0285df957b76ab4e93 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 01:55:03 -0400 Subject: [PATCH 13/14] revert(sessions): settle no longer stops background work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cutting the second and last slice of settle teardown. Six review rounds, every one of them finding a real defect in this specific mechanism: - an unsettle path that skipped the resume (x3, each a route the previous fix had not traced), - settling mid-turn tearing down nothing while the caller still wrote the marker, - the RPC operator bridge never receiving the teardown control, - a stop count that could not be kept honest, - and finally an activity guard that reads lastActivityAt — which is backed by last_output_at, a column clearTurnStartMarkers never writes. The guard I added last round provably cannot fire. The shape is now unambiguous. Teardown is async; settled_at is written and cleared from seven places. A teardown-then-write settle races real activity, and the failure is not one-sided: a user starting a turn during a provider stop call gets their background work stopped AND no settle. Every guard against it either read a column turn-start does not update, or had to be repeated identically at each settle entry point (settleTerminalSession, bulk registry, both IPC handlers, both sync commands, PR auto-settlement, the CTO tool). Doing this correctly needs a synchronous lifecycle revision that teardown can be serialized against — a different change, designed as one, not a wrapper around the existing write. Shipping the half-working version is worse than the status quo, which is the one thing the brief specifically warned about. What ships instead is the half that has been stable since iteration 2 and is what a user actually sees: live background work promoted into the canonical phase across every glanceable surface, the working/monitoring denylist, cross-runtime generalization, running-count subagent badges, and the archive port-lease ordering fix — archive remains the lifecycle path that does stop processes, and its ordering bug is fixed. Removed: sessionMachineryTeardown, stopBackgroundWork, the activity guard, and the teardown calls in every settle entry point. runtimeBackgroundWork stays; it is the surfacing half. Co-Authored-By: Claude Opus 5 --- apps/ade-cli/src/adeRpcServer.ts | 5 - apps/ade-cli/src/bootstrap.ts | 2 - .../services/sync/syncRemoteCommandService.ts | 10 - apps/desktop/src/main/main.ts | 2 - .../main/services/adeActions/registry.test.ts | 14 +- .../src/main/services/adeActions/registry.ts | 21 +- .../services/ai/tools/ctoOperatorTools.ts | 22 -- .../services/chat/agentChatService.test.ts | 6 - .../main/services/chat/agentChatService.ts | 130 --------- .../src/main/services/ipc/registerIpc.ts | 11 +- .../src/main/services/prs/prAsync.test.ts | 41 --- .../prs/prMergeAutoSettlementService.ts | 25 -- .../sessions/deleteTerminalSession.test.ts | 97 +++++++ .../sessions/sessionMachineryTeardown.ts | 124 -------- .../services/sessions/sessionTeardown.test.ts | 274 ------------------ .../sessions/settleTerminalSession.ts | 38 --- .../features/terminals-and-sessions/README.md | 37 +-- 17 files changed, 115 insertions(+), 744 deletions(-) create mode 100644 apps/desktop/src/main/services/sessions/deleteTerminalSession.test.ts delete mode 100644 apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts delete mode 100644 apps/desktop/src/main/services/sessions/sessionTeardown.test.ts diff --git a/apps/ade-cli/src/adeRpcServer.ts b/apps/ade-cli/src/adeRpcServer.ts index d0f4a1084..4f831ba71 100644 --- a/apps/ade-cli/src/adeRpcServer.ts +++ b/apps/ade-cli/src/adeRpcServer.ts @@ -2953,11 +2953,6 @@ async function runCtoOperatorBridgeTool( : null) ?? fallbackModelId; const tools = createCtoOperatorTools({ - // Without this the CTO settle tool reached this construction with a null - // chat service and filed rows without stopping their background work — the - // desktop socket-backed RPC path silently skipping the teardown the - // in-process path runs. - agentChatService: { stopBackgroundWork: agentChatService.stopBackgroundWork }, currentSessionId: session.identity.callerId || "ade-cli-cto", defaultLaneId, defaultModelId, diff --git a/apps/ade-cli/src/bootstrap.ts b/apps/ade-cli/src/bootstrap.ts index 723e9218b..d09b68a91 100644 --- a/apps/ade-cli/src/bootstrap.ts +++ b/apps/ade-cli/src/bootstrap.ts @@ -1434,8 +1434,6 @@ export async function createAdeRuntime(args: { const prMergeAutoSettlementService = createPrMergeAutoSettlementService({ db, sessionService, - agentChatService, - logger, emitEvent: emitPrEvent, }); diff --git a/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts b/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts index 5ec1d4418..c27fd6d5c 100644 --- a/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts +++ b/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts @@ -278,7 +278,6 @@ import type { ProductAnalyticsService } from "../../../../desktop/src/main/servi import { parseProductAnalyticsCapture } from "../../../../desktop/src/shared/types/productAnalytics"; import { deleteTerminalSessionWithRuntimeCleanup } from "../../../../desktop/src/main/services/sessions/deleteTerminalSession"; import { dismissPendingInputBeforeSettle, settleTerminalSession } from "../../../../desktop/src/main/services/sessions/settleTerminalSession"; -import { stopSettledSessionMachinery } from "../../../../desktop/src/main/services/sessions/sessionMachineryTeardown"; import type { createSessionDeltaService } from "../../../../desktop/src/main/services/sessions/sessionDeltaService"; import type { createSessionService } from "../../../../desktop/src/main/services/sessions/sessionService"; import { getSharedModelPickerStore, type ModelPickerStore } from "../modelPickerStore"; @@ -4093,7 +4092,6 @@ function registerWorkRemoteCommands({ args, register }: RemoteCommandRegistratio sessionService: args.sessionService, agentChatService: args.agentChatService ?? null, ptyService: args.ptyService, - logger: args.logger, }); if (!settled) throw new Error(`Session '${sessionId}' was not found.`); return { ok: true, sessionId }; @@ -4131,14 +4129,6 @@ function registerWorkRemoteCommands({ args, register }: RemoteCommandRegistratio ptyService: args.ptyService, }); } - await stopSettledSessionMachinery( - { - sessionService: args.sessionService, - agentChatService: args.agentChatService ?? null, - logger: args.logger, - }, - sessionIds, - ); return args.sessionService.settleSessions(sessionIds); }); register("session.unsettleSessions", { viewerAllowed: true, queueable: true }, async (payload) => { diff --git a/apps/desktop/src/main/main.ts b/apps/desktop/src/main/main.ts index f07806c2c..48f9917b9 100644 --- a/apps/desktop/src/main/main.ts +++ b/apps/desktop/src/main/main.ts @@ -3588,8 +3588,6 @@ app.whenReady().then(async () => { prMergeAutoSettlementServiceRef = createPrMergeAutoSettlementService({ db, sessionService, - agentChatService, - logger, emitEvent: emitPrEvent, }); laneTeardownDeps.agentChatService = { diff --git a/apps/desktop/src/main/services/adeActions/registry.test.ts b/apps/desktop/src/main/services/adeActions/registry.test.ts index e13d93452..c4d218e10 100644 --- a/apps/desktop/src/main/services/adeActions/registry.test.ts +++ b/apps/desktop/src/main/services/adeActions/registry.test.ts @@ -1595,7 +1595,7 @@ describe("runtime session actions", () => { // session. This bulk action never has, and used to drop the key silently — so // the same argument meant "dismiss the prompt" over sync and nothing at all // here. Settling while quietly ignoring half the request is the failure mode. - it("refuses a bulk settle that asks to dismiss pending input", async () => { + it("refuses a bulk settle that asks to dismiss pending input", () => { const settleSessions = vi.fn(() => ["session-1"]); const runtime = { sessionService: { @@ -1608,21 +1608,15 @@ describe("runtime session actions", () => { settleSessions: (args: unknown) => unknown; } & Record; - // SYNCHRONOUS throw, even though the success path is now async: settle grew - // a teardown step, and marking the whole action `async` would have quietly - // turned this guard into a rejected promise that a non-awaiting caller - // drops on the floor. expect(() => sessionService.settleSessions({ sessionIds: ["session-1"], dismissPendingInput: true, })).toThrow(/does not dismiss pending input/); expect(settleSessions).not.toHaveBeenCalled(); - // Without the flag the bulk path is untouched — but it is awaited now, - // because the session's monitors and background shells have to be stopped - // before the settle is written. - await expect(sessionService.settleSessions({ sessionIds: ["session-1", "session-2"] })) - .resolves.toEqual(["session-1"]); + // Without the flag the bulk path is untouched. + expect(sessionService.settleSessions({ sessionIds: ["session-1", "session-2"] })) + .toEqual(["session-1"]); expect(settleSessions).toHaveBeenCalledWith(["session-1", "session-2"]); }); diff --git a/apps/desktop/src/main/services/adeActions/registry.ts b/apps/desktop/src/main/services/adeActions/registry.ts index 360b87259..5264eeb61 100644 --- a/apps/desktop/src/main/services/adeActions/registry.ts +++ b/apps/desktop/src/main/services/adeActions/registry.ts @@ -126,7 +126,6 @@ import { parseWakeReason, } from "../sessions/sessionRequestValidation"; import { settleTerminalSession } from "../sessions/settleTerminalSession"; -import { stopSettledSessionMachinery } from "../sessions/sessionMachineryTeardown"; import { getSessionLifecycleSettings, setSessionLifecycleSettings, @@ -2127,7 +2126,6 @@ function buildSessionDomainService(runtime: AdeRuntime): OpaqueService | null { sessionService, agentChatService: runtime.agentChatService, ptyService: runtime.ptyService, - logger: runtime.logger, })) { throw new Error(`Session '${sessionId}' was not found.`); } @@ -2148,13 +2146,6 @@ function buildSessionDomainService(runtime: AdeRuntime): OpaqueService | null { }, // Bulk settle/unsettle for renderer surfaces on remote-bound projects // (mirrors deleteSession's generic trust posture). - // - // Deliberately NOT an `async` function: argument validation below must keep - // throwing SYNCHRONOUSLY, the way it did before settle grew a teardown - // step. Marking the whole action `async` silently converts every one of - // those guards into a rejected promise, which changes the contract for any - // caller that does not await — so only the success path is async, returned - // as an explicit promise from a sync body. settleSessions: (args?: unknown) => { const record = readObjectActionArg(args, "session.settleSessions"); const sessionIds = Array.isArray(record.sessionIds) @@ -2177,17 +2168,7 @@ function buildSessionDomainService(runtime: AdeRuntime): OpaqueService | null { "session.settleSessions does not dismiss pending input; use session.settleSession for a single session.", ); } - return (async () => { - await stopSettledSessionMachinery( - { - sessionService, - agentChatService: runtime.agentChatService, - logger: runtime.logger, - }, - sessionIds, - ); - return sessionService.settleSessions(sessionIds); - })(); + return sessionService.settleSessions(sessionIds); }, unsettleSessions: (args?: unknown) => { const record = readObjectActionArg(args, "session.unsettleSessions"); diff --git a/apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts b/apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts index 19bb8fae3..416d69c88 100644 --- a/apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts +++ b/apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts @@ -24,10 +24,6 @@ import type { createFileService } from "../../files/fileService"; import type { createLaneService } from "../../lanes/laneService"; import type { createPrService } from "../../prs/prService"; import type { createSessionService } from "../../sessions/sessionService"; -import { - stopSettledSessionMachinery, - type SessionMachineryTeardownDeps, -} from "../../sessions/sessionMachineryTeardown"; import { parseSnoozeDeadline } from "../../sessions/sessionRequestValidation"; import type { createCtoStateService } from "../../cto/ctoStateService"; import type { CtoMemoryService } from "../../cto/ctoMemoryService"; @@ -59,12 +55,6 @@ export interface CtoOperatorToolDeps { | "wakeSession" | "clearWokeMarker" >; - /** - * Only used to tear down what a settled session owns. Optional so the tools - * stay constructible without a chat runtime; absent, the settle still files - * the row, it just cannot stop the row's machinery. - */ - agentChatService?: SessionMachineryTeardownDeps["agentChatService"]; testService?: { listSuites: () => TestSuiteDefinition[]; run: (args: { laneId: string; suiteId: string }) => Promise; @@ -562,18 +552,6 @@ export function createCtoOperatorTools(deps: CtoOperatorToolDeps): Record { try { - // The operator settle is a real settle, so it stops the session's - // machinery like every other one. Without this the CTO could file a - // chat whose monitors kept polling and whose background fleet kept - // spending — the exact bug settle teardown exists to close, preserved - // in the one path that bypassed the shared entry points. - await stopSettledSessionMachinery( - { - sessionService: deps.sessionService, - agentChatService: deps.agentChatService ?? null, - }, - [sessionId], - ); const ok = deps.sessionService.settleSession(sessionId, { ...(outcome ? { outcome } : {}), source: "operator", diff --git a/apps/desktop/src/main/services/chat/agentChatService.test.ts b/apps/desktop/src/main/services/chat/agentChatService.test.ts index ed1221456..b92f4f534 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.test.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.test.ts @@ -11833,12 +11833,6 @@ describe("createAgentChatService", () => { // never in the quiet column. expect(live?.backgroundWork).toEqual({ workingCount: 3, monitoringCount: 1 }); - // A live turn's own subagents are spared — they are work the user can see - // happening. Its DETACHED background work is not: the agent already - // backgrounded it, so an explicit settle stops it even mid-turn. Skipping - // everything mid-turn meant a settle during a turn tore down nothing. - await expect(service.stopBackgroundWork({ sessionId: session.id })) - .resolves.toEqual({ skippedActiveTurn: true }); turnDone!(); await expect(sendPromise).resolves.toBeUndefined(); diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index 875c6e9e7..42815d84d 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -8374,9 +8374,6 @@ export function createAgentChatService(args: { defaultModelId: modelId, defaultReasoningEffort: reasoningEffort, resolveExecutionLane: resolveCtoExecutionLane, - // So the operator settle tears down the session's machinery like every - // other settle entry point rather than being the one path that skips it. - agentChatService: { stopBackgroundWork }, laneService, prService: prService ?? null, fileService: fileService ?? null, @@ -14627,26 +14624,6 @@ export function createAgentChatService(args: { if (stopState && !emittedByProvider) { stopState.emitted = true; } - // A stop we ATTEMPTED and could not confirm leaves the task live. - // - // Emitting a terminal row drops the task from `liveBackgroundTaskIds`, - // which is what `runtimeBackgroundWork` derives the row's user-visible - // liveness from — so an unconfirmed stop would make the session go quiet - // over a shell that is still running, which is the exact lie this whole - // feature exists to remove. Self-correcting: the SDK's next authoritative - // `background_tasks_changed` level drains it if it really did end. - // - // Only applies to a failed STOP. A turn-end close (`status: "completed"`) - // attempts nothing, so it still settles the row as before. - const stopAttemptFailed = canStopProviderTask && terminalStatus === "failed"; - if (stopAttemptFailed) { - logger.warn("agent_chat.claude_background_stop_unconfirmed", { - sessionId: managed.session.id, - taskId, - summary: terminalSummary, - }); - return; - } if (!emittedByProvider) { emitClaudeBackgroundTaskUpdate(managed, runtime, { taskId, @@ -39347,112 +39324,6 @@ export function createAgentChatService(args: { * stop, which silently inflated any before/after measurement. The number had * no consumer, so it is gone rather than approximated. */ - const stopBackgroundWork = async ( - { sessionId }: { sessionId: string }, - ): Promise<{ skippedActiveTurn: boolean }> => { - const managed = managedSessions.get(sessionId.trim()); - if (!managed || managed.closed || managed.deleted) return { skippedActiveTurn: false }; - const runtime = managed.runtime; - if (!runtime) return { skippedActiveTurn: false }; - - // A live turn's own subagents are work the user can see happening, so they - // are never killed here. Its BACKGROUND work is different: the agent already - // detached it, it outlives the turn by construction, and an explicit settle - // is the user saying they are done with it. Skipping everything while a turn - // ran meant a settle during a turn tore down nothing at all, and the row - // then went quiet over shells that were still running. - const turnActive = managed.session.status === "active" || Boolean(runtime.activeTurnId); - if (totalBackgroundWork(runtimeBackgroundWork(runtime)) === 0) { - return { skippedActiveTurn: turnActive }; - } - - try { - switch (runtime.kind) { - case "claude": { - // Children first: this drains workflow agents, then subagents, then - // the background shells each of them owns. Skipped mid-turn — those - // subagents belong to the turn the user is watching. - if (!turnActive) { - await stopActiveClaudeSubagents( - managed, - runtime, - runtime.activeTurnId ?? undefined, - "Stopped when the session was settled", - ); - } - // Anything still on the authoritative level had no `activeSubagents` - // entry to be reached through — a plain backgrounded shell, usually. - // Those are exactly the ones that survived the old teardown. - const control = getClaudeQueryControl(runtime.query); - for (const taskId of [...runtime.liveBackgroundTaskIds]) { - // A task ADE could not actually stop stays LIVE. - // - // Emitting a terminal row here would drop it from - // `liveBackgroundTaskIds`, which is what the caller measures the - // stop against — so a stop that failed, timed out, or found no stop - // control would be counted as a stop that worked, and the settled - // row would hide a process still burning tokens. Leaving it live is - // self-correcting: the SDK's next authoritative level drains it if - // it really did end. - if (typeof control.stopTask !== "function") { - logger.warn("agent_chat.settle_background_stop_unavailable", { - sessionId: managed.session.id, - taskId, - }); - continue; - } - try { - await awaitClaudeControlCall( - `Stopping Claude background task '${taskId}'`, - CLAUDE_STOP_TASK_TIMEOUT_MS, - () => control.stopTask!(taskId), - ); - } catch (error) { - logger.warn("agent_chat.settle_background_stop_failed", { - sessionId: managed.session.id, - taskId, - error: error instanceof Error ? error.message : String(error), - }); - continue; - } - emitClaudeBackgroundTaskUpdate(managed, runtime, { taskId, status: "stopped" }); - } - break; - } - case "cursor": { - // A cloud run is the turn's own execution, not detached background - // work, so a live turn keeps it. - if (turnActive) break; - const agentId = managed.session.cursorCloudAgentId; - if (!agentId) break; - for (const runId of [...runtime.cloudRuns.keys()]) { - await cancelCursorCloudRun({ agentId, runId }).catch((error: unknown) => { - logger.warn("agent_chat.settle_cloud_cancel_failed", { - sessionId: managed.session.id, - runId, - error: error instanceof Error ? error.message : String(error), - }); - }); - } - break; - } - default: - // Codex reports background subagents but exposes no per-subagent stop - // control, and opencode/droid/pi report no background work at all. - // Clearing ADE's tracking without actually stopping anything would - // make the row lie, so this is deliberately a no-op for them. - break; - } - } catch (error) { - logger.warn("agent_chat.settle_background_teardown_failed", { - sessionId: managed.session.id, - error: error instanceof Error ? error.message : String(error), - }); - } - return { skippedActiveTurn: turnActive }; - }; - - const hasActiveWorkloads = (): boolean => { for (const managed of managedSessions.values()) { if (managed.closed || managed.deleted) continue; @@ -44334,7 +44205,6 @@ export function createAgentChatService(args: { getSessionSummary, ensureSessionSurface, hasActiveWorkloads, - stopBackgroundWork, hasRetainableSessions, countActiveForLane, disposeForLane, diff --git a/apps/desktop/src/main/services/ipc/registerIpc.ts b/apps/desktop/src/main/services/ipc/registerIpc.ts index 2d17d6cab..85b28f5c9 100644 --- a/apps/desktop/src/main/services/ipc/registerIpc.ts +++ b/apps/desktop/src/main/services/ipc/registerIpc.ts @@ -51,7 +51,6 @@ import { parseWakeReason, } from "../sessions/sessionRequestValidation"; import { settleTerminalSession } from "../sessions/settleTerminalSession"; -import { stopSettledSessionMachinery } from "../sessions/sessionMachineryTeardown"; import { getSessionLifecycleSettings, setSessionLifecycleSettings, @@ -6967,7 +6966,6 @@ export function registerIpc({ sessionService: ctx.sessionService, agentChatService: ctx.agentChatService, ptyService: ctx.ptyService, - logger: ctx.logger, }); if (!settled) throw new Error(`Session '${sessionId}' was not found.`); }, @@ -6988,14 +6986,9 @@ export function registerIpc({ async (_event, arg: { sessionIds?: unknown }): Promise => { const ctx = ensureSessionContext(); if (!Array.isArray(arg?.sessionIds)) throw new Error("Session ids are required."); - const sessionIds = arg.sessionIds.filter( - (sessionId): sessionId is string => typeof sessionId === "string", - ); - await stopSettledSessionMachinery( - { sessionService: ctx.sessionService, agentChatService: ctx.agentChatService, logger: ctx.logger }, - sessionIds, + return ctx.sessionService.settleSessions( + arg.sessionIds.filter((sessionId): sessionId is string => typeof sessionId === "string"), ); - return ctx.sessionService.settleSessions(sessionIds); }, ); diff --git a/apps/desktop/src/main/services/prs/prAsync.test.ts b/apps/desktop/src/main/services/prs/prAsync.test.ts index 816896811..a586a2123 100644 --- a/apps/desktop/src/main/services/prs/prAsync.test.ts +++ b/apps/desktop/src/main/services/prs/prAsync.test.ts @@ -912,47 +912,6 @@ describe("prMergeAutoSettlementService", () => { expect(settleSessionsWithOutcome).toHaveBeenCalledTimes(2); }); - it("stops the merged session's machinery before filing it", async () => { - // This path deliberately bypasses the settlement blockers, so it is the - // one most likely to file a session that is still running something. Before - // teardown reached it, the merged lane's monitors kept polling and woke the - // thread hours after the PR had landed. - const db = createMemoryDb(); - const order: string[] = []; - const settleSessionsWithOutcome = vi.fn((ids: string[]) => { - order.push("settle"); - return ids; - }); - const stopBackgroundWork = vi.fn(async () => { - order.push("stop"); - return { skippedActiveTurn: false }; - }); - const rows = [{ id: "chat-live", toolType: "claude-chat", archivedAt: null, settledAt: null }]; - const service = createPrMergeAutoSettlementService({ - db: db as any, - sessionService: { - list: vi.fn(() => rows), - get: vi.fn((id: string) => rows.find((row) => row.id === id) ?? null), - settleSessionsWithOutcome, - } as any, - agentChatService: { - stopBackgroundWork, - } as any, - emitEvent: vi.fn(), - }); - - await service.processSnapshot({ - prs: [createSummary({ state: "open" })], - polledAt: "2026-03-24T12:00:00.000Z", - }); - await service.processSnapshot({ - prs: [createSummary({ state: "merged", mergedAt: "2026-03-24T12:01:00.000Z" })], - polledAt: "2026-03-24T12:01:30.000Z", - }); - - expect(stopBackgroundWork).toHaveBeenCalledWith({ sessionId: "chat-live" }); - expect(order).toEqual(["stop", "settle"]); - }); it("does not re-settle after reactivation, but settles for a later PR", async () => { const db = createMemoryDb(); diff --git a/apps/desktop/src/main/services/prs/prMergeAutoSettlementService.ts b/apps/desktop/src/main/services/prs/prMergeAutoSettlementService.ts index da5eb9355..cd6ebf8c3 100644 --- a/apps/desktop/src/main/services/prs/prMergeAutoSettlementService.ts +++ b/apps/desktop/src/main/services/prs/prMergeAutoSettlementService.ts @@ -11,10 +11,6 @@ import { isTrackedAgentCliToolType, } from "../../../shared/types"; import { isChatToolType } from "../sessions/chatSessionProjection"; -import { - stopSettledSessionMachinery, - type SessionMachineryTeardownDeps, -} from "../sessions/sessionMachineryTeardown"; function isMergeAtOrAfter(mergedAt: string | null | undefined, enabledSince: string): boolean { const mergedMs = Date.parse(mergedAt ?? ""); @@ -67,13 +63,6 @@ function resolveMergeSettlementScope(pr: PrSummary, snapshot: PrSummary[]): Merg export function createPrMergeAutoSettlementService(args: { db: Pick; sessionService: Pick, "get" | "list" | "settleSessionsWithOutcome">; - /** - * Optional so the service stays constructible in tests and headless hosts - * without a chat runtime. Absent, the settle still files the row — it just - * cannot stop what the row owns, which is the pre-teardown behaviour. - */ - agentChatService?: SessionMachineryTeardownDeps["agentChatService"]; - logger?: SessionMachineryTeardownDeps["logger"]; emitEvent: (event: PrEventPayload) => void; }) { /** @@ -196,20 +185,6 @@ export function createPrMergeAutoSettlementService(args: { // session even when it still owns scheduled work, a background task, // or another normal settlement blocker. Real activity can unsettle it // again, while handledPrIds prevents this PR from filing it twice. - // - // Because this path deliberately bypasses the settlement blockers, it - // is the one most likely to file a session that IS still running - // something — which is exactly why the teardown has to run here too. - // Without it, the merged lane's monitors kept polling and woke the - // thread hours after the PR landed. - await stopSettledSessionMachinery( - { - sessionService: args.sessionService, - agentChatService: args.agentChatService ?? null, - logger: args.logger ?? null, - }, - [session.id], - ); settledSessionIds.push(...args.sessionService.settleSessionsWithOutcome( [session.id], `PR #${pr.githubPrNumber} merged`, diff --git a/apps/desktop/src/main/services/sessions/deleteTerminalSession.test.ts b/apps/desktop/src/main/services/sessions/deleteTerminalSession.test.ts new file mode 100644 index 000000000..32a4e1147 --- /dev/null +++ b/apps/desktop/src/main/services/sessions/deleteTerminalSession.test.ts @@ -0,0 +1,97 @@ +import { describe, expect, it, vi } from "vitest"; +import type { TerminalSessionSummary } from "../../../shared/types"; +import { deleteTerminalSessionWithRuntimeCleanup } from "./deleteTerminalSession"; +import type { createPtyService } from "../pty/ptyService"; +import type { createSessionService } from "./sessionService"; + + + + +function makeSession(overrides: Partial = {}): TerminalSessionSummary { + return { + id: "session-1", + laneId: "lane-1", + laneName: "Primary", + ptyId: null, + tracked: true, + pinned: false, + goal: null, + toolType: "shell", + title: "Shell", + status: "completed", + startedAt: "2026-08-01T00:00:00.000Z", + endedAt: "2026-08-01T00:01:00.000Z", + exitCode: 0, + transcriptPath: "/tmp/transcript", + headShaStart: null, + headShaEnd: null, + lastOutputPreview: null, + summary: null, + runtimeState: "exited", + resumeCommand: null, + ...overrides, + }; +} + +function makeServices(session: TerminalSessionSummary | null) { + const deleteSession = vi.fn().mockReturnValue(true); + const sessionService = { + get: vi.fn().mockReturnValue(session), + deleteSession, + } as unknown as ReturnType; + const ptyService = { + enrichSessions: vi.fn((sessions: TerminalSessionSummary[]) => sessions), + isSessionOwnedByLivePeerRuntime: vi.fn().mockReturnValue(false), + dispose: vi.fn(), + } as unknown as ReturnType; + return { deleteSession, ptyService, sessionService }; +} + +describe("deleteTerminalSessionWithRuntimeCleanup", () => { + it("deletes a session this runtime owns", () => { + const { deleteSession, ptyService, sessionService } = makeServices(makeSession()); + + expect(deleteTerminalSessionWithRuntimeCleanup({ + sessionId: "session-1", + sessionService, + ptyService, + })).toBe(true); + expect(deleteSession).toHaveBeenCalledWith("session-1"); + }); + + it("treats a session this runtime does not have as already deleted", () => { + // Delete is idempotent: the goal state is "not here", and it already holds. + // Throwing surfaced a red "Delete failed" banner over a list that was + // correct — the renderer routinely asks a runtime to delete a row that + // never persisted there, or that another window already removed. + const { deleteSession, ptyService, sessionService } = makeServices(null); + + expect(deleteTerminalSessionWithRuntimeCleanup({ + sessionId: "missing-session", + sessionService, + ptyService, + })).toBe(false); + expect(deleteSession).not.toHaveBeenCalled(); + }); + + it("still rejects an empty session id", () => { + const { ptyService, sessionService } = makeServices(makeSession()); + + expect(() => deleteTerminalSessionWithRuntimeCleanup({ + sessionId: " ", + sessionService, + ptyService, + })).toThrow("Session id is required."); + }); + + it("still refuses a chat session", () => { + const { ptyService, sessionService } = makeServices(makeSession({ toolType: "codex-chat" })); + + expect(() => deleteTerminalSessionWithRuntimeCleanup({ + sessionId: "session-1", + sessionService, + ptyService, + })).toThrow("Use the chat delete flow instead."); + }); +}); + diff --git a/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts b/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts deleted file mode 100644 index e114b3d68..000000000 --- a/apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts +++ /dev/null @@ -1,124 +0,0 @@ -import type { createAgentChatService } from "../chat/agentChatService"; -import type { createSessionService } from "./sessionService"; -import { isChatToolType } from "./chatSessionProjection"; - -/** - * Stop the machinery a session owns when its lifecycle ends. - * - * ── Why this exists ───────────────────────────────────────────────────────── - * - * Settle used to be a pure column write. The row went quiet and everything the - * session had started kept going: background shells held ports, subagent fleets - * kept spending tokens, and scheduled work woke the thread hours after the user - * had declared it done. "Settled" claimed a conclusion the process tree had not - * reached. - * - * Archive had the mirror problem from the other end — it released the lane's - * port lease and proxy route while the processes were still holding those - * ports, so the lease could be handed to another lane that then could not bind. - * - * Both now converge here so the step list cannot drift into two versions. - * - * ── What settle stops, and what it deliberately does not ──────────────────── - * - * stops live background work — background shells, subagent fleets, - * cursor cloud runs — via `agentChatService.stopBackgroundWork` - * keeps the session itself, and its runtime, alive and resumable - * keeps terminal panes open — a terminal is USER-owned. An agent's - * background shell is thread background work; the pane the user - * opened to watch a build is theirs, and closing it on settle would - * destroy scrollback they never asked to lose. - * keeps an ACTIVE foreground turn running (see `stopBackgroundWork`). - * - * ── Scheduled work is deliberately NOT stopped ────────────────────────────── - * - * An earlier version of this paused the session's durable schedules. It was - * removed, and the reason is worth keeping: the pause is persisted, so it needs - * an exact undo on every route that clears a settle — and `settled_at` is - * cleared from seven places, including `setLastOutputPreview` on the hot PTY - * output path. Three review rounds each found another route that skipped the - * resume and left a chat's monitors and crons disabled forever. A pause without - * a complete undo is a slower deletion of the user's own schedules. - * - * It is also the smaller loss than it looks. ADE's scheduled work is already - * visible and user-manageable (`scheduledWork` and `nextWakeAt` on the summary, - * a per-session pause toggle), and `canonicalSessionState` already handles a - * settled chat woken by scheduled work: it shows green while the turn streams, - * then re-settles. The unmanaged, invisible thing settle needed to stop was - * background work, and that is what it stops. - * - * ── What escapes, stated plainly ──────────────────────────────────────────── - * - * A process an agent detached with `nohup`, `setsid`, or `disown` leaves ADE's - * tree entirely and nothing here can reach it. Codex background subagents are - * reported but expose no stop control. Neither is silently pretended away: - * `stopBackgroundWork` returns what it actually acted on. - */ -export type SessionMachineryTeardownDeps = { - sessionService: Pick, "get">; - agentChatService?: Pick< - ReturnType, - "stopBackgroundWork" - > | null; - logger?: { warn: (message: string, meta?: Record) => void } | null; -}; - -export type SessionMachineryTeardownResult = { - /** Sessions whose machinery this pass touched. */ - sessionIds: string[]; - /** - * Sessions whose foreground turn was still streaming. Their detached - * background work is still stopped; only the turn's own subagents are spared. - */ - skippedActiveTurns: number; -}; - -const EMPTY_RESULT: SessionMachineryTeardownResult = { - sessionIds: [], - skippedActiveTurns: 0, -}; - - -/** - * Stop the background machinery for a set of sessions being settled. - * - * Best-effort by construction: a settle must not fail because a provider could - * not be reached, so every step swallows its own error and the result reports - * what actually happened. - */ -export async function stopSettledSessionMachinery( - deps: SessionMachineryTeardownDeps, - sessionIds: readonly string[], -): Promise { - const unique = [...new Set(sessionIds.map((id) => id.trim()).filter(Boolean))]; - if (unique.length === 0) return EMPTY_RESULT; - - const result: SessionMachineryTeardownResult = { - sessionIds: [], - skippedActiveTurns: 0, - }; - - for (const sessionId of unique) { - const row = deps.sessionService.get(sessionId); - // Only chat-backed sessions own the machinery this tears down. A plain - // terminal's process is the user's, and a tracked agent CLI's work lives in - // its PTY — which settle keeps open on purpose. - if (!row || !isChatToolType(row.toolType)) continue; - result.sessionIds.push(sessionId); - - const service = deps.agentChatService; - if (!service) continue; - try { - const stop = await service.stopBackgroundWork({ sessionId }); - if (stop.skippedActiveTurn) result.skippedActiveTurns += 1; - } catch (error) { - deps.logger?.warn("session_teardown.stop_background_work_failed", { - sessionId, - error: error instanceof Error ? error.message : String(error), - }); - } - } - - return result; -} - diff --git a/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts b/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts deleted file mode 100644 index 73d917f53..000000000 --- a/apps/desktop/src/main/services/sessions/sessionTeardown.test.ts +++ /dev/null @@ -1,274 +0,0 @@ -import { describe, expect, it, vi } from "vitest"; -import type { TerminalSessionSummary } from "../../../shared/types"; -import { deleteTerminalSessionWithRuntimeCleanup } from "./deleteTerminalSession"; -import { stopSettledSessionMachinery } from "./sessionMachineryTeardown"; -import { settleTerminalSession } from "./settleTerminalSession"; -import type { createPtyService } from "../pty/ptyService"; -import type { createSessionService } from "./sessionService"; - -/** - * End-of-life teardown for a session, in one place: what settle stops, what - * delete removes, and — the distinction the whole thing turns on — what each - * deliberately leaves alone. - * - * These three modules (`settleTerminalSession`, `sessionMachineryTeardown`, - * `deleteTerminalSession`) are one contract split across files for dependency - * reasons, not behavioral ones, so they are tested together. - */ - -type Row = { id: string; toolType: string }; - -function deps(rows: Row[], overrides: Record = {}) { - const stopBackgroundWork = vi.fn(async () => ({ skippedActiveTurn: false })); - return { - sessionService: { - get: (id: string) => rows.find((row) => row.id === id) ?? null, - } as never, - agentChatService: { - stopBackgroundWork, - ...overrides, - } as never, - logger: { warn: vi.fn() }, - stopBackgroundWork, - }; -} - -describe("stopSettledSessionMachinery", () => { - it("stops background work and pauses scheduled work for a chat session", async () => { - // Settle used to be a pure column write: the row went quiet while its - // monitors kept polling and its background shells kept holding ports. - const d = deps([{ id: "chat-1", toolType: "claude-chat" }]); - const result = await stopSettledSessionMachinery(d, ["chat-1"]); - - expect(d.stopBackgroundWork).toHaveBeenCalledWith({ sessionId: "chat-1" }); - expect(result).toMatchObject({ - sessionIds: ["chat-1"], - skippedActiveTurns: 0, - }); - }); - - - - - - it("leaves terminal sessions alone — a terminal pane is user-owned", async () => { - // The whole carve-out of settle teardown: an agent's background shell is - // thread background work, but the pane the user opened to watch a build is - // theirs and must survive the settle with its scrollback. - const d = deps([ - { id: "term-1", toolType: "shell" }, - { id: "cli-1", toolType: "claude" }, - ]); - const result = await stopSettledSessionMachinery(d, ["term-1", "cli-1"]); - - expect(result.sessionIds).toEqual([]); - expect(d.stopBackgroundWork).not.toHaveBeenCalled(); - }); - - it("reports a session skipped because its foreground turn is still streaming", async () => { - const d = deps([{ id: "chat-1", toolType: "claude-chat" }], { - stopBackgroundWork: vi.fn(async () => ({ skippedActiveTurn: true })), - }); - const result = await stopSettledSessionMachinery(d, ["chat-1"]); - expect(result.skippedActiveTurns).toBe(1); - }); - - it("never lets a provider failure block the settle", async () => { - const d = deps([{ id: "chat-1", toolType: "claude-chat" }], { - stopBackgroundWork: vi.fn(async () => { - throw new Error("provider unreachable"); - }), - }); - await expect(stopSettledSessionMachinery(d, ["chat-1"])).resolves.toMatchObject({ - sessionIds: ["chat-1"], - skippedActiveTurns: 0, - }); - expect(d.logger.warn).toHaveBeenCalled(); - }); - - it("skips unknown ids and de-duplicates repeats", async () => { - const d = deps([{ id: "chat-1", toolType: "claude-chat" }]); - const result = await stopSettledSessionMachinery(d, ["chat-1", "chat-1", " ", "missing"]); - expect(result.sessionIds).toEqual(["chat-1"]); - expect(d.stopBackgroundWork).toHaveBeenCalledTimes(1); - }); - - it("does nothing at all without a chat service, rather than throwing", async () => { - const result = await stopSettledSessionMachinery( - { - sessionService: { get: () => ({ id: "chat-1", toolType: "claude-chat" }) } as never, - agentChatService: null, - }, - ["chat-1"], - ); - expect(result.sessionIds).toEqual(["chat-1"]); - }); -}); - -describe("settleTerminalSession", () => { - it("tears the machinery down before writing the settled column", async () => { - // Ordering matters: a settle must never report success while the monitors - // it claims to have concluded are still armed. - const order: string[] = []; - const d = deps([{ id: "chat-1", toolType: "claude-chat" }], { - stopBackgroundWork: vi.fn(async () => { - order.push("stop"); - return { skippedActiveTurn: false }; - }), - }); - const sessionService = { - get: (id: string) => (id === "chat-1" ? { id, toolType: "claude-chat", lastActivityAt: "t0" } : null), - settleSession: vi.fn(() => { - order.push("settle"); - return true; - }), - }; - - await expect(settleTerminalSession({ - sessionId: "chat-1", - opts: { source: "user" }, - sessionService: sessionService as never, - agentChatService: d.agentChatService, - logger: d.logger, - })).resolves.toBe(true); - - expect(order).toEqual(["stop", "settle"]); - expect(sessionService.settleSession).toHaveBeenCalledWith("chat-1", { source: "user" }); - }); -}); - -function makeSession(overrides: Partial = {}): TerminalSessionSummary { - return { - id: "session-1", - laneId: "lane-1", - laneName: "Primary", - ptyId: null, - tracked: true, - pinned: false, - goal: null, - toolType: "shell", - title: "Shell", - status: "completed", - startedAt: "2026-08-01T00:00:00.000Z", - endedAt: "2026-08-01T00:01:00.000Z", - exitCode: 0, - transcriptPath: "/tmp/transcript", - headShaStart: null, - headShaEnd: null, - lastOutputPreview: null, - summary: null, - runtimeState: "exited", - resumeCommand: null, - ...overrides, - }; -} - -function makeServices(session: TerminalSessionSummary | null) { - const deleteSession = vi.fn().mockReturnValue(true); - const sessionService = { - get: vi.fn().mockReturnValue(session), - deleteSession, - } as unknown as ReturnType; - const ptyService = { - enrichSessions: vi.fn((sessions: TerminalSessionSummary[]) => sessions), - isSessionOwnedByLivePeerRuntime: vi.fn().mockReturnValue(false), - dispose: vi.fn(), - } as unknown as ReturnType; - return { deleteSession, ptyService, sessionService }; -} - -describe("deleteTerminalSessionWithRuntimeCleanup", () => { - it("deletes a session this runtime owns", () => { - const { deleteSession, ptyService, sessionService } = makeServices(makeSession()); - - expect(deleteTerminalSessionWithRuntimeCleanup({ - sessionId: "session-1", - sessionService, - ptyService, - })).toBe(true); - expect(deleteSession).toHaveBeenCalledWith("session-1"); - }); - - it("treats a session this runtime does not have as already deleted", () => { - // Delete is idempotent: the goal state is "not here", and it already holds. - // Throwing surfaced a red "Delete failed" banner over a list that was - // correct — the renderer routinely asks a runtime to delete a row that - // never persisted there, or that another window already removed. - const { deleteSession, ptyService, sessionService } = makeServices(null); - - expect(deleteTerminalSessionWithRuntimeCleanup({ - sessionId: "missing-session", - sessionService, - ptyService, - })).toBe(false); - expect(deleteSession).not.toHaveBeenCalled(); - }); - - it("still rejects an empty session id", () => { - const { ptyService, sessionService } = makeServices(makeSession()); - - expect(() => deleteTerminalSessionWithRuntimeCleanup({ - sessionId: " ", - sessionService, - ptyService, - })).toThrow("Session id is required."); - }); - - it("still refuses a chat session", () => { - const { ptyService, sessionService } = makeServices(makeSession({ toolType: "codex-chat" })); - - expect(() => deleteTerminalSessionWithRuntimeCleanup({ - sessionId: "session-1", - sessionService, - ptyService, - })).toThrow("Use the chat delete flow instead."); - }); -}); - -describe("settleTerminalSession activity guard", () => { - it("refuses to settle over a turn that started while teardown was awaiting", () => { - // Provider stop calls take seconds. A user sending a message inside that - // window clears a settle marker that does not exist yet, and the write - // would then file the freshly-active session as settled. - let activity = "t0"; - const settleSession = vi.fn(() => true); - const sessionService = { - get: (id: string) => (id === "chat-1" ? { id, toolType: "claude-chat", lastActivityAt: activity } : null), - settleSession, - }; - const agentChatService = { - stopBackgroundWork: vi.fn(async () => { - activity = "t1"; // the user starts a turn mid-teardown - return { skippedActiveTurn: false }; - }), - }; - - return settleTerminalSession({ - sessionId: "chat-1", - sessionService: sessionService as never, - agentChatService: agentChatService as never, - }).then((settled) => { - // Reported as handled — the row exists and the request was honoured — but - // deliberately NOT settled. Returning false would surface a spurious - // "session not found" to the caller. - expect(settled).toBe(true); - expect(settleSession).not.toHaveBeenCalled(); - }); - }); - - it("settles normally when nothing happened during teardown", () => { - const settleSession = vi.fn(() => true); - const sessionService = { - get: () => ({ id: "chat-1", toolType: "claude-chat", lastActivityAt: "t0" }), - settleSession, - }; - return settleTerminalSession({ - sessionId: "chat-1", - sessionService: sessionService as never, - agentChatService: { stopBackgroundWork: vi.fn(async () => ({ skippedActiveTurn: false })) } as never, - }).then((settled) => { - expect(settled).toBe(true); - expect(settleSession).toHaveBeenCalledTimes(1); - }); - }); -}); diff --git a/apps/desktop/src/main/services/sessions/settleTerminalSession.ts b/apps/desktop/src/main/services/sessions/settleTerminalSession.ts index 7c0349293..d1b28df05 100644 --- a/apps/desktop/src/main/services/sessions/settleTerminalSession.ts +++ b/apps/desktop/src/main/services/sessions/settleTerminalSession.ts @@ -3,7 +3,6 @@ import type { createPtyService } from "../pty/ptyService"; import type { createSessionService } from "./sessionService"; import type { SessionSettleSource } from "../../../shared/types"; import { isChatToolType } from "./chatSessionProjection"; -import { stopSettledSessionMachinery } from "./sessionMachineryTeardown"; export type SettleTerminalSessionOptions = { outcome?: string; @@ -46,21 +45,12 @@ export async function dismissPendingInputBeforeSettle(args: { return true; } -/** - * Settle a session AND stop the machinery it owns. - * - * The teardown runs before the column write so a settle can never report - * success while its monitors are still armed. It is best-effort — see - * `stopSettledSessionMachinery` — so a provider that cannot be reached delays - * nothing and blocks nothing. - */ export async function settleTerminalSession(args: { sessionId: string; opts?: SettleTerminalSessionOptions; sessionService: ReturnType; agentChatService?: ReturnType | null; ptyService?: ReturnType | null; - logger?: { warn: (message: string, meta?: Record) => void } | null; }): Promise { if (args.opts?.dismissPendingInput === true) { const dismissed = await dismissPendingInputBeforeSettle({ @@ -72,34 +62,6 @@ export async function settleTerminalSession(args: { if (!dismissed) return false; } - // Teardown awaits provider stop calls that can take seconds, and a user can - // start a new turn inside that window. `clearTurnStartMarkers` would clear a - // settle marker that does not exist yet, and this write would then file the - // freshly-active session as settled. Snapshot the activity stamp first and - // refuse to settle over work that arrived while we were stopping things — - // real activity outranks a settle request that predates it. - const activityBeforeTeardown = args.sessionService.get(args.sessionId)?.lastActivityAt ?? null; - - await stopSettledSessionMachinery( - { - sessionService: args.sessionService, - agentChatService: args.agentChatService ?? null, - logger: args.logger ?? null, - }, - [args.sessionId], - ); - - const after = args.sessionService.get(args.sessionId); - if (!after) return false; - if ((after.lastActivityAt ?? null) !== activityBeforeTeardown) { - args.logger?.warn("session_teardown.settle_skipped_new_activity", { - sessionId: args.sessionId, - }); - // The row exists and the request was honoured; it simply woke, so it is not - // settled. Reporting false here would surface a spurious "not found". - return true; - } - return args.sessionService.settleSession( args.sessionId, { diff --git a/docs/features/terminals-and-sessions/README.md b/docs/features/terminals-and-sessions/README.md index b74103c4a..bffc77896 100644 --- a/docs/features/terminals-and-sessions/README.md +++ b/docs/features/terminals-and-sessions/README.md @@ -205,32 +205,17 @@ and in tests. presenting a false live/green agent. - `apps/desktop/src/main/services/sessions/settleTerminalSession.ts` — single settlement transaction shared by direct IPC and the ADE action - registry. Settle stops the machinery the session owns before it writes the - lifecycle column — see - `apps/desktop/src/main/services/sessions/sessionMachineryTeardown.ts`. It - calls `agentChatService.stopBackgroundWork`, which stops every live child - before the parent. Every settle entry point runs it, including the CTO - operator's `settleSession` tool, because the teardown has to finish *before* - the lifecycle write. **Scheduled work is deliberately left running**: pausing - it would be durable, and `settled_at` is cleared from seven places (including - the hot `setLastOutputPreview` path), so a pause without a complete undo would - silently disable a user's own monitors and crons forever. ADE's scheduled work - is already visible and user-manageable (`scheduledWork` / `nextWakeAt` on the - summary, a per-session pause toggle), and `canonicalSessionState` already - handles a settled chat woken by a schedule — green while the turn streams, - then re-settled. **Terminal panes stay open**: an agent's background shell is - thread background work, but a pane the user opened is theirs, and closing it - on settle would destroy scrollback nobody asked to lose. An ACTIVE foreground - turn is also left alone — its subagents are work the user can see happening, - and the row un-settles on its own activity anyway. What escapes is stated - rather than pretended away: processes an agent detached with - `nohup`/`setsid`/`disown` leave ADE's tree entirely, and Codex background - subagents are reported but expose no stop control. Every settle entry point - runs it — the single/bulk ADE actions, the `sessions.settle`/`settleMany` - IPC handlers, the `session.settle*` sync commands, and the PR-merge - auto-settle (which files a session even when it still owns scheduled work or - a live background task, and is therefore the path most likely to file one - that is still running something). + registry. Settle writes lifecycle state only — it deliberately does NOT stop + the session's background work. That was attempted and removed: teardown is + async, and `settled_at` is written and cleared from seven places, so a + teardown-then-write settle races real activity (a user starting a turn during + a provider stop call gets their background work stopped AND no settle), and + every guard tried against it either read a column that turn-start never + updates or had to be repeated at each of the settle entry points. Making + settle stop work needs a synchronous lifecycle revision that teardown can be + serialized against; it is not a wrapper around the existing write. Archive is + the one lifecycle path that does stop processes — see + `laneService.archive`, where the ordering is load-bearing. `dismissPendingInput: true` first quiets an SDK chat through `agentChatService`, or clears a tracked CLI's explicit `ade chat ask` marker through `ptyService`; arbitrary native From 264eb46a0a71b4cb645d9eafa7bd534be3ea4425 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 02:19:15 -0400 Subject: [PATCH 14/14] fix(sessions): a settled row still reports live background work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile and Codex both landed on the same branch, and cutting settle teardown made them right: the settled branch suppressed background-work liveness, and its comment justified that with 'settle now tears the session's machinery down' — which stopped being true when the teardown was removed. A settled session can now legitimately still own a live background shell, subagent, or cloud run. The PHASE stays settled: a declared settle is a human judgment call, and re-lighting the row would let a stubborn monitor out-vote the user's explicit 'this is done'. But liveness now reports the truth, so a surface that wants to show 'settled, but something is still running' can. Hiding it behind the phase is the same lie this module exists to prevent, just at the other end of the lifecycle. Co-Authored-By: Claude Opus 5 --- .../src/shared/sessionCanonicalState.test.ts | 14 +++++++-- .../src/shared/sessionCanonicalState.ts | 29 +++++++++++++++---- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/apps/desktop/src/shared/sessionCanonicalState.test.ts b/apps/desktop/src/shared/sessionCanonicalState.test.ts index af2fc8131..dcf1bf2ed 100644 --- a/apps/desktop/src/shared/sessionCanonicalState.test.ts +++ b/apps/desktop/src/shared/sessionCanonicalState.test.ts @@ -345,9 +345,17 @@ describe("background work liveness", () => { it("never lets background work mask a raised hand or a declared settle", () => { expect(state({ pendingInputItemId: "i-1", runtimeState: "idle", backgroundWork: working }).phase).toBe("needs_you"); - expect( - state({ runtimeState: "idle", settledAt: new Date(NOW).toISOString(), backgroundWork: working }).phase, - ).toBe("settled"); + const settledWithWork = state({ + runtimeState: "idle", + settledAt: new Date(NOW).toISOString(), + backgroundWork: working, + }); + expect(settledWithWork.phase).toBe("settled"); + // The phase holds — a declared settle is the user's call — but liveness + // still reports the live work, because settle does not stop it today and a + // settled row that silently owns a running shell is the same lie at the + // other end of the lifecycle. + expect(settledWithWork.liveness).toBe("background"); }); it("leaves a silent session stale rather than claiming it is working", () => { diff --git a/apps/desktop/src/shared/sessionCanonicalState.ts b/apps/desktop/src/shared/sessionCanonicalState.ts index 943f56f32..9b0f6074a 100644 --- a/apps/desktop/src/shared/sessionCanonicalState.ts +++ b/apps/desktop/src/shared/sessionCanonicalState.ts @@ -301,12 +301,29 @@ export function canonicalSessionState(args: CanonicalSessionInputs): CanonicalSe const pinnedActive = args.settleOverride === "active"; const atRest = args.status !== "running" || args.runtimeState === "idle"; if (!pinnedActive && atRest && (args.settleOverride === "settled" || args.settledAt)) { - // Deliberately NOT promoted by background work. Settle now tears the - // session's machinery down (`stopSessionBackgroundWork`), so a settled row - // with live work is a teardown that has not finished draining, not a state - // worth advertising — and re-lighting it would let a stubborn monitor - // out-vote the user's explicit "this is done". - return { phase: "settled", badge: null, liveness: null }; + // The PHASE stays settled: a declared settle is a human judgment call, and + // re-lighting the row would let a stubborn monitor out-vote the user's + // explicit "this is done". + // + // But `liveness` still reports the truth, because settle does NOT stop + // background work today — archive is the only lifecycle path that stops + // processes. A settled session can therefore legitimately still own a live + // background shell, subagent, or Cursor cloud run, and a surface that wants + // to show "settled, but something is still running" must be able to. The + // phase alone would hide it, which is the exact failure this module exists + // to prevent — just at the other end of the lifecycle. + // + // Making settle stop that work is a separate change; it needs a synchronous + // lifecycle revision teardown can serialize against, not a wrapper around + // this write. See the settle-teardown design doc. + const settledWork = args.backgroundWork; + return { + phase: "settled", + badge: null, + liveness: totalBackgroundWork(settledWork) <= 0 + ? null + : (settledWork?.workingCount ?? 0) > 0 ? "background" : "monitoring", + }; } const ended = args.status !== "running";