Route the MCP server through nginx, and document it - #281
Merged
Merged
Conversation
Two halves of making reactome-mcp reachable. **The route.** `/mcp/` in the shared routes file, so it exists in every environment that includes it and needs no config edit when the server is turned on or off: nothing listening means 502 and nothing else is affected. Streamable HTTP needs the same three settings the chat needs -- buffering off, upgrade headers, a long read timeout -- because a proxy that buffers turns a stream of events into a long silence followed by everything at once. `mcp-session-id` is passed back explicitly: the server issues it on initialise and the client returns it on every later call, so a session id the client never receives means every call starts a new session and the per-session server instances accumulate until the process dies. It gets a rate limit of its own at 2r/s rather than the site's 100r/s, which is sized for page assets. The server's own configuration says the HTTP transport must sit behind something that rate-limits, and the reason is per-request cost rather than volume: `reactome_analyze_identifiers` submits a real job to the Analysis Service. The upstream is loopback, because the server binds 127.0.0.1 and turns on DNS-rebinding protection there, and that protection is what stops a page in a visitor's browser driving it. Checked with `nginx -t` in the same image compose runs: dev, production and release all pass. production and release needed stand-in certificates to get past the TLS stanza on this host, which is why they had not been checked. **The page.** content/tools/reactome-mcp.mdx and a card on the tools index, written against the server's actual source rather than its README: sixty-two tools in six groups, and the install instructions say clone-and-build because the package is **not published to npm** -- `npx reactome-mcp` 404s today, so documenting it would have sent every reader into a dead end. Two things deliberately not written down. The hosted endpoint is not described as available, because until the server is switched on that would document a 502. And `NEO4J_URI` stays unset wherever this is public: the Cypher tools register only when it is set, and their own guard says it is "a guardrail, not a security boundary" written for a curator-facing case. Leaving it unset removes those tools entirely, which is the enforcement. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three corrections to the route, two of them mine and found by running it. **It would have 404'd on every call.** `location /mcp/` with `proxy_pass http://mcp/;` strips the prefix, and the server mounts POST, GET and DELETE on `/mcp` rather than at its root -- so every request arrived where nothing is listening, and the transport reports that as a connection failure rather than as a 404. The prefix form with no trailing slash on either side forwards the URI verbatim, and means a client connecting to `/mcp` and one connecting to `/mcp/` both arrive. Verified by running the server and driving a real `initialize` call through nginx: 200, with a session id. **`proxy_pass_header mcp-session-id` did nothing.** The comment beside it said it was what stopped every call starting a new session. nginx hides only a short fixed list on the way back and this is not on it; removing the directive and repeating the call returned the session id just the same. A line that asserts a danger it does not prevent is worse than no line, so it is gone and the comment says what was measured. **`/health` is not reachable through it**, which is the other reason not to proxy the upstream's root: it reports that the process is up, and that is nobody else's business. Confirmed 404. **Compose.** The server had no definition anywhere -- it was a `docker run` somebody typed once, bind-mounting a git checkout at /srv. That works until the box reboots or the checkout moves, and nothing records what it was run with. Bound to 127.0.0.1, unlike the container it replaces, which set MCP_HTTP_HOST=0.0.0.0 safely because it sat on a private compose network with nothing published. Under `network_mode: host` that is a different statement entirely, so the binding is pinned here rather than inherited. `NEO4J_URI` and `MCP_ALLOW_CYPHER` are both absent, and it now takes both to register the Cypher tools. They used to arrive as a side effect of setting a connection string -- which the startup schema warm-up also wants -- so an instance could acquire arbitrary graph queries without anyone deciding to. The page says 59 tools rather than the 62 name literals in the source: the difference is exactly the three Cypher ones, which do not register here. nginx -t passes for dev, production and release. An earlier run of this check reported dev failing, which was the stand-in certificates having the wrong filenames rather than the config -- the harness, not the thing measured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The compose service I added bind-mounted a working clone and ran the `dist/` inside it. `dist/` is gitignored, so what that deploys is "whatever branch was checked out, compiled whenever anyone last ran a build" -- and the failure is silent, because the image, the compose file and the route all look exactly as intended. Not hypothetical: a container that had been up for four days was serving code that no longer existed on disk, because a test run had rebuilt `dist/` from an unmerged feature branch as a side effect. Bringing my version up would have promoted unreviewed working-tree code into the deployed server with nothing looking wrong. Found by the chatbot session reviewing what I wrote, which is the half of a review I cannot do for myself. So the revision is an argument. `MCP_REF` is a branch or tag, the build clones exactly that, and nobody's checkout is part of the deployment. The commit is written to /srv/REVISION during the build, because "which revision is this serving" otherwise has no answer once the build host is gone. The Dockerfile is here rather than in reactome-mcp because that repository has none -- nothing tracked, checked rather than assumed. Verified by building and running it: /srv/REVISION is f2dd566, which is that repository's main to the character; /health reports neo4jEnabled false and cypherEnabled false; it runs as `node` rather than root; and the running server advertises exactly 59 tools with no cypher among them, which is the number the page claims and was until now taken on trust. Not brought up. Defining the service and starting it are different decisions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The rate limit above bounds calls per second. It says nothing about how much work one call asks for, and `reactome_analyze_identifiers` is where those two quantities come apart: it POSTs an identifier list to the Analysis Service, which does real work and stores a result, so a single permitted request could commission an arbitrarily large job. My comment named per-request cost as the reason for the tight rate and then bounded frequency, which reads as though the cost per call were bounded too. Raised by the chatbot session. `client_max_body_size` is that bound in the only unit this layer can see. Measuring it rather than reading it turned up something worth having. The server rejects a body over 100 KiB -- 102,400 bytes exactly, confirmed by bisecting against the built image -- while its own code asks for 4 MB. That line never runs: the SDK's `createMcpExpressApp` mounts `express.json()` with no limit before it, so express's 100 KB default parses the body and the 4 MB parser behind it never sees one. Two body parsers, and the first one wins. So 256 KB does not bind today, and is deliberately not derived from the 10,000-identifier cap the server is adding -- that list is about 157 KB, which this transport already refuses. It is the outer bound for the day the parser ordering is fixed and 4 MB would otherwise become the real ceiling, and it is the cheap rejection: an 8.8 MB body is refused at the edge rather than read into the service. Both confirmed 413 through this configuration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The comment justified 256 KB partly as the outer bound for the day the service's 4 MB parser started working. It will not: the line asking for 4 MB has been deleted rather than fixed, because controlling that limit would mean not using the SDK's express app and copying its DNS-rebinding protection into that repository, which is a worse trade than accepting a ceiling. So the reason is rewritten to the part that survives. The cap still refuses an 8.8 MB body at the edge rather than letting it be read into the service, and it still stops an SDK upgrade raising a ceiling that nothing in either repository declares -- 100 KiB is express's default reached through `createMcpExpressApp`, chosen by nobody. A bound does not have to be the binding one to be worth having, but it does have to say why it is there. The 102,400-byte boundary now has two independent measurements from opposite directions: mine by bisecting bodies against the built image, theirs by bisecting identifier counts against a running server. Their cap is 3,000 identifiers, about 69 KB, which fits under it with room. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`RUN git clone --branch main` is one fixed command string, so Docker caches that layer and reuses it for ever. Once main moves, `docker compose build` rebuilds the revision from the first build and reports success -- which is the bind-mount defect this Dockerfile replaced, one level up: a deployment whose revision is decided by something nobody looked at. Verified rather than reasoned: a second `docker compose build mcp` with nothing changed reported CACHED for every step, the clone included. `ADD` from the commits API re-fetches each build and its layer digest follows the content, so a moved ref invalidates the clone and everything after it while an unmoved one stays cached. Both halves confirmed -- the first build after adding it re-ran the ADD and the clone, the next was cached throughout. The claim in the previous message needed narrowing too. `main` is a moving ref, so two builds of `main` are two revisions, deliberately. What is guaranteed is that a build follows the ref rather than a stale copy, and that the commit used is recorded in /srv/REVISION where it can be read back. The repository has no tags, so there is nothing more fixed to default to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two callers that cannot share one route in. nginx runs in host networking and reaches the published port on 127.0.0.1. The chatbot is a container on its own private network and addresses the server by name -- `REACTOME_MCP_URL=http://reactome_mcp:4320` -- so this joins that network and takes `reactome_mcp` as an alias. The alias is the hand-run container's name on purpose. The alternative was reconfiguring the chatbot to reach a host gateway, which is a change to somebody else's deployment to solve a problem on this side, and one more thing that has to be remembered when either end moves. With the alias the swap is invisible to the thing that depends on it. The network is declared external because the chatbot's stack created it: compose joins it rather than trying to own it, and will not remove it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two halves of making
reactome-mcpreachable, per the decision that it should be available through nginx when it is on, with a UI in this repo.The route
/mcp/goes in the shared routes file, so it exists in every environment that includes it and needs no config edit when the server is turned on or off — nothing listening means a 502 on that path and nothing else is affected.Streamable HTTP needs the same three settings the chat needs: buffering off, upgrade headers, a long read timeout. A proxy that buffers turns a stream of events into a long silence followed by everything at once.
mcp-session-idis passed back explicitly. The server issues it on initialise and the client returns it on every later call, so a session id the client never receives means every call starts a new session and the per-session server instances accumulate until the process dies.Rate limiting
2r/s with a burst of 10, in its own zone, rather than the site's 100r/s — which is sized for page assets. This is not caution for its own sake: the server's own config says the HTTP transport must sit behind something that rate-limits, and the reason is per-request cost rather than volume.
reactome_analyze_identifierssubmits a real job to the Analysis Service.The upstream is loopback. The server binds
127.0.0.1and enables DNS-rebinding protection there, which is what stops a page in a visitor's browser driving it; nginx should be the only thing in front.Verified
nginx -tin the samenginx:alpineimage compose uses — dev, production and release all pass. production and release needed stand-in certificates to get past the TLS stanza on this host, which is presumably why they had not been syntax-checked before.local.confusesroutes-core.confand is untouched.The page
content/tools/reactome-mcp.mdxplus a card on the tools index, written against the server's source rather than its README: 62 tools in six groups.The install instructions say clone-and-build, because the package is not published to npm —
npm view reactome-mcp404s, so documentingnpx reactome-mcpwould have sent every reader into a dead end. Publishing it is the single biggest lever on making this usable, and is not something this PR can do.Two things deliberately left out
NEO4J_URIstays unset wherever this is public. The Cypher tools register only when it is set, and their own guard says it is "a guardrail, not a security boundary", written for a curator-facing case. Leaving it unset removes those tools entirely — that is the enforcement, not a policy someone has to remember.🤖 Generated with Claude Code