fix(mcp): give provider-bound tool calls their own fetch budget (#10860)

Obrigado — o hop de routing (route_request) herdava o budget de 10s de management em vez do budget de 60s de upstream que web_search/web_fetch já usavam, então uma rota de 35-40s abortava só pelo lado do MCP.

Validação (worktree combinado a partir de origin/release/v3.8.50, 0 conflitos):
- typecheck:core limpo, complexity/cognitive-complexity dentro do baseline
- tests/unit/mcp-upstream-fetch-timeout-9717.test.ts — 6/6 passando
- Suíte MCP completa — 149/153 (branch) vs 143/147 (release), as 4 falhas são idênticas nos dois lados e não relacionadas (closure de package-files, resolução de bundle dist/)

changelog.d/fixes/10860-mcp-upstream-fetch-timeout.md

@@ -0,0 +1 @@
+- **fix(mcp):** MCP tool calls that wait on a model provider no longer abort after 10 seconds. `omniRouteFetch` applied a single hardcoded `AbortSignal.timeout(10000)` to every internal hop, and `omniroute_route_request` — which posts to `/v1/chat/completions` and waits on the upstream provider, plus auto-combo candidate probing before a provider is even chosen — passed no signal of its own, so it inherited it. Any route slower than 10s failed from the MCP side while the identical request succeeded through the REST API. `omniroute_web_search` and `omniroute_web_fetch` in the same file already carried an explicit 60s signal, so that value is now shared by all three provider-bound calls instead of being repeated as a literal, while management reads (health, resilience, rate limits, combos, quota, usage) keep their fast-fail 10s budget so a stalled local endpoint still cannot hold a tool call open. Both budgets are overridable through `OMNIROUTE_MCP_FETCH_TIMEOUT_MS` and `OMNIROUTE_MCP_UPSTREAM_TIMEOUT_MS`, replacing the reported workaround of patching the compiled `dist/.build/next/server/chunks/*.js`; a malformed or non-positive override falls back to the default rather than disabling the timeout

docs/frameworks/MCP-SERVER.md

@@ -347,6 +347,8 @@ per-key path take precedence once it is. stdio has no per-caller identity (see
 | `OMNIROUTE_MCP_SCOPES`                  | (empty)                            | Comma-separated allowlist of scopes considered "available" by default (used when caller does not provide its own scopes) |
 | `OMNIROUTE_MCP_COMPRESS_DESCRIPTIONS`   | (unset = on)                       | When set to `0/false/off/no`, disables MCP description compression at registration time                                  |
 | `OMNIROUTE_MCP_DESCRIPTION_COMPRESSION` | (unset = on)                       | Alternate alias for the same toggle as above                                                                             |
+| `OMNIROUTE_MCP_FETCH_TIMEOUT_MS`        | `10000`                            | Abort budget for internal management reads (health, resilience, combos, quota, usage)                                    |
+| `OMNIROUTE_MCP_UPSTREAM_TIMEOUT_MS`     | `60000`                            | Abort budget for hops that wait on a provider (`route_request`, `web_search`, `web_fetch`)                               |
 | `MCP_TOOL_DENY`                         | (unset = no filter)                | Comma-separated tool names to drop from `tools/list` (tool-cardinality reduction — see below)                            |
 | `MCP_TOOL_ALLOW`                        | (unset = no filter)                | Comma-separated tool names to keep exclusively (allow-list mode — see below)                                             |
 | `DATA_DIR`                              | `~/.omniroute`                     | Heartbeat file is written to `${DATA_DIR}/runtime/mcp-heartbeat.json`                                                    |

open-sse/mcp-server/fetchTimeout.ts

@@ -0,0 +1,60 @@
+/**
+ * #9717 — timeout policy for the MCP server's internal server→server fetches.
+ *
+ * `omniRouteFetch` serves two call shapes with very different latency budgets:
+ * fast local management reads (health, resilience, combos, quota, usage) and
+ * calls that wait on an upstream provider. A single 10s default aborted
+ * `omniroute_route_request` while the upstream request was still in flight,
+ * even though `omniroute_web_search` / `omniroute_web_fetch` already carried
+ * their own explicit 60s signal in the same file for exactly that reason.
+ *
+ * Kept as a pure, dependency-free module so the policy is unit-testable without
+ * starting the MCP server, mirroring how `tools/poolTools.ts` keeps handlers
+ * separate from server wiring.
+ */
+
+/** Local management reads — a stalled one should fail fast, not hold a tool call open. */
+export const MCP_FETCH_TIMEOUT_MS = 10_000;
+
+/**
+ * Calls that wait on an upstream provider. 60s is not a new number: it is the
+ * value `web_search`/`web_fetch` already used, now shared with model routing
+ * instead of each call site picking its own literal.
+ */
+export const MCP_UPSTREAM_FETCH_TIMEOUT_MS = 60_000;
+
+export const MCP_FETCH_TIMEOUT_ENV = "OMNIROUTE_MCP_FETCH_TIMEOUT_MS";
+export const MCP_UPSTREAM_FETCH_TIMEOUT_ENV = "OMNIROUTE_MCP_UPSTREAM_TIMEOUT_MS";
+
+export type McpFetchTimeoutKind = "management" | "upstream";
+
+function readPositiveIntEnv(raw: string | undefined): number | null {
+  if (typeof raw !== "string" || raw.trim() === "") return null;
+  const parsed = Number(raw);
+  return Number.isSafeInteger(parsed) && parsed > 0 ? parsed : null;
+}
+
+/**
+ * Resolve the timeout for one internal fetch class. An unset, malformed or
+ * non-positive override falls back to the built-in default rather than
+ * disabling the timeout — a bad env value must not turn a bounded wait into an
+ * unbounded one.
+ */
+export function resolveMcpFetchTimeoutMs(
+  kind: McpFetchTimeoutKind,
+  env: Record<string, string | undefined> = process.env
+): number {
+  const upstream = kind === "upstream";
+  const override = readPositiveIntEnv(
+    env[upstream ? MCP_UPSTREAM_FETCH_TIMEOUT_ENV : MCP_FETCH_TIMEOUT_ENV]
+  );
+  return override ?? (upstream ? MCP_UPSTREAM_FETCH_TIMEOUT_MS : MCP_FETCH_TIMEOUT_MS);
+}
+
+/** `AbortSignal` for one internal fetch of the given class. */
+export function mcpFetchTimeoutSignal(
+  kind: McpFetchTimeoutKind,
+  env?: Record<string, string | undefined>
+): AbortSignal {
+  return AbortSignal.timeout(resolveMcpFetchTimeoutMs(kind, env));
+}

open-sse/mcp-server/server.ts

@@ -92,6 +92,7 @@ import { getDbInstance } from "../../src/lib/db/core.ts";
 import { normalizeQuotaResponse } from "../../src/shared/contracts/quota.ts";
 import { resolveOmniRouteBaseUrl } from "../../src/shared/utils/resolveOmniRouteBaseUrl.ts";
 import { sanitizeErrorMessage } from "../utils/error.ts";
+import { mcpFetchTimeoutSignal } from "./fetchTimeout.ts";
 import { getMcpModelsCatalog } from "./catalog.ts";
 import { registerRadarCatalogTool } from "./radarCatalog.ts";
 import type { TextToolResult } from "./toolResult.ts";
@@ -213,7 +214,7 @@ export async function omniRouteFetch(path: string, options: RequestInit = {}): P
     ...getInternalServiceAuthHeaders(),
   };
 
-  const signal = options.signal || AbortSignal.timeout(10000);
+  const signal = options.signal || mcpFetchTimeoutSignal("management");
   const response = await fetch(url, { ...options, headers, signal });
 
   if (!response.ok) {
@@ -518,6 +519,10 @@ async function handleRouteRequest(args: {
     const raw = (await omniRouteFetch("/v1/chat/completions", {
       method: "POST",
       body: JSON.stringify(body),
+      // #9717: this hop waits on an upstream provider (and on auto-combo
+      // candidate probing before one is even chosen), so it must not inherit
+      // the management-read budget.
+      signal: mcpFetchTimeoutSignal("upstream"),
     })) as JsonRecord;
     const choices = toArray(raw.choices);
     const firstChoice = toRecord(choices[0]);
@@ -648,7 +653,7 @@ async function handleWebSearch(args: {
     const result = await omniRouteFetch("/v1/search", {
       method: "POST",
       body: JSON.stringify(body),
-      signal: AbortSignal.timeout(60000),
+      signal: mcpFetchTimeoutSignal("upstream"),
     });
     await logToolCall("omniroute_web_search", args, result, Date.now() - start, true);
     return { content: [{ type: "text" as const, text: JSON.stringify(result, null, 2) }] };
@@ -681,7 +686,7 @@ async function handleWebFetch(args: {
     const result = await omniRouteFetch("/v1/web/fetch", {
       method: "POST",
       body: JSON.stringify(body),
-      signal: AbortSignal.timeout(60000),
+      signal: mcpFetchTimeoutSignal("upstream"),
     });
     await logToolCall("omniroute_web_fetch", args, result, Date.now() - start, true);
     return { content: [{ type: "text" as const, text: JSON.stringify(result, null, 2) }] };

tests/unit/mcp-upstream-fetch-timeout-9717.test.ts

@@ -0,0 +1,191 @@
+/**
+ * #9717 — the MCP server's internal fetch budget.
+ *
+ * `omniRouteFetch` applied one hardcoded 10s `AbortSignal.timeout` to every
+ * internal hop, including `omniroute_route_request`'s call to
+ * `/v1/chat/completions`. That hop waits on an upstream provider (and on
+ * auto-combo candidate probing before a provider is even chosen), so any route
+ * slower than 10s aborted from the MCP side while the same request succeeded
+ * through the REST API. `web_search`/`web_fetch` in the same file already used
+ * an explicit 60s signal, which is the value adopted here.
+ */
+import test from "node:test";
+import assert from "node:assert/strict";
+
+const {
+  resolveMcpFetchTimeoutMs,
+  MCP_FETCH_TIMEOUT_MS,
+  MCP_UPSTREAM_FETCH_TIMEOUT_MS,
+  MCP_FETCH_TIMEOUT_ENV,
+  MCP_UPSTREAM_FETCH_TIMEOUT_ENV,
+} = await import("../../open-sse/mcp-server/fetchTimeout.ts");
+const { createMcpServer, omniRouteFetch } = await import("../../open-sse/mcp-server/server.ts");
+
+type RegisteredTool = {
+  handler: (
+    args: unknown,
+    extra?: unknown
+  ) => Promise<{ content?: Array<{ type: string; text: string }>; isError?: boolean }>;
+};
+
+function getRegisteredHandler(server: unknown, toolName: string) {
+  const registry = (server as { _registeredTools?: Record<string, RegisteredTool> })
+    ._registeredTools;
+  assert.ok(registry, "McpServer should expose _registeredTools");
+  const tool = registry[toolName];
+  assert.ok(tool, `${toolName} must be registered on the live MCP server`);
+  return tool.handler;
+}
+
+const CHAT_COMPLETION_BODY = {
+  choices: [{ message: { content: "ok" } }],
+  model: "test-model",
+  usage: { prompt_tokens: 1, completion_tokens: 1 },
+  provider: "test-provider",
+};
+
+/**
+ * Stand-in for `fetch` that answers after `delayMs` but honours an abort signal
+ * the same way the real implementation does — a stub that ignored the signal
+ * would make every timeout assertion below pass vacuously.
+ */
+function stubFetch(delayMs: number, seen: { signals: AbortSignal[] }) {
+  const original = globalThis.fetch;
+  globalThis.fetch = ((_url: unknown, init?: { signal?: AbortSignal }) => {
+    const signal = init?.signal;
+    if (signal) seen.signals.push(signal);
+    return new Promise((resolve, reject) => {
+      const timer = setTimeout(
+        () =>
+          resolve({
+            ok: true,
+            status: 200,
+            json: async () => CHAT_COMPLETION_BODY,
+            text: async () => JSON.stringify(CHAT_COMPLETION_BODY),
+          }),
+        delayMs
+      );
+      const abort = () => {
+        clearTimeout(timer);
+        reject(signal?.reason ?? new Error("aborted"));
+      };
+      if (signal?.aborted) return abort();
+      signal?.addEventListener("abort", abort, { once: true });
+    });
+  }) as typeof globalThis.fetch;
+  return () => {
+    globalThis.fetch = original;
+  };
+}
+
+function withEnv(vars: Record<string, string | undefined>) {
+  const previous = new Map<string, string | undefined>();
+  for (const [key, value] of Object.entries(vars)) {
+    previous.set(key, process.env[key]);
+    if (value === undefined) delete process.env[key];
+    else process.env[key] = value;
+  }
+  return () => {
+    for (const [key, value] of previous) {
+      if (value === undefined) delete process.env[key];
+      else process.env[key] = value;
+    }
+  };
+}
+
+async function callRouteRequest() {
+  const handler = getRegisteredHandler(createMcpServer(), "omniroute_route_request");
+  return handler(
+    { model: "test-model", messages: [{ role: "user", content: "hi" }] },
+    { authInfo: { clientId: "test-9717", scopes: ["execute:completions"] } }
+  );
+}
+
+// ── Policy ────────────────────────────────────────────────────────────────
+
+test("#9717: the upstream budget is larger than the management budget", () => {
+  assert.equal(resolveMcpFetchTimeoutMs("management"), MCP_FETCH_TIMEOUT_MS);
+  assert.equal(resolveMcpFetchTimeoutMs("upstream"), MCP_UPSTREAM_FETCH_TIMEOUT_MS);
+  assert.equal(MCP_FETCH_TIMEOUT_MS, 10_000);
+  assert.equal(
+    MCP_UPSTREAM_FETCH_TIMEOUT_MS,
+    60_000,
+    "matches the signal web_search/web_fetch already used"
+  );
+  assert.ok(MCP_UPSTREAM_FETCH_TIMEOUT_MS > MCP_FETCH_TIMEOUT_MS);
+});
+
+test("#9717: each budget reads its own env override", () => {
+  const env = {
+    [MCP_FETCH_TIMEOUT_ENV]: "1234",
+    [MCP_UPSTREAM_FETCH_TIMEOUT_ENV]: "222222",
+  };
+  assert.equal(resolveMcpFetchTimeoutMs("management", env), 1234);
+  assert.equal(resolveMcpFetchTimeoutMs("upstream", env), 222222);
+});
+
+test("#9717: a malformed override falls back to the default instead of disabling the timeout", () => {
+  for (const bad of ["", "   ", "0", "-1", "abc", "60000.5", "NaN", "Infinity"]) {
+    assert.equal(
+      resolveMcpFetchTimeoutMs("upstream", { [MCP_UPSTREAM_FETCH_TIMEOUT_ENV]: bad }),
+      MCP_UPSTREAM_FETCH_TIMEOUT_MS,
+      `"${bad}" must not become the effective timeout`
+    );
+  }
+});
+
+// ── Wiring ────────────────────────────────────────────────────────────────
+
+test("#9717: route_request is bound to the upstream budget, not the management default", async () => {
+  const seen = { signals: [] as AbortSignal[] };
+  const restoreEnv = withEnv({ [MCP_UPSTREAM_FETCH_TIMEOUT_ENV]: "40" });
+  const restoreFetch = stubFetch(400, seen);
+  try {
+    const result = await callRouteRequest();
+    assert.equal(
+      result.isError,
+      true,
+      "with the upstream budget set to 40ms a 400ms upstream must abort — before #9717 this " +
+        "call ignored that setting and used the hardcoded 10s default, so it returned a result"
+    );
+    assert.ok(seen.signals.length > 0, "the routing hop must carry an abort signal");
+  } finally {
+    restoreFetch();
+    restoreEnv();
+  }
+});
+
+test("#9717: route_request outlives the management budget", async () => {
+  const seen = { signals: [] as AbortSignal[] };
+  const restoreEnv = withEnv({
+    [MCP_FETCH_TIMEOUT_ENV]: "40",
+    [MCP_UPSTREAM_FETCH_TIMEOUT_ENV]: undefined,
+  });
+  const restoreFetch = stubFetch(400, seen);
+  try {
+    const result = await callRouteRequest();
+    assert.notEqual(
+      result.isError,
+      true,
+      "a 400ms upstream must survive: the routing hop must not inherit the 40ms management budget"
+    );
+  } finally {
+    restoreFetch();
+    restoreEnv();
+  }
+});
+
+test("#9717: management reads honour their own override", async () => {
+  const seen = { signals: [] as AbortSignal[] };
+  const restoreEnv = withEnv({ [MCP_FETCH_TIMEOUT_ENV]: "40" });
+  const restoreFetch = stubFetch(400, seen);
+  try {
+    await assert.rejects(
+      () => omniRouteFetch("/api/monitoring/health"),
+      "a management read must abort at its configured budget"
+    );
+  } finally {
+    restoreFetch();
+    restoreEnv();
+  }
+});