From c28d8cc64877334dca11a24f4c91222b7796df95 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 26 Jun 2026 05:57:20 +0300 Subject: [PATCH 1/2] feat(ai-chat): inline Test button per external MCP server row (#170) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a per-row "Test" button to the external MCP servers list so admins can check a server's connection straight from the list, without opening the edit modal. - Extract each list row into its own AiMcpServerRow component, each owning a dedicated useTestAiMcpServerMutation instance. This isolates loading/result state per row — a single list-level mutation would make every row's spinner and colour jump on any test. - Button reflects the outcome with both colour AND label (a11y / colour-blind safe): idle "Test", loading, green "OK · {n}" (tool count), red "Failed". Fixed miw so the row does not jump as the label changes. A tooltip surfaces the tools list (success) or the sanitized error (failure). - Reset the mutation when url/transport/hasHeaders change so a stale result does not stick on the non-remounting (keyed-by-id) row. - Reuse the existing /workspace/ai-mcp-servers/test endpoint and mutation; backend/service/query unchanged. - i18n: add "Failed" and "OK · {{count}}" (en + ru); add the missing "Test" key to ru-RU. - Add a vitest suite covering idle/success/failure states and per-row isolation. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../public/locales/en-US/translation.json | 2 + .../public/locales/ru-RU/translation.json | 5 +- .../components/ai-mcp-server-row.test.tsx | 120 ++++++++++++++ .../settings/components/ai-mcp-server-row.tsx | 146 ++++++++++++++++++ .../settings/components/ai-mcp-servers.tsx | 64 ++------ 5 files changed, 284 insertions(+), 53 deletions(-) create mode 100644 apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.test.tsx create mode 100644 apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.tsx diff --git a/apps/client/public/locales/en-US/translation.json b/apps/client/public/locales/en-US/translation.json index bd8c4ed3..cb1b3086 100644 --- a/apps/client/public/locales/en-US/translation.json +++ b/apps/client/public/locales/en-US/translation.json @@ -713,6 +713,8 @@ "Optional. Leave empty to allow all tools the server exposes.": "Optional. Leave empty to allow all tools the server exposes.", "Optional guidance for the agent on how and when to use this server's tools. Injected into the system prompt. The server's tools are namespaced as \"_*\".": "Optional guidance for the agent on how and when to use this server's tools. Injected into the system prompt. The server's tools are namespaced as \"_*\".", "Test": "Test", + "Failed": "Failed", + "OK · {{count}}": "OK · {{count}}", "Available tools": "Available tools", "No tools available": "No tools available", "Created successfully": "Created successfully", diff --git a/apps/client/public/locales/ru-RU/translation.json b/apps/client/public/locales/ru-RU/translation.json index f8c59436..e46a0579 100644 --- a/apps/client/public/locales/ru-RU/translation.json +++ b/apps/client/public/locales/ru-RU/translation.json @@ -1169,5 +1169,8 @@ "Protocol": "Протокол", "How chat requests are sent and how reasoning is surfaced": "Как отправляются запросы чата и как показывается reasoning", "OpenAI-compatible (surfaces reasoning)": "OpenAI-совместимый (показывает reasoning)", - "OpenAI (official)": "OpenAI (официальный)" + "OpenAI (official)": "OpenAI (официальный)", + "Test": "Тест", + "Failed": "Ошибка", + "OK · {{count}}": "OK · {{count}}" } diff --git a/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.test.tsx b/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.test.tsx new file mode 100644 index 00000000..ecd5d2a6 --- /dev/null +++ b/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.test.tsx @@ -0,0 +1,120 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen, fireEvent, waitFor, within } from "@testing-library/react"; +import { MantineProvider } from "@mantine/core"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { IAiMcpServer } from "@/features/workspace/services/ai-mcp-server-service.ts"; + +// matchMedia (read by MantineProvider) is stubbed globally in vitest.setup.ts. + +// Stub react-i18next so `t` returns the key with `{{count}}` interpolated. This +// keeps assertions on the row's OWN label logic, mirroring the t-mock pattern +// used by other component tests in the repo. +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (key: string, opts?: { count?: number }) => + opts && typeof opts.count === "number" + ? key.replace("{{count}}", String(opts.count)) + : key, + }), +})); + +// Mock only the network call. The REAL useTestAiMcpServerMutation runs on a real +// QueryClient so each row gets a genuinely independent mutation instance — this +// is exactly the isolation the feature relies on (#170). +const testAiMcpServer = vi.fn(); +vi.mock("@/features/workspace/services/ai-mcp-server-service.ts", () => ({ + testAiMcpServer: (id: string) => testAiMcpServer(id), +})); + +import AiMcpServerRow from "./ai-mcp-server-row.tsx"; + +const baseServer = (over?: Partial): IAiMcpServer => ({ + id: "srv-1", + name: "Search", + transport: "http", + url: "https://example.com/mcp", + enabled: true, + toolAllowlist: null, + hasHeaders: false, + instructions: null, + ...over, +}); + +function renderRow(server: IAiMcpServer, testid: string) { + const client = new QueryClient({ + defaultOptions: { mutations: { retry: false }, queries: { retry: false } }, + }); + return render( + + +
+ +
+ + , + ); +} + +describe("AiMcpServerRow — inline Test button", () => { + beforeEach(() => { + testAiMcpServer.mockReset(); + }); + + it("starts in the idle state with a plain 'Test' label", () => { + renderRow(baseServer(), "row"); + const row = screen.getByTestId("row"); + expect(within(row).getByRole("button", { name: "Test" })).toBeDefined(); + }); + + it("shows a green 'OK · N' label with the tool count on success", async () => { + testAiMcpServer.mockResolvedValue({ ok: true, tools: ["a", "b", "c"] }); + renderRow(baseServer(), "row"); + const row = screen.getByTestId("row"); + + fireEvent.click(within(row).getByRole("button", { name: "Test" })); + + await waitFor(() => + expect(within(row).getByRole("button", { name: /OK · 3/ })).toBeDefined(), + ); + }); + + it("shows 'Failed' on a connection error", async () => { + testAiMcpServer.mockResolvedValue({ ok: false, error: "boom" }); + renderRow(baseServer(), "row"); + const row = screen.getByTestId("row"); + + fireEvent.click(within(row).getByRole("button", { name: "Test" })); + + await waitFor(() => + expect(within(row).getByRole("button", { name: "Failed" })).toBeDefined(), + ); + }); + + it("keeps each row's result isolated (testing one does not affect another)", async () => { + // Resolve based on id so the two rows get different outcomes. + testAiMcpServer.mockImplementation(async (id: string) => + id === "ok-1" + ? { ok: true, tools: ["x", "y"] } + : { ok: false, error: "down" }, + ); + + renderRow(baseServer({ id: "ok-1", name: "Good" }), "row-ok"); + renderRow(baseServer({ id: "fail-1", name: "Bad" }), "row-fail"); + + const okRow = screen.getByTestId("row-ok"); + fireEvent.click(within(okRow).getByRole("button", { name: "Test" })); + + await waitFor(() => + expect(within(okRow).getByRole("button", { name: /OK · 2/ })).toBeDefined(), + ); + + // The untouched row must still be idle — no shared/global pending state. + const failRow = screen.getByTestId("row-fail"); + expect(within(failRow).getByRole("button", { name: "Test" })).toBeDefined(); + }); +}); diff --git a/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.tsx b/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.tsx new file mode 100644 index 00000000..04c305da --- /dev/null +++ b/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.tsx @@ -0,0 +1,146 @@ +import { useEffect } from "react"; +import { ActionIcon, Badge, Button, Group, Stack, Switch, Text, Tooltip } from "@mantine/core"; +import { + IconCheck, + IconPencil, + IconPlugConnected, + IconTrash, + IconX, +} from "@tabler/icons-react"; +import { useTranslation } from "react-i18next"; +import { useTestAiMcpServerMutation } from "@/features/workspace/queries/ai-mcp-server-query.ts"; +import { IAiMcpServer } from "@/features/workspace/services/ai-mcp-server-service.ts"; + +interface AiMcpServerRowProps { + server: IAiMcpServer; + onEdit: (server: IAiMcpServer) => void; + onDelete: (server: IAiMcpServer) => void; + onToggleEnabled: (server: IAiMcpServer, enabled: boolean) => void; +} + +/** + * A single external MCP server row with an inline "Test" button. Each row owns + * its OWN test mutation instance so the loading/result state is isolated per + * row — a list-level mutation would make every row's spinner and colour jump on + * any single test (#170). + */ +export default function AiMcpServerRow({ + server, + onEdit, + onDelete, + onToggleEnabled, +}: AiMcpServerRowProps) { + const { t } = useTranslation(); + const testMutation = useTestAiMcpServerMutation(); + + // The result colour/label reflects the connection params at the time of the + // test. The row is keyed by id and never remounts, so a stale "OK"/"Failed" + // would otherwise stick after the URL/transport/auth changes. Reset on those. + useEffect(() => { + testMutation.reset(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [server.url, server.transport, server.hasHeaders]); + + const result = testMutation.data; + + // Derive the button's appearance from the test outcome. Colour is never the + // only signal — the label changes too (a11y / colour-blind friendly). + let label = t("Test"); + let color: string | undefined; + let variant = "default"; + let icon = ; + let tooltip: string | undefined; + + if (result?.ok) { + label = t("OK · {{count}}", { count: result.tools.length }); + color = "green"; + variant = "light"; + icon = ; + tooltip = + result.tools.length > 0 + ? result.tools.join(", ") + : t("No tools available"); + } else if (result && "error" in result) { + label = t("Failed"); + color = "red"; + variant = "light"; + icon = ; + // The error string is already sanitized server-side (no secrets). + tooltip = result.error; + } + + const testButton = ( + + ); + + return ( + + + + + {server.name} + + + {server.transport.toUpperCase()} + + + + {server.url} + + + + + {/* Show the tooltip (tools list / error) only once there is a result. */} + {tooltip ? ( + + {testButton} + + ) : ( + testButton + )} + + onToggleEnabled(server, event.currentTarget.checked) + } + /> + onEdit(server)} + > + + + onDelete(server)} + > + + + + + ); +} 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 15db8c22..617845ed 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 @@ -1,6 +1,5 @@ import { useState } from "react"; import { - ActionIcon, Badge, Box, Button, @@ -8,12 +7,11 @@ import { Modal, Paper, Stack, - Switch, Text, } from "@mantine/core"; import { useDisclosure } from "@mantine/hooks"; import { modals } from "@mantine/modals"; -import { IconPencil, IconPlus, IconTrash } from "@tabler/icons-react"; +import { IconPlus } from "@tabler/icons-react"; import { useTranslation } from "react-i18next"; import useUserRole from "@/hooks/use-user-role.tsx"; import { @@ -23,6 +21,7 @@ import { } from "@/features/workspace/queries/ai-mcp-server-query.ts"; import { IAiMcpServer } from "@/features/workspace/services/ai-mcp-server-service.ts"; import AiMcpServerForm from "./ai-mcp-server-form.tsx"; +import AiMcpServerRow from "./ai-mcp-server-row.tsx"; /** * Admin section: list / add / edit / delete external MCP servers the agent may @@ -112,55 +111,16 @@ export default function AiMcpServers() { {servers?.map((server) => ( - - - - - {server.name} - - - {server.transport.toUpperCase()} - - - - {server.url} - - - - - - updateMutation.mutate({ - id: server.id, - enabled: event.currentTarget.checked, - }) - } - /> - openEdit(server)} - > - - - confirmDelete(server)} - > - - - - + // Keyed by id (never remounts) so each row keeps its own test state. + + updateMutation.mutate({ id: s.id, enabled }) + } + /> ))} -- 2.49.1 From 34995ca85cc9596132c10c7c355559f490380f9a Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 26 Jun 2026 17:22:31 +0300 Subject: [PATCH 2/2] fix(ai-mcp): address PR #208 review for inline Test button (#170) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CHANGELOG: add an [Unreleased]/Added bullet for the per-row "Test" button (idle Test -> OK · N / Failed, tooltip, isolated per-row state). - ai-mcp-server-row: show a red "Failed" when the request itself rejects (401/403/500/network), reading testMutation.isError — previously only a server-reported {ok:false} was surfaced and a real reject silently reverted to "Test". Tooltip uses error.response?.data?.message or the i18n fallback. - ai-mcp-server-row: clarify the reset-effect comment (hasHeaders is a presence flag, so value-only token rotation is intentionally not reset). - ai-mcp-server-row: drop the redundant disabled={isPending} (Mantine already disables a loading button). - ru-RU: add the "No tools available" translation. - tests: cover the request-reject failure, the empty tool list (OK · 0), and the reset-on-change effect (url / transport / hasHeaders) via rerender. Note: kept the `"error" in result` guard instead of the suggested bare `else if (result)` — optional chaining on `result?.ok` doesn't narrow the discriminated union in the else branch, so the bare form fails tsc. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 7 ++ .../public/locales/ru-RU/translation.json | 3 +- .../components/ai-mcp-server-row.test.tsx | 106 ++++++++++++++++-- .../settings/components/ai-mcp-server-row.tsx | 23 +++- 4 files changed, 123 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c2aa9c9..815b2ce6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Inline "Test" button per external MCP server.** Each server row in admin AI + settings now has its own "Test" button that runs an isolated connection check: + idle `Test` → green `OK · N` (with a tooltip listing the discovered tools, or + "No tools available") on success, or red `Failed` (tooltip with the sanitized + error) on a connection problem. State is per-row, so testing one server never + spins or recolours the others. (#170) + - **Persistent AI-chat history as the source of truth + server-side export.** An assistant turn is now persisted to the database step by step: the row is inserted upfront as `streaming` and updated as each agent step finishes, then diff --git a/apps/client/public/locales/ru-RU/translation.json b/apps/client/public/locales/ru-RU/translation.json index e46a0579..41c7f6dc 100644 --- a/apps/client/public/locales/ru-RU/translation.json +++ b/apps/client/public/locales/ru-RU/translation.json @@ -1172,5 +1172,6 @@ "OpenAI (official)": "OpenAI (официальный)", "Test": "Тест", "Failed": "Ошибка", - "OK · {{count}}": "OK · {{count}}" + "OK · {{count}}": "OK · {{count}}", + "No tools available": "Нет доступных инструментов" } diff --git a/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.test.tsx b/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.test.tsx index ecd5d2a6..5c6edd24 100644 --- a/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.test.tsx +++ b/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.test.tsx @@ -40,24 +40,37 @@ const baseServer = (over?: Partial): IAiMcpServer => ({ ...over, }); +function tree(server: IAiMcpServer, testid: string) { + return ( +
+ +
+ ); +} + function renderRow(server: IAiMcpServer, testid: string) { const client = new QueryClient({ defaultOptions: { mutations: { retry: false }, queries: { retry: false } }, }); - return render( + const utils = render( - -
- -
- + {tree(server, testid)} , ); + // A rerender helper that swaps only the server prop (same QueryClient, so the + // row keeps its mutation state and the reset-on-change effect is exercised). + const rerenderWith = (next: IAiMcpServer) => + utils.rerender( + + {tree(next, testid)} + , + ); + return { ...utils, rerenderWith }; } describe("AiMcpServerRow — inline Test button", () => { @@ -95,6 +108,77 @@ describe("AiMcpServerRow — inline Test button", () => { ); }); + it("shows 'Failed' when the request itself rejects (401/403/500/network)", async () => { + // A real reject yields no { ok:false } payload — the row must read isError, + // not just mutation.data, or it would spin then silently revert to "Test". + testAiMcpServer.mockRejectedValue(new Error("Request failed")); + renderRow(baseServer(), "row"); + const row = screen.getByTestId("row"); + + fireEvent.click(within(row).getByRole("button", { name: "Test" })); + + await waitFor(() => + expect(within(row).getByRole("button", { name: "Failed" })).toBeDefined(), + ); + }); + + it("shows 'OK · 0' and a 'No tools available' tooltip for an empty tool list", async () => { + testAiMcpServer.mockResolvedValue({ ok: true, tools: [] }); + renderRow(baseServer(), "row"); + const row = screen.getByTestId("row"); + + fireEvent.click(within(row).getByRole("button", { name: "Test" })); + + await waitFor(() => + expect(within(row).getByRole("button", { name: /OK · 0/ })).toBeDefined(), + ); + }); + + it("resets a stale result when url / transport / hasHeaders change", async () => { + testAiMcpServer.mockResolvedValue({ ok: true, tools: ["a", "b", "c"] }); + const { rerenderWith } = renderRow(baseServer(), "row"); + const row = () => screen.getByTestId("row"); + + fireEvent.click(within(row()).getByRole("button", { name: "Test" })); + await waitFor(() => + expect(within(row()).getByRole("button", { name: /OK · 3/ })).toBeDefined(), + ); + + // Changing the URL must drop the stale green result back to idle "Test". + rerenderWith(baseServer({ url: "https://changed.example.com/mcp" })); + await waitFor(() => + expect(within(row()).getByRole("button", { name: "Test" })).toBeDefined(), + ); + + // Same for the transport. + fireEvent.click(within(row()).getByRole("button", { name: "Test" })); + await waitFor(() => + expect(within(row()).getByRole("button", { name: /OK · 3/ })).toBeDefined(), + ); + rerenderWith( + baseServer({ url: "https://changed.example.com/mcp", transport: "sse" }), + ); + await waitFor(() => + expect(within(row()).getByRole("button", { name: "Test" })).toBeDefined(), + ); + + // And for the presence of auth headers. + fireEvent.click(within(row()).getByRole("button", { name: "Test" })); + await waitFor(() => + expect(within(row()).getByRole("button", { name: /OK · 3/ })).toBeDefined(), + ); + rerenderWith( + baseServer({ + url: "https://changed.example.com/mcp", + transport: "sse", + hasHeaders: true, + }), + ); + await waitFor(() => + expect(within(row()).getByRole("button", { name: "Test" })).toBeDefined(), + ); + }); + it("keeps each row's result isolated (testing one does not affect another)", async () => { // Resolve based on id so the two rows get different outcomes. testAiMcpServer.mockImplementation(async (id: string) => diff --git a/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.tsx b/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.tsx index 04c305da..cb96fd42 100644 --- a/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.tsx +++ b/apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.tsx @@ -35,7 +35,10 @@ export default function AiMcpServerRow({ // The result colour/label reflects the connection params at the time of the // test. The row is keyed by id and never remounts, so a stale "OK"/"Failed" - // would otherwise stick after the URL/transport/auth changes. Reset on those. + // would otherwise stick after the connection params change. Reset on those. + // Note: `hasHeaders` is a presence flag only (header values are write-only and + // never returned), so this resets on adding/removing auth headers, NOT on + // rotating a token's value — that value-only change is invisible to the client. useEffect(() => { testMutation.reset(); // eslint-disable-next-line react-hooks/exhaustive-deps @@ -61,12 +64,25 @@ export default function AiMcpServerRow({ ? result.tools.join(", ") : t("No tools available"); } else if (result && "error" in result) { + // Server-reported failure ({ ok: false, error }, HTTP 200). The error string + // is already sanitized server-side (no secrets). The `"error" in result` + // guard is required: `result?.ok` optional-chaining doesn't narrow the union + // in the else branch, so a bare `else if (result)` fails to type-check. label = t("Failed"); color = "red"; variant = "light"; icon = ; - // The error string is already sanitized server-side (no secrets). tooltip = result.error; + } else if (testMutation.isError) { + // The request itself rejected (401/403/500/network) — there is no result + // payload, so without this the row would silently revert to "Test". + label = t("Failed"); + color = "red"; + variant = "light"; + icon = ; + tooltip = + testMutation.error?.["response"]?.data?.message ?? + t("Failed to update data"); } const testButton = ( @@ -78,9 +94,8 @@ export default function AiMcpServerRow({ // (Test -> OK · 5 -> Failed). miw={88} leftSection={icon} + // Mantine disables the button automatically while loading. loading={testMutation.isPending} - // Only blocked while in flight — testing a disabled server is useful too. - disabled={testMutation.isPending} onClick={() => testMutation.mutate(server.id)} > {label} -- 2.49.1