Skip to content

Commit 905b80a

Browse files
cliffhallclaude
andauthored
fix(app): clear per-call panels and log level on disconnect (#1368) (#1392)
* fix(app): clear per-call panels and log level on disconnect (#1368) 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5c5e5ab commit 905b80a

2 files changed

Lines changed: 272 additions & 0 deletions

File tree

clients/web/src/App.test.tsx

Lines changed: 257 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,257 @@
1+
import { describe, it, expect, beforeEach, vi } from "vitest";
2+
import {
3+
renderWithMantine,
4+
screen,
5+
waitFor,
6+
act,
7+
} from "./test/renderWithMantine";
8+
import userEvent from "@testing-library/user-event";
9+
10+
// App is a wiring component: it owns session-scoped UI state (the per-call
11+
// result panels and the optimistic log level) and resets it when the active
12+
// InspectorClient emits `disconnect`. These tests exercise that reset in
13+
// isolation by mocking the InspectorClient (a fake EventTarget we can fire
14+
// `disconnect` on), the per-server state managers, the core hooks, and the
15+
// InspectorView (a thin double that surfaces the props under test and lets us
16+
// trigger the handlers). See #1368.
17+
18+
// --- Fake InspectorClient ---------------------------------------------------
19+
// Extends EventTarget so the App's `addEventListener("disconnect", …)` wiring
20+
// is real; the test fires `dispatchEvent(new Event("disconnect"))` to simulate
21+
// any of the three disconnect paths (toggle, header button, transport failure).
22+
vi.mock("@inspector/core/mcp/index.js", () => {
23+
class FakeInspectorClient extends EventTarget {
24+
connect = vi.fn().mockResolvedValue(undefined);
25+
disconnect = vi.fn().mockResolvedValue(undefined);
26+
callTool = vi
27+
.fn()
28+
.mockResolvedValue({ success: true, result: { acts: [] } });
29+
getPrompt = vi.fn().mockResolvedValue({ result: { messages: [] } });
30+
readResource = vi
31+
.fn()
32+
.mockResolvedValue({ result: { contents: [] }, timestamp: 1 });
33+
setLoggingLevel = vi.fn().mockResolvedValue(undefined);
34+
getOAuthState = vi.fn().mockReturnValue(undefined);
35+
}
36+
const instances: FakeInspectorClient[] = [];
37+
return {
38+
InspectorClient: vi.fn(function () {
39+
const client = new FakeInspectorClient();
40+
instances.push(client);
41+
return client;
42+
}),
43+
// Test-only handle so the test can grab the live instance and fire events.
44+
__clientInstances: instances,
45+
};
46+
});
47+
48+
// Per-server state managers — App constructs nine of them per connect and
49+
// calls `destroy()` on teardown. Replace each with a no-op constructor.
50+
vi.mock("@inspector/core/mcp/state/managedToolsState.js", () => ({
51+
ManagedToolsState: vi.fn(function () {
52+
return { destroy: vi.fn() };
53+
}),
54+
}));
55+
vi.mock("@inspector/core/mcp/state/managedPromptsState.js", () => ({
56+
ManagedPromptsState: vi.fn(function () {
57+
return { destroy: vi.fn() };
58+
}),
59+
}));
60+
vi.mock("@inspector/core/mcp/state/managedResourcesState.js", () => ({
61+
ManagedResourcesState: vi.fn(function () {
62+
return { destroy: vi.fn() };
63+
}),
64+
}));
65+
vi.mock("@inspector/core/mcp/state/managedResourceTemplatesState.js", () => ({
66+
ManagedResourceTemplatesState: vi.fn(function () {
67+
return { destroy: vi.fn() };
68+
}),
69+
}));
70+
vi.mock("@inspector/core/mcp/state/managedRequestorTasksState.js", () => ({
71+
ManagedRequestorTasksState: vi.fn(function () {
72+
return { destroy: vi.fn() };
73+
}),
74+
}));
75+
vi.mock("@inspector/core/mcp/state/resourceSubscriptionsState.js", () => ({
76+
ResourceSubscriptionsState: vi.fn(function () {
77+
return { destroy: vi.fn() };
78+
}),
79+
}));
80+
vi.mock("@inspector/core/mcp/state/messageLogState.js", () => ({
81+
MessageLogState: vi.fn(function () {
82+
return { destroy: vi.fn() };
83+
}),
84+
}));
85+
vi.mock("@inspector/core/mcp/state/fetchRequestLogState.js", () => ({
86+
FetchRequestLogState: vi.fn(function () {
87+
return { destroy: vi.fn(), getFetchRequests: vi.fn(() => []) };
88+
}),
89+
}));
90+
vi.mock("@inspector/core/mcp/state/stderrLogState.js", () => ({
91+
StderrLogState: vi.fn(function () {
92+
return { destroy: vi.fn() };
93+
}),
94+
}));
95+
96+
vi.mock("@inspector/core/mcp/remote/index.js", () => ({
97+
RemoteInspectorClientStorage: vi.fn(function () {
98+
return { saveSession: vi.fn() };
99+
}),
100+
}));
101+
102+
vi.mock("./lib/environmentFactory", () => ({
103+
createWebEnvironment: vi.fn(() => ({ environment: {} })),
104+
}));
105+
106+
// --- Core hooks -------------------------------------------------------------
107+
// One server is available; the tools list carries the `get_acts` tool the
108+
// repro runs. Everything else returns empty.
109+
const SERVER_A = {
110+
id: "A",
111+
name: "PlotRocket",
112+
config: { type: "stdio", command: "node" },
113+
connection: { status: "disconnected" },
114+
};
115+
116+
vi.mock("@inspector/core/react/useServers.js", () => ({
117+
useServers: vi.fn(() => ({
118+
servers: [SERVER_A],
119+
addServer: vi.fn(),
120+
updateServer: vi.fn(),
121+
updateServerSettings: vi.fn(),
122+
removeServer: vi.fn(),
123+
})),
124+
}));
125+
vi.mock("@inspector/core/react/useInspectorClient.js", () => ({
126+
useInspectorClient: vi.fn(() => ({
127+
status: "connected",
128+
capabilities: {},
129+
clientCapabilities: {},
130+
// Left undefined so `initializeResult` stays undefined and the
131+
// ConnectionInfoModal (gated on it) never mounts during the test.
132+
serverInfo: undefined,
133+
instructions: undefined,
134+
})),
135+
}));
136+
vi.mock("@inspector/core/react/useManagedTools.js", () => ({
137+
useManagedTools: vi.fn(() => ({
138+
tools: [{ name: "get_acts", inputSchema: { type: "object" } }],
139+
refresh: vi.fn(),
140+
})),
141+
}));
142+
vi.mock("@inspector/core/react/useManagedPrompts.js", () => ({
143+
useManagedPrompts: vi.fn(() => ({ prompts: [], refresh: vi.fn() })),
144+
}));
145+
vi.mock("@inspector/core/react/useManagedResources.js", () => ({
146+
useManagedResources: vi.fn(() => ({ resources: [], refresh: vi.fn() })),
147+
}));
148+
vi.mock("@inspector/core/react/useManagedResourceTemplates.js", () => ({
149+
useManagedResourceTemplates: vi.fn(() => ({ resourceTemplates: [] })),
150+
}));
151+
vi.mock("@inspector/core/react/useManagedRequestorTasks.js", () => ({
152+
useManagedRequestorTasks: vi.fn(() => ({ tasks: [], refresh: vi.fn() })),
153+
}));
154+
vi.mock("@inspector/core/react/useResourceSubscriptions.js", () => ({
155+
useResourceSubscriptions: vi.fn(() => ({ subscriptions: [] })),
156+
}));
157+
vi.mock("@inspector/core/react/useMessageLog.js", () => ({
158+
useMessageLog: vi.fn(() => ({ messages: [] })),
159+
}));
160+
vi.mock("@inspector/core/react/useFetchRequestLog.js", () => ({
161+
useFetchRequestLog: vi.fn(() => ({ fetchRequests: [] })),
162+
}));
163+
vi.mock("@inspector/core/react/useSettingsDraft.js", () => ({
164+
useSettingsDraft: vi.fn(() => ({
165+
draft: undefined,
166+
onChange: vi.fn(),
167+
flush: vi.fn(),
168+
})),
169+
}));
170+
171+
// --- InspectorView double ---------------------------------------------------
172+
// Surfaces each piece of session-scoped state under test and exposes buttons
173+
// that invoke the App's connect / call-tool / get-prompt / read-resource /
174+
// set-log-level handlers.
175+
vi.mock("./components/views/InspectorView/InspectorView", () => ({
176+
InspectorView: (props: {
177+
toolCallState?: { status?: string };
178+
getPromptState?: { status?: string };
179+
readResourceState?: { status?: string };
180+
currentLogLevel?: string;
181+
onToggleConnection: (id: string) => void;
182+
onCallTool: (name: string, args: Record<string, unknown>) => void;
183+
onGetPrompt: (name: string, args: Record<string, string>) => void;
184+
onReadResource: (uri: string) => void;
185+
onSetLogLevel: (level: string) => void;
186+
}) => (
187+
<div>
188+
<span data-testid="tool-status">
189+
{props.toolCallState?.status ?? "none"}
190+
</span>
191+
<span data-testid="prompt-status">
192+
{props.getPromptState?.status ?? "none"}
193+
</span>
194+
<span data-testid="resource-status">
195+
{props.readResourceState?.status ?? "none"}
196+
</span>
197+
<span data-testid="log-level">{props.currentLogLevel}</span>
198+
<button onClick={() => props.onToggleConnection("A")}>connect</button>
199+
<button onClick={() => props.onCallTool("get_acts", {})}>call</button>
200+
<button onClick={() => props.onGetPrompt("greet", {})}>get-prompt</button>
201+
<button onClick={() => props.onReadResource("res://x")}>
202+
read-resource
203+
</button>
204+
<button onClick={() => props.onSetLogLevel("debug")}>set-level</button>
205+
</div>
206+
),
207+
}));
208+
209+
import App from "./App";
210+
import * as McpIndex from "@inspector/core/mcp/index.js";
211+
212+
const clientInstances = (
213+
McpIndex as unknown as { __clientInstances: EventTarget[] }
214+
).__clientInstances;
215+
216+
describe("App session-scoped state reset on disconnect", () => {
217+
beforeEach(() => {
218+
clientInstances.length = 0;
219+
});
220+
221+
it("clears the per-call panels and resets the log level on client disconnect", async () => {
222+
const user = userEvent.setup();
223+
renderWithMantine(<App />);
224+
225+
// Connect: builds the InspectorClient and registers the disconnect listener.
226+
await user.click(screen.getByText("connect"));
227+
await waitFor(() => expect(clientInstances).toHaveLength(1));
228+
229+
// Fill all three per-call panels with their (resolved) results.
230+
await user.click(screen.getByText("call"));
231+
await user.click(screen.getByText("get-prompt"));
232+
await user.click(screen.getByText("read-resource"));
233+
await waitFor(() => {
234+
expect(screen.getByTestId("tool-status")).toHaveTextContent("ok");
235+
expect(screen.getByTestId("prompt-status")).toHaveTextContent("ok");
236+
expect(screen.getByTestId("resource-status")).toHaveTextContent("ok");
237+
});
238+
239+
// Bump the optimistic log level off its "info" default.
240+
await user.click(screen.getByText("set-level"));
241+
await waitFor(() =>
242+
expect(screen.getByTestId("log-level")).toHaveTextContent("debug"),
243+
);
244+
245+
// Disconnect: every panel empties and the level returns to "info".
246+
act(() => {
247+
clientInstances[0].dispatchEvent(new Event("disconnect"));
248+
});
249+
250+
await waitFor(() => {
251+
expect(screen.getByTestId("tool-status")).toHaveTextContent("none");
252+
expect(screen.getByTestId("prompt-status")).toHaveTextContent("none");
253+
expect(screen.getByTestId("resource-status")).toHaveTextContent("none");
254+
});
255+
expect(screen.getByTestId("log-level")).toHaveTextContent("info");
256+
});
257+
});

clients/web/src/App.tsx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,13 +350,28 @@ function App() {
350350
// failure / process exit) and avoids the first-render-clobbers-new-id
351351
// trap that watching connectionStatus has (status starts as
352352
// "disconnected" for the new client before connect() runs).
353+
//
354+
// This is also where session-scoped UI state that lives in App.tsx
355+
// (rather than inside the per-server state managers) gets reset, so the
356+
// next server's screens don't show server A's last result. The per-call
357+
// panels (`toolCallState` / `getPromptState` / `readResourceState`) and
358+
// the optimistic `currentLogLevel` all survive a disconnect/reconnect
359+
// cycle otherwise — see #1368. `latencyMs` resets via the
360+
// `connectionStatus` effect above instead because it has its own
361+
// connecting-edge ref to coordinate with.
353362
useEffect(() => {
354363
if (!inspectorClient) return;
355364
const onDisconnect = () => {
356365
setActiveServerId(undefined);
357366
// Drop the open flag too — without this the modal would pop back the
358367
// next time `initializeResult` re-becomes truthy (e.g. reconnect).
359368
setConnectionInfoModalOpen(false);
369+
// Clear the per-call result panels so the next session starts empty.
370+
setToolCallState(undefined);
371+
setGetPromptState(undefined);
372+
setReadResourceState(undefined);
373+
// Back to the default level the next session begins at.
374+
setCurrentLogLevel("info");
360375
};
361376
inspectorClient.addEventListener("disconnect", onDisconnect);
362377
return () => {

0 commit comments

Comments
 (0)