diff --git a/src/main/native-chat/agent-session-wire/hook-failure-provider-frame.test.ts b/src/main/native-chat/agent-session-wire/hook-failure-provider-frame.test.ts new file mode 100644 index 00000000000..2e47b6b6c2d --- /dev/null +++ b/src/main/native-chat/agent-session-wire/hook-failure-provider-frame.test.ts @@ -0,0 +1,201 @@ +import { describe, expect, it } from 'vitest' +import { projectStructuredItemToNativeChat } from '../../../shared/structured-agent-session-projection' +import { unhandledProviderFrameJournalItem } from './unhandled-provider-frame' + +const CODEX_KIND = 'notification:hook/completed' +const CLAUDE_KIND = 'message:system:hook_response' + +// Shape captured from codex-cli 0.162.0 with a SessionStart command that exits 1. +const CODEX_FAILURE = { + threadId: 'thread-1', + turnId: 'turn-1', + run: { + id: 'session-start:0:hooks.json', + eventName: 'sessionStart', + handlerType: 'command', + executionMode: 'sync', + scope: 'thread', + sourcePath: 'hooks.json', + source: 'user', + status: 'failed', + entries: [{ kind: 'error', text: 'hook exited with code 1' }] + } +} + +describe('failed hook provider frames', () => { + it('projects the Codex hook name and nested failure with its original Details', () => { + const row = unhandledProviderFrameJournalItem('codex', CODEX_KIND, CODEX_FAILURE) + expect(row).toMatchObject({ + classification: 'error-surface', + body: { + kind: 'status', + tone: 'error', + text: 'Codex SessionStart hook failed: hook exited with code 1' + } + }) + expect(row?.body.providerFrame?.payload.head).toBe(JSON.stringify(CODEX_FAILURE)) + expect(row?.body.providerFrame?.payload.truncated).toBe(false) + if (!row) { + throw new Error('Expected failed hook row') + } + expect( + projectStructuredItemToNativeChat({ + itemId: 'failed-hook', + revision: 1, + sequence: 1, + observedAt: 1, + body: row.body + }) + ).toMatchObject({ + role: 'system', + blocks: [ + { type: 'text', tone: 'error', text: row.body.text, providerFrame: row.body.providerFrame } + ] + }) + }) + + it('preserves multiple error entries without displaying hook progress as the error', () => { + const payload = { + run: { + eventName: 'preToolUse', + status: 'failed', + statusMessage: 'Running hook', + entries: [ + { kind: 'info', text: 'Running hook' }, + { kind: 'error', text: ' First failure ' }, + null, + { kind: 'error', text: 1 }, + { kind: 'error', text: 'Second failure' } + ] + } + } + expect(unhandledProviderFrameJournalItem('codex', CODEX_KIND, payload)?.body.text).toBe( + 'Codex PreToolUse hook failed: First failure\nSecond failure' + ) + }) + + it.each([undefined, null, 'invalid', [null], [{ kind: 'error', text: ' ' }]])( + 'keeps a named error visible when Codex entries are missing or malformed: %j', + (entries) => { + expect( + unhandledProviderFrameJournalItem('codex', CODEX_KIND, { + run: { eventName: 'stop', status: 'failed', entries } + })?.body + ).toMatchObject({ text: 'Codex Stop hook failed.', tone: 'error' }) + } + ) + + it('uses a readable fallback when a failed hook carries no name', () => { + expect( + unhandledProviderFrameJournalItem('codex', CODEX_KIND, { + run: { status: 'failed', entries: [{ kind: 'error', text: 'Timed out' }] } + })?.body.text + ).toBe('Codex hook failed: Timed out') + expect( + unhandledProviderFrameJournalItem('codex', CODEX_KIND, { + run: { status: 'failed' }, + message: 'Hook could not start' + })?.body.text + ).toBe('Codex hook failed: Hook could not start') + expect( + unhandledProviderFrameJournalItem('claude', CLAUDE_KIND, { + outcome: 'error' + })?.body.text + ).toBe('Claude hook failed.') + }) + + it('shows Claude stderr and the exit code instead of the wire event name', () => { + expect( + unhandledProviderFrameJournalItem('claude', CLAUDE_KIND, { + hook_name: 'PostToolUse:Bash', + hook_event: 'PostToolUse', + outcome: 'error', + exit_code: 1, + stderr: ' Script not found\n', + output: 'Other output' + })?.body + ).toMatchObject({ + tone: 'error', + text: 'Claude PostToolUse:Bash hook failed (exit code 1): Script not found' + }) + }) + + it.each([ + { + stderr: ' ', + output: 'Hook output', + stdout: 'Other stdout', + exit_code: null, + expected: 'Hook output' + }, + { stderr: null, output: ' ', stdout: 'Hook stdout', exit_code: '1', expected: 'Hook stdout' }, + { + stderr: {}, + output: null, + message: 'Hook could not start', + exit_code: 0, + expected: 'Hook could not start' + } + ])('uses available Claude output when stderr is empty: $expected', ({ expected, ...fields }) => { + expect( + unhandledProviderFrameJournalItem('claude', CLAUDE_KIND, { + hook_event: 'SessionStart', + outcome: 'error', + ...fields + })?.body.text + ).toBe(`Claude SessionStart hook failed: ${expected}`) + }) + + it('retains a Claude exit code when there is no output', () => { + expect( + unhandledProviderFrameJournalItem('claude', CLAUDE_KIND, { + hook_name: 'SessionStart:startup', + outcome: 'error', + exit_code: 1 + })?.body.text + ).toBe('Claude SessionStart:startup hook failed (exit code 1).') + }) + + it('keeps successful hooks hidden even when they produce diagnostic output', () => { + expect( + unhandledProviderFrameJournalItem('codex', CODEX_KIND, { + run: { status: 'completed', entries: [{ kind: 'info', text: 'Hook completed' }] } + }) + ).toBeNull() + expect( + unhandledProviderFrameJournalItem('claude', CLAUDE_KIND, { + hook_name: 'SessionStart', + outcome: 'success', + exit_code: 0, + stderr: 'Diagnostic output' + }) + ).toBeNull() + }) + + it('bounds long hook error text and raw Details through the existing payload limits', () => { + const row = unhandledProviderFrameJournalItem( + 'codex', + CODEX_KIND, + { + run: { status: 'failed', entries: [{ kind: 'error', text: 'x'.repeat(1000) }] } + }, + { inlineHeadBytes: 64 } + ) + expect(row?.body.text).toContain('Codex hook failed: ') + expect(row?.body.text).toContain('[Orca: output truncated') + expect(row?.body.providerFrame?.payload).toMatchObject({ truncated: true }) + expect( + Buffer.byteLength(row?.body.providerFrame?.payload.head ?? '', 'utf8') + ).toBeLessThanOrEqual(64) + }) + + it('leaves other providers and unrelated frame kinds on their existing fallback', () => { + const payload = { outcome: 'error', hook_name: 'Other hook', stderr: 'Failure' } + expect( + unhandledProviderFrameJournalItem('future-provider', CLAUDE_KIND, payload)?.body.text + ).toBe(`future-provider · ${CLAUDE_KIND}`) + expect( + unhandledProviderFrameJournalItem('claude', 'message:future/event', payload)?.body.text + ).toBe('claude · message: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 f540de5a6c4..c0f9363a52d 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 @@ -7,6 +7,7 @@ import { type JournalPayloadLimits } 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 @@ -80,6 +81,47 @@ export function readableProviderFrameText(payload: unknown): string | null { return null } +function hookText(value: unknown): string | null { + return typeof value === 'string' ? value.trim() || null : null +} + +function hookFailureFrameText( + provider: string, + kind: string, + payload: unknown, + fallback: string | null +): string | null { + if (!isProviderRecord(payload)) { + return null + } + if (provider === 'codex' && kind === 'notification:hook/completed') { + const run = isProviderRecord(payload.run) ? payload.run : null + const event = hookText(run?.eventName) + const name = event ? ` ${event.charAt(0).toUpperCase()}${event.slice(1)}` : '' + const errors = Array.isArray(run?.entries) + ? run.entries.flatMap((entry: unknown) => { + const text = + isProviderRecord(entry) && entry.kind === 'error' ? hookText(entry.text) : null + return text ? [text] : [] + }) + : [] + const detail = errors.join('\n') || fallback + return `Codex${name} hook failed${detail ? `: ${detail}` : '.'}` + } + if (provider === 'claude' && kind === 'message:system:hook_response') { + const name = hookText(payload.hook_name) ?? hookText(payload.hook_event) + const code = payload.exit_code + const exit = + typeof code === 'number' && Number.isSafeInteger(code) && code !== 0 + ? ` (exit code ${code})` + : '' + const detail = + hookText(payload.stderr) ?? hookText(payload.output) ?? hookText(payload.stdout) ?? fallback + return `Claude${name ? ` ${name}` : ''} hook failed${exit}${detail ? `: ${detail}` : '.'}` + } + return null +} + /** Substantive adapter fallbacks become bounded journal rows; a chat draws one only with a sentence. */ export function unhandledProviderFrameJournalItem( provider: string, @@ -125,6 +167,9 @@ export function unhandledProviderFrameJournalItem( : undefined const tone = noticeTone ?? (classification === 'error-surface' ? 'error' : undefined) let message = readableProviderFrameText(payload) + if (classification === 'error-surface') { + message = hookFailureFrameText(provider, kind, payload, message) ?? message + } if ( provider === 'codex' && (method === 'configWarning' || method === 'deprecationNotice') &&