mirror of
https://github.com/stablyai/orca.git
synced 2026-10-01 08:01:56 +00:00
fix(native-chat): stop unjournaled provider frames from killing the session
A frame the classifier declines (status-chrome, suppressed-benign, stream-into-item) is deliberately not journaled. #17720 turned that null translation into `{accepted: false, reason: 'untranslated'}`, which is not `backpressure`, so the notification retry queue treated it as unreplayable and escalated through fail() -> forceCloseUnexpected -> connection.close(). The app-server latched `closing` and the create path's next model/list rejected with "codex app-server connection is closed (model/list)". The provider emits `remoteControl/status/changed` right after initialize, so every structured Codex session died on its first chrome frame. Admit the null translation instead, before any bookkeeping or publish. Also restore the error-frame exemption from the generic row cap, dropped by the same PR: the cap now runs after the classification check, so a noisy turn can no longer reduce provider errors to a suppression count. The test that pinned the capped behavior is inverted to assert the exemption.
This commit is contained in:
@@ -64,19 +64,24 @@ export class CodexJournalGenericFrames {
|
||||
threadId = 'session'
|
||||
): CodexJournalTranslationAdmission {
|
||||
const translated = unhandledProviderFrameJournalItem('codex', kind, payload)
|
||||
// A frame the classifier declines is deliberately not journaled, which is success.
|
||||
// Failing admission here force-closes the provider through the retry queue.
|
||||
if (!translated) {
|
||||
return { accepted: false, reason: 'untranslated' }
|
||||
return CODEX_JOURNAL_ADMITTED
|
||||
}
|
||||
const turnId = readCodexTurnId(payload) ?? this.activeTurn(threadId) ?? 'outside-turn'
|
||||
const bucket = this.bucketFor(threadId, turnId)
|
||||
const rowCount = this.genericRowsByTurn.get(bucket) ?? 0
|
||||
if (rowCount >= MAX_CODEX_GENERIC_ROWS_PER_TURN) {
|
||||
// The cap bounds noise, never evidence: an error frame is always journaled, and
|
||||
// capped frames stay countable through one summary row per turn.
|
||||
const isError = translated.classification === 'error-surface'
|
||||
if (!isError && rowCount >= MAX_CODEX_GENERIC_ROWS_PER_TURN) {
|
||||
this.addSuppressed(bucket, 1)
|
||||
this.recordBucket(bucket)
|
||||
this.scheduleSuppressedRows()
|
||||
return CODEX_JOURNAL_ADMITTED
|
||||
}
|
||||
if (translated.classification === 'error-surface') {
|
||||
if (isError) {
|
||||
const suppressionAdmission = this.flush()
|
||||
if (!suppressionAdmission.accepted) {
|
||||
return suppressionAdmission
|
||||
|
||||
@@ -299,6 +299,31 @@ describe('codex journal translation', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// A frame the classifier declines is intentionally unjournaled. #17720 turned that
|
||||
// into an admission failure, which the notification retry queue escalated to a
|
||||
// provider force-close, so no structured session could survive its first chrome frame.
|
||||
it('admits chrome and suppressed-benign frames without journaling them', () => {
|
||||
const { translator, tap } = translatorWith()
|
||||
translator.handle(TURN_STARTED)
|
||||
const before = tap.rows.length
|
||||
for (const method of [
|
||||
'remoteControl/status/changed',
|
||||
'thread/status/changed',
|
||||
'thread/tokenUsage/updated',
|
||||
'hook/started',
|
||||
'fs/changed',
|
||||
'rawResponse/completed',
|
||||
// Delta-shaped unknown methods reach the same early-out via the name heuristic.
|
||||
'future/somethingDelta'
|
||||
]) {
|
||||
expect(translator.handle(notification(method, { status: 'disabled' }))).toEqual({
|
||||
accepted: true
|
||||
})
|
||||
}
|
||||
expect(tap.rows.length).toBe(before)
|
||||
expect(tap.publishes()).toBe(0)
|
||||
})
|
||||
|
||||
it('bounds generic rows per turn while keeping the suppression visible and countable', () => {
|
||||
const { translator, tap, window } = translatorWith()
|
||||
translator.handle(TURN_STARTED)
|
||||
@@ -399,7 +424,9 @@ describe('codex journal translation', () => {
|
||||
expect(tap.rows.filter((row) => row.key.includes('provider-frame-suppressed'))).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('bounds error-surface provider frames under the generic-row cap', () => {
|
||||
// The cap bounds noise, never evidence. #17720 dropped this exemption (and pinned the
|
||||
// capped behavior in a test), so a noisy turn could silently swallow provider errors.
|
||||
it('exempts error-surface provider frames from the generic-row cap', () => {
|
||||
const { translator, tap, window } = translatorWith()
|
||||
translator.handle(TURN_STARTED)
|
||||
for (let index = 0; index < MAX_CODEX_GENERIC_ROWS_PER_TURN + 3; index += 1) {
|
||||
@@ -411,11 +438,15 @@ describe('codex journal translation', () => {
|
||||
const generic = tap.rows.filter(
|
||||
(row) => row.body.kind === 'status' && row.body.providerFrame !== undefined
|
||||
)
|
||||
expect(generic).toHaveLength(MAX_CODEX_GENERIC_ROWS_PER_TURN)
|
||||
expect(generic).toHaveLength(MAX_CODEX_GENERIC_ROWS_PER_TURN + 1)
|
||||
expect(generic.at(-1)?.body).toMatchObject({
|
||||
providerFrame: { kind: 'notification:future/failure' }
|
||||
})
|
||||
// Only the 3 capped noise frames reduce to a summary; the error is not counted there.
|
||||
expect(tap.rows.filter((row) => row.key.includes('provider-frame-suppressed'))).toHaveLength(1)
|
||||
expect(tap.rows.at(-1)?.body).toEqual({
|
||||
expect(tap.rows.find((row) => row.key.includes('provider-frame-suppressed'))?.body).toEqual({
|
||||
kind: 'status',
|
||||
text: '4 more provider notifications not shown for this turn'
|
||||
text: '3 more provider notifications not shown for this turn'
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -10,7 +10,7 @@ import { classifyProviderFrame } from './provider-frame-disposition'
|
||||
export type UnhandledProviderFrameJournalItem = {
|
||||
body: AgentJournalStatusItem
|
||||
blobs: { digest: string; payload: string }[]
|
||||
/** Why the frame surfaced; all classes are subject to the translator's row cap. */
|
||||
/** Why the frame surfaced. Error frames are exempt from generic-row caps. */
|
||||
classification: 'timeline-substantive' | 'error-surface'
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user