diff --git a/clients/web/src/hooks/useExportActions.test.tsx b/clients/web/src/hooks/useExportActions.test.tsx index 3edaaf16..db0bffd8 100644 --- a/clients/web/src/hooks/useExportActions.test.tsx +++ b/clients/web/src/hooks/useExportActions.test.tsx @@ -32,21 +32,21 @@ const { replayProtocolRequest } = vi.hoisted(() => ({ })); vi.mock("../lib/protocolReplay", () => ({ replayProtocolRequest })); -const request = (id: string, method: string): MessageEntry => - ({ - id, - direction: "request", - timestamp: 0, - message: { jsonrpc: "2.0", id: 1, method, params: { a: 1 } }, - }) as unknown as MessageEntry; +const AT = new Date(0); -const logNotification = (id: string): MessageEntry => - ({ - id, - direction: "notification", - timestamp: 0, - message: { jsonrpc: "2.0", method: "notifications/message" }, - }) as unknown as MessageEntry; +const request = (id: string, method: string): MessageEntry => ({ + id, + direction: "request", + timestamp: AT, + message: { jsonrpc: "2.0", id: 1, method, params: { a: 1 } }, +}); + +const logNotification = (id: string): MessageEntry => ({ + id, + direction: "notification", + timestamp: AT, + message: { jsonrpc: "2.0", method: "notifications/message" }, +}); function harness(overrides: Partial = {}) { const messageLogState = { clearMessages: vi.fn() }; @@ -55,12 +55,11 @@ function harness(overrides: Partial = {}) { const setPinnedProtocolIds = vi.fn(); const params: UseExportActionsParams = { activeServerId: "srv", - messageLogState: - messageLogState as unknown as UseExportActionsParams["messageLogState"], - fetchRequestLogState: - fetchRequestLogState as unknown as UseExportActionsParams["fetchRequestLogState"], - stderrLogState: - stderrLogState as unknown as UseExportActionsParams["stderrLogState"], + // Structurally checked against the hook's narrowed store parameters — each + // names only the clearing method these handlers call. + messageLogState, + fetchRequestLogState, + stderrLogState, messages: [], fetchRequests: [], logs: [], @@ -281,9 +280,9 @@ describe("useExportActions", () => { { id: "r1", direction: "response", - timestamp: 0, + timestamp: AT, message: { jsonrpc: "2.0", id: 1, result: {} }, - } as unknown as MessageEntry, + }, ], }); result.run((api) => api.onReplayProtocol("r1")); @@ -298,9 +297,9 @@ describe("useExportActions", () => { { id: "r1", direction: "request", - timestamp: 0, + timestamp: AT, message: { jsonrpc: "2.0", id: 1, method: "ping" }, - } as unknown as MessageEntry, + }, ], }); h.run((api) => api.onReplayProtocol("r1")); diff --git a/clients/web/src/hooks/useExportActions.ts b/clients/web/src/hooks/useExportActions.ts index 2dd4fce4..68f0f10f 100644 --- a/clients/web/src/hooks/useExportActions.ts +++ b/clients/web/src/hooks/useExportActions.ts @@ -20,9 +20,14 @@ export type ProtocolSection = "pinned" | "history"; export interface UseExportActionsParams { /** The active server's id, stamped into every export filename. */ activeServerId?: string; - messageLogState: MessageLogState | null; - fetchRequestLogState: FetchRequestLogState | null; - stderrLogState: StderrLogState | null; + /** + * Only the clearing method of each store is named, rather than the whole + * class: that is all these handlers touch, and a narrower parameter lets a + * test supply a structurally-checked double instead of a cast-away stand-in. + */ + messageLogState: Pick | null; + fetchRequestLogState: Pick | null; + stderrLogState: Pick | null; /** The Protocol view's entries (the message log). */ messages: MessageEntry[]; /** The Network view's entries. */ diff --git a/clients/web/src/hooks/useInspectorStores.test.tsx b/clients/web/src/hooks/useInspectorStores.test.tsx index 71e2d110..6e7c2955 100644 --- a/clients/web/src/hooks/useInspectorStores.test.tsx +++ b/clients/web/src/hooks/useInspectorStores.test.tsx @@ -127,7 +127,16 @@ vi.mock("@inspector/core/react/useStderrLog.js", () => ({ useStderrLog: () => ({ stderrLogs: [{ id: "s1" }] }), })); -const client = () => ({ id: "client" }) as unknown as InspectorClient; +/** + * A per-call identity token standing in for the client. Every store + * constructor is mocked away below, so nothing here ever reads a property off + * it — the assertions only check *which* value each constructor received, and + * that a second `createStores` passes a different one. The double cast is what + * that isolation buys: building a real `InspectorClient` would drag in a + * transport and an environment to assert on object identity. + */ +const client = () => + ({ marker: Symbol("client") }) as unknown as InspectorClient; const fetchLogOptions = { maxFetchRequests: 42, diff --git a/clients/web/src/hooks/useProgressToasts.test.tsx b/clients/web/src/hooks/useProgressToasts.test.tsx index 78c17737..57bdcd22 100644 --- a/clients/web/src/hooks/useProgressToasts.test.tsx +++ b/clients/web/src/hooks/useProgressToasts.test.tsx @@ -1,6 +1,6 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { act } from "react"; -import type { InspectorClient } from "@inspector/core/mcp/index.js"; +import { InspectorClientEventTarget } from "@inspector/core/mcp/inspectorClientEventTarget.js"; import { renderWithMantine } from "../test/renderWithMantine"; import { progressToastId } from "../utils/toasts/progressToasts"; import { useProgressToasts } from "./useProgressToasts"; @@ -20,16 +20,16 @@ vi.mock("@mantine/notifications", () => ({ })); /** - * The hook only ever calls `add/removeEventListener`, so a bare `EventTarget` - * is a faithful stand-in for the client — the events are real DOM events and - * the listener wiring under test is the real wiring. + * The client's own typed event target, which is exactly the surface the hook + * declares — so this is the real wiring under test, not a stand-in: the events + * dispatched below are the same typed events the client emits. */ -function fakeClient(): InspectorClient & EventTarget { - return new EventTarget() as unknown as InspectorClient & EventTarget; +function fakeClient(): InspectorClientEventTarget { + return new InspectorClientEventTarget(); } -function harness(client: InspectorClient & EventTarget) { - function Probe({ c }: { c: InspectorClient | null }) { +function harness(client: InspectorClientEventTarget) { + function Probe({ c }: { c: InspectorClientEventTarget | null }) { useProgressToasts(c); return null; } @@ -37,13 +37,13 @@ function harness(client: InspectorClient & EventTarget) { return { progress: (progressToken: string, progress: number, total?: number) => act(() => { - client.dispatchEvent( - new CustomEvent("progressNotification", { - detail: { progressToken, progress, total }, - }), - ); + client.dispatchTypedEvent("progressNotification", { + progressToken, + progress, + total, + }); }), - swapClient: (next: InspectorClient | null) => + swapClient: (next: InspectorClientEventTarget | null) => act(() => rerender()), unmount: () => act(() => unmount()), }; @@ -125,11 +125,10 @@ describe("useProgressToasts", () => { h.swapClient(fakeClient()); notificationsMock.show.mockClear(); act(() => { - first.dispatchEvent( - new CustomEvent("progressNotification", { - detail: { progressToken: "tok-1", progress: 1 }, - }), - ); + first.dispatchTypedEvent("progressNotification", { + progressToken: "tok-1", + progress: 1, + }); }); expect(notificationsMock.show).not.toHaveBeenCalled(); }); diff --git a/clients/web/src/hooks/useProgressToasts.ts b/clients/web/src/hooks/useProgressToasts.ts index da037bef..61258c1e 100644 --- a/clients/web/src/hooks/useProgressToasts.ts +++ b/clients/web/src/hooks/useProgressToasts.ts @@ -1,9 +1,7 @@ import { useEffect, useRef } from "react"; import { notifications } from "@mantine/notifications"; -import type { - InspectorClient, - InspectorClientEventMap, -} from "@inspector/core/mcp/index.js"; +import type { InspectorClientEventMap } from "@inspector/core/mcp/index.js"; +import type { InspectorClientEventTarget } from "@inspector/core/mcp/inspectorClientEventTarget.js"; import type { TypedEventGeneric } from "@inspector/core/mcp/typedEventTarget.js"; import { formatProgressToastMessage, @@ -11,6 +9,20 @@ import { progressToastId, } from "../utils/toasts/progressToasts"; +/** + * The only part of the client the toast hooks touch: its typed event-listener + * surface. Naming that rather than the whole `InspectorClient` states the real + * dependency, and lets a test drive them with a plain + * `InspectorClientEventTarget` instead of a stand-in cast to a full client. + * + * Declared here (rather than in a module of its own) because it has no runtime + * half, and `useTaskToasts` — its only other consumer — is a sibling. + */ +export type InspectorClientEventSource = Pick< + InspectorClientEventTarget, + "addEventListener" | "removeEventListener" +>; + /** * Surface incoming `notifications/progress` as toasts so the user can watch a * long-running tool's progress while staying on the tool view — the v2 @@ -21,7 +33,9 @@ import { * (see `progressToastId`) and replaced per tick, so a chatty server updates one * toast rather than stacking one per tick. */ -export function useProgressToasts(inspectorClient: InspectorClient | null) { +export function useProgressToasts( + inspectorClient: InspectorClientEventSource | null, +) { // Which progress streams currently have a live toast. Entries are removed // when their toast closes (auto-dismiss or user). A ref (not state) because // it is incidental bookkeeping that must not trigger re-renders. diff --git a/clients/web/src/hooks/useTabUiState.test.tsx b/clients/web/src/hooks/useTabUiState.test.tsx index af064e5e..c02633c5 100644 --- a/clients/web/src/hooks/useTabUiState.test.tsx +++ b/clients/web/src/hooks/useTabUiState.test.tsx @@ -87,14 +87,22 @@ describe("useTabUiState", () => { expect(h.api().activeTab).toBe("Tools"); }); - it("keeps resetTabUiState stable so a []-dep session reset can call it", () => { + it("keeps its callbacks stable, so naming them in a dependency array costs no churn", () => { const h = harness(); - const before = h.api().resetTabUiState; + // Captured BEFORE the update: comparing the post-render values to each + // other would hold however the identities changed. + const resetBefore = h.api().resetTabUiState; + const toggleBefore = h.api().togglePinProtocol; + const setPinsBefore = h.api().setPinnedProtocolIds; + const setTabBefore = h.api().setActiveTab; + h.run((api) => api.setActiveTab("Logs")); - expect(h.api().resetTabUiState).toBe(before); - expect(h.api().togglePinProtocol).toBe( - // togglePin is likewise stable — it is passed straight into the JSX. - h.api().togglePinProtocol, - ); + h.run((api) => api.togglePinProtocol("m1")); + + expect(h.renders()).toBeGreaterThan(1); + expect(h.api().resetTabUiState).toBe(resetBefore); + expect(h.api().togglePinProtocol).toBe(toggleBefore); + expect(h.api().setPinnedProtocolIds).toBe(setPinsBefore); + expect(h.api().setActiveTab).toBe(setTabBefore); }); }); diff --git a/clients/web/src/hooks/useTabUiState.ts b/clients/web/src/hooks/useTabUiState.ts index 3e2ae702..88bd5cdd 100644 --- a/clients/web/src/hooks/useTabUiState.ts +++ b/clients/web/src/hooks/useTabUiState.ts @@ -44,9 +44,12 @@ export interface TabUiStateResult { togglePinProtocol: (id: string) => void; /** * Reset every screen's UI object and the pin set to their empty defaults. - * Stable, so the session-reset callback that drives it needs no dependency - * on this hook. Deliberately leaves `activeTab` alone: the tab the user is - * on is shell state, reset separately on an *explicit* disconnect. + * Its identity never changes, so naming it in the dependency array of the + * session-reset callback that drives it — which `exhaustive-deps` requires, + * since the rule cannot infer stability across a custom-hook boundary — + * costs that callback no churn. Deliberately leaves `activeTab` alone: the + * tab the user is on is shell state, reset separately on an *explicit* + * disconnect. */ resetTabUiState: () => void; } diff --git a/clients/web/src/hooks/useTaskToasts.test.tsx b/clients/web/src/hooks/useTaskToasts.test.tsx index a645dd35..581558a1 100644 --- a/clients/web/src/hooks/useTaskToasts.test.tsx +++ b/clients/web/src/hooks/useTaskToasts.test.tsx @@ -1,7 +1,7 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { act } from "react"; import type { Task } from "@modelcontextprotocol/client"; -import type { InspectorClient } from "@inspector/core/mcp/index.js"; +import { InspectorClientEventTarget } from "@inspector/core/mcp/inspectorClientEventTarget.js"; import { renderWithMantine } from "../test/renderWithMantine"; import { taskToastId } from "../utils/toasts/taskToasts"; import { useTaskToasts, type TaskToasts } from "./useTaskToasts"; @@ -18,46 +18,72 @@ vi.mock("@mantine/notifications", () => ({ notifications: notificationsMock, })); -/** The hook only calls `add/removeEventListener`, so a bare EventTarget does. */ -function fakeClient(): InspectorClient & EventTarget { - return new EventTarget() as unknown as InspectorClient & EventTarget; +/** + * The client's own typed event target — exactly the surface the hook declares, + * so the events dispatched below are the same typed events the client emits. + */ +function fakeClient(): InspectorClientEventTarget { + return new InspectorClientEventTarget(); } -const task = (status: Task["status"]): Pick => ({ status }); +/** A complete `Task`; only `status` varies across these tests. */ +const task = (taskId: string, status: Task["status"]): Task => ({ + taskId, + status, + ttl: null, + createdAt: "2026-01-01T00:00:00.000Z", + lastUpdatedAt: "2026-01-01T00:00:00.000Z", +}); -function harness(client: InspectorClient & EventTarget) { +function harness(client: InspectorClientEventTarget) { let latest: TaskToasts | undefined; - function Probe({ c }: { c: InspectorClient | null }) { + function Probe({ c }: { c: InspectorClientEventTarget | null }) { latest = useTaskToasts(c); return null; } const { rerender, unmount } = renderWithMantine(); - const fire = (type: string, detail: unknown) => - act(() => { - client.dispatchEvent(new CustomEvent(type, { detail })); - }); + const dispatch = (fn: () => void) => act(fn); return { api: () => { if (!latest) throw new Error("hook did not render"); return latest; }, taskProgress: (taskId: string, progress: number, total?: number) => - fire("requestorTaskProgress", { - taskId, - progress: { progress, total }, - }), + dispatch(() => + client.dispatchTypedEvent("requestorTaskProgress", { + taskId, + progress: { progress, total }, + }), + ), statusChange: (taskId: string, status: Task["status"]) => - fire("taskStatusChange", { taskId, task: task(status) }), + dispatch(() => + client.dispatchTypedEvent("taskStatusChange", { + taskId, + task: task(taskId, status), + }), + ), requestorUpdate: (taskId: string, status: Task["status"]) => - fire("requestorTaskUpdated", { taskId, task: task(status) }), - cancelled: (taskId: string) => fire("taskCancelled", { taskId }), - toolCallTask: (taskId: string) => fire("toolCallTaskUpdated", { taskId }), + dispatch(() => + client.dispatchTypedEvent("requestorTaskUpdated", { + taskId, + task: task(taskId, status), + }), + ), + cancelled: (taskId: string) => + dispatch(() => client.dispatchTypedEvent("taskCancelled", { taskId })), + toolCallTask: (taskId: string) => + dispatch(() => + client.dispatchTypedEvent("toolCallTaskUpdated", { + taskId, + task: task(taskId, "working"), + }), + ), run: (fn: (api: TaskToasts) => void) => act(() => { if (!latest) throw new Error("hook did not render"); fn(latest); }), - swapClient: (next: InspectorClient | null) => + swapClient: (next: InspectorClientEventTarget | null) => act(() => rerender()), unmount: () => act(() => unmount()), }; @@ -213,9 +239,10 @@ describe("useTaskToasts", () => { const h = harness(first); h.swapClient(null); act(() => { - first.dispatchEvent( - new CustomEvent("toolCallTaskUpdated", { detail: { taskId: "t9" } }), - ); + first.dispatchTypedEvent("toolCallTaskUpdated", { + taskId: "t9", + task: task("t9", "working"), + }); }); expect(h.api().activeToolCallTaskIdRef.current).toBeUndefined(); }); @@ -235,11 +262,10 @@ describe("useTaskToasts", () => { h.swapClient(fakeClient()); notificationsMock.show.mockClear(); act(() => { - first.dispatchEvent( - new CustomEvent("taskStatusChange", { - detail: { taskId: "t1", task: task("working") }, - }), - ); + first.dispatchTypedEvent("taskStatusChange", { + taskId: "t1", + task: task("t1", "working"), + }); }); expect(notificationsMock.show).not.toHaveBeenCalled(); }); @@ -256,11 +282,10 @@ describe("useTaskToasts", () => { const h = harness(first); h.swapClient(null); act(() => { - first.dispatchEvent( - new CustomEvent("requestorTaskProgress", { - detail: { taskId: "t1", progress: { progress: 1 } }, - }), - ); + first.dispatchTypedEvent("requestorTaskProgress", { + taskId: "t1", + progress: { progress: 1 }, + }); }); expect(h.api().progressByTaskId).toEqual({}); }); diff --git a/clients/web/src/hooks/useTaskToasts.ts b/clients/web/src/hooks/useTaskToasts.ts index 3c00eff5..a48df8b7 100644 --- a/clients/web/src/hooks/useTaskToasts.ts +++ b/clients/web/src/hooks/useTaskToasts.ts @@ -1,10 +1,8 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { notifications } from "@mantine/notifications"; -import type { - InspectorClient, - InspectorClientEventMap, -} from "@inspector/core/mcp/index.js"; +import type { InspectorClientEventMap } from "@inspector/core/mcp/index.js"; import type { TypedEventGeneric } from "@inspector/core/mcp/typedEventTarget.js"; +import type { InspectorClientEventSource } from "./useProgressToasts"; import type { TaskProgress } from "../components/groups/TaskCard/TaskCard"; import { formatTaskToastMessage, @@ -47,7 +45,7 @@ export interface TaskToasts { * status history still lives in the Protocol view. */ export function useTaskToasts( - inspectorClient: InspectorClient | null, + inspectorClient: InspectorClientEventSource | null, ): TaskToasts { // One live task-status toast per taskId, so each `notifications/tasks/status` // tick replaces the existing toast rather than stacking a fresh one.