From e686b3acc062212e1e660451a49f5d95bf6e9d67 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sat, 1 Aug 2026 09:49:24 -0700 Subject: [PATCH] fix(agent-status): make stop-time inventory processing strictly additive Review findings round 13 share one root: stop-time inventories are mutually unordered, so any deletion they drive can be a stale claim against newer live state. Fold only running valid entries (no deletions; listed-non-running cleanup stays with turn-serialized lead Stops), reap only pre-restart restoredFromSnapshot rows on an empty inventory, and drop the untrackable raw-scan working mint (truncated stays silent). --- src/shared/agent-hook-listener.test.ts | 36 +++++++++++--- src/shared/agent-hook-listener.ts | 67 ++++++++------------------ 2 files changed, 47 insertions(+), 56 deletions(-) diff --git a/src/shared/agent-hook-listener.test.ts b/src/shared/agent-hook-listener.test.ts index 59623f8636b..97e86725fe8 100644 --- a/src/shared/agent-hook-listener.test.ts +++ b/src/shared/agent-hook-listener.test.ts @@ -3221,9 +3221,9 @@ describe('shared agent-hook-listener', () => { agent_id: 'a1', background_tasks: cappedTasks }) - // Why: the uncapped raw scan still sees hidden-live running — the pane is - // truthfully working, never done. - expect(stopped?.payload.state).toBe('working') + // Why: hidden-live sits past the parser cap and could never be tracked or + // drained — incomplete evidence claims neither done nor an unresolvable working. + expect(stopped).toBeNull() }) it('claims nothing from a no-lead removal when a truncated inventory shows no live work', () => { @@ -3372,9 +3372,10 @@ describe('shared agent-hook-listener', () => { expect(staleStop?.payload.subagents).toEqual([expect.objectContaining({ id: 'a-new' })]) }) - it('resolves to done when an empty inventory reaps the last inventory-derived rows', () => { - // Why: the authority that minted a1's row now reports empty; a silent - // drain would strand the previously published working until the sweeper. + it('keeps a newer inventory-derived row through a delayed older empty-inventory stop', () => { + // Why: stop-time inventories are mutually unordered — a delayed older empty + // list must not retire a row a newer inventory minted. The tracked row keeps + // the pane truthfully working and its own stop resolves it. const first = claudeEvent({ hook_event_name: 'SubagentStop', agent_id: 'ghost-1', @@ -3388,8 +3389,27 @@ describe('shared agent-hook-listener', () => { agent_id: 'ghost-2', background_tasks: [] }) - expect(second?.payload.state).toBe('done') - expect(second?.payload.subagents).toBeUndefined() + expect(second?.payload.state).toBe('working') + expect(second?.payload.subagents).toEqual([expect.objectContaining({ id: 'a1' })]) + }) + + it('never lets a stop inventory delete a live row via a non-running listing', () => { + claudeEvent({ + hook_event_name: 'SubagentStart', + agent_id: 'a1', + agent_type: 'general-purpose' + }) + // Why: a stale inventory claiming a live child completed is unordered + // evidence; the live row survives and its own stop still resolves done. + const staleClaim = claudeEvent({ + hook_event_name: 'SubagentStop', + agent_id: 'ghost-1', + background_tasks: [{ id: 'a1', type: 'subagent', status: 'completed' }] + }) + expect(staleClaim?.payload.state).toBe('working') + expect(staleClaim?.payload.subagents).toEqual([expect.objectContaining({ id: 'a1' })]) + const drained = claudeEvent({ hook_event_name: 'SubagentStop', agent_id: 'a1' }) + expect(drained?.payload.state).toBe('done') }) it('clears restored phantom rows when a no-lead stop carries a complete empty inventory', () => { diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index 82744b39dc8..6c9d0ff7e45 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -2470,34 +2470,31 @@ function normalizeClaudeSubagentLifecycleEvent( // tracking them keeps their own later drains resolvable instead of start-less-suppressed. // Additive only — a complete-inventory reap here would kill a just-parked teammate row // before its TeammateIdle confirmation arrives. + // Why: stop-time inventories are mutually UNORDERED — any deletion they drive can be a + // stale claim against newer live state, so stop-time processing is strictly additive. + // Listed-non-running cleanup belongs to turn-serialized lead-Stop folds only. const inventory = readClaudeBackgroundAgentTasks(hookPayload) + const runningTasks = inventory.tasks.filter( + // Why: an id the upsert would reject can neither be tracked nor drained — letting it + // allocate or claim liveness recreates the retention/sticky holes through inventory ids. + (task) => task.running && !task.teammate && isValidClaudeSubagentId(task.id) + ) if (inventory.present && inventory.tasks.length === 0 && roster) { - // Why: an empty complete inventory reaps only claims not backed by live lifecycle events - // (restored/inventory-derived phantoms). A delayed stale stop's empty list must not clear - // a newer live start's row — that is the uncorrelated-retirement race again. The reap is - // same-source-correlated removal evidence: the authority that minted those rows now - // reports empty, so a silent drain must not strand the published working. + // Why: restored rows are pre-restart claims, so ANY live inventory postdates them — + // reaping them on an empty inventory is temporally safe, and counts as removal + // evidence (same authority class that persisted the claim now reports empty). Rows + // minted by other stop-time inventories stay: those sources are unordered. for (const [id, tracked] of roster) { - if (tracked.restoredFromSnapshot || tracked.backgroundTasksAuthoritative) { + if (tracked.restoredFromSnapshot) { roster.delete(id) removedRow = true } } - } else if ( - inventory.present && - (roster || - inventory.tasks.some( - // Why: an id the upsert would reject can neither be tracked nor drained — letting it - // allocate or claim liveness recreates the retention/sticky holes through inventory ids. - (task) => task.running && !task.teammate && isValidClaudeSubagentId(task.id) - )) - ) { + } else if (inventory.present && runningTasks.length > 0) { foldClaudeBackgroundTasksIntoRoster( getOrCreateClaudeSubagentRoster(state, paneKey), - inventory.tasks, + runningTasks, Date.now(), - // Why: additive only — a complete-inventory reap here would kill a just-parked - // teammate row before its TeammateIdle confirmation arrives. { inventoryComplete: false } ) } @@ -2624,29 +2621,6 @@ export function clearClaudeAnsweredQuestionWait( } /** Re-emit the lead's cached state on child activity — gated up to 'working' while a child works — without touching the lead's tool/prompt caches, so a live card or permission wait survives child churn. */ -// Why: the roster parser bounds TRACKED rows at the wire cap; this boolean liveness scan of the -// raw array must still see a running subagent-typed entry hidden past it. -function rawInventoryHasRunningSubagentTask(hookPayload: Record): boolean { - const raw = hookPayload['background_tasks'] - return ( - Array.isArray(raw) && - raw.some((item) => { - if (typeof item !== 'object' || item === null) { - return false - } - const task = item as Record - return ( - task.type === 'subagent' && - task.status === 'running' && - // Why: an id the upsert would reject can never be tracked or drained; letting it claim - // liveness would pin an unresolvable working on the pane. - typeof task.id === 'string' && - isValidClaudeSubagentId(task.id) - ) - }) - ) -} - function buildClaudeChildDrivenStatusPayload( state: HookListenerState, eventName: unknown, @@ -2677,18 +2651,15 @@ function buildClaudeChildDrivenStatusPayload( }) } if (evidence.removedRow) { - // Why: live shells/crons veto done, and the uncapped raw scan sees a running subagent hidden - // past the parser cap (tracked folding surfaces the listed ones through the working branch). - if ( - hasActiveClaudeNonAgentBackgroundWork(hookPayload) || - rawInventoryHasRunningSubagentTask(hookPayload) - ) { + // Why: live shells/crons in the payload's own inventory veto done — the pane still works. + if (hasActiveClaudeNonAgentBackgroundWork(hookPayload)) { return buildClaudeStatusPayload(state, eventName, '', paneKey, hookPayload, { stateName: 'working', updateToolSnapshot: false }) } - // Why: a truncated inventory with no visible live work is incomplete evidence — claim nothing. + // Why: a truncated inventory may hide running children past the parser cap — incomplete + // evidence claims nothing (no done, and no working the roster could never resolve). const inventory = readClaudeBackgroundAgentTasks(hookPayload) if (inventory.present && inventory.truncated) { return null