diff --git a/src/main/claude/claude-api-retry-row.test.ts b/src/main/claude/claude-api-retry-row.test.ts index 26ad6d3a26a..c7b6cb2c64e 100644 --- a/src/main/claude/claude-api-retry-row.test.ts +++ b/src/main/claude/claude-api-retry-row.test.ts @@ -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') }) }) diff --git a/src/main/claude/claude-background-task-frames.ts b/src/main/claude/claude-background-task-frames.ts index 7ff5101f77b..818decf5e7a 100644 --- a/src/main/claude/claude-background-task-frames.ts +++ b/src/main/claude/claude-background-task-frames.ts @@ -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 | 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 | undefined { diff --git a/src/main/claude/claude-result-journaling.ts b/src/main/claude/claude-result-journaling.ts index 1568c3a463f..9e68e852f35 100644 --- a/src/main/claude/claude-result-journaling.ts +++ b/src/main/claude/claude-result-journaling.ts @@ -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 diff --git a/src/main/claude/claude-structured-journal-translation-background-tasks.test.ts b/src/main/claude/claude-structured-journal-translation-background-tasks.test.ts index 88c5b2cd4bd..b18eb751472 100644 --- a/src/main/claude/claude-structured-journal-translation-background-tasks.test.ts +++ b/src/main/claude/claude-structured-journal-translation-background-tasks.test.ts @@ -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) diff --git a/src/main/claude/claude-structured-journal-translation.ts b/src/main/claude/claude-structured-journal-translation.ts index b6980a5bae7..a8da78e9a94 100644 --- a/src/main/claude/claude-structured-journal-translation.ts +++ b/src/main/claude/claude-structured-journal-translation.ts @@ -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)) ) } diff --git a/src/main/claude/claude-structured-provider-fallback.ts b/src/main/claude/claude-structured-provider-fallback.ts index 7bda9d53799..b8a84133e22 100644 --- a/src/main/claude/claude-structured-provider-fallback.ts +++ b/src/main/claude/claude-structured-provider-fallback.ts @@ -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 = 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): 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 = new Set( diff --git a/src/main/codex/codex-structured-journal-generic-frames.ts b/src/main/codex/codex-structured-journal-generic-frames.ts index 5cdacc2b978..64e80bffc2b 100644 --- a/src/main/codex/codex-structured-journal-generic-frames.ts +++ b/src/main/codex/codex-structured-journal-generic-frames.ts @@ -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 = diff --git a/src/main/codex/codex-structured-journal-translation-streams.test.ts b/src/main/codex/codex-structured-journal-translation-streams.test.ts index d98ccba5f35..baf2620ea8f 100644 --- a/src/main/codex/codex-structured-journal-translation-streams.test.ts +++ b/src/main/codex/codex-structured-journal-translation-streams.test.ts @@ -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) diff --git a/src/main/codex/codex-structured-journal-translation.ts b/src/main/codex/codex-structured-journal-translation.ts index d140ba3279e..19754d3f045 100644 --- a/src/main/codex/codex-structured-journal-translation.ts +++ b/src/main/codex/codex-structured-journal-translation.ts @@ -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) diff --git a/src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts b/src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts index 141e9ff24bc..0cfd3861a58 100644 --- a/src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts +++ b/src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts @@ -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') - }) }) diff --git a/src/main/native-chat/agent-session-wire/provider-frame-disposition.ts b/src/main/native-chat/agent-session-wire/provider-frame-disposition.ts index 543c8f0cbb6..6e0c1e44ee5 100644 --- a/src/main/native-chat/agent-session-wire/provider-frame-disposition.ts +++ b/src/main/native-chat/agent-session-wire/provider-frame-disposition.ts @@ -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> = new Map< - keyof ProviderFrameClassificationTable, - ReadonlySet ->([ - [ - '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() - 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:` into the transcript. The * notification catalog above is keyed by METHOD and never matches these. */ @@ -285,7 +193,11 @@ const CLAUDE_FRAME_CLASSIFICATIONS: Readonly 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' } diff --git a/src/main/native-chat/agent-session-wire/unhandled-provider-frame.test.ts b/src/main/native-chat/agent-session-wire/unhandled-provider-frame.test.ts index 8c633f9d4e1..dee7c5cc217 100644 --- a/src/main/native-chat/agent-session-wire/unhandled-provider-frame.test.ts +++ b/src/main/native-chat/agent-session-wire/unhandled-provider-frame.test.ts @@ -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( diff --git a/src/main/native-chat/agent-session-wire/unhandled-provider-frame.ts b/src/main/native-chat/agent-session-wire/unhandled-provider-frame.ts index c0f9363a52d..823a4c4b8d6 100644 --- a/src/main/native-chat/agent-session-wire/unhandled-provider-frame.ts +++ b/src/main/native-chat/agent-session-wire/unhandled-provider-frame.ts @@ -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' ||