From b4d435806f5c6543a435923ea8dc8ea5ec4d76bc Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 14 Sep 2026 12:14:07 -0700 Subject: [PATCH] fix(native-chat): bound a dispatch reason before it reaches the journal row (#20654) `AgentJournalSubmission.reason` was the only unbounded field written by Orca's own code. `dispatchSafely` sets it from the adapter's raw `error.message` and `journalDispatchRowBuilder` stored it verbatim, so a provider error carrying a multi-megabyte body -- a stringified HTTP error payload, say -- reached the row at whatever length the provider sent, and stayed on disk at that size for the life of the journal. It now goes through `boundInlineText` with the journal's existing inline limit, the same idiom already applied to arbitrary text on the Claude and Codex translation paths. The bound must stay head-preserving. `dispatchRejectionWasTransportWriteFailure` prefix-matches the value, and `dispatchRejectionReasonIsInternal` builds on it, so a bound that kept the tail instead would stop classifying a clipped transport failure and render raw provider text to the user as an ordinary rejection notice. A test pins that, and clipping stays marked rather than silent so a truncated reason is never presented as the provider's complete explanation. Rows written before this keep their full text, so readers can still meet an unbounded reason. --- .../journal-dispatch-reason-bound.test.ts | 87 +++++++++++++++++++ .../journal-row-builders.ts | 15 +++- 2 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 src/main/native-chat/agent-session-journal/journal-dispatch-reason-bound.test.ts diff --git a/src/main/native-chat/agent-session-journal/journal-dispatch-reason-bound.test.ts b/src/main/native-chat/agent-session-journal/journal-dispatch-reason-bound.test.ts new file mode 100644 index 00000000000..70052865242 --- /dev/null +++ b/src/main/native-chat/agent-session-journal/journal-dispatch-reason-bound.test.ts @@ -0,0 +1,87 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import type { AgentSessionJournalIdentity } from '../../../shared/agent-session-journal-types' +import { + DISPATCH_REJECTED_WRITE_FAILED, + dispatchRejectionReasonIsInternal, + dispatchRejectionWasTransportWriteFailure +} from '../../../shared/structured-agent-session-dispatch-rejection' +import { DEFAULT_JOURNAL_PAYLOAD_LIMITS } from './journal-payload-bounds' +import type { openAgentSessionJournal } from './journal-store-factory' +import { createTrackedJournalOpener } from './journal-store-test-open' + +const IDENTITY: AgentSessionJournalIdentity = { + sessionId: 'session-1', + workspaceId: 'ws-1', + hostId: 'host-1', + agent: 'codex', + providerHandle: { kind: 'codex', threadId: 'thread-1' } +} + +const HUGE = 'x'.repeat(4 * DEFAULT_JOURNAL_PAYLOAD_LIMITS.inlineHeadBytes) + +let root: string +let clock = 1_000 + +const journals = createTrackedJournalOpener() + +async function open(overrides: Partial[0]> = {}) { + return journals.open({ + identity: IDENTITY, + journalDir: root, + now: () => (clock += 1), + mintEpoch: () => `epoch-${clock}`, + ...overrides + }) +} + +async function settle(reason: string): Promise { + const journal = await open() + await journal.appendSubmission({ + clientMessageId: 'msg-1', + payloadFingerprint: 'e'.repeat(64), + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'hi' }] }, + fence: 1 + }) + await journal.resolveDispatch({ clientMessageId: 'msg-1', state: 'rejected', reason, fence: 1 }) + return journal.snapshot().submissions[0]?.reason ?? null +} + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-dispatch-reason-')) + clock = 1_000 +}) + +afterEach(async () => { + await journals.closeAll() + await rm(root, { recursive: true, force: true }) +}) + +describe('dispatch reason bounding', () => { + it('bounds an oversized provider error before it reaches the row', async () => { + const stored = await settle(HUGE) + expect(stored).not.toBeNull() + expect(stored?.length).toBeLessThan(HUGE.length) + }) + + it('marks the clipped reason rather than truncating it silently', async () => { + const stored = await settle(HUGE) + expect(stored).toContain('[Orca: output truncated') + }) + + it('leaves a reason that already fits exactly as written', async () => { + const stored = await settle(`${DISPATCH_REJECTED_WRITE_FAILED}: broken pipe`) + expect(stored).toBe(`${DISPATCH_REJECTED_WRITE_FAILED}: broken pipe`) + }) + + // Head-first, not hash-replacing: the classifier prefix-matches, so a bound that kept + // the tail would render raw provider text to the user as an ordinary rejection notice. + it('keeps a clipped transport failure classifiable', async () => { + const stored = await settle(`${DISPATCH_REJECTED_WRITE_FAILED}: ${HUGE}`) + expect(stored).not.toBe(`${DISPATCH_REJECTED_WRITE_FAILED}: ${HUGE}`) + expect(dispatchRejectionWasTransportWriteFailure(stored)).toBe(true) + expect(dispatchRejectionReasonIsInternal(stored)).toBe(true) + }) +}) diff --git a/src/main/native-chat/agent-session-journal/journal-row-builders.ts b/src/main/native-chat/agent-session-journal/journal-row-builders.ts index be2c2552775..e5e376940fe 100644 --- a/src/main/native-chat/agent-session-journal/journal-row-builders.ts +++ b/src/main/native-chat/agent-session-journal/journal-row-builders.ts @@ -20,6 +20,7 @@ import { MAX_JOURNAL_LIFECYCLE_BATCH_BYTES, MAX_JOURNAL_LIFECYCLE_BATCH_MUTATIONS } from './journal-row-schema' +import { boundInlineText, DEFAULT_JOURNAL_PAYLOAD_LIMITS } from './journal-payload-bounds' import type { ResolveDispatchInput } from './journal-store-contracts' type RowBuilder = (seq: number, ts: number) => T @@ -76,8 +77,7 @@ export function journalDispatchRowBuilder( clientMessageId: input.clientMessageId, dispatchState: input.state, providerItemId, - reason: - input.state === 'accepted' || input.state === 'pending' ? null : (input.reason ?? null), + reason: boundedDispatchReason(input), seq, fence: input.fence, ts, @@ -85,6 +85,17 @@ export function journalDispatchRowBuilder( }) } +/** `reason` is the only unbounded field written by Orca's own code: a provider error is + * arbitrary text, and a multi-megabyte one reached the row verbatim. Bounded head-first, + * because `dispatchRejectionWasTransportWriteFailure` prefix-matches the value. Rows + * written before this keep their full text, so readers still meet unbounded ones. */ +function boundedDispatchReason(input: ResolveDispatchInput): string | null { + if (input.state === 'accepted' || input.state === 'pending' || !input.reason) { + return null + } + return boundInlineText(input.reason, DEFAULT_JOURNAL_PAYLOAD_LIMITS).text +} + export type JournalLifecycleMutationInput = | { kind: 'item'; identity: AgentJournalItemIdentity; body: AgentJournalItemBody } | { kind: 'tombstone'; identity: AgentJournalItemIdentity }