fix(mcpl): a server that fails a tools re-list keeps its known tools

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.

src/framework.ts

@@ -13131,6 +13131,8 @@ export class AgentFramework {
 
     const tools: import('./types/index.js').ToolDefinition[] = [];
     const toolFeatureSets = new Map<string, string>();
+    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)`,
+          );
+        }
       }
     }
 

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<void>;
+};
+
+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 });
+  }
+});