From 62553ee30aa96fab80df263154a3ce5a88788b81 Mon Sep 17 00:00:00 2001 From: Ian de Marcellus <50731233+ian-de-marcellus@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:54:53 +0200 Subject: [PATCH] fix(mcpl): a server that fails a tools re-list keeps its known tools Every reconnect re-lists ALL servers' tools (handleToolsListChanged), and a server whose tools/list threw was skipped silently, so its tools vanished. One connector reconnecting while another was briefly down therefore changed the tool list, the top of every request, forcing a full prompt-cache rewrite, hiding the down server's tools unannounced (the diff announces only NEW tools), and forcing a second rewrite when they came back. A failed re-list now carries that server's previously known tools (and their feature sets) forward, with one stderr line; a call to a down server fails on its own, as before. A server that never listed tools contributes nothing, as before. Test: two servers listed; one fails a re-list and keeps its tools while the other picks up a new one; recovery. Mutation-checked. 1044 pass. Co-Authored-By: Claude Opus 5.5 (1M context) --- changelog.d/mcpl-relist-keeps-tools.fixed.md | 1 + src/framework.ts | 23 +++++++- test/mcpl-tools-keep-on-failed-relist.test.ts | 57 +++++++++++++++++++ 3 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 changelog.d/mcpl-relist-keeps-tools.fixed.md create mode 100644 test/mcpl-tools-keep-on-failed-relist.test.ts diff --git a/changelog.d/mcpl-relist-keeps-tools.fixed.md b/changelog.d/mcpl-relist-keeps-tools.fixed.md new file mode 100644 index 000000000..90bbd0ecc --- /dev/null +++ b/changelog.d/mcpl-relist-keeps-tools.fixed.md @@ -0,0 +1 @@ +- A server that fails `tools/list` during a re-list keeps its known tools instead of vanishing from the tool list. Every reconnect re-lists all servers, so one connector reconnecting while another was briefly down removed the second one's tools: a full prompt-cache rewrite (the tool list tops every request), the tools missing unannounced, and a second rewrite when they returned. A call to a down server fails on its own, as before. diff --git a/src/framework.ts b/src/framework.ts index d918a022a..766f0447e 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -13131,6 +13131,8 @@ export class AgentFramework { const tools: import('./types/index.js').ToolDefinition[] = []; const toolFeatureSets = new Map(); + const previousTools = this.mcplTools; + const previousFeatureSets = this.mcplToolFeatureSets; for (const server of this.mcplServerRegistry.getAllServers()) { const config = this.mcplServerConfigs.get(server.id); @@ -13160,8 +13162,25 @@ export class AgentFramework { toolFeatureSets.set(namespacedName, featureSet); } } - } catch { - // Server may not support tools/list — skip silently + } catch (error) { + // A server that can't answer right now (mid-reconnect, hung, briefly + // down) keeps the tools it had: dropping them would change the tool + // list, which is the top of every request, forcing a full prompt-cache + // rewrite now and another when they come back, and hiding the tools + // unannounced in between. A call to a down server fails on its own. + // A server that never listed tools (none known) contributes nothing. + const kept = previousTools.filter((t) => t.name.startsWith(`${prefix}--`)); + for (const tool of kept) { + tools.push(tool); + const fs = previousFeatureSets.get(tool.name); + if (fs !== undefined) toolFeatureSets.set(tool.name, fs); + } + if (kept.length > 0) { + console.error( + `[mcpl-tools] server "${server.id}" did not answer tools/list ` + + `(${error instanceof Error ? error.message : String(error)}); keeping its ${kept.length} known tool(s)`, + ); + } } } diff --git a/test/mcpl-tools-keep-on-failed-relist.test.ts b/test/mcpl-tools-keep-on-failed-relist.test.ts new file mode 100644 index 000000000..615793777 --- /dev/null +++ b/test/mcpl-tools-keep-on-failed-relist.test.ts @@ -0,0 +1,57 @@ +/** + * A server that fails tools/list during a re-list (every reconnect re-lists + * ALL servers) keeps its known tools instead of vanishing from the list: a + * vanishing tool changes the top of every request (full prompt-cache + * rewrite, twice) and hides the tools unannounced. + */ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { AgentFramework } from '../src/framework.js'; +import { MockMembrane } from './helpers/mock-membrane.js'; + +type Internals = { + mcplServerRegistry: unknown; + mcplTools: Array<{ name: string }>; + refreshMcplTools(): Promise; +}; + +test('a failed re-list keeps that server\'s known tools; others refresh normally', async () => { + const dir = mkdtempSync(join(tmpdir(), 'mcpl-keep-')); + const fw = await AgentFramework.create({ + storePath: join(dir, 'store'), membrane: new MockMembrane().asMembrane(), + agents: [{ name: 'assistant', model: 'test-model', systemPrompt: 'test' }], modules: [], + }); + try { + const state = { aTools: ['send'], bFails: false }; + const server = (id: string, list: () => string[]) => ({ + id, + sendToolsList: async () => { + if (id === 'b' && state.bFails) throw new Error('connection to "b" is closed'); + return { tools: list().map((name) => ({ name, description: name, inputSchema: { type: 'object' } })) }; + }, + }); + const servers = [server('a', () => state.aTools), server('b', () => ['read', 'write']), server('never', () => { throw new Error('no'); })]; + const internals = fw as unknown as Internals; + internals.mcplServerRegistry = { getAllServers: () => servers, closeAll: async () => {} }; + const names = () => internals.mcplTools.map((t) => t.name).sort(); + + await internals.refreshMcplTools(); + assert.deepEqual(names(), ['mcpl--a--send', 'mcpl--b--read', 'mcpl--b--write']); + + state.bFails = true; + state.aTools = ['send', 'react']; + await internals.refreshMcplTools(); + assert.deepEqual(names(), ['mcpl--a--react', 'mcpl--a--send', 'mcpl--b--read', 'mcpl--b--write'], + 'b keeps its tools; a picks up its new one'); + + state.bFails = false; + await internals.refreshMcplTools(); + assert.equal(internals.mcplTools.filter((t) => t.name.startsWith('mcpl--b--')).length, 2); + } finally { + await fw.stop(); + rmSync(dir, { recursive: true, force: true }); + } +});