fix(native-chat): correct the Codex subagent roster's build, journal write, and failure reporting

* Restore the exhaustive block handling that adding `subagent-group` to `NativeChatBlock` broke. `formatWorkerTranscriptMessage` and `boundBlock` both fell through to `image-ref` field access, so `tsc -p` failed for the CLI and node projects and `build:cli` could not emit. Both now guard on `image-ref` explicitly and give the roster block its own branch. * Stop the roster's publish from evicting its own append. The sink queue coalesces by `coalescingKey` alone with no op-kind check, so passing the append's key to `tryPublish` spliced the queued append out and the row never reached the journal — permanently, since `lastSerialized` was already set. `tryPublish()` now takes no argument, matching every other call site. The regression test's fake sink honours the key, which the previous fake did not. * Keep `collabAgentToolCall` substantive. Only the MultiAgentV2 path emits `subAgentActivity`, so a V1 turn has no roster row; suppressing its collab tool calls too would have left a V1 fan-out showing nothing at all. * Surface a settled failure while siblings still work. The summary now reports the worst adverse outcome independently of the group verdict, so the row shows `3 working +1 failed` with a failed-coloured dot instead of a neutral pulsing dot. The plain-text twin names it too. * Treat `/morpheus` as a child. Only `/root` is the turn itself; the old segment-count test silently dropped a valid single-segment agent. * Refresh token-usage recency on update so an active thread is not evicted as the oldest entry, and scope the `agentsStates` comment to the V2 path.
2026-09-29 16:02:50 +00:00 · 2026-09-05 01:38:41 -07:00
parent 05e4fa037d
commit c96dd59904
11 changed files with 280 additions and 37 deletions
@@ -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()
 }
@@ -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. */
@@ -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()

@@ -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}`
-}
@@ -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', () => {
@@ -198,11 +198,15 @@ const CODEX_ITEM_CLASSIFICATIONS: Record<string, ProviderFrameClassification> =
  // 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:<type>` 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 {
@@ -96,6 +96,19 @@ function boundBlock(block: NativeChatBlock, warnings: Set<string>): 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 {
@@ -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(
+      <NativeChatSubagentRun
+        block={group([
+          { id: 'a', label: 'read', state: 'working' },
+          { id: 'b', label: 'search', state: 'working' },
+          { id: 'c', label: 'list', state: 'working' },
+          { id: 'd', label: 'edit', state: 'failed' }
+        ])}
+        activeTurnIsWorking
+      />
+    )
+
+    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(
+      <NativeChatSubagentRun
+        block={group([
+          { id: 'a', label: 'read', state: 'working' },
+          { id: 'b', label: 'search', state: 'completed' }
+        ])}
+        activeTurnIsWorking
+      />
+    )
+
+    expect(screen.getByRole('button')).not.toHaveTextContent('failed')
+    expect(container.querySelector('.bg-destructive')).toBeNull()
+  })
+
  it('reconciles a roster persisted before a restart to unverifiable', () => {
    render(
      <NativeChatSubagentRun
@@ -66,7 +66,7 @@ function subagentStateLabel(
        return translate('components.native-chat.subagents.state.failed', 'failed')
      case 'stopped':
        return translate('components.native-chat.subagents.state.stopped', 'stopped')
-      default:
+      case 'unverifiable':
        return translate('components.native-chat.subagents.state.unverifiable', 'unverifiable')
    }
  }
@@ -95,7 +95,7 @@ function subagentStateLabel(
          value0: count
        }
      )
-    default:
+    case 'unverifiable':
      return translate(
        'components.native-chat.subagents.state.unverifiableCount',
        '{{value0}} unverifiable',
@@ -105,7 +105,7 @@ function subagentStateLabel(
 }

 const STATE_DOT_CLASS: Record<NativeChatSubagentState, string> = {
-  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 (
    <span
      aria-hidden="true"
-      className={cn('size-1.5 shrink-0 rounded-full', STATE_DOT_CLASS[state])}
+      className={cn(
+        'size-1.5 shrink-0 rounded-full',
+        STATE_DOT_CLASS[state],
+        pulsing && 'animate-pulse motion-reduce:animate-none'
+      )}
    />
  )
 }
@@ -193,6 +205,10 @@ export function NativeChatSubagentRun({
  const verdict = working
    ? subagentStateLabel('working', summary.working, summary.total)
    : subagentStateLabel(verdictState, summary.settledCount, summary.total)
+  // A child that already failed must not wait for its siblings to be readable.
+  const alertState = working ? summary.adverseState : null
+  const alert =
+    alertState === null ? null : subagentStateLabel(alertState, summary.adverseCount, summary.total)

  return (
    <div>
@@ -204,12 +220,13 @@ export function NativeChatSubagentRun({
        aria-live="polite"
      >
        <SubagentGlyph />
-        <StatusDot state={verdictState} />
+        <StatusDot state={alertState ?? verdictState} pulsing={working} />
        <span className={cn('min-w-0 flex-1 truncate', working && 'text-foreground/85')}>
          {headline}
        </span>
        <span className="shrink-0 font-mono text-[11px] text-muted-foreground">
          {verdict}
+          {alert === null ? null : ` +${alert}`}
          {summary.startedAt !== null ? (
            <>
              {' · '}
@@ -239,7 +256,7 @@ export function NativeChatSubagentRun({
            const state = normalizeSubagentState(agent.state)
            return (
              <li key={agent.id} className="flex items-center gap-1.5 py-0.5">
-                <StatusDot state={state} />
+                <StatusDot state={state} pulsing={state === 'working'} />
                <code
                  className={cn(
                    'min-w-0 truncate font-mono text-[11px]',
@@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'
 import {
  isTerminalSubagentState,
  normalizeSubagentState,
+  subagentGroupFallbackText,
  summarizeSubagentGroup
 } from './native-chat-subagent-summary'
 import type { NativeChatSubagentEntry } from './native-chat-types'
@@ -82,6 +83,39 @@ describe('summarizeSubagentGroup', () => {
    })
  })

+  it('reports an adverse outcome before the group settles', () => {
+    const summary = summarizeSubagentGroup([
+      agent({ id: 'a', state: 'working' }),
+      agent({ id: 'b', state: 'working' }),
+      agent({ id: 'c', state: 'failed' })
+    ])
+
+    // The group verdict is still withheld, but the failure is not.
+    expect(summary).toMatchObject({
+      working: 2,
+      settledState: null,
+      adverseState: 'failed',
+      adverseCount: 1
+    })
+  })
+
+  it('ranks the adverse outcome worst-first and ignores benign settled states', () => {
+    expect(
+      summarizeSubagentGroup([
+        agent({ id: 'a', state: 'working' }),
+        agent({ id: 'b', state: 'stopped' }),
+        agent({ id: 'c', state: 'failed' })
+      ]).adverseState
+    ).toBe('failed')
+    expect(
+      summarizeSubagentGroup([
+        agent({ id: 'a', state: 'working' }),
+        agent({ id: 'b', state: 'idle' }),
+        agent({ id: 'c', state: 'completed' })
+      ]).adverseState
+    ).toBeNull()
+  })
+
  it('keeps working the only non-terminal state', () => {
    expect(isTerminalSubagentState('working')).toBe(false)
    for (const state of ['idle', 'completed', 'failed', 'stopped', 'unverifiable']) {
@@ -89,3 +123,30 @@ describe('summarizeSubagentGroup', () => {
    }
  })
 })
+
+describe('subagentGroupFallbackText', () => {
+  it('names the failure a client without the block type would otherwise never see', () => {
+    expect(
+      subagentGroupFallbackText([
+        agent({ id: 'a', state: 'working' }),
+        agent({ id: 'b', state: 'working' }),
+        agent({ id: 'c', state: 'failed' })
+      ])
+    ).toBe('Kicked off 3 subagents — 2 working (1 failed)')
+    expect(
+      subagentGroupFallbackText([
+        agent({ id: 'a', state: 'completed' }),
+        agent({ id: 'b', state: 'stopped' })
+      ])
+    ).toBe('Ran 2 subagents (1 stopped)')
+  })
+
+  it('stays quiet when nothing has gone wrong', () => {
+    expect(subagentGroupFallbackText([agent({ id: 'a', state: 'working' })])).toBe(
+      'Kicked off 1 subagent — 1 working'
+    )
+    expect(subagentGroupFallbackText([agent({ id: 'a', state: 'completed' })])).toBe(
+      'Ran 1 subagent'
+    )
+  })
+})
@@ -1,9 +1,10 @@
 // One spawn group's roster → the numbers a single flat row needs.
 //
-// Shared because the desktop transcript and the mobile summary path must agree
-// on what "N working" means, and because the producer uses the same terminal
-// predicate the renderer does — a state that reads terminal here must latch
-// terminal there.
+// Shared because the producer and the desktop transcript must agree on what
+// "N working" means: the producer uses the same terminal predicate the renderer
+// does, so a state that reads terminal here latches terminal there. Mobile has
+// no roster renderer — it shows only the write-time-frozen fallback sentence,
+// which is why that sentence is built from this same summary.

 import {
  isSubagentGroupBlock,
@@ -28,6 +29,10 @@ const TERMINAL_SUBAGENT_STATES: ReadonlySet<string> = new Set([
 *  outcome wins, and `completed` only shows when nothing else is left. */
 const SETTLED_PRECEDENCE = ['failed', 'stopped', 'unverifiable', 'idle', 'completed'] as const

+/** Outcomes that must be visible immediately, not held back until the last
+ *  sibling stops working: a fan-out with a dead child is not a neutral row. */
+const ADVERSE_PRECEDENCE = ['failed', 'stopped', 'unverifiable'] as const
+
 /** A state this build does not know reads as `unverifiable`, never as working:
 *  a roster written by a newer build must not leave the row spinning forever. */
 export function normalizeSubagentState(state: string): NativeChatSubagentState {
@@ -48,6 +53,11 @@ export type NativeChatSubagentSummary = {
  settledState: NativeChatSubagentState | null
  /** How many children hold `settledState`. */
  settledCount: number
+  /** Worst adverse outcome already recorded, reported even while siblings still
+   *  work. Null when nothing has gone wrong. */
+  adverseState: NativeChatSubagentState | null
+  /** How many children hold `adverseState`. */
+  adverseCount: number
  /** Sum of the latest per-child totals. Null when no child reported one.
   *  Children's counters are disjoint from the parent's, so this never
   *  double-counts — and the parent's own usage is deliberately excluded. */
@@ -85,11 +95,14 @@ export function summarizeSubagentGroup(
  }
  const settledState =
    working > 0 ? null : (SETTLED_PRECEDENCE.find((state) => counts.has(state)) ?? null)
+  const adverseState = ADVERSE_PRECEDENCE.find((state) => counts.has(state)) ?? null
  return {
    total: agents.length,
    working,
    settledState,
    settledCount: settledState === null ? 0 : (counts.get(settledState) ?? 0),
+    adverseState,
+    adverseCount: adverseState === null ? 0 : (counts.get(adverseState) ?? 0),
    tokens,
    startedAt,
    settledAt: working > 0 ? null : settledAt
@@ -101,3 +114,15 @@ export function subagentGroupBlocks(
 ): NativeChatSubagentGroupBlock[] {
  return blocks.filter(isSubagentGroupBlock)
 }
+
+/** Plain-text stand-in for the roster, frozen into the journal at write time for
+ *  clients without the block type. It carries the adverse count too: a reader
+ *  that only ever sees this sentence must not be told a failing fan-out is fine. */
+export function subagentGroupFallbackText(agents: readonly NativeChatSubagentEntry[]): string {
+  const { total, working, adverseState, adverseCount } = summarizeSubagentGroup(agents)
+  const noun = total === 1 ? 'subagent' : 'subagents'
+  const adverse = adverseState === null ? '' : ` (${adverseCount} ${adverseState})`
+  return working > 0
+    ? `Kicked off ${total} ${noun} — ${working} working${adverse}`
+    : `Ran ${total} ${noun}${adverse}`
+}