feat(mcp-server): dispatch mounted tool calls to the agent in-process#1743
Conversation
MCP tools call back into the agent's data layer over HTTP. By default the URL is the environment's public api_endpoint, so tool calls leave over the public internet even when the MCP server is mounted inside the agent. Add an opt-in agentUrl option (mountAiMcpServer, ForestMCPServerOptions, FOREST_AGENT_URL for the CLI) that overrides only the internal tool->agent channel — the advertised OAuth discovery URLs stay public so external MCP clients still authenticate. Requested by a self-hosted customer (Swaive) who wants MCP callbacks to stay on their private network. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review follow-ups on the agentUrl option:
- normalizeAgentUrl now parses once, rejects non-http(s) schemes and URLs with
a query string or fragment (which would swallow the request path in
agent-client's `${url}${path}` join), and returns the parsed, whitespace-
normalized form instead of the raw string. Previously such inputs passed
validation and silently misrouted every tool call.
- Add an end-to-end test proving a `tools/call` actually targets agentUrl and
not the environment's public api_endpoint.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ctor Extract normalizeAgentUrl to a shared helper and call it from the ForestMCPServer constructor, so a malformed agentUrl / FOREST_AGENT_URL throws at construction — proximate to the misconfiguration — instead of later inside ForestOAuthProvider during run()/start(). Add a verifyAccessToken test for the fallback path (no agentUrl -> environment api_endpoint). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ForestMCPServer is now the sole validator/normalizer of agentUrl; the internal ForestOAuthProvider accepts a pre-normalized value instead of re-running normalizeAgentUrl. Move the validation/normalization unit tests (invalid inputs, trailing slash, http/https, path preservation) into a dedicated normalize-agent-url.test.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop the agentUrl option from agent.mountAiMcpServer() — mounted mode will dispatch tool calls to the agent in-process (follow-up), making a callback URL unnecessary. agentUrl stays available for the standalone MCP server via FOREST_AGENT_URL / ForestMCPServerOptions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ter override Factor HttpRequester's success-deserialize and error-mapping tail into exported buildAgentHttpError/deserializeAgentBody helpers (HTTP path unchanged), and let createRemoteAgentClient accept a pre-built httpRequester. Groundwork for an in-process transport that reuses the exact same AgentHttpError shape (so the approval-gate detection stays identical). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add InProcessDispatcher, which runs an agent-client request through the agent's
own /forest Koa stack in-memory via light-my-request (no socket) and returns a
superagent-shaped {status, body, text}. FrameworkMounter.getInProcessDispatcher
rebuilds the handler on every (re)mount (stable object, mutable target — same
pattern as McpMiddleware); the /forest prefix is fixed since agent-client
hardcodes those paths. Not yet wired to the MCP server (next increment).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When the MCP server is mounted in an agent, tool calls now reach the agent's data layer in-process (via the agent's InProcessDispatcher) instead of over the public api_endpoint, removing the need for agentUrl in mounted mode. agentUrl stays for standalone (FOREST_AGENT_URL). Tools now receive a single ToolContext; buildClient builds an InProcessHttpRequester (reusing agent-client's shared parse helpers so the deserialized result and AgentHttpError shape stay byte-identical to the HTTP path) when a dispatcher is present. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Guards against a regression to the HTTP path: a real OAuth-authed tools/call must reach the agent through the InProcessDispatcher, across every host framework. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 new issue
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
- Bound in-process dispatch with a timeout (default 10s, honors maxTimeAllowed) so a hung agent handler can't hang a tool call forever, matching the HTTP path's superagent timeout. - Thread a logger into InProcessDispatcher; distinguish an expected empty (204) body from a corrupted non-JSON 2xx body, which is now logged instead of silently returning undefined. - Pin the cross-package contract: InProcessDispatcher now `implements InProcessAgentDispatcher` (exported from mcp-server), so drift fails to compile at the definition, not just at one wiring line. - Make ToolContext.collectionNames required and default it once in the server instead of `= []` in all 10 tool factories. - Document on mountAiMcpServer() + log at startup that in-process tool calls bypass any middleware mounted in front of the agent. Tests: approval-gate 403 error-shape parity, maxTimeAllowed forwarding, timeout, 204/non-JSON body handling, payload+content-type forwarding through a real handler, buildClientWithActions dispatcher wiring, and a status assertion on the in-process integration test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| body: responseBody, | ||
| text, | ||
| } = await this.dispatcher.request({ | ||
| method, |
There was a problem hiding this comment.
🟡 Medium src/in-process-http-requester.ts:45
InProcessHttpRequester.query() forwards path directly to the dispatcher without calling HttpRequester.escapeUrlSlug(), so collection names, record ids, or action endpoints containing reserved URI characters (spaces, +, ?) are sent unescaped. This causes the agent's router to fail or hit the wrong route for valid names — for example /forest/Foo Bar or /forest/orders/1/relationships/line?items are sent literally, with ?items truncated as a query string. Consider applying HttpRequester.escapeUrlSlug() to path before dispatching, matching the HTTP path's behavior.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/mcp-server/src/in-process-http-requester.ts around line 45:
`InProcessHttpRequester.query()` forwards `path` directly to the dispatcher without calling `HttpRequester.escapeUrlSlug()`, so collection names, record ids, or action endpoints containing reserved URI characters (spaces, `+`, `?`) are sent unescaped. This causes the agent's router to fail or hit the wrong route for valid names — for example `/forest/Foo Bar` or `/forest/orders/1/relationships/line?items` are sent literally, with `?items` truncated as a query string. Consider applying `HttpRequester.escapeUrlSlug()` to `path` before dispatching, matching the HTTP path's behavior.
There was a problem hiding this comment.
Real but rare, left as a known limitation. escapeUrlSlug does encodeURI(path).replace(/([+?*])/g, '\\$1') — the \+?* backslash convention targets the agent's koa/path-to-regexp router + the HTTP wire, and can't be replayed through light-my-request (its WHATWG new URL parsing treats \ as /), so applying it blindly would break more than it fixes. Spaces/non-ASCII encode identically both ways; the only divergence is slug names containing literal + ? * (e.g. a record id with ?), which are already fragile over HTTP. Deliberately not changing this under this PR to avoid a riskier fix — noting it as a known edge.
…dyParser Mirrors FrameworkMounter.getInProcessDispatcher()'s wrapping and asserts a mutating POST body reaches the route parsed — the driver router's `router.use(bodyParser)` (agent.ts) is included in `.routes()`, so mounted mutating tool calls (create/update/…) get a parsed body, same as HTTP. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…HttpRequester No MCP tool exports CSV / calls stream(), so the override guarded a path that cannot happen. Removed it (and its test) per YAGNI; query() is the only method the in-process requester needs to override. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Comments describe what, not why — violates project CLAUDE.md style
|
|
Single source of truth for the dispatcher contract The structural InProcessAgentDispatcher interface (mcp-server) re-declares the same shape as InProcessRequest/InProcessResponse (agent), with no mechanical sync. The agent already imports InProcessAgentDispatcher from mcp-server, so exporting named InProcessDispatchRequest/Response types from mcp-server and importing them in the agent would eliminate the duplication with zero new dependency-graph edges. Drift is currently caught only by implements on existing fields, not by additions. |
…st middleware is skipped Addresses Shohan's review: reword so "bypass" doesn't read as "unauthenticated". In-process tool calls skip host-app middleware (rate limit/logging/WAF) but the agent's own JWT auth + permission checks still run, and the inbound MCP request is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ating comments Review (Shohan): - Export named InProcessDispatchRequest/InProcessDispatchResponse from mcp-server and import them in the agent's InProcessDispatcher instead of re-declaring the shape — one source of truth, no manual sync (adds no dependency-graph edge, the agent already imports from mcp-server). - Remove comments that restate the code (agent-caller.ts branch note, tool-context.ts agentDispatcher note) and fix the misleading `/stream` mention in the timeout note (stream isn't dispatched in-process) per CLAUDE.md style. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@ShohanRahman both addressed in Comment style (what-not-why). Removed the two comments that restate the code:
Single source of truth for the dispatcher contract. Exactly your suggestion: |
…, not exports buildAgentHttpError and deserializeAgentBody were free functions exported from agent-client only so InProcessHttpRequester (mcp-server) could rebuild the exact same AgentHttpError shape on the in-process transport. Since that requester already extends HttpRequester, fold them into protected methods (buildError / deserialize) and drop the two public exports — the shared error/deserialize contract now travels through the subclass, not agent-client's public surface. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Weak assertion in the dispatcher-present test agent-caller.test.ts:151-157 only uses .not.toThrow(). It should also assert the client was wired for in-process: |
|
The stream()/CSV story is now internally contradictory
|
…atcher test Review (Shohan): - InProcessHttpRequester inherited the base stream(), which would fire a real request at the sentinel host (http://in-process.agent) → ~10s hang. Restore a throwing override so CSV export fails fast in-process (matches the PR description's "throws explicitly") and fix the stale "base url is unused" comment (it is a never-resolved sentinel now that both query() and stream() are overridden). Covered by a new stream()-throws test. - Strengthen the dispatcher-present test: assert createRemoteAgentClient is wired with an InProcessHttpRequester + empty url, instead of only .not.toThrow() (a silently dropped dispatcher would have passed before). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@ShohanRahman les deux points traités dans 1. Assertion faible (dispatcher-present test). Le test expect(mockCreateRemoteAgentClient).toHaveBeenCalledWith(
expect.objectContaining({ httpRequester: expect.any(InProcessHttpRequester), url: '' }),
);Un dispatcher silencieusement perdu échoue désormais le test. 2. stream()/CSV contradictoire. J'ai suivi ta reco (restaurer l'override) — c'est le choix qui rend tout cohérent d'un coup :
|
…+ silent non-JSON 2xx Review (Shohan): - injectWithTimeout raced inject() against a timer with no handler on the injection promise. If the timeout won and inject() later rejected, it surfaced as an unhandledRejection. Attach a catch() that logs the late settlement before racing, so the losing promise is always observed. - parseBody returned undefined for any non-JSON body (warn only). On a 2xx that handed the tool no data and no error. Now throw on a non-JSON 2xx (misbehaving middleware — HTML/redirect), and include a truncated body preview in both the thrown error and the 4xx+ warn log. 4xx+ still returns undefined (buildError falls back to the raw text). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| ); | ||
| }); | ||
|
|
||
| const injection = inject(this.handler as RequestListener, options); |
There was a problem hiding this comment.
🟡 Medium src/mcp-in-process-dispatcher.ts:82
When the agent handler throws before the timeout fires, injectWithTimeout logs it as "settled after the ${timeoutMs}ms timeout" even though the timeout never elapsed. This emits a bogus timeout warning that obscures the real handler failure. The .catch on injection fires unconditionally on rejection, so any normal-path exception is mislabeled as a late settlement. Consider attaching the late-settlement logger only after the timeout actually wins the race.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/agent/src/mcp-in-process-dispatcher.ts around line 82:
When the agent handler throws before the timeout fires, `injectWithTimeout` logs it as `"settled after the ${timeoutMs}ms timeout"` even though the timeout never elapsed. This emits a bogus timeout warning that obscures the real handler failure. The `.catch` on `injection` fires unconditionally on rejection, so any normal-path exception is mislabeled as a late settlement. Consider attaching the late-settlement logger only after the timeout actually wins the race.
| * Its handler is rebuilt on every (re)mount; the `/forest` prefix is fixed (not the external | ||
| * mount prefix) because agent-client hardcodes `/forest/...` paths. |
|
|
||
| import inject, { type InjectOptions, type InjectPayload } from 'light-my-request'; | ||
|
|
||
| // Single-sourced from mcp-server (the dispatcher contract) — no re-declaration to drift. |
There was a problem hiding this comment.
several useless comments in this file
| private environmentApiEndpoint?: string; | ||
| private agentUrl?: string; |
There was a problem hiding this comment.
I think environmentApiEndpoint is already the agentUrl
There was a problem hiding this comment.
agentUrl leaves the ability to set a local URL for the agent when the MCP server is mounted separately but still in local
| /** | ||
| * Standalone MCP server only (set via FOREST_AGENT_URL). Internal URL the tools use to reach | ||
| * the agent's data layer, defaulting to the environment's public `api_endpoint`. | ||
| */ | ||
| agentUrl?: string; |
There was a problem hiding this comment.
can't we just rely on the environment's public api_endpoint
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# @forestadmin/agent-client [1.11.0](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/agent-client@1.10.2...@forestadmin/agent-client@1.11.0) (2026-07-22) ### Features * **mcp-server:** dispatch mounted tool calls to the agent in-process ([#1743](#1743)) ([7c5efaa](7c5efaa))
# @forestadmin/mcp-server [1.18.0](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/mcp-server@1.17.3...@forestadmin/mcp-server@1.18.0) (2026-07-22) ### Features * **mcp-server:** dispatch mounted tool calls to the agent in-process ([#1743](#1743)) ([7c5efaa](7c5efaa)) ### Dependencies * **@forestadmin/agent-client:** upgraded to 1.11.0
# @forestadmin/agent [1.87.0](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/agent@1.86.7...@forestadmin/agent@1.87.0) (2026-07-22) ### Features * **mcp-server:** dispatch mounted tool calls to the agent in-process ([#1743](#1743)) ([7c5efaa](7c5efaa)) ### Dependencies * **@forestadmin/mcp-server:** upgraded to 1.18.0 * **@forestadmin/workflow-executor:** upgraded to 1.20.2
3 participants
What
When the MCP server is mounted in an agent (
agent.mountAiMcpServer()), tool calls now reach the agent's data layer in-process — dispatched into the agent's own Koa stack vialight-my-request(in-memory, no socket) — instead of calling back over the publicapi_endpoint.This removes the need for
agentUrlin mounted mode (the feedback that motivated #1740).agentUrlstays for the standalone server (FOREST_AGENT_URL), where there is no agent process to dispatch into.Why
Self-hosted setups (Swaive) observed every mounted tool call leaving over the public internet and coming back, even though the MCP server and the agent run in the same process. In-process dispatch keeps that hop internal by construction — nothing to configure.
How
buildAgentHttpError,deserializeAgentBody) and letcreateRemoteAgentClientaccept an injectedhttpRequester.InProcessDispatcherinjects a request into the agent's real router (auth / permissions / serializer all run);getInProcessDispatcher()re-registers the handler on every remount so a captured reference never goes stale.ToolContext; when a dispatcher is present,buildClientbuilds anInProcessHttpRequesterthat reuses the agent-client parse helpers — so the deserialized result andAgentHttpErrorshape are byte-identical to the HTTP path, keeping approval-gate detection and error handling unchanged. CSV export (stream) is not supported in-process and throws explicitly.Tests
mcp-server:InProcessHttpRequesterunit tests +agent-callerwiring (requester built only when a dispatcher is present) — 624 passing.agent: constructor-spy thatmountAiMcpServerpasses anagentDispatcher; and a full-stack guard that a real OAuth-authedtools/callreaches the agent through theInProcessDispatcheracross all 8 host frameworks — 982 passing.should retrieve users via MCP tools/callintegration test now runs through the in-process path and still returns the seeded records.🤖 Generated with Claude Code
Note
Dispatch MCP server tool calls to the agent in-process instead of over HTTP
InProcessDispatcherin the agent andInProcessHttpRequesterin the MCP server so that tool calls bypass the network and go directly into the agent's Koa stack vialight-my-request.agentDispatchertoForestMCPServerOptionsand a sharedToolContext; alldeclare*Toolfunctions are refactored to accept this context so they can route calls in-process when a dispatcher is available.agentUrlsupport (FOREST_AGENT_URLenv var) for standalone mode, normalised via a newnormalizeAgentUrlutility, and plumbed throughForestOAuthProvidersoextra.environmentApiEndpointreflects the agent URL.buildAgentHttpErroranddeserializeAgentBodyhelpers fromagent-clientand extendscreateRemoteAgentClientto accept an injectedHttpRequester, enabling the in-process path to share the same error shapes and deserialization logic.Changes since #1743 opened
agent.Agent.mountAiMcpServermethod to clarify middleware and authentication behavior [929a1c8]InProcessDispatchRequestandInProcessDispatchResponsetypes from@forestadmin/agentto@forestadmin/mcp-server[9db0616]ToolContextinterface,createClientfunction inagent-callermodule, and updated comment onDEFAULT_TIMEOUT_MSconstant to referenceHttpRequester.query[9db0616]buildAgentHttpErroranddeserializeAgentBodyutility functions into protected methods withinHttpRequesterclass [9a1a1c5]InProcessHttpRequester.stream()method that throws an error indicating CSV streaming is not supported over the in-process MCP transport [ac2d6bb]InProcessHttpRequester.stream()error behavior and updated agent caller test to expectInProcessHttpRequesterwiring whenenvironmentApiEndpointis absent [ac2d6bb]InProcessDispatcher.parseBodyto reject with anErrorfor successful HTTP responses (status < 400) containing non-JSON payloads and to log a warning while returningundefinedfor error responses (status >= 400) with non-JSON payloads [c801d5b]InProcessDispatcher.injectWithTimeoutby capturing the injection promise separately and attaching a catch handler to log warnings when the handler rejects after the timeout has fired [c801d5b]mcp-in-process-dispatcher.rejection.test.tsto verify that late injection rejections after timeout are logged as warnings without causing unhandled rejections [c801d5b]mcp-in-process-dispatcher.test.tsto validate that non-JSON 2xx responses throw errors and non-JSON 4xx/5xx responses log warnings while returningundefinedbody [c801d5b]FrameworkMounter.getInProcessDispatchermethod andInProcessDispatcherclass to clarify that the handler is rebuilt on every mount operation and captured references never become stale, removing previous references to a fixed '/forest' prefix [6636d46]DEFAULT_TIMEOUT_MSconstant comment inmcp-in-process-dispatcherto reference the HTTP path's 10 second timeout bound instead ofsuperagentimplementation specifics [6636d46]InProcessHttpRequesterclass and its methods to emphasize reuse ofHttpRequesterparse helpers, identical result and error shapes to the HTTP path, sentinel host behavior, and error throwing behavior for unsupportedstreammethod [6636d46]Macroscope summarized 8abefd1.