diff --git a/.changeset/mcp-retry-unhealthy-servers.md b/.changeset/mcp-retry-unhealthy-servers.md new file mode 100644 index 000000000..d031d4a55 --- /dev/null +++ b/.changeset/mcp-retry-unhealthy-servers.md @@ -0,0 +1,5 @@ +--- +'@nanocollective/nanocoder': minor +--- + +Retry unhealthy MCP servers on periodic health checks with exponential backoff and restore tools automatically when servers recover diff --git a/docs/configuration/mcp-configuration.md b/docs/configuration/mcp-configuration.md index 81c6e226a..79255b2ab 100644 --- a/docs/configuration/mcp-configuration.md +++ b/docs/configuration/mcp-configuration.md @@ -200,7 +200,7 @@ These fields work with all transport types: | `alwaysAllow` | Array of tool names that skip confirmation prompts | | `enabled` | Whether the server is active (default: `true`). `false` skips it entirely — no connection, no tools registered | | `timeout` | Connection timeout in milliseconds. Bounds the connection handshake and the initial tool listing; individual tool calls keep the MCP SDK's default timeout (60 seconds) | -| `healthCheckInterval` | Optional MCP ping interval in milliseconds (default: `30000`). Set to `0` to disable periodic health checks. Transport errors trigger a liveness ping even when periodic checks are disabled; recoverable errors preserve tools if the server responds. Health pings use `timeout` when configured, otherwise 10 seconds. A failed ping or transport close marks the server unhealthy and removes its tools until MCP servers are reinitialized or the session is restarted. | +| `healthCheckInterval` | Optional MCP ping interval in milliseconds (default: `30000`). Set to `0` to disable periodic health checks. Transport errors trigger a liveness ping even when periodic checks are disabled; recoverable errors preserve tools if the server responds. Health pings use `timeout` when configured, otherwise 10 seconds. A failed ping or transport close marks the server unhealthy and removes its tools; unhealthy servers continue to be checked with exponential backoff and automatically restore their tools when they respond again. | | `tags` | Array of tags, shown as `#tag` labels in `/mcp` output | ## Auto-Approve Tools diff --git a/source/mcp/mcp-client.spec.ts b/source/mcp/mcp-client.spec.ts index d67385ae8..d77ccb789 100644 --- a/source/mcp/mcp-client.spec.ts +++ b/source/mcp/mcp-client.spec.ts @@ -1441,7 +1441,229 @@ test('MCPClient health check marks a server unhealthy and removes its tools', as t.false(client.isServerConnected(serverName)); t.deepEqual(client.getServerTools(serverName), []); t.is(client.getServerInfo(serverName)?.health, 'unhealthy'); - clearInterval((client as any).healthTimers.get(serverName)); + clearTimeout((client as any).healthTimers.get(serverName)); +}); + +test('MCPClient health check retries unhealthy server and restores its tools when ping succeeds', async t => { + let pingCalls = 0; + const transport: {onclose?: () => void; onerror?: (error: Error) => void} = {}; + const client = new SeamMCPClient({ + async ping() { + pingCalls++; + if (pingCalls === 1) { + throw new Error('temporary network hiccup'); + } + return {}; + }, + async listTools() { + return { + tools: [ + { + name: 'restored_tool', + description: 'restored tool description', + inputSchema: {type: 'object'}, + }, + ], + }; + }, + getServerCapabilities() { + return {tools: {}}; + }, + }); + const serverName = 'recovering-server'; + (client as any).clients.set(serverName, (client as any).injected); + (client as any).serverConfigs.set(serverName, { + name: serverName, + transport: 'http', + url: 'http://localhost:1/mcp', + }); + (client as any).serverTools.set(serverName, [ + {name: 'restored_tool', serverName}, + ]); + (client as any).health.set(serverName, 'connected'); + (client as any).startHealthChecks( + serverName, + (client as any).injected, + transport, + 10, + ); + + // Wait for first ping to fail and second ping to recover + await new Promise(resolve => { + const deadline = Date.now() + 1000; + const waitForPings = () => { + if (pingCalls >= 2 || Date.now() >= deadline) { + resolve(); + return; + } + setTimeout(waitForPings, 5); + }; + waitForPings(); + }); + + t.true(pingCalls >= 2); + t.true(client.isServerConnected(serverName)); + t.is(client.getServerInfo(serverName)?.health, 'connected'); + const tools = client.getServerTools(serverName); + t.is(tools.length, 1); + t.is(tools[0].name, 'restored_tool'); + clearTimeout((client as any).healthTimers.get(serverName)); +}); + +test('MCPClient health check treats JSON-RPC MethodNotFound as alive and recovers tools', async t => { + let pingCalls = 0; + const transport: {onclose?: () => void; onerror?: (error: Error) => void} = {}; + const client = new SeamMCPClient({ + async ping() { + pingCalls++; + const error = new Error('Method not found'); + (error as any).code = -32601; + throw error; + }, + async listTools() { + return { + tools: [ + { + name: 'alive_tool', + description: 'alive tool description', + inputSchema: {type: 'object'}, + }, + ], + }; + }, + getServerCapabilities() { + return {tools: {}}; + }, + }); + const serverName = 'method-not-found-server'; + (client as any).clients.set(serverName, (client as any).injected); + (client as any).serverConfigs.set(serverName, { + name: serverName, + transport: 'stdio', + }); + (client as any).serverTools.set(serverName, []); + (client as any).health.set(serverName, 'unhealthy'); + (client as any).startHealthChecks( + serverName, + (client as any).injected, + transport, + 10, + ); + + await new Promise(resolve => { + const deadline = Date.now() + 1000; + const waitForPing = () => { + if (pingCalls > 0 || Date.now() >= deadline) { + resolve(); + return; + } + setTimeout(waitForPing, 5); + }; + waitForPing(); + }); + + t.true(pingCalls > 0); + t.true(client.isServerConnected(serverName)); + t.is(client.getServerInfo(serverName)?.health, 'connected'); + const tools = client.getServerTools(serverName); + t.is(tools.length, 1); + t.is(tools[0].name, 'alive_tool'); + clearTimeout((client as any).healthTimers.get(serverName)); +}); + +test('MCPClient health check applies exponential backoff on consecutive failures and resets on recovery', async t => { + let pingCalls = 0; + let pingShouldFail = true; + + const client = new SeamMCPClient({ + async ping() { + pingCalls++; + if (pingShouldFail) { + throw new Error('connection refused'); + } + return {}; + }, + async listTools() { + return { + tools: [ + { + name: 'backoff_tool', + description: 'backoff tool desc', + inputSchema: {type: 'object'}, + }, + ], + }; + }, + getServerCapabilities() { + return {tools: {}}; + }, + }); + + const serverName = 'backoff-server'; + (client as any).clients.set(serverName, (client as any).injected); + (client as any).serverConfigs.set(serverName, { + name: serverName, + transport: 'http', + url: 'http://localhost:1/mcp', + }); + (client as any).serverTools.set(serverName, [ + {name: 'backoff_tool', serverName}, + ]); + (client as any).health.set(serverName, 'connected'); + + const transport: {onclose?: () => void; onerror?: (error: Error) => void} = {}; + (client as any).startHealthChecks( + serverName, + (client as any).injected, + transport, + 10, + ); + + // First failure happens at initial 10ms timer + await new Promise(resolve => { + const check = () => { + if ((client as any).healthFailureCounts.get(serverName) === 1) { + resolve(); + return; + } + setTimeout(check, 5); + }; + check(); + }); + + t.is(client.getServerInfo(serverName)?.health, 'unhealthy'); + t.is((client as any).healthFailureCounts.get(serverName), 1); + + // Wait for second failure -> failure count = 2 + await new Promise(resolve => { + const check = () => { + if ((client as any).healthFailureCounts.get(serverName) >= 2) { + resolve(); + return; + } + setTimeout(check, 5); + }; + check(); + }); + t.is((client as any).healthFailureCounts.get(serverName), 2); + + // Now allow ping to succeed -> should recover and reset failure count + pingShouldFail = false; + await new Promise(resolve => { + const check = () => { + if (client.getServerInfo(serverName)?.health === 'connected') { + resolve(); + return; + } + setTimeout(check, 5); + }; + check(); + }); + + t.is(client.getServerInfo(serverName)?.health, 'connected'); + t.is((client as any).healthFailureCounts.get(serverName), undefined); + t.is(client.getServerTools(serverName).length, 1); + clearTimeout((client as any).healthTimers.get(serverName)); }); test('MCPClient transport close marks a server unhealthy immediately', t => { @@ -1475,7 +1697,7 @@ test('MCPClient transport close marks a server unhealthy immediately', t => { t.false(client.isServerConnected(serverName)); t.is(client.getServerInfo(serverName)?.health, 'unhealthy'); t.deepEqual(client.getServerTools(serverName), []); - clearInterval((client as any).healthTimers.get(serverName)); + clearTimeout((client as any).healthTimers.get(serverName)); }); for (const pingFails of [false, true]) { diff --git a/source/mcp/mcp-client.ts b/source/mcp/mcp-client.ts index 42cf8a890..b49b09490 100644 --- a/source/mcp/mcp-client.ts +++ b/source/mcp/mcp-client.ts @@ -69,7 +69,8 @@ export class MCPClient { private isConnected: boolean = false; private health: Map = new Map(); private healthErrors: Map = new Map(); - private healthTimers: Map> = new Map(); + private healthTimers: Map> = new Map(); + private healthFailureCounts: Map = new Map(); private healthChecksInFlight = new Set(); private healthListeners = new Set<(change: MCPHealthChange) => void>(); private closing = new Set(); @@ -146,6 +147,99 @@ export class MCPClient { this.toolMappingCache = null; } + private isLivenessResponse(error: unknown): boolean { + if (!error || typeof error !== 'object') return false; + const code = (error as {code?: number}).code; + // JSON-RPC MethodNotFound (-32601) means the server received the ping and responded + return code === -32601; + } + + private async restoreServerHealth( + serverName: string, + client: Client, + ): Promise { + if (this.closing.has(serverName)) return; + + try { + // Re-discover tools from this server + const toolsResult = await client.listTools(); + const tools: MCPTool[] = toolsResult.tools.map(tool => ({ + name: tool.name, + description: tool.description || undefined, + inputSchema: isPlainObject(tool.inputSchema) + ? (tool.inputSchema as MCPToolInputSchema) + : undefined, + serverName, + readOnly: tool.annotations?.readOnlyHint === true, + })); + this.serverTools.set(serverName, tools); + + // Re-discover resources if declared + const capabilities = client.getServerCapabilities(); + if (capabilities?.resources) { + try { + const resourcesResult = await client.listResources(); + const resources: MCPResource[] = resourcesResult.resources.map( + resource => ({ + uri: resource.uri, + name: resource.name, + description: resource.description || undefined, + mimeType: resource.mimeType || undefined, + serverName, + }), + ); + this.serverResources.set(serverName, resources); + } catch (error) { + this.logger.warn('MCP resources/list failed on recovery', { + serverName, + error: formatError(error), + }); + } + } + + // Re-discover prompts if declared + if (capabilities?.prompts) { + try { + const promptsResult = await client.listPrompts(); + const prompts: MCPPrompt[] = promptsResult.prompts.map(prompt => ({ + name: prompt.name, + description: prompt.description || undefined, + arguments: prompt.arguments?.map(arg => ({ + name: arg.name, + description: arg.description || undefined, + required: arg.required || false, + })), + serverName, + })); + this.serverPrompts.set(serverName, prompts); + } catch (error) { + this.logger.warn('MCP prompts/list failed on recovery', { + serverName, + error: formatError(error), + }); + } + } + + this.toolMappingCache = null; + this.setServerHealth(serverName, 'connected'); + this.logger.info( + `MCP server "${serverName}" recovered and restored tools`, + { + serverName, + toolCount: tools.length, + }, + ); + } catch (error) { + this.logger.warn( + `Failed to rediscover tools for recovered MCP server "${serverName}"`, + { + serverName, + error: formatError(error), + }, + ); + } + } + private startHealthChecks( serverName: string, client: Client, @@ -155,10 +249,27 @@ export class MCPClient { ): void { const markUnhealthy = (error?: unknown) => this.markServerUnhealthy(serverName, error); + const scheduleNext = (delay: number) => { + if ( + this.closing.has(serverName) || + interval <= 0 || + typeof client.ping !== 'function' + ) { + return; + } + const existingTimer = this.healthTimers.get(serverName); + if (existingTimer) { + clearTimeout(existingTimer); + } + const timer = setTimeout(() => void check(), delay); + if (typeof timer === 'object' && 'unref' in timer) timer.unref(); + this.healthTimers.set(serverName, timer); + }; const check = async () => { + const currentStatus = this.health.get(serverName); if ( this.closing.has(serverName) || - this.health.get(serverName) !== 'connected' || + (currentStatus !== 'connected' && currentStatus !== 'unhealthy') || this.healthChecksInFlight.has(serverName) || typeof client.ping !== 'function' ) { @@ -167,11 +278,34 @@ export class MCPClient { this.healthChecksInFlight.add(serverName); try { await client.ping(pingOptions ?? {timeout: 10_000}); + if (this.health.get(serverName) === 'unhealthy') { + await this.restoreServerHealth(serverName, client); + } + this.healthFailureCounts.delete(serverName); } catch (error) { - // A pending ping can reject after an intentional disconnect. - if (this.health.get(serverName) === 'connected') markUnhealthy(error); + if (this.isLivenessResponse(error)) { + if (this.health.get(serverName) === 'unhealthy') { + await this.restoreServerHealth(serverName, client); + } + this.healthFailureCounts.delete(serverName); + } else { + // A pending ping can reject after an intentional disconnect. + if (this.health.get(serverName) === 'connected') { + markUnhealthy(error); + } + const currentFailures = + (this.healthFailureCounts.get(serverName) ?? 0) + 1; + this.healthFailureCounts.set(serverName, currentFailures); + } } finally { this.healthChecksInFlight.delete(serverName); + if (!this.closing.has(serverName)) { + const failureCount = this.healthFailureCounts.get(serverName) ?? 0; + const multiplier = + failureCount > 0 ? Math.min(2 ** (failureCount - 1), 8) : 1; + const nextDelay = Math.min(interval * multiplier, 300_000); + scheduleNext(nextDelay); + } } }; const previousOnClose = transport.onclose; @@ -194,9 +328,7 @@ export class MCPClient { } }; if (interval <= 0 || typeof client.ping !== 'function') return; - const timer = setInterval(() => void check(), interval); - if (typeof timer === 'object' && 'unref' in timer) timer.unref(); - this.healthTimers.set(serverName, timer); + scheduleNext(interval); } async connectToServer(server: MCPServer): Promise { @@ -662,7 +794,7 @@ export class MCPClient { * * @returns Array of tool entries with name, AI SDK tool, and handler function */ - getToolEntries(): Array<{ + getToolEntries(forServerName?: string): Array<{ name: string; tool: AISDKCoreTool; handler: (args: Record) => Promise; @@ -679,6 +811,7 @@ export class MCPClient { const nativeTools = this.getNativeToolsRegistry(); for (const [serverName, serverTools] of this.serverTools.entries()) { + if (forServerName && serverName !== forServerName) continue; for (const mcpTool of serverTools) { const toolName = mcpTool.name; @@ -1216,8 +1349,9 @@ export class MCPClient { for (const [serverName, client] of this.clients.entries()) { this.closing.add(serverName); const timer = this.healthTimers.get(serverName); - if (timer) clearInterval(timer); + if (timer) clearTimeout(timer); this.healthTimers.delete(serverName); + this.healthFailureCounts.delete(serverName); this.healthChecksInFlight.delete(serverName); try { await client.close(); diff --git a/source/tools/tool-manager.spec.ts b/source/tools/tool-manager.spec.ts index 596af0dc1..06568dedd 100644 --- a/source/tools/tool-manager.spec.ts +++ b/source/tools/tool-manager.spec.ts @@ -196,6 +196,59 @@ test('initializeMCP - unregisters tools when a server becomes unhealthy', async t.false(manager.getToolNames().includes(toolName)); }); +test('ToolManager re-registers MCP tools when server recovers to connected state', async t => { + let healthListener: + | ((change: {serverName: string; status: 'unhealthy' | 'connected'}) => void) + | undefined; + const toolName = 'mcp_health_server_tool'; + const fakeClient = { + onHealthChange(listener: typeof healthListener) { + healthListener = listener; + return () => {}; + }, + async connectToServers() { + return [{serverName: 'health-server', success: true, toolCount: 1}]; + }, + getToolEntries(_serverName?: string) { + return [ + { + name: toolName, + tool: {description: 'health tool', execute: async () => 'ok'} as never, + handler: async () => 'ok', + }, + ]; + }, + getNativeToolsRegistry() { + return {[toolName]: {}}; + }, + getToolMapping() { + return new Map([ + [ + toolName, + { + serverName: 'health-server', + originalName: 'health_tool', + readOnly: true, + }, + ], + ]); + }, + disconnect: async () => {}, + }; + const manager = new HealthSeamToolManager(fakeClient); + + await manager.initializeMCP([ + {name: 'health-server', transport: 'http', url: 'http://localhost/mcp'}, + ]); + t.true(manager.getToolNames().includes(toolName)); + + healthListener?.({serverName: 'health-server', status: 'unhealthy'}); + t.false(manager.getToolNames().includes(toolName)); + + healthListener?.({serverName: 'health-server', status: 'connected'}); + t.true(manager.getToolNames().includes(toolName)); +}); + // ============================================================================ // Tool Access Tests // ============================================================================ diff --git a/source/tools/tool-manager.ts b/source/tools/tool-manager.ts index 3411af98f..37ac7597b 100644 --- a/source/tools/tool-manager.ts +++ b/source/tools/tool-manager.ts @@ -179,6 +179,10 @@ export class ToolManager { change.serverName, ); this.registry.unregisterMany(toolNames); + } else if (change.status === 'connected') { + const entries = + this.mcpClient?.getToolEntries(change.serverName) || []; + this.registry.registerMany(entries); } }), ].filter((unsubscribe): unsubscribe is () => void => !!unsubscribe);