From c9b70a4eeedd68f2e3c4b24205029c5ad180fdc2 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Wed, 14 Oct 2026 14:33:46 +0000 Subject: [PATCH 1/6] Save the graph schema, and serve it only against its own database One extractor, two uses. `reactome-mcp extract-schema --uri ...` runs fetchGraphSchema() once and writes the schema JSON; NEO4J_SCHEMA_FILE has the server serve such a file instead of extracting at startup. Every schema now carries `schemaFormat` and a `source` fingerprint: Neo4j version and edition, the release from DBInfo (absent where there is no DBInfo node), total node and relationship counts, and when it was read. On the first schema request the server reads the live fingerprint (four cheap queries, not the extraction) and serves the file only if it agrees; another release, a changed count, another format, an incomplete snapshot or a missing or malformed file is warned about by name and the schema is extracted live. A database that cannot be fingerprinted is not a match. fetchGraphSchema() asks the gate first, so with graph access off the file is never read; the gate matrix now asserts that in both directions, and that the CLI imports no server, tool, resource or transport. The CLI is an operator command, not an instance, so it opens the gate for its own process only. It reads through the same gated driver in READ sessions under the schema timeouts, refuses a password argument (`ps` shows it) and takes it from NEO4J_PASSWORD or --password-env, ignores an inherited NEO4J_SCHEMA_FILE, and refuses to write a schema with unavailable sections. Also: the driver hooked SIGINT/SIGTERM/beforeExit on every build and never unhooked, so a process that reconnected piled listeners past Node's leak warning. Hooked once per process now. Co-Authored-By: Claude Opus 5.5 --- src/cli/extract-schema.ts | 165 +++++++++++++++++++++ src/clients/neo4j.ts | 31 ++-- src/config.ts | 9 ++ src/graph/format-schema.ts | 5 +- src/graph/gate.ts | 9 ++ src/graph/schema-source.ts | 177 +++++++++++++++++++++++ src/graph/schema.ts | 119 +++++++++++++-- src/index.ts | 27 +++- src/tools/cypher.ts | 2 +- tests/extract-schema-cli.test.ts | 240 +++++++++++++++++++++++++++++++ tests/graph-access-gate.test.ts | 137 +++++++++++++++++- tests/graph-schema-file.test.ts | 228 +++++++++++++++++++++++++++++ tests/graph-schema.test.ts | 136 ++++++------------ tests/helpers/community44.ts | 94 ++++++++++++ tests/helpers/fake-neo4j.ts | 14 +- 15 files changed, 1267 insertions(+), 126 deletions(-) create mode 100644 src/cli/extract-schema.ts create mode 100644 src/graph/schema-source.ts create mode 100644 tests/extract-schema-cli.test.ts create mode 100644 tests/graph-schema-file.test.ts create mode 100644 tests/helpers/community44.ts diff --git a/src/cli/extract-schema.ts b/src/cli/extract-schema.ts new file mode 100644 index 0000000..b91e1fc --- /dev/null +++ b/src/cli/extract-schema.ts @@ -0,0 +1,165 @@ +import { rename, rm, writeFile } from "node:fs/promises"; +import { closeDriver } from "../clients/neo4j.js"; +import { fetchGraphSchema } from "../graph/schema.js"; + +/** + * `reactome-mcp extract-schema` -- write a database's graph schema to JSON. + * + * reactome-mcp extract-schema --uri bolt://127.0.0.1:7687 [--database NAME] + * [--user USER] [--password-env VAR] [--out FILE] + * + * It runs the server's own extractor, `fetchGraphSchema()`, so a saved schema + * has exactly the structure the server builds live, `source` fingerprint + * included; there is no second implementation to drift from the first. The + * output is what `NEO4J_SCHEMA_FILE` reads. + * + * **Not an instance, so not behind MCP_ALLOW_CYPHER.** That switch is consent + * to *publish* query access. This runs once, by hand, against a database the + * operator names on the command line, and serves nothing -- it imports no + * server, tool, resource or transport, which `tests/graph-access-gate.test.ts` + * checks. It opens the gate for its own process only, after its arguments are + * read, and connects through the same gated driver, so it runs the same READ + * sessions under the same timeouts as the server. + * + * **No password argument.** A command line is visible to every user of the + * machine (`ps`), so the password comes from NEO4J_PASSWORD, or from the + * variable `--password-env` names, and `--password` is refused. + * + * A schema with unavailable sections is not written: a saved schema is served + * for as long as its database matches, so a gap in it would never be filled. + * + * Returns the exit code: 0 written, 1 extraction or write failed, 2 usage. + */ + +const USAGE = `Usage: reactome-mcp extract-schema --uri [options] + +Write the graph schema of a Neo4j database as JSON, in the form the server's +NEO4J_SCHEMA_FILE reads. Read-only. Does not need MCP_ALLOW_CYPHER. + + --uri the database, e.g. bolt://127.0.0.1:7687 (required) + --database database name (default: NEO4J_DATABASE, else the server default) + --user user (default: NEO4J_USER, else neo4j) + --password-env read the password from environment variable VAR + (default: NEO4J_PASSWORD; unset connects without credentials) + --out write here instead of stdout + --help show this + +There is no --password: a password on the command line is visible in \`ps\`. +`; + +export interface CliIo { + stdout: (text: string) => void; + stderr: (text: string) => void; +} + +const processIo: CliIo = { + stdout: text => void process.stdout.write(text), + stderr: text => void process.stderr.write(text), +}; + +const VALUE_FLAGS = ["--uri", "--database", "--user", "--password-env", "--out"] as const; +type ValueFlag = (typeof VALUE_FLAGS)[number]; + +class UsageError extends Error {} + +function parse(args: string[]): { help: boolean; values: Partial> } { + const values: Partial> = {}; + let help = false; + for (let i = 0; i < args.length; i++) { + const arg = args[i] ?? ""; + if (arg === "--help" || arg === "-h") { + help = true; + continue; + } + // Refused by name and never echoed: the value is the secret. + if (arg === "-p" || arg === "--password" || arg.startsWith("--password=")) { + throw new UsageError( + "a password is not accepted on the command line, where `ps` shows it to every " + + "user of the machine. Put it in the environment: NEO4J_PASSWORD, or the " + + "variable named by --password-env." + ); + } + const eq = arg.indexOf("="); + const flag = (eq > 0 ? arg.slice(0, eq) : arg) as ValueFlag; + if (!VALUE_FLAGS.includes(flag)) throw new UsageError(`unknown argument: ${arg}`); + const value = eq > 0 ? arg.slice(eq + 1) : args[++i]; + if (value === undefined || value === "" || (eq < 0 && value.startsWith("--"))) { + throw new UsageError(`${flag} needs a value`); + } + values[flag] = value; + } + return { help, values }; +} + +export async function runExtractSchema(args: string[], io: CliIo = processIo): Promise { + let values: Partial>; + try { + const parsed = parse(args); + if (parsed.help) { + io.stdout(USAGE); + return 0; + } + values = parsed.values; + if (!values["--uri"]) throw new UsageError("--uri is required"); + } catch (error) { + if (!(error instanceof UsageError)) throw error; + io.stderr(`extract-schema: ${error.message}\n\n${USAGE}`); + return 2; + } + + const passwordVar = values["--password-env"]; + let password = process.env.NEO4J_PASSWORD; + if (passwordVar) { + password = process.env[passwordVar]; + if (!password) { + // Connecting without credentials instead would hide the typo. + io.stderr(`extract-schema: --password-env names ${passwordVar}, which is not set\n`); + return 2; + } + } + + // This process only, and only now that the arguments are known good. + process.env.NEO4J_URI = values["--uri"]; + process.env.MCP_ALLOW_CYPHER = "1"; + // Always extract: an inherited snapshot would be copied, not regenerated. + delete process.env.NEO4J_SCHEMA_FILE; + if (values["--database"]) process.env.NEO4J_DATABASE = values["--database"]; + if (values["--user"]) process.env.NEO4J_USER = values["--user"]; + if (password === undefined) delete process.env.NEO4J_PASSWORD; + else process.env.NEO4J_PASSWORD = password; + + const out = values["--out"]; + const partial = out ? `${out}.${process.pid}.partial` : undefined; + try { + const schema = await fetchGraphSchema(); + if (schema.unavailable.length > 0) { + io.stderr( + "extract-schema: not written; these sections could not be read:\n" + + schema.unavailable.map(u => ` ${u.section}: ${u.reason}\n`).join("") + ); + return 1; + } + + const json = JSON.stringify(schema, null, 2) + "\n"; + if (out && partial) { + // Written aside and renamed, so a failed run never leaves a truncated + // file where a good one is expected. + await writeFile(partial, json); + await rename(partial, out); + const s = schema.source; + io.stderr( + `extract-schema: wrote ${out} (release ${s?.releaseNumber ?? "none"}, ` + + `${s?.nodeCount ?? "?"} nodes, ${s?.relCount ?? "?"} relationships)\n` + ); + } else { + io.stdout(json); + } + return 0; + } catch (error) { + if (partial) await rm(partial, { force: true }); + io.stderr(`extract-schema: ${error instanceof Error ? error.message : String(error)}\n`); + return 1; + } finally { + await closeDriver(); + } +} diff --git a/src/clients/neo4j.ts b/src/clients/neo4j.ts index e272264..36b08df 100644 --- a/src/clients/neo4j.ts +++ b/src/clients/neo4j.ts @@ -19,6 +19,7 @@ import { logger } from "../logger.js"; type Neo4jModule = Pick; let driverPromise: Promise<{ neo4j: Neo4jModule; driver: Driver }> | null = null; +let shutdownHooked = false; function isLoopback(uri: string): boolean { try { @@ -45,19 +46,25 @@ async function build(): Promise<{ neo4j: Neo4jModule; driver: Driver }> { const driver = neo4j.driver(uri, auth, { disableLosslessIntegers: true }); logger.info("neo4j driver initialized", { database: database ?? "(server default)" }); - // process.once expects a void-returning listener. Handing it an async - // function meant a failed close rejected with nobody listening, which Node - // turns into a hard exit on unhandled rejection. - const onShutdown = () => { - void closeDriver().catch((error: unknown) => { - logger.warn("neo4j driver close failed during shutdown", { - error: error instanceof Error ? error.message : String(error), + // Hooked once per process, not once per driver: closeDriver() closes + // whichever driver is current, and hooking every build piled listeners up + // in a process that closes and reconnects. + if (!shutdownHooked) { + shutdownHooked = true; + // process.once expects a void-returning listener. Handing it an async + // function meant a failed close rejected with nobody listening, which Node + // turns into a hard exit on unhandled rejection. + const onShutdown = () => { + void closeDriver().catch((error: unknown) => { + logger.warn("neo4j driver close failed during shutdown", { + error: error instanceof Error ? error.message : String(error), + }); }); - }); - }; - process.once("SIGINT", onShutdown); - process.once("SIGTERM", onShutdown); - process.once("beforeExit", onShutdown); + }; + process.once("SIGINT", onShutdown); + process.once("SIGTERM", onShutdown); + process.once("beforeExit", onShutdown); + } return { neo4j, driver }; } diff --git a/src/config.ts b/src/config.ts index 31d2a4e..93b5e65 100644 --- a/src/config.ts +++ b/src/config.ts @@ -89,6 +89,15 @@ export function neo4jConnection(env: NodeJS.ProcessEnv = process.env) { }; } +/** + * A graph schema saved by `reactome-mcp extract-schema`, served instead of + * extracting one when it describes the database at NEO4J_URI. Read only when + * graph access is on; see `src/graph/schema.ts`. + */ +export function neo4jSchemaFile(env: NodeJS.ProcessEnv = process.env): string | undefined { + return env.NEO4J_SCHEMA_FILE?.trim() || undefined; +} + /** * Server-side timeout on every Cypher query, in milliseconds. The database * terminates a transaction that runs longer. diff --git a/src/graph/format-schema.ts b/src/graph/format-schema.ts index 91948b1..662b8ee 100644 --- a/src/graph/format-schema.ts +++ b/src/graph/format-schema.ts @@ -54,8 +54,11 @@ export function formatGraphSchemaMarkdown(schema: GraphSchema): string { const lines: string[] = []; lines.push(`## Reactome Graph Schema`); const dbComp = schema.dbComponents[0]; + const release = schema.source?.releaseNumber; lines.push( - `**Neo4j:** ${dbComp?.versions?.[0] ?? "?"} ${dbComp?.edition ?? ""} · **Fetched:** ${schema.fetchedAt}` + `**Neo4j:** ${dbComp?.versions?.[0] ?? "?"} ${dbComp?.edition ?? ""} · ` + + (release === undefined ? "" : `**Reactome release:** ${release} · `) + + `**Fetched:** ${schema.fetchedAt}` ); lines.push( `**Totals:** ${stats.nodeCount.toLocaleString()} nodes · ${stats.relCount.toLocaleString()} relationships · ${labelEntries.length} labels · ${relEntries.length} relationship types` diff --git a/src/graph/gate.ts b/src/graph/gate.ts index 06d4bb6..7610b68 100644 --- a/src/graph/gate.ts +++ b/src/graph/gate.ts @@ -22,6 +22,15 @@ * * Read from the environment on every call rather than captured at import, so * there is one copy of the answer and it cannot go stale. + * + * One process sets both for itself: `reactome-mcp extract-schema` + * (`src/cli/extract-schema.ts`), an operator command that saves a database's + * schema to a file. It is not an instance -- it runs once, by hand, against a + * database named on its command line, and imports no server, tool, resource + * or transport (asserted in the gate test) -- so the switch that means + * "publish query access" does not apply to it. It still connects through the + * gated driver, so it reads in the same READ sessions under the same + * timeouts. */ export function isGraphAccessEnabled(env: NodeJS.ProcessEnv = process.env): boolean { return Boolean(env.NEO4J_URI?.trim()) && env.MCP_ALLOW_CYPHER === "1"; diff --git a/src/graph/schema-source.ts b/src/graph/schema-source.ts new file mode 100644 index 0000000..3a98197 --- /dev/null +++ b/src/graph/schema-source.ts @@ -0,0 +1,177 @@ +import { z } from "zod"; +import type { GraphSchema } from "./schema.js"; + +/** + * Which database a schema describes, and how to tell whether a schema saved + * to a file still describes the database the server is pointed at. + * + * A saved schema (`reactome-mcp extract-schema`, served through + * `NEO4J_SCHEMA_FILE`) is a description of one database. Served against + * another, it would be read as a description of that one -- labels that are + * not there, counts that are wrong -- which is a confident wrong answer, the + * failure this server is built to avoid. So every schema carries a + * fingerprint, and a saved one is served only when the live database gives + * the same fingerprint back. + * + * The fingerprint is the cheap part of the schema: version, release and two + * totals, each answered without scanning (the totals come from the count + * store). It is not proof of identity -- two databases could agree on all of + * it -- but a different release, or a database that has gained or lost a + * single node or relationship since the file was written, does not match. + */ + +/** + * The layout of the schema JSON. Bump it whenever a field is added, removed or + * changes meaning, so a server never reads a file written for another layout + * as if it were its own. + */ +export const SCHEMA_FORMAT = 1; + +export interface SchemaSource { + /** First component reported by `dbms.components()`. */ + neo4jVersion: string | null; + neo4jEdition: string | null; + /** + * `releaseNumber` of the graph's DBInfo node. Absent, not null, on a + * database without one -- a curator database, or not Reactome's. + */ + releaseNumber?: number | string; + nodeCount: number; + relCount: number; + /** When this fingerprint was read. Not compared. */ + generatedAt: string; +} + +/** The fields compared, in the order a mismatch is reported. */ +const COMPARED = [ + "neo4jVersion", + "neo4jEdition", + "releaseNumber", + "nodeCount", + "relCount", +] as const; + +const show = (value: unknown) => (value === undefined ? "(none)" : JSON.stringify(value)); + +/** + * What differs between the fingerprint a file was saved with and the live + * database's, as `field: file X, database Y`. Empty means they match. + */ +export function sourceMismatches(file: SchemaSource, live: SchemaSource): string[] { + const out: string[] = []; + for (const field of COMPARED) { + const a = file[field]; + const b = live[field]; + // A release stored as 97 and read back as "97" is the same release. + const same = a === undefined || b === undefined ? a === b : String(a) === String(b); + if (!same) out.push(`${field}: file ${show(a)}, database ${show(b)}`); + } + return out; +} + +const names = z.array(z.string()).nullable().optional(); + +const sourceSchema = z.object({ + neo4jVersion: z.string().nullable(), + neo4jEdition: z.string().nullable(), + releaseNumber: z.union([z.number(), z.string()]).optional(), + nodeCount: z.number(), + relCount: z.number(), + generatedAt: z.string(), +}); + +const graphSchemaSchema = z.object({ + schemaFormat: z.number(), + fetchedAt: z.string(), + source: sourceSchema, + dbComponents: z.array( + z.object({ name: z.string(), versions: z.array(z.string()), edition: z.string() }) + ), + stats: z.object({ + nodeCount: z.number(), + relCount: z.number(), + labels: z.record(z.string(), z.number()), + relTypesCount: z.record(z.string(), z.number()), + }), + patterns: z.array(z.object({ start: z.string(), type: z.string(), end: z.string() })), + nodeTypeProperties: z.array( + z.object({ + nodeType: z.string(), + nodeLabels: z.array(z.string()), + propertyName: z.string(), + propertyTypes: z.array(z.string()), + mandatory: z.boolean(), + }) + ), + relTypeProperties: z.array( + z.object({ + relType: z.string(), + propertyName: z.string(), + propertyTypes: z.array(z.string()), + mandatory: z.boolean(), + }) + ), + indexes: z.array( + z.object({ + name: z.string().optional(), + type: z.string().optional(), + entityType: z.string().optional(), + labelsOrTypes: names, + properties: names, + state: z.string().optional(), + }) + ), + constraints: z.array( + z.object({ + name: z.string().optional(), + type: z.string().optional(), + entityType: z.string().optional(), + labelsOrTypes: names, + properties: names, + description: z.string().optional(), + }) + ), + unavailable: z.array(z.object({ section: z.string(), reason: z.string() })), +}); + +/** + * Read a saved schema, or throw saying why it cannot be used: not JSON, + * another format, no fingerprint, a missing or mistyped section, or sections + * that were unavailable when it was saved (a saved schema is served for as + * long as the database matches, so a gap in it would never be filled). + * + * Checked field by field rather than cast, because a cast would let a + * truncated or hand-edited file through as a schema. + */ +export function parseSchemaSnapshot(text: string): GraphSchema & { source: SchemaSource } { + let raw: unknown; + try { + raw = JSON.parse(text); + } catch (error) { + throw new Error(`not valid JSON (${error instanceof Error ? error.message : String(error)})`, { + cause: error, + }); + } + + const format = (raw as { schemaFormat?: unknown } | null)?.schemaFormat; + if (format !== SCHEMA_FORMAT) { + throw new Error(`schemaFormat is ${show(format)}; this server reads ${SCHEMA_FORMAT}`); + } + + const parsed = graphSchemaSchema.safeParse(raw); + if (!parsed.success) { + const where = parsed.error.issues + .slice(0, 3) + .map(i => `${i.path.join(".") || "(root)"}: ${i.message}`) + .join("; "); + throw new Error(`not a schema this server can read (${where})`); + } + + const schema = parsed.data; + if (schema.unavailable.length > 0) { + throw new Error( + `saved with unavailable sections (${schema.unavailable.map(u => u.section).join(", ")})` + ); + } + return schema; +} diff --git a/src/graph/schema.ts b/src/graph/schema.ts index 41fce0a..75d3e8b 100644 --- a/src/graph/schema.ts +++ b/src/graph/schema.ts @@ -1,5 +1,14 @@ +import { readFile } from "node:fs/promises"; import { runRead } from "../clients/neo4j.js"; +import { neo4jSchemaFile } from "../config.js"; import { logger } from "../logger.js"; +import { GRAPH_ACCESS_OFF, isGraphAccessEnabled } from "./gate.js"; +import { + parseSchemaSnapshot, + SCHEMA_FORMAT, + sourceMismatches, + type SchemaSource, +} from "./schema-source.js"; /** * The live graph schema, built from Neo4j's **built-in** procedures only. @@ -44,11 +53,21 @@ import { logger } from "../logger.js"; * * On that graph the whole fetch took about 65 s (2.96M nodes, 11.5M * relationships, 108 labels, 88 types, 5,669 label-level patterns), almost - * all of it `db.schema.nodeTypeProperties()`. Hence the warm-up, and the - * timeout below. + * all of it `db.schema.nodeTypeProperties()`. Hence the warm-up, the timeout + * below, and `NEO4J_SCHEMA_FILE` -- a schema saved earlier by + * `reactome-mcp extract-schema`, served instead when the database it was + * saved from is the one the server is pointed at (see `schema-source.ts`). */ export interface GraphSchema { + /** The layout of this structure; see `SCHEMA_FORMAT`. */ + schemaFormat: number; + /** When the schema was read -- or last re-read, for a retried section. */ fetchedAt: string; + /** + * Which database this describes. Null only when the fingerprint itself + * could not be read, in which case `unavailable` names `source`. + */ + source: SchemaSource | null; dbComponents: Array<{ name: string; versions: string[]; edition: string }>; stats: { nodeCount: number; @@ -109,7 +128,7 @@ export const SCHEMA_RETRY_AFTER_MS = 5 * 60_000; let schemaCache: { schema: GraphSchema; at: number } | null = null; let schemaPending: Promise | null = null; -type Section = keyof Omit; +type Section = keyof Omit; class SectionUnavailable extends Error {} @@ -159,14 +178,50 @@ async function countEach(tokens: string[], pattern: (quoted: string) => string) return Object.fromEntries(rows.map(r => [r.key, Number(r.count)])); } +const COMPONENTS = + "CALL dbms.components() YIELD name, versions, edition RETURN name, versions, edition"; +// Both totals are answered from the count store, not by scanning. +const NODE_TOTAL = "MATCH (n) RETURN count(n) AS count"; +const REL_TOTAL = "MATCH ()-[r]->() RETURN count(r) AS count"; +// Reactome's graph has one DBInfo node; a curator or non-Reactome graph has +// none, and this returns no row. Ordered so that a graph with several gives +// the same answer every time. +const RELEASE = + "MATCH (d:DBInfo) RETURN d.releaseNumber AS releaseNumber ORDER BY releaseNumber DESC LIMIT 1"; + +/** + * The cheap part of the schema that identifies the database: four queries, + * none of which scans. This is what a saved schema is checked against before + * it is served; see `schema-source.ts`. + */ +export async function fetchSourceFingerprint(): Promise { + const [components, release, nodeTotal, relTotal] = await Promise.all([ + firstThatWorks<{ versions?: string[]; edition?: string }>([COMPONENTS]), + firstThatWorks<{ releaseNumber: unknown }>([RELEASE]), + firstThatWorks<{ count: number }>([NODE_TOTAL]), + firstThatWorks<{ count: number }>([REL_TOTAL]), + ]); + const releaseNumber = release[0]?.releaseNumber; + return { + neo4jVersion: components[0]?.versions?.[0] ?? null, + neo4jEdition: components[0]?.edition ?? null, + ...(typeof releaseNumber === "number" || typeof releaseNumber === "string" + ? { releaseNumber } + : {}), + nodeCount: Number(nodeTotal[0]?.count ?? 0), + relCount: Number(relTotal[0]?.count ?? 0), + generatedAt: new Date().toISOString(), + }; +} + async function fetchStats(): Promise { const [labelRows, relRows, nodeTotal, relTotal] = await Promise.all([ firstThatWorks<{ label: string }>(["CALL db.labels() YIELD label RETURN label"]), firstThatWorks<{ relationshipType: string }>([ "CALL db.relationshipTypes() YIELD relationshipType RETURN relationshipType", ]), - firstThatWorks<{ count: number }>(["MATCH (n) RETURN count(n) AS count"]), - firstThatWorks<{ count: number }>(["MATCH ()-[r]->() RETURN count(r) AS count"]), + firstThatWorks<{ count: number }>([NODE_TOTAL]), + firstThatWorks<{ count: number }>([REL_TOTAL]), ]); const [labels, relTypesCount] = await Promise.all([ countEach( @@ -220,10 +275,8 @@ async function fetchPatterns(): Promise { } const SECTIONS: { [K in Section]: () => Promise } = { - dbComponents: () => - firstThatWorks([ - "CALL dbms.components() YIELD name, versions, edition RETURN name, versions, edition", - ]), + source: fetchSourceFingerprint, + dbComponents: () => firstThatWorks([COMPONENTS]), stats: fetchStats, patterns: fetchPatterns, nodeTypeProperties: () => @@ -253,6 +306,8 @@ const SECTIONS: { [K in Section]: () => Promise } = { }; const EMPTY: Omit = { + schemaFormat: SCHEMA_FORMAT, + source: null, dbComponents: [], stats: { nodeCount: 0, relCount: 0, labels: {}, relTypesCount: {} }, patterns: [], @@ -273,8 +328,17 @@ const EMPTY: Omit = { * A fetch in which **no** section could be read -- the database is down, the * credentials are wrong -- throws and is not cached, so the next call tries * again instead of serving an empty schema. + * + * With `NEO4J_SCHEMA_FILE` set, the first fetch tries that file before + * extracting: it is served only if the live fingerprint matches the one it + * was saved with. Anything else is logged, naming why, and the schema is + * extracted as though no file were set. + * + * Asks the gate first, so with graph access off neither the database nor the + * file is touched. */ export async function fetchGraphSchema(): Promise { + if (!isGraphAccessEnabled()) throw new Error(GRAPH_ACCESS_OFF); if (schemaCache) { const { schema, at } = schemaCache; if (schema.unavailable.length === 0) return schema; @@ -286,6 +350,19 @@ export async function fetchGraphSchema(): Promise { const start = Date.now(); schemaPending = (async () => { try { + const file = previous ? undefined : neo4jSchemaFile(); + const saved = file ? await matchingSnapshot(file) : null; + if (saved) { + logger.info("graph schema served from file", { + file, + durationMs: Date.now() - start, + releaseNumber: saved.source?.releaseNumber, + nodeCount: saved.stats.nodeCount, + }); + schemaCache = { schema: saved, at: Date.now() }; + return saved; + } + const names = previous ? previous.unavailable.map(u => u.section as Section) : (Object.keys(SECTIONS) as Section[]); @@ -328,6 +405,30 @@ export async function fetchGraphSchema(): Promise { return schemaPending; } +/** + * The schema saved in `file`, if it describes the database this server is + * pointed at; otherwise null, with a warning that says why not. A database + * that cannot be fingerprinted is not a match: unverified, the file could be + * describing anything. + */ +async function matchingSnapshot(file: string): Promise { + try { + const saved = parseSchemaSnapshot(await readFile(file, "utf8")); + const live = await fetchSourceFingerprint(); + const differences = sourceMismatches(saved.source, live); + if (differences.length > 0) { + throw new Error(`it describes a different database (${differences.join("; ")})`); + } + return saved; + } catch (error) { + logger.warn("NEO4J_SCHEMA_FILE not used; extracting the schema from the database", { + file, + reason: message(error), + }); + return null; + } +} + /** For tests -- clears both the cached value and any in-flight fetch. */ export function _resetGraphSchemaCache(): void { schemaCache = null; diff --git a/src/index.ts b/src/index.ts index c0fea30..90387fc 100644 --- a/src/index.ts +++ b/src/index.ts @@ -22,9 +22,26 @@ async function main() { warmGraphSchema(); } -main().catch(error => { - logger.error("fatal error during startup", { - error: error instanceof Error ? error.message : String(error), +if (process.argv[2] === "extract-schema") { + // An operator command, not the server: no server is built and no transport + // opened. `process.exitCode` rather than `process.exit()`, so the JSON + // written to stdout drains before the process ends. + import("./cli/extract-schema.js") + .then(({ runExtractSchema }) => runExtractSchema(process.argv.slice(3))) + .then(code => { + process.exitCode = code; + }) + .catch((error: unknown) => { + logger.error("extract-schema failed", { + error: error instanceof Error ? error.message : String(error), + }); + process.exitCode = 1; + }); +} else { + main().catch(error => { + logger.error("fatal error during startup", { + error: error instanceof Error ? error.message : String(error), + }); + process.exit(1); }); - process.exit(1); -}); +} diff --git a/src/tools/cypher.ts b/src/tools/cypher.ts index 830ab2d..c609df6 100644 --- a/src/tools/cypher.ts +++ b/src/tools/cypher.ts @@ -163,7 +163,7 @@ export function registerCypherTools(server: McpServer) { server.tool( "reactome_cypher_schema", - "Introspect the Reactome graph schema: labels and relationship types with counts, which labels each relationship type connects, property names and types with mandatory flags, indexes and constraints. Read live from Neo4j's built-in schema procedures and cached for the life of the server (the server warms it at startup; a cold first call can take tens of seconds). Call this before writing Cypher. Sections the database could not provide are listed as unavailable rather than shown empty. For the full JSON, read the `reactome://graph/schema` resource.", + "Introspect the Reactome graph schema: labels and relationship types with counts, which labels each relationship type connects, property names and types with mandatory flags, indexes and constraints. Read live from Neo4j's built-in schema procedures and cached for the life of the server (the server warms it at startup; a cold first call can take tens of seconds), or taken from a schema file the operator saved from this same database -- never one saved from another. Call this before writing Cypher. Sections the database could not provide are listed as unavailable rather than shown empty. For the full JSON, read the `reactome://graph/schema` resource.", {}, async () => { logger.info("cypher_schema"); diff --git a/tests/extract-schema-cli.test.ts b/tests/extract-schema-cli.test.ts new file mode 100644 index 0000000..ab34b42 --- /dev/null +++ b/tests/extract-schema-cli.test.ts @@ -0,0 +1,240 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { FakeNeo4jState } from "./helpers/fake-neo4j.js"; + +/** + * `reactome-mcp extract-schema`: the server's own extractor, run once against + * a database named on the command line, written out as JSON. + * + * It is an operator tool run by hand, not an instance: it serves nothing, so + * it does not ask for MCP_ALLOW_CYPHER. It still reads only, under the same + * timeouts as the server, and it never takes a password as an argument -- + * anything on a command line is visible to every user of the machine in `ps`. + */ + +const fake = vi.hoisted((): FakeNeo4jState => ({ drivers: [], sessions: [], respond: () => [] })); + +vi.mock("neo4j-driver", async () => { + const { fakeNeo4jModule } = await import("./helpers/fake-neo4j.js"); + return fakeNeo4jModule(fake); +}); + +import { queriesRun } from "./helpers/fake-neo4j.js"; +import { community44 } from "./helpers/community44.js"; +import { runExtractSchema } from "../src/cli/extract-schema.js"; +import { fetchGraphSchema, _resetGraphSchemaCache } from "../src/graph/schema.js"; +import { SCHEMA_FORMAT } from "../src/graph/schema-source.js"; +import { closeDriver } from "../src/clients/neo4j.js"; + +const URI = "bolt://127.0.0.1:7687"; +const ENV_KEYS = [ + "NEO4J_URI", + "MCP_ALLOW_CYPHER", + "NEO4J_USER", + "NEO4J_PASSWORD", + "NEO4J_DATABASE", + "NEO4J_SCHEMA_FILE", + "SNAPSHOT_PW", +]; + +let saved: Record; +let dir: string; +let out: string[]; +let err: string[]; + +const io = () => ({ stdout: (s: string) => out.push(s), stderr: (s: string) => err.push(s) }); +const run = (...args: string[]) => runExtractSchema(args, io()); + +/** Timestamps differ between two extractions of the same database. */ +function untimed(schema: Record) { + const source = schema.source as Record; + return { ...schema, fetchedAt: "T", source: { ...source, generatedAt: "T" } }; +} + +describe("reactome-mcp extract-schema", () => { + beforeEach(() => { + saved = Object.fromEntries(ENV_KEYS.map(k => [k, process.env[k]])); + for (const k of ENV_KEYS) delete process.env[k]; + fake.drivers.length = 0; + fake.sessions.length = 0; + fake.respond = community44(); + _resetGraphSchemaCache(); + dir = mkdtempSync(join(tmpdir(), "extract-schema-")); + out = []; + err = []; + }); + + afterEach(async () => { + _resetGraphSchemaCache(); + await closeDriver(); + rmSync(dir, { recursive: true, force: true }); + for (const [k, v] of Object.entries(saved)) { + if (v === undefined) delete process.env[k]; + else process.env[k] = v; + } + }); + + it("writes what the server's extractor produces, with the source fingerprint", async () => { + const file = join(dir, "reactome-97-schema.json"); + expect(await run("--uri", URI, "--out", file)).toBe(0); + const written = JSON.parse(readFileSync(file, "utf8")); + + expect(written.schemaFormat).toBe(SCHEMA_FORMAT); + expect(written.source).toMatchObject({ + neo4jVersion: "4.4.40", + neo4jEdition: "community", + releaseNumber: 97, + nodeCount: 3_000_000, + relCount: 12_000_000, + }); + expect(written.unavailable).toEqual([]); + + // One extractor: the same code, in the server, gives the same structure. + _resetGraphSchemaCache(); + await closeDriver(); + process.env.NEO4J_URI = URI; + process.env.MCP_ALLOW_CYPHER = "1"; + const served = JSON.parse(JSON.stringify(await fetchGraphSchema())); + expect(untimed(written)).toEqual(untimed(served)); + expect(out.join("")).toBe(""); + }); + + it("writes to stdout when no --out is given, and nothing else there", async () => { + expect(await run("--uri", URI)).toBe(0); + const parsed = JSON.parse(out.join("")); + expect(parsed.source.releaseNumber).toBe(97); + }); + + it("omits the release for a database without a DBInfo node", async () => { + fake.respond = community44({ DBInfo: () => [] }); + expect(await run("--uri", URI)).toBe(0); + const parsed = JSON.parse(out.join("")); + expect(parsed.source).not.toHaveProperty("releaseNumber"); + expect(parsed.source.nodeCount).toBe(3_000_000); + }); + + it("does not need MCP_ALLOW_CYPHER or NEO4J_URI in the environment", async () => { + expect(process.env.MCP_ALLOW_CYPHER).toBeUndefined(); + expect(await run("--uri", URI)).toBe(0); + expect(fake.drivers.map(d => d.uri)).toEqual([URI]); + }); + + it("reads only, under a timeout, against the database named", async () => { + expect(await run("--uri", URI, "--database", "release97", "--user", "reader")).toBe(0); + expect(fake.sessions.length).toBeGreaterThan(0); + for (const s of fake.sessions) { + expect(s.config.defaultAccessMode).toBe("READ"); + expect(s.config.database).toBe("release97"); + for (const r of s.runs) expect(r.config?.timeout).toBeGreaterThan(0); + } + }); + + it("always extracts, even with NEO4J_SCHEMA_FILE inherited from the environment", async () => { + // Otherwise a matching snapshot would be copied rather than regenerated. + // A snapshot that matches this database, so the server would serve it. + expect(await run("--uri", URI)).toBe(0); + const matching = join(dir, "matching.json"); + writeFileSync(matching, out.join("")); + _resetGraphSchemaCache(); + await closeDriver(); + fake.sessions.length = 0; + out = []; + + process.env.NEO4J_SCHEMA_FILE = matching; + expect(await run("--uri", URI)).toBe(0); + expect(queriesRun(fake).some(q => q.includes("db.schema.nodeTypeProperties"))).toBe(true); + }); + + describe("passwords", () => { + it.each([["--password", "hunter2"], ["--password=hunter2"], ["-p", "hunter2"]])( + "refuses a password on the command line: %s", + async (...flag) => { + expect(await run("--uri", URI, ...flag)).toBe(2); + expect(err.join("")).toMatch(/environment/i); + expect(err.join("")).not.toContain("hunter2"); + expect(fake.drivers).toHaveLength(0); + } + ); + + it("reads NEO4J_PASSWORD from the environment", async () => { + process.env.NEO4J_PASSWORD = "from-env"; + expect(await run("--uri", URI, "--user", "reader")).toBe(0); + expect(fake.drivers[0]?.auth).toEqual({ + scheme: "basic", + principal: "reader", + credentials: "from-env", + }); + }); + + it("reads the variable --password-env names", async () => { + process.env.NEO4J_PASSWORD = "not-this-one"; + process.env.SNAPSHOT_PW = "this-one"; + expect(await run("--uri", URI, "--password-env", "SNAPSHOT_PW")).toBe(0); + expect(fake.drivers[0]?.auth).toMatchObject({ credentials: "this-one" }); + }); + + it("refuses a --password-env that names an unset variable", async () => { + // Connecting without credentials instead would hide the typo. + expect(await run("--uri", URI, "--password-env", "SNAPSHOT_PW")).toBe(2); + expect(err.join("")).toContain("SNAPSHOT_PW"); + expect(fake.drivers).toHaveLength(0); + }); + }); + + it.each([ + [[], /--uri/], + [["--uri"], /--uri/], + [["--uri", URI, "--bogus"], /--bogus/], + [["--uri", URI, "--out"], /--out/], + ])("refuses %j with usage, connecting to nothing", async (args, message) => { + expect(await run(...args)).toBe(2); + expect(err.join("")).toMatch(message); + expect(fake.drivers).toHaveLength(0); + }); + + it("prints usage for --help and connects to nothing", async () => { + expect(await run("--help")).toBe(0); + expect(out.join("")).toContain("--password-env"); + expect(fake.drivers).toHaveLength(0); + }); + + it("refuses to write an incomplete schema", async () => { + // A snapshot is served for as long as the database matches; one with a + // section missing would stay missing for all that time. + fake.respond = community44({ + "db.schema.nodeTypeProperties": () => { + throw new Error("The transaction has been terminated (timeout)"); + }, + }); + const file = join(dir, "schema.json"); + expect(await run("--uri", URI, "--out", file)).toBe(1); + expect(existsSync(file)).toBe(false); + expect(err.join("")).toContain("nodeTypeProperties"); + }); + + it("leaves no shutdown listeners behind, however many times a driver is built", async () => { + // Each build added SIGINT, SIGTERM and beforeExit listeners and nothing + // removed them, so a process that closes and reconnects -- this CLI run + // twice, the tests -- piled them up past Node's leak warning. + const before = process.listenerCount("SIGINT"); + const beforeExit = process.listenerCount("beforeExit"); + for (let i = 0; i < 12; i++) { + _resetGraphSchemaCache(); + expect(await run("--uri", URI)).toBe(0); + } + expect(process.listenerCount("SIGINT")).toBeLessThanOrEqual(before + 1); + expect(process.listenerCount("beforeExit")).toBeLessThanOrEqual(beforeExit + 1); + }); + + it("fails, writing nothing, when the database does not answer", async () => { + fake.respond = () => { + throw new Error("Could not perform discovery. No routing servers available."); + }; + const file = join(dir, "schema.json"); + expect(await run("--uri", URI, "--out", file)).toBe(1); + expect(existsSync(file)).toBe(false); + expect(err.join("")).toMatch(/schema/i); + }); +}); diff --git a/tests/graph-access-gate.test.ts b/tests/graph-access-gate.test.ts index 6a991d3..853ffd8 100644 --- a/tests/graph-access-gate.test.ts +++ b/tests/graph-access-gate.test.ts @@ -1,5 +1,6 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { readFileSync, readdirSync, statSync } from "node:fs"; +import { describe, it, expect, vi, beforeEach, afterEach, afterAll } from "vitest"; +import { mkdtempSync, readFileSync, readdirSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; import { join, relative } from "node:path"; import type { AddressInfo } from "node:net"; import type { FakeNeo4jState } from "./helpers/fake-neo4j.js"; @@ -35,6 +36,35 @@ function mockDriverModule() { const { fakeNeo4jModule } = await import("./helpers/fake-neo4j.js"); return fakeNeo4jModule(fake); }); + mockFileReads(); +} + +/** + * A schema snapshot named by NEO4J_SCHEMA_FILE in every case below. With the + * gate closed it must not even be read: the file describes the graph, and an + * instance without graph access has no business with it. + */ +const SNAPSHOT_DIR = mkdtempSync(join(tmpdir(), "gate-snapshot-")); +const SNAPSHOT = join(SNAPSHOT_DIR, "schema.json"); +writeFileSync(SNAPSHOT, JSON.stringify({ schemaFormat: 1 })); +afterAll(() => rmSync(SNAPSHOT_DIR, { recursive: true, force: true })); + +const fileReads = vi.hoisted(() => [] as string[]); +const snapshotReads = () => fileReads.filter(f => f === SNAPSHOT).length; + +/** Count reads through node:fs/promises, re-registered per test like the driver. */ +function mockFileReads() { + vi.doMock("node:fs/promises", async importOriginal => { + const actual = await importOriginal(); + const readFile = ((...args: Parameters) => { + const [path] = args; + fileReads.push( + typeof path === "string" ? path : path instanceof URL ? path.pathname : "(file handle)" + ); + return actual.readFile(...args); + }) as typeof actual.readFile; + return { ...actual, default: { ...actual, readFile }, readFile }; + }); } // The stdio entrypoint connects a transport on import. This one talks to @@ -51,8 +81,33 @@ vi.mock("@modelcontextprotocol/sdk/server/stdio.js", () => ({ }, })); +/** + * Each test gets a fresh copy of the driver module (vi.resetModules), and each + * copy hooks process shutdown once. In a real process there is one copy; here + * the hooks are removed after every test so they do not pile up across them. + */ +const SHUTDOWN_EVENTS = ["SIGINT", "SIGTERM", "beforeExit"] as const; +let shutdownListeners: Map; +beforeEach(() => { + shutdownListeners = new Map(SHUTDOWN_EVENTS.map(e => [e, process.listeners(e)])); +}); +afterEach(() => { + for (const event of SHUTDOWN_EVENTS) { + const before = shutdownListeners.get(event) ?? []; + for (const listener of process.listeners(event)) { + if (!before.includes(listener)) process.removeListener(event, listener); + } + } +}); + const URI = "bolt://127.0.0.1:7687"; -const GRAPH_VARS = ["NEO4J_URI", "MCP_ALLOW_CYPHER", "MCP_TOOL_GROUPS", "MCP_HTTP_PORT"] as const; +const GRAPH_VARS = [ + "NEO4J_URI", + "MCP_ALLOW_CYPHER", + "MCP_TOOL_GROUPS", + "MCP_HTTP_PORT", + "NEO4J_SCHEMA_FILE", +] as const; interface GateCase { name: string; @@ -105,6 +160,7 @@ function resetFake() { fake.sessions.length = 0; fake.respond = () => []; fake.moduleLoads = 0; + fileReads.length = 0; } /** Let fire-and-forget startup work (the schema warm-up) run to completion. */ @@ -134,7 +190,7 @@ function recordingServer() { describe.each(CASES)("graph access gate: $name", ({ env, open }) => { beforeEach(() => { - applyEnv(env); + applyEnv({ ...env, NEO4J_SCHEMA_FILE: SNAPSHOT }); resetFake(); vi.resetModules(); mockDriverModule(); @@ -180,6 +236,7 @@ describe.each(CASES)("graph access gate: $name", ({ env, open }) => { // Building a server is not a reason to connect: the driver is lazy. expect(fake.drivers).toHaveLength(0); + expect(snapshotReads()).toBe(0); }); it(`registrars given every group explicitly still ${open ? "offer" : "withhold"} graph access`, async () => { @@ -231,12 +288,28 @@ describe.each(CASES)("graph access gate: $name", ({ env, open }) => { } }); + it(`fetchGraphSchema, called directly, ${open ? "checks" : "never reads"} the schema file`, async () => { + // The warm-up asks the gate before calling it; this is the check inside, + // for a caller that does not. + const { fetchGraphSchema } = await import("../src/graph/schema.js"); + const outcome = await fetchGraphSchema().then( + () => "served", + (error: unknown) => String(error) + ); + expect(snapshotReads()).toBe(open ? 1 : 0); + if (!open) expect(outcome).toMatch(/MCP_ALLOW_CYPHER/); + expect(fake.drivers).toHaveLength(open ? 1 : 0); + }); + it(`the stdio entrypoint ${open ? "warms the schema over one driver" : "never constructs a driver"}`, async () => { await import("../src/index.js"); await settle(); expect(fake.drivers).toHaveLength(open ? 1 : 0); // A closed instance never loads neo4j-driver at all. expect(fake.moduleLoads).toBe(open ? 1 : 0); + // Nor reads the schema snapshot; an open one reads it once, to check it + // against the database before trusting it. + expect(snapshotReads()).toBe(open ? 1 : 0); if (open) { // The warm-up ran, read-only, on the dedicated graph. expect(fake.drivers[0]?.uri).toBe(URI); @@ -255,6 +328,7 @@ describe.each(CASES)("graph access gate: $name", ({ env, open }) => { await settle(); expect(fake.drivers).toHaveLength(open ? 1 : 0); expect(fake.moduleLoads).toBe(open ? 1 : 0); + expect(snapshotReads()).toBe(open ? 1 : 0); }); it(`startHttpServer sessions ${open ? "offer" : "withhold"} graph access, and construct no driver`, async () => { @@ -320,6 +394,7 @@ describe.each(CASES)("graph access gate: $name", ({ env, open }) => { // Serving sessions is not a reason to connect, or to load the driver. expect(fake.drivers).toHaveLength(0); expect(fake.moduleLoads).toBe(0); + expect(snapshotReads()).toBe(0); } finally { await new Promise(resolve => http.close(resolve)); } @@ -328,7 +403,12 @@ describe.each(CASES)("graph access gate: $name", ({ env, open }) => { describe("graph access gate: open, but the cypher group withheld", () => { beforeEach(() => { - applyEnv({ NEO4J_URI: URI, MCP_ALLOW_CYPHER: "1", MCP_TOOL_GROUPS: "search,pathway" }); + applyEnv({ + NEO4J_URI: URI, + MCP_ALLOW_CYPHER: "1", + MCP_TOOL_GROUPS: "search,pathway", + NEO4J_SCHEMA_FILE: SNAPSHOT, + }); resetFake(); vi.resetModules(); mockDriverModule(); @@ -353,6 +433,7 @@ describe("graph access gate: open, but the cypher group withheld", () => { await import("../src/index.js"); await settle(); expect(fake.drivers).toHaveLength(0); + expect(snapshotReads()).toBe(0); }); }); @@ -378,6 +459,17 @@ describe("graph access gate: structure", () => { expect(importers).toEqual(["clients/neo4j.ts"]); }); + it("the extract-schema CLI builds no server and registers nothing", () => { + // The CLI connects without MCP_ALLOW_CYPHER because it is not an + // instance: it runs once, by hand, and serves nothing. That holds only + // while it cannot reach a tool, a resource or a transport. + const text = readFileSync(new URL("../src/cli/extract-schema.ts", import.meta.url), "utf8"); + const imports = [...text.matchAll(/from\s+["']([^"']+)["']/g)].map(m => m[1]); + expect(imports.filter(i => /server|http|tools|resources|instructions/.test(i ?? ""))).toEqual( + [] + ); + }); + it("keeps neo4j-driver as a declared dependency", () => { const pkg = JSON.parse(readFileSync(new URL("../package.json", import.meta.url), "utf8")); expect(pkg.dependencies["neo4j-driver"]).toBeDefined(); @@ -419,3 +511,38 @@ describe("graph access gate: /health reports the gate as it is now", () => { } }); }); + +describe("graph access gate: the extract-schema subcommand", () => { + const argv = process.argv; + + beforeEach(() => { + applyEnv({}); + resetFake(); + vi.resetModules(); + mockDriverModule(); + }); + + afterEach(() => { + process.argv = argv; + process.exitCode = undefined; + restoreEnv(); + vi.doUnmock("../src/cli/extract-schema.js"); + vi.doUnmock("../src/server.js"); + vi.resetModules(); + }); + + it("runs the CLI instead of the server, and builds no server", async () => { + const runExtractSchema = vi.fn(async () => 0); + const createServer = vi.fn(); + vi.doMock("../src/cli/extract-schema.js", () => ({ runExtractSchema })); + vi.doMock("../src/server.js", () => ({ createServer })); + process.argv = [argv[0] ?? "node", "dist/index.js", "extract-schema", "--help"]; + + await import("../src/index.js"); + await settle(); + + expect(runExtractSchema).toHaveBeenCalledWith(["--help"]); + expect(createServer).not.toHaveBeenCalled(); + expect(fake.drivers).toHaveLength(0); + }); +}); diff --git a/tests/graph-schema-file.test.ts b/tests/graph-schema-file.test.ts new file mode 100644 index 0000000..164e55b --- /dev/null +++ b/tests/graph-schema-file.test.ts @@ -0,0 +1,228 @@ +import { describe, it, expect, vi, beforeEach, afterEach, type MockInstance } from "vitest"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { FakeNeo4jState } from "./helpers/fake-neo4j.js"; +import type { GraphSchema } from "../src/graph/schema.js"; + +/** + * `NEO4J_SCHEMA_FILE`: serve a schema extracted earlier instead of paying for + * the full extraction at startup -- but only for the database it describes. + * + * The server reads the file, takes the cheap part of the live fingerprint + * (version, release, total counts), and serves the file only if every field + * agrees. Anything else -- another release, a database that has changed, an + * older file format, a file that is not JSON, or is not there -- is warned + * about by name and the schema is extracted live, as if no file were set. A + * schema for a different database is never served: it would be read as a + * description of this one. + */ + +const fake = vi.hoisted((): FakeNeo4jState => ({ drivers: [], sessions: [], respond: () => [] })); + +vi.mock("neo4j-driver", async () => { + const { fakeNeo4jModule } = await import("./helpers/fake-neo4j.js"); + return fakeNeo4jModule(fake); +}); + +import { queriesRun } from "./helpers/fake-neo4j.js"; +import { community44 } from "./helpers/community44.js"; +import { fetchGraphSchema, _resetGraphSchemaCache } from "../src/graph/schema.js"; +import { SCHEMA_FORMAT } from "../src/graph/schema-source.js"; +import { closeDriver } from "../src/clients/neo4j.js"; +import { logger } from "../src/logger.js"; + +/** What only the full extraction runs. None of it may run when the file is used. */ +const EXPENSIVE = [ + "db.schema.nodeTypeProperties", + "db.schema.relTypeProperties", + "db.schema.visualization", + "db.labels", + "db.relationshipTypes", + "SHOW INDEXES", + "SHOW CONSTRAINTS", + "UNION ALL", +]; +const expensiveRun = () => queriesRun(fake).filter(q => EXPENSIVE.some(e => q.includes(e))); + +let dir: string; +let snapshot: GraphSchema; +let warn: MockInstance; + +/** A snapshot of the scripted database, taken by the extractor itself. */ +async function takeSnapshot(): Promise { + const schema = await fetchGraphSchema(); + _resetGraphSchemaCache(); + fake.sessions.length = 0; + return structuredClone(schema); +} + +function writeSnapshot(content: unknown): string { + const file = join(dir, "schema.json"); + writeFileSync(file, typeof content === "string" ? content : JSON.stringify(content)); + process.env.NEO4J_SCHEMA_FILE = file; + return file; +} + +const warnings = () => warn.mock.calls.map(c => JSON.stringify(c)); + +describe("NEO4J_SCHEMA_FILE", () => { + beforeEach(async () => { + process.env.NEO4J_URI = "bolt://127.0.0.1:7687"; + process.env.MCP_ALLOW_CYPHER = "1"; + delete process.env.NEO4J_SCHEMA_FILE; + fake.drivers.length = 0; + fake.sessions.length = 0; + fake.respond = community44(); + _resetGraphSchemaCache(); + dir = mkdtempSync(join(tmpdir(), "schema-file-")); + snapshot = await takeSnapshot(); + warn = vi.spyOn(logger, "warn"); + }); + + afterEach(async () => { + warn.mockRestore(); + _resetGraphSchemaCache(); + await closeDriver(); + rmSync(dir, { recursive: true, force: true }); + delete process.env.NEO4J_URI; + delete process.env.MCP_ALLOW_CYPHER; + delete process.env.NEO4J_SCHEMA_FILE; + }); + + it("serves a matching file without running the full extraction", async () => { + writeSnapshot(snapshot); + + const schema = await fetchGraphSchema(); + + expect(schema).toEqual(snapshot); + expect(expensiveRun()).toEqual([]); + // It did check: the cheap fingerprint queries ran, read-only. + const run = queriesRun(fake); + expect(run.some(q => q.includes("dbms.components"))).toBe(true); + expect(run.some(q => q.includes("DBInfo"))).toBe(true); + for (const s of fake.sessions) expect(s.config.defaultAccessMode).toBe("READ"); + expect(warnings()).toEqual([]); + + // Complete, so cached: a second call runs nothing at all. + const before = queriesRun(fake).length; + expect(await fetchGraphSchema()).toBe(schema); + expect(queriesRun(fake).length).toBe(before); + }); + + it("serves a file without a release against a database without a DBInfo node", async () => { + fake.respond = community44({ DBInfo: () => [] }); + const curator = await takeSnapshot(); + expect(curator.source).not.toHaveProperty("releaseNumber"); + writeSnapshot(curator); + + expect(await fetchGraphSchema()).toEqual(curator); + expect(expensiveRun()).toEqual([]); + }); + + type Case = { + name: string; + /** Applied to the file, the database, or both. */ + file?: (s: GraphSchema) => unknown; + db?: Parameters[0]; + reason: RegExp; + }; + + const CASES: Case[] = [ + { + name: "a different Reactome release", + db: { DBInfo: () => [{ releaseNumber: 98 }] }, + reason: /releaseNumber/, + }, + { + name: "a release in the file where the database has no DBInfo node", + db: { DBInfo: () => [] }, + reason: /releaseNumber/, + }, + { + name: "a different node count (the database has changed since)", + db: { "MATCH (n) RETURN count(n)": () => [{ count: 3_000_001 }] }, + reason: /nodeCount/, + }, + { + name: "a different relationship count", + db: { "MATCH ()-[r]->() RETURN count(r)": () => [{ count: 1 }] }, + reason: /relCount/, + }, + { + name: "a different Neo4j version", + db: { + "dbms.components": () => [ + { name: "Neo4j Kernel", versions: ["5.26.0"], edition: "community" }, + ], + }, + reason: /neo4jVersion/, + }, + { + name: "another schema format", + file: s => ({ ...s, schemaFormat: SCHEMA_FORMAT + 1 }), + reason: /schemaFormat/, + }, + { + name: "no schema format at all (a file from before there was one)", + file: ({ schemaFormat: _f, ...rest }) => rest, + reason: /schemaFormat/, + }, + { + name: "no source fingerprint", + file: s => ({ ...s, source: null }), + reason: /source/, + }, + { + name: "a file that is not JSON", + file: () => '{"schemaFormat": 1, "stats": ', + reason: /JSON/, + }, + { + name: "JSON that is not a schema", + file: s => ({ schemaFormat: s.schemaFormat, source: s.source }), + reason: /patterns|stats|nodeTypeProperties/, + }, + { + name: "a snapshot with unavailable sections", + file: s => ({ ...s, unavailable: [{ section: "indexes", reason: "timeout" }] }), + reason: /unavailable/, + }, + ]; + + it.each(CASES)("falls back to live extraction, with a warning, on $name", async c => { + writeSnapshot(c.file ? c.file(snapshot) : snapshot); + if (c.db) fake.respond = community44(c.db); + + const schema = await fetchGraphSchema(); + + // Extracted live: the expensive procedures ran, and what is served + // describes the database that answered, not the file. + expect(expensiveRun().length).toBeGreaterThan(0); + expect(schema.unavailable).toEqual([]); + expect(schema.fetchedAt).not.toBe(snapshot.fetchedAt); + + const said = warnings().join("\n"); + expect(said).toContain("schema.json"); + expect(said).toMatch(c.reason); + }); + + it("falls back, with a warning, when the file is not there", async () => { + process.env.NEO4J_SCHEMA_FILE = join(dir, "missing.json"); + const schema = await fetchGraphSchema(); + expect(expensiveRun().length).toBeGreaterThan(0); + expect(schema.unavailable).toEqual([]); + expect(warnings().join("\n")).toContain("missing.json"); + }); + + it("never serves the file when the database cannot be fingerprinted", async () => { + // Unverifiable is not a match. With the database down, the live + // extraction fails too, and that is what the caller hears. + writeSnapshot(snapshot); + fake.respond = () => { + throw new Error("Could not perform discovery. No routing servers available."); + }; + await expect(fetchGraphSchema()).rejects.toThrow(/schema/i); + expect(warnings().join("\n")).toContain("schema.json"); + }); +}); diff --git a/tests/graph-schema.test.ts b/tests/graph-schema.test.ts index b5a8669..65f2e4c 100644 --- a/tests/graph-schema.test.ts +++ b/tests/graph-schema.test.ts @@ -19,102 +19,16 @@ vi.mock("neo4j-driver", async () => { }); import { queriesRun } from "./helpers/fake-neo4j.js"; +import { community44, NO_PROCEDURE } from "./helpers/community44.js"; import { fetchGraphSchema, _resetGraphSchemaCache, SCHEMA_RETRY_AFTER_MS, } from "../src/graph/schema.js"; +import { SCHEMA_FORMAT } from "../src/graph/schema-source.js"; import { formatGraphSchemaMarkdown } from "../src/graph/format-schema.js"; import { closeDriver } from "../src/clients/neo4j.js"; -const NO_PROCEDURE = (name: string) => - new Error( - `There is no procedure with the name \`${name}\` registered for this database instance.` - ); - -/** - * Answers the way Neo4j 4.4 Community without APOC does. `overrides` replaces - * the answer for any query containing its key. - */ -function community44( - overrides: Record) => unknown[]> = {} -) { - return (query: string, params: Record): unknown[] => { - for (const [needle, answer] of Object.entries(overrides)) { - if (query.includes(needle)) return answer(params); - } - const apoc = /\b(apoc\.[\w.]+)/.exec(query); - if (apoc?.[1]) throw NO_PROCEDURE(apoc[1]); - - if (query.includes("dbms.components")) - return [{ name: "Neo4j Kernel", versions: ["4.4.40"], edition: "community" }]; - if (query.includes("db.labels")) return [{ label: "Pathway" }, { label: "Reaction" }]; - if (query.includes("db.relationshipTypes")) - return [{ relationshipType: "hasEvent" }, { relationshipType: "input" }]; - if (query.includes("db.schema.nodeTypeProperties")) - return [ - { - nodeType: ":`Pathway`", - nodeLabels: ["Pathway"], - propertyName: "stId", - propertyTypes: ["String"], - mandatory: true, - }, - ]; - if (query.includes("db.schema.relTypeProperties")) - return [ - { - relType: ":`input`", - propertyName: "stoichiometry", - propertyTypes: ["Long"], - mandatory: true, - }, - ]; - if (query.includes("db.schema.visualization")) - return [ - { - nodes: [ - { identity: -1, labels: ["Pathway"], properties: { name: "Pathway" } }, - { identity: -2, labels: ["Reaction"], properties: { name: "Reaction" } }, - ], - relationships: [{ identity: -3, start: -1, end: -2, type: "hasEvent", properties: {} }], - }, - ]; - if (query.includes("SHOW INDEXES")) - return [ - { - name: "index_pathway_stId", - type: "BTREE", - entityType: "NODE", - labelsOrTypes: ["Pathway"], - properties: ["stId"], - state: "ONLINE", - }, - ]; - if (query.includes("SHOW CONSTRAINTS")) - return [ - { - name: "constraint_dbId", - type: "UNIQUENESS", - entityType: "NODE", - labelsOrTypes: ["DatabaseObject"], - properties: ["dbId"], - }, - ]; - // Per-token counts: one UNION ALL query each for labels and for - // relationship types, keyed by parameter so the names need no escaping. - if (query.includes("UNION ALL") || /RETURN \$\w+ AS key/.test(query)) { - return Object.entries(params).map(([, key]) => ({ - key, - count: key === "Pathway" ? 2_000 : key === "hasEvent" ? 9_000 : 500, - })); - } - if (/MATCH \(n\) RETURN count\(n\)/.test(query)) return [{ count: 3_000_000 }]; - if (/MATCH \(\)-\[r\]->\(\) RETURN count\(r\)/.test(query)) return [{ count: 12_000_000 }]; - throw new Error(`unscripted query: ${query}`); - }; -} - describe("fetchGraphSchema on Neo4j 4.4 Community without APOC", () => { beforeEach(() => { process.env.NEO4J_URI = "bolt://127.0.0.1:7687"; @@ -150,6 +64,32 @@ describe("fetchGraphSchema on Neo4j 4.4 Community without APOC", () => { expect(schema.constraints[0]?.name).toBe("constraint_dbId"); }); + it("records which database it describes, and the format it is written in", async () => { + // A schema saved to a file is only usable against the database it came + // from; the fingerprint is how a reader tells. + const schema = await fetchGraphSchema(); + expect(schema.schemaFormat).toBe(SCHEMA_FORMAT); + expect(schema.source).toEqual({ + neo4jVersion: "4.4.40", + neo4jEdition: "community", + releaseNumber: 97, + nodeCount: 3_000_000, + relCount: 12_000_000, + generatedAt: expect.any(String), + }); + expect(Number.isNaN(Date.parse(schema.source?.generatedAt ?? ""))).toBe(false); + }); + + it("leaves the release out, rather than guessing, where there is no DBInfo node", async () => { + // A curator database or a non-Reactome graph has none. That is not a + // failure: the rest of the fingerprint still identifies the database. + fake.respond = community44({ DBInfo: () => [] }); + const schema = await fetchGraphSchema(); + expect(schema.unavailable).toEqual([]); + expect(schema.source).not.toHaveProperty("releaseNumber"); + expect(schema.source?.nodeCount).toBe(3_000_000); + }); + it("reads only, and under a timeout", async () => { await fetchGraphSchema(); expect(fake.sessions.length).toBeGreaterThan(0); @@ -294,13 +234,24 @@ describe("fetchGraphSchema on Neo4j 4.4 Community without APOC", () => { ]); expect(a).toBe(b); expect(b).toBe(c); - expect(queriesRun(fake).filter(q => q.includes("dbms.components"))).toHaveLength(1); + // A query one fetch runs exactly once. (dbms.components runs twice per + // fetch: once for the source fingerprint, once for dbComponents.) + expect(queriesRun(fake).filter(q => q.includes("db.schema.visualization"))).toHaveLength(1); }); }); function minimalSchema(): GraphSchema { return { + schemaFormat: SCHEMA_FORMAT, fetchedAt: "2026-10-14T15:00:00Z", + source: { + neo4jVersion: "4.4.40", + neo4jEdition: "community", + releaseNumber: 97, + nodeCount: 100, + relCount: 200, + generatedAt: "2026-10-14T15:00:00Z", + }, dbComponents: [{ name: "Neo4j Kernel", versions: ["4.4.40"], edition: "community" }], stats: { nodeCount: 100, @@ -348,6 +299,7 @@ describe("formatGraphSchemaMarkdown", () => { it("covers totals, labels, rel types, patterns, properties, indexes, constraints", () => { const md = formatGraphSchemaMarkdown(minimalSchema()); expect(md).toContain("Neo4j:** 4.4.40"); + expect(md).toContain("**Reactome release:** 97"); expect(md).toContain("100 nodes"); expect(md).toContain("`Pathway` 50"); expect(md).toContain("`hasEvent` 80"); @@ -362,6 +314,12 @@ describe("formatGraphSchemaMarkdown", () => { expect(md.toLowerCase()).not.toContain("apoc"); }); + it("names no release for a graph that has none", () => { + const { releaseNumber: _omitted, ...source } = minimalSchema().source!; + const md = formatGraphSchemaMarkdown({ ...minimalSchema(), source }); + expect(md).not.toContain("Reactome release"); + }); + it("stays small enough for an LLM context window", () => { expect(formatGraphSchemaMarkdown(minimalSchema()).length).toBeLessThan(10_000); }); diff --git a/tests/helpers/community44.ts b/tests/helpers/community44.ts new file mode 100644 index 0000000..f3ac779 --- /dev/null +++ b/tests/helpers/community44.ts @@ -0,0 +1,94 @@ +/** + * A scripted database that answers the way the Reactome graph database + * container does: Neo4j 4.4 Community, no APOC, Release 97 in its DBInfo + * node. Hand it to a fake driver as `state.respond`. + */ + +export const NO_PROCEDURE = (name: string) => + new Error( + `There is no procedure with the name \`${name}\` registered for this database instance.` + ); + +/** + * Answers the way Neo4j 4.4 Community without APOC does. `overrides` replaces + * the answer for any query containing its key. + */ +export function community44( + overrides: Record) => unknown[]> = {} +) { + return (query: string, params: Record): unknown[] => { + for (const [needle, answer] of Object.entries(overrides)) { + if (query.includes(needle)) return answer(params); + } + const apoc = /\b(apoc\.[\w.]+)/.exec(query); + if (apoc?.[1]) throw NO_PROCEDURE(apoc[1]); + + if (query.includes("dbms.components")) + return [{ name: "Neo4j Kernel", versions: ["4.4.40"], edition: "community" }]; + if (query.includes("DBInfo")) return [{ releaseNumber: 97 }]; + if (query.includes("db.labels")) return [{ label: "Pathway" }, { label: "Reaction" }]; + if (query.includes("db.relationshipTypes")) + return [{ relationshipType: "hasEvent" }, { relationshipType: "input" }]; + if (query.includes("db.schema.nodeTypeProperties")) + return [ + { + nodeType: ":`Pathway`", + nodeLabels: ["Pathway"], + propertyName: "stId", + propertyTypes: ["String"], + mandatory: true, + }, + ]; + if (query.includes("db.schema.relTypeProperties")) + return [ + { + relType: ":`input`", + propertyName: "stoichiometry", + propertyTypes: ["Long"], + mandatory: true, + }, + ]; + if (query.includes("db.schema.visualization")) + return [ + { + nodes: [ + { identity: -1, labels: ["Pathway"], properties: { name: "Pathway" } }, + { identity: -2, labels: ["Reaction"], properties: { name: "Reaction" } }, + ], + relationships: [{ identity: -3, start: -1, end: -2, type: "hasEvent", properties: {} }], + }, + ]; + if (query.includes("SHOW INDEXES")) + return [ + { + name: "index_pathway_stId", + type: "BTREE", + entityType: "NODE", + labelsOrTypes: ["Pathway"], + properties: ["stId"], + state: "ONLINE", + }, + ]; + if (query.includes("SHOW CONSTRAINTS")) + return [ + { + name: "constraint_dbId", + type: "UNIQUENESS", + entityType: "NODE", + labelsOrTypes: ["DatabaseObject"], + properties: ["dbId"], + }, + ]; + // Per-token counts: one UNION ALL query each for labels and for + // relationship types, keyed by parameter so the names need no escaping. + if (query.includes("UNION ALL") || /RETURN \$\w+ AS key/.test(query)) { + return Object.entries(params).map(([, key]) => ({ + key, + count: key === "Pathway" ? 2_000 : key === "hasEvent" ? 9_000 : 500, + })); + } + if (/MATCH \(n\) RETURN count\(n\)/.test(query)) return [{ count: 3_000_000 }]; + if (/MATCH \(\)-\[r\]->\(\) RETURN count\(r\)/.test(query)) return [{ count: 12_000_000 }]; + throw new Error(`unscripted query: ${query}`); + }; +} diff --git a/tests/helpers/fake-neo4j.ts b/tests/helpers/fake-neo4j.ts index 6210517..e10e26d 100644 --- a/tests/helpers/fake-neo4j.ts +++ b/tests/helpers/fake-neo4j.ts @@ -30,7 +30,7 @@ export interface FakeSession { export type FakeRows = Iterable; export interface FakeNeo4jState { - drivers: Array<{ uri: string; config: Record }>; + drivers: Array<{ uri: string; config: Record; auth?: unknown }>; sessions: FakeSession[]; /** Rows for a query; throw to simulate a database error. */ respond: (query: string, params: Record) => FakeRows | Promise; @@ -105,8 +105,8 @@ function fakeResult(state: FakeNeo4jState, run: FakeRun) { export function fakeNeo4jModule(state: FakeNeo4jState) { state.moduleLoads = (state.moduleLoads ?? 0) + 1; const neo4j = { - driver(uri: string, _auth: unknown, config: Record = {}) { - state.drivers.push({ uri, config }); + driver(uri: string, auth: unknown, config: Record = {}) { + state.drivers.push({ uri, config, auth }); return { session(sessionConfig: Record = {}) { const session: FakeSession = { config: sessionConfig, runs: [], closed: false }; @@ -135,7 +135,13 @@ export function fakeNeo4jModule(state: FakeNeo4jState) { async close() {}, }; }, - auth: { basic: (user: string, _password: string) => ({ scheme: "basic", principal: user }) }, + auth: { + basic: (user: string, password: string) => ({ + scheme: "basic", + principal: user, + credentials: password, + }), + }, session: { READ: "READ", WRITE: "WRITE" }, int: (value: number | string) => ({ __int: Number(value) }), }; From 653b304263acd0646b33b3c0689e605b0f212c18 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Wed, 14 Oct 2026 14:33:46 +0000 Subject: [PATCH 2/6] Document extract-schema, NEO4J_SCHEMA_FILE and curator use README: saving the schema, the fingerprint guard, producing reactome--schema.json from the release pipeline, and pointing an internal instance at your own database. CHANGELOG entries. Constitution 2.0.1 (PATCH): records why the extract-schema command connects without MCP_ALLOW_CYPHER and is not an instance with graph access. Co-Authored-By: Claude Opus 5.5 --- .specify/memory/constitution.md | 26 +++++++++- CHANGELOG.md | 3 ++ README.md | 87 +++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 1 deletion(-) diff --git a/.specify/memory/constitution.md b/.specify/memory/constitution.md index 88fc398..f116c77 100644 --- a/.specify/memory/constitution.md +++ b/.specify/memory/constitution.md @@ -1,6 +1,21 @@