mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 00:57:45 +00:00
fix(native-chat): decide provider failures by kind, never by searching the payload (#29092)
* fix(native-chat): decide provider failures by kind, never by searching the payload A Claude chat opened with a red "claude · message:system:init" card whenever one of the user's MCP servers failed to connect. The classifier searched every nested field of a frame for failure-looking values and let that outrank the per-kind catalogue, so the session-start roster's `status: "failed"` turned the whole frame into an error row. The same search caused the earlier failed-background-task opcode cards. A frame is now a failure only because its kind is one, or because a field its kind declares as its outcome says so (Claude `hook_response.outcome`, Codex `hook/completed` `run.status`). A Claude turn result is a failure when the result journaling says so, since that depends on whether the user stopped the turn. With nothing left to override, the typed-translator coverage list, its option and the task-frame sentence seam go. Failed MCP servers write no row for Claude or Codex. * test(claude): an uncatalogued frame keeps its sentence without becoming a failure * fix(native-chat): keep Claude authentication errors visible
This commit is contained in:
@@ -141,6 +141,8 @@ describe('a Claude api_retry frame', () => {
|
||||
expect(rows).toEqual([
|
||||
unhandledProviderFrameJournalItem('claude', 'message:system:future_notice', unknown)?.body
|
||||
])
|
||||
expect(rows[0]).toMatchObject({ text: 'rate_limit', tone: 'error' })
|
||||
// An uncatalogued kind keeps its sentence but is no failure: only a kind decides that.
|
||||
expect(rows[0]).toMatchObject({ text: 'rate_limit' })
|
||||
expect(rows[0]).not.toHaveProperty('tone')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -8,7 +8,6 @@ import type {
|
||||
AgentSessionBackgroundTask,
|
||||
AgentSessionBackgroundTaskRunState
|
||||
} from '../../shared/agent-session-wire'
|
||||
import { backgroundTaskFallbackText } from '../../shared/native-chat-background-task-row'
|
||||
import { ownRetainedString } from '../../shared/own-retained-string'
|
||||
|
||||
const MAX_TASK_ID_LENGTH = 512
|
||||
@@ -56,59 +55,6 @@ export function taskText(value: unknown): string | undefined {
|
||||
return boundedTaskText(value)
|
||||
}
|
||||
|
||||
/**
|
||||
* The sentence a task frame wrote about itself.
|
||||
*
|
||||
* Only for a frame the row owner could not claim — malformed or capacity-refused.
|
||||
* It reaches the generic fallback, which has no key for `summary` and would
|
||||
* otherwise print the bare opcode. Passed to that fallback as Claude's own
|
||||
* display text rather than taught to its shared key list, because that list is
|
||||
* read for every provider and already resolves `summary` by hand for two Codex
|
||||
* methods; widening it globally to reach one malformed Claude frame would
|
||||
* re-rank the row text of every unmodelled frame on both providers.
|
||||
*/
|
||||
export function taskFrameSentence(frame: Record<string, unknown>): string | undefined {
|
||||
const patch = record(frame.patch)
|
||||
const sentence =
|
||||
taskText(frame.summary) ??
|
||||
taskText(frame.error) ??
|
||||
taskText(patch?.summary) ??
|
||||
taskText(patch?.error)
|
||||
if (sentence) {
|
||||
return sentence
|
||||
}
|
||||
|
||||
// A terminal task update often carries only its status in the nested patch.
|
||||
// Reuse the durable row's frozen sentence so capacity fallbacks never expose
|
||||
// the provider opcode when no human-facing text was supplied.
|
||||
if (
|
||||
frame.subtype !== 'task_started' &&
|
||||
frame.subtype !== 'task_updated' &&
|
||||
frame.subtype !== 'task_progress' &&
|
||||
frame.subtype !== 'task_notification'
|
||||
) {
|
||||
return undefined
|
||||
}
|
||||
const status = patch?.status ?? frame.status
|
||||
const state = terminalClaudeTaskRunState(status)
|
||||
if (state === null) {
|
||||
return undefined
|
||||
}
|
||||
const kind = classifyClaudeBackgroundTaskKind(patch?.task_type ?? frame.task_type)
|
||||
return backgroundTaskFallbackText({
|
||||
type: 'background-task',
|
||||
taskId: taskId(frame) ?? '',
|
||||
kind,
|
||||
label:
|
||||
taskDescription(patch?.description) ??
|
||||
taskDescription(frame.description) ??
|
||||
(patch ? taskName(patch) : undefined) ??
|
||||
taskName(frame) ??
|
||||
'',
|
||||
state
|
||||
})
|
||||
}
|
||||
|
||||
/** The provider-reported identity for a task. Subagent frames have carried the
|
||||
* type under both `agent_type` and `subagent_type` across SDK versions. */
|
||||
export function taskName(frame: Record<string, unknown>): string | undefined {
|
||||
|
||||
@@ -99,7 +99,7 @@ export function journalClaudeResult(
|
||||
message,
|
||||
failure?.text,
|
||||
undefined,
|
||||
undefined,
|
||||
{ reportsFailure: failure !== null },
|
||||
// A result that settles no turn is a CHILD's result: this
|
||||
// translator only ever opens root turns.
|
||||
settlesTurn
|
||||
|
||||
@@ -523,9 +523,8 @@ describe('claude journal translation — background task rows', () => {
|
||||
const { translator, fallbackRows, taskRowIds, taskRowTexts } = harness()
|
||||
playFailedBackgroundCommand(translator)
|
||||
|
||||
// ABLATION: drop `message:system:task_*` from TYPED_TRANSLATOR_KINDS
|
||||
// and this is `['claude · message:system:task_updated', 'claude ·
|
||||
// message:system:task_notification']` — the reported bug exactly.
|
||||
// When a payload search decided failures, this was `['claude ·
|
||||
// message:system:task_updated', 'claude · message:system:task_notification']`.
|
||||
expect(fallbackRows()).toEqual([])
|
||||
// One durable row for one task, however many frames reported it.
|
||||
expect(new Set(taskRowIds())).toEqual(new Set([`claude-background-task:${TASK_ID}`]))
|
||||
@@ -564,34 +563,11 @@ describe('claude journal translation — background task rows', () => {
|
||||
tasks: [{ task_id: TASK_ID, status: 'failed', task_type: 'local_bash' }]
|
||||
})
|
||||
)
|
||||
// It promotes through the payload sniffer exactly as the per-task frames do,
|
||||
// and it creates no row of its own: the task's own frames own that.
|
||||
// The task's own frames own its row; a nested status never makes one here.
|
||||
expect(fallbackRows()).toEqual([])
|
||||
expect(taskRowIds()).toEqual([])
|
||||
})
|
||||
|
||||
it('still surfaces an unmodelled failed frame through the generic fallback', () => {
|
||||
const { translator, fallbackRows } = harness()
|
||||
translator.handle(systemFrame({ subtype: 'future_event', status: 'failed' }))
|
||||
// The coverage contract is per-kind, so nothing about it weakens the payload
|
||||
// sniffer for the kinds nobody has modelled.
|
||||
expect(fallbackRows()).toEqual(['claude · message:system:future_event'])
|
||||
})
|
||||
|
||||
it('falls back visibly when a malformed task frame reports a failure', () => {
|
||||
const { translator, fallbackRows, taskRowIds } = harness()
|
||||
translator.handle(
|
||||
systemFrame({
|
||||
subtype: 'task_notification',
|
||||
status: 'failed',
|
||||
summary: 'Background command "Wait" failed with exit code 1'
|
||||
})
|
||||
)
|
||||
|
||||
expect(taskRowIds()).toEqual([])
|
||||
expect(fallbackRows()).toEqual(['Background command "Wait" failed with exit code 1'])
|
||||
})
|
||||
|
||||
it('keeps a refused live task failure on one typed terminal row', () => {
|
||||
const { translator, fallbackRows, taskRowIds, taskRowTexts } = harness()
|
||||
fillLiveTaskRows(translator)
|
||||
|
||||
@@ -11,7 +11,6 @@ import {
|
||||
createClaudeProviderFrameFallback,
|
||||
isClaudeProgressFrame
|
||||
} from './claude-structured-provider-fallback'
|
||||
import { taskFrameSentence } from './claude-background-task-frames'
|
||||
import { ClaudeBackgroundTaskRows } from './claude-background-task-rows'
|
||||
import { ClaudeToolOriginRegistry } from './claude-tool-origin-registry'
|
||||
import { ClaudeProvisionalRowCorrections } from './claude-provisional-row-corrections'
|
||||
@@ -249,10 +248,7 @@ export function createClaudeJournalTranslator(
|
||||
} else if (event.type === 'message' && event.message.type === 'result') {
|
||||
journalClaudeResult(resultContext, event.message, event.observedAt ?? Date.now())
|
||||
} else if (event.type === 'message') {
|
||||
const backgroundTaskCovered = backgroundTasks.observe(
|
||||
event.message,
|
||||
event.observedAt ?? Date.now()
|
||||
)
|
||||
backgroundTasks.observe(event.message, event.observedAt ?? Date.now())
|
||||
const kind = claudeProviderFrameKind(event.message)
|
||||
if (
|
||||
!handleMessage(
|
||||
@@ -265,9 +261,9 @@ export function createClaudeJournalTranslator(
|
||||
providerFallback.append(
|
||||
kind,
|
||||
event.message,
|
||||
taskFrameSentence(event.message),
|
||||
undefined,
|
||||
{ coveredByTypedTranslator: backgroundTaskCovered },
|
||||
undefined,
|
||||
undefined,
|
||||
corrections.stampFor(claudeFrameParentRef(event.message))
|
||||
)
|
||||
}
|
||||
|
||||
@@ -5,7 +5,6 @@ import {
|
||||
DEFAULT_JOURNAL_PAYLOAD_LIMITS
|
||||
} from '../native-chat/agent-session-journal/journal-payload-bounds'
|
||||
import { CLAUDE_STREAM_JSON_FRAME_KINDS } from '../native-chat/agent-session-wire/claude-stream-json-frame-schema'
|
||||
import { classifyProviderFrame } from '../native-chat/agent-session-wire/provider-frame-disposition'
|
||||
import {
|
||||
type UnhandledProviderFrameJournalItemOptions,
|
||||
readableProviderFrameText,
|
||||
@@ -43,14 +42,9 @@ const PROGRESS_FRAME_KINDS: ReadonlySet<string> = new Set([
|
||||
'message:stream_event:ping'
|
||||
])
|
||||
|
||||
/** A frame that writes no row, so nothing streamed has to be journaled ahead of it. A failure it
|
||||
* reports still surfaces as a row, so it is not one. */
|
||||
/** A frame that writes no row, so nothing streamed has to be journaled ahead of it. */
|
||||
export function isClaudeProgressFrame(message: Record<string, unknown>): boolean {
|
||||
const kind = claudeProviderFrameKind(message)
|
||||
return (
|
||||
PROGRESS_FRAME_KINDS.has(kind) &&
|
||||
classifyProviderFrame('claude', kind, message) !== 'error-surface'
|
||||
)
|
||||
return PROGRESS_FRAME_KINDS.has(claudeProviderFrameKind(message))
|
||||
}
|
||||
|
||||
const SETTLED_RESULT_KINDS: ReadonlySet<string> = new Set(
|
||||
|
||||
@@ -1,10 +1,8 @@
|
||||
import type { AgentJournalStatusItem } from '../../shared/agent-session-journal-types'
|
||||
import {
|
||||
unhandledProviderFrameJournalItem,
|
||||
type UnhandledProviderFrameJournalItem,
|
||||
type UnhandledProviderFrameJournalItemOptions
|
||||
type UnhandledProviderFrameJournalItem
|
||||
} from '../native-chat/agent-session-wire/unhandled-provider-frame'
|
||||
import { DEFAULT_JOURNAL_PAYLOAD_LIMITS } from '../native-chat/agent-session-journal/journal-payload-bounds'
|
||||
import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink'
|
||||
import type {
|
||||
CodexJournalTranslationAdmission,
|
||||
@@ -89,16 +87,9 @@ export class CodexJournalGenericFrames {
|
||||
appendUnhandled(
|
||||
kind: string,
|
||||
payload: unknown,
|
||||
threadId: string,
|
||||
options?: UnhandledProviderFrameJournalItemOptions
|
||||
threadId: string
|
||||
): CodexJournalTranslationAdmission {
|
||||
const translated = unhandledProviderFrameJournalItem(
|
||||
'codex',
|
||||
kind,
|
||||
payload,
|
||||
DEFAULT_JOURNAL_PAYLOAD_LIMITS,
|
||||
options
|
||||
)
|
||||
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.
|
||||
const auth =
|
||||
|
||||
@@ -455,7 +455,7 @@ describe('codex journal translation', () => {
|
||||
notification('future/notification', { message: 'Provider note', value: index })
|
||||
)
|
||||
}
|
||||
translator.handle(notification('future/failure', { error: 'provider exploded' }))
|
||||
translator.handle(notification('warning', { message: 'provider exploded' }))
|
||||
window.fire()
|
||||
|
||||
const generic = tap.rows.filter(
|
||||
@@ -463,7 +463,7 @@ describe('codex journal translation', () => {
|
||||
)
|
||||
expect(generic).toHaveLength(MAX_CODEX_GENERIC_ROWS_PER_TURN + 1)
|
||||
expect(generic.at(-1)?.body).toMatchObject({
|
||||
providerFrame: { kind: 'notification:future/failure' }
|
||||
providerFrame: { kind: 'notification:warning' }
|
||||
})
|
||||
// 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)
|
||||
|
||||
@@ -286,8 +286,7 @@ export function createCodexJournalTranslator(
|
||||
const unhandled = genericFrames.appendUnhandled(
|
||||
`notification:${event.method}`,
|
||||
event.params,
|
||||
event.threadId,
|
||||
{ coveredByTypedTranslator: event.method === 'thread/status/changed' }
|
||||
event.threadId
|
||||
)
|
||||
if (unhandled.accepted && event.method === 'error') {
|
||||
commands.errorShown(event.params)
|
||||
|
||||
@@ -1,8 +1,6 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { DEFAULT_JOURNAL_PAYLOAD_LIMITS } from '../agent-session-journal/journal-payload-bounds'
|
||||
import {
|
||||
classifyProviderFrame,
|
||||
hasTypedProviderFrameTranslator,
|
||||
isDeltaProviderFrameKind,
|
||||
PROVIDER_FRAME_CLASSIFICATIONS
|
||||
} from './provider-frame-disposition'
|
||||
@@ -43,7 +41,7 @@ describe('provider frame classification catalog', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('promotes payload failures over a benign catalog classification', () => {
|
||||
it("surfaces a failed hook from the hook's own outcome field", () => {
|
||||
expect(
|
||||
classifyProviderFrame('codex', 'notification:hook/completed', {
|
||||
run: { status: 'failed' }
|
||||
@@ -57,7 +55,25 @@ describe('provider frame classification catalog', () => {
|
||||
).toBe('error-surface')
|
||||
})
|
||||
|
||||
it('keeps command queue bookkeeping off the transcript without hiding a failed one', () => {
|
||||
it('surfaces a Claude authentication error from its declared field', () => {
|
||||
const failure = unhandledProviderFrameJournalItem('claude', 'message:auth_status', {
|
||||
type: 'auth_status',
|
||||
isAuthenticating: false,
|
||||
output: [],
|
||||
error: 'Login timed out'
|
||||
})
|
||||
expect(failure?.classification).toBe('error-surface')
|
||||
expect(failure?.body.text).toBe('Login timed out')
|
||||
expect(
|
||||
unhandledProviderFrameJournalItem('claude', 'message:auth_status', {
|
||||
type: 'auth_status',
|
||||
isAuthenticating: true,
|
||||
output: []
|
||||
})
|
||||
).toBeNull()
|
||||
})
|
||||
|
||||
it('keeps command queue bookkeeping off the transcript', () => {
|
||||
expect(
|
||||
classifyProviderFrame('claude', 'message:command_lifecycle', {
|
||||
command_uuid: 'command-1',
|
||||
@@ -70,14 +86,6 @@ describe('provider frame classification catalog', () => {
|
||||
state: 'cancelled'
|
||||
})
|
||||
).toBe('status-chrome')
|
||||
// Payload inspection outranks the catalogue, so suppressing the kind cannot
|
||||
// swallow a state the provider reports as a failure.
|
||||
expect(
|
||||
classifyProviderFrame('claude', 'message:command_lifecycle', {
|
||||
command_uuid: 'command-1',
|
||||
state: 'failed'
|
||||
})
|
||||
).toBe('error-surface')
|
||||
})
|
||||
|
||||
it('keeps unknown future frames on the substantive bounded fallback path', () => {
|
||||
@@ -110,11 +118,6 @@ describe('provider frame classification catalog', () => {
|
||||
expect(classifyProviderFrame('codex', 'item:sleep', { id: 's', durationMs: 20_000 })).toBe(
|
||||
'status-chrome'
|
||||
)
|
||||
// Payload inspection still outranks the item catalog, so chroming a type
|
||||
// cannot swallow one that reports a failure.
|
||||
expect(classifyProviderFrame('codex', 'item:sleep', { id: 's', status: 'failed' })).toBe(
|
||||
'error-surface'
|
||||
)
|
||||
})
|
||||
|
||||
it('suppresses subAgentActivity once the roster renders it, but never collabAgentToolCall', () => {
|
||||
@@ -157,80 +160,38 @@ describe('provider frame classification catalog', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('typed translator coverage', () => {
|
||||
it('emits no generic row for a covered kind, whatever the payload reports', () => {
|
||||
// The catalogue calls these `status-chrome`, but `hasProviderError` promotes
|
||||
// any of them that reports a failure — which is how a failed background task
|
||||
// reached users as `claude · message:system:task_notification`. Coverage is
|
||||
// the contract that stops it: the typed translator writes the row instead.
|
||||
for (const kind of [
|
||||
'message:system:task_started',
|
||||
'message:system:task_updated',
|
||||
'message:system:task_progress',
|
||||
'message:system:task_notification',
|
||||
'message:system:background_tasks_changed'
|
||||
]) {
|
||||
expect(
|
||||
unhandledProviderFrameJournalItem(
|
||||
'claude',
|
||||
kind,
|
||||
{
|
||||
task_id: 'byjnee2no',
|
||||
status: 'failed',
|
||||
summary: 'Background command "Wait" failed with exit code 1'
|
||||
},
|
||||
DEFAULT_JOURNAL_PAYLOAD_LIMITS,
|
||||
{ coveredByTypedTranslator: true }
|
||||
),
|
||||
kind
|
||||
).toBeNull()
|
||||
describe('failure is decided by kind, never by searching the payload', () => {
|
||||
it('writes no row for a Claude session start that lists MCP servers which failed to connect', () => {
|
||||
// The roster as Claude Code 2.1.289 sends it (issue #25477): a failed server carries
|
||||
// only its status, and used to turn the whole session start into a red opcode card.
|
||||
const init = {
|
||||
type: 'system',
|
||||
subtype: 'init',
|
||||
mcp_servers: [
|
||||
{ name: 'MCP_DOCKER', status: 'failed', source: 'user' },
|
||||
{ name: 'codegraph', status: 'connected', source: 'user' },
|
||||
{ name: 'jira', status: 'failed', source: 'user' }
|
||||
]
|
||||
}
|
||||
expect(unhandledProviderFrameJournalItem('claude', 'message:system:init', init)).toBeNull()
|
||||
})
|
||||
|
||||
it('keeps malformed covered-kind failures eligible for the generic fallback', () => {
|
||||
// Eligibility is all this layer decides. A frame naming no task is not
|
||||
// claimed by the row owner, which withholds the coverage flag so the
|
||||
// failure still reaches the user. The SENTENCE it leads with is Claude's to
|
||||
// supply, through the fallback's display-text seam — proven in
|
||||
// `claude-structured-journal-translation-background-tasks.test.ts`. Teaching
|
||||
// `summary` to the shared key list here would re-rank the row text of every
|
||||
// unmodelled frame on both providers to reach this one case.
|
||||
it('never turns a nested failure-looking value into an error row', () => {
|
||||
const failed = { task_id: 't-1', status: 'failed', error: 'exit code 1' }
|
||||
expect(
|
||||
unhandledProviderFrameJournalItem('claude', 'message:system:task_notification', {
|
||||
unhandledProviderFrameJournalItem('claude', 'message:system:task_notification', failed)
|
||||
).toBeNull()
|
||||
expect(
|
||||
unhandledProviderFrameJournalItem('codex', 'notification:mcpServer/startupStatus/updated', {
|
||||
name: 'github',
|
||||
status: 'failed',
|
||||
summary: 'Background command "Wait" failed with exit code 1'
|
||||
})
|
||||
).toMatchObject({ classification: 'error-surface' })
|
||||
})
|
||||
|
||||
it('covers the Codex thread status, which reports `systemError` beside the `error` row', () => {
|
||||
const kind = 'notification:thread/status/changed'
|
||||
const payload = { threadId: 'thread-1', status: { type: 'systemError' } }
|
||||
|
||||
expect(hasTypedProviderFrameTranslator('codex', kind)).toBe(true)
|
||||
expect(
|
||||
unhandledProviderFrameJournalItem('codex', kind, payload, DEFAULT_JOURNAL_PAYLOAD_LIMITS, {
|
||||
coveredByTypedTranslator: true
|
||||
error: 'auth expired'
|
||||
})
|
||||
).toBeNull()
|
||||
expect(unhandledProviderFrameJournalItem('codex', kind, payload)).toMatchObject({
|
||||
classification: 'error-surface'
|
||||
})
|
||||
})
|
||||
|
||||
it('covers a kind only for its own provider — a Claude kind from Codex still falls back', () => {
|
||||
expect(
|
||||
unhandledProviderFrameJournalItem('codex', 'message:system:task_notification', {
|
||||
status: 'failed'
|
||||
})
|
||||
).not.toBeNull()
|
||||
})
|
||||
|
||||
it('leaves an unmodelled Claude failure on the visible fallback', () => {
|
||||
const item = unhandledProviderFrameJournalItem('claude', 'message:system:future_task', {
|
||||
status: 'failed'
|
||||
})
|
||||
expect(item?.classification).toBe('error-surface')
|
||||
// A kind nobody catalogued stays an ordinary fallback row, not a failure.
|
||||
const unknown = unhandledProviderFrameJournalItem('claude', 'message:system:future', failed)
|
||||
expect(unknown?.classification).toBe('timeline-substantive')
|
||||
expect(unknown?.body.tone).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -280,13 +241,4 @@ describe('codex subagent item disposition', () => {
|
||||
})
|
||||
).toBeNull()
|
||||
})
|
||||
|
||||
it('still surfaces a subagent frame that reports a failure', () => {
|
||||
expect(
|
||||
classifyProviderFrame('codex', 'item:collabAgentToolCall', {
|
||||
type: 'collabAgentToolCall',
|
||||
status: 'failed'
|
||||
})
|
||||
).toBe('error-surface')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { CodexAppServerNotificationMethod } from '../../codex/codex-app-server-notification-schema'
|
||||
import { CODEX_SUBAGENT_ITEM_TYPE } from '../../codex/codex-subagent-activity'
|
||||
import { isProviderRecord } from '../../provider-process/provider-json-record'
|
||||
import type { ClaudeStreamJsonFrameKind } from './claude-stream-json-frame-schema'
|
||||
|
||||
export type ProviderFrameClassification =
|
||||
@@ -138,11 +139,11 @@ export const PROVIDER_FRAME_CLASSIFICATIONS = {
|
||||
'message:system:informational': 'status-chrome',
|
||||
'message:conversation_reset': 'status-chrome',
|
||||
// A `started`/`completed`/`cancelled` state for one queued command uuid and
|
||||
// nothing else; the CLI keeps it out of its own transcript too. A state that
|
||||
// reads as a failure still surfaces, via the payload check in classify.
|
||||
// nothing else; the CLI keeps it out of its own transcript too.
|
||||
'message:command_lifecycle': 'status-chrome',
|
||||
// The turn-complete signal: lifecycle, never a transcript row. Error subtypes
|
||||
// included — the turn's assistant frames already carry any user-facing text.
|
||||
// The turn-complete signal: lifecycle, never a transcript row by kind. Whether one
|
||||
// failed depends on its turn (a stopped turn is no failure), so the result
|
||||
// journaling writes that row with its own verdict.
|
||||
'message:result:success': 'status-chrome',
|
||||
'message:result:error_during_execution': 'status-chrome',
|
||||
'message:result:error_max_turns': 'status-chrome',
|
||||
@@ -151,99 +152,6 @@ export const PROVIDER_FRAME_CLASSIFICATIONS = {
|
||||
}
|
||||
} as const satisfies ProviderFrameClassificationTable
|
||||
|
||||
/**
|
||||
* Frame kinds a DEDICATED typed translator owns end to end.
|
||||
*
|
||||
* Coverage is a contract, not a label. The catalogue classification alone is
|
||||
* only a hint about where a frame belongs, and `hasProviderError` deliberately
|
||||
* outranks it so an unmodelled failure still reaches the user — which is how a
|
||||
* failed background task ended up rendered by the generic fallback, whose row
|
||||
* text is the wire opcode when the payload carries no key the fallback knows.
|
||||
* A kind listed here is guaranteed no fallback row instead, so its translator
|
||||
* may legitimately emit zero rows for a frame and nothing appears beside it.
|
||||
*
|
||||
* Listing a kind before its translator exists deletes the only report of a
|
||||
* failure, so nothing may be added here except together with the code that
|
||||
* renders it, or that reads it for state while another frame carries its
|
||||
* failure (the Codex thread status, whose fault arrives on `error`).
|
||||
*
|
||||
* A Claude frame of a listed kind that names no task writes nothing. The row it
|
||||
* replaces named no task either — it printed the opcode and a raw payload —
|
||||
* and every frame this protocol sends carries the id its own tracker and
|
||||
* roster have always required.
|
||||
*/
|
||||
const TYPED_TRANSLATOR_KINDS: ReadonlyMap<string, ReadonlySet<string>> = new Map<
|
||||
keyof ProviderFrameClassificationTable,
|
||||
ReadonlySet<string>
|
||||
>([
|
||||
[
|
||||
'claude',
|
||||
new Set([
|
||||
'message:system:task_started',
|
||||
'message:system:task_updated',
|
||||
'message:system:task_progress',
|
||||
'message:system:task_notification',
|
||||
'message:system:background_tasks_changed'
|
||||
] satisfies ClaudeStreamJsonFrameKind[])
|
||||
],
|
||||
[
|
||||
'codex',
|
||||
// Every status arm is thread state the translator reads; a fault's sentence arrives on `error`.
|
||||
new Set([
|
||||
'notification:thread/status/changed'
|
||||
] satisfies `notification:${CodexAppServerNotificationMethod}`[])
|
||||
]
|
||||
])
|
||||
|
||||
export function hasTypedProviderFrameTranslator(provider: string, kind: string): boolean {
|
||||
return TYPED_TRANSLATOR_KINDS.get(provider)?.has(kind) === true
|
||||
}
|
||||
|
||||
const ERROR_VARIANT_KEYS = new Set(['type', 'status', 'state', 'subtype', 'outcome'])
|
||||
const ERROR_VALUE_KEYS = new Set(['error', 'failureReason', 'failure_reason'])
|
||||
|
||||
function isErrorVariant(value: unknown): boolean {
|
||||
if (typeof value !== 'string') {
|
||||
return false
|
||||
}
|
||||
const normalized = value.replace(/[_\s-]/g, '').toLowerCase()
|
||||
return (
|
||||
normalized.startsWith('error') || normalized.startsWith('fail') || normalized === 'systemerror'
|
||||
)
|
||||
}
|
||||
|
||||
function hasProviderError(payload: unknown): boolean {
|
||||
const pending = [payload]
|
||||
const seen = new WeakSet<object>()
|
||||
while (pending.length > 0) {
|
||||
const value = pending.pop()
|
||||
if (typeof value !== 'object' || value === null || seen.has(value)) {
|
||||
continue
|
||||
}
|
||||
seen.add(value)
|
||||
if (Array.isArray(value)) {
|
||||
pending.push(...value)
|
||||
continue
|
||||
}
|
||||
for (const [key, nested] of Object.entries(value)) {
|
||||
if ((key === 'isError' || key === 'is_error') && nested === true) {
|
||||
return true
|
||||
}
|
||||
if (key === 'success' && nested === false) {
|
||||
return true
|
||||
}
|
||||
if (ERROR_VARIANT_KEYS.has(key) && isErrorVariant(nested)) {
|
||||
return true
|
||||
}
|
||||
if (ERROR_VALUE_KEYS.has(key) && nested !== null && nested !== false && nested !== '') {
|
||||
return true
|
||||
}
|
||||
pending.push(nested)
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
/** Codex thread-item types with no typed renderer, dispositioned by hand so a
|
||||
* new item type cannot leak `codex · item:<type>` into the transcript. The
|
||||
* notification catalog above is keyed by METHOD and never matches these. */
|
||||
@@ -285,7 +193,11 @@ const CLAUDE_FRAME_CLASSIFICATIONS: Readonly<Record<string, ProviderFrameClassif
|
||||
PROVIDER_FRAME_CLASSIFICATIONS.claude
|
||||
|
||||
/** Each provider's own table, by provider id: an unlisted provider has none, so every frame it
|
||||
* sends that no rule above claims stays a visible row. */
|
||||
* sends that no rule above claims stays a visible row.
|
||||
*
|
||||
* A frame is a failure only because its kind is one, or because a field its kind declares as its
|
||||
* outcome says so. Nested statuses describe other things (the MCP servers a session start lists,
|
||||
* the tasks a roster holds), so a payload is never searched for failure-looking words. */
|
||||
const PROVIDER_FRAME_CATALOGS: ReadonlyMap<
|
||||
string,
|
||||
(kind: string, payload: unknown) => ProviderFrameClassification | undefined
|
||||
@@ -295,24 +207,37 @@ const PROVIDER_FRAME_CATALOGS: ReadonlyMap<
|
||||
>([
|
||||
[
|
||||
'codex',
|
||||
(kind) => {
|
||||
(kind, payload) => {
|
||||
const item = itemKind(kind)
|
||||
if (item !== null) {
|
||||
return CODEX_ITEM_CLASSIFICATIONS[item]
|
||||
}
|
||||
return CODEX_NOTIFICATION_CLASSIFICATIONS[notificationKind(kind)]
|
||||
const method = notificationKind(kind)
|
||||
if (method === 'hook/completed') {
|
||||
const run = isProviderRecord(payload) && isProviderRecord(payload.run) ? payload.run : null
|
||||
return run?.status === 'failed' ? 'error-surface' : 'suppressed-benign'
|
||||
}
|
||||
return CODEX_NOTIFICATION_CLASSIFICATIONS[method]
|
||||
}
|
||||
],
|
||||
[
|
||||
'claude',
|
||||
(kind, payload) => {
|
||||
if (kind === 'message:result') {
|
||||
const subtype =
|
||||
typeof payload === 'object' && payload !== null && 'subtype' in payload
|
||||
? payload.subtype
|
||||
: undefined
|
||||
const subtype = isProviderRecord(payload) ? payload.subtype : undefined
|
||||
return subtype === 'success' ? 'status-chrome' : 'error-surface'
|
||||
}
|
||||
if (kind === 'message:system:hook_response') {
|
||||
return isProviderRecord(payload) && payload.outcome === 'error'
|
||||
? 'error-surface'
|
||||
: 'suppressed-benign'
|
||||
}
|
||||
if (kind === 'message:auth_status') {
|
||||
const error = isProviderRecord(payload) ? payload.error : undefined
|
||||
return typeof error === 'string' && error.trim().length > 0
|
||||
? 'error-surface'
|
||||
: 'status-chrome'
|
||||
}
|
||||
return CLAUDE_FRAME_CLASSIFICATIONS[kind]
|
||||
}
|
||||
]
|
||||
@@ -323,12 +248,6 @@ export function classifyProviderFrame(
|
||||
kind: string,
|
||||
payload: unknown
|
||||
): ProviderFrameClassification {
|
||||
// Payload failure inspection outranks the name-shape heuristic below: an
|
||||
// unknown frame that reports an error must reach the user even when its
|
||||
// method name happens to look like a stream delta.
|
||||
if (hasProviderError(payload)) {
|
||||
return 'error-surface'
|
||||
}
|
||||
if (isDeltaProviderFrameKind(kind)) {
|
||||
return 'stream-into-item'
|
||||
}
|
||||
|
||||
@@ -59,7 +59,7 @@ describe('unhandled provider frame journal fallback', () => {
|
||||
).toBeNull()
|
||||
})
|
||||
|
||||
it('never creates generic rows for delta-shaped frames that report no failure', () => {
|
||||
it('never creates generic rows for delta-shaped frames', () => {
|
||||
expect(
|
||||
unhandledProviderFrameJournalItem('codex', 'notification:item/commandExecution/outputDelta', {
|
||||
itemId: 'exec-1',
|
||||
@@ -74,34 +74,14 @@ describe('unhandled provider frame journal fallback', () => {
|
||||
).toBeNull()
|
||||
})
|
||||
|
||||
it('surfaces an unknown delta-shaped frame whose payload reports an error', () => {
|
||||
const row = unhandledProviderFrameJournalItem('codex', 'notification:item/future/outputDelta', {
|
||||
error: 'stream broke mid-item'
|
||||
})
|
||||
|
||||
expect(row).not.toBeNull()
|
||||
expect(row?.classification).toBe('error-surface')
|
||||
expect(row?.body.providerFrame).toMatchObject({
|
||||
provider: 'codex',
|
||||
kind: 'notification:item/future/outputDelta'
|
||||
})
|
||||
})
|
||||
|
||||
it('renders codex systemError and Claude error result variants when no typed translator covers the frame', () => {
|
||||
const codex = unhandledProviderFrameJournalItem('codex', 'notification:thread/status/changed', {
|
||||
threadId: 'thread-1',
|
||||
status: { type: 'systemError' }
|
||||
})
|
||||
it('renders a Claude result frame that names no success as a failure row', () => {
|
||||
const claude = unhandledProviderFrameJournalItem('claude', 'message:result', {
|
||||
subtype: 'error_during_execution',
|
||||
is_error: true,
|
||||
result: 'Provider request failed'
|
||||
})
|
||||
|
||||
expect(codex?.body.providerFrame).toMatchObject({
|
||||
provider: 'codex',
|
||||
kind: 'notification:thread/status/changed'
|
||||
})
|
||||
expect(claude?.classification).toBe('error-surface')
|
||||
expect(claude?.body.providerFrame).toMatchObject({
|
||||
provider: 'claude',
|
||||
kind: 'message:result'
|
||||
@@ -126,27 +106,6 @@ describe('unhandled provider frame journal fallback', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps failed startup variants visible while suppressing startup progress', () => {
|
||||
const kind = 'notification:mcpServer/startupStatus/updated'
|
||||
|
||||
expect(
|
||||
unhandledProviderFrameJournalItem('codex', kind, {
|
||||
name: 'filesystem',
|
||||
status: 'starting',
|
||||
error: null,
|
||||
failureReason: null
|
||||
})
|
||||
).toBeNull()
|
||||
expect(
|
||||
unhandledProviderFrameJournalItem('codex', kind, {
|
||||
name: 'filesystem',
|
||||
status: 'failed',
|
||||
error: 'server exited',
|
||||
failureReason: null
|
||||
})
|
||||
).not.toBeNull()
|
||||
})
|
||||
|
||||
it('surfaces a failed hook completion while suppressing successful hook lifecycle', () => {
|
||||
const kind = 'notification:hook/completed'
|
||||
|
||||
@@ -219,35 +178,6 @@ describe('unhandled provider frame journal fallback', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('a failed provider dependency', () => {
|
||||
it('leads with the failure the provider reported, not the method name', () => {
|
||||
const item = unhandledProviderFrameJournalItem(
|
||||
'codex',
|
||||
'notification:mcpServer/startupStatus/updated',
|
||||
{
|
||||
threadId: 'thread-1',
|
||||
name: 'codex_apps',
|
||||
status: 'failed',
|
||||
error: 'MCP client for `codex_apps` failed to start: authentication token invalidated',
|
||||
failureReason: 'reauthenticationRequired'
|
||||
}
|
||||
)
|
||||
expect(item?.classification).toBe('error-surface')
|
||||
expect(item?.body.text).toContain('failed to start')
|
||||
expect(item?.body.text).not.toContain('notification:mcpServer')
|
||||
})
|
||||
|
||||
it('stays out of the timeline while the dependency is merely starting', () => {
|
||||
expect(
|
||||
unhandledProviderFrameJournalItem('codex', 'notification:mcpServer/startupStatus/updated', {
|
||||
threadId: 'thread-1',
|
||||
name: 'codex_apps',
|
||||
status: 'starting'
|
||||
})
|
||||
).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('typed notice metadata', () => {
|
||||
it('publishes readable compaction statuses for both provider forms', () => {
|
||||
expect(
|
||||
|
||||
@@ -8,10 +8,7 @@ import {
|
||||
} from '../agent-session-journal/journal-payload-bounds'
|
||||
import { codexGoalRowText, codexThreadGoalState } from '../../codex/codex-goal-journal-rows'
|
||||
import { isProviderRecord } from '../../provider-process/provider-json-record'
|
||||
import {
|
||||
classifyProviderFrame,
|
||||
hasTypedProviderFrameTranslator
|
||||
} from './provider-frame-disposition'
|
||||
import { classifyProviderFrame } from './provider-frame-disposition'
|
||||
|
||||
export type UnhandledProviderFrameJournalItem = {
|
||||
body: AgentJournalPlainStatusItem
|
||||
@@ -20,8 +17,9 @@ export type UnhandledProviderFrameJournalItem = {
|
||||
}
|
||||
|
||||
export type UnhandledProviderFrameJournalItemOptions = {
|
||||
/** A typed translator accepted this exact frame, not merely this frame kind. */
|
||||
coveredByTypedTranslator?: boolean
|
||||
/** The caller's own translator judged this frame a failure, from facts its kind alone cannot
|
||||
* carry (a Claude result is one only when its turn was not stopped by the user). */
|
||||
reportsFailure?: boolean
|
||||
}
|
||||
|
||||
function serializeProviderPayload(payload: unknown): string {
|
||||
@@ -42,9 +40,7 @@ const MESSAGE_KEYS = [
|
||||
'detail',
|
||||
'description',
|
||||
'reason',
|
||||
// `error` is how a failed dependency reports itself — an MCP server that could not start says
|
||||
// so here and nowhere else. Without it the row falls back to the bare method name, which is how
|
||||
// "MCP server X failed to start: auth expired" reached users as `notification:mcpServer/...`.
|
||||
// Where a failure frame keeps its sentence (Codex's `error` nests it as `error.message`).
|
||||
'error'
|
||||
] as const
|
||||
|
||||
@@ -130,18 +126,8 @@ export function unhandledProviderFrameJournalItem(
|
||||
limits: JournalPayloadLimits = DEFAULT_JOURNAL_PAYLOAD_LIMITS,
|
||||
options: UnhandledProviderFrameJournalItemOptions = {}
|
||||
): UnhandledProviderFrameJournalItem | null {
|
||||
// A kind a typed translator owns never degrades to its opcode here, in either
|
||||
// direction: "no row" is that translator's decision, not a gap this fallback
|
||||
// has to cover. Checked before classification, because the payload sniffer
|
||||
// inside it promotes a covered frame that reports a failure and would
|
||||
// otherwise print `${provider} · ${kind}` beside the typed row.
|
||||
if (
|
||||
options.coveredByTypedTranslator === true &&
|
||||
hasTypedProviderFrameTranslator(provider, kind)
|
||||
) {
|
||||
return null
|
||||
}
|
||||
const classification = classifyProviderFrame(provider, kind, payload)
|
||||
const classification =
|
||||
options.reportsFailure === true ? 'error-surface' : classifyProviderFrame(provider, kind, payload)
|
||||
if (
|
||||
classification === 'stream-into-item' ||
|
||||
classification === 'status-chrome' ||
|
||||
|
||||
Reference in New Issue
Block a user