Skip to content

fix: give every STATIC-mode client its own server - #27

Merged
imbenrabi merged 2 commits into
mainfrom
claude/project-thread-v4fe3u
Oct 10, 2026
Merged

imbenrabi merged 2 commits into
mainfrom
claude/project-thread-v4fe3u

Conversation

@imbenrabi

@imbenrabi imbenrabi commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Requested by Ben · project thread

Before: in STATIC mode, only one client could be connected at a time. When a second client sent initialize, the transport closed the shared server's current connection, so the first client's next request failed with Session not found or expired. Per-session context (?config=) also never reached module loaders in STATIC mode, because tools were loaded once with the base context. This hits the FMP MCP server's default mode, ALL_TOOLS, which runs on toolception STATIC.

After: every client gets its own McpServer and orchestrator in both modes. In STATIC mode, each client orchestrator preloads the startup toolsets using that client's merged session context. Concurrent clients stay connected, and session context works in STATIC mode.

The MCP SDK binds one McpServer to one transport. That's why the old disconnectServer() step had to close another client's transport before connecting a new one.

How:

  • createBundleFactory always builds a fresh server and orchestrator per bundle. The base server still validates the startup toolsets at boot (fail-fast) and backs /tools.
  • FastifyTransport awaits bundle.orchestrator.isReady() before connect(). STATIC bundles load tools asynchronously, and the SDK refuses to register tool capabilities after connecting.
  • I removed the "sessionContext has limited effect in STATIC mode" warning, because session context now works in that mode.
  • Updated the intent nodes (root, src/server, src/http).

Trade-off: STATIC module loaders now run once per client instead of once per process. ClientResourceCache still caps the number of bundles (1000, 1h TTL).

Release note: STATIC consumers that register things on the server after createMcpServer returns, rather than inside createServer, won't see them on clients any more, so this belongs in a minor release (0.7.0). The version bump and changelog are in #28.

Tests:

  • New e2e test: two STATIC clients connected at once can both call tools. It fails on main with Session not found or expired.
  • New e2e test: two STATIC clients with different ?config= tokens each see their own context. It fails on main.
  • Unit tests now assert a fresh server per STATIC bundle and merged context reaching the STATIC loaders. The old "reuses a single instance" assertion is gone.
  • npm run typecheck, npm run build and npm run test:run all pass (331 tests when merged with Register all tools with registerTool #26).
  • I also packed this branch, installed it into the FMP MCP server and started it in ALL_TOOLS mode. Two clients stayed connected, and the client that passed ?config= had its own token used.

Pre-publish checks for 0.7.0 (main + #26 + this PR with the version set to 0.7.0, on Node 25.3.0 and npm 11.6.2 as in release.yml)

  • release.yml's steps pass: npm ci, the version and private check, typecheck, 331/331 tests and build.
  • npm publish --dry-run --access public, which also runs prepublishOnly, exits 0 with + toolception@0.7.0. The tarball is 99.2 kB with 61 files.
  • The tarball has the same 61 files as the published 0.6.3. Runtime exports (5) and type exports (25) are identical to 0.6.3; only .d.ts doc comments changed.
  • A strict TypeScript consumer with skipLibCheck: false compiles with both nodenext and bundler resolution, and it starts and closes on Node 25.
  • publint reports no errors or warnings. npm audit --omit=dev finds 0 vulnerabilities.
  • The FMP MCP server installed from the tarball passes 26/26 checks (Move the server to toolception 0.7.0 imbenrabi/Financial-Modeling-Prep-MCP-Server#109's verify:toolception).
  • Before the server bumps to 0.7.0, fix: register prompts on every per-client MCP server imbenrabi/Financial-Modeling-Prep-MCP-Server#108 must merge first, because it registers prompts inside createServer.

Not covered: PermissionAwareFastifyTransport doesn't await readiness before connect() either. Permission-based servers already build a server per client, so the race is the same one this PR fixes. I left it for a follow-up to keep this change small.

There's no file overlap with #25, #26 or #28 apart from nearby test files.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Pqcus1BTWLVAJaPavwdkGX

STATIC mode shared one McpServer across all clients. The MCP SDK binds a
server to one transport, so each new client's initialize closed the previous
client's session ("Session not found or expired"), and module loaders only
ever saw the base context, so sessionContext had no effect.

The bundle factory now builds a fresh server + orchestrator per client in
both modes; STATIC client orchestrators preload the startup toolsets with
the client's merged context. The transport awaits orchestrator readiness
before connect() since the SDK refuses tool capability registration after
connecting. The base server still validates startup toolsets (fail-fast) and
backs the /tools status endpoint.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pqcus1BTWLVAJaPavwdkGX
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pqcus1BTWLVAJaPavwdkGX

Copy link
Copy Markdown
Contributor Author

CI failed in build-test at the Security audit step (npm audit --audit-level=critical), not in this PR's code. The criticals are proxy-addr and tinypool from main's lockfile, and the same step fails on main today. #25 fixes them (its CI is green), so I merged #25's branch into this one (df9cb41). Once #25 lands, that merge is a no-op.

Checked locally after the merge: npm audit --audit-level=critical passes, typecheck and build pass, and npm run test:run passes 329/329.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Go/no-go from the MCP SDK & spec review: GO

Reviewed against @modelcontextprotocol/sdk 1.32.1 (protocol 2025-11-25).

Correct by SDK semantics

  • One McpServer per transport is required by the SDK: Protocol.connect() holds a single _transport and throws Already connected otherwise. A shared STATIC server can't serve two Streamable HTTP sessions, so a server per bundle is the right fix rather than a workaround.
  • Awaiting orchestrator.isReady() before connect() is needed. Server.registerCapabilities() throws once a transport is attached, and the first registerTool call registers the tools capability. A STATIC bundle with meta-tools off and async loaders would otherwise race.
  • disconnectServer() now only affects the same client's stale connection, which matches the SDK's one-session-per-server model.

Compatibility with #26: I merged this branch onto #26 (registerTool migration) locally. It auto-merges cleanly (only tests/createMcpServer.test.ts overlaps), and typecheck plus 331/331 tests pass. Either merge order works.

Non-blocking follow-ups

  1. isReady() returns false when per-client preload fails, but the result is ignored, so the client connects with whatever tools did load. Because STATIC loaders now run with each client's own context, a context-dependent loader failure would go unnoticed per client. Consider returning a JSON-RPC error when it's false.
  2. STATIC module loaders now run once per client. For the FMP server's ALL_TOOLS mode that means about 250 Zod-backed tool registrations per bundle, held for up to 1000 bundles with a 1h TTL. That's fine for correctness, but worth watching for memory.
  3. PermissionAwareFastifyTransport has the same readiness race, as the PR notes.

Generated by Claude Code

@imbenrabi
imbenrabi marked this pull request as ready for review October 10, 2026 09:23
@imbenrabi imbenrabi mentioned this pull request Oct 10, 2026
imbenrabi pushed a commit that referenced this pull request Oct 10, 2026
The next release carries #25, #26 and #27. #27 changes STATIC mode to a
server per client, so it ships as a minor version: consumers pinned to
^0.6.x keep 0.6.3 until they opt in.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pqcus1BTWLVAJaPavwdkGX
@imbenrabi
imbenrabi force-pushed the claude/project-thread-v4fe3u branch from c02385d to df9cb41 Compare October 10, 2026 09:36
@imbenrabi
imbenrabi merged commit 79f9382 into main Oct 10, 2026
2 checks passed
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.

2 participants