From 027acb4efa2e6b226d40df266b86367423946d62 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 10 Sep 2026 16:29:02 -0700 Subject: [PATCH] fix(native-chat): settle a structured send on admission, not on the provider echo (#19863) * fix(native-chat): settle a structured send on admission, not on the provider echo Sending a message in structured native chat raised "Message delivery is unconfirmed." with a Retry button on a message that had in fact been delivered. Measured across 14 days of local journals: 44 of 173 delivered sends (25.4%) tripped it. The dispatch path wrote the message to the provider, then waited a fixed 10s for the provider to echo the message's uuid back. That echo is emitted when the provider STARTS the turn, so a message queued behind a running turn cannot be echoed until that turn ends. Echo latency is bounded by the previous turn's duration, which is unbounded -- one send took 105 minutes. The 10s constant sat at the p75 of real echo latency, with the slowest clean send at 9.76s, a margin of 0.24s. No constant can work: the wait was measuring the wrong event. The false banner was not cosmetic. It invited a Retry, and Retry bypassed the operation ledger to redeliver. One message reached the model five times through that path. Dispatch now returns as soon as the transport write completes and writes no dispatch row; the submission stays `pending`, a neutral state, and the provider's echo settles it `accepted` through the late-settlement channel whenever the turn ahead of it ends. Delivery doubt is reachable only from process facts -- a refused write, a dead child, a dead host -- never from elapsed time. Retry re-delivers only where the recorded reason proves the message never reached the provider. The list is deliberately fail-closed: refusing a legitimate retry costs the user a re-type, while allowing an illegitimate one sends the model a second copy of their message. A refused entry now leaves the outbox with an explicit notice instead of parking at the head, where it would have wedged every message queued behind it. The send-response classification moves to a pure module beside the existing outbox reconciler, so both writers of an entry's state now live together and the decision is unit-testable rather than reachable only through the hook. Scope and known gaps: - Codex carries the same 10s stopwatch. It has no late-settlement channel, matches waiters by queue order rather than identity, and has no waiter lifecycle at all, so there was no safe subset to land here. A marker constant records the debt and deletes itself when that lands. - A message refused re-delivery loses its standing delivery notice and leaves only a transient error line. A passive "waiting to be accepted" affordance is the follow-up. - The restart reconciler that would decide a dead child or a dead host on evidence rather than refusing them is fully written and has never had a production caller. Wiring it is the next change, and it removes the re-type cost above. * fix(native-chat): harden structured dispatch settlement * fix(native-chat): preserve dispatch recovery evidence * fix(native-chat): preserve pending send compatibility * fix(native-chat): satisfy native import audit * fix(native-chat): bound legacy send settlement --------- Co-authored-by: Merge Sim --- .../mobile-runtime-client-capabilities.ts | 2 + .../transport/rpc-client-capabilities.test.ts | 1 + ...laude-agent-sdk-user-message-queue.test.ts | 32 +- .../claude-agent-sdk-user-message-queue.ts | 38 +- .../claude/claude-stream-json-connection.ts | 11 +- .../claude-structured-compaction.test.ts | 41 +- .../claude/claude-structured-compaction.ts | 23 +- ...aude-structured-dispatch-admission.test.ts | 123 +++ ...claude-structured-dispatch-test-support.ts | 52 ++ .../claude/claude-structured-dispatch.test.ts | 711 ++++++++++-------- src/main/claude/claude-structured-dispatch.ts | 124 +-- ...e-structured-session-adapter-turns.test.ts | 261 +++++++ .../claude-structured-session-adapter.test.ts | 271 +------ .../claude-structured-session-adapter.ts | 15 +- .../claude/claude-structured-session-close.ts | 14 +- .../claude/claude-structured-session-state.ts | 13 +- .../claude-structured-session-test-support.ts | 20 +- ...tui-resume-real-binary.integration.test.ts | 8 +- src/main/codex/codex-structured-turn-start.ts | 3 +- src/main/ipc/runtime.ts | 3 + .../journal-crash-boundary.test.ts | 56 ++ .../journal-dispatch-doubt-reasons.ts | 74 ++ .../journal-item-revision.ts | 14 + .../journal-pending-submission-recovery.ts | 25 +- .../journal-reducer.test.ts | 130 ++++ .../agent-session-journal/journal-reducer.ts | 57 +- .../journal-row-builders.ts | 5 +- .../journal-row-schema.ts | 2 +- .../journal-store-contracts.ts | 1 + .../agent-session-journal/journal-store.ts | 2 +- .../structured-agent-session-adapter.ts | 7 + ...tructured-agent-session-client-delivery.ts | 75 ++ ...uctured-agent-session-host-test-harness.ts | 197 +++++ .../structured-agent-session-host.test.ts | 305 +------- .../structured-agent-session-host.ts | 44 +- ...ured-agent-session-late-settlement.test.ts | 37 + ...red-agent-session-send-idempotency.test.ts | 2 +- ...ured-agent-session-send-settlement.test.ts | 133 ++++ ...tructured-agent-session-send-settlement.ts | 164 ++++ .../structured-agent-session-send.test.ts | 332 ++++++++ ...agent-session-settled-attach-retry.test.ts | 16 +- ...red-agent-session-surface-lifetime.test.ts | 54 ++ .../structured-agent-session-turns.ts | 65 +- ...ured-agent-session-unexpected-exit.test.ts | 52 ++ ...ructured-agent-session-rpc.test-fixture.ts | 37 +- ...ctured-agent-session-send-compatibility.ts | 26 + .../methods/structured-agent-session.test.ts | 120 +++ .../rpc/methods/structured-agent-session.ts | 3 +- .../NativeChatStructuredSession.tsx | 15 +- ...tiveChatStructuredSessionDelivery.test.tsx | 619 +++++++++++++++ ...e-structured-agent-session-outbox.test.tsx | 268 ++++++- .../use-structured-agent-session-outbox.ts | 126 ++-- ...ctured-agent-session-launch-prompt.test.ts | 60 ++ .../structured-agent-session-launch-prompt.ts | 9 +- src/shared/protocol-version.ts | 6 + src/shared/structured-agent-session-outbox.ts | 3 + .../structured-agent-session-projection.ts | 7 +- ...ructured-agent-session-send-disposition.ts | 138 ++++ ...ss-version-agent-session-wire.unit.test.ts | 2 + .../structured-agent-session-host-fixture.ts | 21 +- 60 files changed, 3989 insertions(+), 1086 deletions(-) create mode 100644 src/main/claude/claude-structured-dispatch-admission.test.ts create mode 100644 src/main/claude/claude-structured-dispatch-test-support.ts create mode 100644 src/main/claude/claude-structured-session-adapter-turns.test.ts create mode 100644 src/main/native-chat/agent-session-journal/journal-dispatch-doubt-reasons.ts create mode 100644 src/main/native-chat/agent-session-journal/journal-item-revision.ts create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-client-delivery.ts create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-host-test-harness.ts create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-send-settlement.test.ts create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-send-settlement.ts create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-send.test.ts create mode 100644 src/main/runtime/rpc/methods/structured-agent-session-send-compatibility.ts create mode 100644 src/renderer/src/components/native-chat/NativeChatStructuredSessionDelivery.test.tsx create mode 100644 src/renderer/src/lib/structured-agent-session-launch-prompt.test.ts create mode 100644 src/shared/structured-agent-session-send-disposition.ts 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