From 75a233a0001d9a3ec710e8d6f2f40f6ac4d43a4a Mon Sep 17 00:00:00 2001 From: Jeff Scott Ward Date: Wed, 12 Aug 2026 01:26:28 -0400 Subject: [PATCH] fix(mcp): show config source in startup failures --- packages/coding-agent/CHANGELOG.md | 4 ++ packages/coding-agent/src/mcp/manager.ts | 10 ++++- .../coding-agent/src/mcp/startup-events.ts | 21 +++++++--- .../src/modes/interactive-mode.ts | 14 ++++--- .../interactive-mode-mcp-connecting.test.ts | 5 ++- .../test/mcp-connection-status-events.test.ts | 41 +++++++++++++++++++ .../test/mcp-startup-events.test.ts | 34 +++++++++++++++ 7 files changed, 115 insertions(+), 14 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index b387e76fd..3b9dbe1df 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed MCP startup failures omitting the originating configuration path, so stale imported server entries can be traced to their source file. + ## [17.2.15] - 2026-08-12 ### Added diff --git a/packages/coding-agent/src/mcp/manager.ts b/packages/coding-agent/src/mcp/manager.ts index 60700e665..849fcf7b8 100644 --- a/packages/coding-agent/src/mcp/manager.ts +++ b/packages/coding-agent/src/mcp/manager.ts @@ -74,6 +74,12 @@ type TrackedPromise = { const STARTUP_TIMEOUT_MS = 250; +function createMcpStartupFailure(serverName: string, error: string, source?: SourceMeta): McpConnectionStatusEvent { + return source + ? { type: "failed", serverName, error, sourcePath: source.path } + : { type: "failed", serverName, error }; +} + /** * Per-server reconnect-storm circuit breaker. * @@ -563,7 +569,7 @@ export class MCPManager { if (this.#pendingToolLoads.get(name) !== toolsPromise) return; this.#pendingToolLoads.delete(name); const message = error instanceof Error ? error.message : String(error); - onStatus?.({ type: "failed", serverName: name, error: message }); + onStatus?.(createMcpStartupFailure(name, message, sources[name])); if (!allowBackgroundLogging || reportedErrors.has(name)) return; logger.error("MCP tool load failed", { path: `mcp:${name}`, error: message }); }); @@ -573,7 +579,7 @@ export class MCPManager { if (statusServerNames.length > 0 && onStatus) { onStatus({ type: "connecting", serverNames: statusServerNames }); for (const { name, message } of validationFailures) { - onStatus({ type: "failed", serverName: name, error: message }); + onStatus(createMcpStartupFailure(name, message, sources[name])); } } diff --git a/packages/coding-agent/src/mcp/startup-events.ts b/packages/coding-agent/src/mcp/startup-events.ts index 4b3e2c63e..3a97d3c76 100644 --- a/packages/coding-agent/src/mcp/startup-events.ts +++ b/packages/coding-agent/src/mcp/startup-events.ts @@ -3,15 +3,21 @@ import { replaceTabs, shortenPath, TRUNCATE_LENGTHS, truncateToWidth } from "../ export const MCP_CONNECTION_STATUS_EVENT_CHANNEL = "mcp:connection-status"; +export type McpConnectionFailure = { + serverName: string; + error: string; + sourcePath?: string; +}; + export type McpConnectionStatusEvent = | { type: "connecting"; serverNames: string[] } | { type: "connected"; serverName: string } - | { type: "failed"; serverName: string; error: string }; + | ({ type: "failed" } & McpConnectionFailure); export type McpConnectionStatusSnapshot = { pendingServers: readonly string[]; connectedServers: readonly string[]; - failedServers: readonly { serverName: string; error: string }[]; + failedServers: readonly McpConnectionFailure[]; }; function sanitizeMcpStatusText(value: string, maxWidth: number): string { @@ -55,8 +61,9 @@ export function formatMCPConnectingMessage(serverNames: readonly string[]): stri return `Connecting to MCP servers: ${formatServerList(serverNames)}…`; } -function formatFailedServer({ serverName, error }: { serverName: string; error: string }): string { - return `${sanitizeMcpServerName(serverName)}: ${sanitizeMcpStatusError(error)}`; +function formatFailedServer({ serverName, error, sourcePath }: McpConnectionFailure): string { + const source = sourcePath ? ` [config: ${sanitizeMcpStatusText(sourcePath, TRUNCATE_LENGTHS.CONTENT)}]` : ""; + return `${sanitizeMcpServerName(serverName)}${source}: ${sanitizeMcpStatusError(error)}`; } export function formatMCPConnectionStatusMessage(snapshot: McpConnectionStatusSnapshot): string { @@ -109,7 +116,11 @@ export function isMcpConnectionStatusEvent(data: unknown): data is McpConnection case "connected": return typeof data.serverName === "string"; case "failed": - return typeof data.serverName === "string" && typeof data.error === "string"; + return ( + typeof data.serverName === "string" && + typeof data.error === "string" && + (data.sourcePath === undefined || typeof data.sourcePath === "string") + ); default: return false; } diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 66bb5bccb..96563053d 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -88,6 +88,7 @@ import { formatMCPConnectionStatusMessage, isMcpConnectionStatusEvent, MCP_CONNECTION_STATUS_EVENT_CHANNEL, + type McpConnectionFailure, type McpConnectionStatusEvent, } from "../mcp/startup-events"; import { humanizePlanTitle, type PlanApprovalDetails, resolvePlanTitle } from "../plan-mode/approved-plan"; @@ -723,7 +724,7 @@ export class InteractiveMode implements InteractiveModeContext { #mcpStatusOrder: string[] = []; #mcpPendingServers = new Set(); #mcpConnectedServers = new Set(); - #mcpFailedServers = new Map(); + #mcpFailedServers = new Map(); #welcomeComponent?: WelcomeComponent; readonly #chatHost: ChatBlockHost = { requestRender: () => this.ui.requestRender() }; @@ -897,7 +898,10 @@ export class InteractiveMode implements InteractiveModeContext { this.#trackMcpStatusServer(event.serverName); this.#mcpPendingServers.delete(event.serverName); this.#mcpConnectedServers.delete(event.serverName); - this.#mcpFailedServers.set(event.serverName, event.error); + this.#mcpFailedServers.set(event.serverName, { + error: event.error, + sourcePath: event.sourcePath, + }); } const message = formatMCPConnectionStatusMessage({ @@ -918,10 +922,10 @@ export class InteractiveMode implements InteractiveModeContext { return this.#mcpStatusOrder.filter(serverName => servers.has(serverName)); } - #orderedMcpStatusFailures(): Array<{ serverName: string; error: string }> { + #orderedMcpStatusFailures(): McpConnectionFailure[] { return this.#mcpStatusOrder.flatMap(serverName => { - const error = this.#mcpFailedServers.get(serverName); - return error === undefined ? [] : [{ serverName, error }]; + const failure = this.#mcpFailedServers.get(serverName); + return failure === undefined ? [] : [{ serverName, ...failure }]; }); } diff --git a/packages/coding-agent/test/interactive-mode-mcp-connecting.test.ts b/packages/coding-agent/test/interactive-mode-mcp-connecting.test.ts index 85e00a133..e36efa1d0 100644 --- a/packages/coding-agent/test/interactive-mode-mcp-connecting.test.ts +++ b/packages/coding-agent/test/interactive-mode-mcp-connecting.test.ts @@ -127,6 +127,7 @@ describe("InteractiveMode MCP connection status", () => { type: "failed", serverName: "broken", error: "missing command", + sourcePath: "/tmp/codex/config.toml", } satisfies McpConnectionStatusEvent); eventBus.emit(MCP_CONNECTION_STATUS_EVENT_CHANNEL, { type: "connected", @@ -136,8 +137,8 @@ describe("InteractiveMode MCP connection status", () => { expect(showStatusSpy.mock.calls.map(call => call[0])).toEqual([ "Connecting to MCP servers: alpha, broken, slow…", "Connected: alpha. Still connecting: broken, slow…", - "Connected: alpha. Failed: broken: missing command. Still connecting: slow…", - "MCP finished with failures. Connected: alpha, slow. Failed: broken: missing command", + "Connected: alpha. Failed: broken [config: /tmp/codex/config.toml]: missing command. Still connecting: slow…", + "MCP finished with failures. Connected: alpha, slow. Failed: broken [config: /tmp/codex/config.toml]: missing command", ]); }); diff --git a/packages/coding-agent/test/mcp-connection-status-events.test.ts b/packages/coding-agent/test/mcp-connection-status-events.test.ts index 034b5fca3..54c14bad7 100644 --- a/packages/coding-agent/test/mcp-connection-status-events.test.ts +++ b/packages/coding-agent/test/mcp-connection-status-events.test.ts @@ -47,4 +47,45 @@ describe("MCPManager connection status events", () => { await manager.disconnectAll(); } }); + + it("includes the originating config path when a discovered server fails to start", async () => { + const manager = new MCPManager(workDir); + const events: McpConnectionStatusEvent[] = []; + const missingCommand = path.join(workDir, "missing-mcp-server"); + const configPath = path.join(os.homedir(), ".codex", "config.toml"); + + try { + const result = await manager.connectServers( + { + broken: { + type: "stdio", + command: missingCommand, + }, + }, + { + broken: { + provider: "codex", + providerName: "Codex", + path: configPath, + level: "user", + }, + }, + event => events.push(event), + ); + + const message = result.errors.get("broken") ?? ""; + expect(message).toMatch(/ENOENT|No such file|not found/i); + expect(events).toEqual([ + { type: "connecting", serverNames: ["broken"] }, + { + type: "failed", + serverName: "broken", + error: message, + sourcePath: configPath, + }, + ]); + } finally { + await manager.disconnectAll(); + } + }); }); diff --git a/packages/coding-agent/test/mcp-startup-events.test.ts b/packages/coding-agent/test/mcp-startup-events.test.ts index b8ac40a3e..34482601c 100644 --- a/packages/coding-agent/test/mcp-startup-events.test.ts +++ b/packages/coding-agent/test/mcp-startup-events.test.ts @@ -61,6 +61,24 @@ describe("mcp/startup-events — connection-status cross-module contract", () => expect(message).toContain("broken: failed at ~/.omp/mcp.log"); }); + it("keeps the config source and transport error visible under independent truncation", () => { + const message = formatMCPConnectionStatusMessage({ + pendingServers: [], + connectedServers: [], + failedServers: [ + { + serverName: "broken", + error: `ENOENT ${"missing executable ".repeat(10)}`, + sourcePath: `${os.homedir()}/.codex/config.toml`, + }, + ], + }); + + expect(message).not.toContain(os.homedir()); + expect(message).toContain("broken [config: ~/.codex/config.toml]: ENOENT"); + expect(message).toContain("…"); + }); + it("sanitizes server names before rendering them in status text", () => { const homePath = `${os.homedir()}/.omp`; const message = formatMCPConnectionStatusMessage({ @@ -99,6 +117,14 @@ describe("mcp/startup-events — connection-status cross-module contract", () => expect(isMcpConnectionStatusEvent({ type: "connecting", serverNames: [] })).toBe(true); expect(isMcpConnectionStatusEvent({ type: "connected", serverName: "a" })).toBe(true); expect(isMcpConnectionStatusEvent({ type: "failed", serverName: "a", error: "boom" })).toBe(true); + expect( + isMcpConnectionStatusEvent({ + type: "failed", + serverName: "a", + error: "boom", + sourcePath: "/tmp/config.toml", + }), + ).toBe(true); expect(isMcpConnectionStatusEvent(null)).toBe(false); expect(isMcpConnectionStatusEvent(undefined)).toBe(false); @@ -108,5 +134,13 @@ describe("mcp/startup-events — connection-status cross-module contract", () => expect(isMcpConnectionStatusEvent({ type: "connecting", serverNames: ["ok", 3] })).toBe(false); expect(isMcpConnectionStatusEvent({ type: "connected", serverName: 1 })).toBe(false); expect(isMcpConnectionStatusEvent({ type: "failed", serverName: "a" })).toBe(false); + expect( + isMcpConnectionStatusEvent({ + type: "failed", + serverName: "a", + error: "boom", + sourcePath: 42, + }), + ).toBe(false); }); });