From 9c7e3d73fd46e7388bd0c79ae4a11bfdbb9283a5 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Mon, 14 Sep 2026 15:29:47 +0000 Subject: [PATCH 1/3] Separate server construction from transport, and share argument validation Harvested from #5 by @adidev001, which predates the current vitest suite. The node:test harness and dist-test build in that PR are superseded, but two ideas in it are worth having. **A server factory.** `createServer()` builds a fully-registered server with no transport attached, and `index.ts` becomes the stdio entrypoint that calls it. Construction no longer happens at module scope, so importing the entrypoint does not start a server. This is not only about testability. A hosted deployment has to serve Streamable HTTP from the same registrations, and it needs one server per session -- neither is possible while the only instance is a module-scope constant. The open hosting question gets easier to answer with this in place. **Shared argument validation.** A blank or whitespace-only argument is never a useful request: an empty `q` reaches the Content Service as either an error or a request for everything, and an empty analysis token produces a 404 that reads like the analysis expired. `nonEmptyString` in src/schemas.ts trims and rejects, so the model gets a message it can act on instead of a confusing service error. 62 argument schemas across seven modules now share it, keeping the existing 2048-character cap. Tests for the factory are adapted from the registration tests in #5. One of them asserts the graph tools stay absent while NEO4J_URI is unset -- Principle IV, and verified to fail when the gate is opened rather than passing vacuously. Co-Authored-By: Claude Opus 5 --- src/index.ts | 16 +++--------- src/schemas.ts | 17 ++++++++++++ src/server.ts | 29 +++++++++++++++++++++ src/tools/analysis.ts | 31 +++++++++++----------- src/tools/entity.ts | 17 ++++++------ src/tools/export.ts | 25 +++++++++--------- src/tools/index.ts | 11 ++++---- src/tools/interactors.ts | 13 +++++----- src/tools/pathway.ts | 11 ++++---- src/tools/search.ts | 26 +++++++++---------- tests/server.test.ts | 56 ++++++++++++++++++++++++++++++++++++++++ 11 files changed, 175 insertions(+), 77 deletions(-) create mode 100644 src/schemas.ts create mode 100644 src/server.ts create mode 100644 tests/server.test.ts 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..18c8679 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,8 +177,8 @@ 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)"), + token: nonEmptyString.describe("Analysis token"), + pathway: nonEmptyString.describe("Pathway stable ID (e.g., R-HSA-109582)"), resource: z .string() .max(2048) @@ -221,7 +222,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 +250,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`); @@ -302,10 +303,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 +334,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..7517a3e 100644 --- a/src/tools/entity.ts +++ b/src/tools/entity.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 { PhysicalEntity, Complex, ReferenceEntity } from "../types/index.js"; @@ -106,7 +107,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 +124,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 +164,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 +195,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 +238,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 +279,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 +308,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( @@ -350,7 +351,7 @@ export function registerEntityTools(server: McpServer) { .string() .max(2048) .describe("Database name (e.g., 'UniProt', 'ChEBI', 'Ensembl')"), - identifier: z.string().max(2048).describe("External identifier (e.g., 'P04637' for UniProt)"), + 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..29e2c3d 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,7 +213,7 @@ 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"), + token: nonEmptyString.describe("Analysis token"), species: z .string() .max(2048) @@ -220,7 +221,7 @@ export function registerExportTools(server: McpServer) { .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,7 +248,7 @@ 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() @@ -296,7 +297,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..888b799 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"; @@ -154,7 +155,7 @@ function registerUtilityTools(server: McpServer) { .string() .max(2048) .describe("Database name (e.g., 'UniProt', 'NCBI', 'Ensembl', 'ChEBI')"), - identifier: z.string().max(2048).describe("External identifier"), + identifier: nonEmptyString.describe("External identifier"), }, async ({ resource, identifier }) => { const pathways = await contentClient.get( @@ -189,7 +190,7 @@ function registerUtilityTools(server: McpServer) { .string() .max(2048) .describe("Database name (e.g., 'UniProt', 'NCBI', 'Ensembl', 'ChEBI')"), - identifier: z.string().max(2048).describe("External identifier"), + identifier: nonEmptyString.describe("External identifier"), }, async ({ resource, identifier }) => { interface Reaction { @@ -225,8 +226,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,7 +259,7 @@ 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"), + id: nonEmptyString.describe("Reactome stable ID or database ID"), attribute: z .string() .max(2048) diff --git a/src/tools/interactors.ts b/src/tools/interactors.ts index 9122901..0c7ab1f 100644 --- a/src/tools/interactors.ts +++ b/src/tools/interactors.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 } from "../types/index.js"; @@ -111,7 +112,7 @@ export function registerInteractorTools(server: McpServer) { .string() .max(2048) .describe("PSICQUIC resource name (e.g., 'IntAct', 'MINT', 'BioGRID')"), - accession: z.string().max(2048).describe("Protein accession (e.g., UniProt ID)"), + accession: nonEmptyString.describe("Protein accession (e.g., UniProt ID)"), }, async ({ resource, accession }) => { const result = await contentClient.get( @@ -137,8 +138,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 +175,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 +207,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 +236,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..e21db93 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( @@ -125,7 +126,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 +157,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 +206,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 +245,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 }) => { diff --git a/src/tools/search.ts b/src/tools/search.ts index 4fad3b8..f7254fe 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"; /** @@ -100,14 +101,11 @@ export function registerSearchTools(server: McpServer) { .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 +152,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 +195,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 +218,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 }); @@ -320,7 +318,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 +362,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"); + }); +}); From 34d8a295bd3f27b3888130e9446b41eedbc6b6f3 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Mon, 14 Sep 2026 15:37:42 +0000 Subject: [PATCH 2/3] Harden the live sweep: content expectations, not just marker grepping The sweep added alongside the Spec Kit work greps rendered output for "undefined", "[object Object]" and empty bodies. Testing it against a deliberately reintroduced bug showed it catches the loud failures and misses the quiet one -- the kind this repo actually shipped. Reverting the search_facets fix and re-running produced: no suspicious output EXIT: 0 because a facets response that has silently dropped every facet still renders a heading, a total, and "*No facets available.*". Three plausible lines, no marker to grep for, and a confident report that there was nothing to report. That is exactly the bug that went unnoticed for months, and the tool built to find it could not. So the sweep now also checks, for 16 tools whose arguments are known to return data, that the answer still contains what it should. With that in place the same experiment reports: reactome_search_facets [missing "### Types:", "### Species:"] Two further problems found while testing the sweep against itself: - It would have failed every scheduled run from the first week. Eight tools were flagged only because the sweep sent a pathway stable ID to a tool that wants a complex and got an honest 404. Per-tool arguments fix the eight, and a reply that is the service reporting an error is now listed separately and does not fail the run -- Reactome answering 500 is not something a release of this package can fix. A job that is always red teaches people to ignore it. - A typo in an expectation's tool name would have silently verified nothing. The run now prints how many expectations were actually checked and warns about any that name a tool which does not exist. Also completes the nonEmptyString migration: 14 more argument schemas were written across several lines by Prettier and so were missed by the first pass. Only the definition itself now spells out the constraint. Coverage thresholds raised 44 -> 50 to match the server-factory tests. Co-Authored-By: Claude Opus 5 --- scripts/sweep-live.mjs | 103 +++++++++++++++++++++++++++++++++++++-- src/tools/analysis.ts | 11 ++--- src/tools/entity.ts | 6 +-- src/tools/export.ts | 11 +---- src/tools/index.ts | 20 +++----- src/tools/interactors.ts | 8 ++- src/tools/pathway.ts | 8 +-- src/tools/search.ts | 15 ++---- vitest.config.ts | 8 +-- 9 files changed, 126 insertions(+), 64 deletions(-) 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/src/tools/analysis.ts b/src/tools/analysis.ts index 18c8679..8f4ffff 100644 --- a/src/tools/analysis.ts +++ b/src/tools/analysis.ts @@ -179,9 +179,7 @@ export function registerAnalysisTools(server: McpServer) { { token: nonEmptyString.describe("Analysis token"), pathway: nonEmptyString.describe("Pathway stable ID (e.g., R-HSA-109582)"), - resource: z - .string() - .max(2048) + resource: nonEmptyString .optional() .default("TOTAL") .describe("Resource filter (TOTAL, UNIPROT, ENSEMBL, etc.)"), @@ -274,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"), }, diff --git a/src/tools/entity.ts b/src/tools/entity.ts index 7517a3e..df33a2e 100644 --- a/src/tools/entity.ts +++ b/src/tools/entity.ts @@ -1,5 +1,4 @@ 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"; @@ -347,10 +346,7 @@ 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')"), + resource: nonEmptyString.describe("Database name (e.g., 'UniProt', 'ChEBI', 'Ensembl')"), identifier: nonEmptyString.describe("External identifier (e.g., 'P04637' for UniProt)"), }, async ({ resource, identifier }) => { diff --git a/src/tools/export.ts b/src/tools/export.ts index 29e2c3d..8968fd3 100644 --- a/src/tools/export.ts +++ b/src/tools/export.ts @@ -214,12 +214,7 @@ export function registerExportTools(server: McpServer) { "Generate a PDF report for an analysis result.", { token: nonEmptyString.describe("Analysis token"), - species: z - .string() - .max(2048) - .optional() - .default("Homo sapiens") - .describe("Species for the report"), + 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: nonEmptyString.optional().default("TOTAL").describe("Resource filter"), }, @@ -250,9 +245,7 @@ export function registerExportTools(server: McpServer) { { 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)"), diff --git a/src/tools/index.ts b/src/tools/index.ts index 888b799..461ae15 100644 --- a/src/tools/index.ts +++ b/src/tools/index.ts @@ -151,10 +151,9 @@ 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')"), + resource: nonEmptyString.describe( + "Database name (e.g., 'UniProt', 'NCBI', 'Ensembl', 'ChEBI')" + ), identifier: nonEmptyString.describe("External identifier"), }, async ({ resource, identifier }) => { @@ -186,10 +185,9 @@ 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')"), + resource: nonEmptyString.describe( + "Database name (e.g., 'UniProt', 'NCBI', 'Ensembl', 'ChEBI')" + ), identifier: nonEmptyString.describe("External identifier"), }, async ({ resource, identifier }) => { @@ -260,11 +258,7 @@ function registerUtilityTools(server: McpServer) { "Query any Reactome database object by its identifier. Returns detailed information about the object.", { id: nonEmptyString.describe("Reactome stable ID or database ID"), - attribute: z - .string() - .max(2048) - .optional() - .describe("Specific attribute to retrieve (optional)"), + 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 0c7ab1f..f4fea97 100644 --- a/src/tools/interactors.ts +++ b/src/tools/interactors.ts @@ -1,5 +1,4 @@ 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"; @@ -108,10 +107,9 @@ 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')"), + resource: nonEmptyString.describe( + "PSICQUIC resource name (e.g., 'IntAct', 'MINT', 'BioGRID')" + ), accession: nonEmptyString.describe("Protein accession (e.g., UniProt ID)"), }, async ({ resource, accession }) => { diff --git a/src/tools/pathway.ts b/src/tools/pathway.ts index e21db93..1fed1f7 100644 --- a/src/tools/pathway.ts +++ b/src/tools/pathway.ts @@ -94,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"), @@ -281,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 f7254fe..9bc7280 100644 --- a/src/tools/search.ts +++ b/src/tools/search.ts @@ -91,13 +91,10 @@ 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 @@ -244,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)"), }, 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, }, }, }, From 095e544d7745ed8bc62fa99536ccbd3e4cac1454 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Mon, 14 Sep 2026 15:38:49 +0000 Subject: [PATCH 3/3] Record specs 001 and 002 001 documents what was found and decided about response shapes -- including that marker-grepping missed the quiet failure and what replaced it. 002 states the transport and hosting question rather than answering it: the server factory is a prerequisite that landed, the rest is open, and the boundaries that are already decided (no Neo4j from a deployed instance, analysis in the Analysis Service) are written down so they are not relitigated. Co-Authored-By: Claude Opus 5 --- specs/001-response-shape-verification/spec.md | 75 +++++++++++++++++++ specs/002-transport-and-hosting/spec.md | 55 ++++++++++++++ 2 files changed, 130 insertions(+) create mode 100644 specs/001-response-shape-verification/spec.md create mode 100644 specs/002-transport-and-hosting/spec.md 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.