refactor: bind tool and resource invocations to the server that received them

[Valiunia]

What changed

BaseTool and BaseResource each stored the MCP server they were installed into in a single mutable instance field (this.server, assigned by installTo()), and the callbacks they registered with the SDK read that field at call time rather than capturing the server they were registered on. Both now resolve the server per invocation through an AsyncLocalStorage scope entered by the registered callback, exposed as a new activeServer accessor.

Readers updated to use it:

• BaseTool.log(), and the three tools that communicate with the client mid-request — search_and_geocode_tool and directions_tool (elicitations), ground_location_tool (sampling).
• BaseResource.log(). Both of its registration paths, plain URI and ResourceTemplate, enter the scope.

BasePrompt needs no change — it holds no server reference and is stateless. registerOptimizationV2Task() already takes its server as a parameter and closes over it correctly.

Why

The pre-configured instances exported from @mapbox/mcp-server/tools and @mapbox/mcp-server/resources are module-level singletons: getCoreTools() and getAllResources() return the same objects on every call. An application that installs one of those instances into more than one McpServer in a single process — for example one server per connected session — hits last-install-wins behavior. The second installTo() overwrites the shared field, and a call arriving through the callback registered on the first server dereferences that field and talks to the second.

For resources the effect is misdirected log messages. For tools it is more visible: an elicitation prompt is delivered to a different client than the one that made the request, and that client's answer determines the result the original caller receives.

Nothing in this repository does that today. The server shipped by the package creates one McpServer per process against a single stdio client (src/index.ts:97,265), and the documented import example installs into a single server, so there is no behavior change for any usage we ship or document. But both subpaths are published exports and a server-per-session process is a reasonable way to build on them, so the shared field is a footgun worth removing. The failure mode is silent — nothing throws, the call simply resolves against the wrong client — which is exactly the kind of thing that survives review if it ever does get introduced.

Approach

this.server is deliberately retained as a fallback for run() / read() invoked directly rather than through a registered callback. That keeps the existing direct-invocation API and its tests working untouched (the suite calls .run() directly in ~490 places, and several tests inject a mock by assigning the field). The tradeoff is that direct callers sharing one instance across servers still observe the last-installed server; the code comments say so. Passing the server explicitly through run() would cover that case too, but it changes the signature of all 20 implementations and every mock-injection site, for a path that has no MCP callback to bind to in the first place.

The ~12-line binding pattern is duplicated across the two base classes rather than extracted into a shared helper. They are otherwise independent hierarchies with different registration shapes and different handler signatures, and a shared mixin seemed like more indirection than the duplication costs. Happy to extract it if reviewers disagree.

How to verify

npm test — 920 tests pass. Seven new cases across test/tools/BaseTool.test.ts and test/resources/BaseResource.test.ts:

• a call through server A's callback observes A after B has been installed
• concurrent invocations on A and B each observe their own server, checked both at entry and after an await, so a binding that only holds until the first suspension point fails
• direct run() / read() falls back to the installed server
• an uninstalled tool reports no server

To confirm these are genuine regression tests, revert either callback wrapper to its previous form and re-run — the first two in each file fail with expected 'b' to be 'a'. I checked this for both classes.

Also verified: tsc --noEmit clean, eslint clean on changed files, tshy ESM/CJS build succeeds.

Notes for review

• AsyncLocalStorage is Node core, so no new dependency, and it adds a per-invocation context cost that is negligible next to the outbound HTTP call these tools make. @opentelemetry/context-async-hooks, already in the dependency tree, uses the same mechanism to propagate spans.
• If the async context is ever lost across an await chain, the lookup returns undefined and the fallback yields today's behavior, so the failure mode is no worse than the current state.
• The new tests use as any for the fake-server scaffolding, consistent with the mock style already in BaseTool.test.ts.

🤖 Generated with Claude Code

[mattpodwysocki]

Nice fix. This is a real footgun in the published /tools and /resources exports and AsyncLocalStorage is the right tool for it.

Went through this locally: tsc clean, eslint clean on the changed files, full suite is 919/920 (the one failure is the pre-existing urlSafety.test.ts issue on main, unrelated to this). I also reverted the BaseTool.ts wrapper back to the old form just to make sure the new regression tests actually catch the bug and not just pass trivially, they do, fails with "expected 'b' to be 'a'" like the PR description says.

Two things worth a comment, neither should block merging:

1. ResourceReaderTool.execute() calls resource.read(uri, extra) directly on whatever it pulls from the registry, which skips the installTo() wrapper entirely. That's the direct invocation fallback path you called out, and it's actually live in this repo today, not just a hypothetical downstream case. No resource currently calls this.log() from read() so it's harmless right now, but if one ever does and that resource is shared across servers, resource_reader_tool would hit the same misdirection this PR just fixed for the normal path.

2. The new BaseResource tests only cover the plain URI registration branch. ComputeResource, InlinePayloadResource, and TemporaryDataResource all go through the ResourceTemplate branch instead, same wrapping code but not directly exercised by the new tests. Low risk since that branch is pretty mechanical, just flagging it.

Good to merge as is.