From 80f7511aa5ffd288a2c96e111073e5664c41cfda Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 14 Jul 2026 23:13:47 -0700 Subject: [PATCH] fix(agent-status): reap finished Claude named agents/teammates from the sidebar roster MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #8522 stopped one-shot subagents from squatting as idle rows, but named background agents (Workflow/orchestration/ultracode lanes and agent-teams teammates) still piled up permanent "Idle - " child rows for the rest of the session — the reported regression (11 idle rows under an "Orchestration Messages" pane, all idle 3-6h after finishing). Root cause, confirmed against live hook captures (claude 2.1.210): named agents get teammate-shaped ids (a-) AND now appear in Stop's `background_tasks` as `type: "teammate"` entries whose status stays "running" forever — even after the agent finished. The old code read that shape as a "resumable teammate", so SubagentStop only marked it idle and the fold never reaped it (a present teammate task kept hasTeammateTypedTask true). The rows never left. Fix: the roster now tracks only WORKING children. - SubagentStop removes the child outright (teammate-shaped or not) — it is the reliable finish signal; the teammate task's "running" status is not. - TeammateIdle removes by name as the fallback when a SubagentStop is lost. - A lead Stop's background_tasks still reaps unlisted children: hyphen-free one-shots always, and teammate-shaped rows once a complete inventory shows no teammate-typed task at all. A live named agent whose id never appears is kept only while a teammate-typed task is still present (the done-gate). - Hydration drops persisted idle snapshots so a restart can't re-pile them. Verified live in a dev Electron instance driving a real Claude TUI that spawns four named background agents: pre-fix the pane resolves to done with four persistent "Idle - " rows; post-fix each row disappears the instant its agent finishes and the roster drains to empty (done, zero child rows). Tests: roster + row-lifecycle + hook-listener suites rewritten to the working-only semantics, grounded in the captured 2.1.210 hook stream (126 passing). Typecheck + oxlint clean. --- src/shared/agent-hook-listener.test.ts | 96 +++-- src/shared/agent-hook-listener.ts | 34 +- src/shared/claude-subagent-roster.test.ts | 350 +++++++----------- src/shared/claude-subagent-roster.ts | 226 ++++------- .../claude-subagent-row-lifecycle.test.ts | 145 ++++---- 5 files changed, 356 insertions(+), 495 deletions(-) diff --git a/src/shared/agent-hook-listener.test.ts b/src/shared/agent-hook-listener.test.ts index efafcaa5b06..e9f9e265f62 100644 --- a/src/shared/agent-hook-listener.test.ts +++ b/src/shared/agent-hook-listener.test.ts @@ -2350,92 +2350,83 @@ describe('shared agent-hook-listener', () => { expect(stopped?.payload.state).toBe('done') }) - it('resolves teams-mode teammates to done despite background_tasks reporting running', () => { - // Why: this is the interactive agent-teams shape observed live — + it('removes a finished teammate/named agent on SubagentStop despite its task reading running', () => { + // Why: the interactive agent-teams / orchestration shape observed live — // lifecycle events use `a-` agent ids while background_tasks - // uses unrelated task ids and keeps idle-but-alive teammates as - // status "running". The unmatched task entry must neither create a - // duplicate child row nor keep the pane spinning forever. + // uses unrelated `type: "teammate"` task ids that report "running" + // forever, even after the named agent finished. The finished row must + // leave the sidebar at once (the reported "long idle list" symptom). claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'spawn probe' }) claudeEvent({ hook_event_name: 'SubagentStart', agent_id: 'aprobe1-6d3cb5b52120b7bf', agent_type: 'probe1' }) + const teammateTask = { + id: 'tlkjjs0jv', + type: 'teammate', + status: 'running', + description: 'Run the shell command: sleep 25.' + } const spawnStop = claudeEvent({ hook_event_name: 'Stop', - background_tasks: [ - { - id: 'tlkjjs0jv', - type: 'teammate', - status: 'running', - description: 'Run the shell command: sleep 25.' - } - ] + background_tasks: [teammateTask] }) expect(spawnStop?.payload.state).toBe('working') expect(spawnStop?.payload.subagents).toEqual([ expect.objectContaining({ id: 'aprobe1-6d3cb5b52120b7bf', state: 'working' }) ]) - claudeEvent({ + // SubagentStop is the reliable finish signal — the row goes even though + // its teammate task is still listed "running". + const stopped = claudeEvent({ hook_event_name: 'SubagentStop', agent_id: 'aprobe1-6d3cb5b52120b7bf', - agent_type: 'probe1' + agent_type: 'probe1', + background_tasks: [teammateTask] }) - const idled = claudeEvent({ + expect(stopped?.payload.subagents).toBeUndefined() + + claudeEvent({ hook_event_name: 'TeammateIdle', teammate_name: 'probe1', team_name: 'session-56c87269' }) - expect(idled?.payload.subagents).toEqual([ - expect.objectContaining({ id: 'aprobe1-6d3cb5b52120b7bf', state: 'idle' }) - ]) const wakeStop = claudeEvent({ hook_event_name: 'Stop', - background_tasks: [ - { - id: 'tlkjjs0jv', - type: 'teammate', - status: 'running', - description: 'Run the shell command: sleep 25.' - } - ] + background_tasks: [teammateTask] }) expect(wakeStop?.payload.state).toBe('done') - expect(wakeStop?.payload.subagents).toEqual([ - expect.objectContaining({ id: 'aprobe1-6d3cb5b52120b7bf', state: 'idle' }) - ]) + expect(wakeStop?.payload.subagents).toBeUndefined() }) - it('keeps a teammate whose name differs from its configured agent type', () => { + it('removes a working teammate via TeammateIdle when its id prefix matches the name', () => { claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'spawn reviewer' }) claudeEvent({ hook_event_name: 'SubagentStart', agent_id: 'areviewer-6d3cb5b52120b7bf', agent_type: 'security-reviewer' }) + // Lead turn ends while the teammate works; pane stays working. + const stop = claudeEvent({ + hook_event_name: 'Stop', + background_tasks: [{ id: 'trev', type: 'teammate', status: 'running' }] + }) + expect(stop?.payload.state).toBe('working') // Why: teammate name and agent type are separate Agent-tool inputs; the // lifecycle id embeds the former while the hook reports the latter. - claudeEvent({ - hook_event_name: 'SubagentStop', - agent_id: 'areviewer-6d3cb5b52120b7bf', - agent_type: 'security-reviewer' - }) + // TeammateIdle keyed by name reaps it via the id prefix (fallback when + // its SubagentStop was lost), so the finished row leaves and the pane + // can settle back to the lead's done state. const idled = claudeEvent({ hook_event_name: 'TeammateIdle', teammate_name: 'reviewer', team_name: 'session-x' }) - expect(idled?.payload.subagents).toEqual([ - expect.objectContaining({ - id: 'areviewer-6d3cb5b52120b7bf', - agentType: 'security-reviewer', - state: 'idle' - }) - ]) + expect(idled?.payload.subagents).toBeUndefined() + expect(idled?.payload.state).toBe('done') }) it('scopes subagent rosters per pane', () => { @@ -2679,9 +2670,8 @@ describe('shared agent-hook-listener', () => { teammate_name: 'lane-hooks', team_name: 'session-x' }) - expect(idled?.payload.subagents).toEqual([ - expect.objectContaining({ id: 'alane-hooks-6d3cb5b5', state: 'idle' }) - ]) + // Why: idle means finished — the exact-name match reaps the row. + expect(idled?.payload.subagents).toBeUndefined() }) it('keeps an inferred interrupt terminal across later child lifecycle events', () => { @@ -2703,7 +2693,11 @@ describe('shared agent-hook-listener', () => { expect(idled?.payload.interrupted).toBe(true) }) - it('seeds the roster from persisted snapshots so a teammate-bearing Stop keeps child rows', () => { + it('does not resurrect persisted idle child rows after a restart', () => { + // Why: the roster tracks only working children now. A persisted idle + // snapshot (from a build that kept idle rows) is a finished child, so + // hydration must drop it — otherwise restart would re-pile the exact + // squatting rows this fix removes. seedClaudeSubagentRosterFromSnapshots(state, PANE_KEY, [ { id: 'aprobe2-6d3cb5b52120b7bf', @@ -2720,13 +2714,7 @@ describe('shared agent-hook-listener', () => { ] }) expect(stop?.payload.state).toBe('done') - expect(stop?.payload.subagents).toEqual([ - expect.objectContaining({ - id: 'aprobe2-6d3cb5b52120b7bf', - agentType: 'security-reviewer', - state: 'idle' - }) - ]) + expect(stop?.payload.subagents).toBeUndefined() }) it('rebuilds a running one-shot subagent from background_tasks after restart', () => { diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index 5870fd5f43b..82bb5485a12 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -40,9 +40,8 @@ import { claudeTeammateIdMatchesName, finishClaudeSubagent, foldClaudeBackgroundTasksIntoRoster, - isClaudeTeammateLifecycleId, - markClaudeTeammateIdleByName, readClaudeBackgroundAgentTasks, + removeClaudeTeammateByName, upsertWorkingClaudeSubagent, type ClaudeSubagentRoster } from './claude-subagent-roster' @@ -2436,7 +2435,10 @@ function normalizeClaudeSubagentLifecycleEvent( if (!teammateName) { return null } - markClaudeTeammateIdleByName(roster, teammateName) + // Why: idle means not working, and only working children keep a row. This + // is the fallback finish signal for a named agent whose SubagentStop was + // lost — its background_tasks entry never stops reading "running". + removeClaudeTeammateByName(roster, teammateName) clearClaudePendingWaitForAgent(state, paneKey, (waitingAgentId) => claudeTeammateIdMatchesName(waitingAgentId, teammateName) ) @@ -2453,14 +2455,11 @@ function normalizeClaudeSubagentLifecycleEvent( Date.now() ) } else { - // Why: SubagentStop carries the session's task inventory; a stopping - // agent listed id-exact as a subagent task is a workflow/named one-shot - // (teammate lifecycle ids never appear there), not a resumable teammate. - const stopTasks = readClaudeBackgroundAgentTasks(hookPayload) - finishClaudeSubagent(roster, agentId, { - listedAsSubagentTask: - stopTasks.present && stopTasks.tasks.some((task) => !task.teammate && task.id === agentId) - }) + // Why: a finished child (one-shot, workflow lane, or named teammate) + // leaves the sidebar at once. SubagentStop is the reliable finish + // signal even for teammate-shaped ids — their background_tasks entries + // stay "running" forever — and a resumed teammate re-earns its row. + finishClaudeSubagent(roster, agentId) // Why: a blocked child that dies (killed, errored) without another tool // event would otherwise pin its permission/question wait on the pane // forever — nothing else references that agent again. @@ -2493,18 +2492,19 @@ export function seedClaudeSubagentRosterFromSnapshots( } const roster = getOrCreateClaudeSubagentRoster(state, paneKey) for (const snapshot of snapshots) { + // Why: the roster only tracks working children now. A persisted idle + // snapshot (from a build that kept idle rows) is a finished child — drop + // it so restart doesn't resurrect the stale pile this fix removes. + if (snapshot.state !== 'working') { + continue + } roster.set(snapshot.id, { - state: snapshot.state === 'working' ? 'working' : 'idle', startedAt: snapshot.startedAt, agentType: snapshot.agentType, description: snapshot.description, - // Why: teammate name and agent type can differ, but the provider id - // shape survives persistence and keeps the row across restart folds. - ...(isClaudeTeammateLifecycleId(snapshot.id) ? { teammate: true as const } : {}), // Why: the seed can be a phantom (child finished while Orca was down, // its SubagentStop lost). Let a PRESENT background_tasks list that - // omits the id remove it (or demote a teammate) instead of gating the - // pane 'working' forever. + // omits the id remove it instead of gating the pane 'working' forever. backgroundTasksAuthoritative: true }) } diff --git a/src/shared/claude-subagent-roster.test.ts b/src/shared/claude-subagent-roster.test.ts index 99cd9bbb8dc..71dc28fe979 100644 --- a/src/shared/claude-subagent-roster.test.ts +++ b/src/shared/claude-subagent-roster.test.ts @@ -6,117 +6,53 @@ import { claudeTeammateIdMatchesName, finishClaudeSubagent, foldClaudeBackgroundTasksIntoRoster, - markClaudeTeammateIdleByName, readClaudeBackgroundAgentTasks, + removeClaudeTeammateByName, upsertWorkingClaudeSubagent, type ClaudeSubagentRoster } from './claude-subagent-roster' +const task = ( + over: Partial['tasks'][number]> +) => ({ + id: 'a1', + agentType: undefined, + description: undefined, + running: true, + teammate: false, + ...over +}) + describe('claude-subagent-roster', () => { it('removes a finished one-shot subagent on stop', () => { const roster: ClaudeSubagentRoster = new Map() upsertWorkingClaudeSubagent(roster, 'a1', { agentType: 'general-purpose' }, 100) expect(claudeRosterHasWorkingSubagent(roster)).toBe(true) - // Why: retaining finished one-shots as idle rows piled up dozens of dead + // Why: retaining finished children as idle rows piled up dozens of dead // "Idle - general-purpose" sidebar rows over a long workflow session. finishClaudeSubagent(roster, 'a1') expect(roster.size).toBe(0) expect(claudeRosterToSnapshots(roster)).toBeUndefined() }) - it('idles a teammate on stop instead of removing it', () => { + it('removes a finished teammate-shaped named agent on stop', () => { const roster: ClaudeSubagentRoster = new Map() + // Why: named/workflow agents report teammate-shaped ids, and their + // background_tasks teammate entries never stop reading "running" — so + // SubagentStop is the only reliable finish signal and must remove the row. upsertWorkingClaudeSubagent(roster, 'aprobe1-6d3cb5b5', { agentType: 'probe1' }, 100) finishClaudeSubagent(roster, 'aprobe1-6d3cb5b5') - expect(roster.get('aprobe1-6d3cb5b5')).toMatchObject({ state: 'idle', teammate: true }) + expect(roster.has('aprobe1-6d3cb5b5')).toBe(false) }) - it('removes a finished workflow lane despite its teammate-shaped id when task-listed', () => { - const roster: ClaudeSubagentRoster = new Map() - // Why: workflow lanes get name-embedding ids (afinder-C-) like - // teammates, but their SubagentStop payload lists them id-exact as a - // subagent task — real teammate lifecycle ids never appear there. - upsertWorkingClaudeSubagent( - roster, - 'afinder-C-5d713c0781b7f8d2', - { agentType: 'finder-C' }, - 100 - ) - finishClaudeSubagent(roster, 'afinder-C-5d713c0781b7f8d2', { listedAsSubagentTask: true }) - expect(roster.size).toBe(0) - }) - - it('reclassifies a task-listed teammate-shaped entry as a one-shot', () => { - const roster: ClaudeSubagentRoster = new Map() - upsertWorkingClaudeSubagent( - roster, - 'av1-streaming-0b1c2d3e', - { agentType: 'v1-streaming' }, - 100 - ) - foldClaudeBackgroundTasksIntoRoster( - roster, - [ - { - id: 'av1-streaming-0b1c2d3e', - agentType: 'v1-streaming', - description: undefined, - running: true, - teammate: false - }, - { - id: 'tteam1', - agentType: undefined, - description: undefined, - running: true, - teammate: true - } - ], - 200 - ) - // A later stop without its own inventory still removes it: the fold - // already proved the id is a task id, not a teammate. - finishClaudeSubagent(roster, 'av1-streaming-0b1c2d3e') - expect(roster.has('av1-streaming-0b1c2d3e')).toBe(false) - }) - - it('removes teammate-shaped leftovers when a complete inventory lists no teammates', () => { - const roster: ClaudeSubagentRoster = new Map() - upsertWorkingClaudeSubagent(roster, 'acr-triage-1-c5a0588e', { agentType: 'cr-triage-1' }, 100) - finishClaudeSubagent(roster, 'acr-triage-1-c5a0588e') - expect(roster.get('acr-triage-1-c5a0588e')).toMatchObject({ state: 'idle' }) - upsertWorkingClaudeSubagent(roster, 'afix-main-11223344', { agentType: 'fix-main' }, 150) - - // Why: a teams session lists its teammates (even idle) as teammate-typed - // tasks; an inventory with none proves these name-shaped rows are dead - // workflow lanes, not resumable teammates. - foldClaudeBackgroundTasksIntoRoster( - roster, - [ - { - id: 'aunrelated0000001', - agentType: 'general-purpose', - description: undefined, - running: true, - teammate: false - } - ], - 200 - ) - expect(roster.has('acr-triage-1-c5a0588e')).toBe(false) - expect(roster.has('afix-main-11223344')).toBe(false) - expect(roster.has('aunrelated0000001')).toBe(true) - }) - - it('re-marks an idle teammate working without resetting startedAt', () => { + it('re-adds a resumed agent as working, keeping its original startedAt', () => { const roster: ClaudeSubagentRoster = new Map() upsertWorkingClaudeSubagent(roster, 'aprobe1-6d3cb5b5', { agentType: 'probe1' }, 100) finishClaudeSubagent(roster, 'aprobe1-6d3cb5b5') upsertWorkingClaudeSubagent(roster, 'aprobe1-6d3cb5b5', { description: 'round two' }, 200) expect(roster.get('aprobe1-6d3cb5b5')).toMatchObject({ - state: 'working', - startedAt: 100, + startedAt: 200, description: 'round two' }) }) @@ -127,22 +63,21 @@ describe('claude-subagent-roster', () => { expect(roster.size).toBe(0) }) - it('caps roster size, evicting the oldest idle entry first', () => { + it('drops new spawns at the cap rather than evicting live children', () => { const roster: ClaudeSubagentRoster = new Map() - upsertWorkingClaudeSubagent(roster, 'aprobe1-6d3cb5b5', { agentType: 'probe1' }, 0) - for (let i = 1; i < AGENT_STATUS_MAX_SUBAGENTS; i++) { + for (let i = 0; i < AGENT_STATUS_MAX_SUBAGENTS; i++) { upsertWorkingClaudeSubagent(roster, `a${i}`, {}, i) } - // Why: all working — a new spawn cannot evict live children and is dropped. + // Why: every tracked entry is working — nothing is safe to evict, so the + // overflow spawn is dropped (it would be invisible past the wire cap). upsertWorkingClaudeSubagent(roster, 'overflow', {}, 999) expect(roster.has('overflow')).toBe(false) + expect(roster.size).toBe(AGENT_STATUS_MAX_SUBAGENTS) - // Why: only teammates hold idle entries now; an idle teammate is the - // eviction pool when a burst of new spawns hits the cap. - finishClaudeSubagent(roster, 'aprobe1-6d3cb5b5') + // Once a child finishes, a new spawn takes the freed slot. + finishClaudeSubagent(roster, 'a0') upsertWorkingClaudeSubagent(roster, 'replacement', {}, 1000) expect(roster.has('replacement')).toBe(true) - expect(roster.has('aprobe1-6d3cb5b5')).toBe(false) expect(roster.size).toBe(AGENT_STATUS_MAX_SUBAGENTS) }) @@ -197,70 +132,102 @@ describe('claude-subagent-roster', () => { expect(result.truncated).toBe(true) }) - it('folds background_tasks in without trusting ambiguous entries', () => { + it('trusts id-exact subagent matches and ignores teammate-typed entries', () => { const roster: ClaudeSubagentRoster = new Map() upsertWorkingClaudeSubagent(roster, 'a1', {}, 100) + // A named agent mid-run: teammate-shaped id, never a task id. upsertWorkingClaudeSubagent(roster, 'ateam-6d3cb5b5', { agentType: 'security-reviewer' }, 150) foldClaudeBackgroundTasksIntoRoster( roster, [ - { - id: 'a1', - agentType: 'general-purpose', - description: 'review loop', - running: true, - teammate: false - }, + task({ id: 'a1', agentType: 'general-purpose', description: 'review loop' }), // Why: teammate task ids never match lifecycle agent_ids; unmatched // teammate entries must not create phantom duplicate children. - { - id: 'tlkjjs0jv', - agentType: undefined, - description: 'teammate task', - running: true, - teammate: true - } + task({ id: 'tlkjjs0jv', description: 'teammate task', teammate: true }) ], 200 ) expect(roster.size).toBe(2) - // Why: id-exact matches are one-shot subagents whose run state IS reliable. - expect(roster.get('a1')).toMatchObject({ state: 'working', description: 'review loop' }) - // Why: the working lifecycle-tracked teammate is not listed by id, but - // omission proves nothing for teammates — it must survive the fold. - expect(roster.get('ateam-6d3cb5b5')).toMatchObject({ - state: 'working', - agentType: 'security-reviewer' - }) + expect(roster.get('a1')).toMatchObject({ description: 'review loop' }) + // Why: the working named agent is not listed by id, but a teammate-typed + // task is present — so omission proves nothing and it survives the fold. + expect(roster.get('ateam-6d3cb5b5')).toMatchObject({ agentType: 'security-reviewer' }) }) - it('removes an id-matched task reported not running', () => { + it('removes an id-matched subagent task reported not running', () => { const roster: ClaudeSubagentRoster = new Map() upsertWorkingClaudeSubagent(roster, 'a1', { agentType: 'general-purpose' }, 100) - foldClaudeBackgroundTasksIntoRoster( - roster, - [{ id: 'a1', agentType: undefined, description: undefined, running: false, teammate: false }], - 200 - ) + foldClaudeBackgroundTasksIntoRoster(roster, [task({ id: 'a1', running: false })], 200) expect(roster.size).toBe(0) }) it('removes a killed one-shot missing from a present list', () => { const roster: ClaudeSubagentRoster = new Map() // Why: a running one-shot is always listed id-exact at a lead Stop, so a - // working non-teammate missing from the list is dead (SubagentStop lost); - // keeping it pinned the pane 'working' forever. + // working hyphen-free child missing from the list is dead (SubagentStop + // lost); keeping it pinned the pane 'working' forever. upsertWorkingClaudeSubagent(roster, 'akilled0000000001', { agentType: 'general-purpose' }, 100) + foldClaudeBackgroundTasksIntoRoster(roster, [task({ id: 'other', teammate: true })], 200) + expect(roster.size).toBe(0) + }) + + it('keeps a live named agent whose teammate-typed id never appears in the list', () => { + const roster: ClaudeSubagentRoster = new Map() + // Why: this is the core regression — a running named agent (teammate-shaped + // id, lifecycle-tracked, never a task id) must survive a lead Stop whose + // list still shows teammate-typed tasks, or the pane resolves done early. + upsertWorkingClaudeSubagent( + roster, + 'aweb-research-8a76b7d7', + { agentType: 'web-research' }, + 100 + ) + foldClaudeBackgroundTasksIntoRoster( + roster, + [task({ id: 't6s2brfv7', description: 'named agent task', teammate: true })], + 200 + ) + expect(roster.has('aweb-research-8a76b7d7')).toBe(true) + }) + + it('removes teammate-shaped leftovers when a complete inventory lists no teammates', () => { + const roster: ClaudeSubagentRoster = new Map() + upsertWorkingClaudeSubagent(roster, 'acr-triage-1-c5a0588e', { agentType: 'cr-triage-1' }, 100) + // Why: a session with named agents/teammates always lists at least one + // teammate-typed task while any is alive; an inventory with none proves + // this teammate-shaped row is dead (its SubagentStop was lost). + foldClaudeBackgroundTasksIntoRoster( + roster, + [task({ id: 'aunrelated0000001', agentType: 'general-purpose' })], + 200 + ) + expect(roster.has('acr-triage-1-c5a0588e')).toBe(false) + expect(roster.has('aunrelated0000001')).toBe(true) + }) + + it('removes a leftover once a subagent-typed task listed its id id-exact', () => { + const roster: ClaudeSubagentRoster = new Map() + upsertWorkingClaudeSubagent( + roster, + 'av1-streaming-0b1c2d3e', + { agentType: 'v1-streaming' }, + 100 + ) + // A subagent-typed task lists it id-exact → tag it listedAsSubagentTask. foldClaudeBackgroundTasksIntoRoster( roster, [ - { id: 'other', agentType: undefined, description: undefined, running: true, teammate: true } + task({ id: 'av1-streaming-0b1c2d3e', agentType: 'v1-streaming' }), + task({ id: 'tteam1', teammate: true }) ], 200 ) - expect(roster.size).toBe(0) + // A later Stop omits it while a teammate-typed task is still present: the + // subagent-listing proof overrides the teammate-shape protection. + foldClaudeBackgroundTasksIntoRoster(roster, [task({ id: 'tteam1', teammate: true })], 300) + expect(roster.has('av1-streaming-0b1c2d3e')).toBe(false) }) it('retains an unlisted live child when the background task inventory was truncated', () => { @@ -285,25 +252,13 @@ describe('claude-subagent-roster', () => { foldClaudeBackgroundTasksIntoRoster( roster, [ - { - id: 'a9', - agentType: 'general-purpose', - description: 'long build', - running: true, - teammate: false - }, - { - id: 'gone', - agentType: undefined, - description: undefined, - running: false, - teammate: false - } + task({ id: 'a9', agentType: 'general-purpose', description: 'long build' }), + task({ id: 'gone', running: false }) ], 500 ) - expect(roster.get('a9')).toMatchObject({ state: 'working', startedAt: 500 }) - // Why: a finished unmatched one-shot leaves no reason to add an idle row. + expect(roster.get('a9')).toMatchObject({ startedAt: 500 }) + // Why: a finished unmatched one-shot leaves no reason to add a row. expect(roster.has('gone')).toBe(false) }) @@ -314,86 +269,46 @@ describe('claude-subagent-roster', () => { expect(roster.size).toBe(0) }) - it('removes non-teammate authoritative entries and keeps live teammates on omission', () => { + it('does not clear the roster on an empty but incomplete (truncated) inventory', () => { const roster: ClaudeSubagentRoster = new Map() - roster.set('a-phantom', { - state: 'working', - startedAt: 100, - backgroundTasksAuthoritative: true - }) - upsertWorkingClaudeSubagent(roster, 'ateam-6d3cb5b5', { agentType: 'security-reviewer' }, 150) - - foldClaudeBackgroundTasksIntoRoster( - roster, - [ - { id: 'other', agentType: undefined, description: undefined, running: true, teammate: true } - ], - 200 - ) - expect(roster.has('a-phantom')).toBe(false) - expect(roster.get('ateam-6d3cb5b5')).toMatchObject({ state: 'working' }) + upsertWorkingClaudeSubagent(roster, 'a1', {}, 100) + foldClaudeBackgroundTasksIntoRoster(roster, [], 200, { inventoryComplete: false }) + expect(roster.has('a1')).toBe(true) }) - it('demotes a seeded teammate phantom missing from a present list to idle', () => { + it('removes a seeded phantom missing from a present list', () => { const roster: ClaudeSubagentRoster = new Map() - // Why: a teammate seeded from a pre-restart snapshot may be dead, but its - // task id never appears in background_tasks — demote instead of delete so - // a live idle teammate keeps its row while a phantom stops gating the pane. + // A snapshot-seeded entry (child finished while Orca was down) is + // authoritative — a present list omitting it removes it even though its + // id is teammate-shaped. roster.set('aprobe1-6d3cb5b5', { - state: 'working', startedAt: 100, agentType: 'probe1', - teammate: true, backgroundTasksAuthoritative: true }) - foldClaudeBackgroundTasksIntoRoster( - roster, - [ - { id: 'other', agentType: undefined, description: undefined, running: true, teammate: true } - ], - 200 - ) - expect(roster.get('aprobe1-6d3cb5b5')).toMatchObject({ state: 'idle', teammate: true }) + foldClaudeBackgroundTasksIntoRoster(roster, [task({ id: 'other', teammate: true })], 200) + expect(roster.has('aprobe1-6d3cb5b5')).toBe(false) }) - it('removes fold-recreated entries missing from a later present list', () => { - const roster: ClaudeSubagentRoster = new Map() - foldClaudeBackgroundTasksIntoRoster( - roster, - [{ id: 'a9', agentType: undefined, description: undefined, running: true, teammate: false }], - 100 - ) - foldClaudeBackgroundTasksIntoRoster( - roster, - [ - { id: 'other', agentType: undefined, description: undefined, running: true, teammate: true } - ], - 200 - ) - expect(roster.has('a9')).toBe(false) - }) - - it('keeps a re-tracked working teammate missing from a present list', () => { + it('keeps a re-tracked working named agent missing from a present list', () => { const roster: ClaudeSubagentRoster = new Map() roster.set('aprobe1-6d3cb5b5', { - state: 'working', startedAt: 100, agentType: 'probe1', - teammate: true, backgroundTasksAuthoritative: true }) - // Why: live activity clears the authoritative flag; a busy teammate must - // not be demoted by a Stop that (as always) omits its lifecycle id. + // Why: live activity clears the authoritative flag; a busy named agent + // must not be reaped by a Stop that (as always) omits its lifecycle id. upsertWorkingClaudeSubagent(roster, 'aprobe1-6d3cb5b5', { agentType: 'probe1' }, 150) + foldClaudeBackgroundTasksIntoRoster(roster, [task({ id: 'other', teammate: true })], 200) + expect(roster.has('aprobe1-6d3cb5b5')).toBe(true) + }) - foldClaudeBackgroundTasksIntoRoster( - roster, - [ - { id: 'other', agentType: undefined, description: undefined, running: true, teammate: true } - ], - 200 - ) - expect(roster.get('aprobe1-6d3cb5b5')).toMatchObject({ state: 'working' }) + it('removes fold-recreated one-shots missing from a later present list', () => { + const roster: ClaudeSubagentRoster = new Map() + foldClaudeBackgroundTasksIntoRoster(roster, [task({ id: 'a9' })], 100) + foldClaudeBackgroundTasksIntoRoster(roster, [task({ id: 'other', teammate: true })], 200) + expect(roster.has('a9')).toBe(false) }) it('matches teammate ids by name only up to the hyphen-free suffix', () => { @@ -404,17 +319,29 @@ describe('claude-subagent-roster', () => { expect(claudeTeammateIdMatchesName('aprobe1', 'probe1')).toBe(false) }) - it('marks teammates idle by name via agent_type or agent_id prefix', () => { + it('removes teammates by name via agent_id prefix or agent_type fallback', () => { const roster: ClaudeSubagentRoster = new Map() upsertWorkingClaudeSubagent(roster, 'aprobe1-6d3cb5b5', { agentType: 'probe1' }, 100) upsertWorkingClaudeSubagent(roster, 'aother-123', { agentType: 'other' }, 100) - expect(markClaudeTeammateIdleByName(roster, 'probe1')).toBe(true) - expect(roster.get('aprobe1-6d3cb5b5')?.state).toBe('idle') - expect(roster.get('aother-123')?.state).toBe('working') - // Why: repeat idles are no-ops so lifecycle refreshes don't churn state. - expect(markClaudeTeammateIdleByName(roster, 'probe1')).toBe(false) - expect(markClaudeTeammateIdleByName(roster, 'ghost')).toBe(false) + // Why: TeammateIdle is keyed by name — idle means finished, so the row goes. + expect(removeClaudeTeammateByName(roster, 'probe1')).toBe(true) + expect(roster.has('aprobe1-6d3cb5b5')).toBe(false) + expect(roster.has('aother-123')).toBe(true) + // Repeat/unknown removals are no-ops so lifecycle refreshes don't churn. + expect(removeClaudeTeammateByName(roster, 'probe1')).toBe(false) + expect(removeClaudeTeammateByName(roster, 'ghost')).toBe(false) + }) + + it('falls back to agent_type only when no id matches the teammate name', () => { + const roster: ClaudeSubagentRoster = new Map() + // A one-shot whose agent_type collides with a teammate name, plus the + // real teammate. The id match wins; the colliding one-shot is spared. + upsertWorkingClaudeSubagent(roster, 'areviewer-6d3cb5b5', { agentType: 'reviewer' }, 100) + upsertWorkingClaudeSubagent(roster, 'aoneshot00000001', { agentType: 'reviewer' }, 100) + removeClaudeTeammateByName(roster, 'reviewer') + expect(roster.has('areviewer-6d3cb5b5')).toBe(false) + expect(roster.has('aoneshot00000001')).toBe(true) }) it('serializes snapshots deterministically ordered by startedAt then id', () => { @@ -422,7 +349,10 @@ describe('claude-subagent-roster', () => { upsertWorkingClaudeSubagent(roster, 'b', {}, 200) upsertWorkingClaudeSubagent(roster, 'z', {}, 100) upsertWorkingClaudeSubagent(roster, 'a', {}, 100) - expect(claudeRosterToSnapshots(roster)?.map((s) => s.id)).toEqual(['a', 'z', 'b']) + const snapshots = claudeRosterToSnapshots(roster) + expect(snapshots?.map((s) => s.id)).toEqual(['a', 'z', 'b']) + // Why: only working children are tracked, so every emitted row is working. + expect(snapshots?.every((s) => s.state === 'working')).toBe(true) expect(claudeRosterToSnapshots(new Map())).toBeUndefined() }) }) diff --git a/src/shared/claude-subagent-roster.ts b/src/shared/claude-subagent-roster.ts index b0b945a76e2..32c802349eb 100644 --- a/src/shared/claude-subagent-roster.ts +++ b/src/shared/claude-subagent-roster.ts @@ -5,28 +5,33 @@ import { AGENT_STATUS_MAX_SUBAGENTS, type AgentSubagentSnapshot } from './agent- * invisible in the emitted snapshots (which drop such ids). */ const CLAUDE_SUBAGENT_ID_MAX_LENGTH = 64 -/** Live subagents/teammates tracked for one Claude pane, keyed by the - * provider-assigned `agent_id` from SubagentStart/SubagentStop payloads. */ +/** Currently WORKING subagents/teammates tracked for one Claude pane, keyed + * by the provider-assigned `agent_id` from SubagentStart/SubagentStop + * payloads. The roster intentionally holds only working children: a child + * that finished leaves the sidebar immediately. Claude gives no other + * finish signal for named agents — their `background_tasks` teammate + * entries stay `status: "running"` forever, even after they complete + * (verified live on 2.1.210) — so retaining "idle" rows piled up dead + * entries for hours. A teammate resumed later re-earns its row via + * SubagentStart. */ export type ClaudeSubagentRoster = Map export type TrackedClaudeSubagent = { agentType?: string description?: string - state: 'working' | 'idle' startedAt: number - /** Known agent-teams teammate: its id embeds its name (`a-`) - * while one-shot ids are hyphen-free (`a`). Teammates are long-lived — - * SubagentStop means "finished a task", not "gone" — and their lifecycle - * ids never appear in background_tasks, so omission proves nothing. */ - teammate?: true /** The id came from a persisted snapshot or background_tasks, not live - * lifecycle events. Only matters for teammate-shaped seeds now: a PRESENT - * list omitting a seeded-working teammate demotes it to idle so a phantom - * seeded before restart can't gate the pane 'working' forever (teams - * sessions never send an empty list). Cleared once live activity re-tracks - * the id. Non-teammate entries omitted from a present list are removed - * outright regardless of this flag. */ + * lifecycle events, so it may be a phantom whose SubagentStop was never + * observed (Orca restart). A present complete task list omitting it + * removes it even when teammate-shaped, so it can't gate the pane + * 'working' forever. Cleared once live activity re-tracks the id. */ backgroundTasksAuthoritative?: boolean + /** A subagent-typed background task listed this lifecycle id id-exact + * (workflow/named lanes) — proof the task list tracks this id, so a later + * complete list omitting it means finished/killed even though the id is + * teammate-shaped. Never cleared: the listing mode of an id can't change + * mid-life. */ + listedAsSubagentTask?: true } /** One agent entry from the `background_tasks` array Claude attaches to Stop @@ -37,14 +42,15 @@ export type ClaudeBackgroundAgentTask = { agentType?: string description?: string running: boolean - /** True for `type: "teammate"` entries, whose ids never match lifecycle - * agent_ids and whose "running" status persists while idle. */ + /** True for `type: "teammate"` entries. Their ids never match lifecycle + * agent_ids and they report "running" permanently — even after the named + * agent finished — so they carry no per-agent state at all. */ teammate: boolean } -/** Agent-team lifecycle ids are `a-`. The teammate name - * and agent type are independent spawn fields, so the id shape is the only - * reliable discriminator available on SubagentStart/SubagentStop hooks. */ +/** Agent-team/named-agent lifecycle ids are `a-` while one-shot + * ids are hyphen-free (`a`). Such ids are never listed as task ids in + * `background_tasks`, so omission from the list proves nothing for them. */ export function isClaudeTeammateLifecycleId(id: string): boolean { const separator = id.lastIndexOf('-') return separator > 1 && id.startsWith('a') && /^[0-9a-f]+$/i.test(id.slice(separator + 1)) @@ -59,73 +65,33 @@ export function upsertWorkingClaudeSubagent( if (id.length === 0 || id.length > CLAUDE_SUBAGENT_ID_MAX_LENGTH) { return } - const teammate = isClaudeTeammateLifecycleId(id) const existing = roster.get(id) if (existing) { - existing.state = 'working' existing.agentType = fields.agentType ?? existing.agentType existing.description = fields.description ?? existing.description - if (teammate) { - existing.teammate = true - } // Why: live activity proves the lifecycle stream owns this id again; - // background_tasks absence must stop demoting it (teammate ids never - // appear there). The fold re-tags its own recreations after this call. + // background_tasks omission must stop reaping it (teammate-shaped ids + // never appear there). The fold re-tags its own recreations after this. existing.backgroundTasksAuthoritative = undefined return } - if (roster.size >= AGENT_STATUS_MAX_SUBAGENTS && !evictOldestIdleClaudeSubagent(roster)) { + // Why: beyond the wire cap extra rows would be invisible anyway; with only + // working entries tracked there is nothing safe to evict. + if (roster.size >= AGENT_STATUS_MAX_SUBAGENTS) { return } roster.set(id, { - state: 'working', startedAt: now, agentType: fields.agentType, - description: fields.description, - ...(teammate ? { teammate: true as const } : {}) + description: fields.description }) } -function evictOldestIdleClaudeSubagent(roster: ClaudeSubagentRoster): boolean { - let oldestId: string | null = null - let oldestStartedAt = Infinity - for (const [id, tracked] of roster) { - if (tracked.state === 'idle' && tracked.startedAt < oldestStartedAt) { - oldestId = id - oldestStartedAt = tracked.startedAt - } - } - if (oldestId === null) { - return false - } - roster.delete(oldestId) - return true -} - -/** SubagentStop: a finished one-shot subagent leaves the sidebar immediately — - * retaining it as an idle row made long workflow/ultracode sessions pile up - * dozens of dead rows. Teammates only idle (alive + resumable): SubagentStop - * fires each time a teammate finishes a task, not just at shutdown. - * - * Workflow/named one-shots share the teammate id shape (`a