From a2857cf8fe3df43d39cf94649e17a443d3382ae9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 17:53:25 +0000 Subject: [PATCH 1/3] fix(permissions): reject permission-aware clients whose startup toolsets failed to load Mirrors FastifyTransport: a not-ready bundle gets a 500 / -32603 on initialize and is dropped from the cache. Docs updated for 0.8.0. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01Jke6dnq5CNkrDFWbJSHJWr --- AGENTS.md | 2 +- CHANGELOG.md | 2 +- README.md | 1 + src/permissions/AGENTS.md | 1 + .../PermissionAwareFastifyTransport.ts | 18 ++++- tests/e2e/permissionBased.e2e.test.ts | 69 +++++++++++++++++++ 6 files changed, 90 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a513eee..2440186 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -48,7 +48,7 @@ These are cross-cutting; see leaf Intent Nodes for module-specific invariants. 1. **All tools → ToolRegistry** — Collision detection happens here only 2. **`mcp-client-id` required for /mcp** — POST, GET, DELETE all reject without header (400) 3. **Fail-secure** — Invalid inputs return empty objects/arrays, not errors -4. **Silent module failures** — Toolsets activate with partial tools if loaders fail +4. **Module loader errors are silent, registration failures are not** — A throwing loader is logged and the toolset activates with fewer tools. A failure while registering a startup toolset's tools (for example a duplicate tool name) makes `isReady()` return `false`: STATIC `createMcpServer` throws at startup and each transport answers the client's `initialize` with a 500 / `-32603` and drops the bundle 5. **Disable unregisters tools** — `disableToolset` removes the toolset's tools from the MCP server via the SDK's `RegisteredTool.remove()`; see `src/core/AGENTS.md` ## Module Index diff --git a/CHANGELOG.md b/CHANGELOG.md index 6610f86..8c1768f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ Releases before 0.7.0 are described in [GitHub Releases](https://github.com/code ### Breaking -- A STATIC client whose startup toolsets fail to register their tools now gets a JSON-RPC error (`-32603`, HTTP 500, "Failed to load toolsets for this client.") on `initialize` instead of connecting with only some of its tools. The failed bundle is dropped, so the client's next `initialize` retries the load. ([#33](https://github.com/code-rabi/toolception/pull/33)) +- A STATIC client (`createMcpServer`) or a permission-aware client (`createPermissionBasedMcpServer`) whose startup toolsets fail to register their tools now gets a JSON-RPC error (`-32603`, HTTP 500, "Failed to load toolsets for this client.") on `initialize` instead of connecting with only some of its tools. The failed bundle is dropped, so the client's next `initialize` retries the load. ([#33](https://github.com/code-rabi/toolception/pull/33), [#30](https://github.com/code-rabi/toolception/pull/30)) - `ServerOrchestrator.isReady()` now returns `false`, and `ensureReady()` throws, when registering a startup toolset's tools fails (for example a duplicate tool name, within a toolset or across toolsets when `namespaceToolsWithSetKey` is off). It used to report ready. `createMcpServer` therefore rejects at startup for such a catalog in STATIC mode, where it used to start with the partial toolset. `enableToolset` results (and the `enable_toolset` meta-tool output) gain an optional `code` on that failure. - Toolsets refused before registration (exposure policy, duplicate names in `startup.toolsets`) and module loader errors do not count as failures. - **Migration:** before upgrading, make sure no two startup toolsets register the same tool name (rename or remove the duplicate, or turn on `namespaceToolsWithSetKey`). A catalog that only worked because the collision was swallowed will now fail `initialize`, or fail at startup in STATIC mode. diff --git a/README.md b/README.md index 0f9484d..661dfb5 100644 --- a/README.md +++ b/README.md @@ -899,6 +899,7 @@ Notes - Policy is enforced at enable time (via meta-tools or static preload). - If both `allowlist` and `denylist` are present, the entry must be in `allowlist` and not in `denylist` to pass. - Namespacing is applied consistently at registration time and reflected in `GET /tools`. +- A startup toolset whose tools fail to register (for example a duplicate tool name, within a toolset or across toolsets when `namespaceToolsWithSetKey` is off) is a startup failure, not a partial start: `createMcpServer` in STATIC mode rejects at startup, and over HTTP the client's `initialize` returns a JSON-RPC `-32603` error (HTTP 500) and is retried on its next `initialize`. A module loader that throws is only logged, and its toolset activates with no tools. #### options.context (optional) diff --git a/src/permissions/AGENTS.md b/src/permissions/AGENTS.md index 492502d..47170e4 100644 --- a/src/permissions/AGENTS.md +++ b/src/permissions/AGENTS.md @@ -50,6 +50,7 @@ HTTP Request with mcp-client-id header → Create STATIC mode server with allowed toolsets only → Return bundle (cached for reuse) → On initialize: await bundle.orchestrator.isReady() before server.connect() — the SDK refuses capability registration after connect + → not ready → drop the bundle from the cache, respond 500 / JSON-RPC -32603; never connect with partial tools (same as FastifyTransport) ``` ## Anti-patterns diff --git a/src/permissions/PermissionAwareFastifyTransport.ts b/src/permissions/PermissionAwareFastifyTransport.ts index 02c7ef5..c93cf0e 100644 --- a/src/permissions/PermissionAwareFastifyTransport.ts +++ b/src/permissions/PermissionAwareFastifyTransport.ts @@ -364,7 +364,23 @@ export class PermissionAwareFastifyTransport { try { // Bundles may preload toolsets asynchronously; the SDK refuses to // register tool capabilities once connected, so wait for them first. - await bundle.orchestrator.isReady(); + const ready = await bundle.orchestrator.isReady(); + if (!ready) { + // A failed preload is cached on the orchestrator for good, so drop + // the bundle and let the client's next initialize start a fresh one. + if (this.clientCache.get(context.clientId) === bundle) { + this.clientCache.delete(context.clientId); + } + reply.code(500); + return { + jsonrpc: "2.0", + error: { + code: -32603, + message: "Failed to load toolsets for this client.", + }, + id: null, + }; + } await bundle.server.connect(transport); } catch (error) { reply.code(500); diff --git a/tests/e2e/permissionBased.e2e.test.ts b/tests/e2e/permissionBased.e2e.test.ts index 3c3dd99..807045b 100644 --- a/tests/e2e/permissionBased.e2e.test.ts +++ b/tests/e2e/permissionBased.e2e.test.ts @@ -472,6 +472,75 @@ describe("Permission-based E2E", () => { }); }); + describe("bundles whose startup toolsets fail to load", () => { + let transport: PermissionAwareFastifyTransport; + let port: number; + let flakyAttempts = 0; + + beforeAll(async () => { + port = await getAvailablePort(); + transport = PermissionAwareFastifyTransport.builder() + .defaultManager( + ServerOrchestrator.builder() + .server(new McpServer({ name: "base", version: "1.0.0" })) + .catalog(testCatalog) + .build() + .getManager() + ) + .createPermissionAwareBundle(async (context) => { + const server = new McpServer({ name: "failing-bundle", version: "1.0.0" }); + const orchestrator = ServerOrchestrator.builder() + .server(server) + .catalog({ + dup: { name: "Dup", description: "Duplicate tool name", modules: ["dup"] }, + }) + .moduleLoaders({ + // Returning one tool twice makes registration fail halfway through. + dup: async () => { + const tools = testCatalog.core.tools!; + const fail = + context.clientId === "failing-client" || + (context.clientId === "flaky-client" && flakyAttempts++ === 0); + return fail ? [...tools, ...tools] : tools; + }, + }) + .startup({ mode: "STATIC", toolsets: ["dup"] }) + .registerMetaTools(false) + .build(); + return { server, orchestrator, allowedToolsets: ["dup"], failedToolsets: [] }; + }) + .port(port) + .build(); + await transport.start(); + }); + + afterAll(async () => { + await transport.stop(); + }); + + const connect = async (clientId: string) => { + const client = new Client({ name: "test", version: "1.0.0" }); + await client.connect( + new StreamableHTTPClientTransport( + new URL(`http://localhost:${port}/mcp`), + { requestInit: { headers: { "mcp-client-id": clientId } } } + ) + ); + return client; + }; + + it("rejects the client instead of connecting with partial tools", async () => { + await expect(connect("failing-client")).rejects.toThrow(/Failed to load toolsets/); + }); + + it("lets the same client retry after a failed load", async () => { + await expect(connect("flaky-client")).rejects.toThrow(/Failed to load toolsets/); + const client = await connect("flaky-client"); + expect(extractToolNames(await client.listTools())).toContain("dup.ping"); + await client.close(); + }); + }); + describe("bundles that preload asynchronously", () => { const slowCatalog = { slow: { From 034995724394249f94e5638e94a5235d4c78cc19 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 18:09:44 +0000 Subject: [PATCH 2/3] docs: scope the 0.8.0 failed-startup notes to createMcpServer createPermissionBasedMcpServer enables toolsets in its bundle factory and continues on partial failure, so the isReady() rejection does not apply. Pin that with a public-factory test and label the transport-level tests. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01Jke6dnq5CNkrDFWbJSHJWr --- AGENTS.md | 2 +- CHANGELOG.md | 5 ++- README.md | 2 +- src/permissions/AGENTS.md | 2 +- tests/e2e/permissionBased.e2e.test.ts | 63 ++++++++++++++++++++++++++- 5 files changed, 67 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2440186..da766ee 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -48,7 +48,7 @@ These are cross-cutting; see leaf Intent Nodes for module-specific invariants. 1. **All tools → ToolRegistry** — Collision detection happens here only 2. **`mcp-client-id` required for /mcp** — POST, GET, DELETE all reject without header (400) 3. **Fail-secure** — Invalid inputs return empty objects/arrays, not errors -4. **Module loader errors are silent, registration failures are not** — A throwing loader is logged and the toolset activates with fewer tools. A failure while registering a startup toolset's tools (for example a duplicate tool name) makes `isReady()` return `false`: STATIC `createMcpServer` throws at startup and each transport answers the client's `initialize` with a 500 / `-32603` and drops the bundle +4. **Module loader errors are silent, registration failures are not** — A throwing loader is logged and the toolset activates with fewer tools. A failure while registering a startup toolset's tools (for example a duplicate tool name) makes `isReady()` return `false`: STATIC `createMcpServer` throws at startup and its transport answers the client's `initialize` with a 500 / `-32603` and drops the bundle. `createPermissionBasedMcpServer` is the exception: it enables toolsets in its bundle factory and continues on partial failure 5. **Disable unregisters tools** — `disableToolset` removes the toolset's tools from the MCP server via the SDK's `RegisteredTool.remove()`; see `src/core/AGENTS.md` ## Module Index diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c1768f..f306233 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,9 @@ Releases before 0.7.0 are described in [GitHub Releases](https://github.com/code ### Breaking -- A STATIC client (`createMcpServer`) or a permission-aware client (`createPermissionBasedMcpServer`) whose startup toolsets fail to register their tools now gets a JSON-RPC error (`-32603`, HTTP 500, "Failed to load toolsets for this client.") on `initialize` instead of connecting with only some of its tools. The failed bundle is dropped, so the client's next `initialize` retries the load. ([#33](https://github.com/code-rabi/toolception/pull/33), [#30](https://github.com/code-rabi/toolception/pull/30)) +- A STATIC client of `createMcpServer` whose startup toolsets fail to register their tools now gets a JSON-RPC error (`-32603`, HTTP 500, "Failed to load toolsets for this client.") on `initialize` instead of connecting with only some of its tools. The failed bundle is dropped, so the client's next `initialize` retries the load. ([#33](https://github.com/code-rabi/toolception/pull/33)) - `ServerOrchestrator.isReady()` now returns `false`, and `ensureReady()` throws, when registering a startup toolset's tools fails (for example a duplicate tool name, within a toolset or across toolsets when `namespaceToolsWithSetKey` is off). It used to report ready. `createMcpServer` therefore rejects at startup for such a catalog in STATIC mode, where it used to start with the partial toolset. `enableToolset` results (and the `enable_toolset` meta-tool output) gain an optional `code` on that failure. + - `createPermissionBasedMcpServer` is not affected: it enables each client's permitted toolsets itself, still connects a client when only some of them fail (`failedToolsets`), and still answers 403 when all of them fail. - Toolsets refused before registration (exposure policy, duplicate names in `startup.toolsets`) and module loader errors do not count as failures. - **Migration:** before upgrading, make sure no two startup toolsets register the same tool name (rename or remove the duplicate, or turn on `namespaceToolsWithSetKey`). A catalog that only worked because the collision was swallowed will now fail `initialize`, or fail at startup in STATIC mode. @@ -26,7 +27,7 @@ Releases before 0.7.0 are described in [GitHub Releases](https://github.com/code ### Fixed -- `PermissionAwareFastifyTransport` waits for the client's orchestrator to be ready before connecting, so a bundle that preloads toolsets in the background no longer answers `tools/list` with `Method not found`. ([#30](https://github.com/code-rabi/toolception/pull/30)) +- `PermissionAwareFastifyTransport` waits for the client's orchestrator to be ready before connecting and, like `FastifyTransport`, answers `500` / `-32603` and drops the bundle when it is not ready. This hardens bundles that preload toolsets in the background, which only a hand-built `createPermissionAwareBundle` callback can produce; `createPermissionBasedMcpServer` already loaded toolsets before returning, so its behaviour is unchanged. ([#30](https://github.com/code-rabi/toolception/pull/30)) ## [0.7.0] diff --git a/README.md b/README.md index 661dfb5..f116c67 100644 --- a/README.md +++ b/README.md @@ -899,7 +899,7 @@ Notes - Policy is enforced at enable time (via meta-tools or static preload). - If both `allowlist` and `denylist` are present, the entry must be in `allowlist` and not in `denylist` to pass. - Namespacing is applied consistently at registration time and reflected in `GET /tools`. -- A startup toolset whose tools fail to register (for example a duplicate tool name, within a toolset or across toolsets when `namespaceToolsWithSetKey` is off) is a startup failure, not a partial start: `createMcpServer` in STATIC mode rejects at startup, and over HTTP the client's `initialize` returns a JSON-RPC `-32603` error (HTTP 500) and is retried on its next `initialize`. A module loader that throws is only logged, and its toolset activates with no tools. +- A startup toolset whose tools fail to register (for example a duplicate tool name, within a toolset or across toolsets when `namespaceToolsWithSetKey` is off) is a startup failure, not a partial start: `createMcpServer` in STATIC mode rejects at startup, and over HTTP the client's `initialize` returns a JSON-RPC `-32603` error (HTTP 500) and is retried on its next `initialize`. A module loader that throws is only logged, and its toolset activates with fewer tools. #### options.context (optional) diff --git a/src/permissions/AGENTS.md b/src/permissions/AGENTS.md index 47170e4..5822305 100644 --- a/src/permissions/AGENTS.md +++ b/src/permissions/AGENTS.md @@ -50,7 +50,7 @@ HTTP Request with mcp-client-id header → Create STATIC mode server with allowed toolsets only → Return bundle (cached for reuse) → On initialize: await bundle.orchestrator.isReady() before server.connect() — the SDK refuses capability registration after connect - → not ready → drop the bundle from the cache, respond 500 / JSON-RPC -32603; never connect with partial tools (same as FastifyTransport) + → not ready → drop the bundle from the cache, respond 500 / JSON-RPC -32603 (same as FastifyTransport). Only a hand-built bundle whose orchestrator preloads startup toolsets itself can be not ready: `createPermissionBasedMcpServer` builds its orchestrators with `toolsets: []` and enables the permitted ones in the factory, so invariant 4 (partial success continues) still holds there ``` ## Anti-patterns diff --git a/tests/e2e/permissionBased.e2e.test.ts b/tests/e2e/permissionBased.e2e.test.ts index 807045b..1683693 100644 --- a/tests/e2e/permissionBased.e2e.test.ts +++ b/tests/e2e/permissionBased.e2e.test.ts @@ -472,7 +472,66 @@ describe("Permission-based E2E", () => { }); }); - describe("bundles whose startup toolsets fail to load", () => { + describe("when a permitted toolset fails to register its tools", () => { + let server: Awaited>; + let port: number; + + beforeAll(async () => { + port = await getAvailablePort(); + server = await createPermissionBasedMcpServer({ + createServer: () => + new McpServer({ name: "test-partial-failure", version: "1.0.0" }), + catalog: { + good: { name: "Good", description: "Loads", modules: ["good"] }, + dup: { name: "Dup", description: "Duplicate tool name", modules: ["dup"] }, + }, + moduleLoaders: { + good: async () => testCatalog.core.tools!, + // One tool twice: registration fails halfway through. + dup: async () => [...testCatalog.core.tools!, ...testCatalog.core.tools!], + }, + permissions: { source: "headers" }, + http: { port }, + }); + await server.start(); + }); + + afterAll(async () => { + await server.close(); + }); + + const connect = async (clientId: string, toolsets: string) => { + const client = new Client({ name: "test", version: "1.0.0" }); + await client.connect( + new StreamableHTTPClientTransport( + new URL(`http://localhost:${port}/mcp`), + { + requestInit: { + headers: { + "mcp-client-id": clientId, + "mcp-toolset-permissions": toolsets, + }, + }, + } + ) + ); + return client; + }; + + // Pins the documented permission behaviour: partial success continues, + // so the failed-load rejection of createMcpServer does not apply here. + it("still connects with the toolsets that registered", async () => { + const client = await connect("partial-client", "good,dup"); + expect(extractToolNames(await client.listTools())).toContain("good.ping"); + await client.close(); + }); + + it("refuses a client whose permitted toolsets all failed", async () => { + await expect(connect("all-failed-client", "dup")).rejects.toThrow(); + }); + }); + + describe("transport level: bundles whose startup toolsets fail to load", () => { let transport: PermissionAwareFastifyTransport; let port: number; let flakyAttempts = 0; @@ -541,7 +600,7 @@ describe("Permission-based E2E", () => { }); }); - describe("bundles that preload asynchronously", () => { + describe("transport level: bundles that preload asynchronously", () => { const slowCatalog = { slow: { name: "Slow", From 1c58bb8a512e601ce3525f12d0763a7c45b448a9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 18:11:34 +0000 Subject: [PATCH 3/3] review: tighten permission-factory assertions, call #30 internal hardening Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01Jke6dnq5CNkrDFWbJSHJWr --- CHANGELOG.md | 2 +- tests/e2e/permissionBased.e2e.test.ts | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f306233..e0c5ac6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ Releases before 0.7.0 are described in [GitHub Releases](https://github.com/code ### Fixed -- `PermissionAwareFastifyTransport` waits for the client's orchestrator to be ready before connecting and, like `FastifyTransport`, answers `500` / `-32603` and drops the bundle when it is not ready. This hardens bundles that preload toolsets in the background, which only a hand-built `createPermissionAwareBundle` callback can produce; `createPermissionBasedMcpServer` already loaded toolsets before returning, so its behaviour is unchanged. ([#30](https://github.com/code-rabi/toolception/pull/30)) +- `PermissionAwareFastifyTransport` waits for the client's orchestrator to be ready before connecting and, like `FastifyTransport`, answers `500` / `-32603` and drops the bundle when it is not ready. Internal hardening: the class is not part of the public API and `createPermissionBasedMcpServer` already loaded toolsets before returning, so its behaviour is unchanged. ([#30](https://github.com/code-rabi/toolception/pull/30)) ## [0.7.0] diff --git a/tests/e2e/permissionBased.e2e.test.ts b/tests/e2e/permissionBased.e2e.test.ts index 1683693..133690d 100644 --- a/tests/e2e/permissionBased.e2e.test.ts +++ b/tests/e2e/permissionBased.e2e.test.ts @@ -522,12 +522,14 @@ describe("Permission-based E2E", () => { // so the failed-load rejection of createMcpServer does not apply here. it("still connects with the toolsets that registered", async () => { const client = await connect("partial-client", "good,dup"); - expect(extractToolNames(await client.listTools())).toContain("good.ping"); + const toolNames = extractToolNames(await client.listTools()); + expect(toolNames).toContain("good.ping"); + expect(toolNames).not.toContain("dup.ping"); await client.close(); }); it("refuses a client whose permitted toolsets all failed", async () => { - await expect(connect("all-failed-client", "dup")).rejects.toThrow(); + await expect(connect("all-failed-client", "dup")).rejects.toThrow(/Access denied/); }); });