Compare commits

..

2 Commits

Author SHA1 Message Date
claude code agent 227
34995ca85c fix(ai-mcp): address PR #208 review for inline Test button (#170)
- 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) <noreply@anthropic.com>
2026-06-26 17:22:31 +03:00
claude code agent 227
c28d8cc648 feat(ai-chat): inline Test button per external MCP server row (#170)
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) <noreply@anthropic.com>
2026-06-26 05:57:20 +03:00
11 changed files with 409 additions and 313 deletions

View File

@@ -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
@@ -92,16 +99,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
no longer froze on the previous step's authoritative usage; the current step's
estimate is combined per-component with `max`, so the count rises smoothly and
never jumps backwards. (#163)
- **AI chat: "New chat" pressed during the first turn's stream now resets the
thread instead of leaving the old turn streaming.** While a brand-new,
not-yet-adopted chat streamed its first turn, hitting "New chat" left
`activeChatId === null` (a no-op for the atom), so the reconciler never
remounted and the in-flight thread kept streaming behind the fresh one — and a
late refetch / late `onFinish` from that abandoned thread could yank the user
back into the chat they just left. "New chat" now forces a fresh empty thread
unconditionally and the finished thread's mount key is checked so a late
callback from an abandoned thread no longer adopts or re-arms the fallback.
(#161)
## [0.93.0] - 2026-06-21

View File

@@ -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 \"<server name>_*\".": "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 \"<server name>_*\".",
"Test": "Test",
"Failed": "Failed",
"OK · {{count}}": "OK · {{count}}",
"Available tools": "Available tools",
"No tools available": "No tools available",
"Created successfully": "Created successfully",

View File

@@ -1169,5 +1169,9 @@
"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}}",
"No tools available": "Нет доступных инструментов"
}

View File

@@ -195,7 +195,6 @@ export default function AiChatWindow() {
waitingForHistory,
onTurnFinished,
onServerChatId,
startFreshThread,
cancelPendingAdoption,
} = useChatSession({
activeChatId,
@@ -210,26 +209,18 @@ export default function AiChatWindow() {
// startNewChat/selectChat set the public atom; the hook's render-phase
// reconciler handles the remount when activeChatId actually CHANGES. But
// pressing "New chat" while already in a not-yet-adopted new chat leaves
// activeChatId === null (a no-op for the atom), so the reconciler never fires —
// startFreshThread forces the remount unconditionally (and disarms any armed
// error-path fallback) so a late refetch can't yank the user into a just-failed
// chat after they chose a fresh one (#161).
// pressing "New chat" while already in a new chat leaves activeChatId === null
// (a no-op for the atom), so the reconciler never fires — explicitly disarm any
// armed error-path fallback here so a late refetch can't yank the user into a
// just-failed chat after they chose a fresh one.
const startNewChat = useCallback((): void => {
// Force a fresh thread UNCONDITIONALLY. On a brand-new, not-yet-adopted chat
// (activeChatId === null) whose first turn is still streaming, setActiveChatId
// (null) is a no-op and the render-phase reconciler would not remount — leaving
// the streaming thread in place (#161). startFreshThread guarantees a clean
// remount and already disarms any armed error-path fallback (so a separate
// cancelPendingAdoption() call here is redundant); the abandoned thread's late
// finish is rejected by the threadKey guard in the session hook.
startFreshThread();
cancelPendingAdoption();
setActiveChatId(null);
setHistoryOpen(false);
setDraft("");
// Default the picker back to "Universal assistant" for the fresh chat.
setSelectedRoleId(null);
}, [startFreshThread, setActiveChatId, setDraft, setSelectedRoleId]);
}, [cancelPendingAdoption, setActiveChatId, setDraft, setSelectedRoleId]);
const selectChat = useCallback(
(chatId: string): void => {
@@ -642,7 +633,6 @@ export default function AiChatWindow() {
onRolePicked={(role) => setSelectedRoleId(role.id)}
assistantName={currentRole?.name}
onTurnFinished={onTurnFinished}
threadKey={threadKey}
onServerChatId={onServerChatId}
onLiveTurnTokens={setLiveTurnTokens}
/>

View File

@@ -1,94 +0,0 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { render, act } from "@testing-library/react";
import { MantineProvider } from "@mantine/core";
// Capture the options ChatThread passes to useChat so the test can drive the
// hook's terminal callbacks (here: onError) directly, without a real stream. The
// box is created via vi.hoisted so the hoisted vi.mock factory below can close
// over it.
const { useChatBox } = vi.hoisted(() => ({
useChatBox: { options: null as unknown as Record<string, unknown> | null },
}));
// Mock the AI SDK hook: record the options and return an inert, ready store so
// ChatThread renders without any network/streaming machinery.
vi.mock("@ai-sdk/react", () => ({
useChat: (options: Record<string, unknown>) => {
useChatBox.options = options;
return {
messages: [],
sendMessage: vi.fn(),
status: "ready",
stop: vi.fn(),
error: null,
};
},
}));
// Stub react-i18next so `t` returns the key (other component tests use the same
// pattern); ChatThread's rendered chrome is irrelevant to this wiring test.
vi.mock("react-i18next", () => ({
useTranslation: () => ({ t: (key: string) => key }),
}));
// Mock the heavy presentational children to trivial stubs — this test only
// exercises the onError → onTurnFinished wiring, not their rendering.
vi.mock("@/features/ai-chat/components/message-list.tsx", () => ({
default: () => null,
}));
vi.mock("@/features/ai-chat/components/chat-input.tsx", () => ({
default: () => null,
}));
vi.mock("@/features/ai-chat/components/role-cards.tsx", () => ({
default: () => null,
}));
vi.mock("@/features/ai-chat/components/chat-error-alert.tsx", () => ({
default: () => null,
}));
vi.mock("@/features/ai-chat/components/chat-stopped-notice.tsx", () => ({
default: () => null,
}));
import ChatThread from "./chat-thread";
// matchMedia (read by MantineProvider) is stubbed globally in vitest.setup.ts.
describe("ChatThread onError wiring (#161)", () => {
beforeEach(() => {
useChatBox.options = null;
vi.clearAllMocks();
});
it("onError calls onTurnFinished with (undefined, threadKey) so a late error from an abandoned thread is rejected", () => {
const onTurnFinished = vi.fn();
// Silence the deliberate console.error ChatThread logs for devtools.
const consoleError = vi
.spyOn(console, "error")
.mockImplementation(() => {});
render(
<MantineProvider>
<ChatThread
chatId="c1"
onTurnFinished={onTurnFinished}
threadKey="thread-key-1"
/>
</MantineProvider>,
);
const options = useChatBox.options;
expect(options).not.toBeNull();
expect(typeof options?.onError).toBe("function");
// Drive the captured onError exactly as the AI SDK would on a stream error.
act(() => {
(options!.onError as (e: Error) => void)(new Error("stream blew up"));
});
// The thread's own mount key must be forwarded with NO server id, so the
// session hook can reject this finish if the thread has been abandoned.
expect(onTurnFinished).toHaveBeenCalledWith(undefined, "thread-key-1");
consoleError.mockRestore();
});
});

View File

@@ -60,12 +60,7 @@ interface ChatThreadProps {
* new chat, adopts the freshly created chat id. `serverChatId` is the
* authoritative id the server streamed on the assistant message metadata, or
* undefined on a failed turn — see adopt-chat-id.ts for the full #137 design. */
onTurnFinished: (serverChatId?: string, finishingThreadKey?: string) => void;
/** This thread's mount key (the same value the parent uses as React `key`).
* Forwarded back through onTurnFinished so the session hook can tell a finish
* from THIS still-mounted thread from a late finish of an abandoned thread the
* user already left via New chat / switch (#161). */
threadKey?: string;
onTurnFinished: (serverChatId?: string) => void;
/** Called EARLY (at the stream's `start` chunk) with the authoritative server
* chat id streamed on the assistant message metadata, so a brand-new chat
* adopts its real id WHILE the first turn is still streaming (#174 — makes the
@@ -121,7 +116,6 @@ export default function ChatThread({
onRolePicked,
assistantName,
onTurnFinished,
threadKey,
onServerChatId,
onLiveTurnTokens,
}: ChatThreadProps) {
@@ -264,7 +258,7 @@ export default function ChatThread({
// Forward the authoritative server chatId (streamed on the assistant
// message metadata) so the parent adopts the REAL created chat id for a new
// chat — see adopt-chat-id.ts for the full #137 design.
onTurnFinished(extractServerChatId(message), threadKey);
onTurnFinished(extractServerChatId(message));
// Show a neutral "stopped" marker for an aborted turn; the red error banner
// (via `error`) already covers isError, and a clean finish clears any marker.
if (isError) setStopNotice(null);
@@ -285,7 +279,7 @@ export default function ChatThread({
// Surface the raw failure in the browser console (devtools) for debugging;
// the UI separately shows a friendly classified banner (see errorView).
console.error("AI chat stream error:", streamError);
onTurnFinished(undefined, threadKey);
onTurnFinished();
},
});

View File

@@ -1,5 +1,5 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { renderHook, act } from "@testing-library/react";
import { renderHook } from "@testing-library/react";
import { useChatSession } from "./use-chat-session";
import type { UseChatSessionOptions } from "./use-chat-session";
@@ -120,40 +120,16 @@ describe("useChatSession", () => {
expect(setActiveChatId).not.toHaveBeenCalledWith("new");
});
it("cancelPendingAdoption (selectChat) disarms a late refetch from adopting the just-failed chat", () => {
// cancelPendingAdoption is the explicit disarm the window calls from
// selectChat: switching to a chat whose id == null is a no-op for the atom, so
// the render-phase reconciler never fires and only this call disarms an armed
// error-path fallback. (startNewChat no longer routes through here — it calls
// startFreshThread, covered by the next test — but cancelPendingAdoption still
// backs selectChat, so this guard must hold.)
it("startNewChat while already in a new chat: cancelPendingAdoption stops a late refetch adopting the failed chat", () => {
// The Warning path the render-phase reconciler can't catch: pressing "New
// chat" while already in a new chat keeps activeChatId === null (a no-op for
// the atom), so only the explicit cancelPendingAdoption() disarms.
const { result, rerender, setActiveChatId } = setup({
activeChatId: null,
chats: { items: [{ id: "x" }] },
});
result.current.onTurnFinished(undefined); // first turn failed → arm (before=["x"])
result.current.cancelPendingAdoption(); // window calls this from selectChat
// The just-failed row lands in a late refetch; it must NOT be adopted.
rerender({
activeChatId: null,
chats: { items: [{ id: "x" }, { id: "failed" }] },
});
expect(setActiveChatId).not.toHaveBeenCalledWith("failed");
});
it("#161: startFreshThread disarms the armed error-path fallback (New chat during the first turn)", () => {
// Pressing "New chat" while already in a not-yet-adopted new chat keeps
// activeChatId === null, so the render-phase reconciler never fires. The
// window now calls startFreshThread() (NOT cancelPendingAdoption) to force a
// fresh thread; this test pins the load-bearing fact that startFreshThread
// ALSO nulls pendingNewChatRef, so a late refetch of the just-failed row can't
// yank the user back into the abandoned chat.
const { result, rerender, setActiveChatId } = setup({
activeChatId: null,
chats: { items: [{ id: "x" }] },
});
result.current.onTurnFinished(undefined); // first turn failed → arm (before=["x"])
act(() => result.current.startFreshThread()); // "New chat" → fresh thread + disarm
result.current.cancelPendingAdoption(); // window calls this from startNewChat
// The just-failed row lands in a late refetch; it must NOT be adopted.
rerender({
activeChatId: null,
@@ -251,48 +227,6 @@ describe("useChatSession", () => {
expect(result.current.threadKey).toBe("C");
});
it("#161: startFreshThread remounts even when activeChatId stays null (New chat mid-stream)", () => {
// The bug: pressing New chat while still on a brand-new, not-yet-adopted chat
// leaves activeChatId === null, so the render-phase reconciler never fires.
// startFreshThread must remount UNCONDITIONALLY (a new mount key).
const { result } = setup({ activeChatId: null, chats: { items: [] } });
const keyBefore = result.current.threadKey;
act(() => result.current.startFreshThread());
expect(result.current.threadKey).not.toBe(keyBefore);
});
it("#161: a late finish from an ABANDONED thread does not adopt or invalidate messages", () => {
const {
result,
setActiveChatId,
onInvalidateChatList,
onInvalidateChatMessages,
} = setup({ activeChatId: null, chats: { items: [{ id: "x" }] } });
const abandonedKey = result.current.threadKey;
// User pressed New chat mid-stream → fresh thread (new mount key).
act(() => result.current.startFreshThread());
expect(result.current.threadKey).not.toBe(abandonedKey);
// The left-behind thread's onFinish fires late, carrying ITS (now stale) key
// and the server id of the chat the user just left. It must NOT be adopted.
act(() => result.current.onTurnFinished("A", abandonedKey));
expect(setActiveChatId).not.toHaveBeenCalled();
expect(onInvalidateChatMessages).not.toHaveBeenCalled();
// The abandoned chat should still surface in the history list.
expect(onInvalidateChatList).toHaveBeenCalled();
});
it("#161: a finish from the CURRENT thread (matching key) still adopts", () => {
const { result, setActiveChatId } = setup({
activeChatId: null,
chats: { items: [{ id: "x" }] },
});
// Same thread that is mounted reports its finish with the matching key.
act(() =>
result.current.onTurnFinished("A", result.current.threadKey),
);
expect(setActiveChatId).toHaveBeenCalledWith("A");
});
it("waitingForHistory gates the loader only while opening an unloaded existing chat", () => {
// Open an existing chat whose history is still loading => loader on.
const { result, rerender } = setup({

View File

@@ -32,13 +32,8 @@ export interface UseChatSessionResult {
/** Show the history loader instead of the live thread. */
waitingForHistory: boolean;
/** Call when a turn finishes; `serverChatId` is the authoritative streamed id
* (undefined on a failed turn). `finishingThreadKey` is the mount key of the
* thread that produced this callback — when it no longer matches the mounted
* thread (the user pressed New chat / switched mid-stream), the call is from an
* abandoned thread and must NOT adopt or re-arm the fallback. Omitting it (old
* callers / tests) treats the call as belonging to the current thread. Handles
* new-chat id adoption + invalidations. */
onTurnFinished: (serverChatId?: string, finishingThreadKey?: string) => void;
* (undefined on a failed turn). Handles new-chat id adoption + invalidations. */
onTurnFinished: (serverChatId?: string) => void;
/** Call EARLY (at the stream's `start` chunk) with the authoritative streamed
* chat id so a brand-new chat adopts its real id WHILE its first turn is still
* streaming — making `activeChatId`-gated affordances (e.g. the Copy/export
@@ -46,13 +41,6 @@ export interface UseChatSessionResult {
* no list/messages invalidation — that is left to onTurnFinished at the end).
* Idempotent and a no-op once the chat already has an id. */
onServerChatId: (serverChatId?: string) => void;
/** Force a brand-new, empty thread (new mount key, no chat id) UNCONDITIONALLY.
* The render-phase reconciler only remounts when `activeChatId` actually
* changes; pressing "New chat" while already in a not-yet-adopted new chat
* leaves `activeChatId === null` (a no-op for the atom), so the reconciler
* never fires and the stale streaming thread stays mounted (#161). The window
* calls this from startNewChat to guarantee a fresh thread regardless. */
startFreshThread: () => void;
/** Disarm any pending error-path new-chat fallback. The window calls this from
* startNewChat/selectChat so a late refetch can't yank the user back into a
* just-failed chat after they explicitly moved on. */
@@ -97,14 +85,6 @@ export function useChatSession(
const activeChatIdRef = useRef(activeChatId);
activeChatIdRef.current = activeChatId;
// Live mirror of the mounted thread's key, read by onTurnFinished to tell a
// current-thread finish from an ABANDONED one. ai@6's useChat does not abort
// its request on unmount, and its callbacks are proxied so onFinish/onError of
// a thread the user already left (via New chat / switch) still fire AFTER that
// thread unmounts. By then this ref holds the NEW thread's key, so comparing it
// to the key the finishing thread reports rejects the abandoned turn (#161).
const threadKeyRef = useRef<string>("");
// The mounted thread's identity: ONE atomic value tying ChatThread's mount key
// (`thread.key`) to the chat id that mounted thread holds (`thread.chatId`).
// Consolidating these makes the "key vs chat id diverged" state unrepresentable
@@ -118,10 +98,6 @@ export function useChatSession(
: switchThread(activeChatId),
);
// Keep the live mirror pointed at the currently-mounted thread's key so a late
// onTurnFinished can be matched against it (see threadKeyRef above).
threadKeyRef.current = thread.key;
// Error-path fallback for new-chat id adoption. When a brand-new chat's first
// turn errors BEFORE the server's `start` chunk, no authoritative chatId ever
// reaches the client, so the primary metadata adoption cannot run. We then ARM
@@ -139,21 +115,7 @@ export function useChatSession(
// yet) we adopt the server's AUTHORITATIVE streamed id (never the newest in the
// list, which races a second tab — #137; see adopt-chat-id.ts).
const onTurnFinished = useCallback(
(serverChatId?: string, finishingThreadKey?: string) => {
// Reject a finish from an ABANDONED thread. After the user pressed New chat
// (or switched chats) mid-stream, the left-behind thread's onFinish/onError
// still fire (ai@6 does not abort on unmount). Adopting/arming off that late
// callback would yank the user back into the chat they just left (#161).
// `undefined` (legacy callers/tests) is treated as the current thread.
const isCurrentThread =
finishingThreadKey === undefined ||
finishingThreadKey === threadKeyRef.current;
if (!isCurrentThread) {
// Still surface the abandoned chat in the history list, but do NOT adopt,
// arm the fallback, or invalidate per-chat messages (no thread shows it).
onInvalidateChatList();
return;
}
(serverChatId?: string) => {
// Read the live id from the ref, not the closure: on a failed turn this can
// run twice in one turn (onFinish + onError) before any re-render, and the
// primary branch below updates the ref so the second call sees the adopted id.
@@ -296,29 +258,11 @@ export function useChatSession(
pendingNewChatRef.current = null;
}, []);
// Force a fresh, empty thread regardless of the current `activeChatId`. The
// render-phase reconciler only remounts on an activeChatId CHANGE, so "New chat"
// pressed while already in a not-yet-adopted new chat (activeChatId stays null)
// would otherwise leave the in-flight streaming thread mounted (#161). Dispatch
// `reconcile` to chatId:null with a brand-new key so React remounts ChatThread
// (a fresh useChat store). Disarm any armed fallback too. After this dispatch
// thread.chatId is null; the window also sets activeChatId to null, so the
// render-phase reconciler then finds them equal and does not double-remount.
const startFreshThread = useCallback(() => {
pendingNewChatRef.current = null;
dispatch({
type: "reconcile",
chatId: null,
newKey: `new-${generateId()}`,
});
}, []);
return {
threadKey: thread.key,
waitingForHistory,
onTurnFinished,
onServerChatId,
startFreshThread,
cancelPendingAdoption,
};
}

View File

@@ -0,0 +1,204 @@
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>): IAiMcpServer => ({
id: "srv-1",
name: "Search",
transport: "http",
url: "https://example.com/mcp",
enabled: true,
toolAllowlist: null,
hasHeaders: false,
instructions: null,
...over,
});
function tree(server: IAiMcpServer, testid: string) {
return (
<div data-testid={testid}>
<AiMcpServerRow
server={server}
onEdit={vi.fn()}
onDelete={vi.fn()}
onToggleEnabled={vi.fn()}
/>
</div>
);
}
function renderRow(server: IAiMcpServer, testid: string) {
const client = new QueryClient({
defaultOptions: { mutations: { retry: false }, queries: { retry: false } },
});
const utils = render(
<QueryClientProvider client={client}>
<MantineProvider>{tree(server, testid)}</MantineProvider>
</QueryClientProvider>,
);
// 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(
<QueryClientProvider client={client}>
<MantineProvider>{tree(next, testid)}</MantineProvider>
</QueryClientProvider>,
);
return { ...utils, rerenderWith };
}
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("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) =>
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();
});
});

View File

@@ -0,0 +1,161 @@
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 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
}, [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 = <IconPlugConnected size={16} />;
let tooltip: string | undefined;
if (result?.ok) {
label = t("OK · {{count}}", { count: result.tools.length });
color = "green";
variant = "light";
icon = <IconCheck size={16} />;
tooltip =
result.tools.length > 0
? 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 = <IconX size={16} />;
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 = <IconX size={16} />;
tooltip =
testMutation.error?.["response"]?.data?.message ??
t("Failed to update data");
}
const testButton = (
<Button
size="xs"
variant={variant}
color={color}
// Fixed min-width so the row does not jump as the label changes
// (Test -> OK · 5 -> Failed).
miw={88}
leftSection={icon}
// Mantine disables the button automatically while loading.
loading={testMutation.isPending}
onClick={() => testMutation.mutate(server.id)}
>
{label}
</Button>
);
return (
<Group justify="space-between" wrap="nowrap">
<Stack gap={2} style={{ minWidth: 0 }}>
<Group gap="xs">
<Text fw={500} truncate>
{server.name}
</Text>
<Badge size="xs" variant="light">
{server.transport.toUpperCase()}
</Badge>
</Group>
<Text
size="xs"
c="dimmed"
truncate
style={{ fontFamily: "ui-monospace, Menlo, monospace" }}
>
{server.url}
</Text>
</Stack>
<Group gap="xs" wrap="nowrap">
{/* Show the tooltip (tools list / error) only once there is a result. */}
{tooltip ? (
<Tooltip label={tooltip} multiline maw={320} withArrow>
{testButton}
</Tooltip>
) : (
testButton
)}
<Switch
size="sm"
checked={server.enabled}
aria-label={t("Enabled")}
onChange={(event) =>
onToggleEnabled(server, event.currentTarget.checked)
}
/>
<ActionIcon
variant="subtle"
aria-label={t("Edit")}
onClick={() => onEdit(server)}
>
<IconPencil size={16} />
</ActionIcon>
<ActionIcon
variant="subtle"
color="red"
aria-label={t("Delete")}
onClick={() => onDelete(server)}
>
<IconTrash size={16} />
</ActionIcon>
</Group>
</Group>
);
}

View File

@@ -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() {
<Stack gap="xs" mt="sm">
{servers?.map((server) => (
<Group key={server.id} justify="space-between" wrap="nowrap">
<Stack gap={2} style={{ minWidth: 0 }}>
<Group gap="xs">
<Text fw={500} truncate>
{server.name}
</Text>
<Badge size="xs" variant="light">
{server.transport.toUpperCase()}
</Badge>
</Group>
<Text
size="xs"
c="dimmed"
truncate
style={{ fontFamily: "ui-monospace, Menlo, monospace" }}
>
{server.url}
</Text>
</Stack>
<Group gap="xs" wrap="nowrap">
<Switch
size="sm"
checked={server.enabled}
aria-label={t("Enabled")}
onChange={(event) =>
updateMutation.mutate({
id: server.id,
enabled: event.currentTarget.checked,
})
}
/>
<ActionIcon
variant="subtle"
aria-label={t("Edit")}
onClick={() => openEdit(server)}
>
<IconPencil size={16} />
</ActionIcon>
<ActionIcon
variant="subtle"
color="red"
aria-label={t("Delete")}
onClick={() => confirmDelete(server)}
>
<IconTrash size={16} />
</ActionIcon>
</Group>
</Group>
// Keyed by id (never remounts) so each row keeps its own test state.
<AiMcpServerRow
key={server.id}
server={server}
onEdit={openEdit}
onDelete={confirmDelete}
onToggleEnabled={(s, enabled) =>
updateMutation.mutate({ id: s.id, enabled })
}
/>
))}
</Stack>