diff --git a/mobile/src/transport/mobile-runtime-client-capabilities.ts b/mobile/src/transport/mobile-runtime-client-capabilities.ts index b7699fae168..30b627a9ef3 100644 --- a/mobile/src/transport/mobile-runtime-client-capabilities.ts +++ b/mobile/src/transport/mobile-runtime-client-capabilities.ts @@ -1,4 +1,5 @@ import { + AGENT_SESSION_PENDING_SEND_RESULT_RUNTIME_CAPABILITY, AGENT_SESSION_TURN_ITEM_CAPABILITY, CLAUDE_STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY, @@ -8,6 +9,7 @@ import { remoteRuntimeClientCapabilities } from '../../../src/shared/remote-runt export const MOBILE_RUNTIME_CLIENT_CAPABILITIES = remoteRuntimeClientCapabilities([ STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, + AGENT_SESSION_PENDING_SEND_RESULT_RUNTIME_CAPABILITY, STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY, CLAUDE_STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, // Opts into the typed turn record; without it the host sends the legacy status carrier. diff --git a/mobile/src/transport/rpc-client-capabilities.test.ts b/mobile/src/transport/rpc-client-capabilities.test.ts index 41bb4091a0b..cc670ab8a41 100644 --- a/mobile/src/transport/rpc-client-capabilities.test.ts +++ b/mobile/src/transport/rpc-client-capabilities.test.ts @@ -92,6 +92,7 @@ describe('mobile rpc-client capabilities', () => { expect(capabilityRequest.params).toMatchObject({ clientCapabilities: expect.arrayContaining([ 'agent-session.structured.v1', + 'agent-session.pending-send-result.v1', 'agent-session.structured.claude.v1' ]) }) diff --git a/src/main/claude/claude-agent-sdk-user-message-queue.test.ts b/src/main/claude/claude-agent-sdk-user-message-queue.test.ts index 62fbd8cf203..2270b0627fd 100644 --- a/src/main/claude/claude-agent-sdk-user-message-queue.test.ts +++ b/src/main/claude/claude-agent-sdk-user-message-queue.test.ts @@ -1,6 +1,9 @@ import type { SDKUserMessage } from '@anthropic-ai/claude-agent-sdk' import { describe, expect, it } from 'vitest' -import { createClaudeUserMessageQueue } from './claude-agent-sdk-user-message-queue' +import { + claudeUserMessageWasProvablyUnwritten, + createClaudeUserMessageQueue +} from './claude-agent-sdk-user-message-queue' /** * The SDK's input pump is `for await (const frame of prompt) { await transport.write(frame) }`. @@ -24,7 +27,7 @@ const settled = (promise: Promise): Promise<'settled' | 'pending'> => ]) describe('claude user message queue', () => { - it('rejects the frame the SDK pulled but abandoned without writing', async () => { + it('treats a frame the SDK pulled and abandoned as write-outcome unknown', async () => { const queue = createClaudeUserMessageQueue() const pump = queue.messages[Symbol.asyncIterator]() const sent = queue.push(frame('hello')) @@ -33,9 +36,11 @@ describe('claude user message queue', () => { await pump.return?.(undefined) await expect(settled(sent)).resolves.toBe('settled') - await expect(sent).rejects.toThrow( - 'claude stream-json input ended before the frame was written' - ) + const error = await sent.catch((caught: unknown) => caught) + expect(error).toMatchObject({ + message: 'claude stream-json input ended before confirming the frame write' + }) + expect(claudeUserMessageWasProvablyUnwritten(error)).toBe(false) }) it('rejects an in-flight frame from fail() when the SDK never resumes the pump', async () => { @@ -47,7 +52,22 @@ describe('claude user message queue', () => { queue.fail(new Error('claude stream-json exited: child died')) await expect(settled(sent)).resolves.toBe('settled') - await expect(sent).rejects.toThrow('claude stream-json exited: child died') + const error = await sent.catch((caught: unknown) => caught) + expect(error).toMatchObject({ message: 'claude stream-json exited: child died' }) + expect(claudeUserMessageWasProvablyUnwritten(error)).toBe(false) + }) + + it('marks only frames still queued in Orca as provably unwritten', async () => { + const queue = createClaudeUserMessageQueue() + const pump = queue.messages[Symbol.asyncIterator]() + const inFlight = queue.push(frame('first')).catch((caught: unknown) => caught) + await pump.next() + const queued = queue.push(frame('second')).catch((caught: unknown) => caught) + + queue.fail(new Error('claude stream-json exited: child died')) + + expect(claudeUserMessageWasProvablyUnwritten(await inFlight)).toBe(false) + expect(claudeUserMessageWasProvablyUnwritten(await queued)).toBe(true) }) it('still settles a written frame only once the pump asks for the next one', async () => { diff --git a/src/main/claude/claude-agent-sdk-user-message-queue.ts b/src/main/claude/claude-agent-sdk-user-message-queue.ts index 87fa6660159..3cd0ad7a6f9 100644 --- a/src/main/claude/claude-agent-sdk-user-message-queue.ts +++ b/src/main/claude/claude-agent-sdk-user-message-queue.ts @@ -6,18 +6,42 @@ type QueuedMessage = { reject: (error: Error) => void } +type ClaudeUserMessageFailureDisposition = 'unwritten' | 'write-outcome-unknown' + +class ClaudeUserMessageFailure extends Error { + readonly disposition: ClaudeUserMessageFailureDisposition + + constructor(disposition: ClaudeUserMessageFailureDisposition, cause: Error) { + super(cause.message, { cause }) + this.name = 'ClaudeUserMessageFailure' + this.disposition = disposition + } +} + +export function claudeUnwrittenUserMessageError(cause: Error): Error { + return new ClaudeUserMessageFailure('unwritten', cause) +} + +export function claudeUserMessageWasProvablyUnwritten(error: unknown): boolean { + return error instanceof ClaudeUserMessageFailure && error.disposition === 'unwritten' +} + +function claudeAmbiguousUserMessageError(cause: Error): Error { + return new ClaudeUserMessageFailure('write-outcome-unknown', cause) +} + export type ClaudeUserMessageQueue = { /** The SDK's streaming-input prompt; it stays open until `end`. */ messages: AsyncIterable /** Resolves once the SDK has finished writing the frame to the child. */ push: (message: SDKUserMessage) => Promise - /** Reject every unwritten frame, in-flight included; a caller waiting on a send must not hang past the exit. */ + /** Reject every unsettled frame; an in-flight frame carries an ambiguous write outcome. */ fail: (error: Error) => void end: () => void } /** The rejection an abandoned frame carries when nothing else has named a cause yet. */ -const UNWRITTEN_FRAME_MESSAGE = 'claude stream-json input ended before the frame was written' +const UNCONFIRMED_FRAME_MESSAGE = 'claude stream-json input ended before confirming the frame write' export function createClaudeUserMessageQueue(): ClaudeUserMessageQueue { const queued: QueuedMessage[] = [] @@ -57,7 +81,9 @@ export function createClaudeUserMessageQueue(): ClaudeUserMessageQueue { // is the same "the frame reached the child" proof the hand-rolled write gave. next.resolve() } else { - rejectInFlight(failure ?? new Error(UNWRITTEN_FRAME_MESSAGE)) + rejectInFlight( + claudeAmbiguousUserMessageError(failure ?? new Error(UNCONFIRMED_FRAME_MESSAGE)) + ) } } continue @@ -76,7 +102,7 @@ export function createClaudeUserMessageQueue(): ClaudeUserMessageQueue { push: (message) => new Promise((resolve, reject) => { if (failure) { - reject(failure) + reject(claudeUnwrittenUserMessageError(failure)) return } queued.push({ message, resolve, reject }) @@ -85,11 +111,11 @@ export function createClaudeUserMessageQueue(): ClaudeUserMessageQueue { fail: (error) => { failure ??= error for (const entry of queued.splice(0)) { - entry.reject(error) + entry.reject(claudeUnwrittenUserMessageError(error)) } // A pump that never resumes cannot run the generator's cleanup, so the // exit path has to reach the in-flight frame itself. - rejectInFlight(error) + rejectInFlight(claudeAmbiguousUserMessageError(error)) notify() }, end: () => { diff --git a/src/main/claude/claude-stream-json-connection.ts b/src/main/claude/claude-stream-json-connection.ts index dd6bbc8a5eb..1a99bb92064 100644 --- a/src/main/claude/claude-stream-json-connection.ts +++ b/src/main/claude/claude-stream-json-connection.ts @@ -15,7 +15,10 @@ import { import { createClaudeChildTreeReaper, proveClaudeChildExit } from './claude-agent-sdk-exit-proof' import type { DescendantTreeVerdict } from '../pty-descendant-exit-verification' import { createClaudeCodeProcessSpawn } from './claude-agent-sdk-process-spawn' -import { createClaudeUserMessageQueue } from './claude-agent-sdk-user-message-queue' +import { + claudeUnwrittenUserMessageError, + createClaudeUserMessageQueue +} from './claude-agent-sdk-user-message-queue' import type { ClaudeStructuredSdkOptions } from './claude-structured-launch-resolution' export { ClaudeControlRequestError } @@ -236,7 +239,11 @@ export async function openClaudeStreamJsonConnection( const send = (message: Record): Promise => { if (closing || exited || terminalError || child.stdin.destroyed || !child.stdin.writable) { - return Promise.reject(terminalError ?? new Error('claude stream-json connection is closed')) + return Promise.reject( + claudeUnwrittenUserMessageError( + terminalError ?? new Error('claude stream-json connection is closed') + ) + ) } return inbox.push(message as unknown as SDKUserMessage) } diff --git a/src/main/claude/claude-structured-compaction.test.ts b/src/main/claude/claude-structured-compaction.test.ts index 46dac01f768..5255ce37ea1 100644 --- a/src/main/claude/claude-structured-compaction.test.ts +++ b/src/main/claude/claude-structured-compaction.test.ts @@ -1,6 +1,12 @@ -import { describe, expect, it } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { StructuredSessionCompaction } from '../native-chat/agent-session-wire/structured-session-compaction' -import { isClaudeCompactionContent } from './claude-structured-compaction' +import { claudeUnwrittenUserMessageError } from './claude-agent-sdk-user-message-queue' +import { compactClaudeSession, isClaudeCompactionContent } from './claude-structured-compaction' +import { sessionFor } from './claude-structured-dispatch-test-support' + +afterEach(() => { + vi.useRealTimers() +}) describe('Claude compaction transcript content', () => { it('keeps generated summaries and command echoes out of the transcript only during explicit compaction', async () => { @@ -26,4 +32,35 @@ describe('Claude compaction transcript content', () => { await completion expect(isClaudeCompactionContent(tracker, event)).toBe(false) }) + + it('fails a provably unwritten command without waiting for the completion deadline', async () => { + vi.useFakeTimers() + const session = sessionFor( + vi.fn().mockRejectedValue(claudeUnwrittenUserMessageError(new Error('input closed'))) + ) + const pending = compactClaudeSession(session, new StructuredSessionCompaction(60_000), { + sessionId: 'orca-session', + fence: 1, + turnId: 'compact-1' + }) + + await vi.advanceTimersByTimeAsync(1) + + await expect(pending).resolves.toEqual({ error: 'provider_write_failed: input closed' }) + }) + + it('keeps waiting when the command write outcome is ambiguous', async () => { + vi.useFakeTimers() + const session = sessionFor(vi.fn().mockRejectedValue(new Error('input pump stopped'))) + const pending = compactClaudeSession(session, new StructuredSessionCompaction(10), { + sessionId: 'orca-session', + fence: 1, + turnId: 'compact-1' + }) + const rejection = expect(pending).rejects.toThrow('Compaction completion is unconfirmed.') + + await vi.advanceTimersByTimeAsync(10) + + await rejection + }) }) diff --git a/src/main/claude/claude-structured-compaction.ts b/src/main/claude/claude-structured-compaction.ts index a5bd3aa96e3..cda4652b6ca 100644 --- a/src/main/claude/claude-structured-compaction.ts +++ b/src/main/claude/claude-structured-compaction.ts @@ -2,25 +2,26 @@ import type { ClaudeSession, ClaudeStructuredSessionEvent } from './claude-struc import type { StructuredSessionCompaction } from '../native-chat/agent-session-wire/structured-session-compaction' import { dispatchClaudeTurn } from './claude-structured-dispatch' import type { StructuredAgentSessionAdapter } from '../native-chat/agent-session-wire/structured-agent-session-adapter' +import { dispatchDoubtProvesUndelivered } from '../native-chat/agent-session-journal/journal-dispatch-doubt-reasons' +/** Compaction needs no ack deadline of its own: `compactions.run` keeps its own + * 180s completion window and settles on Claude's terminal `result` frame, so + * the dispatch here only has to report a refusal to send. */ export function compactClaudeSession( session: ClaudeSession, compactions: StructuredSessionCompaction, - input: Parameters>[0], - timeoutMs: number + input: Parameters>[0] ): Promise<{ error?: string }> { return compactions.run( input.sessionId, session.providerSessionId, async () => { - const result = await dispatchClaudeTurn( - session, - { - clientMessageId: `compact-${input.fence}`, - body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: '/compact' }] } - }, - timeoutMs - ) - if (result.state === 'rejected') { + const result = await dispatchClaudeTurn(session, { + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: '/compact' }] } + }) + if ( + result.state === 'rejected' || + (result.state === 'unknown' && dispatchDoubtProvesUndelivered(result.reason)) + ) { return { error: result.reason } } return undefined diff --git a/src/main/claude/claude-structured-dispatch-admission.test.ts b/src/main/claude/claude-structured-dispatch-admission.test.ts new file mode 100644 index 00000000000..6ed8f91f072 --- /dev/null +++ b/src/main/claude/claude-structured-dispatch-admission.test.ts @@ -0,0 +1,123 @@ +// The contract the admission fix exists for: dispatch settles when the write +// completes, and nothing about elapsed time ever puts a message in doubt. + +import { describe, expect, it, vi } from 'vitest' +import { dispatchClaudeTurn, resolveClaudeReplayWaiter } from './claude-structured-dispatch' +import { + childExited, + sessionFor, + userMessage, + userReplayFrame +} from './claude-structured-dispatch-test-support' + +describe('Claude structured dispatch admission', () => { + it('settles a send queued behind a running turn when that turn starts, with no doubt in between', async () => { + vi.useFakeTimers() + try { + const session = sessionFor() + const settled = vi.fn() + const running = await dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: 'one' }]) + }) + const runningUuid = session.dispatchWaiters[0]!.sentUuid + expect(resolveClaudeReplayWaiter(session, userReplayFrame(runningUuid, 'one'), settled)).toBe( + true + ) + + // Queued while turn one is still running: Claude cannot echo it until that + // turn ends, so nothing about the wait is evidence of a delivery problem. + const queued = await dispatchClaudeTurn(session, { + clientMessageId: 'client-2', + body: userMessage([{ type: 'text', text: 'two' }]) + }) + const queuedUuid = session.dispatchWaiters[0]!.sentUuid + expect(running).toEqual({ state: 'admitted' }) + expect(queued).toEqual({ state: 'admitted' }) + + await vi.advanceTimersByTimeAsync(10 * 60_000) + expect(session.dispatchWaiters).toHaveLength(1) + expect(session.retiredDispatchWaiters).toHaveLength(0) + expect(settled).toHaveBeenCalledTimes(1) + + // Turn one ends and turn two starts: the echo lands and settles the send. + expect(resolveClaudeReplayWaiter(session, userReplayFrame(queuedUuid, 'two'), settled)).toBe( + true + ) + expect(settled).toHaveBeenLastCalledWith({ + clientMessageId: 'client-2', + providerIdentity: { provider: 'claude', sessionId: 'provider-session', uuid: queuedUuid } + }) + expect(session.activeTurnId).toBe(queuedUuid) + } finally { + vi.useRealTimers() + } + }) + + it('returns as soon as the write completes, without awaiting the echo', async () => { + const session = sessionFor() + await expect( + dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: 'one' }]) + }) + ).resolves.toEqual({ state: 'admitted' }) + expect(session.connection.send).toHaveBeenCalledTimes(1) + // Still unacknowledged, and deliberately so: the waiter outlives the call. + expect(session.dispatchWaiters).toHaveLength(1) + expect(session.dispatchWaiters[0]!.settledUuid).toBeUndefined() + }) + + it('resolves every live waiter and retires it when the child exits', async () => { + const session = sessionFor() + await dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: 'one' }]) + }) + await dispatchClaudeTurn(session, { + clientMessageId: 'client-2', + body: userMessage([{ type: 'text', text: 'two' }]) + }) + expect(session.dispatchWaiters).toHaveLength(2) + + childExited(session) + + expect(session.dispatchWaiters).toHaveLength(0) + expect(session.retiredDispatchWaiters).toHaveLength(2) + expect(session.retiredDispatchWaiters.every((waiter) => waiter.retired === true)).toBe(true) + }) + + it('bounds pending replay identities instead of retaining an unbounded queue', async () => { + const session = sessionFor() + for (let index = 0; index < 64; index += 1) { + await expect( + dispatchClaudeTurn(session, { + clientMessageId: `client-${index}`, + body: userMessage([{ type: 'text', text: String(index) }]) + }) + ).resolves.toEqual({ state: 'admitted' }) + } + + await expect( + dispatchClaudeTurn(session, { + clientMessageId: 'client-over-capacity', + body: userMessage([{ type: 'text', text: 'one too many' }]) + }) + ).resolves.toEqual({ state: 'rejected', reason: 'claude structured dispatch queue is full' }) + expect(session.dispatchWaiters).toHaveLength(64) + expect(session.connection.send).toHaveBeenCalledTimes(64) + }) + + it('does not publish a journal settlement for a provider-control turn', async () => { + const session = sessionFor() + const settled = vi.fn() + await dispatchClaudeTurn(session, { + body: userMessage([{ type: 'text', text: '/compact' }]) + }) + const uuid = session.dispatchWaiters[0]!.sentUuid + + resolveClaudeReplayWaiter(session, userReplayFrame(uuid, '/compact'), settled) + + expect(settled).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/claude/claude-structured-dispatch-test-support.ts b/src/main/claude/claude-structured-dispatch-test-support.ts new file mode 100644 index 00000000000..83971a38d13 --- /dev/null +++ b/src/main/claude/claude-structured-dispatch-test-support.ts @@ -0,0 +1,52 @@ +import { vi, type Mock } from 'vitest' +import type { AgentJournalMessageItem } from '../../shared/agent-session-journal-types' +import { retireClaudeDispatchWaiters } from './claude-structured-dispatch' +import type { ClaudeSession } from './claude-structured-session-state' +import { ClaudeBackgroundTaskTracker } from './claude-background-task-tracker' +import { ClaudeSlashCommandCatalog } from './claude-slash-command-catalog' + +export function sessionFor(send: Mock = vi.fn().mockResolvedValue(undefined)): ClaudeSession { + return { + connection: { send } as unknown as ClaudeSession['connection'], + providerSessionId: 'provider-session', + claudeConfigDir: '/accounts/claude', + leafUuid: null, + fence: 1, + acquisitionGeneration: 'generation-1', + prompts: {} as ClaudeSession['prompts'], + dispatchWaiters: [], + retiredDispatchWaiters: [], + replayContentFallbackBlocked: false, + backgroundTasks: new ClaudeBackgroundTaskTracker(), + commands: new ClaudeSlashCommandCatalog(), + dispatchSequence: 0, + optionMutationSequence: 0, + options: new Map(), + reportedOptions: {}, + reportedModelMutation: 0, + confirmedOptions: new Set(), + restoreSkippedOptions: new Set(), + capabilities: [], + events: undefined, + translator: null + } +} + +export function userMessage(blocks: AgentJournalMessageItem['blocks']): AgentJournalMessageItem { + return { kind: 'message', role: 'user', blocks } +} + +/** The child died. Nothing else retires a live waiter now that no deadline does. */ +export function childExited(session: ClaudeSession): void { + retireClaudeDispatchWaiters(session) +} + +export function userReplayFrame(uuid: string, text: string): Record { + return { + type: 'user', + parent_tool_use_id: null, + session_id: 'provider-session', + uuid, + message: { role: 'user', content: [{ type: 'text', text }] } + } +} diff --git a/src/main/claude/claude-structured-dispatch.test.ts b/src/main/claude/claude-structured-dispatch.test.ts index 933bd77774b..6ea5832b25d 100644 --- a/src/main/claude/claude-structured-dispatch.test.ts +++ b/src/main/claude/claude-structured-dispatch.test.ts @@ -2,104 +2,86 @@ import { mkdtemp, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { describe, expect, it, vi } from 'vitest' -import type { AgentJournalMessageItem } from '../../shared/agent-session-journal-types' import { dispatchClaudeTurn, resolveClaudeReplayWaiter } from './claude-structured-dispatch' import { readClaudeImage } from './claude-structured-dispatch-content' +import { claudeUnwrittenUserMessageError } from './claude-agent-sdk-user-message-queue' import type { ClaudeSession } from './claude-structured-session-state' -import { ClaudeBackgroundTaskTracker } from './claude-background-task-tracker' -import { ClaudeSlashCommandCatalog } from './claude-slash-command-catalog' - -function sessionFor(send = vi.fn().mockResolvedValue(undefined)): ClaudeSession { - return { - connection: { send } as unknown as ClaudeSession['connection'], - providerSessionId: 'provider-session', - claudeConfigDir: '/accounts/claude', - leafUuid: null, - fence: 1, - acquisitionGeneration: 'generation-1', - prompts: {} as ClaudeSession['prompts'], - dispatchWaiters: [], - retiredDispatchWaiters: [], - replayContentFallbackBlocked: false, - backgroundTasks: new ClaudeBackgroundTaskTracker(), - commands: new ClaudeSlashCommandCatalog(), - dispatchSequence: 0, - optionMutationSequence: 0, - options: new Map(), - reportedOptions: {}, - reportedModelMutation: 0, - confirmedOptions: new Set(), - restoreSkippedOptions: new Set(), - capabilities: [], - events: undefined, - translator: null - } -} - -function userMessage(blocks: AgentJournalMessageItem['blocks']): AgentJournalMessageItem { - return { kind: 'message', role: 'user', blocks } -} - -function userReplayFrame(uuid: string, text: string): Record { - return { - type: 'user', - parent_tool_use_id: null, - session_id: 'provider-session', - uuid, - message: { role: 'user', content: [{ type: 'text', text }] } - } -} +import { + childExited, + sessionFor, + userMessage, + userReplayFrame +} from './claude-structured-dispatch-test-support' describe('Claude structured dispatch image limits', () => { it.each(['isMeta', 'isSynthetic', 'isCompactSummary'])( 'does not acknowledge a dispatch with %s context even when the client uuid matches', async (flag) => { const session = sessionFor() - const dispatched = dispatchClaudeTurn( - session, - { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: '/example' }]) }, - 1000 - ) + const settled = vi.fn() + const dispatched = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: '/example' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) const sentUuid = session.dispatchWaiters[0]!.sentUuid const replay = userReplayFrame(sentUuid, '/example') - expect(resolveClaudeReplayWaiter(session, { ...replay, [flag]: true })).toBe(false) + expect(resolveClaudeReplayWaiter(session, { ...replay, [flag]: true }, settled)).toBe(false) expect(session.dispatchWaiters).toHaveLength(1) - expect(resolveClaudeReplayWaiter(session, replay)).toBe(true) - await expect(dispatched).resolves.toMatchObject({ - state: 'accepted', - providerIdentity: { uuid: sentUuid } + expect(settled).not.toHaveBeenCalled() + expect(resolveClaudeReplayWaiter(session, replay, settled)).toBe(true) + await expect(dispatched).resolves.toEqual({ state: 'admitted' }) + expect(settled).toHaveBeenCalledWith({ + clientMessageId: 'client-1', + providerIdentity: { provider: 'claude', sessionId: 'provider-session', uuid: sentUuid } }) } ) - it('recovers the active identity when a timed-out replay arrives late', async () => { + it('takes the active turn identity from a replay that lands after dispatch returned', async () => { const session = sessionFor() - const dispatched = dispatchClaudeTurn( - session, - { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: 'one' }]) }, - 500 - ) + const dispatched = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: 'one' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) const sentUuid = (session.dispatchWaiters[0] as { sentUuid?: string }).sentUuid - await expect(dispatched).resolves.toMatchObject({ state: 'unknown' }) + await expect(dispatched).resolves.toEqual({ state: 'admitted' }) + expect(session.activeTurnId).toBeUndefined() expect(resolveClaudeReplayWaiter(session, userReplayFrame(sentUuid!, 'one'))).toBe(true) expect(session.activeTurnId).toBe(sentUuid) expect(session.activeTurnSequence).toBe(session.dispatchSequence) }) - it('settles the send a timed-out replay proves was delivered', async () => { + it('recovers the active identity when a replay lands after the child died', async () => { const session = sessionFor() - const settled = vi.fn() - const dispatched = dispatchClaudeTurn( - session, - { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: 'one' }]) }, - 500 - ) + const dispatched = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: 'one' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) const sentUuid = (session.dispatchWaiters[0] as { sentUuid?: string }).sentUuid - await expect(dispatched).resolves.toMatchObject({ state: 'unknown' }) + await expect(dispatched).resolves.toEqual({ state: 'admitted' }) + childExited(session) + expect(session.dispatchWaiters).toHaveLength(0) + expect(session.retiredDispatchWaiters).toHaveLength(1) + + expect(resolveClaudeReplayWaiter(session, userReplayFrame(sentUuid!, 'one'))).toBe(true) + expect(session.activeTurnId).toBe(sentUuid) + expect(session.activeTurnSequence).toBe(session.dispatchSequence) + }) + + it('settles the send the replay proves was delivered, whenever it arrives', async () => { + const session = sessionFor() + const settled = vi.fn() + const dispatched = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: 'one' }]) + }) + await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) + const sentUuid = (session.dispatchWaiters[0] as { sentUuid?: string }).sentUuid + await expect(dispatched).resolves.toEqual({ state: 'admitted' }) resolveClaudeReplayWaiter(session, userReplayFrame(sentUuid!, 'one'), settled) expect(settled).toHaveBeenCalledWith({ @@ -111,20 +93,19 @@ describe('Claude structured dispatch image limits', () => { it('settles a superseded dispatch even though it no longer owns the turn identity', async () => { const session = sessionFor() const settled = vi.fn() - const first = dispatchClaudeTurn( - session, - { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: 'one' }]) }, - 500 - ) + const first = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: 'one' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) const firstUuid = (session.dispatchWaiters[0] as { sentUuid?: string }).sentUuid - await expect(first).resolves.toMatchObject({ state: 'unknown' }) + await expect(first).resolves.toEqual({ state: 'admitted' }) + childExited(session) - const second = dispatchClaudeTurn( - session, - { clientMessageId: 'client-2', body: userMessage([{ type: 'text', text: 'two' }]) }, - 100 - ) + const second = dispatchClaudeTurn(session, { + clientMessageId: 'client-2', + body: userMessage([{ type: 'text', text: 'two' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) const secondUuid = (session.dispatchWaiters[0] as { sentUuid?: string }).sentUuid @@ -138,51 +119,62 @@ describe('Claude structured dispatch image limits', () => { clientMessageId: 'client-1', providerIdentity: { provider: 'claude', sessionId: 'provider-session', uuid: firstUuid } }) - resolveClaudeReplayWaiter(session, userReplayFrame(secondUuid!, 'two'), settled) - await expect(second).resolves.toMatchObject({ state: 'accepted' }) - expect(settled).toHaveBeenCalledTimes(1) + expect(resolveClaudeReplayWaiter(session, userReplayFrame(secondUuid!, 'two'), settled)).toBe( + true + ) + await expect(second).resolves.toEqual({ state: 'admitted' }) + expect(settled).toHaveBeenLastCalledWith({ + clientMessageId: 'client-2', + providerIdentity: { provider: 'claude', sessionId: 'provider-session', uuid: secondUuid } + }) }) it('never lets a late replay for dispatch A resolve dispatch B', async () => { const session = sessionFor() - const first = dispatchClaudeTurn( - session, - { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: 'one' }]) }, - 500 - ) + const settled = vi.fn() + const first = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: 'one' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) const firstUuid = (session.dispatchWaiters[0] as { sentUuid?: string }).sentUuid - await expect(first).resolves.toMatchObject({ state: 'unknown' }) + await expect(first).resolves.toEqual({ state: 'admitted' }) + childExited(session) - const second = dispatchClaudeTurn( - session, - { clientMessageId: 'client-2', body: userMessage([{ type: 'text', text: 'two' }]) }, - 100 - ) - await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) + await expect( + dispatchClaudeTurn(session, { + clientMessageId: 'client-2', + body: userMessage([{ type: 'text', text: 'two' }]) + }) + ).resolves.toEqual({ state: 'admitted' }) const secondUuid = (session.dispatchWaiters[0] as { sentUuid?: string }).sentUuid expect(resolveClaudeReplayWaiter(session, userReplayFrame(firstUuid!, 'one'))).toBe(false) expect(session.dispatchWaiters[0]).toMatchObject({ sentUuid: secondUuid }) - expect(resolveClaudeReplayWaiter(session, userReplayFrame(secondUuid!, 'two'))).toBe(true) - await expect(second).resolves.toMatchObject({ providerIdentity: { uuid: secondUuid } }) + expect(resolveClaudeReplayWaiter(session, userReplayFrame(secondUuid!, 'two'), settled)).toBe( + true + ) + expect(settled).toHaveBeenCalledWith({ + clientMessageId: 'client-2', + providerIdentity: { provider: 'claude', sessionId: 'provider-session', uuid: secondUuid } + }) }) it('does not let an identical late replay for dispatch A resolve active dispatch B', async () => { const session = sessionFor() - const first = dispatchClaudeTurn( - session, - { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: 'same prompt' }]) }, - 500 - ) + const settled = vi.fn() + const first = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: 'same prompt' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) - await expect(first).resolves.toMatchObject({ state: 'unknown' }) + await expect(first).resolves.toEqual({ state: 'admitted' }) + childExited(session) - const second = dispatchClaudeTurn( - session, - { clientMessageId: 'client-2', body: userMessage([{ type: 'text', text: 'same prompt' }]) }, - 100 - ) + const second = dispatchClaudeTurn(session, { + clientMessageId: 'client-2', + body: userMessage([{ type: 'text', text: 'same prompt' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) const secondUuid = session.dispatchWaiters[0]!.sentUuid @@ -191,34 +183,35 @@ describe('Claude structured dispatch image limits', () => { ) expect(session.dispatchWaiters[0]).toMatchObject({ sentUuid: secondUuid }) - resolveClaudeReplayWaiter(session, userReplayFrame(secondUuid, 'same prompt')) - await expect(second).resolves.toMatchObject({ providerIdentity: { uuid: secondUuid } }) + resolveClaudeReplayWaiter(session, userReplayFrame(secondUuid, 'same prompt'), settled) + await expect(second).resolves.toEqual({ state: 'admitted' }) + expect(settled).toHaveBeenCalledWith({ + clientMessageId: 'client-2', + providerIdentity: { provider: 'claude', sessionId: 'provider-session', uuid: secondUuid } + }) }) it('does not let a fresh-UUID replay for an evicted dispatch resolve active dispatch B', async () => { const session = sessionFor() - const first = dispatchClaudeTurn( - session, - { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: 'same prompt' }]) }, - 100 - ) + const first = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: 'same prompt' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) - await expect(first).resolves.toMatchObject({ state: 'unknown' }) + await expect(first).resolves.toEqual({ state: 'admitted' }) + childExited(session) const firstUuid = session.retiredDispatchWaiters[0]!.sentUuid const fillerDispatches = await Promise.all( Array.from({ length: 64 }, (_, index) => - dispatchClaudeTurn( - session, - { - clientMessageId: `filler-${index}`, - body: userMessage([{ type: 'text', text: 'same prompt' }]) - }, - 5 - ) + dispatchClaudeTurn(session, { + clientMessageId: `filler-${index}`, + body: userMessage([{ type: 'text', text: 'same prompt' }]) + }) ) ) - expect(fillerDispatches.every((outcome) => outcome.state === 'unknown')).toBe(true) + expect(fillerDispatches.every((outcome) => outcome.state === 'admitted')).toBe(true) + childExited(session) expect(session.retiredDispatchWaiters).toHaveLength(64) expect(session.replayContentFallbackBlocked).toBe(true) expect(session.retiredDispatchWaiters.some((waiter) => waiter.sentUuid === firstUuid)).toBe( @@ -231,47 +224,48 @@ describe('Claude structured dispatch image limits', () => { } expect(session.retiredDispatchWaiters).toHaveLength(0) - const second = dispatchClaudeTurn( - session, - { clientMessageId: 'client-2', body: userMessage([{ type: 'text', text: 'same prompt' }]) }, - 100 - ) + const second = dispatchClaudeTurn(session, { + clientMessageId: 'client-2', + body: userMessage([{ type: 'text', text: 'same prompt' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) const secondUuid = session.dispatchWaiters[0]!.sentUuid + const settled = vi.fn() expect( resolveClaudeReplayWaiter(session, userReplayFrame('provider-a-late', 'same prompt')) ).toBe(false) expect(session.dispatchWaiters[0]).toMatchObject({ sentUuid: secondUuid }) - resolveClaudeReplayWaiter(session, userReplayFrame(secondUuid, 'same prompt')) - await expect(second).resolves.toMatchObject({ providerIdentity: { uuid: secondUuid } }) + resolveClaudeReplayWaiter(session, userReplayFrame(secondUuid, 'same prompt'), settled) + await expect(second).resolves.toEqual({ state: 'admitted' }) + expect(settled).toHaveBeenCalledWith({ + clientMessageId: 'client-2', + providerIdentity: { provider: 'claude', sessionId: 'provider-session', uuid: secondUuid } + }) }) it('does not let a fresh-UUID result for an evicted slash dispatch resolve active dispatch B', async () => { const session = sessionFor() - const first = dispatchClaudeTurn( - session, - { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: '/permissions' }]) }, - 100 - ) + const first = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: '/permissions' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) - await expect(first).resolves.toMatchObject({ state: 'unknown' }) + await expect(first).resolves.toEqual({ state: 'admitted' }) + childExited(session) const firstUuid = session.retiredDispatchWaiters[0]!.sentUuid const fillerDispatches = await Promise.all( Array.from({ length: 64 }, (_, index) => - dispatchClaudeTurn( - session, - { - clientMessageId: `filler-${index}`, - body: userMessage([{ type: 'text', text: '/permissions' }]) - }, - 5 - ) + dispatchClaudeTurn(session, { + clientMessageId: `filler-${index}`, + body: userMessage([{ type: 'text', text: '/permissions' }]) + }) ) ) - expect(fillerDispatches.every((outcome) => outcome.state === 'unknown')).toBe(true) + expect(fillerDispatches.every((outcome) => outcome.state === 'admitted')).toBe(true) + childExited(session) expect(session.retiredDispatchWaiters).toHaveLength(64) expect(session.replayContentFallbackBlocked).toBe(true) expect(session.retiredDispatchWaiters.some((waiter) => waiter.sentUuid === firstUuid)).toBe( @@ -292,13 +286,13 @@ describe('Claude structured dispatch image limits', () => { } expect(session.retiredDispatchWaiters).toHaveLength(0) - const second = dispatchClaudeTurn( - session, - { clientMessageId: 'client-2', body: userMessage([{ type: 'text', text: '/permissions' }]) }, - 100 - ) + const second = dispatchClaudeTurn(session, { + clientMessageId: 'client-2', + body: userMessage([{ type: 'text', text: '/permissions' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) const secondUuid = session.dispatchWaiters[0]!.sentUuid + const settled = vi.fn() expect( resolveClaudeReplayWaiter(session, { @@ -311,70 +305,127 @@ describe('Claude structured dispatch image limits', () => { expect(session.dispatchWaiters[0]).toMatchObject({ sentUuid: secondUuid }) expect( - resolveClaudeReplayWaiter(session, { - type: 'result', - subtype: 'success', - session_id: 'provider-session', - uuid: 'result-b', - user_message_uuid: secondUuid - }) + resolveClaudeReplayWaiter( + session, + { + type: 'result', + subtype: 'success', + session_id: 'provider-session', + uuid: 'result-b', + user_message_uuid: secondUuid + }, + settled + ) ).toBe(false) - await expect(second).resolves.toMatchObject({ - providerIdentity: { uuid: 'result-b' } + await expect(second).resolves.toEqual({ state: 'admitted' }) + expect(settled).toHaveBeenCalledWith({ + clientMessageId: 'client-2', + providerIdentity: { provider: 'claude', sessionId: 'provider-session', uuid: 'result-b' } }) }) it('does not let a legacy result for timed-out ordinary dispatch A resolve slash dispatch B', async () => { const session = sessionFor() - const first = dispatchClaudeTurn( - session, - { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: 'ordinary' }]) }, - 100 - ) + const settled = vi.fn() + const first = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: 'ordinary' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) - await expect(first).resolves.toMatchObject({ state: 'unknown' }) + await expect(first).resolves.toEqual({ state: 'admitted' }) + childExited(session) - const second = dispatchClaudeTurn( - session, - { clientMessageId: 'client-2', body: userMessage([{ type: 'text', text: '/permissions' }]) }, - 100 - ) + const second = dispatchClaudeTurn(session, { + clientMessageId: 'client-2', + body: userMessage([{ type: 'text', text: '/permissions' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) expect( - resolveClaudeReplayWaiter(session, { - type: 'result', - subtype: 'success', - session_id: 'provider-session', - uuid: 'legacy-result-a' - }) + resolveClaudeReplayWaiter( + session, + { + type: 'result', + subtype: 'success', + session_id: 'provider-session', + uuid: 'legacy-result-a' + }, + settled + ) ).toBe(false) - await expect(second).resolves.toMatchObject({ state: 'unknown' }) + await expect(second).resolves.toEqual({ state: 'admitted' }) + // Ambiguous, so it settles nothing: the slash waiter is still waiting. + expect(session.dispatchWaiters).toHaveLength(1) + expect(settled).not.toHaveBeenCalled() }) it('removes only its own waiter when a later send fails', async () => { const session = sessionFor() - const first = dispatchClaudeTurn( - session, - { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: 'one' }]) }, - 100 - ) + const settled = vi.fn() + const first = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: 'one' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) const firstWaiter = session.dispatchWaiters[0] - session.connection.send = vi.fn().mockRejectedValue(new Error('broken pipe')) + session.connection.send = vi + .fn() + .mockRejectedValue(claudeUnwrittenUserMessageError(new Error('broken pipe'))) + // A refused write is a transport fact, and the only thing besides child exit + // that puts one message's delivery in doubt. await expect( - dispatchClaudeTurn( - session, - { clientMessageId: 'client-2', body: userMessage([{ type: 'text', text: 'two' }]) }, - 100 - ) - ).resolves.toMatchObject({ state: 'unknown', reason: 'broken pipe' }) + dispatchClaudeTurn(session, { + clientMessageId: 'client-2', + body: userMessage([{ type: 'text', text: 'two' }]) + }) + ).resolves.toEqual({ state: 'unknown', reason: 'provider_write_failed: broken pipe' }) expect(session.dispatchWaiters).toEqual([firstWaiter]) const firstUuid = (firstWaiter as { sentUuid?: string }).sentUuid - resolveClaudeReplayWaiter(session, userReplayFrame(firstUuid!, 'one')) - await expect(first).resolves.toMatchObject({ providerIdentity: { uuid: firstUuid } }) + resolveClaudeReplayWaiter(session, userReplayFrame(firstUuid!, 'one'), settled) + await expect(first).resolves.toEqual({ state: 'admitted' }) + expect(settled).toHaveBeenCalledWith({ + clientMessageId: 'client-1', + providerIdentity: { provider: 'claude', sessionId: 'provider-session', uuid: firstUuid } + }) + }) + + it('does not let a provably unwritten attempt block retry correlation', async () => { + const send = vi + .fn() + .mockRejectedValueOnce(claudeUnwrittenUserMessageError(new Error('broken pipe'))) + .mockResolvedValue(undefined) + const session = sessionFor(send) + const body = userMessage([{ type: 'text', text: 'retry me' }]) + + await expect( + dispatchClaudeTurn(session, { clientMessageId: 'client-1', body }) + ).resolves.toEqual({ state: 'unknown', reason: 'provider_write_failed: broken pipe' }) + expect(session.dispatchWaiters).toHaveLength(0) + expect(session.retiredDispatchWaiters).toHaveLength(0) + + await expect( + dispatchClaudeTurn(session, { clientMessageId: 'client-1', body }) + ).resolves.toEqual({ state: 'admitted' }) + expect(resolveClaudeReplayWaiter(session, userReplayFrame('fresh-replay', 'retry me'))).toBe( + true + ) + expect(session.activeTurnId).toBe('fresh-replay') + }) + + it('does not claim an SDK-pulled frame was unwritten when its write outcome is ambiguous', async () => { + const session = sessionFor(vi.fn().mockRejectedValue(new Error('input pump stopped'))) + + await expect( + dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: 'one' }]) + }) + ).resolves.toEqual({ + state: 'unknown', + reason: 'provider_write_outcome_unknown: input pump stopped' + }) }) it('keeps a replay accepted before its send reports failure', async () => { @@ -386,35 +437,39 @@ describe('Claude structured dispatch image limits', () => { session = sessionFor(send) await expect( - dispatchClaudeTurn( - session, - { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: 'one' }]) }, - 100 - ) + dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: 'one' }]) + }) ).resolves.toMatchObject({ state: 'accepted', providerIdentity: { uuid: 'turn-race' } }) expect(session.dispatchWaiters).toHaveLength(0) }) it('accepts a slash command from its result receipt when Claude omits the user replay', async () => { const session = sessionFor() - const dispatched = dispatchClaudeTurn( - session, - { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: '/permissions' }]) }, - 100 - ) + const settled = vi.fn() + const dispatched = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: '/permissions' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) expect( - resolveClaudeReplayWaiter(session, { - type: 'result', - subtype: 'success', - session_id: 'provider-session', - uuid: 'command-result-uuid' - }) + resolveClaudeReplayWaiter( + session, + { + type: 'result', + subtype: 'success', + session_id: 'provider-session', + uuid: 'command-result-uuid' + }, + settled + ) ).toBe(false) - await expect(dispatched).resolves.toEqual({ - state: 'accepted', + await expect(dispatched).resolves.toEqual({ state: 'admitted' }) + expect(settled).toHaveBeenCalledWith({ + clientMessageId: 'client-1', providerIdentity: { provider: 'claude', sessionId: 'provider-session', @@ -425,32 +480,38 @@ describe('Claude structured dispatch image limits', () => { it('accepts a slash command sent with an attachment from its result receipt', async () => { const session = sessionFor() - const dispatched = dispatchClaudeTurn( - session, - { - clientMessageId: 'client-1', - body: userMessage([ - { type: 'text', text: '/permissions' }, - { type: 'image-ref', url: 'https://example.test/a.png' } - ]) - }, - 100 - ) + const settled = vi.fn() + const dispatched = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([ + { type: 'text', text: '/permissions' }, + { type: 'image-ref', url: 'https://example.test/a.png' } + ]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) // The mapper moves the image ahead of the prompt, so Claude runs the command and replies // with a result receipt instead of a user replay. expect( - resolveClaudeReplayWaiter(session, { - type: 'result', - subtype: 'success', - session_id: 'provider-session', - uuid: 'command-result-uuid' - }) + resolveClaudeReplayWaiter( + session, + { + type: 'result', + subtype: 'success', + session_id: 'provider-session', + uuid: 'command-result-uuid' + }, + settled + ) ).toBe(false) - await expect(dispatched).resolves.toMatchObject({ - state: 'accepted', - providerIdentity: { uuid: 'command-result-uuid' } + await expect(dispatched).resolves.toEqual({ state: 'admitted' }) + expect(settled).toHaveBeenCalledWith({ + clientMessageId: 'client-1', + providerIdentity: { + provider: 'claude', + sessionId: 'provider-session', + uuid: 'command-result-uuid' + } }) // The sent order is the fix: the waiter's verdict alone was already what it is today. expect(session.connection.send).toHaveBeenCalledWith( @@ -468,68 +529,76 @@ describe('Claude structured dispatch image limits', () => { it('does not take a result receipt for leading whitespace Claude never reads as a command', async () => { const session = sessionFor() - const dispatched = dispatchClaudeTurn( - session, - { - clientMessageId: 'client-1', - body: userMessage([{ type: 'text', text: ' /permissions' }]) - }, - 100 - ) + const settled = vi.fn() + const dispatched = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: ' /permissions' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) expect( - resolveClaudeReplayWaiter(session, { - type: 'result', - subtype: 'success', - session_id: 'provider-session', - uuid: 'unrelated-result-uuid' - }) + resolveClaudeReplayWaiter( + session, + { + type: 'result', + subtype: 'success', + session_id: 'provider-session', + uuid: 'unrelated-result-uuid' + }, + settled + ) ).toBe(false) - await expect(dispatched).resolves.toMatchObject({ state: 'unknown' }) + await expect(dispatched).resolves.toEqual({ state: 'admitted' }) + expect(session.dispatchWaiters).toHaveLength(1) + expect(settled).not.toHaveBeenCalled() }) - it('correlates a later slash-command result by user_message_uuid despite a timed-out slash waiter', async () => { + it('correlates a later slash-command result by user_message_uuid despite a retired slash waiter', async () => { const session = sessionFor() - const first = dispatchClaudeTurn( - session, - { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: '/permissions' }]) }, - 500 - ) + const settled = vi.fn() + const first = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: '/permissions' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) - await expect(first).resolves.toMatchObject({ state: 'unknown' }) + await expect(first).resolves.toEqual({ state: 'admitted' }) + childExited(session) - const second = dispatchClaudeTurn( - session, - { clientMessageId: 'client-2', body: userMessage([{ type: 'text', text: '/permissions' }]) }, - 500 - ) + const second = dispatchClaudeTurn(session, { + clientMessageId: 'client-2', + body: userMessage([{ type: 'text', text: '/permissions' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) const secondUuid = session.dispatchWaiters[0]!.sentUuid expect( - resolveClaudeReplayWaiter(session, { - type: 'result', - subtype: 'success', - session_id: 'provider-session', - uuid: 'result-b', - user_message_uuid: secondUuid - }) + resolveClaudeReplayWaiter( + session, + { + type: 'result', + subtype: 'success', + session_id: 'provider-session', + uuid: 'result-b', + user_message_uuid: secondUuid + }, + settled + ) ).toBe(false) - await expect(second).resolves.toMatchObject({ - state: 'accepted', - providerIdentity: { uuid: 'result-b' } + await expect(second).resolves.toEqual({ state: 'admitted' }) + expect(settled).toHaveBeenCalledWith({ + clientMessageId: 'client-2', + providerIdentity: { provider: 'claude', sessionId: 'provider-session', uuid: 'result-b' } }) }) it('does not mistake a normal turn result for its missing user replay', async () => { const session = sessionFor() - const dispatched = dispatchClaudeTurn( - session, - { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: 'hello' }]) }, - 100 - ) + const settled = vi.fn() + const dispatched = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: 'hello' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) expect( @@ -541,31 +610,40 @@ describe('Claude structured dispatch image limits', () => { ).toBe(false) expect(session.dispatchWaiters).toHaveLength(1) expect( - resolveClaudeReplayWaiter(session, { - type: 'user', - parent_tool_use_id: null, - session_id: 'provider-session', - uuid: 'user-replay-uuid', - message: { - role: 'user', - content: [{ type: 'text', text: 'hello' }] - } - }) + resolveClaudeReplayWaiter( + session, + { + type: 'user', + parent_tool_use_id: null, + session_id: 'provider-session', + uuid: 'user-replay-uuid', + message: { + role: 'user', + content: [{ type: 'text', text: 'hello' }] + } + }, + settled + ) ).toBe(true) - await expect(dispatched).resolves.toMatchObject({ - state: 'accepted', - providerIdentity: { uuid: 'user-replay-uuid' } + await expect(dispatched).resolves.toEqual({ state: 'admitted' }) + expect(settled).toHaveBeenCalledWith({ + clientMessageId: 'client-1', + providerIdentity: { + provider: 'claude', + sessionId: 'provider-session', + uuid: 'user-replay-uuid' + } }) }) it('ignores a top-level tool-result user frame while waiting for a slash command replay', async () => { const session = sessionFor() - const dispatched = dispatchClaudeTurn( - session, - { clientMessageId: 'client-1', body: userMessage([{ type: 'text', text: '/permissions' }]) }, - 100 - ) + const settled = vi.fn() + const dispatched = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'text', text: '/permissions' }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) resolveClaudeReplayWaiter(session, { @@ -580,19 +658,24 @@ describe('Claude structured dispatch image limits', () => { }) expect(session.dispatchWaiters).toHaveLength(1) - resolveClaudeReplayWaiter(session, { - type: 'user', - parent_tool_use_id: null, - session_id: 'provider-session', - uuid: 'user-replay-uuid', - message: { - role: 'user', - content: [{ type: 'text', text: '/permissions' }] - } - }) + resolveClaudeReplayWaiter( + session, + { + type: 'user', + parent_tool_use_id: null, + session_id: 'provider-session', + uuid: 'user-replay-uuid', + message: { + role: 'user', + content: [{ type: 'text', text: '/permissions' }] + } + }, + settled + ) - await expect(dispatched).resolves.toEqual({ - state: 'accepted', + await expect(dispatched).resolves.toEqual({ state: 'admitted' }) + expect(settled).toHaveBeenCalledWith({ + clientMessageId: 'client-1', providerIdentity: { provider: 'claude', sessionId: 'provider-session', @@ -611,7 +694,7 @@ describe('Claude structured dispatch image limits', () => { ) await expect( - dispatchClaudeTurn(session, { clientMessageId: 'client-1', body }, 1) + dispatchClaudeTurn(session, { clientMessageId: 'client-1', body }) ).resolves.toEqual({ state: 'rejected', reason: 'Claude messages support at most 20 images' }) expect(session.connection.send).not.toHaveBeenCalled() }) @@ -630,7 +713,7 @@ describe('Claude structured dispatch image limits', () => { const body = userMessage(paths.map((path) => ({ type: 'image-ref' as const, path }))) await expect( - dispatchClaudeTurn(session, { clientMessageId: 'client-1', body }, 1) + dispatchClaudeTurn(session, { clientMessageId: 'client-1', body }) ).resolves.toEqual({ state: 'rejected', reason: `Claude images must total no more than ${20 * 1024 * 1024} bytes` @@ -650,7 +733,7 @@ describe('Claude structured dispatch image limits', () => { const body = userMessage([{ type: 'image-ref', path }]) await expect( - dispatchClaudeTurn(session, { clientMessageId: 'client-1', body }, 1) + dispatchClaudeTurn(session, { clientMessageId: 'client-1', body }) ).resolves.toEqual({ state: 'rejected', reason: `Claude image must be a non-empty file no larger than ${5 * 1024 * 1024} bytes` @@ -668,11 +751,10 @@ describe('Claude structured dispatch image limits', () => { const path = join(directory, 'small.png') await writeFile(path, Buffer.alloc(64)) const session = sessionFor() - const dispatched = dispatchClaudeTurn( - session, - { clientMessageId: 'client-1', body: userMessage([{ type: 'image-ref', path }]) }, - 100 - ) + const dispatched = dispatchClaudeTurn(session, { + clientMessageId: 'client-1', + body: userMessage([{ type: 'image-ref', path }]) + }) await vi.waitFor(() => expect(session.dispatchWaiters).toHaveLength(1)) const sentUuid = (session.dispatchWaiters[0] as { sentUuid?: string }).sentUuid resolveClaudeReplayWaiter(session, { @@ -684,7 +766,7 @@ describe('Claude structured dispatch image limits', () => { ] } }) - await expect(dispatched).resolves.toMatchObject({ state: 'accepted' }) + await expect(dispatched).resolves.toEqual({ state: 'admitted' }) expect(allocUnsafe).toHaveBeenCalled() expect(allocUnsafe.mock.calls.some(([size]) => size === 64 + 1)).toBe(true) expect(allocUnsafe.mock.calls.some(([size]) => size >= 5 * 1024 * 1024)).toBe(false) @@ -694,7 +776,7 @@ describe('Claude structured dispatch image limits', () => { } }) - it('bounds retained waiter identity bytes when image dispatches time out', async () => { + it('bounds retained waiter identity bytes when image dispatches are retired', async () => { const directory = await mkdtemp(join(tmpdir(), 'orca-claude-image-')) try { const path = join(directory, 'large.png') @@ -703,9 +785,10 @@ describe('Claude structured dispatch image limits', () => { const body = userMessage([{ type: 'image-ref', path }]) await Promise.all( Array.from({ length: 64 }, (_, index) => - dispatchClaudeTurn(session, { clientMessageId: `client-${index}`, body }, 1) + dispatchClaudeTurn(session, { clientMessageId: `client-${index}`, body }) ) ) + childExited(session) expect(session.retiredDispatchWaiters).toHaveLength(64) const retainedKeyBytes = session.retiredDispatchWaiters.reduce( diff --git a/src/main/claude/claude-structured-dispatch.ts b/src/main/claude/claude-structured-dispatch.ts index 3080b5479e4..7533102c7aa 100644 --- a/src/main/claude/claude-structured-dispatch.ts +++ b/src/main/claude/claude-structured-dispatch.ts @@ -15,10 +15,16 @@ import { claudeDispatchInvokesSlashCommand, claudeDispatchMessageContent } from './claude-structured-dispatch-content' +import { + dispatchWriteFailureReason, + dispatchWriteOutcomeUnknownReason +} from '../native-chat/agent-session-journal/journal-dispatch-doubt-reasons' +import { claudeUserMessageWasProvablyUnwritten } from './claude-agent-sdk-user-message-queue' const MAX_RETIRED_DISPATCH_WAITERS = 64 +const MAX_ACTIVE_DISPATCH_WAITERS = 64 -/** A dispatch whose ack window expired, proven delivered by this replay. */ +/** Directly settles provider-proven delivery; the durable replay row independently reconciles it. */ export type ClaudeLateDispatchSettlement = (input: { clientMessageId: string providerIdentity: AgentJournalItemIdentity @@ -55,7 +61,7 @@ export function resolveClaudeReplayWaiter( (candidate) => candidate.sentUuid === userMessageUuid ) if (exact) { - settleWaiter(session, exact, uuid) + settleWaiter(session, exact, uuid, onSettledLate) return isUserReplay && exact.dispatchSequence === session.dispatchSequence } const retired = session.retiredDispatchWaiters.find( @@ -70,7 +76,7 @@ export function resolveClaudeReplayWaiter( const exact = session.dispatchWaiters.find((candidate) => candidate.sentUuid === uuid) if (exact) { - settleWaiter(session, exact, uuid) + settleWaiter(session, exact, uuid, onSettledLate) return isUserReplay && exact.dispatchSequence === session.dispatchSequence } const retired = session.retiredDispatchWaiters.find((candidate) => candidate.sentUuid === uuid) @@ -90,7 +96,7 @@ export function resolveClaudeReplayWaiter( (candidate) => candidate.replayContentKey === replayContentKey ) if (compatible.length === 1) { - settleWaiter(session, compatible[0]!, uuid) + settleWaiter(session, compatible[0]!, uuid, onSettledLate) return compatible[0]!.dispatchSequence === session.dispatchSequence } } else if (!session.replayContentFallbackBlocked && session.dispatchWaiters.length === 0) { @@ -120,22 +126,36 @@ export function resolveClaudeReplayWaiter( } const waiter = uuid ? session.dispatchWaiters.shift() : undefined if (waiter && uuid) { - clearTimeout(waiter.timer) - waiter.settledUuid = uuid - waiter.resolve(uuid) + settleWaiter(session, waiter, uuid, onSettledLate) return isUserReplay } return false } -function settleWaiter(session: ClaudeSession, waiter: ClaudeDispatchWaiter, uuid: string): void { +function settleWaiter( + session: ClaudeSession, + waiter: ClaudeDispatchWaiter, + uuid: string, + onSettledLate?: ClaudeLateDispatchSettlement +): void { const index = session.dispatchWaiters.indexOf(waiter) if (index !== -1) { session.dispatchWaiters.splice(index, 1) } - clearTimeout(waiter.timer) waiter.settledUuid = uuid waiter.resolve(uuid) + // Dispatch returned on admission. Settle delivery unfenced while the sequence + // still fences which turn owns the identity; see `recoverLateIdentity`. + if (waiter.clientMessageId) { + onSettledLate?.({ + clientMessageId: waiter.clientMessageId, + providerIdentity: { provider: 'claude', sessionId: session.providerSessionId, uuid } + }) + } + if (waiter.dispatchSequence === session.dispatchSequence) { + session.activeTurnId = uuid + session.activeTurnSequence = waiter.dispatchSequence + } } function forgetRetiredWaiter(session: ClaudeSession, waiter: ClaudeDispatchWaiter): void { @@ -158,10 +178,12 @@ function recoverLateIdentity( // The provider acted on this dispatch, so the send it came from is delivered. // Unfenced on purpose: the dispatch-sequence check below only decides which // turn owns the identity, while delivery is settled for good either way. - onSettledLate?.({ - clientMessageId: waiter.clientMessageId, - providerIdentity: { provider: 'claude', sessionId: session.providerSessionId, uuid } - }) + if (waiter.clientMessageId) { + onSettledLate?.({ + clientMessageId: waiter.clientMessageId, + providerIdentity: { provider: 'claude', sessionId: session.providerSessionId, uuid } + }) + } if (waiter.dispatchSequence === session.dispatchSequence) { session.activeTurnId = uuid session.activeTurnSequence = waiter.dispatchSequence @@ -169,13 +191,19 @@ function recoverLateIdentity( return isUserReplay && waiter.dispatchSequence === session.dispatchSequence } +/** + * A waiter with no deadline. The echo Claude sends is emitted when the provider + * STARTS the turn, so a message queued behind a running turn cannot be echoed + * until that turn ends — an interval bounded only by the previous turn. Elapsed + * time is therefore not evidence about delivery, and nothing here expires. + * Waiters are retired by process facts instead: a failed write, or child exit. + */ function waitForReplay( session: ClaudeSession, - timeoutMs: number, acceptsResult: boolean, sentUuid: string, replayContentKey: string, - clientMessageId: string + clientMessageId: string | null ): { waiter: ClaudeDispatchWaiter; promise: Promise } { let waiter!: ClaudeDispatchWaiter const promise = new Promise((resolve) => { @@ -185,28 +213,22 @@ function waitForReplay( sentUuid, dispatchSequence: session.dispatchSequence, replayContentKey, - resolve, - timer: setTimeout(() => { - const index = session.dispatchWaiters.indexOf(waiter) - if (index !== -1) { - session.dispatchWaiters.splice(index, 1) - } - retireWaiter(session, waiter) - resolve(null) - }, timeoutMs) + resolve } - waiter.timer.unref?.() session.dispatchWaiters.push(waiter) }) return { waiter, promise } } -function retireWaiter(session: ClaudeSession, waiter: ClaudeDispatchWaiter): void { +function forgetWaiter(session: ClaudeSession, waiter: ClaudeDispatchWaiter): void { const index = session.dispatchWaiters.indexOf(waiter) if (index !== -1) { session.dispatchWaiters.splice(index, 1) } - clearTimeout(waiter.timer) +} + +function retireWaiter(session: ClaudeSession, waiter: ClaudeDispatchWaiter): void { + forgetWaiter(session, waiter) if (!waiter.retired) { waiter.retired = true session.retiredDispatchWaiters.push(waiter) @@ -220,10 +242,19 @@ function retireWaiter(session: ClaudeSession, waiter: ClaudeDispatchWaiter): voi } } +/** Nothing expires a waiter, so the child's death is what ends every live one. + * Retired rather than dropped: their identities stay joinable, bounded by + * `MAX_RETIRED_DISPATCH_WAITERS`. */ +export function retireClaudeDispatchWaiters(session: ClaudeSession): void { + for (const waiter of session.dispatchWaiters.splice(0)) { + retireWaiter(session, waiter) + waiter.resolve(null) + } +} + export async function dispatchClaudeTurn( session: ClaudeSession, - input: { clientMessageId: string; body: AgentJournalMessageItem }, - timeoutMs: number + input: { clientMessageId?: string; body: AgentJournalMessageItem } ): Promise { let content: unknown[] try { @@ -231,6 +262,9 @@ export async function dispatchClaudeTurn( } catch (error) { return { state: 'rejected', reason: (error as Error).message } } + if (session.dispatchWaiters.length >= MAX_ACTIVE_DISPATCH_WAITERS) { + return { state: 'rejected', reason: 'claude structured dispatch queue is full' } + } const dispatchSequence = ++session.dispatchSequence // Read the sent content, not the journal blocks: only the mapped trailing prompt decides // whether Claude runs a command, so the two cannot disagree about which frame settles this. @@ -238,11 +272,10 @@ export async function dispatchClaudeTurn( const sentUuid = randomUUID() const replay = waitForReplay( session, - timeoutMs, acceptsResult, sentUuid, claudeDispatchContentKey(content), - input.clientMessageId + input.clientMessageId ?? null ) const replayed = replay.promise try { @@ -266,21 +299,24 @@ export async function dispatchClaudeTurn( } } } - if (!waiter.retired) { + const provablyUnwritten = claudeUserMessageWasProvablyUnwritten(error) + if (provablyUnwritten) { + forgetWaiter(session, waiter) + forgetRetiredWaiter(session, waiter) + waiter.resolve(null) + } else if (!waiter.retired) { retireWaiter(session, waiter) waiter.resolve(null) } - return { state: 'unknown', reason: (error as Error).message } + return { + state: 'unknown', + reason: provablyUnwritten + ? dispatchWriteFailureReason(error) + : dispatchWriteOutcomeUnknownReason(error) + } } - const uuid = await replayed - if (uuid) { - session.activeTurnId = uuid - session.activeTurnSequence = dispatchSequence - } - return uuid - ? { - state: 'accepted', - providerIdentity: { provider: 'claude', sessionId: session.providerSessionId, uuid } - } - : { state: 'unknown', reason: 'claude accepted a message but did not replay its uuid in time' } + // The write is the admission signal. Awaiting the echo here would block on the + // turn already running, which is why the deadline this replaces kept declaring + // doubt about messages that were delivered. `settleWaiter` finishes the job. + return { state: 'admitted' } } diff --git a/src/main/claude/claude-structured-session-adapter-turns.test.ts b/src/main/claude/claude-structured-session-adapter-turns.test.ts new file mode 100644 index 00000000000..fc5eaa6b66e --- /dev/null +++ b/src/main/claude/claude-structured-session-adapter-turns.test.ts @@ -0,0 +1,261 @@ +// What the adapter reports for one turn: how a dispatch is admitted and named, +// and which turn a cancellation is allowed to interrupt. + +import { describe, expect, it, vi } from 'vitest' +import { ClaudeControlRequestError } from './claude-stream-json-connection' +import { claudeUnwrittenUserMessageError } from './claude-agent-sdk-user-message-queue' +import { + acquired, + fakeClaude, + PROVIDER_SESSION_ID, + USER_MESSAGE +} from './claude-structured-session-test-support' + +describe('ClaudeStructuredSessionAdapter turns and controls', () => { + it("admits a dispatch on the write and names it from Claude's replay", async () => { + const claude = fakeClaude({ replayUuid: 'user-provider-uuid' }) + const settled = vi.fn() + const adapter = await acquired(claude, {}, [], settled) + + const result = await adapter.dispatch({ + sessionId: 'session-1', + clientMessageId: 'client-1', + body: USER_MESSAGE, + fence: 7 + }) + + expect(result).toEqual({ state: 'admitted' }) + expect(settled).toHaveBeenCalledWith({ + sessionId: 'session-1', + clientMessageId: 'client-1', + providerIdentity: { + provider: 'claude', + sessionId: PROVIDER_SESSION_ID, + uuid: 'user-provider-uuid' + } + }) + expect(claude.connections[0].sent[0]).toMatchObject({ + type: 'user', + message: { role: 'user', content: [{ type: 'text', text: 'ship it' }] }, + session_id: PROVIDER_SESSION_ID + }) + }) + + it('does not put delivery in doubt while no replay uuid has arrived', async () => { + const settled = vi.fn() + const adapter = await acquired(fakeClaude({ replayUuid: null }), {}, [], settled) + await expect( + adapter.dispatch({ + sessionId: 'session-1', + clientMessageId: 'client-1', + body: USER_MESSAGE, + fence: 7 + }) + ).resolves.toEqual({ state: 'admitted' }) + expect(settled).not.toHaveBeenCalled() + }) + + it('puts delivery in doubt only when the write itself fails', async () => { + const claude = fakeClaude({ replayUuid: null }) + const adapter = await acquired(claude) + claude.connections[0]!.send = async () => { + throw claudeUnwrittenUserMessageError(new Error('broken pipe')) + } + await expect( + adapter.dispatch({ + sessionId: 'session-1', + clientMessageId: 'client-1', + body: USER_MESSAGE, + fence: 7 + }) + ).resolves.toEqual({ state: 'unknown', reason: 'provider_write_failed: broken pipe' }) + }) + + it('requires an acknowledged interrupt and supports controlled options', async () => { + const claude = fakeClaude() + const adapter = await acquired(claude) + await expect( + adapter.cancelTurn({ sessionId: 'session-1', turnId: 'turn-1', fence: 7 }) + ).resolves.toEqual({ cancelled: true }) + await expect( + adapter.setOption({ sessionId: 'session-1', key: 'model', value: 'sonnet', fence: 7 }) + ).resolves.toEqual({ model: 'sonnet' }) + expect(claude.connections[0].calls.slice(-2)).toEqual([ + { subtype: 'interrupt', params: {} }, + { subtype: 'set_model', params: { model: 'sonnet' } } + ]) + + claude.routes.interrupt = () => { + throw new ClaudeControlRequestError('interrupt', 'not running') + } + await expect( + adapter.cancelTurn({ sessionId: 'session-1', turnId: 'turn-2', fence: 7 }) + ).resolves.toEqual({ cancelled: false }) + + claude.routes.interrupt = () => { + throw new Error('claude interrupt request timed out') + } + await expect( + adapter.cancelTurn({ sessionId: 'session-1', turnId: 'turn-3', fence: 7 }) + ).rejects.toThrow('timed out') + }) + + it('does not let a delayed cancellation for an earlier turn interrupt the later turn', async () => { + const claude = fakeClaude({ replayUuids: ['turn-T', 'turn-U'] }) + const adapter = await acquired(claude) + + await adapter.dispatch({ + sessionId: 'session-1', + clientMessageId: 'client-T', + body: USER_MESSAGE, + fence: 7 + }) + await adapter.dispatch({ + sessionId: 'session-1', + clientMessageId: 'client-U', + body: USER_MESSAGE, + fence: 7 + }) + + await expect( + adapter.cancelTurn({ sessionId: 'session-1', turnId: 'turn-T', fence: 7 }) + ).resolves.toEqual({ cancelled: false }) + expect(claude.connections[0].calls.filter((call) => call.subtype === 'interrupt')).toHaveLength( + 0 + ) + + await expect( + adapter.cancelTurn({ sessionId: 'session-1', turnId: 'turn-U', fence: 6 }) + ).resolves.toEqual({ cancelled: false }) + + await expect( + adapter.cancelTurn({ sessionId: 'session-1', turnId: 'turn-U', fence: 7 }) + ).resolves.toEqual({ cancelled: true }) + expect(claude.connections[0].calls.filter((call) => call.subtype === 'interrupt')).toHaveLength( + 1 + ) + }) + + it('does not cancel an acknowledged turn after a later dispatch is still unacknowledged', async () => { + const claude = fakeClaude({ replayUuids: ['turn-T', null] }) + const settled = vi.fn() + const adapter = await acquired(claude, {}, [], settled) + + await expect( + adapter.dispatch({ + sessionId: 'session-1', + clientMessageId: 'client-T', + body: USER_MESSAGE, + fence: 7 + }) + ).resolves.toEqual({ state: 'admitted' }) + expect(settled).toHaveBeenCalledWith({ + sessionId: 'session-1', + clientMessageId: 'client-T', + providerIdentity: { provider: 'claude', sessionId: PROVIDER_SESSION_ID, uuid: 'turn-T' } + }) + await expect( + adapter.dispatch({ + sessionId: 'session-1', + clientMessageId: 'client-U', + body: USER_MESSAGE, + fence: 7 + }) + ).resolves.toEqual({ state: 'admitted' }) + expect(claude.connections[0].sent).toHaveLength(2) + + await expect( + adapter.cancelTurn({ sessionId: 'session-1', turnId: 'turn-T', fence: 7 }) + ).resolves.toEqual({ cancelled: false }) + expect(claude.connections[0].calls.filter((call) => call.subtype === 'interrupt')).toHaveLength( + 0 + ) + }) + + it('classifies provider-declined options without treating timeouts as settled', async () => { + const claude = fakeClaude({ + routes: { + set_model: () => { + throw new ClaudeControlRequestError('set_model', 'model unavailable') + } + } + }) + const adapter = await acquired(claude) + + await expect( + adapter.setOption({ sessionId: 'session-1', key: 'model', value: 'fable', fence: 7 }) + ).rejects.toMatchObject({ name: 'AgentSessionOptionRejectedError' }) + claude.routes.set_model = () => { + throw new Error('claude set_model request timed out') + } + await expect( + adapter.setOption({ sessionId: 'session-1', key: 'model', value: 'opus', fence: 7 }) + ).rejects.toThrow('timed out') + }) + + it('hydrates live model choices and maps the resolved current model to its CLI id', async () => { + const claude = fakeClaude({ + initModel: 'claude-sonnet-5', + routes: { + list_models: () => [ + { value: 'default', resolvedModel: 'claude-opus-5', displayName: 'Default' }, + { + value: 'opus', + resolvedModel: 'claude-opus-5', + displayName: 'Opus', + supportsEffort: true, + supportedEffortLevels: ['low', 'high'] + }, + { + value: 'sonnet', + resolvedModel: 'claude-sonnet-5', + displayName: 'Sonnet' + } + ] + } + }) + const adapter = await acquired(claude) + + await expect(adapter.readOptions({ sessionId: 'session-1', fence: 7 })).resolves.toEqual({ + models: [ + { + id: 'opus', + label: 'Opus', + isDefault: true, + efforts: [ + { value: 'low', label: 'Low' }, + { value: 'high', label: 'High' } + ] + }, + { id: 'sonnet', label: 'Sonnet', isDefault: false, efforts: [] } + ], + current: { model: 'sonnet', effort: 'high', confirmed: ['model', 'effort'] } + }) + }) + + it('keeps the shared Claude seed when live model discovery is unavailable', async () => { + const claude = fakeClaude({ + initModel: 'custom-model', + routes: { + list_models: () => { + throw new Error('unsupported') + } + } + }) + const adapter = await acquired(claude) + const result = await adapter.readOptions({ sessionId: 'session-1', fence: 7 }) + + expect(result.models.map((model) => model.id)).toEqual([ + 'fable', + 'opus', + 'sonnet', + 'haiku', + 'custom-model' + ]) + expect(result.current).toEqual({ + model: 'custom-model', + effort: 'high', + confirmed: ['model', 'effort'] + }) + }) +}) diff --git a/src/main/claude/claude-structured-session-adapter.test.ts b/src/main/claude/claude-structured-session-adapter.test.ts index 859ba9a8ed8..44a76f8402a 100644 --- a/src/main/claude/claude-structured-session-adapter.test.ts +++ b/src/main/claude/claude-structured-session-adapter.test.ts @@ -144,10 +144,11 @@ describe('ClaudeStructuredSessionAdapter.acquire', () => { expect(claude.connections[0]?.closeCount).toBe(1) }) - it('recovers a cancellable lifecycle when a timed-out replay arrives late', async () => { + it('recovers a cancellable lifecycle when the replay arrives after dispatch returned', async () => { const claude = fakeClaude({ replayUuid: null }) const events: ClaudeStructuredSessionEvent[] = [] - const adapter = await acquired(claude, {}, events) + const settled = vi.fn() + const adapter = await acquired(claude, {}, events, settled) await expect( adapter.dispatch({ @@ -156,7 +157,7 @@ describe('ClaudeStructuredSessionAdapter.acquire', () => { body: USER_MESSAGE, fence: 7 }) - ).resolves.toMatchObject({ state: 'unknown' }) + ).resolves.toEqual({ state: 'admitted' }) const sent = claude.connections[0]!.sent[0]! claude.connections[0]!.handlers.onMessage?.({ ...sent, @@ -170,6 +171,15 @@ describe('ClaudeStructuredSessionAdapter.acquire', () => { message: expect.objectContaining({ uuid: 'late-turn-1' }) }) ) + expect(settled).toHaveBeenCalledWith({ + sessionId: 'session-1', + clientMessageId: 'client-1', + providerIdentity: { + provider: 'claude', + sessionId: PROVIDER_SESSION_ID, + uuid: 'late-turn-1' + } + }) await expect( adapter.cancelTurn({ sessionId: 'session-1', turnId: 'late-turn-1', fence: 7 }) ).resolves.toEqual({ cancelled: true }) @@ -178,7 +188,8 @@ describe('ClaudeStructuredSessionAdapter.acquire', () => { it('quarantines SDK frames without the acquired session identity', async () => { const claude = fakeClaude({ replayUuid: null }) const events: ClaudeStructuredSessionEvent[] = [] - const adapter = await acquired(claude, {}, events) + const settled = vi.fn() + const adapter = await acquired(claude, {}, events, settled) const connection = claude.connections[0]! connection.handlers.onMessage?.({ @@ -193,13 +204,14 @@ describe('ClaudeStructuredSessionAdapter.acquire', () => { message: { role: 'assistant', content: [{ type: 'text', text: 'do not admit' }] } }) - const dispatch = adapter.dispatch({ - sessionId: 'session-1', - clientMessageId: 'client-1', - body: USER_MESSAGE, - fence: 7 - }) - await Promise.resolve() + await expect( + adapter.dispatch({ + sessionId: 'session-1', + clientMessageId: 'client-1', + body: USER_MESSAGE, + fence: 7 + }) + ).resolves.toEqual({ state: 'admitted' }) expect(connection.sent).toHaveLength(1) connection.handlers.onMessage?.({ ...connection.sent[0], @@ -208,14 +220,20 @@ describe('ClaudeStructuredSessionAdapter.acquire', () => { }) await Promise.resolve() expect(events.filter((event) => event.type === 'message')).toHaveLength(1) + expect(settled).not.toHaveBeenCalled() connection.handlers.onMessage?.({ ...connection.sent[0], session_id: PROVIDER_SESSION_ID }) - await expect(dispatch).resolves.toMatchObject({ - state: 'accepted', - providerIdentity: { uuid: connection.sent[0]!.uuid } + expect(settled).toHaveBeenCalledWith({ + sessionId: 'session-1', + clientMessageId: 'client-1', + providerIdentity: { + provider: 'claude', + sessionId: PROVIDER_SESSION_ID, + uuid: connection.sent[0]!.uuid + } }) }) @@ -382,231 +400,6 @@ describe('ClaudeStructuredSessionAdapter.acquire', () => { }) }) -describe('ClaudeStructuredSessionAdapter turns and controls', () => { - it('accepts a dispatch only after Claude replays its provider uuid', async () => { - const claude = fakeClaude({ replayUuid: 'user-provider-uuid' }) - const adapter = await acquired(claude) - - const result = await adapter.dispatch({ - sessionId: 'session-1', - clientMessageId: 'client-1', - body: USER_MESSAGE, - fence: 7 - }) - - expect(result).toEqual({ - state: 'accepted', - providerIdentity: { - provider: 'claude', - sessionId: PROVIDER_SESSION_ID, - uuid: 'user-provider-uuid' - } - }) - expect(claude.connections[0].sent[0]).toMatchObject({ - type: 'user', - message: { role: 'user', content: [{ type: 'text', text: 'ship it' }] }, - session_id: PROVIDER_SESSION_ID - }) - }) - - it('leaves delivery unconfirmed when no replay uuid arrives', async () => { - const adapter = await acquired(fakeClaude({ replayUuid: null })) - await expect( - adapter.dispatch({ - sessionId: 'session-1', - clientMessageId: 'client-1', - body: USER_MESSAGE, - fence: 7 - }) - ).resolves.toMatchObject({ state: 'unknown' }) - }) - - it('requires an acknowledged interrupt and supports controlled options', async () => { - const claude = fakeClaude() - const adapter = await acquired(claude) - await expect( - adapter.cancelTurn({ sessionId: 'session-1', turnId: 'turn-1', fence: 7 }) - ).resolves.toEqual({ cancelled: true }) - await expect( - adapter.setOption({ sessionId: 'session-1', key: 'model', value: 'sonnet', fence: 7 }) - ).resolves.toEqual({ model: 'sonnet' }) - expect(claude.connections[0].calls.slice(-2)).toEqual([ - { subtype: 'interrupt', params: {} }, - { subtype: 'set_model', params: { model: 'sonnet' } } - ]) - - claude.routes.interrupt = () => { - throw new ClaudeControlRequestError('interrupt', 'not running') - } - await expect( - adapter.cancelTurn({ sessionId: 'session-1', turnId: 'turn-2', fence: 7 }) - ).resolves.toEqual({ cancelled: false }) - - claude.routes.interrupt = () => { - throw new Error('claude interrupt request timed out') - } - await expect( - adapter.cancelTurn({ sessionId: 'session-1', turnId: 'turn-3', fence: 7 }) - ).rejects.toThrow('timed out') - }) - - it('does not let a delayed cancellation for an earlier turn interrupt the later turn', async () => { - const claude = fakeClaude({ replayUuids: ['turn-T', 'turn-U'] }) - const adapter = await acquired(claude) - - await adapter.dispatch({ - sessionId: 'session-1', - clientMessageId: 'client-T', - body: USER_MESSAGE, - fence: 7 - }) - await adapter.dispatch({ - sessionId: 'session-1', - clientMessageId: 'client-U', - body: USER_MESSAGE, - fence: 7 - }) - - await expect( - adapter.cancelTurn({ sessionId: 'session-1', turnId: 'turn-T', fence: 7 }) - ).resolves.toEqual({ cancelled: false }) - expect(claude.connections[0].calls.filter((call) => call.subtype === 'interrupt')).toHaveLength( - 0 - ) - - await expect( - adapter.cancelTurn({ sessionId: 'session-1', turnId: 'turn-U', fence: 6 }) - ).resolves.toEqual({ cancelled: false }) - - await expect( - adapter.cancelTurn({ sessionId: 'session-1', turnId: 'turn-U', fence: 7 }) - ).resolves.toEqual({ cancelled: true }) - expect(claude.connections[0].calls.filter((call) => call.subtype === 'interrupt')).toHaveLength( - 1 - ) - }) - - it('does not cancel an acknowledged turn after a later dispatch returns unknown', async () => { - const claude = fakeClaude({ replayUuids: ['turn-T', null] }) - const adapter = await acquired(claude) - - await expect( - adapter.dispatch({ - sessionId: 'session-1', - clientMessageId: 'client-T', - body: USER_MESSAGE, - fence: 7 - }) - ).resolves.toMatchObject({ - state: 'accepted', - providerIdentity: { uuid: 'turn-T' } - }) - await expect( - adapter.dispatch({ - sessionId: 'session-1', - clientMessageId: 'client-U', - body: USER_MESSAGE, - fence: 7 - }) - ).resolves.toMatchObject({ state: 'unknown' }) - expect(claude.connections[0].sent).toHaveLength(2) - - await expect( - adapter.cancelTurn({ sessionId: 'session-1', turnId: 'turn-T', fence: 7 }) - ).resolves.toEqual({ cancelled: false }) - expect(claude.connections[0].calls.filter((call) => call.subtype === 'interrupt')).toHaveLength( - 0 - ) - }) - - it('classifies provider-declined options without treating timeouts as settled', async () => { - const claude = fakeClaude({ - routes: { - set_model: () => { - throw new ClaudeControlRequestError('set_model', 'model unavailable') - } - } - }) - const adapter = await acquired(claude) - - await expect( - adapter.setOption({ sessionId: 'session-1', key: 'model', value: 'fable', fence: 7 }) - ).rejects.toMatchObject({ name: 'AgentSessionOptionRejectedError' }) - claude.routes.set_model = () => { - throw new Error('claude set_model request timed out') - } - await expect( - adapter.setOption({ sessionId: 'session-1', key: 'model', value: 'opus', fence: 7 }) - ).rejects.toThrow('timed out') - }) - - it('hydrates live model choices and maps the resolved current model to its CLI id', async () => { - const claude = fakeClaude({ - initModel: 'claude-sonnet-5', - routes: { - list_models: () => [ - { value: 'default', resolvedModel: 'claude-opus-5', displayName: 'Default' }, - { - value: 'opus', - resolvedModel: 'claude-opus-5', - displayName: 'Opus', - supportsEffort: true, - supportedEffortLevels: ['low', 'high'] - }, - { - value: 'sonnet', - resolvedModel: 'claude-sonnet-5', - displayName: 'Sonnet' - } - ] - } - }) - const adapter = await acquired(claude) - - await expect(adapter.readOptions({ sessionId: 'session-1', fence: 7 })).resolves.toEqual({ - models: [ - { - id: 'opus', - label: 'Opus', - isDefault: true, - efforts: [ - { value: 'low', label: 'Low' }, - { value: 'high', label: 'High' } - ] - }, - { id: 'sonnet', label: 'Sonnet', isDefault: false, efforts: [] } - ], - current: { model: 'sonnet', effort: 'high', confirmed: ['model', 'effort'] } - }) - }) - - it('keeps the shared Claude seed when live model discovery is unavailable', async () => { - const claude = fakeClaude({ - initModel: 'custom-model', - routes: { - list_models: () => { - throw new Error('unsupported') - } - } - }) - const adapter = await acquired(claude) - const result = await adapter.readOptions({ sessionId: 'session-1', fence: 7 }) - - expect(result.models.map((model) => model.id)).toEqual([ - 'fable', - 'opus', - 'sonnet', - 'haiku', - 'custom-model' - ]) - expect(result.current).toEqual({ - model: 'custom-model', - effort: 'high', - confirmed: ['model', 'effort'] - }) - }) -}) - describe('ClaudeStructuredSessionAdapter acquisition cleanup', () => { /** A start that fails after the child self-exited, with its close verdict scripted. */ function failedStart( diff --git a/src/main/claude/claude-structured-session-adapter.ts b/src/main/claude/claude-structured-session-adapter.ts index e2256b6210c..d613a175917 100644 --- a/src/main/claude/claude-structured-session-adapter.ts +++ b/src/main/claude/claude-structured-session-adapter.ts @@ -40,8 +40,6 @@ export type { ClaudeStructuredSessionEvent } from './claude-structured-session-state' -const DISPATCH_ACK_TIMEOUT_MS = 10_000 - function backgroundTaskState(session: ClaudeSession): AgentSessionBackgroundTaskState | null { const state = session.backgroundTasks.state return state ? { ...state, supportsTaskStop: true } : null @@ -220,19 +218,10 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda } dispatch: StructuredAgentSessionAdapter['dispatch'] = (input) => - dispatchClaudeTurn( - this.session(input.sessionId), - input, - this.deps.dispatchAckTimeoutMs ?? DISPATCH_ACK_TIMEOUT_MS - ) + dispatchClaudeTurn(this.session(input.sessionId), input) compact: NonNullable = (input) => - compactClaudeSession( - this.session(input.sessionId), - this.compactions, - input, - this.deps.dispatchAckTimeoutMs ?? DISPATCH_ACK_TIMEOUT_MS - ) + compactClaudeSession(this.session(input.sessionId), this.compactions, input) cancelTurn: StructuredAgentSessionAdapter['cancelTurn'] = (input) => { const session = this.session(input.sessionId) diff --git a/src/main/claude/claude-structured-session-close.ts b/src/main/claude/claude-structured-session-close.ts index eac681ff291..18948ff6819 100644 --- a/src/main/claude/claude-structured-session-close.ts +++ b/src/main/claude/claude-structured-session-close.ts @@ -13,6 +13,7 @@ import { import type { ClaudeStreamJsonConnection } from './claude-stream-json-connection' import type { AgentSessionBackgroundTaskState } from '../../shared/agent-session-wire' import { closeProcessRegistry } from '../../shared/child-process/close-process-registry' +import { retireClaudeDispatchWaiters } from './claude-structured-dispatch' import { readClaudeTranscriptLeafWithReproof } from './claude-transcript-branch-proof' export function claudeAcquisitionCleanupError( @@ -28,15 +29,10 @@ export function claudeAcquisitionCleanupError( : new AgentSessionAcquisitionExitUnprovenError(cause) } -export function settleClaudeDispatchWaiters(session: ClaudeSession): void { - for (const waiter of session.dispatchWaiters.splice(0)) { - clearTimeout(waiter.timer) - waiter.resolve(null) - } -} - export function settleClaudeExitedSession(session: ClaudeSession): void { - settleClaudeDispatchWaiters(session) + // The child is gone, so no replay can start these turns. Nothing else ends a + // waiter's life now that no deadline does. + retireClaudeDispatchWaiters(session) for (const prompt of session.prompts.clear()) { prompt.settle(null) } @@ -68,7 +64,7 @@ async function finalizeClaudePublishedSession( input: CloseClaudePublishedSessionInput, session: ClaudeSession ): Promise { - settleClaudeDispatchWaiters(session) + retireClaudeDispatchWaiters(session) // Settle every in-flight permission callback so closing leaves no dangling promise; `null` // writes no response, and the SDK ignores any post-cleanup answer regardless. for (const prompt of session.prompts.clear()) { diff --git a/src/main/claude/claude-structured-session-state.ts b/src/main/claude/claude-structured-session-state.ts index ab259f62097..7c3dc5e050e 100644 --- a/src/main/claude/claude-structured-session-state.ts +++ b/src/main/claude/claude-structured-session-state.ts @@ -64,7 +64,7 @@ export type ClaudeStructuredSessionAdapterDeps = { identity: AgentSessionJournalIdentity }) => Promise onEvent?: (event: ClaudeStructuredSessionEvent) => void - /** A dispatch whose ack timed out, proven delivered by a later provider replay. */ + /** Direct settlement path for a provider replay; its durable item row also reconciles delivery. */ onDispatchSettledLate?: (input: { sessionId: string clientMessageId: string @@ -81,7 +81,6 @@ export type ClaudeStructuredSessionAdapterDeps = { now?: () => number requestTimeoutMs?: number initTimeoutMs?: number - dispatchAckTimeoutMs?: number persistHandle?: (input: { sessionId: string providerSessionId: string @@ -100,18 +99,16 @@ export type ClaudeStructuredSessionAdapterDeps = { export type ClaudeDispatchWaiter = { resolve: (uuid: string | null) => void - timer: ReturnType acceptsResult: boolean - /** Carried so a replay that lands after the ack window can settle the journal - * submission this dispatch came from, not just the in-memory turn identity. */ - clientMessageId: string + /** Submission settled by the replay, or null for provider-control turns. */ + clientMessageId: string | null /** Client uuid echoed by Claude so a replay is tied to its own dispatch. */ sentUuid: string /** Sequence used to fence a late identity from a newer dispatch. */ dispatchSequence: number /** Set when the provider replay settled this waiter before send returned. */ settledUuid?: string - /** The waiter timed out or its write failed, but its replay may still arrive. */ + /** The write failed or the child died, but a replay may still name it. */ retired?: boolean /** Bounded digest/summary for compatibility CLIs that mint UUIDs. */ replayContentKey: string @@ -127,7 +124,7 @@ export type ClaudeSession = { acquisitionGeneration: string prompts: ClaudePromptRegistry dispatchWaiters: ClaudeDispatchWaiter[] - /** Bounded identities for dispatches whose ack was unknown when they returned. */ + /** Bounded identities for dispatches whose child died or whose write failed. */ retiredDispatchWaiters: ClaudeDispatchWaiter[] /** Once a retired waiter is evicted, legacy content-only replay matching is unsafe. */ replayContentFallbackBlocked: boolean diff --git a/src/main/claude/claude-structured-session-test-support.ts b/src/main/claude/claude-structured-session-test-support.ts index 15a9fcbbb8f..e728263d058 100644 --- a/src/main/claude/claude-structured-session-test-support.ts +++ b/src/main/claude/claude-structured-session-test-support.ts @@ -198,7 +198,8 @@ export function adapterFor( initTimeoutMs?: number, readTranscriptLeaf?: ClaudeStructuredSessionAdapterDeps['readTranscriptLeaf'], persistHandle?: ClaudeStructuredSessionAdapterDeps['persistHandle'], - onBackgroundTasksChanged?: ClaudeStructuredSessionAdapterDeps['onBackgroundTasksChanged'] + onBackgroundTasksChanged?: ClaudeStructuredSessionAdapterDeps['onBackgroundTasksChanged'], + onDispatchSettledLate?: ClaudeStructuredSessionAdapterDeps['onDispatchSettledLate'] ): ClaudeStructuredSessionAdapter { return new ClaudeStructuredSessionAdapter({ resolveLaunch: async () => ({ @@ -216,13 +217,13 @@ export function adapterFor( readProcessStartTime: async () => 1_700_000_000_000, now: () => 1_700_000_000_500, ...(initTimeoutMs === undefined ? {} : { initTimeoutMs }), - dispatchAckTimeoutMs: 10, persistHandle: persistHandle ?? (async (handle) => { persistedHandles.push(handle) }), ...(onBackgroundTasksChanged ? { onBackgroundTasksChanged } : {}), + ...(onDispatchSettledLate ? { onDispatchSettledLate } : {}), ...(readTranscriptLeaf ? { readTranscriptLeaf } : {}) }) } @@ -230,9 +231,20 @@ export function adapterFor( export async function acquired( claude: ReturnType, launch: Partial = {}, - events: ClaudeStructuredSessionEvent[] = [] + events: ClaudeStructuredSessionEvent[] = [], + onDispatchSettledLate?: ClaudeStructuredSessionAdapterDeps['onDispatchSettledLate'] ): Promise { - const adapter = adapterFor(claude, launch, events) + const adapter = adapterFor( + claude, + launch, + events, + undefined, + undefined, + undefined, + undefined, + undefined, + onDispatchSettledLate + ) await adapter.acquire({ identity: identityFor(), fence: 7, spawnToken: 'spawn-9' }) return adapter } diff --git a/src/main/claude/claude-tui-resume-real-binary.integration.test.ts b/src/main/claude/claude-tui-resume-real-binary.integration.test.ts index 9ba3daf2285..f8822505c5a 100644 --- a/src/main/claude/claude-tui-resume-real-binary.integration.test.ts +++ b/src/main/claude/claude-tui-resume-real-binary.integration.test.ts @@ -176,6 +176,7 @@ describe.skipIf(!claudeAuthenticated)('real Claude TUI resume proof', () => { const providerSessionId = randomUUID() const claudeConfigDir = process.env.CLAUDE_CONFIG_DIR?.trim() || join(homedir(), '.claude') const events: ClaudeStructuredSessionEvent[] = [] + const settlements: { clientMessageId: string }[] = [] const adapter = new ClaudeStructuredSessionAdapter({ resolveLaunch: async () => ({ pathToClaudeCodeExecutable: command, @@ -191,6 +192,7 @@ describe.skipIf(!claudeAuthenticated)('real Claude TUI resume proof', () => { resumed: false }), onEvent: (event) => events.push(event), + onDispatchSettledLate: (settlement) => settlements.push(settlement), readProcessStartTime: async () => 1 }) let resumed: RunningTui | null = null @@ -211,8 +213,12 @@ describe.skipIf(!claudeAuthenticated)('real Claude TUI resume proof', () => { blocks: [{ type: 'text', text: 'Reply only with ORCA_RESUME_READY.' }] } }) - ).resolves.toMatchObject({ state: 'accepted' }) + ).resolves.toEqual({ state: 'admitted' }) await waitForStructuredResult(events) + // The real CLI's replay is what settles the send; dispatch only admitted it. + expect(settlements.map((settlement) => settlement.clientMessageId)).toContain( + 'real-product-turn' + ) const started = await waitForHook(eventsPath, 'startup') const transcriptPath = String(started.transcript_path) transcripts.push(transcriptPath) diff --git a/src/main/codex/codex-structured-turn-start.ts b/src/main/codex/codex-structured-turn-start.ts index ffe4c850d5c..a24c5b6d5b8 100644 --- a/src/main/codex/codex-structured-turn-start.ts +++ b/src/main/codex/codex-structured-turn-start.ts @@ -7,6 +7,7 @@ import { } from './codex-app-server-connection' import { isCodexAppServerUnsupportedError } from './codex-app-server-session' import { readCodexTurnId } from './codex-structured-thread-facts' +import { DISPATCH_DOUBT_CODEX_TURN_UNNAMED } from '../native-chat/agent-session-journal/journal-dispatch-doubt-reasons' // Starting a Codex turn and learning its id, which are not the same event: // `turn/start` returns the id on newer builds and acks before it exists on @@ -115,7 +116,7 @@ export async function dispatchCodexTurn( throw error } return turnId === null - ? { state: 'unknown', reason: 'codex app-server started a turn it did not name in time' } + ? { state: 'unknown', reason: DISPATCH_DOUBT_CODEX_TURN_UNNAMED } : { state: 'accepted', providerIdentity: { diff --git a/src/main/ipc/runtime.ts b/src/main/ipc/runtime.ts index 71091e39243..ff55c8dbab2 100644 --- a/src/main/ipc/runtime.ts +++ b/src/main/ipc/runtime.ts @@ -13,6 +13,7 @@ import { TERMINAL_FIT_RESTORE_DEADLINE_MS } from '../../shared/terminal-fit-rest import { AGENT_SESSION_BACKGROUND_TASK_ROW_STOP_CAPABILITY, AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, + AGENT_SESSION_PENDING_SEND_RESULT_RUNTIME_CAPABILITY, AGENT_SESSION_TURN_ITEM_CAPABILITY, CLAUDE_STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY @@ -84,6 +85,7 @@ export function registerRuntimeHandlers(runtime: OrcaRuntimeService): void { connectionId: desktopSenders.connectionIdFor(event.sender), clientCapabilities: [ AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, + AGENT_SESSION_PENDING_SEND_RESULT_RUNTIME_CAPABILITY, AGENT_SESSION_TURN_ITEM_CAPABILITY, AGENT_SESSION_BACKGROUND_TASK_ROW_STOP_CAPABILITY, STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, @@ -135,6 +137,7 @@ export function registerRuntimeHandlers(runtime: OrcaRuntimeService): void { connectionId, clientCapabilities: [ AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, + AGENT_SESSION_PENDING_SEND_RESULT_RUNTIME_CAPABILITY, AGENT_SESSION_TURN_ITEM_CAPABILITY, AGENT_SESSION_BACKGROUND_TASK_ROW_STOP_CAPABILITY, STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, diff --git a/src/main/native-chat/agent-session-journal/journal-crash-boundary.test.ts b/src/main/native-chat/agent-session-journal/journal-crash-boundary.test.ts index 85cda398c1d..d9ca68de6fa 100644 --- a/src/main/native-chat/agent-session-journal/journal-crash-boundary.test.ts +++ b/src/main/native-chat/agent-session-journal/journal-crash-boundary.test.ts @@ -16,6 +16,10 @@ import type { AgentSessionJournalIdentity } from '../../../shared/agent-session-journal-types' import { hasUnansweredStructuredAgentSessionDispatch } from '../../../shared/structured-agent-session-projection' +import { + DISPATCH_DOUBT_RETRY_IN_PROGRESS, + dispatchDoubtProvesUndelivered +} from './journal-dispatch-doubt-reasons' import { digestPayload } from './journal-payload-bounds' import { reconcileSubmissions, @@ -166,6 +170,58 @@ describe('crash between provider accept and journal commit', () => { expect(restarted.cursor()).toEqual(cursor) }) + it('preserves a proven write failure while retiring its live dispatch', async () => { + const journal = await open() + await journal.appendSubmission({ + clientMessageId: 'cm_write_failed', + payloadFingerprint: digestPayload('safe to retry'), + body: userMessage('safe to retry'), + fence: 1 + }) + await journal.resolveDispatch({ + clientMessageId: 'cm_write_failed', + state: 'unknown', + reason: 'provider_write_failed: broken pipe', + fence: 1 + }) + + const restarted = await open() + await restarted.markPendingSubmissionsUnknown(2) + + expect(restarted.submissions()[0]).toMatchObject({ + dispatchState: 'unknown', + reason: 'provider_write_failed: broken pipe', + recovered: true + }) + expect(dispatchDoubtProvesUndelivered(restarted.submissions()[0]?.reason)).toBe(true) + }) + + it('turns an interrupted retry marker into recovery doubt', async () => { + const journal = await open() + await journal.appendSubmission({ + clientMessageId: 'cm_retrying', + payloadFingerprint: digestPayload('retry interrupted'), + body: userMessage('retry interrupted'), + fence: 1 + }) + await journal.resolveDispatch({ + clientMessageId: 'cm_retrying', + state: 'unknown', + reason: DISPATCH_DOUBT_RETRY_IN_PROGRESS, + fence: 1 + }) + + const restarted = await open() + await restarted.markPendingSubmissionsUnknown(2, 'provider_exited_before_acknowledgement') + + expect(restarted.submissions()[0]).toMatchObject({ + dispatchState: 'unknown', + reason: 'provider_exited_before_acknowledgement', + recovered: true + }) + expect(dispatchDoubtProvesUndelivered(restarted.submissions()[0]?.reason)).toBe(false) + }) + it('reports a rejected submission as never delivered, and never re-sends it', async () => { const journal = await open() await journal.appendSubmission({ diff --git a/src/main/native-chat/agent-session-journal/journal-dispatch-doubt-reasons.ts b/src/main/native-chat/agent-session-journal/journal-dispatch-doubt-reasons.ts new file mode 100644 index 00000000000..3ac02fc89df --- /dev/null +++ b/src/main/native-chat/agent-session-journal/journal-dispatch-doubt-reasons.ts @@ -0,0 +1,74 @@ +// Why a submission is in doubt, and whether Orca may put the message on the +// wire a second time. + +import type { AgentJournalDispatchState } from '../../../shared/agent-session-journal-types' +// +// `unknown` is never raised by elapsed time; what survives is a process fact. +// But a process fact that ends the WAIT is not the same claim as one that +// proves the message never reached a provider, and only the second justifies a +// re-delivery. The allowlist below names the reasons that carry the stronger +// claim, and it is deliberately FAIL-CLOSED: a reason nobody adds to it is +// refused. Refusing a legitimate retry costs the user one re-typed message; +// allowing an illegitimate one silently sends the model a second copy, which is +// the harm this whole path exists to remove. When those two are in tension, +// choose the re-type. + +/** A previous process wrote the message and died before learning its outcome. */ +export const DISPATCH_DOUBT_HOST_RESTARTED = 'host_restarted_before_acknowledgement' + +/** The child that would have acknowledged the message exited first. */ +export const DISPATCH_DOUBT_PROVIDER_EXITED = 'provider_exited_before_acknowledgement' + +/** The adapter took the message and only the journal write failed after it. */ +export const DISPATCH_DOUBT_PERSISTENCE_FAILED = 'dispatch_result_persistence_failed' + +/** A retry was durably armed but had not yet recorded its dispatch outcome. */ +export const DISPATCH_DOUBT_RETRY_IN_PROGRESS = 'dispatch_retry_in_progress' + +/** Codex owns a turn it started but did not name, because its turn-start still + * settles on a deadline. Delete this once Codex settles on the app-server's + * turn-start response instead; until then this reason is never re-delivered, + * which is what the allowlist below already does by omitting it. */ +export const DISPATCH_DOUBT_CODEX_TURN_UNNAMED = + 'codex app-server started a turn it did not name in time' + +/** The transport refused the frame; the underlying error follows the colon. */ +export const DISPATCH_DOUBT_WRITE_FAILED = 'provider_write_failed' + +/** The SDK took the frame, but its input pump did not prove whether the write completed. */ +export const DISPATCH_DOUBT_WRITE_OUTCOME_UNKNOWN = 'provider_write_outcome_unknown' + +export function dispatchWriteFailureReason(error: unknown): string { + const detail = error instanceof Error ? error.message : String(error) + return `${DISPATCH_DOUBT_WRITE_FAILED}: ${detail}` +} + +export function dispatchWriteOutcomeUnknownReason(error: unknown): string { + const detail = error instanceof Error ? error.message : String(error) + return `${DISPATCH_DOUBT_WRITE_OUTCOME_UNKNOWN}: ${detail}` +} + +/** + * The allowlist. True only where the frame is known never to have been taken by + * a provider, so sending it again is a first delivery rather than a second. + * + * A dead child and a dead host are NOT on this list. Both end the wait, neither + * proves non-delivery: the message was already written to that child's stdin, + * and Claude resumes the same provider session by id, so a message that child + * processed before dying is in the conversation Orca resumes. Deciding those + * needs the message matched against provider history — which is exactly what + * `journal-submission-reconciler.ts` does, and that module has no caller yet. + */ +export function dispatchDoubtProvesUndelivered(reason: string | null | undefined): boolean { + return ( + reason === DISPATCH_DOUBT_WRITE_FAILED || + reason?.startsWith(`${DISPATCH_DOUBT_WRITE_FAILED}: `) === true + ) +} + +export function dispatchMayMatchProviderEcho( + state: AgentJournalDispatchState, + reason: string | null +): boolean { + return state !== 'rejected' && !(state === 'unknown' && dispatchDoubtProvesUndelivered(reason)) +} diff --git a/src/main/native-chat/agent-session-journal/journal-item-revision.ts b/src/main/native-chat/agent-session-journal/journal-item-revision.ts new file mode 100644 index 00000000000..14b45368886 --- /dev/null +++ b/src/main/native-chat/agent-session-journal/journal-item-revision.ts @@ -0,0 +1,14 @@ +import type { JournalReducerState } from './journal-reducer' + +export function journalItemRevisionIsStale( + state: JournalReducerState, + itemId: string, + revision: number +): boolean { + const tombstoned = state.tombstones.get(itemId) + const existing = state.items.get(itemId) + return ( + (tombstoned !== undefined && revision <= tombstoned) || + (existing !== undefined && revision <= existing.revision) + ) +} 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 e09dd109d50..c0a2bee5431 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 @@ -1,26 +1,37 @@ +import { + DISPATCH_DOUBT_HOST_RESTARTED, + DISPATCH_DOUBT_RETRY_IN_PROGRESS +} from './journal-dispatch-doubt-reasons' import type { AgentSessionJournal } from './journal-store' +/** Settles every submission a process fact left unanswerable. The retry policy + * separately decides whether that fact proves the provider never received it. */ export async function markJournalPendingSubmissionsUnknown( journal: AgentSessionJournal, fence: number, - reason = 'host_restarted_before_acknowledgement' + reason: string = DISPATCH_DOUBT_HOST_RESTARTED ): Promise { - const pending = journal + const unresolved = journal .submissions() .filter( (entry) => entry.dispatchState === 'pending' || (entry.dispatchState === 'unknown' && entry.recovered !== true) ) - .map((entry) => entry.clientMessageId) - for (const clientMessageId of pending) { + for (const entry of unresolved) { + const resolvedReason = + entry.dispatchState === 'unknown' && + entry.reason !== null && + entry.reason !== DISPATCH_DOUBT_RETRY_IN_PROGRESS + ? entry.reason + : reason await journal.resolveDispatch({ - clientMessageId, + clientMessageId: entry.clientMessageId, state: 'unknown', - reason, + reason: resolvedReason, fence, recovered: true }) } - return pending + return unresolved.map((entry) => entry.clientMessageId) } diff --git a/src/main/native-chat/agent-session-journal/journal-reducer.test.ts b/src/main/native-chat/agent-session-journal/journal-reducer.test.ts index 255e4ed184c..bbe78e41b64 100644 --- a/src/main/native-chat/agent-session-journal/journal-reducer.test.ts +++ b/src/main/native-chat/agent-session-journal/journal-reducer.test.ts @@ -216,6 +216,106 @@ describe('submission and dispatch state machine', () => { expect(items[0]?.revision).toBe(1) }) + it('durably accepts a pending submission from the provider echo row itself', () => { + const body = userText('hi') + const state = fold([ + { ...submission, payloadFingerprint: sendFingerprint(body) }, + { + kind: 'item', + itemId: 'claude:session-1:user-1', + revision: 1, + body, + ...base(2) + } + ]) + + expect(state.submissions.get('cm_1')).toMatchObject({ + dispatchState: 'accepted', + providerItemId: 'claude:session-1:user-1', + resolvedAt: 1_002 + }) + expect(state.receipts.get('cm_1')).toMatchObject({ + providerItemId: 'claude:session-1:user-1', + cursor: { epoch: EPOCH, sequence: 2 } + }) + }) + + it('does not give a newer identical echo to an older proven-undelivered submission', () => { + const body = userText('same message') + const state = fold([ + { + ...submission, + body, + payloadFingerprint: sendFingerprint(body) + }, + { + kind: 'dispatch', + clientMessageId: 'cm_1', + state: 'unknown', + providerItemId: null, + reason: 'provider_write_failed: closed before enqueue', + ...base(2) + }, + { + ...submission, + clientMessageId: 'cm_2', + body, + payloadFingerprint: sendFingerprint(body), + ...base(3) + }, + { + kind: 'item', + itemId: 'claude:session-1:user-1', + revision: 1, + body, + ...base(4) + } + ]) + + expect(state.submissions.get('cm_1')?.dispatchState).toBe('unknown') + expect(state.submissions.get('cm_2')).toMatchObject({ + dispatchState: 'accepted', + providerItemId: 'claude:session-1:user-1' + }) + expect(state.receipts.has('cm_1')).toBe(false) + expect(state.receipts.get('cm_2')?.providerItemId).toBe('claude:session-1:user-1') + }) + + it('does not accept a submission from a stale provider item behind its tombstone', () => { + const body = userText('hi') + const providerItemId = 'claude:session-1:user-1' + const state = fold([ + { ...submission, payloadFingerprint: sendFingerprint(body) }, + { kind: 'tombstone', itemId: providerItemId, revision: 2, ...base(2) }, + { kind: 'item', itemId: providerItemId, revision: 1, body, ...base(3) } + ]) + + expect(state.submissions.get('cm_1')?.dispatchState).toBe('pending') + expect(state.receipts.has('cm_1')).toBe(false) + expect(state.aliases.has(providerItemId)).toBe(false) + }) + + it('does not accept a submission from a stale lifecycle item behind its tombstone', () => { + const body = userText('hi') + const providerItemId = 'claude:session-1:user-1' + const state = fold([ + { ...submission, payloadFingerprint: sendFingerprint(body) }, + { + kind: 'lifecycle-batch', + settlementId: 'settlement-1', + mutations: [ + { kind: 'tombstone', itemId: providerItemId, revision: 2 }, + { kind: 'item', itemId: providerItemId, revision: 1, body } + ], + ...base(2) + } + ]) + + expect(state.submissions.get('cm_1')?.dispatchState).toBe('pending') + expect(state.receipts.has('cm_1')).toBe(false) + expect(state.aliases.has(providerItemId)).toBe(false) + }) + it.each(['codex:thread-1:turn-1:0', 'claude:session-1:user-1'])( 'preserves submitted text and attachments when %s is restored', (providerItemId) => { @@ -384,6 +484,36 @@ describe('submission and dispatch state machine', () => { expect(state.receipts.get('cm_1')).toBeTruthy() }) + it('returns a proven retry to pending without moving its original submission', () => { + const state = fold([ + submission, + { + kind: 'dispatch', + clientMessageId: 'cm_1', + state: 'unknown', + providerItemId: null, + reason: 'provider_write_failed: closed before enqueue', + ...base(2) + }, + { + kind: 'dispatch', + clientMessageId: 'cm_1', + state: 'pending', + providerItemId: null, + reason: null, + ...base(3) + } + ]) + + expect(state.submissions.get('cm_1')).toMatchObject({ + dispatchState: 'pending', + submittedAt: submission.ts, + reason: null, + resolvedAt: null + }) + expect(renderJournalState(state).items[0]?.sequence).toBe(submission.seq) + }) + it('ignores a dispatch for a submission this epoch never saw', () => { const state = fold([ { diff --git a/src/main/native-chat/agent-session-journal/journal-reducer.ts b/src/main/native-chat/agent-session-journal/journal-reducer.ts index e01e7d6158f..9759f449f2e 100644 --- a/src/main/native-chat/agent-session-journal/journal-reducer.ts +++ b/src/main/native-chat/agent-session-journal/journal-reducer.ts @@ -18,6 +18,8 @@ import { parseAgentJournalItemKey } from '../../../shared/agent-session-journal-item-key' import { structuredAgentSessionPayloadFingerprint } from '../../../shared/structured-agent-session-mutation' +import { dispatchMayMatchProviderEcho } from './journal-dispatch-doubt-reasons' +import { journalItemRevisionIsStale } from './journal-item-revision' import type { JournalRow } from './journal-row-schema' export const MAX_JOURNAL_APPLIED_SETTLEMENT_IDS = 4_096 @@ -66,7 +68,11 @@ export function applyJournalRow(state: JournalReducerState, row: JournalRow): vo } state.lastActivityAt = Math.max(state.lastActivityAt, row.ts) if (row.kind === 'item') { + if (journalItemRevisionIsStale(state, row.itemId, row.revision)) { + return + } const itemId = resolveJournalItemId(state, row.itemId, row.body) + acceptSubmissionFromProviderItem(state, row.itemId, itemId, row) upsertItem(state, itemId, row.revision, { itemId, revision: row.revision, @@ -87,7 +93,11 @@ export function applyJournalRow(state: JournalReducerState, row: JournalRow): vo } for (const mutation of row.mutations) { if (mutation.kind === 'item') { + if (journalItemRevisionIsStale(state, mutation.itemId, mutation.revision)) { + continue + } const itemId = resolveJournalItemId(state, mutation.itemId, mutation.body) + acceptSubmissionFromProviderItem(state, mutation.itemId, itemId, row) upsertItem(state, itemId, mutation.revision, { itemId, revision: mutation.revision, @@ -151,12 +161,12 @@ export function resolveJournalItemId( // Exact payload plus queue order preserves repeated identical sends one-for-one. const submission = [...state.submissions.values()] .sort((left, right) => left.submittedAt - right.submittedAt) - .find((candidate) => { - if (candidate.dispatchState === 'rejected' || candidate.payloadFingerprint !== fingerprint) { - return false - } - return state.items.get(agentJournalSubmissionKey(candidate.clientMessageId))?.revision === 0 - }) + .find( + (candidate) => + dispatchMayMatchProviderEcho(candidate.dispatchState, candidate.reason) && + candidate.payloadFingerprint === fingerprint && + state.items.get(agentJournalSubmissionKey(candidate.clientMessageId))?.revision === 0 + ) if (!submission) { return itemId } @@ -260,7 +270,7 @@ function applyDispatch( submission.dispatchState = row.state submission.providerItemId = row.providerItemId submission.reason = row.reason - submission.resolvedAt = row.ts + submission.resolvedAt = row.state === 'pending' ? null : row.ts if (row.recovered) { submission.recovered = row.recovered } else { @@ -278,6 +288,39 @@ function applyDispatch( }) } +function acceptSubmissionFromProviderItem( + state: JournalReducerState, + providerItemId: string, + resolvedItemId: string, + row: Pick +): void { + if (providerItemId === resolvedItemId) { + return + } + const submission = [...state.submissions.values()].find( + (candidate) => agentJournalSubmissionKey(candidate.clientMessageId) === resolvedItemId + ) + if ( + !submission || + submission.dispatchState === 'accepted' || + submission.dispatchState === 'rejected' + ) { + return + } + submission.fence = row.fence + submission.dispatchState = 'accepted' + submission.providerItemId = providerItemId + submission.reason = null + submission.resolvedAt = row.ts + delete submission.recovered + state.receipts.set(submission.clientMessageId, { + clientMessageId: submission.clientMessageId, + providerItemId, + cursor: { epoch: row.epoch, sequence: row.seq }, + acceptedAt: row.ts + }) +} + /** Project the folded state into the client-facing snapshot. */ export function renderJournalState(state: JournalReducerState): AgentJournalSnapshot { // Sequence is the sole ordering key; map insertion order is not, because a diff --git a/src/main/native-chat/agent-session-journal/journal-row-builders.ts b/src/main/native-chat/agent-session-journal/journal-row-builders.ts index d7e46e23663..be2c2552775 100644 --- a/src/main/native-chat/agent-session-journal/journal-row-builders.ts +++ b/src/main/native-chat/agent-session-journal/journal-row-builders.ts @@ -76,7 +76,8 @@ export function journalDispatchRowBuilder( clientMessageId: input.clientMessageId, dispatchState: input.state, providerItemId, - reason: input.state === 'accepted' ? null : (input.reason ?? null), + reason: + input.state === 'accepted' || input.state === 'pending' ? null : (input.reason ?? null), seq, fence: input.fence, ts, @@ -219,7 +220,7 @@ export function buildJournalSubmissionRow(input: { export function buildJournalDispatchRow(input: { state: JournalReducerState clientMessageId: string - dispatchState: Exclude + dispatchState: AgentJournalDispatchState providerItemId: string | null reason: string | null seq: number diff --git a/src/main/native-chat/agent-session-journal/journal-row-schema.ts b/src/main/native-chat/agent-session-journal/journal-row-schema.ts index dd8b0ce9f3e..7dc02dd197b 100644 --- a/src/main/native-chat/agent-session-journal/journal-row-schema.ts +++ b/src/main/native-chat/agent-session-journal/journal-row-schema.ts @@ -74,7 +74,7 @@ export type JournalSubmissionRow = JournalRowBase & { export type JournalDispatchRow = JournalRowBase & { kind: 'dispatch' clientMessageId: string - state: Exclude + state: AgentJournalDispatchState /** Provider item identity adopted on accept. */ providerItemId: string | null reason: string | null diff --git a/src/main/native-chat/agent-session-journal/journal-store-contracts.ts b/src/main/native-chat/agent-session-journal/journal-store-contracts.ts index 22e3a4c7cca..80c806b02e3 100644 --- a/src/main/native-chat/agent-session-journal/journal-store-contracts.ts +++ b/src/main/native-chat/agent-session-journal/journal-store-contracts.ts @@ -29,6 +29,7 @@ export type ResolveDispatchInput = { recovered?: true } & ( | { state: 'accepted'; providerIdentity: AgentJournalItemIdentity } + | { state: 'pending' } | { state: 'rejected' | 'unknown'; reason?: string | null } ) 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 812e2bca834..3be64d9ceca 100644 --- a/src/main/native-chat/agent-session-journal/journal-store.ts +++ b/src/main/native-chat/agent-session-journal/journal-store.ts @@ -232,7 +232,7 @@ export class AgentSessionJournal { } /** - * Advance a submission to exactly one of accepted / rejected / unknown. + * Record a dispatch transition, including a proven retry returning to pending. * * Accepting REQUIRES the provider identity rather than a free-form id: the * adopted key is what the provider's echo will upsert into, so a mismatched diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts index 50e4a704fbf..3bd14a057aa 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts @@ -91,6 +91,13 @@ export function isAgentSessionPreSpawnError(error: unknown): error is AgentSessi export type AgentSessionDispatchOutcome = /** The provider owns the turn now, under this identity. */ | { state: 'accepted'; providerIdentity: AgentJournalItemIdentity } + /** + * The provider transport took the message; identity settles later, out of band. + * The submission stays `pending`: a message queued behind a running turn is + * acknowledged only when that turn starts, so elapsed time is not evidence of + * anything and never promotes this to `unknown`. + */ + | { state: 'admitted' } | { state: 'rejected'; reason: string } /** The call did not settle. Never re-send on the user's behalf. */ | { state: 'unknown'; reason: string } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-client-delivery.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-client-delivery.ts new file mode 100644 index 00000000000..92a4f2653e0 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-client-delivery.ts @@ -0,0 +1,75 @@ +import type { AgentSessionJournal } from '../agent-session-journal/journal-store' +import { AgentSessionSubscribers } from './structured-agent-session-subscribers' +import type { + StructuredAgentSessionHostDeps, + StructuredAgentSessionHostSession +} from './structured-agent-session-host-types' +import { AGENT_SESSION_NOT_ATTACHED } from './structured-agent-session-mutation-admission' +import { StructuredAgentSessionSendSettlement } from './structured-agent-session-send-settlement' +import { + createStructuredAgentSessionHostStatusFeed, + type StructuredAgentSessionStatusSubscriber +} from './structured-agent-session-status-feed' + +/** Owns every host-to-client publication edge, including compatibility waits. */ +export class StructuredAgentSessionClientDelivery { + readonly subscribers: AgentSessionSubscribers + readonly waitForSendSettlement: StructuredAgentSessionSendSettlement['wait'] + private readonly statusFeed + private readonly sendSettlement + + constructor( + private readonly sessions: Map, + now: () => number, + deps: () => StructuredAgentSessionHostDeps + ) { + this.statusFeed = createStructuredAgentSessionHostStatusFeed({ sessions, now, deps }) + this.sendSettlement = new StructuredAgentSessionSendSettlement((sessionId) => + this.requireJournal(sessionId) + ) + this.waitForSendSettlement = this.sendSettlement.wait + this.subscribers = new AgentSessionSubscribers({ + readCommands: (sessionId) => deps().adapter.readCommands?.(sessionId), + onJournalPublished: (sessionId, journal) => this.publishJournal(sessionId, journal) + }) + } + + publishStatus = (sessionId: string): void => this.statusFeed.publish(sessionId) + + publishStatusAndSettlement = (sessionId: string): void => { + this.statusFeed.publish(sessionId) + const journal = this.sessions.get(sessionId)?.journal + if (journal) { + this.sendSettlement.publish(sessionId, journal) + } + } + + publishRestored = (sessionId: string): void => + this.statusFeed.publish(sessionId, undefined, { replay: true }) + + subscribeStatus = (subscriber: StructuredAgentSessionStatusSubscriber): (() => void) => + this.statusFeed.subscribe(subscriber) + forgetStatus = (sessionId: string): void => this.statusFeed.forget(sessionId) + + closeSession(sessionId: string): void { + this.sendSettlement.closeSession(sessionId) + this.statusFeed.close(sessionId) + } + + closeAll(): void { + this.sendSettlement.closeAll() + } + + private publishJournal(sessionId: string, journal: AgentSessionJournal): void { + this.statusFeed.publish(sessionId, journal) + this.sendSettlement.publish(sessionId, journal) + } + + private requireJournal(sessionId: string): AgentSessionJournal { + const journal = this.sessions.get(sessionId)?.journal + if (!journal) { + throw new Error(AGENT_SESSION_NOT_ATTACHED.code) + } + return journal + } +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-test-harness.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-test-harness.ts new file mode 100644 index 00000000000..c67a8fabf58 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-test-harness.ts @@ -0,0 +1,197 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, expect, vi, type Mock } from 'vitest' +import { computeAgentSessionPayloadFingerprint } from '../../../shared/agent-session-mutation-envelope' +import type { AgentSessionRecord } from '../../../shared/agent-session-record' +import type { AgentSessionMutationEnvelope } from '../../../shared/agent-session-wire' +import { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' +import { journalDirectoryFor } from '../agent-session-journal/journal-paths' +import { createTrackedJournalOpener } from '../agent-session-journal/journal-store-test-open' +import type { + AgentSessionDispatchOutcome, + StructuredAgentSessionAdapter +} from './structured-agent-session-adapter' +import type { AgentSessionAttachParams } from './structured-agent-session-attach' +import { StructuredAgentSessionHost } from './structured-agent-session-host' +import { + HOST_TEST_NOW as NOW, + HOST_TEST_SESSION as SESSION, + HOST_TEST_THREAD as THREAD, + hostTestAttachParams, + hostTestOperationId, + resetHostTestOperationIds +} from './structured-agent-session-host-test-data' + +const journals = createTrackedJournalOpener() + +const CALLER = { callerKey: 'client-1' } + +function envelope( + method: string, + fields: Record, + overrides: Partial = {} +): AgentSessionMutationEnvelope { + return { + sessionId: SESSION, + clientOperationId: hostTestOperationId(), + expectedRuntimeFence: store.getRecord(SESSION)?.lease.runtimeFence ?? 1, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method, + sessionId: SESSION, + fields + }), + ...overrides + } +} + +const attachParams = ( + overrides: Partial = {} +): AgentSessionAttachParams => hostTestAttachParams(null, overrides) + +const ensureParams = (fence: number): AgentSessionAttachParams => hostTestAttachParams(fence) + +let root: string +let store: AgentSessionRecordStore +let host: StructuredAgentSessionHost +let acquire: Mock +let releaseAcquisition: Mock> +let dispatch: Mock +let cancelTurn: Mock +let answerPrompt: Mock +let setOption: Mock +let ordinal = 0 + +function accepted(): AgentSessionDispatchOutcome { + ordinal += 1 + return { + state: 'accepted', + providerIdentity: { provider: 'codex', threadId: THREAD, turnId: 'turn-1', ordinal } + } +} + +function adapter(): StructuredAgentSessionAdapter { + return { + acquire, + releaseAcquisition, + dispatch, + cancelTurn, + answerPrompt, + setOption + } +} + +async function attach(): Promise { + const result = await host.attach(CALLER, attachParams()) + expect(result.ok).toBe(true) + return store.getRecord(SESSION) +} + +/** Puts a pending approval in the journal BEFORE attach, which is the only way + * 1d can stage one: the adapter that would emit it is phase 2's. */ +async function seedApproval(optionId = 'allow'): Promise<{ itemId: string; revision: number }> { + const identity = { provider: 'codex' as const, threadId: THREAD, turnId: 'turn-1', ordinal: 99 } + const journalDir = journalDirectoryFor(root, { workspaceId: 'workspace-1', sessionId: SESSION }) + const journal = await journals.open({ + identity: { + sessionId: SESSION, + workspaceId: 'workspace-1', + hostId: 'local', + agent: 'codex', + providerHandle: { kind: 'codex', threadId: THREAD } + }, + journalDir + }) + const appended = await journal.appendItem( + identity, + { + kind: 'approval', + title: 'Run the command?', + detail: null, + options: [{ id: optionId, label: 'Allow' }], + resolution: { state: 'pending', selectedOptionId: null, resolvedBy: null, resolvedAt: null } + }, + { fence: 1 } + ) + return { itemId: appended.itemId, revision: appended.revision } +} + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-wire-host-')) + resetHostTestOperationIds() + ordinal = 0 + acquire = vi.fn(async ({ fence }) => ({ + process: { + hostId: 'local', + pid: 4242, + processStartTimeMs: 1_700_000_000_000, + spawnToken: store.getRecord(SESSION)?.lease.reservedSpawnToken ?? 'spawn-a' + }, + link: { + linkId: `link-${fence}`, + handle: { provider: 'codex', threadId: THREAD }, + origin: store.getRecord(SESSION)?.providerHandleChain.length ? 'resumed' : 'created', + mintedAtFence: fence, + observedAt: NOW + } + })) + releaseAcquisition = vi.fn(async () => true) + dispatch = vi.fn(async () => accepted()) + cancelTurn = vi.fn(async () => ({ cancelled: true })) + answerPrompt = vi.fn(async () => undefined) + setOption = vi.fn(async () => undefined) + store = await AgentSessionRecordStore.open({ directory: join(root, 'store'), hostId: 'local' }) + host = new StructuredAgentSessionHost({ + store, + adapter: adapter(), + journalRoot: root, + claimKeyId: 'key-1', + mintSpawnToken: () => 'spawn-a', + now: () => NOW + }) +}) + +afterEach(async () => { + await journals.closeAll() + await host.flushAllStreamedEvents() + await rm(root, { recursive: true, force: true }) +}) + +/** A restarted process swaps the store and the host under the same directories. + * The helpers here close over both, so they have to be told. */ +export function replaceHostTestState(next: { + store: AgentSessionRecordStore + host: StructuredAgentSessionHost +}): void { + store = next.store + host = next.host +} + +/** The live per-test state. Read it in a `beforeEach` so a suite's test bodies + * keep using bare `host` / `store` / `dispatch` exactly as they did when this + * setup was inline. */ +export function hostTestState() { + return { + root, + store, + host, + acquire, + releaseAcquisition, + dispatch, + cancelTurn, + answerPrompt, + setOption + } +} + +export { + CALLER, + accepted, + adapter, + attach, + attachParams, + ensureParams, + envelope, + journals, + seedApproval +} 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 e7d389633f3..d53c3c30e50 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 @@ -1,63 +1,31 @@ -import { mkdtemp, rm } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import { beforeEach, describe, expect, it, vi, type Mock } from 'vitest' import { agentJournalItemKey } from '../../../shared/agent-session-journal-item-key' import type { AgentSessionOwnerProbe } from '../../../shared/agent-session-lease-adjudication' -import { computeAgentSessionPayloadFingerprint } from '../../../shared/agent-session-mutation-envelope' import type { AgentSessionRecord } from '../../../shared/agent-session-record' -import type { - AgentSessionMutationEnvelope, - AgentSessionSubscribeEvent -} from '../../../shared/agent-session-wire' +import type { AgentSessionSubscribeEvent } from '../../../shared/agent-session-wire' +import { join } from 'node:path' import { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' -import { journalDirectoryFor } from '../agent-session-journal/journal-paths' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' -import { createTrackedJournalOpener } from '../agent-session-journal/journal-store-test-open' -import type { - AgentSessionDispatchOutcome, - StructuredAgentSessionAdapter -} from './structured-agent-session-adapter' -import type { AgentSessionAttachParams } from './structured-agent-session-attach' +import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' import { StructuredAgentSessionHost } from './structured-agent-session-host' +import { + adapter, + attach, + attachParams, + CALLER, + ensureParams, + envelope, + hostTestState, + replaceHostTestState, + seedApproval +} from './structured-agent-session-host-test-harness' import { HOST_TEST_NOW as NOW, HOST_TEST_SESSION as SESSION, HOST_TEST_THREAD as THREAD, - hostTestAttachParams, - hostTestMessage, - hostTestOperationId, - resetHostTestOperationIds + hostTestMessage } from './structured-agent-session-host-test-data' -const journals = createTrackedJournalOpener() - -const CALLER = { callerKey: 'client-1' } - -function envelope( - method: string, - fields: Record, - overrides: Partial = {} -): AgentSessionMutationEnvelope { - return { - sessionId: SESSION, - clientOperationId: hostTestOperationId(), - expectedRuntimeFence: store.getRecord(SESSION)?.lease.runtimeFence ?? 1, - payloadFingerprint: computeAgentSessionPayloadFingerprint({ - method, - sessionId: SESSION, - fields - }), - ...overrides - } -} - -const attachParams = ( - overrides: Partial = {} -): AgentSessionAttachParams => hostTestAttachParams(null, overrides) - -const ensureParams = (fence: number): AgentSessionAttachParams => hostTestAttachParams(fence) - let root: string let store: AgentSessionRecordStore let host: StructuredAgentSessionHost @@ -67,101 +35,19 @@ let dispatch: Mock let cancelTurn: Mock let answerPrompt: Mock let setOption: Mock -let ordinal = 0 -function accepted(): AgentSessionDispatchOutcome { - ordinal += 1 - return { - state: 'accepted', - providerIdentity: { provider: 'codex', threadId: THREAD, turnId: 'turn-1', ordinal } - } -} - -function adapter(): StructuredAgentSessionAdapter { - return { +beforeEach(() => { + ;({ + root, + store, + host, acquire, releaseAcquisition, dispatch, cancelTurn, answerPrompt, setOption - } -} - -async function attach(): Promise { - const result = await host.attach(CALLER, attachParams()) - expect(result.ok).toBe(true) - return store.getRecord(SESSION) -} - -/** Puts a pending approval in the journal BEFORE attach, which is the only way - * 1d can stage one: the adapter that would emit it is phase 2's. */ -async function seedApproval(optionId = 'allow'): Promise<{ itemId: string; revision: number }> { - const identity = { provider: 'codex' as const, threadId: THREAD, turnId: 'turn-1', ordinal: 99 } - const journalDir = journalDirectoryFor(root, { workspaceId: 'workspace-1', sessionId: SESSION }) - const journal = await journals.open({ - identity: { - sessionId: SESSION, - workspaceId: 'workspace-1', - hostId: 'local', - agent: 'codex', - providerHandle: { kind: 'codex', threadId: THREAD } - }, - journalDir - }) - const appended = await journal.appendItem( - identity, - { - kind: 'approval', - title: 'Run the command?', - detail: null, - options: [{ id: optionId, label: 'Allow' }], - resolution: { state: 'pending', selectedOptionId: null, resolvedBy: null, resolvedAt: null } - }, - { fence: 1 } - ) - return { itemId: appended.itemId, revision: appended.revision } -} - -beforeEach(async () => { - root = await mkdtemp(join(tmpdir(), 'orca-wire-host-')) - resetHostTestOperationIds() - ordinal = 0 - acquire = vi.fn(async ({ fence }) => ({ - process: { - hostId: 'local', - pid: 4242, - processStartTimeMs: 1_700_000_000_000, - spawnToken: store.getRecord(SESSION)?.lease.reservedSpawnToken ?? 'spawn-a' - }, - link: { - linkId: `link-${fence}`, - handle: { provider: 'codex', threadId: THREAD }, - origin: store.getRecord(SESSION)?.providerHandleChain.length ? 'resumed' : 'created', - mintedAtFence: fence, - observedAt: NOW - } - })) - releaseAcquisition = vi.fn(async () => true) - dispatch = vi.fn(async () => accepted()) - cancelTurn = vi.fn(async () => ({ cancelled: true })) - answerPrompt = vi.fn(async () => undefined) - setOption = vi.fn(async () => undefined) - store = await AgentSessionRecordStore.open({ directory: join(root, 'store'), hostId: 'local' }) - host = new StructuredAgentSessionHost({ - store, - adapter: adapter(), - journalRoot: root, - claimKeyId: 'key-1', - mintSpawnToken: () => 'spawn-a', - now: () => NOW - }) -}) - -afterEach(async () => { - await journals.closeAll() - await host.flushAllStreamedEvents() - await rm(root, { recursive: true, force: true }) + } = hostTestState()) }) describe('attach', () => { @@ -308,152 +194,6 @@ describe('attach', () => { }) }) -describe('send', () => { - it('writes the submission before dispatching and resolves it accepted', async () => { - await attach() - const body = hostTestMessage('add a retry') - const result = await host.send(CALLER, { - envelope: envelope('agentSession.send', { body }), - body - }) - if (!result.ok) { - throw new Error(`expected a send, got ${result.refusal.code}`) - } - expect(result.value.submission.dispatchState).toBe('accepted') - expect(dispatch).toHaveBeenCalledTimes(1) - const page = host.history({ sessionId: SESSION, direction: 'tail' }) - expect(page.ok && page.page.items).toHaveLength(1) - expect(page.ok && page.page.fence).toBe(1) - // The injected host clock, so a client can anchor a live counter on it. - expect(page.page.hostNow).toBe(NOW) - expect(page.providerSession).toEqual({ key: 'session_id', id: THREAD }) - }) - - it('settles a thrown dispatch as unknown, never as a rejection', async () => { - await attach() - dispatch.mockRejectedValueOnce(new Error('socket closed')) - const body = hostTestMessage('add a retry') - const result = await host.send(CALLER, { - envelope: envelope('agentSession.send', { body }), - body - }) - expect(result).toMatchObject({ ok: true, value: { submission: { dispatchState: 'unknown' } } }) - }) - - it('replays a retried send from the journal without dispatching twice', async () => { - await attach() - const body = hostTestMessage('add a retry') - const params = { envelope: envelope('agentSession.send', { body }), body } - await host.send(CALLER, params) - const retry = await host.send(CALLER, params) - expect(retry).toMatchObject({ ok: true, replayed: true }) - expect(dispatch).toHaveBeenCalledTimes(1) - }) - - it('redispatches an explicitly retried durable unknown without appending a second submission', async () => { - await attach() - dispatch - .mockRejectedValueOnce(new Error('socket closed')) - .mockImplementationOnce(async () => accepted()) - const body = hostTestMessage('possibly delivered') - const params = { envelope: envelope('agentSession.send', { body }), body } - - const first = await host.send(CALLER, params) - expect(first).toMatchObject({ - ok: true, - value: { submission: { dispatchState: 'unknown' } } - }) - const retried = await host.send(CALLER, { ...params, retryUnknown: true }) - - expect(retried).toMatchObject({ - ok: true, - replayed: false, - value: { submission: { dispatchState: 'accepted' } } - }) - expect(dispatch).toHaveBeenCalledTimes(2) - const state = host.history({ sessionId: SESSION, direction: 'tail' }) - expect(state.ok && state.page.submissions).toHaveLength(1) - }) - - it('advances an explicit retry after a ledger-unknown send is reconciled in the journal', async () => { - await attach() - const journal = ( - host as unknown as { sessions: Map } - ).sessions.get(SESSION)!.journal - vi.spyOn(journal, 'resolveDispatch').mockRejectedValueOnce(new Error('journal resolve failed')) - const body = hostTestMessage('possibly delivered before persistence failed') - const params = { envelope: envelope('agentSession.send', { body }), body } - - await expect(host.send(CALLER, params)).rejects.toThrow('journal resolve failed') - expect(journal.submissions()).toMatchObject([ - { clientMessageId: params.envelope.clientOperationId, dispatchState: 'unknown' } - ]) - expect( - store.listOperationRows().find((row) => row.operationId === params.envelope.clientOperationId) - ?.outcome - ).toEqual({ status: 'unknown' }) - expect(dispatch).toHaveBeenCalledTimes(1) - - await journal.markPendingSubmissionsUnknown(store.getRecord(SESSION)?.lease.runtimeFence ?? 1) - await expect(host.send(CALLER, params)).resolves.toMatchObject({ - ok: false, - refusal: { code: 'agent_session_operation_unknown' } - }) - expect(dispatch).toHaveBeenCalledTimes(1) - - await expect(host.send(CALLER, { ...params, retryUnknown: true })).resolves.toMatchObject({ - ok: true, - replayed: false, - value: { submission: { dispatchState: 'accepted' } } - }) - expect(dispatch).toHaveBeenCalledTimes(2) - expect(journal.submissions()).toHaveLength(1) - }) - - it('refuses a stale fence and hands back the current one', async () => { - const record = await attach() - const body = hostTestMessage('add a retry') - const result = await host.send(CALLER, { - envelope: envelope( - 'agentSession.send', - { body }, - { expectedRuntimeFence: (record?.lease.runtimeFence ?? 1) + 5 } - ), - body - }) - expect(result).toMatchObject({ - ok: false, - refusal: { code: 'agent_session_checkpoint_stale', currentFence: record?.lease.runtimeFence } - }) - }) - - it('does not let a refused call leave a ledger row that replays past the fence', async () => { - const record = await attach() - const body = hostTestMessage('add a retry') - const params = { - envelope: envelope( - 'agentSession.send', - { body }, - { expectedRuntimeFence: (record?.lease.runtimeFence ?? 1) + 5 } - ), - body - } - await host.send(CALLER, params) - expect(await host.send(CALLER, params)).toMatchObject({ - ok: false, - refusal: { code: 'agent_session_checkpoint_stale' } - }) - expect(dispatch).not.toHaveBeenCalled() - }) - - it('refuses any mutation against a session this host has not attached', async () => { - const body = hostTestMessage('add a retry') - expect( - await host.send(CALLER, { envelope: envelope('agentSession.send', { body }), body }) - ).toMatchObject({ ok: false, refusal: { code: 'agent_session_ownership_unknown' } }) - }) -}) - describe('cancel', () => { it('records the request acknowledgement as a status item keyed by the operation id', async () => { await attach() @@ -665,6 +405,7 @@ describe('restart', () => { probeOwner, now: () => NOW }) + replaceHostTestState({ store, host }) } /** The refusal a restarted host owes a client holding the dead generation's 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 06a1e773e6e..176cd66b674 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 @@ -10,10 +10,7 @@ import type * as SessionWire from '../../../shared/agent-session-wire' import type { AgentSessionAttachParams } from './structured-agent-session-attach' import { AGENT_SESSION_NOT_ATTACHED } from './structured-agent-session-mutation-admission' import { createRestartReconciler } from './structured-agent-session-restart-reconcile' -import { - AgentSessionSubscribers, - type AgentSessionSubscribeInput -} from './structured-agent-session-subscribers' +import type { AgentSessionSubscribeInput } from './structured-agent-session-subscribers' import { StructuredAgentSessionTaskQueue } from './structured-agent-session-task-queue' import * as providerSupport from './structured-agent-session-provider-support' import { createStructuredAgentSessionHostRestore } from './structured-agent-session-reveal' @@ -50,10 +47,10 @@ import type { StructuredAgentSessionHostSession, StructuredAgentSessionReveal } from './structured-agent-session-host-types' -import { createStructuredAgentSessionHostStatusFeed } from './structured-agent-session-status-feed' import type { StructuredAgentSessionStatusSubscriber } from './structured-agent-session-status-feed' import { StructuredAgentSessionEventRecovery } from './structured-agent-session-event-recovery' import { StructuredAgentSessionBackgroundTaskChannel } from './structured-agent-session-background-task-channel' +import { StructuredAgentSessionClientDelivery } from './structured-agent-session-client-delivery' export type { StructuredAgentSessionHostDeps } from './structured-agent-session-host-types' export class StructuredAgentSessionHost { @@ -62,16 +59,12 @@ export class StructuredAgentSessionHost { this ) private readonly sessions = new Map() - private readonly statusFeed = createStructuredAgentSessionHostStatusFeed({ - sessions: this.sessions, - now: () => this.now(), - deps: () => this.deps - }) - private readonly subscribers = new AgentSessionSubscribers({ - readCommands: (sessionId) => this.deps.adapter.readCommands?.(sessionId), - onJournalPublished: (sessionId, journal) => this.statusFeed.publish(sessionId, journal), - now: () => this.now() - }) + private readonly clientDelivery = new StructuredAgentSessionClientDelivery( + this.sessions, + () => this.now(), + () => this.deps + ) + private readonly subscribers = this.clientDelivery.subscribers private readonly tasks = new StructuredAgentSessionTaskQueue() private readonly runtimeState: StructuredAgentSessionHostRuntimeState private readonly reconcileLeases: ( @@ -90,7 +83,7 @@ export class StructuredAgentSessionHost { this.subscribers, (sessionId) => this.requireSession(sessionId), (sessionId) => this.handoffs.status(sessionId), - (sessionId) => this.statusFeed.publish(sessionId) + this.clientDelivery.publishStatus ) this.runtimeState = new StructuredAgentSessionHostRuntimeState( deps, @@ -116,7 +109,7 @@ export class StructuredAgentSessionHost { flush: (sessionId) => this.flushStreamedEvents(sessionId), serialize: (sessionId, task) => this.serialize(sessionId, task), subscribers: this.subscribers, - publishStatus: (sessionId) => this.statusFeed.publish(sessionId), + publishStatus: this.clientDelivery.publishStatus, now: this.now }) this.holds = createStructuredAgentSessionHolds(this.lifetimeContext(), { @@ -133,7 +126,7 @@ export class StructuredAgentSessionHost { // `hasSession` inside the same serialized step as this `set`. onReadable: (sessionId, restored) => { this.sessions.set(sessionId, restored) - this.statusFeed.publish(sessionId, undefined, { replay: true }) + this.clientDelivery.publishRestored(sessionId) }, restoreHandoff: (sessionId) => this.handoffs.restore(sessionId) }) @@ -144,7 +137,7 @@ export class StructuredAgentSessionHost { flushLifecycle: (sessionId) => this.runtimeState.lifecycleBarrier(sessionId), publishFence: (sessionId, session) => this.subscribers.snapshot(sessionId, session.journal, session.fence), - publishStatus: (sessionId) => this.statusFeed.publish(sessionId), + publishStatus: this.clientDelivery.publishStatusAndSettlement, hasResumeCapableHolder: (sessionId) => this.holds.hasResumeCapableHolder(sessionId), serialize: (sessionId, task) => this.serialize(sessionId, task), now: () => this.now(), @@ -179,7 +172,7 @@ export class StructuredAgentSessionHost { runtimeState: this.runtimeState, sessions: this.sessions, now: () => this.now(), - forgetStatus: (sessionId) => this.statusFeed.forget(sessionId) + forgetStatus: this.clientDelivery.forgetStatus } } @@ -191,7 +184,7 @@ export class StructuredAgentSessionHost { tasks: this.tasks, reconcileLeases: (sessionId) => this.reconcileLeases(sessionId), serialize: (sessionId, task) => this.serialize(sessionId, task), - publishStatus: (sessionId) => this.statusFeed.publish(sessionId) + publishStatus: this.clientDelivery.publishStatus } } /** Releases a session's resources without ending the conversation: the record and journal stay @@ -200,7 +193,7 @@ export class StructuredAgentSessionHost { return this.serialize(sessionId, async () => { await this.handoffs.closeRetainedTuiOwner(sessionId) await evictHeldStructuredAgentSession(this.lifetimeContext(), sessionId) - this.statusFeed.close(sessionId) + this.clientDelivery.closeSession(sessionId) // Whoever asked for the close, the surfaces that were holding this session are looking at a // session that no longer exists. A failed eviction throws above and keeps them. this.holds.forget(sessionId) @@ -211,7 +204,6 @@ export class StructuredAgentSessionHost { providerSupport.adapterSupportsCreate(this.deps.adapter, location, agent) listSessionTabs = () => listStructuredAgentSessionTabs(this.sessions) - getPersistedVisibleSessionTabIndex = () => this.deps.store.getVisibleSessionTabIndex() setSessionTabVisibility = (sessionId: string, visible: boolean): Promise => @@ -260,7 +252,7 @@ export class StructuredAgentSessionHost { tasks: this.tasks }), sessions: this.sessions - }) + }).finally(() => this.clientDelivery.closeAll()) } private mutationContext(): StructuredAgentSessionMutationContext { @@ -277,6 +269,8 @@ export class StructuredAgentSessionHost { send = (...args: Parameters) => this.conversationCommands.send(...args) + waitForSendSettlement = this.clientDelivery.waitForSendSettlement + private mutations = structuredAgentSessionMutationDelegates(() => this.mutationContext()) cancel = this.mutations.cancel respondToPrompt = this.mutations.respondToPrompt @@ -327,7 +321,7 @@ export class StructuredAgentSessionHost { /** Every session's projected status for session lists; unlike `subscribe`, retains nothing. */ subscribeStatus = (subscriber: StructuredAgentSessionStatusSubscriber): (() => void) => - this.statusFeed.subscribe(subscriber) + this.clientDelivery.subscribeStatus(subscriber) private requireSession(sessionId: string): StructuredAgentSessionHostSession { const session = this.sessions.get(sessionId) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-late-settlement.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-late-settlement.test.ts index a75d2ea6512..293f6ab2d8c 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-late-settlement.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-late-settlement.test.ts @@ -8,6 +8,7 @@ import type { AgentSessionSubscribeEvent } from '../../../shared/agent-session-wire' import { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' +import type { AgentSessionJournal } from '../agent-session-journal/journal-store' import type { AgentSessionDispatchOutcome, StructuredAgentSessionAdapter @@ -63,6 +64,12 @@ function submissions(): unknown { return state.ok ? state.page.submissions : null } +function journal(): AgentSessionJournal { + return ( + host as unknown as { sessions: Map } + ).sessions.get(SESSION)!.journal +} + beforeEach(async () => { root = await mkdtemp(join(tmpdir(), 'orca-wire-late-settle-')) resetHostTestOperationIds() @@ -197,6 +204,36 @@ describe('settling a send the provider proves it received after the ack window', expect(dispatch).toHaveBeenCalledTimes(1) }) + it('accepts from the durable echo row when the direct settlement write fails', async () => { + dispatch.mockResolvedValueOnce({ state: 'admitted' }) + const params = sendParams('settle from provider echo') + await host.send(CALLER, params) + vi.spyOn(journal(), 'resolveDispatch').mockRejectedValueOnce( + new Error('direct settlement write failed') + ) + + await expect( + host.settleLateDispatch({ + sessionId: SESSION, + clientMessageId: params.envelope.clientOperationId, + providerIdentity: { provider: 'claude', sessionId: THREAD, uuid: 'echo-row' } + }) + ).rejects.toThrow('direct settlement write failed') + await journal().appendItem( + { provider: 'claude', sessionId: THREAD, uuid: 'echo-row' }, + params.body, + { fence: store.getRecord(SESSION)?.lease.runtimeFence ?? 1 } + ) + + expect(submissions()).toMatchObject([ + { + clientMessageId: params.envelope.clientOperationId, + dispatchState: 'accepted', + providerItemId: `claude:${THREAD}:echo-row` + } + ]) + }) + it('leaves an already accepted send alone', async () => { const params = sendParams('ordinary send') await host.send(CALLER, params) 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 be9386815dd..14df2b3df6a 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 @@ -43,7 +43,7 @@ describe('structured send idempotency', () => { } const input = { clientMessageId: 'retry-id', payloadFingerprint: 'fingerprint', body } await journal.appendSubmission({ ...input, fence: 1 }) - await journal.markPendingSubmissionsUnknown(2) + await journal.markPendingSubmissionsUnknown(2, 'provider_write_failed: broken pipe') const originalItem = journal.snapshot().items[0] const publish = vi.fn() const dispatch = vi.fn(async () => { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-send-settlement.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-send-settlement.test.ts new file mode 100644 index 00000000000..b461d508f42 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-send-settlement.test.ts @@ -0,0 +1,133 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { AgentSessionJournal } from '../agent-session-journal/journal-store' +import { StructuredAgentSessionSendSettlement } from './structured-agent-session-send-settlement' + +function journal(dispatchState: 'pending' | 'accepted' | 'unknown'): AgentSessionJournal { + return { + cursor: () => ({ epoch: 'epoch-1', sequence: dispatchState === 'pending' ? 1 : 2 }), + submissions: () => [ + { + clientMessageId: 'client-1', + fence: 1, + payloadFingerprint: 'fingerprint', + dispatchState, + providerItemId: dispatchState === 'accepted' ? 'provider-1' : null, + reason: dispatchState === 'unknown' ? 'provider exited' : null, + submittedAt: 1, + resolvedAt: dispatchState === 'pending' ? null : 2 + } + ] + } as AgentSessionJournal +} + +function emptyJournal(): AgentSessionJournal { + return { + cursor: () => ({ epoch: 'epoch-1', sequence: 2 }), + submissions: () => [] + } as unknown as AgentSessionJournal +} + +describe('structured send settlement compatibility wait', () => { + afterEach(() => vi.useRealTimers()) + + it('returns a settlement already present in the journal', async () => { + const settlements = new StructuredAgentSessionSendSettlement(() => journal('accepted')) + + await expect(settlements.wait('session-1', 'client-1')).resolves.toMatchObject({ + value: { submission: { dispatchState: 'accepted' } } + }) + }) + + it('rejects when the send is absent from the current session generation', async () => { + const settlements = new StructuredAgentSessionSendSettlement(() => emptyJournal()) + + await expect(settlements.wait('session-1', 'client-1')).rejects.toThrow( + 'agent session send disappeared before settlement' + ) + }) + + it('resolves from a journal publication after durable admission', async () => { + const settlements = new StructuredAgentSessionSendSettlement(() => journal('pending')) + const pending = settlements.wait('session-1', 'client-1') + + settlements.publish('session-1', journal('accepted')) + + await expect(pending).resolves.toMatchObject({ + cursor: { sequence: 2 }, + value: { submission: { dispatchState: 'accepted' } } + }) + }) + + it('removes an abandoned wait on transport cancellation', async () => { + const settlements = new StructuredAgentSessionSendSettlement(() => journal('pending')) + const controller = new AbortController() + const pending = settlements.wait('session-1', 'client-1', controller.signal) + + controller.abort(new Error('transport closed')) + await expect(pending).rejects.toThrow('transport closed') + settlements.publish('session-1', journal('accepted')) + }) + + it('expires only the compatibility observer when the client leaves its socket open', async () => { + vi.useFakeTimers() + const settlements = new StructuredAgentSessionSendSettlement(() => journal('pending')) + const pending = settlements.wait('session-1', 'client-1') + + await vi.advanceTimersByTimeAsync(30_000) + + await expect(pending).resolves.toBeUndefined() + settlements.publish('session-1', journal('accepted')) + }) + + it('caps compatibility observers retained for one session', async () => { + const settlements = new StructuredAgentSessionSendSettlement(() => journal('pending')) + const retained = Array.from({ length: 64 }, () => + settlements.wait('session-1', 'client-1').catch(() => undefined) + ) + + await expect(settlements.wait('session-1', 'client-1')).resolves.toBeUndefined() + settlements.closeAll() + await Promise.all(retained) + }) + + it('caps compatibility observers retained across sessions', async () => { + const settlements = new StructuredAgentSessionSendSettlement(() => journal('pending')) + const retained = Array.from({ length: 1_024 }, (_, index) => + settlements.wait(`session-${index}`, 'client-1').catch(() => undefined) + ) + + await expect(settlements.wait('session-overflow', 'client-1')).resolves.toBeUndefined() + settlements.closeAll() + await Promise.all(retained) + }) + + it('ends only the compatibility observation when the session closes', async () => { + const settlements = new StructuredAgentSessionSendSettlement(() => journal('pending')) + const pending = settlements.wait('session-1', 'client-1') + + settlements.closeSession('session-1') + + await expect(pending).resolves.toBeUndefined() + settlements.publish('session-1', journal('accepted')) + }) + + it('rejects a wait when an authoritative publication drops the submission', async () => { + const settlements = new StructuredAgentSessionSendSettlement(() => journal('pending')) + const pending = settlements.wait('session-1', 'client-1') + + settlements.publish('session-1', emptyJournal()) + + await expect(pending).rejects.toThrow('agent session send disappeared before settlement') + }) + + it('ends every compatibility observation when the host closes', async () => { + const settlements = new StructuredAgentSessionSendSettlement(() => journal('pending')) + const first = settlements.wait('session-1', 'client-1') + const second = settlements.wait('session-2', 'client-1') + + settlements.closeAll() + + await expect(first).resolves.toBeUndefined() + await expect(second).resolves.toBeUndefined() + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-send-settlement.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-send-settlement.ts new file mode 100644 index 00000000000..6150e3412a6 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-send-settlement.ts @@ -0,0 +1,164 @@ +import type { + AgentJournalCursor, + AgentJournalSubmission +} from '../../../shared/agent-session-journal-types' +import type { AgentSessionSendResult } from '../../../shared/agent-session-wire' +import type { AgentSessionJournal } from '../agent-session-journal/journal-store' + +type SettledSend = { + cursor: AgentJournalCursor + value: AgentSessionSendResult +} + +type SendSettlement = SettledSend | 'pending' | 'missing' + +type SendSettlementWaiter = { + clientMessageId: string + resolve: (result: SettledSend | undefined) => void + reject: (error: Error) => void + timer: ReturnType + signal?: AbortSignal + onAbort?: () => void +} + +// Known legacy clients abandon the RPC after 15s without cancelling its socket dispatch. +const SEND_SETTLEMENT_WAIT_TIMEOUT_MS = 30_000 +const MAX_SEND_SETTLEMENT_WAITERS_PER_SESSION = 64 +const MAX_SEND_SETTLEMENT_WAITERS = 1_024 + +function settledSend( + journal: AgentSessionJournal, + clientMessageId: string, + submission: AgentJournalSubmission | undefined = journal + .submissions() + .find((candidate) => candidate.clientMessageId === clientMessageId) +): SendSettlement { + if (!submission) { + return 'missing' + } + return submission.dispatchState === 'pending' + ? 'pending' + : { cursor: journal.cursor(), value: { clientMessageId, submission } } +} + +function abortError(signal: AbortSignal): Error { + return signal.reason instanceof Error + ? signal.reason + : new Error('agent session send settlement wait aborted') +} + +/** Best-effort settlement observation for clients that predate admitted pending replies. */ +export class StructuredAgentSessionSendSettlement { + private readonly waiters = new Map>() + private waiterCount = 0 + + constructor(private readonly journalFor: (sessionId: string) => AgentSessionJournal) {} + + wait = ( + sessionId: string, + clientMessageId: string, + signal?: AbortSignal + ): Promise => { + if (signal?.aborted) { + return Promise.reject(abortError(signal)) + } + const immediate = settledSend(this.journalFor(sessionId), clientMessageId) + if (immediate === 'missing') { + return Promise.reject(new Error('agent session send disappeared before settlement')) + } + if (immediate !== 'pending') { + return Promise.resolve(immediate) + } + const existingSession = this.waiters.get(sessionId) + if ( + this.waiterCount >= MAX_SEND_SETTLEMENT_WAITERS || + (existingSession?.size ?? 0) >= MAX_SEND_SETTLEMENT_WAITERS_PER_SESSION + ) { + return Promise.resolve(undefined) + } + return new Promise((resolve, reject) => { + const waiter: SendSettlementWaiter = { + clientMessageId, + resolve, + reject, + timer: setTimeout(() => { + this.remove(sessionId, waiter) + resolve(undefined) + }, SEND_SETTLEMENT_WAIT_TIMEOUT_MS) + } + waiter.timer.unref?.() + const session = existingSession ?? new Set() + session.add(waiter) + this.waiters.set(sessionId, session) + this.waiterCount += 1 + if (signal) { + const onAbort = (): void => { + this.remove(sessionId, waiter) + reject(abortError(signal)) + } + waiter.signal = signal + waiter.onAbort = onAbort + signal.addEventListener('abort', onAbort, { once: true }) + if (signal.aborted) { + onAbort() + } + } + }) + } + + publish(sessionId: string, journal: AgentSessionJournal): void { + const waiters = this.waiters.get(sessionId) + if (!waiters) { + return + } + const submissions = new Map( + journal.submissions().map((submission) => [submission.clientMessageId, submission]) + ) + for (const waiter of waiters) { + const result = settledSend( + journal, + waiter.clientMessageId, + submissions.get(waiter.clientMessageId) + ) + if (result !== 'pending') { + this.remove(sessionId, waiter) + if (result === 'missing') { + waiter.reject(new Error('agent session send disappeared before settlement')) + } else { + waiter.resolve(result) + } + } + } + } + + closeSession(sessionId: string): void { + const waiters = this.waiters.get(sessionId) + if (!waiters) { + return + } + for (const waiter of waiters) { + this.remove(sessionId, waiter) + waiter.resolve(undefined) + } + } + + closeAll(): void { + for (const sessionId of this.waiters.keys()) { + this.closeSession(sessionId) + } + } + + private remove(sessionId: string, waiter: SendSettlementWaiter): void { + clearTimeout(waiter.timer) + if (waiter.signal && waiter.onAbort) { + waiter.signal.removeEventListener('abort', waiter.onAbort) + } + const session = this.waiters.get(sessionId) + if (session?.delete(waiter)) { + this.waiterCount -= 1 + } + if (session?.size === 0) { + this.waiters.delete(sessionId) + } + } +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-send.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-send.test.ts new file mode 100644 index 00000000000..7f62a01c864 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-send.test.ts @@ -0,0 +1,332 @@ +// What one `agentSession.send` writes, and when a user's Retry is allowed to +// put the same message on the wire a second time. + +import { beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' +import type { StructuredAgentSessionHost } from './structured-agent-session-host' +import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' +import type { AgentSessionJournal } from '../agent-session-journal/journal-store' +import { + accepted, + attach, + CALLER, + envelope, + hostTestState +} from './structured-agent-session-host-test-harness' +import { + HOST_TEST_NOW as NOW, + HOST_TEST_SESSION as SESSION, + HOST_TEST_THREAD as THREAD, + hostTestMessage +} from './structured-agent-session-host-test-data' + +let store: AgentSessionRecordStore +let host: StructuredAgentSessionHost +let dispatch: Mock + +beforeEach(() => { + ;({ store, host, dispatch } = hostTestState()) +}) + +describe('send', () => { + it('writes the submission before dispatching and resolves it accepted', async () => { + await attach() + const body = hostTestMessage('add a retry') + const result = await host.send(CALLER, { + envelope: envelope('agentSession.send', { body }), + body + }) + if (!result.ok) { + throw new Error(`expected a send, got ${result.refusal.code}`) + } + expect(result.value.submission.dispatchState).toBe('accepted') + expect(dispatch).toHaveBeenCalledTimes(1) + const page = host.history({ sessionId: SESSION, direction: 'tail' }) + expect(page.ok && page.page.items).toHaveLength(1) + expect(page.ok && page.page.fence).toBe(1) + expect(page.page.hostNow).toBe(NOW) + expect(page.providerSession).toEqual({ key: 'session_id', id: THREAD }) + }) + + it('settles a thrown dispatch as unknown, never as a rejection', async () => { + await attach() + dispatch.mockRejectedValueOnce(new Error('socket closed')) + const body = hostTestMessage('add a retry') + const result = await host.send(CALLER, { + envelope: envelope('agentSession.send', { body }), + body + }) + expect(result).toMatchObject({ ok: true, value: { submission: { dispatchState: 'unknown' } } }) + }) + + it('replays a retried send from the journal without dispatching twice', async () => { + await attach() + const body = hostTestMessage('add a retry') + const params = { envelope: envelope('agentSession.send', { body }), body } + await host.send(CALLER, params) + const retry = await host.send(CALLER, params) + expect(retry).toMatchObject({ ok: true, replayed: true }) + expect(dispatch).toHaveBeenCalledTimes(1) + }) + + it('refuses to redeliver an explicitly retried unknown from a thrown adapter call', async () => { + await attach() + dispatch.mockRejectedValueOnce(new Error('socket closed')) + const body = hostTestMessage('possibly delivered') + const params = { envelope: envelope('agentSession.send', { body }), body } + + const first = await host.send(CALLER, params) + expect(first).toMatchObject({ + ok: true, + value: { submission: { dispatchState: 'unknown' } } + }) + // A thrown adapter call is indistinguishable from a lost reply, so it is not + // on the allowlist: Retry replays the recorded outcome. + await expect(host.send(CALLER, { ...params, retryUnknown: true })).resolves.toMatchObject({ + ok: true, + value: { submission: { dispatchState: 'unknown' } } + }) + expect(dispatch).toHaveBeenCalledTimes(1) + const state = host.history({ sessionId: SESSION, direction: 'tail' }) + expect(state.ok && state.page.submissions).toHaveLength(1) + }) + + it('redispatches an explicitly retried unknown the write itself refused', async () => { + await attach() + dispatch + .mockImplementationOnce(async () => ({ + state: 'unknown' as const, + reason: 'provider_write_failed: broken pipe' + })) + .mockImplementationOnce(async () => accepted()) + const body = hostTestMessage('never written') + const params = { envelope: envelope('agentSession.send', { body }), body } + + await host.send(CALLER, params) + // The only doubt on the allowlist: the transport refused the frame, so this + // is a first delivery and not a second. + await expect(host.send(CALLER, { ...params, retryUnknown: true })).resolves.toMatchObject({ + ok: true, + replayed: false, + value: { submission: { dispatchState: 'accepted' } } + }) + expect(dispatch).toHaveBeenCalledTimes(2) + const state = host.history({ sessionId: SESSION, direction: 'tail' }) + expect(state.ok && state.page.submissions).toHaveLength(1) + }) + + it('returns an admitted retry to pending until the provider echo accepts it', async () => { + await attach() + dispatch + .mockImplementationOnce(async () => ({ + state: 'unknown' as const, + reason: 'provider_write_failed: connection closed before enqueue' + })) + .mockImplementationOnce(async () => ({ state: 'admitted' as const })) + const body = hostTestMessage('admitted on retry') + const params = { envelope: envelope('agentSession.send', { body }), body } + + await host.send(CALLER, params) + await expect(host.send(CALLER, { ...params, retryUnknown: true })).resolves.toMatchObject({ + ok: true, + replayed: false, + value: { + submission: { dispatchState: 'pending', reason: null, resolvedAt: null } + } + }) + expect(dispatch).toHaveBeenCalledTimes(2) + }) + + it('refuses to redeliver a retry for a turn the provider already owns', async () => { + await attach() + dispatch.mockImplementationOnce(async () => ({ + state: 'unknown' as const, + reason: 'codex app-server started a turn it did not name in time' + })) + const body = hostTestMessage('a turn codex owns but did not name') + const params = { envelope: envelope('agentSession.send', { body }), body } + + const first = await host.send(CALLER, params) + expect(first).toMatchObject({ + ok: true, + value: { submission: { dispatchState: 'unknown' } } + }) + // The turn is running; a second delivery would be a duplicate, so Retry + // replays the recorded outcome instead of re-sending. + await expect(host.send(CALLER, { ...params, retryUnknown: true })).resolves.toMatchObject({ + ok: true, + value: { submission: { dispatchState: 'unknown' } } + }) + expect(dispatch).toHaveBeenCalledTimes(1) + }) + + it('never reopens a submission the provider already proved delivered', async () => { + await attach() + dispatch.mockImplementationOnce(async () => accepted()) + const body = hostTestMessage('settled for good') + const params = { envelope: envelope('agentSession.send', { body }), body } + await host.send(CALLER, params) + const journal = ( + host as unknown as { sessions: Map } + ).sessions.get(SESSION)!.journal + const fence = store.getRecord(SESSION)?.lease.runtimeFence ?? 1 + + // Every later signal that could assert doubt: the attach sweep, and a + // direct unknown resolution. Neither may unsettle an accepted answer. + await journal.markPendingSubmissionsUnknown(fence) + await journal.resolveDispatch({ + clientMessageId: params.envelope.clientOperationId, + state: 'unknown', + reason: 'provider_write_failed: late transport error', + fence, + recovered: true + }) + + expect(journal.submissions()).toMatchObject([{ dispatchState: 'accepted', reason: null }]) + expect(journal.receiptFor(params.envelope.clientOperationId)).not.toBeNull() + }) + + it('leaves an admitted send pending and writes no dispatch row', async () => { + await attach() + dispatch.mockImplementationOnce(async () => ({ state: 'admitted' as const })) + const body = hostTestMessage('queued behind a running turn') + const params = { envelope: envelope('agentSession.send', { body }), body } + + await expect(host.send(CALLER, params)).resolves.toMatchObject({ + ok: true, + value: { submission: { dispatchState: 'pending', reason: null, resolvedAt: null } } + }) + const journal = ( + host as unknown as { sessions: Map } + ).sessions.get(SESSION)!.journal + expect(journal.pendingSubmissions()).toHaveLength(1) + }) + + it('refuses to redeliver an admitted send a host restart left unanswered', async () => { + await attach() + dispatch.mockImplementationOnce(async () => ({ state: 'admitted' as const })) + const body = hostTestMessage('written, never acknowledged') + const params = { envelope: envelope('agentSession.send', { body }), body } + await host.send(CALLER, params) + const journal = ( + host as unknown as { sessions: Map } + ).sessions.get(SESSION)!.journal + + await journal.markPendingSubmissionsUnknown(store.getRecord(SESSION)?.lease.runtimeFence ?? 1) + expect(journal.submissions()).toMatchObject([ + { dispatchState: 'unknown', reason: 'host_restarted_before_acknowledgement' } + ]) + + // The frame was already written to the dead child's stdin, and Claude resumes + // the same provider session by id, so the restart ends the wait without + // proving non-delivery. Re-typing costs a message; redelivering costs a + // duplicate in the model's conversation. + await expect(host.send(CALLER, { ...params, retryUnknown: true })).resolves.toMatchObject({ + ok: true, + value: { submission: { dispatchState: 'unknown' } } + }) + expect(dispatch).toHaveBeenCalledTimes(1) + expect(journal.submissions()).toHaveLength(1) + }) + + it('refuses to redeliver an admitted send whose child exited first', async () => { + await attach() + dispatch.mockImplementationOnce(async () => ({ state: 'admitted' as const })) + const body = hostTestMessage('written, then the child died') + const params = { envelope: envelope('agentSession.send', { body }), body } + await host.send(CALLER, params) + const journal = ( + host as unknown as { sessions: Map } + ).sessions.get(SESSION)!.journal + + await journal.markPendingSubmissionsUnknown( + store.getRecord(SESSION)?.lease.runtimeFence ?? 1, + 'provider_exited_before_acknowledgement' + ) + + await expect(host.send(CALLER, { ...params, retryUnknown: true })).resolves.toMatchObject({ + ok: true, + value: { submission: { dispatchState: 'unknown' } } + }) + expect(dispatch).toHaveBeenCalledTimes(1) + }) + + it('advances an explicit retry after a ledger-unknown send is reconciled in the journal', async () => { + await attach() + const journal = ( + host as unknown as { sessions: Map } + ).sessions.get(SESSION)!.journal + vi.spyOn(journal, 'resolveDispatch').mockRejectedValueOnce(new Error('journal resolve failed')) + const body = hostTestMessage('possibly delivered before persistence failed') + const params = { envelope: envelope('agentSession.send', { body }), body } + + await expect(host.send(CALLER, params)).rejects.toThrow('journal resolve failed') + expect(journal.submissions()).toMatchObject([ + { clientMessageId: params.envelope.clientOperationId, dispatchState: 'unknown' } + ]) + expect( + store.listOperationRows().find((row) => row.operationId === params.envelope.clientOperationId) + ?.outcome + ).toEqual({ status: 'unknown' }) + expect(dispatch).toHaveBeenCalledTimes(1) + + await journal.markPendingSubmissionsUnknown(store.getRecord(SESSION)?.lease.runtimeFence ?? 1) + await expect(host.send(CALLER, params)).resolves.toMatchObject({ + ok: false, + refusal: { code: 'agent_session_operation_unknown' } + }) + expect(dispatch).toHaveBeenCalledTimes(1) + + // The adapter took the message before the journal write failed, so the + // provider may already have it: an explicit retry replays, never redelivers. + await expect(host.send(CALLER, { ...params, retryUnknown: true })).resolves.toMatchObject({ + ok: true, + value: { submission: { dispatchState: 'unknown' } } + }) + expect(dispatch).toHaveBeenCalledTimes(1) + expect(journal.submissions()).toHaveLength(1) + }) + + it('refuses a stale fence and hands back the current one', async () => { + const record = await attach() + const body = hostTestMessage('add a retry') + const result = await host.send(CALLER, { + envelope: envelope( + 'agentSession.send', + { body }, + { expectedRuntimeFence: (record?.lease.runtimeFence ?? 1) + 5 } + ), + body + }) + expect(result).toMatchObject({ + ok: false, + refusal: { code: 'agent_session_checkpoint_stale', currentFence: record?.lease.runtimeFence } + }) + }) + + it('does not let a refused call leave a ledger row that replays past the fence', async () => { + const record = await attach() + const body = hostTestMessage('add a retry') + const params = { + envelope: envelope( + 'agentSession.send', + { body }, + { expectedRuntimeFence: (record?.lease.runtimeFence ?? 1) + 5 } + ), + body + } + await host.send(CALLER, params) + expect(await host.send(CALLER, params)).toMatchObject({ + ok: false, + refusal: { code: 'agent_session_checkpoint_stale' } + }) + expect(dispatch).not.toHaveBeenCalled() + }) + + it('refuses any mutation against a session this host has not attached', async () => { + const body = hostTestMessage('add a retry') + expect( + await host.send(CALLER, { envelope: envelope('agentSession.send', { body }), body }) + ).toMatchObject({ ok: false, refusal: { code: 'agent_session_ownership_unknown' } }) + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-settled-attach-retry.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-settled-attach-retry.test.ts index 24dc81f4684..278619c2c61 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-settled-attach-retry.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-settled-attach-retry.test.ts @@ -310,16 +310,18 @@ describe('settled attach retry', () => { }) }) - it('restores an unknown submission without redispatch before a distinct send', async () => { + it('settles a submission the host restart left pending, and never redelivers it', async () => { expect((await host.attach(CALLER, hostTestAttachParams(null))).ok).toBe(true) - dispatch.mockRejectedValueOnce(new Error('socket closed')) - const body = hostTestMessage('possibly delivered') + // Admitted: written to the child, acknowledgement still outstanding. The + // restart below is the process fact that ends the wait, not a stopwatch. + dispatch.mockImplementationOnce(async () => ({ state: 'admitted' as const })) + const body = hostTestMessage('written before the host died') const unknownParams = { envelope: envelope('agentSession.send', { body }), body } const first = await host.send(CALLER, unknownParams) - expect(first).toMatchObject({ ok: true, value: { submission: { dispatchState: 'unknown' } } }) + expect(first).toMatchObject({ ok: true, value: { submission: { dispatchState: 'pending' } } }) await host.flushAllStreamedEvents() store = await AgentSessionRecordStore.open({ directory: join(root, 'store'), hostId: 'local' }) @@ -360,6 +362,8 @@ describe('settled attach retry', () => { )?.dispatchState ).toBe('unknown') + // A restart ends the wait without proving the dead child never took the + // frame, so even an explicit retry replays rather than sending a second copy. const explicitRetry = await host.send(CALLER, { ...unknownParams, envelope: { @@ -370,9 +374,9 @@ describe('settled attach retry', () => { }) expect(explicitRetry).toMatchObject({ ok: true, - value: { submission: { dispatchState: 'accepted' } } + value: { submission: { dispatchState: 'unknown' } } }) - expect(dispatch).toHaveBeenCalledTimes(3) + expect(dispatch).toHaveBeenCalledTimes(2) }) it('records proven acquisition cleanup as durable death evidence', async () => { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts index f4e02f4491b..1cb8439c391 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts @@ -193,6 +193,28 @@ describe('a chat that closes', () => { expect(closeSession).not.toHaveBeenCalled() expect(host.hasSession(SESSION)).toBe(true) }) + + it('releases a compatibility wait when the session is evicted', async () => { + await attach() + dispatch.mockResolvedValueOnce({ state: 'admitted' }) + const body = hostTestMessage('pending until close') + const result = await host.send(CALLER, { + envelope: envelope('agentSession.send', { body }), + body + }) + expect(result).toMatchObject({ + ok: true, + value: { submission: { dispatchState: 'pending' } } + }) + if (!result.ok) { + throw new Error('send was refused') + } + const settlement = host.waitForSendSettlement(SESSION, result.value.clientMessageId) + + await host.close(SESSION) + + await expect(settlement).resolves.toBeUndefined() + }) }) describe('a session with a turn in flight', () => { @@ -274,6 +296,38 @@ describe('a session evicted and opened again', () => { }) describe('an unexpected provider exit', () => { + it('publishes terminal settlement to a waiting older client', async () => { + await attach() + dispatch.mockResolvedValueOnce({ state: 'admitted' }) + const body = hostTestMessage('pending until provider exit') + const result = await host.send(CALLER, { + envelope: envelope('agentSession.send', { body }), + body + }) + expect(result).toMatchObject({ + ok: true, + value: { submission: { dispatchState: 'pending' } } + }) + if (!result.ok) { + throw new Error('send was refused') + } + const settlement = host.waitForSendSettlement(SESSION, result.value.clientMessageId) + const exitedFence = store.getRecord(SESSION)?.lease.runtimeFence ?? 0 + + await host.handleAdapterEvent({ + type: 'ended', + sessionId: SESSION, + reason: 'provider exited', + cause: 'unexpected-exit', + fence: exitedFence, + acquisitionGeneration: 'generation-1' + }) + + await expect(settlement).resolves.toMatchObject({ + value: { submission: { dispatchState: 'unknown' } } + }) + }) + it('turns a journal sink failure into observed-exit settlement and lease release', async () => { await attach() const session = ( 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 ba83b7e54ac..35ca56a451c 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 @@ -6,12 +6,20 @@ // row the next attach settles as `unknown`, whereas the reverse would lose a // turn the provider already accepted. -import type { AgentJournalMessageItem } from '../../../shared/agent-session-journal-types' +import type { + AgentJournalMessageItem, + AgentJournalSubmission +} from '../../../shared/agent-session-journal-types' import type { AgentSessionCancelResult, AgentSessionSendResult, AgentSessionWireRefusal } from '../../../shared/agent-session-wire' +import { + DISPATCH_DOUBT_PERSISTENCE_FAILED, + DISPATCH_DOUBT_RETRY_IN_PROGRESS, + dispatchDoubtProvesUndelivered +} from '../agent-session-journal/journal-dispatch-doubt-reasons' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' import type { AgentSessionDispatchOutcome, @@ -73,6 +81,16 @@ async function appendStatus( ctx.publish() } +/** + * Whether a user's Retry may put this message on the wire again: only where the + * recorded doubt proves the frame never reached a provider. Everything else + * replays the recorded outcome instead — one message reached the model five + * times through this path. Orca never re-sends on its own either way. + */ +function retryWouldRedeliver(existing: AgentJournalSubmission | undefined): boolean { + return existing?.dispatchState === 'unknown' && dispatchDoubtProvesUndelivered(existing.reason) +} + export async function performSend( ctx: AgentSessionTurnContext, input: { @@ -88,13 +106,14 @@ export async function performSend( if (existing && existing.payloadFingerprint !== input.payloadFingerprint) { return invalid(`Message id ${input.clientMessageId} was already used for another send.`) } - if (existing && !(input.retryUnknown && existing.dispatchState === 'unknown')) { + const redeliver = input.retryUnknown === true && retryWouldRedeliver(existing) + if (existing && !redeliver) { return { ok: true, value: { clientMessageId: input.clientMessageId, submission: existing } } } - if (!(input.retryUnknown && existing?.dispatchState === 'unknown')) { + if (!redeliver) { await ctx.journal.appendSubmission({ ...input, fence: ctx.fence }) ctx.publish() } else { @@ -102,13 +121,33 @@ export async function performSend( await ctx.journal.resolveDispatch({ clientMessageId: input.clientMessageId, state: 'unknown', - reason: 'dispatch_retry_in_progress', + reason: DISPATCH_DOUBT_RETRY_IN_PROGRESS, fence: ctx.fence }) ctx.publish() } const outcome = await dispatchSafely(ctx, input.clientMessageId, input.body) + // A first admission needs no dispatch row: the submission is already pending. + // A retry must durably clear the old doubt so clients do not mistake a + // successful re-admission for a refused redelivery. + if (outcome.state === 'admitted') { + if (redeliver) { + await ctx.journal.resolveDispatch({ + clientMessageId: input.clientMessageId, + state: 'pending', + fence: ctx.fence + }) + } + ctx.publish() + return { + ok: true, + value: { + clientMessageId: input.clientMessageId, + submission: requireSubmission(ctx, input.clientMessageId) + } + } + } try { await ctx.journal.resolveDispatch( outcome.state === 'accepted' @@ -132,7 +171,7 @@ export async function performSend( await ctx.journal.resolveDispatch({ clientMessageId: input.clientMessageId, state: 'unknown', - reason: 'dispatch_result_persistence_failed', + reason: DISPATCH_DOUBT_PERSISTENCE_FAILED, fence: ctx.fence }) } catch { @@ -142,14 +181,26 @@ export async function performSend( throw error } ctx.publish() + return { + ok: true, + value: { + clientMessageId: input.clientMessageId, + submission: requireSubmission(ctx, input.clientMessageId) + } + } +} +function requireSubmission( + ctx: AgentSessionTurnContext, + clientMessageId: string +): AgentJournalSubmission { const submission = ctx.journal .submissions() - .find((entry) => entry.clientMessageId === input.clientMessageId) + .find((entry) => entry.clientMessageId === clientMessageId) if (!submission) { throw new Error('agent_session_submission_lost') } - return { ok: true, value: { clientMessageId: input.clientMessageId, submission } } + return submission } export async function performCancel( diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.test.ts index 207a321d6a0..813e2f8f3e8 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.test.ts @@ -235,6 +235,58 @@ describe('provider-exit recovery tickets', () => { }) }) + it('settles a submission the dead child never acknowledged', async () => { + const markPendingSubmissionsUnknown = vi.fn(async () => ['client-1']) + const session = { + hasProviderChild: true, + fence: 7, + acquisitionGeneration: GENERATION, + journal: { + snapshot: () => ({ items: [] }), + appendLifecycleBatch: vi.fn(async () => ({ epoch: 'epoch-1', sequence: 1 })), + markPendingSubmissionsUnknown + } + } as unknown as StructuredAgentSessionHostSession + + await settleUnexpectedStructuredAgentSessionExit( + { + store: { + getRecord: () => ({ + lease: { + handoffStage: null, + runtimeFence: 7, + runtimeKind: 'native', + claimStatus: 'live', + ownerProcess: 'provider', + reservedSpawnToken: null, + processlessAt: null + } + }), + transitionHandoff: async () => ({ lease: { runtimeFence: 8 } }) + }, + sessions: new Map([[SESSION, session]]), + flushLifecycle: async () => ({ ok: true }), + publishFence: vi.fn(), + hasResumeCapableHolder: () => true, + serialize: async (_sessionId, task: () => Promise) => task(), + now: () => 1 + } as never, + { + type: 'ended', + sessionId: SESSION, + reason: 'provider exited', + cause: 'unexpected-exit', + fence: 7, + acquisitionGeneration: GENERATION + } + ) + + expect(markPendingSubmissionsUnknown).toHaveBeenCalledWith( + 7, + 'provider_exited_before_acknowledgement' + ) + }) + it('does not release or reacquire while terminal settlement retry is still failing', async () => { const session = { hasProviderChild: true, diff --git a/src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts b/src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts index f9f60ded887..b8fe9d5b01b 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts @@ -12,7 +12,10 @@ import { type StructuredAgentSessionStatusSubscriber } from '../../../native-chat/agent-session-wire/structured-agent-session-status-feed' import type { OrcaRuntimeService } from '../../orca-runtime' -import { STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' +import { + AGENT_SESSION_PENDING_SEND_RESULT_RUNTIME_CAPABILITY, + STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY +} from '../../../../shared/protocol-version' import type { RpcRequest, RpcResponse } from '../core' import { RpcDispatcher } from '../dispatcher' import { STRUCTURED_AGENT_SESSION_METHODS } from './structured-agent-session' @@ -141,7 +144,26 @@ export function hostStub(): StructuredAgentSessionHost { } })), rewind: vi.fn(async () => ({ ok: true, value: { itemId: 'chosen', epoch: 'next' } })), - send: vi.fn(async () => ({ ok: true, replayed: false })), + send: vi.fn(async () => ({ + ok: true, + replayed: false, + fence: 1, + cursor: { epoch: 'epoch-a', sequence: 1 }, + value: { + clientMessageId: OPERATION, + submission: { + clientMessageId: OPERATION, + fence: 1, + payloadFingerprint: FINGERPRINT, + dispatchState: 'accepted', + providerItemId: 'provider-1', + reason: null, + submittedAt: 1, + resolvedAt: 2 + } + } + })), + waitForSendSettlement: vi.fn(), cancel: vi.fn(async () => ({ ok: true, replayed: false })), close: vi.fn(async () => undefined), revealSession: vi.fn(async () => ({ @@ -237,6 +259,7 @@ export async function call( clientId?: string clientKind?: 'mobile' | 'runtime' clientCapabilities?: string[] + signal?: AbortSignal }, runtimeOverrides: Record = {} ): Promise { @@ -255,11 +278,17 @@ export async function call( export const STRUCTURED_CLIENT = { clientKind: 'runtime' as const, - clientCapabilities: [STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY] + clientCapabilities: [ + STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, + AGENT_SESSION_PENDING_SEND_RESULT_RUNTIME_CAPABILITY + ] } export const STRUCTURED_MOBILE_CLIENT = { clientKind: 'mobile' as const, - clientCapabilities: [STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY] + clientCapabilities: [ + STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, + AGENT_SESSION_PENDING_SEND_RESULT_RUNTIME_CAPABILITY + ] } /** Every suite wants the same lifecycle: a fresh stub per test, no host left installed. */ diff --git a/src/main/runtime/rpc/methods/structured-agent-session-send-compatibility.ts b/src/main/runtime/rpc/methods/structured-agent-session-send-compatibility.ts new file mode 100644 index 00000000000..8b948869990 --- /dev/null +++ b/src/main/runtime/rpc/methods/structured-agent-session-send-compatibility.ts @@ -0,0 +1,26 @@ +import { AGENT_SESSION_PENDING_SEND_RESULT_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' +import type { StructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-host' +import type { RpcContext } from '../core' +import { requireStructuredHost, structuredCallerFor } from './structured-agent-session-gate' + +export async function sendStructuredAgentSessionForClient( + params: Parameters[1], + context: RpcContext +) { + const host = requireStructuredHost(context) + const result = await host.send(structuredCallerFor(context), params) + if ( + !result.ok || + result.value.submission.dispatchState !== 'pending' || + context.clientKind === undefined || + context.clientCapabilities?.includes(AGENT_SESSION_PENDING_SEND_RESULT_RUNTIME_CAPABILITY) + ) { + return result + } + const settled = await host.waitForSendSettlement( + params.envelope.sessionId, + result.value.clientMessageId, + context.signal + ) + return settled ? { ...result, ...settled } : result +} diff --git a/src/main/runtime/rpc/methods/structured-agent-session.test.ts b/src/main/runtime/rpc/methods/structured-agent-session.test.ts index 94a344b6f18..c2d46b09818 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session.test.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session.test.ts @@ -4,6 +4,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { setStructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-registry' import { + AGENT_SESSION_PENDING_SEND_RESULT_RUNTIME_CAPABILITY, RUNTIME_CAPABILITIES, RUNTIME_PROTOCOL_VERSION, STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY, @@ -146,6 +147,7 @@ describe('capability gating', () => { it('advertises the capability without bumping the protocol version', () => { expect(RUNTIME_CAPABILITIES).toContain(STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY) + expect(RUNTIME_CAPABILITIES).toContain(AGENT_SESSION_PENDING_SEND_RESULT_RUNTIME_CAPABILITY) expect(RUNTIME_CAPABILITIES).toContain(STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY) expect(RUNTIME_CAPABILITIES).toContain(STRUCTURED_AGENT_SESSION_REVEAL_RUNTIME_CAPABILITY) // Additive methods do not break an old client; bumping would strand every @@ -207,6 +209,124 @@ describe('capability gating', () => { expect(hostCalls.send).toHaveBeenCalledTimes(1) }) + it('returns a settlement to older structured clients when observed within the window', async () => { + const pendingSubmission = { + clientMessageId: 'client-1', + fence: 1, + payloadFingerprint: 'fingerprint', + dispatchState: 'pending' as const, + providerItemId: null, + reason: null, + submittedAt: 1, + resolvedAt: null + } + hostCalls.send.mockResolvedValueOnce({ + ok: true, + replayed: true, + fence: 7, + cursor: { epoch: 'epoch-a', sequence: 1 }, + value: { clientMessageId: 'client-1', submission: pendingSubmission } + }) + hostCalls.waitForSendSettlement.mockResolvedValueOnce({ + cursor: { epoch: 'epoch-a', sequence: 2 }, + value: { + clientMessageId: 'client-1', + submission: { + ...pendingSubmission, + dispatchState: 'accepted', + providerItemId: 'provider-1', + resolvedAt: 2 + } + } + }) + const controller = new AbortController() + + const response = await call('agentSession.send', sendParams(), { + clientKind: 'runtime', + clientCapabilities: [STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY], + signal: controller.signal + }) + + expect(hostCalls.waitForSendSettlement).toHaveBeenCalledWith( + SESSION, + 'client-1', + controller.signal + ) + expect(response).toMatchObject({ + ok: true, + result: { + ok: true, + replayed: true, + fence: 7, + cursor: { sequence: 2 }, + value: { submission: { dispatchState: 'accepted' } } + } + }) + }) + + it('returns durable pending when an older-client settlement observer cannot be retained', async () => { + hostCalls.send.mockResolvedValueOnce({ + ok: true, + replayed: false, + fence: 1, + cursor: { epoch: 'epoch-a', sequence: 1 }, + value: { + clientMessageId: 'client-1', + submission: { + clientMessageId: 'client-1', + fence: 1, + payloadFingerprint: 'fingerprint', + dispatchState: 'pending', + providerItemId: null, + reason: null, + submittedAt: 1, + resolvedAt: null + } + } + }) + hostCalls.waitForSendSettlement.mockResolvedValueOnce(undefined) + + const response = await call('agentSession.send', sendParams(), { + clientKind: 'runtime', + clientCapabilities: [STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY] + }) + + expect(response).toMatchObject({ + ok: true, + result: { value: { submission: { dispatchState: 'pending' } } } + }) + }) + + it('returns durable pending immediately to clients that understand admission', async () => { + hostCalls.send.mockResolvedValueOnce({ + ok: true, + replayed: false, + fence: 1, + cursor: { epoch: 'epoch-a', sequence: 1 }, + value: { + clientMessageId: 'client-1', + submission: { + clientMessageId: 'client-1', + fence: 1, + payloadFingerprint: 'fingerprint', + dispatchState: 'pending', + providerItemId: null, + reason: null, + submittedAt: 1, + resolvedAt: null + } + } + }) + + const response = await call('agentSession.send', sendParams(), STRUCTURED_CLIENT) + + expect(hostCalls.waitForSendSettlement).not.toHaveBeenCalled() + expect(response).toMatchObject({ + ok: true, + result: { value: { submission: { dispatchState: 'pending' } } } + }) + }) + it('requires the host structured-chat setting for mobile clients', async () => { const response = await call('agentSession.send', sendParams(), STRUCTURED_MOBILE_CLIENT, { getClientSettings: () => ({ experimentalStructuredNativeChat: false }) diff --git a/src/main/runtime/rpc/methods/structured-agent-session.ts b/src/main/runtime/rpc/methods/structured-agent-session.ts index bb949d5e4e3..c09ae4cfcbf 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session.ts @@ -60,6 +60,7 @@ import { SubscribeParams, UnsubscribeParams } from './structured-agent-session-schemas' +import { sendStructuredAgentSessionForClient } from './structured-agent-session-send-compatibility' /** * The attach-shaped entries take the location from the client instead of resolving it from a @@ -194,7 +195,7 @@ export const STRUCTURED_AGENT_SESSION_METHODS: RpcAnyMethod[] = [ defineMethod({ name: 'agentSession.send', params: SendParams, - handler: async (params, ctx) => requireHost(ctx).send(callerFor(ctx), params) + handler: sendStructuredAgentSessionForClient }), defineMethod({ // Stopping a turn, so it stays available after admission is revoked: see the gate's rule. diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx index dbd1ea97004..9e75556ccbc 100644 --- a/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx @@ -138,12 +138,17 @@ export function NativeChatStructuredSession( } ] : []) + // Only the head of the outbox is ever dispatched, so it is the only entry a + // Retry can act on and the only one whose state can be holding the queue. + // Scanning past it named a message the user was not looking at and re-sent + // one from earlier in the session while their newest sat behind it. + const outboxHead = controller.outbox[0] ?? null const retryableOutboxEntry = - controller.outbox.find((entry) => entry.state === 'unconfirmed') ?? - controller.outbox.find( - (entry) => entry.clientMessageId === controller.blockedClientMessageId - ) ?? - null + outboxHead && + (outboxHead.state === 'unconfirmed' || + outboxHead.clientMessageId === controller.blockedClientMessageId) + ? outboxHead + : null const structuredTransport = useMemo( () => ({ send: (text: string, attachments: readonly { id: string; path: string }[]): boolean => diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSessionDelivery.test.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSessionDelivery.test.tsx new file mode 100644 index 00000000000..3775da4a9e7 --- /dev/null +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSessionDelivery.test.tsx @@ -0,0 +1,619 @@ +// The delivery notice and the outbox queue behind it: which entry a Retry acts +// on, when no notice is owed at all, and how a host-confirmed unknown is probed. + +// @vitest-environment happy-dom + +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import React, { forwardRef, useImperativeHandle, useRef } from 'react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { AgentJournalRenderItem } from '../../../../shared/agent-session-journal-types' +import type { AgentSessionBackgroundTask } from '../../../../shared/agent-session-wire' +import type { NativeChatQuestionCardProps } from './NativeChatQuestionCard' + +const mocks = vi.hoisted(() => ({ + call: vi.fn(), + fileLinkClick: vi.fn(), + mode: 'static' as 'static' | 'outbox', + messageListProps: null as null | { + allowFileUriLinks?: boolean + onLinkClick?: (...args: unknown[]) => void + showTurnStatus?: boolean + runtimeContext?: unknown + }, + composerProps: null as null | { + structuredTransport?: Record + isWorking?: boolean + }, + questionCardProps: null as NativeChatQuestionCardProps | null, + promptItems: [] as AgentJournalRenderItem[], + respond: vi.fn(), + handlePasteEvent: vi.fn(), + pasteFromClipboard: vi.fn(), + submissions: [] as unknown[], + monitoringBackgroundTasks: false, + supportsBackgroundTaskStop: false, + supportsBackgroundTaskStopAll: true, + backgroundTasks: [] as AgentSessionBackgroundTask[], + stopBackgroundTask: vi.fn() +})) + +vi.mock('@/runtime/structured-agent-session-client', () => ({ + callStructuredAgentSession: mocks.call +})) + +vi.mock('./use-structured-agent-session', async () => { + const { useStructuredAgentSessionOutbox } = await import('./use-structured-agent-session-outbox') + return { + useStructuredAgentSession: (props: { + sessionId: string + target: { kind: 'local' } | { kind: 'environment'; environmentId: string } + }) => { + const outbox = useStructuredAgentSessionOutbox({ + sessionId: props.sessionId, + target: props.target, + fence: 1, + submissions: mocks.submissions as never + }) + return { + messages: + mocks.mode === 'outbox' + ? [] + : [ + { + id: 'message-1', + role: 'assistant', + source: 'transcript', + timestamp: 1, + blocks: [{ type: 'text', text: '[file](file:///repo/src/main.ts)' }] + } + ], + status: 'ready' as const, + error: outbox.error, + hasOlder: false, + loadingOlder: false, + loadOlder: vi.fn(), + prompts: mocks.promptItems, + outbox: outbox.outbox, + blockedClientMessageId: outbox.blockedClientMessageId, + send: outbox.send, + retry: outbox.retry, + isWorking: false, + isMonitoringBackgroundTasks: mocks.monitoringBackgroundTasks, + supportsBackgroundTaskStop: mocks.supportsBackgroundTaskStop, + supportsBackgroundTaskStopAll: mocks.supportsBackgroundTaskStopAll, + backgroundTasks: mocks.backgroundTasks, + turnId: null, + cancel: vi.fn(), + stopBackgroundTask: (taskId?: string) => mocks.stopBackgroundTask(props.sessionId, taskId), + respond: mocks.respond, + optionSnapshot: [ + { + id: 'model', + label: 'Model', + category: 'model', + kind: { + type: 'select', + currentValue: 'gpt-live', + choices: [{ value: 'gpt-live', label: 'GPT Live' }] + }, + valueSource: 'reported', + settable: true + } + ], + optionSurface: { + getSnapshot: () => [], + setOption: vi.fn(), + invokeAction: vi.fn(), + subscribe: () => () => {} + }, + setStructuredOption: vi.fn() + } + } + } +}) + +vi.mock('./use-native-chat-font-scale', () => ({ + useNativeChatFontScale: () => ({ scale: 1 }) +})) + +vi.mock('./use-native-chat-file-link-context', () => ({ + useNativeChatFileLinkContext: () => ({ + worktreeId: 'wt-1', + worktreePath: '/repo', + runtimeEnvironmentId: null + }) +})) + +vi.mock('./use-native-chat-file-link-click', () => ({ + useNativeChatFileLinkClick: (context: unknown) => (context ? mocks.fileLinkClick : undefined) +})) + +vi.mock('./NativeChatMessageList', () => ({ + NativeChatMessageList: (props: typeof mocks.messageListProps) => { + mocks.messageListProps = props + return
+ } +})) + +vi.mock('./NativeChatComposer', () => ({ + NativeChatComposer: forwardRef((props: typeof mocks.composerProps, ref) => { + mocks.composerProps = props + const fieldRef = useRef(null) + useImperativeHandle(ref, () => ({ + // Match the real composer so focus ownership is observable in this split suite. + focus: () => { + fieldRef.current?.focus() + return true + }, + insertTypedText: () => true, + handlePasteEvent: mocks.handlePasteEvent, + pasteFromClipboard: mocks.pasteFromClipboard + })) + return