mirror of
https://github.com/stablyai/orca.git
synced 2026-10-27 08:02:51 +00:00
fix(agent-status): infer done only from one-shot removals when no lead record exists
Review finding round 4: teammate ids persist across turns, so a delayed stale SubagentStop parks a live teammate's row and the drain->done inference retired newer work — PR #11353's uncorrelated-completion race. With no lead record, only a live working child (working) or a one-shot row removal (done; duplicate-proof since the row is gone) may speak; teammate parks and idle-only refreshes stay silent.
This commit is contained in:
@@ -3156,6 +3156,37 @@ describe('shared agent-hook-listener', () => {
|
||||
expect(duplicate).toBeNull()
|
||||
})
|
||||
|
||||
it('never claims done from a teammate park with no lead record', () => {
|
||||
// Why: teammate ids are persistent across turns — a delayed stop from turn N
|
||||
// parks the id even while its newer turn N+1 is live. Claiming done here is
|
||||
// the uncorrelated-completion race that closed PR #11353.
|
||||
claudeEvent({
|
||||
hook_event_name: 'SubagentStart',
|
||||
agent_id: 'alane-6d3cb5b52120b7bf',
|
||||
agent_type: 'lane'
|
||||
})
|
||||
const staleStop = claudeEvent({
|
||||
hook_event_name: 'SubagentStop',
|
||||
agent_id: 'alane-6d3cb5b52120b7bf'
|
||||
})
|
||||
expect(staleStop).toBeNull()
|
||||
const duplicatePark = claudeEvent({
|
||||
hook_event_name: 'SubagentStop',
|
||||
agent_id: 'alane-6d3cb5b52120b7bf'
|
||||
})
|
||||
expect(duplicatePark).toBeNull()
|
||||
})
|
||||
|
||||
it('never claims done from a no-lead TeammateIdle park', () => {
|
||||
claudeEvent({
|
||||
hook_event_name: 'SubagentStart',
|
||||
agent_id: 'alane-6d3cb5b52120b7bf',
|
||||
agent_type: 'lane'
|
||||
})
|
||||
const idled = claudeEvent({ hook_event_name: 'TeammateIdle', teammate_name: 'lane' })
|
||||
expect(idled).toBeNull()
|
||||
})
|
||||
|
||||
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: [] })
|
||||
@@ -3525,6 +3556,11 @@ describe('shared agent-hook-listener', () => {
|
||||
})
|
||||
|
||||
it('scopes TeammateIdle to the exact teammate name for hyphen-prefix names', () => {
|
||||
// Why: a lead record anchors the emissions — no-lead teammate parks are
|
||||
// deliberately silent (uncorrelated-completion guard), which is not what
|
||||
// this test pins; it pins the name-scoping of the park itself.
|
||||
claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'spawn lanes' })
|
||||
claudeEvent({ hook_event_name: 'Stop', background_tasks: [] })
|
||||
claudeEvent({
|
||||
hook_event_name: 'SubagentStart',
|
||||
agent_id: 'alane-hooks-6d3cb5b5',
|
||||
|
||||
@@ -2436,11 +2436,13 @@ function normalizeClaudeSubagentLifecycleEvent(
|
||||
// Why: only a spawn may allocate a roster — stop/idle on an unknown pane would retain an empty
|
||||
// roster per untrusted pane key with no lifecycle bound (STA-2915 review finding).
|
||||
const roster = state.claudeSubagentRosterByPaneKey.get(paneKey)
|
||||
let rosterChanged = false
|
||||
let removedRow = false
|
||||
if (eventName === 'TeammateIdle') {
|
||||
const teammateName = lifecycleId
|
||||
// Why: on claude 2.1.21x teammates are turn-based — TeammateIdle means "turn over, awaiting mail", not finished. The row parks as idle (confirmed teammate) instead of leaving, so the sidebar keeps showing resumable children.
|
||||
rosterChanged = roster ? idleClaudeTeammateByName(roster, teammateName) : false
|
||||
if (roster) {
|
||||
idleClaudeTeammateByName(roster, teammateName)
|
||||
}
|
||||
clearClaudePendingWaitForAgent(state, paneKey, (waitingAgentId) =>
|
||||
claudeTeammateIdMatchesName(waitingAgentId, teammateName)
|
||||
)
|
||||
@@ -2455,20 +2457,21 @@ function normalizeClaudeSubagentLifecycleEvent(
|
||||
{ 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.
|
||||
rosterChanged = roster.has(agentId)
|
||||
const hadRow = roster.has(agentId)
|
||||
// Why: one-shot stops are true finishes (row removed); teammate-shaped stops are turn ends on 2.1.21x — the row parks idle and a later SubagentStart revives it.
|
||||
stopClaudeSubagent(roster, agentId)
|
||||
removedRow = hadRow && !roster.has(agentId)
|
||||
}
|
||||
// Why: a blocked child that dies without another tool event would pin its permission/question wait on the pane forever — nothing else references that agent again.
|
||||
clearClaudePendingWaitForAgent(state, paneKey, (waitingAgentId) => waitingAgentId === agentId)
|
||||
}
|
||||
}
|
||||
return buildClaudeChildDrivenStatusPayload(state, eventName, paneKey, hookPayload, rosterChanged)
|
||||
return buildClaudeChildDrivenStatusPayload(state, eventName, paneKey, hookPayload, {
|
||||
removedRow
|
||||
})
|
||||
}
|
||||
|
||||
/** Sync the Claude lead-turn record when the SERVER infers an interrupt outside the hook stream (Ctrl+C with a missed Stop); else a later child lifecycle event resurrects the cancelled pane. */
|
||||
@@ -2566,26 +2569,37 @@ function buildClaudeChildDrivenStatusPayload(
|
||||
eventName: unknown,
|
||||
paneKey: string,
|
||||
hookPayload: Record<string, unknown>,
|
||||
rosterChanged = false
|
||||
evidence: { removedRow?: boolean } = {}
|
||||
): ParsedAgentStatusPayload | null {
|
||||
const lead = state.claudeLeadStateByPaneKey.get(paneKey)
|
||||
const roster = state.claudeSubagentRosterByPaneKey.get(paneKey)
|
||||
// Why: no lead record, no live child, and an unchanged roster mean the event proves nothing is
|
||||
// running. Compact's start-less SubagentStop is exactly that; minting the default below left a
|
||||
// sticky 'working' no Stop ever clears on freshly resumed panes (STA-2915).
|
||||
const rosterHasWorking = claudeRosterHasWorkingSubagent(roster)
|
||||
if (!lead && !rosterChanged && !rosterHasWorking) {
|
||||
return null
|
||||
if (lead) {
|
||||
return buildClaudeStatusPayload(state, eventName, '', paneKey, hookPayload, {
|
||||
stateName: lead.state === 'done' && rosterHasWorking ? 'working' : lead.state,
|
||||
updateToolSnapshot: false,
|
||||
interrupted: lead.interrupted
|
||||
})
|
||||
}
|
||||
// Why: with no lead record the pane state is inferred from tracked children only — a live child
|
||||
// shows working, while an identity-matched drain/park resolves to done. The old unconditional
|
||||
// 'working' default stuck until the stale sweeper once the last child left (STA-2915 review).
|
||||
const leadState = lead?.state ?? (rosterHasWorking ? 'working' : 'done')
|
||||
return buildClaudeStatusPayload(state, eventName, '', paneKey, hookPayload, {
|
||||
stateName: leadState === 'done' && rosterHasWorking ? 'working' : leadState,
|
||||
updateToolSnapshot: false,
|
||||
interrupted: lead?.interrupted
|
||||
})
|
||||
// Why: with no lead record only positive evidence may speak. A live child shows working; a
|
||||
// one-shot removal resolves to done — the row is gone, so a delayed duplicate stop finds
|
||||
// nothing and cannot re-claim. Teammate parks and idle-only refreshes claim nothing: a stale
|
||||
// stop from a previous turn parks a persistent id even while its newer turn is live, so
|
||||
// inferring done there is PR #11353's uncorrelated-completion race. Everything else (e.g.
|
||||
// compact's start-less SubagentStop) proves nothing and stays silent (STA-2915).
|
||||
if (rosterHasWorking) {
|
||||
return buildClaudeStatusPayload(state, eventName, '', paneKey, hookPayload, {
|
||||
stateName: 'working',
|
||||
updateToolSnapshot: false
|
||||
})
|
||||
}
|
||||
if (evidence.removedRow) {
|
||||
return buildClaudeStatusPayload(state, eventName, '', paneKey, hookPayload, {
|
||||
stateName: 'done',
|
||||
updateToolSnapshot: false
|
||||
})
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
function normalizeClaudeEvent(
|
||||
|
||||
Reference in New Issue
Block a user