Skip to content

Commit 4fead2e

Browse files
review fix
1 parent 0efa2dc commit 4fead2e

3 files changed

Lines changed: 84 additions & 19 deletions

File tree

src/CodexEventHandler.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -271,28 +271,28 @@ export class CodexEventHandler {
271271
}
272272

273273
private createMcpToolProgressEvent(event: { itemId: string, message: string }): UpdateSessionEvent {
274-
const logs = this.appendMcpToolLog(event.itemId, event.message);
274+
const logDelta = this.appendMcpToolLog(event.itemId, event.message);
275275
return {
276276
sessionUpdate: "tool_call_update",
277277
toolCallId: event.itemId,
278-
rawOutput: {
279-
formatted_output: logs.join("\n\n"),
278+
_meta: {
279+
mcp_output_delta: {
280+
data: logDelta,
281+
}
280282
}
281283
};
282284
}
283285

284-
private appendMcpToolLog(toolCallId: string, message: string): Array<string> {
286+
private appendMcpToolLog(toolCallId: string, message: string): string {
285287
const cleaned = message.trim();
286288
if (cleaned.length === 0) {
287-
return this.sessionState.mcpToolLogs.get(toolCallId) ?? [];
289+
return "";
288290
}
289291

290292
const logs = this.sessionState.mcpToolLogs.get(toolCallId) ?? [];
291-
if (logs.at(-1) !== cleaned && !logs.includes(cleaned)) {
292-
logs.push(cleaned);
293-
}
293+
logs.push(cleaned);
294294
this.sessionState.mcpToolLogs.set(toolCallId, logs);
295-
return logs;
295+
return cleaned;
296296
}
297297

298298
private consumeMcpToolLogs(toolCallId: string): Array<string> {

src/CodexToolCallMapper.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -166,23 +166,20 @@ function normalizeMcpLogLines(
166166
error: McpToolCallError | null,
167167
result: McpToolCallResult | null,
168168
): Array<string> {
169-
const lines: string[] = [];
170-
const seen = new Set<string>();
169+
const lines = logs
170+
.map((log) => log.trim())
171+
.filter((log) => log.length > 0);
171172

172-
const append = (value: string | null | undefined) => {
173+
const appendTrailingUnique = (value: string | null | undefined) => {
173174
const cleaned = value?.trim();
174-
if (!cleaned || seen.has(cleaned)) {
175+
if (!cleaned || lines.at(-1) === cleaned) {
175176
return;
176177
}
177-
seen.add(cleaned);
178178
lines.push(cleaned);
179179
};
180180

181-
for (const log of logs) {
182-
append(log);
183-
}
184-
append(error?.message);
185-
append(formatMcpResult(result));
181+
appendTrailingUnique(error?.message);
182+
appendTrailingUnique(formatMcpResult(result));
186183

187184
return lines;
188185
}

src/__tests__/CodexACPAgent/command-action-events.test.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,74 @@ describe('CodexEventHandler - command action events', () => {
318318
);
319319
});
320320

321+
it('should preserve repeated mcp progress messages in final output', async () => {
322+
const repeatedMessage = 'Polling for status';
323+
const notifications: ServerNotification[] = [
324+
{
325+
method: 'item/started',
326+
params: {
327+
threadId: 'thread-1',
328+
turnId: 'turn-1',
329+
item: {
330+
type: "mcpToolCall",
331+
id: "call-id",
332+
server: "server-name",
333+
tool: "tool-name",
334+
status: "inProgress",
335+
arguments: { argument: "example" },
336+
result: null,
337+
error: null,
338+
durationMs: null,
339+
},
340+
},
341+
},
342+
{
343+
method: 'item/mcpToolCall/progress',
344+
params: {
345+
threadId: 'thread-1',
346+
turnId: 'turn-1',
347+
itemId: 'call-id',
348+
message: repeatedMessage,
349+
},
350+
},
351+
{
352+
method: 'item/mcpToolCall/progress',
353+
params: {
354+
threadId: 'thread-1',
355+
turnId: 'turn-1',
356+
itemId: 'call-id',
357+
message: repeatedMessage,
358+
},
359+
},
360+
{
361+
method: 'item/completed',
362+
params: {
363+
threadId: 'thread-1',
364+
turnId: 'turn-1',
365+
item: {
366+
type: "mcpToolCall",
367+
id: "call-id",
368+
server: "server-name",
369+
tool: "tool-name",
370+
status: "failed",
371+
arguments: { argument: "example" },
372+
result: null,
373+
error: {
374+
message: repeatedMessage,
375+
},
376+
durationMs: 15,
377+
},
378+
},
379+
},
380+
];
381+
382+
await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, notifications);
383+
384+
await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot(
385+
'data/mcp-tool-repeated-progress.json'
386+
);
387+
});
388+
321389
it('should handle dynamic tools', async () => {
322390
const dynamicToolNotification: ServerNotification = {
323391
method: 'item/started',

0 commit comments

Comments
 (0)