fix(cli): harden ACP/Codex event handling (#211)

Co-authored-by: Lihengwannafly <Lihengwannafly@users.noreply.github.com>
This commit is contained in:
Lihengwannafly
2026-02-25 16:57:28 +08:00
committed by GitHub
co-authored by Lihengwannafly
parent 77cfa715f9
commit 10f7e1404e
17 changed files with 1529 additions and 67 deletions
+21 -1
View File
@@ -15,11 +15,13 @@ describe('appServerConfig', () => {
expect(params.sandbox).toBe('danger-full-access');
expect(params.approvalPolicy).toBe('never');
expect(params.baseInstructions).toBe(codexSystemPrompt);
expect(params.developerInstructions).toBe(codexSystemPrompt);
expect(params.config).toEqual({
'mcp_servers.hapi': {
command: 'node',
args: ['mcp']
}
},
developer_instructions: codexSystemPrompt
});
});
@@ -34,6 +36,24 @@ describe('appServerConfig', () => {
expect(params.approvalPolicy).toBe('on-failure');
});
it('concatenates custom developer instructions after base instructions', () => {
const params = buildThreadStartParams({
mode: { permissionMode: 'default' },
mcpServers,
developerInstructions: 'Only respond in Chinese.'
});
expect(params.baseInstructions).toBe(codexSystemPrompt);
expect(params.developerInstructions).toBe(`${codexSystemPrompt}\n\nOnly respond in Chinese.`);
expect(params.config).toEqual({
'mcp_servers.hapi': {
command: 'node',
args: ['mcp']
},
developer_instructions: `${codexSystemPrompt}\n\nOnly respond in Chinese.`
});
});
it('builds turn params with mode defaults', () => {
const params = buildTurnStartParams({
threadId: 'thread-1',
+9 -2
View File
@@ -88,13 +88,20 @@ export function buildThreadStartParams(args: {
const config = buildMcpServerConfig(args.mcpServers);
const baseInstructions = args.baseInstructions ?? codexSystemPrompt;
const resolvedDeveloperInstructions = args.developerInstructions
? `${baseInstructions}\n\n${args.developerInstructions}`
: baseInstructions;
const configWithInstructions = {
...config,
developer_instructions: resolvedDeveloperInstructions
};
const params: ThreadStartParams = {
approvalPolicy: resolvedApprovalPolicy,
sandbox: resolvedSandbox,
baseInstructions,
...(args.developerInstructions ? { developerInstructions: args.developerInstructions } : {}),
...(Object.keys(config).length > 0 ? { config } : {})
developerInstructions: resolvedDeveloperInstructions,
...(Object.keys(configWithInstructions).length > 0 ? { config: configWithInstructions } : {})
};
if (args.mode.model) {
@@ -44,6 +44,21 @@ describe('AppServerEventConverter', () => {
expect(completed).toEqual([{ type: 'agent_message', message: 'Hello world' }]);
});
it('deduplicates repeated agent message completions for the same item', () => {
const converter = new AppServerEventConverter();
converter.handleNotification('item/agentMessage/delta', { itemId: 'msg-1', delta: 'Hello' });
const first = converter.handleNotification('item/completed', {
item: { id: 'msg-1', type: 'AgentMessage' }
});
const second = converter.handleNotification('item/completed', {
item: { id: 'msg-1', type: 'agentMessage' }
});
expect(first).toEqual([{ type: 'agent_message', message: 'Hello' }]);
expect(second).toEqual([]);
});
it('maps command execution items and output deltas', () => {
const converter = new AppServerEventConverter();
@@ -77,10 +92,172 @@ describe('AppServerEventConverter', () => {
expect(events).toEqual([{ type: 'agent_reasoning_delta', delta: 'step' }]);
});
it('dedupes duplicate reasoning deltas', () => {
const converter = new AppServerEventConverter();
expect(converter.handleNotification('item/reasoning/textDelta', { itemId: 'r1', delta: 'Hello ' }))
.toEqual([{ type: 'agent_reasoning_delta', delta: 'Hello ' }]);
expect(converter.handleNotification('item/reasoning/textDelta', { itemId: 'r1', delta: 'Hello ' }))
.toEqual([]);
converter.handleNotification('item/reasoning/textDelta', { itemId: 'r1', delta: 'world' });
const completed = converter.handleNotification('item/completed', {
item: { id: 'r1', type: 'reasoning' }
});
expect(completed).toEqual([{ type: 'agent_reasoning', text: 'Hello world' }]);
});
it('maps reasoning summary deltas', () => {
const converter = new AppServerEventConverter();
const events = converter.handleNotification('item/reasoning/summaryTextDelta', { itemId: 'r1', delta: 'step' });
expect(events).toEqual([{ type: 'agent_reasoning_delta', delta: 'step' }]);
});
it('deduplicates repeated reasoning completions for the same item', () => {
const converter = new AppServerEventConverter();
const first = converter.handleNotification('item/completed', {
item: { id: 'r1', type: 'Reasoning', summary_text: ['Plan'] }
});
const second = converter.handleNotification('item/completed', {
item: { id: 'r1', type: 'reasoning', summary_text: ['Plan'] }
});
expect(first).toEqual([{ type: 'agent_reasoning', text: 'Plan' }]);
expect(second).toEqual([]);
});
it('maps diff updates', () => {
const converter = new AppServerEventConverter();
const events = converter.handleNotification('turn/diff/updated', { diff: 'diff --git a b' });
expect(events).toEqual([{ type: 'turn_diff', unified_diff: 'diff --git a b' }]);
});
it('unwraps codex/event task lifecycle', () => {
const converter = new AppServerEventConverter();
const started = converter.handleNotification('codex/event/task_started', {
msg: { type: 'task_started', turn_id: 'turn-1' }
});
expect(started).toEqual([{ type: 'task_started', turn_id: 'turn-1' }]);
const completed = converter.handleNotification('codex/event/task_complete', {
msg: { type: 'task_complete', turn_id: 'turn-1' }
});
expect(completed).toEqual([{ type: 'task_complete', turn_id: 'turn-1' }]);
});
it('ignores wrapped terminal lifecycle events without turn_id', () => {
const converter = new AppServerEventConverter();
const completed = converter.handleNotification('codex/event/task_complete', {
msg: { type: 'task_complete' }
});
expect(completed).toEqual([]);
});
it('unwraps codex/event agent deltas and item completion', () => {
const converter = new AppServerEventConverter();
converter.handleNotification('codex/event/agent_message_delta', {
msg: { type: 'agent_message_delta', item_id: 'msg-1', delta: 'Hello' }
});
converter.handleNotification('codex/event/agent_message_content_delta', {
msg: { type: 'agent_message_content_delta', item_id: 'msg-1', delta: ' world' }
});
const completed = converter.handleNotification('codex/event/item_completed', {
msg: {
type: 'item_completed',
item_id: 'msg-1',
item: { id: 'msg-1', type: 'AgentMessage' }
}
});
expect(completed).toEqual([{ type: 'agent_message', message: 'Hello world' }]);
});
it('unwraps codex/event reasoning completion from summary text', () => {
const converter = new AppServerEventConverter();
converter.handleNotification('codex/event/reasoning_content_delta', {
msg: { type: 'reasoning_content_delta', item_id: 'r1', delta: 'Plan' }
});
const completed = converter.handleNotification('codex/event/item_completed', {
msg: {
type: 'item_completed',
item_id: 'r1',
item: { id: 'r1', type: 'Reasoning', summary_text: ['Plan done'] }
}
});
expect(completed).toEqual([{ type: 'agent_reasoning', text: 'Plan done' }]);
});
it('prefers canonical reasoning stream over wrapped agent_reasoning events', () => {
const converter = new AppServerEventConverter();
const section = converter.handleNotification('codex/event/agent_reasoning_section_break', {
msg: { type: 'agent_reasoning_section_break', item_id: 'r1' }
});
const delta = converter.handleNotification('codex/event/agent_reasoning_delta', {
msg: { type: 'agent_reasoning_delta', item_id: 'r1', delta: 'step' }
});
const reasoning = converter.handleNotification('codex/event/agent_reasoning', {
msg: { type: 'agent_reasoning', item_id: 'r1', text: 'Plan' }
});
expect(section).toEqual([{ type: 'agent_reasoning_section_break' }]);
expect(delta).toEqual([]);
expect(reasoning).toEqual([]);
});
it('deduplicates section break when wrapped and direct summary part events share the same index', () => {
const converter = new AppServerEventConverter();
const wrapped = converter.handleNotification('codex/event/agent_reasoning_section_break', {
msg: { type: 'agent_reasoning_section_break', item_id: 'r1', summary_index: 0 }
});
const direct = converter.handleNotification('item/reasoning/summaryPartAdded', {
itemId: 'r1',
summaryIndex: 0
});
expect(wrapped).toEqual([{ type: 'agent_reasoning_section_break' }]);
expect(direct).toEqual([]);
});
it('ignores wrapped final agent message and relies on item completion', () => {
const converter = new AppServerEventConverter();
const wrapped = converter.handleNotification('codex/event/agent_message', {
msg: { type: 'agent_message', item_id: 'msg-1', message: 'Hello' }
});
expect(wrapped).toEqual([]);
});
it('ignores wrapped retryable errors', () => {
const converter = new AppServerEventConverter();
const events = converter.handleNotification('codex/event/error', {
msg: { type: 'error', message: 'temporary', will_retry: true }
});
expect(events).toEqual([]);
});
it('maps wrapped non-retryable errors to task_failed', () => {
const converter = new AppServerEventConverter();
const events = converter.handleNotification('codex/event/error', {
msg: { type: 'error', message: 'fatal' }
});
expect(events).toEqual([{ type: 'task_failed', error: 'fatal' }]);
});
});
+216 -5
View File
@@ -76,17 +76,187 @@ function extractChanges(value: unknown): Record<string, unknown> | null {
return null;
}
function extractTextFromContent(value: unknown): string | null {
if (typeof value === 'string' && value.length > 0) {
return value;
}
if (!Array.isArray(value)) {
return null;
}
const chunks: string[] = [];
for (const entry of value) {
const record = asRecord(entry);
if (!record) continue;
const text = asString(record.text ?? record.message ?? record.content);
if (text) {
chunks.push(text);
}
}
if (chunks.length === 0) {
return null;
}
return chunks.join('');
}
function extractItemText(item: Record<string, unknown>): string | null {
return asString(item.text ?? item.message) ?? extractTextFromContent(item.content);
}
function extractReasoningText(item: Record<string, unknown>): string | null {
const direct = extractItemText(item);
if (direct) {
return direct;
}
const summary = item.summary_text ?? item.summaryText;
if (Array.isArray(summary)) {
const chunks = summary.filter((part): part is string => typeof part === 'string' && part.length > 0);
if (chunks.length > 0) {
return chunks.join('\n');
}
}
return null;
}
export class AppServerEventConverter {
private readonly agentMessageBuffers = new Map<string, string>();
private readonly reasoningBuffers = new Map<string, string>();
private readonly commandOutputBuffers = new Map<string, string>();
private readonly commandMeta = new Map<string, Record<string, unknown>>();
private readonly fileChangeMeta = new Map<string, Record<string, unknown>>();
private readonly completedAgentMessageItems = new Set<string>();
private readonly completedReasoningItems = new Set<string>();
private readonly reasoningSectionBreakKeys = new Set<string>();
private readonly lastAgentMessageDeltaByItemId = new Map<string, string>();
private readonly lastReasoningDeltaByItemId = new Map<string, string>();
private readonly lastCommandOutputDeltaByItemId = new Map<string, string>();
private handleWrappedCodexEvent(paramsRecord: Record<string, unknown>): ConvertedEvent[] | null {
const msg = asRecord(paramsRecord.msg);
if (!msg) {
return [];
}
const msgType = asString(msg.type);
if (!msgType) {
return [];
}
if (msgType === 'item_started' || msgType === 'item_completed') {
const itemMethod = msgType === 'item_started' ? 'item/started' : 'item/completed';
const item = asRecord(msg.item) ?? {};
const params: Record<string, unknown> = {
item,
itemId: asString(msg.item_id ?? msg.itemId ?? item.id),
threadId: asString(msg.thread_id ?? msg.threadId),
turnId: asString(msg.turn_id ?? msg.turnId)
};
return this.handleNotification(itemMethod, params);
}
if (
msgType === 'task_started' ||
msgType === 'task_complete' ||
msgType === 'turn_aborted' ||
msgType === 'task_failed'
) {
const turnId = asString(msg.turn_id ?? msg.turnId);
if ((msgType === 'task_complete' || msgType === 'turn_aborted' || msgType === 'task_failed') && !turnId) {
logger.debug('[AppServerEventConverter] Ignoring wrapped terminal event without turn_id', { msgType });
return [];
}
const event: ConvertedEvent = { type: msgType };
if (turnId) {
event.turn_id = turnId;
}
if (msgType === 'task_failed') {
const error = asString(msg.error ?? msg.message ?? asRecord(msg.error)?.message);
if (error) {
event.error = error;
}
}
return [event];
}
if (msgType === 'agent_message_delta' || msgType === 'agent_message_content_delta') {
const itemId = asString(msg.item_id ?? msg.itemId ?? msg.id) ?? 'agent-message';
const delta = asString(msg.delta ?? msg.text ?? msg.message);
if (!delta) return [];
return this.handleNotification('item/agentMessage/delta', { itemId, delta });
}
if (msgType === 'reasoning_content_delta') {
const itemId = asString(msg.item_id ?? msg.itemId ?? msg.id) ?? 'reasoning';
const delta = asString(msg.delta ?? msg.text ?? msg.message);
if (!delta) return [];
return this.handleNotification('item/reasoning/summaryTextDelta', { itemId, delta });
}
if (msgType === 'agent_reasoning_section_break') {
const itemId = asString(msg.item_id ?? msg.itemId ?? msg.id) ?? 'reasoning';
const summaryIndex = asNumber(msg.summary_index ?? msg.summaryIndex);
return this.handleNotification('item/reasoning/summaryPartAdded', {
itemId,
...(summaryIndex !== null ? { summaryIndex } : {})
});
}
if (msgType === 'agent_reasoning_delta' || msgType === 'agent_reasoning' || msgType === 'agent_message') {
return [];
}
if (msgType === 'exec_command_output_delta') {
const itemId = asString(msg.call_id ?? msg.callId ?? msg.item_id ?? msg.itemId ?? msg.id);
const delta = asString(msg.delta ?? msg.output ?? msg.stdout ?? msg.text);
if (!itemId || !delta) return [];
return this.handleNotification('item/commandExecution/outputDelta', { itemId, delta });
}
if (msgType === 'error') {
const errorRecord = asRecord(msg.error);
const willRetry = asBoolean(msg.will_retry ?? msg.willRetry ?? errorRecord?.will_retry ?? errorRecord?.willRetry) ?? false;
if (willRetry) {
return [];
}
const error = asString(msg.message ?? msg.reason ?? errorRecord?.message);
return error ? [{ type: 'task_failed', error }] : [];
}
if (
msgType === 'mcp_startup_update' ||
msgType === 'mcp_startup_complete' ||
msgType === 'plan_update' ||
msgType === 'skills_update_available' ||
msgType === 'stream_error' ||
msgType === 'warning' ||
msgType === 'context_compacted' ||
msgType === 'terminal_interaction' ||
msgType === 'user_message'
) {
return [];
}
return [msg as ConvertedEvent];
}
handleNotification(method: string, params: unknown): ConvertedEvent[] {
const events: ConvertedEvent[] = [];
const paramsRecord = asRecord(params) ?? {};
if (method.startsWith('codex/event/')) {
return this.handleWrappedCodexEvent(paramsRecord) ?? events;
}
if (method === 'account/rateLimits/updated' || method === 'turn/plan/updated' || method === 'thread/compacted') {
return events;
}
if (method === 'thread/started' || method === 'thread/resumed') {
const thread = asRecord(paramsRecord.thread) ?? paramsRecord;
const threadId = asString(thread.threadId ?? thread.thread_id ?? thread.id);
@@ -152,16 +322,26 @@ export class AppServerEventConverter {
const itemId = extractItemId(paramsRecord);
const delta = asString(paramsRecord.delta ?? paramsRecord.text ?? paramsRecord.message);
if (itemId && delta) {
const lastDelta = this.lastAgentMessageDeltaByItemId.get(itemId);
if (lastDelta === delta) {
return events;
}
this.lastAgentMessageDeltaByItemId.set(itemId, delta);
const prev = this.agentMessageBuffers.get(itemId) ?? '';
this.agentMessageBuffers.set(itemId, prev + delta);
}
return events;
}
if (method === 'item/reasoning/textDelta') {
if (method === 'item/reasoning/textDelta' || method === 'item/reasoning/summaryTextDelta') {
const itemId = extractItemId(paramsRecord) ?? 'reasoning';
const delta = asString(paramsRecord.delta ?? paramsRecord.text ?? paramsRecord.message);
if (delta) {
const lastDelta = this.lastReasoningDeltaByItemId.get(itemId);
if (lastDelta === delta) {
return events;
}
this.lastReasoningDeltaByItemId.set(itemId, delta);
const prev = this.reasoningBuffers.get(itemId) ?? '';
this.reasoningBuffers.set(itemId, prev + delta);
events.push({ type: 'agent_reasoning_delta', delta });
@@ -170,6 +350,15 @@ export class AppServerEventConverter {
}
if (method === 'item/reasoning/summaryPartAdded') {
const itemId = extractItemId(paramsRecord) ?? 'reasoning';
const summaryIndex = asNumber(paramsRecord.summaryIndex ?? paramsRecord.summary_index);
if (summaryIndex !== null) {
const key = `${itemId}:${summaryIndex}`;
if (this.reasoningSectionBreakKeys.has(key)) {
return events;
}
this.reasoningSectionBreakKeys.add(key);
}
events.push({ type: 'agent_reasoning_section_break' });
return events;
}
@@ -178,6 +367,11 @@ export class AppServerEventConverter {
const itemId = extractItemId(paramsRecord);
const delta = asString(paramsRecord.delta ?? paramsRecord.text ?? paramsRecord.output ?? paramsRecord.stdout);
if (itemId && delta) {
const lastDelta = this.lastCommandOutputDeltaByItemId.get(itemId);
if (lastDelta === delta) {
return events;
}
this.lastCommandOutputDeltaByItemId.set(itemId, delta);
const prev = this.commandOutputBuffers.get(itemId) ?? '';
this.commandOutputBuffers.set(itemId, prev + delta);
}
@@ -197,22 +391,32 @@ export class AppServerEventConverter {
if (itemType === 'agentmessage') {
if (method === 'item/completed') {
const text = asString(item.text ?? item.message ?? item.content) ?? this.agentMessageBuffers.get(itemId);
if (this.completedAgentMessageItems.has(itemId)) {
return events;
}
const text = extractItemText(item) ?? this.agentMessageBuffers.get(itemId);
if (text) {
events.push({ type: 'agent_message', message: text });
this.completedAgentMessageItems.add(itemId);
this.agentMessageBuffers.delete(itemId);
}
this.agentMessageBuffers.delete(itemId);
this.lastAgentMessageDeltaByItemId.delete(itemId);
}
return events;
}
if (itemType === 'reasoning') {
if (method === 'item/completed') {
const text = asString(item.text ?? item.message ?? item.content) ?? this.reasoningBuffers.get(itemId);
if (this.completedReasoningItems.has(itemId)) {
return events;
}
const text = extractReasoningText(item) ?? this.reasoningBuffers.get(itemId);
if (text) {
events.push({ type: 'agent_reasoning', text });
this.completedReasoningItems.add(itemId);
this.reasoningBuffers.delete(itemId);
}
this.reasoningBuffers.delete(itemId);
this.lastReasoningDeltaByItemId.delete(itemId);
}
return events;
}
@@ -256,6 +460,7 @@ export class AppServerEventConverter {
this.commandMeta.delete(itemId);
this.commandOutputBuffers.delete(itemId);
this.lastCommandOutputDeltaByItemId.delete(itemId);
}
return events;
@@ -309,5 +514,11 @@ export class AppServerEventConverter {
this.commandOutputBuffers.clear();
this.commandMeta.clear();
this.fileChangeMeta.clear();
this.completedAgentMessageItems.clear();
this.completedReasoningItems.clear();
this.reasoningSectionBreakKeys.clear();
this.lastAgentMessageDeltaByItemId.clear();
this.lastReasoningDeltaByItemId.clear();
this.lastCommandOutputDeltaByItemId.clear();
}
}
+4 -1
View File
@@ -13,7 +13,10 @@ import { trimIdent } from '@/utils/trimIdent';
* so the tool is called as `functions.hapi__change_title`.
*/
export const TITLE_INSTRUCTION = trimIdent(`
Based on this message, call functions.hapi__change_title to change chat session title that would represent the current task. If chat idea would change dramatically - call this function again to update the title.
ALWAYS when you start a new chat, call the title tool to set a concise task title.
Prefer calling functions.hapi__change_title.
If that exact tool name is unavailable, call an equivalent alias such as hapi__change_title, mcp__hapi__change_title, or hapi_change_title.
If the task focus changes significantly later, call the title tool again with a better title.
`);
/**
@@ -0,0 +1,94 @@
import { describe, expect, it } from 'vitest';
import { shouldIgnoreTerminalEvent } from './terminalEventGuard';
describe('shouldIgnoreTerminalEvent', () => {
it('returns false for non app-server mode', () => {
const ignored = shouldIgnoreTerminalEvent({
useAppServer: false,
eventTurnId: null,
currentTurnId: 'turn-1',
turnInFlight: true
});
expect(ignored).toBe(false);
});
it('ignores terminal events without turn_id when current turn id exists', () => {
const ignored = shouldIgnoreTerminalEvent({
useAppServer: true,
eventTurnId: null,
currentTurnId: 'turn-1',
turnInFlight: true
});
expect(ignored).toBe(true);
});
it('ignores terminal events without turn_id while a turn is still in flight', () => {
const ignored = shouldIgnoreTerminalEvent({
useAppServer: true,
eventTurnId: null,
currentTurnId: null,
turnInFlight: true
});
expect(ignored).toBe(true);
});
it('accepts terminal events without turn_id when anonymous terminal is explicitly allowed', () => {
const ignored = shouldIgnoreTerminalEvent({
useAppServer: true,
eventTurnId: null,
currentTurnId: null,
turnInFlight: true,
allowAnonymousTerminalEvent: true
});
expect(ignored).toBe(false);
});
it('still ignores terminal events without turn_id when current turn id exists', () => {
const ignored = shouldIgnoreTerminalEvent({
useAppServer: true,
eventTurnId: null,
currentTurnId: 'turn-1',
turnInFlight: true,
allowAnonymousTerminalEvent: true
});
expect(ignored).toBe(true);
});
it('ignores stale terminal events from another turn', () => {
const ignored = shouldIgnoreTerminalEvent({
useAppServer: true,
eventTurnId: 'turn-old',
currentTurnId: 'turn-current',
turnInFlight: true
});
expect(ignored).toBe(true);
});
it('accepts terminal events that match the current turn id', () => {
const ignored = shouldIgnoreTerminalEvent({
useAppServer: true,
eventTurnId: 'turn-current',
currentTurnId: 'turn-current',
turnInFlight: true
});
expect(ignored).toBe(false);
});
it('accepts terminal events without turn_id when no turn is active', () => {
const ignored = shouldIgnoreTerminalEvent({
useAppServer: true,
eventTurnId: null,
currentTurnId: null,
turnInFlight: false
});
expect(ignored).toBe(false);
});
});
+29
View File
@@ -0,0 +1,29 @@
export type TerminalEventGuardInput = {
useAppServer: boolean;
eventTurnId: string | null;
currentTurnId: string | null;
turnInFlight: boolean;
allowAnonymousTerminalEvent?: boolean;
};
export function shouldIgnoreTerminalEvent(input: TerminalEventGuardInput): boolean {
const allowAnonymousTerminalEvent = input.allowAnonymousTerminalEvent === true;
if (!input.useAppServer) {
return false;
}
if (input.eventTurnId) {
return Boolean(input.currentTurnId && input.eventTurnId !== input.currentTurnId);
}
if (input.currentTurnId) {
return true;
}
if (input.turnInFlight && !allowAnonymousTerminalEvent) {
return true;
}
return false;
}