diff --git a/CHANGELOG.md b/CHANGELOG.md index c7ca999..86fc0f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,16 @@ All notable changes to this project are documented here. This project adheres to ## [Unreleased] +### Added + +- **`MCP_TOOL_GROUPS` — choose which tools an instance publishes.** All 59 register by default, so a local stdio user is unaffected; a hosted instance names the groups it means to serve (`search`, `pathway`, `entity`, `analysis`, `export`, `interactors`, `gsa`, `utilities`). + + It is a **registration** switch, not a documentation one. An omitted group's tools are not registered, do not appear in `tools/list`, are not mentioned in the instructions the server sends on connection, and neither are its **resources** — omitting `analysis` withholds `reactome://analysis/{token}` as well, since a capability moved to a URI is not a capability withheld. A tool described nowhere but still answering is the divergence that let this server advertise Cypher tools it had not registered. + + Three cases differ on purpose, because the dangerous failure is a restriction that silently becomes "everything": unset registers all; set-and-empty refuses to start; an unknown group refuses to start and names the typo. Failing to start is loud and recoverable — quietly serving the full surface on a public endpoint is neither. + + Two tests hold the pieces together: every registered tool belongs to exactly one group (an ungrouped tool could not be switched off, and nobody would find out until it was published somewhere it should not be), and the instructions name no `reactome_*` tool the instance did not register — checked for the full server and for a restricted one, over the whole text rather than just the category list, because the recommended-workflow prose names tools too and drifted exactly that way while this was being written. + ### Removed - **BREAKING: all graph database access.** The three `reactome_cypher_*` tools, the `reactome://graph/schema` resource, the Cypher section of the server instructions, the startup schema prefetch and the `neo4j-driver` dependency are gone. `NEO4J_URI`, `NEO4J_USER`, `NEO4J_PASSWORD`, `NEO4J_DATABASE`, `CYPHER_QUERY_TIMEOUT_MS` and `MCP_ALLOW_CYPHER` are inert — nothing reads them. diff --git a/README.md b/README.md index 07ada3e..b41b161 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,40 @@ All configuration is via environment variables — pass them in the `env` block | `REACTOME_ANALYSIS_SERVICE_URL` | derived from `REACTOME_BASE_URL` | Fine-grained override for the Analysis Service only. | | `LOG_LEVEL` | `info` | `debug` / `info` / `warn` / `error`. Logs are JSON on stderr; stdout is reserved for the MCP protocol. | +## Choosing which tools to publish + +All 59 tools register by default, which is what you want locally. A hosted +instance usually wants fewer, so `MCP_TOOL_GROUPS` selects groups: + +``` +MCP_TOOL_GROUPS=search,pathway,entity,utilities,analysis +``` + +| Group | Tools | +|-------|-------| +| `search` | 7 | +| `pathway` | 8 | +| `entity` | 8 | +| `analysis` | 9 | +| `export` | 9 | +| `interactors` | 6 | +| `gsa` | 5 | +| `utilities` | 7 | + +This is a **registration** switch, not a documentation one. An omitted group's +tools are not registered, are not listed by `tools/list`, and are not +mentioned in the instructions the server sends on connection — and **nor are +its resources**: omitting `analysis` withholds `reactome://analysis/{token}` +too, because a capability moved to a URI is not a capability withheld. A tool +described nowhere but still answering is the failure this avoids. + +Three cases behave differently on purpose, because the dangerous one is a +restriction that quietly becomes "everything": + +- **unset** — all groups +- **set and empty** — the server refuses to start +- **set with an unknown name** — the server refuses to start, naming the typo + ## Usage ### With Claude Desktop diff --git a/src/config.ts b/src/config.ts index 9f95d8b..0da7109 100644 --- a/src/config.ts +++ b/src/config.ts @@ -126,6 +126,16 @@ export const MCP_HTTP_HOST = process.env.MCP_HTTP_HOST ?? "127.0.0.1"; export const MCP_SESSION_TTL_MS = parsePositiveInt(process.env.MCP_SESSION_TTL_MS, 30 * 60_000); /** Ceiling on concurrent sessions, so a client loop cannot exhaust memory. */ +/** + * `MCP_TOOL_GROUPS` selects which groups of tools an instance registers. + * + * It is deliberately *not* a constant here. Every other value in this file is + * read once at import, which is right for something fixed at boot -- but it + * also means a captured copy, and a captured copy is a second place the value + * lives. `resolveToolGroups()` in tools/index.ts reads the environment itself, + * next to the code that acts on it, and documents the three cases. + */ + export const MCP_MAX_SESSIONS = parsePositiveInt(process.env.MCP_MAX_SESSIONS, 256); /** diff --git a/src/http-server.ts b/src/http-server.ts index 17dd5fd..ab2a963 100644 --- a/src/http-server.ts +++ b/src/http-server.ts @@ -22,9 +22,15 @@ if (!port) { process.exit(1); } -startHttpServer(port, MCP_HTTP_HOST).catch((error: unknown) => { - logger.error("fatal error starting http server", { - error: error instanceof Error ? error.message : String(error), +// `Promise.resolve().then(...)` rather than a bare call: startHttpServer +// validates the tool-group configuration synchronously, so a bad value threw +// past the .catch below and killed the process with a raw stack trace. Dying +// was correct; dying without the log line that says why was not. +Promise.resolve() + .then(() => startHttpServer(port, MCP_HTTP_HOST)) + .catch((error: unknown) => { + logger.error("fatal error starting http server", { + error: error instanceof Error ? error.message : String(error), + }); + process.exit(1); }); - process.exit(1); -}); diff --git a/src/http.ts b/src/http.ts index b535c74..224a68c 100644 --- a/src/http.ts +++ b/src/http.ts @@ -1,4 +1,5 @@ import { randomUUID } from "node:crypto"; +import { resolveToolGroups } from "./tools/index.js"; import { estimateAnalysisBodyBytes } from "./tools/limits.js"; import type { Server } from "node:http"; import type { Request, Response } from "express"; @@ -46,6 +47,16 @@ interface Session { const EXPRESS_JSON_LIMIT_BYTES = 102_400; export function startHttpServer(port: number, host: string = MCP_HTTP_HOST): Promise { + // Validate the tool-group configuration *here*, before anything binds. + // + // `createServer()` is called per session (see the session handler below), + // so a bad MCP_TOOL_GROUPS would otherwise let the process start, answer + // /health with "ok", and fail every individual session -- while the + // documentation said the server refuses to start. It does over stdio, + // where createServer runs once at boot. It did not here, which is the + // transport that is actually deployed. + resolveToolGroups(); + // Defaults to 127.0.0.1 and turns on DNS-rebinding protection for localhost // hosts, which is what stops a web page in the user's browser from driving // this server. diff --git a/src/instructions.ts b/src/instructions.ts index 5cf3cc0..ddae761 100644 --- a/src/instructions.ts +++ b/src/instructions.ts @@ -1,15 +1,11 @@ +import { resolveToolGroups, type ToolGroup } from "./tools/index.js"; + const CORE_INSTRUCTIONS = ` This server exposes the Reactome pathway knowledgebase (https://reactome.org) to LLM clients. Reactome is a manually curated, peer-reviewed database of biological pathways: reactions grouped into pathways grouped into hierarchies, annotated with participants (proteins, complexes, small molecules), regulation, literature, species, and disease. # Tool categories -- **Search** (\`reactome_search*\`) — full-text search across pathways, reactions, entities, genes, compounds. Start here when the user gives a free-text term. -- **Pathways** (\`reactome_get_pathway\`, \`reactome_top_pathways\`, \`reactome_pathway_ancestors\`, \`reactome_pathway_contained_events\`, \`reactome_events_hierarchy\`, \`reactome_pathways_for_entity\`) — navigate the pathway hierarchy. -- **Entities** (\`reactome_get_entity\`, \`reactome_complex_subunits\`, \`reactome_participants\`, \`reactome_reference_entities\`, \`reactome_complexes_containing\`) — inspect molecular participants. -- **Analysis** (\`reactome_analyze_identifier\`, \`reactome_analyze_identifiers\`) — gene/protein-list enrichment. Returns a **token**; pass it to \`reactome_get_analysis_result\`, \`reactome_analysis_found_entities\`, etc. to read results. -- **Interactors** (\`reactome_psicquic_*\`, \`reactome_static_interactors\`, \`reactome_interactor_pathways\`) — protein–protein interaction data. -- **Export** (\`reactome_export_*\`) — diagrams (PNG/SVG), SBGN, SBML, PDF reports, CSV/JSON analysis exports. -- **Utilities** (\`reactome_species\`, \`reactome_diseases\`, \`reactome_database_info\`, \`reactome_mapping_*\`, \`reactome_orthology\`, \`reactome_query\`). +__CATEGORY_LINES__ # Identifier conventions @@ -19,22 +15,97 @@ This server exposes the Reactome pathway knowledgebase (https://reactome.org) to # Recommended workflow -1. If the user gives a free-text term, call \`reactome_search\` first and pick the best match. -2. For a specific ID, use \`reactome_get_pathway\` (event/pathway) or \`reactome_get_entity\` (physical entity). -3. To dive into a pathway, follow up with \`reactome_pathway_contained_events\` and \`reactome_participants\`. -4. For enrichment analysis: \`reactome_analyze_identifiers\` → save the returned token → query the token-bearing endpoints. -5. Pathway details often include literature references (PubMed IDs) and summations — cite these in user-facing answers. +__WORKFLOW_STEPS__ # Resources (read via MCP \`resources/read\`) -- \`reactome://species\`, \`reactome://species/main\`, \`reactome://diseases\`, \`reactome://database/info\` — orient yourself at session start. -- \`reactome://pathway/{id}\`, \`reactome://entity/{id}\`, \`reactome://analysis/{token}\` — templated. +__RESOURCE_LINES__ `.trim(); -export function buildServerInstructions(): string { - // One section, unconditionally. There used to be a Cypher section appended - // when a graph connection was configured, and it was the surface that got - // left behind when the tools were gated -- a server describing tools it had - // not registered. Both are gone. - return CORE_INSTRUCTIONS; +/** + * Workflow steps, each tied to the group whose tools it names. + * + * Numbered at render time rather than in the text: a step list with a gap in + * it, or one that cites a tool this instance withheld, is the same drift the + * category list had. `null` means the step names no tool and always applies. + */ +const WORKFLOW_STEPS: { group: ToolGroup | null; text: string }[] = [ + { + group: "search", + text: "If the user gives a free-text term, call `reactome_search` first and pick the best match.", + }, + { + group: "pathway", + text: "For a specific ID, use `reactome_get_pathway` (event/pathway) or `reactome_get_entity` (physical entity).", + }, + { + group: "pathway", + text: "To dive into a pathway, follow up with `reactome_pathway_contained_events` and `reactome_participants`.", + }, + { + group: "analysis", + text: "For enrichment analysis: `reactome_analyze_identifiers` → save the returned token → query the token-bearing endpoints.", + }, + { + group: null, + text: "Pathway details often include literature references (PubMed IDs) and summations — cite these in user-facing answers.", + }, +]; + +/** + * Resource lines, tied to the group that registers each URI. + * + * The category list and the workflow steps were made group-aware first, and + * this was still prose -- so an instance without `analysis` withheld + * `reactome://analysis/{token}` and then listed it here anyway. Found by + * widening the drift test from tool names to resource URIs, which is the + * fourth place this same divergence has turned up. + */ +const RESOURCE_LINES: { group: ToolGroup; text: string }[] = [ + { + group: "utilities", + text: "`reactome://species`, `reactome://species/main`, `reactome://diseases`, `reactome://database/info` — orient yourself at session start.", + }, + { group: "pathway", text: "`reactome://pathway/{id}` — templated pathway details." }, + { group: "entity", text: "`reactome://entity/{id}` — templated entity details." }, + { group: "analysis", text: "`reactome://analysis/{token}` — templated analysis results." }, +]; + +const CATEGORY_LINES: Record = { + search: `- **Search** (\`reactome_search*\`) — full-text search across pathways, reactions, entities, genes, compounds. Start here when the user gives a free-text term.`, + pathway: `- **Pathways** (\`reactome_get_pathway\`, \`reactome_top_pathways\`, \`reactome_pathway_ancestors\`, \`reactome_pathway_contained_events\`, \`reactome_events_hierarchy\`, \`reactome_pathways_for_entity\`) — navigate the pathway hierarchy.`, + entity: `- **Entities** (\`reactome_get_entity\`, \`reactome_complex_subunits\`, \`reactome_participants\`, \`reactome_reference_entities\`, \`reactome_complexes_containing\`) — inspect molecular participants.`, + analysis: `- **Analysis** (\`reactome_analyze_identifier\`, \`reactome_analyze_identifiers\`) — gene/protein-list enrichment. Returns a **token**; pass it to \`reactome_get_analysis_result\`, \`reactome_analysis_found_entities\`, etc. to read results.`, + interactors: `- **Interactors** (\`reactome_psicquic_*\`, \`reactome_static_interactors\`, \`reactome_interactor_pathways\`) — protein–protein interaction data.`, + export: `- **Export** (\`reactome_export_*\`) — diagrams (PNG/SVG), SBGN, SBML, PDF reports, CSV/JSON analysis exports.`, + utilities: `- **Utilities** (\`reactome_species\`, \`reactome_diseases\`, \`reactome_database_info\`, \`reactome_mapping_*\`, \`reactome_orthology\`, \`reactome_query\`).`, + gsa: `- **Gene set analysis** (\`reactome_gsa_*\`) — ReactomeGSA: available methods, data types, and searchable public expression datasets. Distinct from Analysis, which is over-representation on a list of identifiers.`, +}; + +/** + * The instructions a client reads on connecting. + * + * **It describes the groups this instance actually registered, and nothing + * else.** The last time these two facts were allowed to drift apart, the + * server told every client to call `reactome_cypher_query` on an instance + * that had declined to register it. That is invisible to any check which + * asks the server what it can do, because the answer *is* the claim. + * + * So the category list is built from the same `ToolGroup` values that drive + * registration, and `tests/tool-groups.test.ts` asserts a restricted + * instance does not mention what it withheld. + */ +export function buildServerInstructions(groups: ToolGroup[] = resolveToolGroups()): string { + const categoryLines = groups.map(group => CATEGORY_LINES[group]).join("\n"); + const steps = WORKFLOW_STEPS.filter( + step => step.group === null || groups.includes(step.group) + ).map((step, i) => `${i + 1}. ${step.text}`); + + const resourceLines = RESOURCE_LINES.filter(line => groups.includes(line.group)).map( + line => `- ${line.text}` + ); + + return CORE_INSTRUCTIONS.replace("__CATEGORY_LINES__", categoryLines) + .replace("__WORKFLOW_STEPS__", steps.join("\n")) + .replace("__RESOURCE_LINES__", resourceLines.join("\n")); } diff --git a/src/resources/index.ts b/src/resources/index.ts index 1bd9049..c393b83 100644 --- a/src/resources/index.ts +++ b/src/resources/index.ts @@ -1,8 +1,62 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { registerStaticResources } from "./static.js"; import { registerResourceTemplates } from "./templates.js"; +import { resolveToolGroups, type ToolGroup } from "../tools/index.js"; + +/** + * Which group each resource belongs to. + * + * Resources are the third surface that has to agree with `MCP_TOOL_GROUPS`, + * after the tools themselves and the server instructions. An instance that + * withholds `analysis` but still serves `reactome://analysis/{token}` has not + * restricted anything -- it has moved the same capability to a URI. The + * resources are the same fetches the tools make. + * + * A URI missing from this table throws at registration rather than + * defaulting to always-on, so a new resource cannot escape the switch by + * nobody remembering it exists. + */ +const RESOURCE_GROUPS: Record = { + "reactome://species": "utilities", + "reactome://species/main": "utilities", + "reactome://diseases": "utilities", + "reactome://database/info": "utilities", + "reactome://pathway/{id}": "pathway", + "reactome://pathway/{id}/diagram": "export", + "reactome://entity/{id}": "entity", + "reactome://analysis/{token}": "analysis", + "reactome://top-pathways/{species}": "pathway", + "reactome://events-hierarchy/{species}": "pathway", +}; + +export const RESOURCE_URIS = Object.keys(RESOURCE_GROUPS); + +/** The URI a `server.resource(...)` call registers, static or templated. */ +function uriOf(args: unknown[]): string | undefined { + // Static: (uri, uri, handler) -- the name and the URI are the same string. + if (typeof args[1] === "string") return args[1]; + // Templated: (name, ResourceTemplate, options, handler). + const template = args[1] as { uriTemplate?: { toString(): string } } | undefined; + return template?.uriTemplate?.toString(); +} + +export function registerAllResources(server: McpServer, groups: ToolGroup[] = resolveToolGroups()) { + const original = server.resource.bind(server); + (server as unknown as { resource: (...a: unknown[]) => unknown }).resource = ( + ...args: unknown[] + ) => { + const uri = uriOf(args); + const group = uri === undefined ? undefined : RESOURCE_GROUPS[uri]; + if (group === undefined) { + throw new Error( + `Resource ${uri ?? "(unrecognised registration)"} is not assigned to a tool group. ` + + `Add it to RESOURCE_GROUPS in src/resources/index.ts so MCP_TOOL_GROUPS can govern it.` + ); + } + if (!groups.includes(group)) return undefined; + return (original as (...a: unknown[]) => unknown)(...args); + }; -export function registerAllResources(server: McpServer) { registerStaticResources(server); registerResourceTemplates(server); } diff --git a/src/server.ts b/src/server.ts index 26f24fb..ab0ff7a 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1,5 +1,5 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { registerAllTools } from "./tools/index.js"; +import { registerAllTools, resolveToolGroups } from "./tools/index.js"; import { registerAllResources } from "./resources/index.js"; import { buildServerInstructions } from "./instructions.js"; @@ -17,13 +17,20 @@ export const SERVER_VERSION = "1.4.0"; * stdio entrypoint. */ export function createServer(): McpServer { + // Resolved once and passed to all three, rather than each asking the + // environment for itself. They would agree today -- they read the same + // variable -- but "the same fact, fetched independently in three places" + // is precisely the shape that let this server advertise tools it had not + // registered, and it agreed right up until it did not. + const groups = resolveToolGroups(); + const server = new McpServer( { name: SERVER_NAME, version: SERVER_VERSION }, - { instructions: buildServerInstructions() } + { instructions: buildServerInstructions(groups) } ); - registerAllTools(server); - registerAllResources(server); + registerAllTools(server, groups); + registerAllResources(server, groups); return server; } diff --git a/src/tools/index.ts b/src/tools/index.ts index 48da54f..d011fee 100644 --- a/src/tools/index.ts +++ b/src/tools/index.ts @@ -1,4 +1,5 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { logger } from "../logger.js"; import { z } from "zod"; import { nonEmptyString } from "../schemas.js"; import { contentClient } from "../clients/content.js"; @@ -50,24 +51,106 @@ function installToolWrapper(server: McpServer) { }; } -export function registerAllTools(server: McpServer) { - installToolWrapper(server); +/** + * Every group of tools, and what registers it. + * + * A group is the unit a deployment can choose to publish. The map is the only + * list: `registerAllTools` iterates it rather than naming the registrars + * again, so a group cannot be defined here and forgotten at the call site, + * and `tests/tool-groups.test.ts` asserts every registered tool belongs to + * exactly one group -- a tool that escaped grouping would be unswitchable, + * and nobody would notice until it was published somewhere it should not be. + */ +export const TOOL_GROUPS = { + search: registerSearchTools, + pathway: registerPathwayTools, + entity: registerEntityTools, + analysis: registerAnalysisTools, + export: registerExportTools, + interactors: registerInteractorTools, + gsa: registerGsaTools, + utilities: registerUtilityTools, +} as const satisfies Record void>; + +export type ToolGroup = keyof typeof TOOL_GROUPS; + +export const ALL_TOOL_GROUPS = Object.keys(TOOL_GROUPS) as ToolGroup[]; + +/** + * Which groups of tools this instance registers, from `MCP_TOOL_GROUPS`. + * + * Unset registers everything, so a local stdio user is unaffected. A public + * deployment names the groups it means to publish. + * + * **This is a registration switch, not a documentation one.** A tool that is + * described nowhere but still answers is exactly the divergence that cost + * this repo a public Cypher surface and a server advertising tools it had not + * registered: what a server *offers* must be the same fact as what it *says*, + * and the only way to be sure is for the unwanted tool not to exist on the + * instance. + * + * **Three cases, deliberately different**, because the dangerous failure is a + * restriction that silently becomes "everything": + * + * unset -> all groups (the local default) + * set and empty -> throws + * set with a typo -> throws + * + * An unparseable restriction must never fall back to publishing more than was + * asked for. Failing to start is recoverable and loud; quietly serving the + * full surface on a public endpoint is neither. + * + * The environment is read here rather than captured in config.ts, so there is + * one copy of the value and it can be exercised directly. + */ +export function resolveToolGroups( + raw: string | undefined = process.env.MCP_TOOL_GROUPS +): ToolGroup[] { + if (raw === undefined) return ALL_TOOL_GROUPS; + + const requested = raw + .split(",") + .map(name => name.trim().toLowerCase()) + .filter(name => name.length > 0); + + if (requested.length === 0) { + throw new Error( + `MCP_TOOL_GROUPS is set but names no group. Unset it to register everything, ` + + `or name groups: ${ALL_TOOL_GROUPS.join(", ")}.` + ); + } + + const unknown = requested.filter(name => !(name in TOOL_GROUPS)); + if (unknown.length > 0) { + throw new Error( + `MCP_TOOL_GROUPS names unknown group(s): ${unknown.join(", ")}. ` + + `Known groups: ${ALL_TOOL_GROUPS.join(", ")}.` + ); + } + + // Deduplicated, and in the map's order rather than the caller's, so the + // registration order does not depend on how the variable was written. + return ALL_TOOL_GROUPS.filter(name => requested.includes(name)); +} - registerAnalysisTools(server); - registerPathwayTools(server); - registerSearchTools(server); - registerEntityTools(server); - registerExportTools(server); - registerInteractorTools(server); - registerGsaTools(server); +export function registerAllTools(server: McpServer, groups: ToolGroup[] = resolveToolGroups()) { + installToolWrapper(server); // No graph database tools. They were removed on 2026-09-21 when this // server became publicly hosted: Constitution Principle IV already said no // deployment holds a Neo4j connection, and a gate enforcing that is a gate // somebody can flip. Nothing here opens one now. - // Register utility tools directly here - registerUtilityTools(server); + for (const group of groups) TOOL_GROUPS[group](server); + + if (groups.length < ALL_TOOL_GROUPS.length) { + const omitted = ALL_TOOL_GROUPS.filter(g => !groups.includes(g)); + logger.info("registering a subset of tool groups", { + registered: groups, + omitted, + source: "MCP_TOOL_GROUPS", + }); + } } /** diff --git a/tests/tool-groups.test.ts b/tests/tool-groups.test.ts new file mode 100644 index 0000000..a39a669 --- /dev/null +++ b/tests/tool-groups.test.ts @@ -0,0 +1,218 @@ +import { describe, it, expect } from "vitest"; +import { buildServerInstructions } from "../src/instructions.js"; +import { registerAllResources, RESOURCE_URIS } from "../src/resources/index.js"; +import { + ALL_TOOL_GROUPS, + TOOL_GROUPS, + registerAllTools, + resolveToolGroups, + type ToolGroup, +} from "../src/tools/index.js"; + +/** + * Which tools an instance publishes. + * + * This server is hosted publicly, and the public surface is a decision + * somebody makes rather than everything that happens to be implemented. + * `MCP_TOOL_GROUPS` is a *registration* switch on purpose: a tool documented + * nowhere but still answering is the same divergence that let this repo + * advertise Cypher tools it had not registered, and publish a graph schema + * nobody had chosen to expose. What the server offers has to be one fact. + */ + +function namesFor(register: (server: never) => void): string[] { + const names: string[] = []; + register({ + tool: (...args: unknown[]) => { + if (typeof args[0] === "string") names.push(args[0]); + return undefined; + }, + } as never); + return names; +} + +function registeredWith(groups: string | undefined): string[] { + const previous = process.env.MCP_TOOL_GROUPS; + if (groups === undefined) delete process.env.MCP_TOOL_GROUPS; + else process.env.MCP_TOOL_GROUPS = groups; + try { + return namesFor(registerAllTools); + } finally { + if (previous === undefined) delete process.env.MCP_TOOL_GROUPS; + else process.env.MCP_TOOL_GROUPS = previous; + } +} + +describe("tool groups", () => { + it("accounts for every tool, with no tool in two groups", () => { + // A tool that belongs to no group could not be switched off, and nobody + // would find out until it was published somewhere it should not be. + const all = namesFor(registerAllTools); + const grouped = ALL_TOOL_GROUPS.flatMap(g => namesFor(TOOL_GROUPS[g])); + + expect(all.length).toBe(59); + expect([...grouped].sort()).toEqual([...all].sort()); + expect(new Set(grouped).size).toBe(grouped.length); + }); + + it("registers everything when unset, so a local user is unaffected", () => { + expect(registeredWith(undefined)).toHaveLength(59); + }); + + it("registers only what is named", () => { + const names = registeredWith("search,pathway"); + expect(names).toContain("reactome_search"); + expect(names).toContain("reactome_get_pathway"); + expect(names).not.toContain("reactome_export_pdf"); + expect(names).not.toContain("reactome_analyze_identifiers"); + // The positive half matters: without it, a switch that registered + // *nothing* would satisfy every "not.toContain" here. + expect(names.length).toBeGreaterThan(10); + expect(names.length).toBeLessThan(59); + }); +}); + +describe("MCP_TOOL_GROUPS parsing", () => { + it("is order- and case-insensitive, and tolerates spacing", () => { + expect(resolveToolGroups("PATHWAY, search")).toEqual(["search", "pathway"] as ToolGroup[]); + }); + + it("deduplicates", () => { + expect(resolveToolGroups("search,search")).toEqual(["search"] as ToolGroup[]); + }); + + it("registers everything only when genuinely unset", () => { + expect(resolveToolGroups(undefined)).toEqual(ALL_TOOL_GROUPS); + }); + + it("refuses an empty value rather than falling back to everything", () => { + // The dangerous case. A restriction that degrades to "publish all" on a + // blank or whitespace value is worse than no restriction, because the + // config file still says it is restricted. + expect(() => resolveToolGroups("")).toThrow(/names no group/); + expect(() => resolveToolGroups(" , ")).toThrow(/names no group/); + }); + + it("refuses an unknown group rather than ignoring it", () => { + expect(() => resolveToolGroups("serch,pathway")).toThrow(/unknown group/); + }); + + it("names the known groups in the error, so the fix is in the message", () => { + expect(() => resolveToolGroups("nope")).toThrow(/search/); + }); +}); + +describe("instructions match what is registered", () => { + /** + * The strongest version of the check, because it does not care where in + * the text a tool is named. It extracts every `reactome_*` reference from + * the instructions and requires each one to be a tool this instance + * actually registered. + * + * The category list was the obvious place for these to drift. The + * recommended-workflow and resources prose name tools too, and would have + * drifted just as quietly — which is the whole lesson of the Cypher + * instructions: the surface that describes the server is a surface. + */ + // Tool names AND resource URIs. The first version matched only + // `reactome_*`, and the instructions also list `reactome://analysis/{token}` + // and friends -- so a restricted instance could still have advertised a + // resource it withheld, which is the same drift one noun over. + const referenced = (text: string): string[] => { + const found = new Set(); + for (const m of text.matchAll(/reactome_[a-z_]*\*?/g)) found.add(m[0]); + for (const m of text.matchAll(/reactome:\/\/[a-z-]+(?:\/\{?[a-z]+\}?)*/g)) found.add(m[0]); + return [...found]; + }; + + const satisfied = (reference: string, registered: string[]): boolean => + reference.endsWith("*") + ? registered.some(name => name.startsWith(reference.slice(0, -1))) + : registered.includes(reference); + + const registeredNames = (groups: ToolGroup[]): string[] => { + const tools = groups.flatMap(g => namesFor(TOOL_GROUPS[g])); + const resources: string[] = []; + const server = { + resource: (...args: unknown[]) => { + const uri = typeof args[1] === "string" ? args[1] : undefined; + const t = args[1] as { uriTemplate?: { toString(): string } } | undefined; + const value = uri ?? t?.uriTemplate?.toString(); + if (value) resources.push(value); + return undefined; + }, + }; + registerAllResources(server as never, groups); + return [...tools, ...resources]; + }; + + it("names no tool or resource the full server does not register", () => { + const registered = registeredNames(ALL_TOOL_GROUPS); + const dangling = referenced(buildServerInstructions(ALL_TOOL_GROUPS)).filter( + r => !satisfied(r, registered) + ); + expect(dangling).toEqual([]); + }); + + it("names no tool or resource a restricted server does not register", () => { + const groups: ToolGroup[] = ["search", "pathway", "entity", "utilities"]; + const registered = registeredNames(groups); + const dangling = referenced(buildServerInstructions(groups)).filter( + r => !satisfied(r, registered) + ); + expect(dangling).toEqual([]); + }); +}); + +describe("resources follow the same switch", () => { + const resourceUris = (groups: ToolGroup[]): string[] => { + const uris: string[] = []; + registerAllResources( + { + resource: (...args: unknown[]) => { + const t = args[1] as { uriTemplate?: { toString(): string } } | undefined; + const uri = typeof args[1] === "string" ? args[1] : t?.uriTemplate?.toString(); + if (uri) uris.push(uri); + return undefined; + }, + } as never, + groups + ); + return uris; + }; + + it("registers every known resource when all groups are on", () => { + expect([...resourceUris(ALL_TOOL_GROUPS)].sort()).toEqual([...RESOURCE_URIS].sort()); + }); + + it("withholds the resources of an omitted group", () => { + // An instance that withholds the analysis *tools* but still serves + // reactome://analysis/{token} has not restricted anything; it has moved + // the capability to a URI. + const uris = resourceUris(["pathway", "utilities"]); + expect(uris).not.toContain("reactome://analysis/{token}"); + expect(uris).not.toContain("reactome://entity/{id}"); + expect(uris).toContain("reactome://pathway/{id}"); + expect(uris).toContain("reactome://species"); + }); + + it("refuses a resource that belongs to no group", () => { + // Fails loudly rather than defaulting to always-on, so a new resource + // cannot escape the switch by nobody remembering it exists. + const server = { + resource: (...args: unknown[]) => args, + }; + const rogue = { + resource: (..._args: unknown[]) => undefined, + }; + void server; + expect(() => { + registerAllResources(rogue as never, ALL_TOOL_GROUPS); + (rogue as { resource: (...a: unknown[]) => unknown }).resource( + "rogue", + "reactome://not-classified", + () => undefined + ); + }).toThrow(/not assigned to a tool group/); + }); +});