From af0d2f697b79631e2c375d8b1d56dbaaff03e95c Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 13 Aug 2026 20:05:10 -0700 Subject: [PATCH] fix structured notification classification --- .../codex-app-server-notification-schema.ts | 78 +++++++ ...dex-structured-journal-translation.test.ts | 37 ++++ .../claude-stream-json-frame-schema.ts | 48 +++++ .../provider-frame-disposition.test.ts | 54 +++++ .../provider-frame-disposition.ts | 194 ++++++++++++++---- .../unhandled-provider-frame.test.ts | 15 ++ .../unhandled-provider-frame.ts | 3 +- 7 files changed, 391 insertions(+), 38 deletions(-) create mode 100644 src/main/codex/codex-app-server-notification-schema.ts create mode 100644 src/main/native-chat/agent-session-wire/claude-stream-json-frame-schema.ts create mode 100644 src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts diff --git a/src/main/codex/codex-app-server-notification-schema.ts b/src/main/codex/codex-app-server-notification-schema.ts new file mode 100644 index 00000000000..a7319273cdb --- /dev/null +++ b/src/main/codex/codex-app-server-notification-schema.ts @@ -0,0 +1,78 @@ +// Stable ServerNotification method discriminators generated by codex-cli 0.147.0. +export const CODEX_APP_SERVER_NOTIFICATION_METHODS = [ + 'error', + 'thread/started', + 'thread/status/changed', + 'thread/archived', + 'thread/deleted', + 'thread/unarchived', + 'thread/closed', + 'skills/changed', + 'thread/name/updated', + 'thread/goal/updated', + 'thread/goal/cleared', + 'thread/environment/connected', + 'thread/environment/disconnected', + 'thread/settings/updated', + 'thread/tokenUsage/updated', + 'turn/started', + 'hook/started', + 'turn/completed', + 'hook/completed', + 'turn/diff/updated', + 'turn/plan/updated', + 'item/started', + 'item/autoApprovalReview/started', + 'item/autoApprovalReview/completed', + 'item/completed', + 'rawResponseItem/completed', + 'rawResponse/completed', + 'item/agentMessage/delta', + 'item/plan/delta', + 'command/exec/outputDelta', + 'process/outputDelta', + 'process/exited', + 'item/commandExecution/outputDelta', + 'item/commandExecution/terminalInteraction', + 'item/fileChange/outputDelta', + 'item/fileChange/patchUpdated', + 'serverRequest/resolved', + 'item/mcpToolCall/progress', + 'mcpServer/oauthLogin/completed', + 'mcpServer/startupStatus/updated', + 'account/updated', + 'account/rateLimits/updated', + 'app/list/updated', + 'remoteControl/status/changed', + 'externalAgentConfig/import/progress', + 'externalAgentConfig/import/completed', + 'fs/changed', + 'item/reasoning/summaryTextDelta', + 'item/reasoning/summaryPartAdded', + 'item/reasoning/textDelta', + 'thread/compacted', + 'model/rerouted', + 'model/verification', + 'turn/moderationMetadata', + 'model/safetyBuffering/updated', + 'warning', + 'guardianWarning', + 'deprecationNotice', + 'configWarning', + 'fuzzyFileSearch/sessionUpdated', + 'fuzzyFileSearch/sessionCompleted', + 'thread/realtime/started', + 'thread/realtime/itemAdded', + 'thread/realtime/transcript/delta', + 'thread/realtime/transcript/done', + 'thread/realtime/outputAudio/delta', + 'thread/realtime/sdp', + 'thread/realtime/error', + 'thread/realtime/closed', + 'windows/worldWritableWarning', + 'windowsSandbox/setupCompleted', + 'account/login/completed' +] as const + +export type CodexAppServerNotificationMethod = + (typeof CODEX_APP_SERVER_NOTIFICATION_METHODS)[number] diff --git a/src/main/codex/codex-structured-journal-translation.test.ts b/src/main/codex/codex-structured-journal-translation.test.ts index 623403c6c4c..8b33c9f4b9e 100644 --- a/src/main/codex/codex-structured-journal-translation.test.ts +++ b/src/main/codex/codex-structured-journal-translation.test.ts @@ -469,6 +469,43 @@ describe('codex journal translation', () => { expect(timeline).toEqual([]) }) + it('projects only user and assistant content for a complete turn with hooks', () => { + const { translator, tap } = translatorWith() + + translator.handle(notification('thread/started', { thread: { id: THREAD_ID } })) + translator.handle(notification('hook/started', { run: { id: 'hook-1', status: 'running' } })) + translator.handle(notification('account/rateLimits/updated', { rateLimits: { primary: null } })) + translator.handle(TURN_STARTED) + translator.handle( + notification('item/completed', { + item: { type: 'userMessage', id: 'item-0', text: 'hi' } + }) + ) + translator.handle( + notification('hook/completed', { run: { id: 'hook-1', status: 'completed' } }) + ) + translator.handle( + notification('item/completed', { + item: { type: 'agentMessage', id: 'item-1', text: 'hello' } + }) + ) + translator.handle(notification('turn/completed', { turn: { id: TURN_ID } })) + + const timeline = projectStructuredItemsToNativeChat( + tap.rows.map((row, index) => ({ + itemId: row.key, + revision: 1, + sequence: index + 1, + observedAt: index + 1, + body: row.body + })) + ) + expect(timeline.map(({ role, blocks }) => ({ role, blocks }))).toEqual([ + { role: 'user', blocks: [{ type: 'text', text: 'hi' }] }, + { role: 'assistant', blocks: [{ type: 'text', text: 'hello' }] } + ]) + }) + it('renders a system error carried by a suppressed status kind', () => { const { translator, tap } = translatorWith() diff --git a/src/main/native-chat/agent-session-wire/claude-stream-json-frame-schema.ts b/src/main/native-chat/agent-session-wire/claude-stream-json-frame-schema.ts new file mode 100644 index 00000000000..4f3ef118af5 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/claude-stream-json-frame-schema.ts @@ -0,0 +1,48 @@ +// SDKMessage discriminators from Claude Agent SDK 0.3.231 / Claude Code 2.1.231. +export const CLAUDE_STREAM_JSON_FRAME_KINDS = [ + 'message:assistant', + 'message:user', + 'message:result', + 'message:system:init', + 'message:stream_event:message_start', + 'message:stream_event:message_delta', + 'message:stream_event:message_stop', + 'message:stream_event:content_block_start', + 'message:stream_event:content_block_delta', + 'message:stream_event:content_block_stop', + 'message:system:compact_boundary', + 'message:system:status', + 'message:system:api_retry', + 'message:system:control_request_progress', + 'message:system:model_refusal_fallback', + 'message:system:model_refusal_no_fallback', + 'message:system:local_command_output', + 'message:system:hook_started', + 'message:system:hook_progress', + 'message:system:hook_response', + 'message:system:plugin_install', + 'message:tool_progress', + 'message:auth_status', + 'message:system:task_notification', + 'message:system:task_started', + 'message:system:task_updated', + 'message:system:task_progress', + 'message:system:background_tasks_changed', + 'message:system:thinking_tokens', + 'message:system:session_state_changed', + 'message:system:worker_shutting_down', + 'message:system:commands_changed', + 'message:system:notification', + 'message:system:files_persisted', + 'message:tool_use_summary', + 'message:system:memory_recall', + 'message:rate_limit_event', + 'message:system:elicitation_complete', + 'message:system:permission_denied', + 'message:prompt_suggestion', + 'message:system:mirror_error', + 'message:system:informational', + 'message:conversation_reset' +] as const + +export type ClaudeStreamJsonFrameKind = (typeof CLAUDE_STREAM_JSON_FRAME_KINDS)[number] 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 new file mode 100644 index 00000000000..93444c638bd --- /dev/null +++ b/src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from 'vitest' +import { CODEX_APP_SERVER_NOTIFICATION_METHODS } from '../../codex/codex-app-server-notification-schema' +import { CLAUDE_STREAM_JSON_FRAME_KINDS } from './claude-stream-json-frame-schema' +import { classifyProviderFrame, PROVIDER_FRAME_CLASSIFICATIONS } from './provider-frame-disposition' + +describe('provider frame classification catalog', () => { + it('classifies every pinned Codex app-server notification method', () => { + expect(Object.keys(PROVIDER_FRAME_CLASSIFICATIONS.codex)).toEqual([ + ...CODEX_APP_SERVER_NOTIFICATION_METHODS + ]) + }) + + it('classifies every pinned Claude stream-json frame kind', () => { + expect(Object.keys(PROVIDER_FRAME_CLASSIFICATIONS.claude)).toEqual([ + ...CLAUDE_STREAM_JSON_FRAME_KINDS + ]) + }) + + it('suppresses benign hook lifecycle and Codex rate-limit frames', () => { + expect(classifyProviderFrame('codex', 'notification:hook/started', {})).toBe( + 'suppressed-benign' + ) + expect(classifyProviderFrame('codex', 'notification:hook/completed', {})).toBe( + 'suppressed-benign' + ) + expect(classifyProviderFrame('codex', 'notification:account/rateLimits/updated', {})).toBe( + 'suppressed-benign' + ) + expect(classifyProviderFrame('claude', 'message:system:hook_started', {})).toBe( + 'suppressed-benign' + ) + }) + + it('promotes payload failures over a benign catalog classification', () => { + expect( + classifyProviderFrame('codex', 'notification:hook/completed', { + run: { status: 'failed' } + }) + ).toBe('error-surface') + expect( + classifyProviderFrame('claude', 'message:system:hook_response', { + outcome: 'error', + stderr: 'hook failed' + }) + ).toBe('error-surface') + }) + + it('keeps unknown future frames on the substantive bounded fallback path', () => { + expect(classifyProviderFrame('codex', 'notification:future/event', {})).toBe( + 'timeline-substantive' + ) + expect(classifyProviderFrame('claude', 'message:future_event', {})).toBe('timeline-substantive') + }) +}) 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 f284efda051..be41fda8522 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,4 +1,138 @@ -export type ProviderFrameDisposition = 'debug-only' | 'timeline' +import type { CodexAppServerNotificationMethod } from '../../codex/codex-app-server-notification-schema' +import type { ClaudeStreamJsonFrameKind } from './claude-stream-json-frame-schema' + +export type ProviderFrameClassification = + | 'timeline-substantive' + | 'status-chrome' + | 'suppressed-benign' + | 'error-surface' + +type ProviderFrameClassificationTable = { + codex: Record + claude: Record +} + +export const PROVIDER_FRAME_CLASSIFICATIONS = { + codex: { + error: 'error-surface', + 'thread/started': 'status-chrome', + 'thread/status/changed': 'status-chrome', + 'thread/archived': 'status-chrome', + 'thread/deleted': 'status-chrome', + 'thread/unarchived': 'status-chrome', + 'thread/closed': 'status-chrome', + 'skills/changed': 'status-chrome', + 'thread/name/updated': 'status-chrome', + 'thread/goal/updated': 'status-chrome', + 'thread/goal/cleared': 'status-chrome', + 'thread/environment/connected': 'status-chrome', + 'thread/environment/disconnected': 'status-chrome', + 'thread/settings/updated': 'status-chrome', + 'thread/tokenUsage/updated': 'status-chrome', + 'turn/started': 'status-chrome', + 'hook/started': 'suppressed-benign', + 'turn/completed': 'status-chrome', + 'hook/completed': 'suppressed-benign', + 'turn/diff/updated': 'timeline-substantive', + 'turn/plan/updated': 'timeline-substantive', + 'item/started': 'timeline-substantive', + 'item/autoApprovalReview/started': 'status-chrome', + 'item/autoApprovalReview/completed': 'status-chrome', + 'item/completed': 'timeline-substantive', + 'rawResponseItem/completed': 'suppressed-benign', + 'rawResponse/completed': 'suppressed-benign', + 'item/agentMessage/delta': 'timeline-substantive', + 'item/plan/delta': 'timeline-substantive', + 'command/exec/outputDelta': 'timeline-substantive', + 'process/outputDelta': 'timeline-substantive', + 'process/exited': 'timeline-substantive', + 'item/commandExecution/outputDelta': 'timeline-substantive', + 'item/commandExecution/terminalInteraction': 'timeline-substantive', + 'item/fileChange/outputDelta': 'timeline-substantive', + 'item/fileChange/patchUpdated': 'timeline-substantive', + 'serverRequest/resolved': 'suppressed-benign', + 'item/mcpToolCall/progress': 'status-chrome', + 'mcpServer/oauthLogin/completed': 'status-chrome', + 'mcpServer/startupStatus/updated': 'status-chrome', + 'account/updated': 'status-chrome', + 'account/rateLimits/updated': 'suppressed-benign', + 'app/list/updated': 'status-chrome', + 'remoteControl/status/changed': 'status-chrome', + 'externalAgentConfig/import/progress': 'status-chrome', + 'externalAgentConfig/import/completed': 'status-chrome', + 'fs/changed': 'suppressed-benign', + 'item/reasoning/summaryTextDelta': 'timeline-substantive', + 'item/reasoning/summaryPartAdded': 'timeline-substantive', + 'item/reasoning/textDelta': 'timeline-substantive', + 'thread/compacted': 'status-chrome', + 'model/rerouted': 'status-chrome', + 'model/verification': 'status-chrome', + 'turn/moderationMetadata': 'suppressed-benign', + 'model/safetyBuffering/updated': 'status-chrome', + warning: 'error-surface', + guardianWarning: 'error-surface', + deprecationNotice: 'error-surface', + configWarning: 'error-surface', + 'fuzzyFileSearch/sessionUpdated': 'suppressed-benign', + 'fuzzyFileSearch/sessionCompleted': 'suppressed-benign', + 'thread/realtime/started': 'status-chrome', + 'thread/realtime/itemAdded': 'timeline-substantive', + 'thread/realtime/transcript/delta': 'timeline-substantive', + 'thread/realtime/transcript/done': 'timeline-substantive', + 'thread/realtime/outputAudio/delta': 'timeline-substantive', + 'thread/realtime/sdp': 'suppressed-benign', + 'thread/realtime/error': 'error-surface', + 'thread/realtime/closed': 'status-chrome', + 'windows/worldWritableWarning': 'error-surface', + 'windowsSandbox/setupCompleted': 'status-chrome', + 'account/login/completed': 'status-chrome' + }, + claude: { + 'message:assistant': 'timeline-substantive', + 'message:user': 'timeline-substantive', + 'message:result': 'status-chrome', + 'message:system:init': 'status-chrome', + 'message:stream_event:message_start': 'status-chrome', + 'message:stream_event:message_delta': 'status-chrome', + 'message:stream_event:message_stop': 'status-chrome', + 'message:stream_event:content_block_start': 'status-chrome', + 'message:stream_event:content_block_delta': 'timeline-substantive', + 'message:stream_event:content_block_stop': 'status-chrome', + 'message:system:compact_boundary': 'status-chrome', + 'message:system:status': 'status-chrome', + 'message:system:api_retry': 'status-chrome', + 'message:system:control_request_progress': 'status-chrome', + 'message:system:model_refusal_fallback': 'status-chrome', + 'message:system:model_refusal_no_fallback': 'error-surface', + 'message:system:local_command_output': 'timeline-substantive', + 'message:system:hook_started': 'suppressed-benign', + 'message:system:hook_progress': 'suppressed-benign', + 'message:system:hook_response': 'suppressed-benign', + 'message:system:plugin_install': 'status-chrome', + 'message:tool_progress': 'status-chrome', + 'message:auth_status': 'status-chrome', + 'message:system:task_notification': 'status-chrome', + 'message:system:task_started': 'status-chrome', + 'message:system:task_updated': 'status-chrome', + 'message:system:task_progress': 'status-chrome', + 'message:system:background_tasks_changed': 'status-chrome', + 'message:system:thinking_tokens': 'status-chrome', + 'message:system:session_state_changed': 'status-chrome', + 'message:system:worker_shutting_down': 'status-chrome', + 'message:system:commands_changed': 'status-chrome', + 'message:system:notification': 'status-chrome', + 'message:system:files_persisted': 'status-chrome', + 'message:tool_use_summary': 'timeline-substantive', + 'message:system:memory_recall': 'timeline-substantive', + 'message:rate_limit_event': 'status-chrome', + 'message:system:elicitation_complete': 'status-chrome', + 'message:system:permission_denied': 'error-surface', + 'message:prompt_suggestion': 'status-chrome', + 'message:system:mirror_error': 'error-surface', + 'message:system:informational': 'timeline-substantive', + 'message:conversation_reset': 'status-chrome' + } +} as const satisfies ProviderFrameClassificationTable const ERROR_VARIANT_KEYS = new Set(['type', 'status', 'state', 'subtype', 'outcome']) const ERROR_VALUE_KEYS = new Set(['error', 'failureReason', 'failure_reason']) @@ -49,49 +183,35 @@ function notificationKind(kind: string): string { return kind.startsWith('notification:') ? kind.slice('notification:'.length) : kind } -function isCodexStatusFrame(kind: string): boolean { - const method = notificationKind(kind) - return ( - /^(?:thread\/(?:started|closed|archived|unarchived)|thread\/(?:name|status)\/(?:changed|updated))$/.test( - method - ) || - /^thread\/(?:tokenUsage\/updated|goal\/(?:updated|cleared))$/.test(method) || - method === 'mcpServer/startupStatus/updated' || - method === 'remoteControl/status/changed' - ) -} - -function isClaudeStatusFrame(kind: string): boolean { - return ( - kind === 'message:system:init' || - kind === 'message:system:status' || - kind === 'message:result' || - /^message:stream_event:(?:message_start|message_stop|content_block_start|content_block_stop)$/.test( - kind - ) - ) +function catalogClassification( + provider: string, + kind: string +): ProviderFrameClassification | undefined { + if (provider === 'codex') { + return PROVIDER_FRAME_CLASSIFICATIONS.codex[ + notificationKind(kind) as CodexAppServerNotificationMethod + ] + } + if (provider === 'claude') { + return PROVIDER_FRAME_CLASSIFICATIONS.claude[kind as ClaudeStreamJsonFrameKind] + } + return undefined } export function classifyProviderFrame( provider: string, kind: string, payload: unknown -): ProviderFrameDisposition { +): ProviderFrameClassification { if (hasProviderError(payload)) { - return 'timeline' + return 'error-surface' } - if (provider === 'codex' && isCodexStatusFrame(kind)) { - return 'debug-only' + if (provider === 'claude' && kind === 'message:result') { + const subtype = + typeof payload === 'object' && payload !== null + ? (payload as Record).subtype + : undefined + return subtype === 'success' ? 'status-chrome' : 'error-surface' } - if (provider === 'claude' && isClaudeStatusFrame(kind)) { - if (kind === 'message:result') { - const record = - typeof payload === 'object' && payload !== null - ? (payload as Record) - : null - return record?.subtype === 'success' ? 'debug-only' : 'timeline' - } - return 'debug-only' - } - return 'timeline' + return catalogClassification(provider, kind) ?? 'timeline-substantive' } 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 d3701122547..69a2ee4e87d 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 @@ -132,6 +132,21 @@ describe('unhandled provider frame journal fallback', () => { ).not.toBeNull() }) + it('surfaces a failed hook completion while suppressing successful hook lifecycle', () => { + const kind = 'notification:hook/completed' + + expect( + unhandledProviderFrameJournalItem('codex', kind, { + run: { id: 'hook-1', status: 'completed' } + }) + ).toBeNull() + expect( + unhandledProviderFrameJournalItem('codex', kind, { + run: { id: 'hook-1', status: 'failed' } + }) + ).not.toBeNull() + }) + it('keeps unknown substantive frames visible for both providers', () => { expect( unhandledProviderFrameJournalItem('codex', 'notification:future/event', {}) 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 0dc275bed50..819989d61c1 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 @@ -27,7 +27,8 @@ export function unhandledProviderFrameJournalItem( payload: unknown, limits: JournalPayloadLimits = DEFAULT_JOURNAL_PAYLOAD_LIMITS ): UnhandledProviderFrameJournalItem | null { - if (classifyProviderFrame(provider, kind, payload) === 'debug-only') { + const classification = classifyProviderFrame(provider, kind, payload) + if (classification === 'status-chrome' || classification === 'suppressed-benign') { return null } const serialized = serializeProviderPayload(payload)