Skip to content

Commit a79e28f

Browse files
committed
fix: various mcp fixes and error/warnings
1 parent 63bd5dc commit a79e28f

File tree

5 files changed

+68
-57
lines changed

5 files changed

+68
-57
lines changed

extensions/cli/src/configLoader.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,8 @@ async function loadFromSource(
194194
injectBlocks,
195195
);
196196

197+
// TODO this is currently skipped because we are forcing default config
198+
// Because models add on won't work for injected blocks e.g. default model, (only default config)
197199
case "no-config":
198200
return await unrollPackageIdentifiersAsConfigYaml(
199201
injectBlocks,

extensions/cli/src/services/MCPService.ts

Lines changed: 52 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ export class MCPService
5050
private connections: Map<string, ServerConnection> = new Map();
5151
private assistant: AssistantConfig | null = null;
5252
private isShuttingDown = false;
53-
private isHeadless = false;
5453

5554
getDependencies(): string[] {
5655
return [SERVICE_NAMES.CONFIG];
@@ -71,16 +70,14 @@ export class MCPService
7170
*/
7271
async doInitialize(
7372
assistant: AssistantConfig,
74-
waitForConnections = false,
73+
hasAgentFile: boolean,
74+
isHeadless: boolean | undefined,
7575
): Promise<MCPServiceState> {
7676
logger.debug("Initializing MCPService", {
7777
configName: assistant.name,
7878
serverCount: assistant.mcpServers?.length || 0,
7979
});
8080

81-
// Store headless mode flag
82-
this.isHeadless = waitForConnections;
83-
8481
await this.shutdownConnections();
8582

8683
this.assistant = assistant;
@@ -102,23 +99,24 @@ export class MCPService
10299
logger.debug("MCP connections established", {
103100
connectionCount: connections.length,
104101
});
105-
this.updateState();
106102
},
107103
);
108-
if (waitForConnections) {
104+
if (isHeadless || hasAgentFile) {
109105
await connectionInit;
110106

111-
// In headless mode, throw error if any MCP server failed to connect
112-
const failedConnections = Array.from(this.connections.values()).filter(
113-
(c) => c.status === "error",
114-
);
115-
if (failedConnections.length > 0) {
116-
const errorMessages = failedConnections.map(
117-
(c) => `${c.config?.name}: ${c.error}`,
118-
);
119-
throw new Error(
120-
`MCP server(s) failed to load in headless mode:\n${errorMessages.join("\n")}`,
107+
if (isHeadless) {
108+
// With headless or agent, throw error if any MCP server failed to connect
109+
const failedConnections = Array.from(this.connections.values()).filter(
110+
(c) => c.status === "error",
121111
);
112+
if (failedConnections.length > 0) {
113+
const errorMessages = failedConnections.map(
114+
(c) => `${c.config?.name}: ${c.error}`,
115+
);
116+
throw new Error(
117+
`MCP server(s) failed to load:\n${errorMessages.join("\n")}`,
118+
);
119+
}
122120
}
123121
} else {
124122
this.updateState();
@@ -249,7 +247,7 @@ export class MCPService
249247
this.updateState();
250248

251249
try {
252-
const client = await this.getConnectedClient(serverConfig);
250+
const client = await this.getConnectedClient(serverConfig, connection);
253251

254252
connection.client = client;
255253
connection.status = "connected";
@@ -276,13 +274,6 @@ export class MCPService
276274
name: serverName,
277275
error: errorMessage,
278276
});
279-
280-
// In headless mode, throw error on capability failures
281-
if (this.isHeadless) {
282-
throw new Error(
283-
`Failed to load prompts from MCP server ${serverName}: ${errorMessage}`,
284-
);
285-
}
286277
}
287278
}
288279

@@ -300,30 +291,27 @@ export class MCPService
300291
name: serverName,
301292
error: errorMessage,
302293
});
303-
304-
// In headless mode, throw error on capability failures
305-
if (this.isHeadless) {
306-
throw new Error(
307-
`Failed to load tools from MCP server ${serverName}: ${errorMessage}`,
308-
);
309-
}
310294
}
311295
}
312296

313297
logger.debug("MCP server connected successfully", { name: serverName });
314298
} catch (error) {
315299
const errorMessage = getErrorString(error);
316300
connection.status = "error";
317-
connection.error = errorMessage;
301+
302+
// Convert any warnings to error at time of connection failure
303+
if (connection.warnings.length > 0) {
304+
const stderrContent = connection.warnings.join("\n");
305+
connection.error = `${errorMessage}\n\nServer stderr:\n${stderrContent}`;
306+
connection.warnings = [];
307+
} else {
308+
connection.error = errorMessage;
309+
}
310+
318311
logger.error("Failed to connect to MCP server", {
319312
name: serverName,
320-
error: errorMessage,
313+
error: connection.error,
321314
});
322-
323-
// In headless mode, re-throw the error to fail fast
324-
if (this.isHeadless) {
325-
throw error;
326-
}
327315
}
328316

329317
this.updateState();
@@ -392,6 +380,7 @@ export class MCPService
392380
*/
393381
private async getConnectedClient(
394382
serverConfig: MCPServerConfig,
383+
connection: ServerConnection,
395384
): Promise<Client> {
396385
const client = new Client(
397386
{ name: "continue-cli-client", version: "1.0.0" },
@@ -404,7 +393,7 @@ export class MCPService
404393
name: serverConfig.name,
405394
command: serverConfig.command,
406395
});
407-
const transport = this.constructStdioTransport(serverConfig);
396+
const transport = this.constructStdioTransport(serverConfig, connection);
408397
await client.connect(transport, {});
409398
} else {
410399
// SSE/HTTP: if type isn't explicit: try http and fall back to sse
@@ -424,11 +413,14 @@ export class MCPService
424413
} catch (error: unknown) {
425414
// on authorization error, use "mcp-remote" with stdio transport to connect
426415
if (is401Error(error)) {
427-
const transport = this.constructStdioTransport({
428-
name: serverConfig.name,
429-
command: "npx",
430-
args: ["-y", "mcp-remote", serverConfig.url],
431-
});
416+
const transport = this.constructStdioTransport(
417+
{
418+
name: serverConfig.name,
419+
command: "npx",
420+
args: ["-y", "mcp-remote", serverConfig.url],
421+
},
422+
connection,
423+
);
432424
await client.connect(transport, {});
433425
} else {
434426
throw error;
@@ -507,6 +499,7 @@ export class MCPService
507499
}
508500
private constructStdioTransport(
509501
serverConfig: StdioMcpServer,
502+
connection: ServerConnection,
510503
): StdioClientTransport {
511504
const env: Record<string, string> = serverConfig.env || {};
512505
if (process.env) {
@@ -517,12 +510,24 @@ export class MCPService
517510
}
518511
}
519512

520-
return new StdioClientTransport({
513+
const transport = new StdioClientTransport({
521514
command: serverConfig.command,
522515
args: serverConfig.args || [],
523516
env,
524517
cwd: serverConfig.cwd,
525-
stderr: "ignore",
518+
stderr: "pipe",
526519
});
520+
521+
const stderrStream = transport.stderr;
522+
if (stderrStream) {
523+
stderrStream.on("data", (data: Buffer) => {
524+
const stderrOutput = data.toString().trim();
525+
if (stderrOutput) {
526+
connection.warnings.push(stderrOutput);
527+
}
528+
});
529+
}
530+
531+
return transport;
527532
}
528533
}

extensions/cli/src/services/index.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
ApiClientServiceState,
2424
AuthServiceState,
2525
ConfigServiceState,
26+
MCPServiceState,
2627
SERVICE_NAMES,
2728
ServiceInitOptions,
2829
} from "./types.js";
@@ -114,7 +115,7 @@ export async function initializeServices(initOptions: ServiceInitOptions = {}) {
114115
SERVICE_NAMES.TOOL_PERMISSIONS,
115116
async () => {
116117
const [mcpState, agentFileState] = await Promise.all([
117-
serviceContainer.get<AuthServiceState>(SERVICE_NAMES.MCP),
118+
serviceContainer.get<MCPServiceState>(SERVICE_NAMES.MCP),
118119
serviceContainer.get<AgentFileServiceState>(SERVICE_NAMES.AGENT_FILE),
119120
]);
120121

@@ -264,7 +265,8 @@ export async function initializeServices(initOptions: ServiceInitOptions = {}) {
264265
}
265266
return mcpService.initialize(
266267
configState.config,
267-
initOptions.headless || initOptions.options?.agent,
268+
!!initOptions.options?.agent,
269+
initOptions.headless,
268270
);
269271
},
270272
[SERVICE_NAMES.CONFIG], // Depends on config

extensions/cli/src/ui/MCPSelector.tsx

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -267,8 +267,6 @@ export const MCPSelector: React.FC<MCPSelectorProps> = ({ onCancel }) => {
267267
});
268268

269269
const handleMainMenuSelect = async (value: string) => {
270-
if (!mcpService) return;
271-
272270
if (value.startsWith("server:")) {
273271
// Handle server selection
274272
const serverName = value.replace("server:", "");
@@ -280,11 +278,11 @@ export const MCPSelector: React.FC<MCPSelectorProps> = ({ onCancel }) => {
280278

281279
switch (value) {
282280
case "restart-all":
283-
await mcpService.restartAllServers();
281+
await mcpService?.restartAllServers();
284282
setMessage("All servers restarted");
285283
break;
286284
case "stop-all":
287-
await mcpService.shutdownConnections();
285+
await mcpService?.shutdownConnections();
288286
setMessage("All servers stopped");
289287
break;
290288
case "explore-mcp-servers":
@@ -300,15 +298,20 @@ export const MCPSelector: React.FC<MCPSelectorProps> = ({ onCancel }) => {
300298
};
301299

302300
const handleServerAction = async (action: string) => {
303-
if (!mcpService || !selectedServer) return;
304301
try {
305302
switch (action) {
306303
case "restart":
307-
await mcpService.restartServer(selectedServer);
304+
if (!selectedServer) {
305+
return;
306+
}
307+
await mcpService?.restartServer(selectedServer);
308308
setMessage(`Server "${selectedServer}" restarted`);
309309
break;
310310
case "stop":
311-
await mcpService.stopServer(selectedServer);
311+
if (!selectedServer) {
312+
return;
313+
}
314+
await mcpService?.stopServer(selectedServer);
312315
setMessage(`Server "${selectedServer}" stopped`);
313316
break;
314317
case "back":

packages/config-yaml/src/load/unroll.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ export function parseConfigYaml(configYaml: string): ConfigYaml {
4444
cause: "result.success was false",
4545
});
4646
} catch (e) {
47-
console.error("Failed to parse rolled assistant:", configYaml);
4847
if (
4948
e instanceof Error &&
5049
"cause" in e &&

0 commit comments

Comments
 (0)