From 8782b8a110f1425a7bdc083d587ffce590f34ad7 Mon Sep 17 00:00:00 2001 From: weishu Date: Sun, 12 Jul 2026 14:33:02 +0800 Subject: [PATCH] fix(codex): align local transcript message projection Use semantic events as the visible text source and preserve completed plans as ordered plan proposal cards. --- cli/src/codex/codexLocalLauncher.test.ts | 122 ++++++++++++++++-- cli/src/codex/codexLocalLauncher.ts | 54 +++++++- .../codex/utils/codexEventConverter.test.ts | 92 +++++++++---- cli/src/codex/utils/codexEventConverter.ts | 74 +++++------ .../utils/codexTranscriptLocator.test.ts | 8 +- web/src/chat/reducer.test.ts | 45 +++++++ 6 files changed, 313 insertions(+), 82 deletions(-) diff --git a/cli/src/codex/codexLocalLauncher.test.ts b/cli/src/codex/codexLocalLauncher.test.ts index 2594ca5a..345b6968 100644 --- a/cli/src/codex/codexLocalLauncher.test.ts +++ b/cli/src/codex/codexLocalLauncher.test.ts @@ -459,7 +459,7 @@ describe('codexLocalLauncher', () => { }); }); - it('replays existing response_item chat messages when importing a Codex thread into a new Hapi session', async () => { + it('replays semantic chat events once and keeps a same-turn preface before its plan', async () => { const transcriptPath = join(tempDir, 'codex-import-response-item-transcript.jsonl'); const { session, userMessages, agentMessages, getUserActivityCount } = createSessionStub('default', undefined, '/tmp/worktree', null, true); let releaseRunBarrier: (() => void) | undefined; @@ -476,7 +476,23 @@ describe('codexLocalLauncher', () => { payload: { type: 'message', role: 'user', - content: [{ type: 'input_text', text: 'old response_item user message' }] + content: [{ type: 'input_text', text: 'visible user message' }] + } + }), + JSON.stringify({ + type: 'event_msg', + payload: { type: 'user_message', message: 'visible user message' } + }), + JSON.stringify({ + type: 'event_msg', + payload: { + type: 'item_completed', + turn_id: 'turn-with-preface', + item: { + type: 'Plan', + id: 'plan-1', + text: '## Proposed plan\n\n1. Inspect\n2. Implement' + } } }), JSON.stringify({ @@ -484,15 +500,31 @@ describe('codexLocalLauncher', () => { payload: { type: 'message', role: 'assistant', - content: [{ type: 'output_text', text: 'old response_item assistant message' }] + content: [{ + type: 'output_text', + text: 'visible assistant preface\n\n## Proposed plan\n\n1. Inspect\n2. Implement' + }], + internal_chat_message_metadata_passthrough: { turn_id: 'turn-with-preface' } } }), + JSON.stringify({ + type: 'event_msg', + payload: { + type: 'agent_message', + message: 'visible assistant preface', + phase: 'final_answer' + } + }), + JSON.stringify({ + type: 'event_msg', + payload: { type: 'task_complete', turn_id: 'turn-with-preface' } + }), JSON.stringify({ type: 'response_item', payload: { type: 'message', role: 'user', - content: [{ type: 'input_image', image_url: 'data:image/png;base64,abc' }] + content: [{ type: 'input_text', text: 'hidden context' }] } }) ].join('\n') + '\n' @@ -511,13 +543,87 @@ describe('codexLocalLauncher', () => { } await launcherPromise; - expect(userMessages).toContain('old response_item user message'); - expect(getUserActivityCount()).toBe(1); - expect(agentMessages).toContainEqual({ + expect(userMessages).toEqual(['visible user message']); + expect(getUserActivityCount()).toBe(0); + expect(agentMessages).toEqual([{ type: 'message', - message: 'old response_item assistant message', + message: 'visible assistant preface', id: expect.any(String) + }, { + type: 'tool-call', + name: 'ExitPlanMode', + callId: 'codex-proposed-plan:plan-1', + input: { plan: '## Proposed plan\n\n1. Inspect\n2. Implement' }, + id: 'plan-1' + }, { + type: 'tool-call-result', + callId: 'codex-proposed-plan:plan-1', + output: null, + id: 'plan-1:result' + }]); + }); + + it('replays a plan-only turn when the turn completes', async () => { + const transcriptPath = join(tempDir, 'codex-import-plan-only-transcript.jsonl'); + const { session, agentMessages } = createSessionStub('default', undefined, '/tmp/worktree', null, true); + let releaseRunBarrier: (() => void) | undefined; + harness.runBarrier = new Promise((resolve) => { + releaseRunBarrier = resolve; }); + + await writeFile( + transcriptPath, + [ + JSON.stringify({ type: 'session_meta', payload: { id: 'codex-thread-plan-only' } }), + JSON.stringify({ + type: 'event_msg', + payload: { + type: 'item_completed', + turn_id: 'turn-plan-only', + item: { type: 'Plan', id: 'plan-only', text: '## Plan only' } + } + }), + JSON.stringify({ + type: 'response_item', + payload: { + type: 'message', + role: 'assistant', + content: [{ type: 'output_text', text: '## Plan only' }], + internal_chat_message_metadata_passthrough: { turn_id: 'turn-plan-only' } + } + }), + JSON.stringify({ + type: 'event_msg', + payload: { type: 'task_complete', turn_id: 'turn-plan-only' } + }) + ].join('\n') + '\n' + ); + + const launcherPromise = codexLocalLauncher(session as never); + await wait(50); + + harness.sessionHookHandlers[0]?.('codex-thread-plan-only', { + transcript_path: transcriptPath + }); + await wait(300); + + if (releaseRunBarrier) { + releaseRunBarrier(); + } + await launcherPromise; + + expect(agentMessages).toEqual([{ + type: 'tool-call', + name: 'ExitPlanMode', + callId: 'codex-proposed-plan:plan-only', + input: { plan: '## Plan only' }, + id: 'plan-only' + }, { + type: 'tool-call-result', + callId: 'codex-proposed-plan:plan-only', + output: null, + id: 'plan-only:result' + }]); }); it('does not let a later non-clear hook replace the primary session', async () => { diff --git a/cli/src/codex/codexLocalLauncher.ts b/cli/src/codex/codexLocalLauncher.ts index 7df3e955..4589a259 100644 --- a/cli/src/codex/codexLocalLauncher.ts +++ b/cli/src/codex/codexLocalLauncher.ts @@ -5,13 +5,15 @@ import { codexLocal } from './codexLocal'; import type { ReasoningEffort } from './appServerTypes'; import { CodexSession } from './session'; import { createCodexSessionScanner, type CodexSessionScanner } from './utils/codexSessionScanner'; -import { convertCodexEvent } from './utils/codexEventConverter'; +import { convertCodexEvent, type CodexMessage } from './utils/codexEventConverter'; import { buildHapiMcpBridge } from './utils/buildHapiMcpBridge'; import { parseCodexCliOverrides, stripCodexCliOverrides } from './utils/codexCliOverrides'; import { buildCodexPermissionModeCliArgs } from './utils/permissionModeConfig'; import { BaseLocalLauncher } from '@/modules/common/launcher/BaseLocalLauncher'; import { createCodexTranscriptLocator, type CodexTranscriptLocator } from './utils/codexTranscriptLocator'; +type ProposedPlanMessage = Extract; + export async function codexLocalLauncher(session: CodexSession): Promise<'switch' | 'exit'> { const resumeSessionId = session.sessionId; let primarySessionId = resumeSessionId; @@ -21,6 +23,8 @@ export async function codexLocalLauncher(session: CodexSession): Promise<'switch let shuttingDown = false; let pendingScannerSetup: Promise | null = null; let transcriptLocator: CodexTranscriptLocator | null = null; + let scannerTranscriptPath: string | null = null; + const pendingPlansByTurnId = new Map(); const permissionMode = session.getPermissionMode(); const managedPermissionMode = permissionMode === 'read-only' || permissionMode === 'safe-yolo' || permissionMode === 'yolo' ? permissionMode @@ -61,6 +65,38 @@ export async function codexLocalLauncher(session: CodexSession): Promise<'switch return primarySessionId === null || primarySessionId === sessionId; }; + const sendProposedPlan = (message: ProposedPlanMessage): void => { + const callId = `codex-proposed-plan:${message.id}`; + session.sendAgentMessage({ + type: 'tool-call', + name: 'ExitPlanMode', + callId, + input: { plan: message.plan }, + id: message.id + }); + session.sendAgentMessage({ + type: 'tool-call-result', + callId, + output: null, + id: `${message.id}:result` + }); + }; + + const flushPendingPlan = (turnId: string): void => { + const message = pendingPlansByTurnId.get(turnId); + if (!message) { + return; + } + pendingPlansByTurnId.delete(turnId); + sendProposedPlan(message); + }; + + const flushAllPendingPlans = (): void => { + for (const turnId of pendingPlansByTurnId.keys()) { + flushPendingPlan(turnId); + } + }; + const bindPrimarySession = (sessionId: string, transcriptPath: string, allowSwitch = false): void => { if (primarySessionId && primarySessionId !== sessionId && !allowSwitch) { logger.debug(`[codex-local]: Ignoring non-primary SessionStart hook ${sessionId}; primary is ${primarySessionId}`); @@ -83,7 +119,11 @@ export async function codexLocalLauncher(session: CodexSession): Promise<'switch return; } if (scanner) { + if (scannerTranscriptPath !== transcriptPath) { + flushAllPendingPlans(); + } await scanner.setTranscriptPath(transcriptPath); + scannerTranscriptPath = transcriptPath; return; } const createdScanner = await createCodexSessionScanner({ @@ -112,7 +152,15 @@ export async function codexLocalLauncher(session: CodexSession): Promise<'switch session.notifyUserActivity(); } if (converted?.message) { - session.sendAgentMessage(converted.message); + if (converted.message.type === 'proposed_plan') { + // Codex may complete the Plan item before emitting its final text preface. + pendingPlansByTurnId.set(converted.message.turnId, converted.message); + } else { + session.sendAgentMessage(converted.message); + } + } + if (converted?.finishedTurnId) { + flushPendingPlan(converted.finishedTurnId); } } }); @@ -121,6 +169,7 @@ export async function codexLocalLauncher(session: CodexSession): Promise<'switch return; } scanner = createdScanner; + scannerTranscriptPath = transcriptPath; }; const handleTranscriptPath = (transcriptPath: string): Promise => { @@ -248,6 +297,7 @@ export async function codexLocalLauncher(session: CodexSession): Promise<'switch if (activeScanner) { await activeScanner.cleanup(); } + flushAllPendingPlans(); happyServer.stop(); if (!hookReady) { logger.debug('[codex-local]: SessionStart hook did not provide transcript path before shutdown'); diff --git a/cli/src/codex/utils/codexEventConverter.test.ts b/cli/src/codex/utils/codexEventConverter.test.ts index adbc3bd3..d7cf1008 100644 --- a/cli/src/codex/utils/codexEventConverter.test.ts +++ b/cli/src/codex/utils/codexEventConverter.test.ts @@ -32,49 +32,93 @@ describe('convertCodexEvent', () => { expect(result?.userMessage).toBe('hello user'); }); - it('converts response_item user messages', () => { + it('converts completed plan items into proposed plan messages', () => { const result = convertCodexEvent({ + type: 'event_msg', + payload: { + type: 'item_completed', + turn_id: 'turn-1', + item: { type: 'Plan', id: 'plan-1', text: '## Plan\n\n1. Inspect\n2. Implement' } + } + }); + + expect(result?.message).toMatchObject({ + type: 'proposed_plan', + plan: '## Plan\n\n1. Inspect\n2. Implement', + id: 'plan-1', + turnId: 'turn-1' + }); + }); + + it('ignores empty completed plan items', () => { + const result = convertCodexEvent({ + type: 'event_msg', + payload: { + type: 'item_completed', + turn_id: 'turn-1', + item: { type: 'Plan', id: 'plan-1', text: ' ' } + } + }); + + expect(result).toBeNull(); + }); + + it('ignores completed plan items without a turn id', () => { + const result = convertCodexEvent({ + type: 'event_msg', + payload: { + type: 'item_completed', + item: { type: 'Plan', id: 'plan-1', text: '## Plan' } + } + }); + + expect(result).toBeNull(); + }); + + it.each(['task_complete', 'turn_aborted', 'task_failed'])('converts %s into a turn boundary', (type) => { + const result = convertCodexEvent({ + type: 'event_msg', + payload: { type, turn_id: 'turn-1' } + }); + + expect(result).toEqual({ finishedTurnId: 'turn-1' }); + }); + + it.each([ + ['user text', { type: 'response_item', payload: { type: 'message', role: 'user', content: [{ type: 'input_text', text: 'hello from response_item user' }] } - }); - - expect(result).toEqual({ - userMessage: 'hello from response_item user', - userActivity: true - }); - }); - - it('marks image-only response_item messages as user activity', () => { - const result = convertCodexEvent({ + }], + ['user image', { type: 'response_item', payload: { type: 'message', role: 'user', content: [{ type: 'input_image', image_url: 'data:image/png;base64,abc' }] } - }); - - expect(result).toEqual({ userActivity: true }); - }); - - it('converts response_item assistant messages', () => { - const result = convertCodexEvent({ + }], + ['assistant text', { type: 'response_item', payload: { type: 'message', role: 'assistant', content: [{ type: 'output_text', text: 'hello from response_item assistant' }] } - }); - - expect(result?.message).toMatchObject({ - type: 'message', - message: 'hello from response_item assistant' - }); + }], + ['injected user context', { + type: 'response_item', + payload: { + type: 'message', + role: 'user', + content: [{ type: 'input_text', text: '# AGENTS.md\nhidden context' }] + } + }] + ])('ignores %s response_item messages', (_name, event) => { + expect(convertCodexEvent(event)).toBeNull(); }); it('converts reasoning events', () => { diff --git a/cli/src/codex/utils/codexEventConverter.ts b/cli/src/codex/utils/codexEventConverter.ts index 39304805..11024ccc 100644 --- a/cli/src/codex/utils/codexEventConverter.ts +++ b/cli/src/codex/utils/codexEventConverter.ts @@ -14,6 +14,11 @@ export type CodexMessage = { type: 'message'; message: string; id: string; +} | { + type: 'proposed_plan'; + plan: string; + id: string; + turnId: string; } | { type: 'reasoning'; message: string; @@ -43,6 +48,7 @@ export type CodexConversionResult = { message?: CodexMessage; userMessage?: string; userActivity?: true; + finishedTurnId?: string; }; function asRecord(value: unknown): Record | null { @@ -56,30 +62,6 @@ function asString(value: unknown): string | null { return typeof value === 'string' && value.length > 0 ? value : null; } -function extractCodexText(value: unknown): string { - if (typeof value === 'string') { - return value.trim(); - } - if (Array.isArray(value)) { - return value - .map((item) => { - const record = asRecord(item); - if (record?.type === 'input_text' && typeof record.text === 'string') return record.text; - if (record?.type === 'output_text' && typeof record.text === 'string') return record.text; - if (record?.type === 'text' && typeof record.text === 'string') return record.text; - return null; - }) - .filter((part): part is string => Boolean(part)) - .join(' ') - .trim(); - } - const record = asRecord(value); - if (record?.type === 'input_text' && typeof record.text === 'string') return record.text.trim(); - if (record?.type === 'output_text' && typeof record.text === 'string') return record.text.trim(); - if (record?.type === 'text' && typeof record.text === 'string') return record.text.trim(); - return ''; -} - function parseArguments(value: unknown): unknown { if (typeof value !== 'string') { return value; @@ -167,6 +149,29 @@ export function convertCodexEvent(rawEvent: unknown): CodexConversionResult | nu }; } + if (eventType === 'item_completed') { + const item = asRecord(payloadRecord.item); + const itemType = asString(item?.type)?.toLowerCase(); + const message = itemType === 'plan' ? asString(item?.text) : null; + const turnId = asString(payloadRecord.turn_id); + if (!message || message.trim().length === 0 || !turnId) { + return null; + } + return { + message: { + type: 'proposed_plan', + plan: message, + id: asString(item?.id) ?? randomUUID(), + turnId + } + }; + } + + if (eventType === 'task_complete' || eventType === 'turn_aborted' || eventType === 'task_failed') { + const turnId = asString(payloadRecord.turn_id); + return turnId ? { finishedTurnId: turnId } : null; + } + if (eventType === 'agent_reasoning') { const message = asString(payloadRecord.text) ?? asString(payloadRecord.message); if (!message) { @@ -218,26 +223,7 @@ export function convertCodexEvent(rawEvent: unknown): CodexConversionResult | nu } if (itemType === 'message') { - const role = asString(payloadRecord.role); - const text = extractCodexText(payloadRecord.content); - if (role === 'user') { - return { - userActivity: true, - ...(text ? { userMessage: text } : {}) - }; - } - if (role === 'assistant') { - if (!text) { - return null; - } - return { - message: { - type: 'message', - message: text, - id: randomUUID() - } - }; - } + // Response messages are model conversation state; event_msg carries visible chat. return null; } diff --git a/cli/src/codex/utils/codexTranscriptLocator.test.ts b/cli/src/codex/utils/codexTranscriptLocator.test.ts index b633f51b..0a75bc14 100644 --- a/cli/src/codex/utils/codexTranscriptLocator.test.ts +++ b/cli/src/codex/utils/codexTranscriptLocator.test.ts @@ -89,11 +89,11 @@ describe('codexTranscriptLocator', () => { await appendFile(transcriptPath, `${JSON.stringify({ timestamp: new Date().toISOString(), - type: 'response_item', + type: 'event_msg', payload: { - type: 'message', - role: 'user', - content: [{ type: 'input_image', image_url: 'data:image/png;base64,abc' }] + type: 'user_message', + message: '', + images: ['data:image/png;base64,abc'] } })}\n`); await wait(100); diff --git a/web/src/chat/reducer.test.ts b/web/src/chat/reducer.test.ts index 6000d70c..93e1d3db 100644 --- a/web/src/chat/reducer.test.ts +++ b/web/src/chat/reducer.test.ts @@ -80,6 +80,51 @@ function decryptedMessage(id: string, content: unknown, createdAt: number): Decr } describe('reduceChatBlocks', () => { + it('renders Codex proposed plan tool messages as a completed plan card', () => { + const plan = '# Plan\n\n1. Inspect\n2. Implement' + const messages = [ + decryptedMessage('plan-call', { + role: 'agent', + content: { + type: 'codex', + data: { + type: 'tool-call', + name: 'ExitPlanMode', + callId: 'codex-proposed-plan:plan-1', + input: { plan }, + id: 'plan-1' + } + } + }, 1), + decryptedMessage('plan-result', { + role: 'agent', + content: { + type: 'codex', + data: { + type: 'tool-call-result', + callId: 'codex-proposed-plan:plan-1', + output: null, + id: 'plan-1:result' + } + } + }, 2) + ].map(message => normalizeDecryptedMessage(message)) + .filter((message): message is NormalizedMessage => message !== null) + + const reduced = reduceChatBlocks(messages, null) + + expect(reduced.blocks).toContainEqual(expect.objectContaining({ + kind: 'tool-call', + id: 'codex-proposed-plan:plan-1', + tool: expect.objectContaining({ + name: 'ExitPlanMode', + state: 'completed', + input: { plan }, + result: null + }) + })) + }) + it('ignores child agent usage when calculating parent latest usage', () => { const messages: NormalizedMessage[] = [ {