mirror of
https://github.com/stablyai/orca.git
synced 2026-10-01 08:01:56 +00:00
fix(native-chat): stop an unrelated turn end settling an outside-turn child
`settleTurn` swept the `outside-turn` group on every turn end, so a child Claude announced while no turn was live — a frame trailing the previous turn's result, or one that arrives before the first turn starts — was marked `unverifiable` by the next, unrelated turn ending. That state is terminal and latches, so the `task_updated: completed` that followed was discarded: loss of contact was recorded as the child's outcome on evidence that was never about that child. A turn end now sweeps exactly the group its key names. `outside-turn` belongs to no turn, so only an end with no key of its own reaches it, and what no turn end reaches `settleSession` does — reliably, since teardown without an `ended` event also routes through it. The cost is a child outside every turn showing `working` a little longer; the alternative prints a wrong outcome that nothing can revise. Also pins that a subagent announced after a task the filter rejected still rosters: the announcement path was never what the child-traffic gate closes.
This commit is contained in:
@@ -27,12 +27,7 @@ function harness() {
|
||||
const translator = createClaudeJournalTranslator({ sink, fallbackIdPrefix: 'test' })
|
||||
const groupRows = () =>
|
||||
items.filter((item) => orcaClientMessageId(item.identity) === GROUP_ITEM_ID)
|
||||
/** The last roster row written for one turn, so a test can read a turn that is
|
||||
* no longer the live one. */
|
||||
const rosterOf = (turnUuid: string): NativeChatSubagentEntry[] => {
|
||||
const body = items.findLast(
|
||||
(item) => orcaClientMessageId(item.identity) === `claude-subagents:claude-session:${turnUuid}`
|
||||
)?.body
|
||||
const agentsOf = (body: AgentJournalItemBody | undefined): NativeChatSubagentEntry[] => {
|
||||
if (!body || body.kind !== 'message') {
|
||||
return []
|
||||
}
|
||||
@@ -41,21 +36,21 @@ function harness() {
|
||||
)
|
||||
return block ? block.agents : []
|
||||
}
|
||||
const roster = (): NativeChatSubagentEntry[] => {
|
||||
const body = groupRows().at(-1)?.body
|
||||
if (!body || body.kind !== 'message') {
|
||||
return []
|
||||
}
|
||||
const block = body.blocks.find(
|
||||
(candidate): candidate is NativeChatSubagentGroupBlock => candidate.type === 'subagent-group'
|
||||
/** The last roster row written for one group, so a test can read a group that
|
||||
* is no longer the live one. */
|
||||
const rosterIn = (groupId: string): NativeChatSubagentEntry[] =>
|
||||
agentsOf(
|
||||
items.findLast((item) => orcaClientMessageId(item.identity) === `claude-subagents:${groupId}`)
|
||||
?.body
|
||||
)
|
||||
return block ? block.agents : []
|
||||
}
|
||||
const rosterOf = (turnUuid: string): NativeChatSubagentEntry[] =>
|
||||
rosterIn(`claude-session:${turnUuid}`)
|
||||
const roster = (): NativeChatSubagentEntry[] => agentsOf(groupRows().at(-1)?.body)
|
||||
const fallbackRows = (): AgentJournalItemBody[] =>
|
||||
items
|
||||
.filter((item) => (orcaClientMessageId(item.identity) ?? '').startsWith('provider-frame:'))
|
||||
.map((item) => item.body)
|
||||
return { translator, groupRows, roster, rosterOf, fallbackRows }
|
||||
return { translator, groupRows, roster, rosterIn, rosterOf, fallbackRows }
|
||||
}
|
||||
|
||||
function userTurn(uuid: string) {
|
||||
@@ -226,4 +221,39 @@ describe('claude journal translation — subagents', () => {
|
||||
expect(rosterOf('user-1')).toEqual([expect.objectContaining({ state: 'unverifiable' })])
|
||||
expect(rosterOf('user-2')).toEqual([expect.objectContaining({ state: 'working' })])
|
||||
})
|
||||
|
||||
it('does not let an unrelated turn end settle a child announced outside a turn', () => {
|
||||
const { translator, rosterIn } = harness()
|
||||
// No turn is live yet, so this child has no turn key to belong to.
|
||||
translator.handle(
|
||||
systemFrame('task_started', {
|
||||
task_id: 'task-early',
|
||||
task_type: 'local_agent',
|
||||
description: 'Before the turn'
|
||||
})
|
||||
)
|
||||
translator.handle(userTurn('user-1'))
|
||||
translator.handle(resultFrame())
|
||||
expect(rosterIn('outside-turn')).toEqual([expect.objectContaining({ state: 'working' })])
|
||||
// The outcome still lands, which a latched `unverifiable` would have lost.
|
||||
translator.handle(
|
||||
systemFrame('task_updated', { task_id: 'task-early', patch: { status: 'completed' } })
|
||||
)
|
||||
expect(rosterIn('outside-turn')).toEqual([expect.objectContaining({ state: 'completed' })])
|
||||
})
|
||||
|
||||
it('settles a child left outside every turn when the session ends', () => {
|
||||
const { translator, rosterIn } = harness()
|
||||
translator.handle(
|
||||
systemFrame('task_started', {
|
||||
task_id: 'task-early',
|
||||
task_type: 'local_agent',
|
||||
description: 'Before the turn'
|
||||
})
|
||||
)
|
||||
translator.handle(userTurn('user-1'))
|
||||
translator.handle(resultFrame())
|
||||
translator.handle({ type: 'ended', sessionId: 'orca-session', reason: 'closed' })
|
||||
expect(rosterIn('outside-turn')).toEqual([expect.objectContaining({ state: 'unverifiable' })])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -296,6 +296,21 @@ describe('ClaudeSubagentRoster', () => {
|
||||
expect(items).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('still rosters a subagent announced after a task the filter rejected', () => {
|
||||
const { roster, roles } = harness()
|
||||
roster.observeSystemFrame(
|
||||
system('task_started', { task_id: 'task-bash', task_type: 'local_bash' })
|
||||
)
|
||||
// The gate closes the child-traffic fallback, never the announcement path.
|
||||
roster.observeSystemFrame(
|
||||
started({ task_id: 'task-1', tool_use_id: 'toolu_1', description: 'Explore' })
|
||||
)
|
||||
roster.observeChildActivity('toolu_1')
|
||||
expect(roles()).toEqual([
|
||||
expect.objectContaining({ id: 'task-1', label: 'Explore', state: 'working' })
|
||||
])
|
||||
})
|
||||
|
||||
it('leaves a grandchild parented inside the sidechain out of the roster', () => {
|
||||
const { roster, roles } = harness()
|
||||
roster.observeSystemFrame(
|
||||
@@ -362,14 +377,36 @@ describe('ClaudeSubagentRoster', () => {
|
||||
})
|
||||
|
||||
describe('ClaudeSubagentRoster — the turn that is ending', () => {
|
||||
it('sweeps children rostered before any turn key existed', () => {
|
||||
it('leaves a child announced outside any turn alone when an unrelated turn ends', () => {
|
||||
const { roster, rolesIn, setGroupKey } = harness(null)
|
||||
roster.observeSystemFrame(started({ task_id: 'task-early', description: 'Early' }))
|
||||
setGroupKey(TURN_1)
|
||||
roster.observeSystemFrame(started({ task_id: 'task-turn', description: 'In turn' }))
|
||||
roster.settleTurn(TURN_1)
|
||||
expect(rolesIn('outside-turn')).toEqual([expect.objectContaining({ state: 'unverifiable' })])
|
||||
expect(rolesIn('outside-turn')).toEqual([expect.objectContaining({ state: 'working' })])
|
||||
expect(rolesIn(TURN_1)).toEqual([expect.objectContaining({ state: 'unverifiable' })])
|
||||
// `unverifiable` latches, so sweeping it above would have swallowed this.
|
||||
roster.observeSystemFrame(
|
||||
system('task_updated', { task_id: 'task-early', patch: { status: 'completed' } })
|
||||
)
|
||||
expect(rolesIn('outside-turn')).toEqual([expect.objectContaining({ state: 'completed' })])
|
||||
})
|
||||
|
||||
it('sweeps the outside-turn group when a turn with no key of its own ends', () => {
|
||||
const { roster, rolesIn } = harness(null)
|
||||
roster.observeSystemFrame(started({ task_id: 'task-early', description: 'Early' }))
|
||||
roster.settleTurn(null)
|
||||
expect(rolesIn('outside-turn')).toEqual([expect.objectContaining({ state: 'unverifiable' })])
|
||||
})
|
||||
|
||||
it('still settles an outside-turn child once the session itself ends', () => {
|
||||
const { roster, rolesIn, setGroupKey } = harness(null)
|
||||
roster.observeSystemFrame(started({ task_id: 'task-early', description: 'Early' }))
|
||||
setGroupKey(TURN_1)
|
||||
roster.observeSystemFrame(started({ task_id: 'task-turn', description: 'In turn' }))
|
||||
roster.settleTurn(TURN_1)
|
||||
roster.settleSession()
|
||||
expect(rolesIn('outside-turn')).toEqual([expect.objectContaining({ state: 'unverifiable' })])
|
||||
})
|
||||
|
||||
it('sweeps the turn that ended, not whichever turn is live now', () => {
|
||||
|
||||
@@ -46,7 +46,9 @@ type RosterGroup = {
|
||||
/** Insertion order is the display order; the map holds the state. */
|
||||
entries: Map<string, TrackedEntry>
|
||||
/** High-water mark of claims per label, so a repeat gets an ordinal suffix.
|
||||
* It never decreases: re-issuing an ordinal would print two identical rows. */
|
||||
* It never decreases: re-issuing an ordinal would print two identical rows.
|
||||
* Growing it past the entry cap takes a stream that re-announces a rostered
|
||||
* agent as a shell task, which churns the row far harder than the map. */
|
||||
labelCounts: Map<string, number>
|
||||
/** Last body written, so an idempotent replay writes no new revision. */
|
||||
lastSerialized: string | null
|
||||
@@ -166,13 +168,11 @@ export class ClaudeSubagentRoster {
|
||||
* explicitly told to outlive the turn and is left alone.
|
||||
*/
|
||||
settleTurn(groupKey: string | null): void {
|
||||
const ending = groupKey ?? OUTSIDE_TURN
|
||||
this.sweep(this.groups.get(ending), false)
|
||||
if (ending !== OUTSIDE_TURN) {
|
||||
// Children Claude reported before any turn key existed sit here, and no
|
||||
// turn will ever name this group, so every turn end sweeps it too.
|
||||
this.sweep(this.groups.get(OUTSIDE_TURN), false)
|
||||
}
|
||||
// Only the group this key names. `OUTSIDE_TURN` belongs to no turn, so an
|
||||
// unrelated turn ending is no evidence about a child announced outside it —
|
||||
// and `unverifiable` latches, so sweeping it there would swallow the
|
||||
// `completed` that still arrives. `settleSession` reaches what no turn does.
|
||||
this.sweep(this.groups.get(groupKey ?? OUTSIDE_TURN), false)
|
||||
}
|
||||
|
||||
/** The provider is gone. Nothing more will arrive for any child, backgrounded
|
||||
|
||||
Reference in New Issue
Block a user