Skip to content

Commit 14e865e

Browse files
NikolaiSviridovNikolaiSviridov
authored
LLM-26222 Show input params, result of running mcp tools, LLM-26016 Show mcp initialization errors in chat, LLM-26758 Failed to initialize ACP process (#90)
* LLM-26016 [Codex] Show mcp initialization errors in chat * LLM-26758 fix Failed to initialize ACP process, add script for building binary * LLM-26222 [Codex] Show input params, result of running mcp tools * fix possible mcp race * review: remove log stored on acp side * review: revert LLM-26758 changes * review: remove unused code * review: refactor publishMcpStartupStatusAsync * review: cleanup
1 parent 3a71ad9 commit 14e865e

12 files changed

Lines changed: 631 additions & 10 deletions

‎src/CodexAcpClient.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import open from "open";
77
import type {Disposable} from "vscode-jsonrpc";
88
import type {
99
ClientInfo,
10+
McpStartupCompleteEvent,
1011
ReasoningEffort,
1112
ServerNotification
1213
} from "./app-server";
@@ -275,6 +276,10 @@ export class CodexAcpClient {
275276
return startup.ready;
276277
}
277278

279+
async awaitMcpStartupResult(mcpStartupVersion: number): Promise<McpStartupCompleteEvent> {
280+
return await this.codexClient.awaitMcpStartup(mcpStartupVersion);
281+
}
282+
278283
getMcpStartupCompleteVersion(): number {
279284
return this.codexClient.getMcpStartupCompleteVersion();
280285
}

‎src/CodexAcpServer.ts‎

Lines changed: 75 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,17 @@ import {CodexApprovalHandler} from "./CodexApprovalHandler";
1010
import {CodexAuthMethods, type CodexAuthRequest} from "./CodexAuthMethod";
1111
import {CodexAcpClient, type SessionMetadata, type SessionMetadataWithThread} from "./CodexAcpClient";
1212
import {ACPSessionConnection, type UpdateSessionEvent} from "./ACPSessionConnection";
13-
import type {Account, CollabAgentToolCallStatus, Model, Thread, ThreadItem, UserInput, ReasoningEffortOption} from "./app-server/v2";
13+
import type {McpStartupCompleteEvent, InputModality, ReasoningEffort} from "./app-server";
14+
import type {
15+
Account,
16+
CollabAgentToolCallStatus,
17+
Model,
18+
Thread,
19+
ThreadItem,
20+
UserInput,
21+
ReasoningEffortOption
22+
} from "./app-server/v2";
1423
import type {RateLimitsMap} from "./RateLimitsMap";
15-
import type {InputModality, ReasoningEffort} from "./app-server";
1624
import {ModelId} from "./ModelId";
1725
import {AgentMode} from "./AgentMode";
1826
import type {TokenCount} from "./TokenCount";
@@ -43,6 +51,10 @@ export interface SessionState {
4351
sessionMcpServers?: Array<string>;
4452
}
4553

54+
interface PendingMcpStartupSession {
55+
requestedServers: Set<string>;
56+
}
57+
4658
export class CodexAcpServer implements acp.Agent {
4759
private readonly codexAcpClient: CodexAcpClient;
4860
private readonly connection: acp.AgentSideConnection;
@@ -51,6 +63,7 @@ export class CodexAcpServer implements acp.Agent {
5163
private readonly availableCommands: CodexCommands;
5264

5365
private readonly sessions: Map<string, SessionState>;
66+
private readonly pendingMcpStartupSessions: Map<string, PendingMcpStartupSession>;
5467

5568
constructor(
5669
connection: acp.AgentSideConnection,
@@ -59,6 +72,7 @@ export class CodexAcpServer implements acp.Agent {
5972
getExitCode?: () => number | null,
6073
) {
6174
this.sessions = new Map();
75+
this.pendingMcpStartupSessions = new Map();
6276
this.connection = connection;
6377
this.codexAcpClient = codexAcpClient;
6478
this.defaultAuthRequest = defaultAuthRequest ?? null;
@@ -159,6 +173,14 @@ export class CodexAcpServer implements acp.Agent {
159173
}
160174
this.sessions.set(sessionId, sessionState);
161175

176+
const requestedMcpServers = request.mcpServers ?? [];
177+
if (requestedMcpServers.length > 0) {
178+
this.pendingMcpStartupSessions.set(sessionId, {
179+
requestedServers: new Set(requestedMcpServers.map(server => server.name)),
180+
});
181+
this.publishMcpStartupStatusAsync(sessionId, mcpStartupVersion);
182+
}
183+
162184
this.publishAvailableCommandsAsync(sessionId);
163185
const sessionModelState: SessionModelState = this.createModelState(models, currentModelId);
164186
const sessionModeState: SessionModeState = sessionState.agentMode.toSessionModeState();
@@ -359,6 +381,14 @@ export class CodexAcpServer implements acp.Agent {
359381
};
360382
this.sessions.set(sessionId, sessionState);
361383

384+
const requestedMcpServers = request.mcpServers ?? [];
385+
if (requestedMcpServers.length > 0) {
386+
this.pendingMcpStartupSessions.set(sessionId, {
387+
requestedServers: new Set(requestedMcpServers.map(server => server.name)),
388+
});
389+
this.publishMcpStartupStatusAsync(sessionId, mcpStartupVersion);
390+
}
391+
362392
await this.availableCommands.publish(sessionId);
363393
const sessionModelState: SessionModelState = this.createModelState(models, currentModelId);
364394
const sessionModeState: SessionModeState = sessionState.agentMode.toSessionModeState();
@@ -621,6 +651,49 @@ export class CodexAcpServer implements acp.Agent {
621651
return await this.runWithProcessCheck(() => this.codexAcpClient.awaitMcpStartup(mcpStartupVersion));
622652
}
623653

654+
private publishMcpStartupStatusAsync(sessionId: string, mcpStartupVersion: number): void {
655+
void this.doPublishMcpStartupStatus(sessionId, mcpStartupVersion);
656+
}
657+
658+
private async doPublishMcpStartupStatus(sessionId: string, mcpStartupVersion: number): Promise<void> {
659+
try {
660+
const mcpStartup = await this.runWithProcessCheck(() => this.codexAcpClient.awaitMcpStartupResult(mcpStartupVersion));
661+
const sessionState = this.sessions.get(sessionId);
662+
const pendingStartup = this.pendingMcpStartupSessions.get(sessionId);
663+
if (sessionState && pendingStartup) {
664+
sessionState.sessionMcpServers = mcpStartup.ready.filter(serverName =>
665+
pendingStartup.requestedServers.has(serverName)
666+
);
667+
}
668+
await this.publishMcpStartupStatus(sessionId, mcpStartup, pendingStartup?.requestedServers);
669+
} catch (err) {
670+
logger.error(`Failed to publish MCP startup status for session ${sessionId}`, err);
671+
} finally {
672+
this.pendingMcpStartupSessions.delete(sessionId);
673+
}
674+
}
675+
676+
private async publishMcpStartupStatus(
677+
sessionId: string,
678+
mcpStartup: McpStartupCompleteEvent,
679+
requestedServers?: Set<string>
680+
): Promise<void> {
681+
const filteredStartup = requestedServers
682+
? {
683+
ready: mcpStartup.ready.filter(server => requestedServers.has(server)),
684+
failed: mcpStartup.failed.filter(server => requestedServers.has(server.server)),
685+
cancelled: mcpStartup.cancelled.filter(server => requestedServers.has(server)),
686+
}
687+
: mcpStartup;
688+
689+
for (const update of CodexEventHandler.createMcpStartupUpdates(filteredStartup)) {
690+
await this.connection.sessionUpdate({
691+
sessionId,
692+
update,
693+
});
694+
}
695+
}
696+
624697
async prompt(params: acp.PromptRequest): Promise<acp.PromptResponse> {
625698
logger.log("Prompt received", {
626699
sessionId: params.sessionId,

‎src/CodexAppServerClient.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ export class CodexAppServerClient {
7676
}
7777
return;
7878
}
79-
8079
const serverNotification = data as ServerNotification;
8180
this.notify(serverNotification);
8281
for (const callback of this.codexEventHandlers) {

‎src/CodexEventHandler.ts‎

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,14 @@ import type {
2020
ThreadTokenUsageUpdatedNotification,
2121
TurnPlanUpdatedNotification
2222
} from "./app-server/v2";
23+
import type { McpStartupCompleteEvent } from "./app-server";
2324
import {toTokenCount} from "./TokenCount";
2425
import {
2526
createCommandExecutionUpdate,
2627
createDynamicToolCallUpdate,
2728
createFileChangeUpdate,
29+
createMcpRawInput,
30+
createMcpRawOutput,
2831
createFuzzyFileSearchComplete,
2932
createFuzzyFileSearchStartOrUpdate,
3033
createMcpToolCallUpdate,
@@ -101,12 +104,13 @@ export class CodexEventHandler {
101104
case "turn/diff/updated":
102105
case "item/commandExecution/terminalInteraction":
103106
case "item/fileChange/outputDelta":
104-
case "item/mcpToolCall/progress":
105107
case "serverRequest/resolved":
106108
case "account/updated":
107109
case "fs/changed":
108110
case "mcpServer/startupStatus/updated":
109111
return null;
112+
case "item/mcpToolCall/progress":
113+
return this.createMcpToolProgressEvent(notification.params);
110114
case "account/rateLimits/updated":
111115
this.handleRateLimitsUpdated(notification.params);
112116
return null;
@@ -210,13 +214,20 @@ export class CodexEventHandler {
210214

211215
private async completeItemEvent(event: ItemCompletedNotification): Promise<UpdateSessionEvent | null> {
212216
switch (event.item.type) {
213-
case "mcpToolCall":
214217
case "fileChange":
215218
case "dynamicToolCall":
216219
return {
217220
sessionUpdate: "tool_call_update",
218221
toolCallId: event.item.id,
219-
status: event.item.status === "completed" ? "completed" : "failed"
222+
status: event.item.status === "completed" ? "completed" : "failed",
223+
}
224+
case "mcpToolCall":
225+
return {
226+
sessionUpdate: "tool_call_update",
227+
toolCallId: event.item.id,
228+
status: event.item.status === "completed" ? "completed" : "failed",
229+
rawInput: createMcpRawInput(event.item.server, event.item.tool, event.item.arguments),
230+
rawOutput: createMcpRawOutput(event.item.result, event.item.error),
220231
}
221232
case "commandExecution":
222233
return this.completeCommandExecutionEvent(event.item);
@@ -258,6 +269,53 @@ export class CodexEventHandler {
258269
}
259270
}
260271

272+
private createMcpToolProgressEvent(event: { itemId: string, message: string }): UpdateSessionEvent {
273+
const logDelta = event.message.trim();
274+
return {
275+
sessionUpdate: "tool_call_update",
276+
toolCallId: event.itemId,
277+
_meta: {
278+
mcp_output_delta: {
279+
data: logDelta,
280+
}
281+
}
282+
};
283+
}
284+
285+
static createMcpStartupUpdates(event: McpStartupCompleteEvent): UpdateSessionEvent[] {
286+
const failedUpdates = event.failed.map((server: McpStartupCompleteEvent["failed"][number]) => this.createMcpStartupToolCallUpdate(
287+
server.server,
288+
`[codex-acp forwarded startup error] MCP server \`${server.server}\` failed to start: ${server.error}`
289+
));
290+
const cancelledUpdates = event.cancelled.map((server: McpStartupCompleteEvent["cancelled"][number]) => this.createMcpStartupToolCallUpdate(
291+
server,
292+
`[codex-acp forwarded startup error] MCP server \`${server}\` startup was cancelled.`
293+
));
294+
295+
return [...failedUpdates, ...cancelledUpdates];
296+
}
297+
298+
private static createMcpStartupToolCallUpdate(serverName: string, message: string): UpdateSessionEvent {
299+
return {
300+
sessionUpdate: "tool_call",
301+
toolCallId: this.getMcpStartupToolCallId(serverName),
302+
kind: "other",
303+
title: `mcp__${serverName}__startup`,
304+
status: "failed",
305+
content: [{
306+
type: "content",
307+
content: {
308+
type: "text",
309+
text: message,
310+
},
311+
}],
312+
};
313+
}
314+
315+
private static getMcpStartupToolCallId(serverName: string): string {
316+
return `mcp_startup.${encodeURIComponent(serverName)}`;
317+
}
318+
261319
private completeCommandExecutionEvent(item: ThreadItem & { "type": "commandExecution" }): UpdateSessionEvent {
262320
return {
263321
sessionUpdate: "tool_call_update",

‎src/CodexToolCallMapper.ts‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import type {
1313
CommandExecutionStatus,
1414
DynamicToolCallStatus,
1515
FileUpdateChange,
16+
McpToolCallError,
17+
McpToolCallResult,
1618
McpToolCallStatus,
1719
PatchApplyStatus,
1820
ThreadItem,
@@ -84,7 +86,12 @@ export async function createCommandExecutionUpdate(
8486
export async function createMcpToolCallUpdate(
8587
item: ThreadItem & { type: "mcpToolCall" }
8688
): Promise<UpdateSessionEvent> {
87-
return createExecuteToolCallUpdate(item, `mcp.${item.server}.${item.tool}`);
89+
return createExecuteToolCallUpdate(
90+
item,
91+
`mcp.${item.server}.${item.tool}`,
92+
createMcpRawInput(item.server, item.tool, item.arguments),
93+
createMcpRawOutput(item.result, item.error),
94+
);
8895
}
8996

9097
export async function createDynamicToolCallUpdate(
@@ -96,7 +103,8 @@ export async function createDynamicToolCallUpdate(
96103
export async function createExecuteToolCallUpdate(
97104
item: ThreadItem & ({ type: "mcpToolCall" } | { type: "dynamicToolCall" }),
98105
title: string,
99-
rawInput?: { arguments: JsonValue }
106+
rawInput?: Record<string, JsonValue | string>,
107+
rawOutput?: Record<string, JsonValue | string | null>,
100108
): Promise<UpdateSessionEvent> {
101109
return {
102110
sessionUpdate: "tool_call",
@@ -105,6 +113,29 @@ export async function createExecuteToolCallUpdate(
105113
title: title,
106114
status: toAcpStatus(item.status),
107115
rawInput: rawInput,
116+
rawOutput: rawOutput,
117+
};
118+
}
119+
120+
export function createMcpRawInput(server: string, tool: string, argumentsValue: JsonValue): Record<string, JsonValue | string> {
121+
return {
122+
server,
123+
tool,
124+
arguments: argumentsValue,
125+
};
126+
}
127+
128+
export function createMcpRawOutput(
129+
result: McpToolCallResult | null,
130+
error: McpToolCallError | null,
131+
): Record<string, JsonValue | string | null> | undefined {
132+
if (result === null && error === null) {
133+
return undefined;
134+
}
135+
136+
return {
137+
result,
138+
error,
108139
};
109140
}
110141

‎src/__tests__/CodexACPAgent/CodexAcpClient.test.ts‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,69 @@ describe('ACP server test', { timeout: 40_000 }, () => {
277277
expect(mcpServers).toEqual(["alpha", "beta"]);
278278
});
279279

280+
it('forwards failed MCP startup as failed tool call updates after new session', async () => {
281+
const mockFixture = createCodexMockTestFixture();
282+
const codexAcpAgent = mockFixture.getCodexAcpAgent();
283+
const codexAppServerClient = mockFixture.getCodexAppServerClient();
284+
285+
vi.spyOn(codexAcpAgent, "checkAuthorization").mockResolvedValue(undefined);
286+
vi.spyOn(codexAppServerClient, "threadStart").mockResolvedValue({
287+
thread: { id: "thread-id" } as any,
288+
model: "gpt-5",
289+
reasoningEffort: "medium",
290+
} as any);
291+
vi.spyOn(codexAppServerClient, "listModels").mockResolvedValue({
292+
data: [{
293+
id: "gpt-5",
294+
name: "GPT-5",
295+
inputModalities: ["text"],
296+
supportedReasoningEfforts: [],
297+
}],
298+
hasMore: false,
299+
} as any);
300+
vi.spyOn(codexAppServerClient, "accountRead").mockResolvedValue({
301+
requiresOpenaiAuth: false,
302+
account: null,
303+
} as any);
304+
vi.spyOn(codexAppServerClient, "listSkills").mockResolvedValue({ data: [] });
305+
const mcpServer = {
306+
name: "broken-mcp",
307+
command: "npx",
308+
args: ["broken"],
309+
env: [],
310+
} as unknown as acp.McpServerStdio;
311+
312+
const session = await codexAcpAgent.newSession({
313+
cwd: "/workspace",
314+
mcpServers: [mcpServer]
315+
});
316+
317+
mockFixture.sendServerNotification({
318+
method: "codex/event/mcp_startup_complete",
319+
params: {
320+
msg: {
321+
type: "mcp_startup_complete",
322+
ready: [],
323+
failed: [{
324+
server: "broken-mcp",
325+
error: "boom",
326+
}],
327+
cancelled: [],
328+
}
329+
}
330+
});
331+
332+
await vi.waitFor(() => {
333+
const dump = mockFixture.getAcpConnectionDump([]);
334+
expect(dump).toContain('"sessionId": "thread-id"');
335+
expect(dump).toContain('"sessionUpdate": "tool_call"');
336+
expect(dump).toContain('"toolCallId": "mcp_startup.broken-mcp"');
337+
expect(dump).toContain('MCP server `broken-mcp` failed to start: boom');
338+
});
339+
340+
expect(session.sessionId).toBe("thread-id");
341+
});
342+
280343
it('prefetches session additional skill roots before turn start', async () => {
281344
const mockFixture = createCodexMockTestFixture();
282345
const codexAcpAgent = mockFixture.getCodexAcpAgent();

0 commit comments

Comments
 (0)