diff --git a/config/scripts/vitest-sqlite-database-runtime-files.mjs b/config/scripts/vitest-sqlite-database-runtime-files.mjs index 9098a6f566db..2e659139c87c 100644 --- a/config/scripts/vitest-sqlite-database-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-database-runtime-files.mjs @@ -31,6 +31,7 @@ export const SQLITE_DATABASE_RUNTIME_INCLUDE = [ 'src/main/runtime/claude-structured-resumed-start-failure.test.ts', 'src/main/runtime/claude-structured-send-during-startup.test.ts', 'src/main/runtime/claude-structured-send-restart-dies-before-dispatch.test.ts', + 'src/main/runtime/claude-structured-send-signed-out-run.test.ts', 'src/main/runtime/claude-structured-session-integration.test.ts', 'src/main/runtime/claude-structured-startup-fault-is-not-an-exit.test.ts', 'src/main/runtime/exit-provenance-audit.test.ts', diff --git a/config/scripts/vitest-sqlite-runtime-files.mjs b/config/scripts/vitest-sqlite-runtime-files.mjs index f1820fd89065..d266c7492d6c 100644 --- a/config/scripts/vitest-sqlite-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-runtime-files.mjs @@ -124,6 +124,7 @@ export const SQLITE_RUNTIME_INCLUDE = [ 'src/main/native-chat/agent-session-wire/structured-agent-session-settled-attach-retry.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-shutdown-cut-row.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-stale-turn-verdict.test.ts', + 'src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-run-lane.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-writer.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-startup-reconcile-failure.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-status-feed-child-work.test.ts', diff --git a/src/main/native-chat/agent-session-journal/journal-dispatch-reducer.ts b/src/main/native-chat/agent-session-journal/journal-dispatch-reducer.ts index 305b7383ad9e..7e5e3b0d0def 100644 --- a/src/main/native-chat/agent-session-journal/journal-dispatch-reducer.ts +++ b/src/main/native-chat/agent-session-journal/journal-dispatch-reducer.ts @@ -61,6 +61,7 @@ export function applyJournalDispatchRow( } if (row.state === 'accepted') { noteTurnAccepted(state, submission) + state.latestAcceptedSequence = Math.max(state.latestAcceptedSequence, row.seq) } if (row.state !== 'accepted' || !row.providerItemId) { return diff --git a/src/main/native-chat/agent-session-journal/journal-lifecycle-batch-appender.ts b/src/main/native-chat/agent-session-journal/journal-lifecycle-batch-appender.ts index 3cac8d03c211..268902917256 100644 --- a/src/main/native-chat/agent-session-journal/journal-lifecycle-batch-appender.ts +++ b/src/main/native-chat/agent-session-journal/journal-lifecycle-batch-appender.ts @@ -11,9 +11,11 @@ import type { JournalResolvedLifecycleBatchInput } from './journal-store-contracts' import type { JournalRow } from './journal-row-schema' -import { journalQueuedRejectionRowBuilders } from './journal-pending-submission-recovery' +import { journalBatchRejectionRowBuilders } from './journal-pending-submission-recovery' +import { withoutRestatedStartFailureRows } from './journal-start-failure-run' const SETTLEMENT_ALREADY_APPLIED = new Error('journal_settlement_already_applied') +const NOTHING_LEFT_TO_WRITE = new Error('journal_lifecycle_batch_nothing_left') export class JournalLifecycleBatchAppender { constructor( @@ -28,25 +30,25 @@ export class JournalLifecycleBatchAppender { ) {} append(input: JournalLifecycleBatchInput): Promise { - const { rejectsQueued } = input - if (rejectsQueued) { - // Planned on the lane: the sends queued then, and this batch unless it already landed. With - // none left (a Stop withdrew them first) it failed no one, so nothing is written. + const { rejects } = input + if (rejects) { + // Planned on the lane: the send if it is still `which` then, and this batch unless it already + // landed or nothing is left once its run's row is counted. With the send settled meanwhile it + // failed no one, so nothing is written. return this.deps .enqueueRows(() => { - const rejections = journalQueuedRejectionRowBuilders( - this.deps.state, - input.fence, - rejectsQueued - ) - return rejections.length === 0 || this.wasApplied(input.settlementId) + const rejections = journalBatchRejectionRowBuilders(this.deps.state, input.fence, rejects) + const mutations = withoutRestatedStartFailureRows(this.deps.state(), input.mutations) + return rejections.length === 0 || + mutations.length === 0 || + this.wasApplied(input.settlementId) ? rejections : [ ...rejections, journalLifecycleBatchRowBuilder( this.deps.state, input.settlementId, - input.mutations, + mutations, input ) ] @@ -56,22 +58,26 @@ export class JournalLifecycleBatchAppender { if (this.wasApplied(input.settlementId)) { return Promise.resolve(this.deps.cursor()) } - const build = journalLifecycleBatchRowBuilder( - this.deps.state, - input.settlementId, - input.mutations, - input - ) return this.deps .enqueue((seq, ts) => { if (this.wasApplied(input.settlementId)) { throw SETTLEMENT_ALREADY_APPLIED } - return build(seq, ts) + const mutations = withoutRestatedStartFailureRows(this.deps.state(), input.mutations) + // Its only row restated its run's: nothing to write. An empty batch still fails its bound. + if (mutations.length === 0 && input.mutations.length > 0) { + throw NOTHING_LEFT_TO_WRITE + } + return journalLifecycleBatchRowBuilder( + this.deps.state, + input.settlementId, + mutations, + input + )(seq, ts) }) .then((row) => ({ epoch: row.epoch, sequence: row.seq })) .catch((error: unknown) => { - if (error === SETTLEMENT_ALREADY_APPLIED) { + if (error === SETTLEMENT_ALREADY_APPLIED || error === NOTHING_LEFT_TO_WRITE) { return this.deps.cursor() } throw error 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 9e9b1f6c57cd..4c5ace097d6f 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 @@ -6,6 +6,7 @@ import type { JournalReducerState } from './journal-reducer' import { journalDispatchRowBuilder } from './journal-row-builders' import type { JournalRow } from './journal-row-schema' import type { AgentSessionJournal } from './journal-store' +import type { JournalBatchRejection } from './journal-store-contracts' /** Settles every submission a process fact left unanswerable. Doubt is never * proof of non-delivery, so nothing here ever becomes re-deliverable. A queued @@ -92,20 +93,22 @@ export async function rejectJournalQueuedSubmissions( return queued.map((entry) => entry.clientMessageId) } -/** Rows rejecting every submission still queued, read from `state` when called: for an append - * that must carry them with what follows, in one transaction. */ -export function journalQueuedRejectionRowBuilders( +/** The row rejecting that one submission, read from `state` when called, or none when `which` no + * longer holds for it: for an append that must carry it with what follows, in one transaction. */ +export function journalBatchRejectionRowBuilders( state: () => JournalReducerState, fence: number, - rejection: AgentJournalDispatchRejection + { clientMessageId, which, ...rejection }: JournalBatchRejection ): ((seq: number, ts: number) => JournalRow)[] { - return [...state().submissions.values()].filter(isQueuedAgentJournalSubmission).map((entry) => - journalDispatchRowBuilder(state, { - clientMessageId: entry.clientMessageId, - state: 'rejected', - ...rejection, - fence, - recovered: true - }) - ) + const submission = state().submissions.get(clientMessageId) + return submission && which(submission) + ? [ + journalDispatchRowBuilder(state, { + clientMessageId, + state: 'rejected', + ...rejection, + fence + }) + ] + : [] } diff --git a/src/main/native-chat/agent-session-journal/journal-queued-rejection-batch.test.ts b/src/main/native-chat/agent-session-journal/journal-queued-rejection-batch.test.ts index cd14ce2fe943..3bb187fc9ca3 100644 --- a/src/main/native-chat/agent-session-journal/journal-queued-rejection-batch.test.ts +++ b/src/main/native-chat/agent-session-journal/journal-queued-rejection-batch.test.ts @@ -1,5 +1,6 @@ -// A failed start's row and the queued messages it failed land in ONE append, the messages first: -// no reader meets one without the other, and the messages sit above the row that says why. +// A failed start's row and the queued message it failed land in ONE append, the message first: no +// reader meets one without the other, the message sits above the row that says why, and a message +// queued behind it is left for its own start. import { mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' @@ -13,7 +14,9 @@ import { AGENT_JOURNAL_THREAD_SCOPE, type AgentSessionJournalIdentity } from '../../../shared/agent-session-journal-types' +import { isQueuedAgentJournalSubmission } from '../../../shared/agent-session-queued-submission' import type { AgentSessionJournal } from './journal-store' +import { MAX_JOURNAL_LIFECYCLE_BATCH_MUTATIONS } from './journal-row-schema' import { closeTestJournalHostDatabases, createTrackedJournalOpener @@ -30,7 +33,7 @@ const IDENTITY: AgentSessionJournalIdentity = { const START_FAILED = agentSessionFailureWords(agentSessionFailureFact('providerStartFailed'), { surface: 'rejection' }) -const ERROR_ROW = { provider: 'orca', clientMessageId: 'start-failure:gen-1' } as const +const ERROR_ROW = { provider: 'orca', clientMessageId: 'start-failure:first' } as const let root: string let clock = 1_000 @@ -66,7 +69,7 @@ async function openWithQueued(...ids: string[]): Promise { function startFailureBatch(mutations = 1) { return { - settlementId: 'start-failure:gen-1', + settlementId: 'start-failure:first', fence: 0, recovered: true as const, mutations: Array.from({ length: mutations }, () => ({ @@ -75,44 +78,55 @@ function startFailureBatch(mutations = 1) { body: { kind: 'status' as const, tone: 'error' as const, text: 'Claude did not start.' }, turnScope: AGENT_JOURNAL_THREAD_SCOPE })), - rejectsQueued: START_FAILED + rejects: { ...START_FAILED, clientMessageId: 'first', which: isQueuedAgentJournalSubmission } } } -it('writes the rejections first and the row after them, and draws the messages above it', async () => { +it('writes the rejection first and the row right after it, leaving the message behind it queued', async () => { const journal = await openWithQueued('first', 'second') const before = journal.cursor().sequence await journal.appendLifecycleBatch(startFailureBatch()) - expect(journal.cursor().sequence).toBe(before + 3) - expect(journal.submissions().map((entry) => entry.dispatchState)).toEqual([ - 'rejected', - 'rejected' - ]) - const order = journal.snapshot().items.map((item) => item.itemId) - expect(order).toEqual([ - agentJournalSubmissionKey('first'), - agentJournalSubmissionKey('second'), - 'orca:start-failure%3Agen-1' - ]) + expect(journal.cursor().sequence).toBe(before + 2) + expect(journal.submissions().map((entry) => entry.dispatchState)).toEqual(['rejected', 'pending']) + const placed = new Map(journal.snapshot().items.map((item) => [item.itemId, item])) + const message = placed.get(agentJournalSubmissionKey('first')) + const row = placed.get('orca:start-failure%3Afirst') + // One write: the row is the next row after the rejection, at the same instant. + expect(row?.sequence).toBe((message?.sequence ?? 0) + 1) + expect(row?.observedAt).toBe(message?.observedAt) }) it('writes neither when the row cannot be written', async () => { const journal = await openWithQueued('first') const before = journal.cursor().sequence - // An empty batch breaks the row's bound, so the transaction rolls back as a whole. - await expect(journal.appendLifecycleBatch(startFailureBatch(0))).rejects.toThrow( - 'journal_lifecycle_batch_mutation_bound_exceeded' - ) + // A batch over the row's bound fails to build, so the transaction rolls back as a whole. + await expect( + journal.appendLifecycleBatch(startFailureBatch(MAX_JOURNAL_LIFECYCLE_BATCH_MUTATIONS + 1)) + ).rejects.toThrow('journal_lifecycle_batch_mutation_bound_exceeded') expect(journal.cursor().sequence).toBe(before) expect(journal.submissions().map((entry) => entry.dispatchState)).toEqual(['pending']) }) +// A start failing as its run's row already says rejects its message and writes no row. +it('writes only the rejection when the batch carries no row', async () => { + const journal = await openWithQueued('first', 'second') + const before = journal.cursor().sequence + + await journal.appendLifecycleBatch(startFailureBatch(0)) + + expect(journal.cursor().sequence).toBe(before + 1) + expect(journal.submissions().map((entry) => entry.dispatchState)).toEqual(['rejected', 'pending']) + expect(journal.snapshot().items.map((item) => item.itemId)).not.toContain( + 'orca:start-failure%3Afirst' + ) +}) + // A Stop that reaches the lane first takes the message back; the failed start then failed no one. -it('writes nothing when a Stop withdrew every queued message first', async () => { +it('writes nothing when a Stop withdrew the message first', async () => { const journal = await openWithQueued('first') const withdrawal = agentSessionFailureWords(agentSessionFailureFact('cancelled'), { surface: 'rejection' 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 264933b7fb33..f802481384e6 100644 --- a/src/main/native-chat/agent-session-journal/journal-reducer.ts +++ b/src/main/native-chat/agent-session-journal/journal-reducer.ts @@ -63,6 +63,9 @@ export type JournalReducerState = { /** The submission row of the latest turn the provider accepted, whoever sent it; 0 when none. * Kept as it folds so the queue's pause reads it in O(1). */ latestAcceptedTurnSequence: number + /** The row at which the provider last accepted a send, with or without an item of its own; 0 + * when none. Kept as it folds; it ends a run of failed starts. */ + latestAcceptedSequence: number /** The latest person's Stop event and Resume, what the queue's pause is derived from. */ queuePauseMarks: JournalQueuePauseMarks } @@ -84,6 +87,7 @@ export function createJournalReducerState(sessionId: string, epoch: string): Jou appliedSettlementIds: new Set(), derivedTurnScope: new JournalDerivedTurnScope(), latestAcceptedTurnSequence: 0, + latestAcceptedSequence: 0, queuePauseMarks: createJournalQueuePauseMarks() } } diff --git a/src/main/native-chat/agent-session-journal/journal-start-failure-run.ts b/src/main/native-chat/agent-session-journal/journal-start-failure-run.ts new file mode 100644 index 000000000000..bc5b2d070f03 --- /dev/null +++ b/src/main/native-chat/agent-session-journal/journal-start-failure-run.ts @@ -0,0 +1,70 @@ +// One row speaks for a run of starts that fail alike. Until a turn is delivered, a start that fails +// as the chat's latest start-failure row says writes no row of its own, and its rejected message is +// read under that row. Decided on the journal's lane, from the fold every earlier write landed in, +// as the batch carrying the row is planned; nothing marks the run. + +import { + readAgentSessionFailureFact, + sameAgentSessionFailureFact, + type AgentSessionFailureFact +} from '../../../shared/agent-session-failure' +import type { AgentJournalRenderItem } from '../../../shared/agent-session-journal-types' +import { agentJournalItemKey } from '../../../shared/agent-session-journal-item-key' +import { + isStructuredAgentSessionCommandStartFailureRow, + isStructuredAgentSessionStartFailureRow +} from '../../../shared/structured-agent-session-start-failure-row-key' +import type { JournalReducerState } from './journal-reducer' +import type { JournalLifecycleMutationInput } from './journal-row-builders' + +/** A batch's mutations less any start-failure row whose failure its run's row already states. */ +export function withoutRestatedStartFailureRows( + state: Pick, + mutations: readonly JournalLifecycleMutationInput[] +): readonly JournalLifecycleMutationInput[] { + return mutations.filter((mutation) => { + if (mutation.kind !== 'item' || mutation.body.kind !== 'status') { + return true + } + const itemId = agentJournalItemKey(mutation.identity) + if ( + !isStructuredAgentSessionStartFailureRow(itemId) || + // A command's failed start always says so: a client that hides its message has only this. + isStructuredAgentSessionCommandStartFailureRow(itemId, (id) => state.items.get(id)?.body) + ) { + return true + } + const fact = readAgentSessionFailureFact(mutation.body.failure) + return fact === undefined || !journalStartFailureAlreadyStated(state, fact) + }) +} + +/** Whether the latest start-failure row states this failure, with no turn delivered since. */ +function journalStartFailureAlreadyStated( + state: Pick, + failure: AgentSessionFailureFact +): boolean { + let latest: AgentJournalRenderItem | undefined + for (const item of state.items.values()) { + if ( + isStructuredAgentSessionStartFailureRow(item.itemId) && + (latest === undefined || item.sequence > latest.sequence) + ) { + latest = item + } + } + const stated = + latest?.body.kind === 'status' ? readAgentSessionFailureFact(latest.body.failure) : undefined + return ( + latest !== undefined && + stated !== undefined && + sameAgentSessionFailureFact(stated, failure) && + // A command's row names the command's next step, so it starts no run for a message. + !isStructuredAgentSessionCommandStartFailureRow( + latest.itemId, + (id) => state.items.get(id)?.body + ) && + // Any send accepted after the row, a command too, ends its run: the next failure is news. + state.latestAcceptedSequence < latest.sequence + ) +} 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 41b892adf348..6efc7ba6ab9a 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 @@ -8,6 +8,7 @@ import type { AgentJournalProducerLinkage, AgentJournalResetReason, AgentJournalRowAttribution, + AgentJournalSubmission, AgentJournalTurnScope, AgentSessionJournalIdentity } from '../../../shared/agent-session-journal-types' @@ -74,15 +75,21 @@ export type JournalLifecycleBatchInput = { mutations: readonly JournalLifecycleMutationInput[] fence: number recovered?: true - /** Rejects the sends still queued with this first, in the same append: a failed start's row - * follows the messages it failed, and no reader meets one without the other. With none still - * queued, the batch is not written either. */ - rejectsQueued?: AgentJournalDispatchRejection + /** Rejects that send first, in the same append: a failed start's row follows the message it + * failed, and no reader meets one without the other. */ + rejects?: JournalBatchRejection +} + +/** One send a batch rejects, when `which` still holds for it on the lane. When it no longer does — + * a Stop withdrew it, another writer settled it — the batch is not written either. */ +export type JournalBatchRejection = AgentJournalDispatchRejection & { + clientMessageId: string + which: (submission: AgentJournalSubmission) => boolean } export type JournalResolvedLifecycleBatchInput = Omit< JournalLifecycleBatchInput, - 'mutations' | 'rejectsQueued' + 'mutations' | 'rejects' > & { /** Read from the fold with every earlier write landed; may return none. */ resolve: () => readonly JournalLifecycleMutationInput[] diff --git a/src/main/native-chat/agent-session-journal/journal-submission-fold.ts b/src/main/native-chat/agent-session-journal/journal-submission-fold.ts index 7b03b4f457f8..a283e1c04c03 100644 --- a/src/main/native-chat/agent-session-journal/journal-submission-fold.ts +++ b/src/main/native-chat/agent-session-journal/journal-submission-fold.ts @@ -113,6 +113,7 @@ export function acceptSubmissionFromProviderItem( submission.fence = row.fence submission.dispatchState = 'accepted' noteTurnAccepted(state, submission) + state.latestAcceptedSequence = Math.max(state.latestAcceptedSequence, row.seq) submission.providerItemId = providerItemId submission.reason = null submission.resolvedAt = row.ts diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-accept-then-deliver.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-accept-then-deliver.test.ts index 263de102f532..8a2e857edbd8 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-accept-then-deliver.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-accept-then-deliver.test.ts @@ -3,12 +3,12 @@ import { AGENT_JOURNAL_THREAD_SCOPE } from '../../../shared/agent-session-journa // session's delivery loop starts a provider child for it and hands it over. Against the real host, // store and journal; each assertion reads what an open chat or the journal's next reader sees. -import type { AgentSessionFailureFact } from '../../../shared/agent-session-failure' 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 { computeAgentSessionPayloadFingerprint } from '../../../shared/agent-session-mutation-envelope' +import { projectStructuredAgentSessionMessages } from '../../../shared/structured-agent-session-message-projection' import { agentJournalSubmissionKey } from '../../../shared/agent-session-journal-item-key' import type { AgentJournalSubmission } from '../../../shared/agent-session-journal-types' import type { @@ -37,7 +37,6 @@ import { HOST_TEST_SESSION as SESSION, HOST_TEST_THREAD as THREAD, hostTestAttachParams, - hostTestDrawnRowIds, hostTestMessage, hostTestOperationId, resetHostTestOperationIds @@ -189,12 +188,6 @@ async function errorRows(): Promise { ) } -async function errorFailures(): Promise<(AgentSessionFailureFact | undefined)[]> { - return (await host.journalSnapshot(SESSION)).items.flatMap((item) => - item.body.kind === 'status' && item.body.tone === 'error' ? [item.body.failure] : [] - ) -} - let subscriptions = 0 async function subscribe(): Promise { @@ -348,71 +341,74 @@ describe('a start the chat needed and did not get', () => { expect(JSON.stringify(await host.journalSnapshot(SESSION))).not.toContain('private') }) - it('writes one error row and rejects every queued message with it; the next send starts (W3)', async () => { + it('rejects only the message whose start failed; the one behind it starts and is delivered (W3)', async () => { await host.close(SESSION, 'evict') acquire.mockRejectedValueOnce(new Error('spawn codex ENOENT')) const first = await accept('first') const second = await accept('second') - await eventually(async () => expect((await submission(second))?.dispatchState).toBe('rejected')) - const rows = await errorRows() - expect(rows).toHaveLength(1) - // Orca's spawn error goes to the log; the row and every message say what failed, typed. The - // child is gone, but no exit was observed, so nothing blames the provider. - expect(rows[0]).toBe("Codex couldn't restart. Send your message to try again.") - const failure = { - kind: 'restartFailed', - refusal: { code: 'agent_session_operation_invalid', details: { ownerVerdict: 'exited' } } - } - expect(await errorFailures()).toEqual([failure]) - for (const id of [first, second]) { - expect(await submission(id)).toMatchObject({ - dispatchState: 'rejected', - reason: rows[0], - rejection: failure - }) - } - - const next = await accept('after the fix') - await eventually(async () => expect((await submission(next))?.dispatchState).toBe('accepted')) - expect(await errorRows()).toHaveLength(1) + await eventually(async () => expect((await submission(second))?.dispatchState).toBe('accepted')) + // Orca's spawn error goes to the log; the message says what failed, typed. The child is gone, + // but no exit was observed, so nothing blames the provider. + expect(await submission(first)).toMatchObject({ + dispatchState: 'rejected', + reason: "Codex couldn't restart. Send your message to try again.", + rejection: { + kind: 'restartFailed', + refusal: { code: 'agent_session_operation_invalid', details: { ownerVerdict: 'exited' } } + } + }) + // The failed start's one row, which a client that hides rejected messages still shows. + expect(await errorRows()).toEqual(["Codex couldn't restart. Send your message to try again."]) + expect(dispatch.mock.calls.map(([input]) => input.clientMessageId)).toEqual([second]) }) - it('draws the messages it failed above the error row, since they were accepted first', async () => { + // A client that hides rejected messages — a released desktop, any phone — still reads why. + it('leaves the failure in the transcript of a client that hides a rejected message sent elsewhere', async () => { await host.close(SESSION, 'evict') acquire.mockRejectedValueOnce(new Error('spawn codex ENOENT')) - const first = await accept('first') - const second = await accept('second') - await eventually(async () => expect((await submission(second))?.dispatchState).toBe('rejected')) + const id = await accept('from another device') + await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected')) const snapshot = await host.journalSnapshot(SESSION) - const errorRow = snapshot.items.find( - (item) => item.body.kind === 'status' && item.body.tone === 'error' - )?.itemId - const shown = [agentJournalSubmissionKey(first), agentJournalSubmissionKey(second), errorRow] - const drawn = hostTestDrawnRowIds(snapshot, [ - { clientMessageId: first, text: 'first' }, - { clientMessageId: second, text: 'second' } - ]) - expect(drawn.filter((id) => shown.includes(id))).toEqual(shown) + const drawn = projectStructuredAgentSessionMessages(snapshot.items, [], snapshot.submissions, { + rejectedInPlace: false + }) + expect(drawn.map((message) => message.id)).not.toContain(agentJournalSubmissionKey(id)) + expect(drawn.flatMap((message) => message.blocks)).toContainEqual( + expect.objectContaining({ text: "Codex couldn't restart. Send your message to try again." }) + ) }) - it('notifies failed once for the queued messages one start failure refused', async () => { + // Messages sent straight to the chat, not queued as cards: each makes its own start, and the + // session announces once, when it owes nothing more. + it('notifies once, for the last, when sent messages whose starts all fail alike are each rejected', async () => { await host.close(SESSION, 'evict') - acquire.mockRejectedValueOnce(new Error('spawn codex ENOENT')) + acquire.mockRejectedValue(new Error('spawn codex ENOENT')) const completions: AgentSessionTurnCompletionEvent[] = [] host.subscribeTurnCompletions({ id: 'dot-1', emit: (event) => completions.push(event) }) - await accept('first') + const first = await accept('first') const second = await accept('second') + const third = await accept('third') - await eventually(async () => expect((await submission(second))?.dispatchState).toBe('rejected')) + await eventually(async () => expect((await submission(third))?.dispatchState).toBe('rejected')) + // Each message made its own start. + for (const id of [first, second, third]) { + expect(await submission(id)).toMatchObject({ + dispatchState: 'rejected', + rejection: { kind: 'restartFailed' } + }) + } + expect(acquire.mock.calls.length).toBeGreaterThanOrEqual(4) + // One row for the run: the starts all failed alike, with nothing delivered between. + expect(await errorRows()).toHaveLength(1) await host.flushAllStreamedEvents() expect(completions).toEqual([ { type: 'completion', completion: expect.objectContaining({ sessionId: SESSION, - turnId: agentJournalSubmissionKey(second), + turnId: agentJournalSubmissionKey(third), outcome: 'failure' }) } @@ -462,28 +458,34 @@ describe('a start the chat needed and did not get', () => { } } ] - ])('writes one row a live chat sees for a %s refusal (W14)', async (_source, arrange, row) => { - await host.close(SESSION, 'evict') - arrange() - await host.flushAllStreamedEvents() - await startHost() - const id = await accept('hello') - const events = await subscribe() - - await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected')) - expect(await errorRows()).toEqual([row.text]) - expect(await errorFailures()).toEqual([row.failure]) - const framedRows = events.flatMap((event) => - event.type === 'batch' || event.type === 'snapshot' - ? (event.type === 'batch' ? event.batch.items : event.page.items).filter( - (item) => item.body.kind === 'status' && item.body.tone === 'error' - ) - : [] - ) - expect(framedRows.length).toBeGreaterThan(0) - }) + ])( + "tells a live chat a %s refusal on the message it failed, beside the start's one row (W14)", + async (_source, arrange, expected) => { + await host.close(SESSION, 'evict') + arrange() + await host.flushAllStreamedEvents() + await startHost() + const id = await accept('hello') + const events = await subscribe() + + const told = () => + events.flatMap((event) => + event.type === 'batch' + ? event.batch.submissions.filter((entry) => entry.clientMessageId === id) + : [] + ) + await eventually(() => + expect(told().at(-1)).toMatchObject({ + dispatchState: 'rejected', + reason: expected.text, + rejection: expected.failure + }) + ) + expect(await errorRows()).toEqual([expected.text]) + } + ) - it('names the start failure on queued messages when the attach fails after acquiring (W4′a)', async () => { + it('names the start failure on the queued message when the attach fails after acquiring (W4′a)', async () => { await host.close(SESSION, 'evict') const id = await accept('hello') // The attach's own success record is the post-acquisition step that fails. @@ -507,7 +509,7 @@ describe('a start the chat needed and did not get', () => { // the store's own error, which is Orca's and goes to the log. expect(settled).toMatchObject({ dispatchState: 'rejected', - rejection: (await errorFailures())[0] + rejection: { kind: 'restartFailed' } }) expect(settled?.reason).not.toContain('record store write failed') expect(await errorRows()).toEqual([settled?.reason]) @@ -625,7 +627,7 @@ describe('a child that exits before its message is handed over', () => { }) describe('a start that fails after it was handed messages', () => { - it('keeps one row in the words its rejected messages carry', async () => { + it('keeps one row in the words its rejected messages carry, each from its own start', async () => { // A starting child takes the first at once; the second waits until that turn opens, which it // never does. Neither is answered. dispatch.mockImplementation(async () => ({ state: 'admitted' as const })) @@ -635,35 +637,41 @@ describe('a start that fails after it was handed messages', () => { })) await host.close(SESSION, 'evict') await startHost() + // The latest child exits before it proves its start, with the CLI's own words for why. + const exitDuringStart = () => + host.handleAdapterEvent({ + type: 'ended', + sessionId: SESSION, + fence: store.getRecord(SESSION)!.lease.runtimeFence, + acquisitionGeneration: `generation-${acquire.mock.calls.length}`, + reason: 'codex app-server exited with code 1', + failure: agentSessionFailureFact('providerStartFailed', { + detail: { text: 'codex: config.toml is invalid', audience: 'person' } + }), + cause: 'unexpected-exit', + startupUnproven: true + }) const first = await accept('first') const second = await accept('second') await eventually(() => expect(dispatch).toHaveBeenCalledTimes(1)) - await host.handleAdapterEvent({ - type: 'ended', - sessionId: SESSION, - fence: store.getRecord(SESSION)!.lease.runtimeFence, - acquisitionGeneration: `generation-${acquire.mock.calls.length}`, - reason: 'codex app-server exited with code 1', - failure: agentSessionFailureFact('providerStartFailed', { - detail: { text: 'codex: config.toml is invalid', audience: 'person' } - }), - cause: 'unexpected-exit', - startupUnproven: true - }) - await host.flushStreamedEvents(SESSION) - - // The held message is rejected by the delivery loop's next step, after the start's own. + // The second's pass found nothing to hand over yet, so it waits on no start. + await eventually(() => expect(host['conversationDelivery'].loop.isRunning(SESSION)).toBe(false)) + await exitDuringStart() + + // The held message was never handed to that child: it gets a start of its own, which fails alike. + await eventually(() => expect(dispatch).toHaveBeenCalledTimes(2)) + expect((await submission(second))?.dispatchState).toBe('pending') + await exitDuringStart() await eventually(async () => expect((await submission(second))?.dispatchState).toBe('rejected')) const rows = (await host.journalSnapshot(SESSION)).items.filter((item) => item.itemId.includes('start-failure') ) + // Both starts failed alike with nothing delivered between: one row, worded from the exit's own + // diagnostic, as is every message it speaks for. expect(rows).toHaveLength(1) - // Worded once, from the exit's own diagnostic, for the row and every message sent to that start. const row = rows[0].body - expect(row).toMatchObject({ - failure: { kind: 'providerStartFailed', detail: { text: 'codex: config.toml is invalid' } } - }) + expect(row).toMatchObject({ failure: { detail: { text: 'codex: config.toml is invalid' } } }) for (const id of [first, second]) { expect(await submission(id)).toMatchObject({ dispatchState: 'rejected', diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-append-delivery.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-append-delivery.test.ts index b80135108056..fae7aa03e984 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-append-delivery.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-append-delivery.test.ts @@ -161,13 +161,17 @@ afterEach(async () => { describe('an open chat receives every row its journal commits', () => { it('shows a failed start whose lease could not be handed back', async () => { const held = await send('hello') + // Handed to the starting child, so that child's failed start is this message's. + await vi.waitFor(async () => + expect((await host.journalSnapshot(SESSION)).submissions[0]?.handedOverAt).toBeDefined() + ) const pane = await liveReader() // The exit settles the journal, then fails to release the lease: nothing moves the fence. vi.spyOn(store, 'transitionHandoff').mockRejectedValueOnce(new Error('record store busy')) await exitBeforeProof() - // The exit ends the child; the delivery loop, which reads why, rejects what it had queued. + // The exit ends the child and rejects what it was handed. await vi.waitFor(() => expect(pane.received().submissions).toContainEqual( expect.objectContaining({ clientMessageId: held, dispatchState: 'rejected' }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts index a983753e2a99..158cfadebabb 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts @@ -213,7 +213,12 @@ export async function endExitedStructuredAgentSessionChildUnderSerialize< )), ...(startFailure ? { exitFailure: startFailure } : {}), ...(startFailed && child.generation - ? { exitedDuringStartup: { generation: child.generation } } + ? { + exitedDuringStartup: { + generation: child.generation, + startedFor: child.startedFor + } + } : {}), exit: watched, ...(unrunRejection ? { unrunRejection } : {}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-exit-ends-record.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-exit-ends-record.test.ts index 00a0644136c3..e2e9e4a3fd6e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-exit-ends-record.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-exit-ends-record.test.ts @@ -452,6 +452,21 @@ it('rejects a message whose start meets a close still unverifiable, and starts n expect(scopes()).toContain('provider-close-unproven') }) +it('fails only the message its refused step was for: one queued behind it starts once the close proves', async () => { + const connection = await stopWithUnprovenClose(2) + + // Both accepted before the first's start step runs. + const [first, second] = await Promise.all([send('First.'), send('Second.')]) + + await eventually(async () => expect((await submission(first))?.dispatchState).toBe('rejected')) + expect((await submission(first))?.rejection).toMatchObject({ + refusal: { details: { reason: 'previousExitUnverifiable' } } + }) + // Its own start joins the close again, which proves this time, and goes to a resumed child. + await resumedWith(connection, 'Second.') + expect((await submission(second))?.dispatchState).not.toBe('rejected') +}) + it('refuses an option change while the close stays unverifiable, never writing to the old child', async () => { const connection = await stopWithUnprovenClose(2) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts index e0130423d5e2..4cf1ffc8b455 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts @@ -6,6 +6,7 @@ // and a child that ends first is settled with it. The host writes a command's end only when the // provider never took it. While the turn runs it takes no input, so the loop hands nothing over. +import { isQueuedAgentJournalSubmission } from '../../../shared/agent-session-queued-submission' import { agentSessionFailureFact, type AgentSessionFailureFact, @@ -60,7 +61,7 @@ export function structuredAgentSessionCompactBody(): AgentJournalMessageItem { type AwaitedSubmission = Pick< AgentJournalSubmission, - 'clientMessageId' | 'dispatchState' | 'acceptedSequence' + 'clientMessageId' | 'dispatchState' | 'acceptedSequence' | 'handoverRecorded' | 'handedOverAt' > /** Optional `submissions` as the dead-generation journal reads it. */ @@ -69,21 +70,40 @@ export type StructuredAgentSessionAwaitedCommandJournal = { itemBody: AgentSessionJournal['itemBody'] } -/** The command the oldest message still waiting on the provider names: a start that fails now - * fails that message first, so its next step is to run the command again. */ -export function structuredAgentSessionAwaitedCommand( - journal: StructuredAgentSessionAwaitedCommandJournal -): AgentSessionConversationCommand | undefined { +/** The oldest message handed to the provider and not yet answered: a start that fails now fails it + * first. A queued one gets a start of its own. */ +export function structuredAgentSessionAwaitedMessage( + journal: Pick +): string | undefined { let oldest: AwaitedSubmission | undefined for (const submission of journal.submissions?.() ?? []) { if ( submission.dispatchState === 'pending' && + !isQueuedAgentJournalSubmission(submission) && (oldest === undefined || (submission.acceptedSequence ?? 0) < (oldest.acceptedSequence ?? 0)) ) { oldest = submission } } - const body = oldest && journal.itemBody(agentJournalSubmissionKey(oldest.clientMessageId)) + return oldest?.clientMessageId +} + +/** The command the oldest message handed to the provider names: its next step is to run the command + * again. */ +export function structuredAgentSessionAwaitedCommand( + journal: StructuredAgentSessionAwaitedCommandJournal +): AgentSessionConversationCommand | undefined { + const awaited = structuredAgentSessionAwaitedMessage(journal) + return awaited === undefined ? undefined : structuredAgentSessionMessageCommand(journal, awaited) +} + +/** The command a message's own body sends: a start that fails it leaves that command to run again, + * not a message to send. */ +export function structuredAgentSessionMessageCommand( + journal: Pick, + clientMessageId: string +): AgentSessionConversationCommand | undefined { + const body = journal.itemBody(agentJournalSubmissionKey(clientMessageId)) return body?.kind === 'message' && body.command?.name === STRUCTURED_AGENT_SESSION_COMPACT_COMMAND ? STRUCTURED_AGENT_SESSION_COMPACT_COMMAND : undefined diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts index 1100c0abf81b..25314131eb03 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts @@ -85,9 +85,8 @@ export function createStructuredAgentSessionConversationLifetime(host: { providerHoldsDispatch: (sessionId) => deps().adapter.holdsDispatch?.(sessionId) === true, // The host puts an idle agent to rest: a turn it cuts short is news, not the user's Stop. stopAgent: (sessionId) => stopAgent(sessionId, { cause: 'evict', resting: true }), - // A host stop: the child's end rejects what it was handed and writes the one error row, and - // the delivery loop rejects what is still queued under the same row, all worded from the - // hostStopped fact. + // A host stop: the child's end rejects what it was handed, and the delivery loop the message it + // was started for, each with that start's one row, worded from the hostStopped fact. stopStartingAgent: (sessionId) => stopStructuredAgentSessionAgentUnderSerialize(host.context(), sessionId, { cause: 'host-stop' diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-dead-generation-settlement.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-dead-generation-settlement.ts index f3c8c5df9fea..159ccf8af152 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-dead-generation-settlement.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-dead-generation-settlement.ts @@ -8,7 +8,6 @@ import { parseAgentJournalItemKey } from '../../../shared/agent-session-journal-item-key' import { STALE_SESSION_ROW_PREFIX } from '../../../shared/agent-session-stop-row-identity' -import { isQueuedAgentJournalSubmission } from '../../../shared/agent-session-queued-submission' import { AGENT_JOURNAL_THREAD_SCOPE, type AgentJournalItemBody, @@ -27,10 +26,8 @@ import { type AgentSessionFailureWordsContext } from '../../../shared/agent-session-failure-words' import { structuredAgentSessionStartFailure } from './structured-agent-session-failure-text' -import { - hasStructuredAgentSessionStartFailureRow, - structuredAgentSessionStartFailureRow -} from './structured-agent-session-start-failure-row' +import { exitStartFailureRow } from './structured-agent-session-start-failure-settlement' +import { structuredAgentSessionAwaitedMessage } from './structured-agent-session-command-turn' import type { AgentSessionDeathEvidence } from '../../../shared/agent-session-record' import { endedByPersonsStop, @@ -77,8 +74,9 @@ export async function settleStructuredAgentSessionDeadGeneration(input: { /** Who a failed start's sentence names. */ failureTextContext?: AgentSessionFailureWordsContext /** The provider never finished starting: the start that failed, keyed by the child's - * generation. Its row is the one the delivery loop writes for the same start. */ - exitedDuringStartup?: { generation: string | null } + * generation, for the row a start no message carries leaves; `startedFor`, the message it was + * started for, which the delivery loop rejects with its own row while it is still queued. */ + exitedDuringStartup?: { generation: string | null; startedFor?: string } /** The exit, watched: what that child's own translator could only end `unverifiable` (its stream * closed before the exit was proven) is revised in this batch. */ exit?: StructuredAgentSessionWatchedExit @@ -104,23 +102,25 @@ export async function settleStructuredAgentSessionDeadGeneration(input: { if (!unrun) { await withdrawCodexSendsNoTurnOpenedFor(input.journal, input.fence) } - await (unrun + // Read before it is rejected: the message a failed start's words are for. + const wordedFor = structuredAgentSessionAwaitedMessage(input.journal) + const settled = await (unrun ? input.journal.rejectPendingSubmissions(input.fence, unrun) : input.journal.markPendingSubmissionsUnknown(input.fence, input.pendingSubmissionReason)) const items = input.journal.snapshot().items const proven = watchedExitRevisions(items, input.exit, input.journal) const mutations: JournalLifecycleMutationInput[] = [] if (showUnexpectedExitOutcome && input.exitedDuringStartup && startupFailure) { - const startKey = input.exitedDuringStartup.generation ?? input.settlementId - // A start a message waited on is the delivery loop's to record, before or after this exit, - // in the words it rejected the message with; this row is for a command, goal or rewind start. - // A row already written stays: rejected is terminal, so its words are not reworded. - const recordedByDeliveryLoop = - input.journal.submissions?.().some(isQueuedAgentJournalSubmission) || - hasStructuredAgentSessionStartFailureRow(items, startKey) - if (!recordedByDeliveryLoop) { - mutations.push(structuredAgentSessionStartFailureRow(startKey, startupFailure)) - } + mutations.push( + ...exitStartFailureRow(input.journal, { + startKey: input.exitedDuringStartup.generation ?? input.settlementId, + wordedFor, + fence: input.fence, + rejected: settled, + startedFor: input.exitedDuringStartup.startedFor, + words: startupFailure + }) + ) } else if (showUnexpectedExitOutcome) { // The turn the exit ended, and an error so no fold ever hides why it stopped. mutations.push({ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.test.ts index 242daf74faf6..61f596359165 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' import { agentSessionFailureFact } from '../../../shared/agent-session-failure' -import { structuredAgentSessionEndedChildFailure } from './structured-agent-session-delivery-loop' +import { structuredAgentSessionEndedChildFailure } from './structured-agent-session-ended-child-failure' import type { StructuredAgentSessionChildEndCause, StructuredAgentSessionEndedChild diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts index 380742f0e235..1a0e972ece17 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts @@ -11,6 +11,10 @@ // queued message: a child's exit only ends the child, and this loop reads why. A message an // earlier host process left queued is never handed over: the open, or this loop's first step, // settles it first (`journal-unsent-send-hold.ts`). +// +// A start that fails is recorded on the one message it was for, and the messages behind it each +// get their own start. Every write names a message fixed when its pass chose it, never the queue's +// head read again after a failure. import type { AgentJournalSubmission } from '../../../shared/agent-session-journal-types' import type { AgentSessionRecord } from '../../../shared/agent-session-record' @@ -19,22 +23,21 @@ import { agentSessionFailureFact } from '../../../shared/agent-session-failure' import type { AgentSessionFailureWordsContext } from '../../../shared/agent-session-failure-words' import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' import type { StructuredAgentRegistry } from './structured-agent-registry' -import { - structuredAgentSessionStartFailure, - type StructuredAgentSessionStartFailureCause -} from './structured-agent-session-failure-text' +import type { StructuredAgentSessionStartFailureCause } from './structured-agent-session-failure-text' import type { StructuredAgentSessionResumeOutcome } from './structured-agent-session-agent-start' import type { - StructuredAgentSessionChildEndCause, - StructuredAgentSessionEndedChild, StructuredAgentSessionHostSession, StructuredAgentSessionProviderChildIdentity } from './structured-agent-session-host-types' import { oldestQueuedSubmission, - recordStructuredAgentSessionStartFailure -} from './structured-agent-session-start-failure-row' -import { failedProviderChildStart } from './structured-agent-session-provider-child' + rejectStructuredAgentSessionStartFailure +} from './structured-agent-session-start-failure-settlement' +import { + childWhoseStartFailed, + startThatFailedWhileQueued, + structuredAgentSessionEndedChildFailure +} from './structured-agent-session-ended-child-failure' import { handOverSubmission } from './structured-agent-session-turns' import { structuredAgentSessionNextHandover } from './structured-agent-session-opening-send' import { structuredAgentSessionCommandRunning } from './structured-agent-session-command-turn' @@ -54,6 +57,8 @@ export type StructuredAgentSessionDeliveryLoopDeps = { sessionId: string, startedFor: string ) => Promise + /** Ends the session's child, whose start failed, as a host stop; for a caller inside `serialize`. */ + endFailedStart: (sessionId: string) => Promise /** The fence the conversation's own writes carry; see `structuredAgentSessionConversationFence`. */ conversationFence: (sessionId: string) => number /** Settles queued messages as a completed close of the chat does; false when that failed. */ @@ -76,12 +81,14 @@ type Step = 'continue' | 'stop' type RefusedStart = Extract type Prepared = - | 'stop' - | RefusedStart - | { ok: true; awaited: StructuredAgentSessionProviderChildIdentity | null } + | Step + | (RefusedStart & { startedFor: string }) + // `waitingFor`: the message the start was for. + | { ok: true; awaited: StructuredAgentSessionProviderChildIdentity | null; waitingFor: string } -/** A failed start before it is worded; `fail` words it once, through the one wording point. */ -type StartFailure = { startKey: string | null; cause: StructuredAgentSessionStartFailureCause } +/** The message one pass is attempting, set before each await that could fail for it, so a throw + * still knows its target. Dies with the pass. */ +type Attempt = { clientMessageId?: string } export class StructuredAgentSessionDeliveryLoop { private readonly running = new Set() @@ -109,63 +116,82 @@ export class StructuredAgentSessionDeliveryLoop { } private async run(sessionId: string): Promise { - try { - for (;;) { - const prepared = await this.deps.trackStart( - this.deps.serialize(sessionId, () => this.prepare(sessionId)) - ) - if (prepared === 'stop') { - return - } - if (!prepared.ok) { - if (prepared.aborted) { - // A close, Stop or quit stopped this start on purpose, and settled what it was for; a - // message accepted since gets its own start, and quit's next step stops the loop. - continue - } - await this.deps.serialize(sessionId, () => - this.fail(sessionId, this.refusedStart(sessionId, prepared)) - ) - return - } - const handed = await this.deps.serialize(sessionId, () => - this.handOver(sessionId, prepared.awaited) - ) - if (handed === 'stop') { - return - } + for (;;) { + const attempt: Attempt = {} + let step: Step + try { + step = await this.attempt(sessionId, attempt) + } catch (error) { + step = await this.failAttempt(sessionId, attempt, error) } - } catch (error) { - // The error is Orca's own and goes to the log; the chat says only that Orca failed. - this.deps.logger.warn('delivering a queued message failed', { - scope: 'delivery-loop', + if (step === 'stop') { + return + } + } + } + + /** One message's pass: a start for it, then its handover. A failed start fails only that + * message; the next pass takes the one behind it. */ + private async attempt(sessionId: string, attempt: Attempt): Promise { + const prepared = await this.deps.trackStart( + this.deps.serialize(sessionId, () => this.prepare(sessionId, attempt)) + ) + if (prepared === 'stop' || prepared === 'continue') { + return prepared + } + if (!prepared.ok) { + if (prepared.aborted) { + // A close, Stop or quit stopped this start on purpose, and settled what it was for; a + // message accepted since gets its own start, and quit's next step stops the loop. + return 'continue' + } + return this.deps.serialize(sessionId, () => + this.rejectStartFailure( + sessionId, + this.refusedStart(sessionId, prepared), + prepared.startedFor + ) + ) + } + return this.deps.serialize(sessionId, () => this.handOver(sessionId, prepared, attempt)) + } + + /** The error is Orca's own and goes to the log; the chat says only that Orca failed, on the + * message this pass attempted and never on another. */ + private async failAttempt(sessionId: string, attempt: Attempt, error: unknown): Promise { + this.deps.logger.warn('delivering a queued message failed', { + scope: 'delivery-loop', + sessionId, + error + }) + const { clientMessageId } = attempt + try { + return await this.deps.serialize(sessionId, async () => + clientMessageId === undefined + ? this.stop(sessionId) + : this.rejectStartFailure(sessionId, { hostFault: true }, clientMessageId) + ) + } catch (failure) { + // Left queued, it is taken by the next wake, or rejected as a leftover by the next open. + this.running.delete(sessionId) + this.deps.logger.warn('recording a failed delivery failed', { + scope: 'delivery-loop-fail', sessionId, - error + error: failure }) - const cause = { hostFault: true } as const - await this.deps - .serialize(sessionId, () => this.fail(sessionId, { startKey: null, cause })) - .catch((failure: unknown) => { - // Rows left queued go to the next loop an accept wakes, or the next open settles them. - this.running.delete(sessionId) - this.deps.logger.warn('recording a failed delivery failed', { - scope: 'delivery-loop-fail', - sessionId, - error: failure - }) - }) + return 'stop' } } /** Settles what an earlier host process left queued, then makes the session ready. */ - private async prepare(sessionId: string): Promise { + private async prepare(sessionId: string, attempt: Attempt): Promise { const session = this.deps.sessions.get(sessionId) if (!session || this.disposed) { return this.stop(sessionId) } // The open already did, unless its write failed: a row an earlier handle wrote is never handed - // over, whether it outlived a quit or a crash. A failure here throws, so none is: this run then - // fails, which rejects every queued send, this process's own too. + // over, whether it outlived a quit or a crash. A failure here throws before any message is + // attempted, so none is handed over: the pass stops, and the next wake or open tries again. await holdRestartedStructuredAgentSessionSends( this.deps.logger, sessionId, @@ -182,30 +208,42 @@ export class StructuredAgentSessionDeliveryLoop { if (!oldest || (session.child && structuredAgentSessionCommandRunning(session.journal))) { return this.stop(sessionId) } - const failedStart = startThatFailedWhileQueued(session, oldest) + const failedStart = startThatFailedWhileQueued(session) if (failedStart) { - return this.fail(sessionId, failedStart) + attempt.clientMessageId = failedStart.clientMessageId + return this.rejectStartFailure(sessionId, failedStart.cause, failedStart.clientMessageId) + } + if (childWhoseStartFailed(session)) { + // The next message gets a fresh start rather than this child's failure. + await this.endFailedStart(sessionId) } + attempt.clientMessageId = oldest.clientMessageId const ready = await this.deps.ensureProviderChild(sessionId, oldest.clientMessageId) if (!ready.ok && ready.refusal.details?.reason === 'previousExitUnverifiable') { // Failed in the step that was refused: a message accepted, or an exit proven, after it must // not be failed for a verdict that no longer holds. - return this.fail(sessionId, this.refusedStart(sessionId, ready)) + return this.rejectStartFailure( + sessionId, + this.refusedStart(sessionId, ready), + oldest.clientMessageId + ) } if (!ready.ok) { - return ready + return { ...ready, startedFor: oldest.clientMessageId } } const child = this.deps.sessions.get(sessionId)?.child // The child this run waits on; handover checks it is still the one there. return { ok: true, - awaited: child ? { generation: child.generation, fence: child.fence } : null + awaited: child ? { generation: child.generation, fence: child.fence } : null, + waitingFor: oldest.clientMessageId } } private async handOver( sessionId: string, - awaited: StructuredAgentSessionProviderChildIdentity | null + { awaited, waitingFor }: Extract, + attempt: Attempt ): Promise { const session = this.deps.sessions.get(sessionId) if (!session || this.disposed) { @@ -228,12 +266,19 @@ export class StructuredAgentSessionDeliveryLoop { if (endedFailure === null) { return 'continue' } - return this.fail(sessionId, { - startKey: awaited?.generation ?? null, - cause: endedFailure ?? + // A start this message only joined fails the message it was for; this one starts afresh. + if (ended && ended.startedFor !== waitingFor) { + return 'continue' + } + // A child still here is ended by the next message's step (`childWhoseStartFailed`), after any + // exit of its own already queued: the message's failure never waits on that cleanup. + return this.rejectStartFailure( + sessionId, + endedFailure ?? // Gone with no end observed: nothing says the provider stopped. - { failure: agentSessionFailureFact('startFailed') } - }) + { failure: agentSessionFailureFact('startFailed') }, + waitingFor + ) } // Never steer into a turn a person's Stop is ending: the message runs after it, as its own // turn, and the turn's end is a commit that wakes the loop again. Read here, at the handover, @@ -246,6 +291,7 @@ export class StructuredAgentSessionDeliveryLoop { if (!next) { return this.stop(sessionId) } + attempt.clientMessageId = next.clientMessageId await handOverSubmission( { sessionId, @@ -264,39 +310,55 @@ export class StructuredAgentSessionDeliveryLoop { return 'continue' } - /** A start the session refused, as the failure every queued message it was for is rejected with. */ + /** A start the session refused, as the failure the message it was for is rejected with. */ private refusedStart( sessionId: string, { refusal, diagnostic, argumentProblem }: RefusedStart - ): StartFailure { + ): StructuredAgentSessionStartFailureCause { // A conversation no agent ever ran, such as a cleared chat's, failed to start, not restart. const newSession = this.deps.record(sessionId)?.providerHandleChain.length === 0 return { - startKey: null, - cause: { - refusal, - ...(diagnostic ? { diagnostic } : {}), - ...(argumentProblem ? { argumentProblem } : {}), - ...(newSession ? { newSession: true as const } : {}) - } + refusal, + ...(diagnostic ? { diagnostic } : {}), + ...(argumentProblem ? { argumentProblem } : {}), + ...(newSession ? { newSession: true as const } : {}) } } - private async fail(sessionId: string, failure: StartFailure): Promise<'stop'> { + /** Fails only the message the start was for; the next pass takes the one behind it. */ + private async rejectStartFailure( + sessionId: string, + cause: StructuredAgentSessionStartFailureCause, + clientMessageId: string + ): Promise<'continue'> { const session = this.deps.sessions.get(sessionId) if (session) { - await recordStructuredAgentSessionStartFailure( - { journal: session.journal, fence: this.deps.conversationFence(sessionId) }, + await rejectStructuredAgentSessionStartFailure( { - startKey: failure.startKey, - ...structuredAgentSessionStartFailure( - failure.cause, - this.deps.failureTextContext(sessionId) - ) - } + journal: session.journal, + fence: this.deps.conversationFence(sessionId), + record: this.deps.record(sessionId) + }, + cause, + clientMessageId ) } - return this.stop(sessionId) + return 'continue' + } + + /** Ends a child whose start failed, once its message is settled. Best effort: a stop that could + * not prove the exit leaves the child closing, and the next start is refused for that + * (`previousExitUnverifiable`), as after any such stop. */ + private async endFailedStart(sessionId: string): Promise { + try { + await this.deps.endFailedStart(sessionId) + } catch (error) { + this.deps.logger.warn('ending a failed start failed', { + scope: 'delivery-loop-end-failed-start', + sessionId, + error + }) + } } /** A close of this chat that stopped its child and then did not complete still closed what was @@ -326,55 +388,3 @@ export class StructuredAgentSessionDeliveryLoop { return 'stop' } } - -/** A start that died while this message waited on it — a view's, say — is the message's failed - * start: settled with it, under its key, rather than started again into the same failure. */ -function startThatFailedWhileQueued( - session: StructuredAgentSessionHostSession, - oldest: NonNullable> -): StartFailure | null { - const ended = failedProviderChildStart(session) - if ( - !ended || - oldest.acceptedSequence === undefined || - ended.endedAt.epoch !== session.journal.cursor().epoch || - ended.endedAt.sequence < oldest.acceptedSequence - ) { - return null - } - const cause = structuredAgentSessionEndedChildFailure(ended) - return cause ? { startKey: ended.generation, cause } : null -} - -function providerEndFailure( - ended: StructuredAgentSessionEndedChild -): StructuredAgentSessionStartFailureCause { - if (ended.duringStartup) { - return { exit: ended.failure } - } - return { failure: ended.failure ?? agentSessionFailureFact('providerExited') } -} - -// Every end cause, so a new one does not compile until it says whether it fails what is queued. -const ENDED_CHILD_FAILURE = { - 'user-stop': () => null, - // The user closing this chat closes what was queued before it; see `closeWhatTheUserClosed`. - 'user-close': () => null, - // The host stopping the child is Orca's cause, never the provider's: a start that never finished. - 'host-stop': () => ({ failure: agentSessionFailureFact('hostStopped') }), - exit: providerEndFailure, - // The attach records its own fault as the end's failure. - 'attach-failed': providerEndFailure, - // Reached only when an eviction's stop landed and a later step failed, leaving the conversation. - evict: providerEndFailure -} satisfies Record< - StructuredAgentSessionChildEndCause, - (ended: StructuredAgentSessionEndedChild) => StructuredAgentSessionStartFailureCause | null -> - -/** Why a queued message the child never took is rejected; null when its end fails nothing. */ -export function structuredAgentSessionEndedChildFailure( - ended: StructuredAgentSessionEndedChild -): StructuredAgentSessionStartFailureCause | null { - return ENDED_CHILD_FAILURE[ended.cause](ended) -} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-ended-child-failure.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-ended-child-failure.ts new file mode 100644 index 000000000000..d496bfefc2f1 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-ended-child-failure.ts @@ -0,0 +1,76 @@ +// What a provider child's end means for the queued message it was started for: a failed start that +// message takes, or nothing. Read from the child records and the journal each step, never latched. + +import { agentSessionFailureFact } from '../../../shared/agent-session-failure' +import type { StructuredAgentSessionStartFailureCause } from './structured-agent-session-failure-text' +import type { + StructuredAgentSessionChildEndCause, + StructuredAgentSessionEndedChild, + StructuredAgentSessionHostSession +} from './structured-agent-session-host-types' +import { failedProviderChildStart } from './structured-agent-session-provider-child' +import { + isStillQueued, + rejectedAsFailedStartAt +} from './structured-agent-session-start-failure-settlement' + +/** A start that died between steps fails the message it was for, while that message is still + * queued: once it is rejected nothing matches, so the next message starts afresh. A view's start + * was for no message and fails none. */ +export function startThatFailedWhileQueued( + session: StructuredAgentSessionHostSession +): { cause: StructuredAgentSessionStartFailureCause; clientMessageId: string } | null { + const ended = failedProviderChildStart(session) + const startedFor = ended?.startedFor + if (!ended || startedFor === undefined || !isStillQueued(session.journal, startedFor)) { + return null + } + const cause = structuredAgentSessionEndedChildFailure(ended) + return cause ? { cause, clientMessageId: startedFor } : null +} + +/** A child still starting whose start a message was already rejected for: read from the journal, + * so no flag can outlive it. Ended before the next message's start rather than at once, so an exit + * of its own, already on its way, settles it first. One a stop is closing is that stop's. */ +export function childWhoseStartFailed(session: StructuredAgentSessionHostSession): boolean { + const child = session.child + if (!child || child.phase !== 'starting' || child.close) { + return false + } + return session.journal + .submissions() + .some((submission) => rejectedAsFailedStartAt(submission, child.fence)) +} + +function providerEndFailure( + ended: StructuredAgentSessionEndedChild +): StructuredAgentSessionStartFailureCause { + if (ended.duringStartup) { + return { exit: ended.failure } + } + return { failure: ended.failure ?? agentSessionFailureFact('providerExited') } +} + +// Every end cause, so a new one does not compile until it says whether it fails what is queued. +const ENDED_CHILD_FAILURE = { + 'user-stop': () => null, + // The user closing this chat closes what was queued before it; see `closeWhatTheUserClosed`. + 'user-close': () => null, + // The host stopping the child is Orca's cause, never the provider's: a start that never finished. + 'host-stop': () => ({ failure: agentSessionFailureFact('hostStopped') }), + exit: providerEndFailure, + // The attach records its own fault as the end's failure. + 'attach-failed': providerEndFailure, + // Reached only when an eviction's stop landed and a later step failed, leaving the conversation. + evict: providerEndFailure +} satisfies Record< + StructuredAgentSessionChildEndCause, + (ended: StructuredAgentSessionEndedChild) => StructuredAgentSessionStartFailureCause | null +> + +/** Why a queued message the child never took is rejected; null when its end fails nothing. */ +export function structuredAgentSessionEndedChildFailure( + ended: StructuredAgentSessionEndedChild +): StructuredAgentSessionStartFailureCause | null { + return ENDED_CHILD_FAILURE[ended.cause](ended) +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts index 9abd58aa0218..52fb60e77a09 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts @@ -56,6 +56,8 @@ export function createStructuredAgentSessionConversationDelivery(input: { sessionId: string, startedFor: string ) => Promise + /** Ends the session's child, whose start failed, as a host stop; for a caller inside `serialize`. */ + endFailedStart: (sessionId: string) => Promise clientDelivery: Pick< StructuredAgentSessionClientDelivery, 'publishRestored' | 'readChildWork' | 'readStopping' | 'publishStatus' @@ -69,6 +71,7 @@ export function createStructuredAgentSessionConversationDelivery(input: { serialize: input.serialize, trackStart: input.trackStart, ensureProviderChild: input.ensureProviderChild, + endFailedStart: input.endFailedStart, conversationFence: (sessionId) => structuredAgentSessionConversationFence(deps.store, sessionId), holdClosed: async (sessionId, which) => { 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 e0f193ba0b49..9d04148860e7 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 @@ -121,6 +121,7 @@ export class StructuredAgentSessionHost { trackStart: (start) => this.tasks.trackAttach(start), ensureProviderChild: (sessionId, startedFor) => agentStart.ensureStructuredAgentSessionAgent(this.attachContext(), sessionId, startedFor), + endFailedStart: (sessionId) => this.lifetime.stopAgent(sessionId, { cause: 'host-stop' }), clientDelivery: this.clientDelivery }) this.restore = reveal.createStructuredAgentSessionHostRestore(deps, { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-opening-send.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-opening-send.ts index 355d61bf3d77..9344fef5b321 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-opening-send.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-opening-send.ts @@ -3,7 +3,7 @@ import { structuredAgentSessionSendOpeningTurn } from '../../../shared/structured-agent-session-opening-send' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' import type { StructuredAgentSessionHostSession } from './structured-agent-session-host-types' -import { oldestQueuedSubmission } from './structured-agent-session-start-failure-row' +import { oldestQueuedSubmission } from './structured-agent-session-start-failure-settlement' /** Whether a send this child (`fence`) was handed is still opening its turn * (`structuredAgentSessionSendOpeningTurn`), read off the journal's fold without a snapshot. */ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.ts index 8c98a685c7d2..efd424107a22 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.ts @@ -21,6 +21,8 @@ import { } from '../../../shared/agent-session-failure' import { agentSessionFailureWords } from '../../../shared/agent-session-failure-words' import { readAgentJournalTurn } from '../../../shared/agent-session-turn-record' +import { agentJournalItemKey } from '../../../shared/agent-session-journal-item-key' +import { structuredAgentSessionStartFailureRowIdentity } from '../../../shared/structured-agent-session-start-failure-row-key' import { openAgentSessionJournal } from '../agent-session-journal/journal-store-factory' import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' @@ -184,6 +186,11 @@ async function submission(id: string): Promise, reason: string, startupUnp }) } +/** Whether a subscriber was told the message was rejected. */ function rejectedIn(events: AgentSessionSubscribeEvent[], id: string): boolean { return events.some( (event) => @@ -237,6 +245,10 @@ function rejectedIn(events: AgentSessionSubscribeEvent[], id: string): boolean { ) } +async function rejected(id: string): Promise { + await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected')) +} + function conversation() { return host['sessions'].get(SESSION) } @@ -383,7 +395,7 @@ describe('a published child that dies while it proves its start', () => { const EXIT = START_EXIT const TEXT = START_TEXT - it('leaves one error row keyed by the start, and every message it was handed rejected with it (R2)', async () => { + it('fails the message it was handed, under one row keyed by it, and the one behind starts afresh (R2)', async () => { await restartHost() acquire.mockImplementation(spawnStartingChild) // The first is written to the starting child at once; it never answers, so its turn never @@ -393,28 +405,22 @@ describe('a published child that dies while it proves its start', () => { const events = await subscribe() const second = await accept('second') await eventually(() => expect(dispatch).toHaveBeenCalledTimes(1)) + await settleLoop() const child = currentChild() await exit(child, EXIT, true) - await eventually(async () => expect((await submission(second))?.dispatchState).toBe('rejected')) + await rejected(first) + // The second was never that child's: it is handed to a start of its own. + await eventually(() => expect(dispatch).toHaveBeenCalledTimes(2)) + expect(await submission(first)).toMatchObject({ reason: TEXT, rejection: START_FAILURE }) + expect(await submission(second)).toMatchObject({ dispatchState: 'pending' }) + expect(rejectedIn(events, first)).toBe(true) + expect(rejectedIn(events, second)).toBe(false) expect(await statusRows()).toEqual([ - { - itemId: `orca:${encodeURIComponent(`start-failure:${child.acquisitionGeneration}`)}`, - text: TEXT, - tone: 'error', - failure: START_FAILURE - } + { itemId: startRowKey(first), text: TEXT, tone: 'error', failure: START_FAILURE } ]) - for (const id of [first, second]) { - expect(await submission(id)).toMatchObject({ - dispatchState: 'rejected', - reason: TEXT, - rejection: START_FAILURE - }) - } - expect(rejectedIn(events, second)).toBe(true) - expect(acquire).toHaveBeenCalledTimes(2) + expect(acquire).toHaveBeenCalledTimes(3) }) }) @@ -429,9 +435,11 @@ describe('a start another operation made that dies while a sent message waits on const EXIT = START_EXIT const TEXT = START_TEXT - it("is the message's own failed start: one error row, the message rejected, no second start (R2)", async () => { + // The exit lands between the message's accept and its first step: that start was for no message, + // so its exit's row is the only place its failure is said, and the message makes its own start. + it("is not the message's: its exit's row says why, and the message starts afresh (R2)", async () => { await restartHost() - acquire.mockImplementation(spawnStartingChild) + acquire.mockImplementationOnce(spawnStartingChild) // An operation that needs the agent starts a child that has not proven its start. await startForOperation() const operationChild = currentChild() @@ -448,21 +456,15 @@ describe('a start another operation made that dies while a sent message waits on await exited const id = params.envelope.clientOperationId - await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected')) + await eventually(async () => expect((await submission(id))?.dispatchState).toBe('accepted')) await settleLoop() - expect((await submission(id))?.reason).toBe(TEXT) - expect(await statusRows()).toEqual([ - { - itemId: `orca:${encodeURIComponent(`start-failure:${operationChild.acquisitionGeneration}`)}`, - text: TEXT, - tone: 'error', - failure: START_FAILURE - } + expect(rejectedIn(events, id)).toBe(false) + // The setup's child, the operation's, and the message's own. + expect(acquire).toHaveBeenCalledTimes(3) + expect(dispatch.mock.calls.map(([input]) => input.clientMessageId)).toEqual([id]) + expect((await statusRows()).map((row) => row.itemId)).toEqual([ + startRowKey(operationChild.acquisitionGeneration) ]) - expect(rejectedIn(events, id)).toBe(true) - // The setup's child and the operation's: nothing started again into the same failure. - expect(acquire).toHaveBeenCalledTimes(2) - expect(dispatch).not.toHaveBeenCalled() }) it('leaves a message sent after that start failed to a fresh start (R2)', async () => { @@ -470,7 +472,10 @@ describe('a start another operation made that dies while a sent message waits on acquire.mockImplementationOnce(spawnStartingChild) await startForOperation() await exit(currentChild(), EXIT, true) - expect(await statusRows()).toHaveLength(1) + // A start for no message says why it failed in its exit's row. + expect(await statusRows()).toEqual([ + expect.objectContaining({ text: TEXT, tone: 'error', failure: START_FAILURE }) + ]) const id = await accept('after the failure') @@ -543,18 +548,22 @@ describe('a child that ends before its message is handed over', () => { const id = await accept('hello') const events = await subscribe() - await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected')) + await rejected(id) // The exit's stderr rides beside the sentence, never in it. - const failure = { - kind: 'providerExited', - detail: { text: 'codex app-server crashed', audience: 'log' } - } expect(await submission(id)).toMatchObject({ + dispatchState: 'rejected', reason: 'Codex stopped before this message was sent.', - rejection: failure + rejection: { + kind: 'providerExited', + detail: { text: 'codex app-server crashed', audience: 'log' } + } }) expect(await statusRows()).toEqual([ - { itemId: expect.any(String), text: (await submission(id))?.reason, tone: 'error', failure } + expect.objectContaining({ + itemId: startRowKey(id), + text: 'Codex stopped before this message was sent.', + tone: 'error' + }) ]) expect(rejectedIn(events, id)).toBe(true) await eventually(() => expect(host['conversationDelivery'].loop.isRunning(SESSION)).toBe(false)) @@ -670,19 +679,20 @@ describe('a quit with a message still queued', () => { }) describe('a send whose start failed, sent again with the same operation id', () => { - it('replays the recorded rejection and starts no second agent (R2)', async () => { + it('replays the recorded answer and starts no second agent (R2)', async () => { acquire.mockRejectedValueOnce(new Error('spawn claude ENOENT')) const fenceBefore = store.getRecord(SESSION)!.lease.runtimeFence const params = sendParams('hello') - // A failed start is a rejected message, never a refused send. + // A failed start rejects the accepted message, never the send. expect(await host.send(CALLER, params)).toMatchObject({ ok: true, value: { submission: { dispatchState: 'pending' } } }) const id = params.envelope.clientOperationId - await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected')) + await rejected(id) // The start moved the fence while the message was out. expect(store.getRecord(SESSION)!.lease.runtimeFence).toBeGreaterThan(fenceBefore) + await settleLoop() const starts = acquire.mock.calls.length const replay = await host.send(CALLER, params) @@ -690,7 +700,13 @@ describe('a send whose start failed, sent again with the same operation id', () expect(replay).toMatchObject({ ok: true, replayed: true, - value: { submission: { clientMessageId: id, dispatchState: 'rejected' } } + value: { + submission: { + clientMessageId: id, + dispatchState: 'rejected', + rejection: { kind: 'restartFailed' } + } + } }) await settleLoop() expect(acquire).toHaveBeenCalledTimes(starts) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-send-preparation.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-send-preparation.test.ts index f1bbd673cd62..eda3b4166702 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-send-preparation.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-send-preparation.test.ts @@ -472,12 +472,12 @@ describe('a send with no live owner', () => { expect(acquire).toHaveBeenCalledTimes(1) expect(await errorStatuses()).toHaveLength(1) - // The outbox's Retry rotates the id: a fresh attempt, with its own row. + // The outbox's Retry rotates the id: a fresh attempt. It failed alike, so the run's row says why. expect(await settled(await accept(sendParams('while signed out')))).toMatchObject({ dispatchState: 'rejected' }) expect(acquire).toHaveBeenCalledTimes(2) - expect(await errorStatuses()).toHaveLength(2) + expect(await errorStatuses()).toHaveLength(1) expect(dispatch).not.toHaveBeenCalled() }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-send-restarts-failed-start.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-send-restarts-failed-start.test.ts index 592f82653750..322611847c5a 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-send-restarts-failed-start.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-send-restarts-failed-start.test.ts @@ -217,7 +217,8 @@ describe('a send into a published session whose child ended before startup', () await exitBeforeProof() // The child never proved its start, so it accepted nothing: the exit rejects the message this - // host admitted, so nothing pins the session and Retry stays offered, and one row names the cause. + // host admitted, so nothing pins the session and Retry stays offered. The first start failed + // alike with nothing delivered since, so its row names the cause for this one too. expect(await submission(held)).toMatchObject({ dispatchState: 'rejected', reason: 'Codex stopped before it finished starting. Send your message to try again.', @@ -227,13 +228,11 @@ describe('a send into a published session whose child ended before startup', () expect( (await host.journalSnapshot(SESSION)).submissions.filter((e) => e.dispatchState === 'pending') ).toEqual([]) - expect((await journalStatuses()).slice(rowsBefore)).toEqual([ - 'Codex stopped before it finished starting. Send your message to try again.' - ]) - // Accepted before the restart it needed, so the chat draws it above the row naming the cause. + expect((await journalStatuses()).slice(rowsBefore)).toEqual([]) + // Rejected after the run's row, so the chat draws it below the row naming the cause. const snapshot = await host.journalSnapshot(SESSION) const causeRow = snapshot.items.findLast((item) => item.body.kind === 'status')?.itemId - const shown = [agentJournalSubmissionKey(held), causeRow] + const shown = [causeRow, agentJournalSubmissionKey(held)] expect( hostTestDrawnRowIds(snapshot, [ { clientMessageId: held, text: 'still not signed in' } @@ -249,7 +248,7 @@ describe('a send into a published session whose child ended before startup', () await send('signed in now') expect(acquire).toHaveBeenCalledTimes(3) expect(dispatch).toHaveBeenCalledTimes(2) - expect((await journalStatuses()).slice(rowsBefore)).toHaveLength(1) + expect((await journalStatuses()).slice(rowsBefore)).toEqual([]) }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-row.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-row.ts deleted file mode 100644 index 9206f95fde0b..000000000000 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-row.ts +++ /dev/null @@ -1,80 +0,0 @@ -import { agentJournalItemKey } from '../../../shared/agent-session-journal-item-key' -import { AGENT_JOURNAL_THREAD_SCOPE } from '../../../shared/agent-session-journal-types' -import { isQueuedAgentJournalSubmission } from '../../../shared/agent-session-queued-submission' -import { structuredAgentSessionStartFailureRowIdentity } from '../../../shared/structured-agent-session-start-failure-row-key' -import type { JournalLifecycleMutationInput } from '../agent-session-journal/journal-row-builders' -import type { StructuredAgentSessionHostSession } from './structured-agent-session-host-types' -import type { StructuredAgentSessionStartFailureWords } from './structured-agent-session-failure-text' - -/** A start that failed, keyed like its row, in the words `structuredAgentSessionStartFailure` gave. */ -export type StructuredAgentSessionStartFailure = StructuredAgentSessionStartFailureWords & { - startKey: string | null -} - -/** - * The one row a start that failed leaves in the chat, whoever saw it fail: an error row, so the - * reason outlives any error strip. Keyed by the start — the child's generation, or the oldest - * message it was for when no child was ever published — so a second report of the same failure - * lands on the same row instead of adding one. - */ -export function structuredAgentSessionStartFailureRow( - startKey: string, - words: StructuredAgentSessionStartFailureWords -): JournalLifecycleMutationInput { - return { - kind: 'item', - identity: structuredAgentSessionStartFailureRowIdentity(startKey), - // The row repeats the sentence the start's rejected messages carry. - body: { kind: 'status', text: words.reason, tone: 'error', failure: words.rejection }, - // A start that failed opened no turn. - turnScope: AGENT_JOURNAL_THREAD_SCOPE - } -} - -/** Whether the start's row is already written. Its words are the ones its rejected messages carry, - * and rejected is terminal, so the exit's later report of the same start must not reword it. */ -export function hasStructuredAgentSessionStartFailureRow( - items: readonly { itemId: string }[], - startKey: string -): boolean { - const itemId = agentJournalItemKey(structuredAgentSessionStartFailureRowIdentity(startKey)) - return items.some((item) => item.itemId === itemId) -} - -/** - * A start the delivery loop needed and did not get: every queued message rejected with the same - * words, then the start's row. Writes nothing when nothing is still queued: a start whose messages - * Stop withdrew did not fail anyone. - */ -export async function recordStructuredAgentSessionStartFailure( - session: Pick & { fence: number }, - failure: StructuredAgentSessionStartFailure -): Promise { - const oldest = oldestQueuedSubmission(session) - if (!oldest) { - return - } - const startKey = failure.startKey ?? oldest.clientMessageId - await session.journal.appendLifecycleBatch({ - settlementId: `start-failure:${startKey}`, - fence: session.fence, - recovered: true, - mutations: [structuredAgentSessionStartFailureRow(startKey, failure)], - rejectsQueued: { reason: failure.reason, rejection: failure.rejection } - }) -} - -export function oldestQueuedSubmission( - session: Pick -): ReturnType[number] | undefined { - let oldest: ReturnType - for (const submission of session.journal.submissions()) { - if ( - isQueuedAgentJournalSubmission(submission) && - (oldest === undefined || (submission.acceptedSequence ?? 0) < (oldest.acceptedSequence ?? 0)) - ) { - oldest = submission - } - } - return oldest -} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-run-lane.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-run-lane.test.ts new file mode 100644 index 000000000000..b6f1b8e65755 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-run-lane.test.ts @@ -0,0 +1,185 @@ +// Whether a failed start's row repeats its run's is decided on the journal's lane, as the batch +// carrying it is planned: a send accepted ahead of it in that lane ends the run, however late the +// writer read the journal. Each case holds the lane with a write in progress and issues the others +// from inside it, so the lane, not timing, orders them. + +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, expect, it } from 'vitest' +import { agentSessionFailureFact } from '../../../shared/agent-session-failure' +import { agentSessionFailureWords } from '../../../shared/agent-session-failure-words' +import { agentJournalItemKey } from '../../../shared/agent-session-journal-item-key' +import { claudeProviderHandle } from '../../../shared/agent-session-provider-handle-encoding' +import { + AGENT_JOURNAL_THREAD_SCOPE, + type AgentSessionJournalIdentity +} from '../../../shared/agent-session-journal-types' +import { isQueuedAgentJournalSubmission } from '../../../shared/agent-session-queued-submission' +import { + isStructuredAgentSessionStartFailureRow, + structuredAgentSessionStartFailureRowIdentity +} from '../../../shared/structured-agent-session-start-failure-row-key' +import type { AgentSessionJournal } from '../agent-session-journal/journal-store' +import { + closeTestJournalHostDatabases, + createTrackedJournalOpener +} from '../agent-session-journal/journal-host-database-test-support' +import { + rejectWithStartFailureRow, + structuredAgentSessionStartFailureRow +} from './structured-agent-session-start-failure-settlement' + +const IDENTITY: AgentSessionJournalIdentity = { + sessionId: 'session-run-lane', + workspaceId: 'ws-1', + hostId: 'host-1', + agent: 'claude', + providerHandle: claudeProviderHandle('native-1', null) +} +const SIGNED_OUT = agentSessionFailureWords(agentSessionFailureFact('notSignedIn'), { + surface: 'rejection' +}) +const NOTE = { provider: 'orca', clientMessageId: 'note' } as const + +let root: string +let clock = 1_000 +const journals = createTrackedJournalOpener() + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-start-failure-run-lane-')) +}) + +afterEach(async () => { + await closeTestJournalHostDatabases() + await rm(root, { recursive: true, force: true }) +}) + +/** A chat whose first queued message already failed its start, leaving the run's row. */ +async function openAfterFailedStart(...queued: string[]): Promise { + const journal = await journals.open({ + identity: IDENTITY, + stateDirectory: root, + now: () => (clock += 1), + mintEpoch: () => 'epoch-1' + }) + for (const id of ['first', ...queued]) { + await journal.appendSubmission({ + clientMessageId: id, + payloadFingerprint: `fp-${id}`, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: id }] }, + fence: 0, + handoverRecorded: true + }) + } + await reject(journal, 'first') + return journal +} + +function reject(journal: AgentSessionJournal, clientMessageId: string): Promise { + return rejectWithStartFailureRow(journal, { + clientMessageId, + words: SIGNED_OUT, + fence: 0, + which: isQueuedAgentJournalSubmission + }) +} + +function accept(journal: AgentSessionJournal, clientMessageId: string): Promise { + return journal.resolveDispatch({ + clientMessageId, + state: 'accepted', + providerIdentity: { + provider: 'claude', + sessionId: 'native-1', + uuid: `echo-${clientMessageId}` + }, + fence: 0 + }) +} + +/** Runs `issue` from inside a write in progress, so whatever it issues joins the lane behind it. */ +async function fromInsideAWrite( + journal: AgentSessionJournal, + issue: () => Promise[] +): Promise { + let issued: Promise[] = [] + await journal.appendResolvedItem( + () => { + issued = issue() + return { identity: NOTE, body: { kind: 'status', tone: 'info', text: 'held' } } + }, + { fence: 0, turnScope: AGENT_JOURNAL_THREAD_SCOPE } + ) + await Promise.all(issued) +} + +function startRows(journal: AgentSessionJournal): string[] { + return journal + .snapshot() + .items.flatMap((item) => + isStructuredAgentSessionStartFailureRow(item.itemId) ? [item.itemId] : [] + ) +} + +const rowKey = (id: string) => + agentJournalItemKey(structuredAgentSessionStartFailureRowIdentity(id)) + +it('writes the row of a start failing alike when a send was accepted ahead of it in the lane', async () => { + const journal = await openAfterFailedStart('delivered', 'second') + + // Both issued before either lands: the writer reads a journal with no accept in it yet. + await fromInsideAWrite(journal, () => [accept(journal, 'delivered'), reject(journal, 'second')]) + + expect(journal.submission('delivered')?.dispatchState).toBe('accepted') + expect(journal.submission('second')?.dispatchState).toBe('rejected') + expect(startRows(journal)).toEqual([rowKey('first'), rowKey('second')]) +}) + +it('writes no row for a start failing alike when nothing was accepted ahead of it', async () => { + const journal = await openAfterFailedStart('second') + + await fromInsideAWrite(journal, () => [reject(journal, 'second')]) + + expect(journal.submission('second')?.dispatchState).toBe('rejected') + expect(startRows(journal)).toEqual([rowKey('first')]) +}) + +// The exit's batch also ends any turn its child ran: only the restated row is dropped. +it('drops only a restated row from a batch that carries other writes', async () => { + const journal = await openAfterFailedStart() + + await journal.appendLifecycleBatch({ + settlementId: 'dead-generation:generation-2', + fence: 0, + recovered: true, + mutations: [ + structuredAgentSessionStartFailureRow('generation-2', SIGNED_OUT), + { + kind: 'item', + identity: NOTE, + body: { kind: 'status', tone: 'info', text: 'settled' }, + turnScope: AGENT_JOURNAL_THREAD_SCOPE + } + ] + }) + + expect(startRows(journal)).toEqual([rowKey('first')]) + expect(journal.itemBody(agentJournalItemKey(NOTE))).toMatchObject({ text: 'settled' }) +}) + +it('writes nothing, and settles, for a batch whose only write restates its run', async () => { + const journal = await openAfterFailedStart() + const before = journal.cursor().sequence + + await expect( + journal.appendLifecycleBatch({ + settlementId: 'dead-generation:generation-2', + fence: 0, + recovered: true, + mutations: [structuredAgentSessionStartFailureRow('generation-2', SIGNED_OUT)] + }) + ).resolves.toEqual({ epoch: 'epoch-1', sequence: before }) + + expect(startRows(journal)).toEqual([rowKey('first')]) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-settlement.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-settlement.ts new file mode 100644 index 000000000000..3ec0334316f9 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-settlement.ts @@ -0,0 +1,178 @@ +// The writer of a failed start's record: the message it failed, rejected, and its row keyed by that +// message, in one write, so a client that hides rejected messages still sees why. The delivery loop +// writes it for the queued message a start was for, the handover for the message it was handing +// over; the exit writes one row for the handed messages it rejected, keyed by the oldest, whose +// words it carries, or one keyed by the start for a start no message carries. Each message state +// has one writer, and `rejected` is terminal, so no message is failed twice. A start failing as +// its run's row already says writes no row; its message is read under that one. + +import type { + AgentJournalDispatchRejection, + AgentSessionFailureWordsContext +} from '../../../shared/agent-session-failure-words' +import { + AGENT_JOURNAL_THREAD_SCOPE, + type AgentJournalSubmission +} from '../../../shared/agent-session-journal-types' +import { isQueuedAgentJournalSubmission } from '../../../shared/agent-session-queued-submission' +import type { AgentSessionRecord } from '../../../shared/agent-session-record' +import { isFailedStartOrHostFault } from '../../../shared/structured-agent-session-dispatch-rejection' +import { structuredAgentSessionStartFailureRowIdentity } from '../../../shared/structured-agent-session-start-failure-row-key' +import type { JournalLifecycleMutationInput } from '../agent-session-journal/journal-row-builders' +import type { AgentSessionJournal } from '../agent-session-journal/journal-store' +import type { StructuredAgentSessionHostSession } from './structured-agent-session-host-types' +import { structuredAgentSessionMessageCommand } from './structured-agent-session-command-turn' +import { + structuredAgentSessionStartFailure, + type StructuredAgentSessionStartFailureCause +} from './structured-agent-session-failure-text' +import { structuredAgentSessionFailureWordsContext } from './structured-agent-session-send-preparation' + +type StartFailureJournal = Pick< + AgentSessionJournal, + 'submissions' | 'itemBody' | 'appendLifecycleBatch' +> + +/** A failed start's row in the chat: an error row keyed by its message (or by the start, for one no + * message carries), repeating the message's sentence, so the reason outlives any client that hides + * it. */ +export function structuredAgentSessionStartFailureRow( + startKey: string, + words: AgentJournalDispatchRejection +): JournalLifecycleMutationInput { + return { + kind: 'item', + identity: structuredAgentSessionStartFailureRowIdentity(startKey), + body: { kind: 'status', text: words.reason, tone: 'error', failure: words.rejection }, + // A start that failed opened no turn. + turnScope: AGENT_JOURNAL_THREAD_SCOPE + } +} + +/** Rejects a message its start failed and writes that start's row with it, keyed by the message, in + * one append; the journal's lane drops the row when its run's row already states it. Writes + * nothing once `which` no longer holds for it: Stop withdrew it, or another writer settled it. */ +export async function rejectWithStartFailureRow( + journal: Pick, + input: { + clientMessageId: string + words: AgentJournalDispatchRejection + fence: number + which: (submission: AgentJournalSubmission) => boolean + } +): Promise { + const { clientMessageId, words, fence, which } = input + await journal.appendLifecycleBatch({ + settlementId: `start-failure:${clientMessageId}`, + fence, + recovered: true, + mutations: [structuredAgentSessionStartFailureRow(clientMessageId, words)], + rejects: { clientMessageId, ...words, which } + }) +} + +/** Who the message's sentence names, and the command its own body sends. */ +function startFailureWordsContext( + journal: Pick, + record: AgentSessionRecord | null, + clientMessageId: string +): AgentSessionFailureWordsContext { + const command = structuredAgentSessionMessageCommand(journal, clientMessageId) + return { + ...structuredAgentSessionFailureWordsContext(record), + ...(command ? { command } : {}) + } +} + +/** Whether the message is still accepted and not yet handed over. */ +export function isStillQueued( + journal: Pick, + clientMessageId: string +): boolean { + const submission = journal + .submissions() + .find((entry) => entry.clientMessageId === clientMessageId) + return submission !== undefined && isQueuedAgentJournalSubmission(submission) +} + +/** The delivery loop's record of the queued message a failed start was for. */ +export function rejectStructuredAgentSessionStartFailure( + writer: { journal: StartFailureJournal; fence: number; record: AgentSessionRecord | null }, + cause: StructuredAgentSessionStartFailureCause, + clientMessageId: string +): Promise { + return rejectWithStartFailureRow(writer.journal, { + clientMessageId, + words: structuredAgentSessionStartFailure( + cause, + startFailureWordsContext(writer.journal, writer.record, clientMessageId) + ), + fence: writer.fence, + which: isQueuedAgentJournalSubmission + }) +} + +type FailedStartSubmission = Pick & + Partial> + +type ExitSubmission = FailedStartSubmission & + Pick + +/** Whether a message was rejected as the failed start of the child under `fence`. */ +export function rejectedAsFailedStartAt(submission: FailedStartSubmission, fence: number): boolean { + return ( + submission.dispatchState === 'rejected' && + submission.fence === fence && + isFailedStartOrHostFault({ reason: submission.reason ?? null, rejection: submission.rejection }) + ) +} + +/** The row an exit during startup writes for its start. The messages it rejected take this row, + * keyed by the one its words are for, so a row worded for a command is that command's. With none, + * a message still waiting on the start is the delivery loop's to record with its own row, and one + * already rejected as this start's failure has its writer's; anything else — a command, goal or + * rewind start, or one no message is charged with — has only this row, keyed by the start, to say + * why. The journal's lane drops it when its run's row already says it. */ +export function exitStartFailureRow( + journal: { submissions?: () => ExitSubmission[] }, + exit: { + startKey: string + /** The message the words are for, if any. */ + wordedFor: string | undefined + /** The message the start was for, if any. */ + startedFor: string | undefined + fence: number + rejected: readonly string[] + words: AgentJournalDispatchRejection + } +): JournalLifecycleMutationInput[] { + if (exit.rejected.length > 0) { + const key = + exit.wordedFor !== undefined && exit.rejected.includes(exit.wordedFor) + ? exit.wordedFor + : exit.startKey + return [structuredAgentSessionStartFailureRow(key, exit.words)] + } + const recordedElsewhere = (journal.submissions?.() ?? []).some( + (submission) => + (submission.clientMessageId === exit.startedFor && + isQueuedAgentJournalSubmission(submission)) || + rejectedAsFailedStartAt(submission, exit.fence) + ) + return recordedElsewhere ? [] : [structuredAgentSessionStartFailureRow(exit.startKey, exit.words)] +} + +export function oldestQueuedSubmission( + session: Pick +): ReturnType[number] | undefined { + let oldest: ReturnType + for (const submission of session.journal.submissions()) { + if ( + isQueuedAgentJournalSubmission(submission) && + (oldest === undefined || (submission.acceptedSequence ?? 0) < (oldest.acceptedSequence ?? 0)) + ) { + oldest = submission + } + } + return oldest +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-writer.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-writer.test.ts index 6cef8ef9a1f1..7b9377800fc4 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-writer.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-writer.test.ts @@ -1,17 +1,42 @@ -// A start that fails can be written up twice: by the exit settlement, for a message the starting -// child was handed, and by the delivery loop, for a message the exit found still queued. The chat -// gets one row for that start, in the words the message was rejected with. +// A start that fails fails only the message it was for, and the messages behind it each get their +// own start. Three writers settle a failed start, one per state the message is in: the delivery +// loop the queued message the start was for, the handover the message being handed over, and the +// exit any message handed to a child that never proved its start. Every write names a message fixed +// when its pass chose it, so a failure on the way never lands on another message. The chat gets one +// row for that start, in the words the message was rejected with. 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 { agentJournalItemKey } from '../../../shared/agent-session-journal-item-key' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + agentSessionFailureFact, + type SubmissionRejectionFact +} from '../../../shared/agent-session-failure' +import { agentSessionFailureWords } from '../../../shared/agent-session-failure-words' import { computeAgentSessionPayloadFingerprint } from '../../../shared/agent-session-mutation-envelope' -import type { AgentSessionSubscribeEvent } from '../../../shared/agent-session-wire' +import type { AgentJournalSubmission } from '../../../shared/agent-session-journal-types' +import type { + AgentSessionSubscribeEvent, + AgentSessionTurnCompletionEvent +} from '../../../shared/agent-session-wire' +import { projectStructuredAgentSessionMessages } from '../../../shared/structured-agent-session-message-projection' +import { + isStructuredAgentSessionStartFailureRow, + structuredAgentSessionStartFailureRowIdentity +} from '../../../shared/structured-agent-session-start-failure-row-key' +import { + agentJournalItemKey, + agentJournalSubmissionKey +} from '../../../shared/agent-session-journal-item-key' import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' -import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' +import { + AgentSessionPreSpawnError, + type StructuredAgentSessionAdapter +} from './structured-agent-session-adapter' +import { AgentSessionJournal } from '../agent-session-journal/journal-store' +import type { JournalLifecycleBatchInput } from '../agent-session-journal/journal-store-contracts' import { StructuredAgentSessionHost } from './structured-agent-session-host' import { HOST_TEST_NOW as NOW, @@ -23,18 +48,22 @@ import { resetHostTestOperationIds } from './structured-agent-session-host-test-data' import { openTestJournalHostDatabase } from '../agent-session-journal/journal-host-database-test-support' -import { createStructuredAgentSessionLogger } from './structured-agent-session-logger' +import { recordingStructuredAgentSessionLogger } from './structured-agent-session-logger-test-support' import { codexProviderHandle } from '../../../shared/agent-session-provider-handle-encoding' import { NO_STRUCTURED_AGENTS } from './structured-agent-session-adapter-router-test-support' const CALLER = { callerKey: 'client-1' } +const SETUP_FAILURE = agentSessionFailureFact('managedAccountUnsupported') +const SETUP_ROW = agentJournalItemKey(structuredAgentSessionStartFailureRowIdentity('generation-1')) const EXIT_REASON = 'Claude Code is not signed in. Sign in with the Claude CLI' // The exit's reason is Orca's log text; the row says only that the start stopped. const EXIT_TEXT = 'Codex stopped before it finished starting. Send your message to try again.' -// The first child (generation-1) is lost at setup; the send starts generation-2. -const START_ROW = agentJournalItemKey({ - provider: 'orca', - clientMessageId: 'start-failure:generation-2' +// A child that ran and could not finish its start. +const PROVIDER_START_FAILED = agentSessionFailureFact('providerStartFailed') +// A start refused before its child ran: the CLI is not there. +const MISSING_CLI = () => new AgentSessionPreSpawnError(new Error('spawn codex ENOENT')) +const HOST_FAULT_WORDS = agentSessionFailureWords(agentSessionFailureFact('hostFault'), { + surface: 'rejection' }) function eventually(assertion: () => void | Promise): Promise { @@ -44,13 +73,17 @@ function eventually(assertion: () => void | Promise): Promise { let root: string let store: AgentSessionRecordStore let host: StructuredAgentSessionHost +let log: ReturnType let generation = 0 +// Runs before each spawn; throwing refuses that start before it ran. +let beforeSpawn = vi.fn<() => Promise>() /** The next start's child exits as its start step returns, before the handover step. */ let exitOnStart = false -let dispatch: Mock +let dispatch = vi.fn() +let closeSession = vi.fn>() let frames: AgentSessionSubscribeEvent[] = [] -function exitBeforeProof(): Promise { +function exitBeforeProof(failure?: SubmissionRejectionFact): Promise { return host.handleAdapterEvent({ type: 'ended', sessionId: SESSION, @@ -58,67 +91,19 @@ function exitBeforeProof(): Promise { acquisitionGeneration: `generation-${generation}`, reason: EXIT_REASON, cause: 'unexpected-exit', - startupUnproven: true - }) -} - -async function send(text: string): Promise { - const body = hostTestMessage(text) - const sent = await host.send(CALLER, { - envelope: { - sessionId: SESSION, - clientOperationId: hostTestOperationId(), - expectedRuntimeFence: store.getRecord(SESSION)?.lease.runtimeFence ?? 0, - payloadFingerprint: computeAgentSessionPayloadFingerprint({ - method: 'agentSession.send', - sessionId: SESSION, - fields: { body } - }) - }, - body + startupUnproven: true, + ...(failure ? { failure } : {}) }) - expect(sent).toMatchObject({ ok: true }) - await eventually(() => expect(generation).toBe(2)) - return sent.ok ? sent.value.clientMessageId : '' -} - -async function submission(clientMessageId: string) { - return (await host.journalSnapshot(SESSION)).submissions.find( - (entry) => entry.clientMessageId === clientMessageId - ) -} - -/** Every text the subscriber was sent for the start's row, in order. */ -function publishedStartRows(): string[] { - return frames.flatMap((frame) => - frame.type === 'batch' - ? frame.batch.items.flatMap((item) => - item.itemId === START_ROW && item.body.kind === 'status' ? [item.body.text] : [] - ) - : [] - ) -} - -async function startRows(): Promise { - return (await host.journalSnapshot(SESSION)).items.flatMap((item) => - item.itemId === START_ROW && item.body.kind === 'status' ? [item.body.text] : [] - ) } -beforeEach(async () => { - root = await mkdtemp(join(tmpdir(), 'orca-start-failure-writer-')) - resetHostTestOperationIds() - generation = 0 - frames = [] - exitOnStart = false - dispatch = vi.fn(async () => ({ state: 'admitted' as const })) - store = await openTestAgentSessionRecordStore(root) +function startHost(): void { host = new StructuredAgentSessionHost({ agents: NO_STRUCTURED_AGENTS, - logger: createStructuredAgentSessionLogger(), + logger: log.logger, store, adapter: { acquire: vi.fn(async ({ fence, spawnToken }) => { + await beforeSpawn() const child = { process: { hostId: 'local', @@ -137,6 +122,7 @@ beforeEach(async () => { providerChildPhase: 'starting' as const } if (exitOnStart) { + exitOnStart = false // Asked for on the lane while the start step runs, so it lands before the handover. void host.handleAdapterEvent({ type: 'ended', @@ -151,8 +137,8 @@ beforeEach(async () => { return child }), releaseAcquisition: vi.fn(async () => true), - closeSession: vi.fn(async () => true), - dispatch: (...args) => dispatch(...args), + closeSession, + dispatch, cancelTurn: vi.fn(async () => ({ cancelled: true })), answerPrompt: vi.fn(async () => undefined), setOption: vi.fn(async () => undefined) @@ -162,46 +148,529 @@ beforeEach(async () => { mintSpawnToken: () => `spawn-${generation + 1}`, now: () => NOW }) +} + +async function send(text: string): Promise { + const body = hostTestMessage(text) + const sent = await host.send(CALLER, { + envelope: { + sessionId: SESSION, + clientOperationId: hostTestOperationId(), + expectedRuntimeFence: store.getRecord(SESSION)?.lease.runtimeFence ?? 0, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method: 'agentSession.send', + sessionId: SESSION, + fields: { body } + }) + }, + body + }) + expect(sent).toMatchObject({ ok: true }) + return sent.ok ? sent.value.clientMessageId : '' +} + +/** Sent, and handed to its starting child, which has not answered. */ +async function sendHanded(text: string): Promise { + const id = await send(text) + await eventually(() => expect(dispatched()).toContain(id)) + return id +} + +async function rejected(clientMessageId: string): Promise { + await eventually(async () => + expect(await submission(clientMessageId)).toMatchObject({ dispatchState: 'rejected' }) + ) +} + +async function submission(clientMessageId: string): Promise { + return (await host.journalSnapshot(SESSION)).submissions.find( + (entry) => entry.clientMessageId === clientMessageId + ) +} + +/** The start-failure rows written since setup, whose own failed start (generation-1) left one. */ +async function startRows(): Promise { + return (await host.journalSnapshot(SESSION)).items.flatMap((item) => + isStructuredAgentSessionStartFailureRow(item.itemId) && item.itemId !== SETUP_ROW + ? [item.itemId] + : [] + ) +} + +/** The row a failed start leaves for the message it was for. */ +function rowFor(clientMessageId: string): string { + return agentJournalItemKey(structuredAgentSessionStartFailureRowIdentity(clientMessageId)) +} + +/** Every dispatch state a subscriber was told for one message, in order. */ +function framedStates(clientMessageId: string): string[] { + return frames.flatMap((frame) => + frame.type === 'batch' + ? frame.batch.submissions + .filter((entry) => entry.clientMessageId === clientMessageId) + .map((entry) => entry.dispatchState) + : [] + ) +} + +function dispatched(): string[] { + return dispatch.mock.calls.map(([input]) => input.clientMessageId) +} + +/** Records each failed start's write the journal is asked for, in order; with `fail`, fails the + * first `times` (one by default), before they land or after. */ +function spyRejections(fail?: { when: 'before' | 'after'; times?: number }): string[] { + const order: string[] = [] + const append = AgentSessionJournal.prototype.appendLifecycleBatch + let failures = 0 + vi.spyOn(AgentSessionJournal.prototype, 'appendLifecycleBatch').mockImplementation( + async function (this: AgentSessionJournal, input: JournalLifecycleBatchInput) { + const rejected = input.rejects?.clientMessageId + if (rejected !== undefined) { + order.push(`rejected ${rejected}`) + } + if (!fail || failures >= (fail.times ?? 1) || rejected === undefined) { + return append.call(this, input) + } + failures += 1 + if (fail.when === 'after') { + await append.call(this, input) + } + throw new Error('journal write failed') + } + ) + return order +} + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-start-failure-writer-')) + resetHostTestOperationIds() + generation = 0 + frames = [] + log = recordingStructuredAgentSessionLogger() + beforeSpawn = vi.fn(async () => undefined) + exitOnStart = false + dispatch = vi.fn(async () => ({ state: 'admitted' as const })) + closeSession = vi.fn(async () => true) + store = await openTestAgentSessionRecordStore(root) + startHost() await expect(host.attach(CALLER, hostTestAttachParams(null))).resolves.toMatchObject({ ok: true }) - await exitBeforeProof() + // The first child (generation-1) is lost at setup; the first send starts generation-2. Its row + // states another failure, so it never speaks for a test's own failed start. + await exitBeforeProof(SETUP_FAILURE) await host.subscribe({ id: 'pane', sessionId: SESSION, emit: (event) => frames.push(event) }) }) afterEach(async () => { + vi.restoreAllMocks() await host.flushAllStreamedEvents() await rm(root, { recursive: true, force: true }) }) -describe('a start that fails before it proves itself', () => { - it("writes one row in the exit's words for a message the child was handed", async () => { - const sent = await send('hello') - // A starting child takes the message at once. - await eventually(() => expect(dispatch).toHaveBeenCalledOnce()) +// The provider took the message and opened its turn, so the next one may be handed over. +const accepted: StructuredAgentSessionAdapter['dispatch'] = async () => ({ + state: 'accepted', + providerIdentity: { + provider: 'codex', + threadId: THREAD, + turnId: `turn-${dispatch.mock.calls.length}`, + ordinal: dispatch.mock.calls.length + } +}) - await exitBeforeProof() +describe('a queued message whose start fails', () => { + it('fails only itself: each message behind it gets its own start', async () => { + beforeSpawn.mockImplementationOnce(async () => { + throw MISSING_CLI() + }) + dispatch.mockImplementation(accepted) + const first = await send('first') + const second = await send('second') + const third = await send('third') + + await eventually(() => expect(dispatched()).toEqual([second, third])) + expect(await submission(first)).toMatchObject({ + dispatchState: 'rejected', + rejection: { kind: 'restartFailed' } + }) + for (const id of [second, third]) { + expect(framedStates(id)).not.toContain('rejected') + } + // One row per failed start: the first's, and none for the messages that went on. + expect(await startRows()).toEqual([rowFor(first)]) + }) + + it('ends a failed child still there when the next message comes, and starts afresh', async () => { + // Its dispatch faults while the child is still starting: a failed start, with no exit seen. + dispatch.mockRejectedValueOnce(new Error('codex app-server pipe closed')) + const failed = await send('first') + await rejected(failed) + expect(await startRows()).toEqual([rowFor(failed)]) + // Not ended at once: an exit of its own may already be on its way. + expect(closeSession).not.toHaveBeenCalled() + + const next = await send('second') + + await eventually(() => expect(dispatched()).toEqual([failed, next])) + expect(closeSession).toHaveBeenCalledOnce() + expect(generation).toBe(3) + }) + // A child past its start that dies says so in its exit's own row. + it('writes no start row for a send a started child refused because it ended', async () => { + dispatch.mockImplementation(accepted) + const first = await send('first') await eventually(async () => - expect(await submission(sent)).toMatchObject({ dispatchState: 'rejected', reason: EXIT_TEXT }) + expect(await submission(first)).toMatchObject({ dispatchState: 'accepted' }) ) - await host.flushStreamedEvents(SESSION) + await host.handleAdapterEvent({ + type: 'started', + sessionId: SESSION, + fence: store.getRecord(SESSION)?.lease.runtimeFence ?? 0, + acquisitionGeneration: `generation-${generation}`, + reportedOptions: { model: 'gpt-5' }, + restoreSkippedOptions: [] + }) + dispatch.mockResolvedValueOnce({ + state: 'rejected', + ...agentSessionFailureWords(agentSessionFailureFact('providerExited'), { + surface: 'rejection' + }) + }) + + const second = await send('second') + + await rejected(second) + expect(await startRows()).toEqual([]) + }) +}) + +describe('a failure on the way to recording a failed start', () => { + it('retries the same message when the rejection did not land, and never fails the one behind it', async () => { + beforeSpawn.mockImplementationOnce(async () => { + throw MISSING_CLI() + }) + spyRejections({ when: 'before' }) + const first = await send('first') + const second = await send('second') + + // The catch's retry is Orca's fault, worded as one; the message behind it goes on. + await eventually(async () => + expect(await submission(first)).toMatchObject({ + dispatchState: 'rejected', + ...HOST_FAULT_WORDS + }) + ) + await eventually(() => expect(dispatched()).toEqual([second])) + expect(framedStates(second)).not.toContain('rejected') + }) + + it('never fails the message behind one whose rejection landed before the error', async () => { + beforeSpawn.mockImplementationOnce(async () => { + throw MISSING_CLI() + }) + spyRejections({ when: 'after' }) + const first = await send('first') + const second = await send('second') + + await eventually(() => expect(dispatched()).toEqual([second])) + expect(await submission(first)).toMatchObject({ + dispatchState: 'rejected', + rejection: { kind: 'restartFailed' } + }) + expect(framedStates(second)).not.toContain('rejected') + expect(log.entries.map((entry) => entry.fields.scope)).toContain('delivery-loop') + }) - expect(await startRows()).toEqual([EXIT_TEXT]) - expect(publishedStartRows()).toEqual([EXIT_TEXT]) + it('records the failure before ending the failed child, so a failed cleanup loses nothing', async () => { + const order = spyRejections() + dispatch.mockRejectedValueOnce(new Error('codex app-server pipe closed')) + closeSession.mockImplementation(async () => { + order.push('cleanup') + throw new Error('stop failed') + }) + const first = await send('first') + await rejected(first) + const second = await send('second') + + await rejected(second) + expect(order.slice(0, 2)).toEqual([`rejected ${first}`, 'cleanup']) + expect(log.entries.map((entry) => entry.fields.scope)).toContain( + 'delivery-loop-end-failed-start' + ) + // The stop could not prove the exit, so the next start is refused for that, in its own words. + expect(await submission(second)).toMatchObject({ + rejection: { + kind: 'restartFailed', + refusal: { details: { reason: 'previousExitUnverifiable' } } + } + }) + expect(dispatched()).toEqual([first]) + // Quit's own stop of the old child succeeds. + closeSession.mockImplementation(async () => true) }) +}) +describe('a start that fails while its child exits', () => { it('leaves the row to the loop when the exit lands while the message still waits', async () => { exitOnStart = true const queued = await send('hello') + await rejected(queued) + await host.flushStreamedEvents(SESSION) + + expect(dispatch).not.toHaveBeenCalled() + expect(await submission(queued)).toMatchObject({ reason: EXIT_TEXT }) + // Written once, with the message, not first by the exit and again by the loop. + expect(await startRows()).toEqual([rowFor(queued)]) + }) + + // Neither the loop nor its catch could record the failure, so the message stayed queued. + it('records the start on the message it was for when its child ends, rather than starting again', async () => { + exitOnStart = true + spyRejections({ when: 'before', times: 2 }) + const queued = await send('hello') + await eventually(() => + expect(log.entries.map((entry) => entry.fields.scope)).toContain('delivery-loop-fail') + ) + expect(await submission(queued)).toMatchObject({ dispatchState: 'pending' }) + const startsBefore = generation + + // The next wake reads the ended start off its child, not a fresh start for it. + const next = await send('next') + await eventually(async () => - expect(await submission(queued)).toMatchObject({ dispatchState: 'rejected' }) + expect(await submission(queued)).toMatchObject({ + dispatchState: 'rejected', + rejection: PROVIDER_START_FAILED + }) ) + await eventually(() => expect(dispatched()).toEqual([next])) + expect(generation).toBe(startsBefore + 1) + expect(await startRows()).toEqual([rowFor(queued)]) + }) + + it("writes one row in the exit's words for a message the child was handed, never in doubt", async () => { + const handed = await sendHanded('hello') + expect(await submission(handed)).toMatchObject({ + dispatchState: 'pending', + handedOverAt: expect.any(Number) + }) + + await exitBeforeProof() + + await eventually(async () => + expect(await submission(handed)).toMatchObject({ + dispatchState: 'rejected', + reason: EXIT_TEXT, + rejection: PROVIDER_START_FAILED + }) + ) + // Never in doubt on the way: the child it was handed to took nothing. + expect(framedStates(handed)).not.toContain('unknown') + // The exit rejected it, so the exit writes the start's one row, keyed by it. + expect(await startRows()).toEqual([rowFor(handed)]) + }) + + // The message behind it was never handed to that child: it gets a start of its own. + it('fails a handed message with its start, and the message waiting behind it starts afresh', async () => { + const handed = await sendHanded('handed') + // Held while the turn ahead opens, which it never does; its pass has found nothing to hand over. + const queued = await send('queued') + await eventually(() => expect(host['conversationDelivery'].loop.isRunning(SESSION)).toBe(false)) + dispatch.mockImplementation(accepted) + + await exitBeforeProof() + + await rejected(handed) + await eventually(() => expect(dispatched()).toEqual([handed, queued])) + expect(framedStates(queued)).not.toContain('rejected') + expect(await startRows()).toEqual([rowFor(handed)]) + }) + + // Its pass joined the starting child and waits to hand it over when the exit lands between steps. + it('gives a message that only joined the failed start its own start', async () => { + const handed = await sendHanded('handed') + dispatch.mockImplementation(accepted) + const joined = await send('joined') + + await exitBeforeProof() + + await rejected(handed) + await eventually(() => expect(dispatched()).toEqual([handed, joined])) + expect(framedStates(joined)).not.toContain('rejected') + expect(await startRows()).toEqual([rowFor(handed)]) + }) +}) + +// One row speaks for a run of starts that fail alike, until a turn is delivered. Each message still +// makes its own start and is rejected on its own; the run's row says why for all of them. +describe('a run of starts that fail alike', () => { + it('writes one row when every start fails alike, each message making its own attempt', async () => { + const completions: AgentSessionTurnCompletionEvent[] = [] + host.subscribeTurnCompletions({ id: 'dot', emit: (event) => completions.push(event) }) + beforeSpawn.mockImplementation(async () => { + throw MISSING_CLI() + }) + const startsBefore = beforeSpawn.mock.calls.length + const first = await send('first') + const second = await send('second') + const third = await send('third') + + await rejected(third) + for (const id of [first, second, third]) { + expect(await submission(id)).toMatchObject({ + dispatchState: 'rejected', + rejection: { kind: 'restartFailed' } + }) + } + expect(beforeSpawn.mock.calls.length - startsBefore).toBe(3) + expect(await startRows()).toEqual([rowFor(first)]) await host.flushStreamedEvents(SESSION) + expect(completions).toHaveLength(1) + // A client that hides rejected messages reads the run's one row, as before. + const snapshot = await host.journalSnapshot(SESSION) + const row = snapshot.items.find((item) => item.itemId === rowFor(first))?.body + const drawn = projectStructuredAgentSessionMessages(snapshot.items, [], snapshot.submissions, { + rejectedInPlace: false + }) + const texts = drawn.flatMap((message) => + message.blocks.flatMap((block) => ('text' in block ? [block.text] : [])) + ) + expect(row?.kind === 'status' ? texts.filter((text) => text === row.text) : []).toHaveLength(1) + for (const id of [first, second, third]) { + expect(drawn.map((message) => message.id)).not.toContain(agentJournalSubmissionKey(id)) + } + }) - expect(dispatch).not.toHaveBeenCalled() - expect(await startRows()).toEqual([EXIT_TEXT]) - // Written once, after the message was settled, not first by the exit and again by the loop. - expect(publishedStartRows()).toEqual([EXIT_TEXT]) + it('writes one row when a start fails once and the messages behind it are delivered', async () => { + const first = await sendHanded('first') + const second = await send('second') + const third = await send('third') + await eventually(() => expect(host['conversationDelivery'].loop.isRunning(SESSION)).toBe(false)) + dispatch.mockImplementation(accepted) + + await exitBeforeProof() + + await eventually(() => expect(dispatched()).toEqual([first, second, third])) + expect(await submission(first)).toMatchObject({ + dispatchState: 'rejected', + rejection: PROVIDER_START_FAILED + }) + for (const id of [second, third]) { + expect(await submission(id)).toMatchObject({ dispatchState: 'accepted' }) + } + expect(await startRows()).toEqual([rowFor(first)]) + }) + + it('writes a row for each failure when the starts fail differently', async () => { + beforeSpawn.mockImplementationOnce(async () => { + throw MISSING_CLI() + }) + const first = await send('first') + await rejected(first) + const second = await sendHanded('second') + + await exitBeforeProof() + + await rejected(second) + expect(await startRows()).toEqual([rowFor(first), rowFor(second)]) + }) + + it('writes a row again for a failure alike once a turn was delivered after the last row', async () => { + beforeSpawn.mockImplementationOnce(async () => { + throw MISSING_CLI() + }) + dispatch.mockImplementation(accepted) + const first = await send('first') + await rejected(first) + const delivered = await send('delivered') + await eventually(async () => + expect(await submission(delivered)).toMatchObject({ dispatchState: 'accepted' }) + ) + // The delivered turn's child proved its start and later ends, so the next message makes its own. + const child = { + sessionId: SESSION, + fence: store.getRecord(SESSION)?.lease.runtimeFence ?? 0, + acquisitionGeneration: `generation-${generation}` + } + await host.handleAdapterEvent({ + type: 'started', + ...child, + reportedOptions: { model: 'gpt-5' }, + restoreSkippedOptions: [] + }) + await host.handleAdapterEvent({ + type: 'ended', + ...child, + reason: 'codex app-server exited', + cause: 'unexpected-exit' + }) + beforeSpawn.mockImplementationOnce(async () => { + throw MISSING_CLI() + }) + + const last = await send('last') + + await rejected(last) + expect(await startRows()).toEqual([rowFor(first), rowFor(last)]) + }) + + // Codex's stderr is a log: its tracing stamps each line, so two exits alike differ in the time. + it('writes one row for two startup exits whose stderr differs only by its timestamp', async () => { + const stderr = (at: string) => + agentSessionFailureFact('providerExited', { + detail: { + text: `${at} ERROR codex_app_server: unknown field \`foo\` in config.toml`, + audience: 'log' + } + }) + const first = await sendHanded('first') + await exitBeforeProof(stderr('2026-10-07T01:02:03.456789Z')) + await rejected(first) + const second = await sendHanded('second') + + await exitBeforeProof(stderr('2026-10-07T01:02:09.012345Z')) + + await eventually(async () => + expect(await submission(second)).toMatchObject({ + dispatchState: 'rejected', + rejection: { kind: 'providerStartFailed', detail: { audience: 'log' } } + }) + ) + expect(await startRows()).toEqual([rowFor(first)]) + }) + + // Words written for a person name what failed: two that differ are two failures. + it('writes a row for each of two exits whose words for a person differ', async () => { + const refused = (text: string) => + agentSessionFailureFact('providerStartFailed', { detail: { text, audience: 'person' } }) + const first = await sendHanded('first') + await exitBeforeProof(refused('no rollout found for thread id t-1')) + await rejected(first) + const second = await sendHanded('second') + + await exitBeforeProof(refused('no rollout found for thread id t-2')) + + await rejected(second) + expect(await startRows()).toEqual([rowFor(first), rowFor(second)]) + }) + + it("writes no row for an exit that fails alike its run's row, though it rejected the message", async () => { + const first = await sendHanded('first') + await exitBeforeProof() + await rejected(first) + const second = await sendHanded('second') + + await exitBeforeProof() + + await eventually(async () => + expect(await submission(second)).toMatchObject({ + dispatchState: 'rejected', + rejection: PROVIDER_START_FAILED + }) + ) + expect(await startRows()).toEqual([rowFor(first)]) }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-failure-exit.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-failure-exit.test.ts index 0f11d0c6949a..98bbb5c17888 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-failure-exit.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-failure-exit.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from 'vitest' +import type { AgentJournalSubmission } from '../../../shared/agent-session-journal-types' import type { AgentSessionRecord } from '../../../shared/agent-session-record' import { agentSessionLeaseFixture, @@ -172,3 +173,136 @@ describe('a provider that ends before it finished starting', () => { ) }) }) + +describe('the one row a failed start leaves', () => { + type Submission = Pick & + Partial + + function startingSession(submissions: Submission[], rejectedHere: string[] = []) { + const base = startedSession() + return { + ...base, + child: { generation: GENERATION, fence: 7, phase: 'starting' as const }, + journal: { + ...base.journal, + submissions: () => submissions, + rejectPendingSubmissions: vi.fn(async () => rejectedHere) + } + } + } + + function rowsWritten(session: ReturnType): unknown[] { + return session.journal.appendLifecycleBatch.mock.calls.flatMap((call) => { + const [batch] = call + // The mock records whatever batch it was given; each names its mutations. + const mutations: { body?: { kind?: string } }[] = batch.mutations + return mutations.filter((mutation) => mutation.body?.kind === 'status') + }) + } + + it("is the exit's for the messages it rejected, keyed by the one its words are for", async () => { + const session = startingSession( + [{ clientMessageId: 'handed-1', dispatchState: 'pending', handedOverAt: 1, fence: 7 }], + ['handed-1'] + ) + + await settleStructuredAgentSessionChildExit(contextFor(session), ended) + + expect(rowsWritten(session)).toEqual([ + expect.objectContaining({ + identity: { provider: 'orca', clientMessageId: 'start-failure:handed-1' } + }) + ]) + }) + + // Keyed by the /compact, it is a command's row: it never speaks for a later message's failure. + it('is keyed by the /compact it rejected when its words name the command', async () => { + const base = startingSession( + [{ clientMessageId: 'compact-1', dispatchState: 'pending', handedOverAt: 1, fence: 7 }], + ['compact-1'] + ) + const session = { + ...base, + journal: { ...base.journal, itemBody: () => structuredAgentSessionCompactBody() } + } + + await settleStructuredAgentSessionChildExit(contextFor(session), ended) + + expect(rowsWritten(session)).toEqual([ + expect.objectContaining({ + identity: { provider: 'orca', clientMessageId: 'start-failure:compact-1' }, + body: expect.objectContaining({ + text: 'Claude stopped before it finished starting. Run /compact again.' + }) + }) + ]) + }) + + const QUEUED: Submission = { + clientMessageId: 'queued-1', + dispatchState: 'pending', + handoverRecorded: true + } + + function startedFor(session: ReturnType, clientMessageId: string) { + return { ...session, child: { ...session.child, startedFor: clientMessageId } } + } + + it('is not written again beside the queued message the start was for: the loop rejects it with its own', async () => { + const session = startedFor(startingSession([QUEUED]), 'queued-1') + + await settleStructuredAgentSessionChildExit(contextFor(session), ended) + + expect(rowsWritten(session)).toEqual([]) + }) + + // The /compact waits for a start of its own, which writes its own row if it fails too. + it('is not worded for a /compact still queued behind the start', async () => { + const base = startingSession([ + { clientMessageId: 'compact-1', dispatchState: 'pending', handoverRecorded: true } + ]) + const session = { + ...base, + journal: { ...base.journal, itemBody: () => structuredAgentSessionCompactBody() } + } + + await settleStructuredAgentSessionChildExit(contextFor(session), ended) + + expect(rowsWritten(session)).toEqual([ + expect.objectContaining({ + identity: { provider: 'orca', clientMessageId: `start-failure:${GENERATION}` }, + body: expect.objectContaining({ text: STARTUP_TEXT }) + }) + ]) + }) + + it("is not written again beside a message already rejected as this start, with its writer's row", async () => { + const session = startingSession([ + { + clientMessageId: 'rejected-1', + dispatchState: 'rejected', + fence: 7, + reason: STARTUP_TEXT, + rejection: { kind: 'providerStartFailed' } + } + ]) + + await settleStructuredAgentSessionChildExit(contextFor(session), ended) + + expect(rowsWritten(session)).toEqual([]) + }) + + // A message queued but neither started for nor waited on is no message the loop charges with this + // start: it starts afresh, so this start's failure has only this row. + it('is written beside a queued message no pass charges with this start', async () => { + const session = startingSession([QUEUED]) + + await settleStructuredAgentSessionChildExit(contextFor(session), ended) + + expect(rowsWritten(session)).toEqual([ + expect.objectContaining({ + identity: { provider: 'orca', clientMessageId: `start-failure:${GENERATION}` } + }) + ]) + }) +}) 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 200ce5a338ab..9e0f2c18c2b9 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 @@ -32,6 +32,8 @@ import type { StructuredAgentSessionProviderChildPhase } from './structured-agent-session-adapter' import { structuredAgentSessionStartFailure } from './structured-agent-session-failure-text' +import { rejectWithStartFailureRow } from './structured-agent-session-start-failure-settlement' +import { isFailedStartOrHostFault } from '../../../shared/structured-agent-session-dispatch-rejection' import { agentJournalSubmissionKey } from '../../../shared/agent-session-journal-item-key' import { handOverStructuredAgentSessionCommand, @@ -240,6 +242,21 @@ export async function handOverSubmission( return } try { + if ( + outcome.state === 'rejected' && + isFailedStartOrHostFault(outcome) && + ctx.providerChildPhase?.() !== 'ready' + ) { + // A start that failed under it: the message and that start's row, as any failed start leaves. + // A child past its start that dies says so in its exit's own row. + await rejectWithStartFailureRow(ctx.journal, { + clientMessageId, + words: { reason: outcome.reason, rejection: outcome.rejection }, + fence: ctx.fence, + which: (entry) => entry.dispatchState === 'pending' + }) + return + } await ctx.journal.resolveDispatch( outcome.state === 'accepted' ? { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-unfinished-work.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-unfinished-work.ts index 26148f1c8d4f..a80d4a26d044 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-unfinished-work.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-unfinished-work.ts @@ -1,6 +1,9 @@ // What a gone child generation left unfinished in the journal, and whether its exit interrupted it. -import type { AgentJournalRenderItem } from '../../../shared/agent-session-journal-types' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../shared/agent-session-journal-types' import { readAgentJournalTurn } from '../../../shared/agent-session-turn-record' import { requiresTerminalSettlement } from '../agent-session-journal/journal-terminal-settlement' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' @@ -10,7 +13,13 @@ import { } from './structured-agent-session-stale-turn-verdict' import type { UnopenedSendJournal } from './structured-agent-session-unopened-send-withdrawal' -export type DeadGenerationJournal = UnopenedSendJournal & { +type DeadGenerationSubmission = ReturnType< + NonNullable +>[number] & + Partial> + +export type DeadGenerationJournal = Omit & { + submissions?: () => DeadGenerationSubmission[] appendLifecycleBatch: AgentSessionJournal['appendLifecycleBatch'] markPendingSubmissionsUnknown: AgentSessionJournal['markPendingSubmissionsUnknown'] rejectPendingSubmissions: AgentSessionJournal['rejectPendingSubmissions'] diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-view-start-after-failed-start.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-view-start-after-failed-start.test.ts index 8f6037c6d9c7..177641038184 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-view-start-after-failed-start.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-view-start-after-failed-start.test.ts @@ -154,7 +154,7 @@ describe('a fresh chat whose Claude start fails', () => { ['after the create already died', false], ['while the create is still starting', true] ] as const)( - 'starts once for the open and once for a send, one row each, when the view binds %s', + 'starts once for the open and once for a send, under one row, when the view binds %s', async (_when, createStillStarting) => { // Released only once the views bound, so no runner is slow enough to let the create die first. let releaseCreate = (): void => {} @@ -203,16 +203,16 @@ describe('a fresh chat whose Claude start fails', () => { ).toMatchObject({ dispatchState: 'rejected', reason: startFailure }) ) await settleExits() - // The send's own start, once, and one row for it below the message. + // The send's own start, once. It failed alike with nothing delivered since: no second row. expect(claude.connections).toHaveLength(2) - expect(await timeline()).toEqual([startFailure, 'message', startFailure]) + expect(await timeline()).toEqual([startFailure, 'message']) // Switching away and back re-subscribes; it starts nothing and adds no row. unsubscribe() await view(SURFACE) await settleExits() expect(claude.connections).toHaveLength(2) - expect(await timeline()).toEqual([startFailure, 'message', startFailure]) + expect(await timeline()).toEqual([startFailure, 'message']) } ) }) diff --git a/src/main/native-chat/agent-session-wire/structured-conversation-command-start-failure-run.test.ts b/src/main/native-chat/agent-session-wire/structured-conversation-command-start-failure-run.test.ts new file mode 100644 index 000000000000..0adabeaae2f8 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-conversation-command-start-failure-run.test.ts @@ -0,0 +1,154 @@ +// A run of failed starts against conversation commands. A command's failed start always writes its +// own row, which never speaks for a message's; and a send the provider accepted ends a run, a +// /compact Codex takes without an item of its own too. Against the real host, store and journal. + +import { beforeEach, expect, it, vi, type Mock } from 'vitest' +import { agentJournalItemKey } from '../../../shared/agent-session-journal-item-key' +import { AGENT_JOURNAL_THREAD_SCOPE } from '../../../shared/agent-session-journal-types' +import { + isStructuredAgentSessionStartFailureRow, + structuredAgentSessionStartFailureRowIdentity +} from '../../../shared/structured-agent-session-start-failure-row-key' +import { projectStructuredAgentSessionMessages } from '../../../shared/structured-agent-session-message-projection' +import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' +import { + attach, + CALLER, + envelope, + hostTestState +} from './structured-agent-session-host-test-harness' +import { + HOST_TEST_NOW, + HOST_TEST_SESSION as SESSION, + hostTestMessage +} from './structured-agent-session-host-test-data' + +let state: ReturnType +let compact: Mock> + +beforeEach(() => { + state = hostTestState() + // Codex's ack: the provider took the command, with no item of its own. + compact = vi.fn(async () => ({ state: 'accepted' as const, providerIdentity: null })) + Object.assign(state.host.deps.adapter, { compact, closeSession: vi.fn(async () => true) }) +}) + +/** The provider ends the command it took, as its translator writes it. */ +function finishCompact(): void { + const { command } = compact.mock.calls.at(-1)![0] + const events = state.acquire.mock.calls.at(-1)![0].events! + events.appendLifecycleBatch!( + `turn-completed:${command.clientMessageId}`, + [ + { + kind: 'item', + identity: command.identity, + body: { + ...command.running, + state: 'completed', + outcome: 'success', + completedAt: HOST_TEST_NOW + }, + turnScope: AGENT_JOURNAL_THREAD_SCOPE + } + ], + { lifecycle: true } + ) +} + +/** The chat's child is gone; the next start it needs crashes the same way each time. */ +async function startsCrash(times?: 'once'): Promise { + await state.host.close(SESSION, 'evict') + const crash = new Error('codex app-server exited (code 1)') + if (times === 'once') { + state.acquire.mockRejectedValueOnce(crash) + } else { + state.acquire.mockRejectedValue(crash) + } +} + +async function send(text: string): Promise { + const body = hostTestMessage(text) + const params = { envelope: envelope('agentSession.send', { body }), body } + await expect(state.host.send(CALLER, params)).resolves.toMatchObject({ ok: true }) + return params.envelope.clientOperationId +} + +async function compactCommand(): Promise { + const params = { + command: 'compact' as const, + envelope: envelope('agentSession.conversationCommand', { command: 'compact' }) + } + await expect(state.host.conversationCommand(CALLER, params)).resolves.toMatchObject({ ok: true }) + return params.envelope.clientOperationId +} + +async function settled(clientMessageId: string, dispatchState: 'accepted' | 'rejected') { + await vi.waitFor(async () => + expect( + (await state.host.journalSnapshot(SESSION)).submissions.find( + (entry) => entry.clientMessageId === clientMessageId + ) + ).toMatchObject({ dispatchState }) + ) +} + +async function startRows(): Promise { + return (await state.host.journalSnapshot(SESSION)).items.flatMap((item) => + isStructuredAgentSessionStartFailureRow(item.itemId) ? [item.itemId] : [] + ) +} + +const rowFor = (id: string) => + agentJournalItemKey(structuredAgentSessionStartFailureRowIdentity(id)) + +it('starts a new run after a /compact the provider took with no item of its own', async () => { + await attach() + await startsCrash('once') + const first = await send('first') + await settled(first, 'rejected') + const compacted = await compactCommand() + await vi.waitFor(() => expect(compact).toHaveBeenCalledOnce()) + await settled(compacted, 'accepted') + finishCompact() + await state.host.flushStreamedEvents(SESSION) + + await startsCrash('once') + const second = await send('second') + await settled(second, 'rejected') + + // The same crash, but a turn was delivered between: news, with its own row. + expect(await startRows()).toEqual([rowFor(first), rowFor(second)]) +}) + +it('gives a /compact that fails like the message before it a row of its own', async () => { + await attach() + await startsCrash() + const first = await send('first') + await settled(first, 'rejected') + + const compacted = await compactCommand() + await settled(compacted, 'rejected') + + expect(await startRows()).toEqual([rowFor(first), rowFor(compacted)]) + // A client that hides rejected messages still reads the command's own row. + const snapshot = await state.host.journalSnapshot(SESSION) + const drawn = projectStructuredAgentSessionMessages(snapshot.items, [], snapshot.submissions, { + rejectedInPlace: false + }) + expect(drawn.flatMap((message) => message.blocks)).toContainEqual( + expect.objectContaining({ text: "Codex couldn't restart. Run /compact again." }) + ) +}) + +it('gives a message that fails like the /compact before it a row of its own', async () => { + await attach() + await startsCrash() + const compacted = await compactCommand() + await settled(compacted, 'rejected') + + const later = await send('later') + await settled(later, 'rejected') + + expect(await startRows()).toEqual([rowFor(compacted), rowFor(later)]) +}) diff --git a/src/main/runtime/claude-structured-send-during-startup.test.ts b/src/main/runtime/claude-structured-send-during-startup.test.ts index a50615fbed4a..e7adc3164793 100644 --- a/src/main/runtime/claude-structured-send-during-startup.test.ts +++ b/src/main/runtime/claude-structured-send-during-startup.test.ts @@ -100,7 +100,7 @@ describe('a send into a Claude chat whose CLI keeps failing at startup', () => { ) await failLatestStart(host, 2) - // Rejected with the cause, not left in doubt; one row for this attempt names it. + // Rejected with the cause, not left in doubt; the open's row already names it, so no second. await vi.waitFor(async () => expect(await submission(host, held)).toMatchObject({ dispatchState: 'rejected', @@ -109,7 +109,7 @@ describe('a send into a Claude chat whose CLI keeps failing at startup', () => { rejection: { kind: 'providerStartFailed' } }) ) - expect(await statusRows(host)).toEqual([STARTUP_TEXT, STARTUP_TEXT]) + expect(await statusRows(host)).toEqual([STARTUP_TEXT]) // The restart moved the fence twice: its acquisition, and the exit that released it. expect(fence(host)).toBe(releasedFence + 2) expect(claude.children(SESSION)).toHaveLength(2) @@ -123,7 +123,7 @@ describe('a send into a Claude chat whose CLI keeps failing at startup', () => { await vi.waitFor(() => expect(host.deps.store.getRecord(SESSION)?.lease.claimStatus).toBe('live') ) - expect(await statusRows(host)).toHaveLength(2) + expect(await statusRows(host)).toHaveLength(1) }) }) diff --git a/src/main/runtime/claude-structured-send-restart-dies-before-dispatch.test.ts b/src/main/runtime/claude-structured-send-restart-dies-before-dispatch.test.ts index 3eb815bca50e..eeaf2dac50d5 100644 --- a/src/main/runtime/claude-structured-send-restart-dies-before-dispatch.test.ts +++ b/src/main/runtime/claude-structured-send-restart-dies-before-dispatch.test.ts @@ -153,7 +153,8 @@ describe('a send whose restarted Claude child dies before it proves its start', rejection: { kind: 'providerStartFailed' } }) ) - expect(await statusRows(host)).toEqual([STARTUP_TEXT, STARTUP_TEXT]) + // The open's row already says why: a start failing alike writes no second. + expect(await statusRows(host)).toEqual([STARTUP_TEXT]) expect(fence(host)).toBe(releasedFence + 2) expect(claude.children(SESSION)).toHaveLength(2) @@ -163,13 +164,13 @@ describe('a send whose restarted Claude child dies before it proves its start', await eventually(() => expect(claude.children(SESSION)).toHaveLength(3)) await eventually(() => expect(claude.child(SESSION).calls).toContain('send')) expect(claude.child(SESSION).calls.filter((call) => call === 'send')).toHaveLength(1) - expect(await statusRows(host)).toHaveLength(2) + expect(await statusRows(host)).toHaveLength(1) }) - // A restart refused because its child died before it was handed over leaves one row, from the - // delivery, in the words any failed start uses, and rejects the message with them. + // A restart refused because its child died before it was handed over rejects the message in the + // words any failed start uses; the open's row already says them, so it adds no row. it.each(['spawn', 'start-time-read'] as const)( - 'leaves one row for a restart whose child exits at %s', + 'adds no row to the run for a restart whose child exits at %s', async (at) => { claude.behave(SESSION, { initHangs: true }) const host = await claude.install() @@ -189,7 +190,7 @@ describe('a send whose restarted Claude child dies before it proves its start', ) await waitForStructuredAgentSessionRecovery() - expect(await statusRows(host)).toEqual([STARTUP_TEXT, STARTUP_TEXT]) + expect(await statusRows(host)).toEqual([STARTUP_TEXT]) } ) @@ -246,7 +247,7 @@ describe('a chat whose Claude CLI keeps failing to start, seen by a subscriber o return rows } - it('shows one row naming the cause per failed attempt, however the start died, and none once the CLI is fixed', async () => { + it('shows one row naming the cause for a run of failed attempts, however the start died, and none once the CLI is fixed', async () => { claude.behave(SESSION, { initHangs: true }) const host = await claude.install() const events: AgentSessionSubscribeEvent[] = [] @@ -273,12 +274,12 @@ describe('a chat whose Claude CLI keeps failing to start, seen by a subscriber o reason: STARTUP_FAILURE }) ) - await eventually(() => - expect([...shownRows(events).values()]).toEqual([STARTUP_FAILURE, STARTUP_FAILURE]) - ) + // Failed alike, with nothing delivered since the open's row: that row says why for it too. + await waitForStructuredAgentSessionRecovery() + expect([...shownRows(events).values()]).toEqual([STARTUP_FAILURE]) // Retry while still broken: this restart dies before its child is handed over, so the - // delivery's start is refused. Still one row, saying the same thing, on the rejected message. + // delivery's start is refused. It failed alike, so the run's one row still says why. claude.behave(SESSION, { exitsDuringSpawn: { diagnostic: 'claude stream-json exited (code 1): claude: not signed in (rig)', @@ -293,19 +294,13 @@ describe('a chat whose Claude CLI keeps failing to start, seen by a subscriber o }) ) await waitForStructuredAgentSessionRecovery() - await eventually(() => - expect([...shownRows(events).values()]).toEqual([ - STARTUP_FAILURE, - STARTUP_FAILURE, - STARTUP_FAILURE - ]) - ) + expect([...shownRows(events).values()]).toEqual([STARTUP_FAILURE]) // The CLI is fixed: Retry delivers and adds no row. claude.behave(SESSION, {}) await expect(attempt(host, 'hello?')).resolves.toMatchObject({ ok: true }) await eventually(() => expect(claude.child(SESSION).calls).toContain('send')) - expect(shownRows(events).size).toBe(3) + expect(shownRows(events).size).toBe(1) } finally { unsubscribe() } diff --git a/src/main/runtime/claude-structured-send-signed-out-run.test.ts b/src/main/runtime/claude-structured-send-signed-out-run.test.ts new file mode 100644 index 000000000000..03f06026d80b --- /dev/null +++ b/src/main/runtime/claude-structured-send-signed-out-run.test.ts @@ -0,0 +1,78 @@ +// A Claude CLI that is not signed in fails every start the same way. The chat's own start leaves one +// row; each message sent after it makes its own start, is rejected with the same typed failure, and +// adds no row: nothing was delivered between them. Against the production runtime, adapter, record +// store and host, with only the CLI process scripted. + +import { afterEach, expect, it, vi } from 'vitest' +import { computeAgentSessionPayloadFingerprint } from '../../shared/agent-session-mutation-envelope' +import { hostTestMessage } from '../native-chat/agent-session-wire/structured-agent-session-host-test-data' +import type { StructuredAgentSessionHost } from '../native-chat/agent-session-wire/structured-agent-session-host' +import { waitForStructuredAgentSessionRecovery } from './structured-agent-session-runtime' +import { createScriptedClaudeRuntime } from './structured-claude-scripted-runtime-test-support' + +const SESSION = 'claude-signed-out-run' +const CALLER = { callerKey: 'client-1' } +const SIGNED_OUT = + 'Claude is not signed in for the selected account. Sign in, then send your message again.' + +let claude = createScriptedClaudeRuntime([SESSION]) +let operations = 0 + +afterEach(async () => { + await claude.dispose() + claude = createScriptedClaudeRuntime([SESSION]) +}) + +async function send(host: StructuredAgentSessionHost, text: string): Promise { + const body = hostTestMessage(text) + const sent = await host.send(CALLER, { + envelope: { + sessionId: SESSION, + clientOperationId: `${Date.now()}-${(++operations).toString(16).padStart(32, '0')}`, + expectedRuntimeFence: host.deps.store.getRecord(SESSION)?.lease.runtimeFence ?? 0, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method: 'agentSession.send', + sessionId: SESSION, + fields: { body } + }) + }, + body + }) + expect(sent, JSON.stringify(sent)).toMatchObject({ ok: true }) + return sent.ok ? sent.value.clientMessageId : '' +} + +async function statusRows(host: StructuredAgentSessionHost): Promise { + return (await host.journalSnapshot(SESSION)).items.flatMap((item) => + item.body.kind === 'status' ? [item.body.text] : [] + ) +} + +async function submission(host: StructuredAgentSessionHost, clientMessageId: string) { + return (await host.journalSnapshot(SESSION)).submissions.find( + (entry) => entry.clientMessageId === clientMessageId + ) +} + +it('keeps one row for a chat whose every start fails not signed in, however often it is sent to', async () => { + claude.behave(SESSION, { signedOut: true }) + const host = await claude.install() + await expect(host.attach(CALLER, claude.attachParams(SESSION, null))).resolves.toMatchObject({ + ok: true + }) + await waitForStructuredAgentSessionRecovery() + await vi.waitFor(async () => expect(await statusRows(host)).toEqual([SIGNED_OUT])) + + for (const [index, text] of ['first', 'second', 'third'].entries()) { + const id = await send(host, text) + await vi.waitFor(async () => + expect(await submission(host, id)).toMatchObject({ dispatchState: 'rejected' }) + ) + // The typed failure carries no provider words, so each attempt states the same one. + expect((await submission(host, id))?.rejection).toEqual({ kind: 'notSignedIn' }) + expect(claude.children(SESSION)).toHaveLength(index + 2) + } + await waitForStructuredAgentSessionRecovery() + + expect(await statusRows(host)).toEqual([SIGNED_OUT]) +}) diff --git a/src/main/runtime/structured-claude-scripted-runtime-test-support.ts b/src/main/runtime/structured-claude-scripted-runtime-test-support.ts index 77e66d31d639..a8d4191cb5d7 100644 --- a/src/main/runtime/structured-claude-scripted-runtime-test-support.ts +++ b/src/main/runtime/structured-claude-scripted-runtime-test-support.ts @@ -47,6 +47,8 @@ export type ScriptedClaudeBehavior = { optionWritesFail?: boolean /** The init frame names another provider session than the one launched. */ initNamesForeignSession?: boolean + /** Initialize reports an account with no credentials, as a signed-out CLI does. */ + signedOut?: boolean /** No start frame before the first turn, as with no SessionStart hook configured. */ sendsNoStartFrame?: boolean } @@ -123,7 +125,12 @@ export function createScriptedClaudeRuntime(sessionIds: readonly string[]) { closed: false, exitVerdict: { root: 'live', tree: 'unverifiable' }, initializationResult: () => { - const initialized = { models: [{ value: 'sonnet', displayName: 'Sonnet' }] } + const initialized = { + models: [{ value: 'sonnet', displayName: 'Sonnet' }], + ...(behavior.signedOut + ? { account: { apiProvider: 'firstParty', tokenSource: 'none' } } + : {}) + } const announce = (): void => { if (behavior.sendsNoStartFrame) { return diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSession.start-failure-notice.test.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSession.start-failure-notice.test.tsx index 73334d71ca3b..31adcafdaf04 100644 --- a/src/renderer/src/components/native-chat/NativeChatStructuredSession.start-failure-notice.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSession.start-failure-notice.test.tsx @@ -41,12 +41,12 @@ const START_FAILED: AgentSessionFailureFact = { kind: 'providerStartFailed' } const START_FAILED_REASON = 'Claude stopped before it finished starting. Send your message to try again.' -function startFailureRow(fact: AgentSessionFailureFact): AgentJournalRenderItem { +function startFailureRow(fact: AgentSessionFailureFact, sequence = 1): AgentJournalRenderItem { return { itemId: agentJournalItemKey(structuredAgentSessionStartFailureRowIdentity('generation-1')), revision: 1, - sequence: 1, - observedAt: 1, + sequence, + observedAt: sequence, body: { kind: 'status', tone: 'error', @@ -55,6 +55,17 @@ function startFailureRow(fact: AgentSessionFailureFact): AgentJournalRenderItem } } +/** A rejected message where the journal placed it: where it was rejected. */ +function rejectedAt(clientMessageId: string, sequence: number): AgentJournalRenderItem { + return { + itemId: agentJournalSubmissionKey(clientMessageId), + revision: 0, + sequence, + observedAt: sequence, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: clientMessageId }] } + } +} + function rejected(clientMessageId: string, reason: string, rejection: AgentSessionFailureFact) { return { outbox: { @@ -116,7 +127,12 @@ async function notice(clientMessageId: string): Promise { // The start's own row says why, so its rejected messages say only that they were not sent. Sending // one again is a new message, so none offers a Retry. it("says only 'not sent', with no Retry, on each message the failed start's row explains", async () => { - mocks.journalItems = [startFailureRow(START_FAILED)] + // The start rejected both, then wrote its row after them. + mocks.journalItems = [ + rejectedAt('first', 1), + rejectedAt('second', 2), + startFailureRow(START_FAILED, 3) + ] renderPane([ rejected('first', START_FAILED_REASON, START_FAILED), @@ -132,7 +148,11 @@ it("says only 'not sent', with no Retry, on each message the failed start's row }) it('keeps the full notice on a message rejected for a reason no start-failure row states', async () => { - mocks.journalItems = [startFailureRow(START_FAILED)] + mocks.journalItems = [ + rejectedAt('stated', 1), + rejectedAt('other', 2), + startFailureRow(START_FAILED, 3) + ] const providerRejected: AgentSessionFailureFact = { kind: 'providerRejected', detail: { text: 'Image type .bmp', audience: 'person' } diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx index 5d34f797df60..d06c2a7d97c0 100644 --- a/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx @@ -211,7 +211,7 @@ export function NativeChatStructuredSession( agent: props.agent, target: props.target, failure: provisionalLaunch.failure, - failureRows: startFailures.filter((fact) => isClaudeSignInFailureKind(fact.kind)).length + failureRows: startFailures.filter(({ fact }) => isClaudeSignInFailureKind(fact.kind)).length }) const notices = structuredSessionNotices({ launch, diff --git a/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.start-row.test.ts b/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.start-row.test.ts new file mode 100644 index 000000000000..45b8f2631222 --- /dev/null +++ b/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.start-row.test.ts @@ -0,0 +1,214 @@ +// A message rejected by a start whose row says why says only that it was not sent. A row keyed by +// the message is its own start's and speaks for it alone; a message with no row of its own, as in a +// run or under an exit's row keyed by its start, takes any loaded row with the same failure. + +import { describe, expect, it } from 'vitest' +import type { AgentSessionFailureFact } from '../../../../shared/agent-session-failure' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../../shared/agent-session-journal-types' +import { agentSessionFailureWords } from '../../../../shared/agent-session-failure-words' +import { + agentJournalItemKey, + agentJournalSubmissionKey +} from '../../../../shared/agent-session-journal-item-key' +import { structuredAgentSessionStartFailureRowIdentity } from '../../../../shared/structured-agent-session-start-failure-row-key' +import { + structuredAgentSessionDeliveryNotices, + structuredAgentSessionStartFailureFacts +} from './structured-agent-session-delivery-notices' + +const startFailed: AgentSessionFailureFact = { + kind: 'startFailed', + refusal: { code: 'agent_session_identity_required', details: { reason: 'recordMissing' } } +} +const otherRefusal: AgentSessionFailureFact = { + kind: 'startFailed', + refusal: { code: 'agent_session_conflict', details: { reason: 'claimConflicted' } } +} +const NOT_SENT = 'Your message was not sent.' +const START_FAILED_WORDS = "Claude couldn't start. Start a new chat to continue." + +const recorded = (id: string, fact: AgentSessionFailureFact): AgentJournalSubmission => ({ + clientMessageId: id, + fence: 1, + payloadFingerprint: id, + dispatchState: 'rejected', + providerItemId: null, + reason: 'Written by the host.', + rejection: fact, + submittedAt: 1, + resolvedAt: 1 +}) +const row = ( + startKey: string, + fact: AgentSessionFailureFact, + sequence: number +): AgentJournalRenderItem => ({ + itemId: rowKey(startKey), + revision: 1, + sequence, + observedAt: sequence, + body: { + kind: 'status', + tone: 'error', + ...agentSessionFailureWords(fact, { agentName: 'Claude', surface: 'row' }) + } +}) +/** A rejected message where the journal placed it: where it was rejected. */ +const messageAt = (id: string, sequence: number): AgentJournalRenderItem => ({ + itemId: agentJournalSubmissionKey(id), + revision: 0, + sequence, + observedAt: sequence, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: id }] } +}) +function rowKey(startKey: string): string { + return agentJournalItemKey(structuredAgentSessionStartFailureRowIdentity(startKey)) +} + +function noticesFor( + ids: readonly [string, AgentSessionFailureFact][], + items: readonly AgentJournalRenderItem[] +): Record { + const notices = structuredAgentSessionDeliveryNotices({ + pending: [], + submissions: ids.map(([id, fact]) => recorded(id, fact)), + agentName: 'Claude', + startFailures: structuredAgentSessionStartFailureFacts(items) + }) + return Object.fromEntries([...notices].map(([id, notice]) => [id, notice.text ?? ''])) +} + +const key = agentJournalSubmissionKey + +// Matched on the typed fact of a row found by its identity, never on either sentence. +describe('a message rejected by a start whose row already says why', () => { + it('reads only the start-failure rows', () => { + expect( + structuredAgentSessionStartFailureFacts([ + row('gen', startFailed, 2), + { ...row('gen', { kind: 'providerExited' }, 3), itemId: key('exit-row') } + ]) + ).toEqual([{ itemId: rowKey('gen'), fact: startFailed, ofCommand: false }]) + }) + + // This host: each failed start rejects its message and writes the row keyed by it, in one write. + it('hushes a message by its own row, and only by its own', () => { + expect( + noticesFor( + [ + ['first', startFailed], + ['later', startFailed] + ], + [ + messageAt('first', 1), + row('first', startFailed, 2), + messageAt('later', 5), + row('later', otherRefusal, 6) + ] + ) + ).toEqual({ + [key('first')]: NOT_SENT, + // Its own row states another failure: an equal failure in another message's row is not its. + [key('later')]: START_FAILED_WORDS + }) + }) + + // An exit: the messages it was handed rejected, then its row keyed by the start. + it('hushes the messages an exit rejected under its row keyed by the start', () => { + expect( + noticesFor( + [ + ['first', startFailed], + ['second', startFailed] + ], + [messageAt('first', 1), messageAt('second', 2), row('generation-1', startFailed, 3)] + ) + ).toEqual({ [key('first')]: NOT_SENT, [key('second')]: NOT_SENT }) + }) + + // This host: a run of starts that fail alike writes only its first message's row. + it('hushes the messages of a run under its one row, and not one that failed otherwise', () => { + expect( + noticesFor( + [ + ['first', startFailed], + ['second', startFailed], + ['third', startFailed], + ['other', otherRefusal] + ], + [ + messageAt('first', 1), + row('first', startFailed, 2), + messageAt('second', 3), + messageAt('third', 4), + messageAt('other', 5) + ] + ) + ).toEqual({ + [key('first')]: NOT_SENT, + [key('second')]: NOT_SENT, + [key('third')]: NOT_SENT, + [key('other')]: + "Claude couldn't start. This chat is still open in a terminal agent. Quit that agent to continue the chat here." + }) + }) + + // A log diagnostic's text is not the failure: a CLI's stderr can carry a timestamp per attempt. + it('hushes a message under a row whose log diagnostic differs, not one whose words for a person do', () => { + const exited = (text: string, audience: 'log' | 'person'): AgentSessionFailureFact => ({ + kind: 'providerStartFailed', + detail: { text, audience } + }) + expect( + noticesFor( + [ + ['first', exited('01:02:03 ERROR config', 'log')], + ['logged', exited('01:02:09 ERROR config', 'log')] + ], + [ + messageAt('first', 1), + row('first', exited('01:02:03 ERROR config', 'log'), 2), + messageAt('logged', 3) + ] + ) + ).toEqual({ [key('first')]: NOT_SENT, [key('logged')]: NOT_SENT }) + expect( + noticesFor( + [ + ['first', exited('no rollout for t-1', 'person')], + ['worded', exited('no rollout for t-2', 'person')] + ], + [ + messageAt('first', 1), + row('first', exited('no rollout for t-1', 'person'), 2), + messageAt('worded', 3) + ] + )[key('worded')] + ).not.toBe(NOT_SENT) + }) + + // A command's row says to run the command again: never the next step for a message. + it("hushes a /compact under its own row, and never a message under the command's row", () => { + const compactAt = (id: string, sequence: number): AgentJournalRenderItem => ({ + ...messageAt(id, sequence), + body: { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: '/compact' }], + command: { name: 'compact' } + } + }) + expect( + noticesFor( + [ + ['compacted', startFailed], + ['later', startFailed] + ], + [compactAt('compacted', 1), row('compacted', startFailed, 2), messageAt('later', 3)] + ) + ).toEqual({ [key('compacted')]: NOT_SENT, [key('later')]: START_FAILED_WORDS }) + }) +}) diff --git a/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.test.ts b/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.test.ts index 3aacca132633..ddbc435133ee 100644 --- a/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.test.ts +++ b/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.test.ts @@ -13,7 +13,8 @@ import { import { structuredAgentSessionStartFailureRowIdentity } from '../../../../shared/structured-agent-session-start-failure-row-key' import { structuredAgentSessionDeliveryNotices, - structuredAgentSessionStartFailureFacts + structuredAgentSessionStartFailureFacts, + type StatedStartFailure } from './structured-agent-session-delivery-notices' import type { StructuredAgentSessionPendingSend } from './structured-agent-session-pending-sends' @@ -59,7 +60,7 @@ function row( function notices(args: { pending?: StructuredAgentSessionPendingSend[] submissions?: AgentJournalSubmission[] - startFailures?: AgentSessionFailureFact[] + startFailures?: StatedStartFailure[] }) { return structuredAgentSessionDeliveryNotices({ pending: args.pending ?? [], @@ -133,7 +134,7 @@ describe('the line under each of the chat own messages', () => { statusRow(startRowKey, startFailed), statusRow(agentJournalSubmissionKey('exit-row'), { kind: 'providerExited' }) ]) - ).toEqual([startFailed]) + ).toEqual([{ itemId: startRowKey, fact: startFailed, ofCommand: false }]) }) it('says only that it was not sent, and words a rejection no row states in full', () => { diff --git a/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.ts b/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.ts index 64cf71eadfb3..ca3d930476f8 100644 --- a/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.ts +++ b/src/renderer/src/components/native-chat/structured-agent-session-delivery-notices.ts @@ -1,18 +1,27 @@ // What each of the structured chat's own messages says under it about its delivery. Derived from -// the sender's in-memory sends and the host's rows on every render, never stored. +// the sender's in-memory sends and the host's rows on every render, never stored. A rejection whose +// own start's row states the same failure says only that it was not sent: the row already says why. import { readAgentSessionFailureFact, readWholeAgentSessionFailureFact, + sameAgentSessionFailureFact, type AgentSessionFailureFact } from '../../../../shared/agent-session-failure' -import { agentJournalSubmissionKey } from '../../../../shared/agent-session-journal-item-key' +import { + agentJournalItemKey, + agentJournalSubmissionKey +} from '../../../../shared/agent-session-journal-item-key' import type { AgentJournalRenderItem, AgentJournalSubmission } from '../../../../shared/agent-session-journal-types' import { agentSessionWriteNotDoneParts } from '../../../../shared/agent-session-refusal-notice' -import { isStructuredAgentSessionStartFailureRow } from '../../../../shared/structured-agent-session-start-failure-row-key' +import { + isStructuredAgentSessionCommandStartFailureRow, + isStructuredAgentSessionStartFailureRow, + structuredAgentSessionStartFailureRowIdentity +} from '../../../../shared/structured-agent-session-start-failure-row-key' import { structuredAgentSessionRejectionParts } from '../../../../shared/structured-agent-session-rejection-words' import { structuredAgentSessionRejectedShownInPlace } from '../../../../shared/structured-agent-session-message-projection' import { agentSessionWriteNoticeText } from './agent-session-write-notice-text' @@ -23,58 +32,74 @@ import type { StructuredAgentSessionPendingSend } from './structured-agent-sessi const STRUCTURED_AGENT_SESSION_DELIVERY_SENDING: NativeChatDeliveryNotice = { sending: true } const NO_COMMANDS: ReadonlySet = new Set() -/** The facts the chat's loaded start-failure rows state. */ +/** A loaded start-failure row: its item id, the failure it states, and whether a command's start + * wrote it (its words name the command's next step, so it speaks for no message). */ +export type StatedStartFailure = { + itemId: string + fact: AgentSessionFailureFact + ofCommand: boolean +} + +/** What the chat's loaded start-failure rows state. */ export function structuredAgentSessionStartFailureFacts( items: readonly AgentJournalRenderItem[] -): AgentSessionFailureFact[] { - const facts: AgentSessionFailureFact[] = [] +): StatedStartFailure[] { + const stated: StatedStartFailure[] = [] + let bodies: Map | undefined + const bodyOf = (id: string) => + (bodies ??= new Map(items.map((entry) => [entry.itemId, entry.body]))).get(id) for (const item of items) { if (item.body.kind === 'status' && isStructuredAgentSessionStartFailureRow(item.itemId)) { const fact = readAgentSessionFailureFact(item.body.failure) if (fact) { - facts.push(fact) + stated.push({ + itemId: item.itemId, + fact, + ofCommand: isStructuredAgentSessionCommandStartFailureRow(item.itemId, bodyOf) + }) } } } - return facts -} - -/** Whether two facts are one failure: a start's row and the messages it rejected share one. */ -export function sameAgentSessionFailureFact( - a: AgentSessionFailureFact, - b: AgentSessionFailureFact -): boolean { - return ( - a.kind === b.kind && - a.detail?.text === b.detail?.text && - a.detail?.audience === b.detail?.audience && - a.refusal?.code === b.refusal?.code && - a.refusal?.details?.reason === b.refusal?.details?.reason && - a.attachment?.reason === b.attachment?.reason && - a.attachment?.limit === b.attachment?.limit && - a.retry?.error === b.retry?.error && - a.retry?.status === b.retry?.status - ) + return stated } -/** Whether a loaded start-failure row already states this failure. Matching is identity, not +/** Whether one of these start-failure rows already states this failure. Matching is identity, not * wording: what this build can read is enough. */ export function agentSessionFailureStatedByStartRow( failure: unknown, - startFailures: readonly AgentSessionFailureFact[] + startFailures: readonly StatedStartFailure[] ): boolean { const fact = readAgentSessionFailureFact(failure) return ( - fact !== undefined && startFailures.some((stated) => sameAgentSessionFailureFact(stated, fact)) + fact !== undefined && + startFailures.some((stated) => sameAgentSessionFailureFact(stated.fact, fact)) + ) +} + +/** Whether the row of the start that rejected this message already says why. A row keyed by the + * message is its own start's and speaks for it alone, so its failure decides. With none — a run + * left one row for it, or an exit wrote one keyed by its start — any loaded row with the same + * failure does, but a command's. */ +function rejectionStatedByItsStartRow( + recorded: AgentJournalSubmission, + startFailures: readonly StatedStartFailure[] +): boolean { + const ownRow = agentJournalItemKey( + structuredAgentSessionStartFailureRowIdentity(recorded.clientMessageId) + ) + const own = startFailures.filter(({ itemId }) => itemId === ownRow) + return agentSessionFailureStatedByStartRow( + recorded.rejection, + own.length > 0 ? own : startFailures.filter(({ ofCommand }) => !ofCommand) ) } function hostRejectionNoticeText( submission: AgentJournalSubmission, agentName: string, - startFailures: readonly AgentSessionFailureFact[] + startFailures: readonly StatedStartFailure[] ): string { - if (agentSessionFailureStatedByStartRow(submission.rejection, startFailures)) { + if (rejectionStatedByItsStartRow(submission, startFailures)) { return agentSessionWriteNoticeText(agentSessionWriteNotDoneParts('send')) } return agentSessionWriteNoticeText( @@ -98,7 +123,7 @@ export function structuredAgentSessionDeliveryNotices(args: { submissions: readonly AgentJournalSubmission[] agentName: string /** What the loaded start-failure rows state, from `structuredAgentSessionStartFailureFacts`. */ - startFailures: readonly AgentSessionFailureFact[] + startFailures: readonly StatedStartFailure[] /** The loaded commands, from `structuredAgentSessionCommandItemIds`: they report their own. */ commandItemIds?: ReadonlySet }): ReadonlyMap { diff --git a/src/renderer/src/components/native-chat/structured-agent-session-message-projection.rejected-in-place.test.ts b/src/renderer/src/components/native-chat/structured-agent-session-message-projection.rejected-in-place.test.ts index 6381611738cd..ed7f92377f66 100644 --- a/src/renderer/src/components/native-chat/structured-agent-session-message-projection.rejected-in-place.test.ts +++ b/src/renderer/src/components/native-chat/structured-agent-session-message-projection.rejected-in-place.test.ts @@ -17,7 +17,10 @@ import { } from '../../../../shared/structured-agent-session-message-projection' import type { StructuredAgentSessionOptimisticMessage } from '../../../../shared/structured-agent-session-message-projection' import { structuredAgentSessionSendBody } from '../../../../shared/structured-agent-session-send-mutation' -import { structuredAgentSessionDeliveryNotices } from './structured-agent-session-delivery-notices' +import { + structuredAgentSessionDeliveryNotices, + type StatedStartFailure +} from './structured-agent-session-delivery-notices' import { projectStructuredAgentSessionMessages } from './structured-agent-session-message-projection' function optimisticMessage(args: { @@ -36,7 +39,7 @@ function optimisticMessage(args: { function deliveryNotices( submissions: readonly AgentJournalSubmission[], - startFailures: { kind: 'notSignedIn' }[] = [], + startFailures: StatedStartFailure[] = [], commandItemIds?: ReadonlySet ) { return structuredAgentSessionDeliveryNotices({ @@ -193,7 +196,11 @@ describe('a message the host accepted and then rejected, on the desktop', () => reason: 'Claude is not signed in.', rejection: { kind: 'notSignedIn' } }) - const notices = deliveryNotices([failedStart], [{ kind: 'notSignedIn' }]) + const notices = deliveryNotices( + [failedStart], + // The start's row, keyed by the message it failed. + [{ itemId: 'orca:start-failure%3Afirst', fact: { kind: 'notSignedIn' }, ofCommand: false }] + ) expect(notices.get(agentJournalSubmissionKey('first'))).toEqual({ text: 'Your message was not sent.' diff --git a/src/renderer/src/components/native-chat/structured-conversation-command-send.test.ts b/src/renderer/src/components/native-chat/structured-conversation-command-send.test.ts index 498e9d4fc011..8f7b63876970 100644 --- a/src/renderer/src/components/native-chat/structured-conversation-command-send.test.ts +++ b/src/renderer/src/components/native-chat/structured-conversation-command-send.test.ts @@ -155,7 +155,9 @@ describe('the line under the composer after a conversation command failed', () = agentName: 'Claude', pending: { current: false }, blocked: false, - startFailures: () => [START_FAILED], + startFailures: () => [ + { itemId: 'orca:start-failure', fact: START_FAILED, ofCommand: true } + ], send: async () => ({ kind: 'done', value: result }) }) ).toEqual({ accepted: false, error }) diff --git a/src/renderer/src/components/native-chat/structured-conversation-command-send.ts b/src/renderer/src/components/native-chat/structured-conversation-command-send.ts index 0a351db917f1..f3267d127c52 100644 --- a/src/renderer/src/components/native-chat/structured-conversation-command-send.ts +++ b/src/renderer/src/components/native-chat/structured-conversation-command-send.ts @@ -3,14 +3,14 @@ import { type AgentSessionConversationCommand, type AgentSessionConversationCommandResult } from '../../../../shared/agent-session-conversation-command' -import { - readWholeAgentSessionFailureFact, - type AgentSessionFailureFact -} from '../../../../shared/agent-session-failure' +import { readWholeAgentSessionFailureFact } from '../../../../shared/agent-session-failure' import { agentSessionFailureSentence } from '../../../../shared/agent-session-failure-words' import { translate } from '@/i18n/i18n' import { sayAgentSessionFailureTranslated } from './agent-session-failure-words-text' -import { agentSessionFailureStatedByStartRow } from './structured-agent-session-delivery-notices' +import { + agentSessionFailureStatedByStartRow, + type StatedStartFailure +} from './structured-agent-session-delivery-notices' import type { StructuredAgentSessionWriteOutcome } from './use-structured-agent-session-mutate' export async function sendStructuredConversationCommand(input: { @@ -20,7 +20,7 @@ export async function sendStructuredConversationCommand(input: { pending: { current: boolean } blocked: boolean /** What the chat's loaded start-failure rows state, read when the reply lands. */ - startFailures: () => readonly AgentSessionFailureFact[] + startFailures: () => readonly StatedStartFailure[] send: ( command: AgentSessionConversationCommand ) => Promise> diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-start-failure-facts.test.ts b/src/renderer/src/components/native-chat/use-structured-agent-session-start-failure-facts.test.ts index 97ef023dffb7..6533f9611565 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-start-failure-facts.test.ts +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-start-failure-facts.test.ts @@ -31,7 +31,9 @@ it('holds the same facts while the start rows state nothing new', () => { { initialProps: { items: [row] } } ) const first = result.current - expect(first).toEqual([{ kind: 'providerStartFailed' }]) + expect(first).toEqual([ + { itemId: row.itemId, fact: { kind: 'providerStartFailed' }, ofCommand: false } + ]) rerender({ items: [row, startFailureRow('other-start', { kind: 'providerStartFailed' })] }) expect(result.current).not.toBe(first) diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-start-failure-facts.ts b/src/renderer/src/components/native-chat/use-structured-agent-session-start-failure-facts.ts index 83f20103de11..6b0ed03c4f79 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-start-failure-facts.ts +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-start-failure-facts.ts @@ -1,28 +1,36 @@ import { useEffect, useMemo, useRef } from 'react' -import type { AgentSessionFailureFact } from '../../../../shared/agent-session-failure' import type { AgentJournalRenderItem } from '../../../../shared/agent-session-journal-types' +import { sameAgentSessionFailureFact } from '../../../../shared/agent-session-failure' import { - sameAgentSessionFailureFact, - structuredAgentSessionStartFailureFacts + structuredAgentSessionStartFailureFacts, + type StatedStartFailure } from './structured-agent-session-delivery-notices' -const NO_FACTS: readonly AgentSessionFailureFact[] = [] +const NO_FACTS: readonly StatedStartFailure[] = [] /** What the loaded start-failure rows state, read only while `enabled`. Held while unchanged, so a * streaming turn does not rebuild every row's delivery notice. */ export function useStructuredAgentSessionStartFailureFacts( items: readonly AgentJournalRenderItem[], enabled: boolean -): readonly AgentSessionFailureFact[] { +): readonly StatedStartFailure[] { const facts = useMemo( () => (enabled ? structuredAgentSessionStartFailureFacts(items) : NO_FACTS), [enabled, items] ) - const previousRef = useRef(NO_FACTS) + const previousRef = useRef(NO_FACTS) const previous = previousRef.current const stable = previous.length === facts.length && - previous.every((fact, index) => sameAgentSessionFailureFact(fact, facts[index])) + previous.every((stated, index) => { + const next = facts[index] + return ( + next !== undefined && + stated.itemId === next.itemId && + stated.ofCommand === next.ofCommand && + sameAgentSessionFailureFact(stated.fact, next.fact) + ) + }) ? previous : facts // Written after commit, so render stays pure. diff --git a/src/shared/agent-session-failure.test.ts b/src/shared/agent-session-failure.test.ts index 14cd75f7d6f0..c62dcfd2e4e0 100644 --- a/src/shared/agent-session-failure.test.ts +++ b/src/shared/agent-session-failure.test.ts @@ -7,6 +7,7 @@ import { readAgentSessionFailureFact, readProviderRetry, readWholeAgentSessionFailureFact, + sameAgentSessionFailureFact, withProviderDiagnostic } from './agent-session-failure' import { AgentSessionFailureFactSchema } from './agent-session-failure-fact-schema' @@ -194,3 +195,31 @@ describe('reading all of a failure fact', () => { } }) }) + +describe('one failure', () => { + const exited = (text: string, audience: 'log' | 'person') => + agentSessionFailureFact('providerStartFailed', { detail: { text, audience } }) + + it('ignores the text of a log diagnostic, which can carry a timestamp per attempt', () => { + expect( + sameAgentSessionFailureFact( + exited('01:02:03 ERROR x', 'log'), + exited('01:02:09 ERROR x', 'log') + ) + ).toBe(true) + }) + + it('compares words for a person, and the audience either way', () => { + expect(sameAgentSessionFailureFact(exited('t-1', 'person'), exited('t-2', 'person'))).toBe( + false + ) + expect(sameAgentSessionFailureFact(exited('t-1', 'person'), exited('t-1', 'person'))).toBe(true) + expect(sameAgentSessionFailureFact(exited('t-1', 'person'), exited('t-1', 'log'))).toBe(false) + expect( + sameAgentSessionFailureFact( + exited('t-1', 'log'), + agentSessionFailureFact('providerStartFailed') + ) + ).toBe(false) + }) +}) diff --git a/src/shared/agent-session-failure.ts b/src/shared/agent-session-failure.ts index cca1b1c9b27a..f8b420f779d4 100644 --- a/src/shared/agent-session-failure.ts +++ b/src/shared/agent-session-failure.ts @@ -286,6 +286,26 @@ export function readWholeAgentSessionFailureFact( return fact && structuralValuesEqualIgnoringUndefined(fact, value) ? fact : undefined } +/** Whether two facts are one failure, compared by their fields, never Orca's words: a start's row + * and the messages it rejected share one, and the host writes one row for a run of them. A log + * diagnostic's text is not compared: a CLI's stderr can carry a timestamp per attempt. */ +export function sameAgentSessionFailureFact( + a: AgentSessionFailureFact, + b: AgentSessionFailureFact +): boolean { + return ( + a.kind === b.kind && + a.detail?.audience === b.detail?.audience && + (a.detail?.audience !== 'person' || a.detail.text === b.detail?.text) && + a.refusal?.code === b.refusal?.code && + a.refusal?.details?.reason === b.refusal?.details?.reason && + a.attachment?.reason === b.attachment?.reason && + a.attachment?.limit === b.attachment?.limit && + a.retry?.error === b.retry?.error && + a.retry?.status === b.retry?.status + ) +} + /** The provider-authored diagnostic an error carries, set only where it was composed. Follows the * `cause` chain, since wrappers such as the acquisition errors keep the original as their cause. */ export function providerDiagnosticOf(error: unknown): ProviderDiagnostic | undefined { diff --git a/src/shared/structured-agent-session-dispatch-rejection.ts b/src/shared/structured-agent-session-dispatch-rejection.ts index 4aa598339d13..8225750c5e81 100644 --- a/src/shared/structured-agent-session-dispatch-rejection.ts +++ b/src/shared/structured-agent-session-dispatch-rejection.ts @@ -178,6 +178,21 @@ export function classifyDispatchRejection( return { category: KIND_CATEGORY[kind], verdict: KIND_VERDICT[kind], kind } } +/** Rejected because the agent it waited on never took it — a failed start, Orca stopping a start + * that hung, the agent ending first — or Orca's own fault on the way: the message never reached an + * agent, and its record says why. */ +export function isFailedStartOrHostFault( + submission: Pick & { rejection?: unknown } +): boolean { + const { category, kind } = classifyDispatchRejection(submission) + return ( + category === 'startFailed' || + kind === 'hostStopped' || + kind === 'providerExited' || + kind === 'hostFault' + ) +} + /** A Stop withdrew it before it ran: it will not land, and only a person can send it again. */ export function dispatchWasWithdrawn( submission: Pick | undefined diff --git a/src/shared/structured-agent-session-start-failure-row-key.ts b/src/shared/structured-agent-session-start-failure-row-key.ts index 6dc4a4817b14..ec42c06a4858 100644 --- a/src/shared/structured-agent-session-start-failure-row-key.ts +++ b/src/shared/structured-agent-session-start-failure-row-key.ts @@ -1,7 +1,10 @@ // A failed start's one row is keyed by the start, so a reader finds it by identity, not by its words. -import { parseAgentJournalItemKey } from './agent-session-journal-item-key' -import type { AgentJournalItemIdentity } from './agent-session-journal-types' +import { + agentJournalSubmissionKey, + parseAgentJournalItemKey +} from './agent-session-journal-item-key' +import type { AgentJournalItemBody, AgentJournalItemIdentity } from './agent-session-journal-types' const START_FAILURE_ROW = 'start-failure:' @@ -15,3 +18,18 @@ export function isStructuredAgentSessionStartFailureRow(itemId: string): boolean const identity = parseAgentJournalItemKey(itemId) return identity?.provider === 'orca' && identity.clientMessageId.startsWith(START_FAILURE_ROW) } + +/** Whether a start-failure row is a conversation command's: keyed by a sent message that names one. + * Its words name the command's next step, so it never speaks for a message's failure. */ +export function isStructuredAgentSessionCommandStartFailureRow( + itemId: string, + bodyOf: (itemId: string) => AgentJournalItemBody | undefined +): boolean { + const identity = parseAgentJournalItemKey(itemId) + if (identity?.provider !== 'orca' || !identity.clientMessageId.startsWith(START_FAILURE_ROW)) { + return false + } + const startKey = identity.clientMessageId.slice(START_FAILURE_ROW.length) + const body = bodyOf(agentJournalSubmissionKey(startKey)) + return body?.kind === 'message' && body.command !== undefined +}