From f9e80a55af6d6b70958e8cc99663bbecc48b13c8 Mon Sep 17 00:00:00 2001 From: Ben Rabinovich Date: Wed, 13 Aug 2025 12:01:53 +0300 Subject: [PATCH 1/5] refactor: package api to accept server or server factory --- README.md | 73 +++++++++++++-- package.json | 2 + src/server/createMcpServer.ts | 75 +++++---------- src/server/validateOptions.ts | 36 ++++++++ tests/createMcpServer.test.ts | 117 +++++++++++++++++++----- tests/mcpServerOptionsValidator.test.ts | 62 +++++++++++++ tests/smoke-e2e/client-demo.ts | 11 +++ tests/smoke-e2e/server-demo.ts | 11 ++- 8 files changed, 307 insertions(+), 80 deletions(-) create mode 100644 src/server/validateOptions.ts create mode 100644 tests/mcpServerOptionsValidator.test.ts diff --git a/README.md b/README.md index 5e1f4af..af273f3 100644 --- a/README.md +++ b/README.md @@ -75,14 +75,25 @@ const configSchema = { } as const; ``` -### Step 7: Create and start the MCP server +### Step 7: Create the MCP SDK server and start Toolception ```ts +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; + +// You own the SDK server; pass a factory into Toolception (required in DYNAMIC mode) +const createServer = () => + new McpServer({ + name: "my-mcp-server", + version: "0.0.0", + capabilities: { tools: { listChanged: true } }, + }); + const { start, close } = await createMcpServer({ catalog, moduleLoaders, startup: { mode: "DYNAMIC" }, http: { port: 3000 }, + createServer, // configSchema, // uncomment to expose at /.well-known/mcp-config }); await start(); @@ -103,9 +114,11 @@ process.on("SIGTERM", async () => { ## Static startup -Enable some or ALL toolsets at bootstrap: +Enable some or ALL toolsets at bootstrap. Note: provide a server or factory: ```ts +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; + const staticCatalog = { search: { name: "Search", description: "Search tools", modules: ["search"] }, quotes: { name: "Quotes", description: "Market quotes", modules: ["quotes"] }, @@ -115,12 +128,22 @@ createMcpServer({ catalog: staticCatalog, startup: { mode: "STATIC", toolsets: ["search", "quotes"] }, http: { port: 3001 }, + server: new McpServer({ + name: "static-1", + version: "0.0.0", + capabilities: { tools: { listChanged: false } }, + }), }); createMcpServer({ catalog: staticCatalog, startup: { mode: "STATIC", toolsets: "ALL" }, http: { port: 3002 }, + server: new McpServer({ + name: "static-2", + version: "0.0.0", + capabilities: { tools: { listChanged: false } }, + }), }); ``` @@ -128,7 +151,12 @@ createMcpServer({ ### createMcpServer(options) -Creates an MCP server with dynamic/static tool management and Fastify HTTP transport. +Wires your MCP SDK server to dynamic/static tool management and a Fastify HTTP transport. + +Requirements + +- Either `server` or `createServer` must be provided. +- In DYNAMIC mode, `createServer` is required (per-client isolation). Passing only `server` will throw. #### options.catalog (required) @@ -172,11 +200,44 @@ Creates an MCP server with dynamic/static tool management and Fastify HTTP trans - Fastify transport configuration. Defaults: host `0.0.0.0`, port `3000`, basePath `/`, CORS enabled, logger disabled. -#### options.mcp (optional) +#### options.server (optional) + +`McpServer` + +- A pre-created SDK server instance to use. + +#### options.createServer (optional) + +`() => McpServer` + +- Factory to create a fresh SDK server for each client bundle. If omitted, `options.server` is reused. -`{ name?: string; version?: string; capabilities?: Record }` +
+Validation and diagnostics + +![Error](https://img.shields.io/badge/Validation-Error-red) Required inputs + +- Error: neither `server` nor `createServer` provided. + +```text +createMcpServer: either `server` or `createServer` must be provided +``` + +- Error: `startup.mode === "DYNAMIC"` without `createServer`. + +```text +createMcpServer: in DYNAMIC mode `createServer` is required to create per-client server instances +``` + +![Warning](https://img.shields.io/badge/Validation-Warning-yellow) + +- Warning: both `server` and `createServer` provided. The base instance uses `server`; per-client bundles use `createServer`. + +```text +[TOOLCEPTION_CREATE_MCP_SERVER_BOTH] Both `server` and `createServer` were provided. The base instance will use `server`, and per-client bundles will use `createServer`. +``` -- Overrides MCP server identity and capabilities; `tools.listChanged` is set automatically based on mode. +
#### options.configSchema (optional) diff --git a/package.json b/package.json index ab5250f..26ab735 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,8 @@ "test": "vitest", "test:run": "vitest run", "test:coverage": "vitest run --coverage", + "dev:server-demo": "tsx tests/smoke-e2e/server-demo.ts", + "dev:client-demo": "tsx tests/smoke-e2e/client-demo.ts", "prepublishOnly": "npm run typecheck && npm run build && npm run test:run" }, "peerDependencies": {}, diff --git a/src/server/createMcpServer.ts b/src/server/createMcpServer.ts index f4b9e09..b22de45 100644 --- a/src/server/createMcpServer.ts +++ b/src/server/createMcpServer.ts @@ -1,10 +1,11 @@ -import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import type { ExposurePolicy, Mode, ToolSetCatalog } from "../types/index.js"; import { ServerOrchestrator } from "../core/ServerOrchestrator.js"; import { FastifyTransport, type FastifyTransportOptions, } from "../http/FastifyTransport.js"; +import { McpServerOptionsValidator } from "./validateOptions.js"; export interface CreateMcpServerOptions { catalog: ToolSetCatalog; @@ -14,34 +15,24 @@ export interface CreateMcpServerOptions { startup?: { mode?: Exclude; toolsets?: string[] | "ALL" }; registerMetaTools?: boolean; http?: FastifyTransportOptions; - mcp?: { - name?: string; - version?: string; - capabilities?: Record; - }; + /** + * Provide an existing MCP server instance. If omitted, you must provide createServer. + * When provided together with createServer, this is used for the default (non-cached) manager, + * while createServer is used for per-client bundles. + */ + server?: McpServer; + /** + * Factory to create a fresh MCP server instance for each client bundle. + * If omitted, the provided `server` instance will be reused for all clients. + */ + createServer?: () => McpServer; configSchema?: object; } export async function createMcpServer(options: CreateMcpServerOptions) { const mode: Exclude = options.startup?.mode ?? "DYNAMIC"; - const name = options.mcp?.name ?? "mcp-dynamic-tooling"; - const version = options.mcp?.version ?? "0.0.0"; - const baseCaps = options.mcp?.capabilities ?? {}; - const mergedCaps = { - ...baseCaps, - tools: { - ...(typeof (baseCaps as any).tools === "object" - ? (baseCaps as any).tools - : {}), - // listChanged is internal-only and computed by mode - listChanged: mode === "DYNAMIC", - }, - } as any; - const server = new McpServer({ - name, - version, - capabilities: mergedCaps, - }); + McpServerOptionsValidator.validate(options); + const baseServer: McpServer = options.server ?? options.createServer!(); // Typed, guarded notifier type NotifierA = { @@ -67,12 +58,12 @@ export async function createMcpServer(options: CreateMcpServerOptions) { }; const orchestrator = new ServerOrchestrator({ - server, + server: baseServer, catalog: options.catalog, moduleLoaders: options.moduleLoaders, exposurePolicy: options.exposurePolicy, context: options.context, - notifyToolsListChanged: async () => notifyToolsChanged(server), + notifyToolsListChanged: async () => notifyToolsChanged(baseServer), startup: options.startup, registerMetaTools: options.registerMetaTools !== undefined @@ -84,46 +75,30 @@ export async function createMcpServer(options: CreateMcpServerOptions) { orchestrator.getManager(), () => { // Create a fresh server + orchestrator bundle for a new client when needed - const innerMode: Exclude = - options.startup?.mode ?? "DYNAMIC"; - const innerName = options.mcp?.name ?? name; - const innerVersion = options.mcp?.version ?? version; - const innerBaseCaps = options.mcp?.capabilities ?? baseCaps; - const innerMergedCaps = { - ...innerBaseCaps, - tools: { - ...(typeof (innerBaseCaps as any).tools === "object" - ? (innerBaseCaps as any).tools - : {}), - listChanged: innerMode === "DYNAMIC", - }, - } as any; - const server = new McpServer({ - name: innerName, - version: innerVersion, - capabilities: innerMergedCaps, - }); + const createdServer: McpServer = options.createServer + ? options.createServer() + : baseServer; const orchestrator = new ServerOrchestrator({ - server, + server: createdServer, catalog: options.catalog, moduleLoaders: options.moduleLoaders, exposurePolicy: options.exposurePolicy, context: options.context, - notifyToolsListChanged: async () => notifyToolsChanged(server), + notifyToolsListChanged: async () => notifyToolsChanged(createdServer), startup: options.startup, registerMetaTools: options.registerMetaTools !== undefined ? options.registerMetaTools - : innerMode === "DYNAMIC", + : mode === "DYNAMIC", }); - return { server, orchestrator }; + return { server: createdServer, orchestrator }; }, options.http, options.configSchema ); return { - server, + server: baseServer, start: async () => { await transport.start(); }, diff --git a/src/server/validateOptions.ts b/src/server/validateOptions.ts new file mode 100644 index 0000000..fd164fe --- /dev/null +++ b/src/server/validateOptions.ts @@ -0,0 +1,36 @@ +import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import type { Mode } from "../types/index.js"; + +export type CreateMcpServerValidationInput = { + server?: McpServer; + createServer?: () => McpServer; + startup?: { mode?: Exclude }; +}; + +export class McpServerOptionsValidator { + public static validate(input: CreateMcpServerValidationInput): void { + const mode: Exclude = input.startup?.mode ?? "DYNAMIC"; + + if (!input.server && !input.createServer) { + throw new Error( + "createMcpServer: either `server` or `createServer` must be provided" + ); + } + + if (input.server && input.createServer) { + // eslint-disable-next-line no-console + if (typeof process.emitWarning === "function") { + process.emitWarning( + "Both `server` and `createServer` were provided. The base instance will use `server`, and per-client bundles will use `createServer`.", + { code: "TOOLCEPTION_CREATE_MCP_SERVER_BOTH" } + ); + } + } + + if (mode === "DYNAMIC" && !input.createServer) { + throw new Error( + "createMcpServer: in DYNAMIC mode `createServer` is required to create per-client server instances" + ); + } + } +} diff --git a/tests/createMcpServer.test.ts b/tests/createMcpServer.test.ts index 53bb1eb..62dd8ae 100644 --- a/tests/createMcpServer.test.ts +++ b/tests/createMcpServer.test.ts @@ -1,19 +1,6 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -// Mock SDK McpServer to capture constructor args -vi.mock("@modelcontextprotocol/sdk/server/mcp.js", () => { - return { - McpServer: class McpServerMock { - public static lastArgs: any; - public server: any = {}; - constructor(args: any) { - (McpServerMock as any).lastArgs = args; - } - tool() {} - async connect() {} - }, - }; -}); +// We no longer construct McpServer inside the library; provide a simple fake // Mock FastifyTransport to capture constructor args and avoid opening sockets vi.mock("../src/http/FastifyTransport.js", () => { @@ -29,7 +16,6 @@ vi.mock("../src/http/FastifyTransport.js", () => { }; }); -import { McpServer as McpServerMock } from "@modelcontextprotocol/sdk/server/mcp.js"; import { FastifyTransport as FastifyTransportMock } from "../src/http/FastifyTransport.js"; import { createMcpServer } from "../src/server/createMcpServer.js"; @@ -37,20 +23,49 @@ const catalog = { core: { name: "Core", description: "", tools: [] }, } as any; +function makeFakeServer() { + const calls: string[] = []; + const server = { + tool: (name: string) => { + calls.push(name); + }, + } as any; + return { server, calls }; +} + +function makeFakeServerFactory() { + const created: Array<{ server: any; calls: string[] }> = []; + const createServer = () => { + const s = makeFakeServer(); + created.push(s); + return s.server; + }; + return { createServer, created } as const; +} + describe("createMcpServer", () => { beforeEach(() => { - (McpServerMock as any).lastArgs = undefined; (FastifyTransportMock as any).lastArgs = undefined; }); - it("sets listChanged true in dynamic mode, false in static mode", async () => { - await createMcpServer({ catalog, startup: { mode: "DYNAMIC" } }); - const dyn = (McpServerMock as any).lastArgs; - expect(dyn.capabilities.tools.listChanged).toBe(true); + it("registers meta-tools by default in dynamic mode, not in static mode", async () => { + const d = makeFakeServerFactory(); + await createMcpServer({ + catalog, + startup: { mode: "DYNAMIC" }, + createServer: d.createServer, + }); + const baseDyn = d.created[0]; + expect(baseDyn.calls.includes("list_tools")).toBe(true); + expect(baseDyn.calls.includes("list_toolsets")).toBe(true); - await createMcpServer({ catalog, startup: { mode: "STATIC" } }); - const stat = (McpServerMock as any).lastArgs; - expect(stat.capabilities.tools.listChanged).toBe(false); + const s = makeFakeServer(); + await createMcpServer({ + catalog, + startup: { mode: "STATIC" }, + server: s.server, + }); + expect(s.calls.length).toBe(0); }); it("passes configSchema to FastifyTransport constructor", async () => { @@ -58,13 +73,69 @@ describe("createMcpServer", () => { type: "object", properties: { FOO: { type: "string" } }, }; + const { createServer } = makeFakeServerFactory(); await createMcpServer({ catalog, startup: { mode: "DYNAMIC" }, + createServer, configSchema, }); const args = (FastifyTransportMock as any).lastArgs; // args: [manager, createBundle, httpOptions, configSchema] expect(args?.[3]).toEqual(configSchema); }); + + it("uses provided server when only server is set, and reuses it for bundles (STATIC)", async () => { + const base = makeFakeServer(); + await createMcpServer({ + catalog, + startup: { mode: "STATIC" }, + server: base.server, + }); + const bundleFactory = (FastifyTransportMock as any).lastArgs?.[1]; + const bundle = bundleFactory(); + expect(bundle.server).toBe(base.server); + }); + + it("uses createServer when only factory is set (base + per-client)", async () => { + const factory = makeFakeServerFactory(); + await createMcpServer({ + catalog, + startup: { mode: "DYNAMIC" }, + createServer: factory.createServer, + }); + // base server created immediately + expect(factory.created.length).toBe(1); + const base = factory.created[0]; + expect(base.calls.includes("list_tools")).toBe(true); + + // per-client bundle uses a fresh server + const bundleFactory = (FastifyTransportMock as any).lastArgs?.[1]; + const bundle = bundleFactory(); + expect(factory.created.length).toBe(2); + const perClient = factory.created[1]; + expect(bundle.server).toBe(perClient.server); + expect(perClient.calls.includes("list_tools")).toBe(true); + }); + + it("uses provided server for base and factory for bundles when both provided", async () => { + const base = makeFakeServer(); + const factory = makeFakeServerFactory(); + await createMcpServer({ + catalog, + startup: { mode: "DYNAMIC" }, + server: base.server, + createServer: factory.createServer, + }); + // base server remains the provided one + expect(factory.created.length).toBe(0); + const bundleFactory = (FastifyTransportMock as any).lastArgs?.[1]; + const bundle = bundleFactory(); + expect(factory.created.length).toBe(1); + const perClient = factory.created[0]; + expect(bundle.server).not.toBe(base.server); + expect(bundle.server).toBe(perClient.server); + expect(base.calls.includes("list_tools")).toBe(true); + expect(perClient.calls.includes("list_tools")).toBe(true); + }); }); diff --git a/tests/mcpServerOptionsValidator.test.ts b/tests/mcpServerOptionsValidator.test.ts new file mode 100644 index 0000000..beab4db --- /dev/null +++ b/tests/mcpServerOptionsValidator.test.ts @@ -0,0 +1,62 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { McpServerOptionsValidator } from "../src/server/validateOptions.js"; + +function fakeServer() { + return { tool() {} } as any; +} + +describe("McpServerOptionsValidator", () => { + const originalEmitWarning = process.emitWarning; + + beforeEach(() => { + // @ts-expect-error allow spy replacement + process.emitWarning = vi.fn(); + }); + + afterEach(() => { + // @ts-expect-error restore type + process.emitWarning = originalEmitWarning; + }); + + it("throws when neither server nor createServer provided", () => { + expect(() => + McpServerOptionsValidator.validate({ startup: { mode: "DYNAMIC" } }) + ).toThrow(/either `server` or `createServer`/); + }); + + it("throws in DYNAMIC mode without createServer", () => { + expect(() => + McpServerOptionsValidator.validate({ + server: fakeServer(), + startup: { mode: "DYNAMIC" }, + }) + ).toThrow(/DYNAMIC mode `createServer` is required/); + }); + + it("warns when both server and createServer provided", () => { + McpServerOptionsValidator.validate({ + server: fakeServer(), + createServer: () => fakeServer(), + startup: { mode: "STATIC" }, + }); + expect(process.emitWarning).toHaveBeenCalled(); + }); + + it("passes when STATIC with only server", () => { + expect(() => + McpServerOptionsValidator.validate({ + server: fakeServer(), + startup: { mode: "STATIC" }, + }) + ).not.toThrow(); + }); + + it("passes when DYNAMIC with createServer", () => { + expect(() => + McpServerOptionsValidator.validate({ + createServer: () => fakeServer(), + startup: { mode: "DYNAMIC" }, + }) + ).not.toThrow(); + }); +}); diff --git a/tests/smoke-e2e/client-demo.ts b/tests/smoke-e2e/client-demo.ts index 95b4b7a..1f84d3d 100644 --- a/tests/smoke-e2e/client-demo.ts +++ b/tests/smoke-e2e/client-demo.ts @@ -33,6 +33,12 @@ async function main() { arguments: {}, } as any); console.log("core.ping:", JSON.stringify(ping, null, 2)); + const pingText = (ping as any)?.content?.[0]?.text ?? ""; + if (!String(pingText).toLowerCase().includes("pong")) { + throw new Error( + "Smoke check failed: core.ping did not return expected text" + ); + } await client.callTool({ name: "enable_toolset", @@ -43,11 +49,16 @@ async function main() { arguments: { text: "hello" }, } as any); console.log("ext.echo:", JSON.stringify(echo, null, 2)); + const echoText = (echo as any)?.content?.[0]?.text ?? ""; + if (String(echoText) !== "hello") { + throw new Error("Smoke check failed: ext.echo did not echo expected text"); + } const listAfter = await client.listTools(); console.log("tools after:", JSON.stringify(listAfter, null, 2)); await client.close(); + console.log("Smoke test OK"); } main().catch((err) => { diff --git a/tests/smoke-e2e/server-demo.ts b/tests/smoke-e2e/server-demo.ts index 0845b62..188d158 100644 --- a/tests/smoke-e2e/server-demo.ts +++ b/tests/smoke-e2e/server-demo.ts @@ -1,5 +1,6 @@ // Run with: npx --yes tsx tests/smoke-e2e/server-demo.ts import { createMcpServer } from "../../src/server/createMcpServer.js"; +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import type { ToolSetCatalog, ModuleLoader } from "../../src/types/index.js"; import { z } from "zod"; @@ -36,12 +37,20 @@ const moduleLoaders: Record = { const PORT = Number(process.env.PORT ?? 3003); +// Provide SDK server instances externally +const createServer = () => + new McpServer({ + name: "toolception-server-demo", + version: "0.1.0", + capabilities: { tools: { listChanged: true } }, + }); + const { start, close } = await createMcpServer({ catalog, moduleLoaders, startup: { mode: "DYNAMIC" }, http: { port: PORT }, - mcp: { name: "toolception-server-demo", version: "0.1.0" }, + createServer, configSchema: { $schema: "https://json-schema.org/draft/2020-12/schema", type: "object", From 5c365bc034f852bb2a631455c06a4a3d85d9fb3e Mon Sep 17 00:00:00 2001 From: Ben Rabinovich Date: Wed, 13 Aug 2025 12:02:32 +0300 Subject: [PATCH 2/5] refactor: package minor bump --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 26ab735..240d1a8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "toolception", - "version": "0.1.0", + "version": "0.2.0", "private": false, "type": "module", "main": "dist/index.js", From 6598aa86023b91965d644c9b2f85f31103d6bf4f Mon Sep 17 00:00:00 2001 From: Ben Rabinovich Date: Wed, 13 Aug 2025 12:08:42 +0300 Subject: [PATCH 3/5] chore: cleanup --- src/server/validateOptions.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/server/validateOptions.ts b/src/server/validateOptions.ts index fd164fe..fb80562 100644 --- a/src/server/validateOptions.ts +++ b/src/server/validateOptions.ts @@ -18,7 +18,6 @@ export class McpServerOptionsValidator { } if (input.server && input.createServer) { - // eslint-disable-next-line no-console if (typeof process.emitWarning === "function") { process.emitWarning( "Both `server` and `createServer` were provided. The base instance will use `server`, and per-client bundles will use `createServer`.", From 650121df48bfe5473328f0f729a9b1038f0c5a6b Mon Sep 17 00:00:00 2001 From: Ben Rabinovich Date: Wed, 13 Aug 2025 19:15:12 +0300 Subject: [PATCH 4/5] chore: update API --- README.md | 135 +++++++++++++++++------- src/core/ServerOrchestrator.ts | 65 +++++++++++- src/mode/ModeResolver.ts | 89 ++++++++++++---- src/server/createMcpServer.ts | 29 +++-- src/server/validateOptions.ts | 35 ------ tests/createMcpServer.test.ts | 56 ++++------ tests/mcpServerOptionsValidator.test.ts | 62 ----------- tests/modeResolver.test.ts | 10 +- tests/serverOrchestrator.test.ts | 31 +++++- tests/smoke-e2e/README.md | 4 + tests/smoke-e2e/client-demo.ts | 23 ++-- tests/smoke-e2e/server-demo.ts | 10 +- 12 files changed, 325 insertions(+), 224 deletions(-) delete mode 100644 src/server/validateOptions.ts delete mode 100644 tests/mcpServerOptionsValidator.test.ts diff --git a/README.md b/README.md index af273f3..1c304ec 100644 --- a/README.md +++ b/README.md @@ -155,8 +155,9 @@ Wires your MCP SDK server to dynamic/static tool management and a Fastify HTTP t Requirements -- Either `server` or `createServer` must be provided. -- In DYNAMIC mode, `createServer` is required (per-client isolation). Passing only `server` will throw. +- `createServer` must be provided. +- In DYNAMIC mode, a fresh server instance is created per client via `createServer`. +- In STATIC mode, a single server instance is created once via `createServer` and reused for all clients. #### options.catalog (required) @@ -170,12 +171,59 @@ Requirements - Maps module keys to async loaders returning `McpToolDefinition[]`. Referenced by toolsets via `modules: [key]`. +Usage and behavior + +| Aspect | Details | +| ---------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Key naming | The object key is the module identifier referenced in `catalog[toolset].modules`. Example: `{ ext: async () => [...] }` and `modules: ["ext"]`. | +| Loader signature | `(context?: unknown) => Promise` or `McpToolDefinition[]` | +| When called | STATIC mode: at startup (for specified toolsets or ALL). DYNAMIC mode: when a toolset is enabled via meta-tools. | +| Return value | An array of tools to register. Tool names should be unique per toolset; if `namespaceToolsWithSetKey` is true, names are prefixed at registration. | +| Errors | Throwing rejects the enable/preload flow for that toolset and surfaces an error to the caller. | +| Idempotency | Loaders may be invoked multiple times across runs/clients. Keep them deterministic/idempotent. Implement internal caching if they perform expensive I/O. | + +Example + +```ts +const moduleLoaders = { + ext: async (ctx?: unknown) => [ + { + name: "echo", + description: "Echo back provided text", + inputSchema: { + type: "object", + properties: { text: { type: "string" } }, + required: ["text"], + }, + handler: async ({ text }: { text: string }) => ({ + content: [{ type: "text", text }], + }), + }, + ], +}; + +const catalog = { + ext: { name: "Extensions", description: "Extra tools", modules: ["ext"] }, +}; +``` + #### options.startup (optional) `{ mode?: "DYNAMIC" | "STATIC"; toolsets?: string[] | "ALL" }` - Controls startup behavior. In STATIC mode, pre-load specific toolsets (or ALL). In DYNAMIC, register meta-tools and load on demand. +Startup precedence and validation + +| Input | Effective mode | Toolset handling | Outcome/Notes | +| ---------------------------------------------------- | -------------- | ----------------------------------- | -------------------------------------------------------------------------------- | +| `startup.mode = "DYNAMIC"` (toolsets present or not) | DYNAMIC | `startup.toolsets` is ignored | Manage toolsets at runtime via meta-tools; logs a warning if `toolsets` provided | +| `startup.mode = "STATIC"`, `toolsets = "ALL"` | STATIC | Preload all toolsets from `catalog` | OK | +| `startup.mode = "STATIC"`, `toolsets = [names]` | STATIC | Validate names against `catalog` | Invalid names warn; if none valid remain → error | +| No `startup.mode`, `toolsets = "ALL"` | STATIC | Preload all toolsets | OK | +| No `startup.mode`, `toolsets = [names]` | STATIC | Validate names against `catalog` | Invalid names warn; if none valid remain → error | +| No `startup.mode`, no `toolsets` | DYNAMIC | No preloads | Default behavior; manage toolsets at runtime via meta-tools | + #### options.registerMetaTools (optional) `boolean` (default: true in DYNAMIC mode; false in STATIC unless explicitly set) @@ -186,58 +234,73 @@ Requirements `ExposurePolicy` -- Limits and namespacing for registered tools (e.g., `maxActiveToolsets`, `namespaceToolsWithSetKey`, `allowlist`/`denylist`). - -#### options.context (optional) - -`unknown` +- Controls which toolsets can be activated and how tools are named when registered. -- Arbitrary context passed to `moduleLoaders` during tool resolution. +| Field | Type | Purpose | Example | +| -------------------------- | ----------------------------- | ---------------------------------------------------------------------------------- | -------------------------------------------------------------------- | +| `maxActiveToolsets` | `number` | Limit how many toolsets can be active at once. Prevents tool bloat. | `{ maxActiveToolsets: 1 }` blocks enabling a second toolset | +| `namespaceToolsWithSetKey` | `boolean` | Prefix tool names with the toolset key when registering, to avoid name collisions. | With `true`, enabling `core` registers `core.ping` instead of `ping` | +| `allowlist` | `string[]` | Only these toolsets may be enabled. Others are denied. | `{ allowlist: ["core"] }` prevents enabling `ext` | +| `denylist` | `string[]` | These toolsets cannot be enabled. | `{ denylist: ["ext"] }` blocks `ext` | +| `onLimitExceeded` | `(attempted, active) => void` | Callback when `maxActiveToolsets` would be exceeded. | Log or telemetry hook | -#### options.http (optional) +Notes -`{ host?: string; port?: number; basePath?: string; cors?: boolean; logger?: boolean }` +- 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`. -- Fastify transport configuration. Defaults: host `0.0.0.0`, port `3000`, basePath `/`, CORS enabled, logger disabled. - -#### options.server (optional) - -`McpServer` - -- A pre-created SDK server instance to use. +#### options.context (optional) -#### options.createServer (optional) +`unknown` -`() => McpServer` +- Arbitrary context passed to `moduleLoaders` during tool resolution. -- Factory to create a fresh SDK server for each client bundle. If omitted, `options.server` is reused. +| Field | Type | Purpose | Example | +| --------- | --------- | -------------------------------------------------------------------------------------------- | -------------------------------------------------------------- | +| `context` | `unknown` | Extra data/injectables available to every `ModuleLoader(context)` call when resolving tools. | `{ db, cache, apiClients }` used inside loaders to build tools | -
-Validation and diagnostics +Notes -![Error](https://img.shields.io/badge/Validation-Error-red) Required inputs +- Only `moduleLoaders` receive `context`. Direct tools defined inline in `catalog` do not. +- Not exposed to clients over HTTP; it stays in-process on the server. +- Keep it lightweight and stable; prefer passing handles (e.g., db client) rather than huge data blobs. +- STATIC mode: loaders are invoked at startup with the same `context`. +- DYNAMIC mode: loaders are invoked at enable time with the same `context`. -- Error: neither `server` nor `createServer` provided. +Example -```text -createMcpServer: either `server` or `createServer` must be provided +```ts +const moduleLoaders = { + ext: async (ctx: any) => [ + { + name: "echo", + description: "Echo using a backing service", + inputSchema: { + type: "object", + properties: { text: { type: "string" } }, + required: ["text"], + }, + handler: async ({ text }: { text: string }) => { + const result = await ctx.apiClients.echoService.send(text); + return { content: [{ type: "text", text: result }] } as any; + }, + }, + ], +}; ``` -- Error: `startup.mode === "DYNAMIC"` without `createServer`. +#### options.http (optional) -```text -createMcpServer: in DYNAMIC mode `createServer` is required to create per-client server instances -``` +`{ host?: string; port?: number; basePath?: string; cors?: boolean; logger?: boolean }` -![Warning](https://img.shields.io/badge/Validation-Warning-yellow) +- Fastify transport configuration. Defaults: host `0.0.0.0`, port `3000`, basePath `/`, CORS enabled, logger disabled. -- Warning: both `server` and `createServer` provided. The base instance uses `server`; per-client bundles use `createServer`. +#### options.createServer (optional) -```text -[TOOLCEPTION_CREATE_MCP_SERVER_BOTH] Both `server` and `createServer` were provided. The base instance will use `server`, and per-client bundles will use `createServer`. -``` +`() => McpServer` -
+Required factory to create the SDK server instance(s). #### options.configSchema (optional) diff --git a/src/core/ServerOrchestrator.ts b/src/core/ServerOrchestrator.ts index acca43a..a7cc4ec 100644 --- a/src/core/ServerOrchestrator.ts +++ b/src/core/ServerOrchestrator.ts @@ -1,5 +1,5 @@ import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { ModeResolver } from "../mode/ModeResolver.js"; +import { ToolsetValidator } from "../mode/ModeResolver.js"; import { ModuleResolver } from "../mode/ModuleResolver.js"; import { DynamicToolManager } from "./DynamicToolManager.js"; import { registerMetaTools } from "../meta/registerMetaTools.js"; @@ -21,14 +21,16 @@ export class ServerOrchestrator { private readonly mode: Exclude; private readonly resolver: ModuleResolver; private readonly manager: DynamicToolManager; + private readonly toolsetValidator: ToolsetValidator; constructor(options: ServerOrchestratorOptions) { - const modeResolver = new ModeResolver(); + this.toolsetValidator = new ToolsetValidator(); const startup = options.startup ?? {}; - this.mode = startup.mode ?? "DYNAMIC"; + const resolved = this.resolveStartupConfig(startup, options.catalog); + this.mode = resolved.mode; this.resolver = new ModuleResolver({ catalog: options.catalog, - moduleLoaders: options.moduleLoaders as any, + moduleLoaders: options.moduleLoaders, }); const toolRegistry = new ToolRegistry({ namespaceWithToolset: @@ -49,7 +51,7 @@ export class ServerOrchestrator { } // Startup behavior - const initial = startup.toolsets; + const initial = resolved.toolsets; if (initial === "ALL") { void this.manager.enableToolsets(this.resolver.getAvailableToolsets()); } else if (Array.isArray(initial) && initial.length > 0) { @@ -57,6 +59,59 @@ export class ServerOrchestrator { } } + private resolveStartupConfig( + startup: { mode?: Exclude; toolsets?: string[] | "ALL" }, + catalog: ToolSetCatalog + ): { mode: Exclude; toolsets?: string[] | "ALL" } { + // Explicit mode dominates + if (startup.mode) { + if (startup.mode === "DYNAMIC" && startup.toolsets) { + console.warn("startup.toolsets provided but ignored in DYNAMIC mode"); + return { mode: "DYNAMIC" }; + } + if (startup.mode === "STATIC") { + if (startup.toolsets === "ALL") + return { mode: "STATIC", toolsets: "ALL" }; + const names = Array.isArray(startup.toolsets) ? startup.toolsets : []; + const valid: string[] = []; + for (const name of names) { + const { isValid, sanitized, error } = + this.toolsetValidator.validateToolsetName(name, catalog); + if (isValid && sanitized) valid.push(sanitized); + else if (error) console.warn(error); + } + if (names.length > 0 && valid.length === 0) { + throw new Error( + "STATIC mode requires valid toolsets or 'ALL'; none were valid" + ); + } + return { mode: "STATIC", toolsets: valid }; + } + return { mode: startup.mode }; + } + + // No explicit mode; infer from toolsets + if (startup.toolsets === "ALL") return { mode: "STATIC", toolsets: "ALL" }; + if (Array.isArray(startup.toolsets) && startup.toolsets.length > 0) { + const valid: string[] = []; + for (const name of startup.toolsets) { + const { isValid, sanitized, error } = + this.toolsetValidator.validateToolsetName(name, catalog); + if (isValid && sanitized) valid.push(sanitized); + else if (error) console.warn(error); + } + if (valid.length === 0) { + throw new Error( + "STATIC mode requires valid toolsets or 'ALL'; none were valid" + ); + } + return { mode: "STATIC", toolsets: valid }; + } + + // Default + return { mode: "DYNAMIC" }; + } + public getMode(): Exclude { return this.mode; } diff --git a/src/mode/ModeResolver.ts b/src/mode/ModeResolver.ts index 0d36648..356d791 100644 --- a/src/mode/ModeResolver.ts +++ b/src/mode/ModeResolver.ts @@ -1,20 +1,24 @@ import type { Mode, ToolSetCatalog } from "../types/index.js"; -export interface ModeResolverKeys { +interface ModeResolverKeys { dynamic?: string[]; // keys that, when present/true, enable dynamic mode toolsets?: string[]; // keys that carry comma-separated toolsets } -export interface ModeResolverOptions { +interface ModeResolverOptions { keys?: ModeResolverKeys; } const DEFAULT_KEYS: Required = { - dynamic: ["dynamic-tool-discovery", "dynamicToolDiscovery", "DYNAMIC_TOOL_DISCOVERY"], + dynamic: [ + "dynamic-tool-discovery", + "dynamicToolDiscovery", + "DYNAMIC_TOOL_DISCOVERY", + ], toolsets: ["tool-sets", "toolSets", "FMP_TOOL_SETS"], }; -export class ModeResolver { +export class ToolsetValidator { private readonly keys: Required; constructor(options: ModeResolverOptions = {}) { @@ -24,7 +28,10 @@ export class ModeResolver { }; } - public resolveMode(env?: Record, args?: Record): Mode | null { + public resolveMode( + env?: Record, + args?: Record + ): Mode | null { // Check args first if (this.isDynamicEnabled(args)) return "DYNAMIC"; @@ -40,7 +47,10 @@ export class ModeResolver { return null; // no override } - public parseCommaSeparatedToolSets(input: string, catalog: ToolSetCatalog): string[] { + public parseCommaSeparatedToolSets( + input: string, + catalog: ToolSetCatalog + ): string[] { if (!input || typeof input !== "string") return []; const raw = input .split(",") @@ -51,12 +61,20 @@ export class ModeResolver { const result: string[] = []; for (const name of raw) { if (valid.has(name)) result.push(name); - else console.warn(`Invalid toolset '${name}' ignored. Available: ${Array.from(valid).join(", ")}`); + else + console.warn( + `Invalid toolset '${name}' ignored. Available: ${Array.from( + valid + ).join(", ")}` + ); } return result; } - public getModulesForToolSets(toolsets: string[], catalog: ToolSetCatalog): string[] { + public getModulesForToolSets( + toolsets: string[], + catalog: ToolSetCatalog + ): string[] { const modules = new Set(); for (const name of toolsets) { const def = catalog[name]; @@ -66,33 +84,64 @@ export class ModeResolver { return Array.from(modules); } - public validateToolsetName(name: unknown, catalog: ToolSetCatalog): { isValid: boolean; sanitized?: string; error?: string } { + public validateToolsetName( + name: unknown, + catalog: ToolSetCatalog + ): { isValid: boolean; sanitized?: string; error?: string } { if (!name || typeof name !== "string") { - return { isValid: false, error: `Invalid toolset name provided. Must be a non-empty string. Available toolsets: ${Object.keys(catalog).join(", ")}` }; + return { + isValid: false, + error: `Invalid toolset name provided. Must be a non-empty string. Available toolsets: ${Object.keys( + catalog + ).join(", ")}`, + }; } const sanitized = name.trim(); if (sanitized.length === 0) { - return { isValid: false, error: `Empty toolset name provided. Available toolsets: ${Object.keys(catalog).join(", ")}` }; + return { + isValid: false, + error: `Empty toolset name provided. Available toolsets: ${Object.keys( + catalog + ).join(", ")}`, + }; } if (!catalog[sanitized]) { - return { isValid: false, error: `Toolset '${sanitized}' not found. Available toolsets: ${Object.keys(catalog).join(", ")}` }; + return { + isValid: false, + error: `Toolset '${sanitized}' not found. Available toolsets: ${Object.keys( + catalog + ).join(", ")}`, + }; } return { isValid: true, sanitized }; } - public validateToolsetModules(toolsetNames: string[], catalog: ToolSetCatalog): { isValid: boolean; modules?: string[]; error?: string } { + public validateToolsetModules( + toolsetNames: string[], + catalog: ToolSetCatalog + ): { isValid: boolean; modules?: string[]; error?: string } { try { const modules = this.getModulesForToolSets(toolsetNames, catalog); if (!modules || modules.length === 0) { - return { isValid: false, error: `No modules found for toolsets: ${toolsetNames.join(", ")}` }; + return { + isValid: false, + error: `No modules found for toolsets: ${toolsetNames.join(", ")}`, + }; } return { isValid: true, modules }; } catch (error) { - return { isValid: false, error: `Error resolving modules for ${toolsetNames.join(", ")}: ${error instanceof Error ? error.message : "Unknown error"}` }; + return { + isValid: false, + error: `Error resolving modules for ${toolsetNames.join(", ")}: ${ + error instanceof Error ? error.message : "Unknown error" + }`, + }; } } - private isDynamicEnabled(source?: Record | Record): boolean { + private isDynamicEnabled( + source?: Record | Record + ): boolean { if (!source) return false; for (const key of this.keys.dynamic) { const value = (source as any)[key]; @@ -105,13 +154,15 @@ export class ModeResolver { return false; } - private getToolsetsString(source?: Record | Record): string | undefined { + private getToolsetsString( + source?: Record | Record + ): string | undefined { if (!source) return undefined; for (const key of this.keys.toolsets) { const value = (source as any)[key]; - if (typeof value === "string" && value.trim().length > 0) return value as string; + if (typeof value === "string" && value.trim().length > 0) + return value as string; } return undefined; } } - diff --git a/src/server/createMcpServer.ts b/src/server/createMcpServer.ts index b22de45..ab33aeb 100644 --- a/src/server/createMcpServer.ts +++ b/src/server/createMcpServer.ts @@ -5,7 +5,6 @@ import { FastifyTransport, type FastifyTransportOptions, } from "../http/FastifyTransport.js"; -import { McpServerOptionsValidator } from "./validateOptions.js"; export interface CreateMcpServerOptions { catalog: ToolSetCatalog; @@ -15,24 +14,20 @@ export interface CreateMcpServerOptions { startup?: { mode?: Exclude; toolsets?: string[] | "ALL" }; registerMetaTools?: boolean; http?: FastifyTransportOptions; - /** - * Provide an existing MCP server instance. If omitted, you must provide createServer. - * When provided together with createServer, this is used for the default (non-cached) manager, - * while createServer is used for per-client bundles. + /** Factory to create an MCP server instance. Required. + * In DYNAMIC mode, a new instance is created per client bundle. + * In STATIC mode, a single instance is created and reused across bundles. */ - server?: McpServer; - /** - * Factory to create a fresh MCP server instance for each client bundle. - * If omitted, the provided `server` instance will be reused for all clients. - */ - createServer?: () => McpServer; + createServer: () => McpServer; configSchema?: object; } export async function createMcpServer(options: CreateMcpServerOptions) { const mode: Exclude = options.startup?.mode ?? "DYNAMIC"; - McpServerOptionsValidator.validate(options); - const baseServer: McpServer = options.server ?? options.createServer!(); + if (typeof options.createServer !== "function") { + throw new Error("createMcpServer: `createServer` (factory) is required"); + } + const baseServer: McpServer = options.createServer(); // Typed, guarded notifier type NotifierA = { @@ -74,10 +69,10 @@ export async function createMcpServer(options: CreateMcpServerOptions) { const transport = new FastifyTransport( orchestrator.getManager(), () => { - // Create a fresh server + orchestrator bundle for a new client when needed - const createdServer: McpServer = options.createServer - ? options.createServer() - : baseServer; + // Create a server + orchestrator bundle + // for a new client when needed + const createdServer: McpServer = + mode === "DYNAMIC" ? options.createServer() : baseServer; const orchestrator = new ServerOrchestrator({ server: createdServer, catalog: options.catalog, diff --git a/src/server/validateOptions.ts b/src/server/validateOptions.ts deleted file mode 100644 index fb80562..0000000 --- a/src/server/validateOptions.ts +++ /dev/null @@ -1,35 +0,0 @@ -import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import type { Mode } from "../types/index.js"; - -export type CreateMcpServerValidationInput = { - server?: McpServer; - createServer?: () => McpServer; - startup?: { mode?: Exclude }; -}; - -export class McpServerOptionsValidator { - public static validate(input: CreateMcpServerValidationInput): void { - const mode: Exclude = input.startup?.mode ?? "DYNAMIC"; - - if (!input.server && !input.createServer) { - throw new Error( - "createMcpServer: either `server` or `createServer` must be provided" - ); - } - - if (input.server && input.createServer) { - if (typeof process.emitWarning === "function") { - process.emitWarning( - "Both `server` and `createServer` were provided. The base instance will use `server`, and per-client bundles will use `createServer`.", - { code: "TOOLCEPTION_CREATE_MCP_SERVER_BOTH" } - ); - } - } - - if (mode === "DYNAMIC" && !input.createServer) { - throw new Error( - "createMcpServer: in DYNAMIC mode `createServer` is required to create per-client server instances" - ); - } - } -} diff --git a/tests/createMcpServer.test.ts b/tests/createMcpServer.test.ts index 62dd8ae..09d28a1 100644 --- a/tests/createMcpServer.test.ts +++ b/tests/createMcpServer.test.ts @@ -59,13 +59,14 @@ describe("createMcpServer", () => { expect(baseDyn.calls.includes("list_tools")).toBe(true); expect(baseDyn.calls.includes("list_toolsets")).toBe(true); - const s = makeFakeServer(); + const s = makeFakeServerFactory(); await createMcpServer({ catalog, startup: { mode: "STATIC" }, - server: s.server, + createServer: s.createServer, }); - expect(s.calls.length).toBe(0); + const baseStat = s.created[0]; + expect(baseStat.calls.length).toBe(0); }); it("passes configSchema to FastifyTransport constructor", async () => { @@ -85,19 +86,20 @@ describe("createMcpServer", () => { expect(args?.[3]).toEqual(configSchema); }); - it("uses provided server when only server is set, and reuses it for bundles (STATIC)", async () => { - const base = makeFakeServer(); + it("reuses a single instance in STATIC mode bundles", async () => { + const f = makeFakeServerFactory(); await createMcpServer({ catalog, startup: { mode: "STATIC" }, - server: base.server, + createServer: f.createServer, }); const bundleFactory = (FastifyTransportMock as any).lastArgs?.[1]; - const bundle = bundleFactory(); - expect(bundle.server).toBe(base.server); + const b1 = bundleFactory(); + const b2 = bundleFactory(); + expect(b1.server).toBe(b2.server); }); - it("uses createServer when only factory is set (base + per-client)", async () => { + it("creates a fresh instance per bundle in DYNAMIC mode", async () => { const factory = makeFakeServerFactory(); await createMcpServer({ catalog, @@ -111,31 +113,15 @@ describe("createMcpServer", () => { // per-client bundle uses a fresh server const bundleFactory = (FastifyTransportMock as any).lastArgs?.[1]; - const bundle = bundleFactory(); - expect(factory.created.length).toBe(2); - const perClient = factory.created[1]; - expect(bundle.server).toBe(perClient.server); - expect(perClient.calls.includes("list_tools")).toBe(true); - }); - - it("uses provided server for base and factory for bundles when both provided", async () => { - const base = makeFakeServer(); - const factory = makeFakeServerFactory(); - await createMcpServer({ - catalog, - startup: { mode: "DYNAMIC" }, - server: base.server, - createServer: factory.createServer, - }); - // base server remains the provided one - expect(factory.created.length).toBe(0); - const bundleFactory = (FastifyTransportMock as any).lastArgs?.[1]; - const bundle = bundleFactory(); - expect(factory.created.length).toBe(1); - const perClient = factory.created[0]; - expect(bundle.server).not.toBe(base.server); - expect(bundle.server).toBe(perClient.server); - expect(base.calls.includes("list_tools")).toBe(true); - expect(perClient.calls.includes("list_tools")).toBe(true); + const b1 = bundleFactory(); + const b2 = bundleFactory(); + expect(factory.created.length).toBe(3); + const s1 = factory.created[1]; + const s2 = factory.created[2]; + expect(b1.server).toBe(s1.server); + expect(b2.server).toBe(s2.server); + expect(b1.server).not.toBe(b2.server); + expect(s1.calls.includes("list_tools")).toBe(true); + expect(s2.calls.includes("list_tools")).toBe(true); }); }); diff --git a/tests/mcpServerOptionsValidator.test.ts b/tests/mcpServerOptionsValidator.test.ts deleted file mode 100644 index beab4db..0000000 --- a/tests/mcpServerOptionsValidator.test.ts +++ /dev/null @@ -1,62 +0,0 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { McpServerOptionsValidator } from "../src/server/validateOptions.js"; - -function fakeServer() { - return { tool() {} } as any; -} - -describe("McpServerOptionsValidator", () => { - const originalEmitWarning = process.emitWarning; - - beforeEach(() => { - // @ts-expect-error allow spy replacement - process.emitWarning = vi.fn(); - }); - - afterEach(() => { - // @ts-expect-error restore type - process.emitWarning = originalEmitWarning; - }); - - it("throws when neither server nor createServer provided", () => { - expect(() => - McpServerOptionsValidator.validate({ startup: { mode: "DYNAMIC" } }) - ).toThrow(/either `server` or `createServer`/); - }); - - it("throws in DYNAMIC mode without createServer", () => { - expect(() => - McpServerOptionsValidator.validate({ - server: fakeServer(), - startup: { mode: "DYNAMIC" }, - }) - ).toThrow(/DYNAMIC mode `createServer` is required/); - }); - - it("warns when both server and createServer provided", () => { - McpServerOptionsValidator.validate({ - server: fakeServer(), - createServer: () => fakeServer(), - startup: { mode: "STATIC" }, - }); - expect(process.emitWarning).toHaveBeenCalled(); - }); - - it("passes when STATIC with only server", () => { - expect(() => - McpServerOptionsValidator.validate({ - server: fakeServer(), - startup: { mode: "STATIC" }, - }) - ).not.toThrow(); - }); - - it("passes when DYNAMIC with createServer", () => { - expect(() => - McpServerOptionsValidator.validate({ - createServer: () => fakeServer(), - startup: { mode: "DYNAMIC" }, - }) - ).not.toThrow(); - }); -}); diff --git a/tests/modeResolver.test.ts b/tests/modeResolver.test.ts index ef37cd2..084909f 100644 --- a/tests/modeResolver.test.ts +++ b/tests/modeResolver.test.ts @@ -1,9 +1,9 @@ import { describe, it, expect } from "vitest"; -import { ModeResolver } from "../src/mode/ModeResolver.js"; +import { ToolsetValidator } from "../src/mode/ModeResolver.js"; -describe("ModeResolver", () => { +describe("ToolsetValidator", () => { it("detects dynamic from args/env", () => { - const r = new ModeResolver(); + const r = new ToolsetValidator(); expect(r.resolveMode(undefined, { DYNAMIC_TOOL_DISCOVERY: "true" })).toBe( "DYNAMIC" ); @@ -13,13 +13,13 @@ describe("ModeResolver", () => { }); it("detects static when toolsets present", () => { - const r = new ModeResolver(); + const r = new ToolsetValidator(); expect(r.resolveMode(undefined, { FMP_TOOL_SETS: "a,b" })).toBe("STATIC"); expect(r.resolveMode({ FMP_TOOL_SETS: "a" }, undefined)).toBe("STATIC"); }); it("parses comma separated toolsets and validates", () => { - const r = new ModeResolver(); + const r = new ToolsetValidator(); const catalog = { a: {} as any, b: {} as any }; expect(r.parseCommaSeparatedToolSets("a,b,c", catalog as any)).toEqual([ "a", diff --git a/tests/serverOrchestrator.test.ts b/tests/serverOrchestrator.test.ts index 08901e0..32f357d 100644 --- a/tests/serverOrchestrator.test.ts +++ b/tests/serverOrchestrator.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from "vitest"; +import { describe, it, expect, vi } from "vitest"; import { ServerOrchestrator } from "../src/core/ServerOrchestrator.js"; import { createFakeMcpServer } from "./helpers/fakes.js"; @@ -72,4 +72,33 @@ describe("ServerOrchestrator", () => { expect(names).toContain("enable_toolset"); expect(names).toContain("disable_toolset"); }); + + it("ignores toolsets in DYNAMIC mode with a warning", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const { server, tools } = createFakeMcpServer(); + new ServerOrchestrator({ + server, + catalog: catalog as any, + startup: { mode: "DYNAMIC", toolsets: ["core", "ext"] }, + }); + await new Promise((r) => setTimeout(r, 0)); + const names = tools.map((t) => t.name); + expect(names).not.toContain("core.ping"); + expect(names).not.toContain("ext.echo"); + expect(warn).toHaveBeenCalledWith( + "startup.toolsets provided but ignored in DYNAMIC mode" + ); + warn.mockRestore(); + }); + + it("throws in STATIC mode when toolsets are invalid/empty", async () => { + expect( + () => + new ServerOrchestrator({ + server: createFakeMcpServer().server, + catalog: catalog as any, + startup: { toolsets: ["nope"], mode: "STATIC" }, + }) + ).toThrow(/STATIC mode requires valid toolsets or 'ALL'; none were valid/); + }); }); diff --git a/tests/smoke-e2e/README.md b/tests/smoke-e2e/README.md index 66310dc..442da59 100644 --- a/tests/smoke-e2e/README.md +++ b/tests/smoke-e2e/README.md @@ -8,6 +8,10 @@ This directory contains a runnable MCP server and client to smoke-test the HTTP/ ```bash npm run dev:server-demo ``` +- Run in STATIC mode (preload ALL toolsets): + ```bash + STARTUP_MODE=STATIC TOOLSETS=ALL npm run dev:server-demo + ``` - Or directly with tsx: ```bash npx --yes tsx tests/smoke-e2e/server-demo.ts diff --git a/tests/smoke-e2e/client-demo.ts b/tests/smoke-e2e/client-demo.ts index 1f84d3d..e66454a 100644 --- a/tests/smoke-e2e/client-demo.ts +++ b/tests/smoke-e2e/client-demo.ts @@ -24,10 +24,15 @@ async function main() { const listBefore = await client.listTools(); console.log("tools before:", JSON.stringify(listBefore, null, 2)); - await client.callTool({ - name: "enable_toolset", - arguments: { name: "core" }, - } as any); + const toolNamesBefore = new Set( + (listBefore as any)?.tools?.map((t: any) => t.name) ?? [] + ); + if (!toolNamesBefore.has("core.ping")) { + await client.callTool({ + name: "enable_toolset", + arguments: { name: "core" }, + } as any); + } const ping = await client.callTool({ name: "core.ping", arguments: {}, @@ -40,10 +45,12 @@ async function main() { ); } - await client.callTool({ - name: "enable_toolset", - arguments: { name: "ext" }, - } as any); + if (!toolNamesBefore.has("ext.echo")) { + await client.callTool({ + name: "enable_toolset", + arguments: { name: "ext" }, + } as any); + } const echo = await client.callTool({ name: "ext.echo", arguments: { text: "hello" }, diff --git a/tests/smoke-e2e/server-demo.ts b/tests/smoke-e2e/server-demo.ts index 188d158..770e091 100644 --- a/tests/smoke-e2e/server-demo.ts +++ b/tests/smoke-e2e/server-demo.ts @@ -45,10 +45,18 @@ const createServer = () => capabilities: { tools: { listChanged: true } }, }); +const STATIC = (process.env.STARTUP_MODE || "").toUpperCase() === "STATIC"; + const { start, close } = await createMcpServer({ catalog, moduleLoaders, - startup: { mode: "DYNAMIC" }, + startup: STATIC + ? { + mode: "STATIC", + toolsets: + (process.env.TOOLSETS as any) === "ALL" ? "ALL" : ["core", "ext"], + } + : { mode: "DYNAMIC" }, http: { port: PORT }, createServer, configSchema: { From d4c02a2a9f12759fa731d5e42934d4ba6406f65f Mon Sep 17 00:00:00 2001 From: Ben Rabinovich Date: Wed, 13 Aug 2025 19:27:31 +0300 Subject: [PATCH 5/5] chore: update tests --- src/core/ServerOrchestrator.ts | 2 +- src/meta/registerMetaTools.ts | 12 ++--- src/mode/ToolsetValidator.ts | 1 + tests/dynamicToolManager.test.ts | 52 ++++++++++++++++++- ...olver.test.ts => toolsetValidator.test.ts} | 2 +- vitest.config.ts | 2 + 6 files changed, 62 insertions(+), 9 deletions(-) create mode 100644 src/mode/ToolsetValidator.ts rename tests/{modeResolver.test.ts => toolsetValidator.test.ts} (92%) diff --git a/src/core/ServerOrchestrator.ts b/src/core/ServerOrchestrator.ts index a7cc4ec..28bc115 100644 --- a/src/core/ServerOrchestrator.ts +++ b/src/core/ServerOrchestrator.ts @@ -1,5 +1,5 @@ import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { ToolsetValidator } from "../mode/ModeResolver.js"; +import { ToolsetValidator } from "../mode/ToolsetValidator.js"; import { ModuleResolver } from "../mode/ModuleResolver.js"; import { DynamicToolManager } from "./DynamicToolManager.js"; import { registerMetaTools } from "../meta/registerMetaTools.js"; diff --git a/src/meta/registerMetaTools.ts b/src/meta/registerMetaTools.ts index da3f62a..f92e5ca 100644 --- a/src/meta/registerMetaTools.ts +++ b/src/meta/registerMetaTools.ts @@ -19,7 +19,7 @@ export function registerMetaTools( const result = await manager.enableToolset(name); return { content: [{ type: "text", text: JSON.stringify(result) }], - } as any; + }; } ); @@ -32,7 +32,7 @@ export function registerMetaTools( const result = await manager.disableToolset(name); return { content: [{ type: "text", text: JSON.stringify(result) }], - } as any; + }; } ); @@ -64,7 +64,7 @@ export function registerMetaTools( content: [ { type: "text", text: JSON.stringify({ toolsets: items }) }, ], - } as any; + }; } ); @@ -84,7 +84,7 @@ export function registerMetaTools( text: JSON.stringify({ error: `Unknown toolset '${name}'` }), }, ], - } as any; + }; } const payload = { key: name, @@ -99,7 +99,7 @@ export function registerMetaTools( }; return { content: [{ type: "text", text: JSON.stringify(payload) }], - } as any; + }; } ); } @@ -116,7 +116,7 @@ export function registerMetaTools( }; return { content: [{ type: "text", text: JSON.stringify(payload) }], - } as any; + }; } ); } diff --git a/src/mode/ToolsetValidator.ts b/src/mode/ToolsetValidator.ts new file mode 100644 index 0000000..889c754 --- /dev/null +++ b/src/mode/ToolsetValidator.ts @@ -0,0 +1 @@ +export * from "./ModeResolver.js"; diff --git a/tests/dynamicToolManager.test.ts b/tests/dynamicToolManager.test.ts index 438c7dd..ed6e517 100644 --- a/tests/dynamicToolManager.test.ts +++ b/tests/dynamicToolManager.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from "vitest"; +import { describe, it, expect, vi } from "vitest"; import { DynamicToolManager } from "../src/core/DynamicToolManager.js"; import { ModuleResolver } from "../src/mode/ModuleResolver.js"; import { ToolRegistry } from "../src/core/ToolRegistry.js"; @@ -65,6 +65,56 @@ describe("DynamicToolManager", () => { expect((await manager.enableToolset("ext")).success).toBe(false); // exceeds max }); + it("returns validation error for unknown toolset and handles resolver failure", async () => { + const { server } = createFakeMcpServer(); + const resolver = new ModuleResolver({ catalog }); + const manager = new DynamicToolManager({ server, resolver }); + // Unknown toolset + const bad = await manager.enableToolset("does-not-exist"); + expect(bad.success).toBe(false); + expect(bad.message).toMatch(/not found|Invalid/); + + // Force resolver failure + vi.spyOn(resolver, "resolveToolsForToolsets").mockRejectedValue( + new Error("loader exploded") + ); + const err = await manager.enableToolset("core"); + expect(err.success).toBe(false); + expect(err.message).toMatch(/loader exploded/); + }); + + it("disableToolset validates input and warns when notify fails", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const { server } = createFakeMcpServer(); + const resolver = new ModuleResolver({ catalog }); + const manager = new DynamicToolManager({ + server, + resolver, + // simulate notify failing + onToolsListChanged: async () => { + throw new Error("notify failed"); + }, + }); + + // invalid disable + const invalid = await manager.disableToolset(""); + expect(invalid.success).toBe(false); + + // not active + const notActive = await manager.disableToolset("core"); + expect(notActive.success).toBe(false); + + // enable then disable, hitting notify warning path + await manager.enableToolset("core"); + const res = await manager.disableToolset("core"); + expect(res.success).toBe(true); + expect(warn).toHaveBeenCalledWith( + "Failed to send tool list change notification:", + expect.any(Error) + ); + warn.mockRestore(); + }); + it("disableToolset updates state and returns message", async () => { const { server } = createFakeMcpServer(); const resolver = new ModuleResolver({ catalog }); diff --git a/tests/modeResolver.test.ts b/tests/toolsetValidator.test.ts similarity index 92% rename from tests/modeResolver.test.ts rename to tests/toolsetValidator.test.ts index 084909f..178dbbb 100644 --- a/tests/modeResolver.test.ts +++ b/tests/toolsetValidator.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "vitest"; -import { ToolsetValidator } from "../src/mode/ModeResolver.js"; +import { ToolsetValidator } from "../src/mode/ToolsetValidator.js"; describe("ToolsetValidator", () => { it("detects dynamic from args/env", () => { diff --git a/vitest.config.ts b/vitest.config.ts index 2df950c..f3d5a89 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -4,9 +4,11 @@ export default defineConfig({ test: { coverage: { exclude: [ + "tests/**", "examples/**", "vite.config.ts", "vitest.config.ts", + "src/types/**", "src/index.ts", ], },