diff --git a/src/main/claude/claude-structured-content-parts.test.ts b/src/main/claude/claude-structured-content-parts.test.ts new file mode 100644 index 00000000000..d2142150937 --- /dev/null +++ b/src/main/claude/claude-structured-content-parts.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, it, vi } from 'vitest' +import type { + AgentJournalItemBody, + AgentJournalItemIdentity +} from '../../shared/agent-session-journal-types' +import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' +import { createClaudeJournalTranslator } from './claude-structured-journal-translation' + +function sinkState() { + const items: { identity: AgentJournalItemIdentity; body: AgentJournalItemBody }[] = [] + const sink: StructuredAgentSessionEventSink = { + appendItem: (identity, body) => items.push({ identity, body }), + appendTombstone: () => {}, + publish: vi.fn() + } + return { sink, items } +} + +function providerRows(items: { body: AgentJournalItemBody }[]) { + return items.flatMap((item) => + item.body.kind === 'status' && item.body.providerFrame + ? [{ kind: item.body.providerFrame.kind, text: item.body.text }] + : [] + ) +} + +function userMessageWith(part: unknown) { + return { + type: 'message' as const, + sessionId: 'orca-session', + startsTurn: true as const, + message: { + type: 'user', + uuid: 'user-1', + session_id: 'claude-session', + parent_tool_use_id: null, + isReplay: true, + message: { role: 'user', content: [{ type: 'text', text: 'look at this' }, part] } + } + } +} + +/** Exactly what claudeDispatchMessageContent sends for a local attachment. */ +const BASE64_IMAGE = { + type: 'image', + source: { type: 'base64', media_type: 'image/png', data: 'iVBORw0KGgoAAAANSUhEUg==' } +} + +describe('Claude message content parts', () => { + it('does not leak a wire kind for a locally attached image', () => { + const state = sinkState() + const translator = createClaudeJournalTranslator({ sink: state.sink }) + + translator.handle(userMessageWith(BASE64_IMAGE)) + + expect(providerRows(state.items)).toEqual([]) + }) + + it('still renders an image the CLI sends by url', () => { + const state = sinkState() + const translator = createClaudeJournalTranslator({ sink: state.sink }) + + translator.handle( + userMessageWith({ type: 'image', source: { type: 'url', url: 'https://x.test/a.png' } }) + ) + + expect(providerRows(state.items)).toEqual([]) + expect( + state.items.flatMap((item) => (item.body.kind === 'message' ? item.body.blocks : [])) + ).toContainEqual({ type: 'image-ref', url: 'https://x.test/a.png' }) + }) + + it('says what is true for a content part it cannot render, not the wire kind', () => { + const state = sinkState() + const translator = createClaudeJournalTranslator({ sink: state.sink }) + + translator.handle(userMessageWith({ type: 'some_future_part', payload: { a: 1 } })) + + const rows = providerRows(state.items) + expect(rows).toHaveLength(1) + // The kind stays on the row for debugging, behind the disclosure. + expect(rows[0].kind).toBe('message:user:content:some_future_part') + // ...but the visible text is a sentence, not the opcode. + expect(rows[0].text).not.toContain('message:user:content') + expect(rows[0].text.toLowerCase()).toContain('claude') + }) + + it('prefers a readable sentence the part carries over the placeholder', () => { + const state = sinkState() + const translator = createClaudeJournalTranslator({ sink: state.sink }) + + translator.handle( + userMessageWith({ type: 'some_future_part', message: 'the server refused the upload' }) + ) + + expect(providerRows(state.items)[0].text).toBe('the server refused the upload') + }) +}) diff --git a/src/main/claude/claude-structured-effort-reporting.test.ts b/src/main/claude/claude-structured-effort-reporting.test.ts new file mode 100644 index 00000000000..61ff1700bf6 --- /dev/null +++ b/src/main/claude/claude-structured-effort-reporting.test.ts @@ -0,0 +1,116 @@ +import { describe, expect, it } from 'vitest' +import { AgentSessionOptionRejectedError } from '../native-chat/agent-session-wire/structured-agent-session-option-error' +import { setClaudeStructuredOption } from './claude-structured-options' +import { readClaudeSettingsEffort } from './claude-structured-session-options' +import type { ClaudeSession } from './claude-structured-session-state' +import type { ClaudeStructuredSessionEvent } from './claude-structured-session-adapter' +import { acquired, fakeClaude } from './claude-structured-session-test-support' + +/** Verbatim from Claude Code 2.1.258's get_settings response. */ +const REAL_SETTINGS = { + applied: { model: 'claude-opus-5[1m]', effort: 'high', advisor: null, ultracode: false }, + effective: { model: 'claude-opus-5[1m]', effortLevel: 'high', env: {} }, + sources: {} +} + +describe('Claude effort reporting', () => { + it('reads the effort get_settings reports', () => { + expect(readClaudeSettingsEffort(REAL_SETTINGS)).toBe('high') + }) + + it.each([ + [ + 'the provider stops reporting it', + { applied: { effort: 'high' }, effective: {}, sources: {} } + ], + ['the payload carries no effective block', { applied: { effort: 'high' } }], + ['the request failed outright', null] + ])('reports no effort when %s', (_case, settings) => { + // Never defaulted: an effort nothing measured would be worse than a blank + // pill, and this is the assertion that goes red if the key is renamed. + expect(readClaudeSettingsEffort(settings)).toBeNull() + }) + + it('publishes the effort from get_settings, which system/init never carries', async () => { + const claude = fakeClaude({ settings: REAL_SETTINGS }) + const adapter = await acquired(claude) + + await expect(adapter.readOptions({ sessionId: 'session-1', fence: 7 })).resolves.toMatchObject({ + current: { effort: 'high' } + }) + }) + + it('leaves the effort unreported when the session never learns one', async () => { + const claude = fakeClaude({ settings: { applied: {}, effective: {}, sources: {} } }) + const adapter = await acquired(claude) + + const options = await adapter.readOptions({ sessionId: 'session-1', fence: 7 }) + expect(options.current.effort).toBeUndefined() + expect(options.current.model).toBeTruthy() + }) + + it('keeps the init fixture free of an effort the real frame never sends', async () => { + const events: ClaudeStructuredSessionEvent[] = [] + await acquired(fakeClaude(), {}, events) + const init = events.flatMap((event) => + event.type === 'message' && event.message.subtype === 'init' ? [event.message] : [] + ) + + expect(init).toHaveLength(1) + expect(init[0]).toHaveProperty('model') + // The regression that hid this defect: a fixture inventing `effortLevel` + // kept every gate green over a value that is always empty in production. + expect(Object.keys(init[0])).not.toContain('effortLevel') + }) +}) + +describe('Claude effort readback', () => { + function sessionWith(reported: string | null, calls: string[] = []) { + return { + session: { + options: new Map(), + optionMutationSequence: 0, + connection: { + applyFlagSettings: async (settings: { effortLevel?: string }) => { + // The measured behaviour: an unknown effort is accepted and ignored. + calls.push(`apply:${settings.effortLevel}`) + }, + getSettings: async () => { + calls.push('get_settings') + return reported === null + ? { applied: {}, effective: {}, sources: {} } + : { applied: { effort: reported }, effective: { effortLevel: reported }, sources: {} } + } + } + } as unknown as ClaudeSession, + calls + } + } + + it('refuses to record an effort the child did not adopt', async () => { + const { session, calls } = sessionWith('high') + + await expect( + setClaudeStructuredOption(session, { key: 'effort', value: 'bogus-effort-xyz' }, undefined) + ).rejects.toBeInstanceOf(AgentSessionOptionRejectedError) + expect(session.options.has('effort')).toBe(false) + expect(calls).toEqual(['apply:bogus-effort-xyz', 'get_settings']) + }) + + it('records an effort the child confirms', async () => { + const { session } = sessionWith('low') + + await expect( + setClaudeStructuredOption(session, { key: 'effort', value: 'low' }, undefined) + ).resolves.toEqual({ effort: 'low' }) + }) + + it('records the request when the readback is unavailable', async () => { + // No evidence of a refusal is not evidence of one; the apply itself succeeded. + const { session } = sessionWith(null) + + await expect( + setClaudeStructuredOption(session, { key: 'effort', value: 'low' }, undefined) + ).resolves.toEqual({ effort: 'low' }) + }) +}) diff --git a/src/main/claude/claude-structured-journal-translation.ts b/src/main/claude/claude-structured-journal-translation.ts index 28c7f1ff2f4..ffaad4da570 100644 --- a/src/main/claude/claude-structured-journal-translation.ts +++ b/src/main/claude/claude-structured-journal-translation.ts @@ -29,7 +29,9 @@ import { claudeQuestionItems } from './claude-structured-prompt-items' import type { ClaudePromptRegistry } from './claude-structured-prompt-replies' +import { readableProviderFrameText } from '../native-chat/agent-session-wire/unhandled-provider-frame' import { + CLAUDE_UNRENDERABLE_CONTENT_TEXT, claudeProviderFrameKind, claudeResultFailure, createClaudeProviderFrameFallback, @@ -171,7 +173,11 @@ export function createClaudeJournalTranslator( const unhandledContent = envelope.content.filter((part) => !isModeledClaudeContent(part)) for (const part of unhandledContent) { const partType = claudeText(claudeRecord(part)?.type) ?? 'unknown' - providerFallback.append(`message:${envelope.role}:content:${partType}`, part) + providerFallback.append( + `message:${envelope.role}:content:${partType}`, + part, + readableProviderFrameText(part) ?? CLAUDE_UNRENDERABLE_CONTENT_TEXT + ) changed = true } // An empty user frame is a replay with nothing to show, not an unknown kind. diff --git a/src/main/claude/claude-structured-options.ts b/src/main/claude/claude-structured-options.ts index b6fc07da11c..4d7db7d613a 100644 --- a/src/main/claude/claude-structured-options.ts +++ b/src/main/claude/claude-structured-options.ts @@ -4,6 +4,7 @@ import { AgentSessionOptionRejectedError, isAgentSessionOptionRejectedError } from '../native-chat/agent-session-wire/structured-agent-session-option-error' +import { readClaudeSettingsEffort } from './claude-structured-session-options' import type { ClaudeSession } from './claude-structured-session-state' const OPTION_ORDER = ['model', 'effort', 'permissionMode'] as const @@ -50,9 +51,25 @@ export async function setClaudeStructuredOption( } throw error } + // apply_flag_settings answers `success` for an effort it then ignores, so the + // absence of a throw proves nothing. Ask what the child actually holds. + const adopted = + input.key === 'effort' + ? await session.connection + .getSettings({ timeoutMs }) + .then(readClaudeSettingsEffort) + .catch(() => null) + : null if (mutationSequence !== session.optionMutationSequence) { return Object.fromEntries(session.options) } + // A readback that could not be taken is not evidence of a refusal; one that + // disagrees is, and recording it anyway would show an effort nothing adopted. + if (adopted !== null && adopted !== input.value) { + throw new AgentSessionOptionRejectedError( + `claude kept effort ${adopted} instead of ${input.value}` + ) + } session.options.set(input.key, input.value) return Object.fromEntries(session.options) } diff --git a/src/main/claude/claude-structured-provider-fallback.ts b/src/main/claude/claude-structured-provider-fallback.ts index afa9125e98f..ea461b173a2 100644 --- a/src/main/claude/claude-structured-provider-fallback.ts +++ b/src/main/claude/claude-structured-provider-fallback.ts @@ -58,6 +58,15 @@ export function claudeResultFailure( return { text: errors.length > 0 ? errors.join('\n') : null } } +/** + * What a message part that Orca cannot render says for itself. The kinds under + * `message::content:*` are synthesised from whatever `part.type` the CLI + * sends, so they can never be catalogued ahead of time; printing one is leaking + * wire vocabulary at a user who cannot act on it. The frame stays on the row's + * disclosure, so nothing is dropped and the next reader can still name it. + */ +export const CLAUDE_UNRENDERABLE_CONTENT_TEXT = 'Claude sent content Orca cannot display yet' + export function isModeledClaudeContent(value: unknown): boolean { const part = claudeRecord(value) if (!part) { @@ -68,7 +77,12 @@ export function isModeledClaudeContent(value: unknown): boolean { } if (part.type === 'image') { const source = claudeRecord(part.source) - return source?.type === 'url' && claudeText(source.url) !== null + if (source?.type === 'url') { + return claudeText(source.url) !== null + } + // A local attachment is replayed as the base64 (or file) source Orca itself + // sent, so it is content we recognise -- not an unknown part to surface. + return source?.type === 'base64' || source?.type === 'file' } if (part.type === 'tool_use') { return claudeText(part.id) !== null && claudeText(part.name) !== null diff --git a/src/main/claude/claude-structured-real-cli.test.ts b/src/main/claude/claude-structured-real-cli.test.ts index af799318cbb..b7269060ac1 100644 --- a/src/main/claude/claude-structured-real-cli.test.ts +++ b/src/main/claude/claude-structured-real-cli.test.ts @@ -127,6 +127,45 @@ describe.skipIf(!realClaudeAvailable)('Claude structured real CLI handshake', () 10_000 ) + // Unit tests can only pin the shape we read, which is exactly how the blank + // Effort pill survived every gate: the fixture invented an `effortLevel` on a + // frame the CLI does not send. This asserts both halves against the live + // binary — that get_settings reports the effort, and that init does not. + it.skipIf(!realClaudeAuthenticated)( + 'reports the current effort through get_settings and never on the init frame', + async () => { + const providerSessionId = randomUUID() + const claudeConfigDir = process.env.CLAUDE_CONFIG_DIR?.trim() || join(homedir(), '.claude') + const events: ClaudeStructuredSessionEvent[] = [] + const adapter = realAdapter(providerSessionId, claudeConfigDir, events) + + try { + await adapter.acquire({ + identity: identity(providerSessionId), + fence: 1, + spawnToken: 'real-cli-effort' + }) + const published = events.flatMap((event) => + event.type === 'message' ? [event.message] : [] + ) + const options = await adapter.readOptions({ sessionId: 'real-cli-handshake', fence: 1 }) + + expect(published.length).toBeGreaterThan(0) + // Not just the init frame: no frame the CLI publishes carries an effort + // at all. Goes red the day one does, which is when the simpler fix + // becomes available. Which frame proves the session varies by host, so + // this asserts over all of them rather than picking one. + expect(published.filter((frame) => 'effortLevel' in frame)).toEqual([]) + // Goes red if `effective.effortLevel` is renamed or dropped, which no + // fixture-backed test can see. + expect(options.current.effort).toEqual(expect.any(String)) + } finally { + await adapter.closeAll() + } + }, + 15_000 + ) + // Mobile native chat never reads the structured journal — it reads the CLI's own // transcript through native-chat/session-file-resolver.ts. So this resolves the way // transcript-read-cache.ts:104 does, with NO root override, and checks the answer diff --git a/src/main/claude/claude-structured-session-acquisition.ts b/src/main/claude/claude-structured-session-acquisition.ts index e15a866fa41..955b55c7fc6 100644 --- a/src/main/claude/claude-structured-session-acquisition.ts +++ b/src/main/claude/claude-structured-session-acquisition.ts @@ -30,6 +30,7 @@ import { } from './claude-structured-options' import { ClaudePromptRegistry } from './claude-structured-prompt-replies' import { createClaudeSessionJournalTranslator } from './claude-structured-journal-translation' +import { readClaudeSettingsEffort } from './claude-structured-session-options' import { createClaudeSessionPublication } from './claude-structured-session-publication' import { cancelClaudeAcquisitionAttempt, @@ -243,6 +244,7 @@ export async function acquireClaudeSession({ claudeConfigDir: launch.claudeConfigDir, leafUuid: observedLeafUuid, fence: input.fence, + effort: readClaudeSettingsEffort(settings), resumed: launch.resumed, prompts, translator, diff --git a/src/main/claude/claude-structured-session-adapter.test.ts b/src/main/claude/claude-structured-session-adapter.test.ts index 678e7f59fa4..693b08f5e47 100644 --- a/src/main/claude/claude-structured-session-adapter.test.ts +++ b/src/main/claude/claude-structured-session-adapter.test.ts @@ -82,9 +82,11 @@ describe('ClaudeStructuredSessionAdapter.acquire', () => { options: { model: 'opus', effort: 'high' } }) - expect(claude.connections[0].calls.slice(-2)).toEqual([ + expect(claude.connections[0].calls.slice(-3)).toEqual([ { subtype: 'set_model', params: { model: 'opus' } }, - { subtype: 'apply_flag_settings', params: { settings: { effortLevel: 'high' } } } + { subtype: 'apply_flag_settings', params: { settings: { effortLevel: 'high' } } }, + // The effort is only recorded once the child reports having adopted it. + { subtype: 'get_settings' } ]) await expect(adapter.readOptions({ sessionId: 'session-1', fence: 7 })).resolves.toMatchObject({ current: { model: 'opus', effort: 'high' } diff --git a/src/main/claude/claude-structured-session-options.ts b/src/main/claude/claude-structured-session-options.ts index d61db72fbc5..7b6cad63d74 100644 --- a/src/main/claude/claude-structured-session-options.ts +++ b/src/main/claude/claude-structured-session-options.ts @@ -19,6 +19,16 @@ function text(value: unknown): string | null { return typeof value === 'string' && value.trim() ? value : null } +/** + * The session's current effort, which only `get_settings` reports: the + * `system/init` frame carries `model` but has never carried an effort of any + * kind. Null when the provider stops reporting it, so the pill goes empty + * rather than showing an effort nothing measured. + */ +export function readClaudeSettingsEffort(settings: unknown): string | null { + return text(record(record(settings)?.effective)?.effortLevel) +} + function effortLabel(value: string): string { return value === 'xhigh' ? 'Extra high' : `${value.charAt(0).toUpperCase()}${value.slice(1)}` } diff --git a/src/main/claude/claude-structured-session-publication.ts b/src/main/claude/claude-structured-session-publication.ts index 652d897128b..3dec735d3aa 100644 --- a/src/main/claude/claude-structured-session-publication.ts +++ b/src/main/claude/claude-structured-session-publication.ts @@ -21,9 +21,11 @@ export function createClaudeSessionPublication(input: { observedAt: number options?: ReadonlyMap capabilities: readonly string[] + /** Read from `get_settings`; `system/init` never reports an effort. */ + effort: string | null }): { acquisition: AgentSessionAcquisition; session: ClaudeSession } { const model = readClaudeFrameString(input.init.message, 'model') - const effort = readClaudeFrameString(input.init.message, 'effortLevel') + const effort = input.effort return { acquisition: { process: input.process, diff --git a/src/main/claude/claude-structured-session-test-support.ts b/src/main/claude/claude-structured-session-test-support.ts index 2cb8e15d882..6b0768b5134 100644 --- a/src/main/claude/claude-structured-session-test-support.ts +++ b/src/main/claude/claude-structured-session-test-support.ts @@ -50,7 +50,6 @@ export function fakeClaude( initSessionId?: string initUuid?: string initModel?: string - initEffort?: string initProof?: 'init' | 'session-start' | 'none' initAccount?: unknown exitBeforeInit?: string @@ -97,13 +96,15 @@ export function fakeClaude( uuid: options.initUuid ?? 'init-uuid' }) } else if (options.initProof !== 'none') { + // Keys mirror the real system/init frame, which carries `model` but no + // effort of any kind: the current effort only comes back from + // get_settings. Never add a field the CLI does not send. handlers.onMessage?.({ type: 'system', subtype: 'init', session_id: options.initSessionId ?? PROVIDER_SESSION_ID, uuid: options.initUuid ?? 'init-uuid', model: options.initModel ?? 'claude-sonnet-5', - effortLevel: options.initEffort ?? 'high', apiKeySource: 'none', ...(options.capabilities ? { capabilities: options.capabilities } : {}) }) @@ -115,7 +116,15 @@ export function fakeClaude( }, getSettings: async () => { connection.calls.push({ subtype: 'get_settings' }) - return options.settings ?? { env: {} } + // Shape measured from Claude Code 2.1.258: {applied, effective, sources}, + // and the only place the session's current effort is reported. + return ( + options.settings ?? { + applied: { model: 'claude-sonnet-5', effort: 'high', advisor: null, ultracode: false }, + effective: { model: 'claude-sonnet-5', effortLevel: 'high', env: {} }, + sources: {} + } + ) }, supportedModels: async () => { connection.calls.push({ subtype: 'list_models' }) 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 b4651cfc952..336d90e724e 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 @@ -55,7 +55,8 @@ function directReadableMessage(payload: unknown): string | null { return null } -function readableMessage(payload: unknown): string | null { +/** The provider's own sentence for a frame, when it carries one. */ +export function readableProviderFrameText(payload: unknown): string | null { const direct = directReadableMessage(payload) if (direct || typeof payload !== 'object' || payload === null || Array.isArray(payload)) { return direct @@ -90,7 +91,7 @@ export function unhandledProviderFrameJournalItem( // Why: the opcode alone ("codex · notification:warning") tells the user nothing // and reads as protocol noise. Lead with the provider's own sentence when it has // one; the raw frame stays behind the row's disclosure either way. - const message = readableMessage(payload) + const message = readableProviderFrameText(payload) const display = message ? boundInlineText(message, limits) : null return { body: {