From b392219659d61842c02d434c55b75f4e9c93fb20 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 26 Jun 2026 17:37:08 +0300 Subject: [PATCH] fix(ai-mcp): show Failed when the inline Test request itself rejects (#170) The per-row MCP Test button derived its presentation solely from the test mutation's data ({ ok, tools } | { ok, error }). When the request itself rejected (401/403/500/network) there is no payload, so the row silently spun back to the idle "Test" instead of reporting the failure. Feed the mutation error into mcpTestButtonView so a reject also renders a red "Failed", with the tooltip taken from the server message (error.response.data.message) or a generic i18n fallback. Enable the tooltip for any non-idle state. Cover the reject branch (with and without a server message) in the helper unit test. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ai-mcp-server-test-view.test.ts | 28 +++++++++++++++++ .../components/ai-mcp-server-test-view.ts | 31 +++++++++++++++++++ .../settings/components/ai-mcp-servers.tsx | 13 ++++++-- 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-test-view.test.ts b/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-test-view.test.ts index cde45cc6..60b10c4e 100644 --- a/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-test-view.test.ts +++ b/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-test-view.test.ts @@ -56,4 +56,32 @@ describe("mcpTestButtonView", () => { tooltip: "402: nope", }); }); + + it("failed when the request itself rejects (no result payload)", () => { + // 401/403/500/network: there is no { ok } body, only a thrown error. The + // row must still show a red "Failed" rather than reverting to idle "Test". + expect( + mcpTestButtonView(undefined, t, { + response: { data: { message: "Unauthorized" } }, + }), + ).toEqual({ + state: "failed", + color: "red", + variant: "light", + label: "Failed", + tooltip: "Unauthorized", + }); + }); + + it("reject without a server message falls back to the generic label", () => { + // A bare network error (no response body) still surfaces as failed, using + // the i18n fallback for the tooltip. + expect(mcpTestButtonView(undefined, t, new Error("network down"))).toEqual({ + state: "failed", + color: "red", + variant: "light", + label: "Failed", + tooltip: "Failed to update data", + }); + }); }); diff --git a/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-test-view.ts b/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-test-view.ts index b438935a..8f8db84c 100644 --- a/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-test-view.ts +++ b/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-test-view.ts @@ -3,6 +3,23 @@ import type { IAiMcpServerTestResult } from "@/features/workspace/services/ai-mc /** Minimal translator shape (i18next `t`): key + optional interpolation. */ type Translate = (key: string, options?: Record) => string; +/** Subset of an axios-style rejection we read for the reject tooltip. */ +type McpTestRequestError = { + response?: { data?: { message?: string } }; +}; + +/** + * Best-effort extraction of a server-sent message from a rejected test request + * (axios stores it at `error.response.data.message`). Returns undefined for a + * bare/network error so the caller can fall back to a generic label. + */ +function readRequestErrorMessage(error: unknown): string | undefined { + if (error && typeof error === "object" && "response" in error) { + return (error as McpTestRequestError).response?.data?.message; + } + return undefined; +} + /** * Presentation for the inline "Test" button, derived from the current test * result tristate (no result yet / ok / failed). Color is never the only signal @@ -27,6 +44,7 @@ export interface McpTestButtonView { export function mcpTestButtonView( result: IAiMcpServerTestResult | undefined, t: Translate, + error?: unknown, ): McpTestButtonView { if (result?.ok) { return { @@ -49,6 +67,19 @@ export function mcpTestButtonView( tooltip: result.error, }; } + if (error) { + // The test request itself rejected (401/403/500/network) — there is no + // `{ ok }` payload, so without this branch the row would silently revert to + // the idle "Test" instead of reporting the failure. Tooltip prefers the + // server-sent message, else the generic i18n fallback. + return { + state: "failed", + color: "red", + variant: "light", + label: t("Failed"), + tooltip: readRequestErrorMessage(error) ?? t("Failed to update data"), + }; + } return { state: "idle", color: undefined, diff --git a/apps/client/src/features/workspace/components/settings/components/ai-mcp-servers.tsx b/apps/client/src/features/workspace/components/settings/components/ai-mcp-servers.tsx index 5ccdb380..d0a19247 100644 --- a/apps/client/src/features/workspace/components/settings/components/ai-mcp-servers.tsx +++ b/apps/client/src/features/workspace/components/settings/components/ai-mcp-servers.tsx @@ -185,8 +185,15 @@ function AiMcpServerRow({ // Single derivation of the button/tooltip presentation from the test tristate // (idle / ok / failed), so the two can never drift apart. Tooltip is "" while - // there is no result; the icon is mapped from `view.state` below. - const view = mcpTestButtonView(result, t); + // there is no result; the icon is mapped from `view.state` below. When the + // request itself rejects (401/403/500/network) there is no `data` payload, so + // we feed the mutation error in too — otherwise the row would silently revert + // to "Test" instead of showing a red "Failed". + const view = mcpTestButtonView( + result, + t, + testMutation.isError ? testMutation.error : undefined, + ); const tooltipLabel = view.tooltip; const buttonColor = view.color; const buttonVariant = view.variant; @@ -225,7 +232,7 @@ function AiMcpServerRow({ {/* Always clickable: testing a disabled server before enabling it is useful. */}