diff --git a/src/main/claude/claude-background-task-row-journal.test.ts b/src/main/claude/claude-background-task-row-journal.test.ts index 736583d03240..287d3aeff9c5 100644 --- a/src/main/claude/claude-background-task-row-journal.test.ts +++ b/src/main/claude/claude-background-task-row-journal.test.ts @@ -111,25 +111,19 @@ describe('Claude background task row journal', () => { expect(appendAndPublish).toHaveBeenCalledTimes(2) }) - it('keeps fallback append coalescing separate from its ordered publication', () => { - const calls: { operation: 'append' | 'publish'; coalescingKey?: string }[] = [] + it('appends the row, then publishes it', () => { + const calls: ('append' | 'publish')[] = [] const task = row() const sink: StructuredAgentSessionEventSink = { appendItem: vi.fn(), appendTombstone: vi.fn(), publish: vi.fn(), - tryAppendResolvedItem: vi.fn((_identity, _body, _resolve, options) => { - calls.push({ - operation: 'append', - ...(options?.coalescingKey ? { coalescingKey: options.coalescingKey } : {}) - }) + tryAppendResolvedItem: vi.fn(() => { + calls.push('append') return { accepted: true as const } }), - tryPublish: vi.fn((options) => { - calls.push({ - operation: 'publish', - ...(options?.coalescingKey ? { coalescingKey: options.coalescingKey } : {}) - }) + tryPublish: vi.fn(() => { + calls.push('publish') return { accepted: true as const } }) } @@ -143,13 +137,7 @@ describe('Claude background task row journal', () => { () => AGENT_JOURNAL_THREAD_SCOPE ) ).toEqual({ accepted: true }) - expect(calls).toEqual([ - { - operation: 'append', - coalescingKey: JSON.stringify(['claude-background-task', 'task-1', 'tool-1']) - }, - { operation: 'publish' } - ]) + expect(calls).toEqual(['append', 'publish']) expect(task.lastSerialized).not.toBeNull() }) diff --git a/src/main/claude/claude-background-task-row-journal.ts b/src/main/claude/claude-background-task-row-journal.ts index 956bc770f74d..45f8dfc2ae17 100644 --- a/src/main/claude/claude-background-task-row-journal.ts +++ b/src/main/claude/claude-background-task-row-journal.ts @@ -156,12 +156,7 @@ export function writeClaudeBackgroundTaskRow( } beforeAppend?.() const identity = claudeBackgroundTaskIdentity(id, row.generation) - // Generation is translator-local and resets when a provider stream is - // recreated. Keep unresolved writes from distinct provider runs queued side - // by using the provider's parent tool identity as the coalescing discriminator. - const coalescingKey = JSON.stringify(['claude-background-task', id, row.toolUseId ?? null]) const appendOptions = { - coalescingKey, turnScope: turnScope(), ...(lifecycle ? { lifecycle: true } : {}) } diff --git a/src/main/claude/claude-open-turn.ts b/src/main/claude/claude-open-turn.ts index f6edf49d56b2..ddc1bd553907 100644 --- a/src/main/claude/claude-open-turn.ts +++ b/src/main/claude/claude-open-turn.ts @@ -209,7 +209,7 @@ export class ClaudeOpenTurn { { lifecycle: item.body, ...(contextUsage ? { contextUsage } : {}) }, { publish: false, options: item.options } ) - // Preserve first-work evidence when completion arrives before the journal drains. + // Keyed apart, so this never replaces the start's publication while it still waits to run. this.deps.sink.publish({ coalescingKey: item.publishCoalescingKey }) } } diff --git a/src/main/claude/claude-structured-journal-translation-background-tasks.test.ts b/src/main/claude/claude-structured-journal-translation-background-tasks.test.ts index db3b75ecf325..719e03d24476 100644 --- a/src/main/claude/claude-structured-journal-translation-background-tasks.test.ts +++ b/src/main/claude/claude-structured-journal-translation-background-tasks.test.ts @@ -15,6 +15,7 @@ import { import type { AgentSessionJournal } from '../native-chat/agent-session-journal/journal-store' import { createClaudeJournalTranslator } from './claude-structured-journal-translation' import { blockOf } from './claude-background-task-row-test-support' +import { withJournalQueueMembers } from '../native-chat/agent-session-wire/structured-agent-session-journal-double-test-support' import { testEventSinkLogging } from '../native-chat/agent-session-wire/structured-agent-session-logger-test-support' // The frames below are the ones the reported session actually carried: two real @@ -144,7 +145,7 @@ function persistedTarget( ): StructuredAgentSessionEventTarget { const journal = // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: this test double implements the journal methods exercised by the deferred sink. - { + withJournalQueueMembers({ appendItem: async (identity: AgentJournalItemIdentity, body: AgentJournalItemBody) => { persisted.set(agentJournalItemKey(identity), body) return { cursor: { epoch: 'test', sequence: persisted.size }, itemId: '', revision: 1 } @@ -163,7 +164,7 @@ function persistedTarget( }) satisfies StructuredAgentSessionLinkageJournal['visitItemsWithLinkage'], itemBody: (itemId: string) => persisted.get(itemId) ?? null, epoch: 'test' - } as unknown as AgentSessionJournal + }) as unknown as AgentSessionJournal return { journal, fence: 1, publish: vi.fn() } } diff --git a/src/main/claude/claude-structured-prompt-ownership.ts b/src/main/claude/claude-structured-prompt-ownership.ts index 9a5dc477e9ba..7f68b5838651 100644 --- a/src/main/claude/claude-structured-prompt-ownership.ts +++ b/src/main/claude/claude-structured-prompt-ownership.ts @@ -87,8 +87,8 @@ export async function cancelClaudeStructuredTurn(input: { ) } // Judge against the published journal, because that is the only turn a client could have been - // shown — but only while it HAS an answer. The journal drains through a serialized async queue, - // so a null read means the row has not landed yet, not that nothing is running; falling back to + // shown — but only while it HAS an answer. A null read can mean the row never landed (a sink not + // yet bound, or refusing under backpressure), not that nothing is running; falling back to // the in-memory turn there keeps Stop from being gated on bookkeeping. No live turn either way // means nothing has published an identity this request can contradict. const ownsRequestedTurn = (): boolean => { diff --git a/src/main/claude/claude-structured-session-reading-control.test.ts b/src/main/claude/claude-structured-session-reading-control.test.ts index d90e5538964e..e75338f26e2f 100644 --- a/src/main/claude/claude-structured-session-reading-control.test.ts +++ b/src/main/claude/claude-structured-session-reading-control.test.ts @@ -20,6 +20,7 @@ import { identityFor, PROVIDER_SESSION_ID } from './claude-structured-session-test-support' +import { withJournalQueueMembers } from '../native-chat/agent-session-wire/structured-agent-session-journal-double-test-support' import { testEventSinkLogging } from '../native-chat/agent-session-wire/structured-agent-session-logger-test-support' function controlledSink(): { @@ -49,7 +50,7 @@ function persistedTarget( ): StructuredAgentSessionEventTarget { const journal = // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: This test double implements the journal methods exercised by the deferred sink. - { + withJournalQueueMembers({ appendItem: async (identity: AgentJournalItemIdentity, body: AgentJournalItemBody) => { persisted.set(agentJournalItemKey(identity), body) return { cursor: { epoch: 'test', sequence: persisted.size }, itemId: '', revision: 1 } @@ -68,7 +69,7 @@ function persistedTarget( }) satisfies StructuredAgentSessionLinkageJournal['visitItemsWithLinkage'], itemBody: (itemId: string) => persisted.get(itemId) ?? null, epoch: 'test' - } as unknown as AgentSessionJournal + }) as unknown as AgentSessionJournal return { journal, fence: 1, publish: vi.fn() } } diff --git a/src/main/claude/claude-subagent-group-row.ts b/src/main/claude/claude-subagent-group-row.ts index 5341d53af627..051970a5b36d 100644 --- a/src/main/claude/claude-subagent-group-row.ts +++ b/src/main/claude/claude-subagent-group-row.ts @@ -92,13 +92,12 @@ export function writeClaudeSubagentGroupRow( group: RosterGroup ): void { const agents = [...group.entries.values()].map((tracked) => tracked.entry) - const options = { coalescingKey: `claude-subagents:${group.groupId}`, turnScope: group.turnScope } 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.appendTombstone(group.identity) sink.publish() } return @@ -110,8 +109,6 @@ export function writeClaudeSubagentGroupRow( 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.appendItem(group.identity, body, { turnScope: group.turnScope }) sink.publish() } diff --git a/src/main/claude/claude-subagent-roster.test.ts b/src/main/claude/claude-subagent-roster.test.ts index 2bcffcfb49fe..db942502ee03 100644 --- a/src/main/claude/claude-subagent-roster.test.ts +++ b/src/main/claude/claude-subagent-roster.test.ts @@ -14,6 +14,7 @@ import { type StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' import { ClaudeSubagentRoster } from './claude-subagent-roster' +import { withJournalQueueMembers } from '../native-chat/agent-session-wire/structured-agent-session-journal-double-test-support' import { testEventSinkLogging } from '../native-chat/agent-session-wire/structured-agent-session-logger-test-support' const TURN_1 = 'claude-session:turn-1' @@ -456,13 +457,14 @@ describe('ClaudeSubagentRoster — through the real sink queue', () => { it('lands every revision, not just the one that was already in flight', async () => { const appended: AgentJournalItemBody[] = [] let published = 0 - const journal = { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: a double for the journal members this path calls; the helper adds the in-order ones. + const journal = withJournalQueueMembers({ appendItem: async (_identity: AgentJournalItemIdentity, body: AgentJournalItemBody) => { appended.push(body) return { cursor: { epoch: 'e', sequence: appended.length } } }, appendTombstone: async () => ({ epoch: 'e', sequence: 0 }) - } as unknown as AgentSessionJournal + }) as unknown as AgentSessionJournal const deferred = createDeferredStructuredAgentSessionEventSink(testEventSinkLogging()) deferred.bind({ journal, @@ -477,8 +479,7 @@ describe('ClaudeSubagentRoster — through the real sink queue', () => { currentTurnScope: () => AGENT_JOURNAL_THREAD_SCOPE }) - // The first append is in flight while the rest are submitted, so a publish - // sharing the row's coalescing key would evict them. + // The first append is in flight while the rest are submitted; each must still land. roster.observeSystemFrame(started({ task_id: 'task-1', description: 'One' })) roster.observeSystemFrame(started({ task_id: 'task-2', description: 'Two' })) roster.observeSystemFrame( diff --git a/src/main/codex/codex-structured-item-streams.ts b/src/main/codex/codex-structured-item-streams.ts index 306fcf276198..33b3fa7b3cb9 100644 --- a/src/main/codex/codex-structured-item-streams.ts +++ b/src/main/codex/codex-structured-item-streams.ts @@ -1,4 +1,3 @@ -import { agentJournalItemKey } from '../../shared/agent-session-journal-item-key' import { AGENT_JOURNAL_THREAD_SCOPE, type AgentJournalRowAttribution @@ -118,10 +117,8 @@ export function createCodexStructuredItemStreams( if (!translated.body) { return true } - return appendCodexItemAndPublish(deps.sink, state.identity, translated.body, { - coalescingKey: `checkpoint:${agentJournalItemKey(state.identity)}`, - ...attributionOf(key) - }).accepted + return appendCodexItemAndPublish(deps.sink, state.identity, translated.body, attributionOf(key)) + .accepted } const persist = (key: string, text: string, force: boolean): boolean => { diff --git a/src/main/codex/codex-structured-journal-generic-frames.ts b/src/main/codex/codex-structured-journal-generic-frames.ts index bdbed8717aeb..601596d0de5d 100644 --- a/src/main/codex/codex-structured-journal-generic-frames.ts +++ b/src/main/codex/codex-structured-journal-generic-frames.ts @@ -167,7 +167,6 @@ export class CodexJournalGenericFrames { } // A summary across evicted turns names no producer and belongs to no turn. const options = { - coalescingKey: `provider-frame-suppressed:codex:${bucket}`, ...(summary.producer ? this.deps.attributionFor(summary.producer.threadId, summary.producer.turnId) : { turnScope: AGENT_JOURNAL_THREAD_SCOPE }) diff --git a/src/main/codex/codex-structured-journal-goal-resume.test.ts b/src/main/codex/codex-structured-journal-goal-resume.test.ts index 9edeb0043322..a84f04e377d3 100644 --- a/src/main/codex/codex-structured-journal-goal-resume.test.ts +++ b/src/main/codex/codex-structured-journal-goal-resume.test.ts @@ -12,6 +12,7 @@ import { } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' import { CodexJournalGoals } from './codex-structured-journal-goals' import { MAX_CODEX_GOAL_THREADS } from './codex-structured-journal-limits' +import { withJournalQueueMembers } from '../native-chat/agent-session-wire/structured-agent-session-journal-double-test-support' import { testEventSinkLogging } from '../native-chat/agent-session-wire/structured-agent-session-logger-test-support' const THREAD = '01a08cc2-f96e-76d0-bb74-88b9bc0b03fc' @@ -49,7 +50,7 @@ function goalJournal( ...options }) // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: a fake journal exposing only the members the goal translator and deferred sink call. - const journal = { + const journal = withJournalQueueMembers({ get epoch() { return `epoch-${epochNumber}` }, @@ -81,7 +82,7 @@ function goalJournal( visit(item.itemId, item.sequence, item.body) } } - } as unknown as StructuredAgentSessionEventTarget['journal'] + }) as unknown as StructuredAgentSessionEventTarget['journal'] const target = { journal, fence: 1, diff --git a/src/main/codex/codex-structured-journal-translation-settlement.test.ts b/src/main/codex/codex-structured-journal-translation-settlement.test.ts index 6f00bdbe2b85..ea25ad2dcfda 100644 --- a/src/main/codex/codex-structured-journal-translation-settlement.test.ts +++ b/src/main/codex/codex-structured-journal-translation-settlement.test.ts @@ -25,6 +25,7 @@ import { CODEX_USER_INPUT_METHOD } from './codex-structured-prompt-replies' import type { CodexStructuredSessionEvent } from './codex-structured-session-adapter' +import { withJournalQueueMembers } from '../native-chat/agent-session-wire/structured-agent-session-journal-double-test-support' import { testEventSinkLogging } from '../native-chat/agent-session-wire/structured-agent-session-logger-test-support' const SESSION_ID = 'session-1' @@ -105,7 +106,8 @@ function deferredTarget( ): StructuredAgentSessionEventTarget { return { fence: 7, - journal: { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: a double for the journal members this path calls; the helper adds the in-order ones. + journal: withJournalQueueMembers({ appendItem: vi.fn(async (_identity: AgentJournalItemIdentity, body: AgentJournalItemBody) => { log.push(body) return { cursor: { epoch: 'e', sequence: log.length } } @@ -121,7 +123,7 @@ function deferredTarget( return { epoch: 'e', sequence: log.length } } ) - } as unknown as StructuredAgentSessionEventTarget['journal'], + }) as unknown as StructuredAgentSessionEventTarget['journal'], publish: vi.fn(() => { publishes.push('publish') }) diff --git a/src/main/codex/codex-structured-journal-translation-turns.ts b/src/main/codex/codex-structured-journal-translation-turns.ts index 349233a32d1d..33f138c6578d 100644 --- a/src/main/codex/codex-structured-journal-translation-turns.ts +++ b/src/main/codex/codex-structured-journal-translation-turns.ts @@ -123,7 +123,7 @@ export function publishCodexTurnLifecycle(input: { } else { input.sink.appendItem(identity, body, appendOptions) } - // Preserve first-work evidence when completion arrives before the journal drains. + // Keyed apart, so a completion's publication never replaces a start one still waiting to run. const publishOptions = { lifecycle: true, ...(input.state === 'running' diff --git a/src/main/codex/codex-structured-journal-translation.test.ts b/src/main/codex/codex-structured-journal-translation.test.ts index 523dd1894f83..23249a959b76 100644 --- a/src/main/codex/codex-structured-journal-translation.test.ts +++ b/src/main/codex/codex-structured-journal-translation.test.ts @@ -18,6 +18,7 @@ import { CODEX_USER_INPUT_METHOD } from './codex-structured-prompt-replies' import type { CodexStructuredSessionEvent } from './codex-structured-session-adapter' +import { withJournalQueueMembers } from '../native-chat/agent-session-wire/structured-agent-session-journal-double-test-support' import { testEventSinkLogging } from '../native-chat/agent-session-wire/structured-agent-session-logger-test-support' const SESSION_ID = 'session-1' @@ -103,7 +104,8 @@ function deferredTarget( ): StructuredAgentSessionEventTarget { return { fence: 7, - journal: { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: a double for the journal members this path calls; the helper adds the in-order ones. + journal: withJournalQueueMembers({ appendItem: vi.fn(async (_identity: AgentJournalItemIdentity, body: AgentJournalItemBody) => { log.push(body) return { cursor: { epoch: 'e', sequence: log.length } } @@ -119,7 +121,7 @@ function deferredTarget( return { epoch: 'e', sequence: log.length } } ) - } as unknown as StructuredAgentSessionEventTarget['journal'], + }) as unknown as StructuredAgentSessionEventTarget['journal'], publish: vi.fn(() => { publishes.push('publish') }) diff --git a/src/main/codex/codex-subagent-roster.test.ts b/src/main/codex/codex-subagent-roster.test.ts index dcaecb0e3e56..3f411fff1a3d 100644 --- a/src/main/codex/codex-subagent-roster.test.ts +++ b/src/main/codex/codex-subagent-roster.test.ts @@ -7,7 +7,13 @@ import type { } from '../../shared/agent-session-journal-types' import { MAX_SUBAGENT_FIELD_CHARS } from '../../shared/native-chat-subagent-summary' import { isSubagentGroupBlock, type NativeChatSubagentEntry } from '../../shared/native-chat-types' -import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' +import { + createDeferredStructuredAgentSessionEventSink, + type StructuredAgentSessionEventSink, + type StructuredAgentSessionEventTarget +} from '../native-chat/agent-session-wire/structured-agent-session-event-sink' +import { withJournalQueueMembers } from '../native-chat/agent-session-wire/structured-agent-session-journal-double-test-support' +import { testEventSinkLogging } from '../native-chat/agent-session-wire/structured-agent-session-logger-test-support' import { CodexSubagentRoster, codexSubagentGroupIdentity, @@ -106,42 +112,24 @@ function deliver( roster.handleItem({ threadId: THREAD, turnId, item }) } -/** - * A sink that coalesces the way the real queue does: by `coalescingKey` ALONE, - * with no op-kind check, and only draining when released. A fake that ignores - * the key cannot see an append being spliced out by its own publish. - */ -function createCoalescingHarness(): { +/** The real event sink, unbound until `drain`, over a journal double that records appends. */ +function createQueuedHarness(): { roster: CodexSubagentRoster appended: Appended[] drain: () => void } { const appended: Appended[] = [] - const queue: { key?: string; run: () => void }[] = [] let clock = 1_000 - const submit = (key: string | undefined, run: () => void): void => { - const at = key === undefined ? -1 : queue.findIndex((queued) => queued.key === key) - if (at >= 0) { - queue.splice(at, 1) - } - queue.push(key === undefined ? { run } : { key, run }) - } - const sink: StructuredAgentSessionEventSink = { - appendItem: () => {}, - appendTombstone: () => {}, - publish: () => {}, - tryAppendItem: (identity, body, options) => { - submit(options?.coalescingKey, () => appended.push({ identity, body })) - return { accepted: true } - }, - tryPublish: (options) => { - submit(options?.coalescingKey ?? 'publish', () => {}) - return { accepted: true } + const deferred = createDeferredStructuredAgentSessionEventSink(testEventSinkLogging()) + const journal = withJournalQueueMembers({ + appendItem: async (identity: AgentJournalItemIdentity, body: AgentJournalItemBody) => { + appended.push({ identity, body }) + return { cursor: { epoch: 'e', sequence: appended.length } } } - } + }) const roster = new CodexSubagentRoster({ turnScopeFor: () => AGENT_JOURNAL_THREAD_SCOPE, - sink, + sink: deferred.sink, primaryThreadId: () => THREAD, activeTurn: () => TURN, now: () => (clock += 1) @@ -149,17 +137,19 @@ function createCoalescingHarness(): { return { roster, appended, - drain: () => { - while (queue.length > 0) { - queue.shift()?.run() - } - } + drain: () => + deferred.bind({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the roster's rows reach only appendItem and the in-order members the helper adds. + journal: journal as unknown as StructuredAgentSessionEventTarget['journal'], + fence: 1, + publish: () => {} + }) } } describe('CodexSubagentRoster', () => { - it('does not let its own publish evict the still-queued roster append', () => { - const { roster, appended, drain } = createCoalescingHarness() + it('lands the roster append its own publish was buffered beside', () => { + const { roster, appended, drain } = createQueuedHarness() deliver( roster, @@ -167,8 +157,8 @@ describe('CodexSubagentRoster', () => { ) drain() - // Sharing the append's coalescing key with the publish spliced the append - // out of the queue, and `lastSerialized` then suppressed every retry. + // A publish once shared the append's coalescing key and spliced it out of the queue, and + // `lastSerialized` then suppressed every retry. expect(appended).toHaveLength(1) }) @@ -707,8 +697,8 @@ describe('CodexSubagentRoster', () => { accepted: true }) // The retry re-appends when the publish was the half that failed; the real - // queue coalesces those two by the group key into one journal write. What - // must not happen is the revision never being published at all. + // sink writes both, as revisions of the one group row. What must not happen + // is the revision never being published at all. expect(published).toHaveLength(1) const body = appended.at(-1)?.body expect( diff --git a/src/main/codex/codex-subagent-roster.ts b/src/main/codex/codex-subagent-roster.ts index 042183a6e6f8..0308dd70bba3 100644 --- a/src/main/codex/codex-subagent-roster.ts +++ b/src/main/codex/codex-subagent-roster.ts @@ -339,14 +339,7 @@ export class CodexSubagentRoster { group.lastSerialized = serialized // Deliberately unstamped: a child's frame can trigger this write, but the // row is the PARENT's roster of its children. - // The append coalesces per group so a burst collapses to the latest roster. - // The publish must NOT reuse that key: the queue coalesces by key alone, - // with no op-kind check, so a publish carrying it would splice out the - // still-queued append and the row would never reach the journal. - const options = { - coalescingKey: `codex-subagents:${group.groupId}`, - turnScope: group.turnScope - } + const options = { turnScope: group.turnScope } const admission = this.deps.sink.tryAppendItem ? this.deps.sink.tryAppendItem(group.identity, body, options) : (this.deps.sink.appendItem(group.identity, body, options), ADMITTED) diff --git a/src/main/native-chat/agent-session-journal/journal-epoch-controller.ts b/src/main/native-chat/agent-session-journal/journal-epoch-controller.ts index 51d3a4b2fd53..485e4d784174 100644 --- a/src/main/native-chat/agent-session-journal/journal-epoch-controller.ts +++ b/src/main/native-chat/agent-session-journal/journal-epoch-controller.ts @@ -9,6 +9,7 @@ import { publishNewEpoch } from './journal-epoch-rollover' import type { JournalLoad } from './journal-open' import type { AgentJournalEpochReason } from './journal-row-schema' import { assertJournalFence, assertJournalWritable } from './journal-write-guards' +import type { JournalWriteBody } from './journal-write-queue' export class JournalEpochController { constructor( @@ -16,7 +17,7 @@ export class JournalEpochController { identity: AgentSessionJournalIdentity now: () => number mintEpoch: () => string - serialize: (run: () => Promise) => Promise + serialize: (run: JournalWriteBody) => Promise database: () => JournalHostDatabase readOnly: () => boolean setReadOnly: (readOnly: boolean) => void @@ -48,7 +49,7 @@ export class JournalEpochController { * admitted append's sequence assignment and its commit. */ roll(reason: AgentJournalEpochReason, fence: number): Promise { - return this.deps.serialize(async () => { + return this.deps.serialize(() => { assertJournalWritable(this.deps.readOnly(), this.deps.identity.sessionId) this.start(reason, fence) this.deps.setReadOnly(false) @@ -61,7 +62,7 @@ export class JournalEpochController { fence: number, items: readonly JournalReplacementItem[] ): Promise { - return this.deps.serialize(async () => { + return this.deps.serialize(() => { assertJournalWritable(this.deps.readOnly(), this.deps.identity.sessionId) assertJournalFence(fence, this.deps.highestFence()) replaceJournalEpoch({ 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 e0bf1ad62381..146078d830c9 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 @@ -8,6 +8,10 @@ import type { JournalReducerState } from './journal-reducer' import type { JournalAppendResult, JournalItemAppendOptions } from './journal-store-contracts' import type { JournalRow } from './journal-row-schema' +const NOTHING_RESOLVED = new Error('journal_item_resolved_to_nothing') + +export type JournalResolvedItem = { identity: AgentJournalItemIdentity; body: AgentJournalItemBody } + export class JournalItemAppender { constructor( private readonly deps: { @@ -24,10 +28,45 @@ export class JournalItemAppender { const itemId = agentJournalItemKey(identity) return this.deps .enqueue(journalItemRowBuilder(this.deps.state, identity, body, options)) - .then((row) => ({ - cursor: { epoch: row.epoch, sequence: row.seq }, - itemId, - revision: (row as Extract).revision - })) + .then((row) => appendResult(row, itemId)) + } + + /** `resolve` reads the fold at the write's own place in the queue; null writes nothing. */ + appendResolved( + resolve: () => JournalResolvedItem | null, + options: JournalItemAppendOptions + ): Promise { + let itemId = '' + return this.deps + .enqueue((seq, ts) => { + const resolved = resolve() + if (resolved === null) { + throw NOTHING_RESOLVED + } + itemId = agentJournalItemKey(resolved.identity) + return journalItemRowBuilder( + this.deps.state, + resolved.identity, + resolved.body, + options + )(seq, ts) + }) + .then( + (row) => appendResult(row, itemId), + (error: unknown) => { + if (error === NOTHING_RESOLVED) { + return null + } + throw error + } + ) + } +} + +function appendResult(row: JournalRow, itemId: string): JournalAppendResult { + return { + cursor: { epoch: row.epoch, sequence: row.seq }, + itemId, + revision: (row as Extract).revision } } diff --git a/src/main/native-chat/agent-session-journal/journal-open.ts b/src/main/native-chat/agent-session-journal/journal-open.ts index 8f947272dbc7..cba4d530653d 100644 --- a/src/main/native-chat/agent-session-journal/journal-open.ts +++ b/src/main/native-chat/agent-session-journal/journal-open.ts @@ -147,6 +147,16 @@ export function startJournalRowFold(input: JournalRowFoldInput): { return { add, finish } } +/** `readJournalRowsAfterCursor` over one epoch, for a reader that supplies only the sequence. */ +export function journalRowsAfterReader( + db: Database.Database, + sessionId: string, + epoch: string, + limit?: number +): (afterSequence: number) => JournalRow[] { + return (afterSequence) => readJournalRowsAfterCursor(db, sessionId, epoch, afterSequence, limit) +} + /** Rows after a cursor, in sequence order. Stops at the first row this build * cannot parse, exactly as replay does. */ export function readJournalRowsAfterCursor( diff --git a/src/main/native-chat/agent-session-journal/journal-owed-import-test-support.ts b/src/main/native-chat/agent-session-journal/journal-owed-import-test-support.ts new file mode 100644 index 000000000000..6f1b3e9dbc3e --- /dev/null +++ b/src/main/native-chat/agent-session-journal/journal-owed-import-test-support.ts @@ -0,0 +1,112 @@ +// A chat still in the per-chat file an earlier build left, opened the way startup restore opens +// one: its copy into the host's database is owed, and the chat's first write pays it before that +// write lands. It is the one backlog a chat's write queue really holds. + +import { mkdir } from 'node:fs/promises' +import { dirname, join } from 'node:path' +import { + AGENT_JOURNAL_THREAD_SCOPE, + type AgentSessionJournalIdentity +} from '../../../shared/agent-session-journal-types' +import Database from '../../sqlite/sync-database' +import { + openTestJournalHostDatabase, + readTestJournalRows +} from './journal-host-database-test-support' +import { journalDirectoryFor, legacyJournalDatabaseFile } from './journal-paths' +import type { JournalStoredRow } from './journal-row-table' +import type { AgentSessionJournal } from './journal-store' +import { openAgentSessionJournal } from './journal-store-factory' + +const OWED_IMPORT_HISTORY_TEXT = 'history from the earlier build' + +/** Opens `identity`'s chat from a per-chat file of real history rows, its copy still owed. */ +export async function openJournalOwingImport(input: { + stateDirectory: string + identity: AgentSessionJournalIdentity + now?: () => number +}): Promise<{ journal: AgentSessionJournal; history: JournalStoredRow[] }> { + const { stateDirectory, identity } = input + const history = await historyRows(input) + await writePerChatFile(journalDirectoryFor(stateDirectory, identity), identity, history) + const journal = await openAgentSessionJournal({ + identity, + database: openTestJournalHostDatabase(stateDirectory), + ...(input.now ? { now: input.now } : {}), + deferPerSessionImport: true + }) + if (!journal.importPending) { + throw new Error('the chat opened with its copy already made') + } + return { journal, history } +} + +/** Real rows, written by today's store into a scratch database, as an earlier build wrote them. */ +async function historyRows(input: { + stateDirectory: string + identity: AgentSessionJournalIdentity + now?: () => number +}): Promise { + const scratch = join(input.stateDirectory, `scratch-${input.identity.sessionId}`) + const journal = await openAgentSessionJournal({ + identity: input.identity, + database: openTestJournalHostDatabase(scratch), + ...(input.now ? { now: input.now } : {}) + }) + await journal.appendSubmission({ + clientMessageId: 'client-history', + payloadFingerprint: 'fp-history', + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'add a retry' }] }, + fence: 1, + handoverRecorded: true + }) + await journal.appendItem( + { provider: 'codex', threadId: 'thread-history', turnId: 'turn-history', ordinal: 1 }, + { + kind: 'message', + role: 'assistant', + blocks: [{ type: 'text', text: OWED_IMPORT_HISTORY_TEXT }] + }, + { fence: 1, turnScope: AGENT_JOURNAL_THREAD_SCOPE } + ) + await journal.close() + return readTestJournalRows( + openTestJournalHostDatabase(scratch).db, + input.identity.sessionId, + journal.epoch + ) +} + +/** The per-chat file an earlier build left, in its own schema. */ +async function writePerChatFile( + directory: string, + identity: AgentSessionJournalIdentity, + rows: readonly JournalStoredRow[] +): Promise { + const path = legacyJournalDatabaseFile(directory) + await mkdir(dirname(path), { recursive: true }) + const db = new Database(path) + try { + db.pragma('journal_mode = WAL') + db.exec(` +CREATE TABLE journal_rows (session_id TEXT NOT NULL, epoch TEXT NOT NULL, seq INTEGER NOT NULL, + ts INTEGER NOT NULL, row_json TEXT NOT NULL, PRIMARY KEY (session_id, epoch, seq)); +CREATE TABLE journal_sessions (session_id TEXT PRIMARY KEY, epoch TEXT NOT NULL, updated_at INTEGER NOT NULL); +CREATE TABLE journal_repairs (session_id TEXT PRIMARY KEY, epoch TEXT NOT NULL, + content_from INTEGER NOT NULL, repaired_at INTEGER NOT NULL);`) + db.pragma('user_version = 2') + const insert = db.prepare( + 'INSERT INTO journal_rows (session_id, epoch, seq, ts, row_json) VALUES (?, ?, ?, ?, ?)' + ) + for (const row of rows) { + insert.run(identity.sessionId, row.epoch, row.seq, row.ts, row.rowJson) + } + db.prepare('INSERT INTO journal_sessions VALUES (?, ?, ?)').run( + identity.sessionId, + rows[0]!.epoch, + 1 + ) + } finally { + db.close() + } +} diff --git a/src/main/native-chat/agent-session-journal/journal-pending-submission-recovery.ts b/src/main/native-chat/agent-session-journal/journal-pending-submission-recovery.ts index c252a57d8a57..964ea42f9318 100644 --- a/src/main/native-chat/agent-session-journal/journal-pending-submission-recovery.ts +++ b/src/main/native-chat/agent-session-journal/journal-pending-submission-recovery.ts @@ -73,14 +73,18 @@ export async function rejectJournalQueuedSubmissions( const queued = journal .submissions() .filter((entry) => isQueuedAgentJournalSubmission(entry) && which(entry)) - for (const entry of queued) { - await journal.resolveDispatch({ - clientMessageId: entry.clientMessageId, - state: 'rejected', - ...rejection, - fence, - recovered: true - }) - } + // Issued together, so the fold shows none of them queued once this call returns: a Stop decides + // whether anything is working from it without awaiting the withdrawal. + await Promise.all( + queued.map((entry) => + journal.resolveDispatch({ + clientMessageId: entry.clientMessageId, + state: 'rejected', + ...rejection, + fence, + recovered: true + }) + ) + ) return queued.map((entry) => entry.clientMessageId) } diff --git a/src/main/native-chat/agent-session-journal/journal-queued-messages.ts b/src/main/native-chat/agent-session-journal/journal-queued-messages.ts index b8fcafc0b6c0..a66d43697f64 100644 --- a/src/main/native-chat/agent-session-journal/journal-queued-messages.ts +++ b/src/main/native-chat/agent-session-journal/journal-queued-messages.ts @@ -41,6 +41,7 @@ import { settleQueuedMessagesForRow } from './queued-message-settlement' import { AgentSessionJournalError, assertJournalWritable } from './journal-write-guards' +import type { JournalWriteBody, JournalWriteResult } from './journal-write-queue' /** Tombstones must outlive the window in which their operation id could still be admitted as new. */ export const QUEUED_MESSAGE_REPLAY_WINDOW_MS = @@ -49,7 +50,7 @@ export const QUEUED_MESSAGE_REPLAY_WINDOW_MS = export type JournalQueuedMessagesDeps = { sessionId: string now: () => number - serialize: (run: () => Promise) => Promise + serialize: (run: JournalWriteBody) => Promise database: () => JournalHostDatabase readOnly: () => boolean state: () => JournalReducerState @@ -207,10 +208,10 @@ export class JournalQueuedMessages { /** One standalone draft-table transaction on the journal's queue; one that * changed rows bumps the revision and notifies after COMMIT. */ private transact( - run: (db: Database.Database) => T, + run: (db: Database.Database) => JournalWriteResult, changed: (result: T) => boolean ): Promise { - return this.deps.serialize(async () => { + return this.deps.serialize(() => { assertJournalWritable(this.deps.readOnly(), this.deps.sessionId) const result = this.deps.database().transaction(run) if (changed(result)) { diff --git a/src/main/native-chat/agent-session-journal/journal-row-writer.test.ts b/src/main/native-chat/agent-session-journal/journal-row-writer.test.ts index aa0078b88a59..c0824a2e4cad 100644 --- a/src/main/native-chat/agent-session-journal/journal-row-writer.test.ts +++ b/src/main/native-chat/agent-session-journal/journal-row-writer.test.ts @@ -12,6 +12,7 @@ import { AGENT_SESSION_JOURNAL_SCHEMA_VERSION } from '../../../shared/agent-sess import type { JournalHostDatabase } from './journal-host-database' import type { JournalRow } from './journal-row-schema' import { JournalRowWriter } from './journal-row-writer' +import { JournalWriteQueue } from './journal-write-queue' import { openTestJournalHostDatabase, readTestJournalRows, @@ -60,10 +61,11 @@ describe('journal row writer', () => { function writerHarness() { const committedRows: JournalRow[] = [] let sequence = 1 + const queue = new JournalWriteQueue(SESSION_ID) const writer = new JournalRowWriter({ sessionId: SESSION_ID, now: () => 1, - serialize: (run) => run(), + serialize: (run) => queue.serialize(run), database: () => database, readOnly: () => readOnly, highestFence: () => 0, diff --git a/src/main/native-chat/agent-session-journal/journal-row-writer.ts b/src/main/native-chat/agent-session-journal/journal-row-writer.ts index a29585580ebe..b07fda9459bc 100644 --- a/src/main/native-chat/agent-session-journal/journal-row-writer.ts +++ b/src/main/native-chat/agent-session-journal/journal-row-writer.ts @@ -4,6 +4,7 @@ import type { JournalHostDatabase } from './journal-host-database' import type { AgentJournalCursor } from '../../../shared/agent-session-journal-types' import type { JournalRow } from './journal-row-schema' import { assertJournalFence, assertJournalWritable } from './journal-write-guards' +import type { JournalWriteBody } from './journal-write-queue' /** Runs between BEGIN IMMEDIATE and COMMIT, on the SAME connection as the row * insert; a throw rolls the whole append back. Synchronous by construction so @@ -13,7 +14,7 @@ export type JournalRowTransactionHook = (db: Database.Database, row: JournalRow) export type JournalRowWriterDeps = { sessionId: string now: () => number - serialize: (run: () => Promise) => Promise + serialize: (run: JournalWriteBody) => Promise database: () => JournalHostDatabase readOnly: () => boolean highestFence: () => number @@ -36,7 +37,7 @@ export class JournalRowWriter { build: (seq: number, ts: number) => JournalRow, hook?: JournalRowTransactionHook ): Promise { - return this.deps.serialize(async () => { + return this.deps.serialize(() => { assertJournalWritable(this.deps.readOnly(), this.deps.sessionId) const row = build(this.deps.nextSequence(), this.deps.now()) assertJournalFence(row.fence, this.deps.highestFence()) diff --git a/src/main/native-chat/agent-session-journal/journal-store-collaborators.ts b/src/main/native-chat/agent-session-journal/journal-store-collaborators.ts index b938efa3c5c9..65ea665879b5 100644 --- a/src/main/native-chat/agent-session-journal/journal-store-collaborators.ts +++ b/src/main/native-chat/agent-session-journal/journal-store-collaborators.ts @@ -20,6 +20,7 @@ import { JournalRowWriter } from './journal-row-writer' import { restoreJournalStore } from './journal-store-restore' import type { JournalRow } from './journal-row-schema' import type { AgentSessionJournal } from './journal-store' +import type { JournalWriteBody } from './journal-write-queue' export type JournalStoreHost = { /** Fires the journal's commit listener for a durable change that appended no @@ -31,7 +32,7 @@ export type JournalStoreHost = { legacyDirectory: string now: () => number mintEpoch: () => string - serialize: (run: () => Promise) => Promise + serialize: (run: JournalWriteBody) => Promise /** Leave a chat still in its per-chat file uncopied until its first use. */ deferPerSessionImport: boolean /** Work the chat's next write waits for. */ diff --git a/src/main/native-chat/agent-session-journal/journal-store.ts b/src/main/native-chat/agent-session-journal/journal-store.ts index 3de15cc3df60..5a4880155dec 100644 --- a/src/main/native-chat/agent-session-journal/journal-store.ts +++ b/src/main/native-chat/agent-session-journal/journal-store.ts @@ -27,7 +27,7 @@ import { import type { JournalReplacementItem } from './journal-epoch-replacement' import { readJournalSince } from './journal-cursor' import type { JournalHostDatabase } from './journal-host-database' -import { readJournalRowsAfterCursor, type JournalLoad } from './journal-open' +import { journalRowsAfterReader, type JournalLoad } from './journal-open' import { markJournalPendingSubmissionsUnknown, rejectJournalPendingSubmissions, @@ -68,7 +68,7 @@ import type { JournalEpochController } from './journal-epoch-controller' import { JournalWriteQueue } from './journal-write-queue' import { createJournalStoreCollaborators } from './journal-store-collaborators' import { journalStoreLoadedFields } from './journal-store-open' -import type { JournalItemAppender } from './journal-item-appender' +import type { JournalItemAppender, JournalResolvedItem } from './journal-item-appender' import type { JournalLifecycleBatchAppender } from './journal-lifecycle-batch-appender' import type { JournalStopMarks } from './journal-stop-marks' @@ -195,11 +195,12 @@ export class AgentSessionJournal { /** * Resolves once the chat's rows are in the host's database. A restore's open serves a chat still - * in its per-chat file from a read-only fold of it; the copy runs before the chat's first write, - * and a reader that needs rows (forward pages, catch-up) awaits it here. + * in its per-chat file from a read-only fold of it; the copy runs before the chat's first write. + * A reader that needs rows (forward pages, catch-up) and every mutation's open await it here, so + * each reads the fold after every earlier write. */ whenImported(): Promise { - return this.queue.serialize(async () => undefined) + return this.queue.serialize(() => undefined) } get importPending(): boolean { @@ -275,23 +276,17 @@ export class AgentSessionJournal { canonicalItemId = (itemId: string): string => resolveJournalItemId(this.state, itemId) + /** Reads the fold with every write issued before this call committed, and none issued after: at + * once unless writes still wait behind an owed import or a running write. */ + readInOrder(read: () => T): Promise { + return this.queue.readInOrder(read) + } + readSince(cursor: AgentJournalCursor, limit?: number): JournalReadSince { - return readJournalSince( - { - state: this.state, - rowsAfter: (afterSequence) => - readJournalRowsAfterCursor( - this.database.db, - this.identity.sessionId, - this.state.epoch, - afterSequence, - limit - ), - readOnly: this.readOnly - }, - cursor, - () => this.cursor() - ) + const { sessionId } = this.identity + const rowsAfter = journalRowsAfterReader(this.database.db, sessionId, this.state.epoch, limit) + const source = { state: this.state, rowsAfter, readOnly: this.readOnly } + return readJournalSince(source, cursor, () => this.cursor()) } /** Upsert by stable identity. The revision is assigned here so a caller @@ -304,6 +299,14 @@ export class AgentSessionJournal { return this.itemAppender.append(identity, body, options) } + /** An upsert whose row is chosen from the fold at its own turn in the queue; null writes nothing. */ + appendResolvedItem( + resolve: () => JournalResolvedItem | null, + options: JournalItemAppendOptions + ): Promise { + return this.itemAppender.appendResolved(resolve, options) + } + appendTombstone( identity: AgentJournalItemIdentity, options: JournalTombstoneInput diff --git a/src/main/native-chat/agent-session-journal/journal-write-queue.test.ts b/src/main/native-chat/agent-session-journal/journal-write-queue.test.ts new file mode 100644 index 000000000000..3cdd554cc4e8 --- /dev/null +++ b/src/main/native-chat/agent-session-journal/journal-write-queue.test.ts @@ -0,0 +1,252 @@ +// A write has landed in the fold when its call returns, except during an owed import, when it lands +// in queue order. A write issued from inside a running write joins the line behind it, never nested. +// A read in the queue always settles: behind a write that failed, and refused once closed. + +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { agentJournalItemKey } from '../../../shared/agent-session-journal-item-key' +import { + AGENT_JOURNAL_THREAD_SCOPE, + type AgentJournalItemIdentity, + type AgentJournalMessageItem +} from '../../../shared/agent-session-journal-types' +import { + closeTestJournalHostDatabases, + openTestJournalHostDatabase +} from './journal-host-database-test-support' +import { openJournalOwingImport } from './journal-owed-import-test-support' +import type { AgentSessionJournal } from './journal-store' +import { openAgentSessionJournal } from './journal-store-factory' +import { JournalWriteQueue } from './journal-write-queue' + +const IDENTITY = { + sessionId: 'session-1', + workspaceId: 'workspace-1', + hostId: 'host-1', + agent: 'codex', + providerHandle: { kind: 'codex', threadId: 'thread-1' } +} as const + +const reply = (text: string): AgentJournalMessageItem => ({ + kind: 'message', + role: 'assistant', + blocks: [{ type: 'text', text }] +}) + +const item = (ordinal: number): AgentJournalItemIdentity => ({ + provider: 'codex', + threadId: 'thread-1', + turnId: 'turn-1', + ordinal +}) + +const OPTIONS = { fence: 1, turnScope: AGENT_JOURNAL_THREAD_SCOPE } + +let root = '' +const opened: AgentSessionJournal[] = [] + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-journal-write-queue-')) +}) + +afterEach(async () => { + await Promise.allSettled(opened.splice(0).map((journal) => journal.close())) + closeTestJournalHostDatabases() + await rm(root, { recursive: true, force: true }) +}) + +async function idleJournal(): Promise { + const journal = await openAgentSessionJournal({ + identity: IDENTITY, + database: openTestJournalHostDatabase(root) + }) + opened.push(journal) + return journal +} + +async function journalOwingImport(): Promise<{ journal: AgentSessionJournal; history: number }> { + const { journal, history } = await openJournalOwingImport({ + stateDirectory: root, + identity: IDENTITY + }) + opened.push(journal) + return { journal, history: history.length } +} + +function sequenceOf(journal: AgentSessionJournal, ordinal: number): number | undefined { + const key = agentJournalItemKey(item(ordinal)) + let found: number | undefined + journal.visitItems((itemId, sequence) => { + if (itemId === key) { + found = sequence + } + }) + return found +} + +describe('when a journal write lands', () => { + it('is in the fold when its call returns, on an idle queue', async () => { + const journal = await idleJournal() + const before = journal.cursor().sequence + + const first = journal.appendItem(item(1), reply('one'), OPTIONS) + expect(journal.itemBody(agentJournalItemKey(item(1)))).toEqual(reply('one')) + const second = journal.appendItem(item(2), reply('two'), OPTIONS) + expect(journal.cursor().sequence).toBe(before + 2) + expect(sequenceOf(journal, 2)).toBeGreaterThan(sequenceOf(journal, 1)!) + + await expect(Promise.all([first, second])).resolves.toHaveLength(2) + }) + + it('lands in queue order, behind the history it copies, while an import is owed', async () => { + const { journal, history } = await journalOwingImport() + + const first = journal.appendItem(item(1), reply('one'), OPTIONS) + const second = journal.appendItem(item(2), reply('two'), OPTIONS) + // Neither has landed: both wait behind the copy the first write pays. + expect(journal.itemBody(agentJournalItemKey(item(1)))).toBeNull() + expect(journal.importPending).toBe(true) + + await second + await first + expect(journal.importPending).toBe(false) + expect(sequenceOf(journal, 1)).toBe(history + 1) + expect(sequenceOf(journal, 2)).toBe(history + 2) + // The copy is paid: the next write lands at its call again. + journal.appendItem(item(3), reply('three'), OPTIONS).catch(() => undefined) + expect(sequenceOf(journal, 3)).toBe(history + 3) + }) + + it('joins the line behind a write it was issued from inside, never nested in it', async () => { + const journal = await idleJournal() + let inner: Promise | null = null + let innerLandedDuringOuter = false + + // Issued from inside the outer write, before that write has committed its row. + const outer = journal.appendResolvedItem(() => { + inner = journal.appendItem(item(2), reply('inner'), OPTIONS) + innerLandedDuringOuter = journal.itemBody(agentJournalItemKey(item(2))) !== null + return { identity: item(1), body: reply('outer') } + }, OPTIONS) + + expect(innerLandedDuringOuter).toBe(false) + expect(journal.itemBody(agentJournalItemKey(item(1)))).toEqual(reply('outer')) + expect(journal.itemBody(agentJournalItemKey(item(2)))).toBeNull() + await outer + await inner + expect(sequenceOf(journal, 2)).toBe(sequenceOf(journal, 1)! + 1) + }) +}) + +describe('the journal write queue', () => { + it('runs a write before it returns when nothing is owed or running', () => { + const queue = new JournalWriteQueue('session-1') + const ran: string[] = [] + void queue.serialize(() => { + ran.push('write') + }) + expect(ran).toEqual(['write']) + }) + + it('takes only a synchronous body', async () => { + const queue = new JournalWriteQueue('session-1') + // @ts-expect-error an await inside a write would let a later write land first + await queue.serialize(async () => undefined) + }) + + it('answers a write that throws with a rejection, never a throw', async () => { + const queue = new JournalWriteQueue('session-1') + const failed = queue.serialize(() => { + throw new Error('disk I/O error') + }) + await expect(failed).rejects.toThrow('disk I/O error') + const ran: string[] = [] + void queue.serialize(() => { + ran.push('next') + }) + expect(ran).toEqual(['next']) + }) + + it('runs a write issued from inside a running one after it, in issue order', async () => { + const queue = new JournalWriteQueue('session-1') + const ran: string[] = [] + let nested: Promise | null = null + const outer = queue.serialize(() => { + nested = queue.serialize(() => { + ran.push('nested') + }) + ran.push('outer') + }) + const after = queue.serialize(() => { + ran.push('after') + }) + expect(ran).toEqual(['outer']) + await Promise.all([outer, nested, after]) + expect(ran).toEqual(['outer', 'nested', 'after']) + }) + + it('runs every write admitted while work is owed after it, in order', async () => { + const queue = new JournalWriteQueue('session-1') + const ran: string[] = [] + const owed = Promise.withResolvers() + queue.owe(async () => { + await owed.promise + ran.push('owed') + }) + const first = queue.serialize(() => { + ran.push('first') + }) + const read = queue.readInOrder(() => ran.push('read')) + owed.resolve() + const second = queue.serialize(() => { + ran.push('second') + }) + await Promise.all([first, read, second]) + expect(ran).toEqual(['owed', 'first', 'read', 'second']) + expect(queue.owing).toBe(false) + }) +}) + +describe('a read in the journal write queue', () => { + it('runs at once when no write waits', () => { + const queue = new JournalWriteQueue('session-1') + const ran: string[] = [] + void queue.readInOrder(() => ran.push('read')) + expect(ran).toEqual(['read']) + }) + + it('does not pay owed work, and runs behind a write that pays it', async () => { + const queue = new JournalWriteQueue('session-1') + const ran: string[] = [] + queue.owe(async () => { + ran.push('owed') + }) + void queue.readInOrder(() => ran.push('read before')) + expect(ran).toEqual(['read before']) + const write = queue.serialize(() => { + ran.push('write') + }) + const read = queue.readInOrder(() => ran.push('read after')) + await Promise.all([write, read]) + expect(ran).toEqual(['read before', 'owed', 'write', 'read after']) + }) + + it('runs behind a write that failed', async () => { + const queue = new JournalWriteQueue('session-1') + queue.owe(async () => undefined) + const failed = queue.serialize(() => { + throw new Error('disk I/O error') + }) + const read = queue.readInOrder(() => 'read') + await expect(failed).rejects.toThrow('disk I/O error') + await expect(read).resolves.toBe('read') + }) + + it('is refused at once after close, not queued', async () => { + const queue = new JournalWriteQueue('session-1') + queue.markClosed() + await expect(queue.readInOrder(() => 'read')).rejects.toMatchObject({ code: 'journal_closed' }) + }) +}) diff --git a/src/main/native-chat/agent-session-journal/journal-write-queue.ts b/src/main/native-chat/agent-session-journal/journal-write-queue.ts index 13fd9e4b96c2..74ef33472bd4 100644 --- a/src/main/native-chat/agent-session-journal/journal-write-queue.ts +++ b/src/main/native-chat/agent-session-journal/journal-write-queue.ts @@ -6,9 +6,27 @@ import { AgentSessionJournalError } from './journal-write-guards' -/** Admission is checked at ENQUEUE and is permanent. */ +/** What a write body returns: never a promise, since an await inside one would let a later write + * land first. */ +export type JournalWriteResult = T extends PromiseLike ? never : T +export type JournalWriteBody = () => JournalWriteResult + +/** + * A write has landed in the fold when its call returns, except during an owed import, when it + * lands in queue order. + * + * A write finds the queue idle unless work is owed, a write is running, or writes wait in line; + * then it runs before `serialize` returns. Every write body is synchronous (`JournalWriteBody` + * refuses a promise), so it has committed by then. Otherwise it joins the line: behind the owed + * import it pays first, or behind the running write it was issued from, never nested inside it. + * Admission is checked at ENQUEUE and is permanent. + */ export class JournalWriteQueue { + /** Settles once every write admitted so far has, whatever its outcome. */ private writes: Promise = Promise.resolve() + /** Writes that joined the line and have not settled. */ + private waiting = 0 + private running = false private closed = false /** Runs before the next write, and stays owed until it succeeds. */ private owed: (() => Promise) | null = null @@ -19,16 +37,34 @@ export class JournalWriteQueue { this.closed = true } - serialize(run: () => Promise): Promise { + serialize(run: JournalWriteBody): Promise { if (this.closed) { - return Promise.reject( - new AgentSessionJournalError( - 'journal_closed', - `agent-session journal for ${this.sessionId} is closed` - ) - ) + return Promise.reject(this.closedError()) } - return this.serializePastGate(run) + return this.owed !== null || this.lineBusy + ? this.join(run, this.owed !== null) + : this.runNow(run) + } + + /** Runs `read` after every write admitted before it, whatever each one's outcome, and ahead of any + * admitted after: at once when none waits. Owed work is not paid for a read. A closed queue + * refuses it, as it refuses a write: its fold may be replaced. */ + readInOrder(read: () => T): Promise { + if (this.closed) { + return Promise.reject(this.closedError()) + } + return this.lineBusy ? this.join(read, false) : this.runNow(read) + } + + private get lineBusy(): boolean { + return this.running || this.waiting > 0 + } + + private closedError(): AgentSessionJournalError { + return new AgentSessionJournalError( + 'journal_closed', + `agent-session journal for ${this.sessionId} is closed` + ) } owe(work: () => Promise): void { @@ -52,10 +88,33 @@ export class JournalWriteQueue { return this.writes.then(() => undefined) } - private serializePastGate(run: () => Promise): Promise { - // Only a write admitted while work is owed takes the extra step, so no other write's timing moves. - const started = this.owed ? this.writes.then(this.payOwed).then(run) : this.writes.then(run) + /** Runs before returning; a write issued from inside it joins the line behind it. */ + private runNow(run: () => T): Promise { + let release: (settled: Promise) => void = () => undefined + this.writes = new Promise((resolve) => { + release = resolve + }) + this.running = true + let result: Promise + try { + result = Promise.resolve(run()) + } catch (error) { + result = Promise.reject(error) + } finally { + this.running = false + } + release(result.catch(() => undefined)) + return result + } + + private join(run: () => T, paysOwed: boolean): Promise { + const started = paysOwed ? this.writes.then(this.payOwed).then(run) : this.writes.then(run) this.writes = started.catch(() => undefined) + this.waiting++ + const settle = (): void => { + this.waiting-- + } + started.then(settle, settle) return started } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-append-delivery.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-append-delivery.test.ts index e2adb2953cb4..6111e9029621 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-append-delivery.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-append-delivery.test.ts @@ -37,10 +37,10 @@ let host: StructuredAgentSessionHost let acquire: Mock let generation = 0 -/** Everything a live subscriber was sent after it opened. */ -function liveReader() { +/** Everything a live subscriber was sent after its opening snapshot. */ +async function liveReader() { const events: AgentSessionSubscribeEvent[] = [] - host.subscribe({ id: 'pane', sessionId: SESSION, emit: (event) => events.push(event) }) + await host.subscribe({ id: 'pane', sessionId: SESSION, emit: (event) => events.push(event) }) const opened = events.length const received = () => { const items: AgentJournalRenderItem[] = [] @@ -158,7 +158,7 @@ afterEach(async () => { describe('an open chat receives every row its journal commits', () => { it('shows a failed start whose lease could not be handed back', async () => { const held = await send('hello') - const pane = liveReader() + const pane = await liveReader() // The exit settles the journal, then fails to release the lease: nothing moves the fence. vi.spyOn(store, 'transitionHandoff').mockRejectedValueOnce(new Error('record store busy')) @@ -177,7 +177,7 @@ describe('an open chat receives every row its journal commits', () => { }) it('shows a revision the provider queued with no publish behind it', async () => { - const pane = liveReader() + const pane = await liveReader() const identity = { provider: 'orca' as const, clientMessageId: 'context-usage' } const body = { kind: 'status' as const, text: 'context usage answered after the turn' } @@ -194,7 +194,7 @@ describe('an open chat receives every row its journal commits', () => { }) it('shows a row appended straight to the journal', async () => { - const pane = liveReader() + const pane = await liveReader() const journal = host['sessions'].get(SESSION)?.journal if (!journal) { throw new Error('the attached chat has no journal') @@ -215,7 +215,7 @@ describe('an open chat receives every row its journal commits', () => { describe('an open chat receives each row once', () => { it('when the provider frame that wrote it also publishes', async () => { - const pane = liveReader() + const pane = await liveReader() const sink = providerSink() const journal = host['sessions'].get(SESSION)?.journal if (!journal) { @@ -237,7 +237,7 @@ describe('an open chat receives each row once', () => { }) it('when a writer publishes the row it appended', async () => { - const pane = liveReader() + const pane = await liveReader() const journal = host['sessions'].get(SESSION)?.journal if (!journal) { throw new Error('the attached chat has no journal') diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-chat-stop.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-chat-stop.ts index 77a6fa3434e0..c89c0fc3d6bd 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-chat-stop.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-chat-stop.ts @@ -7,6 +7,7 @@ import { agentSessionFailureWords } from '../../../shared/agent-session-failure-words' import { agentSessionFailureFact } from '../../../shared/agent-session-failure' +import { isQueuedAgentJournalSubmission } from '../../../shared/agent-session-queued-submission' import type { AgentSessionCancelResult, AgentSessionMutationEnvelope, @@ -18,14 +19,18 @@ import { } from './structured-agent-session-mutation-context' import type { StructuredAgentSessionCaller } from './structured-agent-session-host-types' import type { MutationPlan } from './structured-agent-session-mutation-plans' -import { runRecordedStop, stopReachesUnrecordedWork } from './structured-agent-session-queued-stop' +import { + runRecordedStop, + stopReachesUnrecordedWork, + withdrawQueuedForStop +} from './structured-agent-session-queued-stop' import { openForWrite, structuredAgentSessionFailureWordsContext } from './structured-agent-session-send-preparation' import { endStoppedStructuredAgentSession, - isMainAgentWorkingOnceFlushed, + isMainAgentWorking, performCancel, type StructuredAgentSessionStopWindDown } from './structured-agent-session-turns-cancel' @@ -51,6 +56,8 @@ export function mutateWithChatStop( const { sessionId } = envelope // Set by the Stop's step only when its provider's session ends; a replay leaves it unset. let windDown: StructuredAgentSessionStopWindDown | undefined + // The Stop's event, still landing when its session ends: the next step holds the lane for it. + let eventAfterEnd: Promise | undefined const named = turnId !== undefined ? { turnId } : {} // Its own step wrote the Stop's event first. const stopChild = () => context.stopAgent(sessionId, { recorded: 'user-stop' }) @@ -67,52 +74,74 @@ export function mutateWithChatStop( ...(ctx.adapter.stopEndsSession?.(ctx.sessionId) === true ? {} : named) }, async (tookEffect) => { + // Read before the withdrawal it decides on is issued. + const hadQueued = ctx.journal.submissions().some(isQueuedAgentJournalSubmission) // Stop withdraws every queued SUBMISSION first, whatever the start or the child is doing. - const withdrawn = await ctx.journal.rejectQueuedSubmissions( - ctx.fence, - agentSessionFailureWords(agentSessionFailureFact('cancelled'), { surface: 'rejection' }) + // Issued, not awaited: the interrupt never waits on bookkeeping. (The open paid any owed import.) + const withdrew = withdrawQueuedForStop(ctx, () => + ctx.journal.rejectQueuedSubmissions( + ctx.fence, + agentSessionFailureWords(agentSessionFailureFact('cancelled'), { surface: 'rejection' }) + ) ) const child = context.sessions.get(ctx.sessionId)?.child if (child?.phase === 'starting') { // A start that may never land is the one thing here Stop has to end; the chat stays. - await tookEffect() - await stopChild() + // The event is issued first and lands behind the withdrawal, in the journal's queue order. + const effect = tookEffect() + try { + await stopChild() + } finally { + // Even a failed Stop holds the lane until its event lands: the drain shares the lane. + await effect + } return { ok: true, value: { ...named, cancelled: true } } } // A Stop naming no turn ends nothing more unless the session reads working, by the rule - // every session list and the chat's own Stop read it. - const inFlight = turnId !== undefined || (await isMainAgentWorkingOnceFlushed(ctx)) + // every session list and the chat's own Stop read it, over the fold as it stands. + const inFlight = turnId !== undefined || isMainAgentWorking(ctx) const record = context.deps.store.getRecord(ctx.sessionId) if (!child || !inFlight) { - if (withdrawn.length > 0) { + // Nothing to interrupt, so the answer may wait for the withdrawal. + const withdrewAny = await withdrew + if (withdrewAny) { await tookEffect() } - return { ok: true, value: { ...named, cancelled: withdrawn.length > 0 } } - } - // Awaited until journal appends are synchronous; then issued here, and a `finally` awaits it. - if (withdrawn.length > 0 || (await stopReachesUnrecordedWork(ctx, turnId))) { - await tookEffect() + return { ok: true, value: { ...named, cancelled: withdrewAny } } } - return performCancel( - { ...ctx, failureTextContext: structuredAgentSessionFailureWordsContext(record) }, - { - clientOperationId: envelope.clientOperationId, - ...named, - stopChild, - onStopChildError: (error) => - context.deps.logger.warn('ending the agent process on Stop failed', { - scope: 'stop-child', - sessionId, - error - }), - // The host drops its child only once the exit is proven, and nothing else runs meanwhile. - childReleased: () => context.sessions.get(sessionId)?.child !== child, - endSession: (owed) => { - windDown = owed - }, - withdrewQueued: withdrawn.length > 0 + // Issued, not awaited, before the interrupt or any child end; the `finally` awaits it. + const effect = + hadQueued || stopReachesUnrecordedWork(ctx, turnId) ? tookEffect() : Promise.resolve() + try { + return await performCancel( + { ...ctx, failureTextContext: structuredAgentSessionFailureWordsContext(record) }, + { + clientOperationId: envelope.clientOperationId, + ...named, + stopChild, + onStopChildError: (error) => + context.deps.logger.warn('ending the agent process on Stop failed', { + scope: 'stop-child', + sessionId, + error + }), + // The host drops its child only once the exit is proven, and nothing else runs meanwhile. + childReleased: () => context.sessions.get(sessionId)?.child !== child, + endSession: (owed) => { + windDown = owed + }, + withdrewQueued: withdrew + } + ) + } finally { + // `effect` never rejects, so the Stop's own error survives. A session-ending Stop hands this + // hold to its next step, so the child's end never waits on it. + if (windDown) { + eventAfterEnd = effect + } else { + await effect } - ) + } } ) const result = mutateStructuredAgentSession( @@ -141,6 +170,7 @@ export function mutateWithChatStop( }) ) } + await eventAfterEnd }) return result } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-echo-working.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-echo-working.test.ts index 747319c3c66b..ac1111ac629e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-echo-working.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-echo-working.test.ts @@ -96,7 +96,7 @@ function settled(): Promise { return new Promise((resolve) => setTimeout(resolve, 20)) } -it('reads working at every published frame from the send through the echo that opens its turn', async () => { +async function expectWorkingThroughEcho(backlog: boolean): Promise { const submissions = new Map() const turns = new Map() const working: boolean[] = [] @@ -138,6 +138,20 @@ it('reads working at every published frame from the send through the echo that o }) await settled() const connection = claude.current.connections[0]! + if (backlog) { + // The previous cycle's result in the same read, its write issued just ahead of the echo's. + connection.handlers.onMessage?.({ + type: 'result', + subtype: 'success', + uuid: 'result-0', + session_id: PROVIDER_SESSION_ID, + duration_ms: 1, + duration_api_ms: 1, + num_turns: 1, + is_error: false, + result: '' + }) + } // Claude echoes the written message back, which is what opens its turn. connection.handlers.onMessage?.({ ...connection.sent.at(-1)!, uuid: 'echo-uuid' }) await settled() @@ -147,4 +161,10 @@ it('reads working at every published frame from the send through the echo that o ]) expect([...turns.values()]).toEqual(['running']) expect(working).not.toContain(false) -}) +} + +it('reads working at every published frame from the send through the echo that opens its turn', () => + expectWorkingThroughEcho(false)) + +it('keeps the settlement behind the turn its echo opens when the previous result arrives in the same read', () => + expectWorkingThroughEcho(true)) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-unproven-stop-send.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-unproven-stop-send.test.ts index b86e25674641..d318e962a121 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-unproven-stop-send.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-unproven-stop-send.test.ts @@ -545,12 +545,12 @@ it('notes why a message waits when another operation failed its retry before the it('keeps an option change at rest when the Stop proved the exit and only its bookkeeping keeps failing', async () => { const connection = claude.connections[0]! await openTurn(connection) - // The close proves the exit; draining what the old agent wrote fails for the Stop. + // The close proves the exit; draining what the old agent wrote fails for the Stop. The Stop reads + // the journal without a drain, so its close is the first and only drain to fail here. const barrierLost = { ok: false as const, error: new Error('drain barrier lost') } const drained = vi .spyOn(host['runtimeState'].eventSinkFor(SESSION), 'drained') .mockResolvedValueOnce(barrierLost) - .mockResolvedValueOnce(barrierLost) await expect(stop()).resolves.toMatchObject({ ok: true, value: { cancelled: true } }) frame(connection, INTERRUPTED_RESULT) await laneDrained() diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-codex-stop-row.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-codex-stop-row.test.ts index ef254fdc765c..7f828a484688 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-codex-stop-row.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-codex-stop-row.test.ts @@ -157,22 +157,6 @@ function interruptFailure(failure: 'internal error' | 'unanswered'): Error { : new CodexAppServerTimeoutError('codex app-server turn/interrupt exceeded 30000ms') } -/** The journal's writes wait a moment, so frames Orca received are not yet in the journal. */ -function holdJournalWrites(): void { - const journal = host['sessions'].get(SESSION)!.journal - const released = new Promise((resolve) => setTimeout(resolve, 20)) - const appendItem = journal.appendItem.bind(journal) - const appendLifecycleBatch = journal.appendLifecycleBatch.bind(journal) - vi.spyOn(journal, 'appendItem').mockImplementation(async (...args) => { - await released - return appendItem(...args) - }) - vi.spyOn(journal, 'appendLifecycleBatch').mockImplementation(async (...args) => { - await released - return appendLifecycleBatch(...args) - }) -} - /** Codex picked the follow-up's turn and answered the send, and has not started it. */ async function followUpUnopened(): Promise { const sent = await send('and then this') @@ -369,8 +353,8 @@ describe('a Codex Stop whose interrupt failed', () => { it("decides on Codex's frames received before the interrupt failed, not on the journal's last write", async () => { await runningTurn() + // Each frame lands in the journal as Codex hands it over, before the interrupt fails. codex.routes['turn/interrupt'] = () => { - holdJournalWrites() turns.end('completed') notify('turn/started', { threadId: THREAD, turn: { id: 'turn-2', status: 'inProgress' } }) throw interruptFailure('internal error') diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts index 9eb5cb02ea47..b1654ff70790 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts @@ -159,7 +159,6 @@ export type StructuredAgentSessionCommandHandoverContext = { record: () => AgentSessionRecord | null /** The session's child records, the same read the strip and conversation-command admission use. */ childWork: () => readonly AgentChildWorkView[] | undefined - flushStreamedEvents: () => Promise now: () => number } @@ -170,8 +169,7 @@ export async function handOverStructuredAgentSessionCommand( body: AgentJournalMessageItem ): Promise { const { clientMessageId } = submission - // Provider frames already received decide whether a turn is running. - await ctx.flushStreamedEvents() + // Provider frames already received decide whether a turn is running: each landed at its call. const blocked = commandBlocked(ctx, body) if (blocked) { await ctx.journal.resolveDispatch({ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts index 155837a01e16..aab219c779d6 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts @@ -369,34 +369,22 @@ describe('a Stop that names no turn', () => { it('interrupts a turn whose accepted send is in the journal before its row lands', async () => { await acceptedWithTurnRowUnlanded() - const drain = host.flushStreamedEvents - vi.spyOn(host, 'flushStreamedEvents').mockImplementation((sessionId) => { - events!.appendItem( - { provider: 'legacy', agent: 'codex', sessionId, recordId: 'turn-lifecycle:turn-2' }, - { - kind: 'status', - text: 'Agent is working…', - turnLifecycle: { turnId: 'turn-2', state: 'running' } - }, - { turnScope: AGENT_JOURNAL_THREAD_SCOPE } - ) - return drain(sessionId) - }) + // Emitted as the Stop arrives: the Stop's read takes its place behind it in the journal. + events!.appendItem( + { provider: 'legacy', agent: 'codex', sessionId: SESSION, recordId: 'turn-lifecycle:turn-2' }, + { + kind: 'status', + text: 'Agent is working…', + turnLifecycle: { turnId: 'turn-2', state: 'running' } + }, + { turnScope: AGENT_JOURNAL_THREAD_SCOPE } + ) expect(await stop()).toMatchObject({ ok: true, value: { cancelled: true } }) expect(cancelTurn).toHaveBeenCalledOnce() expect(await statusRows()).toContain('Cancellation requested.') }) - it('still interrupts when draining the streamed rows fails', async () => { - await acceptedWithTurnRowUnlanded() - vi.spyOn(host, 'flushStreamedEvents').mockRejectedValueOnce(new Error('sink barrier failed')) - - expect(await stop()).toMatchObject({ ok: true, value: { cancelled: true } }) - expect(cancelTurn).toHaveBeenCalledOnce() - expect(await statusRows()).toEqual(['Cancellation requested.']) - }) - it('is a quiet no-op with nothing in flight', async () => { expect(await stop()).toMatchObject({ ok: true, value: { cancelled: false } }) expect(cancelTurn).not.toHaveBeenCalled() diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts index 0414836f4a5e..83ab0e8e6feb 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts @@ -73,7 +73,6 @@ export type StructuredAgentSessionDeliveryLoopDeps = { logger: StructuredAgentSessionLogger record: (sessionId: string) => AgentSessionRecord | null readChildWork: (sessionId: string) => readonly AgentChildWorkView[] | undefined - flushStreamedEvents: (sessionId: string) => Promise now: () => number } @@ -266,7 +265,6 @@ export class StructuredAgentSessionDeliveryLoop { failureTextContext: this.deps.failureTextContext(sessionId), record: () => this.deps.record(sessionId), childWork: () => this.deps.readChildWork(sessionId), - flushStreamedEvents: () => this.deps.flushStreamedEvents(sessionId), now: this.deps.now }, next diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-event-sink-queue.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-event-sink-queue.ts index 60ca5d535c53..5d657bf0928f 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-event-sink-queue.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-event-sink-queue.ts @@ -15,23 +15,35 @@ export type StructuredAgentSessionSinkOperation = { /** Lifecycle rows use their own bounded reservation budget. */ lifecycleBytes?: number lifecycle?: boolean - coalescingKey?: string - /** The queued operation with this key wins, as the journal keeps a settlement's first batch. */ - keepsFirst?: boolean + /** Marks a publication, which writes no row: it runs at handover, or at its place in the + * journal's queue while writes wait there, and one still waiting with the same key is replaced + * by the next. Journal writes never coalesce: + * replacing one would move it behind whatever was issued after it. */ + publicationKey?: string + /** Called at handover, so a journal write it issues takes its place in the chat's one write queue + * in the same tick it was submitted. */ run: (target: StructuredAgentSessionEventTarget) => Promise | void } +type Admitted = StructuredAgentSessionSinkOperation & { superseded?: boolean } + export type StructuredAgentSessionDrainWaiter = { through: number resolve: (result: StructuredAgentSessionSinkBarrier) => void } +/** + * Admission and backpressure for one provider stream's writes. It holds nothing once bound: each + * operation is handed to the journal as it is submitted, so a streamed row and a host write are + * ordered by when they were issued, in the journal's one queue. Only while no journal is bound do + * operations wait here, in arrival order, and bind hands them over before it returns. Pressure + * counts what was admitted and has not yet settled, wherever it waits. + */ export class StructuredAgentSessionSinkQueue { private readingControl: StructuredAgentSessionReadingControl | undefined private target: StructuredAgentSessionEventTarget | null = null private closed = false private failure: { error: unknown } | null = null - private running = false private queuedBytes = 0 private queuedOperations = 0 private lifecycleQueuedBytes = 0 @@ -39,7 +51,10 @@ export class StructuredAgentSessionSinkQueue { private backpressured = false private acceptedSequence = 0 private settledSequence = 0 - private readonly queue: StructuredAgentSessionSinkOperation[] = [] + /** Settles once every operation handed over so far has, in handover order. */ + private handedOverSettled: Promise = Promise.resolve() + private readonly buffered: Admitted[] = [] + private readonly waitingPublications = new Map() private readonly waiters: StructuredAgentSessionDrainWaiter[] = [] constructor( @@ -84,9 +99,12 @@ export class StructuredAgentSessionSinkQueue { } bind(target: StructuredAgentSessionEventTarget): void { - if (!this.closed) { - this.target = target - this.pump() + if (this.closed) { + return + } + this.target = target + for (const operation of this.buffered.splice(0)) { + this.handOver(operation, target) } } @@ -94,24 +112,17 @@ export class StructuredAgentSessionSinkQueue { this.target = null } + /** Operations already handed over are the journal's and still land; only buffered ones drop. */ close(): void { this.closed = true - this.queue.length = 0 - this.queuedBytes = 0 - this.queuedOperations = 0 - this.lifecycleQueuedBytes = 0 - this.lifecycleQueuedOperations = 0 - this.settledSequence = this.acceptedSequence + this.dropBuffered() this.updateBackpressure() - this.settleWaiters() } barrier = (): Promise => { const through = this.acceptedSequence if (this.settledSequence >= through) { - return Promise.resolve( - this.failure === null ? { ok: true } : { ok: false, error: this.failure.error } - ) + return Promise.resolve(this.barrierResult()) } return new Promise((resolve) => this.waiters.push({ through, resolve })) } @@ -126,21 +137,21 @@ export class StructuredAgentSessionSinkQueue { if (this.failure !== null) { return { accepted: false, reason: 'failed' } } - const key = options.coalescingKey ?? operation.coalescingKey - const replaceAt = key ? this.queue.findIndex((queued) => queued.coalescingKey === key) : -1 - const replaced = replaceAt >= 0 ? this.queue[replaceAt] : undefined - if (replaced && operation.keepsFirst) { - return { accepted: true } - } - const sequence = ++this.acceptedSequence + const key = operation.publicationKey + const replaceAt = + key === undefined ? -1 : this.buffered.findIndex((queued) => queued.publicationKey === key) + const replaced = + replaceAt >= 0 + ? this.buffered[replaceAt] + : key === undefined + ? undefined + : this.waitingPublications.get(key) const lifecycle = operation.lifecycle ?? options.lifecycle === true const lifecycleBytes = lifecycle ? (operation.lifecycleBytes ?? operation.bytes) : 0 const nextBytes = this.queuedBytes - (replaced?.bytes ?? 0) + operation.bytes const nextOperations = this.queuedOperations + (replaced ? 0 : 1) const nextLifecycleBytes = - this.lifecycleQueuedBytes - - (replaced?.lifecycle ? (replaced.lifecycleBytes ?? replaced.bytes) : 0) + - lifecycleBytes + this.lifecycleQueuedBytes - (replaced ? lifecycleCost(replaced) : 0) + lifecycleBytes const nextLifecycleOperations = this.lifecycleQueuedOperations - (replaced?.lifecycle ? 1 : 0) + (lifecycle ? 1 : 0) const exceedsOrdinary = @@ -152,30 +163,95 @@ export class StructuredAgentSessionSinkQueue { (nextLifecycleBytes > this.deps.watermarks.maxLifecycleQueuedBytes || nextLifecycleOperations > this.deps.watermarks.maxLifecycleQueuedOperations) if (exceedsOrdinary || exceedsLifecycle) { - this.acceptedSequence -= 1 this.setBackpressure(true) return { accepted: false, reason: 'backpressure' } } - const accepted = { + const accepted: Admitted = { ...operation, - sequence, + sequence: ++this.acceptedSequence, lifecycle, - lifecycleBytes, - ...(key ? { coalescingKey: key } : {}) + lifecycleBytes } - if (replaced) { - this.queue.splice(replaceAt, 1) + if (replaceAt >= 0) { + this.buffered.splice(replaceAt, 1) + } else if (replaced) { + // Its turn still comes, and settles as nothing: the publication after it covers it. + replaced.superseded = true } - this.queue.push(accepted) this.queuedBytes = nextBytes this.queuedOperations = nextOperations this.lifecycleQueuedBytes = nextLifecycleBytes this.lifecycleQueuedOperations = nextLifecycleOperations + if (this.target) { + this.handOver(accepted, this.target) + } else { + this.buffered.push(accepted) + } this.updateBackpressure() - this.pump() return { accepted: true } } + private handOver(operation: Admitted, bound: StructuredAgentSessionEventTarget): void { + const earlier = this.handedOverSettled + const key = operation.publicationKey + let outcome: Promise + if (key === undefined) { + outcome = runNow(() => operation.run(bound)) + } else { + this.waitingPublications.set(key, operation) + // At handover, unless writes still wait behind an owed import; then at its place in line, so + // it never announces ahead of the writes issued before it. + outcome = runNow(() => + bound.journal.readInOrder(() => { + if (this.waitingPublications.get(key) === operation) { + this.waitingPublications.delete(key) + } + // A publication writes no row, so a closed or failed sink has nothing left to announce. + return operation.superseded || this.closed || this.failure !== null + ? undefined + : operation.run(bound) + }) + ) + } + // Handled at once: a write can fail while an earlier one is still landing. + const landed = outcome.then(() => undefined, this.fail) + this.handedOverSettled = Promise.all([earlier, landed]).then(() => this.settle(operation)) + } + + private settle(operation: Admitted): void { + if (!operation.superseded) { + this.release(operation) + } + this.settledSequence = Math.max(this.settledSequence, operation.sequence) + this.updateBackpressure() + this.settleWaiters() + } + + private release(operation: Admitted): void { + this.queuedBytes = Math.max(0, this.queuedBytes - operation.bytes) + this.queuedOperations = Math.max(0, this.queuedOperations - 1) + if (operation.lifecycle) { + this.lifecycleQueuedBytes = Math.max(0, this.lifecycleQueuedBytes - lifecycleCost(operation)) + this.lifecycleQueuedOperations = Math.max(0, this.lifecycleQueuedOperations - 1) + } + } + + /** Buffered operations never reached a journal. They count as settled once everything handed + * over before them has, so a barrier never resolves ahead of a write still landing. */ + private dropBuffered(): void { + const dropped = this.buffered.splice(0) + for (const operation of dropped) { + this.release(operation) + } + const last = dropped.at(-1) + if (last) { + this.handedOverSettled = this.handedOverSettled.then(() => { + this.settledSequence = Math.max(this.settledSequence, last.sequence) + this.settleWaiters() + }) + } + } + private setBackpressure(next: boolean): void { if (next === this.backpressured) { return @@ -201,60 +277,40 @@ export class StructuredAgentSessionSinkQueue { this.setBackpressure(next) } + private barrierResult(): StructuredAgentSessionSinkBarrier { + return this.failure === null ? { ok: true } : { ok: false, error: this.failure.error } + } + private settleWaiters(): void { for (let index = this.waiters.length - 1; index >= 0; index -= 1) { const waiter = this.waiters[index] if (waiter && waiter.through <= this.settledSequence) { this.waiters.splice(index, 1) - waiter.resolve( - this.failure === null ? { ok: true } : { ok: false, error: this.failure.error } - ) + waiter.resolve(this.barrierResult()) } } } + /** Writes handed over before this runs still land; nothing submitted after it is admitted. */ private fail = (error: unknown): void => { if (this.failure === null) { this.failure = { error } this.deps.onFailed?.(error) } - this.queue.length = 0 - this.queuedBytes = 0 - this.queuedOperations = 0 - this.lifecycleQueuedBytes = 0 - this.lifecycleQueuedOperations = 0 - this.settledSequence = this.acceptedSequence + this.dropBuffered() this.updateBackpressure() - this.settleWaiters() } +} - private pump(): void { - if (this.running || !this.target || this.closed || this.failure !== null) { - return - } - const operation = this.queue.shift() - if (!operation) { - return - } - this.running = true - const bound = this.target - void Promise.resolve(operation.run(bound)) - .catch(this.fail) - .finally(() => { - this.running = false - this.queuedBytes = Math.max(0, this.queuedBytes - operation.bytes) - this.queuedOperations = Math.max(0, this.queuedOperations - 1) - if (operation.lifecycle) { - this.lifecycleQueuedBytes = Math.max( - 0, - this.lifecycleQueuedBytes - (operation.lifecycleBytes ?? operation.bytes) - ) - this.lifecycleQueuedOperations = Math.max(0, this.lifecycleQueuedOperations - 1) - } - this.settledSequence = Math.max(this.settledSequence, operation.sequence) - this.updateBackpressure() - this.settleWaiters() - this.pump() - }) +function lifecycleCost(operation: StructuredAgentSessionSinkOperation): number { + return operation.lifecycle ? (operation.lifecycleBytes ?? operation.bytes) : 0 +} + +/** A synchronous throw settles as a rejection, like any other failed write. */ +function runNow(run: () => Promise | void): Promise { + try { + return Promise.resolve(run()) + } catch (error) { + return Promise.reject(error) } } 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 2993ce35a4e3..b19c3db0c4f3 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 @@ -1,5 +1,9 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { AGENT_JOURNAL_THREAD_SCOPE } from '../../../shared/agent-session-journal-types' import { describe, expect, it, vi } from 'vitest' +import { agentJournalItemKey } from '../../../shared/agent-session-journal-item-key' import type { AgentJournalItemBody, AgentJournalItemIdentity @@ -15,6 +19,10 @@ import { type StructuredAgentSessionEventTarget } from './structured-agent-session-event-sink' import { StructuredAgentSessionHostRuntimeState } from './structured-agent-session-host-runtime-state' +import { openAgentSessionJournal } from '../agent-session-journal/journal-store-factory' +import { openTestJournalHostDatabase } from '../agent-session-journal/journal-host-database-test-support' +import { openJournalOwingImport } from '../agent-session-journal/journal-owed-import-test-support' +import { createCodexJournalTranslator } from '../../codex/codex-structured-journal-translation' import { createStructuredAgentSessionLogger } from './structured-agent-session-logger' import { testEventSinkLogging } from './structured-agent-session-logger-test-support' @@ -24,6 +32,14 @@ const BODY: AgentJournalItemBody = { blocks: [{ type: 'text', text: 'hi' }] } +const JOURNAL_IDENTITY = { + sessionId: 'session-1', + workspaceId: 'workspace-1', + hostId: 'host-1', + agent: 'codex', + providerHandle: { kind: 'codex', threadId: 'thread-1' } +} as const + function identity(ordinal: number): AgentJournalItemIdentity { return { provider: 'codex', threadId: 'thread-1', turnId: 'turn-1', ordinal } } @@ -80,7 +96,20 @@ function target( log.push({ call: 'appendLifecycleBatch', fence, settlementId: input.settlementId }) return { epoch: 'e', sequence: 0 } }), - latestItemMatching: vi.fn(() => null) + latestItemMatching: vi.fn(() => null), + readInOrder: vi.fn(async (read: () => T) => read()), + // The journal resolves at the write's place in its queue; this double has no queue to wait on. + appendResolvedItem: vi.fn( + async ( + resolve: () => { identity: AgentJournalItemIdentity; body: AgentJournalItemBody } | null, + options: JournalItemAppendOptions + ) => { + const resolved = resolve() + return resolved === null + ? null + : journal.appendItem(resolved.identity, resolved.body, options) + } + ) } as unknown as AgentSessionJournal return { journal, @@ -90,6 +119,41 @@ function target( } } +/** A real journal whose copy from an earlier build's per-chat file is still owed: its first write + * pays it, and every write issued meanwhile waits in its queue. The one real backlog a chat has. */ +async function owingTarget(fence: number) { + const root = await mkdtemp(join(tmpdir(), 'orca-event-sink-owed-')) + const { journal, history } = await openJournalOwingImport({ + stateDirectory: root, + identity: JOURNAL_IDENTITY + }) + const published: (AgentSessionTurnActivity | null | undefined)[] = [] + const bound: StructuredAgentSessionEventTarget = { + journal, + fence, + publish: (activity) => published.push(activity) + } + /** The row each of these ordinals created, in journal order. */ + const landed = (): number[] => + journal.snapshot().items.flatMap((item) => { + const ordinal = [0, 1, 2, 3, 4].find( + (index) => item.itemId === agentJournalItemKey(identity(index)) + ) + return ordinal === undefined ? [] : [ordinal] + }) + return { + bound, + journal, + history: history.length, + published, + landed, + dispose: async () => { + await journal.close() + await rm(root, { recursive: true, force: true }) + } + } +} + describe('deferred structured agent-session event sink', () => { it('buffers writes made before the journal exists and drains them in arrival order', async () => { const log: Recorded[] = [] @@ -179,6 +243,27 @@ describe('deferred structured agent-session event sink', () => { expect(log).toEqual([]) }) + it('lands writes already handed to the journal when closed; only never-bound ones drop', async () => { + const owed = await owingTarget(3) + const deferred = createDeferredStructuredAgentSessionEventSink(testEventSinkLogging()) + deferred.bind(owed.bound) + + deferred.sink.appendItem(identity(0), BODY, { turnScope: AGENT_JOURNAL_THREAD_SCOPE }) + deferred.sink.appendItem(identity(1), BODY, { turnScope: AGENT_JOURNAL_THREAD_SCOPE }) + deferred.sink.appendItem(identity(2), BODY, { turnScope: AGENT_JOURNAL_THREAD_SCOPE }) + // Handed over, waiting behind the copy: none has landed when the sink closes. + expect(owed.landed()).toEqual([]) + deferred.close() + expect( + deferred.sink.tryAppendItem?.(identity(3), BODY, { turnScope: AGENT_JOURNAL_THREAD_SCOPE }) + ).toEqual({ accepted: false, reason: 'closed' }) + await expect(deferred.drained()).resolves.toEqual({ ok: true }) + + expect(owed.landed()).toEqual([0, 1, 2]) + expect(owed.journal.cursor().sequence).toBe(owed.history + 3) + await owed.dispose() + }) + it('reports one refused append, fails the barrier, and stops later writes', async () => { const log: Recorded[] = [] const errors: unknown[] = [] @@ -191,8 +276,11 @@ describe('deferred structured agent-session event sink', () => { deferred.bind(target(4, log, 0)) deferred.sink.appendItem(identity(0), BODY, { turnScope: AGENT_JOURNAL_THREAD_SCOPE }) - deferred.sink.appendTombstone(identity(1)) const barrier = await deferred.drained() + expect(deferred.sink.tryAppendTombstone?.(identity(1))).toEqual({ + accepted: false, + reason: 'failed' + }) expect(errors).toHaveLength(1) expect((errors[0] as Error).message).toBe('refused 0') @@ -267,6 +355,47 @@ describe('deferred structured agent-session event sink', () => { expect(log).toHaveLength(2) }) + it('pauses and resumes the reader at the same watermarks over writes the journal has not landed', async () => { + const changes: boolean[] = [] + const readingControl = { pauseReading: vi.fn(), resumeReading: vi.fn() } + const deferred = createDeferredStructuredAgentSessionEventSink({ + ...testEventSinkLogging(), + watermarks: { + pauseQueuedBytes: 1_000_000, + maxQueuedBytes: 1_000_000, + lowQueuedBytes: 1_000_000, + pauseQueuedOperations: 3, + maxQueuedOperations: 4, + lowQueuedOperations: 1 + }, + readingControl, + onBackpressureChange: (paused) => changes.push(paused) + }) + const owed = await owingTarget(5) + deferred.bind(owed.bound) + const append = (ordinal: number) => + deferred.sink.tryAppendItem?.(identity(ordinal), BODY, { + turnScope: AGENT_JOURNAL_THREAD_SCOPE + }) + + expect([append(0), append(1)]).toEqual([{ accepted: true }, { accepted: true }]) + expect(readingControl.pauseReading).not.toHaveBeenCalled() + expect(append(2)).toEqual({ accepted: true }) + expect(deferred.state()).toMatchObject({ backpressured: true, queuedOperations: 3 }) + expect(readingControl.pauseReading).toHaveBeenCalledOnce() + expect(append(3)).toEqual({ accepted: true }) + expect(append(4)).toEqual({ accepted: false, reason: 'backpressure' }) + + // Nothing has landed yet: every admitted write waits behind the copy it owes. + expect(owed.landed()).toEqual([]) + await expect(deferred.drained()).resolves.toEqual({ ok: true }) + expect(changes).toEqual([true, false]) + expect(readingControl.resumeReading).toHaveBeenCalledOnce() + expect(deferred.state()).toMatchObject({ backpressured: false, queuedOperations: 0 }) + expect(owed.landed()).toEqual([0, 1, 2, 3]) + await owed.dispose() + }) + it('admits a resolved append and publication as one bounded operation', async () => { const log: Recorded[] = [] const deferred = createDeferredStructuredAgentSessionEventSink({ @@ -405,24 +534,14 @@ describe('deferred structured agent-session event sink', () => { releaseSecond?.() }) - it('replaces a queued same-item checkpoint before it runs', async () => { - const log: Recorded[] = [] - const deferred = createDeferredStructuredAgentSessionEventSink(testEventSinkLogging()) - const options = { coalescingKey: 'checkpoint:item-1', turnScope: AGENT_JOURNAL_THREAD_SCOPE } - - deferred.sink.appendItem(identity(0), BODY, options) - deferred.sink.appendItem(identity(1), BODY, options) - expect(deferred.state().queuedOperations).toBe(1) - - deferred.bind(target(6, log)) - await deferred.drained() - expect(log).toEqual([{ call: 'appendItem', fence: 6, ordinal: 1 }]) - }) - - it('keeps the first queued lifecycle batch for a settlement, as the journal does', async () => { - // The journal applies a settlement id once and skips any later batch with it, - // so the queue must not let a later batch replace one it has not run yet. - const log: Recorded[] = [] + it('applies the first lifecycle batch for a settlement once, in the journal', async () => { + // The journal applies a settlement id once; the sink hands both over in order. + const root = await mkdtemp(join(tmpdir(), 'orca-event-sink-settlement-')) + const journal = await openAgentSessionJournal({ + identity: JOURNAL_IDENTITY, + database: openTestJournalHostDatabase(root), + mintEpoch: () => 'epoch-1' + }) const deferred = createDeferredStructuredAgentSessionEventSink(testEventSinkLogging()) const batch = (ordinal: number) => [ { @@ -437,34 +556,64 @@ describe('deferred structured agent-session event sink', () => { expect(deferred.sink.tryAppendLifecycleBatch?.('turn-completed:turn-1', batch(1))).toEqual({ accepted: true }) - expect(deferred.state().queuedOperations).toBe(1) - - const bound = target(6, log) - deferred.bind(bound) - await deferred.drained() + deferred.bind({ journal, fence: 1, publish: () => {} }) + await expect(deferred.drained()).resolves.toEqual({ ok: true }) - expect( - vi - .mocked(bound.journal.appendLifecycleBatch) - .mock.calls.map(([input]) => input.mutations.map((mutation) => mutation.identity)) - ).toEqual([[identity(0)]]) + expect(journal.snapshot().items.map((item) => item.itemId)).toEqual([ + agentJournalItemKey(identity(0)) + ]) + await journal.close() + await rm(root, { recursive: true, force: true }) }) - it('keeps a replacement checkpoint after distinct intervening operations', async () => { + it('keeps a rewritten streamed row in its place ahead of what was issued after it', async () => { + // A Codex reply checkpointed, then another row, then the reply's next checkpoint. const log: Recorded[] = [] const deferred = createDeferredStructuredAgentSessionEventSink(testEventSinkLogging()) - const options = { coalescingKey: 'checkpoint:item-1', turnScope: AGENT_JOURNAL_THREAD_SCOPE } + const window: (() => void)[] = [] + const translator = createCodexJournalTranslator({ + sink: deferred.sink, + sessionId: 'session-1', + primaryThreadId: () => 'thread-1', + schedule: (run) => { + window.push(run) + return () => {} + } + }) + const notify = (method: string, params: Record) => + translator.handle({ + type: 'notification', + sessionId: 'session-1', + threadId: 'thread-1', + method, + params: { threadId: 'thread-1', turnId: 'turn-1', turn: { id: 'turn-1' }, ...params } + }) + const checkpoint = (text: string) => { + notify('item/agentMessage/delta', { itemId: 'reply', delta: text }) + for (const run of window.splice(0)) { + run() + } + } + notify('turn/started', {}) + notify('item/started', { item: { type: 'agentMessage', id: 'reply', text: '' } }) + checkpoint('Looking at the tests first. '.repeat(4)) + notify('item/completed', { item: { type: 'agentMessage', id: 'aside', text: 'An aside.' } }) + checkpoint('Then the build. '.repeat(40)) - deferred.sink.appendItem(identity(0), BODY, options) - deferred.sink.appendItem(identity(1), BODY, { turnScope: AGENT_JOURNAL_THREAD_SCOPE }) - deferred.sink.appendItem(identity(2), BODY, options) - deferred.bind(target(6, log)) + const bound = target(6, log) + deferred.bind(bound) await deferred.drained() - - expect(log).toEqual([ - { call: 'appendItem', fence: 6, ordinal: 1 }, - { call: 'appendItem', fence: 6, ordinal: 2 } - ]) + const written = vi.mocked(bound.journal.appendItem).mock.calls.flatMap(([id, body]) => { + const block = body.kind === 'message' ? body.blocks[0] : undefined + const text = block?.type === 'text' ? block.text : '' + return text.startsWith('Looking') || text === 'An aside.' + ? [`${text.startsWith('Looking') ? 'reply' : 'aside'} ${agentJournalItemKey(id)}`] + : [] + }) + const [reply, aside] = written + // The reply's row is created before the aside and rewritten after it; never moved behind it. + expect(reply?.startsWith('reply')).toBe(true) + expect(written).toEqual([reply, aside, reply]) }) it('coalesces provider activity as a publication without a journal write', async () => { @@ -484,6 +633,28 @@ describe('deferred structured agent-session event sink', () => { } ]) }) + + it('skips a publication handed over and then replaced, and runs its replacement after the write between them', async () => { + const owed = await owingTarget(6) + const deferred = createDeferredStructuredAgentSessionEventSink(testEventSinkLogging()) + deferred.bind(owed.bound) + const seen: string[] = [] + owed.bound.publish = (activity) => + seen.push(`${activity?.text ?? 'none'} over ${owed.landed().join(',')}`) + + deferred.sink.appendItem(identity(0), BODY, { turnScope: AGENT_JOURNAL_THREAD_SCOPE }) + deferred.sink.setActivity?.({ turnId: 'turn-1', text: 'Thinking' }) + deferred.sink.appendItem(identity(1), BODY, { turnScope: AGENT_JOURNAL_THREAD_SCOPE }) + deferred.sink.setActivity?.({ turnId: 'turn-1', text: 'Checking the result' }) + // The first publication was handed over and waits behind the copy; the second replaced it. + expect(deferred.state().queuedOperations).toBe(3) + expect(seen).toEqual([]) + + await expect(deferred.drained()).resolves.toEqual({ ok: true }) + expect(seen).toEqual(['Checking the result over 0,1']) + expect(deferred.state().queuedOperations).toBe(0) + await owed.dispose() + }) }) describe('producer linkage reaches the journal through every append path', () => { 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 47f9e03d0cc4..be0da7d9b299 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 @@ -30,14 +30,20 @@ export type StructuredAgentSessionSinkBarrier = { ok: true } | { ok: false; erro /** 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. */ lifecycle?: boolean /** Host clock to stamp on the row instead of its append time. */ observedAt?: number } +export type StructuredAgentSessionPublishOptions = Pick< + StructuredAgentSessionAppendOptions, + 'lifecycle' +> & { + /** A publication still waiting with this key is replaced by this one. */ + coalescingKey?: string +} + /** An item write states which turn its row belongs to; the write that creates the row decides. */ export type StructuredAgentSessionItemAppendOptions = StructuredAgentSessionAppendOptions & { turnScope: AgentJournalTurnScope @@ -69,11 +75,7 @@ export type StructuredAgentSessionRevisionResolver = ( journal: StructuredAgentSessionRevisionJournal ) => { identity: AgentJournalItemIdentity; body: AgentJournalItemBody } | null -/** A revision's body is derived from the row it revises, so coalescing one away would lose it. */ -export type StructuredAgentSessionRevisionOptions = Omit< - StructuredAgentSessionItemAppendOptions, - 'coalescingKey' -> +export type StructuredAgentSessionRevisionOptions = StructuredAgentSessionItemAppendOptions /** Compatibility alias for lifecycle callers that already use this resolver. */ export type StructuredAgentSessionLifecycleIdentityResolver = StructuredAgentSessionIdentityResolver @@ -92,7 +94,7 @@ export type StructuredAgentSessionEventSink = { identity: AgentJournalItemIdentity, options?: StructuredAgentSessionAppendOptions ): StructuredAgentSessionSinkAdmission - publish(options?: StructuredAgentSessionAppendOptions): void + publish(options?: StructuredAgentSessionPublishOptions): void setActivity?(activity: AgentSessionTurnActivity | null): void tryAppendItem?( identity: AgentJournalItemIdentity, @@ -150,7 +152,7 @@ export type StructuredAgentSessionEventSink = { mutations: readonly JournalLifecycleMutationInput[], options?: StructuredAgentSessionAppendOptions ): StructuredAgentSessionSinkAdmission - tryPublish?(options?: StructuredAgentSessionAppendOptions): StructuredAgentSessionSinkAdmission + tryPublish?(options?: StructuredAgentSessionPublishOptions): StructuredAgentSessionSinkAdmission /** Couples durable-queue pressure to the exact provider stream producing it. */ bindReadingControl?(control: StructuredAgentSessionReadingControl): () => void } @@ -233,8 +235,6 @@ export function createDeferredStructuredAgentSessionEventSink(deps: { queue.submit( { bytes: Buffer.byteLength(JSON.stringify({ settlementId, mutations }), 'utf8') + 512, - coalescingKey: `lifecycle:${settlementId}`, - keepsFirst: true, run: (bound) => bound.journal.appendLifecycleBatch({ settlementId, @@ -247,48 +247,41 @@ export function createDeferredStructuredAgentSessionEventSink(deps: { ) const publish = ( - options: StructuredAgentSessionAppendOptions = {} + options: StructuredAgentSessionPublishOptions = {} ): StructuredAgentSessionSinkAdmission => queue.submit( { bytes: 1, - coalescingKey: options.coalescingKey ?? 'publish', + publicationKey: options.coalescingKey ?? 'publish', run: (bound) => bound.publish() }, options ) + const appendItem: NonNullable = ( + identity, + body, + options + ) => + queue.submit( + { + bytes: estimateStructuredAgentSessionItemBytes(identity, body), + run: (bound) => + bound.journal.appendItem( + identity, + body, + structuredAgentSessionJournalAppendOptions(bound.fence, options) + ) + }, + options + ) + return { sink: { appendItem: (identity, body, options) => { - queue.submit( - { - bytes: estimateStructuredAgentSessionItemBytes(identity, body), - coalescingKey: options.coalescingKey, - run: (bound) => - bound.journal.appendItem( - identity, - body, - structuredAgentSessionJournalAppendOptions(bound.fence, options) - ) - }, - options - ) + appendItem(identity, body, options) }, - tryAppendItem: (identity, body, options) => - queue.submit( - { - bytes: estimateStructuredAgentSessionItemBytes(identity, body), - coalescingKey: options.coalescingKey, - run: (bound) => - bound.journal.appendItem( - identity, - body, - structuredAgentSessionJournalAppendOptions(bound.fence, options) - ) - }, - options - ), + tryAppendItem: appendItem, ...resolvedAppend, journalEpoch: queue.journalEpoch, journalLinkage: queue.journalLinkage, @@ -331,7 +324,7 @@ export function createDeferredStructuredAgentSessionEventSink(deps: { setActivity: (activity) => { queue.submit({ bytes: Buffer.byteLength(JSON.stringify(activity), 'utf8') + 64, - coalescingKey: 'turn-activity', + publicationKey: 'turn-activity', run: (bound) => bound.publish(activity) }) }, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts index d12a6509c747..6cd06bbd61e8 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts @@ -10,6 +10,7 @@ import { AgentSessionPreSpawnError } from './structured-agent-session-adapter' import { StructuredAgentSessionHostRuntimeState } from './structured-agent-session-host-runtime-state' +import { withJournalQueueMembers } from './structured-agent-session-journal-double-test-support' import { createStructuredAgentSessionLogger } from './structured-agent-session-logger' import { recordingStructuredAgentSessionLogger } from './structured-agent-session-logger-test-support' @@ -147,7 +148,12 @@ describe('rows the provider emits while closing', () => { const sessionId = 'session-closing-rows' const sink = state.eventSinkFor(sessionId) const published: string[] = [] - sink.bind({ journal: {} as never, fence: 1, publish: () => published.push('final-flush') }) + sink.bind({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: these rows are publications only, which reach nothing on the journal but its in-order read. + journal: withJournalQueueMembers({ appendItem: async () => ({}) }) as never, + fence: 1, + publish: () => published.push('final-flush') + }) await evictStructuredAgentSession({ sessionId, @@ -243,7 +249,12 @@ describe('eviction against the real sink cache', () => { const published: string[] = [] const reattached = state.eventSinkFor(sessionId) - reattached.bind({ journal: {} as never, fence: 2, publish: () => published.push('published') }) + reattached.bind({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: these rows are publications only, which reach nothing on the journal but its in-order read. + journal: withJournalQueueMembers({ appendItem: async () => ({}) }) as never, + fence: 2, + publish: () => published.push('published') + }) reattached.sink.publish() await reattached.drained() diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts index f2f885e07638..696236805ac8 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts @@ -56,7 +56,6 @@ export function createStructuredAgentSessionConversationDelivery(input: { ) => Promise reset: (sessionId: string, journal: AgentSessionJournal, reset: AgentJournalResetReason) => void clientDelivery: Pick - flushStreamedEvents: (sessionId: string) => Promise }): StructuredAgentSessionConversationDelivery { const { deps, sessions } = input const loop = new StructuredAgentSessionDeliveryLoop({ @@ -81,7 +80,6 @@ export function createStructuredAgentSessionConversationDelivery(input: { logger: deps.logger, record: (sessionId) => deps.store.getRecord(sessionId), readChildWork: input.clientDelivery.readChildWork, - flushStreamedEvents: input.flushStreamedEvents, now: () => deps.now?.() ?? Date.now() }) const adoptOpened = async ( diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts index 1df3bc349097..fd0fd17b71e0 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts @@ -92,9 +92,11 @@ export class StructuredAgentSessionHostRuntimeState { this.eventSinks.delete(sessionId) } + /** Through `currentEventSink`: a failed sink is terminal, so its old error never fails a later + * caller's barrier. */ flushEventSink(sessionId: string): Promise { return this.requireSuccessfulBarrier( - this.eventSinks.get(sessionId)?.drained() ?? Promise.resolve({ ok: true } as const) + this.currentEventSink(sessionId)?.drained() ?? Promise.resolve({ ok: true } as const) ) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host.test.ts index d666e4a5c94f..f66d2506804d 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host.test.ts @@ -303,27 +303,6 @@ describe('cancel', () => { expect(cancelTurn).not.toHaveBeenCalled() }) - it('records an unknown outcome when lifecycle draining fails and never interrupts on replay', async () => { - await attach() - const prompt = await seedApproval() - vi.spyOn(host, 'flushStreamedEvents').mockRejectedValueOnce(new Error('journal drain failed')) - const fields = { - turnId: 'turn-1', - prompt: { itemId: prompt.itemId, expectedRevision: prompt.revision } - } - const params = { - envelope: envelope('agentSession.cancel', fields), - ...fields - } - - await expect(host.cancel(CALLER, params)).rejects.toThrow('journal drain failed') - expect(await host.cancel(CALLER, params)).toMatchObject({ - ok: false, - refusal: { code: 'agent_session_operation_unknown' } - }) - expect(cancelTurn).toHaveBeenCalledTimes(1) - }) - it('records an unknown outcome when strict prompt interruption throws and never retries it', async () => { await attach() const prompt = await seedApproval() diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts index 44a576e2f781..5b5a4d19c3c1 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts @@ -133,8 +133,7 @@ export class StructuredAgentSessionHost { reset, structuredAgentSessionConversationFence(deps.store, sessionId) ), - clientDelivery: this.clientDelivery, - flushStreamedEvents: (sessionId) => this.flushStreamedEvents(sessionId) + clientDelivery: this.clientDelivery }) this.restore = createStructuredAgentSessionHostRestore(deps, { reconcileLeases: this.reconcileLeases, @@ -250,6 +249,7 @@ export class StructuredAgentSessionHost { return attachStructuredAgentSession(this.attachContext(), caller.callerKey, params) } + /** Test barrier: every write has landed by its call's return, so no production path needs it. */ flushStreamedEvents = (sessionId: string): Promise => this.runtimeState.flushEventSink(sessionId) @@ -272,7 +272,6 @@ export class StructuredAgentSessionHost { deps: this.deps, sessions: this.sessions, publish: (sessionId, journal) => this.subscribers.publish(sessionId, journal), - flushStreamedEvents: this.flushStreamedEvents, conversation: this.lifetime.conversation, readChildWork: this.clientDelivery.readChildWork, serialize: (sessionId, task) => this.serialize(sessionId, task), diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-journal-double-test-support.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-journal-double-test-support.ts new file mode 100644 index 000000000000..dc27fc5d320a --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-journal-double-test-support.ts @@ -0,0 +1,40 @@ +// The journal members the event sink reaches besides a plain append, for tests whose journal double +// has no write queue: a read and a resolved append each run as they are issued, as on an idle queue. + +import type { + AgentJournalItemBody, + AgentJournalItemIdentity +} from '../../../shared/agent-session-journal-types' +import type { JournalItemAppendOptions } from '../agent-session-journal/journal-store-contracts' + +type AppendingDouble = { + appendItem: ( + identity: AgentJournalItemIdentity, + body: AgentJournalItemBody, + options: JournalItemAppendOptions + ) => Promise +} + +export function withJournalQueueMembers( + journal: T +): T & { + readInOrder: (read: () => R) => Promise + appendResolvedItem: ( + resolve: () => { identity: AgentJournalItemIdentity; body: AgentJournalItemBody } | null, + options: JournalItemAppendOptions + ) => Promise +} { + return Object.assign(journal, { + readInOrder: async (read: () => R): Promise => read(), + appendResolvedItem: async ( + resolve: () => { identity: AgentJournalItemIdentity; body: AgentJournalItemBody } | null, + options: JournalItemAppendOptions + ): Promise => { + const resolved = resolve() + // Read at call time, so a test that swaps the double's append later is still the one called. + return resolved === null + ? null + : journal.appendItem(resolved.identity, resolved.body, options) + } + }) +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-admission.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-admission.ts index 6199531c8ee5..5bb072f25946 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-admission.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-admission.ts @@ -75,7 +75,6 @@ export type AgentSessionMutationRequest = { record: AgentSessionRecord ) => Promise publish: (journal: AgentSessionJournal) => void - flushStreamedEvents: (sessionId: string) => Promise providerChildPhase?: AgentSessionTurnContext['providerChildPhase'] now: () => number } @@ -247,7 +246,6 @@ function turnContext( .then(() => undefined), resolvedBy: request.callerKey, publish: () => request.publish(journal), - flushStreamedEvents: () => request.flushStreamedEvents(request.envelope.sessionId), ...(request.providerChildPhase ? { providerChildPhase: request.providerChildPhase } : {}), now: () => request.now() } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts index 6a66836fc8a6..e77975dc5c22 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts @@ -23,7 +23,6 @@ export type StructuredAgentSessionMutationContext = { deps: StructuredAgentSessionHostDeps sessions: Map publish: (sessionId: string, journal: StructuredAgentSessionHostSession['journal']) => void - flushStreamedEvents: (sessionId: string) => Promise /** The host's accessor, for a caller outside the session's serialize. */ conversation: (sessionId: string) => Promise /** The session's child records, as the strip reads them; what command admission decides on. */ @@ -67,7 +66,6 @@ export function mutateStructuredAgentSession( journal: () => context.sessions.get(envelope.sessionId)?.journal, prepareSession, publish: (journal) => context.publish(envelope.sessionId, journal), - flushStreamedEvents: context.flushStreamedEvents, providerChildPhase: () => context.sessions.get(envelope.sessionId)?.child?.phase, now: () => context.now() }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-owed-import.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-owed-import.test.ts new file mode 100644 index 000000000000..7e9ffcd2ac2b --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-owed-import.test.ts @@ -0,0 +1,148 @@ +// A mutation reads the fold after every write issued before it, even while a restore's import is +// owed: the open pays the import first, so provider rows queued behind it have landed. A failed +// import is reported and never refuses the mutation. + +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import { AGENT_JOURNAL_THREAD_SCOPE } from '../../../shared/agent-session-journal-types' +import type { AgentSessionJournal } from '../agent-session-journal/journal-store' +import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' +import type { StructuredAgentSessionEventSink } from './structured-agent-session-event-sink' +import type { StructuredAgentSessionHost } from './structured-agent-session-host' +import { + attach, + CALLER, + envelope, + hostTestState +} from './structured-agent-session-host-test-harness' +import { + HOST_TEST_SESSION as SESSION, + HOST_TEST_THREAD as THREAD +} from './structured-agent-session-host-test-data' + +let host: StructuredAgentSessionHost +let acquire: Mock +let cancelTurn: Mock +let warned: ReturnType + +beforeEach(() => { + ;({ host, acquire, cancelTurn } = hostTestState()) + warned = vi.spyOn(console, 'warn').mockImplementation(() => undefined) +}) + +afterEach(() => { + vi.restoreAllMocks() +}) + +function journal(): AgentSessionJournal { + const open = host.collaboratorsForTests().sessions.get(SESSION)?.journal + if (!open) { + throw new Error('no open journal') + } + return open +} + +function providerEvents(): StructuredAgentSessionEventSink { + const events = acquire.mock.calls.at(-1)?.[0].events + if (!events) { + throw new Error('no provider bound') + } + return events +} + +/** Owed work standing in for a restore's import: every write waits behind it until it settles. */ +function oweImport(): PromiseWithResolvers { + const owed = Promise.withResolvers() + journal()['queue'].owe(() => owed.promise) + return owed +} + +/** A turn the provider opened by itself, its row queued behind the owed import. */ +function providerOpensTurn(turnId: string, ordinal: number): void { + providerEvents().appendItem( + { provider: 'codex', threadId: THREAD, turnId, ordinal }, + { kind: 'turn', turnId, state: 'running' }, + { turnScope: AGENT_JOURNAL_THREAD_SCOPE } + ) +} + +function stopNamingNoTurn() { + return host.cancel(CALLER, { envelope: envelope('agentSession.cancel', {}) }) +} + +describe('a mutation while an import is owed', () => { + it('a Stop naming no turn sees the turn the provider opened, and interrupts it', async () => { + await attach() + const owed = oweImport() + providerOpensTurn('turn-p', 900) + expect(journal().activeTurnId()).toBeNull() + + const stopping = stopNamingNoTurn() + await new Promise((resolve) => setTimeout(resolve, 50)) + // It waits for the import, as a reader does, then reads the turn. + expect(cancelTurn).not.toHaveBeenCalled() + owed.resolve() + + expect(await stopping).toMatchObject({ ok: true, value: { cancelled: true } }) + expect(cancelTurn).toHaveBeenCalledOnce() + }) + + it('a goal set sees the goal and the turn the provider wrote, and replaces it in that turn', async () => { + await attach() + const changeThreadGoal = vi.fn(async () => ({ ok: true as const })) + Object.assign(host.deps.adapter, { changeThreadGoal, supportsThreadGoal: () => true }) + const owed = oweImport() + providerOpensTurn('turn-g', 901) + providerEvents().appendItem( + { provider: 'codex', threadId: THREAD, turnId: 'turn-g', ordinal: 902 }, + { + kind: 'status', + text: 'Goal', + threadGoal: { + state: 'set', + goal: { + objective: 'Earlier goal', + status: 'active', + tokenBudget: null, + tokensUsed: 1, + timeUsedSeconds: 2, + createdAt: 3_000, + updatedAt: 4_000 + } + } + }, + { turnScope: AGENT_JOURNAL_THREAD_SCOPE } + ) + + const change = { kind: 'set' as const, objective: 'New goal' } + const setting = host.changeThreadGoal(CALLER, { + envelope: envelope('agentSession.threadGoal', { change }), + change + }) + // Long enough for the goal set to reach its read before the import lands. + await new Promise((resolve) => setTimeout(resolve, 50)) + expect(changeThreadGoal).not.toHaveBeenCalled() + owed.resolve() + + expect(await setting).toMatchObject({ ok: true }) + expect(changeThreadGoal).toHaveBeenCalledWith(expect.objectContaining({ replacesGoal: true })) + const objective = journal() + .snapshot() + .items.find((item) => item.body.kind === 'message' && item.body.sentAs === 'goal') + expect(objective?.turnScope).toEqual({ + kind: 'turn', + turnItemId: expect.stringContaining(':turn-g:') + }) + }) + + it('a failed import is reported and the Stop still answers', async () => { + await attach() + const owed = oweImport() + owed.reject(new Error('disk I/O error')) + + expect(await stopNamingNoTurn()).toMatchObject({ ok: true }) + expect(warned).toHaveBeenCalledWith( + '[agent-session] open-for-write: the import owed before a write failed', + expect.objectContaining({ error: new Error('disk I/O error') }) + ) + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-operation-settlement.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-operation-settlement.test.ts index 50c887c85df7..203e31f4ea05 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-operation-settlement.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-operation-settlement.test.ts @@ -38,7 +38,6 @@ async function context(): Promise { persistOptions: async () => {}, resolvedBy: 'test', publish: () => {}, - flushStreamedEvents: async () => {}, now: () => 0 } } @@ -133,15 +132,12 @@ it.each([1, 2])( } ) -// A send's plan only accepts: it records the submission and never waits on the provider's stream -// barrier, so a sink that stalls or fails cannot delay or double a send. Handing it over is the -// delivery loop's. -it('accepts without touching the event-stream barrier or the provider', async () => { +// A send's plan only accepts: it records the submission and never reaches the provider. Handing +// it over is the delivery loop's. +it('accepts without touching the provider', async () => { const ctx = await context() const { store } = hostTestState() vi.spyOn(store, 'recordOperationOutcome').mockResolvedValue() - const barrier = vi.fn(() => new Promise(() => {})) - ctx.flushStreamedEvents = barrier const beforeRun = vi.fn() const body = hostTestMessage('Continue the interrupted work') const operation = envelope('agentSession.send', { body }) @@ -159,7 +155,6 @@ it('accepts without touching the event-stream barrier or the provider', async () dispatchState: 'pending', handoverRecorded: true }) - expect(barrier).not.toHaveBeenCalled() }) it('refuses a superseded send at acceptance, recording and dispatching nothing', async () => { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.test.ts index 8d67ba0aa7c0..3fed3c962919 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.test.ts @@ -78,8 +78,7 @@ async function pendingPrompt( function context( journal: AgentSessionJournal, - cancelTurn: StructuredAgentSessionAdapter['cancelTurn'], - flushStreamedEvents: () => Promise + cancelTurn: StructuredAgentSessionAdapter['cancelTurn'] ): AgentSessionTurnContext { return { logger: createStructuredAgentSessionLogger(), @@ -90,7 +89,6 @@ function context( persistOptions: async () => undefined, resolvedBy: 'client-1', publish: vi.fn(), - flushStreamedEvents, now: () => 1 } } @@ -99,9 +97,8 @@ describe('performCancel for a pending prompt', () => { it('refuses a stale prompt revision before reaching the provider', async () => { const { journal, itemId } = await pendingPrompt() const cancelTurn = vi.fn(async () => ({ cancelled: true })) - const flush = vi.fn(async () => undefined) - const result = await performCancel(context(journal, cancelTurn, flush), { + const result = await performCancel(context(journal, cancelTurn), { clientOperationId: 'cancel-1', turnId: 'turn-1', prompt: { itemId, expectedRevision: 2 } @@ -112,39 +109,38 @@ describe('performCancel for a pending prompt', () => { refusal: { code: 'agent_session_item_revision_stale', currentRevision: 1 } }) expect(cancelTurn).not.toHaveBeenCalled() - expect(flush).not.toHaveBeenCalled() }) - it('drains terminal lifecycle before recording a confirmed cancellation', async () => { + it("records a confirmed cancellation after the prompt's own terminal row", async () => { const { journal, itemId } = await pendingPrompt() const order: string[] = [] const cancelTurn = vi.fn(async () => { order.push('interrupt') - return { cancelled: true } - }) - const flush = vi.fn(async () => { - order.push('lifecycle') const current = journal.snapshot().items.find((item) => item.itemId === itemId)! if (current.body.kind !== 'approval') { throw new Error('expected approval prompt') } - await journal.appendItem( - PROMPT_IDENTITY, - { - ...current.body, - resolution: { - state: 'cancelled', - selectedOptionId: null, - resolvedBy: null, - resolvedAt: null - } - }, - { fence: 1, turnScope: AGENT_JOURNAL_THREAD_SCOPE } - ) + // The provider's frame, issued during the interrupt and not yet landed when it answers. + void journal + .appendItem( + PROMPT_IDENTITY, + { + ...current.body, + resolution: { + state: 'cancelled', + selectedOptionId: null, + resolvedBy: null, + resolvedAt: null + } + }, + { fence: 1, turnScope: AGENT_JOURNAL_THREAD_SCOPE } + ) + .then(() => order.push('lifecycle')) + return { cancelled: true } }) await expect( - performCancel(context(journal, cancelTurn, flush), { + performCancel(context(journal, cancelTurn), { clientOperationId: 'cancel-1', turnId: 'turn-1', prompt: { itemId, expectedRevision: 1 } @@ -159,16 +155,19 @@ describe('performCancel for a pending prompt', () => { resolveLiveTurnId: expect.any(Function), prompt: { itemId } }) - expect(journal.snapshot().items.map((item) => item.body)).toEqual([ + const items = journal.snapshot().items + expect(items.map((item) => item.body)).toEqual([ expect.objectContaining({ resolution: expect.objectContaining({ state: 'cancelled' }) }), { kind: 'status', text: 'Cancellation requested.' } ]) + // Issued after the prompt's terminal row, so it lands after it. + expect(items[1]!.sequence).toBeGreaterThan(items[0]!.sequence) }) it('answers another Cancel of the prompt it cancelled quietly, without reaching the provider', async () => { const { journal, itemId } = await pendingPrompt() - const cancelTurn = vi.fn(async () => ({ cancelled: true })) - const flush = vi.fn(async () => { + // The provider's own cancel of the card, issued while it takes the interrupt. + const cancelTurn = vi.fn(async () => { const current = journal.snapshot().items.find((item) => item.itemId === itemId)! if (current.body.kind !== 'approval') { throw new Error('expected approval prompt') @@ -186,8 +185,9 @@ describe('performCancel for a pending prompt', () => { }, { fence: 1, turnScope: AGENT_JOURNAL_THREAD_SCOPE } ) + return { cancelled: true } }) - const ctx = context(journal, cancelTurn, flush) + const ctx = context(journal, cancelTurn) const cancel = (clientOperationId: string) => performCancel(ctx, { clientOperationId, @@ -223,7 +223,7 @@ describe('performCancel for a pending prompt', () => { ) const cancelTurn = vi.fn(async () => ({ cancelled: true })) - const result = await performCancel(context(journal, cancelTurn, vi.fn()), { + const result = await performCancel(context(journal, cancelTurn), { clientOperationId: 'cancel-1', turnId: 'turn-1', prompt: { itemId, expectedRevision: 1 } @@ -238,11 +238,10 @@ describe('performCancel for a pending prompt', () => { it('keeps the callback answerable when interruption is declined', async () => { const { journal, itemId } = await pendingPrompt() - const flush = vi.fn(async () => undefined) await expect( performCancel( - context(journal, async () => ({ cancelled: false }), flush), + context(journal, async () => ({ cancelled: false })), { clientOperationId: 'cancel-1', turnId: 'turn-1', @@ -251,7 +250,6 @@ describe('performCancel for a pending prompt', () => { ) ).resolves.toEqual({ ok: true, value: { turnId: 'turn-1', cancelled: false } }) - expect(flush).not.toHaveBeenCalled() expect(journal.snapshot().items.map((item) => item.body)).toEqual([ expect.objectContaining({ resolution: expect.objectContaining({ state: 'pending' }) }) ]) @@ -259,17 +257,12 @@ describe('performCancel for a pending prompt', () => { it('propagates an unconfirmed adapter failure and leaves the prompt pending', async () => { const { journal, itemId } = await pendingPrompt() - const flush = vi.fn(async () => undefined) await expect( performCancel( - context( - journal, - async () => { - throw new Error('interrupt receipt lost') - }, - flush - ), + context(journal, async () => { + throw new Error('interrupt receipt lost') + }), { clientOperationId: 'cancel-1', turnId: 'turn-1', @@ -278,30 +271,10 @@ describe('performCancel for a pending prompt', () => { ) ).rejects.toThrow('interrupt receipt lost') - expect(flush).not.toHaveBeenCalled() expect(journal.snapshot().items.map((item) => item.body)).toEqual([ expect.objectContaining({ resolution: expect.objectContaining({ state: 'pending' }) }) ]) }) - - it('surfaces a lifecycle drain failure after the provider confirms interruption', async () => { - const { journal, itemId } = await pendingPrompt() - const flush = vi.fn(async () => { - throw new Error('journal drain failed') - }) - - await expect( - performCancel( - context(journal, async () => ({ cancelled: true }), flush), - { - clientOperationId: 'cancel-1', - turnId: 'turn-1', - prompt: { itemId, expectedRevision: 1 } - } - ) - ).rejects.toThrow('journal drain failed') - expect(journal.snapshot().items).toHaveLength(1) - }) }) describe("a card's own Cancel, as its provider answers it", () => { @@ -320,8 +293,7 @@ describe("a card's own Cancel, as its provider answers it", () => { ) const ctx = context( journal, - vi.fn(async () => ({ cancelled: true })), - vi.fn(async () => undefined) + vi.fn(async () => ({ cancelled: true })) ) const answerPrompt = vi.fn(async (input) => { await input.commit() @@ -395,8 +367,7 @@ describe("a card's own Cancel, as its provider answers it", () => { const { journal, itemId } = await pendingPrompt() const ctx = context( journal, - vi.fn(async () => ({ cancelled: true })), - vi.fn(async () => undefined) + vi.fn(async () => ({ cancelled: true })) ) Object.assign(ctx.adapter, { dismissPrompt: async () => { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.ts index 3719d5a9364f..b4281859aba0 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.ts @@ -45,17 +45,12 @@ export async function cancelStructuredAgentSessionPrompt( const dismissed = await dismissPrompt(ctx, validated, true) return dismissed.ok ? cancelled : dismissed } - // Judged once the provider's accepted lifecycle has landed: its own cancel of the card, or the end - // of the turn that raised it, may still be queued. A failed drain judges what has landed. - await ctx.flushStreamedEvents().catch(() => undefined) - const current = validatePendingPrompt(ctx, input.prompt) - if (!current.ok) { - return answerCancelOfSettledPrompt(ctx, input, current) - } - if (!raisedByLiveTurn(ctx, current)) { + // Judged on the fold as it stands: the provider's own cancel of the card, or the end of the turn + // that raised it, landed when it was handed over. + if (!raisedByLiveTurn(ctx, validated)) { // A request that outlived its turn, such as a background agent's: the turn running now is not // the one the user is cancelling, so nothing stops and the request is declined. - const dismissed = await dismissPrompt(ctx, current, true) + const dismissed = await dismissPrompt(ctx, validated, true) return dismissed.ok ? cancelled : dismissed } const stopped = await routes.stop() @@ -64,7 +59,7 @@ export async function cancelStructuredAgentSessionPrompt( } // Settled in the Stop's own step, so the card is not answerable while the child ends. That end // takes the provider's request with it; a Stop that ends nothing must answer the request itself. - const dismissed = await dismissPrompt(ctx, current, !stopped.endsSession) + const dismissed = await dismissPrompt(ctx, validated, !stopped.endsSession) return dismissed.ok ? cancelled : dismissed } @@ -74,8 +69,8 @@ function raisedByLiveTurn(ctx: AgentSessionTurnContext, pending: PendingPrompt): return live.kind === 'turn' && raised?.kind === 'turn' && raised.turnItemId === live.turnItemId } -/** Records the card as cancelled by the caller, the provider's lifecycle drained first so nothing - * it already sent lands after; `answer` also declines the provider's request. */ +/** Records the card as cancelled by the caller, after every row the provider already sent (each + * landed at its call); `answer` also declines the provider's request. */ async function dismissPrompt( ctx: AgentSessionTurnContext, pending: PendingPrompt, @@ -95,7 +90,6 @@ async function dismissPrompt( } let committed = false const commit = async (): Promise => { - await ctx.flushStreamedEvents() await ctx.journal.appendItem( identity, { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts index 54cfbb470128..f94d091a5f97 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts @@ -269,6 +269,14 @@ export async function createQueuedMessageTestRig( ) } + /** The event sink the provider writes through. */ + function providerEvents(): StructuredAgentSessionEventSink { + if (!events) { + throw new Error('no provider bound') + } + return events + } + /** A host-process restart, as the queue sees it: the conversation closes, and * opens afresh under a new instance id while its rows survive. The close is an eviction, whose * Stop event ends a person's Stop pause if work runs; a quit writes none, so a test of that @@ -316,6 +324,7 @@ export async function createQueuedMessageTestRig( awaitStarted, compact, finishCompact, + providerEvents, envelope, send, stop, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.test.ts index b972c56877e4..8a7c5c4c3d36 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.test.ts @@ -157,18 +157,19 @@ describe('drain', () => { it('takes no serialized drain step while the session is working, then drains when the work settles', async () => { const working = await workingSend() - const flush = vi.spyOn(host, 'flushStreamedEvents') + // Read only by a drain step, so it marks one. + const step = vi.spyOn(JournalQueuedMessages.prototype, 'deliveredByEchoOwed') const queued = await send('waits for the turn', 'queue-if-active').result await send('and another', 'queue-if-active').result if (!queued.ok || !('queued' in queued.value)) { throw new Error('expected a queued receipt') } // Every wake during the turn is answered by the pre-check, not a step. - expect(flush).not.toHaveBeenCalled() + expect(step).not.toHaveBeenCalled() await settleAccepted(working, 'a') const draftId = queued.value.queued.messageId await eventually(async () => expect(await rig.handoff(draftId)).toBeDefined()) - expect(flush).toHaveBeenCalled() + expect(step).toHaveBeenCalled() }) it('a refused conversion returns the card with its stored reason, and an idle send overtakes a lone returned card (N1)', async () => { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.ts index 7d0eb8581e66..ac1fdbb0617c 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.ts @@ -250,9 +250,6 @@ export type QueuedMessageDrainDeps = { sessions: ReadonlyMap getRecord: (sessionId: string) => AgentSessionRecord | null serialize: (sessionId: string, task: () => Promise) => Promise - /** The streamed-event barrier: a turn-open already accepted by the host is - * committed before the gates are read, so no stored busy flag is needed. */ - flushStreamedEvents: (sessionId: string) => Promise conversationFence: (sessionId: string) => number /** The consumed submission is ordinary #22821 work from here on. */ wakeDelivery: (sessionId: string) => void @@ -277,7 +274,7 @@ export class StructuredAgentSessionQueuedMessageDrain { } // Cheap pre-check so token streams do not pay a serialized step per delta. // Skipping while working is safe: whatever ends the work is itself a commit - // that schedules again, and the step re-reads every gate after its flush. + // that schedules again, and the step re-reads every gate from the fold. try { if ( !journal.queuedMessages.settlementOwed() && @@ -318,7 +315,6 @@ export class StructuredAgentSessionQueuedMessageDrain { if (!session || session.journal.isReadOnly) { return } - await this.deps.flushStreamedEvents(sessionId) const journal = session.journal if (journal.queuedMessages.settlementOwed() || journal.queuedMessages.deliveredByEchoOwed()) { // A live per-row hook was skipped; heal now, before a draft sends, rather than at reopen. diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-pause-lift.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-pause-lift.test.ts index d5aa5e497d45..c5baeed25def 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-pause-lift.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-pause-lift.test.ts @@ -4,8 +4,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { AgentSessionJournal } from '../agent-session-journal/journal-store' +import { AGENT_JOURNAL_THREAD_SCOPE } from '../../../shared/agent-session-journal-types' import { HOST_TEST_SESSION, + HOST_TEST_THREAD as THREAD, hostTestMessage, hostTestOperationId } from './structured-agent-session-host-test-data' @@ -448,20 +450,25 @@ describe('Resume', () => { }) describe('a failed Stop', () => { - it('records nothing when it fails before taking effect, so the queue sends as if no Stop was pressed', async () => { - const working = await rig.workingSend() + // Withdrawing is bookkeeping: its failure is reported, and the Stop still interrupts and pauses. + it('still takes effect when its withdrawal fails, and pauses the queue', async () => { + await rig.workingSend() const draftId = await queuedDraft('queued before the stop') const reject = vi .spyOn(AgentSessionJournal.prototype, 'rejectQueuedSubmissions') .mockRejectedValueOnce(new Error('disk full')) + const warned = vi.spyOn(console, 'warn').mockImplementation(() => undefined) try { - await expect(rig.stop()).rejects.toThrow('disk full') + expect(await rig.stop()).toMatchObject({ ok: true }) + expect(warned).toHaveBeenCalledWith( + "[agent-session] stop-queued-bookkeeping: Stop's withdrawal failed", + expect.objectContaining({ step: 'withdrawal', error: new Error('disk full') }) + ) } finally { reject.mockRestore() + warned.mockRestore() } - expect(await rig.queuePause()).toBeNull() - await rig.settleAccepted(working, 'working') - await eventually(async () => expect(await rig.handoff(draftId)).toBeDefined()) + await expectPaused(draftId) }) it('keeps its pause when it fails after the interrupt reached the agent', async () => { @@ -484,4 +491,50 @@ describe('a failed Stop', () => { } await expectPaused(draftId) }) + + // The drain shares the Stop's lane, so a failed Stop must hold it until its pause lands. + it('keeps its pause when it fails while its writes wait behind owed work', async () => { + const turn = { provider: 'codex' as const, threadId: THREAD, turnId: 'turn-w', ordinal: 900 } + const turnRow = (state: 'running' | 'completed') => { + rig + .providerEvents() + .appendItem( + turn, + { kind: 'turn', turnId: 'turn-w', state, startedAt: 1 }, + { turnScope: AGENT_JOURNAL_THREAD_SCOPE } + ) + } + const working = await rig.workingSend() + turnRow('running') + await rig.settleAccepted(working, 'w') + const draftId = await queuedDraft('paused by stop') + // The interrupt ends the turn, so the drain runs as soon as the lane frees. + vi.mocked(rig.host.deps.adapter.cancelTurn).mockImplementationOnce(async () => { + turnRow('completed') + return { cancelled: true } + }) + const journal = rig.host.collaboratorsForTests().sessions.get(HOST_TEST_SESSION)?.journal + if (!journal) { + throw new Error('no open journal') + } + const append = journal.appendItem.bind(journal) + vi.spyOn(journal, 'appendItem').mockImplementation((identity, body, options) => + body.kind === 'status' + ? Promise.reject(new Error('disk full')) + : append(identity, body, options) + ) + // Owed from the Stop's first write, after the open paid any import: its writes wait behind it. + const owed = Promise.withResolvers() + const withdraw = journal.rejectQueuedSubmissions.bind(journal) + vi.spyOn(journal, 'rejectQueuedSubmissions').mockImplementation((...args) => { + journal['queue'].owe(() => owed.promise) + return withdraw(...args) + }) + const stopping = rig.stop() + await eventually(() => expect(rig.host.deps.adapter.cancelTurn).toHaveBeenCalledOnce()) + owed.resolve() + await expect(stopping).rejects.toThrow('disk full') + expect(journal.activeTurnId()).toBeNull() + await expectPaused(draftId) + }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-stop.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-stop.ts index 316793751c55..ccdc429e34e2 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-stop.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-stop.ts @@ -13,7 +13,7 @@ import { type QueuedMessageRow } from '../agent-session-journal/queued-message-table' import type { AgentSessionTurnContext, TurnOutcome } from './structured-agent-session-turns' -import { isMainAgentWorkingOnceFlushed } from './structured-agent-session-turns-cancel' +import { isMainAgentWorking } from './structured-agent-session-turns-cancel' import { structuredAgentSessionStopNamesTurnNotLive, structuredAgentSessionStoppedTurnId @@ -26,14 +26,15 @@ export function unsettledQueuedMessages(journal: AgentSessionJournal): QueuedMes } /** - * Runs a Stop, which calls `tookEffect` where it takes effect: after it withdrew the queued - * sends, and BEFORE the interrupt or anything that ends the child (the at-start stop, a running - * command's stop, a kill after the interrupt), or, reaching no agent, once it withdrew something. - * That writes the Stop's event, whatever the queue holds, so a card its interrupt later withdraws - * comes back to waiting under the pause, and whatever ends the child finds the event already - * written. A Stop that throws before then, or stops nothing (`stopReachesUnrecordedWork`), changed - * nothing and writes nothing. - * The drain cannot slip a card in between: the Stop runs on the drain's serialized lane. + * Runs a Stop, which calls `tookEffect` where it takes effect: after it issued the withdrawal of + * the queued sends, and BEFORE the interrupt or anything that ends the child (the at-start stop, a + * running command's stop, a kill after the interrupt), or, reaching no agent, once it withdrew + * something. That issues the Stop's event, whatever the queue holds, so a card its interrupt later + * withdraws comes back to waiting under the pause, and whatever ends the child finds the event + * ahead of it in the journal. A Stop that throws before then, or stops nothing + * (`stopReachesUnrecordedWork`), changed nothing and writes nothing. + * The drain cannot slip a card in between: the Stop runs on the drain's serialized lane and holds + * it until the event lands, even when it fails. */ export async function runRecordedStop( ctx: AgentSessionTurnContext, @@ -62,15 +63,15 @@ export async function runRecordedStop( * with nothing sent since, on the same turn or one that opened after a Stop pressed before any * turn showed: a card queued between the presses then sends normally, as after one Stop. */ -export async function stopReachesUnrecordedWork( - ctx: Pick, +export function stopReachesUnrecordedWork( + ctx: Pick, namedTurnId: string | undefined -): Promise { +): boolean { const live = ctx.journal.activeTurnId() // No turn published yet while the agent works: the named one may still be opening. if ( structuredAgentSessionStopNamesTurnNotLive(namedTurnId, live) && - (live !== null || !(await isMainAgentWorkingOnceFlushed(ctx))) + (live !== null || !isMainAgentWorking(ctx)) ) { return false } @@ -91,6 +92,23 @@ export async function stopReachesUnrecordedWork( return sentSince || structuredAgentSessionStopNamesTurnNotLive(inForce.event.turnId, live) } +/** The Stop's withdrawal of every queued send, issued at once and never awaited ahead of the + * interrupt. Bookkeeping: one that fails is reported and withdrew nothing. */ +export function withdrawQueuedForStop( + ctx: AgentSessionTurnContext, + withdraw: () => Promise +): Promise { + const failed = (error: unknown): boolean => { + report(ctx, 'withdrawal', error) + return false + } + try { + return withdraw().then((withdrawn) => withdrawn.length > 0, failed) + } catch (error) { + return Promise.resolve(failed(error)) + } +} + function report(ctx: AgentSessionTurnContext, step: string, error: unknown): void { ctx.logger.warn(`Stop's ${step} failed`, { scope: 'stop-queued-bookkeeping', diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-wiring.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-wiring.ts index 1717f2e7ec3b..a0d3dbe7b529 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-wiring.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-wiring.ts @@ -30,7 +30,6 @@ export function wireStructuredAgentSessionQueuedMessages( sessions, getRecord: (sessionId) => context().deps.store.getRecord(sessionId), serialize: (sessionId, task) => context().serialize(sessionId, task), - flushStreamedEvents: (sessionId) => context().flushStreamedEvents(sessionId), conversationFence: (sessionId) => structuredAgentSessionConversationFence(context().deps.store, sessionId), wakeDelivery: (sessionId) => context().wakeDelivery(sessionId), diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-withdrawn-draft.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-withdrawn-draft.test.ts index 7ae2baada74c..5e23fbe1a84a 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-withdrawn-draft.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-withdrawn-draft.test.ts @@ -99,7 +99,9 @@ it('Stop, then a user send: the withdrawn draft and the paused cards behind it d expect(await rig.drafts()).toEqual([]) }) -it('a Stop that fails after withdrawing a consumed draft releases it, and it sends again under a fresh id', async () => { +// Bookkeeping never fails a Stop: a withdrawal that throws is reported and counts as nothing +// withdrawn, so no pause holds the card it did send back, and that card sends again. +it('a Stop whose withdrawal throws after landing still answers; the draft it released sends again under a fresh id', async () => { const working = await rig.workingSend() const a = await queuedDraft('A') let release: () => void = () => undefined @@ -120,10 +122,16 @@ it('a Stop that fails after withdrawing a consumed draft releases it, and it sen } return withdrawn }) + const warned = vi.spyOn(console, 'warn').mockImplementation(() => undefined) try { - await expect(rig.stop()).rejects.toThrow('disk full') + expect(await rig.stop()).toMatchObject({ ok: true }) + expect(warned).toHaveBeenCalledWith( + "[agent-session] stop-queued-bookkeeping: Stop's withdrawal failed", + expect.objectContaining({ step: 'withdrawal', error: new Error('disk full') }) + ) } finally { failing.mockRestore() + warned.mockRestore() release() } expect((await rig.submission(firstA))?.dispatchState).toBe('rejected') diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-resolved-append.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-resolved-append.test.ts index 4b06198e373b..a2e843378a54 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-resolved-append.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-resolved-append.test.ts @@ -1,11 +1,15 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { AGENT_JOURNAL_THREAD_SCOPE } from '../../../shared/agent-session-journal-types' -import { describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { agentJournalItemKey } from '../../../shared/agent-session-journal-item-key' import type { AgentJournalItemBody, AgentJournalItemIdentity } from '../../../shared/agent-session-journal-types' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' +import { openJournalOwingImport } from '../agent-session-journal/journal-owed-import-test-support' import { createDeferredStructuredAgentSessionEventSink, type StructuredAgentSessionEventTarget, @@ -27,26 +31,37 @@ function textOf(body: AgentJournalItemBody | undefined): string { return block?.type === 'text' ? block.text : '' } -/** A journal whose appends land a tick later, so an unserialized read would race. */ -function journalTarget(rows: Map): StructuredAgentSessionEventTarget { - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: a double for the journal methods the resolved paths call. - const journal = { - epoch: 'e', - appendItem: vi.fn(async (identity: AgentJournalItemIdentity, body: AgentJournalItemBody) => { - await new Promise((resolve) => setTimeout(resolve, 1)) - rows.set(agentJournalItemKey(identity), body) - return { cursor: { epoch: 'e', sequence: rows.size } } - }), - visitItems: (visit: (itemId: string, sequence: number, body: AgentJournalItemBody) => void) => { - let sequence = 0 - for (const [itemId, body] of rows) { - visit(itemId, ++sequence, body) - } +let root = '' +let journal: AgentSessionJournal + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-resolved-append-')) + // Its copy owed, so every write handed over waits in the queue: a resolver that read at submit + // would read the row before the revisions ahead of it had landed. + ;({ journal } = await openJournalOwingImport({ + stateDirectory: root, + identity: { + sessionId: 'session-1', + workspaceId: 'workspace-1', + hostId: 'host-1', + agent: 'codex', + providerHandle: { kind: 'codex', threadId: 'thread-1' } } - } as unknown as AgentSessionJournal + })) +}) + +afterEach(async () => { + await journal.close() + await rm(root, { recursive: true, force: true }) +}) + +/** The real journal: each resolved write reads the fold at its own place in the write queue. */ +function journalTarget(): StructuredAgentSessionEventTarget { return { journal, fence: 1, publish: vi.fn() } } +const rowText = (): string => textOf(journal.itemBody(agentJournalItemKey(ROW)) ?? undefined) + const appendSuffix = (suffix: string): StructuredAgentSessionRevisionResolver => (journal) => { @@ -61,7 +76,6 @@ const appendSuffix = describe('resolved revisions', () => { it('reads the row as the journal holds it when each queued revision runs', async () => { - const rows = new Map() const deferred = createDeferredStructuredAgentSessionEventSink(testEventSinkLogging()) const bytes = estimateStructuredAgentSessionItemBytes(ROW, text('abc')) for (const suffix of ['a', 'b', 'c']) { @@ -75,25 +89,23 @@ describe('resolved revisions', () => { } // Three revisions of one row stay three operations; none replaces another. expect(deferred.state().queuedOperations).toBe(3) - deferred.bind(journalTarget(rows)) + deferred.bind(journalTarget()) await expect(deferred.drained()).resolves.toEqual({ ok: true }) - expect(textOf(rows.get(agentJournalItemKey(ROW)))).toBe('abc') + expect(rowText()).toBe('abc') }) it('skips a revision that resolves to nothing', async () => { - const rows = new Map() const deferred = createDeferredStructuredAgentSessionEventSink(testEventSinkLogging()) deferred.sink.tryReviseResolvedItem?.(1_000, () => null, { turnScope: AGENT_JOURNAL_THREAD_SCOPE }) - const target = journalTarget(rows) - deferred.bind(target) + const before = journal.cursor() + deferred.bind(journalTarget()) await expect(deferred.drained()).resolves.toEqual({ ok: true }) - expect(target.journal.appendItem).not.toHaveBeenCalled() + expect(journal.cursor()).toEqual(before) }) it('publishes a revision in the operation that writes it, within the same reservation', async () => { - const rows = new Map() const deferred = createDeferredStructuredAgentSessionEventSink(testEventSinkLogging()) const bytes = estimateStructuredAgentSessionItemBytes(ROW, text('a')) deferred.sink.tryReviseResolvedItemAndPublish?.(bytes, appendSuffix('a'), { @@ -102,11 +114,9 @@ describe('resolved revisions', () => { deferred.sink.tryReviseResolvedItemAndPublish?.(bytes, () => null, { turnScope: AGENT_JOURNAL_THREAD_SCOPE }) - const target = journalTarget(rows) + const target = journalTarget() const published: string[] = [] - vi.mocked(target.publish).mockImplementation(() => - published.push(textOf(rows.get(agentJournalItemKey(ROW)))) - ) + vi.mocked(target.publish).mockImplementation(() => published.push(rowText())) deferred.bind(target) await expect(deferred.drained()).resolves.toEqual({ ok: true }) // Published once, after the append: the revision that resolves to nothing publishes nothing. @@ -114,7 +124,6 @@ describe('resolved revisions', () => { }) it('refuses a resolved write larger than the reservation it was admitted with', async () => { - const rows = new Map() const deferred = createDeferredStructuredAgentSessionEventSink(testEventSinkLogging()) const bytes = estimateStructuredAgentSessionItemBytes(ROW, text('a')) deferred.sink.tryReviseResolvedItem?.( @@ -125,8 +134,8 @@ describe('resolved revisions', () => { }), { turnScope: AGENT_JOURNAL_THREAD_SCOPE } ) - deferred.bind(journalTarget(rows)) + deferred.bind(journalTarget()) await expect(deferred.drained()).resolves.toMatchObject({ ok: false }) - expect(rows.size).toBe(0) + expect(journal.itemBody(agentJournalItemKey(ROW))).toBeNull() }) }) 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 eb2fbea97fb7..d3640db2a0cf 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 @@ -13,8 +13,8 @@ import type { StructuredAgentSessionSinkQueue } from './structured-agent-session type ResolvedItem = { identity: AgentJournalItemIdentity; body: AgentJournalItemBody } -/** Resolve a queued item against the journal bound at execution. The queue runs - * one operation at a time, so what the resolver reads is what the append revises. */ +/** Resolve an item against the journal it lands in, at its own place in that journal's write + * queue, so what the resolver reads is every write issued before it and none after. */ export function createStructuredAgentSessionResolvedAppend( queue: StructuredAgentSessionSinkQueue ): Required< @@ -30,33 +30,37 @@ export function createStructuredAgentSessionResolvedAppend( const submit = ( reservedBytes: number, resolve: (journal: StructuredAgentSessionRevisionJournal) => ResolvedItem | null, - options: Omit, - publish: boolean + options: StructuredAgentSessionItemAppendOptions, + input: { publish: boolean; lifecycle?: true; overflow: string } ) => queue.submit( { bytes: reservedBytes, + ...(input.lifecycle ? { lifecycle: true } : {}), run: async (bound) => { - const resolved = resolve(bound.journal) - if (resolved === null) { - return - } - const bytes = estimateStructuredAgentSessionItemBytes(resolved.identity, resolved.body) - if (bytes + (publish ? 1 : 0) > reservedBytes) { - throw new Error('structured agent-session resolved item exceeded its reserved size') - } - await bound.journal.appendItem( - resolved.identity, - resolved.body, + const landed = await bound.journal.appendResolvedItem( + () => { + const resolved = resolve(bound.journal) + if ( + resolved !== null && + estimateStructuredAgentSessionItemBytes(resolved.identity, resolved.body) + + (input.publish && !input.lifecycle ? 1 : 0) > + reservedBytes + ) { + throw new Error(input.overflow) + } + return resolved + }, structuredAgentSessionJournalAppendOptions(bound.fence, options) ) - if (publish) { + if (landed !== null && input.publish) { bound.publish() } } }, - options + input.lifecycle ? { lifecycle: true } : options ) + const ITEM_OVERFLOW = 'structured agent-session resolved item exceeded its reserved size' const identityOnly = (publish: boolean) => ((identitySizeBound, body, resolveIdentity, options) => submit( @@ -66,39 +70,28 @@ export function createStructuredAgentSessionResolvedAppend( return identity === null ? null : { identity, body } }, options, - publish + { publish, overflow: ITEM_OVERFLOW } )) satisfies NonNullable return { tryAppendResolvedItem: identityOnly(false), tryAppendResolvedItemAndPublish: identityOnly(true), tryReviseResolvedItem: (reservedBytes, resolve, options) => - submit(reservedBytes, resolve, options, false), + submit(reservedBytes, resolve, options, { publish: false, overflow: ITEM_OVERFLOW }), tryReviseResolvedItemAndPublish: (reservedBytes, resolve, options) => - submit(reservedBytes + 1, resolve, options, true), - tryAppendLifecycleTransition: (identitySizeBound, body, resolveIdentity, options) => { - const bytes = estimateStructuredAgentSessionItemBytes(identitySizeBound, body) - return queue.submit( + submit(reservedBytes + 1, resolve, options, { publish: true, overflow: ITEM_OVERFLOW }), + tryAppendLifecycleTransition: (identitySizeBound, body, resolveIdentity, options) => + submit( + estimateStructuredAgentSessionItemBytes(identitySizeBound, body), + (journal) => { + const identity = resolveIdentity(journal) + return identity === null ? null : { identity, body } + }, + options, { - bytes, + publish: true, lifecycle: true, - run: async (bound) => { - const identity = resolveIdentity(bound.journal) - if (identity === null) { - return - } - if (estimateStructuredAgentSessionItemBytes(identity, body) > bytes) { - throw new Error('structured agent-session item identity exceeded its reserved size') - } - await bound.journal.appendItem( - identity, - body, - structuredAgentSessionJournalAppendOptions(bound.fence, options) - ) - bound.publish() - } - }, - { lifecycle: true } + overflow: 'structured agent-session item identity exceeded its reserved size' + } ) - } } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-rewind.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-rewind.test.ts index 6cc37ccb724f..dc8eb1117005 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-rewind.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-rewind.test.ts @@ -263,6 +263,26 @@ describe('host rewind', () => { expect(await host.rewind(caller, request)).toMatchObject({ ok: true, replayed: true }) expect(rewind).toHaveBeenCalledTimes(1) }) + // The stream's failure is the stream's to recover from; its stale error is not the rewind's. + it("rewinds after the chat's event sink failed, its error never failing the rewind", async () => { + const target = await seed() + const request = await params(target) + const refused = vi + .spyOn(AgentSessionJournal.prototype, 'appendItem') + .mockRejectedValueOnce(new Error('disk full')) + vi.spyOn(console, 'warn').mockImplementation(() => undefined) + sink.appendItem( + { provider: 'codex', threadId: HOST_TEST_THREAD, turnId: 'tip', ordinal: 1 }, + hostTestMessage('refused'), + { turnScope: AGENT_JOURNAL_THREAD_SCOPE } + ) + await vi.waitFor(() => expect(refused).toHaveBeenCalled()) + refused.mockRestore() + + expect(await host.rewind(caller, request)).toMatchObject({ ok: true }) + expect((await host.journalSnapshot(HOST_TEST_SESSION)).items).toHaveLength(1) + }) + it('refuses a rewind racing an active turn before provider execution', async () => { const target = await seed() sink.appendItem( diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-rewind.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-rewind.ts index f2a1c088e261..f95579d7750d 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-rewind.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-rewind.ts @@ -43,7 +43,6 @@ export async function rewindStructuredAgentSession( prepareSession: openWithAgent(context, params.envelope), journal: () => context.sessions.get(sessionId)?.journal, publish: (journal) => context.publish(sessionId, journal), - flushStreamedEvents: context.flushStreamedEvents, now: context.now, plan: { method: 'agentSession.rewind', diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-send-idempotency.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-send-idempotency.test.ts index 48ceef370e78..e170461095b8 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-send-idempotency.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-send-idempotency.test.ts @@ -66,7 +66,6 @@ describe('structured send idempotency', () => { persistOptions: async () => undefined, resolvedBy: 'caller', publish: vi.fn(), - flushStreamedEvents: async () => undefined, now: () => 1 }, input @@ -108,7 +107,6 @@ describe('structured send idempotency', () => { persistOptions: async () => undefined, resolvedBy: 'caller', publish: vi.fn(), - flushStreamedEvents: async () => undefined, now: () => 1 } const input = { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-send-preparation.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-send-preparation.ts index 7c6511dd2f21..77413f456340 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-send-preparation.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-send-preparation.ts @@ -54,17 +54,16 @@ export function structuredAgentSessionSendBlock( return null } -/** The conversation a send or a Stop writes to, opened when this host holds it closed. */ +/** The conversation a send or a Stop writes to, opened when this host holds it closed. Once it + * answers, every write issued before it has settled, so a mutation reads a whole fold. */ export async function openConversationForWrite( openConversation: (sessionId: string) => Promise, envelope: AgentSessionMutationEnvelope, logger: StructuredAgentSessionLogger ): Promise { + let session: StructuredAgentSessionHostSession | null try { - if (await openConversation(envelope.sessionId)) { - return { ok: true } - } - return { ok: false, refusal: AGENT_SESSION_NOT_ATTACHED } + session = await openConversation(envelope.sessionId) } catch (error) { logger.warn('opening the conversation for a write failed', { scope: 'open-for-write', @@ -73,6 +72,21 @@ export async function openConversationForWrite( }) return { ok: false, refusal: journalOpenRefusal(error) } } + if (!session) { + return { ok: false, refusal: AGENT_SESSION_NOT_ATTACHED } + } + // Writes wait behind a restore's owed import. A failed import settled them too (they failed + // with it), so it is reported and never refuses the mutation: its own writes fail as they would. + if (session.journal.importPending) { + await session.journal.whenImported().catch((error: unknown) => { + logger.warn('the import owed before a write failed', { + scope: 'open-for-write', + sessionId: envelope.sessionId, + error + }) + }) + } + return { ok: true } } /** The conversation a write lands in, opened when this host holds it closed. */ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-never-waits.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-never-waits.test.ts new file mode 100644 index 000000000000..f2f3ad7ad697 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-never-waits.test.ts @@ -0,0 +1,290 @@ +// A Stop's interrupt never waits on the journal. Its withdrawal and its Stop event are bookkeeping: +// one that throws is reported and the Stop still interrupts. Its writes may wait behind owed work; +// they are issued, and the interrupt goes out before they land. + +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import { AGENT_JOURNAL_THREAD_SCOPE } from '../../../shared/agent-session-journal-types' +import type { AgentSessionJournal } from '../agent-session-journal/journal-store' +import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' +import type { StructuredAgentSessionHost } from './structured-agent-session-host' +import { + attach, + CALLER, + envelope, + hostTestState +} from './structured-agent-session-host-test-harness' +import { + HOST_TEST_SESSION as SESSION, + HOST_TEST_THREAD as THREAD +} from './structured-agent-session-host-test-data' + +let host: StructuredAgentSessionHost +let acquire: Mock +let cancelTurn: Mock +let log: ReturnType['log'] + +beforeEach(() => { + ;({ host, acquire, cancelTurn, log } = hostTestState()) + vi.spyOn(console, 'warn').mockImplementation(() => undefined) +}) + +afterEach(() => { + vi.restoreAllMocks() +}) + +async function runningTurn(): Promise { + await attach() + acquire.mock.calls + .at(-1)![0] + .events!.appendItem( + { provider: 'codex', threadId: THREAD, turnId: 'turn-1', ordinal: 900 }, + { kind: 'turn', turnId: 'turn-1', state: 'running' }, + { turnScope: AGENT_JOURNAL_THREAD_SCOPE } + ) + const journal = host.collaboratorsForTests().sessions.get(SESSION)?.journal + if (!journal) { + throw new Error('no open journal') + } + // The turn row landed when the provider's event was handed over. + expect(journal.activeTurnId()).toBe('turn-1') + return journal +} + +/** A child the adapter published before proving its startup, so a Stop ends the start. */ +async function startingChild(): Promise<{ journal: AgentSessionJournal; closeSession: Mock }> { + const acquired = acquire.getMockImplementation()! + acquire.mockImplementationOnce(async (input) => ({ + ...(await acquired(input)), + providerChildPhase: 'starting' as const + })) + const closeSession = vi.fn(async () => true) + Object.assign(host.deps.adapter, { closeSession }) + await attach() + const session = host.collaboratorsForTests().sessions.get(SESSION) + if (!session) { + throw new Error('no open session') + } + expect(session.child?.phase).toBe('starting') + return { journal: session.journal, closeSession } +} + +function stop(fields: { turnId?: string }) { + return host.cancel(CALLER, { envelope: envelope('agentSession.cancel', fields), ...fields }) +} + +const MALFORMED = 'database disk image is malformed' + +/** The Stop reported `step`'s failure through the host's logger. */ +function expectReported(step: 'withdrawal' | 'event row'): void { + expect(log.entries).toContainEqual( + expect.objectContaining({ + level: 'warn', + message: `Stop's ${step} failed`, + fields: expect.objectContaining({ error: expect.objectContaining({ message: MALFORMED }) }) + }) + ) +} +/** The held write fails only once the interrupt went out; a Stop that awaited it times out here. */ +const INTERRUPT_WAIT = { timeout: 5_000 } + +describe.each([ + ['naming no turn', {}], + ['naming its turn', { turnId: 'turn-1' }] +])('a Stop %s', (_label, fields) => { + it('interrupts when withdrawing the queued sends throws, and reports it', async () => { + const journal = await runningTurn() + vi.spyOn(journal, 'rejectQueuedSubmissions').mockImplementation(() => { + throw new Error(MALFORMED) + }) + + expect(await stop(fields)).toMatchObject({ ok: true }) + expect(cancelTurn).toHaveBeenCalledOnce() + expectReported('withdrawal') + }) + + it('interrupts when writing its Stop event throws, and reports it', async () => { + const journal = await runningTurn() + vi.spyOn(journal, 'appendStopEvent').mockImplementation(() => { + throw new Error(MALFORMED) + }) + + expect(await stop(fields)).toMatchObject({ ok: true }) + expect(cancelTurn).toHaveBeenCalledOnce() + expectReported('event row') + }) + + // Held, then failing: the interrupt goes out while the write is still pending, not after it settles. + it.each([ + ['withdrawing the queued sends', 'withdrawal'], + ['writing its Stop event', 'event row'] + ] as const)('interrupts before %s settles, then reports its failure', async (step, failed) => { + const journal = await runningTurn() + const order: string[] = [] + const held = Promise.withResolvers() + if (step === 'withdrawing the queued sends') { + vi.spyOn(journal, 'rejectQueuedSubmissions').mockImplementation(() => held.promise) + } else { + vi.spyOn(journal, 'appendStopEvent').mockImplementation(() => held.promise) + } + cancelTurn.mockImplementation(async () => { + order.push('interrupt') + return { cancelled: true } + }) + + const stopping = stop(fields) + try { + await vi.waitFor(() => expect(cancelTurn).toHaveBeenCalledOnce(), INTERRUPT_WAIT) + } finally { + order.push('write fails') + held.reject(new Error(MALFORMED)) + } + + expect(await stopping).toMatchObject({ ok: true }) + expect(order).toEqual(['interrupt', 'write fails']) + expectReported(failed) + }) + + it.each([ + ['withdrawing the queued sends', 'withdrawal'], + ['writing its Stop event', 'event row'] + ] as const)( + 'ends a starting child before %s settles, then reports its failure', + async (step, failed) => { + const { journal, closeSession } = await startingChild() + const order: string[] = [] + const held = Promise.withResolvers() + if (step === 'withdrawing the queued sends') { + vi.spyOn(journal, 'rejectQueuedSubmissions').mockImplementation(() => held.promise) + } else { + vi.spyOn(journal, 'appendStopEvent').mockImplementation(() => held.promise) + } + closeSession.mockImplementation(async () => { + order.push('stop') + return true + }) + + const stopping = stop(fields) + try { + await vi.waitFor(() => expect(closeSession).toHaveBeenCalledOnce(), INTERRUPT_WAIT) + } finally { + order.push('write fails') + held.reject(new Error(MALFORMED)) + } + + expect(await stopping).toMatchObject({ ok: true, value: { cancelled: true } }) + expect(order).toEqual(['stop', 'write fails']) + expectReported(failed) + } + ) + + // A provider whose Stop ends its session: the child's end, in the Stop's next step, goes out + // before the write settles too; that step holds the lane for the Stop event instead. + it.each([ + ['withdrawing the queued sends', 'withdrawal'], + ['writing its Stop event', 'event row'] + ] as const)( + "ends a session-ending provider's child before %s settles, then reports its failure", + async (step, failed) => { + const journal = await runningTurn() + const order: string[] = [] + const closeSession = vi.fn(async () => { + order.push('kill') + return true + }) + Object.assign(host.deps.adapter, { stopEndsSession: () => true, closeSession }) + const held = Promise.withResolvers() + if (step === 'withdrawing the queued sends') { + vi.spyOn(journal, 'rejectQueuedSubmissions').mockImplementation(() => held.promise) + } else { + vi.spyOn(journal, 'appendStopEvent').mockImplementation(() => held.promise) + } + cancelTurn.mockImplementation(async () => { + order.push('interrupt') + return { cancelled: true } + }) + + const stopping = stop(fields) + try { + await vi.waitFor(() => expect(closeSession).toHaveBeenCalledOnce(), INTERRUPT_WAIT) + } finally { + order.push('write fails') + held.reject(new Error(MALFORMED)) + } + + expect(await stopping).toMatchObject({ ok: true, value: { cancelled: true } }) + expect(order).toEqual(['interrupt', 'kill', 'write fails']) + await vi.waitFor(() => expectReported(failed)) + } + ) + + // The open pays an import owed before the Stop, so the work falls owed at the Stop's first write: + // the one moment the Stop's own writes can wait behind it. + it('interrupts before owed work its writes wait behind is paid', async () => { + const journal = await runningTurn() + const owed = Promise.withResolvers() + const withdraw = journal.rejectQueuedSubmissions.bind(journal) + vi.spyOn(journal, 'rejectQueuedSubmissions').mockImplementation((...args) => { + journal['queue'].owe(() => owed.promise) + return withdraw(...args) + }) + let answered = false + const stopping = stop(fields).finally(() => { + answered = true + }) + try { + await vi.waitFor(() => expect(cancelTurn).toHaveBeenCalledOnce()) + expect(journal.importPending).toBe(true) + expect(answered).toBe(false) + } finally { + owed.resolve() + } + expect(await stopping).toMatchObject({ ok: true }) + expect(journal.importPending).toBe(false) + }) + + // Under owed work the event is issued before the interrupt but lands later, still in queue order: + // ahead of the stopped turn's end, which the provider hands over as it takes the interrupt. + it('issues its Stop event before the interrupt, and it lands ahead of the turn it ends', async () => { + const journal = await runningTurn() + const order: string[] = [] + const owed = Promise.withResolvers() + const withdraw = journal.rejectQueuedSubmissions.bind(journal) + vi.spyOn(journal, 'rejectQueuedSubmissions').mockImplementation((...args) => { + journal['queue'].owe(() => owed.promise) + return withdraw(...args) + }) + const appendStopEvent = journal.appendStopEvent.bind(journal) + vi.spyOn(journal, 'appendStopEvent').mockImplementation((...args) => { + order.push('event issued') + return appendStopEvent(...args) + }) + cancelTurn.mockImplementation(async () => { + order.push('interrupt') + acquire.mock.calls + .at(-1)![0] + .events!.appendItem( + { provider: 'codex', threadId: THREAD, turnId: 'turn-1', ordinal: 900 }, + { kind: 'turn', turnId: 'turn-1', state: 'interrupted' }, + { turnScope: AGENT_JOURNAL_THREAD_SCOPE } + ) + return { cancelled: true } + }) + + const stopping = stop(fields) + try { + await vi.waitFor(() => expect(cancelTurn).toHaveBeenCalledOnce(), INTERRUPT_WAIT) + } finally { + owed.resolve() + } + expect(await stopping).toMatchObject({ ok: true }) + + expect(order).toEqual(['event issued', 'interrupt']) + const since = journal.readSince({ epoch: journal.epoch, sequence: 0 }) + const rows = since.ok ? since.rows : [] + const stopRow = rows.find((row) => row.kind === 'tombstone' && row.stopEvent) + const turnEnd = rows.find( + (row) => row.kind === 'item' && row.body.kind === 'turn' && row.body.state === 'interrupted' + ) + expect(stopRow?.seq).toBeLessThan(turnEnd?.seq ?? 0) + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-thread-goal.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-thread-goal.test.ts index 7e59f695d6d4..6349e7b8ef60 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-thread-goal.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-thread-goal.test.ts @@ -65,8 +65,7 @@ function appendGoalRow( function context( journal: AgentSessionJournal, - adapter: Partial, - flushStreamedEvents: () => Promise = async () => undefined + adapter: Partial ): AgentSessionTurnContext { return { logger: createStructuredAgentSessionLogger(), @@ -78,7 +77,6 @@ function context( persistOptions: async () => undefined, resolvedBy: 'client-1', publish: vi.fn(), - flushStreamedEvents, now: () => 1 } } @@ -260,19 +258,19 @@ describe('performThreadGoalChange', () => { ]) }) - it('drains accepted provider events before deciding whether a set replaces a goal', async () => { + it('reads a goal the provider reported, still landing when the set arrives, as the one it replaces', async () => { const journal = await openJournal() const changeThreadGoal = vi.fn(async () => ({ ok: true as const })) - // The goal the provider reported is still in the deferred sink when the set arrives. - const ctx = context(journal, { changeThreadGoal, supportsThreadGoal: () => true }, async () => { - await appendGoalRow(journal, { status: 'active' }) - }) + const ctx = context(journal, { changeThreadGoal, supportsThreadGoal: () => true }) + // Issued, not yet landed: the set's read takes its place behind it in the journal's queue. + const landing = appendGoalRow(journal, { status: 'active' }) await performThreadGoalChange(ctx, { clientOperationId: 'op-10', change: { kind: 'set', objective: 'Ship the tests' } }) + await landing expect(changeThreadGoal).toHaveBeenCalledWith(expect.objectContaining({ replacesGoal: true })) }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-thread-goal.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-thread-goal.ts index a779ba51abe8..f1612e03db2c 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-thread-goal.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-thread-goal.ts @@ -54,9 +54,9 @@ export async function performThreadGoalChange( const identity = objectiveIdentity(input.clientOperationId) let replacesGoal = false if (change.kind === 'set') { - // A goal transition the host accepted but has not journaled yet decides this too. - await ctx.flushStreamedEvents() - // Read before the objective row lands: that row is a message, not a goal transition. + // Read before the objective row lands: that row is a message, not a goal transition. A goal + // transition the host accepted is already in the fold: it landed at its call, or before the + // open's owed import finished. replacesGoal = ctx.journal.threadGoal() !== null } // Journal first: an active goal starts provider work at once, and the objective diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts index 48500afa12f3..5e26d1b620f8 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts @@ -24,16 +24,11 @@ import { import { isStructuredAgentSessionMainAgentWorking } from '../../../shared/structured-agent-session-main-agent-working' import type { AgentSessionTurnContext, TurnOutcome } from './structured-agent-session-turns' -/** Claude's echo accepts a send one sink write before its turn row lands, so read after the drain. - * A failed drain reads working: bookkeeping never talks a Stop out of stopping. */ -export async function isMainAgentWorkingOnceFlushed( - ctx: Pick -): Promise { - try { - await ctx.flushStreamedEvents() - } catch { - return true - } +/** Whether the fold reads working. Every write has landed by its call's return, and the open paid + * any owed import, so a Stop reads it without waiting on the write queue. */ +export function isMainAgentWorking( + ctx: Pick +): boolean { return isStructuredAgentSessionMainAgentWorking( ctx.journal.activeTurnId(), ctx.journal.submissions(), @@ -46,11 +41,11 @@ export async function isMainAgentWorkingOnceFlushed( * rest, or running a different turn, is not the Stop's to end. A child that exited reads at rest: * its exit ends its turn, and the host's own exit handling waits behind this step. */ -async function stillRunsStoppedTurn( - ctx: Pick, +function stillRunsStoppedTurn( + ctx: Pick, stoppedTurnId: string | null -): Promise { - if (!(await isMainAgentWorkingOnceFlushed(ctx))) { +): boolean { + if (!isMainAgentWorking(ctx)) { return false } // Working with no turn open after the Stop's turn is a later send whose turn has not opened. @@ -119,8 +114,9 @@ export async function performCancel( /** Hands the child's end to the Stop's next serialized step, for a provider whose Stop ends * its session. */ endSession?: (windDown: StructuredAgentSessionStopWindDown) => void - /** The host already withdrew queued messages for this Stop. */ - withdrewQueued?: boolean + /** Whether the host's withdrawal of queued messages for this Stop withdrew any; awaited only + * after the interrupt. */ + withdrewQueued?: Promise /** The session's child records: a background Stop reaches the tasks they offer a stop. */ childWork?: () => readonly AgentChildWorkView[] | undefined } @@ -192,7 +188,9 @@ export async function performCancel( cancelled = outcome.cancelled refusal = outcome.refusal interruptFailed = refusal !== undefined && refusal.turnNotRunning !== true - if (!cancelled && input.withdrewQueued && !(await isMainAgentWorkingOnceFlushed(ctx))) { + // Working is read first: a working session never takes this branch, so a child end below never + // waits on the withdrawal. On an idle queue the withdrawal has already landed in the fold. + if (!cancelled && !isMainAgentWorking(ctx) && (await input.withdrewQueued) === true) { // A Stop that withdrew what was queued and left nothing working ended what it was sent for, // named or not. The journal judges it: providers differ on refusing a turn that has ended. cancelled = true @@ -234,7 +232,7 @@ export async function performCancel( interruptFailed && input.stopChild && // An unnamed Stop meant the turn the journal showed when it was sent. - (await stillRunsStoppedTurn(ctx, stoppedTurnId)) + stillRunsStoppedTurn(ctx, stoppedTurnId) ) { // The interrupt failed and the turn runs on: only the child's end stops it. let ended: boolean @@ -257,9 +255,6 @@ export async function performCancel( // Nothing was left of the turn it named and nothing else ended: a Stop that ends nothing writes no row. note = null } - if (cancelled && input.prompt) { - await ctx.flushStreamedEvents() - } const value = { ...(input.turnId !== undefined ? { turnId: input.turnId } : {}), cancelled } if (input.scope || note === null) { return { ok: true, value } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-turns.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-turns.test.ts index a09024ffcdda..b2f30032aa90 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-turns.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-turns.test.ts @@ -8,7 +8,10 @@ import type { AgentChildWorkView } from '../../../shared/agent-status-child-work import { createTrackedJournalOpener } from '../agent-session-journal/journal-host-database-test-support' import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' import { performCancel, type AgentSessionTurnContext } from './structured-agent-session-turns' +import { createDeferredStructuredAgentSessionEventSink } from './structured-agent-session-event-sink' +import { agentJournalItemKey } from '../../../shared/agent-session-journal-item-key' import { createStructuredAgentSessionLogger } from './structured-agent-session-logger' +import { testEventSinkLogging } from './structured-agent-session-logger-test-support' const IDENTITY: AgentSessionJournalIdentity = { sessionId: 'session-1', @@ -58,7 +61,6 @@ describe('performCancel', () => { persistOptions: async () => undefined, resolvedBy: 'client-1', publish: vi.fn(), - flushStreamedEvents: async () => undefined, now: () => 1 } @@ -113,7 +115,6 @@ describe('performCancel', () => { persistOptions: async () => undefined, resolvedBy: 'client-1', publish: vi.fn(), - flushStreamedEvents: async () => undefined, now: () => 1 } @@ -161,7 +162,6 @@ describe('performCancel', () => { persistOptions: async () => undefined, resolvedBy: 'client-1', publish: vi.fn(), - flushStreamedEvents: async () => undefined, now: () => 1 } @@ -194,7 +194,6 @@ describe('performCancel', () => { persistOptions: async () => undefined, resolvedBy: 'client-1', publish: vi.fn(), - flushStreamedEvents: async () => undefined, now: () => 1 } @@ -233,7 +232,6 @@ describe('performCancel', () => { persistOptions: async () => undefined, resolvedBy: 'client-1', publish: vi.fn(), - flushStreamedEvents: async () => undefined, now: () => 1 } @@ -282,7 +280,7 @@ describe('what a conversation Stop reports when the provider stopped nothing', ( async function cancelWith( outcome: Awaited>, input: { turnId?: string; withdrewQueued?: boolean }, - turnRow: 'none' | 'running' | 'lands-on-flush' = 'none' + turnRow: 'none' | 'running' | 'landing' = 'none' ) { root = await mkdtemp(join(tmpdir(), 'orca-turn-cancel-report-')) const journal = await journals.open({ identity: IDENTITY, stateDirectory: root }) @@ -304,6 +302,8 @@ describe('what a conversation Stop reports when the provider stopped nothing', ( if (turnRow === 'running') { await openTurn() } + // Issued, not yet landed: the Stop's read takes its place behind it in the journal's queue. + const landing = turnRow === 'landing' ? openTurn() : null const ctx: AgentSessionTurnContext = { logger: createStructuredAgentSessionLogger(), sessionId: 'session-1', @@ -320,14 +320,15 @@ describe('what a conversation Stop reports when the provider stopped nothing', ( persistOptions: async () => undefined, resolvedBy: 'client-1', publish: vi.fn(), - flushStreamedEvents: async () => { - if (turnRow === 'lands-on-flush') { - await openTurn() - } - }, now: () => 1 } - const result = await performCancel(ctx, { clientOperationId: 'cancel-report-1', ...input }) + const { withdrewQueued, ...named } = input + const result = await performCancel(ctx, { + clientOperationId: 'cancel-report-1', + ...named, + ...(withdrewQueued === undefined ? {} : { withdrewQueued: Promise.resolve(withdrewQueued) }) + }) + await landing const rows = journal .snapshot() .items.flatMap((item) => @@ -350,12 +351,70 @@ describe('what a conversation Stop reports when the provider stopped nothing', ( expect(reported.rows).not.toContain('The provider had already finished this turn.') }) - it('reads the journal after its streamed rows land: a turn whose send was accepted first is still working', async () => { + it('reads the journal behind its streamed rows: a turn whose send was accepted first is still working', async () => { const reported = await cancelWith( { cancelled: false }, { turnId: 'turn-0', withdrewQueued: true }, - 'lands-on-flush' + 'landing' ) expect(reported).toEqual({ cancelled: false, rows: [] }) }) }) + +describe('the note a Stop writes', () => { + it('belongs to the turn whose row was emitted just before the Stop, with no flush', async () => { + root = await mkdtemp(join(tmpdir(), 'orca-turn-cancel-note-scope-')) + const journal = await journals.open({ identity: IDENTITY, stateDirectory: root }) + const deferred = createDeferredStructuredAgentSessionEventSink(testEventSinkLogging()) + deferred.bind({ journal, fence: 1, publish: () => {} }) + const turnIdentity = { + provider: 'legacy' as const, + agent: 'codex' as const, + sessionId: 'session-1', + recordId: 'turn-lifecycle:turn-1' + } + // An earlier streamed row still ahead, then the turn row the Stop names. + deferred.sink.appendItem( + { provider: 'codex', threadId: 'thread-1', turnId: 'turn-0', ordinal: 0 }, + { kind: 'message', role: 'assistant', blocks: [{ type: 'text', text: 'Earlier.' }] }, + { turnScope: AGENT_JOURNAL_THREAD_SCOPE } + ) + deferred.sink.appendItem( + turnIdentity, + { + kind: 'status', + text: 'Agent is working…', + turnLifecycle: { turnId: 'turn-1', state: 'running' } + }, + { turnScope: AGENT_JOURNAL_THREAD_SCOPE } + ) + const ctx: AgentSessionTurnContext = { + sessionId: 'session-1', + journal, + fence: 1, + adapter: { + acquire: vi.fn(), + dispatch: vi.fn(), + closeSession: vi.fn(), + cancelTurn: vi.fn(async () => ({ cancelled: true })), + answerPrompt: vi.fn(), + setOption: vi.fn() + }, + persistOptions: async () => undefined, + resolvedBy: 'client-1', + publish: vi.fn(), + logger: createStructuredAgentSessionLogger(), + now: () => 1 + } + + await performCancel(ctx, { clientOperationId: 'cancel-scope-1', turnId: 'turn-1' }) + await deferred.drained() + + const note = journal + .snapshot() + .items.find( + (item) => item.body.kind === 'status' && item.body.text === 'Cancellation requested.' + ) + expect(note?.turnScope).toEqual({ kind: 'turn', turnItemId: agentJournalItemKey(turnIdentity) }) + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts index 6ee38fa1d62a..fa600a229dcf 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts @@ -59,8 +59,6 @@ export type AgentSessionTurnContext = { /** Republishes state kept outside the journal, such as the record's options or rewind phase. * Journal appends reach readers on their own. */ publish: () => void - /** Drains provider lifecycle already accepted by the execution host. */ - flushStreamedEvents: () => Promise /** What the host holds about the child this dispatch is for, read at the moment it is needed. */ providerChildPhase?: () => StructuredAgentSessionProviderChildPhase | undefined /** Who a Stop's refusal row names. */ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts index 419f7005c40c..232b101e6035 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts @@ -89,8 +89,8 @@ export async function settleUnexpectedStructuredAgentSessionExit< } const stableSettlementId = providerExitSettlementId(unexpectedEvent) - const unfinishedWork = captureUnfinishedStructuredAgentSessionWork(session.journal) try { + // The exited child's own writes land first: its dead generation is settled from all of them. try { const barrier = await context.flushLifecycle(unexpectedEvent.sessionId) if (!barrier.ok) { @@ -99,6 +99,7 @@ export async function settleUnexpectedStructuredAgentSessionExit< } catch (error) { logExitFailure(context, unexpectedEvent, 'exit-lifecycle-barrier', error) } + const unfinishedWork = captureUnfinishedStructuredAgentSessionWork(session.journal) await retryUnexpectedExitSettlement({ context, event: unexpectedEvent, diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-command-controller.ts b/src/main/native-chat/agent-session-wire/structured-conversation-command-controller.ts index fae3c944ee26..3c7f71dbe1f9 100644 --- a/src/main/native-chat/agent-session-wire/structured-conversation-command-controller.ts +++ b/src/main/native-chat/agent-session-wire/structured-conversation-command-controller.ts @@ -15,10 +15,7 @@ export class StructuredConversationCommandController { readonly pending = new Map() constructor( private readonly context: () => StructuredAgentSessionMutationContext, - private readonly host: Pick< - StructuredAgentSessionHost, - 'flushStreamedEvents' | 'waitForSendSettlement' - > + private readonly host: Pick ) {} send = ( caller: StructuredAgentSessionCaller, @@ -40,16 +37,14 @@ export class StructuredConversationCommandController { const entry = pending ?? { key, count: 0 } entry.count++ this.pending.set(params.envelope.sessionId, entry) - return runStructuredConversationCommand(this.context(), this.host, caller, params).finally( - () => { - if (--entry.count === 0 && this.pending.get(params.envelope.sessionId) === entry) { - this.pending.delete(params.envelope.sessionId) - } - // A clear can settle with no journal commit (a refusal), and drafts held behind it - // would otherwise wait for an unrelated commit. - this.context().wakeQueuedDrain?.(params.envelope.sessionId) + return runStructuredConversationCommand(this.context(), caller, params).finally(() => { + if (--entry.count === 0 && this.pending.get(params.envelope.sessionId) === entry) { + this.pending.delete(params.envelope.sessionId) } - ) + // A clear can settle with no journal commit (a refusal), and drafts held behind it + // would otherwise wait for an unrelated commit. + this.context().wakeQueuedDrain?.(params.envelope.sessionId) + }) } replacements = () => { diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-command.ts b/src/main/native-chat/agent-session-wire/structured-conversation-command.ts index 7518220a3b31..73093da47447 100644 --- a/src/main/native-chat/agent-session-wire/structured-conversation-command.ts +++ b/src/main/native-chat/agent-session-wire/structured-conversation-command.ts @@ -11,7 +11,6 @@ import { admitAndRunAgentSessionMutation } from './structured-agent-session-muta import type { StructuredAgentSessionMutationContext } from './structured-agent-session-host-mutations' import { sendPreparation } from './structured-agent-session-send-preparation' import type { StructuredAgentSessionCaller } from './structured-agent-session-host-types' -import type { StructuredAgentSessionHost } from './structured-agent-session-host' import { committedClearOfCaller, conversationCommandBlocked @@ -86,7 +85,6 @@ async function answerFromCommittedClear( */ export function runStructuredConversationCommand( context: StructuredAgentSessionMutationContext, - host: Pick, caller: StructuredAgentSessionCaller, params: ConversationCommandParams ): Promise> { @@ -114,7 +112,6 @@ export function runStructuredConversationCommand( prepareSession: sendPreparation(context, envelope), journal: () => context.sessions.get(sessionId)?.journal, publish: (journal) => context.publish(sessionId, journal), - flushStreamedEvents: context.flushStreamedEvents, now: context.now, plan: { method: 'agentSession.conversationCommand', @@ -133,7 +130,6 @@ export function runStructuredConversationCommand( // The commit is the only write, so a clear with no committed answer changed nothing. rerunWhenReplayMissing: () => true, run: async (ctx) => { - await host.flushStreamedEvents(sessionId) const record = store.getRecord(sessionId)! const blocked = conversationCommandBlocked( ctx, diff --git a/src/main/runtime/structured-agent-session-codex-settlement-order.test.ts b/src/main/runtime/structured-agent-session-codex-settlement-order.test.ts new file mode 100644 index 000000000000..47a8c609db20 --- /dev/null +++ b/src/main/runtime/structured-agent-session-codex-settlement-order.test.ts @@ -0,0 +1,217 @@ +// Codex's own frames go through the event sink; the settlements they prove (a failed turn's +// rejection, an echo's acceptance) are written straight to the journal. Each settlement must land +// after the frame that proved it, even with an earlier streamed write issued in the same read. +// Driven through the shipped host, journal and Codex adapter; only the Codex child is fake. + +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { + CodexAppServerConnection, + CodexAppServerConnectionHandlers, + openCodexAppServerConnection +} from '../codex/codex-app-server-connection' +import { codexTurnLifecycleFake } from '../codex/codex-turn-lifecycle-fake' +import { computeAgentSessionPayloadFingerprint } from '../../shared/agent-session-mutation-envelope' +import type { + AgentJournalDispatchState, + AgentJournalTurnItem +} from '../../shared/agent-session-journal-types' +import { + HOST_TEST_SESSION as SESSION, + HOST_TEST_THREAD as THREAD, + hostTestAttachParams, + hostTestMessage +} from '../native-chat/agent-session-wire/structured-agent-session-host-test-data' +import type { StructuredAgentSessionHost } from '../native-chat/agent-session-wire/structured-agent-session-host' +import { + liveTestJournalRows, + openTestJournalHostDatabase +} from '../native-chat/agent-session-journal/journal-host-database-test-support' +import { + parseJournalRow, + type JournalLifecycleMutation, + type JournalRow +} from '../native-chat/agent-session-journal/journal-row-schema' +import { createStructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger' +import { + ensureStructuredAgentSessionHost, + stopStructuredAgentSessionRuntime +} from './structured-agent-session-runtime' + +const CALLER = { callerKey: 'codex-settlement-order-test' } +const MODEL = { + model: 'gpt-test', + displayName: 'GPT Test', + hidden: false, + supportedReasoningEfforts: [], + defaultReasoningEffort: null, + isDefault: true +} + +let root: string +let host: StructuredAgentSessionHost +let fence: number +let handlers: CodexAppServerConnectionHandlers | undefined +let answers: number +let turns: ReturnType +let operations = 0 + +/** The durable ledger stamps its own clock and refuses an id far from it. */ +const operationId = (): string => `${Date.now()}-${(++operations).toString(16).padStart(32, '0')}` + +function envelope(method: string, fields: Record) { + return { + sessionId: SESSION, + clientOperationId: operationId(), + expectedRuntimeFence: fence, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method, + sessionId: SESSION, + fields + }) + } +} + +async function send(text: string): Promise { + const body = hostTestMessage(text) + const sent = await host.send(CALLER, { envelope: envelope('agentSession.send', { body }), body }) + if (!sent.ok) { + throw new Error(JSON.stringify(sent.refusal)) + } + return sent.value.clientMessageId +} + +/** A stored row, or one mutation of a lifecycle batch: what a settlement or a frame lands as. */ +type JournalEntry = JournalRow | JournalLifecycleMutation + +/** The sequence of the first row that `matches`, or of the lifecycle batch carrying it. */ +function seqOf(matches: (entry: JournalEntry) => boolean): number | undefined { + return liveTestJournalRows(openTestJournalHostDatabase(root).db, SESSION).find((stored) => { + const parsed = parseJournalRow(stored.rowJson) + if (!parsed.ok) { + return false + } + const { row } = parsed + return ( + matches(row) || + (row.kind === 'lifecycle-batch' && row.mutations.some((mutation) => matches(mutation))) + ) + })?.seq +} + +const dispatchRow = + (clientMessageId: string, state: AgentJournalDispatchState) => (entry: JournalEntry) => + entry.kind === 'dispatch' && entry.clientMessageId === clientMessageId && entry.state === state + +/** The turn lifecycle an item row or mutation carries, if it carries one. */ +function turnOf(entry: JournalEntry): AgentJournalTurnItem | null { + return entry.kind === 'item' && entry.body.kind === 'turn' ? entry.body : null +} + +/** An assistant reply Codex finishes in the same read as the frame under test. */ +function streamedReplyAhead(): void { + handlers?.onNotification?.('item/completed', { + threadId: THREAD, + turn: { id: 'turn-1' }, + item: { type: 'agentMessage', id: 'item-reply', text: 'Done looking.' } + }) +} + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-codex-settlement-order-')) + answers = 0 + turns = codexTurnLifecycleFake( + THREAD, + () => (method, params) => handlers?.onNotification?.(method, params) + ) + const openConnection: typeof openCodexAppServerConnection = async ( + _launch, + connectionHandlers = {} + ) => { + handlers = connectionHandlers + const connection: CodexAppServerConnection = { + pid: 4321, + closed: false, + request: async (method) => { + if (method === 'thread/start' || method === 'thread/resume') { + return { thread: { id: THREAD } } + } + if (method === 'model/list') { + return { data: [MODEL], nextCursor: null } + } + if (method === 'turn/start') { + answers += 1 + return turns.routes['turn/start']() + } + return {} + }, + notify: () => {}, + respond: () => {}, + respondWithError: () => {}, + close: async () => true + } + return connection + } + host = await ensureStructuredAgentSessionHost({ + logger: createStructuredAgentSessionLogger(), + stateDirectory: root, + hostId: 'local', + claimKeyId: 'key-1', + resolveWorkspacePath: async () => root, + resolveClaudeAuthPolicy: () => ({ stripAuthEnv: true }), + resolveCodexCommand: () => 'codex', + resolveEnvironment: async () => ({ PATH: process.env.PATH }), + openCodexConnection: openConnection, + readProcessStartTime: async () => 1_700_000_000_000 + }) + const attachParams = hostTestAttachParams(null, { providerHandle: undefined }) + attachParams.envelope.clientOperationId = operationId() + const attached = await host.attach(CALLER, attachParams) + if (!attached.ok) { + throw new Error(JSON.stringify(attached.refusal)) + } + fence = attached.value.fence +}) + +afterEach(async () => { + await stopStructuredAgentSessionRuntime() + await rm(root, { recursive: true, force: true }) +}) + +describe('a settlement Codex proves keeps its place behind the frame that proved it', () => { + it('rejects a send only after the failed turn that refused it has ended', async () => { + const sent = await send('look around') + await vi.waitFor(() => expect(answers).toBe(1)) + turns.start() + + streamedReplyAhead() + turns.end('failed', 'The model is overloaded.') + + await vi.waitFor(() => expect(seqOf(dispatchRow(sent, 'rejected'))).toBeDefined()) + await host.flushStreamedEvents(SESSION) + const ended = seqOf((entry) => { + const turn = turnOf(entry) + return turn !== null && turn.state !== 'running' + }) + expect(ended).toBeDefined() + expect(seqOf(dispatchRow(sent, 'rejected'))).toBeGreaterThan(ended!) + }) + + it('accepts a send only after the echo that proves it has landed', async () => { + const sent = await send('look around') + await vi.waitFor(() => expect(answers).toBe(1)) + turns.start() + + streamedReplyAhead() + turns.echo(sent) + + await vi.waitFor(() => expect(seqOf(dispatchRow(sent, 'accepted'))).toBeDefined()) + await host.flushStreamedEvents(SESSION) + // The echo reconciles into the send's own bubble: its row is the turn now naming that send. + const echoed = seqOf((entry) => turnOf(entry)?.userItemId === `orca:${sent}`) + expect(echoed).toBeDefined() + expect(seqOf(dispatchRow(sent, 'accepted'))).toBeGreaterThan(echoed!) + }) +})