From dfa5ef70792cba0357942a9dd984f13bd1c85574 Mon Sep 17 00:00:00 2001 From: Ben Rabinovich Date: Tue, 26 Aug 2025 22:25:55 +0300 Subject: [PATCH 1/6] chore: add plan for STATIC mode orchestrator reuse and tests --- .../01_plan.md | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 TASKS-fix-static-mode-orchestrator-dup-reg/01_plan.md diff --git a/TASKS-fix-static-mode-orchestrator-dup-reg/01_plan.md b/TASKS-fix-static-mode-orchestrator-dup-reg/01_plan.md new file mode 100644 index 0000000..7137ac5 --- /dev/null +++ b/TASKS-fix-static-mode-orchestrator-dup-reg/01_plan.md @@ -0,0 +1,32 @@ +# Fix STATIC mode duplicate tool registration + +## Objectives +- Prevent duplicate tool registrations in STATIC mode when multiple clients connect. +- Preserve DYNAMIC mode behavior. +- Optional: ensure DELETE /mcp tears down sessions. + +## Plan +1. Reuse initial orchestrator in STATIC mode per-client bundles (preferred) + - In createMcpServer, if mode === STATIC, return the base orchestrator from createBundle. + - Ensure no per-client startup enable runs on shared server. +2. Add guard to skip startup enabling for per-client orchestrators (fallback) + - If we must create per-client orchestrators, pass a flag to skip startup enabling. +3. Tests for duplicate registration prevention + - Start server in STATIC with a toolset. + - Simulate two clients; assert tools registered once. +4. Optional: DELETE /mcp enhancements + - On DELETE, close transport and evict session. +5. Docs updates + - README: Clarify STATIC vs DYNAMIC bundling and namespacing guidance. + +## Tasks +- [ ] Implement STATIC-mode orchestrator reuse in createMcpServer +- [ ] Add duplicate registration tests for STATIC mode +- [ ] Add fallback guard to avoid per-client startup enabling (if needed) +- [ ] Update README examples and notes +- [ ] (Optional) Close and evict sessions on DELETE /mcp + +## Verification +- STATIC mode: multiple clients -> no duplicate tool registration. +- DYNAMIC mode unaffected. +- README reflects accurate behavior. From 124bac9cb5c7ee382bc56fe524ccdc7f4189342d Mon Sep 17 00:00:00 2001 From: Ben Rabinovich Date: Tue, 26 Aug 2025 22:31:57 +0300 Subject: [PATCH 2/6] fix(server): reuse base orchestrator in STATIC mode bundles to avoid duplicate registrations --- src/server/createMcpServer.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/server/createMcpServer.ts b/src/server/createMcpServer.ts index ab33aeb..8a2e86f 100644 --- a/src/server/createMcpServer.ts +++ b/src/server/createMcpServer.ts @@ -71,9 +71,12 @@ export async function createMcpServer(options: CreateMcpServerOptions) { () => { // Create a server + orchestrator bundle // for a new client when needed - const createdServer: McpServer = - mode === "DYNAMIC" ? options.createServer() : baseServer; - const orchestrator = new ServerOrchestrator({ + if (mode === "STATIC") { + // Reuse the base server and orchestrator to avoid duplicate registrations + return { server: baseServer, orchestrator }; + } + const createdServer: McpServer = options.createServer(); + const createdOrchestrator = new ServerOrchestrator({ server: createdServer, catalog: options.catalog, moduleLoaders: options.moduleLoaders, @@ -86,7 +89,7 @@ export async function createMcpServer(options: CreateMcpServerOptions) { ? options.registerMetaTools : mode === "DYNAMIC", }); - return { server: createdServer, orchestrator }; + return { server: createdServer, orchestrator: createdOrchestrator }; }, options.http, options.configSchema From f76a7f226f373aef04a50cffd6639fe317426b13 Mon Sep 17 00:00:00 2001 From: Ben Rabinovich Date: Tue, 26 Aug 2025 22:34:16 +0300 Subject: [PATCH 3/6] test(server): ensure no duplicate tool registrations in STATIC mode across clients --- tests/createMcpServer.test.ts | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/createMcpServer.test.ts b/tests/createMcpServer.test.ts index 09d28a1..4a15105 100644 --- a/tests/createMcpServer.test.ts +++ b/tests/createMcpServer.test.ts @@ -124,4 +124,40 @@ describe("createMcpServer", () => { expect(s1.calls.includes("list_tools")).toBe(true); expect(s2.calls.includes("list_tools")).toBe(true); }); + + it("does not re-register tools in STATIC mode across multiple clients", async () => { + const f = makeFakeServerFactory(); + const staticCatalog = { + core: { + name: "Core", + description: "", + tools: [ + { + name: "ping", + description: "", + inputSchema: {}, + handler: async () => ({ content: [{ type: "text", text: "pong" }] }), + }, + ], + }, + } as any; + + await createMcpServer({ + catalog: staticCatalog, + startup: { mode: "STATIC", toolsets: ["core"] }, + createServer: f.createServer, + }); + + const base = f.created[0]; + // One registration at startup, namespaced by toolset key + expect(base.calls.filter((n) => n === "core.ping").length).toBe(1); + + const bundleFactory = (FastifyTransportMock as any).lastArgs?.[1]; + // Simulate two more clients (bundles) + bundleFactory(); + bundleFactory(); + + // No additional registrations should have occurred on the shared server + expect(base.calls.filter((n) => n === "core.ping").length).toBe(1); + }); }); From b994cb625c2f4aad67365876d25cfd2f31efc72e Mon Sep 17 00:00:00 2001 From: Ben Rabinovich Date: Tue, 26 Aug 2025 22:39:23 +0300 Subject: [PATCH 4/6] feat(transport): DELETE /mcp closes and evicts session; allow DI sessions map in bundle --- src/http/FastifyTransport.ts | 15 ++++++++- tests/fastifyTransport.test.ts | 57 ++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/src/http/FastifyTransport.ts b/src/http/FastifyTransport.ts index 03a5163..a466912 100644 --- a/src/http/FastifyTransport.ts +++ b/src/http/FastifyTransport.ts @@ -114,10 +114,12 @@ export class FastifyTransport { let bundle = useCache ? this.clientCache.get(clientId) : null; if (!bundle) { const created = this.createBundle(); + const providedSessions = (created as any).sessions; bundle = { server: created.server, orchestrator: created.orchestrator, - sessions: new Map(), + sessions: + providedSessions instanceof Map ? providedSessions : new Map(), }; if (useCache) this.clientCache.set(clientId, bundle); } @@ -230,6 +232,17 @@ export class FastifyTransport { id: null, }; } + try { + // Best-effort close and evict + if (typeof (transport as any).close === "function") { + try { + await (transport as any).close(); + } catch {} + } + } finally { + if (transport?.sessionId) bundle.sessions.delete(transport.sessionId); + else bundle.sessions.delete(sessionId); + } reply.code(204).send(); return reply; } diff --git a/tests/fastifyTransport.test.ts b/tests/fastifyTransport.test.ts index ed121ed..6cfa669 100644 --- a/tests/fastifyTransport.test.ts +++ b/tests/fastifyTransport.test.ts @@ -41,4 +41,61 @@ describe("FastifyTransport", () => { await transport.stop(); }); + + it("DELETE /mcp evicts session after close", async () => { + // Fake server that supports connect() + const server: any = { + async connect(_t: any) { + // no-op + }, + }; + const resolver = new ModuleResolver({ + catalog: { core: { name: "Core", description: "", tools: [] } } as any, + }); + const manager = new DynamicToolManager({ server, resolver }); + + const app = Fastify({ logger: false }); + // Stub createBundle with a minimal streamable transport-like object + const sessions = new Map(); + const bundle = { server, orchestrator: {} as any, sessions } as any; + + const transport = new FastifyTransport( + manager, + () => bundle, + { port: 0, logger: false, app } + ); + await transport.start(); + + const clientId = "c1"; + // Seed bundle in cache with a non-initialize POST (will 400 but caches bundle) + await app.inject({ + method: "POST", + url: "/mcp", + headers: { "mcp-client-id": clientId }, + payload: { jsonrpc: "2.0", id: 1, method: "unknown", params: {} }, + }); + + // Now create a fake session inside the cached bundle + const createdSessionId = "s-1"; + const storedTransport: any = { + sessionId: createdSessionId, + async handleRequest() {}, + async close() { + this._closed = true; + }, + }; + sessions.set(createdSessionId, storedTransport); + + // Attempt DELETE + const res = await app.inject({ + method: "DELETE", + url: "/mcp", + headers: { "mcp-client-id": clientId, "mcp-session-id": createdSessionId }, + }); + expect(res.statusCode).toBe(204); + expect(storedTransport._closed).toBe(true); + expect(sessions.has(createdSessionId)).toBe(false); + + await transport.stop(); + }); }); From c7fa68891aed88fae41a9e9464a37796760f1ab7 Mon Sep 17 00:00:00 2001 From: Ben Rabinovich Date: Tue, 26 Aug 2025 23:36:19 +0300 Subject: [PATCH 5/6] chore: update docs --- .../01_plan.md | 32 ------------------- 1 file changed, 32 deletions(-) delete mode 100644 TASKS-fix-static-mode-orchestrator-dup-reg/01_plan.md diff --git a/TASKS-fix-static-mode-orchestrator-dup-reg/01_plan.md b/TASKS-fix-static-mode-orchestrator-dup-reg/01_plan.md deleted file mode 100644 index 7137ac5..0000000 --- a/TASKS-fix-static-mode-orchestrator-dup-reg/01_plan.md +++ /dev/null @@ -1,32 +0,0 @@ -# Fix STATIC mode duplicate tool registration - -## Objectives -- Prevent duplicate tool registrations in STATIC mode when multiple clients connect. -- Preserve DYNAMIC mode behavior. -- Optional: ensure DELETE /mcp tears down sessions. - -## Plan -1. Reuse initial orchestrator in STATIC mode per-client bundles (preferred) - - In createMcpServer, if mode === STATIC, return the base orchestrator from createBundle. - - Ensure no per-client startup enable runs on shared server. -2. Add guard to skip startup enabling for per-client orchestrators (fallback) - - If we must create per-client orchestrators, pass a flag to skip startup enabling. -3. Tests for duplicate registration prevention - - Start server in STATIC with a toolset. - - Simulate two clients; assert tools registered once. -4. Optional: DELETE /mcp enhancements - - On DELETE, close transport and evict session. -5. Docs updates - - README: Clarify STATIC vs DYNAMIC bundling and namespacing guidance. - -## Tasks -- [ ] Implement STATIC-mode orchestrator reuse in createMcpServer -- [ ] Add duplicate registration tests for STATIC mode -- [ ] Add fallback guard to avoid per-client startup enabling (if needed) -- [ ] Update README examples and notes -- [ ] (Optional) Close and evict sessions on DELETE /mcp - -## Verification -- STATIC mode: multiple clients -> no duplicate tool registration. -- DYNAMIC mode unaffected. -- README reflects accurate behavior. From 786635e64d0625ecc896f17ec6a09bfac44bb8fb Mon Sep 17 00:00:00 2001 From: Ben Rabinovich Date: Tue, 26 Aug 2025 23:36:51 +0300 Subject: [PATCH 6/6] chore: update version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 51bc66f..080e838 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "toolception", - "version": "0.2.3", + "version": "0.2.4", "private": false, "type": "module", "main": "dist/index.js",