From 2515a0bc891ebdcc0163e08107a6b456a4ebc12e Mon Sep 17 00:00:00 2001 From: aton-of-data Date: Mon, 21 Sep 2026 01:35:47 +0000 Subject: [PATCH] fix(everything): keep server instructions in step with capability-gated tools The instructions string is read once in the server factory and handed to the McpServer constructor, before oninitialized runs and client capabilities are known, so every client receives the same text. Six of the server's tools are registered only when the client declared a matching capability, and the instructions referred to three of them -- two as unconditional imperatives. A client that declares no capabilities gets 13 tools in tools/list and none of the six, so an agent following the instructions calls a tool that is not in its catalog. Document the gated set and the capability each one needs, qualify the remaining references, and add two tests that derive the gated set from registerConditionalTools so the document cannot drift from it again. --- .../__tests__/registrations.test.ts | 94 +++++++++++++++++++ src/everything/docs/instructions.md | 25 ++++- 2 files changed, 115 insertions(+), 4 deletions(-) diff --git a/src/everything/__tests__/registrations.test.ts b/src/everything/__tests__/registrations.test.ts index 421d759e07..3286430109 100644 --- a/src/everything/__tests__/registrations.test.ts +++ b/src/everything/__tests__/registrations.test.ts @@ -126,6 +126,100 @@ describe('Registration Index Files', () => { }); }); + describe('instructions vs. capability-gated tools', () => { + // The instructions are read once in the server factory and handed to the + // McpServer constructor, before `oninitialized` runs and client capabilities + // are known. They are therefore the same string for every client, while the + // tools registered by registerConditionalTools are not. These tests pin the + // two together. + + // Every capability the conditional tools gate on, so the "all capabilities" + // registration below is the full set. + const allCapabilities = { + roots: {}, + sampling: {}, + elicitation: { url: {} }, + tasks: { + requests: { + sampling: { createMessage: {} }, + elicitation: { create: {} }, + }, + }, + }; + + const registeredWith = async (capabilities: object): Promise => { + const { registerConditionalTools } = await import('../tools/index.js'); + const mockServer = { + registerTool: vi.fn(), + server: { + getClientCapabilities: vi.fn(() => capabilities), + }, + experimental: { + tasks: { + registerToolTask: vi.fn(), + }, + }, + } as unknown as McpServer; + + registerConditionalTools(mockServer); + + const viaRegisterTool = (mockServer.registerTool as any).mock.calls.map( + (call: any[]) => call[0] + ); + const viaRegisterToolTask = ( + mockServer.experimental.tasks.registerToolTask as any + ).mock.calls.map((call: any[]) => call[0]); + return [...viaRegisterTool, ...viaRegisterToolTask]; + }; + + // A tool is capability-gated if declaring the capabilities makes it appear. + // Deriving it as a difference rather than hard-coding a list means a tool + // that stops being gated drops out of these assertions on its own. + const gatedTools = async (): Promise => { + const withAll = await registeredWith(allCapabilities); + const withNone = await registeredWith({}); + return withAll.filter((name) => !withNone.includes(name)).sort(); + }; + + // The rows of the "Capability-Gated Tools" table, by tool name. + const documentedTools = (instructions: string): string[] => { + const section = instructions + .split(/^## /m) + .find((part) => part.startsWith('Capability-Gated Tools')); + expect(section, 'instructions.md has no "Capability-Gated Tools" section').toBeDefined(); + return [...section!.matchAll(/^\|\s*`([a-z0-9-]+)`\s*\|/gm)] + .map((match) => match[1]) + .sort(); + }; + + it('documents exactly the tools that client capabilities gate', async () => { + const { readInstructions } = await import('../resources/index.js'); + + // A tool registered only when a capability is declared is absent from + // tools/list for every other client, so an agent told to use it has + // nothing to call. The instructions must name the same set the gates do. + expect(documentedTools(readInstructions())).toEqual(await gatedTools()); + }); + + it('never tells an agent to use a capability-gated tool unconditionally', async () => { + const { readInstructions } = await import('../resources/index.js'); + const instructions = readInstructions(); + const gated = await gatedTools(); + + // Lines in the gated section are already qualified by the section itself. + const sections = instructions.split(/^## /m); + const otherLines = sections + .filter((part) => !part.startsWith('Capability-Gated Tools')) + .flatMap((part) => part.split('\n')); + + const unconditional = otherLines.filter( + (line) => gated.some((name) => line.includes(`\`${name}\``)) && !/\bif\b/i.test(line) + ); + + expect(unconditional, 'mention a gated tool without saying it may be absent').toEqual([]); + }); + }); + describe('resources/index.ts', () => { it('should register resource templates', async () => { const { registerResources } = await import('../resources/index.js'); diff --git a/src/everything/docs/instructions.md b/src/everything/docs/instructions.md index 5806dc0ba9..51df627e58 100644 --- a/src/everything/docs/instructions.md +++ b/src/everything/docs/instructions.md @@ -5,7 +5,7 @@ Follow them to use, extend, and troubleshoot the server safely and effectively. ## Cross-Feature Relationships -- Use `get-roots-list` to see client workspace roots before file operations +- If `get-roots-list` is in your tool list, use it to see client workspace roots before file operations - `gzip-file-as-resource` creates session-scoped resources accessible only during the current session - Enable `toggle-simulated-logging` before debugging to see server log messages - Enable `toggle-subscriber-updates` to receive periodic resource update notifications @@ -14,14 +14,31 @@ Follow them to use, extend, and troubleshoot the server safely and effectively. - `gzip-file-as-resource`: Max fetch size controlled by `GZIP_MAX_FETCH_SIZE` (default 10MB), timeout by `GZIP_MAX_FETCH_TIME_MILLIS` (default 30s), allowed domains by `GZIP_ALLOWED_DOMAINS` - Session resources are ephemeral and lost when the session ends -- Sampling requests (`trigger-sampling-request`) require client sampling capability -- Elicitation requests (`trigger-elicitation-request`) require client elicitation capability +- Some tools are only registered for clients that declared the matching capability; see Capability-Gated Tools below + +## Capability-Gated Tools + +These tools are registered after initialization, and only if your client declared the matching +capability. If it did not, the tool is absent from `tools/list` for this session rather than +present-and-failing, so do not attempt to call it. + +| Tool | Required client capability | +| ----------------------------------- | ------------------------------------------------------ | +| `get-roots-list` | `roots` | +| `trigger-sampling-request` | `sampling` | +| `trigger-elicitation-request` | `elicitation` | +| `trigger-url-elicitation` | `elicitation.url` | +| `trigger-sampling-request-async` | `sampling` and `tasks.requests.sampling.createMessage` | +| `trigger-elicitation-request-async` | `elicitation` and `tasks.requests.elicitation.create` | + +Every other tool in this document is registered for every client. Treat `tools/list` as +authoritative: it reflects the capabilities you declared. ## Operational Patterns - For long operations, use `trigger-long-running-operation` which sends progress notifications - Prefer reading resources before calling mutating tools -- Check `get-roots-list` output to understand the client's workspace context +- If `get-roots-list` is in your tool list, check its output to understand the client's workspace context ## Easter Egg