Skip to content

Commit 6b9ed8d

Browse files
authored
Track MCP startup per server status updates (#112)
Replace the legacy startup-complete event flow with typed `mcpServer/startupStatus/updated` handling and wait for requested servers to reach terminal states. Also stop restoring persisted MCP servers on load when the request omits them, and update tests to cover the new behavior.
1 parent 8b20edb commit 6b9ed8d

5 files changed

Lines changed: 195 additions & 153 deletions

File tree

src/CodexAcpClient.ts

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,16 @@ import {isCodexAuthRequest} from "./CodexAuthMethod";
22
import type {EmbeddedResourceResource} from "@agentclientprotocol/sdk";
33
import * as acp from "@agentclientprotocol/sdk";
44
import {type McpServer, RequestError} from "@agentclientprotocol/sdk";
5-
import type {ApprovalHandler, CodexAppServerClient, ElicitationHandler} from "./CodexAppServerClient";
5+
import type {
6+
ApprovalHandler,
7+
CodexAppServerClient,
8+
ElicitationHandler,
9+
McpStartupResult,
10+
} from "./CodexAppServerClient";
611
import open from "open";
712
import type {Disposable} from "vscode-jsonrpc";
813
import type {
914
ClientInfo,
10-
McpStartupCompleteEvent,
1115
ReasoningEffort,
1216
ServerNotification
1317
} from "./app-server";
@@ -274,17 +278,12 @@ export class CodexAcpClient {
274278
};
275279
}
276280

277-
async awaitMcpStartup(mcpStartupVersion: number): Promise<Array<string>> {
278-
const startup = await this.codexClient.awaitMcpStartup(mcpStartupVersion);
279-
return startup.ready;
280-
}
281-
282-
async awaitMcpStartupResult(mcpStartupVersion: number): Promise<McpStartupCompleteEvent> {
283-
return await this.codexClient.awaitMcpStartup(mcpStartupVersion);
281+
async awaitMcpServerStartup(serverNames: Array<string>, afterVersion: number): Promise<McpStartupResult> {
282+
return await this.codexClient.awaitMcpServerStartup(serverNames, afterVersion);
284283
}
285284

286-
getMcpStartupCompleteVersion(): number {
287-
return this.codexClient.getMcpStartupCompleteVersion();
285+
getMcpServerStartupVersion(): number {
286+
return this.codexClient.getMcpServerStartupVersion();
288287
}
289288

290289
private createSessionConfig(projectPath: string, mcpServers: Array<McpServer>): JsonObject {

src/CodexAcpServer.ts

Lines changed: 42 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ import {CodexApprovalHandler} from "./CodexApprovalHandler";
1010
import {CodexElicitationHandler} from "./CodexElicitationHandler";
1111
import {CodexAuthMethods, type CodexAuthRequest} from "./CodexAuthMethod";
1212
import {CodexAcpClient, type SessionMetadata, type SessionMetadataWithThread} from "./CodexAcpClient";
13+
import type {McpStartupResult} from "./CodexAppServerClient";
1314
import {ACPSessionConnection, type UpdateSessionEvent} from "./ACPSessionConnection";
14-
import type {McpStartupCompleteEvent, InputModality, ReasoningEffort} from "./app-server";
15+
import type {InputModality, ReasoningEffort} from "./app-server";
1516
import type {
1617
Account,
1718
CollabAgentToolCallStatus,
@@ -55,6 +56,7 @@ export interface SessionState {
5556

5657
interface PendingMcpStartupSession {
5758
requestedServers: Set<string>;
59+
afterVersion: number;
5860
}
5961

6062
export class CodexAcpServer implements acp.Agent {
@@ -143,7 +145,10 @@ export class CodexAcpServer implements acp.Agent {
143145

144146
async getOrCreateSession(request: acp.NewSessionRequest | acp.ResumeSessionRequest): Promise<[SessionId, SessionModelState, SessionModeState]> {
145147
await this.checkAuthorization();
146-
const mcpStartupVersion = this.codexAcpClient.getMcpStartupCompleteVersion();
148+
const requestedMcpServers = request.mcpServers ?? [];
149+
const mcpServerStartupVersion = requestedMcpServers.length > 0
150+
? this.codexAcpClient.getMcpServerStartupVersion()
151+
: null;
147152

148153
let sessionMetadata: SessionMetadata;
149154
if ("sessionId" in request) {
@@ -156,7 +161,7 @@ export class CodexAcpServer implements acp.Agent {
156161

157162
const accountResponse = await this.runWithProcessCheck(() => this.codexAcpClient.getAccount());
158163
const {sessionId, currentModelId, models} = sessionMetadata;
159-
const sessionMcpServers = await this.resolveSessionMcpServers(request.mcpServers ?? [], mcpStartupVersion, "sessionId" in request);
164+
const sessionMcpServers = this.resolveSessionMcpServers(requestedMcpServers, "sessionId" in request);
160165
const currentModel = this.findCurrentModel(models, currentModelId);
161166
const sessionState: SessionState = {
162167
sessionId: sessionId,
@@ -175,12 +180,12 @@ export class CodexAcpServer implements acp.Agent {
175180
}
176181
this.sessions.set(sessionId, sessionState);
177182

178-
const requestedMcpServers = request.mcpServers ?? [];
179-
if (requestedMcpServers.length > 0) {
183+
if (requestedMcpServers.length > 0 && mcpServerStartupVersion !== null) {
180184
this.pendingMcpStartupSessions.set(sessionId, {
181185
requestedServers: new Set(requestedMcpServers.map(server => server.name)),
186+
afterVersion: mcpServerStartupVersion,
182187
});
183-
this.publishMcpStartupStatusAsync(sessionId, mcpStartupVersion);
188+
this.publishMcpStartupStatusAsync(sessionId);
184189
}
185190

186191
this.publishAvailableCommandsAsync(sessionId);
@@ -355,7 +360,10 @@ export class CodexAcpServer implements acp.Agent {
355360
thread: Thread;
356361
}> {
357362
await this.checkAuthorization();
358-
const mcpStartupVersion = this.codexAcpClient.getMcpStartupCompleteVersion();
363+
const requestedMcpServers = request.mcpServers ?? [];
364+
const mcpServerStartupVersion = requestedMcpServers.length > 0
365+
? this.codexAcpClient.getMcpServerStartupVersion()
366+
: null;
359367

360368
logger.log(`Load existing session: ${request.sessionId}...`);
361369
const sessionMetadata: SessionMetadataWithThread = await this.runWithProcessCheck(() =>
@@ -364,7 +372,7 @@ export class CodexAcpServer implements acp.Agent {
364372

365373
const accountResponse = await this.runWithProcessCheck(() => this.codexAcpClient.getAccount());
366374
const {sessionId, currentModelId, models, thread} = sessionMetadata;
367-
const sessionMcpServers = await this.resolveSessionMcpServers(request.mcpServers ?? [], mcpStartupVersion, true);
375+
const sessionMcpServers = this.resolveSessionMcpServers(requestedMcpServers, true);
368376
const currentModel = this.findCurrentModel(models, currentModelId);
369377
const sessionState: SessionState = {
370378
sessionId: sessionId,
@@ -383,12 +391,12 @@ export class CodexAcpServer implements acp.Agent {
383391
};
384392
this.sessions.set(sessionId, sessionState);
385393

386-
const requestedMcpServers = request.mcpServers ?? [];
387-
if (requestedMcpServers.length > 0) {
394+
if (requestedMcpServers.length > 0 && mcpServerStartupVersion !== null) {
388395
this.pendingMcpStartupSessions.set(sessionId, {
389396
requestedServers: new Set(requestedMcpServers.map(server => server.name)),
397+
afterVersion: mcpServerStartupVersion,
390398
});
391-
this.publishMcpStartupStatusAsync(sessionId, mcpStartupVersion);
399+
this.publishMcpStartupStatusAsync(sessionId);
392400
}
393401

394402
await this.availableCommands.publish(sessionId);
@@ -633,11 +641,10 @@ export class CodexAcpServer implements acp.Agent {
633641
return sessionState;
634642
}
635643

636-
private async resolveSessionMcpServers(
644+
private resolveSessionMcpServers(
637645
mcpServers: Array<acp.McpServer>,
638-
mcpStartupVersion: number,
639646
recoverFromStartup: boolean,
640-
): Promise<Array<string>> {
647+
): Array<string> {
641648
// Explicit MCP servers from the request are the primary source of truth for the session.
642649
const requestedServerNames = getRequestedMcpServerNames(mcpServers);
643650
if (requestedServerNames.length > 0) {
@@ -647,27 +654,31 @@ export class CodexAcpServer implements acp.Agent {
647654
if (!recoverFromStartup) {
648655
return [];
649656
}
650-
// loadSession/resumeSession may omit mcpServers; in that case recover the ready names
651-
// from the startup event associated with this thread start/resume checkpoint.
652-
logger.log("Recovering MCP servers from startup state...");
653-
return await this.runWithProcessCheck(() => this.codexAcpClient.awaitMcpStartup(mcpStartupVersion));
657+
// Without a thread-scoped startup completion event, loadSession/resumeSession can no longer
658+
// recover omitted session MCP server names. Treat the session set as unknown unless ACP
659+
// explicitly provided mcpServers in the request.
660+
logger.log("Skipping MCP server recovery for load/resume without explicit mcpServers");
661+
return [];
654662
}
655663

656-
private publishMcpStartupStatusAsync(sessionId: string, mcpStartupVersion: number): void {
657-
void this.doPublishMcpStartupStatus(sessionId, mcpStartupVersion);
664+
private publishMcpStartupStatusAsync(sessionId: string): void {
665+
void this.doPublishMcpStartupStatus(sessionId);
658666
}
659667

660-
private async doPublishMcpStartupStatus(sessionId: string, mcpStartupVersion: number): Promise<void> {
668+
private async doPublishMcpStartupStatus(sessionId: string): Promise<void> {
669+
const pendingStartup = this.pendingMcpStartupSessions.get(sessionId);
670+
if (!pendingStartup) {
671+
return;
672+
}
673+
661674
try {
662-
const mcpStartup = await this.runWithProcessCheck(() => this.codexAcpClient.awaitMcpStartupResult(mcpStartupVersion));
663-
const sessionState = this.sessions.get(sessionId);
664-
const pendingStartup = this.pendingMcpStartupSessions.get(sessionId);
665-
if (sessionState && pendingStartup) {
666-
sessionState.sessionMcpServers = mcpStartup.ready.filter(serverName =>
667-
pendingStartup.requestedServers.has(serverName)
668-
);
669-
}
670-
await this.publishMcpStartupStatus(sessionId, mcpStartup, pendingStartup?.requestedServers);
675+
const mcpStartup = await this.runWithProcessCheck(() =>
676+
this.codexAcpClient.awaitMcpServerStartup(
677+
Array.from(pendingStartup.requestedServers),
678+
pendingStartup.afterVersion,
679+
)
680+
);
681+
await this.publishMcpStartupStatus(sessionId, mcpStartup, pendingStartup.requestedServers);
671682
} catch (err) {
672683
logger.error(`Failed to publish MCP startup status for session ${sessionId}`, err);
673684
} finally {
@@ -677,7 +688,7 @@ export class CodexAcpServer implements acp.Agent {
677688

678689
private async publishMcpStartupStatus(
679690
sessionId: string,
680-
mcpStartup: McpStartupCompleteEvent,
691+
mcpStartup: McpStartupResult,
681692
requestedServers?: Set<string>
682693
): Promise<void> {
683694
const filteredStartup = requestedServers

0 commit comments

Comments
 (0)