fix(mcp): stop omniroute_get_health silently discarding real data (#9959)

process.uptime() returns a number, but the handler ran it through a
string-only toString() helper that fell back to "unknown" for anything
that wasn't already a string -- so every real uptime value was
discarded, 100% reproducibly.

Also stop masking upstream fetch failures as fake healthy defaults:
when /api/monitoring/health, /api/resilience, or /api/rate-limits
can't be reached, the tool now reports which source failed (via a new
optional `degraded` field) instead of returning zeros/empty arrays
indistinguishable from genuine "no data".

Regression coverage dispatches through the real MCP handler (client.callTool)
rather than asserting on the mock directly, since the prior mock-only
tests could never have caught either bug.

open-sse/mcp-server/__tests__/essentialTools.test.ts

@@ -296,3 +296,108 @@ describe("omniroute_web_search handler (via MCP dispatch)", () => {
     expect(result.isError).toBe(true);
   });
 });
+
+// ── omniroute_get_health: handler dispatch tests ──────────────────────────────
+// These tests use InMemoryTransport + Client to exercise the actual registered
+// handler (not mockFetch directly), so they catch the real bug the original
+// mock-only tests above (lines 39-56) could never catch: process.uptime()
+// returns a *number*, and a naive toString() guard silently discards it.
+
+describe("omniroute_get_health handler (via MCP dispatch)", () => {
+  let client: Client;
+
+  beforeEach(async () => {
+    mockFetch.mockReset();
+
+    const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
+    const server = createMcpServer();
+    await server.connect(serverTransport);
+    client = new Client({ name: "test-client", version: "1.0.0" });
+    await client.connect(clientTransport);
+  });
+
+  afterEach(async () => {
+    await client.close();
+  });
+
+  function mockHealthSources(opts: {
+    health?: unknown;
+    healthError?: Error;
+    resilience?: unknown;
+    resilienceError?: Error;
+    rateLimits?: unknown;
+    rateLimitsError?: Error;
+  }) {
+    mockFetch.mockImplementation(async (url: string) => {
+      if (url.includes("/api/monitoring/health")) {
+        if (opts.healthError) throw opts.healthError;
+        return { ok: true, json: async () => opts.health ?? {} };
+      }
+      if (url.includes("/api/resilience")) {
+        if (opts.resilienceError) throw opts.resilienceError;
+        return { ok: true, json: async () => opts.resilience ?? {} };
+      }
+      if (url.includes("/api/rate-limits")) {
+        if (opts.rateLimitsError) throw opts.rateLimitsError;
+        return { ok: true, json: async () => opts.rateLimits ?? {} };
+      }
+      throw new Error(`unexpected fetch: ${url}`);
+    });
+  }
+
+  it("should render a real numeric uptime as a string, not fall back to unknown", async () => {
+    mockHealthSources({
+      health: {
+        uptime: 4731.9817064,
+        version: "3.8.50",
+        memoryUsage: { heapUsed: 746337096, heapTotal: 765358080 },
+      },
+      resilience: { circuitBreakers: [] },
+      rateLimits: { limits: [] },
+    });
+
+    const result = await client.callTool({ name: "omniroute_get_health", arguments: {} });
+
+    expect(result.isError).toBeFalsy();
+    const content = result.content as Array<{ type: string; text: string }>;
+    const data = JSON.parse(content[0].text);
+    expect(data.uptime).toBe("4731.9817064");
+    expect(data.version).toBe("3.8.50");
+  });
+
+  it("should surface a degraded entry when one source fetch fails, instead of silently faking success", async () => {
+    mockHealthSources({
+      health: { uptime: 100, version: "3.8.50" },
+      resilience: { circuitBreakers: [] },
+      rateLimitsError: new Error("connect ECONNREFUSED 127.0.0.1:20128"),
+    });
+
+    const result = await client.callTool({ name: "omniroute_get_health", arguments: {} });
+
+    expect(result.isError).toBeFalsy();
+    const content = result.content as Array<{ type: string; text: string }>;
+    const data = JSON.parse(content[0].text);
+    // The two healthy sources still come through untouched.
+    expect(data.uptime).toBe("100");
+    expect(data.rateLimits).toEqual([]);
+    // But the failure is visible instead of being indistinguishable from "no rate limits".
+    expect(Array.isArray(data.degraded)).toBe(true);
+    expect(data.degraded).toHaveLength(1);
+    expect(data.degraded[0].source).toBe("rateLimits");
+    expect(data.degraded[0].error).toContain("ECONNREFUSED");
+  });
+
+  it("should omit degraded entirely when every source succeeds", async () => {
+    mockHealthSources({
+      health: { uptime: 1, version: "3.8.50" },
+      resilience: { circuitBreakers: [] },
+      rateLimits: { limits: [] },
+    });
+
+    const result = await client.callTool({ name: "omniroute_get_health", arguments: {} });
+
+    const content = result.content as Array<{ type: string; text: string }>;
+    const data = JSON.parse(content[0].text);
+    expect(data.degraded).toBeUndefined();
+  });
+});

open-sse/mcp-server/schemas/tools.ts

@@ -68,12 +68,20 @@ export const getHealthOutput = z.object({
       provider: z.string(),
     })
     .optional(),
+  degraded: z
+    .array(
+      z.object({
+        source: z.enum(["health", "resilience", "rateLimits"]),
+        error: z.string(),
+      })
+    )
+    .optional(),
 });
 
 export const getHealthTool: McpToolDefinition<typeof getHealthInput, typeof getHealthOutput> = {
   name: "omniroute_get_health",
   description:
-    "Returns the current health status of OmniRoute including uptime, memory usage, circuit breaker states for all providers, rate limit status, and cache statistics.",
+    "Returns the current health status of OmniRoute including uptime, memory usage, circuit breaker states for all providers, rate limit status, and cache statistics. If an underlying source (health/resilience/rate-limits) could not be reached, it is listed in `degraded` instead of being silently reported as empty/zero.",
   inputSchema: getHealthInput,
   outputSchema: getHealthOutput,
   scopes: ["read:health"],

open-sse/mcp-server/server.ts

@@ -270,6 +270,15 @@ function withScopeEnforcement(
   };
 }
 
+// process.uptime() (the source of health.uptime) returns a number, not a string;
+// the shared toString() helper only passes through actual strings, so a naive
+// toString(health.uptime, "unknown") silently discarded every real uptime value.
+function toUptimeString(value: unknown): string {
+  if (typeof value === "string") return value;
+  if (typeof value === "number" && Number.isFinite(value)) return String(value);
+  return "unknown";
+}
+
 async function handleGetHealth() {
   const start = Date.now();
   try {
@@ -287,8 +296,25 @@ async function handleGetHealth() {
     const resilienceCircuitBreakers = toArray(resilience.circuitBreakers);
     const rateLimitEntries = toArray(rateLimits.limits);
 
+    // Surface fetch failures instead of letting Promise.allSettled's {} fallback
+    // masquerade as genuine zero/empty data (indistinguishable "no data" vs.
+    // "couldn't reach the source" was the actual root confusion this fixes).
+    const degradedSources: Array<{ source: string; settled: PromiseSettledResult<unknown> }> = [
+      { source: "health", settled: healthRaw },
+      { source: "resilience", settled: resilienceRaw },
+      { source: "rateLimits", settled: rateLimitsRaw },
+    ];
+    const degraded = degradedSources
+      .filter(({ settled }) => settled.status === "rejected")
+      .map(({ source, settled }) => ({
+        source,
+        error: sanitizeErrorMessage(
+          settled.status === "rejected" ? (settled as PromiseRejectedResult).reason : undefined
+        ),
+      }));
+
     const result = {
-      uptime: toString(health.uptime, "unknown"),
+      uptime: toUptimeString(health.uptime),
       version: toString(health.version, "unknown"),
       memoryUsage: {
         heapUsed: toNumber(memoryUsageRaw.heapUsed, 0),
@@ -310,6 +336,7 @@ async function handleGetHealth() {
             provider: toString(toRecord(health.cryptography).provider, "unknown"),
           }
         : undefined,
+      degraded: degraded.length > 0 ? degraded : undefined,
     };
 
     await logToolCall("omniroute_get_health", {}, result, Date.now() - start, true);