diff --git a/src/cli/handlers/orchestration/worker-output.ts b/src/cli/handlers/orchestration/worker-output.ts index 572f33117dd..dc20cb5a6f7 100644 --- a/src/cli/handlers/orchestration/worker-output.ts +++ b/src/cli/handlers/orchestration/worker-output.ts @@ -1,3 +1,4 @@ +import { subagentGroupFallbackText } from '../../../shared/native-chat-subagent-summary' import type { NativeChatMessage } from '../../../shared/native-chat-types' import type { RuntimeTerminalRead } from '../../../shared/runtime-types' import type { OrchestrationWorkerReadResult } from '../../../shared/orchestration-worker-output' @@ -27,7 +28,10 @@ function formatWorkerTranscriptMessage(message: NativeChatMessage): string { if (block.type === 'tool-result') { return `[tool result${block.isError ? ' error' : ''}] ${block.output}` } - return block.url ? `[image] ${block.url}` : `[image omitted]` + if (block.type === 'image-ref') { + return block.url ? `[image] ${block.url}` : `[image omitted]` + } + return `[subagents] ${subagentGroupFallbackText(block.agents)}` }) return `[${message.role}] ${blocks.join('\n')}`.trimEnd() } diff --git a/src/main/codex/codex-subagent-activity.ts b/src/main/codex/codex-subagent-activity.ts index fcddfd189d0..f7c8e9a167e 100644 --- a/src/main/codex/codex-subagent-activity.ts +++ b/src/main/codex/codex-subagent-activity.ts @@ -6,7 +6,8 @@ // * `agentPath` is a tree path (`/root`, `/root/list_directory`); the trailing // segment is a semantic task name and the only label available. There is no // `thread/started` for a child, so nickname/role/depth do not exist. -// * `agentsStates` on `collabAgentToolCall` is ALWAYS `{}`. Nothing here reads it. +// * `agentsStates` on `collabAgentToolCall` is `{}` on the MultiAgentV2 path; +// the V1 path does populate it. Nothing here reads it on either path. // * `thread/tokenUsage/updated` reports a per-thread RUNNING TOTAL, so the // latest frame replaces the previous one — it is never accumulated. @@ -70,18 +71,21 @@ export function codexSubagentPathSegments(agentPath: string | null): string[] { return agentPath === null ? [] : agentPath.split('/').filter((part) => part.length > 0) } +/** The one agent path that is the parent turn itself. Matched literally, as + * Codex does: `/morpheus` is also a single-segment path but IS a child. */ +const CODEX_ROOT_AGENT_PATH = '/root' + /** * Whether an activity item describes the ROOT of the agent tree rather than a - * spawned child. The root's path is a single segment (`/root`); every child - * carries at least one segment beneath it. Counting the root would make the - * parent turn report itself as its own subagent. + * spawned child. Counting the root would make the parent turn report itself as + * its own subagent. * * A path-less item cannot be placed in the tree at all, so it is treated as a * child: dropping it would lose a real spawn, while an extra row is visible and * self-correcting. */ export function isCodexRootAgentActivity(activity: CodexSubagentActivity): boolean { - return codexSubagentPathSegments(activity.agentPath).length === 1 + return activity.agentPath === CODEX_ROOT_AGENT_PATH } /** Row label: the agent path's trailing segment. */ diff --git a/src/main/codex/codex-subagent-roster.test.ts b/src/main/codex/codex-subagent-roster.test.ts index 79988d23567..82c62d30ed7 100644 --- a/src/main/codex/codex-subagent-roster.test.ts +++ b/src/main/codex/codex-subagent-roster.test.ts @@ -78,7 +78,79 @@ function deliver( roster.handleItem({ threadId: THREAD, turnId, item }) } +/** + * A sink that coalesces the way the real queue does: by `coalescingKey` ALONE, + * with no op-kind check, and only draining when released. A fake that ignores + * the key cannot see an append being spliced out by its own publish. + */ +function createCoalescingHarness(): { + roster: CodexSubagentRoster + appended: Appended[] + drain: () => void +} { + const appended: Appended[] = [] + const queue: { key?: string; run: () => void }[] = [] + let clock = 1_000 + const submit = (key: string | undefined, run: () => void): void => { + const at = key === undefined ? -1 : queue.findIndex((queued) => queued.key === key) + if (at >= 0) { + queue.splice(at, 1) + } + queue.push(key === undefined ? { run } : { key, run }) + } + const sink: StructuredAgentSessionEventSink = { + appendItem: () => {}, + appendTombstone: () => {}, + publish: () => {}, + tryAppendItem: (identity, body, options) => { + submit(options?.coalescingKey, () => appended.push({ identity, body })) + return { accepted: true } + }, + tryPublish: (options) => { + submit(options?.coalescingKey ?? 'publish', () => {}) + return { accepted: true } + } + } + const roster = new CodexSubagentRoster({ + sink, + primaryThreadId: () => THREAD, + activeTurn: () => TURN, + now: () => (clock += 1) + }) + return { + roster, + appended, + drain: () => { + while (queue.length > 0) { + queue.shift()?.run() + } + } + } +} + describe('CodexSubagentRoster', () => { + it('does not let its own publish evict the still-queued roster append', () => { + const { roster, appended, drain } = createCoalescingHarness() + + deliver( + roster, + activity({ kind: 'started', agentThreadId: 'child-1', agentPath: '/root/read' }) + ) + drain() + + // Sharing the append's coalescing key with the publish spliced the append + // out of the queue, and `lastSerialized` then suppressed every retry. + expect(appended).toHaveLength(1) + }) + + it('counts a /morpheus agent as a child — only /root is the turn itself', () => { + const { roster, agents } = createHarness() + + deliver(roster, activity({ kind: 'started', agentThreadId: 'child-m', agentPath: '/morpheus' })) + + expect(agents()).toMatchObject([{ id: 'child-m', label: 'morpheus', state: 'working' }]) + }) + it('ignores the root node so a turn is not its own subagent', () => { const { roster, appended } = createHarness() diff --git a/src/main/codex/codex-subagent-roster.ts b/src/main/codex/codex-subagent-roster.ts index 746f4ccb239..efa62d67249 100644 --- a/src/main/codex/codex-subagent-roster.ts +++ b/src/main/codex/codex-subagent-roster.ts @@ -6,12 +6,20 @@ // `item/completed`). Every transition here is therefore idempotent, and a // terminal state latches: duplicate and out-of-order delivery must not resurrect // a settled child. +// +// KNOWN LIMITATION: `groups` is process-local and is never seeded from the +// journal. After a group is evicted, or a reconnect reuses a `threadId:turnId`, +// the next activity item rebuilds the row from scratch — an N-child roster can +// be rewritten down to one child. Seeding from the journal is the real fix. import type { AgentJournalItemBody, AgentJournalItemIdentity } from '../../shared/agent-session-journal-types' -import { isTerminalSubagentState } from '../../shared/native-chat-subagent-summary' +import { + isTerminalSubagentState, + subagentGroupFallbackText +} from '../../shared/native-chat-subagent-summary' import type { NativeChatSubagentEntry } from '../../shared/native-chat-types' import type { StructuredAgentSessionEventSink, @@ -136,6 +144,9 @@ export class CodexSubagentRoster { } // A running total: the newest frame REPLACES the previous one. Summing // updates would multiply a single child's usage by its frame count. + // Re-insert so the eviction scan below sees recency: `set` on an existing + // key keeps its original position, which would age out an active thread. + this.tokensByThread.delete(usage.threadId) this.tokensByThread.set(usage.threadId, usage.totalTokens) while (this.tokensByThread.size > MAX_CODEX_TOKEN_USAGE_THREADS) { const oldest = this.tokensByThread.keys().next().value @@ -245,6 +256,10 @@ export class CodexSubagentRoster { return ADMITTED } group.lastSerialized = serialized + // The append coalesces per group so a burst collapses to the latest roster. + // The publish must NOT reuse that key: the queue coalesces by key alone, + // with no op-kind check, so a publish carrying it would splice out the + // still-queued append and the row would never reach the journal. const options = { coalescingKey: `codex-subagents:${group.groupId}` } const admission = this.deps.sink.tryAppendItem ? this.deps.sink.tryAppendItem(group.identity, body, options) @@ -254,8 +269,8 @@ export class CodexSubagentRoster { return admission } return this.deps.sink.tryPublish - ? this.deps.sink.tryPublish(options) - : (this.deps.sink.publish(options), ADMITTED) + ? this.deps.sink.tryPublish() + : (this.deps.sink.publish(), ADMITTED) } } @@ -275,12 +290,3 @@ export function codexSubagentGroupBody( ] } } - -/** Plain-text stand-in for the roster, for clients without the block type. */ -export function subagentGroupFallbackText(agents: readonly NativeChatSubagentEntry[]): string { - const working = agents.filter((agent) => !isTerminalSubagentState(agent.state)).length - const noun = agents.length === 1 ? 'subagent' : 'subagents' - return working > 0 - ? `Kicked off ${agents.length} ${noun} — ${working} working` - : `Ran ${agents.length} ${noun}` -} diff --git a/src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts b/src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts index 76216411697..a7757f2ff54 100644 --- a/src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts +++ b/src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts @@ -125,24 +125,26 @@ describe('codex subagent item disposition', () => { agentPath: '/root/read' }) ).toBe('status-chrome') + }) + + it("leaves collab tool calls substantive — they are a V1 turn's only subagent signal", () => { + // Only the MultiAgentV2 path emits `subAgentActivity`, so the roster row + // never exists on V1. Suppressing this too would render a V1 fan-out blank. expect( classifyProviderFrame('codex', 'item:collabAgentToolCall', { type: 'collabAgentToolCall', agentsStates: {} }) - ).toBe('status-chrome') + ).not.toBe('status-chrome') }) - it('journals no fallback row for either type', () => { + it('journals no fallback row for subagent activity', () => { expect( unhandledProviderFrameJournalItem('codex', 'item:subAgentActivity', { kind: 'completed', agentThreadId: 'child-1' }) ).toBeNull() - expect( - unhandledProviderFrameJournalItem('codex', 'item:collabAgentToolCall', { agentsStates: {} }) - ).toBeNull() }) it('still surfaces a subagent frame that reports a failure', () => { diff --git a/src/main/native-chat/agent-session-wire/provider-frame-disposition.ts b/src/main/native-chat/agent-session-wire/provider-frame-disposition.ts index 5d784568250..6374dd6d881 100644 --- a/src/main/native-chat/agent-session-wire/provider-frame-disposition.ts +++ b/src/main/native-chat/agent-session-wire/provider-frame-disposition.ts @@ -198,11 +198,15 @@ const CODEX_ITEM_CLASSIFICATIONS: Record = // The `thread/compacted` notification is already chrome; its item form is the // same event and must not read as a mysterious opcode row. contextCompaction: 'status-chrome', - // Subagent lifecycle renders as the spawn-group roster row. Leaving these + // Subagent lifecycle renders as the spawn-group roster row. Leaving it // substantive prints a gray `codex · item:` row beside it for every // event — and every one of them arrives twice. - subAgentActivity: 'status-chrome', - collabAgentToolCall: 'status-chrome' + // + // `collabAgentToolCall` is deliberately NOT suppressed with it. Only the + // MultiAgentV2 path emits `subAgentActivity`; a V1 turn emits collab tool + // calls and nothing else, so suppressing them would leave a V1 fan-out + // showing nothing at all. + subAgentActivity: 'status-chrome' } function notificationKind(kind: string): string { diff --git a/src/main/runtime/orchestration/worker-transcript-payload.ts b/src/main/runtime/orchestration/worker-transcript-payload.ts index e4a5c0b3a58..5718fbc5567 100644 --- a/src/main/runtime/orchestration/worker-transcript-payload.ts +++ b/src/main/runtime/orchestration/worker-transcript-payload.ts @@ -96,6 +96,19 @@ function boundBlock(block: NativeChatBlock, warnings: Set): NativeChatBl input: boundToolInput(block.input, budget, 0, warnings) } } + if (block.type === 'subagent-group') { + // Labels come from provider-supplied agent paths, so they get the same + // redaction and clipping every other piece of transcript metadata gets. + return { + ...block, + groupId: clipMetadata(block.groupId, warnings), + agents: block.agents.map((agent) => ({ + ...agent, + id: clipMetadata(agent.id, warnings), + label: clipMetadata(agent.label, warnings) + })) + } + } if (block.path || (block.url && isLocalFileLocator(block.url))) { warnings.add('Local image paths were omitted from transcript output.') return { diff --git a/src/renderer/src/components/native-chat/NativeChatSubagentRun.test.tsx b/src/renderer/src/components/native-chat/NativeChatSubagentRun.test.tsx index 79ab1f6742e..a8ee64012d8 100644 --- a/src/renderer/src/components/native-chat/NativeChatSubagentRun.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatSubagentRun.test.tsx @@ -65,6 +65,41 @@ describe('NativeChatSubagentRun', () => { expect(screen.getByRole('button')).toHaveTextContent('2 failed') }) + it('surfaces a failed child while its siblings still work', () => { + const { container } = render( + + ) + + const row = screen.getByRole('button') + expect(row).toHaveTextContent('3 working') + expect(row).toHaveTextContent('+1 failed') + // The dot carries the failure; the pulse still says the group is in flight. + expect(container.querySelector('.bg-destructive.animate-pulse')).not.toBeNull() + }) + + it('leaves the dot neutral when nothing has gone wrong', () => { + const { container } = render( + + ) + + expect(screen.getByRole('button')).not.toHaveTextContent('failed') + expect(container.querySelector('.bg-destructive')).toBeNull() + }) + it('reconciles a roster persisted before a restart to unverifiable', () => { render( = { - working: 'bg-foreground/70 animate-pulse motion-reduce:animate-none', + working: 'bg-foreground/70', idle: 'bg-muted-foreground/40', completed: 'bg-muted-foreground/60', failed: 'bg-destructive', @@ -130,11 +130,23 @@ function SubagentGlyph(): React.JSX.Element { ) } -function StatusDot({ state }: { state: NativeChatSubagentState }): React.JSX.Element { +/** `pulsing` is separate from `state` so a group that is still working can show + * a failed sibling's colour without losing its in-flight cue. */ +function StatusDot({ + state, + pulsing = false +}: { + state: NativeChatSubagentState + pulsing?: boolean +}): React.JSX.Element { return (