diff --git a/src/main/claude/claude-forwarded-tool-registry.ts b/src/main/claude/claude-forwarded-tool-registry.ts deleted file mode 100644 index 6e376b13bdf..00000000000 --- a/src/main/claude/claude-forwarded-tool-registry.ts +++ /dev/null @@ -1,43 +0,0 @@ -// Which Claude tool calls this session actually forwarded to the top-level -// transcript. -// -// A task announces the tool call that spawned it. That tool call is only -// evidence the user can act on when it was forwarded at the TOP level: a nested -// Task spawned from inside a subagent's sidechain names a tool id that exists -// only in that sidechain, and a row minted for it would claim a top-level -// invocation that never appeared. So admission asks this registry, and a task -// whose parent was never forwarded yields no row at all. - -/** Event-accumulated and pruned by nothing, so bounded. Eviction is oldest - * first: a tool id old enough to fall out can no longer be the parent of a - * task announcement still in flight. */ -const MAX_FORWARDED_TOOL_IDS = 512 - -export class ClaudeForwardedToolRegistry { - private readonly ids = new Set() - - /** Record a tool call forwarded at the top level. Nested traffic must not - * reach here — its caller checks `parent_tool_use_id` first. */ - record(toolUseId: string): void { - if (toolUseId.length === 0) { - return - } - this.ids.delete(toolUseId) - this.ids.add(toolUseId) - while (this.ids.size > MAX_FORWARDED_TOOL_IDS) { - const oldest = this.ids.values().next() - if (oldest.done || oldest.value === toolUseId) { - break - } - this.ids.delete(oldest.value) - } - } - - has(toolUseId: string): boolean { - return this.ids.has(toolUseId) - } - - clear(): void { - this.ids.clear() - } -} diff --git a/src/main/claude/claude-message-journaling.ts b/src/main/claude/claude-message-journaling.ts index 5a278f55210..9d75db1fca9 100644 --- a/src/main/claude/claude-message-journaling.ts +++ b/src/main/claude/claude-message-journaling.ts @@ -7,13 +7,14 @@ // writes through stay owned by the translator. import { agentJournalItemKey } from '../../shared/agent-session-journal-item-key' +import type { AgentJournalItemBody } from '../../shared/agent-session-journal-types' import { boundInlineText, DEFAULT_JOURNAL_PAYLOAD_LIMITS } from '../native-chat/agent-session-journal/journal-payload-bounds' import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' import type { ClaudeBackgroundTaskRows } from './claude-background-task-rows' -import type { ClaudeForwardedToolRegistry } from './claude-forwarded-tool-registry' +import type { ClaudeToolOriginRegistry } from './claude-tool-origin-registry' import { claudeRecord, claudeMessageBody, @@ -34,6 +35,7 @@ import { } from './claude-structured-provider-fallback' import type { createClaudeStreamedBlockRegistry } from './claude-streamed-block-identity' import type { createClaudeStreamedTextCheckpoints } from './claude-streamed-text-checkpoints' +import type { ClaudeProvisionalRowCorrections } from './claude-provisional-row-corrections' import type { ClaudeSubagentRoster } from './claude-subagent-roster' import { claudeTurnOpenedBySendEcho, type ClaudeTurnSource } from './claude-turn-opening' import type { ClaudeOpenTurn } from './claude-open-turn' @@ -44,9 +46,12 @@ export type ClaudeMessageJournalContext = { streamedBlocks: ReturnType streamedText: ReturnType subagents: ClaudeSubagentRoster - forwardedTools: ClaudeForwardedToolRegistry + toolOrigins: ClaudeToolOriginRegistry backgroundTasks: ClaudeBackgroundTaskRows providerFallback: ClaudeProviderFrameFallback + /** Attributes every row this module writes to the agent that produced it, and + * remembers the ones stamped before that agent had a final identity. */ + corrections: ClaudeProvisionalRowCorrections /** The session's open turn. Sole owner of turn identity and of the reopen * latch; this module asks it rather than tracking a copy. */ turn: ClaudeOpenTurn @@ -68,6 +73,27 @@ export function journalClaudeMessage( if (envelope.parentToolUseId) { ctx.subagents.observeChildActivity(envelope.parentToolUseId) } + const results = claudeToolResults(envelope) + // Everything this envelope journals belongs to whoever produced the envelope. + // A child's rows live in the parent's journal, so without this the parent's + // own "what am I doing" readers report the child's newest output as their own. + // + // Delivering the result of the very call it names as parent is the exception: + // that is the CALLER consuming its own tool output, not a child's row. Only a + // spawn call ever gets a sidechain, so reading the field literally here would + // park every ordinary tool result against an announcement never coming. + // + // The caller is not always the session's own agent. A call a child made is + // owned by that child, and its result is the child's row too — collapsing it + // to root would both misattribute it and make the result's write resolve + // through a different reference than the call's, stranding the correction + // owed to that row on the body it had before the result landed. + const producedByCaller = results.some((result) => result.toolUseId === envelope.parentToolUseId) + const producerRef = + producedByCaller && envelope.parentToolUseId !== null + ? ctx.toolOrigins.childOwnerRef(envelope.parentToolUseId) + : envelope.parentToolUseId + const stamp = ctx.corrections.stampFor(producerRef) const outputEnvelope = claudeOutputEnvelope(envelope) const body = claudeMessageBody(outputEnvelope) const identity = @@ -86,37 +112,41 @@ export function journalClaudeMessage( // output; a reader that scans back to the turn record and stops would // otherwise look straight past the row that opened it. ctx.turn.ensureOpen(message, source, observedAt) - ctx.sink.appendItem(identity, body) + ctx.sink.appendItem(identity, body, stamp(identity, body)) changed = true } for (const tool of claudeToolUses(outputEnvelope)) { ctx.turn.ensureOpen(message, source, observedAt) ctx.tools.set(tool.id, tool) // Only a TOP-LEVEL call can be the parent of a top-level task row; a - // sidechain's own tool ids never reach the transcript. - if (!envelope.parentToolUseId) { - ctx.forwardedTools.record(tool.id) + // sidechain's own tool ids never reach the transcript. Those are recorded + // against their owner instead: a grandchild's frames name one of them and + // nothing else, so this is the only place its parent is ever knowable. + if (envelope.parentToolUseId) { + ctx.toolOrigins.recordChildOwned(tool.id, envelope.parentToolUseId) + } else { + ctx.toolOrigins.recordTopLevel(tool.id) } - ctx.sink.appendItem(claudeToolIdentity(envelope.sessionId, tool.id), claudeToolBody({ tool })) + const toolIdentity = claudeToolIdentity(envelope.sessionId, tool.id) + const toolBody = claudeToolBody({ tool }) + ctx.sink.appendItem(toolIdentity, toolBody, stamp(toolIdentity, toolBody)) changed = true } - const results = claudeToolResults(envelope) for (const result of results) { const tool = ctx.tools.get(result.toolUseId) ?? { id: result.toolUseId, name: 'tool', input: null } - ctx.sink.appendItem( - claudeToolIdentity(envelope.sessionId, result.toolUseId), - claudeToolBody({ tool, result }) - ) + const resultIdentity = claudeToolIdentity(envelope.sessionId, result.toolUseId) + const resultBody = claudeToolBody({ tool, result }) + ctx.sink.appendItem(resultIdentity, resultBody, stamp(resultIdentity, resultBody)) ctx.subagents.observeToolResult(result.toolUseId, result.failed) if ( results.length === 1 && envelope.parentToolUseId === null && tool.name === 'Monitor' && - ctx.forwardedTools.has(result.toolUseId) + ctx.toolOrigins.has(result.toolUseId) ) { ctx.backgroundTasks.observeMonitorToolResult(claudeRecord(message.tool_use_result)?.taskId) } @@ -125,17 +155,20 @@ export function journalClaudeMessage( } if (thinking) { ctx.turn.ensureOpen(message, source, observedAt) - ctx.sink.appendItem(claudeThinkingIdentity(envelope.sessionId, envelope.uuid), { + const thinkingIdentity = claudeThinkingIdentity(envelope.sessionId, envelope.uuid) + const thinkingBody: AgentJournalItemBody = { kind: 'message', role: 'reasoning', blocks: [ { type: 'text', text: boundInlineText(thinking, DEFAULT_JOURNAL_PAYLOAD_LIMITS).text } ] - }) + } + ctx.sink.appendItem(thinkingIdentity, thinkingBody, stamp(thinkingIdentity, thinkingBody)) changed = true } changed = - appendUnmodeledContent(ctx.providerFallback, outputEnvelope, message, openOutputTurn) || changed + appendUnmodeledContent(ctx.providerFallback, outputEnvelope, message, openOutputTurn, stamp) || + changed // The send's turn is anchored to the user row journaled just above it. const sendEchoTurn = claudeTurnOpenedBySendEcho({ envelope, diff --git a/src/main/claude/claude-open-turn.ts b/src/main/claude/claude-open-turn.ts index 6e9d4235db6..abcbc065dcb 100644 --- a/src/main/claude/claude-open-turn.ts +++ b/src/main/claude/claude-open-turn.ts @@ -98,6 +98,8 @@ export class ClaudeOpenTurn { this.reopenSuppressed ||= failed } + /** Deliberately root: a turn is the SESSION'S unit of work, and this lane only + * ever opens turns for the session's own agent. A child runs inside one. */ private publish(turn: ClaudeCurrentTurn, end?: ClaudeTurnEnd): void { const item = claudeTurnLifecycleItem(turn, end) this.deps.sink.appendItem(item.identity, item.body, item.options) diff --git a/src/main/claude/claude-provisional-row-corrections.test.ts b/src/main/claude/claude-provisional-row-corrections.test.ts new file mode 100644 index 00000000000..d18a71a17fe --- /dev/null +++ b/src/main/claude/claude-provisional-row-corrections.test.ts @@ -0,0 +1,208 @@ +import { describe, expect, it } from 'vitest' +import type { + AgentJournalItemBody, + AgentJournalItemIdentity +} from '../../shared/agent-session-journal-types' +import type { StructuredAgentSessionAppendOptions } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' +import { ClaudeProvisionalRowCorrections } from './claude-provisional-row-corrections' +import type { ClaudeSubagentLinkageVerdict } from './claude-subagent-linkage' + +function identityOf(toolUseId: string): AgentJournalItemIdentity { + return { provider: 'orca', clientMessageId: `claude-tool:claude-session:${toolUseId}` } +} + +const RUNNING: AgentJournalItemBody = { + kind: 'tool-call', + callId: 'toolu_2', + name: 'Task', + input: null, + state: 'running' +} +const COMPLETED: AgentJournalItemBody = { ...RUNNING, state: 'completed' } + +/** A roster whose verdict a test moves, the way an announcement landing does. */ +function ledger(initial: Record = {}) { + const verdicts = new Map(Object.entries(initial)) + const rewrites: { + identity: AgentJournalItemIdentity + body: AgentJournalItemBody + options: StructuredAgentSessionAppendOptions + }[] = [] + let published = 0 + /** Stands in for a sink refusing the write under backpressure. */ + let refuse = false + const settledFor = (ref: string): ClaudeSubagentLinkageVerdict => { + const verdict = verdicts.get(ref) + return verdict && verdict.kind !== 'pending' + ? verdict + : { kind: 'linked', linkage: { agentId: ref, providerParentRef: ref, producerKind: 'agent' } } + } + const corrections = new ClaudeProvisionalRowCorrections({ + linkageFor: (ref) => verdicts.get(ref) ?? { kind: 'pending' }, + settledLinkageFor: (ref) => { + const verdict = settledFor(ref) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: `settledFor` returns only the linked arm, never `pending`. + return verdict as Exclude + }, + rewrite: (identity, body, options) => { + if (refuse) { + return false + } + rewrites.push({ identity, body, options }) + return true + }, + publish: () => { + published += 1 + } + }) + return { + corrections, + rewrites, + announce: (ref: string, agentId: string) => + verdicts.set(ref, { + kind: 'linked', + linkage: { agentId, providerParentRef: ref, producerKind: 'agent' } + }), + publishes: () => published, + setRefusing: (value: boolean) => { + refuse = value + } + } +} + +describe('ClaudeProvisionalRowCorrections', () => { + it("stamps the session's own rows with nothing and owes them nothing", () => { + const { corrections, rewrites } = ledger() + expect(corrections.stampFor(null)(identityOf('toolu_2'), RUNNING)).toEqual({}) + expect(corrections.pending).toBe(0) + corrections.retry() + expect(rewrites).toEqual([]) + }) + + it('stamps a provisional row at once and re-attributes it on the announcement', () => { + const { corrections, rewrites, announce } = ledger() + const stamped = corrections.stampFor('toolu_1')(identityOf('toolu_2'), RUNNING) + + // Written immediately under the handle that exists, never withheld. + expect(stamped).toMatchObject({ agentId: 'toolu_1' }) + expect(corrections.pending).toBe(1) + + announce('toolu_1', 'task-1') + corrections.retry() + + expect(rewrites).toEqual([ + { + identity: identityOf('toolu_2'), + body: RUNNING, + options: expect.objectContaining({ agentId: 'task-1' }) + } + ]) + expect(corrections.pending).toBe(0) + }) + + it('drops a correction that would change nothing rather than burning a revision', () => { + const { corrections, rewrites, publishes } = ledger() + corrections.stampFor('toolu_1')(identityOf('toolu_2'), RUNNING) + + // Nothing ever names it, so the settled verdict equals the stamp it has. + corrections.abandon() + + expect(rewrites).toEqual([]) + expect(publishes()).toBe(0) + }) + + it('lets a settled write supersede the correction owed to that row', () => { + // One `itemId` can be written under two references — a tool call and its + // result share one. A correction owed to the first must not outlive the + // second, or it restamps the row with the body it had before. + const { corrections, rewrites, announce } = ledger() + corrections.stampFor('toolu_1')(identityOf('toolu_2'), RUNNING) + expect(corrections.pending).toBe(1) + + announce('toolu_other', 'task-other') + corrections.stampFor('toolu_other')(identityOf('toolu_2'), COMPLETED) + + expect(corrections.pending).toBe(0) + announce('toolu_1', 'task-1') + corrections.retry() + expect(rewrites).toEqual([]) + }) + + it('keeps a correction owed when the sink refuses it, and retries at abandon', () => { + // Backpressure refuses the write. Deleting the entry anyway would leave a + // durable obligation with nothing re-deriving it — the row would keep the + // provisional id and no later pass would ever revisit it. + const { corrections, rewrites, announce, setRefusing } = ledger() + corrections.stampFor('toolu_1')(identityOf('toolu_2'), RUNNING) + announce('toolu_1', 'task-1') + + setRefusing(true) + corrections.retry() + expect(rewrites).toEqual([]) + expect(corrections.pending).toBe(1) + + setRefusing(false) + corrections.retry() + expect(rewrites).toHaveLength(1) + expect(corrections.pending).toBe(0) + }) + + it('lets a refusal at abandon end the obligation rather than leaking it', () => { + // The last attempt. A correction that cannot be written has to die here: + // an obligation with no exit is worse than a row keeping a usable id. + const { corrections, rewrites, announce, setRefusing } = ledger() + corrections.stampFor('toolu_1')(identityOf('toolu_2'), RUNNING) + announce('toolu_1', 'task-1') + + setRefusing(true) + corrections.abandon() + + expect(rewrites).toEqual([]) + expect(corrections.pending).toBe(0) + }) + + it('carries the NEWEST body when a row is written provisionally twice', () => { + const { corrections, rewrites, announce } = ledger() + const stamp = corrections.stampFor('toolu_1') + stamp(identityOf('toolu_2'), RUNNING) + stamp(identityOf('toolu_2'), COMPLETED) + + announce('toolu_1', 'task-1') + corrections.retry() + + expect(rewrites).toHaveLength(1) + expect(rewrites[0]?.body).toEqual(COMPLETED) + }) + + it('gives up on a producer WHOLESALE past the bound, never half of it', () => { + // A partial correction splits one child across two ids in one session, which + // is worse than correcting none: the stragglers are what a reader would have + // to reconcile. Past the bound every row keeps the spawn call's own id. + const { corrections, rewrites, announce } = ledger() + const stamp = corrections.stampFor('toolu_1') + for (let index = 0; index < 129; index += 1) { + stamp(identityOf(`toolu_row_${index}`), RUNNING) + } + expect(corrections.pending).toBe(0) + + announce('toolu_1', 'task-1') + corrections.retry() + + expect(rewrites).toEqual([]) + }) + + it('keeps correcting a producer that stays inside the bound', () => { + // The positive control for the case above: giving up must be the exception. + const { corrections, rewrites, announce } = ledger() + const stamp = corrections.stampFor('toolu_1') + for (let index = 0; index < 128; index += 1) { + stamp(identityOf(`toolu_row_${index}`), RUNNING) + } + expect(corrections.pending).toBe(128) + + announce('toolu_1', 'task-1') + corrections.retry() + + expect(rewrites).toHaveLength(128) + }) +}) diff --git a/src/main/claude/claude-provisional-row-corrections.ts b/src/main/claude/claude-provisional-row-corrections.ts new file mode 100644 index 00000000000..a4b7d039902 --- /dev/null +++ b/src/main/claude/claude-provisional-row-corrections.ts @@ -0,0 +1,246 @@ +// Rows written before their producer had a final identity, and the correction +// owed to each. +// +// A subagent's first frames can arrive before the `task_started` that names it. +// The row is written anyway, stamped with the only handle that exists yet — the +// spawn call's own id. It is not held: bookkeeping must never gate a user's +// view of what an agent said, and a row withheld for an announcement that never +// comes is output the user never sees. +// +// The stamp is then corrected in place. Re-appending the same `itemId` bumps +// its revision and the reducer rebuilds the row's linkage from the newest one, +// pinning `sequence` and `observedAt` so the correction refreshes attribution +// without moving the bubble. That is the same mechanism the streamed-text +// checkpoints and the subagent group row already use. +// +// What this must never do is make a row WORSE. Every exit either writes a +// strictly better stamp or drops the correction untouched; losing one costs a +// row the canonical id, never its content and never its author. + +import { agentJournalLinkageFields } from '../../shared/agent-session-journal-producer' +import { agentJournalItemKey } from '../../shared/agent-session-journal-item-key' +import type { + AgentJournalItemBody, + AgentJournalItemIdentity +} from '../../shared/agent-session-journal-types' +import type { StructuredAgentSessionAppendOptions } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' +import type { ClaudeSubagentLinkageSource } from './claude-subagent-linkage' + +/** + * Stamps one row with its producer, and remembers it when that producer's + * identity is still provisional. + * + * The row is always written by the caller, immediately, with whatever this + * returns. No site chooses — the envelope it came from already decided. + */ +export type ClaudeRowStamp = ( + identity: AgentJournalItemIdentity, + body: AgentJournalItemBody +) => StructuredAgentSessionAppendOptions + +/** The session's own agent wrote this row: nothing is stamped, nothing is owed. + * Only for a site with no ledger to consult; a ledger-backed root write goes + * through `stampFor(null)`, which also supersedes anything owed to the row. */ +export const rootClaudeRowStamp: ClaudeRowStamp = () => ({}) + +/** Corrections outstanding at once, across every producer. */ +const MAX_OUTSTANDING_CORRECTIONS = 256 + +/** Per producer, so one child talking hard before its announcement cannot push + * out every other child's corrections along with its own. */ +const MAX_CORRECTIONS_PER_REF = 128 + +type OutstandingCorrection = { + ref: string + identity: AgentJournalItemIdentity + /** The newest body written under this identity. Two writes can share one — + * a tool call and its result do — and a correction carrying the older body + * would revert the row it is only meant to re-attribute. */ + body: AgentJournalItemBody + stamped: StructuredAgentSessionAppendOptions +} + +export type ClaudeProvisionalRowCorrectionsDeps = ClaudeSubagentLinkageSource & { + /** Re-appends a row under its own identity, which revises it in place. + * Returns whether the write was ADMITTED: a sink under backpressure refuses, + * and a correction dropped on a refusal is an obligation nothing re-derives. */ + rewrite: ( + identity: AgentJournalItemIdentity, + body: AgentJournalItemBody, + options: StructuredAgentSessionAppendOptions + ) => boolean + publish: () => void +} + +export class ClaudeProvisionalRowCorrections { + /** Keyed by `itemId`, so a second write to one row replaces the correction + * owed to it rather than queueing a stale body behind the fresh one. */ + private readonly outstanding = new Map() + /** Producers whose correction was abandoned at the bound. Remembered so later + * rows are not queued for a correction their siblings will never get. */ + private readonly givenUp = new Set() + + constructor(private readonly deps: ClaudeProvisionalRowCorrectionsDeps) {} + + get pending(): number { + return this.outstanding.size + } + + /** How rows from one envelope are attributed. A null reference is the + * session's own agent; anything else is a child of it. */ + stampFor(parentToolUseId: string | null): ClaudeRowStamp { + if (parentToolUseId === null) { + return (identity) => { + this.supersede(identity) + return {} + } + } + return (identity, body) => { + const provisional = this.deps.linkageFor(parentToolUseId).kind === 'pending' + const options = this.stamp(parentToolUseId) + if (provisional) { + this.remember(parentToolUseId, identity, body, options) + } else { + this.supersede(identity) + } + return options + } + } + + /** An announcement may have named a producer rows are already stamped with. + * Rewrites those whose stamp would now differ and forgets the rest. */ + retry(): void { + let wrote = false + // Map iteration tolerates deletion of the entry just visited. + for (const [itemId, correction] of this.outstanding) { + if (this.deps.linkageFor(correction.ref).kind === 'pending') { + continue + } + const outcome = this.settle(correction) + // Kept outstanding when the sink refused it, so `abandon` gets another + // go. Dropping it here would strand the row on a stamp nothing revisits. + if (outcome !== 'refused') { + this.outstanding.delete(itemId) + } + wrote = outcome === 'wrote' || wrote + } + if (wrote) { + this.deps.publish() + } + } + + /** Nothing further can name these producers, so no correction is coming. The + * rows keep the stamp they already carry; at settle it is the same verdict, + * so this writes nothing and burns no revision. */ + abandon(): void { + let wrote = false + for (const correction of this.outstanding.values()) { + // Last attempt. A refusal here ends it: the row keeps a usable id, and an + // obligation with no exit is worse than one that settles for less. + wrote = this.settle(correction) === 'wrote' || wrote + } + this.outstanding.clear() + this.givenUp.clear() + if (wrote) { + this.deps.publish() + } + } + + /** + * A settled write lands on a row a correction was owed to, so the correction + * goes. + * + * Dropped rather than re-bodied: a settled write already carries a FINAL + * verdict, so the correction could only restamp the row from a reference this + * write did not use — equal at best, and at worst the older body. One row can + * legitimately be written under two references (a call and its result), and + * this is what keeps a correction owed to the first from outliving the second. + */ + private supersede(identity: AgentJournalItemIdentity): void { + this.outstanding.delete(agentJournalItemKey(identity)) + } + + private stamp(parentToolUseId: string): StructuredAgentSessionAppendOptions { + return agentJournalLinkageFields(this.deps.settledLinkageFor(parentToolUseId).linkage) + } + + /** Writes the correction only when it actually changes the row's attribution. + * A duplicate must not burn a revision. */ + private settle(correction: OutstandingCorrection): 'wrote' | 'unchanged' | 'refused' { + const options = this.stamp(correction.ref) + if (sameLinkage(options, correction.stamped)) { + return 'unchanged' + } + return this.deps.rewrite(correction.identity, correction.body, options) ? 'wrote' : 'refused' + } + + private remember( + ref: string, + identity: AgentJournalItemIdentity, + body: AgentJournalItemBody, + stamped: StructuredAgentSessionAppendOptions + ): void { + if (this.givenUp.has(ref)) { + return + } + const itemId = agentJournalItemKey(identity) + // Re-inserted rather than updated in place, so the newest write is also the + // youngest — insertion order is what `giveUpOnOldest` reads. + this.outstanding.delete(itemId) + this.outstanding.set(itemId, { ref, identity, body, stamped }) + if (this.countFor(ref) > MAX_CORRECTIONS_PER_REF) { + this.giveUp(ref) + } + while (this.outstanding.size > MAX_OUTSTANDING_CORRECTIONS) { + this.giveUpOnOldest() + } + } + + /** + * Stops correcting one producer, and forgets what was owed it. + * + * WHOLESALE, never row by row. Correcting some of a child's rows and not the + * rest splits one child across two ids in the same session — worse than + * correcting none, because the rows left behind are the ones a reader would + * have to reconcile. Giving up leaves every one of them on the spawn call's + * id: still that child's, still not the parent's, and still all the same. + */ + private giveUp(ref: string): void { + this.givenUp.add(ref) + for (const [itemId, entry] of this.outstanding) { + if (entry.ref === ref) { + this.outstanding.delete(itemId) + } + } + } + + private giveUpOnOldest(): void { + for (const entry of this.outstanding.values()) { + this.giveUp(entry.ref) + return + } + } + + private countFor(ref: string): number { + let count = 0 + for (const entry of this.outstanding.values()) { + if (entry.ref === ref) { + count += 1 + } + } + return count + } +} + +function sameLinkage( + left: StructuredAgentSessionAppendOptions, + right: StructuredAgentSessionAppendOptions +): boolean { + return ( + left.agentId === right.agentId && + left.parentAgentId === right.parentAgentId && + left.providerParentRef === right.providerParentRef && + left.producerKind === right.producerKind && + left.attempt === right.attempt + ) +} diff --git a/src/main/claude/claude-streamed-block-identity.ts b/src/main/claude/claude-streamed-block-identity.ts index 5cbf6674159..00bfdd103ab 100644 --- a/src/main/claude/claude-streamed-block-identity.ts +++ b/src/main/claude/claude-streamed-block-identity.ts @@ -7,7 +7,14 @@ import { claudeRecord, claudeText } from './claude-structured-item-translation' // journal identity, and the final frame lands on it in block order instead of // appending a duplicate under its own uuid. -export type ClaudeStreamedTextDelta = { identity: AgentJournalItemIdentity; text: string } +export type ClaudeStreamedTextDelta = { + identity: AgentJournalItemIdentity + text: string + /** The block's own scope, which this registry already keys its map on. Streamed + * prose has no message envelope when it is persisted, so the producer travels + * with the delta rather than being re-read from a frame that is long gone. */ + parentToolUseId: string | null +} type StreamedMessage = { messageId: string | null @@ -64,7 +71,8 @@ export function createClaudeStreamedBlockRegistry(): ClaudeStreamedBlockRegistry if (frame.type !== 'stream_event' || !event || !sessionId || !uuid) { return null } - const scope = scopeKey(sessionId, claudeText(frame.parent_tool_use_id)) + const parentToolUseId = claudeText(frame.parent_tool_use_id) + const scope = scopeKey(sessionId, parentToolUseId) if (event.type === 'message_start') { messages.set(scope, { messageId: claudeText(claudeRecord(event.message)?.id), @@ -81,7 +89,7 @@ export function createClaudeStreamedBlockRegistry(): ClaudeStreamedBlockRegistry } const identity = mint(messageFor(scope), sessionId, index, uuid) const text = claudeText(block.text) - return text ? { identity, text } : null + return text ? { identity, text, parentToolUseId } : null } if (event.type !== 'content_block_delta') { return null @@ -93,7 +101,7 @@ export function createClaudeStreamedBlockRegistry(): ClaudeStreamedBlockRegistry } const streamed = messageFor(scope) const identity = streamed.blocks.get(index) ?? mint(streamed, sessionId, index, uuid) - return { identity, text } + return { identity, text, parentToolUseId } }, reconcile: (frame) => { const streamed = messages.get(scopeKey(frame.sessionId, frame.parentToolUseId)) diff --git a/src/main/claude/claude-streamed-text-checkpoints.test.ts b/src/main/claude/claude-streamed-text-checkpoints.test.ts index 00a0bc0edd6..cc196cf21af 100644 --- a/src/main/claude/claude-streamed-text-checkpoints.test.ts +++ b/src/main/claude/claude-streamed-text-checkpoints.test.ts @@ -1,17 +1,70 @@ import { describe, expect, it } from 'vitest' -import type { AgentJournalItemIdentity } from '../../shared/agent-session-journal-types' +import type { + AgentJournalItemIdentity, + AgentJournalProducerLinkage +} from '../../shared/agent-session-journal-types' +import type { StructuredAgentSessionAppendOptions } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' import { createClaudeStreamedTextCheckpoints } from './claude-streamed-text-checkpoints' +import type { + ClaudeSubagentLinkageSource, + ClaudeSubagentLinkageVerdict +} from './claude-subagent-linkage' function identityOf(uuid: string): AgentJournalItemIdentity { return { provider: 'claude', sessionId: 'claude-session', uuid } } -function checkpoints() { +/** A block streamed with no scope is the session's own agent's, and the producer + * is never asked about it. Throwing pins that: a block that starts consulting + * the resolver for a scopeless row shows up here rather than silently. */ +const unconsultedProducer: ClaudeSubagentLinkageSource = { + linkageFor: () => { + throw new Error('resolver consulted for a block with no scope') + }, + settledLinkageFor: () => { + throw new Error('resolver consulted for a block with no scope') + } +} + +/** The linkage a child's block carries once its announcement has landed. */ +const CHILD_LINKAGE: AgentJournalProducerLinkage = { + agentId: 'task-1', + providerParentRef: 'toolu_1', + producerKind: 'agent' +} + +/** A producer whose answer a test moves from provisional to final, the way an + * announcement arriving mid-stream does. Under settle it never waits: the raw + * reference is the only handle a child that was never announced will have. */ +function scriptedProducer() { + let verdict: ClaudeSubagentLinkageVerdict = { kind: 'pending' } + const settledFallback: Extract = { + kind: 'linked', + linkage: { agentId: 'toolu_1', providerParentRef: 'toolu_1', producerKind: 'agent' } + } + return { + source: { + linkageFor: () => verdict, + settledLinkageFor: () => (verdict.kind === 'pending' ? settledFallback : verdict) + } satisfies ClaudeSubagentLinkageSource, + resolve: (linkage: AgentJournalProducerLinkage) => { + verdict = { kind: 'linked', linkage } + } + } +} + +function checkpoints(producer: ClaudeSubagentLinkageSource = unconsultedProducer) { const rows: { uuid: string; text: string }[] = [] + /** Attribution kept beside the rows rather than on them, so the assertions + * about text stay about text — and so a harness that dropped the argument + * would show up as an empty list rather than as silence. */ + const stamps: StructuredAgentSessionAppendOptions[] = [] let scheduled: (() => void) | null = null const store = createClaudeStreamedTextCheckpoints({ - persist: (identity, text) => { + producer, + persist: (identity, text, options) => { rows.push({ uuid: 'uuid' in identity ? identity.uuid : '', text }) + stamps.push(options) }, schedule: (run) => { scheduled = run @@ -23,6 +76,7 @@ function checkpoints() { return { store, rows, + stamps, runWindow: () => { const run = scheduled as (() => void) | null run?.() @@ -79,6 +133,106 @@ describe('claude streamed text checkpoints', () => { expect(rows).toHaveLength(1) }) + it("stamps a block streamed inside a child with that child's linkage", () => { + const producer = scriptedProducer() + producer.resolve(CHILD_LINKAGE) + const { store, rows, stamps, runWindow } = checkpoints(producer.source) + + store.append(identityOf('block-1'), 'hello', 'toolu_1') + runWindow() + + expect(rows).toEqual([{ uuid: 'block-1', text: 'hello' }]) + expect(stamps).toEqual([CHILD_LINKAGE]) + }) + + it("writes no linkage keys for a block the session's own agent streamed", () => { + const { store, stamps, runWindow } = checkpoints() + + store.append(identityOf('block-1'), 'hello') + runWindow() + + expect(stamps).toEqual([{}]) + }) + + it('writes a checkpoint at once while the producing agent is provisional', () => { + // The prose reaches the user immediately, stamped with the handle that + // exists. Every checkpoint rewrites the same row, so the announcement can + // correct it in place — holding the text back buys nothing and costs the + // user sight of what the child is saying. + const producer = scriptedProducer() + const { store, rows, stamps, runWindow } = checkpoints(producer.source) + + store.append(identityOf('block-1'), 'partial', 'toolu_1') + runWindow() + expect(rows).toEqual([{ uuid: 'block-1', text: 'partial' }]) + expect(stamps.at(-1)).toMatchObject({ agentId: 'toolu_1' }) + + // `flush` rewrites a row whose TEXT moved on; correcting a stamp on text + // that did not is what re-attribution is for, and the translator runs both. + producer.resolve(CHILD_LINKAGE) + store.flush() + expect(stamps.at(-1)).toMatchObject({ agentId: 'toolu_1' }) + + store.reattribute() + + expect(rows.at(-1)).toEqual({ uuid: 'block-1', text: 'partial' }) + expect(stamps.at(-1)).toEqual(CHILD_LINKAGE) + }) + + it('writes a held block under the raw reference when no announcement comes', () => { + // Anti-swallow for the streamed lane: the flush that precedes settlement has + // to write the text, and as a child's rather than as the session's own. + const producer = scriptedProducer() + const { store, rows, stamps, runWindow } = checkpoints(producer.source) + + store.append(identityOf('block-1'), 'never announced', 'toolu_1') + runWindow() + + expect(rows).toEqual([{ uuid: 'block-1', text: 'never announced' }]) + expect(stamps).toEqual([ + { agentId: 'toolu_1', providerParentRef: 'toolu_1', producerKind: 'agent' } + ]) + + // Nothing ever names it, so re-attribution has nothing better to say and + // must not burn a revision repeating itself. + store.reattribute() + expect(rows).toHaveLength(1) + }) + + it('re-resolves a block’s producer on every checkpoint', () => { + // Every checkpoint rewrites the SAME row, so there is only ever one row per + // block and re-resolving can only revise it. Latching the first verdict is + // what made an announcement arriving mid-stream unable to correct it. + const producer = scriptedProducer() + producer.resolve(CHILD_LINKAGE) + const { store, stamps, runWindow } = checkpoints(producer.source) + + store.append(identityOf('block-1'), 'first', 'toolu_1') + runWindow() + producer.resolve({ ...CHILD_LINKAGE, agentId: 'task-2' }) + store.append(identityOf('block-1'), 'first and more', 'toolu_1') + store.flush() + + expect(stamps).toEqual([CHILD_LINKAGE, { ...CHILD_LINKAGE, agentId: 'task-2' }]) + }) + + it('re-attributes a block that stopped streaming before its announcement', () => { + // Nothing revisits such a block: no later checkpoint, no final envelope. + // Without this it keeps the provisional id for the life of the journal. + const producer = scriptedProducer() + const { store, rows, stamps, runWindow } = checkpoints(producer.source) + + store.append(identityOf('block-1'), 'said once', 'toolu_1') + runWindow() + expect(stamps.at(-1)).toMatchObject({ agentId: 'toolu_1' }) + + producer.resolve(CHILD_LINKAGE) + store.reattribute() + + expect(rows.at(-1)).toEqual({ uuid: 'block-1', text: 'said once' }) + expect(stamps.at(-1)).toEqual(CHILD_LINKAGE) + }) + it('stops persisting once disposed', () => { const { store, rows, runWindow } = checkpoints() diff --git a/src/main/claude/claude-streamed-text-checkpoints.ts b/src/main/claude/claude-streamed-text-checkpoints.ts index 348ecd99558..ad21ee790a6 100644 --- a/src/main/claude/claude-streamed-text-checkpoints.ts +++ b/src/main/claude/claude-streamed-text-checkpoints.ts @@ -1,22 +1,39 @@ +import { agentJournalLinkageFields } from '../../shared/agent-session-journal-producer' import type { AgentJournalItemIdentity } from '../../shared/agent-session-journal-types' import { agentJournalItemKey } from '../../shared/agent-session-journal-item-key' +import type { StructuredAgentSessionAppendOptions } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' import { createAgentSessionDeltaCoalescer, type AgentSessionDeltaCoalescerDeps } from '../native-chat/agent-session-wire/agent-session-delta-coalescer' +import type { ClaudeSubagentLinkageSource } from './claude-subagent-linkage' export type ClaudeStreamedTextCheckpointDeps = { /** Rewrites the block's journal row with the text accumulated so far. */ - persist: (identity: AgentJournalItemIdentity, text: string) => void + persist: ( + identity: AgentJournalItemIdentity, + text: string, + options: StructuredAgentSessionAppendOptions + ) => void + /** Who produced a block, asked by the scope the block streamed under. */ + producer: ClaudeSubagentLinkageSource coalesceMs?: number schedule?: AgentSessionDeltaCoalescerDeps['schedule'] } export type ClaudeStreamedTextCheckpoints = { /** Accumulate a delta; the row is rewritten on the coalescer's own cadence. */ - append: (identity: AgentJournalItemIdentity, text: string) => void + append: ( + identity: AgentJournalItemIdentity, + text: string, + parentToolUseId?: string | null + ) => void /** Write every block whose row is behind the text received for it. */ flush: () => void + /** Rewrite every block whose producer now resolves differently. A block that + * stopped streaming before its announcement is never revisited otherwise, + * and would keep a provisional id no later checkpoint comes to correct. */ + reattribute: () => void /** Drop one block's state, for a block whose final frame has now landed. */ forget: (key: string) => void /** @@ -36,13 +53,45 @@ export type ClaudeStreamedTextCheckpoints = { * The row is rewritten on a widening interval rather than per delta: a 200-line * reply would otherwise rewrite the same journal row once per token. */ +function sameLinkage( + left: StructuredAgentSessionAppendOptions, + right: StructuredAgentSessionAppendOptions +): boolean { + return ( + left.agentId === right.agentId && + left.parentAgentId === right.parentAgentId && + left.providerParentRef === right.providerParentRef && + left.producerKind === right.producerKind && + left.attempt === right.attempt + ) +} + export function createClaudeStreamedTextCheckpoints( deps: ClaudeStreamedTextCheckpointDeps ): ClaudeStreamedTextCheckpoints { const identities = new Map() + /** The scope a block streamed under, kept because the persist callback has no + * frame to re-read it from. */ + const scopes = new Map() + /** What each block's row was last written WITH — never a latch on resolving + * it again. Every checkpoint rewrites the same identity, so a block has one + * row and re-resolving can only revise it; this exists so a re-attribution + * that would change nothing does not burn a revision. */ + const writtenLinkage = new Map() const latestText = new Map() const checkpointLengths = new Map() + /** Resolved FRESH on every checkpoint. A provisional producer is stamped with + * the handle it has rather than holding the prose back: the next checkpoint, + * or `reattribute` once the announcement lands, revises the same row. */ + const producerOptions = (key: string): StructuredAgentSessionAppendOptions => { + const scope = scopes.get(key) ?? null + if (scope === null) { + return {} + } + return agentJournalLinkageFields(deps.producer.settledLinkageFor(scope).linkage) + } + const persist = (key: string, text: string, force: boolean): void => { latestText.set(key, text) const checkpointLength = checkpointLengths.get(key) ?? 0 @@ -54,8 +103,10 @@ export function createClaudeStreamedTextCheckpoints( if (!identity) { return } + const options = producerOptions(key) checkpointLengths.set(key, text.length) - deps.persist(identity, text) + writtenLinkage.set(key, options) + deps.persist(identity, text, options) } const coalescer = createAgentSessionDeltaCoalescer({ @@ -67,14 +118,17 @@ export function createClaudeStreamedTextCheckpoints( const drop = (key: string): void => { coalescer.forget(key) identities.delete(key) + scopes.delete(key) + writtenLinkage.delete(key) latestText.delete(key) checkpointLengths.delete(key) } return { - append: (identity, text) => { + append: (identity, text, parentToolUseId = null) => { const key = agentJournalItemKey(identity) identities.set(key, identity) + scopes.set(key, parentToolUseId) coalescer.append(key, text) }, flush: () => { @@ -85,6 +139,22 @@ export function createClaudeStreamedTextCheckpoints( } } }, + reattribute: () => { + for (const [key, identity] of identities) { + const text = latestText.get(key) + if (text === undefined) { + continue + } + const options = producerOptions(key) + const written = writtenLinkage.get(key) + // Nothing resolved differently: a duplicate must not burn a revision. + if (written && sameLinkage(written, options)) { + continue + } + writtenLinkage.set(key, options) + deps.persist(identity, text, options) + } + }, forget: drop, settle: () => { // Map iteration tolerates deletion of the entry just visited. @@ -98,6 +168,8 @@ export function createClaudeStreamedTextCheckpoints( dispose: () => { coalescer.dispose() identities.clear() + scopes.clear() + writtenLinkage.clear() latestText.clear() checkpointLengths.clear() } diff --git a/src/main/claude/claude-structured-journal-prompts.ts b/src/main/claude/claude-structured-journal-prompts.ts index 3c660223257..0d2509723f6 100644 --- a/src/main/claude/claude-structured-journal-prompts.ts +++ b/src/main/claude/claude-structured-journal-prompts.ts @@ -62,6 +62,17 @@ export class ClaudeJournalPrompts { } ) {} + /** + * Prompt rows carry NO producer linkage, and cannot. + * + * A prompt is not a transcript frame: it reaches Orca through the SDK's + * permission callback, whose options carry a request id and the tool awaiting + * approval and no parent reference of any kind. So when a subagent asks, the + * row cannot name it — unattributable at this site, not deliberately root. + * + * No reader is wrong because of it. A pending prompt projects the session as + * `attention` whoever raised it, which is the truth: the USER has to answer. + */ handle(event: Extract): void { const items: ClaudeJournalPrompt[] = [] if (event.prompt.kind === 'question') { diff --git a/src/main/claude/claude-structured-journal-translation-subagents.test.ts b/src/main/claude/claude-structured-journal-translation-subagents.test.ts index 3b280020c31..d122d0af7a2 100644 --- a/src/main/claude/claude-structured-journal-translation-subagents.test.ts +++ b/src/main/claude/claude-structured-journal-translation-subagents.test.ts @@ -7,7 +7,10 @@ import type { NativeChatSubagentEntry, NativeChatSubagentGroupBlock } from '../../shared/native-chat-types' -import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' +import type { + StructuredAgentSessionAppendOptions, + StructuredAgentSessionEventSink +} from '../native-chat/agent-session-wire/structured-agent-session-event-sink' import { createClaudeJournalTranslator } from './claude-structured-journal-translation' const GROUP_ITEM_ID = 'claude-subagents:claude-session:user-1' @@ -18,13 +21,36 @@ function orcaClientMessageId(identity: AgentJournalItemIdentity): string | null } function harness() { - const items: { identity: AgentJournalItemIdentity; body: AgentJournalItemBody }[] = [] + // `options` is captured, not declared away: producer attribution rides the + // third argument, and a harness that drops it makes every assertion about + // attribution pass against `undefined`. + const items: { + identity: AgentJournalItemIdentity + body: AgentJournalItemBody + options: StructuredAgentSessionAppendOptions | undefined + }[] = [] const sink: StructuredAgentSessionEventSink = { - appendItem: (identity, body) => items.push({ identity, body }), + appendItem: (identity, body, options) => items.push({ identity, body, options }), appendTombstone: vi.fn(), publish: vi.fn() } - const translator = createClaudeJournalTranslator({ sink, fallbackIdPrefix: 'test' }) + let scheduled: (() => void) | null = null + const translator = createClaudeJournalTranslator({ + sink, + fallbackIdPrefix: 'test', + // Drives the streamed coalescer by hand, so a test can place an + // announcement precisely before or after a checkpoint lands. + schedule: (run) => { + scheduled = run + return () => { + scheduled = null + } + } + }) + const runStreamWindow = (): void => { + const run = scheduled as (() => void) | null + run?.() + } const groupRows = () => items.filter((item) => orcaClientMessageId(item.identity) === GROUP_ITEM_ID) const agentsOf = (body: AgentJournalItemBody | undefined): NativeChatSubagentEntry[] => { @@ -50,7 +76,31 @@ function harness() { items .filter((item) => (orcaClientMessageId(item.identity) ?? '').startsWith('provider-frame:')) .map((item) => item.body) - return { translator, groupRows, roster, rosterIn, rosterOf, fallbackRows } + /** Attribution stamped on a row, found by the text it carries, so a test + * names the row it means instead of indexing into the append order. */ + const writesOfProse = (text: string) => + items.filter( + (entry) => + entry.body.kind === 'message' && + entry.body.blocks.some((block) => block.type === 'text' && block.text === text) + ) + /** Attribution a row ENDS UP with. Rows are written immediately and + * re-attributed in place, so the newest write is the one that renders — + * reading the first would assert against a stamp already superseded. */ + const linkageOfProse = (text: string): StructuredAgentSessionAppendOptions | undefined => + writesOfProse(text).at(-1)?.options + return { + translator, + items, + groupRows, + roster, + rosterIn, + rosterOf, + fallbackRows, + linkageOfProse, + writesOfProse, + runStreamWindow + } } function userTurn(uuid: string) { @@ -257,3 +307,551 @@ describe('claude journal translation — subagents', () => { expect(rosterIn('outside-turn')).toEqual([expect.objectContaining({ state: 'unverifiable' })]) }) }) + +describe('claude journal translation — which agent produced a row', () => { + /** One child assistant frame carrying prose, parented to a spawn call. */ + function childProse(uuid: string, parentToolUseId: string, text: string) { + return { + type: 'message' as const, + sessionId: 'orca-session', + message: { + type: 'assistant', + uuid, + session_id: 'claude-session', + parent_tool_use_id: parentToolUseId, + message: { role: 'assistant', content: [{ type: 'text', text }] } + } + } + } + + /** The parent's own `Task` call, which is what forwards the spawn id. */ + function spawnCall(uuid: string, toolUseId: string) { + return { + type: 'message' as const, + sessionId: 'orca-session', + message: { + type: 'assistant', + uuid, + session_id: 'claude-session', + parent_tool_use_id: null, + message: { + role: 'assistant', + content: [ + { type: 'tool_use', id: toolUseId, name: 'Task', input: { description: 'explore' } } + ] + } + } + } + } + + /** A tool call the CHILD makes. Its id exists only inside that sidechain, and + * is the one handle the grandchild's own frames will carry. */ + function childSpawnCall(uuid: string, parentToolUseId: string, toolUseId: string) { + return { + type: 'message' as const, + sessionId: 'orca-session', + message: { + type: 'assistant', + uuid, + session_id: 'claude-session', + parent_tool_use_id: parentToolUseId, + message: { + role: 'assistant', + content: [ + { type: 'tool_use', id: toolUseId, name: 'Task', input: { description: 'deeper' } } + ] + } + } + } + } + + function announce(taskId: string, toolUseId: string) { + return systemFrame('task_started', { + task_id: taskId, + tool_use_id: toolUseId, + task_type: 'local_agent', + subagent_type: 'explorer', + description: 'Map the lane' + }) + } + + it("stamps the child's canonical task id, not the spawn call's rotating id", () => { + const { translator, linkageOfProse } = harness() + translator.handle(userTurn('user-1')) + translator.handle(spawnCall('assistant-1', 'toolu_1')) + translator.handle(announce('task-1', 'toolu_1')) + translator.handle(childProse('child-1', 'toolu_1', 'looking')) + + expect(linkageOfProse('looking')).toMatchObject({ + agentId: 'task-1', + providerParentRef: 'toolu_1', + producerKind: 'agent' + }) + }) + + it("leaves the parent's own rows unstamped, which is what makes absence mean root", () => { + const { translator, linkageOfProse } = harness() + translator.handle(userTurn('user-1')) + translator.handle({ + type: 'message' as const, + sessionId: 'orca-session', + message: { + type: 'assistant', + uuid: 'assistant-1', + session_id: 'claude-session', + parent_tool_use_id: null, + message: { role: 'assistant', content: [{ type: 'text', text: 'delegating' }] } + } + }) + // A control, not a pin: the parent's rows carry no linkage before this + // change either. It is here because "absence means root" is only sound + // while the producer really does leave its own rows alone. + expect(linkageOfProse('delegating')?.agentId).toBeUndefined() + }) + + it('keeps one identity across a resume that re-mints the spawn call id', () => { + // THE case the canonical id exists for: the same child announced twice + // under two different tool ids. Both runs' rows must name one agent. + const { translator, linkageOfProse } = harness() + translator.handle(userTurn('user-1')) + translator.handle(spawnCall('assistant-1', 'toolu_1')) + translator.handle(announce('task-1', 'toolu_1')) + translator.handle(childProse('child-1', 'toolu_1', 'first run')) + + translator.handle(spawnCall('assistant-2', 'toolu_2')) + translator.handle(announce('task-1', 'toolu_2')) + translator.handle(childProse('child-2', 'toolu_2', 'second run')) + + expect(linkageOfProse('first run')?.agentId).toBe('task-1') + expect(linkageOfProse('second run')?.agentId).toBe('task-1') + // Identity answers "which agent"; the attempt answers "which run of it". + expect(linkageOfProse('first run')?.attempt).toBeUndefined() + expect(linkageOfProse('second run')?.attempt).toBe(2) + }) + + it('holds a child row that arrives before its announcement, then writes it linked', () => { + const { translator, linkageOfProse } = harness() + translator.handle(userTurn('user-1')) + translator.handle(spawnCall('assistant-1', 'toolu_1')) + // Another child has already proven this release announces its tasks, so a + // spawn with no announcement yet is a window, not a release without one. + translator.handle(announce('task-other', 'toolu_other')) + translator.handle(childProse('child-1', 'toolu_1', 'arrived early')) + + // Written at once, under the only handle that exists yet — never withheld, + // and never the parent's. + expect(linkageOfProse('arrived early')).toMatchObject({ agentId: 'toolu_1' }) + + translator.handle(announce('task-1', 'toolu_1')) + + // Re-attributed in place once the announcement names it. + expect(linkageOfProse('arrived early')).toMatchObject({ + agentId: 'task-1', + providerParentRef: 'toolu_1' + }) + }) + + it('burns no revision correcting a row whose producer was never named', () => { + // The turn ends with the identity still provisional. The row already says + // what settle would say, so the correction must be DROPPED: a duplicate + // rewrite would cost a revision and change nothing. + const { translator, linkageOfProse, writesOfProse } = harness() + translator.handle(userTurn('user-1')) + translator.handle(spawnCall('assistant-1', 'toolu_1')) + translator.handle(announce('task-other', 'toolu_other')) + translator.handle(childProse('child-1', 'toolu_1', 'never announced')) + expect(writesOfProse('never announced')).toHaveLength(1) + + translator.handle(resultFrame()) + + expect(writesOfProse('never announced')).toHaveLength(1) + expect(linkageOfProse('never announced')).toMatchObject({ + agentId: 'toolu_1', + providerParentRef: 'toolu_1' + }) + }) + + it('stamps a spawn call a release never announces with the call\u2019s own id', () => { + // Older releases name nothing they spawn. The spawn call is still in the + // transcript, so its id is a real handle — and stamping it keeps the child's + // prose off the parent, which reading these rows as root would not. + const { translator, linkageOfProse } = harness() + translator.handle(userTurn('user-1')) + translator.handle(spawnCall('assistant-1', 'toolu_1')) + translator.handle(childProse('child-1', 'toolu_1', 'unannounced release')) + + expect(linkageOfProse('unannounced release')).toMatchObject({ agentId: 'toolu_1' }) + }) + + it('never reads a row naming a parent as the session\u2019s own, whatever the release', () => { + // The hardest case for attribution: a sidechain id no spawn call forwarded, + // on a release that has announced nothing, so no announcement is coming and + // no correction ever will. There is still a handle — the reference itself — + // and the row is stamped with it. Reading it as root would assert the parent + // wrote words a child wrote, which is the defect, not a fallback. + const { translator, linkageOfProse } = harness() + translator.handle(userTurn('user-1')) + translator.handle(childProse('child-1', 'toolu_nested', 'no announcement coming')) + + expect(linkageOfProse('no announcement coming')).toMatchObject({ + agentId: 'toolu_nested', + providerParentRef: 'toolu_nested' + }) + // Positively non-root: this is what every parent-scoped reader tests. + expect(linkageOfProse('no announcement coming')?.agentId).not.toBeUndefined() + }) + + it("attributes a tool result naming its own call to the call's own agent", () => { + // Every top-level call is a forwarded tool id, not just a spawn. Reading the + // parent reference literally on a result frame would park ordinary tool + // output against a `task_started` that is never coming, leaving the tool row + // stuck `running` for the rest of the turn. + const { translator, items } = harness() + translator.handle(userTurn('user-1')) + translator.handle(spawnCall('assistant-1', 'toolu_1')) + translator.handle(announce('task-1', 'toolu_1')) + translator.handle({ + type: 'message' as const, + sessionId: 'orca-session', + message: { + type: 'user', + uuid: 'bash-result', + session_id: 'claude-session', + parent_tool_use_id: 'toolu_bash', + message: { + role: 'user', + content: [{ type: 'tool_result', tool_use_id: 'toolu_bash', content: 'a.ts' }] + } + } + }) + + const row = items.find( + (entry) => orcaClientMessageId(entry.identity) === 'claude-tool:claude-session:toolu_bash' + ) + expect(row?.body).toMatchObject({ kind: 'tool-call', state: 'completed' }) + expect(row?.options).toEqual({}) + }) + + /** One streamed text block, as the SDK sends it: a message start, then deltas. + * Streamed prose has no envelope when it is persisted, so its producer has to + * travel with the delta. */ + function streamStart(uuid: string, parentToolUseId: string) { + return { + type: 'message' as const, + sessionId: 'orca-session', + message: { + type: 'stream_event', + uuid, + session_id: 'claude-session', + parent_tool_use_id: parentToolUseId, + event: { type: 'message_start', message: { id: 'msg-1' } } + } + } + } + + function streamDelta(uuid: string, parentToolUseId: string, text: string) { + return { + type: 'message' as const, + sessionId: 'orca-session', + message: { + type: 'stream_event', + uuid, + session_id: 'claude-session', + parent_tool_use_id: parentToolUseId, + event: { type: 'content_block_delta', index: 0, delta: { type: 'text_delta', text } } + } + } + } + + it('re-attributes streamed prose that stopped before its announcement', () => { + // The primary prose path, and the shape nothing revisits: the child streams, + // stops, and no final envelope ever arrives — so only re-attribution can + // move the row off the id it was written under. + const { translator, items, runStreamWindow } = harness() + translator.handle(userTurn('user-1')) + translator.handle(spawnCall('assistant-1', 'toolu_1')) + translator.handle(streamStart('stream-1', 'toolu_1')) + translator.handle(streamDelta('stream-1', 'toolu_1', 'thinking out loud')) + runStreamWindow() + + const streamed = () => + items.filter( + (entry) => + entry.body.kind === 'message' && + entry.body.blocks.some( + (block) => block.type === 'text' && block.text === 'thinking out loud' + ) + ) + // Written at once, and never as the parent's. + expect(streamed().at(-1)?.options).toMatchObject({ agentId: 'toolu_1' }) + + translator.handle(announce('task-1', 'toolu_1')) + + expect(streamed().at(-1)?.options).toMatchObject({ + agentId: 'task-1', + providerParentRef: 'toolu_1' + }) + expect(streamed().at(-1)?.options?.agentId).not.toBeUndefined() + }) + + /** The result of a call a CHILD made, naming its own call as parent. */ + function childToolResult(uuid: string, toolUseId: string) { + return { + type: 'message' as const, + sessionId: 'orca-session', + message: { + type: 'user', + uuid, + session_id: 'claude-session', + parent_tool_use_id: toolUseId, + message: { + role: 'user', + content: [{ type: 'tool_result', tool_use_id: toolUseId, content: 'deeper done' }] + } + } + } + } + + const toolRowWrites = ( + items: readonly { + identity: AgentJournalItemIdentity + body: AgentJournalItemBody + options: StructuredAgentSessionAppendOptions | undefined + }[], + toolUseId: string + ) => + items.filter( + (entry) => orcaClientMessageId(entry.identity) === `claude-tool:claude-session:${toolUseId}` + ) + + it('does not revert a completed nested tool row when its attribution is corrected', () => { + const { translator, items } = harness() + translator.handle(userTurn('user-1')) + translator.handle(spawnCall('assistant-1', 'toolu_1')) + translator.handle(childSpawnCall('child-1', 'toolu_1', 'toolu_2')) + translator.handle(childToolResult('child-result', 'toolu_2')) + translator.handle(announce('task-1', 'toolu_1')) + + const last = toolRowWrites(items, 'toolu_2').at(-1) + expect(last?.body).toMatchObject({ kind: 'tool-call', state: 'completed' }) + expect(last?.options?.agentId).toBe('task-1') + }) + + it('never lets a correction change a row\u2019s content, only its attribution', () => { + // The module's own invariant, pinned directly. A correction re-appends a + // row to restamp it; if it carries a body older than the row's newest, it + // silently reverts content — which is how a completed tool row went back to + // running. Asserted across every row this session writes, not one shape. + const { translator, items } = harness() + translator.handle(userTurn('user-1')) + translator.handle(spawnCall('assistant-1', 'toolu_1')) + translator.handle(childProse('child-1', 'toolu_1', 'talking')) + translator.handle(childSpawnCall('child-2', 'toolu_1', 'toolu_2')) + translator.handle(childToolResult('child-result', 'toolu_2')) + + const idOf = (entry: { identity: AgentJournalItemIdentity }): string => + orcaClientMessageId(entry.identity) ?? JSON.stringify(entry.identity) + const bodyBeforeAnnouncement = new Map() + for (const entry of items) { + bodyBeforeAnnouncement.set(idOf(entry), entry.body) + } + const writesBefore = items.length + + // ONLY the announcement frame, so the window holds re-attributions and not + // a turn settling or any other legitimate body revision. + translator.handle(announce('task-1', 'toolu_1')) + + const corrections = items.slice(writesBefore) + // The announcement DID rewrite rows, or this proves nothing. + expect(corrections.length).toBeGreaterThan(0) + for (const correction of corrections) { + const itemId = idOf(correction) + // The roster's own group row is excluded: the announcement re-keys a + // provisional child onto its canonical id, so that row's body is SUPPOSED + // to change here. Every other rewrite on this frame is a re-attribution. + if (itemId.startsWith('claude-subagents:')) { + continue + } + const before = bodyBeforeAnnouncement.get(itemId) + if (before === undefined) { + continue + } + // Re-attribution only: the body a correction carries is the body the row + // already had. Carrying an older one silently reverts content. + expect(correction.body).toEqual(before) + } + + const nested = items.findLast( + (entry) => orcaClientMessageId(entry.identity) === 'claude-tool:claude-session:toolu_2' + ) + expect(nested?.body).toMatchObject({ kind: 'tool-call', state: 'completed' }) + expect(nested?.options?.agentId).toBe('task-1') + }) + + it('keeps a long pre-announcement burst on ONE id, with no row left at root', () => { + // The burst that outruns the announcement. Every row must name the same + // producer: a row at `{}` is the parent claiming the child's words, and a + // burst split across two ids is one child appearing as two. + const { translator, items } = harness() + translator.handle(userTurn('user-1')) + translator.handle(spawnCall('assistant-1', 'toolu_1')) + for (let index = 0; index < 70; index += 1) { + translator.handle(childProse(`child-${index}`, 'toolu_1', `line ${index}`)) + } + translator.handle(announce('task-1', 'toolu_1')) + + const finalAgentIds = new Set() + for (let index = 0; index < 70; index += 1) { + const writes = items.filter( + (entry) => + entry.body.kind === 'message' && + entry.body.blocks.some((block) => block.type === 'text' && block.text === `line ${index}`) + ) + expect(writes.length).toBeGreaterThan(0) + finalAgentIds.add(writes.at(-1)?.options?.agentId) + } + // The canonical id, not merely a consistent one: 70 is inside the bound, so + // every row is actually corrected rather than given up on. + expect(finalAgentIds).toEqual(new Set(['task-1'])) + }) + + it("corrects the session's FIRST child, whose spawn is unannounced only so far", () => { + // The release check reads "no task announced yet", which every session looks + // like before its first `task_started`. Without the spawn call outranking + // it, the first child's pre-announcement rows persist as the PARENT's — the + // whole defect, for the first child of every session. + const { translator, linkageOfProse } = harness() + translator.handle(userTurn('user-1')) + translator.handle(spawnCall('assistant-1', 'toolu_1')) + translator.handle(childProse('child-1', 'toolu_1', 'first child prose')) + + // Never the parent's, not even for the window before the announcement. + expect(linkageOfProse('first child prose')).toMatchObject({ agentId: 'toolu_1' }) + + translator.handle(announce('task-1', 'toolu_1')) + + expect(linkageOfProse('first child prose')).toMatchObject({ + agentId: 'task-1', + providerParentRef: 'toolu_1' + }) + }) + + it('stamps nested sidechain traffic this release will never announce', () => { + // A grandchild parented to a tool id that only ever existed inside a + // sidechain. No announcement is coming, so the raw reference is the only + // handle — but the row is still a child's, never the parent's. + const { translator, linkageOfProse } = harness() + translator.handle(userTurn('user-1')) + translator.handle(spawnCall('assistant-1', 'toolu_1')) + translator.handle(announce('task-1', 'toolu_1')) + translator.handle(childProse('grandchild-1', 'toolu_nested', 'deeper')) + + expect(linkageOfProse('deeper')).toMatchObject({ + agentId: 'toolu_nested', + providerParentRef: 'toolu_nested' + }) + // The call that opened this sidechain was never journaled, so who spawned + // it is genuinely unknown and the row claims nothing. + expect(linkageOfProse('deeper')?.parentAgentId).toBeUndefined() + }) + + it('names the child that spawned a grandchild rather than leaving it on the session', () => { + // The grandchild's frames carry one handle: the nested call id. That id was + // journaled on the CHILD's own row, which is the only place the real parent + // is recoverable — and without it the row's absent parent would read as a + // claim that the session's own agent spawned it. + const { translator, linkageOfProse } = harness() + translator.handle(userTurn('user-1')) + translator.handle(spawnCall('assistant-1', 'toolu_1')) + translator.handle(announce('task-1', 'toolu_1')) + translator.handle(childSpawnCall('child-1', 'toolu_1', 'toolu_nested')) + translator.handle(childProse('grandchild-1', 'toolu_nested', 'deeper')) + + expect(linkageOfProse('deeper')).toMatchObject({ + agentId: 'toolu_nested', + parentAgentId: 'task-1', + providerParentRef: 'toolu_nested' + }) + }) + + it('keeps a grandchild and its parent on the same id, before and after', () => { + // A row names its parent as well as its producer, and the parent's identity + // can still be provisional. It is written with whatever the parent's own + // rows carry AT THAT MOMENT, so the two never disagree, and the + // announcement corrects both together. + const { translator, linkageOfProse } = harness() + translator.handle(userTurn('user-1')) + translator.handle(spawnCall('assistant-1', 'toolu_1')) + // Some other task announced, so this release has proven it declares them. + translator.handle(announce('task-other', 'toolu_other')) + translator.handle(childSpawnCall('child-1', 'toolu_1', 'toolu_nested')) + translator.handle(childProse('grandchild-1', 'toolu_nested', 'held deeper')) + // Written at once, naming the parent by the same handle the parent's own + // rows carry right now — not left blank, which would claim the session's + // own agent spawned it. + expect(linkageOfProse('held deeper')).toMatchObject({ + agentId: 'toolu_nested', + parentAgentId: 'toolu_1' + }) + + translator.handle(announce('task-1', 'toolu_1')) + + expect(linkageOfProse('held deeper')).toMatchObject({ + agentId: 'toolu_nested', + parentAgentId: 'task-1' + }) + }) + + it('leaves a row a child’s when the session is torn down mid-flight', () => { + // Teardown cannot improve the stamp and must not undo it: the row was + // already written as this child's, and dispose leaves it that way rather + // than re-resolving after the roster forgets what the session announced. + const { translator, linkageOfProse, writesOfProse } = harness() + translator.handle(userTurn('user-1')) + translator.handle(spawnCall('assistant-1', 'toolu_1')) + translator.handle(announce('task-other', 'toolu_other')) + translator.handle(childProse('child-1', 'toolu_1', 'still open at teardown')) + expect(writesOfProse('still open at teardown')).toHaveLength(1) + + translator.dispose() + + expect(writesOfProse('still open at teardown')).toHaveLength(1) + expect(linkageOfProse('still open at teardown')).toMatchObject({ + agentId: 'toolu_1', + providerParentRef: 'toolu_1' + }) + }) + + it('classifies a backgrounded shell task as background work, not as an agent', () => { + const { translator, linkageOfProse } = harness() + translator.handle(userTurn('user-1')) + translator.handle(spawnCall('assistant-1', 'toolu_bash')) + translator.handle( + systemFrame('task_started', { + task_id: 'task-bash', + tool_use_id: 'toolu_bash', + task_type: 'local_bash', + description: 'sleep 20', + is_backgrounded: true + }) + ) + translator.handle(childProse('bash-1', 'toolu_bash', 'shell output')) + + expect(linkageOfProse('shell output')).toMatchObject({ producerKind: 'background' }) + }) + + it("leaves the spawn-group row the parent's, though a child frame triggered it", () => { + // Hazard: the group row is written from a child's frame but describes the + // PARENT's children. Stamping it as a child's would hide the roster from + // the very row that owns it. Asserted on the written row, not on a field. + const { translator, groupRows } = harness() + translator.handle(userTurn('user-1')) + translator.handle(spawnCall('assistant-1', 'toolu_1')) + translator.handle(announce('task-1', 'toolu_1')) + translator.handle(childProse('child-1', 'toolu_1', 'looking')) + + const groupRow = groupRows().at(-1) + expect(groupRow).toBeDefined() + expect(groupRow?.options?.agentId).toBeUndefined() + }) +}) diff --git a/src/main/claude/claude-structured-journal-translation.ts b/src/main/claude/claude-structured-journal-translation.ts index 34c4df5bdc9..b39984bc4b9 100644 --- a/src/main/claude/claude-structured-journal-translation.ts +++ b/src/main/claude/claude-structured-journal-translation.ts @@ -18,11 +18,13 @@ import { } from './claude-structured-provider-fallback' import { taskFrameSentence } from './claude-background-task-frames' import { ClaudeBackgroundTaskRows } from './claude-background-task-rows' -import { ClaudeForwardedToolRegistry } from './claude-forwarded-tool-registry' +import { ClaudeToolOriginRegistry } from './claude-tool-origin-registry' +import { ClaudeProvisionalRowCorrections } from './claude-provisional-row-corrections' import { ClaudeSubagentRoster } from './claude-subagent-roster' import { createClaudeStreamedBlockRegistry } from './claude-streamed-block-identity' import { createClaudeStreamedTextCheckpoints } from './claude-streamed-text-checkpoints' import { + claudeFrameParentRef, claudeStreamTurnStartSource, claudeStreamTurnSource, isRootClaudeFrame @@ -86,14 +88,34 @@ export function createClaudeJournalTranslator( deps.sink, deps.fallbackIdPrefix ?? 'acquisition' ) + const toolOrigins = new ClaudeToolOriginRegistry() const subagents = new ClaudeSubagentRoster({ sink: deps.sink, - currentGroupKey: () => turn.groupKey + currentGroupKey: () => turn.groupKey, + isForwardedParentTool: (toolUseId) => toolOrigins.has(toolUseId), + childOwnerRefOf: (toolUseId) => toolOrigins.childOwnerRef(toolUseId), + // A settled group can receive no further announcement, so a correction + // still owed is never coming; the rows keep the stamp they already have. + onIdentitiesFinal: () => corrections.abandon() + }) + const corrections = new ClaudeProvisionalRowCorrections({ + ...subagents.linkage, + rewrite: (identity, body, options) => { + // The admission-returning path, so a correction the sink refuses under + // backpressure stays owed instead of vanishing. Sinks without it accept + // unconditionally, which is what the plain append already assumed. + const admission = deps.sink.tryAppendItem?.(identity, body, options) + if (admission === undefined) { + deps.sink.appendItem(identity, body, options) + return true + } + return admission.accepted + }, + publish: () => deps.sink.publish() }) - const forwardedTools = new ClaudeForwardedToolRegistry() const backgroundTasks = new ClaudeBackgroundTaskRows({ sink: deps.sink, - isForwardedParentTool: (toolUseId) => forwardedTools.has(toolUseId), + isForwardedParentTool: (toolUseId) => toolOrigins.has(toolUseId), // A typed task row is provider output: journaling one must open a resumed // turn, or the session shows the row while reading idle. openOutputTurn: (frame, observedAt) => @@ -105,8 +127,9 @@ export function createClaudeJournalTranslator( const streamedText = createClaudeStreamedTextCheckpoints({ ...(deps.coalesceMs === undefined ? {} : { coalesceMs: deps.coalesceMs }), ...(deps.schedule ? { schedule: deps.schedule } : {}), - persist: (identity, text) => { - deps.sink.appendItem(identity, claudeStreamingMessageBody(text)) + producer: subagents.linkage, + persist: (identity, text, options) => { + deps.sink.appendItem(identity, claudeStreamingMessageBody(text), options) deps.sink.publish() } }) @@ -131,7 +154,7 @@ export function createClaudeJournalTranslator( if (!delta) { return false } - streamedText.append(delta.identity, delta.text) + streamedText.append(delta.identity, delta.text, delta.parentToolUseId) return true } @@ -141,9 +164,10 @@ export function createClaudeJournalTranslator( streamedBlocks, streamedText, subagents, - forwardedTools, + toolOrigins, backgroundTasks, providerFallback, + corrections, turn } @@ -171,7 +195,15 @@ export function createClaudeJournalTranslator( if (event.type === 'message' && handleStream(event.message, event.observedAt ?? Date.now())) { return } + // Ahead of the flush: a forced checkpoint resolves attribution as it + // writes, so an announcement landing in this same pass has to be visible + // to it or the row is stamped provisionally one line too early. + const announced = event.type === 'message' && subagents.observeSystemFrame(event.message) streamedText.flush() + if (announced) { + corrections.retry() + streamedText.reattribute() + } if (event.type === 'prompt') { prompts.handle(event) } else if (event.type === 'prompt-cancelled') { @@ -199,10 +231,18 @@ export function createClaudeJournalTranslator( const kind = claudeProviderFrameKind(event.message) const failure = claudeResultFailure(event.message) if (failure || !isSettledClaudeResultKind(kind)) { - providerFallback.append(kind, event.message, failure?.text) + providerFallback.append( + kind, + event.message, + failure?.text, + undefined, + undefined, + // A result that settles no turn is a CHILD's result: this + // translator only ever opens root turns. + settlesTurn ? undefined : corrections.stampFor(claudeFrameParentRef(event.message)) + ) } } else if (event.type === 'message') { - subagents.observeSystemFrame(event.message) const backgroundTaskCovered = backgroundTasks.observe( event.message, event.observedAt ?? Date.now() @@ -221,7 +261,8 @@ export function createClaudeJournalTranslator( event.message, taskFrameSentence(event.message), undefined, - { coveredByTypedTranslator: backgroundTaskCovered } + { coveredByTypedTranslator: backgroundTaskCovered }, + corrections.stampFor(claudeFrameParentRef(event.message)) ) } publishActivity(kind, event.message) @@ -249,13 +290,14 @@ export function createClaudeJournalTranslator( return streamedText.pending }, dispose: () => { + streamedText.flush() streamedText.dispose() tools.clear() prompts.clear() streamedBlocks.clear() subagents.dispose() backgroundTasks.dispose() - forwardedTools.clear() + toolOrigins.clear() } } } diff --git a/src/main/claude/claude-structured-provider-fallback.ts b/src/main/claude/claude-structured-provider-fallback.ts index ebce2762c55..baa6cb83a52 100644 --- a/src/main/claude/claude-structured-provider-fallback.ts +++ b/src/main/claude/claude-structured-provider-fallback.ts @@ -15,6 +15,7 @@ import { type ClaudeMessageEnvelope } from './claude-structured-item-translation' import { claudeResultOutcome } from './claude-result-outcome' +import { rootClaudeRowStamp, type ClaudeRowStamp } from './claude-provisional-row-corrections' export function claudeProviderFrameKind(message: Record): string { const type = claudeText(message.type) ?? 'unknown' @@ -113,12 +114,15 @@ export function createClaudeProviderFrameFallback( /** Runs only when a row is actually going to be written, so a frame that * translates to nothing never opens a turn. */ beforeAppend?: () => void, - options?: UnhandledProviderFrameJournalItemOptions + options?: UnhandledProviderFrameJournalItemOptions, + /** Attributes the row to the agent that produced the frame. Omitted for a + * frame the session's own agent produced. */ + stamp?: ClaudeRowStamp ) => boolean } { let sequence = 0 return { - append: (kind, payload, displayText, beforeAppend, options) => { + append: (kind, payload, displayText, beforeAppend, options, stamp) => { sequence += 1 const translated = unhandledProviderFrameJournalItem( 'claude', @@ -134,13 +138,12 @@ export function createClaudeProviderFrameFallback( const bounded = displayText ? boundInlineText(displayText, DEFAULT_JOURNAL_PAYLOAD_LIMITS).text : null - sink.appendItem( - { - provider: 'orca', - clientMessageId: `provider-frame:claude:${acquisitionId}:${sequence}` - }, - bounded ? { ...translated.body, text: bounded } : translated.body - ) + const identity = { + provider: 'orca', + clientMessageId: `provider-frame:claude:${acquisitionId}:${sequence}` + } as const + const body = bounded ? { ...translated.body, text: bounded } : translated.body + sink.appendItem(identity, body, (stamp ?? rootClaudeRowStamp)(identity, body)) sink.publish() return true } @@ -156,7 +159,8 @@ export function appendUnmodeledContent( fallback: ClaudeProviderFrameFallback, envelope: ClaudeMessageEnvelope, message: Record, - beforeAppend: () => void + beforeAppend: () => void, + stamp: ClaudeRowStamp ): boolean { let changed = false for (const part of envelope.content.filter((part) => !isModeledClaudeContent(part))) { @@ -166,13 +170,23 @@ export function appendUnmodeledContent( `message:${envelope.role}:content:${partType}`, part, readableProviderFrameText(part) ?? CLAUDE_UNRENDERABLE_CONTENT_TEXT, - beforeAppend + beforeAppend, + undefined, + stamp ) || changed } if (envelope.content.length === 0 && envelope.role === 'assistant') { // Empty provider placeholders do not prove work began, and may have no // later result capable of closing a turn. - changed = fallback.append(`message:${envelope.role}:empty`, message) || changed + changed = + fallback.append( + `message:${envelope.role}:empty`, + message, + undefined, + undefined, + undefined, + stamp + ) || changed } return changed } diff --git a/src/main/claude/claude-subagent-group-row.ts b/src/main/claude/claude-subagent-group-row.ts index 58af6b6b346..ab44350606e 100644 --- a/src/main/claude/claude-subagent-group-row.ts +++ b/src/main/claude/claude-subagent-group-row.ts @@ -7,6 +7,8 @@ import type { } from '../../shared/agent-session-journal-types' import { subagentGroupFallbackText } from '../../shared/native-chat-subagent-summary' import type { NativeChatSubagentEntry } from '../../shared/native-chat-types' +import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' +import type { RosterGroup } from './claude-subagent-roster-state' /** Durable journal identity for the group's row — stable across revisions and * across a restart, so replay finds the same row instead of appending a new one. */ @@ -30,3 +32,40 @@ export function claudeSubagentGroupBody( ] } } + +/** + * Revise one group's row to match its current children. + * + * Deliberately ROOT, and it is the one row in this lane where that needs saying: + * it is written from a child's frame but describes the PARENT's children, so it + * is the session's own agent reporting what it spawned. Stamping it as a child's + * would hide the roster from the very row that owns it. + */ +export function writeClaudeSubagentGroupRow( + sink: StructuredAgentSessionEventSink, + group: RosterGroup +): void { + const agents = [...group.entries.values()].map((tracked) => tracked.entry) + const options = { coalescingKey: `claude-subagents:${group.groupId}` } + if (agents.length === 0) { + // The row's last child turned out not to be a subagent. An empty roster is + // not a roster of nothing, so the row goes rather than reading "Ran 0". + if (group.lastSerialized !== null) { + group.lastSerialized = null + sink.appendTombstone(group.identity, options) + sink.publish() + } + return + } + const body = claudeSubagentGroupBody(group.groupId, agents) + const serialized = JSON.stringify(body) + if (serialized === group.lastSerialized) { + // Nothing changed — a duplicate delivery must not burn a revision. + return + } + group.lastSerialized = serialized + sink.appendItem(group.identity, body, options) + // Publish keeps the sink's own coalescing slot: sharing the row's key makes + // each queued publish evict the append it was meant to flush. + sink.publish() +} diff --git a/src/main/claude/claude-subagent-id-aliases.ts b/src/main/claude/claude-subagent-id-aliases.ts index d06bc00bf8d..97c162193be 100644 --- a/src/main/claude/claude-subagent-id-aliases.ts +++ b/src/main/claude/claude-subagent-id-aliases.ts @@ -24,6 +24,13 @@ export class ClaudeSubagentIds { return this.canonicalByToolUse.get(id) ?? id } + /** Whether an announcement has named this tool id. Distinct from `canonical` + * returning the id unchanged, which is also what an unknown id gets: only + * this says the identity behind the id is settled rather than provisional. */ + isAnnounced(toolUseId: string): boolean { + return this.canonicalByToolUse.has(toolUseId) + } + alias(toolUseId: string, taskId: string): void { if (!isBoundedClaudeTaskId(toolUseId) || !isBoundedClaudeTaskId(taskId)) { return diff --git a/src/main/claude/claude-subagent-linkage.ts b/src/main/claude/claude-subagent-linkage.ts new file mode 100644 index 00000000000..876da765137 --- /dev/null +++ b/src/main/claude/claude-subagent-linkage.ts @@ -0,0 +1,192 @@ +// Who produced a Claude journal row, decided from the roster's own knowledge. +// +// Separate from the roster because it answers a different question. The roster +// maintains the spawn-group row a user reads; this answers, for one frame's +// `parent_tool_use_id`, WHICH child produced the rows that frame carries. Never +// whether one did: a non-null reference already settles that. +// +// It prefers the CANONICAL task id over the tool id the frame arrived under. +// Claude re-announces a resumed task under a new tool id while its task id +// stays put, so a row stamped with the tool id splits one child into two the +// moment it resumes. That is a reason to prefer the task id, not to withhold a +// row until one exists: a row written under the tool id is re-stamped in place +// once the announcement lands, and a split child still beats a child whose +// words are filed under its parent. + +import type { AgentJournalProducerLinkage } from '../../shared/agent-session-journal-types' +import type { ClaudeSubagentIds } from './claude-subagent-id-aliases' + +/** What a frame's `parent_tool_use_id` says about the rows it produces. */ +export type ClaudeSubagentLinkageVerdict = + /** A subagent produced them, under an identity that will not change. */ + | { kind: 'linked'; linkage: AgentJournalProducerLinkage } + /** A subagent produced them under an identity that is not final yet. The rows + * are still written now — with `settledLinkageFor`'s stamp — and this is what + * marks them as owing a correction once the announcement lands. */ + | { kind: 'pending' } + +// There is deliberately no ROOT arm. A non-null `parent_tool_use_id` names a +// child, always — so every row it produces is a child's, and the only open +// question is which identity to stamp. Reading any of them as the session's own +// would assert the parent wrote words it did not. + +/** This resolver, as a write site asking who produced a row sees it. */ +export type ClaudeSubagentLinkageSource = { + linkageFor: (parentToolUseId: string) => ClaudeSubagentLinkageVerdict + settledLinkageFor: ( + parentToolUseId: string + ) => Exclude +} + +/** What the roster knows about one child, reduced to what attribution needs. */ +export type ClaudeSubagentLinkageEntry = { attempt: number } + +export type ClaudeSubagentLinkageDeps = { + ids: ClaudeSubagentIds + trackedFor: (canonicalId: string) => ClaudeSubagentLinkageEntry | null + /** Whether a tool id was forwarded at the TOP level. A child parented to one + * was spawned by a call the transcript shows, so an announcement naming it is + * still expected. Gates only whether a CORRECTION is owed, never whether the + * row is a child's, so a stale answer costs precision and not correctness. */ + isForwardedParentTool?: (toolUseId: string) => boolean + /** The reference naming the child that journaled a tool call, when a child + * did rather than the session's own agent. A grandchild's own + * `parent_tool_use_id` is one of those ids, and this is the only route from + * it to the agent that actually spawned the grandchild. */ + childOwnerRefOf?: (toolUseId: string) => string | null +} + +/** How far a sidechain is followed when naming a row's parent. Depth beyond + * this is past anything a transcript shows, and the guard is also what stops a + * malformed chain that points at itself from recursing. */ +const MAX_PARENT_RESOLUTION_DEPTH = 8 + +/** A parent's identity, or the fact that it is not final yet. `agentId` absent + * with kind `known` is the truthful claim that the session's own agent is the + * parent, which is what an unrecorded owner also means. */ +type ParentAgentVerdict = { kind: 'known'; agentId?: string } | { kind: 'pending' } + +export class ClaudeSubagentLinkage implements ClaudeSubagentLinkageSource { + constructor(private readonly deps: ClaudeSubagentLinkageDeps) {} + + linkageFor = (parentToolUseId: string): ClaudeSubagentLinkageVerdict => + this.resolve(parentToolUseId, false, 0) + + /** The verdict for rows that can wait no longer — the pre-announcement buffer + * draining on eviction, at turn settle, or at teardown. Never `pending`: + * under settle, a spawn call whose announcement never came resolves to its + * own raw id, which is the only handle that child will ever have. */ + settledLinkageFor = ( + parentToolUseId: string + ): Exclude => { + const verdict = this.resolve(parentToolUseId, true, 0) + return verdict.kind === 'pending' + ? linked(parentToolUseId, parentToolUseId, 'agent', null, undefined) + : verdict + } + + private resolve( + parentToolUseId: string, + settled: boolean, + depth: number + ): ClaudeSubagentLinkageVerdict { + const canonical = this.deps.ids.canonical(parentToolUseId) + // An announcement said this task is not a subagent — a backgrounded shell + // or a workflow. Its output is still not the session's own agent's. + const excluded = this.deps.ids.isExcluded(parentToolUseId, canonical) + // An announcement named this spawn call, so the task id behind it is + // settled — including the case where the two ids are the same string, which + // comparing them could not tell from never having been announced. + const announced = this.deps.ids.isAnnounced(parentToolUseId) + if ( + !settled && + !excluded && + !announced && + this.deps.isForwardedParentTool?.(parentToolUseId) === true + ) { + // A top-level spawn call whose `task_started` has not landed yet. Its rows + // are written immediately under the id it already has and re-attributed + // when the announcement names it; `pending` is what marks them as owing + // that correction. Nothing WAITS on this — it only decides whether a + // correction is owed — so a reference this misses costs a row the + // canonical id, never its author. + return { kind: 'pending' } + } + // A row names its parent as well as its producer, and it persists only once + // BOTH are final: a sidechain call's parent is another agent, whose own + // identity can still be provisional. + const parent = this.parentAgentFor(parentToolUseId, settled, depth) + if (parent.kind === 'pending') { + return { kind: 'pending' } + } + if (excluded) { + return linked(parentToolUseId, canonical, 'background', null, parent.agentId) + } + if (announced) { + return linked( + parentToolUseId, + canonical, + 'agent', + this.deps.trackedFor(canonical), + parent.agentId + ) + } + // Nothing is coming for this id: nested tool traffic, a grandchild inside a + // sidechain, or a forwarded spawn call that can wait no longer. The raw + // reference is the only handle there will ever be for it. + return linked( + parentToolUseId, + canonical, + 'agent', + this.deps.trackedFor(canonical), + parent.agentId + ) + } + + /** Who spawned the agent this reference names, resolved through the same path + * that reference's own rows resolve through — so a parent id always matches + * the `agentId` the parent's own rows carry, however either was settled. */ + private parentAgentFor( + parentToolUseId: string, + settled: boolean, + depth: number + ): ParentAgentVerdict { + const ownerRef = this.deps.childOwnerRefOf?.(parentToolUseId) ?? null + if (ownerRef === null || depth >= MAX_PARENT_RESOLUTION_DEPTH) { + return { kind: 'known' } + } + const owner = this.resolve(ownerRef, settled, depth + 1) + if (owner.kind === 'pending') { + return { kind: 'pending' } + } + return owner.kind === 'linked' && owner.linkage.agentId !== undefined + ? { kind: 'known', agentId: owner.linkage.agentId } + : { kind: 'known' } + } +} + +function linked( + parentToolUseId: string, + agentId: string, + producerKind: NonNullable, + tracked: ClaudeSubagentLinkageEntry | null, + parentAgentId: string | undefined +): Extract { + return { + kind: 'linked', + linkage: { + agentId, + // Absent means the session's own agent spawned this one, so it is only + // ever written when ANOTHER agent is known to have. A malformed chain + // that loops back names the agent its own ancestor; the depth guard + // bounds that walk but cannot make its answer mean anything, and absence + // is the truthful claim rather than a self-parent persisted for ever. + ...(parentAgentId === undefined || parentAgentId === agentId ? {} : { parentAgentId }), + providerParentRef: parentToolUseId, + producerKind, + // The first run is the absence of an attempt, like every other field + // here: absence is the claim, so only a reopened run states one. + ...(tracked && tracked.attempt > 1 ? { attempt: tracked.attempt } : {}) + } + } +} diff --git a/src/main/claude/claude-subagent-roster-state.ts b/src/main/claude/claude-subagent-roster-state.ts index 2fa8971d856..f7fc3374661 100644 --- a/src/main/claude/claude-subagent-roster-state.ts +++ b/src/main/claude/claude-subagent-roster-state.ts @@ -14,6 +14,9 @@ export type TrackedEntry = { /** Label before its ordinal suffix, so a later announcement can tell a * provisional row from one that already carries the provider's own name. */ labelBase: string + /** Which run of this child. Identity survives a resume by design, so without + * this the retained rows of two runs read as one uninterrupted timeline. */ + attempt: number } export type RosterGroup = { @@ -48,6 +51,10 @@ export function applyClaudeSubagentInvocation( } tracked.invocationIds.add(frame.toolUseId) if (tracked.toolUseId !== null && tracked.toolUseId !== frame.toolUseId) { + // THE reactivation: a new spawn alias reopening this entry. The one place + // the attempt moves, and it is gated on the observed alias change rather + // than on the counter, so a late duplicate cannot advance a settled run. + tracked.attempt += 1 tracked.backgrounded = frame.backgrounded ?? false tracked.entry = { ...tracked.entry, state: frame.state ?? 'working', settledAt: undefined } } diff --git a/src/main/claude/claude-subagent-roster.ts b/src/main/claude/claude-subagent-roster.ts index 34083f2ee8c..d45a17e15e9 100644 --- a/src/main/claude/claude-subagent-roster.ts +++ b/src/main/claude/claude-subagent-roster.ts @@ -18,8 +18,12 @@ import { import type { NativeChatSubagentEntry } from '../../shared/native-chat-types' import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' import { isBoundedClaudeTaskId } from './claude-background-task-tracker' -import { claudeSubagentGroupBody, claudeSubagentGroupIdentity } from './claude-subagent-group-row' +import { + claudeSubagentGroupIdentity, + writeClaudeSubagentGroupRow +} from './claude-subagent-group-row' import { ClaudeSubagentIds } from './claude-subagent-id-aliases' +import { ClaudeSubagentLinkage, type ClaudeSubagentLinkageSource } from './claude-subagent-linkage' import { readClaudeSubagentTaskFrame } from './claude-subagent-task-frames' import { applyClaudeSubagentInvocation, @@ -42,6 +46,18 @@ export type ClaudeSubagentRosterDeps = { sink: StructuredAgentSessionEventSink /** The turn that owns children spawned right now; null outside any turn. */ currentGroupKey: () => string | null + /** Whether a tool id was forwarded at the TOP level. A child parented to one + * was spawned by a call the transcript shows, so its announcement is still + * expected; a child parented to anything else names an id that only ever + * existed inside a sidechain, which this CLI will never announce. */ + isForwardedParentTool?: (toolUseId: string) => boolean + /** The reference naming the child that journaled a tool call, when a child + * did. It is how a grandchild's row reaches the agent that spawned it. */ + childOwnerRefOf?: (toolUseId: string) => string | null + /** A settled group can receive no further announcement, so an identity still + * provisional will stay that way. Fires on EVERY settle path, so a caller + * holding rows against a pending identity cannot miss one. */ + onIdentitiesFinal?: () => void now?: () => number } @@ -51,6 +67,8 @@ export class ClaudeSubagentRoster { * from an earlier turn revises that turn's row instead of the live one. */ private readonly groupIdByEntry = new Map() private readonly ids = new ClaudeSubagentIds() + /** Who produced a row, for every write site journaling this session. */ + readonly linkage: ClaudeSubagentLinkageSource /** Set by ANY `task_started`, including one the subagent filter rejects. Once * this CLI has proven it declares its tasks, child traffic for an id it never * announced is a nested tool or a grandchild, not a subagent. */ @@ -59,6 +77,12 @@ export class ClaudeSubagentRoster { constructor(private readonly deps: ClaudeSubagentRosterDeps) { this.now = deps.now ?? (() => Date.now()) + this.linkage = new ClaudeSubagentLinkage({ + ids: this.ids, + trackedFor: (canonicalId) => this.locate(canonicalId)?.tracked ?? null, + isForwardedParentTool: deps.isForwardedParentTool, + childOwnerRefOf: deps.childOwnerRefOf + }) } /** Consume a `message:system:task_*` frame. Returns false when it is not one. */ @@ -177,6 +201,7 @@ export class ClaudeSubagentRoster { // unrelated turn ending is no evidence about a child announced outside it. // `settleSession` reaches what no turn does. this.sweep(this.groups.get(groupKey ?? OUTSIDE_TURN), false) + this.deps.onIdentitiesFinal?.() } /** The provider is gone. Nothing more will arrive for any child, backgrounded @@ -185,6 +210,7 @@ export class ClaudeSubagentRoster { for (const group of this.groups.values()) { this.sweep(group, true) } + this.deps.onIdentitiesFinal?.() } dispose(): void { @@ -217,7 +243,7 @@ export class ClaudeSubagentRoster { changed = true } if (changed) { - this.write(group) + writeClaudeSubagentGroupRow(this.deps.sink, group) } } @@ -240,6 +266,7 @@ export class ClaudeSubagentRoster { toolUseId, invocationIds: new Set(toolUseId ? [toolUseId] : []), labelBase, + attempt: 1, entry: { id, label: claimClaudeSubagentLabel(group, labelBase), @@ -249,7 +276,7 @@ export class ClaudeSubagentRoster { } }) this.groupIdByEntry.set(id, group.groupId) - this.write(group) + writeClaudeSubagentGroupRow(this.deps.sink, group) } private revise( @@ -288,7 +315,7 @@ export class ClaudeSubagentRoster { } } group.entries.set(id, next) - this.write(group) + writeClaudeSubagentGroupRow(this.deps.sink, group) } /** Re-key a provisional entry from its tool id onto the canonical task id the @@ -318,7 +345,7 @@ export class ClaudeSubagentRoster { } located.group.entries.delete(id) this.groupIdByEntry.delete(id) - this.write(located.group) + writeClaudeSubagentGroupRow(this.deps.sink, located.group) } private locate(id: string): { group: RosterGroup; tracked: TrackedEntry } | null { @@ -359,30 +386,4 @@ export class ClaudeSubagentRoster { } return group } - - private write(group: RosterGroup): void { - const agents = [...group.entries.values()].map((tracked) => tracked.entry) - const options = { coalescingKey: `claude-subagents:${group.groupId}` } - if (agents.length === 0) { - // The row's last child turned out not to be a subagent. An empty roster is - // not a roster of nothing, so the row goes rather than reading "Ran 0". - if (group.lastSerialized !== null) { - group.lastSerialized = null - this.deps.sink.appendTombstone(group.identity, options) - this.deps.sink.publish() - } - return - } - const body = claudeSubagentGroupBody(group.groupId, agents) - const serialized = JSON.stringify(body) - if (serialized === group.lastSerialized) { - // Nothing changed — a duplicate delivery must not burn a revision. - return - } - group.lastSerialized = serialized - this.deps.sink.appendItem(group.identity, body, options) - // Publish keeps the sink's own coalescing slot: sharing the row's key makes - // each queued publish evict the append it was meant to flush. - this.deps.sink.publish() - } } diff --git a/src/main/claude/claude-tool-origin-registry.ts b/src/main/claude/claude-tool-origin-registry.ts new file mode 100644 index 00000000000..2bf4445d84e --- /dev/null +++ b/src/main/claude/claude-tool-origin-registry.ts @@ -0,0 +1,78 @@ +// Where each Claude tool call this session journaled came from. +// +// Two questions, one registry, because both are answered by the same fact — +// which agent's row carried a tool call: +// +// A task announces the tool call that spawned it. That tool call is only +// evidence the user can act on when it was forwarded at the TOP level: a nested +// Task spawned from inside a subagent's sidechain names a tool id that exists +// only in that sidechain, and a row minted for it would claim a top-level +// invocation that never appeared. So admission asks this registry, and a task +// whose parent was never forwarded yields no row at all. +// +// The same sidechain tool id is also the only handle a GRANDCHILD's frames +// carry. Recording which child journaled it is what lets a grandchild row name +// its real parent instead of leaving the field absent, which under this +// journal's semantics would claim the session's own agent spawned it. + +/** Both stores are event-accumulated and pruned by nothing, so both are + * bounded. Eviction is oldest first: a tool id old enough to fall out can no + * longer be the parent of a task announcement still in flight. They are kept + * separate so that heavy sidechain traffic cannot evict the top-level spawn + * ids an announcement is still expected for. */ +const MAX_TOP_LEVEL_TOOL_IDS = 512 +const MAX_CHILD_TOOL_ORIGINS = 512 + +export class ClaudeToolOriginRegistry { + private readonly topLevel = new Set() + private readonly childOwnerRefs = new Map() + + /** Record a tool call journaled at the top level, by the session's own agent. */ + recordTopLevel(toolUseId: string): void { + if (toolUseId.length === 0) { + return + } + this.topLevel.delete(toolUseId) + this.topLevel.add(toolUseId) + while (this.topLevel.size > MAX_TOP_LEVEL_TOOL_IDS) { + const oldest = this.topLevel.values().next() + if (oldest.done || oldest.value === toolUseId) { + break + } + this.topLevel.delete(oldest.value) + } + } + + /** Record a tool call journaled by a CHILD, against the reference that names + * the child. The reference, not an identity: the child's own identity may + * still be provisional here, and is resolved when a row is stamped. */ + recordChildOwned(toolUseId: string, ownerRef: string): void { + if (toolUseId.length === 0 || ownerRef.length === 0) { + return + } + this.childOwnerRefs.delete(toolUseId) + this.childOwnerRefs.set(toolUseId, ownerRef) + while (this.childOwnerRefs.size > MAX_CHILD_TOOL_ORIGINS) { + const oldest = this.childOwnerRefs.keys().next() + if (oldest.done || oldest.value === toolUseId) { + break + } + this.childOwnerRefs.delete(oldest.value) + } + } + + has(toolUseId: string): boolean { + return this.topLevel.has(toolUseId) + } + + /** The reference naming the child that journaled this tool call, or null when + * no child did — either the session's own agent did, or it was never seen. */ + childOwnerRef(toolUseId: string): string | null { + return this.childOwnerRefs.get(toolUseId) ?? null + } + + clear(): void { + this.topLevel.clear() + this.childOwnerRefs.clear() + } +} diff --git a/src/main/claude/claude-turn-opening.ts b/src/main/claude/claude-turn-opening.ts index 55adfcfad3f..1406928b37e 100644 --- a/src/main/claude/claude-turn-opening.ts +++ b/src/main/claude/claude-turn-opening.ts @@ -59,6 +59,19 @@ export function isRootClaudeFrame(frame: Record): boolean { return typeof frame.parent_tool_use_id !== 'string' } +/** The parent this frame names, or null when it names none. + * + * Deliberately STRICTER than `isRootClaudeFrame` above, which asks only whether + * the field is a string: an empty string is a string but names no parent, and + * attribution must not mint a producer out of it. The two therefore disagree on + * `''` — and this is the side that decides who produced a row, where treating + * `''` as a parent would stamp an id no reader could ever resolve. */ +export function claudeFrameParentRef(frame: Record): string | null { + return typeof frame.parent_tool_use_id === 'string' && frame.parent_tool_use_id.length > 0 + ? frame.parent_tool_use_id + : null +} + export type ClaudeTurnSource = { sessionId: string; uuid: string; assistant: boolean } /** Reads a turn source off a raw frame, for the streamed path that has no envelope. */ diff --git a/src/main/native-chat/agent-session-journal/journal-item-appender.ts b/src/main/native-chat/agent-session-journal/journal-item-appender.ts index 1bbc3d4fab3..e0bf1ad6238 100644 --- a/src/main/native-chat/agent-session-journal/journal-item-appender.ts +++ b/src/main/native-chat/agent-session-journal/journal-item-appender.ts @@ -5,11 +5,9 @@ import type { } from '../../../shared/agent-session-journal-types' import { journalItemRowBuilder } from './journal-row-builders' import type { JournalReducerState } from './journal-reducer' -import type { JournalAppendResult } from './journal-store-contracts' +import type { JournalAppendResult, JournalItemAppendOptions } from './journal-store-contracts' import type { JournalRow } from './journal-row-schema' -type ItemAppendOptions = { fence: number; observedAt?: number; recovered?: true } - export class JournalItemAppender { constructor( private readonly deps: { @@ -21,7 +19,7 @@ export class JournalItemAppender { append( identity: AgentJournalItemIdentity, body: AgentJournalItemBody, - options: ItemAppendOptions + options: JournalItemAppendOptions ): Promise { const itemId = agentJournalItemKey(identity) return this.deps diff --git a/src/main/native-chat/agent-session-journal/journal-reducer.test.ts b/src/main/native-chat/agent-session-journal/journal-reducer.test.ts index 184e4a19a01..1a0b00e5d85 100644 --- a/src/main/native-chat/agent-session-journal/journal-reducer.test.ts +++ b/src/main/native-chat/agent-session-journal/journal-reducer.test.ts @@ -658,3 +658,142 @@ describe('re-adding a tombstoned row', () => { expect(renderJournalState(state).items).toEqual([]) }) }) + +describe('producer linkage round-trips through the reducer', () => { + const identity: AgentJournalItemIdentity = { + provider: 'claude', + sessionId: 'claude-session', + uuid: 'child-1' + } + const linkage = { + agentId: 'task-1', + parentAgentId: 'task-parent', + providerParentRef: 'toolu_1', + producerKind: 'agent' as const, + attempt: 2 + } + + it('copies the whole bundle onto the render item on the plain item path', () => { + const state = createJournalReducerState('session-1', EPOCH) + applyJournalRow( + state, + buildJournalItemRow({ + state, + identity, + body: text('looking'), + seq: 1, + fence: 1, + ts: 1_001, + linkage + }) + ) + expect(renderJournalState(state).items[0]).toMatchObject(linkage) + }) + + it('copies it on the lifecycle-batch path too, which is a separate upsert', () => { + const state = createJournalReducerState('session-1', EPOCH) + applyJournalRow(state, { + kind: 'lifecycle-batch', + settlementId: 'settle-1', + mutations: [{ kind: 'item', itemId: 'i-child', revision: 1, body: text('looking') }], + ...base(1), + ...linkage + }) + expect(renderJournalState(state).items[0]).toMatchObject(linkage) + }) + + it('lets a correction win over the provisional row, without moving the bubble', () => { + // Write-through then correct: the row is written under the spawn call's own + // id, then re-appended under the canonical one. Revision is assigned inside + // the journal's serialized write step, so the later append always outranks + // — and `sequence`/`observedAt` stay pinned, so re-attributing a row does + // not relocate it in the timeline. + const state = createJournalReducerState('session-1', EPOCH) + const provisional = { agentId: 'toolu_1', providerParentRef: 'toolu_1' } + applyJournalRow( + state, + buildJournalItemRow({ + state, + identity, + body: text('looking'), + seq: 1, + fence: 1, + ts: 1_001, + linkage: provisional + }) + ) + applyJournalRow( + state, + buildJournalItemRow({ + state, + identity, + body: text('looking'), + seq: 9, + fence: 1, + ts: 9_999, + linkage + }) + ) + + const items = renderJournalState(state).items + expect(items).toHaveLength(1) + expect(items[0]).toMatchObject({ revision: 2, ...linkage }) + expect(items[0]).toMatchObject({ sequence: 1, observedAt: 1_001 }) + }) + + it('does not let a stale checkpoint undo a correction that already landed', () => { + // A text checkpoint carrying the OLD stamp, submitted after the correction, + // would re-root the row. It cannot: revision is read at write time, so the + // last write wins and the lane resolves linkage fresh on every checkpoint. + const state = createJournalReducerState('session-1', EPOCH) + applyJournalRow( + state, + buildJournalItemRow({ state, identity, body: text('a'), seq: 1, fence: 1, ts: 1, linkage }) + ) + applyJournalRow( + state, + buildJournalItemRow({ + state, + identity, + body: text('a and more'), + seq: 2, + fence: 1, + ts: 2, + linkage + }) + ) + const items = renderJournalState(state).items + expect(items[0]).toMatchObject({ revision: 2, ...linkage }) + }) + + it('keeps linkage when a later revision rewrites the row', () => { + // The resolved-append path lost the marker once before by rebuilding the + // row without it, so the SECOND write is the one that matters here. + const state = createJournalReducerState('session-1', EPOCH) + for (const [seq, body] of [ + [1, text('look')], + [2, text('looking at the lane')] + ] as const) { + applyJournalRow( + state, + buildJournalItemRow({ state, identity, body, seq, fence: 1, ts: 1_000 + seq, linkage }) + ) + } + const items = renderJournalState(state).items + expect(items).toHaveLength(1) + expect(items[0]).toMatchObject({ revision: 2, ...linkage }) + }) + + it("renders a row that predates linkage as the session's own", () => { + const state = createJournalReducerState('session-1', EPOCH) + applyJournalRow(state, { + kind: 'item', + itemId: 'i-legacy', + revision: 1, + body: text('written before linkage existed'), + ...base(1) + }) + const item = renderJournalState(state).items[0] + expect(item && 'agentId' in item).toBe(false) + }) +}) diff --git a/src/main/native-chat/agent-session-journal/journal-reducer.ts b/src/main/native-chat/agent-session-journal/journal-reducer.ts index 5c4917bec46..03352d8e9a5 100644 --- a/src/main/native-chat/agent-session-journal/journal-reducer.ts +++ b/src/main/native-chat/agent-session-journal/journal-reducer.ts @@ -13,6 +13,7 @@ import type { AgentJournalSnapshot, AgentJournalSubmission } from '../../../shared/agent-session-journal-types' +import { journalRenderItem } from './journal-render-item' import { agentJournalSubmissionKey, parseAgentJournalItemKey @@ -73,14 +74,7 @@ export function applyJournalRow(state: JournalReducerState, row: JournalRow): vo } const itemId = resolveJournalItemId(state, row.itemId, row.body) acceptSubmissionFromProviderItem(state, row.itemId, itemId, row) - upsertItem(state, itemId, row.revision, { - itemId, - revision: row.revision, - body: row.body, - sequence: row.seq, - observedAt: row.ts, - ...(row.recovered ? { recovered: row.recovered } : {}) - }) + upsertItem(state, itemId, row.revision, journalRenderItem(itemId, row.revision, row.body, row)) return } if (row.kind === 'tombstone') { @@ -98,14 +92,12 @@ export function applyJournalRow(state: JournalReducerState, row: JournalRow): vo } const itemId = resolveJournalItemId(state, mutation.itemId, mutation.body) acceptSubmissionFromProviderItem(state, mutation.itemId, itemId, row) - upsertItem(state, itemId, mutation.revision, { + upsertItem( + state, itemId, - revision: mutation.revision, - body: mutation.body, - sequence: row.seq, - observedAt: row.ts, - ...(row.recovered ? { recovered: row.recovered } : {}) - }) + mutation.revision, + journalRenderItem(itemId, mutation.revision, mutation.body, row) + ) } else { removeItem(state, resolveItemId(state, mutation.itemId), mutation.revision) } @@ -251,13 +243,7 @@ function applySubmission( resolvedAt: null }) const itemId = agentJournalSubmissionKey(row.clientMessageId) - upsertItem(state, itemId, 0, { - itemId, - revision: 0, - body: row.body, - sequence: row.seq, - observedAt: row.ts - }) + upsertItem(state, itemId, 0, journalRenderItem(itemId, 0, row.body, row)) } function applyDispatch( diff --git a/src/main/native-chat/agent-session-journal/journal-render-item.ts b/src/main/native-chat/agent-session-journal/journal-render-item.ts new file mode 100644 index 00000000000..3324ffedbd1 --- /dev/null +++ b/src/main/native-chat/agent-session-journal/journal-render-item.ts @@ -0,0 +1,27 @@ +import type { + AgentJournalItemBody, + AgentJournalRenderItem +} from '../../../shared/agent-session-journal-types' +import { agentJournalLinkageFields } from '../../../shared/agent-session-journal-producer' +import type { JournalRow } from './journal-row-schema' + +/** One render item, built the same way by every upsert path in the reducer. + * The row-level markers are copied here rather than at each call site: they + * were three separate spreads that had to stay in sync, and absence is the + * claim in each case — appended live, and produced by the session's own agent. */ +export function journalRenderItem( + itemId: string, + revision: number, + body: AgentJournalItemBody, + row: JournalRow +): AgentJournalRenderItem { + return { + itemId, + revision, + body, + sequence: row.seq, + observedAt: row.ts, + ...(row.recovered ? { recovered: row.recovered } : {}), + ...agentJournalLinkageFields(row) + } +} diff --git a/src/main/native-chat/agent-session-journal/journal-row-builders.ts b/src/main/native-chat/agent-session-journal/journal-row-builders.ts index e5e376940fe..ba06f76792f 100644 --- a/src/main/native-chat/agent-session-journal/journal-row-builders.ts +++ b/src/main/native-chat/agent-session-journal/journal-row-builders.ts @@ -3,9 +3,11 @@ import type { AgentJournalItemBody, AgentJournalItemIdentity, AgentJournalMessageItem, + AgentJournalProducerLinkage, AgentSessionProviderHandle } from '../../../shared/agent-session-journal-types' import { journalRowSchemaVersion } from '../../../shared/agent-session-journal-types' +import { agentJournalLinkageFields } from '../../../shared/agent-session-journal-producer' import { agentJournalItemKey } from '../../../shared/agent-session-journal-item-key' import type { JournalReducerState } from './journal-reducer' import type { @@ -29,7 +31,7 @@ export function journalItemRowBuilder( state: () => JournalReducerState, identity: AgentJournalItemIdentity, body: AgentJournalItemBody, - options: { fence: number; observedAt?: number; recovered?: true } + options: AgentJournalProducerLinkage & { fence: number; observedAt?: number; recovered?: true } ): RowBuilder { return (seq, ts) => buildJournalItemRow({ @@ -39,7 +41,8 @@ export function journalItemRowBuilder( seq, fence: options.fence, ts: options.observedAt ?? ts, - recovered: options.recovered + recovered: options.recovered, + linkage: options }) } @@ -104,6 +107,11 @@ export function journalLifecycleBatchRowBuilder( state: () => JournalReducerState, settlementId: string, mutations: readonly JournalLifecycleMutationInput[], + /** No producer linkage: one batch row covers N mutations, so a row-level + * producer would stamp whoever opened the batch onto every one of them. The + * reducer still READS linkage off a batch row, because a row may come from a + * host that writes one; a mixed-producer batch would have to stamp per + * mutation, which nothing needs yet. */ options: { fence: number; recovered?: true } ): RowBuilder { return (seq, ts) => { @@ -164,6 +172,7 @@ export function buildJournalItemRow(input: { fence: number ts: number recovered?: true + linkage?: AgentJournalProducerLinkage }): JournalItemRow { const itemId = agentJournalItemKey(input.identity) const resolved = input.state.aliases.get(itemId) ?? itemId @@ -180,7 +189,8 @@ export function buildJournalItemRow(input: { revision, body: input.body, ...journalRowBase(input.state.epoch, input.seq, input.fence, input.ts, [input.body]), - ...(input.recovered ? { recovered: input.recovered } : {}) + ...(input.recovered ? { recovered: input.recovered } : {}), + ...agentJournalLinkageFields(input.linkage) } } diff --git a/src/main/native-chat/agent-session-journal/journal-row-schema.test.ts b/src/main/native-chat/agent-session-journal/journal-row-schema.test.ts index 4ba1ad349a5..88a81e070f2 100644 --- a/src/main/native-chat/agent-session-journal/journal-row-schema.test.ts +++ b/src/main/native-chat/agent-session-journal/journal-row-schema.test.ts @@ -1,6 +1,12 @@ import { describe, expect, it } from 'vitest' import { AGENT_SESSION_JOURNAL_SCHEMA_VERSION } from '../../../shared/agent-session-journal-types' -import { MAX_JOURNAL_LIFECYCLE_BATCH_MUTATIONS, parseJournalRow } from './journal-row-schema' +import { + MAX_JOURNAL_LIFECYCLE_BATCH_MUTATIONS, + parseJournalRow, + type JournalRow +} from './journal-row-schema' +import { createJournalReducerState } from './journal-reducer' +import { buildJournalItemRow } from './journal-row-builders' const BASE = { v: 1, epoch: 'epoch-1', seq: 1, fence: 1, ts: 1 } @@ -238,3 +244,158 @@ describe('journal row validation', () => { ).toBe(false) }) }) + +describe('producer linkage on the persisted row', () => { + const identity = { provider: 'claude' as const, sessionId: 'claude-session', uuid: 'u-1' } + const body = { kind: 'status' as const, text: 'child work' } + const linkage = { + agentId: 'task-1', + parentAgentId: 'task-parent', + providerParentRef: 'toolu_1', + producerKind: 'agent' as const, + attempt: 2 + } + const resolution = { + state: 'pending', + selectedOptionId: null, + resolvedBy: null, + resolvedAt: null + } + + /** The full durable path: build the row the appender would write, serialize it + * the way the journal file does, and read it back. */ + function roundTrip(withLinkage: boolean): JournalRow | null { + const state = createJournalReducerState('session-1', 'epoch-1') + const row = buildJournalItemRow({ + state, + identity, + body, + seq: 1, + fence: 1, + ts: 1_700_000_000_000, + ...(withLinkage ? { linkage } : {}) + }) + const parsed = parseJournalRow(JSON.stringify(row)) + return parsed.ok ? parsed.row : null + } + + it('writes and reads the bundle back without bumping the schema version', () => { + const row = roundTrip(true) + expect(row).toMatchObject(linkage) + // Deliberately NOT a version bump: an unknown `v` is unreadable and latches + // the host read-only, while an unknown KEY is simply ignored by an older host. + expect(row?.v).toBe(AGENT_SESSION_JOURNAL_SCHEMA_VERSION) + }) + + /** A row this host did not write: a remote peer's, or a corrupted line. + * Narrowed to the item arm it always builds, so a caller can read `body` + * without re-discriminating a union of six. */ + function parseForeign( + overrides: Record + ): Extract | null { + const parsed = parseJournalRow( + JSON.stringify({ + v: AGENT_SESSION_JOURNAL_SCHEMA_VERSION, + epoch: 'epoch-1', + seq: 7, + fence: 1, + ts: 1_700_000_000_000, + kind: 'item', + itemId: 'claude:claude-session:u-1', + revision: 0, + body, + ...overrides + }) + ) + return parsed.ok && parsed.row.kind === 'item' ? parsed.row : null + } + + it('keeps the row but drops an empty agentId, which would read as a subagent', () => { + // Presence, not truthiness: `''` left in place hides the row from its own + // author on every parent-scoped surface, permanently and with no backfill. + const row = parseForeign({ agentId: '' }) + expect(row).not.toBeNull() + expect(row && 'agentId' in row).toBe(false) + }) + + it('keeps the row but drops a wrong-typed linkage field', () => { + const row = parseForeign({ agentId: 42, attempt: 'two', producerKind: '' }) + expect(row).not.toBeNull() + expect(row && 'agentId' in row).toBe(false) + expect(row && 'attempt' in row).toBe(false) + expect(row && 'producerKind' in row).toBe(false) + }) + + it('never lets a bad linkage field reject the row itself', () => { + // A row validator that REJECTS is a whole-store kill switch: the row leaves + // the timeline entirely. The content must survive its own bad metadata. + const row = parseForeign({ agentId: '', parentAgentId: null, providerParentRef: 7 }) + expect(row?.body).toEqual(body) + expect(row?.seq).toBe(7) + }) + + it('leaves a well-formed foreign linkage bundle untouched', () => { + // The positive control: the sanitizer must not be dropping everything. + expect(parseForeign({ agentId: 'task-9', attempt: 3 })).toMatchObject({ + agentId: 'task-9', + attempt: 3 + }) + }) + + it("omits every key on a row the session's own agent produced", () => { + const row = roundTrip(false) + expect(row && 'agentId' in row).toBe(false) + expect(row && 'producerKind' in row).toBe(false) + }) + + it('accepts a real pre-linkage journal line, which carries no bundle at all', () => { + // A literal line rather than a constructed row, so this also pins that no + // unknown-key rejection crept in. + const legacy = + '{"v":3,"epoch":"epoch-1","seq":7,"fence":1,"ts":1700000000000,"kind":"item",' + + '"itemId":"i-1","revision":1,"body":{"kind":"message","role":"assistant",' + + '"blocks":[{"type":"text","text":"hello"}]}}' + const parsed = parseJournalRow(legacy) + expect(parsed.ok).toBe(true) + expect(parsed.ok && 'agentId' in parsed.row).toBe(false) + }) + + it('leaves a strict prompt shape able to parse, because linkage rides the row', () => { + const question = { + ...BASE, + v: 3, + kind: 'item', + itemId: 'i-q', + revision: 1, + ...linkage, + body: { + kind: 'question', + question: 'Which lane?', + options: [{ id: 'o-1', label: 'First' }], + resolution + } + } + const parsed = parseJournalRow(JSON.stringify(question)) + expect(parsed.ok).toBe(true) + }) + + it('rejects the same row when linkage is put INSIDE the strict shape', () => { + // The positive control for the test above: proof the strictness it avoids is + // real, rather than the row parsing for some unrelated reason. This is why + // the bundle rides the row base and never a body. + const smuggled = { + ...BASE, + v: 3, + kind: 'item', + itemId: 'i-q', + revision: 1, + body: { + kind: 'question', + question: 'Which lane?', + options: [{ id: 'o-1', label: 'First', agentId: 'task-1' }], + resolution + } + } + expect(parseJournalRow(JSON.stringify(smuggled)).ok).toBe(false) + }) +}) diff --git a/src/main/native-chat/agent-session-journal/journal-row-schema.ts b/src/main/native-chat/agent-session-journal/journal-row-schema.ts index 7dc02dd197b..2f2dd8ceb7f 100644 --- a/src/main/native-chat/agent-session-journal/journal-row-schema.ts +++ b/src/main/native-chat/agent-session-journal/journal-row-schema.ts @@ -10,6 +10,7 @@ import { type AgentJournalDispatchState, type AgentJournalItemBody, type AgentJournalMessageItem, + type AgentJournalProducerLinkage, type AgentSessionProviderHandle } from '../../../shared/agent-session-journal-types' import { @@ -17,7 +18,13 @@ import { isAdmissibleAgentJournalMessageBody } from '../../../shared/agent-session-journal-schemas' -type JournalRowBase = { +/** Producer linkage rides the row BASE rather than the body: the two nested + * prompt shapes are `.strict()`, so an unknown key on a body would make the + * whole row parse as malformed. It is also deliberately not a `v` bump — an + * unknown `v` makes a row unreadable and latches the host read-only, while an + * unknown KEY is ignored below, so an older host reads a stamped row and + * behaves exactly as it does today. */ +type JournalRowBase = AgentJournalProducerLinkage & { /** Schema version of THIS row. */ v: number epoch: string @@ -150,9 +157,31 @@ export function parseJournalRow(line: string): JournalRowParse { return { ok: false, unreadable: true } } const upcast = upcastRow(record, version) + dropUnusableProducerLinkage(upcast) return isJournalRow(upcast) ? { ok: true, row: upcast } : { ok: false, unreadable: false } } +/** Linkage ids this build cannot trust, removed from a row it still keeps. + * + * Deliberately NOT part of `isJournalRow`: rejecting a row there drops it from + * the timeline, so a validator tightened against one bad field becomes a + * whole-store kill switch. Dropping the field degrades the row to the + * session's own agent — what every row said before linkage existed — while + * keeping the content, which is always the safer direction. An `agentId` that + * survives is a real one: the reader scopes on PRESENCE, so `''` or a + * non-string left in place would hide the row from its own author for good. */ +function dropUnusableProducerLinkage(record: Record): void { + for (const field of ['agentId', 'parentAgentId', 'providerParentRef', 'producerKind']) { + const value = record[field] + if (value !== undefined && (typeof value !== 'string' || value.length === 0)) { + delete record[field] + } + } + if (record.attempt !== undefined && !Number.isInteger(record.attempt)) { + delete record.attempt + } +} + /** Read-time upcast chain. Each step raises a row exactly one version. */ function upcastRow(record: Record, version: number): Record { let current = record diff --git a/src/main/native-chat/agent-session-journal/journal-store-contracts.ts b/src/main/native-chat/agent-session-journal/journal-store-contracts.ts index 80c806b02e3..cea47e381fe 100644 --- a/src/main/native-chat/agent-session-journal/journal-store-contracts.ts +++ b/src/main/native-chat/agent-session-journal/journal-store-contracts.ts @@ -3,6 +3,7 @@ import type { AgentJournalItemBody, AgentJournalItemIdentity, AgentJournalMessageItem, + AgentJournalProducerLinkage, AgentJournalResetReason, AgentSessionJournalIdentity } from '../../../shared/agent-session-journal-types' @@ -39,7 +40,11 @@ export type JournalAppendResult = { revision: number } -export type JournalItemAppendOptions = { fence: number; observedAt?: number; recovered?: true } +export type JournalItemAppendOptions = AgentJournalProducerLinkage & { + fence: number + observedAt?: number + recovered?: true +} export type JournalTombstoneInput = { fence: number } export type JournalLifecycleBatchInput = { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.test.ts index 8a2485473f8..d9f567a8a75 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.test.ts @@ -5,6 +5,10 @@ import type { } from '../../../shared/agent-session-journal-types' import type { AgentSessionTurnActivity } from '../../../shared/agent-session-wire' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' +import type { + JournalItemAppendOptions, + JournalLifecycleBatchInput +} from '../agent-session-journal/journal-store-contracts' import { createDeferredStructuredAgentSessionEventSink, type StructuredAgentSessionEventTarget @@ -29,20 +33,35 @@ type Recorded = { activity?: AgentSessionTurnActivity | null } +/** The journal's OWN append options, captured beside the call log rather than on + * it: a double that omits the third parameter makes every assertion about what + * the sink forwards pass against `undefined`, which is how this went unnoticed + * before. Kept separate so the call-order assertions stay about call order. */ +const journalAppendOptions: JournalItemAppendOptions[] = [] + function target( fence: number, log: Recorded[], failOn?: number ): StructuredAgentSessionEventTarget { + journalAppendOptions.length = 0 + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: a double for the handful of journal methods this sink calls; nothing else on it is ever reached. const journal = { - appendItem: vi.fn(async (id: AgentJournalItemIdentity, _body: AgentJournalItemBody) => { - const ordinal = id.provider === 'codex' ? id.ordinal : -1 - if (ordinal === failOn) { - throw new Error(`refused ${ordinal}`) + appendItem: vi.fn( + async ( + id: AgentJournalItemIdentity, + _body: AgentJournalItemBody, + options: JournalItemAppendOptions + ) => { + const ordinal = id.provider === 'codex' ? id.ordinal : -1 + if (ordinal === failOn) { + throw new Error(`refused ${ordinal}`) + } + journalAppendOptions.push(options) + log.push({ call: 'appendItem', fence, ordinal }) + return { cursor: { epoch: 'e', sequence: ordinal } } } - log.push({ call: 'appendItem', fence, ordinal }) - return { cursor: { epoch: 'e', sequence: ordinal } } - }), + ), appendTombstone: vi.fn(async (id: AgentJournalItemIdentity) => { log.push({ call: 'appendTombstone', @@ -51,7 +70,10 @@ function target( }) return { epoch: 'e', sequence: 0 } }), - appendLifecycleBatch: vi.fn(async (input: { settlementId: string }) => { + appendLifecycleBatch: vi.fn(async (input: JournalLifecycleBatchInput) => { + // A batch carries no producer linkage by design, so the fence is all + // there is to record — see the batch row builder. + journalAppendOptions.push({ fence: input.fence }) log.push({ call: 'appendLifecycleBatch', fence, settlementId: input.settlementId }) return { epoch: 'e', sequence: 0 } }), @@ -397,3 +419,73 @@ describe('deferred structured agent-session event sink', () => { ]) }) }) + +describe('producer linkage reaches the journal through every append path', () => { + const LINKAGE = { + agentId: 'task-1', + parentAgentId: 'task-parent', + providerParentRef: 'toolu_1', + producerKind: 'agent', + attempt: 2 + } as const + + it('forwards the whole bundle on the plain and try append paths', async () => { + for (const append of ['appendItem', 'tryAppendItem'] as const) { + const log: Recorded[] = [] + const deferred = createDeferredStructuredAgentSessionEventSink() + deferred.bind(target(5, log)) + deferred.sink[append]?.(identity(1), BODY, { ...LINKAGE }) + await deferred.drained() + + expect(journalAppendOptions).toEqual([{ fence: 5, ...LINKAGE }]) + deferred.close() + } + }) + + it('forwards it on the resolved-append paths, which lost it once before', async () => { + for (const append of ['tryAppendResolvedItem', 'tryAppendResolvedItemAndPublish'] as const) { + const log: Recorded[] = [] + const deferred = createDeferredStructuredAgentSessionEventSink() + deferred.bind(target(5, log)) + deferred.sink[append]?.(identity(1), BODY, () => identity(1), { ...LINKAGE }) + await deferred.drained() + + expect(journalAppendOptions).toEqual([{ fence: 5, ...LINKAGE }]) + deferred.close() + } + }) + + it('does NOT forward it on the lifecycle-batch path, which is one row for N mutations', async () => { + // A batch row carries one producer for every mutation in it, so forwarding + // would stamp whoever opened the batch onto all of them. Both callers are + // single-producer today; a mixed batch would have to stamp per mutation. + const log: Recorded[] = [] + const deferred = createDeferredStructuredAgentSessionEventSink() + deferred.bind(target(5, log)) + deferred.sink.appendLifecycleBatch?.( + 'settle-1', + [{ kind: 'item', identity: identity(1), body: BODY }], + { ...LINKAGE } + ) + + await deferred.drained() + + // The fence and nothing else: no linkage key reaches the batch row. + expect(journalAppendOptions).toEqual([{ fence: 5 }]) + deferred.close() + }) + + it("writes no linkage keys at all for the session's own agent", async () => { + const log: Recorded[] = [] + const deferred = createDeferredStructuredAgentSessionEventSink() + deferred.bind(target(5, log)) + deferred.sink.appendItem(identity(1), BODY) + await deferred.drained() + + // A control, not a pin. Absence is the claim, so the keys must be missing + // rather than present-and-undefined: a reader holding this options object + // would read `agentId: undefined` as a key that exists. + expect(journalAppendOptions).toEqual([{ fence: 5 }]) + deferred.close() + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.ts index b27e466d94b..394517eb19b 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.ts @@ -1,13 +1,15 @@ import { agentJournalItemKey } from '../../../shared/agent-session-journal-item-key' import type { AgentJournalItemBody, - AgentJournalItemIdentity + AgentJournalItemIdentity, + AgentJournalProducerLinkage } from '../../../shared/agent-session-journal-types' import type { AgentSessionTurnActivity } from '../../../shared/agent-session-wire' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' import type { JournalLifecycleMutationInput } from '../agent-session-journal/journal-row-builders' import { estimateStructuredAgentSessionItemBytes } from './structured-agent-session-event-sink-estimate' import { StructuredAgentSessionSinkQueue } from './structured-agent-session-event-sink-queue' +import { structuredAgentSessionJournalAppendOptions } from './structured-agent-session-journal-append-options' import { createStructuredAgentSessionResolvedAppend } from './structured-agent-session-resolved-append' export type StructuredAgentSessionSinkAdmission = @@ -23,7 +25,9 @@ export type StructuredAgentSessionSinkState = { export type StructuredAgentSessionSinkBarrier = { ok: true } | { ok: false; error: unknown } -export type StructuredAgentSessionAppendOptions = { +/** Linkage a producer stamps on the rows it writes. Absent on every append the + * session's own agent makes, which is what makes absence mean root. */ +export type StructuredAgentSessionAppendOptions = AgentJournalProducerLinkage & { /** Pending checkpoints with this key replace one another before they run. */ coalescingKey?: string /** Marks a critical lifecycle operation for lifecycle barriers and diagnostics. */ @@ -175,6 +179,7 @@ export function createDeferredStructuredAgentSessionEventSink( bound.journal.appendLifecycleBatch({ settlementId, mutations, + // Linkage is deliberately not forwarded: see the batch row builder. fence: bound.fence }) }, @@ -201,10 +206,11 @@ export function createDeferredStructuredAgentSessionEventSink( bytes: estimateStructuredAgentSessionItemBytes(identity, body), coalescingKey: options.coalescingKey, run: (bound) => - bound.journal.appendItem(identity, body, { - fence: bound.fence, - ...(options.observedAt === undefined ? {} : { observedAt: options.observedAt }) - }) + bound.journal.appendItem( + identity, + body, + structuredAgentSessionJournalAppendOptions(bound.fence, options) + ) }, options ) @@ -215,10 +221,11 @@ export function createDeferredStructuredAgentSessionEventSink( bytes: estimateStructuredAgentSessionItemBytes(identity, body), coalescingKey: options.coalescingKey, run: (bound) => - bound.journal.appendItem(identity, body, { - fence: bound.fence, - ...(options.observedAt === undefined ? {} : { observedAt: options.observedAt }) - }) + bound.journal.appendItem( + identity, + body, + structuredAgentSessionJournalAppendOptions(bound.fence, options) + ) }, options ), diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-journal-append-options.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-journal-append-options.ts new file mode 100644 index 00000000000..fdab5ebb0a6 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-journal-append-options.ts @@ -0,0 +1,21 @@ +// The journal options one admitted sink append forwards. +// +// Its own module because four append paths need it and the sink's own file +// already depends on two of them. Every path calls this, so a row-level field +// added to the sink's options reaches the durable row through all four rather +// than through whichever spread the next change remembers to edit. + +import { agentJournalLinkageFields } from '../../../shared/agent-session-journal-producer' +import type { JournalItemAppendOptions } from '../agent-session-journal/journal-store-contracts' +import type { StructuredAgentSessionAppendOptions } from './structured-agent-session-event-sink' + +export function structuredAgentSessionJournalAppendOptions( + fence: number, + options: StructuredAgentSessionAppendOptions +): JournalItemAppendOptions { + return { + fence, + ...(options.observedAt === undefined ? {} : { observedAt: options.observedAt }), + ...agentJournalLinkageFields(options) + } +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-resolved-append.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-resolved-append.ts index b8fb2f1986c..cf33d91e11a 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-resolved-append.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-resolved-append.ts @@ -1,5 +1,6 @@ import { estimateStructuredAgentSessionItemBytes } from './structured-agent-session-event-sink-estimate' import type { StructuredAgentSessionEventSink } from './structured-agent-session-event-sink' +import { structuredAgentSessionJournalAppendOptions } from './structured-agent-session-journal-append-options' import type { StructuredAgentSessionSinkQueue } from './structured-agent-session-event-sink-queue' /** Resolve a queued item's run identity against the journal bound at execution. */ @@ -25,10 +26,11 @@ export function createStructuredAgentSessionResolvedAppend( if (estimateStructuredAgentSessionItemBytes(identity, body) > bytes) { throw new Error('structured agent-session item identity exceeded its reserved size') } - await bound.journal.appendItem(identity, body, { - fence: bound.fence, - ...(options.observedAt === undefined ? {} : { observedAt: options.observedAt }) - }) + await bound.journal.appendItem( + identity, + body, + structuredAgentSessionJournalAppendOptions(bound.fence, options) + ) } }, options @@ -47,10 +49,11 @@ export function createStructuredAgentSessionResolvedAppend( if (estimateStructuredAgentSessionItemBytes(identity, body) + 1 > bytes) { throw new Error('structured agent-session item identity exceeded its reserved size') } - await bound.journal.appendItem(identity, body, { - fence: bound.fence, - ...(options.observedAt === undefined ? {} : { observedAt: options.observedAt }) - }) + await bound.journal.appendItem( + identity, + body, + structuredAgentSessionJournalAppendOptions(bound.fence, options) + ) bound.publish() } }, diff --git a/src/shared/agent-session-journal-producer.test.ts b/src/shared/agent-session-journal-producer.test.ts new file mode 100644 index 00000000000..f6eb4bb07b7 --- /dev/null +++ b/src/shared/agent-session-journal-producer.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest' +import { agentJournalLinkageFields, isRootAgentJournalItem } from './agent-session-journal-producer' + +describe('isRootAgentJournalItem', () => { + it("reads a row carrying no agent id as the session's own", () => { + expect(isRootAgentJournalItem({})).toBe(true) + }) + + it('reads a row carrying one as a subagent’s', () => { + expect(isRootAgentJournalItem({ agentId: 'task-1' })).toBe(false) + }) + + it('reads an id that failed to resolve as a subagent’s, not as root', () => { + // Presence, not truthiness. This is the whole point of the predicate: a + // truthy test answers "root" here, which puts the child's content back on + // the parent — the defect this attribution exists to remove. + expect(isRootAgentJournalItem({ agentId: '' })).toBe(false) + }) + + it('reads a missing item as root rather than throwing', () => { + // Every caller walks a list backwards with an index that can fall off it. + expect(isRootAgentJournalItem(undefined)).toBe(true) + }) +}) + +describe('agentJournalLinkageFields', () => { + it('omits absent members rather than writing them as undefined', () => { + // Absence is the claim these fields make, so a key present with an + // undefined value is not the same statement as no key at all. + expect(agentJournalLinkageFields({ agentId: 'task-1' })).toEqual({ agentId: 'task-1' }) + expect(agentJournalLinkageFields(undefined)).toEqual({}) + }) + + it('carries every member of the bundle through', () => { + const linkage = { + agentId: 'task-1', + parentAgentId: 'task-parent', + providerParentRef: 'toolu_1', + producerKind: 'background' as const, + attempt: 3 + } + expect(agentJournalLinkageFields(linkage)).toEqual(linkage) + }) +}) diff --git a/src/shared/agent-session-journal-producer.ts b/src/shared/agent-session-journal-producer.ts new file mode 100644 index 00000000000..fe313294677 --- /dev/null +++ b/src/shared/agent-session-journal-producer.ts @@ -0,0 +1,47 @@ +// Which agent produced a journal row, and the one place absence is interpreted. +// +// One journal is the durable record of one agent SESSION, and a session may run +// subagents. Both agents' rows land in the same timeline, so every "what is this +// agent doing right now" reader has to say which producer it means. Readers ask +// "is this NOT mine", never "is this mine": the session's own agent stamps +// nothing, so root-ness is the absence of an id rather than a value to match. + +import type { + AgentJournalProducerLinkage, + AgentJournalRenderItem +} from './agent-session-journal-types' + +/** + * Whether the session's own agent produced this row, rather than a subagent. + * + * Presence, not truthiness. An id that failed to resolve is still an id, and a + * truthy test would read it as root and put the child's content back on the + * parent — the defect this attribution exists to remove, reintroduced through a + * soft predicate. Rows written before linkage existed carry no id and read as + * root, which reproduces exactly what those journals always showed. + */ +export function isRootAgentJournalItem( + item: Pick | undefined +): boolean { + return item?.agentId == null +} + +/** Linkage as row fields, with absent members omitted rather than set to + * `undefined`. Every carrier spreads this, so a new field reaches the row + * through one edit instead of one per hop. */ +export function agentJournalLinkageFields( + linkage: AgentJournalProducerLinkage | undefined +): AgentJournalProducerLinkage { + if (!linkage) { + return {} + } + return { + ...(linkage.agentId === undefined ? {} : { agentId: linkage.agentId }), + ...(linkage.parentAgentId === undefined ? {} : { parentAgentId: linkage.parentAgentId }), + ...(linkage.providerParentRef === undefined + ? {} + : { providerParentRef: linkage.providerParentRef }), + ...(linkage.producerKind === undefined ? {} : { producerKind: linkage.producerKind }), + ...(linkage.attempt === undefined ? {} : { attempt: linkage.attempt }) + } +} diff --git a/src/shared/agent-session-journal-schemas.test.ts b/src/shared/agent-session-journal-schemas.test.ts index 4a41437a3b4..8f07cca800a 100644 --- a/src/shared/agent-session-journal-schemas.test.ts +++ b/src/shared/agent-session-journal-schemas.test.ts @@ -216,6 +216,23 @@ describe('nested corruption is rejected', () => { ).toBe(true) }) + it('refuses an empty producer id, which a presence test would read as a subagent', () => { + const base = { + itemId: 'codex:t:turn:0', + revision: 1, + body: CANONICAL_BODIES[0] as AgentJournalItemBody, + sequence: 1, + observedAt: 1_000 + } + expect(isAdmissibleAgentJournalRenderItem({ ...base, agentId: 'task-1' })).toBe(true) + // `''` is PRESENT. Admitting it would hide the row from its own author on + // every parent-scoped surface — the defect linkage exists to remove. + expect(isAdmissibleAgentJournalRenderItem({ ...base, agentId: '' })).toBe(false) + expect(isAdmissibleAgentJournalRenderItem({ ...base, parentAgentId: '' })).toBe(false) + expect(isAdmissibleAgentJournalRenderItem({ ...base, providerParentRef: '' })).toBe(false) + expect(isAdmissibleAgentJournalRenderItem({ ...base, producerKind: '' })).toBe(false) + }) + it('rejects shallow render items and submissions', () => { expect( isAdmissibleAgentJournalRenderItem({ diff --git a/src/shared/agent-session-journal-schemas.ts b/src/shared/agent-session-journal-schemas.ts index ca95cd2892c..43f96242785 100644 --- a/src/shared/agent-session-journal-schemas.ts +++ b/src/shared/agent-session-journal-schemas.ts @@ -237,13 +237,28 @@ export const AgentJournalItemBodySchema = z.discriminatedUnion('kind', [ }) ]) +/** Producer linkage as it rides a render item across the process boundary. + * `producerKind` stays an open string for the reason the header gives: a host + * that learns a third kind must not make its rows unreadable to this client. */ +export const AgentJournalProducerLinkageFields = { + // `.min(1)` on every id: an EMPTY string is present, and the reader that + // scopes a parent's surfaces tests presence, not truthiness. `agentId: ''` + // would read as a subagent and hide the row from its own author for good. + agentId: z.string().min(1).optional(), + parentAgentId: z.string().min(1).optional(), + providerParentRef: z.string().min(1).optional(), + producerKind: z.string().min(1).optional(), + attempt: z.number().int().optional() +} as const + export const AgentJournalRenderItemSchema = z.object({ itemId: z.string().min(1), revision: z.number().int(), body: AgentJournalItemBodySchema, sequence: z.number().int(), observedAt: z.number(), - recovered: z.literal(true).optional() + recovered: z.literal(true).optional(), + ...AgentJournalProducerLinkageFields }) export const AgentJournalSubmissionSchema = z.object({ diff --git a/src/shared/agent-session-journal-types.ts b/src/shared/agent-session-journal-types.ts index 287e07e2a5d..51ff5d1dc8d 100644 --- a/src/shared/agent-session-journal-types.ts +++ b/src/shared/agent-session-journal-types.ts @@ -250,10 +250,41 @@ export type AgentJournalItemBody = | AgentJournalStatusItem | AgentJournalTurnItem +/** Agent work, versus a backgrounded shell or command task. Classified once by + * the producer, which holds the provider vocabulary, so no reader re-derives it. */ +export type AgentJournalProducerKind = 'agent' | 'background' + +/** + * Which agent produced a row, repeated on every row that agent produced. + * + * One journal is the durable record of one agent SESSION, and a session that + * runs subagents journals their rows into it too. Absence is a positive claim + * and never "unknown": no `agentId` means the session's own agent wrote the row. + * Repeated per row rather than held once on a start row, so a row answers for + * itself: every reader here scans backwards from the tail and stops at the + * turn, so one that had to find a start row first would have to scan past that + * stop to attribute anything. Repetition is near-free — absent on the session's + * own rows, which are most of them — and it is what keeps the field correct + * without a second lookup. + */ +export type AgentJournalProducerLinkage = { + /** The producing subagent's canonical id. Absent ⇒ the session's own agent. */ + agentId?: string + /** The producing agent's own parent. Absent ⇒ its parent is the session root. */ + parentAgentId?: string + /** The provider's own parent reference for this row. Provenance only: it names + * the tool CALL, which is re-minted on every resume, so it is never a join key. */ + providerParentRef?: string + producerKind?: AgentJournalProducerKind + /** Which run of the agent, when past the first. Identity answers "which agent"; + * this answers "which run of it", and is deliberately not part of the identity. */ + attempt?: number +} + /** One reduced timeline entry. `sequence` orders the list; `observedAt` is the * provider's own clock and may sort earlier than a later sequence when the row * was recovered after a crash. */ -export type AgentJournalRenderItem = { +export type AgentJournalRenderItem = AgentJournalProducerLinkage & { itemId: string revision: number body: AgentJournalItemBody diff --git a/src/shared/native-chat-turn-activity.test.ts b/src/shared/native-chat-turn-activity.test.ts index 3cc64f4e106..a0f563d8643 100644 --- a/src/shared/native-chat-turn-activity.test.ts +++ b/src/shared/native-chat-turn-activity.test.ts @@ -179,3 +179,56 @@ describe('selectStructuredAgentTurnActivity', () => { expect(selectStructuredAgentTurnActivity([turnStart, diagnostic], null)).toBeNull() }) }) + +describe('selectStructuredAgentTurnActivity — which agent it answers for', () => { + /** A row a subagent produced, which shares the session's journal. */ + function childItem(sequence: number, body: AgentJournalItemBody): AgentJournalRenderItem { + return { ...item(sequence, body), agentId: 'task-1', producerKind: 'agent' } + } + + const childRunningBash = childItem(2, { + kind: 'tool-call', + name: 'shell', + input: { command: 'pnpm test' }, + state: 'running' + }) + + it("does not let a child's tool label suppress the provider's line for the parent", () => { + // The provider line is the SESSION'S OWN; a child running a tool of the same + // name must not make it read as a repeat and blank the indicator. + expect( + selectStructuredAgentTurnActivity([turnStart, childRunningBash], 'turn-1', { + turnId: 'turn-1', + text: 'pnpm test' + }) + ).toEqual({ kind: 'description', text: 'pnpm test' }) + }) + + it("does not let a child's tool label suppress the parent's own status line", () => { + expect( + selectStructuredAgentTurnActivity( + [turnStart, childRunningBash, item(3, { kind: 'status', text: 'pnpm test' })], + 'turn-1' + ) + ).toEqual({ kind: 'description', text: 'pnpm test' }) + }) + + it("still suppresses a line repeating the session's OWN running tool", () => { + // The scoping must not disable the de-duplication it was narrowing. + expect( + selectStructuredAgentTurnActivity( + [ + turnStart, + item(2, { + kind: 'tool-call', + name: 'shell', + input: { command: 'pnpm test' }, + state: 'running' + }) + ], + 'turn-1', + { turnId: 'turn-1', text: 'pnpm test' } + ) + ).toBeNull() + }) +}) diff --git a/src/shared/native-chat-turn-activity.ts b/src/shared/native-chat-turn-activity.ts index cac19796334..2ea1ae8b021 100644 --- a/src/shared/native-chat-turn-activity.ts +++ b/src/shared/native-chat-turn-activity.ts @@ -1,6 +1,7 @@ import { readAgentJournalTurn } from './agent-session-turn-record' import type { AgentJournalRenderItem } from './agent-session-journal-types' import type { AgentSessionTurnActivity } from './agent-session-wire' +import { isRootAgentJournalItem } from './agent-session-journal-producer' import { normalizePromptField } from './agent-status-field-normalization' import { describeActiveToolCall, formatActiveToolLabel } from './native-chat-tool-activity' @@ -86,7 +87,11 @@ export function selectStructuredAgentTurnActivity( const turn = readAgentJournalTurn(item.body) return turn?.turnId === turnId && turn.state === 'running' }) - const turnItems = items.slice(Math.max(0, turnStartIndex)) + // Scoped ONCE, for everything below: this answers what the session's own agent + // is doing. Both readers below consult the label set, so a subagent left in it + // would let a child's tool label suppress the parent's own activity line — + // child data deciding what the parent's surface shows. + const turnItems = items.slice(Math.max(0, turnStartIndex)).filter(isRootAgentJournalItem) const toolLabels = recentToolActivityLabels(turnItems) if (providerActivity?.turnId === turnId) { const text = activityLine(providerActivity.text) diff --git a/src/shared/structured-agent-session-live-turn.test.ts b/src/shared/structured-agent-session-live-turn.test.ts index b9ce5f2c022..7fb837f77f9 100644 --- a/src/shared/structured-agent-session-live-turn.test.ts +++ b/src/shared/structured-agent-session-live-turn.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it } from 'vitest' import type { AgentJournalRenderItem } from './agent-session-journal-types' -import { isStructuredAgentSessionThinking } from './structured-agent-session-live-turn' +import { + activeStructuredAgentSessionToolCall, + isStructuredAgentSessionThinking +} from './structured-agent-session-live-turn' function item( itemId: string, @@ -111,3 +114,90 @@ describe('isStructuredAgentSessionThinking', () => { ).toBe(false) }) }) + +describe("the live-turn readers answer for the session's own agent", () => { + const turnStart = item('turn-start', 1, { + kind: 'status', + text: 'Working', + turnLifecycle: { turnId: 'turn-1', state: 'running' } + }) + const spawnCall = item('root-task', 2, { + kind: 'tool-call', + name: 'Task', + input: { description: 'explore' }, + state: 'running' + }) + const child = ( + itemId: string, + sequence: number, + body: AgentJournalRenderItem['body'], + agentId = 'task-1' + ): AgentJournalRenderItem => ({ ...item(itemId, sequence, body), agentId }) + + it('does not report the parent as thinking because a subagent is reasoning', () => { + const childReasoning = child('child-reasoning', 3, { + kind: 'message', + role: 'reasoning', + blocks: [{ type: 'text', text: 'Weighing two approaches' }] + }) + expect(isStructuredAgentSessionThinking([turnStart, spawnCall, childReasoning])).toBe(false) + }) + + it('still reports the parent as thinking when the parent itself is reasoning', () => { + const ownReasoning = item('own-reasoning', 3, { + kind: 'message', + role: 'reasoning', + blocks: [{ type: 'text', text: 'Weighing two approaches' }] + }) + expect(isStructuredAgentSessionThinking([turnStart, spawnCall, ownReasoning])).toBe(true) + }) + + it("reports the parent's own running call while a subagent runs its own", () => { + const childCall = child('child-grep', 3, { + kind: 'tool-call', + name: 'Grep', + input: { pattern: 'x' }, + state: 'running' + }) + expect(activeStructuredAgentSessionToolCall([turnStart, spawnCall, childCall])?.name).toBe( + 'Task' + ) + }) + + it('reports nothing running when only a subagent has a live call', () => { + const childCall = child('child-grep', 2, { + kind: 'tool-call', + name: 'Grep', + input: { pattern: 'x' }, + state: 'running' + }) + expect(activeStructuredAgentSessionToolCall([turnStart, childCall])).toBeNull() + }) + + it('treats an agent id that failed to resolve as a child, not as the parent', () => { + // Presence, not truthiness. A truthy test would read the empty id as root + // and put the child's tool call straight back on the parent's row — the + // exact defect this attribution exists to remove. + const unresolved = child( + 'child-grep', + 3, + { kind: 'tool-call', name: 'Grep', input: { pattern: 'x' }, state: 'running' }, + '' + ) + expect(activeStructuredAgentSessionToolCall([turnStart, spawnCall, unresolved])?.name).toBe( + 'Task' + ) + }) + + it("reads a row written before linkage existed as the parent's own", () => { + const legacyChildCall = item('legacy-call', 3, { + kind: 'tool-call', + name: 'Grep', + input: { pattern: 'x' }, + state: 'running' + }) + expect( + activeStructuredAgentSessionToolCall([turnStart, spawnCall, legacyChildCall])?.name + ).toBe('Grep') + }) +}) diff --git a/src/shared/structured-agent-session-live-turn.ts b/src/shared/structured-agent-session-live-turn.ts index 84ed3fdab0c..f7dbcd009d7 100644 --- a/src/shared/structured-agent-session-live-turn.ts +++ b/src/shared/structured-agent-session-live-turn.ts @@ -2,12 +2,26 @@ // tail of the item list. Every scan here stops at the turn's own record — the // typed `turn` item, or the legacy status row that carries one — because state // from an earlier turn is never this turn's state. +// +// These scans answer for the SESSION'S OWN agent. A subagent's rows share this +// journal and are usually the newer ones while a child runs, so each scan skips +// anything a subagent produced; the transcript still renders every agent. +// +// Each scan reads the turn record BEFORE it checks the producer, which is only +// safe because a turn row can never carry linkage: a turn is the SESSION'S unit +// of work, and no producer of a turn-bearing body stamps one. Both lanes were +// checked — Claude's turn rows are built with no linkage at all, Codex has no +// linkage concept, the compact row passes only a fence, and the stale-turn +// sweep goes through the lifecycle-batch path, which cannot carry linkage by +// type. So a child-linked row can never be what terminates one of these scans. +// Re-check that before giving any of those sites a producer. import type { AgentJournalRenderItem, AgentJournalToolCallItem, AgentJournalTurnLifecycle } from './agent-session-journal-types' +import { isRootAgentJournalItem } from './agent-session-journal-producer' import { readAgentJournalTurn } from './agent-session-turn-record' export function activeStructuredAgentSessionTurnId( @@ -84,12 +98,13 @@ export function isStructuredAgentSessionThinking( ): boolean { let newestContentIsReasoning: boolean | null = null for (let index = items.length - 1; index >= 0; index -= 1) { - const body = items[index]?.body + const item = items[index] + const body = item?.body const turn = readAgentJournalTurn(body) if (turn) { return turn.state === 'running' && newestContentIsReasoning === true } - if (newestContentIsReasoning !== null) { + if (newestContentIsReasoning !== null || !isRootAgentJournalItem(item)) { continue } if (body?.kind === 'message') { @@ -107,18 +122,20 @@ export function isStructuredAgentSessionThinking( return false } -/** The tool call the newest turn is still inside, or null when nothing is running. - * An abandoned `running` call from an earlier crashed turn can never be reported - * as live work. */ +/** The tool call the SESSION'S OWN agent is still inside, or null when nothing is + * running. An abandoned `running` call from an earlier crashed turn can never be + * reported as live work, and neither can a subagent's — while a child runs a + * tool, the parent is still inside the call that spawned it. */ export function activeStructuredAgentSessionToolCall( items: readonly AgentJournalRenderItem[] ): AgentJournalToolCallItem | null { for (let index = items.length - 1; index >= 0; index -= 1) { - const body = items[index]?.body + const item = items[index] + const body = item?.body if (readAgentJournalTurn(body)) { return null } - if (body?.kind === 'tool-call' && body.state === 'running') { + if (body?.kind === 'tool-call' && body.state === 'running' && isRootAgentJournalItem(item)) { return body } } diff --git a/src/shared/structured-agent-session-projection.test.ts b/src/shared/structured-agent-session-projection.test.ts index 928180e7a6c..24f110ed037 100644 --- a/src/shared/structured-agent-session-projection.test.ts +++ b/src/shared/structured-agent-session-projection.test.ts @@ -7,6 +7,9 @@ import { hasPersistedStructuredAgentSessionTurn, hasUnansweredStructuredAgentSessionDispatch, projectStructuredItemToNativeChat, + projectStructuredItemsToNativeChat, + latestStructuredAgentSessionAssistantMessage, + activeStructuredAgentSessionToolCall, projectStructuredAgentSessionStatus, projectStructuredAgentSessionStatusSummary, structuredAgentSessionPaneKey @@ -513,3 +516,165 @@ it('preserves confirmed MCP identity and the raw name through projection', () => type: 'tool-call' }) }) + +describe("producer linkage — a subagent's output never speaks for the parent", () => { + /** A row a subagent produced. Same journal, same session; only linkage differs. */ + function childItem( + itemId: string, + sequence: number, + body: AgentJournalRenderItem['body'], + agentId = 'task-1' + ): AgentJournalRenderItem { + return { ...item(itemId, sequence, body), agentId, producerKind: 'agent' } + } + + const userAsk = item('user-1', 1, { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: 'summarise the repo' }] + }) + const turnRunning = item('turn-1', 2, { kind: 'turn', turnId: 'turn-1', state: 'running' }) + const parentProse = item('root-prose', 3, { + kind: 'message', + role: 'assistant', + blocks: [{ type: 'text', text: 'delegating' }] + }) + const spawnCall = item('root-task', 4, { + kind: 'tool-call', + name: 'Task', + input: { description: 'explore the lane' }, + state: 'running' + }) + const childProse = childItem('child-prose', 5, { + kind: 'message', + role: 'assistant', + blocks: [{ type: 'text', text: 'looking' }] + }) + const childCall = childItem('child-grep', 6, { + kind: 'tool-call', + name: 'Grep', + input: { pattern: 'x' }, + state: 'running' + }) + const items = [userAsk, turnRunning, parentProse, spawnCall, childProse, childCall] + + it("shows the parent's own prose and its own running call, not the child's newer ones", () => { + expect(latestStructuredAgentSessionAssistantMessage(items)).toBe('delegating') + expect(activeStructuredAgentSessionToolCall(items)?.name).toBe('Task') + }) + + it("publishes the parent's own line and call on the summary the sidebar reads", () => { + const summary = projectStructuredAgentSessionStatusSummary(items) + expect(summary.status).toBe('working') + expect(summary.lastAssistantMessage).toBe('delegating') + expect(summary.toolName).toBe('Task') + // The row does not go blank while a child runs: the spawn call is still the + // parent's own live work. + expect(summary.toolInput).toBeTruthy() + }) + + it("still renders the child's output in the transcript", () => { + // The other direction: scoping the STATUS readers must not delete subagent + // output from the chat. + const prose = projectStructuredItemsToNativeChat(items).flatMap((message) => + message.blocks.flatMap((block) => (block.type === 'text' ? [block.text] : [])) + ) + expect(prose).toContain('looking') + expect(prose).toContain('delegating') + }) + + it("falls back to nothing rather than a child's line when the parent said nothing", () => { + const summary = projectStructuredAgentSessionStatusSummary([ + userAsk, + turnRunning, + spawnCall, + childProse, + childCall + ]) + expect(summary.lastAssistantMessage).toBeUndefined() + expect(summary.toolName).toBe('Task') + }) + + it('attributes rows without reading their neighbours', () => { + // A window holding only the child's own rows: no spawn call, no turn record. + // Nothing here is re-derived from a start row, so attribution does not + // depend on how much of the timeline a reader happens to hold. (This store + // has no compaction and paginates complete-or-reset, so such a window is not + // reachable today — the point is that the rule does not rely on that.) + const windowed = [childProse, childCall] + expect(latestStructuredAgentSessionAssistantMessage(windowed)).toBe('') + expect(activeStructuredAgentSessionToolCall(windowed)).toBeNull() + }) + + it("does not quote a subagent's own user-role prompt as the session's", () => { + const childPrompt = childItem('child-prompt', 5, { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: 'explore the lane' }] + }) + expect( + projectStructuredAgentSessionStatusSummary([userAsk, turnRunning, spawnCall, childPrompt]) + .latestPrompt + ).toBe('summarise the repo') + }) + + it('keeps a nested child off the parent, read through the projection', () => { + // A grandchild: its own agent id, and a parent that is not the session root. + const grandchild: AgentJournalRenderItem = { + ...item('grandchild-prose', 7, { + kind: 'message', + role: 'assistant', + blocks: [{ type: 'text', text: 'deeper' }] + }), + agentId: 'task-2', + parentAgentId: 'task-1', + producerKind: 'agent' + } + const nested = [...items, grandchild] + expect(latestStructuredAgentSessionAssistantMessage(nested)).toBe('delegating') + // Naming a parent does not make the row that parent's: the summary the + // sidebar reads still shows the session's own line. + expect(projectStructuredAgentSessionStatusSummary(nested).lastAssistantMessage).toBe( + 'delegating' + ) + // And the transcript still renders it, so naming a parent is not a filter. + expect( + projectStructuredItemsToNativeChat(nested).some((block) => + JSON.stringify(block).includes('deeper') + ) + ).toBe(true) + }) + + it('treats an agent id that failed to resolve as a child rather than as the parent', () => { + const unresolved = childItem( + 'child-unresolved', + 5, + { kind: 'message', role: 'assistant', blocks: [{ type: 'text', text: 'looking' }] }, + '' + ) + expect( + latestStructuredAgentSessionAssistantMessage([userAsk, turnRunning, parentProse, unresolved]) + ).toBe('delegating') + }) + + it("reads a row written before linkage existed as the parent's own", () => { + // A journal open across the upgrade has unmarked child rows below marked + // ones. Absence means root, which reproduces exactly what those journals + // always showed — it is never "unknown". + const legacyChildProse = item('legacy-child', 5, { + kind: 'message', + role: 'assistant', + blocks: [{ type: 'text', text: 'legacy child line' }] + }) + expect( + latestStructuredAgentSessionAssistantMessage([ + userAsk, + turnRunning, + parentProse, + spawnCall, + legacyChildProse, + childProse + ]) + ).toBe('legacy child line') + }) +}) diff --git a/src/shared/structured-agent-session-projection.ts b/src/shared/structured-agent-session-projection.ts index 73824531578..f0d0e5b8e15 100644 --- a/src/shared/structured-agent-session-projection.ts +++ b/src/shared/structured-agent-session-projection.ts @@ -4,6 +4,7 @@ import { normalizePromptField } from './agent-status-field-normalization' import type { AgentJournalRenderItem, AgentJournalSubmission } from './agent-session-journal-types' +import { isRootAgentJournalItem } from './agent-session-journal-producer' import { AGENT_STATUS_TOOL_INPUT_MAX_LENGTH, AGENT_STATUS_TOOL_NAME_MAX_LENGTH @@ -135,6 +136,10 @@ function itemBlocks(item: AgentJournalRenderItem): { const projectedItems = new WeakMap() +/** Deliberately NOT scoped by producer: the transcript shows every agent's + * output. The line this module draws is that the transcript renders every item, + * while every "what is this agent doing right now" scan renders only the + * session's own agent's. */ export function projectStructuredItemsToNativeChat( items: readonly AgentJournalRenderItem[] ): NativeChatMessage[] { @@ -170,6 +175,9 @@ export function projectStructuredItemToNativeChat( return message } +/** Deliberately NOT scoped by producer: this is an existence test ("is this + * session listable at all"), not an attribution one. A session whose only + * content came from a subagent still has content. */ export function hasPersistedStructuredAgentSessionTurn( items: readonly AgentJournalRenderItem[] ): boolean { @@ -236,7 +244,10 @@ function messageProse(blocks: readonly NativeChatBlock[]): string { return blocks.flatMap((block) => (block.type === 'text' ? [block.text] : [])).join('\n') } -/** The newest user prompt, as the sidebar quotes it. */ +/** The newest prompt the session's own user turn carries, as the sidebar quotes + * it. Scoped to root rows for the same reason the assistant line is: a provider + * that journals a subagent's own prompt would otherwise requote it as the + * session's. */ export function latestStructuredAgentSessionPrompt( items: readonly AgentJournalRenderItem[] ): string { @@ -249,20 +260,30 @@ export function latestStructuredAgentSessionUserItem( ): AgentJournalRenderItem | null { for (let index = items.length - 1; index >= 0; index -= 1) { const item = items[index] - if (item?.body.kind === 'message' && item.body.role === 'user') { + if ( + item?.body.kind === 'message' && + item.body.role === 'user' && + isRootAgentJournalItem(item) + ) { return item } } return null } -/** The newest assistant prose in the latest user turn. Tool-only assistant items - * are skipped; the user boundary clears prose from the preceding turn. */ +/** The newest prose THE SESSION'S OWN AGENT wrote in the latest user turn — not a + * subagent's, whose rows share this journal and are usually the newer ones while + * a child runs. Tool-only assistant items are skipped; the user boundary clears + * prose from the preceding turn. */ export function latestStructuredAgentSessionAssistantMessage( items: readonly AgentJournalRenderItem[] ): string { for (let index = items.length - 1; index >= 0; index -= 1) { - const body = items[index]?.body + const item = items[index] + const body = item?.body + if (!isRootAgentJournalItem(item)) { + continue + } if (body?.kind === 'message' && body.role === 'user') { return '' }