mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 00:03:20 +00:00
Fix Claude subagent roster reconciliation
This commit is contained in:
@@ -26,6 +26,7 @@ import {
|
||||
clearGrokSessionPathLookupCacheForTests,
|
||||
findGrokChatHistoryBySessionId
|
||||
} from './grok-session-paths'
|
||||
import { AGENT_STATUS_MAX_SUBAGENTS } from './agent-status-types'
|
||||
import { makePaneKey } from './stable-pane-id'
|
||||
|
||||
const LEAF_ID = '11111111-1111-4111-8111-111111111111'
|
||||
@@ -2408,6 +2409,35 @@ describe('shared agent-hook-listener', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('keeps a teammate whose name differs from its configured agent type', () => {
|
||||
claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'spawn reviewer' })
|
||||
claudeEvent({
|
||||
hook_event_name: 'SubagentStart',
|
||||
agent_id: 'areviewer-6d3cb5b52120b7bf',
|
||||
agent_type: 'security-reviewer'
|
||||
})
|
||||
|
||||
// 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'
|
||||
})
|
||||
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'
|
||||
})
|
||||
])
|
||||
})
|
||||
|
||||
it('scopes subagent rosters per pane', () => {
|
||||
claudeEvent(
|
||||
{ hook_event_name: 'SubagentStart', agent_id: 'a1', agent_type: 'general-purpose' },
|
||||
@@ -2587,6 +2617,29 @@ describe('shared agent-hook-listener', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('keeps a live child omitted by the background task snapshot cap', () => {
|
||||
claudeEvent({
|
||||
hook_event_name: 'SubagentStart',
|
||||
agent_id: 'alive-after-cap',
|
||||
agent_type: 'general-purpose'
|
||||
})
|
||||
const stop = claudeEvent({
|
||||
hook_event_name: 'Stop',
|
||||
background_tasks: Array.from({ length: AGENT_STATUS_MAX_SUBAGENTS + 1 }, (_, index) => ({
|
||||
id: index === AGENT_STATUS_MAX_SUBAGENTS ? 'alive-after-cap' : `a${index}`,
|
||||
type: 'subagent',
|
||||
status: 'running'
|
||||
}))
|
||||
})
|
||||
|
||||
// Why: the inventory was capped before this id, so omission cannot
|
||||
// prove the lifecycle-tracked child finished or was killed.
|
||||
expect(stop?.payload.subagents).toContainEqual(
|
||||
expect.objectContaining({ id: 'alive-after-cap', state: 'working' })
|
||||
)
|
||||
expect(stop?.payload.state).toBe('working')
|
||||
})
|
||||
|
||||
it('does not adopt a known child turn-boundary event as the lead state', () => {
|
||||
claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'go' })
|
||||
claudeEvent({
|
||||
@@ -2652,7 +2705,12 @@ describe('shared agent-hook-listener', () => {
|
||||
|
||||
it('seeds the roster from persisted snapshots so a teammate-bearing Stop keeps child rows', () => {
|
||||
seedClaudeSubagentRosterFromSnapshots(state, PANE_KEY, [
|
||||
{ id: 'aprobe2-abc', state: 'idle', startedAt: 1000, agentType: 'probe2' }
|
||||
{
|
||||
id: 'aprobe2-6d3cb5b52120b7bf',
|
||||
state: 'idle',
|
||||
startedAt: 1000,
|
||||
agentType: 'security-reviewer'
|
||||
}
|
||||
])
|
||||
claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'after restart' })
|
||||
const stop = claudeEvent({
|
||||
@@ -2663,7 +2721,11 @@ describe('shared agent-hook-listener', () => {
|
||||
})
|
||||
expect(stop?.payload.state).toBe('done')
|
||||
expect(stop?.payload.subagents).toEqual([
|
||||
expect.objectContaining({ id: 'aprobe2-abc', state: 'idle' })
|
||||
expect.objectContaining({
|
||||
id: 'aprobe2-6d3cb5b52120b7bf',
|
||||
agentType: 'security-reviewer',
|
||||
state: 'idle'
|
||||
})
|
||||
])
|
||||
})
|
||||
|
||||
|
||||
@@ -40,6 +40,7 @@ import {
|
||||
claudeTeammateIdMatchesName,
|
||||
finishClaudeSubagent,
|
||||
foldClaudeBackgroundTasksIntoRoster,
|
||||
isClaudeTeammateLifecycleId,
|
||||
markClaudeTeammateIdleByName,
|
||||
readClaudeBackgroundAgentTasks,
|
||||
upsertWorkingClaudeSubagent,
|
||||
@@ -2449,12 +2450,9 @@ export function seedClaudeSubagentRosterFromSnapshots(
|
||||
startedAt: snapshot.startedAt,
|
||||
agentType: snapshot.agentType,
|
||||
description: snapshot.description,
|
||||
// Why: re-derive teammate-ness from the persisted id/agentType so a
|
||||
// seeded teammate keeps its idle row instead of being removed as a
|
||||
// finished one-shot by the next fold.
|
||||
...(snapshot.agentType && claudeTeammateIdMatchesName(snapshot.id, snapshot.agentType)
|
||||
? { teammate: true as const }
|
||||
: {}),
|
||||
// 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
|
||||
@@ -2616,7 +2614,8 @@ function normalizeClaudeEvent(
|
||||
foldClaudeBackgroundTasksIntoRoster(
|
||||
getOrCreateClaudeSubagentRoster(state, paneKey),
|
||||
backgroundTasks.tasks,
|
||||
Date.now()
|
||||
Date.now(),
|
||||
{ inventoryComplete: !backgroundTasks.truncated }
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -109,10 +109,21 @@ describe('claude-subagent-roster', () => {
|
||||
expect(readClaudeBackgroundAgentTasks({ background_tasks: 'nope' }).present).toBe(false)
|
||||
})
|
||||
|
||||
it('marks a background task inventory truncated after the snapshot cap', () => {
|
||||
const tasks = Array.from({ length: AGENT_STATUS_MAX_SUBAGENTS + 1 }, (_, index) => ({
|
||||
id: `a${index}`,
|
||||
type: 'subagent',
|
||||
status: 'running'
|
||||
}))
|
||||
const result = readClaudeBackgroundAgentTasks({ background_tasks: tasks })
|
||||
expect(result.tasks).toHaveLength(AGENT_STATUS_MAX_SUBAGENTS)
|
||||
expect(result.truncated).toBe(true)
|
||||
})
|
||||
|
||||
it('folds background_tasks in without trusting ambiguous entries', () => {
|
||||
const roster: ClaudeSubagentRoster = new Map()
|
||||
upsertWorkingClaudeSubagent(roster, 'a1', {}, 100)
|
||||
upsertWorkingClaudeSubagent(roster, 'ateam-xyz', { agentType: 'team' }, 150)
|
||||
upsertWorkingClaudeSubagent(roster, 'ateam-6d3cb5b5', { agentType: 'security-reviewer' }, 150)
|
||||
|
||||
foldClaudeBackgroundTasksIntoRoster(
|
||||
roster,
|
||||
@@ -142,7 +153,10 @@ describe('claude-subagent-roster', () => {
|
||||
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-xyz')).toMatchObject({ state: 'working', agentType: 'team' })
|
||||
expect(roster.get('ateam-6d3cb5b5')).toMatchObject({
|
||||
state: 'working',
|
||||
agentType: 'security-reviewer'
|
||||
})
|
||||
})
|
||||
|
||||
it('removes an id-matched task reported not running', () => {
|
||||
@@ -172,6 +186,23 @@ describe('claude-subagent-roster', () => {
|
||||
expect(roster.size).toBe(0)
|
||||
})
|
||||
|
||||
it('retains an unlisted live child when the background task inventory was truncated', () => {
|
||||
const roster: ClaudeSubagentRoster = new Map()
|
||||
upsertWorkingClaudeSubagent(roster, 'alive-after-cap', {}, 100)
|
||||
const parsed = readClaudeBackgroundAgentTasks({
|
||||
background_tasks: Array.from({ length: AGENT_STATUS_MAX_SUBAGENTS + 1 }, (_, index) => ({
|
||||
id: index === AGENT_STATUS_MAX_SUBAGENTS ? 'alive-after-cap' : `a${index}`,
|
||||
type: 'subagent',
|
||||
status: 'running'
|
||||
}))
|
||||
})
|
||||
|
||||
foldClaudeBackgroundTasksIntoRoster(roster, parsed.tasks, 200, {
|
||||
inventoryComplete: !parsed.truncated
|
||||
})
|
||||
expect(roster.has('alive-after-cap')).toBe(true)
|
||||
})
|
||||
|
||||
it('recreates unmatched running one-shot subagents after a listener restart', () => {
|
||||
const roster: ClaudeSubagentRoster = new Map()
|
||||
foldClaudeBackgroundTasksIntoRoster(
|
||||
@@ -213,7 +244,7 @@ describe('claude-subagent-roster', () => {
|
||||
startedAt: 100,
|
||||
backgroundTasksAuthoritative: true
|
||||
})
|
||||
upsertWorkingClaudeSubagent(roster, 'ateam-xyz', { agentType: 'team' }, 150)
|
||||
upsertWorkingClaudeSubagent(roster, 'ateam-6d3cb5b5', { agentType: 'security-reviewer' }, 150)
|
||||
|
||||
foldClaudeBackgroundTasksIntoRoster(
|
||||
roster,
|
||||
@@ -223,7 +254,7 @@ describe('claude-subagent-roster', () => {
|
||||
200
|
||||
)
|
||||
expect(roster.has('a-phantom')).toBe(false)
|
||||
expect(roster.get('ateam-xyz')).toMatchObject({ state: 'working' })
|
||||
expect(roster.get('ateam-6d3cb5b5')).toMatchObject({ state: 'working' })
|
||||
})
|
||||
|
||||
it('demotes a seeded teammate phantom missing from a present list to idle', () => {
|
||||
|
||||
@@ -42,6 +42,14 @@ export type ClaudeBackgroundAgentTask = {
|
||||
teammate: boolean
|
||||
}
|
||||
|
||||
/** Agent-team lifecycle ids are `a<teammate-name>-<hex>`. The teammate name
|
||||
* and agent type are independent spawn fields, so the id shape is the only
|
||||
* reliable discriminator available on SubagentStart/SubagentStop hooks. */
|
||||
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))
|
||||
}
|
||||
|
||||
export function upsertWorkingClaudeSubagent(
|
||||
roster: ClaudeSubagentRoster,
|
||||
id: string,
|
||||
@@ -51,10 +59,7 @@ export function upsertWorkingClaudeSubagent(
|
||||
if (id.length === 0 || id.length > CLAUDE_SUBAGENT_ID_MAX_LENGTH) {
|
||||
return
|
||||
}
|
||||
// Why: teammate SubagentStart carries agent_type = teammate name and the id
|
||||
// embeds it; one-shot ids are hyphen-free hex so they can never match.
|
||||
const teammate =
|
||||
fields.agentType !== undefined && claudeTeammateIdMatchesName(id, fields.agentType)
|
||||
const teammate = isClaudeTeammateLifecycleId(id)
|
||||
const existing = roster.get(id)
|
||||
if (existing) {
|
||||
existing.state = 'working'
|
||||
@@ -119,12 +124,14 @@ export function finishClaudeSubagent(roster: ClaudeSubagentRoster, id: string):
|
||||
export function readClaudeBackgroundAgentTasks(hookPayload: Record<string, unknown>): {
|
||||
present: boolean
|
||||
tasks: ClaudeBackgroundAgentTask[]
|
||||
truncated: boolean
|
||||
} {
|
||||
const raw = hookPayload['background_tasks']
|
||||
if (!Array.isArray(raw)) {
|
||||
return { present: false, tasks: [] }
|
||||
return { present: false, tasks: [], truncated: false }
|
||||
}
|
||||
const tasks: ClaudeBackgroundAgentTask[] = []
|
||||
let truncated = false
|
||||
for (const item of raw) {
|
||||
if (typeof item !== 'object' || item === null) {
|
||||
continue
|
||||
@@ -136,6 +143,12 @@ export function readClaudeBackgroundAgentTasks(hookPayload: Record<string, unkno
|
||||
if (typeof obj.id !== 'string' || obj.id.trim().length === 0) {
|
||||
continue
|
||||
}
|
||||
if (tasks.length >= AGENT_STATUS_MAX_SUBAGENTS) {
|
||||
// Why: a capped inventory cannot prove a tracked id is absent; callers
|
||||
// must retain unlisted rows rather than deleting live overflow tasks.
|
||||
truncated = true
|
||||
break
|
||||
}
|
||||
tasks.push({
|
||||
id: obj.id,
|
||||
agentType: typeof obj.agent_type === 'string' ? obj.agent_type : undefined,
|
||||
@@ -143,11 +156,8 @@ export function readClaudeBackgroundAgentTasks(hookPayload: Record<string, unkno
|
||||
running: obj.status === 'running',
|
||||
teammate: obj.type === 'teammate'
|
||||
})
|
||||
if (tasks.length >= AGENT_STATUS_MAX_SUBAGENTS) {
|
||||
break
|
||||
}
|
||||
}
|
||||
return { present: true, tasks }
|
||||
return { present: true, tasks, truncated }
|
||||
}
|
||||
|
||||
/** Fold a lead Stop's `background_tasks` into the lifecycle-tracked roster.
|
||||
@@ -172,10 +182,13 @@ export function readClaudeBackgroundAgentTasks(hookPayload: Record<string, unkno
|
||||
export function foldClaudeBackgroundTasksIntoRoster(
|
||||
roster: ClaudeSubagentRoster,
|
||||
tasks: ClaudeBackgroundAgentTask[],
|
||||
now: number
|
||||
now: number,
|
||||
options?: { inventoryComplete?: boolean }
|
||||
): void {
|
||||
if (tasks.length === 0) {
|
||||
roster.clear()
|
||||
if (options?.inventoryComplete !== false) {
|
||||
roster.clear()
|
||||
}
|
||||
return
|
||||
}
|
||||
const listedIds = new Set<string>()
|
||||
@@ -206,6 +219,9 @@ export function foldClaudeBackgroundTasksIntoRoster(
|
||||
created.backgroundTasksAuthoritative = true
|
||||
}
|
||||
}
|
||||
if (options?.inventoryComplete === false) {
|
||||
return
|
||||
}
|
||||
for (const [id, tracked] of roster) {
|
||||
if (listedIds.has(id)) {
|
||||
continue
|
||||
|
||||
@@ -113,7 +113,7 @@ describe('claude subagent sidebar row lifecycle', () => {
|
||||
claudeEvent({
|
||||
hook_event_name: 'SubagentStart',
|
||||
agent_id: 'areviewer-6d3cb5b52120b7bf',
|
||||
agent_type: 'reviewer'
|
||||
agent_type: 'security-reviewer'
|
||||
})
|
||||
claudeEvent({
|
||||
hook_event_name: 'SubagentStart',
|
||||
@@ -125,7 +125,7 @@ describe('claude subagent sidebar row lifecycle', () => {
|
||||
claudeEvent({
|
||||
hook_event_name: 'SubagentStop',
|
||||
agent_id: 'areviewer-6d3cb5b52120b7bf',
|
||||
agent_type: 'reviewer'
|
||||
agent_type: 'security-reviewer'
|
||||
})
|
||||
const idled = claudeEvent({
|
||||
hook_event_name: 'TeammateIdle',
|
||||
|
||||
Reference in New Issue
Block a user