feat(mcp-server): agentUrl option to route MCP tool calls internally

[Scra3]

Context

MCP tools don't touch the datasource directly — each tool call (list, create, executeAction…) calls back into the agent's data layer over HTTP via agent-client. The URL used is the environment's public api_endpoint (as registered in Forest), read from AuthInfo.extra.environmentApiEndpoint.

Consequence: even when the MCP server is mounted inside the agent (same process), every tool call leaves over the public internet and comes back. A self-hosted customer (Swaive, KYB prod use case with n8n) asked to keep this traffic on their private network.

Change

Opt-in agentUrl option that overrides only the internal tool→agent channel:

agent.mountAiMcpServer({ agentUrl: 'http://forest-agent.internal:3310' });

• forest-oauth-provider.ts — new agentUrl (validated as an absolute URL, trailing slash stripped); verifyAccessToken puts agentUrl ?? environmentApiEndpoint into extra.environmentApiEndpoint (the single consumer, in agent-caller.ts). getBaseUrl() untouched.
• server.ts — ForestMCPServerOptions.agentUrl, threaded to the provider.
• cli.ts — FOREST_AGENT_URL env var for the standalone binary.
• agent.ts — mountAiMcpServer({ agentUrl }) threading.

The advertised OAuth URLs (issuer, .well-known metadata, authorize/token) stay public — external MCP clients (n8n) still authenticate from outside. agentUrl only changes where tools reach the agent.

Same concept and name as addWorkflowExecutor({ agentUrl }). Default unchanged (public api_endpoint), so zero impact on existing deployments.

Tests

• forest-oauth-provider.test.ts — extra.environmentApiEndpoint = agentUrl when set (fallback to api_endpoint otherwise), trailing slash stripped, invalid URL throws.
• server.test.ts — with agentUrl set, advertised OAuth metadata URLs stay public.
• agent.test.ts — mountAiMcpServer({ agentUrl }) reaches ForestMCPServer.

mcp-server: 608 tests pass, lint clean.

Definition of Done

General

• Explicit title following Conventional Commits
• Test manually the implemented changes
• Validate the code quality

Security

• Consider the security impact — agentUrl is validated; default behavior unchanged; the option improves security posture (keeps tool traffic off the public internet).

🤖 Generated with Claude Code

Note

Add agentUrl option to ForestMCPServer to route MCP tool calls internally

• Adds an optional agentUrl to ForestMCPServer (and propagates it through Agent.mountAiMcpServer and the CLI via FOREST_AGENT_URL) so tool callbacks can be directed to an internal agent URL instead of the public api_endpoint.
• A new normalizeAgentUrl utility validates that the URL is an absolute http(s) URL with no query string or fragment, stripping trailing slashes; invalid values throw at construction time.
• When set, ForestOAuthProvider places agentUrl in extra.environmentApiEndpoint during token verification, overriding the environment's api_endpoint for tool routing only — OAuth metadata endpoints are unaffected.
• Risk: passing a malformed agentUrl throws an error at server startup.

Changes since #1740 opened

• Removed agentUrl parameter support from Agent.mountAiMcpServer method and internal MCP server instantiation [0327a3c]
• Removed test verification for agentUrl passthrough in MCP server configuration [0327a3c]
• Updated JSDoc documentation for agentUrl in ForestMCPServerOptions interface [0327a3c]

^{Macroscope summarized 0507fb9.}

[qltysh]

Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (3)

Rating	File	% Diff	Uncovered Line #s
	packages/mcp-server/src/forest-oauth-provider.ts	100.0%
	packages/mcp-server/src/server.ts	100.0%
	packages/mcp-server/src/utils/normalize-agent-url.ts	100.0%
	Total	100.0%

🚦 See full report on Qlty Cloud »

🛟 Help

• Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

• Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

• File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

  • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

[ShohanRahman]

1. agentUrl validation is deferred and fails at a confusing point (converged: silent-failure-hunter, type-design-analyzer, code-simplifier — high confidence)
   ForestMCPServer stores agentUrl raw (server.ts:175). The only validator, normalizeAgentUrl, runs inside ForestOAuthProvider's constructor — which is created lazily in buildExpressApp(). So a malformed FOREST_AGENT_URL / agentUrl:

• passes new ForestMCPServer({...}) silently,
• throws only at server.run() / agent.start() time, with a stack trace pointing at ForestOAuthProvider and no hint it came from the env var or the mountAiMcpServer call.

Fix: extract normalizeAgentUrl to a shared/exported helper and call it in the ForestMCPServer constructor so invalid values throw eagerly, proximate to the misconfiguration. forest-oauth-provider.test.ts already tests the throw via the direct constructor path, so moving the throw site earlier won't break existing tests.

2. Missing test for the fallback path (agentUrl absent → environmentApiEndpoint) (pr-test-analyzer)
   The logic is this.agentUrl ?? this.environmentApiEndpoint. Only the agentUrl-present side is tested for this feature. A ??→|| regression, or agentUrl accidentally set on the no-agentUrl path, would go uncaught. Add a verifyAccessToken test: no agentUrl, initialize() with a mock api_endpoint, assert extra.environmentApiEndpoint equals the public endpoint.

[Scra3]

Thanks @ShohanRahman — both addressed in c26d56b:

1. Eager validation — extracted normalizeAgentUrl to a shared helper (src/utils/normalize-agent-url.ts) and called it in the ForestMCPServer constructor, so a malformed agentUrl/FOREST_AGENT_URL now throws at construction instead of later in ForestOAuthProvider. The provider still calls the same helper, so its direct-constructor throw tests stay green.
2. Fallback test — added a verifyAccessToken test: no agentUrl, initialize() with a mock api_endpoint, asserts extra.environmentApiEndpoint equals the public endpoint.

613 tests green, coverage 100%.

[ShohanRahman]

No direct unit test for normalize-agent-url.ts; https:// and path-preserving URLs uncovered

Confirmed: no test/utils/normalize-agent-url.test.ts exists, and every test agentUrl is http://. Two silent-regression risks:

• https:// acceptance is untested — the protocol guard (normalize-agent-url.ts:14) could drop https: support and all tests would still pass.
• Path preservation is untested — the util returns parsed.href (keeps a base path like http://host/base); if it ever changed to parsed.origin, tool calls would route to the wrong path and no test would catch it. This is the highest-risk gap since the util is the single enforcement point.

Add a focused unit test covering: valid https://, path-preserving base, trailing-slash strip, empty/undefined → undefined, and the four throw cases. Also worth a new ForestMCPServer({ agentUrl: 'not-a-url' }) throws test in server.test.ts to lock the eager-fail guarantee (currently only proven via the provider test).

[Scra3]

@ShohanRahman both addressed in 0507fb9:

• Single owner — ForestMCPServer now validates/normalizes agentUrl; ForestOAuthProvider accepts the pre-normalized value (dropped its normalizeAgentUrl call). No more double-normalization.
• Direct unit test — added test/utils/normalize-agent-url.test.ts covering: http:// and https:// acceptance (locks the protocol guard), base-path preservation + trailing-slash strip (locks the parsed.href behavior), and the invalid cases (non-http(s), query, fragment, garbage). Moved the invalid/trailing-slash cases out of the provider test since the provider no longer owns that.

616 tests green.

[ShohanRahman]

1. Timeout-raced inject() promise has no .catch() → unhandled rejection risk
   packages/agent/src/mcp-in-process-dispatcher.ts:69-83
   Promise.race([inject(...), timeout]) — when the timeout wins, the inject() promise keeps running with no observer. If the hung agent handler later rejects (e.g. a DB call fails 30s after the tool call already timed out), it becomes an unhandled promise rejection, which can crash processes with a strict unhandledRejection handler. Fix: attach a .catch() that logs, before racing.
   const injectPromise = inject(this.handler as RequestListener, options);
   injectPromise.catch(err => this.logger?.('Warn', In-process handler settled after timeout: ${err}));
   return await Promise.race([injectPromise, timeout]);

2. Non-JSON 2xx body silently deserializes to undefined
   packages/agent/src/mcp-in-process-dispatcher.ts:85-95 → in-process-http-requester.ts:59
   parseBody returns undefined for a non-JSON body (warn-logged only). On a 2xx, deserializeAgentBody then returns undefined as Data — the tool hands the LLM no data and no error. (On 4xx+ it's fine: buildAgentHttpError falls back to parseJson(text).) The agent's JSON:API routes normally return JSON, so this is an edge case (unexpected HTML/redirect from misbehaving middleware), but it fails invisibly. Fix: include the raw text (truncated) in the warn log, and treat a non-empty non-JSON 2xx body as an error rather than returning undefined.

[Scra3]

@ShohanRahman les deux corrigés dans c801d5b38 (sur la branche in-process dispatcher) :

1. Unhandled rejection sur inject(). injectWithTimeout attache maintenant un .catch() à la promesse inject() avant la race — si le timeout gagne et qu'inject() rejette ensuite, le rejet est observé (loggé en Warn « settled after the … timeout ») au lieu de fuir en unhandledRejection. Couvert par un test dédié qui mocke inject pour rejeter après le timeout.

À noter : dans la vraie stack Koa, une erreur du handler est catchée par Koa → réponse 500 → inject() résout (un résolu ignoré ne fuit pas). Le .catch couvre donc le cas plus rare où inject() lui-même rejette — cheap insurance, comme tu le suggérais.

2. Non-JSON 2xx → undefined silencieux. parseBody prend désormais le status :

• 2xx non-JSON → throw (au lieu de rendre undefined) : un corps de succès non-JSON = middleware qui déraille (HTML/redirect), on échoue bruyamment.
• 4xx+ non-JSON → undefined + Warn (comportement conservé : buildError retombe sur le text brut).
• Dans les deux cas, un extrait tronqué (200 car.) du corps est inclus dans l'erreur/le log.

Tests mis à jour en conséquence (le cas 200-non-JSON devient un throw, ajout d'un cas 4xx-non-JSON).