From 552ee741439269cd60e6520c0766be983ef9b69a Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 31 May 2026 17:45:04 -0400 Subject: [PATCH 1/2] fix(app): clear per-call panels and log level on disconnect (#1368) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Session-scoped UI state owned by App.tsx — the Tools / Prompts / Resources result panels (`toolCallState` / `getPromptState` / `readResourceState`) and the optimistic `currentLogLevel` — survived a disconnect/reconnect cycle, so server B's screens showed server A's last result. Extend the existing `disconnect` listener to reset all four back to empty / "info", alongside the activeServerId + modal-flag resets it already does. This covers all three disconnect paths the listener handles (explicit toggle, header Disconnect button, mid-session transport failure). `latencyMs` continues to reset via the connectionStatus effect. Adds App.test.tsx covering the disconnect → tool-call-panel-cleared and log-level-reset transition, mocking the InspectorClient (a fake EventTarget), the per-server state managers, the core hooks, and InspectorView. Co-Authored-By: Claude Opus 4.8 (1M context) --- clients/web/src/App.test.tsx | 232 +++++++++++++++++++++++++++++++++++ clients/web/src/App.tsx | 15 +++ 2 files changed, 247 insertions(+) create mode 100644 clients/web/src/App.test.tsx diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx new file mode 100644 index 000000000..fa89129f6 --- /dev/null +++ b/clients/web/src/App.test.tsx @@ -0,0 +1,232 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { + renderWithMantine, + screen, + waitFor, + act, +} from "./test/renderWithMantine"; +import userEvent from "@testing-library/user-event"; + +// App is a wiring component: it owns session-scoped UI state (the per-call +// result panels and the optimistic log level) and resets it when the active +// InspectorClient emits `disconnect`. These tests exercise that reset in +// isolation by mocking the InspectorClient (a fake EventTarget we can fire +// `disconnect` on), the per-server state managers, the core hooks, and the +// InspectorView (a thin double that surfaces the props under test and lets us +// trigger the handlers). See #1368. + +// --- Fake InspectorClient --------------------------------------------------- +// Extends EventTarget so the App's `addEventListener("disconnect", …)` wiring +// is real; the test fires `dispatchEvent(new Event("disconnect"))` to simulate +// any of the three disconnect paths (toggle, header button, transport failure). +vi.mock("@inspector/core/mcp/index.js", () => { + class FakeInspectorClient extends EventTarget { + connect = vi.fn().mockResolvedValue(undefined); + disconnect = vi.fn().mockResolvedValue(undefined); + callTool = vi + .fn() + .mockResolvedValue({ success: true, result: { acts: [] } }); + setLoggingLevel = vi.fn().mockResolvedValue(undefined); + getOAuthState = vi.fn().mockReturnValue(undefined); + } + const instances: FakeInspectorClient[] = []; + return { + InspectorClient: vi.fn(function () { + const client = new FakeInspectorClient(); + instances.push(client); + return client; + }), + // Test-only handle so the test can grab the live instance and fire events. + __clientInstances: instances, + }; +}); + +// Per-server state managers — App constructs nine of them per connect and +// calls `destroy()` on teardown. Replace each with a no-op constructor. +vi.mock("@inspector/core/mcp/state/managedToolsState.js", () => ({ + ManagedToolsState: vi.fn(function () { + return { destroy: vi.fn() }; + }), +})); +vi.mock("@inspector/core/mcp/state/managedPromptsState.js", () => ({ + ManagedPromptsState: vi.fn(function () { + return { destroy: vi.fn() }; + }), +})); +vi.mock("@inspector/core/mcp/state/managedResourcesState.js", () => ({ + ManagedResourcesState: vi.fn(function () { + return { destroy: vi.fn() }; + }), +})); +vi.mock("@inspector/core/mcp/state/managedResourceTemplatesState.js", () => ({ + ManagedResourceTemplatesState: vi.fn(function () { + return { destroy: vi.fn() }; + }), +})); +vi.mock("@inspector/core/mcp/state/managedRequestorTasksState.js", () => ({ + ManagedRequestorTasksState: vi.fn(function () { + return { destroy: vi.fn() }; + }), +})); +vi.mock("@inspector/core/mcp/state/resourceSubscriptionsState.js", () => ({ + ResourceSubscriptionsState: vi.fn(function () { + return { destroy: vi.fn() }; + }), +})); +vi.mock("@inspector/core/mcp/state/messageLogState.js", () => ({ + MessageLogState: vi.fn(function () { + return { destroy: vi.fn() }; + }), +})); +vi.mock("@inspector/core/mcp/state/fetchRequestLogState.js", () => ({ + FetchRequestLogState: vi.fn(function () { + return { destroy: vi.fn(), getFetchRequests: vi.fn(() => []) }; + }), +})); +vi.mock("@inspector/core/mcp/state/stderrLogState.js", () => ({ + StderrLogState: vi.fn(function () { + return { destroy: vi.fn() }; + }), +})); + +vi.mock("@inspector/core/mcp/remote/index.js", () => ({ + RemoteInspectorClientStorage: vi.fn(function () { + return { saveSession: vi.fn() }; + }), +})); + +vi.mock("./lib/environmentFactory", () => ({ + createWebEnvironment: vi.fn(() => ({ environment: {} })), +})); + +// --- Core hooks ------------------------------------------------------------- +// One server is available; the tools list carries the `get_acts` tool the +// repro runs. Everything else returns empty. +const SERVER_A = { + id: "A", + name: "PlotRocket", + config: { type: "stdio", command: "node" }, + connection: { status: "disconnected" }, +}; + +vi.mock("@inspector/core/react/useServers.js", () => ({ + useServers: vi.fn(() => ({ + servers: [SERVER_A], + addServer: vi.fn(), + updateServer: vi.fn(), + updateServerSettings: vi.fn(), + removeServer: vi.fn(), + })), +})); +vi.mock("@inspector/core/react/useInspectorClient.js", () => ({ + useInspectorClient: vi.fn(() => ({ + status: "connected", + capabilities: {}, + clientCapabilities: {}, + // Left undefined so `initializeResult` stays undefined and the + // ConnectionInfoModal (gated on it) never mounts during the test. + serverInfo: undefined, + instructions: undefined, + })), +})); +vi.mock("@inspector/core/react/useManagedTools.js", () => ({ + useManagedTools: vi.fn(() => ({ + tools: [{ name: "get_acts", inputSchema: { type: "object" } }], + refresh: vi.fn(), + })), +})); +vi.mock("@inspector/core/react/useManagedPrompts.js", () => ({ + useManagedPrompts: vi.fn(() => ({ prompts: [], refresh: vi.fn() })), +})); +vi.mock("@inspector/core/react/useManagedResources.js", () => ({ + useManagedResources: vi.fn(() => ({ resources: [], refresh: vi.fn() })), +})); +vi.mock("@inspector/core/react/useManagedResourceTemplates.js", () => ({ + useManagedResourceTemplates: vi.fn(() => ({ resourceTemplates: [] })), +})); +vi.mock("@inspector/core/react/useManagedRequestorTasks.js", () => ({ + useManagedRequestorTasks: vi.fn(() => ({ tasks: [], refresh: vi.fn() })), +})); +vi.mock("@inspector/core/react/useResourceSubscriptions.js", () => ({ + useResourceSubscriptions: vi.fn(() => ({ subscriptions: [] })), +})); +vi.mock("@inspector/core/react/useMessageLog.js", () => ({ + useMessageLog: vi.fn(() => ({ messages: [] })), +})); +vi.mock("@inspector/core/react/useFetchRequestLog.js", () => ({ + useFetchRequestLog: vi.fn(() => ({ fetchRequests: [] })), +})); +vi.mock("@inspector/core/react/useSettingsDraft.js", () => ({ + useSettingsDraft: vi.fn(() => ({ + draft: undefined, + onChange: vi.fn(), + flush: vi.fn(), + })), +})); + +// --- InspectorView double --------------------------------------------------- +// Surfaces the two pieces of state under test and exposes buttons that invoke +// the App's connect / call-tool / set-log-level handlers. +vi.mock("./components/views/InspectorView/InspectorView", () => ({ + InspectorView: (props: { + toolCallState?: { status?: string }; + currentLogLevel?: string; + onToggleConnection: (id: string) => void; + onCallTool: (name: string, args: Record) => void; + onSetLogLevel: (level: string) => void; + }) => ( +
+ + {props.toolCallState?.status ?? "none"} + + {props.currentLogLevel} + + + +
+ ), +})); + +import App from "./App"; +import * as McpIndex from "@inspector/core/mcp/index.js"; + +const clientInstances = ( + McpIndex as unknown as { __clientInstances: EventTarget[] } +).__clientInstances; + +describe("App session-scoped state reset on disconnect", () => { + beforeEach(() => { + clientInstances.length = 0; + }); + + it("clears the tool-call result panel and resets the log level when the client disconnects", async () => { + const user = userEvent.setup(); + renderWithMantine(); + + // Connect: builds the InspectorClient and registers the disconnect listener. + await user.click(screen.getByText("connect")); + await waitFor(() => expect(clientInstances).toHaveLength(1)); + + // Run a tool — the panel fills with the (resolved) result. + await user.click(screen.getByText("call")); + await waitFor(() => + expect(screen.getByTestId("tool-status")).toHaveTextContent("ok"), + ); + + // Bump the optimistic log level off its "info" default. + await user.click(screen.getByText("set-level")); + await waitFor(() => + expect(screen.getByTestId("log-level")).toHaveTextContent("debug"), + ); + + // Disconnect: the panel empties and the level returns to "info". + act(() => { + clientInstances[0].dispatchEvent(new Event("disconnect")); + }); + + await waitFor(() => + expect(screen.getByTestId("tool-status")).toHaveTextContent("none"), + ); + expect(screen.getByTestId("log-level")).toHaveTextContent("info"); + }); +}); diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 41b88f95e..18160dcd8 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -350,6 +350,15 @@ function App() { // failure / process exit) and avoids the first-render-clobbers-new-id // trap that watching connectionStatus has (status starts as // "disconnected" for the new client before connect() runs). + // + // This is also where session-scoped UI state that lives in App.tsx + // (rather than inside the per-server state managers) gets reset, so the + // next server's screens don't show server A's last result. The per-call + // panels (`toolCallState` / `getPromptState` / `readResourceState`) and + // the optimistic `currentLogLevel` all survive a disconnect/reconnect + // cycle otherwise — see #1368. `latencyMs` resets via the + // `connectionStatus` effect above instead because it has its own + // connecting-edge ref to coordinate with. useEffect(() => { if (!inspectorClient) return; const onDisconnect = () => { @@ -357,6 +366,12 @@ function App() { // Drop the open flag too — without this the modal would pop back the // next time `initializeResult` re-becomes truthy (e.g. reconnect). setConnectionInfoModalOpen(false); + // Clear the per-call result panels so the next session starts empty. + setToolCallState(undefined); + setGetPromptState(undefined); + setReadResourceState(undefined); + // Back to the default level the next session begins at. + setCurrentLogLevel("info"); }; inspectorClient.addEventListener("disconnect", onDisconnect); return () => { From faf5fac3890d46f3018cae34ba376da5463a5bbd Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 31 May 2026 19:07:02 -0400 Subject: [PATCH 2/2] test(app): assert all four panels reset on disconnect (#1368) Address PR review: the disconnect test asserted only toolCallState and currentLogLevel. Extend the InspectorView double with prompt/resource status spans and get-prompt/read-resource buttons so the test fills and then asserts the reset of all four pieces of session-scoped state (tool / prompt / resource panels + log level), fully matching the "reset on disconnect" contract. Retitle the test accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) --- clients/web/src/App.test.tsx | 47 +++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx index fa89129f6..cf1133d47 100644 --- a/clients/web/src/App.test.tsx +++ b/clients/web/src/App.test.tsx @@ -26,6 +26,10 @@ vi.mock("@inspector/core/mcp/index.js", () => { callTool = vi .fn() .mockResolvedValue({ success: true, result: { acts: [] } }); + getPrompt = vi.fn().mockResolvedValue({ result: { messages: [] } }); + readResource = vi + .fn() + .mockResolvedValue({ result: { contents: [] }, timestamp: 1 }); setLoggingLevel = vi.fn().mockResolvedValue(undefined); getOAuthState = vi.fn().mockReturnValue(undefined); } @@ -165,23 +169,38 @@ vi.mock("@inspector/core/react/useSettingsDraft.js", () => ({ })); // --- InspectorView double --------------------------------------------------- -// Surfaces the two pieces of state under test and exposes buttons that invoke -// the App's connect / call-tool / set-log-level handlers. +// Surfaces each piece of session-scoped state under test and exposes buttons +// that invoke the App's connect / call-tool / get-prompt / read-resource / +// set-log-level handlers. vi.mock("./components/views/InspectorView/InspectorView", () => ({ InspectorView: (props: { toolCallState?: { status?: string }; + getPromptState?: { status?: string }; + readResourceState?: { status?: string }; currentLogLevel?: string; onToggleConnection: (id: string) => void; onCallTool: (name: string, args: Record) => void; + onGetPrompt: (name: string, args: Record) => void; + onReadResource: (uri: string) => void; onSetLogLevel: (level: string) => void; }) => (
{props.toolCallState?.status ?? "none"} + + {props.getPromptState?.status ?? "none"} + + + {props.readResourceState?.status ?? "none"} + {props.currentLogLevel} + +
), @@ -199,7 +218,7 @@ describe("App session-scoped state reset on disconnect", () => { clientInstances.length = 0; }); - it("clears the tool-call result panel and resets the log level when the client disconnects", async () => { + it("clears the per-call panels and resets the log level on client disconnect", async () => { const user = userEvent.setup(); renderWithMantine(); @@ -207,11 +226,15 @@ describe("App session-scoped state reset on disconnect", () => { await user.click(screen.getByText("connect")); await waitFor(() => expect(clientInstances).toHaveLength(1)); - // Run a tool — the panel fills with the (resolved) result. + // Fill all three per-call panels with their (resolved) results. await user.click(screen.getByText("call")); - await waitFor(() => - expect(screen.getByTestId("tool-status")).toHaveTextContent("ok"), - ); + await user.click(screen.getByText("get-prompt")); + await user.click(screen.getByText("read-resource")); + await waitFor(() => { + expect(screen.getByTestId("tool-status")).toHaveTextContent("ok"); + expect(screen.getByTestId("prompt-status")).toHaveTextContent("ok"); + expect(screen.getByTestId("resource-status")).toHaveTextContent("ok"); + }); // Bump the optimistic log level off its "info" default. await user.click(screen.getByText("set-level")); @@ -219,14 +242,16 @@ describe("App session-scoped state reset on disconnect", () => { expect(screen.getByTestId("log-level")).toHaveTextContent("debug"), ); - // Disconnect: the panel empties and the level returns to "info". + // Disconnect: every panel empties and the level returns to "info". act(() => { clientInstances[0].dispatchEvent(new Event("disconnect")); }); - await waitFor(() => - expect(screen.getByTestId("tool-status")).toHaveTextContent("none"), - ); + await waitFor(() => { + expect(screen.getByTestId("tool-status")).toHaveTextContent("none"); + expect(screen.getByTestId("prompt-status")).toHaveTextContent("none"); + expect(screen.getByTestId("resource-status")).toHaveTextContent("none"); + }); expect(screen.getByTestId("log-level")).toHaveTextContent("info"); }); });