Separate server construction from transport, and share argument validation

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;
 }
 

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<T>(...)` 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.

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.

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", {

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);

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;
+}