Skip to content

Fix cross-user MCP replies, and recover live lookups (review, area 3) - #313

Merged
adamjohnwright merged 1 commit into
mainfrom
review-3-mcp
Oct 4, 2026
Merged

adamjohnwright merged 1 commit into
mainfrom
review-3-mcp

Conversation

@adamjohnwright

Copy link
Copy Markdown
Contributor

These come from the max-level review of the agent and retrieval code. Each was reproduced by the reviewer, and the first by four reviewers independently against the real reactome-mcp.

Finding Fix Test
Cross-user replies. Every request used JSON-RPC id 2 on the shared session. Overlapping lookups from different users got each other's results, including another reader's identifiers and analysis token, and the other call hung. A unique, increasing id per request. A reply to any other id is refused. Unit tests: distinct ids under concurrency; a mismatched reply is refused. Against the real MCP: 20 overlapping calls, 0 crossed. The old client, in the same test, hung until timeout. Sabotaged.
One MCP restart, or a failed first connect, turned live lookups off until the chatbot restarted A lost session (400 "No valid session", or 404) is re-initialized once, under a lock. A failed start is retried after 60s. Session-renewal test, sabotaged. Retry-after-failure test.
With the MCP down, the search page streamed the query expander's alternate questions as the answer The stream opens at preprocess only if live tools are actually available Fallback stream test, sabotaged
The live analysis tool's raw output, token included, went to OpenAI without_token() Unit test
A failed lookup sent the MCP's internal URL to OpenAI The model gets the exception type only Updated test
Tool calls per round were unbounded: 150 calls became 300 upstream requests 6 per round; the rest are answered unrun Unit test

I didn't restart the MCP server to test session renewal live: it's the website's container, and the website shares it. The unit test uses the exact 400 response shape the reviewer recorded from it.

./checks.sh passes.

🤖 Generated with Claude Code

- Every MCP HTTP request was sent with JSON-RPC id 2 on the one session
  the process shares; the server matches replies by id, so two users'
  overlapping lookups got each other's results -- one carrying another
  reader's identifiers and analysis token -- and the other hung. Each
  request now has its own id, and a reply to any other id is refused.
  Against the real reactome-mcp: 20 overlapping calls, 0 crossed (the old
  client hung until timeout on the same test).
- A lost session (reactome-mcp answers 400 'No valid session' after a
  restart) is re-initialized once and the call retried; initialization is
  locked. A failed MCP start is retried after 60s rather than remembered
  for the life of the process.
- Without live tools, a live-routed question falls back to retrieval at
  the same node; the search-page stream no longer opens at preprocess
  then, so the query expander's alternate questions are not streamed as
  the answer.
- The live analysis tool's output reaches the model without the analysis
  token or token-bearing links; a failed lookup tells the model its type,
  not a message carrying the MCP's internal URL.
- At most 6 tool calls run per round; the rest are answered unrun.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@adamjohnwright
adamjohnwright merged commit f5d87ae into main Oct 4, 2026
10 checks passed
@adamjohnwright
adamjohnwright deleted the review-3-mcp branch October 4, 2026 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant