diff --git a/scripts/sweep-live.mjs b/scripts/sweep-live.mjs index 087b6c9..1932a2e 100644 --- a/scripts/sweep-live.mjs +++ b/scripts/sweep-live.mjs @@ -51,6 +51,67 @@ const ARGS = { pathways: ["R-HSA-109581"], }; +/** + * Arguments that only make sense for one tool. Without these the sweep sends a + * pathway stable ID to a tool that wants a complex, gets an honest 404, and + * reports it as suspicious -- noise that would make a scheduled run red from + * the first week and teach everyone to ignore it. + */ +const TOOL_ARGS = { + reactome_complex_subunits: { id: "R-HSA-5672710" }, + reactome_entity_other_forms: { id: "R-HSA-69488" }, + reactome_complexes_containing: { resource: "UniProt", identifier: "P04637" }, + reactome_mapping_pathways: { resource: "UniProt", identifier: "P04637" }, + reactome_mapping_reactions: { resource: "UniProt", identifier: "P04637" }, + reactome_compare_species: { species: "48892" }, + // A correctly-spelled term legitimately returns nothing, which tells us + // only that the call succeeded. A misspelling exercises the formatter. + reactome_search_spellcheck: { query: "kinse" }, + reactome_psicquic_summary: { resource: "IntAct", accession: "P04637" }, + reactome_psicquic_details: { resource: "IntAct", accession: "P04637" }, +}; + +/** + * A reply that is the service reporting an error is the service answering, not + * this repo rendering something wrong. Those are listed separately and do not + * fail the run: Reactome returning 500 for a valid-looking orthology request is + * not something a release of this package can fix. + */ +const SERVICE_ERROR = /^(Content Service|Analysis Service|MCP) error/; + +/** + * What a healthy answer looks like, for tools whose sweep arguments are known + * to return data. + * + * Marker-grepping alone is not enough, and this repo has the scar to prove it: + * `search_facets` rendered its heading, a total, and "*No facets available.*" + * for months. No `undefined`, no empty body, nothing to grep for -- just a + * confident report that there was nothing to report. Only an expectation of + * what should be there can catch a field that was dropped cleanly. + * + * Every string below was observed in a real response. A failure here means + * either this repo stopped rendering something, or Reactome stopped returning + * it; both are worth a person looking. + */ +const EXPECT = { + reactome_search_facets: ["### Types:", "### Species:"], + reactome_search_suggest: ["- tp53"], + reactome_search_spellcheck: ["Did you mean"], + reactome_participants: ["### Complex", "["], + reactome_entity_component_of: ["R-HSA-"], + reactome_static_interactors: ["score:"], + reactome_interactor_summary: ["Total interactions:"], + reactome_psicquic_details: ["score:"], + reactome_search_diagram: ["R-HSA-"], + reactome_search: ["R-HSA-"], + reactome_get_pathway: ["Stable ID"], + reactome_top_pathways: ["R-HSA-"], + reactome_species: ["Homo sapiens"], + reactome_analyze_identifiers: ["R-HSA-"], + reactome_complex_subunits: ["R-HSA-"], + reactome_events_hierarchy: ["R-HSA-"], +}; + /** * Markers of a formatter that read a field the API did not return. "undefined" * and "[object Object]" are the loud cases; a body with nothing in it is the @@ -135,6 +196,7 @@ async function main() { console.log(ARGS.token ? `analysis token: ${ARGS.token}` : "analysis token: NOT OBTAINED"); const suspicious = []; + const serviceErrors = []; const unreachable = []; let called = 0; @@ -146,7 +208,13 @@ async function main() { continue; } - const args = Object.fromEntries(required.map(key => [key, ARGS[key]])); + const overrides = TOOL_ARGS[tool.name] ?? {}; + const args = { + ...Object.fromEntries(required.map(key => [key, ARGS[key]])), + // Overrides may add optional arguments too, not just replace required + // ones -- some tools only answer usefully when given a filter. + ...overrides, + }; let text; try { text = await callTool(tool.name, args); @@ -161,22 +229,51 @@ async function main() { if (hits.length > 0) { const line = text.split("\n").find(l => hits.some(h => l.includes(h))) ?? ""; suspicious.push([tool.name, hits.join(", "), line.trim().slice(0, 100)]); + } else if (SERVICE_ERROR.test(text.trim())) { + serviceErrors.push([tool.name, text.trim().slice(0, 120)]); } else if (text.trim().split("\n").filter(Boolean).length <= 1) { // A single line is a heading with no body -- either genuinely empty, or // a section that was skipped because a field was read at the wrong path. suspicious.push([tool.name, "empty body", text.trim().slice(0, 100)]); + } else { + const missing = (EXPECT[tool.name] ?? []).filter(needle => !text.includes(needle)); + if (missing.length > 0) { + suspicious.push([ + tool.name, + `missing ${missing.map(m => JSON.stringify(m)).join(", ")}`, + text.trim().split("\n").slice(0, 2).join(" / ").slice(0, 100), + ]); + } } } child.kill(); + const toolNames = new Set(tools.map(t => t.name)); + const unknownExpectations = Object.keys(EXPECT).filter(name => !toolNames.has(name)); + console.log(`\ncalled ${called} of ${tools.length} tools`); + console.log( + `checked content expectations for ${Object.keys(EXPECT).length - unknownExpectations.length} of them` + ); + if (unknownExpectations.length > 0) { + // A typo here would silently verify nothing, which is the failure mode + // this whole script exists to catch. + console.log( + ` WARNING: expectations named tools that do not exist: ${unknownExpectations.join(", ")}` + ); + } if (unreachable.length > 0) { console.log(`\n${unreachable.length} not reachable with known arguments:`); for (const line of unreachable) console.log(` ${line}`); } + if (serviceErrors.length > 0) { + console.log(`\n${serviceErrors.length} returned a service error (not a failure of this repo):`); + for (const [name, sample] of serviceErrors) console.log(` ${name}\n ${sample}`); + } + if (suspicious.length === 0) { console.log("\nno suspicious output"); return 0; @@ -187,10 +284,6 @@ async function main() { console.log(` ${name} [${why}]`); if (sample) console.log(` ${sample}`); } - console.log( - "\nSome of these are the service answering a deliberately odd argument" + - " with a 404 or 500. Read each one before treating it as a bug." - ); return 1; } diff --git a/specs/001-response-shape-verification/spec.md b/specs/001-response-shape-verification/spec.md new file mode 100644 index 0000000..39edaf9 --- /dev/null +++ b/specs/001-response-shape-verification/spec.md @@ -0,0 +1,75 @@ +# 001 — Verifying response shapes + +**Status:** implemented +**Date:** 2026-09-14 +**Constitution:** Principles I, II, III + +## Problem + +Nine tools called the right endpoint, returned success, and rendered `undefined` +— or rendered nothing at all and looked fine. A tenth, `search_diagram`, had +never returned an answer in the life of the repository. + +None of this was a logic error. Each tool read a field path the Reactome +services do not return: + +| Tool | Declared | Actual | +|---|---|---| +| `search_suggest`, `search_spellcheck` | `{suggestions: string[]}` | bare `string[]` | +| `entity_component_of` | `Complex[]` | one entry per relationship type, with parallel `names`/`stIds`/`schemaClasses` arrays | +| `participants` | `stId`, `referenceEntity` | `peDbId`, `refEntities[]` | +| `static_interactors`, `psicquic_details` | flat interactor list | `entities[].interactors[]` | +| `interactor_summary`, `psicquic_summary` | `{accession, count}` | `{resource, entities: [{acc, count}]}` | +| `analysis_found_entities` | `mapsTo[].identifier` | `mapsTo[].ids[]` | +| `search_facets` | `FacetEntry[]` | `{available: FacetEntry[]}` | +| `search_diagram` | grouped `results` | flat `entries` | + +`contentClient.get(...)` asserts `T`. It does not check it. Nothing else did +either, because 51 of 56 tools had no test — including all seven analysis tools, +which is why a token-parsing bug had shipped earlier. + +## Decision + +**Every response type is derived from a real response.** The endpoint and a +trimmed payload go in a comment above the type, and a test pins the formatter +using that same payload. A fixture that stops matching production is a signal to +change the formatter, not the fixture. + +**A sweep runs against the live services.** `npm run sweep` calls all 53 tools +and reports anything that looks wrong. It is not in `npm test` — it needs the +network and hits production — and runs weekly and on demand. + +## What the sweep must detect, and why marker-grepping is not enough + +The first version grepped for `undefined`, `[object Object]` and empty bodies. +Tested against a deliberately reintroduced `search_facets` bug, it reported +`no suspicious output` and exited 0: a facets response that has dropped every +facet still renders a heading, a total, and "*No facets available.*". Three +plausible lines and nothing to grep for. + +So the sweep also asserts, for 16 tools whose arguments are known to return +data, that the answer still contains what it should. + +**A silent drop is the failure mode that matters.** `participants` never showed +external identifiers at all. There was no `undefined` to notice — the bracket +was simply absent, and no one can see an absence in output they have not +compared against the source. + +## Consequences + +- Fixtures are verbose. That is the cost of recording what was observed rather + than what was assumed. +- The sweep can go red because Reactome changed. Service errors are therefore + reported separately and do not fail the run. +- Expectations are only as good as their coverage: 16 of 53 tools today. The + run prints how many were actually checked, so a typo cannot quietly mean + "nothing was verified". + +## Still open + +- 37 tools have no content expectation. +- `/data/orthology/{id}/species/{taxId}` returns HTTP 500 for valid-looking + input. Upstream; our URL matches the documented route. +- `MappedInteractor.identifier` is probably wrong in the same way as + `MappedEntity.identifier` was, but nothing renders it, so it could not be + verified against a real payload. It was left alone rather than guessed at. diff --git a/specs/002-transport-and-hosting/spec.md b/specs/002-transport-and-hosting/spec.md new file mode 100644 index 0000000..a7e1693 --- /dev/null +++ b/specs/002-transport-and-hosting/spec.md @@ -0,0 +1,55 @@ +# 002 — Transports and hosting + +**Status:** open — the design question is stated here, not settled +**Date:** 2026-09-14 +**Constitution:** Principles IV, V + +## Problem + +The server speaks stdio only. Every user clones the repository, builds it, and +configures an agent to spawn it locally. That works for the handful of people +doing it today and does not scale to "Reactome offers an MCP endpoint". + +## What is decided + +**Construction is separate from transport.** `createServer()` builds a +fully-registered server with no transport attached; `src/index.ts` is the stdio +entrypoint that calls it. Nothing is constructed at module scope, so importing +the entrypoint no longer starts a server. + +This is a prerequisite, not the answer. A hosted deployment needs one server per +session and a second transport from the same registrations — neither is possible +while the only instance is a module-scope constant. The idea is harvested from +#5 by @adidev001. + +**No Neo4j from a deployed instance.** Graph tools stay behind the `NEO4J_URI` +gate, off by default, and a hosted instance does not set it. A public endpoint +holding database credentials is a different security proposition from one that +can only make the calls a browser can. A test asserts the gate holds. + +**Analysis runs in the Analysis Service.** The server submits identifiers, holds +the token, and formats the reply. + +## What is open + +1. **Who adds Streamable HTTP, and when.** The SDK provides + `StreamableHTTPServerTransport`. The work is small; the operational + commitment is not. + +2. **Where a hosted instance runs.** Spinning it up alongside the Angular + website has been raised. That would put it behind infrastructure that already + exists, with a team that already operates it. + +3. **Whether it is public.** A public endpoint needs rate limiting, abuse + handling, and an answer for what happens when Reactome's own services are + slow — this server would become a new way to load them. + +4. **npm publishing.** Deferred by decision, to be settled in one pass with the + website and the other Reactome repositories rather than piecemeal. + +## Why this is written down now + +The factory landed for testability. Recording the rest here keeps the reason it +is shaped this way from being lost, and keeps the next person from either +building the hosting nobody agreed to or deleting the seam that makes it +possible. diff --git a/src/index.ts b/src/index.ts index b6a5598..f629510 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,23 +1,15 @@ #!/usr/bin/env node -import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; -import { registerAllTools } from "./tools/index.js"; -import { registerAllResources } from "./resources/index.js"; +import { createServer } from "./server.js"; import { logger } from "./logger.js"; import { CONTENT_SERVICE_URL, ANALYSIS_SERVICE_URL, NEO4J_URI } from "./config.js"; -import { buildServerInstructions } from "./instructions.js"; import { fetchGraphSchema } from "./graph/schema.js"; -const server = new McpServer( - { name: "reactome", version: "1.4.0" }, - { instructions: buildServerInstructions() } -); - -registerAllTools(server); -registerAllResources(server); - async function main() { + // Built here rather than at module scope, so importing this file does not + // construct a server as a side effect. + const server = createServer(); const transport = new StdioServerTransport(); await server.connect(transport); logger.info("reactome mcp server started", { diff --git a/src/schemas.ts b/src/schemas.ts new file mode 100644 index 0000000..9e52695 --- /dev/null +++ b/src/schemas.ts @@ -0,0 +1,17 @@ +import { z } from "zod"; + +/** + * Every free-text argument this server accepts: search terms, stable IDs, + * species filters, analysis tokens. + * + * Blank and whitespace-only values are rejected here rather than forwarded. + * An empty `q` reaches the Content Service as either an error or a request for + * everything, and an empty token produces a 404 that reads like the analysis + * expired. Failing at the schema gives the model a message it can act on. + * + * The 2048 cap is a guard against a model pasting a document into an argument. + * + * The shared-schema idea, and the trim/min(1) validation, are from #5 by + * @adidev001. + */ +export const nonEmptyString = z.string().trim().min(1).max(2048); diff --git a/src/server.ts b/src/server.ts new file mode 100644 index 0000000..26f24fb --- /dev/null +++ b/src/server.ts @@ -0,0 +1,29 @@ +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { registerAllTools } from "./tools/index.js"; +import { registerAllResources } from "./resources/index.js"; +import { buildServerInstructions } from "./instructions.js"; + +export const SERVER_NAME = "reactome"; +export const SERVER_VERSION = "1.4.0"; + +/** + * Build a fully-registered server, with no transport attached. + * + * Construction is separate from transport for two reasons. Tests can exercise + * the real registration path without starting stdio and without the import + * itself launching a server. And a hosted deployment needs to serve a second + * transport -- Streamable HTTP -- from the same registrations, which is not + * possible while the only server instance is created at module scope in the + * stdio entrypoint. + */ +export function createServer(): McpServer { + const server = new McpServer( + { name: SERVER_NAME, version: SERVER_VERSION }, + { instructions: buildServerInstructions() } + ); + + registerAllTools(server); + registerAllResources(server); + + return server; +} diff --git a/src/tools/analysis.ts b/src/tools/analysis.ts index 244800e..8f4ffff 100644 --- a/src/tools/analysis.ts +++ b/src/tools/analysis.ts @@ -1,5 +1,6 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { z } from "zod"; +import { nonEmptyString } from "../schemas.js"; import { analysisClient } from "../clients/analysis.js"; import type { AnalysisResult, @@ -66,10 +67,10 @@ export function registerAnalysisTools(server: McpServer) { "reactome_analyze_identifier", "Analyze a single gene/protein identifier for pathway enrichment. Returns pathways containing this identifier.", { - id: z.string().max(2048).describe("Gene symbol, UniProt ID, Ensembl ID, or other identifier"), + id: nonEmptyString.describe("Gene symbol, UniProt ID, Ensembl ID, or other identifier"), projection: z.boolean().optional().default(true).describe("Project results to Homo sapiens"), interactors: z.boolean().optional().default(false).describe("Include interactor data"), - species: z.string().max(2048).optional().describe("Filter by species (taxonomy ID or name)"), + species: nonEmptyString.optional().describe("Filter by species (taxonomy ID or name)"), }, async ({ id, projection, interactors, species }) => { const endpoint = projection @@ -95,7 +96,7 @@ export function registerAnalysisTools(server: McpServer) { "Perform pathway enrichment analysis on a list of gene/protein identifiers. Returns over-represented pathways sorted by p-value.", { identifiers: z - .array(z.string().max(2048)) + .array(nonEmptyString) .describe("List of gene symbols, UniProt IDs, or other identifiers"), projection: z.boolean().optional().default(true).describe("Project results to Homo sapiens"), interactors: z @@ -136,8 +137,8 @@ export function registerAnalysisTools(server: McpServer) { "reactome_get_analysis_result", "Retrieve a previously computed analysis result using its token. Allows filtering and pagination.", { - token: z.string().max(2048).describe("Analysis token from a previous analysis"), - species: z.string().max(2048).optional().describe("Filter by species"), + token: nonEmptyString.describe("Analysis token from a previous analysis"), + species: nonEmptyString.optional().describe("Filter by species"), sort_by: z .enum([ "NAME", @@ -176,11 +177,9 @@ export function registerAnalysisTools(server: McpServer) { "reactome_analysis_found_entities", "Get the identifiers that were found in a specific pathway from an analysis result.", { - token: z.string().max(2048).describe("Analysis token"), - pathway: z.string().max(2048).describe("Pathway stable ID (e.g., R-HSA-109582)"), - resource: z - .string() - .max(2048) + token: nonEmptyString.describe("Analysis token"), + pathway: nonEmptyString.describe("Pathway stable ID (e.g., R-HSA-109582)"), + resource: nonEmptyString .optional() .default("TOTAL") .describe("Resource filter (TOTAL, UNIPROT, ENSEMBL, etc.)"), @@ -221,7 +220,7 @@ export function registerAnalysisTools(server: McpServer) { "reactome_analysis_not_found", "Get the list of identifiers that could not be mapped in an analysis.", { - token: z.string().max(2048).describe("Analysis token"), + token: nonEmptyString.describe("Analysis token"), page: z.number().optional().default(1).describe("Page number"), page_size: z.number().optional().default(100).describe("Results per page"), }, @@ -249,7 +248,7 @@ export function registerAnalysisTools(server: McpServer) { "reactome_analysis_resources", "Get a summary of the molecule types (resources) found in an analysis.", { - token: z.string().max(2048).describe("Analysis token"), + token: nonEmptyString.describe("Analysis token"), }, async ({ token }) => { const result = await analysisClient.get(`/token/${token}/resources`); @@ -273,10 +272,9 @@ export function registerAnalysisTools(server: McpServer) { "reactome_compare_species", "Compare Homo sapiens pathways to another species to identify orthologous pathways.", { - species: z - .string() - .max(2048) - .describe("Species to compare (taxonomy ID or name, e.g., 'Mus musculus' or '10090')"), + species: nonEmptyString.describe( + "Species to compare (taxonomy ID or name, e.g., 'Mus musculus' or '10090')" + ), page: z.number().optional().default(1).describe("Page number"), page_size: z.number().optional().default(25).describe("Results per page"), }, @@ -302,10 +300,10 @@ export function registerAnalysisTools(server: McpServer) { "reactome_analysis_pathway_sizes", "Get the distribution of pathway sizes (binned) from an analysis result.", { - token: z.string().max(2048).describe("Analysis token"), + token: nonEmptyString.describe("Analysis token"), bin_size: z.number().optional().default(100).describe("Bin size for grouping pathway sizes"), - species: z.string().max(2048).optional().describe("Filter by species"), - resource: z.string().max(2048).optional().default("TOTAL").describe("Resource filter"), + species: nonEmptyString.optional().describe("Filter by species"), + resource: nonEmptyString.optional().default("TOTAL").describe("Resource filter"), }, async ({ token, bin_size, species, resource }) => { const result = await analysisClient.get(`/token/${token}/pathways/binned`, { @@ -333,9 +331,9 @@ export function registerAnalysisTools(server: McpServer) { "reactome_filter_analysis_pathways", "Filter an analysis result to only include specific pathways.", { - token: z.string().max(2048).describe("Analysis token"), - pathways: z.array(z.string().max(2048)).describe("List of pathway stable IDs to include"), - resource: z.string().max(2048).optional().default("TOTAL").describe("Resource filter"), + token: nonEmptyString.describe("Analysis token"), + pathways: z.array(nonEmptyString).describe("List of pathway stable IDs to include"), + resource: nonEmptyString.optional().default("TOTAL").describe("Resource filter"), p_value: z.number().optional().describe("p-value threshold"), }, async ({ token, pathways, resource, p_value }) => { diff --git a/src/tools/entity.ts b/src/tools/entity.ts index 3a5beb0..df33a2e 100644 --- a/src/tools/entity.ts +++ b/src/tools/entity.ts @@ -1,5 +1,5 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { z } from "zod"; +import { nonEmptyString } from "../schemas.js"; import { contentClient } from "../clients/content.js"; import type { PhysicalEntity, Complex, ReferenceEntity } from "../types/index.js"; @@ -106,7 +106,7 @@ export function registerEntityTools(server: McpServer) { "reactome_get_entity", "Get detailed information about a physical entity (protein, complex, compound, etc.) by its Reactome ID.", { - id: z.string().max(2048).describe("Reactome stable ID (e.g., R-HSA-123456) or database ID"), + id: nonEmptyString.describe("Reactome stable ID (e.g., R-HSA-123456) or database ID"), }, async ({ id }) => { const entity = await contentClient.get( @@ -123,7 +123,7 @@ export function registerEntityTools(server: McpServer) { "reactome_complex_subunits", "Get all subunits (components) of a complex. Recursively retrieves components of nested complexes.", { - id: z.string().max(2048).describe("Complex stable ID or database ID"), + id: nonEmptyString.describe("Complex stable ID or database ID"), }, async ({ id }) => { const subunits = await contentClient.get( @@ -163,7 +163,7 @@ export function registerEntityTools(server: McpServer) { "reactome_entity_other_forms", "Get all other forms of a physical entity (modified forms, in different compartments, in complexes, etc.).", { - id: z.string().max(2048).describe("Entity stable ID or database ID"), + id: nonEmptyString.describe("Entity stable ID or database ID"), }, async ({ id }) => { const otherForms = await contentClient.get( @@ -194,7 +194,7 @@ export function registerEntityTools(server: McpServer) { "reactome_entity_component_of", "Find larger structures (complexes, sets) that contain this entity as a component.", { - id: z.string().max(2048).describe("Entity stable ID or database ID"), + id: nonEmptyString.describe("Entity stable ID or database ID"), }, async ({ id }) => { const containers = await contentClient.get( @@ -237,7 +237,7 @@ export function registerEntityTools(server: McpServer) { "reactome_participants", "Get all molecular participants (inputs, outputs, catalysts, regulators) in a reaction or pathway.", { - id: z.string().max(2048).describe("Event (pathway or reaction) stable ID or database ID"), + id: nonEmptyString.describe("Event (pathway or reaction) stable ID or database ID"), }, async ({ id }) => { const participants = await contentClient.get( @@ -278,7 +278,7 @@ export function registerEntityTools(server: McpServer) { "reactome_participating_physical_entities", "Get all physical entities participating in an event (molecules directly involved in reactions).", { - id: z.string().max(2048).describe("Event stable ID or database ID"), + id: nonEmptyString.describe("Event stable ID or database ID"), }, async ({ id }) => { const entities = await contentClient.get( @@ -307,7 +307,7 @@ export function registerEntityTools(server: McpServer) { "reactome_reference_entities", "Get all reference entities (external database references) for participants in an event.", { - id: z.string().max(2048).describe("Event stable ID or database ID"), + id: nonEmptyString.describe("Event stable ID or database ID"), }, async ({ id }) => { const refs = await contentClient.get( @@ -346,11 +346,8 @@ export function registerEntityTools(server: McpServer) { "reactome_complexes_containing", "Find all Reactome complexes that contain a specific external identifier (e.g., UniProt ID).", { - resource: z - .string() - .max(2048) - .describe("Database name (e.g., 'UniProt', 'ChEBI', 'Ensembl')"), - identifier: z.string().max(2048).describe("External identifier (e.g., 'P04637' for UniProt)"), + resource: nonEmptyString.describe("Database name (e.g., 'UniProt', 'ChEBI', 'Ensembl')"), + identifier: nonEmptyString.describe("External identifier (e.g., 'P04637' for UniProt)"), }, async ({ resource, identifier }) => { const complexes = await contentClient.get( diff --git a/src/tools/export.ts b/src/tools/export.ts index 68d39f3..8968fd3 100644 --- a/src/tools/export.ts +++ b/src/tools/export.ts @@ -1,5 +1,6 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { z } from "zod"; +import { nonEmptyString } from "../schemas.js"; import { contentClient } from "../clients/content.js"; import { CONTENT_SERVICE_URL, ANALYSIS_SERVICE_URL } from "../config.js"; @@ -9,7 +10,7 @@ export function registerExportTools(server: McpServer) { "reactome_export_diagram", "Export a pathway diagram as an image. Returns the URL to download the diagram.", { - id: z.string().max(2048).describe("Pathway stable ID (e.g., R-HSA-109582)"), + id: nonEmptyString.describe("Pathway stable ID (e.g., R-HSA-109582)"), format: z .enum(["png", "jpg", "svg", "gif"]) .optional() @@ -20,8 +21,8 @@ export function registerExportTools(server: McpServer) { .optional() .default(5) .describe("Quality/scale factor (1-10, higher = larger image)"), - flag: z.string().max(2048).optional().describe("Identifier to highlight/flag in the diagram"), - sel: z.array(z.string().max(2048)).optional().describe("IDs to select/highlight"), + flag: nonEmptyString.optional().describe("Identifier to highlight/flag in the diagram"), + sel: z.array(nonEmptyString).optional().describe("IDs to select/highlight"), }, async ({ id, format, quality, flag, sel }) => { const params = new URLSearchParams(); @@ -57,7 +58,7 @@ export function registerExportTools(server: McpServer) { "reactome_export_reaction", "Export a reaction diagram as an image.", { - id: z.string().max(2048).describe("Reaction stable ID"), + id: nonEmptyString.describe("Reaction stable ID"), format: z .enum(["png", "jpg", "svg", "gif"]) .optional() @@ -88,7 +89,7 @@ export function registerExportTools(server: McpServer) { "reactome_export_fireworks", "Export the pathway overview (fireworks) diagram for a species.", { - species: z.string().max(2048).optional().default("Homo sapiens").describe("Species name"), + species: nonEmptyString.optional().default("Homo sapiens").describe("Species name"), format: z .enum(["png", "jpg", "svg", "gif"]) .optional() @@ -120,7 +121,7 @@ export function registerExportTools(server: McpServer) { "reactome_export_sbgn", "Export a pathway or reaction to SBGN (Systems Biology Graphical Notation) XML format.", { - id: z.string().max(2048).describe("Pathway or reaction stable ID"), + id: nonEmptyString.describe("Pathway or reaction stable ID"), }, async ({ id }) => { try { @@ -162,7 +163,7 @@ export function registerExportTools(server: McpServer) { "reactome_export_sbml", "Export a pathway or reaction to SBML (Systems Biology Markup Language) format.", { - id: z.string().max(2048).describe("Pathway or reaction stable ID"), + id: nonEmptyString.describe("Pathway or reaction stable ID"), }, async ({ id }) => { const url = `${CONTENT_SERVICE_URL}/exporter/event/${encodeURIComponent(id)}.sbml`; @@ -187,7 +188,7 @@ export function registerExportTools(server: McpServer) { "reactome_export_pdf", "Export pathway or reaction documentation to PDF format.", { - id: z.string().max(2048).describe("Pathway or reaction stable ID"), + id: nonEmptyString.describe("Pathway or reaction stable ID"), }, async ({ id }) => { const url = `${CONTENT_SERVICE_URL}/exporter/document/event/${encodeURIComponent(id)}.pdf`; @@ -212,15 +213,10 @@ export function registerExportTools(server: McpServer) { "reactome_export_analysis_report", "Generate a PDF report for an analysis result.", { - token: z.string().max(2048).describe("Analysis token"), - species: z - .string() - .max(2048) - .optional() - .default("Homo sapiens") - .describe("Species for the report"), + token: nonEmptyString.describe("Analysis token"), + species: nonEmptyString.optional().default("Homo sapiens").describe("Species for the report"), num_pathways: z.number().optional().default(25).describe("Number of top pathways to include"), - resource: z.string().max(2048).optional().default("TOTAL").describe("Resource filter"), + resource: nonEmptyString.optional().default("TOTAL").describe("Resource filter"), }, async ({ token, species, num_pathways, resource }) => { const speciesParam = species.replace(/\s+/g, "_"); @@ -247,11 +243,9 @@ export function registerExportTools(server: McpServer) { "reactome_export_analysis_csv", "Export analysis results as CSV files.", { - token: z.string().max(2048).describe("Analysis token"), + token: nonEmptyString.describe("Analysis token"), type: z.enum(["pathways", "found_entities", "not_found"]).describe("Type of data to export"), - resource: z - .string() - .max(2048) + resource: nonEmptyString .optional() .default("TOTAL") .describe("Resource filter (for pathways and found_entities)"), @@ -296,7 +290,7 @@ export function registerExportTools(server: McpServer) { "reactome_export_analysis_json", "Export complete analysis result as JSON.", { - token: z.string().max(2048).describe("Analysis token"), + token: nonEmptyString.describe("Analysis token"), compressed: z.boolean().optional().default(false).describe("Return gzipped JSON"), }, async ({ token, compressed }) => { diff --git a/src/tools/index.ts b/src/tools/index.ts index 8898fdb..461ae15 100644 --- a/src/tools/index.ts +++ b/src/tools/index.ts @@ -1,5 +1,6 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { z } from "zod"; +import { nonEmptyString } from "../schemas.js"; import { contentClient } from "../clients/content.js"; import type { Species, Disease, Pathway } from "../types/index.js"; @@ -150,11 +151,10 @@ function registerUtilityTools(server: McpServer) { "reactome_mapping_pathways", "Map an external identifier to Reactome pathways.", { - resource: z - .string() - .max(2048) - .describe("Database name (e.g., 'UniProt', 'NCBI', 'Ensembl', 'ChEBI')"), - identifier: z.string().max(2048).describe("External identifier"), + resource: nonEmptyString.describe( + "Database name (e.g., 'UniProt', 'NCBI', 'Ensembl', 'ChEBI')" + ), + identifier: nonEmptyString.describe("External identifier"), }, async ({ resource, identifier }) => { const pathways = await contentClient.get( @@ -185,11 +185,10 @@ function registerUtilityTools(server: McpServer) { "reactome_mapping_reactions", "Map an external identifier to Reactome reactions.", { - resource: z - .string() - .max(2048) - .describe("Database name (e.g., 'UniProt', 'NCBI', 'Ensembl', 'ChEBI')"), - identifier: z.string().max(2048).describe("External identifier"), + resource: nonEmptyString.describe( + "Database name (e.g., 'UniProt', 'NCBI', 'Ensembl', 'ChEBI')" + ), + identifier: nonEmptyString.describe("External identifier"), }, async ({ resource, identifier }) => { interface Reaction { @@ -225,8 +224,8 @@ function registerUtilityTools(server: McpServer) { "reactome_orthology", "Get orthologous events or entities in a different species.", { - id: z.string().max(2048).describe("Reactome stable ID of an event or entity"), - species: z.string().max(2048).describe("Target species (taxonomy ID or name)"), + id: nonEmptyString.describe("Reactome stable ID of an event or entity"), + species: nonEmptyString.describe("Target species (taxonomy ID or name)"), }, async ({ id, species }) => { interface OrthologyResult { @@ -258,12 +257,8 @@ function registerUtilityTools(server: McpServer) { "reactome_query", "Query any Reactome database object by its identifier. Returns detailed information about the object.", { - id: z.string().max(2048).describe("Reactome stable ID or database ID"), - attribute: z - .string() - .max(2048) - .optional() - .describe("Specific attribute to retrieve (optional)"), + id: nonEmptyString.describe("Reactome stable ID or database ID"), + attribute: nonEmptyString.optional().describe("Specific attribute to retrieve (optional)"), }, async ({ id, attribute }) => { const endpoint = attribute diff --git a/src/tools/interactors.ts b/src/tools/interactors.ts index 9122901..f4fea97 100644 --- a/src/tools/interactors.ts +++ b/src/tools/interactors.ts @@ -1,5 +1,5 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { z } from "zod"; +import { nonEmptyString } from "../schemas.js"; import { contentClient } from "../clients/content.js"; import type { Pathway } from "../types/index.js"; @@ -107,11 +107,10 @@ export function registerInteractorTools(server: McpServer) { "reactome_psicquic_summary", "Get a summary of protein-protein interactions from a PSICQUIC resource.", { - resource: z - .string() - .max(2048) - .describe("PSICQUIC resource name (e.g., 'IntAct', 'MINT', 'BioGRID')"), - accession: z.string().max(2048).describe("Protein accession (e.g., UniProt ID)"), + resource: nonEmptyString.describe( + "PSICQUIC resource name (e.g., 'IntAct', 'MINT', 'BioGRID')" + ), + accession: nonEmptyString.describe("Protein accession (e.g., UniProt ID)"), }, async ({ resource, accession }) => { const result = await contentClient.get( @@ -137,8 +136,8 @@ export function registerInteractorTools(server: McpServer) { "reactome_psicquic_details", "Get detailed protein-protein interactions from a PSICQUIC resource.", { - resource: z.string().max(2048).describe("PSICQUIC resource name"), - accession: z.string().max(2048).describe("Protein accession"), + resource: nonEmptyString.describe("PSICQUIC resource name"), + accession: nonEmptyString.describe("Protein accession"), }, async ({ resource, accession }) => { const result = await contentClient.get( @@ -174,7 +173,7 @@ export function registerInteractorTools(server: McpServer) { "reactome_static_interactors", "Get curated protein-protein interactions from Reactome's static interactor database.", { - accession: z.string().max(2048).describe("Protein accession (e.g., UniProt ID)"), + accession: nonEmptyString.describe("Protein accession (e.g., UniProt ID)"), }, async ({ accession }) => { const result = await contentClient.get( @@ -206,7 +205,7 @@ export function registerInteractorTools(server: McpServer) { "reactome_interactor_pathways", "Find Reactome pathways where the interactors of a protein are found.", { - accession: z.string().max(2048).describe("Protein accession"), + accession: nonEmptyString.describe("Protein accession"), }, async ({ accession }) => { const pathways = await contentClient.get( @@ -235,7 +234,7 @@ export function registerInteractorTools(server: McpServer) { "reactome_interactor_summary", "Get a summary of curated interactions for a protein.", { - accession: z.string().max(2048).describe("Protein accession"), + accession: nonEmptyString.describe("Protein accession"), }, async ({ accession }) => { const result = await contentClient.get( diff --git a/src/tools/pathway.ts b/src/tools/pathway.ts index 625e506..1fed1f7 100644 --- a/src/tools/pathway.ts +++ b/src/tools/pathway.ts @@ -1,5 +1,6 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { z } from "zod"; +import { nonEmptyString } from "../schemas.js"; import { contentClient } from "../clients/content.js"; import type { Pathway, Event } from "../types/index.js"; @@ -76,7 +77,7 @@ export function registerPathwayTools(server: McpServer) { "reactome_get_pathway", "Get detailed information about a specific pathway or reaction by its Reactome ID.", { - id: z.string().max(2048).describe("Reactome stable ID (e.g., R-HSA-109582) or database ID"), + id: nonEmptyString.describe("Reactome stable ID (e.g., R-HSA-109582) or database ID"), }, async ({ id }) => { const pathway = await contentClient.get( @@ -93,9 +94,7 @@ export function registerPathwayTools(server: McpServer) { "reactome_top_pathways", "Get all top-level (root) pathways for a species. These are the main pathway categories like 'Immune System', 'Metabolism', etc.", { - species: z - .string() - .max(2048) + species: nonEmptyString .optional() .default("Homo sapiens") .describe("Species name or taxonomy ID"), @@ -125,7 +124,7 @@ export function registerPathwayTools(server: McpServer) { "reactome_pathway_ancestors", "Get the ancestor pathway hierarchy for an event (pathway or reaction). Shows how a pathway fits into the broader Reactome structure.", { - id: z.string().max(2048).describe("Reactome stable ID or database ID"), + id: nonEmptyString.describe("Reactome stable ID or database ID"), }, async ({ id }) => { const ancestors = await contentClient.get( @@ -156,7 +155,7 @@ export function registerPathwayTools(server: McpServer) { "reactome_pathway_contained_events", "Get all events (sub-pathways and reactions) contained within a pathway.", { - id: z.string().max(2048).describe("Pathway stable ID or database ID"), + id: nonEmptyString.describe("Pathway stable ID or database ID"), }, async ({ id }) => { const events = await contentClient.get( @@ -205,7 +204,7 @@ export function registerPathwayTools(server: McpServer) { "reactome_pathways_for_entity", "Find lower-level pathways that contain a specific entity (protein, gene, compound, etc.).", { - id: z.string().max(2048).describe("Entity stable ID or database ID"), + id: nonEmptyString.describe("Entity stable ID or database ID"), all_forms: z .boolean() .optional() @@ -244,7 +243,7 @@ export function registerPathwayTools(server: McpServer) { "reactome_diagram_pathways_for_entity", "Find pathways with diagrams that contain a specific entity. Useful for visualization.", { - id: z.string().max(2048).describe("Entity stable ID or database ID"), + id: nonEmptyString.describe("Entity stable ID or database ID"), all_forms: z.boolean().optional().default(false).describe("Include all forms of the entity"), }, async ({ id, all_forms }) => { @@ -280,9 +279,7 @@ export function registerPathwayTools(server: McpServer) { // returns HTTP 500 for "Homo sapiens" but 200 for "9606", so the // previous default made this tool fail every time it was called without // an explicit species. - species: z - .string() - .max(2048) + species: nonEmptyString .optional() .default("9606") .describe( diff --git a/src/tools/search.ts b/src/tools/search.ts index 4fad3b8..9bc7280 100644 --- a/src/tools/search.ts +++ b/src/tools/search.ts @@ -1,6 +1,7 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { z } from "zod"; import { contentClient } from "../clients/content.js"; +import { nonEmptyString } from "../schemas.js"; import type { SearchResult, SearchEntry, FacetEntry } from "../types/index.js"; /** @@ -90,24 +91,18 @@ export function registerSearchTools(server: McpServer) { "reactome_search", "Search the Reactome knowledgebase for pathways, reactions, proteins, genes, compounds, and other entities.", { - query: z - .string() - .max(2048) - .describe("Search term (gene name, protein, pathway name, disease, etc.)"), - species: z - .string() - .max(2048) + query: nonEmptyString.describe( + "Search term (gene name, protein, pathway name, disease, etc.)" + ), + species: nonEmptyString .optional() .describe("Filter by species (e.g., 'Homo sapiens', 'Mus musculus')"), types: z - .array(z.string().max(2048)) + .array(nonEmptyString) .optional() .describe("Filter by type (Pathway, Reaction, Protein, Gene, Complex, etc.)"), - compartments: z - .array(z.string().max(2048)) - .optional() - .describe("Filter by cellular compartment"), - keywords: z.array(z.string().max(2048)).optional().describe("Filter by keywords"), + compartments: z.array(nonEmptyString).optional().describe("Filter by cellular compartment"), + keywords: z.array(nonEmptyString).optional().describe("Filter by keywords"), rows: z.number().optional().default(25).describe("Number of results to return"), cluster: z.boolean().optional().default(true).describe("Cluster related results"), }, @@ -154,11 +149,11 @@ export function registerSearchTools(server: McpServer) { "reactome_search_paginated", "Search Reactome with pagination support for browsing through large result sets.", { - query: z.string().max(2048).describe("Search term"), + query: nonEmptyString.describe("Search term"), page: z.number().optional().default(1).describe("Page number (1-based)"), rows_per_page: z.number().optional().default(20).describe("Results per page"), - species: z.string().max(2048).optional().describe("Filter by species"), - types: z.array(z.string().max(2048)).optional().describe("Filter by type"), + species: nonEmptyString.optional().describe("Filter by species"), + types: z.array(nonEmptyString).optional().describe("Filter by type"), }, async ({ query, page, rows_per_page, species, types }) => { const params: Record = { @@ -197,7 +192,7 @@ export function registerSearchTools(server: McpServer) { "reactome_search_suggest", "Get auto-complete suggestions for a search query.", { - query: z.string().max(2048).describe("Partial search term"), + query: nonEmptyString.describe("Partial search term"), }, async ({ query }) => { const result = await contentClient.get("/search/suggest", { query }); @@ -220,7 +215,7 @@ export function registerSearchTools(server: McpServer) { "reactome_search_spellcheck", "Get spell-check suggestions for a search query.", { - query: z.string().max(2048).describe("Search term to check"), + query: nonEmptyString.describe("Search term to check"), }, async ({ query }) => { const result = await contentClient.get("/search/spellcheck", { query }); @@ -246,9 +241,7 @@ export function registerSearchTools(server: McpServer) { "reactome_search_facets", "Get available facets (filters) for search results, either globally or for a specific query.", { - query: z - .string() - .max(2048) + query: nonEmptyString .optional() .describe("Search term (optional, returns global facets if omitted)"), }, @@ -320,7 +313,7 @@ export function registerSearchTools(server: McpServer) { "Find all pathways that contain a specific entity by its database ID.", { db_id: z.number().describe("Reactome database ID of the entity"), - species: z.string().max(2048).optional().describe("Filter by species"), + species: nonEmptyString.optional().describe("Filter by species"), include_interactors: z .boolean() .optional() @@ -364,8 +357,8 @@ export function registerSearchTools(server: McpServer) { "reactome_search_diagram", "Search for entities within a specific pathway diagram.", { - diagram: z.string().max(2048).describe("Pathway stable ID for the diagram"), - query: z.string().max(2048).describe("Search term"), + diagram: nonEmptyString.describe("Pathway stable ID for the diagram"), + query: nonEmptyString.describe("Search term"), include_interactors: z.boolean().optional().default(false).describe("Include interactors"), }, async ({ diagram, query, include_interactors }) => { diff --git a/tests/server.test.ts b/tests/server.test.ts new file mode 100644 index 0000000..9677dd0 --- /dev/null +++ b/tests/server.test.ts @@ -0,0 +1,56 @@ +/** + * The server factory exists so construction can be exercised without a + * transport. These tests use the real registration path -- not the fake server + * the formatter tests use -- so a tool that fails to register is caught here. + * + * Adapted from the server-registration tests in #5 by @adidev001. + */ +import { describe, it, expect } from "vitest"; +import { createServer, SERVER_NAME, SERVER_VERSION } from "../src/server.js"; + +describe("createServer", () => { + it("builds a server without connecting a transport", () => { + const server = createServer(); + expect(server).toBeDefined(); + }); + + it("can be called more than once, returning independent servers", () => { + // A hosted deployment builds one per session; module-scope construction + // could not do this. + const a = createServer(); + const b = createServer(); + expect(a).not.toBe(b); + }); + + it("names itself consistently", () => { + expect(SERVER_NAME).toBe("reactome"); + expect(SERVER_VERSION).toMatch(/^\d+\.\d+\.\d+$/); + }); + + it("registers tools and resources", () => { + const server = createServer(); + // Reach through to the underlying Server's registered handlers: the + // McpServer wrapper does not expose a public inventory. + const registered = server as unknown as { + _registeredTools?: Record; + _registeredResources?: Record; + }; + const toolNames = Object.keys(registered._registeredTools ?? {}); + + expect(toolNames.length).toBeGreaterThan(40); + expect(toolNames).toContain("reactome_search"); + expect(toolNames).toContain("reactome_get_pathway"); + expect(toolNames).toContain("reactome_analyze_identifiers"); + }); + + it("keeps the graph tools behind the NEO4J_URI gate", () => { + // Principle IV: no deployment the team runs holds a Neo4j connection. + // NEO4J_URI is unset in the test environment, so these must be absent. + const server = createServer(); + const registered = server as unknown as { _registeredTools?: Record }; + const toolNames = Object.keys(registered._registeredTools ?? {}); + + expect(process.env.NEO4J_URI).toBeFalsy(); + expect(toolNames).not.toContain("reactome_cypher_query"); + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 491d90a..bc882ee 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -16,10 +16,10 @@ export default defineConfig({ // tools still have no test, which is how a token-parsing bug and nine // wrong field paths all shipped unnoticed. thresholds: { - lines: 44, - functions: 41, - branches: 40, - statements: 44, + lines: 50, + functions: 46, + branches: 42, + statements: 50, }, }, },