mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
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.
This commit is contained in:
@@ -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<Parameters<typeof openAgentSessionJournal>[0]> = {}) {
|
||||
return journals.open({
|
||||
identity: IDENTITY,
|
||||
journalDir: root,
|
||||
now: () => (clock += 1),
|
||||
mintEpoch: () => `epoch-${clock}`,
|
||||
...overrides
|
||||
})
|
||||
}
|
||||
|
||||
async function settle(reason: string): Promise<string | null> {
|
||||
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)
|
||||
})
|
||||
})
|
||||
@@ -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<T> = (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 }
|
||||
|
||||
Reference in New Issue
Block a user