From 6afef239c44bdd2eb0a97d91ca1305ca414bf605 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sat, 1 Aug 2026 07:32:30 -0700 Subject: [PATCH] fix(agent-status): prevalidate subagent ids before allocating a roster Review finding round 2: upsert silently rejects over-length agent ids after the caller already allocated, stranding one empty roster per invented pane key with zero emitted payloads. Export the id validity check and gate both wire-reachable allocation sites. --- src/shared/agent-hook-listener.test.ts | 19 +++++++++++++++++++ src/shared/agent-hook-listener.ts | 25 +++++++++++++++++-------- src/shared/claude-subagent-roster.ts | 6 ++++++ 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/shared/agent-hook-listener.test.ts b/src/shared/agent-hook-listener.test.ts index 357cd14147f..fa07472cd47 100644 --- a/src/shared/agent-hook-listener.test.ts +++ b/src/shared/agent-hook-listener.test.ts @@ -3113,6 +3113,25 @@ describe('shared agent-hook-listener', () => { expect(state.claudeSubagentRosterByPaneKey.has(PANE_KEY)).toBe(false) }) + it('does not retain a roster when a child tool event carries an id the upsert rejects', () => { + // Why: upsert silently drops over-length ids; allocating first would strand + // an empty roster per invented pane key with zero emitted payloads. + const childTool = claudeEvent({ + hook_event_name: 'PreToolUse', + agent_id: 'a'.repeat(65), + tool_name: 'Bash', + tool_input: { command: 'true' } + }) + expect(childTool).toBeNull() + expect(state.claudeSubagentRosterByPaneKey.has(PANE_KEY)).toBe(false) + }) + + it('does not retain a roster for a SubagentStart with an id the upsert rejects', () => { + const spawned = claudeEvent({ hook_event_name: 'SubagentStart', agent_id: 'a'.repeat(65) }) + expect(spawned).toBeNull() + expect(state.claudeSubagentRosterByPaneKey.has(PANE_KEY)).toBe(false) + }) + it('re-emits the lead done state on a start-less SubagentStop after a completed turn', () => { claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'go' }) claudeEvent({ hook_event_name: 'Stop', background_tasks: [] }) diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index 3c3bc2117e8..0924655910e 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -33,6 +33,7 @@ import { foldClaudeBackgroundTasksIntoRoster, hasActiveClaudeNonAgentBackgroundWork, idleClaudeTeammateByName, + isValidClaudeSubagentId, readClaudeBackgroundAgentTasks, reapRestoredClaudeSubagentsWithoutLiveAgent, stopClaudeSubagent, @@ -2446,13 +2447,16 @@ function normalizeClaudeSubagentLifecycleEvent( } else { const agentId = lifecycleId if (eventName === 'SubagentStart') { - upsertWorkingClaudeSubagent( - getOrCreateClaudeSubagentRoster(state, paneKey), - agentId, - { agentType: readString(hookPayload, 'agent_type') }, - Date.now() - ) - rosterChanged = true + // Why: upsert rejects invalid ids silently — allocate only for an id it will accept. + if (isValidClaudeSubagentId(agentId)) { + upsertWorkingClaudeSubagent( + getOrCreateClaudeSubagentRoster(state, paneKey), + agentId, + { agentType: readString(hookPayload, 'agent_type') }, + Date.now() + ) + rosterChanged = true + } } else { if (roster) { // Why: a stop of an untracked id (e.g. compact's start-less SubagentStop) is a roster no-op, not evidence. @@ -2631,7 +2635,12 @@ function normalizeClaudeEvent( eventName === 'PostToolUseFailure') ? eventAgentId : undefined - if (eventAgentId && (subagentOriginId || isWaitingInducing)) { + if ( + eventAgentId && + (subagentOriginId || isWaitingInducing) && + isValidClaudeSubagentId(eventAgentId) + ) { + // Why: upsert rejects invalid ids silently — allocate only for an id it will accept. upsertWorkingClaudeSubagent( getOrCreateClaudeSubagentRoster(state, paneKey), eventAgentId, diff --git a/src/shared/claude-subagent-roster.ts b/src/shared/claude-subagent-roster.ts index 50be0ab5ab9..250127d4103 100644 --- a/src/shared/claude-subagent-roster.ts +++ b/src/shared/claude-subagent-roster.ts @@ -73,6 +73,12 @@ export function isClaudeTeammateLifecycleId(id: string): boolean { return separator > 1 && id.startsWith('a') && /^[0-9a-f]+$/i.test(id.slice(separator + 1)) } +/** Pre-check for callers that allocate a roster to upsert into — upsert rejects + * invalid ids silently, which would strand the fresh roster empty forever. */ +export function isValidClaudeSubagentId(id: string): boolean { + return id.length > 0 && id.length <= CLAUDE_SUBAGENT_ID_MAX_LENGTH +} + export function upsertWorkingClaudeSubagent( roster: ClaudeSubagentRoster, id: string,