feat(web): list-changed handling — indicator, pull-on-demand default, and per-server auto-refresh opt-in (#1402) (#1443)
* 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cliff Hall
GitHub
parent
9b558afee1
commit
b0a024269e
7 changed files with 1109 additions and 93 deletions
|
|
@ -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(
|
||||
<AppControls {...baseProps} onRefreshList={onRefreshList} />,
|
||||
);
|
||||
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(<AppControls {...baseProps} />);
|
||||
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(
|
||||
<AppControls {...baseProps} listChanged onRefreshList={onRefreshList} />,
|
||||
);
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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({
|
|||
<Stack gap="sm">
|
||||
<Group justify="space-between">
|
||||
<Title order={4}>MCP Apps ({tools.length})</Title>
|
||||
<ToolbarButton onClick={onRefreshList}>Refresh</ToolbarButton>
|
||||
<ListChangedIndicator visible={listChanged} onRefresh={onRefreshList} />
|
||||
</Group>
|
||||
<ListChangedIndicator visible={listChanged} onRefresh={onRefreshList} />
|
||||
<TextInput
|
||||
placeholder="Search apps..."
|
||||
value={searchText}
|
||||
|
|
|
|||
|
|
@ -90,6 +90,12 @@ function InteractiveRender(args: ServerSettingsFormProps) {
|
|||
settings: { ...args.settings, [field]: value },
|
||||
});
|
||||
}}
|
||||
onAutoRefreshChange={(value) => {
|
||||
args.onAutoRefreshChange(value);
|
||||
updateArgs({
|
||||
settings: { ...args.settings, autoRefreshOnListChanged: value },
|
||||
});
|
||||
}}
|
||||
onOAuthChange={(oauth) => {
|
||||
args.onOAuthChange(oauth);
|
||||
updateArgs({
|
||||
|
|
@ -110,7 +116,7 @@ const meta: Meta<typeof ServerSettingsForm> = {
|
|||
component: ServerSettingsForm,
|
||||
render: InteractiveRender,
|
||||
args: {
|
||||
expandedSections: ["headers"],
|
||||
expandedSections: ["options"],
|
||||
onExpandedSectionsChange: fn(),
|
||||
onAddHeader: fn(),
|
||||
onRemoveHeader: fn(),
|
||||
|
|
@ -119,6 +125,7 @@ const meta: Meta<typeof ServerSettingsForm> = {
|
|||
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: [
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
<ServerSettingsForm
|
||||
{...baseHandlers}
|
||||
settings={emptySettings}
|
||||
expandedSections={["options"]}
|
||||
/>,
|
||||
);
|
||||
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(
|
||||
<ServerSettingsForm
|
||||
{...baseHandlers}
|
||||
settings={{ ...emptySettings, autoRefreshOnListChanged: true }}
|
||||
expandedSections={["options"]}
|
||||
/>,
|
||||
);
|
||||
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(
|
||||
<ServerSettingsForm
|
||||
{...baseHandlers}
|
||||
onAutoRefreshChange={onAutoRefreshChange}
|
||||
settings={emptySettings}
|
||||
expandedSections={["options"]}
|
||||
/>,
|
||||
);
|
||||
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();
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
>
|
||||
<Accordion.Item value="options">
|
||||
<Accordion.Control>Options</Accordion.Control>
|
||||
<Accordion.Panel>
|
||||
<Stack gap="md">
|
||||
<Checkbox
|
||||
label="Auto Refresh on List Changed Notifications"
|
||||
description="When checked, tool/prompt/resource lists refresh automatically when the server sends a */list_changed notification. When unchecked, the list-changed indicator appears and you refresh on demand."
|
||||
checked={settings.autoRefreshOnListChanged ?? false}
|
||||
onChange={(e) => onAutoRefreshChange(e.currentTarget.checked)}
|
||||
/>
|
||||
</Stack>
|
||||
</Accordion.Panel>
|
||||
</Accordion.Item>
|
||||
|
||||
<Accordion.Item value="headers">
|
||||
<Accordion.Control>Custom Headers</Accordion.Control>
|
||||
<Accordion.Panel>
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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}
|
||||
|
|
|
|||
Loading…
Reference in a new issue