diff --git a/clients/tui/src/App.tsx b/clients/tui/src/App.tsx index 6950361f3..c9029276f 100644 --- a/clients/tui/src/App.tsx +++ b/clients/tui/src/App.tsx @@ -204,7 +204,7 @@ function App({ const [inspectorClients, setInspectorClients] = useState< Record >({}); - // ManagedToolsState per server (tools list from manager, not client) + // ManagedToolsState per server (tools list from manager) const [managedToolsStates, setManagedToolsStates] = useState< Record >({}); @@ -463,10 +463,7 @@ function App({ : null, [selectedServer, managedToolsStates], ); - const { tools: managedTools } = useManagedTools( - selectedInspectorClient, - selectedManagedToolsState, - ); + const { tools: managedTools } = useManagedTools(selectedManagedToolsState); // Resources, resource templates, prompts from managed state managers const selectedManagedResourcesState = useMemo( @@ -776,7 +773,7 @@ function App({ } }, [selectedInspectorClient]); - // Build current server state from InspectorClient data (tools from ManagedToolsState) + // Build current server state from InspectorClient data (tools from managed tools store) const currentServerState = useMemo(() => { if (!selectedServer) return null; return { diff --git a/core/__tests__/mcp/state/managedToolsState.test.ts b/core/__tests__/mcp/state/managedToolsState.test.ts index d4de3181d..82c30e996 100644 --- a/core/__tests__/mcp/state/managedToolsState.test.ts +++ b/core/__tests__/mcp/state/managedToolsState.test.ts @@ -15,6 +15,24 @@ import { type TestServerHttp, } from "@modelcontextprotocol/inspector-test-server"; +function waitForTools(manager: ManagedToolsState): Promise { + return new Promise((resolve) => { + const store = manager.getStore(); + const tools = store.getState().tools; + if (tools.length > 0) { + resolve(tools); + return; + } + const unsub = store.subscribe(() => { + const next = store.getState().tools; + if (next.length > 0) { + unsub(); + resolve(next); + } + }); + }); +} + describe("ManagedToolsState", () => { let client: InspectorClient | null = null; let server: TestServerHttp | null = null; @@ -37,14 +55,6 @@ describe("ManagedToolsState", () => { } }); - function waitForToolsChange(s: ManagedToolsState): Promise { - return new Promise((resolve) => { - s.addEventListener("toolsChange", (e) => resolve(e.detail), { - once: true, - }); - }); - } - it("starts with empty tools before connect", () => { client = new InspectorClient( { type: "streamable-http", url: "http://localhost:0" }, @@ -54,7 +64,7 @@ describe("ManagedToolsState", () => { expect(state.getTools()).toEqual([]); }); - it("on connect loads initial tools and dispatches toolsChange", async () => { + it("on connect loads initial tools and updates store", async () => { server = createTestServerHttp({ serverInfo: createTestServerInfo(), tools: [createEchoTool()], @@ -67,16 +77,15 @@ describe("ManagedToolsState", () => { }, ); state = new ManagedToolsState(client); - const toolsPromise = waitForToolsChange(state); + const toolsPromise = waitForTools(state); await client.connect(); const tools = await toolsPromise; expect(tools.length).toBeGreaterThan(0); expect(tools.some((t) => t.name === "echo")).toBe(true); - expect(state.getTools()).toEqual(tools); + expect(state!.getTools()).toEqual(tools); }); - it("refresh fetches all pages and dispatches toolsChange", async () => { - // Same server config as inspectorClient.test "should accumulate tools when paginating with cursor" + it("refresh fetches all pages and updates store", async () => { server = createTestServerHttp({ serverInfo: createTestServerInfo(), tools: createNumberedTools(6), @@ -95,10 +104,9 @@ describe("ManagedToolsState", () => { ); await client.connect(); - // Manager refresh must see exactly 6 tools (uses listTools(), so no list interactions) state = new ManagedToolsState(client); - const toolsPromise = waitForToolsChange(state); - const tools = await state.refresh(); + const toolsPromise = waitForTools(state); + const tools = await state!.refresh(); await toolsPromise; expect(tools).toHaveLength(6); expect(tools.map((t) => t.name)).toEqual([ @@ -109,7 +117,7 @@ describe("ManagedToolsState", () => { "tool_5", "tool_6", ]); - expect(state.getTools()).toEqual(tools); + expect(state!.getTools()).toEqual(tools); }); it("on toolsListChanged refreshes and updates tools", async () => { @@ -129,18 +137,18 @@ describe("ManagedToolsState", () => { ); state = new ManagedToolsState(client); await client.connect(); - await waitForToolsChange(state!); + await waitForTools(state!); const toolsBefore = state!.getTools(); expect(toolsBefore.length).toBeGreaterThan(0); const addTool = state!.getTools().find((t) => t.name === "add_tool"); expect(addTool).toBeDefined(); - const toolsChangePromise = waitForToolsChange(state!); + const toolsPromise = waitForTools(state!); await client!.callTool(addTool!, { name: "newTool", description: "A new test tool", }); - await toolsChangePromise; + await toolsPromise; const toolsAfter = state!.getTools(); expect(toolsAfter.find((t) => t.name === "newTool")).toBeDefined(); }); @@ -159,7 +167,7 @@ describe("ManagedToolsState", () => { ); state = new ManagedToolsState(client); await client.connect(); - await waitForToolsChange(state!); + await waitForTools(state!); expect(state!.getTools().length).toBeGreaterThan(0); await client!.disconnect(100); expect(state!.getTools()).toEqual([]); diff --git a/core/__tests__/react/useManagedTools.test.tsx b/core/__tests__/react/useManagedTools.test.tsx index c4243423b..a368e5fe3 100644 --- a/core/__tests__/react/useManagedTools.test.tsx +++ b/core/__tests__/react/useManagedTools.test.tsx @@ -4,84 +4,85 @@ import { describe, it, expect } from "vitest"; import { renderHook, act } from "@testing-library/react"; import { useManagedTools } from "../../react/useManagedTools.js"; +import { createStore, type StoreApi } from "zustand/vanilla"; import type { ManagedToolsState } from "../../mcp/state/managedToolsState.js"; -import type { InspectorClient } from "../../mcp/inspectorClient.js"; import type { Tool } from "@modelcontextprotocol/sdk/types.js"; /** - * Mock ManagedToolsState: getTools(), refresh(), and toolsChange events. + * Mock ManagedToolsState: getStore(), getTools(), refresh(), setMetadata(), destroy(). + * Not typed as implements ManagedToolsState because the real class has private fields. */ -class MockManagedToolsState extends EventTarget { - private _tools: Tool[] = []; +class MockManagedToolsState { + private store: StoreApi<{ tools: Tool[] }>; + + constructor() { + this.store = createStore<{ tools: Tool[] }>()((_set) => ({ tools: [] })); + } + + getStore() { + return { + getState: () => this.store.getState(), + subscribe: (listener: () => void) => this.store.subscribe(listener), + }; + } getTools(): Tool[] { - return [...this._tools]; + return this.store.getState().tools; } setTools(tools: Tool[]): void { - this._tools = tools; - this.dispatchEvent(new CustomEvent("toolsChange", { detail: tools })); + this.store.setState({ tools }); } + setMetadata(): void {} + async refresh(): Promise { return this.getTools(); } destroy(): void { - this._tools = []; + this.store.setState({ tools: [] }); } } describe("useManagedTools", () => { - it("returns empty tools and no-op refresh when given null client and null manager", async () => { - const { result } = renderHook(() => useManagedTools(null, null)); - - expect(result.current.tools).toEqual([]); - - await act(async () => { - const next = await result.current.refresh(); - expect(next).toEqual([]); - }); - expect(result.current.tools).toEqual([]); - }); - - it("returns empty tools when manager is null", async () => { - const client = {} as InspectorClient; - const { result } = renderHook(() => useManagedTools(client, null)); - - expect(result.current.tools).toEqual([]); - - await act(async () => { - const next = await result.current.refresh(); - expect(next).toEqual([]); - }); + it("returns empty tools and no-op refresh when manager is null or undefined", async () => { + const { result: resultNull } = renderHook(() => useManagedTools(null)); + expect(resultNull.current.tools).toEqual([]); + const fromNull = await resultNull.current.refresh(); + expect(fromNull).toEqual([]); + + const { result: resultUndef } = renderHook(() => + useManagedTools(undefined), + ); + expect(resultUndef.current.tools).toEqual([]); + const fromUndef = await resultUndef.current.refresh(); + expect(fromUndef).toEqual([]); }); - it("syncs initial tools from manager", () => { + it("syncs initial tools from manager store", () => { const manager = new MockManagedToolsState(); manager.setTools([ { name: "a", inputSchema: { type: "object" as const } }, { name: "b", inputSchema: { type: "object" as const } }, ]); - const client = {} as InspectorClient; const { result } = renderHook(() => - useManagedTools(client, manager as unknown as ManagedToolsState), + useManagedTools(manager as unknown as ManagedToolsState), ); expect(result.current.tools).toHaveLength(2); expect(result.current.tools.map((t) => t.name)).toEqual(["a", "b"]); }); - it("updates tools when manager dispatches toolsChange", async () => { + it("updates tools when manager store updates", async () => { const manager = new MockManagedToolsState(); manager.setTools([ { name: "first", inputSchema: { type: "object" as const } }, ]); - const client = {} as InspectorClient; const { result } = renderHook(() => - useManagedTools(client, manager as unknown as ManagedToolsState), + useManagedTools(manager as unknown as ManagedToolsState), ); expect(result.current.tools).toHaveLength(1); @@ -101,13 +102,12 @@ describe("useManagedTools", () => { ]); }); - it("refresh updates state from manager", async () => { + it("refresh returns tools from manager", async () => { const manager = new MockManagedToolsState(); manager.setTools([{ name: "x", inputSchema: { type: "object" as const } }]); - const client = {} as InspectorClient; const { result } = renderHook(() => - useManagedTools(client, manager as unknown as ManagedToolsState), + useManagedTools(manager as unknown as ManagedToolsState), ); expect(result.current.tools).toHaveLength(1); @@ -126,28 +126,4 @@ describe("useManagedTools", () => { expect(result.current.tools).toHaveLength(2); }); - - it("clears tools when manager switches to null", async () => { - const manager = new MockManagedToolsState(); - manager.setTools([ - { name: "only", inputSchema: { type: "object" as const } }, - ]); - const client = {} as InspectorClient; - - const { result, rerender } = renderHook( - ({ client: c, manager: m }) => useManagedTools(c, m), - { - initialProps: { - client, - manager: manager as unknown as ManagedToolsState, - }, - }, - ); - - expect(result.current.tools).toHaveLength(1); - - rerender({ client, manager: null }); - - expect(result.current.tools).toEqual([]); - }); }); diff --git a/core/mcp/state/index.ts b/core/mcp/state/index.ts index 7f1cb2a37..75ecd1f5c 100644 --- a/core/mcp/state/index.ts +++ b/core/mcp/state/index.ts @@ -1,5 +1,5 @@ export { ManagedToolsState } from "./managedToolsState.js"; -export type { ManagedToolsStateEventMap } from "./managedToolsState.js"; +export type { ManagedToolsReadOnlyStore } from "./managedToolsState.js"; export { MessageLogState } from "./messageLogState.js"; export type { MessageLogStateEventMap, diff --git a/core/mcp/state/managedToolsState.ts b/core/mcp/state/managedToolsState.ts index 7fa563458..6a8171b9e 100644 --- a/core/mcp/state/managedToolsState.ts +++ b/core/mcp/state/managedToolsState.ts @@ -1,31 +1,38 @@ /** - * ManagedToolsState: holds full tool list, syncs on toolsListChanged. - * Takes InspectorClient (will become InspectorClientProtocol in Stage 5). + * ManagedToolsState: keeps full tool list in sync with the server. + * State is held in a Zustand store; manager updates it and exposes getStore() for read-only subscription. */ +import { createStore } from "zustand/vanilla"; import type { InspectorClient } from "../inspectorClient.js"; import type { Tool } from "@modelcontextprotocol/sdk/types.js"; -import { TypedEventTarget } from "../typedEventTarget.js"; const MAX_PAGES = 100; -export interface ManagedToolsStateEventMap { - toolsChange: Tool[]; +/** Read-only store view: getState + subscribe, no setState. Use for hooks and other consumers. */ +export interface ManagedToolsReadOnlyStore { + getState: () => { tools: Tool[] }; + subscribe: (listener: () => void) => () => void; } /** * State manager that keeps a full tool list in sync with the server. - * Subscribes to client's connect (initial load), toolsListChanged, and statusChange; fetches all pages on refresh. - * If the caller wants metadata on list_tools (e.g. CLI --metadata), set it via setMetadata() so internal refresh() calls use it. + * Subscribes to client's connect, toolsListChanged, and statusChange; fetches all pages on refresh. + * Use getStore() for subscription (e.g. in React via useStore); use getTools() for one-off read. */ -export class ManagedToolsState extends TypedEventTarget { - private tools: Tool[] = []; +export class ManagedToolsState { + private readonly store = createStore<{ tools: Tool[] }>()((_set) => ({ + tools: [], + })); + private readonly readOnlyStore: ManagedToolsReadOnlyStore = { + getState: () => this.store.getState(), + subscribe: (listener) => this.store.subscribe(listener), + }; private client: InspectorClient | null = null; private unsubscribe: (() => void) | null = null; private _metadata: Record | undefined = undefined; constructor(client: InspectorClient) { - super(); this.client = client; const onConnect = (): void => { void this.refresh(); @@ -35,8 +42,7 @@ export class ManagedToolsState extends TypedEventTarget { if (this.client?.getStatus() === "disconnected") { - this.tools = []; - this.dispatchTypedEvent("toolsChange", []); + this.store.setState({ tools: [] }); } }; this.client.addEventListener("connect", onConnect); @@ -52,8 +58,13 @@ export class ManagedToolsState extends TypedEventTarget s.tools)). */ + getStore(): ManagedToolsReadOnlyStore { + return this.readOnlyStore; + } + getTools(): Tool[] { - return [...this.tools]; + return [...this.store.getState().tools]; } /** @@ -75,12 +86,12 @@ export class ManagedToolsState extends TypedEventTarget= MAX_PAGES) { @@ -89,7 +100,7 @@ export class ManagedToolsState extends TypedEventTarget Promise; + refresh: (metadata?: Record) => Promise; } +const EMPTY_TOOLS: Tool[] = []; // Stable empty array reference to avoid re-renders +const NOOP_SUBSCRIBE = () => () => {}; + /** - * React hook that subscribes to ManagedToolsState and returns tools + refresh. - * Pass the same InspectorClient and ManagedToolsState (or create manager inside hook via ref). + * Subscribes to the manager's store and returns tools + refresh. + * When manager is null/undefined, returns empty tools and a no-op refresh. */ export function useManagedTools( - client: InspectorClient | null, - managedToolsState: ManagedToolsState | null, + managedToolsState: ManagedToolsState | null | undefined, ): UseManagedToolsResult { - const [tools, setTools] = useState( - managedToolsState?.getTools() ?? [], + const store = managedToolsState?.getStore() ?? null; + const tools = useSyncExternalStore( + store?.subscribe ?? NOOP_SUBSCRIBE, + store ? () => store.getState().tools : () => EMPTY_TOOLS, ); - useEffect(() => { - if (!managedToolsState) { - setTools([]); - return; - } - setTools(managedToolsState.getTools()); - const onToolsChange = ( - event: TypedEventGeneric, - ) => { - setTools(event.detail); - }; - managedToolsState.addEventListener("toolsChange", onToolsChange); - return () => { - managedToolsState.removeEventListener("toolsChange", onToolsChange); - }; - }, [managedToolsState]); - - const refresh = useCallback(async (): Promise => { - if (!managedToolsState || !client) return []; - const next = await managedToolsState.refresh(); - setTools(next); - return next; - }, [client, managedToolsState]); + const refresh = useCallback( + async (metadata?: Record): Promise => { + if (!managedToolsState) return EMPTY_TOOLS; + return managedToolsState.refresh(metadata); + }, + [managedToolsState], + ); return { tools, refresh }; }