mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 08:03:28 +00:00
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.
This commit is contained in:
@@ -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: [] })
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user