From b0a024269e2f17be70bb509b8469e66f0afd3ce6 Mon Sep 17 00:00:00 2001 From: Cliff Hall Date: Mon, 8 Jun 2026 12:58:21 -0400 Subject: [PATCH] =?UTF-8?q?feat(web):=20list-changed=20handling=20?= =?UTF-8?q?=E2=80=94=20indicator,=20pull-on-demand=20default,=20and=20per-?= =?UTF-8?q?server=20auto-refresh=20opt-in=20(#1402)=20(#1443)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(web): wire the listChanged indicator across all screens (#1402) The per-screen list-changed indicator was hardcoded false in InspectorView for Tools, Apps, Prompts, and Resources, so it could never light up. Source it from the managed-state layer instead. Each ManagedXState now tracks a listChanged flag: set true when its */list_changed notification arrives (the existing auto-refresh leaves it set), exposed via getListChanged()/a listChangedChange event, and cleared by clearListChanged() or on disconnect. The useManagedX hooks surface the flag and clear it on a user-initiated refresh (the indicator's Refresh button = acknowledge). App threads the three flags to InspectorView, which maps them to the screens — Apps shares the tools flag since apps are a filtered view of tools. Tests cover set/clear/disconnect at the state-manager layer, the hook subscription + refresh-clears behavior, and the InspectorView routing (including Apps sharing the tools flag and no cross-wiring between screens). Co-Authored-By: Claude Opus 4.8 (1M context) * fix(core): close clear-after-await race in managed-list refresh (#1402 review) The useManagedX refresh() callbacks cleared the listChanged flag AFTER awaiting the fetch. If a */list_changed arrived mid-fetch, the state re-set the flag (and auto-refreshed), then the trailing clear wiped that genuinely-new signal. Clear BEFORE the await instead: the user's refresh acknowledges only the change in hand, and a mid-fetch change re-sets the flag and survives. Adds a race regression test on the tools hook. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(web): drop duplicate Refresh on the Apps screen (#1402) AppControls uniquely had a standalone toolbar Refresh button AND the ListChangedIndicator on a separate row, so once the indicator could actually appear (this PR), the Apps screen showed two Refresh buttons. Match the other three controls (Tools/Prompts/Resources): the indicator is the single header-right refresh affordance. Updates AppControls and AppsScreen tests to assert exactly one Refresh (and none when the list hasn't changed). Co-Authored-By: Claude Opus 4.8 (1M context) * feat(core): stop auto-invoking */list on list_changed; user pulls via Refresh (#1402) With auto-refresh on, the list was already current by the time the list-changed indicator appeared, making its Refresh button moot. Now the managed Tools/Prompts/Resources state managers only flag the change on */list_changed (they no longer auto-refresh); the list stays as-is until the user clicks Refresh, which fetches the new list and clears the indicator. The connect-time initial load is unchanged. Resource templates (no own indicator, part of the Resources screen) also stop auto-refreshing; onRefreshResources now refreshes both resources and templates, since a single resources/list_changed covers both. Tasks are left auto-refreshing — they have no list-changed indicator and need live status updates. Co-Authored-By: Claude Opus 4.8 (1M context) * docs(core): correct ManagedResourceTemplatesState header comments (#1402 review) The class no longer subscribes to resourceTemplatesListChanged; update the stale file/class docstrings that still claimed it syncs on that notification to reflect connect + statusChange only. Co-Authored-By: Claude Opus 4.8 (1M context) * feat(web): add per-server 'Auto Refresh on List Changed Notifications' option (#1402) New Options section in ServerSettingsForm with a single checkbox. When checked, the managed Tools/Prompts/Resources (and resource templates) state auto-refreshes on */list_changed notifications; when unchecked (default) the notification only lights the list-changed indicator and the user pulls via Refresh. Plumbing: new optional autoRefreshOnListChanged on InspectorServerSettings and StoredMCPServer, round-tripped through serverList (lifted with a false default, persisted only when true, added to the exhaustive field-key map); surfaced from InspectorClient via a new getServerSettings() on the protocol; the state managers read it live at notification time to decide auto-refresh vs flag-only. Resource templates re-subscribe to their list_changed but only auto-refresh when the option is on (they have no indicator of their own). Tests cover the round-trip (true persists, false omitted), the form section (render/checked/toggle), and each state manager's on/off branch. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(server): persist autoRefreshOnListChanged through validateSettings (#1402) The PUT /api/servers write path runs settings through validateSettings, which rebuilds the value from explicitly-named fields and silently drops unknown keys — so autoRefreshOnListChanged never reached disk (the serverList round-trip test passed because it calls inspectorSettingsToStoredFields directly, bypassing this path). Validate it as an optional boolean and include it in the built value; false is omitted on disk by inspectorSettingsToStoredFields as before. Adds integration tests over the real PUT path: true persists, false is omitted, and a non-boolean is rejected (400). Co-Authored-By: Claude Opus 4.8 (1M context) * feat(web): default the Server Settings modal to the Options section (#1402) Open 'Options' instead of 'Custom Headers' when the Server Settings modal first opens, so the new Auto Refresh toggle is the first thing surfaced. Updates the modal default-expanded section, the modal test's initial-state assertions, the three header tests (now expand 'Custom Headers' first since it's no longer open by default), and the story defaults. Co-Authored-By: Claude Opus 4.8 (1M context) * chore: drop accidentally-committed Inspector history dump; gitignore the class 83076441 swept an exported Inspector message-history JSON (manual-testing artifact at the repo root) into the tree via git add -A. Untrack it and extend the existing repo-root debug-artifact .gitignore block (alongside /*.png) to cover /inspector-history-*.json so these can't be re-added. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .gitignore | 6 +- clients/web/src/App.test.tsx | 5 +- clients/web/src/App.tsx | 43 +++++--- .../groups/AppControls/AppControls.test.tsx | 27 ++--- .../groups/AppControls/AppControls.tsx | 9 +- .../ServerSettingsForm.stories.tsx | 10 +- .../ServerSettingsForm.test.tsx | 49 +++++++++ .../ServerSettingsForm/ServerSettingsForm.tsx | 18 +++ .../ServerSettingsModal.test.tsx | 15 ++- .../ServerSettingsModal.tsx | 8 +- .../screens/AppsScreen/AppsScreen.test.tsx | 19 ++-- .../InspectorView/InspectorView.test.tsx | 103 ++++++++++++++++++ .../views/InspectorView/InspectorView.tsx | 18 ++- .../web/src/test/core/mcp/serverList.test.ts | 40 +++++++ .../mcp/state/managedPromptsState.test.ts | 91 +++++++++++++++- .../managedResourceTemplatesState.test.ts | 37 ++++++- .../mcp/state/managedResourcesState.test.ts | 91 +++++++++++++++- .../core/mcp/state/managedToolsState.test.ts | 91 +++++++++++++++- .../core/react/useManagedPrompts.test.tsx | 37 +++++++ .../core/react/useManagedResources.test.tsx | 37 +++++++ .../test/core/react/useManagedTools.test.tsx | 78 +++++++++++++ .../src/test/core/react/useServers.test.tsx | 3 + .../mcp/remote/servers-route.test.ts | 81 ++++++++++++++ core/mcp/__tests__/fakeInspectorClient.ts | 8 ++ core/mcp/inspectorClient.ts | 10 ++ core/mcp/inspectorClientProtocol.ts | 2 + core/mcp/remote/node/server.ts | 14 +++ core/mcp/serverList.ts | 10 ++ core/mcp/state/managedPromptsState.ts | 38 ++++++- .../state/managedResourceTemplatesState.ts | 22 +++- core/mcp/state/managedResourcesState.ts | 39 ++++++- core/mcp/state/managedToolsState.ts | 38 ++++++- core/mcp/types.ts | 13 +++ core/react/useManagedPrompts.ts | 31 +++++- core/react/useManagedResources.ts | 33 +++++- core/react/useManagedTools.ts | 28 ++++- 36 files changed, 1109 insertions(+), 93 deletions(-) diff --git a/.gitignore b/.gitignore index e360080f..ced7cf11 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,8 @@ test-servers/build .env /.playwright-mcp/ -# Local debugging screenshots dropped at the repo root (e.g. attached to -# Claude Code conversations). Should never be part of the source tree. +# Local debugging artifacts dropped at the repo root (e.g. attached to +# Claude Code conversations): screenshots and exported Inspector history +# dumps. Should never be part of the source tree. /*.png +/inspector-history-*.json diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx index 086174f1..d9871056 100644 --- a/clients/web/src/App.test.tsx +++ b/clients/web/src/App.test.tsx @@ -183,7 +183,10 @@ 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: [] })), + useManagedResourceTemplates: vi.fn(() => ({ + resourceTemplates: [], + refresh: vi.fn(), + })), })); vi.mock("@inspector/core/react/useManagedRequestorTasks.js", () => ({ useManagedRequestorTasks: vi.fn(() => ({ diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 878799fb..c8b58998 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -277,6 +277,7 @@ const EMPTY_SETTINGS: InspectorServerSettings = { connectionTimeout: 0, requestTimeout: 0, taskTtl: DEFAULT_TASK_TTL_MS, + autoRefreshOnListChanged: false, roots: [], }; @@ -602,22 +603,23 @@ function App() { instructions, protocolVersion, } = useInspectorClient(inspectorClient); - const { tools, refresh: refreshTools } = useManagedTools( - inspectorClient, - managedToolsState, - ); - const { prompts, refresh: refreshPrompts } = useManagedPrompts( - inspectorClient, - managedPromptsState, - ); - const { resources, refresh: refreshResources } = useManagedResources( - inspectorClient, - managedResourcesState, - ); - const { resourceTemplates } = useManagedResourceTemplates( - inspectorClient, - managedResourceTemplatesState, - ); + const { + tools, + listChanged: toolsListChanged, + refresh: refreshTools, + } = useManagedTools(inspectorClient, managedToolsState); + const { + prompts, + listChanged: promptsListChanged, + refresh: refreshPrompts, + } = useManagedPrompts(inspectorClient, managedPromptsState); + const { + resources, + listChanged: resourcesListChanged, + refresh: refreshResources, + } = useManagedResources(inspectorClient, managedResourcesState); + const { resourceTemplates, refresh: refreshResourceTemplates } = + useManagedResourceTemplates(inspectorClient, managedResourceTemplatesState); const { tasks, refresh: refreshTasks, @@ -1677,8 +1679,12 @@ function App() { void refreshPrompts(); }, [refreshPrompts]); const onRefreshResources = useCallback(() => { + // Refresh both lists shown on the Resources screen. A single + // `notifications/resources/list_changed` covers resources and templates, + // and neither auto-refreshes anymore, so the user's Refresh pulls both. void refreshResources(); - }, [refreshResources]); + void refreshResourceTemplates(); + }, [refreshResources, refreshResourceTemplates]); const onRefreshTasks = useCallback(() => { // Surface list failures (e.g. the MAX_PAGES guard or a tasks/list error) // instead of letting the rejected promise go unhandled. @@ -2095,6 +2101,9 @@ function App() { prompts={prompts} resources={resources} resourceTemplates={resourceTemplates} + toolsListChanged={toolsListChanged} + promptsListChanged={promptsListChanged} + resourcesListChanged={resourcesListChanged} subscriptions={subscriptions} logs={logs} tasks={tasks} diff --git a/clients/web/src/components/groups/AppControls/AppControls.test.tsx b/clients/web/src/components/groups/AppControls/AppControls.test.tsx index 54a4629a..ac7c15a5 100644 --- a/clients/web/src/components/groups/AppControls/AppControls.test.tsx +++ b/clients/web/src/components/groups/AppControls/AppControls.test.tsx @@ -119,33 +119,28 @@ describe("AppControls", () => { expect(onSelectApp).not.toHaveBeenCalled(); }); - it("invokes onRefreshList when the toolbar Refresh button is clicked", async () => { - const user = userEvent.setup(); - const onRefreshList = vi.fn(); - renderWithMantine( - , - ); - await user.click(screen.getByRole("button", { name: "Refresh" })); - expect(onRefreshList).toHaveBeenCalledTimes(1); - }); - - it("does not show the list-changed indicator when listChanged is false", () => { + it("does not show the list-changed indicator (and no Refresh) when listChanged is false", () => { renderWithMantine(); expect(screen.queryByText("List updated")).not.toBeInTheDocument(); + // The indicator is the sole refresh affordance, so there's no standalone + // toolbar Refresh button to duplicate it. + expect( + screen.queryByRole("button", { name: "Refresh" }), + ).not.toBeInTheDocument(); }); - it("shows the list-changed indicator when listChanged is true", async () => { + it("shows a single Refresh via the list-changed indicator when listChanged is true", async () => { const user = userEvent.setup(); const onRefreshList = vi.fn(); renderWithMantine( , ); expect(screen.getByText("List updated")).toBeInTheDocument(); - // Both the toolbar button and the list-changed indicator's button render - // as "Refresh"; either one should drive onRefreshList. + // Exactly one Refresh button — the indicator's — not a duplicate alongside + // a standalone toolbar button. const refreshButtons = screen.getAllByRole("button", { name: "Refresh" }); - expect(refreshButtons).toHaveLength(2); - await user.click(refreshButtons[1]); + expect(refreshButtons).toHaveLength(1); + await user.click(refreshButtons[0]); expect(onRefreshList).toHaveBeenCalledTimes(1); }); }); diff --git a/clients/web/src/components/groups/AppControls/AppControls.tsx b/clients/web/src/components/groups/AppControls/AppControls.tsx index 44d3c654..d39d201a 100644 --- a/clients/web/src/components/groups/AppControls/AppControls.tsx +++ b/clients/web/src/components/groups/AppControls/AppControls.tsx @@ -1,5 +1,4 @@ import { - Button, Group, ScrollArea, Stack, @@ -27,11 +26,6 @@ export interface AppControlsProps { const LIST_MAX_HEIGHT = "calc(100vh - var(--app-shell-header-height, 0px) - var(--mantine-spacing-xl) * 2 - 220px)"; -const ToolbarButton = Button.withProps({ - variant: "subtle", - size: "sm", -}); - const EmptyState = Text.withProps({ c: "dimmed", ta: "center", @@ -61,9 +55,8 @@ export function AppControls({ MCP Apps ({tools.length}) - Refresh + - { + args.onAutoRefreshChange(value); + updateArgs({ + settings: { ...args.settings, autoRefreshOnListChanged: value }, + }); + }} onOAuthChange={(oauth) => { args.onOAuthChange(oauth); updateArgs({ @@ -110,7 +116,7 @@ const meta: Meta = { component: ServerSettingsForm, render: InteractiveRender, args: { - expandedSections: ["headers"], + expandedSections: ["options"], onExpandedSectionsChange: fn(), onAddHeader: fn(), onRemoveHeader: fn(), @@ -119,6 +125,7 @@ const meta: Meta = { onRemoveMetadata: fn(), onMetadataChange: fn(), onTimeoutChange: fn(), + onAutoRefreshChange: fn(), onOAuthChange: fn(), }, }; @@ -134,6 +141,7 @@ export const DefaultSettings: Story = { export const WithHeaders: Story = { args: { + expandedSections: ["headers"], settings: { ...defaultSettings, headers: [ diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx index c8ce9bdf..6be68e1c 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx @@ -45,6 +45,7 @@ const baseHandlers = { onRemoveMetadata: vi.fn(), onMetadataChange: vi.fn(), onTimeoutChange: vi.fn(), + onAutoRefreshChange: vi.fn(), onOAuthChange: vi.fn(), onAddRoot: vi.fn(), onRemoveRoot: vi.fn(), @@ -236,6 +237,54 @@ describe("ServerSettingsForm", () => { expect(screen.getByLabelText(/Task TTL/)).toHaveValue("45000 ms"); }); + it("renders the Options section with the Auto Refresh checkbox unchecked by default", () => { + renderWithMantine( + , + ); + const checkbox = screen.getByRole("checkbox", { + name: /Auto Refresh on List Changed Notifications/, + }); + expect(checkbox).not.toBeChecked(); + }); + + it("reflects autoRefreshOnListChanged=true as a checked box", () => { + renderWithMantine( + , + ); + expect( + screen.getByRole("checkbox", { + name: /Auto Refresh on List Changed Notifications/, + }), + ).toBeChecked(); + }); + + it("invokes onAutoRefreshChange when the Auto Refresh checkbox is toggled", async () => { + const user = userEvent.setup(); + const onAutoRefreshChange = vi.fn(); + renderWithMantine( + , + ); + await user.click( + screen.getByRole("checkbox", { + name: /Auto Refresh on List Changed Notifications/, + }), + ); + expect(onAutoRefreshChange).toHaveBeenCalledWith(true); + }); + it("invokes onOAuthChange when typing in client id", async () => { const user = userEvent.setup(); const onOAuthChange = vi.fn(); diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx index 25479aa0..c9eeed2b 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx @@ -2,6 +2,7 @@ import { Accordion, ActionIcon, Button, + Checkbox, Group, NumberInput, Stack, @@ -15,6 +16,7 @@ import type { import type { Root } from "@modelcontextprotocol/sdk/types.js"; export type ServerSettingsSection = + | "options" | "headers" | "metadata" | "timeouts" @@ -35,6 +37,7 @@ export interface ServerSettingsFormProps { field: "connectionTimeout" | "requestTimeout" | "taskTtl", value: number, ) => void; + onAutoRefreshChange: (value: boolean) => void; onOAuthChange: (oauth: OAuthSettings) => void; onAddRoot: () => void; onRemoveRoot: (index: number) => void; @@ -143,6 +146,7 @@ export function ServerSettingsForm({ onRemoveMetadata, onMetadataChange, onTimeoutChange, + onAutoRefreshChange, onOAuthChange, onAddRoot, onRemoveRoot, @@ -173,6 +177,20 @@ export function ServerSettingsForm({ } variant="separated" > + + Options + + + onAutoRefreshChange(e.currentTarget.checked)} + /> + + + + Custom Headers diff --git a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.test.tsx b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.test.tsx index 7709a563..54025427 100644 --- a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.test.tsx +++ b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.test.tsx @@ -71,7 +71,7 @@ describe("ServerSettingsModal", () => { expect(onClose).toHaveBeenCalled(); }); - it("calls onSettingsChange when adding a header", async () => { + it("calls onSettingsChange when adding a header after expanding the section", async () => { const user = userEvent.setup(); const onSettingsChange = vi.fn(); renderWithMantine( @@ -82,6 +82,7 @@ describe("ServerSettingsModal", () => { onSettingsChange={onSettingsChange} />, ); + await user.click(screen.getByRole("button", { name: "Custom Headers" })); await user.click(screen.getByRole("button", { name: "+ Add Header" })); expect(onSettingsChange).toHaveBeenCalledWith({ ...emptySettings, @@ -100,6 +101,7 @@ describe("ServerSettingsModal", () => { onSettingsChange={onSettingsChange} />, ); + await user.click(screen.getByRole("button", { name: "Custom Headers" })); const removeButtons = screen.getAllByRole("button", { name: "X" }); await user.click(removeButtons[0]); expect(onSettingsChange).toHaveBeenCalledWith({ @@ -119,6 +121,7 @@ describe("ServerSettingsModal", () => { onSettingsChange={onSettingsChange} />, ); + await user.click(screen.getByRole("button", { name: "Custom Headers" })); const valueInput = screen.getByDisplayValue("Bearer abc"); await user.type(valueInput, "1"); expect(onSettingsChange).toHaveBeenCalled(); @@ -298,6 +301,7 @@ describe("ServerSettingsModal", () => { onSettingsChange={vi.fn()} />, ); + const optionsControl = screen.getByRole("button", { name: "Options" }); const headersControl = screen.getByRole("button", { name: "Custom Headers", }); @@ -308,14 +312,15 @@ describe("ServerSettingsModal", () => { const oauthControl = screen.getByRole("button", { name: "OAuth Settings" }); const rootsControl = screen.getByRole("button", { name: "Roots" }); - // Initially only "headers" is expanded. - expect(headersControl.getAttribute("aria-expanded")).toBe("true"); - expect(metadataControl.getAttribute("aria-expanded")).toBe("false"); + // Initially only "options" is expanded. + expect(optionsControl.getAttribute("aria-expanded")).toBe("true"); + expect(headersControl.getAttribute("aria-expanded")).toBe("false"); // Not every section is expanded → ListToggle starts in compact mode // and exposes "Expand all" as its aria-label. await user.click(screen.getByRole("button", { name: "Expand all" })); + expect(headersControl.getAttribute("aria-expanded")).toBe("true"); expect(metadataControl.getAttribute("aria-expanded")).toBe("true"); expect(timeoutsControl.getAttribute("aria-expanded")).toBe("true"); expect(oauthControl.getAttribute("aria-expanded")).toBe("true"); @@ -323,7 +328,7 @@ describe("ServerSettingsModal", () => { // After expanding every section the toggle flips to "Collapse all". await user.click(screen.getByRole("button", { name: "Collapse all" })); + expect(optionsControl.getAttribute("aria-expanded")).toBe("false"); expect(headersControl.getAttribute("aria-expanded")).toBe("false"); - expect(metadataControl.getAttribute("aria-expanded")).toBe("false"); }); }); diff --git a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.tsx b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.tsx index 9e67461d..8dfb1ab4 100644 --- a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.tsx +++ b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.tsx @@ -11,6 +11,7 @@ import { } from "../ServerSettingsForm/ServerSettingsForm"; const ALL_SECTIONS: ServerSettingsSection[] = [ + "options", "headers", "metadata", "timeouts", @@ -33,7 +34,7 @@ export function ServerSettingsModal({ }: ServerSettingsModalProps) { const [expandedSections, setExpandedSections] = useState< ServerSettingsSection[] - >(["headers"]); + >(["options"]); const allExpanded = expandedSections.length === ALL_SECTIONS.length; @@ -99,6 +100,10 @@ export function ServerSettingsModal({ }); } + function handleAutoRefreshChange(value: boolean) { + onSettingsChange({ ...settings, autoRefreshOnListChanged: value }); + } + function handleAddRoot() { onSettingsChange({ ...settings, @@ -153,6 +158,7 @@ export function ServerSettingsModal({ onRemoveMetadata={handleRemoveMetadata} onMetadataChange={handleMetadataChange} onTimeoutChange={handleTimeoutChange} + onAutoRefreshChange={handleAutoRefreshChange} onOAuthChange={handleOAuthChange} onAddRoot={handleAddRoot} onRemoveRoot={handleRemoveRoot} diff --git a/clients/web/src/components/screens/AppsScreen/AppsScreen.test.tsx b/clients/web/src/components/screens/AppsScreen/AppsScreen.test.tsx index 9ec51b3a..9157918b 100644 --- a/clients/web/src/components/screens/AppsScreen/AppsScreen.test.tsx +++ b/clients/web/src/components/screens/AppsScreen/AppsScreen.test.tsx @@ -288,27 +288,26 @@ describe("AppsScreen", () => { ).toBeInTheDocument(); }); - it("renders the ListChangedIndicator when listChanged is true", async () => { + it("renders a single Refresh via the ListChangedIndicator when listChanged is true", async () => { const user = userEvent.setup(); const onRefreshList = vi.fn(); renderWithMantine( , ); - // The indicator and toolbar both render "Refresh" buttons; the indicator - // is a sibling of the "List updated" label, so target it via that label. + // The indicator's Refresh is the only refresh affordance — no standalone + // toolbar button duplicating it. expect(screen.getByText("List updated")).toBeInTheDocument(); const refreshButtons = screen.getAllByRole("button", { name: "Refresh" }); - expect(refreshButtons).toHaveLength(2); + expect(refreshButtons).toHaveLength(1); await user.click(refreshButtons[0]); expect(onRefreshList).toHaveBeenCalledTimes(1); }); - it("invokes onRefreshList when the toolbar Refresh button is clicked", async () => { - const user = userEvent.setup(); - const onRefreshList = vi.fn(); - renderWithMantine(); - await user.click(screen.getByRole("button", { name: "Refresh" })); - expect(onRefreshList).toHaveBeenCalledTimes(1); + it("shows no Refresh button when listChanged is false", () => { + renderWithMantine(); + expect( + screen.queryByRole("button", { name: "Refresh" }), + ).not.toBeInTheDocument(); }); it("renders the tool icon next to the header title when present", async () => { diff --git a/clients/web/src/components/views/InspectorView/InspectorView.test.tsx b/clients/web/src/components/views/InspectorView/InspectorView.test.tsx index 485f459d..ae0d3265 100644 --- a/clients/web/src/components/views/InspectorView/InspectorView.test.tsx +++ b/clients/web/src/components/views/InspectorView/InspectorView.test.tsx @@ -56,6 +56,9 @@ function makeProps( prompts: [], resources: [], resourceTemplates: [], + toolsListChanged: false, + promptsListChanged: false, + resourcesListChanged: false, subscriptions: [], logs: [], tasks: [], @@ -585,4 +588,104 @@ describe("InspectorView", () => { await screen.findByRole("button", { name: "Expand all" }), ).toBeInTheDocument(); }); + + describe("listChanged indicator wiring (#1402)", () => { + // The indicator only mounts on the active screen, so each case connects, + // navigates to the target tab, and asserts the "List updated" affordance. + async function gotoTab(tab: string) { + const user = userEvent.setup(); + const tabSelect = await screen.findByDisplayValue("Servers"); + await user.click(tabSelect); + await user.click(await screen.findByText(tab)); + return user; + } + + it("routes toolsListChanged to the Tools screen indicator", async () => { + renderWithMantine( + , + ); + await gotoTab("Tools"); + expect(await screen.findByText("List updated")).toBeInTheDocument(); + }); + + it("shares the tools flag with the Apps screen (apps are filtered tools)", async () => { + renderWithMantine( + , + ); + await gotoTab("Apps"); + expect(await screen.findByText("List updated")).toBeInTheDocument(); + }); + + it("routes promptsListChanged to the Prompts screen indicator", async () => { + renderWithMantine( + , + ); + await gotoTab("Prompts"); + expect(await screen.findByText("List updated")).toBeInTheDocument(); + }); + + it("routes resourcesListChanged to the Resources screen indicator", async () => { + renderWithMantine( + , + ); + await gotoTab("Resources"); + expect(await screen.findByText("List updated")).toBeInTheDocument(); + }); + + it("does not show the indicator on a screen whose flag is false (no cross-wiring)", async () => { + renderWithMantine( + , + ); + await gotoTab("Prompts"); + // The Prompts screen has mounted (its heading is present)... + expect( + await screen.findByRole("heading", { name: "Prompts" }), + ).toBeInTheDocument(); + // ...but the indicator is not, since promptsListChanged is false. + expect(screen.queryByText("List updated")).not.toBeInTheDocument(); + }); + }); }); diff --git a/clients/web/src/components/views/InspectorView/InspectorView.tsx b/clients/web/src/components/views/InspectorView/InspectorView.tsx index d989fea7..38b21317 100644 --- a/clients/web/src/components/views/InspectorView/InspectorView.tsx +++ b/clients/web/src/components/views/InspectorView/InspectorView.tsx @@ -201,6 +201,13 @@ export interface InspectorViewProps { resources: Resource[]; resourceTemplates: ResourceTemplate[]; subscriptions: InspectorResourceSubscription[]; + + // "List changed since last refresh" flags, sourced from the managed-state + // layer (#1402). They light the per-screen list-changed indicator. Apps is a + // filtered view of tools, so it shares the tools flag. + toolsListChanged: boolean; + promptsListChanged: boolean; + resourcesListChanged: boolean; logs: LogEntryData[]; tasks: Task[]; progressByTaskId?: Record; @@ -336,6 +343,9 @@ export function InspectorView({ prompts, resources, resourceTemplates, + toolsListChanged, + promptsListChanged, + resourcesListChanged, subscriptions, logs, tasks, @@ -548,7 +558,7 @@ export function InspectorView({ tools={tools} callState={toolCallState} ui={toolsUi} - listChanged={false} + listChanged={toolsListChanged} serverSupportsTaskToolCalls={serverSupportsTaskToolCalls} onUiChange={onToolsUiChange} onRefreshList={onRefreshTools} @@ -560,7 +570,7 @@ export function InspectorView({ { expect(round).toEqual(original); }); + it("round-trips autoRefreshOnListChanged: lifts true to settings and back to disk", () => { + const original: MCPConfig = { + mcpServers: { + delta: { + type: "streamable-http", + url: "https://x.test/mcp", + autoRefreshOnListChanged: true, + }, + }, + }; + const [entry] = mcpConfigToServerEntries(original); + expect(entry?.settings?.autoRefreshOnListChanged).toBe(true); + const round = serverEntriesToMcpConfig(mcpConfigToServerEntries(original)); + expect(round).toEqual(original); + }); + + it("omits autoRefreshOnListChanged from disk when false (the default)", () => { + // Absent on disk lifts to false in memory; writing it back must NOT inject + // the field, keeping the diff minimal for the default-off case. A benign + // inspector field (connectionTimeout) is present so `settings` is built. + const original: MCPConfig = { + mcpServers: { + epsilon: { + type: "streamable-http", + url: "https://x.test/mcp", + connectionTimeout: 5000, + }, + }, + }; + const [entry] = mcpConfigToServerEntries(original); + expect(entry?.settings?.autoRefreshOnListChanged).toBe(false); + const round = serverEntriesToMcpConfig(mcpConfigToServerEntries(original)); + expect("autoRefreshOnListChanged" in (round.mcpServers.epsilon ?? {})).toBe( + false, + ); + expect(round).toEqual(original); + }); + it("lifts top-level Inspector-extension fields onto ServerEntry.settings (form shape)", () => { const cfg: MCPConfig = { mcpServers: { @@ -229,6 +267,8 @@ describe("serverEntriesToMcpConfig", () => { requestTimeout: 0, // Absent taskTtl on disk → product default in memory (for the form) taskTtl: 60000, + // Absent autoRefreshOnListChanged on disk → false in memory (for the form) + autoRefreshOnListChanged: false, // Absent roots on disk → empty list in memory (for the form) roots: [], // Nested oauth on disk → flat oauthClientId in memory diff --git a/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts b/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts index 21166356..e98fbc43 100644 --- a/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedPromptsState.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, beforeEach } from "vitest"; import type { Prompt } from "@modelcontextprotocol/sdk/types.js"; +import type { InspectorServerSettings } from "@inspector/core/mcp/types.js"; import { ManagedPromptsState } from "@inspector/core/mcp/state/managedPromptsState"; import { FakeInspectorClient } from "@inspector/core/mcp/__tests__/fakeInspectorClient"; @@ -7,6 +8,16 @@ function prompt(name: string): Prompt { return { name }; } +const AUTO_REFRESH_SETTINGS: InspectorServerSettings = { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + taskTtl: 60000, + autoRefreshOnListChanged: true, + roots: [], +}; + function waitForPromptsChange(state: ManagedPromptsState): Promise { return new Promise((resolve) => { state.addEventListener("promptsChange", (e) => resolve(e.detail), { @@ -124,13 +135,29 @@ describe("ManagedPromptsState", () => { expect(next.map((p) => p.name)).toEqual(["a"]); }); - it("promptsListChanged event triggers a refresh", async () => { + it("promptsListChanged does NOT auto-refresh by default (the user pulls via Refresh)", async () => { client.setStatus("connected"); client.queuePromptPages({ prompts: [prompt("a"), prompt("b")] }); - const changePromise = waitForPromptsChange(state); client.dispatchTypedEvent("promptsListChanged"); - const next = await changePromise; - expect(next.map((p) => p.name)).toEqual(["a", "b"]); + // Yield so a stray refresh would have landed. + await Promise.resolve(); + await Promise.resolve(); + expect(client.listPrompts).not.toHaveBeenCalled(); + expect(state.getPrompts()).toEqual([]); + }); + + it("promptsListChanged auto-refreshes when the server opts in", async () => { + const autoClient = new FakeInspectorClient({ + capabilities: { prompts: {} }, + serverSettings: AUTO_REFRESH_SETTINGS, + }); + autoClient.setStatus("connected"); + const autoState = new ManagedPromptsState(autoClient); + autoClient.queuePromptPages({ prompts: [prompt("a")] }); + const changed = waitForPromptsChange(autoState); + autoClient.dispatchTypedEvent("promptsListChanged"); + expect((await changed).map((p) => p.name)).toEqual(["a"]); + expect(autoClient.listPrompts).toHaveBeenCalled(); }); it("statusChange to disconnected clears prompts and dispatches promptsChange", async () => { @@ -178,6 +205,62 @@ describe("ManagedPromptsState", () => { expect(state.getPrompts()).toEqual([]); }); + describe("listChanged (#1402)", () => { + function waitForListChanged(s: ManagedPromptsState): Promise { + return new Promise((resolve) => { + s.addEventListener("listChangedChange", (e) => resolve(e.detail), { + once: true, + }); + }); + } + + it("starts cleared", () => { + expect(state.getListChanged()).toBe(false); + }); + + it("promptsListChanged sets the flag and dispatches listChangedChange", async () => { + client.setStatus("connected"); + client.queuePromptPages({ prompts: [prompt("a")] }); + const changed = waitForListChanged(state); + client.dispatchTypedEvent("promptsListChanged"); + expect(await changed).toBe(true); + expect(state.getListChanged()).toBe(true); + }); + + it("clearListChanged resets the flag and dispatches false", async () => { + client.setStatus("connected"); + client.queuePromptPages({ prompts: [prompt("a")] }); + client.dispatchTypedEvent("promptsListChanged"); + expect(state.getListChanged()).toBe(true); + + const changed = waitForListChanged(state); + state.clearListChanged(); + expect(await changed).toBe(false); + expect(state.getListChanged()).toBe(false); + }); + + it("clearListChanged is a no-op (no event) when already cleared", () => { + let fired = false; + state.addEventListener("listChangedChange", () => { + fired = true; + }); + state.clearListChanged(); + expect(fired).toBe(false); + }); + + it("disconnect clears the flag", async () => { + client.setStatus("connected"); + client.queuePromptPages({ prompts: [prompt("a")] }); + client.dispatchTypedEvent("promptsListChanged"); + expect(state.getListChanged()).toBe(true); + + const changed = waitForListChanged(state); + client.setStatus("disconnected"); + expect(await changed).toBe(false); + expect(state.getListChanged()).toBe(false); + }); + }); + it("destroy is idempotent", () => { state.destroy(); expect(() => state.destroy()).not.toThrow(); diff --git a/clients/web/src/test/core/mcp/state/managedResourceTemplatesState.test.ts b/clients/web/src/test/core/mcp/state/managedResourceTemplatesState.test.ts index d7b6372d..ea26751d 100644 --- a/clients/web/src/test/core/mcp/state/managedResourceTemplatesState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedResourceTemplatesState.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, beforeEach } from "vitest"; import type { ResourceTemplate } from "@modelcontextprotocol/sdk/types.js"; +import type { InspectorServerSettings } from "@inspector/core/mcp/types.js"; import { ManagedResourceTemplatesState } from "@inspector/core/mcp/state/managedResourceTemplatesState"; import { FakeInspectorClient } from "@inspector/core/mcp/__tests__/fakeInspectorClient"; @@ -7,6 +8,16 @@ function template(name: string): ResourceTemplate { return { uriTemplate: `tpl://{${name}}`, name }; } +const AUTO_REFRESH_SETTINGS: InspectorServerSettings = { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + taskTtl: 60000, + autoRefreshOnListChanged: true, + roots: [], +}; + function waitForChange( state: ManagedResourceTemplatesState, ): Promise { @@ -136,15 +147,33 @@ describe("ManagedResourceTemplatesState", () => { expect(next.map((t) => t.name)).toEqual(["a"]); }); - it("resourceTemplatesListChanged event triggers a refresh", async () => { + it("resourceTemplatesListChanged does NOT auto-refresh by default (refreshed via the Resources Refresh)", async () => { client.setStatus("connected"); client.queueResourceTemplatePages({ resourceTemplates: [template("a"), template("b")], }); - const changePromise = waitForChange(state); client.dispatchTypedEvent("resourceTemplatesListChanged"); - const next = await changePromise; - expect(next.map((t) => t.name)).toEqual(["a", "b"]); + // Yield so a stray refresh would have landed. + await Promise.resolve(); + await Promise.resolve(); + expect(client.listResourceTemplates).not.toHaveBeenCalled(); + expect(state.getResourceTemplates()).toEqual([]); + }); + + it("resourceTemplatesListChanged auto-refreshes when the server opts in", async () => { + const autoClient = new FakeInspectorClient({ + capabilities: { resources: {} }, + serverSettings: AUTO_REFRESH_SETTINGS, + }); + autoClient.setStatus("connected"); + const autoState = new ManagedResourceTemplatesState(autoClient); + autoClient.queueResourceTemplatePages({ + resourceTemplates: [template("a")], + }); + const changed = waitForChange(autoState); + autoClient.dispatchTypedEvent("resourceTemplatesListChanged"); + expect((await changed).map((t) => t.name)).toEqual(["a"]); + expect(autoClient.listResourceTemplates).toHaveBeenCalled(); }); it("statusChange to disconnected clears templates and dispatches change", async () => { diff --git a/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts b/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts index cb22ca27..c4c8db3e 100644 --- a/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedResourcesState.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, beforeEach } from "vitest"; import type { Resource } from "@modelcontextprotocol/sdk/types.js"; +import type { InspectorServerSettings } from "@inspector/core/mcp/types.js"; import { ManagedResourcesState } from "@inspector/core/mcp/state/managedResourcesState"; import { FakeInspectorClient } from "@inspector/core/mcp/__tests__/fakeInspectorClient"; @@ -7,6 +8,16 @@ function resource(uri: string): Resource { return { uri, name: uri }; } +const AUTO_REFRESH_SETTINGS: InspectorServerSettings = { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + taskTtl: 60000, + autoRefreshOnListChanged: true, + roots: [], +}; + function waitForResourcesChange( state: ManagedResourcesState, ): Promise { @@ -131,15 +142,31 @@ describe("ManagedResourcesState", () => { expect(next.map((r) => r.uri)).toEqual(["a://1"]); }); - it("resourcesListChanged event triggers a refresh", async () => { + it("resourcesListChanged does NOT auto-refresh by default (the user pulls via Refresh)", async () => { client.setStatus("connected"); client.queueResourcePages({ resources: [resource("a://1"), resource("a://2")], }); - const changePromise = waitForResourcesChange(state); client.dispatchTypedEvent("resourcesListChanged"); - const next = await changePromise; - expect(next.map((r) => r.uri)).toEqual(["a://1", "a://2"]); + // Yield so a stray refresh would have landed. + await Promise.resolve(); + await Promise.resolve(); + expect(client.listResources).not.toHaveBeenCalled(); + expect(state.getResources()).toEqual([]); + }); + + it("resourcesListChanged auto-refreshes when the server opts in", async () => { + const autoClient = new FakeInspectorClient({ + capabilities: { resources: {} }, + serverSettings: AUTO_REFRESH_SETTINGS, + }); + autoClient.setStatus("connected"); + const autoState = new ManagedResourcesState(autoClient); + autoClient.queueResourcePages({ resources: [resource("a://1")] }); + const changed = waitForResourcesChange(autoState); + autoClient.dispatchTypedEvent("resourcesListChanged"); + expect((await changed).map((r) => r.uri)).toEqual(["a://1"]); + expect(autoClient.listResources).toHaveBeenCalled(); }); it("statusChange to disconnected clears resources and dispatches resourcesChange", async () => { @@ -187,6 +214,62 @@ describe("ManagedResourcesState", () => { expect(state.getResources()).toEqual([]); }); + describe("listChanged (#1402)", () => { + function waitForListChanged(s: ManagedResourcesState): Promise { + return new Promise((resolve) => { + s.addEventListener("listChangedChange", (e) => resolve(e.detail), { + once: true, + }); + }); + } + + it("starts cleared", () => { + expect(state.getListChanged()).toBe(false); + }); + + it("resourcesListChanged sets the flag and dispatches listChangedChange", async () => { + client.setStatus("connected"); + client.queueResourcePages({ resources: [resource("a://1")] }); + const changed = waitForListChanged(state); + client.dispatchTypedEvent("resourcesListChanged"); + expect(await changed).toBe(true); + expect(state.getListChanged()).toBe(true); + }); + + it("clearListChanged resets the flag and dispatches false", async () => { + client.setStatus("connected"); + client.queueResourcePages({ resources: [resource("a://1")] }); + client.dispatchTypedEvent("resourcesListChanged"); + expect(state.getListChanged()).toBe(true); + + const changed = waitForListChanged(state); + state.clearListChanged(); + expect(await changed).toBe(false); + expect(state.getListChanged()).toBe(false); + }); + + it("clearListChanged is a no-op (no event) when already cleared", () => { + let fired = false; + state.addEventListener("listChangedChange", () => { + fired = true; + }); + state.clearListChanged(); + expect(fired).toBe(false); + }); + + it("disconnect clears the flag", async () => { + client.setStatus("connected"); + client.queueResourcePages({ resources: [resource("a://1")] }); + client.dispatchTypedEvent("resourcesListChanged"); + expect(state.getListChanged()).toBe(true); + + const changed = waitForListChanged(state); + client.setStatus("disconnected"); + expect(await changed).toBe(false); + expect(state.getListChanged()).toBe(false); + }); + }); + it("destroy is idempotent", () => { state.destroy(); expect(() => state.destroy()).not.toThrow(); diff --git a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts index edb04b92..bf839861 100644 --- a/clients/web/src/test/core/mcp/state/managedToolsState.test.ts +++ b/clients/web/src/test/core/mcp/state/managedToolsState.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, beforeEach } from "vitest"; import type { Tool } from "@modelcontextprotocol/sdk/types.js"; +import type { InspectorServerSettings } from "@inspector/core/mcp/types.js"; import { ManagedToolsState } from "@inspector/core/mcp/state/managedToolsState"; import { FakeInspectorClient } from "@inspector/core/mcp/__tests__/fakeInspectorClient"; @@ -7,6 +8,16 @@ function tool(name: string): Tool { return { name, inputSchema: { type: "object" } }; } +const AUTO_REFRESH_SETTINGS: InspectorServerSettings = { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + taskTtl: 60000, + autoRefreshOnListChanged: true, + roots: [], +}; + function waitForToolsChange(state: ManagedToolsState): Promise { return new Promise((resolve) => { state.addEventListener("toolsChange", (e) => resolve(e.detail), { @@ -122,13 +133,29 @@ describe("ManagedToolsState", () => { expect(next.map((t) => t.name)).toEqual(["a"]); }); - it("toolsListChanged event triggers a refresh", async () => { + it("toolsListChanged does NOT auto-refresh by default (the user pulls via Refresh)", async () => { client.setStatus("connected"); client.queueToolPages({ tools: [tool("a"), tool("b")] }); - const changePromise = waitForToolsChange(state); client.dispatchTypedEvent("toolsListChanged"); - const next = await changePromise; - expect(next.map((t) => t.name)).toEqual(["a", "b"]); + // Yield so a stray refresh would have landed. + await Promise.resolve(); + await Promise.resolve(); + expect(client.listTools).not.toHaveBeenCalled(); + expect(state.getTools()).toEqual([]); + }); + + it("toolsListChanged auto-refreshes when the server opts in", async () => { + const autoClient = new FakeInspectorClient({ + capabilities: { tools: {} }, + serverSettings: AUTO_REFRESH_SETTINGS, + }); + autoClient.setStatus("connected"); + const autoState = new ManagedToolsState(autoClient); + autoClient.queueToolPages({ tools: [tool("a")] }); + const changed = waitForToolsChange(autoState); + autoClient.dispatchTypedEvent("toolsListChanged"); + expect((await changed).map((t) => t.name)).toEqual(["a"]); + expect(autoClient.listTools).toHaveBeenCalled(); }); it("statusChange to disconnected clears tools and dispatches toolsChange", async () => { @@ -179,6 +206,62 @@ describe("ManagedToolsState", () => { expect(state.getTools()).toEqual([]); }); + describe("listChanged (#1402)", () => { + function waitForListChanged(s: ManagedToolsState): Promise { + return new Promise((resolve) => { + s.addEventListener("listChangedChange", (e) => resolve(e.detail), { + once: true, + }); + }); + } + + it("starts cleared", () => { + expect(state.getListChanged()).toBe(false); + }); + + it("toolsListChanged sets the flag and dispatches listChangedChange", async () => { + client.setStatus("connected"); + client.queueToolPages({ tools: [tool("a")] }); + const changed = waitForListChanged(state); + client.dispatchTypedEvent("toolsListChanged"); + expect(await changed).toBe(true); + expect(state.getListChanged()).toBe(true); + }); + + it("clearListChanged resets the flag and dispatches false", async () => { + client.setStatus("connected"); + client.queueToolPages({ tools: [tool("a")] }); + client.dispatchTypedEvent("toolsListChanged"); + expect(state.getListChanged()).toBe(true); + + const changed = waitForListChanged(state); + state.clearListChanged(); + expect(await changed).toBe(false); + expect(state.getListChanged()).toBe(false); + }); + + it("clearListChanged is a no-op (no event) when already cleared", () => { + let fired = false; + state.addEventListener("listChangedChange", () => { + fired = true; + }); + state.clearListChanged(); + expect(fired).toBe(false); + }); + + it("disconnect clears the flag", async () => { + client.setStatus("connected"); + client.queueToolPages({ tools: [tool("a")] }); + client.dispatchTypedEvent("toolsListChanged"); + expect(state.getListChanged()).toBe(true); + + const changed = waitForListChanged(state); + client.setStatus("disconnected"); + expect(await changed).toBe(false); + expect(state.getListChanged()).toBe(false); + }); + }); + it("destroy is idempotent", () => { state.destroy(); expect(() => state.destroy()).not.toThrow(); diff --git a/clients/web/src/test/core/react/useManagedPrompts.test.tsx b/clients/web/src/test/core/react/useManagedPrompts.test.tsx index 43888fc6..489c1509 100644 --- a/clients/web/src/test/core/react/useManagedPrompts.test.tsx +++ b/clients/web/src/test/core/react/useManagedPrompts.test.tsx @@ -102,4 +102,41 @@ describe("useManagedPrompts", () => { expect(result.current.prompts).toEqual([]); }); + + describe("listChanged (#1402)", () => { + it("starts false and reflects listChangedChange from the state", async () => { + const { result } = renderHook(() => useManagedPrompts(client, state)); + expect(result.current.listChanged).toBe(false); + + client.queuePromptPages({ prompts: [prompt("a")] }); + act(() => { + client.dispatchTypedEvent("promptsListChanged"); + }); + await waitFor(() => { + expect(result.current.listChanged).toBe(true); + }); + }); + + it("refresh() clears the indicator", async () => { + const { result } = renderHook(() => useManagedPrompts(client, state)); + client.queuePromptPages({ prompts: [prompt("a")] }); + act(() => { + client.dispatchTypedEvent("promptsListChanged"); + }); + await waitFor(() => expect(result.current.listChanged).toBe(true)); + + client.queuePromptPages({ prompts: [prompt("a")] }); + await act(async () => { + await result.current.refresh(); + }); + await waitFor(() => { + expect(result.current.listChanged).toBe(false); + }); + }); + + it("defaults to false when state is null", () => { + const { result } = renderHook(() => useManagedPrompts(client, null)); + expect(result.current.listChanged).toBe(false); + }); + }); }); diff --git a/clients/web/src/test/core/react/useManagedResources.test.tsx b/clients/web/src/test/core/react/useManagedResources.test.tsx index 7a6d385b..72eeab2a 100644 --- a/clients/web/src/test/core/react/useManagedResources.test.tsx +++ b/clients/web/src/test/core/react/useManagedResources.test.tsx @@ -107,4 +107,41 @@ describe("useManagedResources", () => { expect(result.current.resources).toEqual([]); }); + + describe("listChanged (#1402)", () => { + it("starts false and reflects listChangedChange from the state", async () => { + const { result } = renderHook(() => useManagedResources(client, state)); + expect(result.current.listChanged).toBe(false); + + client.queueResourcePages({ resources: [resource("a://1")] }); + act(() => { + client.dispatchTypedEvent("resourcesListChanged"); + }); + await waitFor(() => { + expect(result.current.listChanged).toBe(true); + }); + }); + + it("refresh() clears the indicator", async () => { + const { result } = renderHook(() => useManagedResources(client, state)); + client.queueResourcePages({ resources: [resource("a://1")] }); + act(() => { + client.dispatchTypedEvent("resourcesListChanged"); + }); + await waitFor(() => expect(result.current.listChanged).toBe(true)); + + client.queueResourcePages({ resources: [resource("a://1")] }); + await act(async () => { + await result.current.refresh(); + }); + await waitFor(() => { + expect(result.current.listChanged).toBe(false); + }); + }); + + it("defaults to false when state is null", () => { + const { result } = renderHook(() => useManagedResources(client, null)); + expect(result.current.listChanged).toBe(false); + }); + }); }); diff --git a/clients/web/src/test/core/react/useManagedTools.test.tsx b/clients/web/src/test/core/react/useManagedTools.test.tsx index 4b1c0f24..aadeb0a7 100644 --- a/clients/web/src/test/core/react/useManagedTools.test.tsx +++ b/clients/web/src/test/core/react/useManagedTools.test.tsx @@ -101,4 +101,82 @@ describe("useManagedTools", () => { // observed value is the empty snapshot captured before the refresh. expect(result.current.tools).toEqual([]); }); + + describe("listChanged (#1402)", () => { + it("starts false and reflects listChangedChange from the state", async () => { + const { result } = renderHook(() => useManagedTools(client, state)); + expect(result.current.listChanged).toBe(false); + + client.queueToolPages({ tools: [tool("a")] }); + act(() => { + client.dispatchTypedEvent("toolsListChanged"); + }); + await waitFor(() => { + expect(result.current.listChanged).toBe(true); + }); + }); + + it("refresh() clears the indicator", async () => { + const { result } = renderHook(() => useManagedTools(client, state)); + client.queueToolPages({ tools: [tool("a")] }); + act(() => { + client.dispatchTypedEvent("toolsListChanged"); + }); + await waitFor(() => expect(result.current.listChanged).toBe(true)); + + client.queueToolPages({ tools: [tool("a")] }); + await act(async () => { + await result.current.refresh(); + }); + await waitFor(() => { + expect(result.current.listChanged).toBe(false); + }); + }); + + it("defaults to false when state is null", () => { + const { result } = renderHook(() => useManagedTools(client, null)); + expect(result.current.listChanged).toBe(false); + }); + + it("keeps a change that fires during the awaited refresh (race regression)", async () => { + const { result } = renderHook(() => useManagedTools(client, state)); + + // Indicator starts set from an initial change. + client.queueToolPages({ tools: [tool("a")] }); + act(() => { + client.dispatchTypedEvent("toolsListChanged"); + }); + await waitFor(() => expect(result.current.listChanged).toBe(true)); + + // Hang the user-refresh's fetch so a new change can land mid-flight. + let releaseRefresh: (value: { tools: Tool[] }) => void = () => {}; + client.listTools.mockImplementationOnce( + () => + new Promise<{ tools: Tool[] }>((resolve) => { + releaseRefresh = resolve; + }), + ); + + let refreshPromise!: Promise; + act(() => { + refreshPromise = result.current.refresh(); + }); + // The hook clears BEFORE awaiting, so the indicator is already off. + await waitFor(() => expect(result.current.listChanged).toBe(false)); + + // A genuinely-new change arrives while the refresh is still in flight. + client.queueToolPages({ tools: [tool("a"), tool("b")] }); + act(() => { + client.dispatchTypedEvent("toolsListChanged"); + }); + await waitFor(() => expect(result.current.listChanged).toBe(true)); + + // Completing the user refresh must NOT wipe the mid-flight change. + await act(async () => { + releaseRefresh({ tools: [tool("a")] }); + await refreshPromise; + }); + expect(result.current.listChanged).toBe(true); + }); + }); }); diff --git a/clients/web/src/test/core/react/useServers.test.tsx b/clients/web/src/test/core/react/useServers.test.tsx index 9869da47..1288a303 100644 --- a/clients/web/src/test/core/react/useServers.test.tsx +++ b/clients/web/src/test/core/react/useServers.test.tsx @@ -403,6 +403,7 @@ describe("useServers", () => { connectionTimeout: 5000, requestTimeout: 30000, taskTtl: 30000, + autoRefreshOnListChanged: false, roots: [], }); }); @@ -484,6 +485,8 @@ describe("useServers", () => { requestTimeout: 0, // Absent taskTtl on disk reads back as the product default for the form. taskTtl: 60000, + // Absent autoRefreshOnListChanged on disk reads back as false. + autoRefreshOnListChanged: false, // Absent roots on disk reads back as an empty list for the form. roots: [], }); diff --git a/clients/web/src/test/integration/mcp/remote/servers-route.test.ts b/clients/web/src/test/integration/mcp/remote/servers-route.test.ts index ee52c4a6..1b954e6c 100644 --- a/clients/web/src/test/integration/mcp/remote/servers-route.test.ts +++ b/clients/web/src/test/integration/mcp/remote/servers-route.test.ts @@ -452,6 +452,87 @@ describe("/api/servers routes", () => { expect(stored).not.toHaveProperty("connectionTimeout"); }); + it("persists autoRefreshOnListChanged: true through the PUT write path", async () => { + // Regression: validateSettings rebuilds the value from named fields, so a + // new field is silently dropped unless it's explicitly handled there. + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { + delta: { type: "streamable-http", url: "https://x.test/mcp" }, + }, + }), + ); + const res = await fetch(`${h.baseUrl}/api/servers/delta`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + autoRefreshOnListChanged: true, + }, + }), + }); + expect(res.status).toBe(200); + const stored = readConfig(h.configPath).mcpServers + .delta as unknown as Record; + expect(stored.autoRefreshOnListChanged).toBe(true); + }); + + it("omits autoRefreshOnListChanged from disk when false", async () => { + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { + delta: { + type: "streamable-http", + url: "https://x.test/mcp", + autoRefreshOnListChanged: true, + }, + }, + }), + ); + const res = await fetch(`${h.baseUrl}/api/servers/delta`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + autoRefreshOnListChanged: false, + }, + }), + }); + expect(res.status).toBe(200); + const stored = readConfig(h.configPath).mcpServers + .delta as unknown as Record; + expect(stored).not.toHaveProperty("autoRefreshOnListChanged"); + }); + + it("rejects a non-boolean autoRefreshOnListChanged", async () => { + const res = await fetch(`${h.baseUrl}/api/servers`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + id: "bad-autorefresh", + config: { type: "stdio", command: "node" }, + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + autoRefreshOnListChanged: "yes", + }, + }), + }); + expect(res.status).toBe(400); + }); + it("rejects a non-object settings field", async () => { const res = await fetch(`${h.baseUrl}/api/servers`, { method: "POST", diff --git a/core/mcp/__tests__/fakeInspectorClient.ts b/core/mcp/__tests__/fakeInspectorClient.ts index e2f00774..916d12e1 100644 --- a/core/mcp/__tests__/fakeInspectorClient.ts +++ b/core/mcp/__tests__/fakeInspectorClient.ts @@ -26,6 +26,7 @@ import type { SamplingCreateMessage } from "../samplingCreateMessage.js"; import type { ElicitationCreateMessage } from "../elicitationCreateMessage.js"; import type { ConnectionStatus, + InspectorServerSettings, PromptGetInvocation, ResourceReadInvocation, ResourceTemplateReadInvocation, @@ -44,6 +45,7 @@ export interface FakeInspectorClientOptions { serverInfo?: Implementation; instructions?: string; protocolVersion?: string; + serverSettings?: InspectorServerSettings; } export class FakeInspectorClient @@ -56,6 +58,7 @@ export class FakeInspectorClient private serverInfo: Implementation | undefined; private instructions: string | undefined; private protocolVersion: string | undefined; + private serverSettings: InspectorServerSettings | undefined; private appRendererClient: AppRendererClient | null = null; private sessionId: string | undefined; private pendingSamples: SamplingCreateMessage[] = []; @@ -150,6 +153,7 @@ export class FakeInspectorClient this.serverInfo = options.serverInfo; this.instructions = options.instructions; this.protocolVersion = options.protocolVersion; + this.serverSettings = options.serverSettings; } getStatus(): ConnectionStatus { @@ -176,6 +180,10 @@ export class FakeInspectorClient return this.protocolVersion; } + getServerSettings(): InspectorServerSettings | undefined { + return this.serverSettings; + } + getAppRendererClient(): AppRendererClient | null { return this.appRendererClient; } diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index d90bd2a1..c6d2c5c1 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -1306,6 +1306,16 @@ export class InspectorClient extends InspectorClientEventTarget { return this.protocolVersion; } + /** + * The per-server settings this client was constructed with (headers, + * timeouts, roots, OAuth, the auto-refresh-on-list-changed option, etc.). + * Read by the managed list state to decide whether to auto-refresh on + * `list_changed` notifications (#1402). + */ + getServerSettings(): InspectorServerSettings | undefined { + return this.serverSettings; + } + /** * Set the logging level for the MCP server * @param level Logging level to set diff --git a/core/mcp/inspectorClientProtocol.ts b/core/mcp/inspectorClientProtocol.ts index 64102ad0..5a6eb250 100644 --- a/core/mcp/inspectorClientProtocol.ts +++ b/core/mcp/inspectorClientProtocol.ts @@ -14,6 +14,7 @@ import type { ConnectionStatus, + InspectorServerSettings, ResourceReadInvocation, ResourceTemplateReadInvocation, PromptGetInvocation, @@ -55,6 +56,7 @@ export interface InspectorClientProtocol extends InspectorClientEventTarget { getServerInfo(): Implementation | undefined; getInstructions(): string | undefined; getProtocolVersion(): string | undefined; + getServerSettings(): InspectorServerSettings | undefined; getAppRendererClient(): AppRendererClient | null; // Connection control diff --git a/core/mcp/remote/node/server.ts b/core/mcp/remote/node/server.ts index 55c7659c..4a906a98 100644 --- a/core/mcp/remote/node/server.ts +++ b/core/mcp/remote/node/server.ts @@ -1109,6 +1109,17 @@ export function createRemoteApp( error: "settings.taskTtl must be a non-negative number", }; } + // Optional on the wire (older clients won't send it); when present it must + // be a boolean. Absent reads back as false below. + if ( + obj.autoRefreshOnListChanged !== undefined && + typeof obj.autoRefreshOnListChanged !== "boolean" + ) { + return { + ok: false, + error: "settings.autoRefreshOnListChanged must be a boolean", + }; + } for (const optional of [ "oauthClientId", "oauthClientSecret", @@ -1146,6 +1157,9 @@ export function createRemoteApp( // to disk for a client that didn't send one. taskTtl: typeof obj.taskTtl === "number" ? obj.taskTtl : DEFAULT_TASK_TTL_MS, + // Absent → false, matching the read side. The omit-on-false logic lives + // in inspectorSettingsToStoredFields, so a false value writes nothing. + autoRefreshOnListChanged: obj.autoRefreshOnListChanged === true, // Absent → empty list, matching the read side // (storedFieldsToInspectorSettings). Empty rows are dropped on the way // to disk by inspectorSettingsToStoredFields, so an empty array here diff --git a/core/mcp/serverList.ts b/core/mcp/serverList.ts index 43ac296d..13c33f01 100644 --- a/core/mcp/serverList.ts +++ b/core/mcp/serverList.ts @@ -92,6 +92,7 @@ type StoredInspectorFields = Pick< | "connectionTimeout" | "requestTimeout" | "taskTtl" + | "autoRefreshOnListChanged" | "oauth" | "roots" >; @@ -117,6 +118,7 @@ export function storedFieldsToInspectorSettings( stored.connectionTimeout !== undefined || stored.requestTimeout !== undefined || stored.taskTtl !== undefined || + stored.autoRefreshOnListChanged !== undefined || stored.oauth !== undefined || stored.roots !== undefined; if (!hasAny) return undefined; @@ -133,6 +135,7 @@ export function storedFieldsToInspectorSettings( // Unlike the timeouts (0 = "SDK default"), task TTL has a concrete product // default so the form shows it and "Run as task" has a value to send. taskTtl: stored.taskTtl ?? DEFAULT_TASK_TTL_MS, + autoRefreshOnListChanged: stored.autoRefreshOnListChanged ?? false, // Defaults to an empty list so the form always has a concrete array to // render controlled rows from. An absent on-disk `roots` reads back as // `[]`, which `inspectorSettingsToStoredFields` then omits on write. @@ -194,6 +197,12 @@ export function inspectorSettingsToStoredFields( out.taskTtl = settings.taskTtl; } + // Persist only when enabled — absent reads back as false (above), keeping the + // diff minimal for the common (default-off) case. + if (settings.autoRefreshOnListChanged) { + out.autoRefreshOnListChanged = true; + } + const oauthFields: { clientId?: string; clientSecret?: string; @@ -238,6 +247,7 @@ const INSPECTOR_FIELD_KEY_MAP = { connectionTimeout: true, requestTimeout: true, taskTtl: true, + autoRefreshOnListChanged: true, oauth: true, roots: true, } as const satisfies Record; diff --git a/core/mcp/state/managedPromptsState.ts b/core/mcp/state/managedPromptsState.ts index a53c3101..21dfd917 100644 --- a/core/mcp/state/managedPromptsState.ts +++ b/core/mcp/state/managedPromptsState.ts @@ -13,6 +13,12 @@ const MAX_PAGES = 100; export interface ManagedPromptsStateEventMap { promptsChange: Prompt[]; + /** + * Fires when the "list changed since last refresh" flag flips. True when a + * `prompts/list_changed` notification arrives, false once the user refreshes + * or the connection drops. Drives the sidebar list-changed indicator (#1402). + */ + listChangedChange: boolean; } /** @@ -25,6 +31,7 @@ export class ManagedPromptsState extends TypedEventTarget void) | null = null; private _metadata: Record | undefined = undefined; + private listChanged = false; constructor(client: InspectorClientProtocol) { super(); @@ -33,12 +40,21 @@ export class ManagedPromptsState extends TypedEventTarget { - void this.refresh(); + // When the server opts into auto-refresh (per-server setting), pull the + // new list immediately. Otherwise just flag the change for the indicator + // and let the user pull via Refresh, which is what makes the indicator + // meaningful (#1402). + if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { + void this.refresh(); + } else { + this.setListChanged(true); + } }; const onStatusChange = (): void => { if (this.client?.getStatus() === "disconnected") { this.prompts = []; this.dispatchTypedEvent("promptsChange", []); + this.setListChanged(false); } }; this.client.addEventListener("connect", onConnect); @@ -61,6 +77,26 @@ export class ManagedPromptsState extends TypedEventTarget): void { this._metadata = metadata; } diff --git a/core/mcp/state/managedResourceTemplatesState.ts b/core/mcp/state/managedResourceTemplatesState.ts index 5c30529f..839863f1 100644 --- a/core/mcp/state/managedResourceTemplatesState.ts +++ b/core/mcp/state/managedResourceTemplatesState.ts @@ -1,6 +1,6 @@ /** - * ManagedResourceTemplatesState: holds full resource template list, syncs on - * resourceTemplatesListChanged. + * ManagedResourceTemplatesState: holds the full resource template list, loaded + * on connect and on demand via refresh(). * * Ported from v1.5/main. v2 substitutes `InspectorClientProtocol` for the * concrete `InspectorClient` since the runtime class is not yet ported. @@ -17,9 +17,13 @@ export interface ManagedResourceTemplatesStateEventMap { } /** - * State manager that keeps a full resource template list in sync with the server. - * Subscribes to connect, resourceTemplatesListChanged, and statusChange; - * fetches all pages on refresh. + * State manager that keeps a full resource template list. Subscribes to connect + * (initial load) and statusChange (clear on disconnect); fetches all pages on + * refresh. Templates have no list-changed indicator of their own — the + * Resources screen's indicator (driven by `resourcesListChanged`) covers them, + * and its Refresh re-fetches templates too. So on `resourceTemplatesListChanged` + * it auto-refreshes ONLY when the server opts in via `autoRefreshOnListChanged`; + * otherwise it does nothing and the user pulls via the Resources Refresh (#1402). */ export class ManagedResourceTemplatesState extends TypedEventTarget { private resourceTemplates: ResourceTemplate[] = []; @@ -34,7 +38,13 @@ export class ManagedResourceTemplatesState extends TypedEventTarget { - void this.refresh(); + // Auto-refresh only when the server opts in. With auto-refresh off, + // templates stay as-is until the user pulls via the Resources Refresh — + // templates have no indicator of their own (the Resources indicator, + // driven by `resourcesListChanged`, covers this same notification). + if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { + void this.refresh(); + } }; const onStatusChange = (): void => { if (this.client?.getStatus() === "disconnected") { diff --git a/core/mcp/state/managedResourcesState.ts b/core/mcp/state/managedResourcesState.ts index b103a9d1..e8118022 100644 --- a/core/mcp/state/managedResourcesState.ts +++ b/core/mcp/state/managedResourcesState.ts @@ -13,6 +13,13 @@ const MAX_PAGES = 100; export interface ManagedResourcesStateEventMap { resourcesChange: Resource[]; + /** + * Fires when the "list changed since last refresh" flag flips. True when a + * `resources/list_changed` notification arrives, false once the user + * refreshes or the connection drops. Drives the list-changed indicator + * (#1402). + */ + listChangedChange: boolean; } /** @@ -25,6 +32,7 @@ export class ManagedResourcesState extends TypedEventTarget void) | null = null; private _metadata: Record | undefined = undefined; + private listChanged = false; constructor(client: InspectorClientProtocol) { super(); @@ -33,12 +41,21 @@ export class ManagedResourcesState extends TypedEventTarget { - void this.refresh(); + // When the server opts into auto-refresh (per-server setting), pull the + // new list immediately. Otherwise just flag the change for the indicator + // and let the user pull via Refresh, which is what makes the indicator + // meaningful (#1402). + if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { + void this.refresh(); + } else { + this.setListChanged(true); + } }; const onStatusChange = (): void => { if (this.client?.getStatus() === "disconnected") { this.resources = []; this.dispatchTypedEvent("resourcesChange", []); + this.setListChanged(false); } }; this.client.addEventListener("connect", onConnect); @@ -64,6 +81,26 @@ export class ManagedResourcesState extends TypedEventTarget): void { this._metadata = metadata; } diff --git a/core/mcp/state/managedToolsState.ts b/core/mcp/state/managedToolsState.ts index f28e9bfb..5490004f 100644 --- a/core/mcp/state/managedToolsState.ts +++ b/core/mcp/state/managedToolsState.ts @@ -13,6 +13,12 @@ const MAX_PAGES = 100; export interface ManagedToolsStateEventMap { toolsChange: Tool[]; + /** + * Fires when the "list changed since last refresh" flag flips. True when a + * `tools/list_changed` notification arrives, false once the user refreshes + * or the connection drops. Drives the sidebar list-changed indicator (#1402). + */ + listChangedChange: boolean; } /** @@ -25,6 +31,7 @@ export class ManagedToolsState extends TypedEventTarget void) | null = null; private _metadata: Record | undefined = undefined; + private listChanged = false; constructor(client: InspectorClientProtocol) { super(); @@ -33,12 +40,21 @@ export class ManagedToolsState extends TypedEventTarget { - void this.refresh(); + // When the server opts into auto-refresh (per-server setting), pull the + // new list immediately. Otherwise just flag the change for the indicator + // and let the user pull via Refresh, which is what makes the indicator + // meaningful (#1402). + if (this.client?.getServerSettings()?.autoRefreshOnListChanged) { + void this.refresh(); + } else { + this.setListChanged(true); + } }; const onStatusChange = (): void => { if (this.client?.getStatus() === "disconnected") { this.tools = []; this.dispatchTypedEvent("toolsChange", []); + this.setListChanged(false); } }; this.client.addEventListener("connect", onConnect); @@ -58,6 +74,26 @@ export class ManagedToolsState extends TypedEventTarget): void { this._metadata = metadata; } diff --git a/core/mcp/types.ts b/core/mcp/types.ts index 9c02571a..a432f399 100644 --- a/core/mcp/types.ts +++ b/core/mcp/types.ts @@ -104,6 +104,13 @@ export type StoredMCPServer = MCPServerConfig & { requestTimeout?: number; /** Inspector-specific TTL (ms) for tasks created via "Run as task". */ taskTtl?: number; + /** + * When true, the managed list state auto-refreshes on `list_changed` + * notifications instead of only flagging the list-changed indicator and + * waiting for the user to pull. Inspector-specific. Omitted on disk when + * false (the default). (#1402) + */ + autoRefreshOnListChanged?: boolean; /** * Pre-configured OAuth client credentials for HTTP transports. Nested to * match Claude Code's `.mcp.json` shape; lifted into the flat `oauthClientId` @@ -389,6 +396,12 @@ export interface InspectorServerSettings { oauthClientId?: string; oauthClientSecret?: string; oauthScopes?: string; + /** + * When true, lists auto-refresh on `list_changed` notifications; when + * false (default), the notification only lights the list-changed indicator + * and the user pulls the new list via Refresh. (#1402) + */ + autoRefreshOnListChanged?: boolean; /** * Roots advertised to the server via the `roots` client capability. Each * root carries a required `uri` and an optional `name` (SDK `Root`). The diff --git a/core/react/useManagedPrompts.ts b/core/react/useManagedPrompts.ts index 0e22bb43..762dc599 100644 --- a/core/react/useManagedPrompts.ts +++ b/core/react/useManagedPrompts.ts @@ -9,6 +9,8 @@ import type { TypedEventGeneric } from "../mcp/typedEventTarget.js"; export interface UseManagedPromptsResult { prompts: Prompt[]; + /** True when a `prompts/list_changed` arrived since the last user refresh. */ + listChanged: boolean; refresh: () => Promise; } @@ -22,33 +24,60 @@ export function useManagedPrompts( const [prompts, setPrompts] = useState( managedPromptsState?.getPrompts() ?? [], ); + const [listChanged, setListChanged] = useState( + managedPromptsState?.getListChanged() ?? false, + ); useEffect(() => { if (!managedPromptsState) { setPrompts([]); + setListChanged(false); return; } setPrompts(managedPromptsState.getPrompts()); + setListChanged(managedPromptsState.getListChanged()); const onPromptsChange = ( event: TypedEventGeneric, ) => { setPrompts(event.detail); }; + const onListChangedChange = ( + event: TypedEventGeneric< + ManagedPromptsStateEventMap, + "listChangedChange" + >, + ) => { + setListChanged(event.detail); + }; managedPromptsState.addEventListener("promptsChange", onPromptsChange); + managedPromptsState.addEventListener( + "listChangedChange", + onListChangedChange, + ); return () => { managedPromptsState.removeEventListener( "promptsChange", onPromptsChange, ); + managedPromptsState.removeEventListener( + "listChangedChange", + onListChangedChange, + ); }; }, [managedPromptsState]); const refresh = useCallback(async (): Promise => { if (!managedPromptsState || !client) return []; + // A user-initiated refresh acknowledges the change — clear the indicator + // BEFORE awaiting the fetch, not after. If a `prompts/list_changed` arrives + // mid-fetch, the state re-sets the flag (and auto-refreshes); clearing + // afterward would wipe that genuinely-new signal and the user would miss + // it. Clearing up front acknowledges only the change in hand. + managedPromptsState.clearListChanged(); const next = await managedPromptsState.refresh(); setPrompts(next); return next; }, [client, managedPromptsState]); - return { prompts, refresh }; + return { prompts, listChanged, refresh }; } diff --git a/core/react/useManagedResources.ts b/core/react/useManagedResources.ts index 242a4436..94ffae95 100644 --- a/core/react/useManagedResources.ts +++ b/core/react/useManagedResources.ts @@ -9,6 +9,10 @@ import type { TypedEventGeneric } from "../mcp/typedEventTarget.js"; export interface UseManagedResourcesResult { resources: Resource[]; + /** + * True when a `resources/list_changed` arrived since the last user refresh. + */ + listChanged: boolean; refresh: () => Promise; } @@ -22,13 +26,18 @@ export function useManagedResources( const [resources, setResources] = useState( managedResourcesState?.getResources() ?? [], ); + const [listChanged, setListChanged] = useState( + managedResourcesState?.getListChanged() ?? false, + ); useEffect(() => { if (!managedResourcesState) { setResources([]); + setListChanged(false); return; } setResources(managedResourcesState.getResources()); + setListChanged(managedResourcesState.getListChanged()); const onResourcesChange = ( event: TypedEventGeneric< ManagedResourcesStateEventMap, @@ -37,24 +46,46 @@ export function useManagedResources( ) => { setResources(event.detail); }; + const onListChangedChange = ( + event: TypedEventGeneric< + ManagedResourcesStateEventMap, + "listChangedChange" + >, + ) => { + setListChanged(event.detail); + }; managedResourcesState.addEventListener( "resourcesChange", onResourcesChange, ); + managedResourcesState.addEventListener( + "listChangedChange", + onListChangedChange, + ); return () => { managedResourcesState.removeEventListener( "resourcesChange", onResourcesChange, ); + managedResourcesState.removeEventListener( + "listChangedChange", + onListChangedChange, + ); }; }, [managedResourcesState]); const refresh = useCallback(async (): Promise => { if (!managedResourcesState || !client) return []; + // A user-initiated refresh acknowledges the change — clear the indicator + // BEFORE awaiting the fetch, not after. If a `resources/list_changed` + // arrives mid-fetch, the state re-sets the flag (and auto-refreshes); + // clearing afterward would wipe that genuinely-new signal and the user + // would miss it. Clearing up front acknowledges only the change in hand. + managedResourcesState.clearListChanged(); const next = await managedResourcesState.refresh(); setResources(next); return next; }, [client, managedResourcesState]); - return { resources, refresh }; + return { resources, listChanged, refresh }; } diff --git a/core/react/useManagedTools.ts b/core/react/useManagedTools.ts index bfc4ba2f..333b21e1 100644 --- a/core/react/useManagedTools.ts +++ b/core/react/useManagedTools.ts @@ -7,6 +7,8 @@ import type { TypedEventGeneric } from "../mcp/typedEventTarget.js"; export interface UseManagedToolsResult { tools: Tool[]; + /** True when a `tools/list_changed` arrived since the last user refresh. */ + listChanged: boolean; refresh: () => Promise; } @@ -20,30 +22,54 @@ export function useManagedTools( const [tools, setTools] = useState( managedToolsState?.getTools() ?? [], ); + const [listChanged, setListChanged] = useState( + managedToolsState?.getListChanged() ?? false, + ); useEffect(() => { if (!managedToolsState) { setTools([]); + setListChanged(false); return; } setTools(managedToolsState.getTools()); + setListChanged(managedToolsState.getListChanged()); const onToolsChange = ( event: TypedEventGeneric, ) => { setTools(event.detail); }; + const onListChangedChange = ( + event: TypedEventGeneric, + ) => { + setListChanged(event.detail); + }; managedToolsState.addEventListener("toolsChange", onToolsChange); + managedToolsState.addEventListener( + "listChangedChange", + onListChangedChange, + ); return () => { managedToolsState.removeEventListener("toolsChange", onToolsChange); + managedToolsState.removeEventListener( + "listChangedChange", + onListChangedChange, + ); }; }, [managedToolsState]); const refresh = useCallback(async (): Promise => { if (!managedToolsState || !client) return []; + // A user-initiated refresh acknowledges the change — clear the indicator + // BEFORE awaiting the fetch, not after. If a `tools/list_changed` arrives + // mid-fetch, the state re-sets the flag (and auto-refreshes); clearing + // afterward would wipe that genuinely-new signal and the user would miss + // it. Clearing up front acknowledges only the change in hand. + managedToolsState.clearListChanged(); const next = await managedToolsState.refresh(); setTools(next); return next; }, [client, managedToolsState]); - return { tools, refresh }; + return { tools, listChanged, refresh }; }