diff --git a/AGENTS.md b/AGENTS.md index 0baea459..a96c06da 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,7 +60,11 @@ v2/main/ │ ├── mcp/ # InspectorClient runtime + state stores │ │ # (modernTaskSchemas.ts: SEP-2663 modern Tasks │ │ # extension wire schemas + normalize/handle helpers, -│ │ # used by the raw-wire tasks/* channel — #1631) +│ │ # used by the raw-wire tasks/* channel — #1631; +│ │ # listSalvage.ts: per-item salvage for list results — +│ │ # keeps the valid entries when one is non-conforming +│ │ # instead of losing the whole list, and owns the +│ │ # shared isClientDecodeRejection predicate — #1909) │ │ ├── import/ # Config import strategies (#1348): client-config parsers │ │ │ # (Claude Desktop/Cursor/Cline/VS Code), registry │ │ │ # server.json parser, strategy registry + well-known diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx index 0869d0d5..8ca842ed 100644 --- a/clients/web/src/App.test.tsx +++ b/clients/web/src/App.test.tsx @@ -598,6 +598,7 @@ const DEFAULT_USE_INSPECTOR_CLIENT: ReturnType = { serverInfo: undefined, instructions: undefined, excludedTools: [], + malformedListItems: [], appRendererClient: null, connect: vi.fn().mockResolvedValue(undefined), disconnect: vi.fn().mockResolvedValue(undefined), diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 509fc079..b42a2e3b 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -990,6 +990,7 @@ function App() { protocolEra, discoverResult, excludedTools, + malformedListItems, lastError, } = useInspectorClient(inspectorClient); const { @@ -4376,6 +4377,7 @@ function App() { latencyMs={latencyMs} tools={tools} excludedTools={excludedTools} + malformedListItems={malformedListItems} prompts={prompts} resources={resources} resourceTemplates={resourceTemplates} diff --git a/clients/web/src/components/elements/MalformedItemsWarning/MalformedItemsWarning.stories.tsx b/clients/web/src/components/elements/MalformedItemsWarning/MalformedItemsWarning.stories.tsx new file mode 100644 index 00000000..756e8667 --- /dev/null +++ b/clients/web/src/components/elements/MalformedItemsWarning/MalformedItemsWarning.stories.tsx @@ -0,0 +1,89 @@ +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { MalformedItemsWarning } from "./MalformedItemsWarning"; + +const meta: Meta = { + title: "Elements/MalformedItemsWarning", + component: MalformedItemsWarning, +}; + +export default meta; +type Story = StoryObj; + +/** The failure this was built for: a PHP server whose empty `annotations` + * object reaches the wire as `[]` (#1909). */ +export const OneDroppedTemplate: Story = { + args: { + method: "resources/templates/list", + what: "resource templates", + items: [ + { + method: "resources/templates/list", + index: 1, + label: "array_annotations", + reason: "annotations: Invalid input: expected object, received array", + }, + ], + }, +}; + +export const SeveralDropped: Story = { + args: { + method: "tools/list", + what: "tools", + items: [ + { + method: "tools/list", + index: 0, + label: "get_weather", + reason: "inputSchema: Invalid input: expected object, received string", + }, + { + method: "tools/list", + index: 4, + label: "add", + reason: "name: Invalid input: expected string, received number", + }, + ], + }, +}; + +/** An entry too broken to carry a name is reported by its position. */ +export const UnlabeledEntry: Story = { + args: { + method: "prompts/list", + what: "prompts", + items: [ + { + method: "prompts/list", + index: 2, + reason: "Invalid input: expected object, received null", + }, + ], + }, +}; + +/** Entries belonging to another list method are ignored — the same set is + * passed to every panel, and each selects its own. */ +export const OtherMethodOnly: Story = { + args: { + method: "resources/templates/list", + what: "resource templates", + items: [ + { + method: "tools/list", + index: 0, + label: "get_weather", + reason: "inputSchema: Invalid input: expected object, received string", + }, + ], + }, +}; + +/** The resting state: a conforming server, nothing rendered. */ +export const Conforming: Story = { + args: { + method: "resources/templates/list", + what: "resource templates", + items: [], + }, +}; diff --git a/clients/web/src/components/elements/MalformedItemsWarning/MalformedItemsWarning.test.tsx b/clients/web/src/components/elements/MalformedItemsWarning/MalformedItemsWarning.test.tsx new file mode 100644 index 00000000..ed8c03f0 --- /dev/null +++ b/clients/web/src/components/elements/MalformedItemsWarning/MalformedItemsWarning.test.tsx @@ -0,0 +1,97 @@ +import { describe, it, expect } from "vitest"; +import { screen } from "@testing-library/react"; +import type { MalformedListItem } from "@inspector/core/mcp"; +import { renderWithMantine } from "../../../test/renderWithMantine"; +import { MalformedItemsWarning } from "./MalformedItemsWarning"; + +const TEMPLATE_ENTRY: MalformedListItem = { + method: "resources/templates/list", + index: 1, + label: "array_annotations", + reason: "annotations: Invalid input: expected object, received array", +}; + +const TOOL_ENTRY: MalformedListItem = { + method: "tools/list", + index: 0, + label: "get_weather", + reason: "inputSchema: Invalid input: expected object, received string", +}; + +describe("MalformedItemsWarning", () => { + it("names the dropped entry and why it was dropped", () => { + renderWithMantine( + , + ); + + expect(screen.getByText("1 malformed entry dropped")).toBeInTheDocument(); + expect(screen.getByText("array_annotations")).toBeInTheDocument(); + expect(screen.getByText(TEMPLATE_ENTRY.reason)).toBeInTheDocument(); + }); + + it("renders nothing when the server was conforming", () => { + renderWithMantine( + , + ); + expect(screen.queryByRole("alert")).not.toBeInTheDocument(); + }); + + it("ignores entries belonging to another list method", () => { + // Every panel receives the whole set and selects its own — a tools failure + // must not surface above the templates list. + renderWithMantine( + , + ); + expect(screen.queryByRole("alert")).not.toBeInTheDocument(); + expect(screen.queryByText("get_weather")).not.toBeInTheDocument(); + }); + + it("pluralizes the count and lists every entry", () => { + renderWithMantine( + , + ); + + expect(screen.getByText("2 malformed entries dropped")).toBeInTheDocument(); + expect(screen.getByText("get_weather")).toBeInTheDocument(); + expect(screen.getByText("add")).toBeInTheDocument(); + // The template entry belongs to the other panel. + expect(screen.queryByText("array_annotations")).not.toBeInTheDocument(); + }); + + it("falls back to the position when the entry had no usable name", () => { + renderWithMantine( + , + ); + expect(screen.getByText("entry 2")).toBeInTheDocument(); + }); +}); diff --git a/clients/web/src/components/elements/MalformedItemsWarning/MalformedItemsWarning.tsx b/clients/web/src/components/elements/MalformedItemsWarning/MalformedItemsWarning.tsx new file mode 100644 index 00000000..26bc5d5c --- /dev/null +++ b/clients/web/src/components/elements/MalformedItemsWarning/MalformedItemsWarning.tsx @@ -0,0 +1,99 @@ +import { Alert, Code, List, ScrollArea, Text } from "@mantine/core"; +import type { MalformedListItem } from "@inspector/core/mcp"; + +export interface MalformedItemsWarningProps { + /** + * Every malformed entry the client is currently reporting, across all list + * methods — pass the hook's set straight through; the component selects its + * own by {@link method}. + */ + items: MalformedListItem[]; + /** The list method this panel renders, e.g. `"resources/templates/list"`. */ + method: string; + /** What the entries are, for the alert title — e.g. "resource templates". */ + what: string; +} + +// Yellow, not red: unlike ListLoadError this is not a failure — the list DID +// load and is rendered below. It reports what the server sent that the +// Inspector could not accept. +const WarningAlert = Alert.withProps({ + color: "yellow", + variant: "light", +}); + +// The entry list stays compact; the reasons are the payload, so they get the +// monospace treatment while the labels stay in body type. +const EntryList = List.withProps({ + size: "xs", + spacing: 4, + withPadding: true, +}); + +const EntryReason = Code.withProps({ + variant: "wrapping", +}); + +// A server can be wrong about many entries at once; cap the height so a badly +// broken list can't push the list itself off the sidebar. +const EntryScroll = ScrollArea.withProps({ + mah: 180, + type: "auto", +}); + +const SummaryText = Text.withProps({ + size: "xs", + mb: 4, +}); + +// The entry's identity, emphasized against its monospaced reason. +const EntryLabel = Text.withProps({ + component: "span", + size: "xs", + fw: 600, +}); + +/** `label` when the entry had one, else its position in the response. */ +function describeEntry(item: MalformedListItem): string { + return item.label ?? `entry ${item.index}`; +} + +/** + * "Some entries were dropped" — the visible half of the #1909 salvage. + * + * The client keeps the valid entries of a list whose result failed schema + * validation, rather than losing the whole list to one bad entry. Dropping them + * silently would trade one lie for another, so what was dropped, and why, is + * reported here beside the list that rendered without them. + */ +export function MalformedItemsWarning({ + items, + method, + what, +}: MalformedItemsWarningProps) { + const mine = items.filter((item) => item.method === method); + if (mine.length === 0) return null; + + return ( + + + The server returned {what} that don't match the MCP schema. The + rest of the list is shown below. + + + + {mine.map((item) => ( + + {describeEntry(item)}{" "} + {item.reason} + + ))} + + + + ); +} diff --git a/clients/web/src/components/groups/PromptControls/PromptControls.tsx b/clients/web/src/components/groups/PromptControls/PromptControls.tsx index 2a2d5364..2fbb5633 100644 --- a/clients/web/src/components/groups/PromptControls/PromptControls.tsx +++ b/clients/web/src/components/groups/PromptControls/PromptControls.tsx @@ -3,6 +3,8 @@ import { ClearButton } from "../../elements/ClearButton/ClearButton"; import type { Prompt } from "@modelcontextprotocol/client"; import { ListChangedIndicator } from "../../elements/ListChangedIndicator/ListChangedIndicator"; import { ListLoadError } from "../../elements/ListLoadError/ListLoadError"; +import { MalformedItemsWarning } from "../../elements/MalformedItemsWarning/MalformedItemsWarning"; +import type { MalformedListItem } from "@inspector/core/mcp"; import { ListPaginationControls, type ListPaginationControlsProps, @@ -41,6 +43,12 @@ export interface PromptControlsProps { * A failed list load, surfaced above the list instead of leaving the panel * empty (which reads as "this server has none") (#1953). */ + /** + * Entries the client dropped from this list because they failed the MCP + * schema. Rendered as a warning above the list, which still shows the rest + * (#1909). + */ + malformedListItems?: MalformedListItem[]; loadError?: Error | null; /** Pagination controls (#1721). */ pagination: ListPaginationControlsProps; @@ -54,6 +62,7 @@ export function PromptControls({ searchText = "", listChanged, onRefreshList, + malformedListItems = [], loadError, pagination, onSearchChange, @@ -83,6 +92,11 @@ export function PromptControls({ /> + {filteredPrompts.map((prompt) => ( diff --git a/clients/web/src/components/groups/ResourceControls/ResourceControls.tsx b/clients/web/src/components/groups/ResourceControls/ResourceControls.tsx index adf5aa1a..6a319fb1 100644 --- a/clients/web/src/components/groups/ResourceControls/ResourceControls.tsx +++ b/clients/web/src/components/groups/ResourceControls/ResourceControls.tsx @@ -14,6 +14,8 @@ import { isModernEra } from "../../elements/EraBadge/eraUtils"; import { SubscriptionStreamBadge } from "../../elements/SubscriptionStreamBadge/SubscriptionStreamBadge"; import { ListChangedIndicator } from "../../elements/ListChangedIndicator/ListChangedIndicator"; import { ListLoadError } from "../../elements/ListLoadError/ListLoadError"; +import { MalformedItemsWarning } from "../../elements/MalformedItemsWarning/MalformedItemsWarning"; +import type { MalformedListItem } from "@inspector/core/mcp"; import { ListPaginationControls, type ListPaginationControlsProps, @@ -73,6 +75,12 @@ export interface ResourceControlsProps { * A failed list load, surfaced above the list instead of leaving the panel * empty (which reads as "this server has none") (#1953). */ + /** + * Entries the client dropped from `resources/list` or + * `resources/templates/list` because they failed the MCP schema. Rendered as + * a warning above the list, which still shows the rest (#1909). + */ + malformedListItems?: MalformedListItem[]; loadError?: Error | null; /** Pagination controls for the Resources list (#1721). */ pagination: ListPaginationControlsProps; @@ -117,6 +125,7 @@ export function ResourceControls({ openSections: controlledOpenSections, listChanged, onRefreshList, + malformedListItems = [], loadError, pagination, onSearchChange, @@ -248,6 +257,16 @@ export function ResourceControls({ what="resources" onRetry={onRefreshList} /> + + {/* Stays inline: Accordion is a compound, `multiple`-discriminated generic, so `.withProps({ multiple: true, ... })` loses its JSX call signature (same tooling limit as Box). */} diff --git a/clients/web/src/components/groups/ToolControls/ToolControls.tsx b/clients/web/src/components/groups/ToolControls/ToolControls.tsx index 4ad70d85..01ad1f04 100644 --- a/clients/web/src/components/groups/ToolControls/ToolControls.tsx +++ b/clients/web/src/components/groups/ToolControls/ToolControls.tsx @@ -15,6 +15,8 @@ import type { Tool } from "@modelcontextprotocol/client"; import type { ExcludedTool } from "@inspector/core/mcp/types.js"; import { ListChangedIndicator } from "../../elements/ListChangedIndicator/ListChangedIndicator"; import { ListLoadError } from "../../elements/ListLoadError/ListLoadError"; +import { MalformedItemsWarning } from "../../elements/MalformedItemsWarning/MalformedItemsWarning"; +import type { MalformedListItem } from "@inspector/core/mcp"; import { ListPaginationControls, type ListPaginationControlsProps, @@ -37,6 +39,12 @@ export interface ToolControlsProps { * A failed list load, surfaced above the list instead of leaving the panel * empty (which reads as "this server has none") (#1953). */ + /** + * Entries the client dropped from this list because they failed the MCP + * schema. Rendered as a warning above the list, which still shows the rest + * (#1909). + */ + malformedListItems?: MalformedListItem[]; loadError?: Error | null; /** Pagination controls (#1721). */ pagination: ListPaginationControlsProps; @@ -127,6 +135,7 @@ export function ToolControls({ searchText = "", listChanged, onRefreshList, + malformedListItems = [], loadError, pagination, onSearchChange, @@ -163,6 +172,11 @@ export function ToolControls({ /> + {filteredTools.map(({ tool, key }) => ( diff --git a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx index c599ed47..8b491cc8 100644 --- a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx +++ b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx @@ -8,6 +8,7 @@ import { Stack, Text, } from "@mantine/core"; +import type { MalformedListItem } from "@inspector/core/mcp"; import type { GetPromptResult, Prompt } from "@modelcontextprotocol/client"; import { PromptControls } from "../../groups/PromptControls/PromptControls"; import type { ListPaginationControlsProps } from "../../elements/ListPaginationControls/ListPaginationControls"; @@ -34,6 +35,11 @@ export interface PromptsScreenProps { onUiChange: (next: PromptsUiState) => void; onRefreshList: () => void; /** A failed list load, rendered above the sidebar list (#1953). */ + /** + * Entries dropped from this screen's list result(s) as malformed; the list + * panel warns about them above the list (#1909). + */ + malformedListItems?: MalformedListItem[]; loadError?: Error | null; /** Pagination controls rendered in the sidebar (#1721). */ pagination: ListPaginationControlsProps; @@ -138,6 +144,7 @@ export function PromptsScreen({ completionsSupported, onUiChange, onRefreshList, + malformedListItems, loadError, pagination, onGetPrompt, @@ -277,6 +284,7 @@ export function PromptsScreen({ listChanged={listChanged} onRefreshList={onRefreshList} loadError={loadError} + malformedListItems={malformedListItems} pagination={pagination} onSearchChange={(value) => onUiChange({ ...ui, search: value })} onSelectPrompt={handleSelectPrompt} diff --git a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx index a94f9169..15c4a838 100644 --- a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx +++ b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx @@ -8,6 +8,7 @@ import { Stack, Text, } from "@mantine/core"; +import type { MalformedListItem } from "@inspector/core/mcp"; import type { ProtocolEra, ReadResourceResult, @@ -58,6 +59,11 @@ export interface ResourcesScreenProps { onUiChange: (next: ResourcesUiState) => void; onRefreshList: () => void; /** A failed list load, rendered above the sidebar list (#1953). */ + /** + * Entries dropped from this screen's list result(s) as malformed; the list + * panel warns about them above the list (#1909). + */ + malformedListItems?: MalformedListItem[]; loadError?: Error | null; /** Pagination controls rendered in the sidebar (#1721). */ pagination: ListPaginationControlsProps; @@ -172,6 +178,7 @@ export function ResourcesScreen({ subscriptionsSupported = true, onUiChange, onRefreshList, + malformedListItems, loadError, pagination, onReadResource, @@ -329,6 +336,7 @@ export function ResourcesScreen({ listChanged={listChanged} onRefreshList={onRefreshList} loadError={loadError} + malformedListItems={malformedListItems} pagination={pagination} onSearchChange={(value) => onUiChange({ ...ui, search: value })} onOpenSectionsChange={(value) => diff --git a/clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx b/clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx index f6dd4ed3..a8afeb76 100644 --- a/clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx +++ b/clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx @@ -1,4 +1,5 @@ import { Card, Flex, Stack, Text } from "@mantine/core"; +import type { MalformedListItem } from "@inspector/core/mcp"; import type { CallToolResult, ReadResourceResult, @@ -50,6 +51,11 @@ export interface ToolsScreenProps { /** Tools the SDK excluded from `tools/list` for invalid `x-mcp-header` * annotations (SEP-2243), shown in the sidebar with the reason (#1632). */ excludedTools?: ExcludedTool[]; + /** + * Entries dropped from a list result as malformed, across every list method. + * Each list panel filters for its own and warns about them (#1909). + */ + malformedListItems?: MalformedListItem[]; callState?: ToolCallState; ui: ToolsUiState; listChanged: boolean; @@ -147,6 +153,7 @@ const EmptyState = Text.withProps({ export function ToolsScreen({ tools, excludedTools, + malformedListItems, callState, ui, listChanged, @@ -191,6 +198,7 @@ export function ToolsScreen({ { + it("is true for a result the client received and refused", () => { + const err = new SdkError(SdkErrorCode.InvalidResult, "bad result"); + expect(isClientDecodeRejection(err)).toBe(true); + }); + + it("is true for a result type the codec cannot handle", () => { + const err = new SdkError(SdkErrorCode.UnsupportedResultType, "unknown"); + expect(isClientDecodeRejection(err)).toBe(true); + }); + + it("is false for a failure that produced no result to salvage", () => { + // A transport drop / timeout: re-listing would just fail again, and there + // is no response frame to pick entries out of. + expect(isClientDecodeRejection(new Error("socket hang up"))).toBe(false); + expect( + isClientDecodeRejection( + new SdkError(SdkErrorCode.RequestTimeout, "timed out"), + ), + ).toBe(false); + }); +}); + +describe("LenientListPageSchema", () => { + it("keeps the primitive array unvalidated while typing the cursor", () => { + const parsed = LenientListPageSchema.parse({ + resourceTemplates: [{ name: "ok" }, { name: 42 }], + nextCursor: "page-2", + }); + expect(parsed.nextCursor).toBe("page-2"); + expect(rawItemsOf(parsed, "resourceTemplates")).toHaveLength(2); + }); + + it("rejects a page whose cursor is malformed", () => { + // The walk itself depends on `nextCursor`, so a bad one is not salvageable + // — the strict error should stand rather than a partial list being kept. + expect( + LenientListPageSchema.safeParse({ tools: [], nextCursor: 7 }).success, + ).toBe(false); + }); + + it("reads an absent or non-array items key as no entries", () => { + expect(rawItemsOf({}, "tools")).toEqual([]); + expect( + rawItemsOf( + LenientListPageSchema.parse({ tools: "not-a-list" }) as never, + "tools", + ), + ).toEqual([]); + }); +}); + +describe("labelForRawItem", () => { + it("prefers a name, then a uriTemplate, uri, or title", () => { + expect(labelForRawItem({ name: "get_weather" })).toBe("get_weather"); + expect(labelForRawItem({ uriTemplate: "file:///{path}" })).toBe( + "file:///{path}", + ); + expect(labelForRawItem({ uri: "test://one" })).toBe("test://one"); + expect(labelForRawItem({ title: "Titled" })).toBe("Titled"); + }); + + it("has no label for an entry too broken to carry one", () => { + expect(labelForRawItem(null)).toBeUndefined(); + expect(labelForRawItem("just a string")).toBeUndefined(); + expect(labelForRawItem(7)).toBeUndefined(); + expect(labelForRawItem({ name: "" })).toBeUndefined(); + expect(labelForRawItem({ name: 42 })).toBeUndefined(); + }); +}); + +describe("describeIssues", () => { + it("names the offending field so the reader doesn't have to hunt", () => { + const result = ItemSchema.safeParse({ name: "t", annotations: [] }); + expect(result.success).toBe(false); + if (result.success) return; + expect(describeIssues(result.error)).toMatch( + /^annotations: .*expected object/i, + ); + }); + + it("names the expected type even when zod's message omits it", () => { + // The browser build reports a bare "Invalid input" where Node spells out + // "expected object, received array" — without the expected type the warning + // says something is wrong but not what would be right. + const terse = new z.ZodError([ + { + code: "invalid_type", + expected: "object", + path: ["annotations"], + message: "Invalid input", + // Double cast: the terse-message shape is what zod's BROWSER build + // emits, and no parse run under Node can produce it — the issue has to + // be hand-built, and `$ZodIssue`'s union doesn't accept a literal. + } as unknown as z.core.$ZodIssue, + ]); + expect(describeIssues(terse)).toBe( + "annotations: Invalid input (expected object)", + ); + }); + + it("does not repeat an expected type the message already carries", () => { + const result = ItemSchema.safeParse({ name: "t", annotations: [] }); + expect(result.success).toBe(false); + if (result.success) return; + const described = describeIssues(result.error); + expect(described.match(/expected object/g)).toHaveLength(1); + }); + + it("omits the path when the failure is the entry itself", () => { + const result = ItemSchema.safeParse("not an object"); + expect(result.success).toBe(false); + if (result.success) return; + const described = describeIssues(result.error); + expect(described).not.toMatch(/^:/); + expect(described).toMatch(/expected object/i); + }); +}); + +describe("salvageListItems", () => { + it("keeps the valid entries and reports only the broken one", () => { + const { valid, malformed } = salvageListItems({ + method: "resources/templates/list", + items: [ + { name: "empty_annotations", annotations: {} }, + { name: "array_annotations", annotations: [] }, + { name: "full_annotations", annotations: { priority: 0.8 } }, + ], + schema: ItemSchema, + }); + + expect(valid.map((item) => item.name)).toEqual([ + "empty_annotations", + "full_annotations", + ]); + expect(malformed).toEqual([ + { + method: "resources/templates/list", + index: 1, + label: "array_annotations", + reason: expect.stringMatching(/^annotations: /), + }, + ]); + }); + + it("reports an unlabelable entry by its position", () => { + const { valid, malformed } = salvageListItems({ + method: "tools/list", + items: [null], + schema: ItemSchema, + }); + expect(valid).toEqual([]); + expect(malformed[0]).toMatchObject({ index: 0 }); + expect(malformed[0]).not.toHaveProperty("label"); + }); + + it("indexes against the aggregated list, not the page", () => { + // Page 2 of a paginated walk: an index that restarted per page would point + // at the wrong entry in the list the user is looking at. + const { malformed } = salvageListItems({ + method: "tools/list", + items: [{ name: "ok" }, { name: 5 }], + schema: ItemSchema, + startIndex: 10, + }); + expect(malformed[0]?.index).toBe(11); + }); + + it("salvages nothing from a wholly conforming page", () => { + const { valid, malformed } = salvageListItems({ + method: "prompts/list", + items: [{ name: "a" }, { name: "b" }], + schema: ItemSchema, + }); + expect(valid).toHaveLength(2); + expect(malformed).toEqual([]); + }); +}); + +describe("summarizeMalformed", () => { + it("names the single dropped entry and why", () => { + expect( + summarizeMalformed([ + { + method: "resources/templates/list", + index: 1, + label: "array_annotations", + reason: "annotations: expected object, received array", + }, + ]), + ).toBe( + "Dropped 1 malformed entry — array_annotations: annotations: expected object, received array", + ); + }); + + it("counts the rest when several failed, falling back to the index", () => { + const summary = summarizeMalformed([ + { method: "tools/list", index: 0, reason: "expected object" }, + { method: "tools/list", index: 3, reason: "expected object" }, + ]); + expect(summary).toContain("Dropped 2 malformed entries"); + expect(summary).toContain("index 0"); + expect(summary).toContain("(+1 more)"); + }); +}); diff --git a/clients/web/src/test/integration/mcp/inspectorClient-malformed-list.test.ts b/clients/web/src/test/integration/mcp/inspectorClient-malformed-list.test.ts new file mode 100644 index 00000000..d864e9b1 --- /dev/null +++ b/clients/web/src/test/integration/mcp/inspectorClient-malformed-list.test.ts @@ -0,0 +1,320 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { + createServer, + type IncomingMessage, + type Server, + type ServerResponse, +} from "node:http"; +import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js"; +import { MessageLogState } from "@inspector/core/mcp/state/index.js"; +import { createTransportNode } from "@inspector/core/mcp/node/transport.js"; + +/** + * Per-item list salvage against a real connection (#1909). + * + * The reported server was PHP, whose empty `annotations` object reaches the + * wire as `[]` rather than `{}` — one non-conforming entry, and the SDK + * rejected the whole `resources/templates/list` result, so every template + * vanished behind "Couldn't load resources". + * + * A hand-rolled JSON-RPC server is used rather than a composable test server + * because the SDK's own server cannot emit these shapes: they are exactly what + * its types forbid. The transport, the SDK client, and the salvage path are all + * real. + */ + +interface ListPage { + items: unknown[]; + nextCursor?: string; +} + +function jsonRpcResult(id: unknown, result: unknown) { + return { jsonrpc: "2.0", id, result }; +} + +/** + * Minimal streamable-HTTP MCP server that answers `initialize` and the list + * methods it was configured with, returning entries verbatim (malformed ones + * included). + */ +function startMalformedServer(initialPages: { + resourceTemplates?: ListPage[]; + tools?: ListPage[]; +}): Promise<{ + url: string; + stop: () => Promise; + calls: string[]; + /** Swap what the server serves next, to model a server that got fixed. */ + setPages: (next: { + resourceTemplates?: ListPage[]; + tools?: ListPage[]; + }) => void; +}> { + const calls: string[] = []; + let pages = initialPages; + + const pageFor = (key: "resourceTemplates" | "tools", cursor?: string) => { + const configured = pages[key] ?? []; + const index = cursor === undefined ? 0 : Number(cursor); + return configured[index] ?? { items: [] }; + }; + + const handler = async (req: IncomingMessage, res: ServerResponse) => { + const chunks: Buffer[] = []; + for await (const chunk of req) chunks.push(chunk as Buffer); + const raw = Buffer.concat(chunks).toString(); + // The SDK also issues bodyless requests (the SSE GET, the session DELETE on + // teardown). Answer them without pretending they're JSON-RPC. + if (raw.length === 0) { + res.writeHead(405).end(); + return; + } + const body = JSON.parse(raw) as { + id?: unknown; + method: string; + params?: { cursor?: string }; + }; + calls.push(body.method); + + // Notifications carry no id and get an empty 202. + if (body.id === undefined) { + res.writeHead(202).end(); + return; + } + + const send = (result: unknown) => { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify(jsonRpcResult(body.id, result))); + }; + + if (body.method === "initialize") { + send({ + protocolVersion: "2025-06-18", + capabilities: { resources: {}, tools: {} }, + serverInfo: { name: "malformed-list-server", version: "1.0.0" }, + }); + return; + } + if (body.method === "resources/templates/list") { + const page = pageFor("resourceTemplates", body.params?.cursor); + send({ + resourceTemplates: page.items, + ...(page.nextCursor !== undefined && { nextCursor: page.nextCursor }), + }); + return; + } + if (body.method === "tools/list") { + const page = pageFor("tools", body.params?.cursor); + send({ + tools: page.items, + ...(page.nextCursor !== undefined && { nextCursor: page.nextCursor }), + }); + return; + } + send({}); + }; + + const server: Server = createServer((req, res) => { + void handler(req, res); + }); + + return new Promise((resolve) => { + server.listen(0, "127.0.0.1", () => { + const address = server.address(); + /* v8 ignore next -- listen() on a fresh server always yields an AddressInfo */ + const port = typeof address === "object" && address ? address.port : 0; + resolve({ + url: `http://127.0.0.1:${port}/mcp`, + calls, + setPages: (next) => { + pages = next; + }, + stop: () => + new Promise((done) => { + server.close(() => done()); + }), + }); + }); + }); +} + +const VALID_TEMPLATE = { + name: "full_annotations", + uriTemplate: "annotated://full/{id}", + annotations: { audience: ["user"], priority: 0.8 }, +}; +// The #1909 shape: `[]` where the spec says object. +const PHP_EMPTY_ANNOTATIONS = { + name: "array_annotations", + uriTemplate: "annotated://array/{id}", + annotations: [], +}; +const EMPTY_OBJECT_ANNOTATIONS = { + name: "empty_annotations", + uriTemplate: "annotated://empty/{id}", + annotations: {}, +}; + +describe("InspectorClient list salvage (#1909)", () => { + let client: InspectorClient | null = null; + let stopServer: (() => Promise) | null = null; + + async function connectTo(url: string) { + client = new InspectorClient( + { type: "streamable-http", url }, + { environment: { transport: createTransportNode } }, + ); + await client.connect(); + return client; + } + + beforeEach(() => { + client = null; + stopServer = null; + }); + + afterEach(async () => { + try { + await client?.disconnect(); + } catch { + // Teardown only — the assertions already ran. + } + client = null; + await stopServer?.(); + stopServer = null; + }); + + it("keeps the valid templates and reports the malformed one", async () => { + const server = await startMalformedServer({ + resourceTemplates: [ + { + items: [ + EMPTY_OBJECT_ANNOTATIONS, + PHP_EMPTY_ANNOTATIONS, + VALID_TEMPLATE, + ], + }, + ], + }); + stopServer = server.stop; + const connected = await connectTo(server.url); + + const { resourceTemplates } = await connected.listAllResourceTemplates(); + + // Before the fix this threw, and the panel showed zero templates. + expect(resourceTemplates.map((t) => t.name)).toEqual([ + "empty_annotations", + "full_annotations", + ]); + expect(connected.getMalformedListItems()).toEqual([ + { + method: "resources/templates/list", + index: 1, + label: "array_annotations", + reason: expect.stringMatching(/^annotations: /), + }, + ]); + }); + + it("accepts an empty annotations object without salvaging at all", async () => { + // `{}` is legal — every Annotations field is optional — so this must take + // the strict path and report nothing dropped. + const server = await startMalformedServer({ + resourceTemplates: [{ items: [EMPTY_OBJECT_ANNOTATIONS] }], + }); + stopServer = server.stop; + const connected = await connectTo(server.url); + + const { resourceTemplates } = await connected.listAllResourceTemplates(); + expect(resourceTemplates).toHaveLength(1); + expect(connected.getMalformedListItems()).toEqual([]); + }); + + it("emits malformedListItemsChange so the UI can warn", async () => { + const server = await startMalformedServer({ + resourceTemplates: [{ items: [PHP_EMPTY_ANNOTATIONS, VALID_TEMPLATE] }], + }); + stopServer = server.stop; + const connected = await connectTo(server.url); + + const seen: unknown[] = []; + connected.addEventListener("malformedListItemsChange", (event) => { + seen.push(event.detail); + }); + await connected.listAllResourceTemplates(); + + expect(seen).toHaveLength(1); + expect(seen[0]).toMatchObject([{ label: "array_annotations" }]); + }); + + it("marks the Protocol entry rejected even though the list rendered", async () => { + const server = await startMalformedServer({ + resourceTemplates: [{ items: [PHP_EMPTY_ANNOTATIONS, VALID_TEMPLATE] }], + }); + stopServer = server.stop; + const connected = await connectTo(server.url); + const log = new MessageLogState(connected); + + await connected.listAllResourceTemplates(); + + const rejected = log + .getMessages() + .filter((entry) => entry.clientError !== undefined); + expect(rejected.length).toBeGreaterThan(0); + expect(rejected.at(-1)?.clientError).toContain("Dropped 1 malformed entry"); + log.destroy(); + }); + + it("salvages across pages, indexing against the aggregate", async () => { + const server = await startMalformedServer({ + tools: [ + { + items: [{ name: "ok_one", inputSchema: { type: "object" } }], + nextCursor: "1", + }, + { items: [{ name: "broken", inputSchema: "not-a-schema" }] }, + ], + }); + stopServer = server.stop; + const connected = await connectTo(server.url); + + const { tools } = await connected.listAllTools(); + expect(tools.map((t) => t.name)).toEqual(["ok_one"]); + expect(connected.getMalformedListItems()).toMatchObject([ + { method: "tools/list", index: 1, label: "broken" }, + ]); + }); + + it("clears a previous report once the server answers cleanly", async () => { + const server = await startMalformedServer({ + resourceTemplates: [{ items: [PHP_EMPTY_ANNOTATIONS, VALID_TEMPLATE] }], + }); + stopServer = server.stop; + const connected = await connectTo(server.url); + + await connected.listAllResourceTemplates(); + expect(connected.getMalformedListItems()).toHaveLength(1); + + // The server is fixed; the next refresh must clear the stale report rather + // than leaving a warning about entries that are no longer wrong. + server.setPages({ resourceTemplates: [{ items: [VALID_TEMPLATE] }] }); + await connected.listAllResourceTemplates({ cacheMode: "bypass" }); + expect(connected.getMalformedListItems()).toEqual([]); + }); + + it("rethrows when the rejection is not about any single entry", async () => { + // Every entry validates; the result is bad for another reason (a cursor of + // the wrong type). There is no per-item story, so the original error must + // survive rather than being swallowed into a partial list. + const server = await startMalformedServer({ + resourceTemplates: [ + { items: [VALID_TEMPLATE], nextCursor: 42 as unknown as string }, + ], + }); + stopServer = server.stop; + const connected = await connectTo(server.url); + + await expect(connected.listAllResourceTemplates()).rejects.toThrow(); + expect(connected.getMalformedListItems()).toEqual([]); + }); +}); diff --git a/core/mcp/__tests__/fakeInspectorClient.ts b/core/mcp/__tests__/fakeInspectorClient.ts index 00e3ed16..728ef5f7 100644 --- a/core/mcp/__tests__/fakeInspectorClient.ts +++ b/core/mcp/__tests__/fakeInspectorClient.ts @@ -38,6 +38,7 @@ import type { ExcludedTool, } from "../types.js"; import { INACTIVE_SUBSCRIPTION_STREAM_STATE } from "../types.js"; +import type { MalformedListItem } from "../listSalvage.js"; import type { JsonValue } from "../../json/jsonUtils.js"; type ListResult = { @@ -282,6 +283,18 @@ export class FakeInspectorClient this.dispatchTypedEvent("excludedToolsChange", excluded); } + private malformedListItems: MalformedListItem[] = []; + + getMalformedListItems(): MalformedListItem[] { + return this.malformedListItems; + } + + /** Test helper: set the malformed-entry set and emit the change (#1909). */ + setMalformedListItems(malformed: MalformedListItem[]): void { + this.malformedListItems = malformed; + this.dispatchTypedEvent("malformedListItemsChange", malformed); + } + getDiscoverResult(): DiscoverResult | undefined { return this.discoverResult; } diff --git a/core/mcp/index.ts b/core/mcp/index.ts index f8e37cd3..e83f9a5c 100644 --- a/core/mcp/index.ts +++ b/core/mcp/index.ts @@ -42,6 +42,9 @@ export type { ToolCallInvocation, } from "./types.js"; +// Per-item list salvage (#1909) +export type { MalformedListItem } from "./listSalvage.js"; + // Re-export JSON utilities export type { JsonValue } from "../json/jsonUtils.js"; export { diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index b0b809f3..e37994d5 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -159,10 +159,26 @@ import { ListResourcesResultSchema, ListResourceTemplatesResultSchema, ListPromptsResultSchema, + // Per-primitive item schemas — the salvage fallback validates a list's + // entries one at a time with these when the whole-result parse rejects + // (#1909, see `listAllSalvaging`). + ToolSchema, + ResourceSchema, + ResourceTemplateSchema, + PromptSchema, } from "@modelcontextprotocol/core"; import type { ClientResult } from "@modelcontextprotocol/client"; import { AjvJsonSchemaValidator } from "@modelcontextprotocol/client/validators/ajv"; +import { z } from "zod/v4"; import { validateToolOutput } from "./toolOutputValidation.js"; +import { + LenientListPageSchema, + isClientDecodeRejection, + rawItemsOf, + salvageListItems, + summarizeMalformed, + type MalformedListItem, +} from "./listSalvage.js"; import { TasksListChangedNotificationSchema } from "./taskNotificationSchemas.js"; import { type JsonValue, @@ -407,6 +423,11 @@ export class InspectorClient extends InspectorClientEventTarget { // annotations (SEP-2243), recomputed on every aggregate tools refresh and // surfaced so the Tools tab can show why a tool vanished (#1632). private excludedTools: ExcludedTool[] = []; + // Entries the Inspector dropped from a list result because they failed the + // spec schema for that primitive, keyed by list method. Populated only by the + // salvage fallback in `listAllSalvaging` — empty against a conforming server + // (#1909). + private malformedListItems = new Map(); // The capabilities this Inspector client advertises to the server during the // initialize handshake. Built once in setupClient() and snapshotted here so // UI surfaces (Server Info modal) can display them without poking at the @@ -2211,6 +2232,7 @@ export class InspectorClient extends InspectorClientEventTarget { this.protocolEra = undefined; this.discoverResult = undefined; this.excludedTools = []; + this.malformedListItems.clear(); // Read as "not opted in" while disconnected. This is no longer what stops // it leaking into the next connection — `resetSessionState()` re-derives it // at connect, so removing this would leak nothing (#1629). Note the web @@ -2230,6 +2252,7 @@ export class InspectorClient extends InspectorClientEventTarget { this.dispatchTypedEvent("protocolEraChange", this.protocolEra); this.dispatchTypedEvent("discoverResultChange", this.discoverResult); this.dispatchTypedEvent("excludedToolsChange", this.excludedTools); + this.dispatchTypedEvent("malformedListItemsChange", []); } /** @@ -3126,12 +3149,20 @@ export class InspectorClient extends InspectorClientEventTarget { if (!this.client) { throw new Error("Client is not connected"); } - const response = await this.invokeMcpClient(() => - this.client!.listTools( - this.aggregateListParams(options?.metadata), - this.getCacheableRequestOptions(options?.cacheMode), - ), - ); + const tools = await this.listAllSalvaging({ + method: "tools/list", + itemsKey: "tools", + itemSchema: ToolSchema, + metadata: options?.metadata, + aggregate: async () => [ + ...( + await this.client!.listTools( + this.aggregateListParams(options?.metadata), + this.getCacheableRequestOptions(options?.cacheMode), + ) + ).tools, + ], + }); // Recompute the SEP-2243 excluded-tools set alongside the aggregate. The // SDK already filtered `response.tools`, so it can't tell us what it // dropped — {@link refreshExcludedTools} re-lists the RAW `tools/list` to @@ -3153,7 +3184,7 @@ export class InspectorClient extends InspectorClientEventTarget { "Excluded-tools walk failed; the SEP-2243 excluded list may be incomplete", ); }); - return { tools: [...response.tools] }; + return { tools }; } /** @@ -3187,6 +3218,146 @@ export class InspectorClient extends InspectorClientEventTarget { this.dispatchTypedEvent("responseRejected", { id, reason }); } + /** Every entry dropped from a list result as malformed, across methods (#1909). */ + getMalformedListItems(): MalformedListItem[] { + return [...this.malformedListItems.values()].flat(); + } + + /** + * Record (or clear) the malformed entries for one list method and notify. + * + * A no-op when the method had none and still has none, so the common case — + * every conforming refresh, of which there are many — costs no event and no + * re-render. + */ + private setMalformedListItems( + method: string, + malformed: MalformedListItem[], + ): void { + const previous = this.malformedListItems.get(method) ?? []; + if (previous.length === 0 && malformed.length === 0) return; + if (malformed.length === 0) this.malformedListItems.delete(method); + else this.malformedListItems.set(method, malformed); + this.dispatchTypedEvent( + "malformedListItemsChange", + this.getMalformedListItems(), + ); + } + + /** + * Run a list aggregate, falling back to a per-item salvage when the SDK + * rejects the result it received (#1909). + * + * The strict aggregate runs first and unchanged, so a conforming server pays + * nothing: no extra request, no extra parse, and the SDK's response cache + * still serves it. Only an `InvalidResult` rejection — a response in hand + * that failed validation — reaches {@link salvageList}; every other failure + * (transport drop, timeout, server-sent error) rethrows untouched, because + * re-listing would not produce anything to salvage. + */ + private async listAllSalvaging({ + method, + itemsKey, + itemSchema, + aggregate, + metadata, + }: { + method: string; + itemsKey: string; + itemSchema: z.ZodType; + aggregate: () => Promise; + metadata?: Record; + }): Promise { + try { + const items = await this.invokeMcpClient(aggregate, { method }); + this.setMalformedListItems(method, []); + return items; + } catch (err) { + if (!isClientDecodeRejection(err)) throw err; + return this.salvageList({ method, itemsKey, itemSchema, metadata, err }); + } + } + + /** + * Re-walk a list method with pagination validated but entries left raw, then + * validate each entry on its own — keeping the good ones and reporting the + * rest (#1909). + * + * Two deliberate properties: + * + * - **The original error wins when nothing is salvageable.** If every entry + * validates here, the strict rejection was about something else (a + * top-level field, an era codec decision), so there is no per-item story to + * tell and swallowing it would hide a real failure. Rethrow the original. + * - **A salvaged response is still a rejected response.** The Protocol entry + * is marked (#1953) with what was dropped, so the wire truth survives even + * though the list now renders. + * + * This walk is deliberately un-cached (it uses the raw per-page path), which + * is fine: it only runs against a server that just returned a non-conforming + * result. + */ + private async salvageList({ + method, + itemsKey, + itemSchema, + metadata, + err, + }: { + method: string; + itemsKey: string; + itemSchema: z.ZodType; + metadata?: Record; + err: unknown; + }): Promise { + const valid: T[] = []; + const malformed: MalformedListItem[] = []; + const seenCursors = new Set(); + let cursor: string | undefined; + do { + const effectiveMeta = this.mergeMeta(metadata); + const params = { + ...(effectiveMeta ? { _meta: effectiveMeta } : {}), + ...(cursor ? { cursor } : {}), + }; + const page = await this.invokeMcpClient( + () => + this.client!.request( + { method, params }, + LenientListPageSchema, + this.getRequestOptions(metadata?.progressToken), + ), + { method }, + ); + const salvaged = salvageListItems({ + method, + items: rawItemsOf(page, itemsKey), + schema: itemSchema, + startIndex: valid.length + malformed.length, + }); + valid.push(...salvaged.valid); + malformed.push(...salvaged.malformed); + cursor = page.nextCursor; + if (cursor !== undefined) { + /* v8 ignore next -- defensive: a spec-compliant server never repeats a cursor; this guards a non-converging server from an infinite walk (mirrors the SDK's drainList guard) */ + if (seenCursors.has(cursor)) break; + seenCursors.add(cursor); + } + } while (cursor !== undefined); + + // Nothing per-item was wrong, so the strict rejection was about something + // this fallback cannot explain — surface it rather than hide it. + if (malformed.length === 0) throw err; + + this.markResponseRejected(method, summarizeMalformed(malformed)); + this.setMalformedListItems(method, malformed); + this.logger.warn( + { method, malformed }, + "Dropped malformed entries from a list result; the rest of the list was kept", + ); + return valid; + } + /** The current SEP-2243 excluded-tools set (empty on legacy/stdio). */ getExcludedTools(): ExcludedTool[] { return this.excludedTools; @@ -4347,13 +4518,21 @@ export class InspectorClient extends InspectorClientEventTarget { if (!this.client) { throw new Error("Client is not connected"); } - const response = await this.invokeMcpClient(() => - this.client!.listResources( - this.aggregateListParams(options?.metadata), - this.getCacheableRequestOptions(options?.cacheMode), - ), - ); - return { resources: [...response.resources] }; + const resources = await this.listAllSalvaging({ + method: "resources/list", + itemsKey: "resources", + itemSchema: ResourceSchema, + metadata: options?.metadata, + aggregate: async () => [ + ...( + await this.client!.listResources( + this.aggregateListParams(options?.metadata), + this.getCacheableRequestOptions(options?.cacheMode), + ) + ).resources, + ], + }); + return { resources }; } /** @@ -4504,15 +4683,21 @@ export class InspectorClient extends InspectorClientEventTarget { if (!this.client) { throw new Error("Client is not connected"); } - const response = await this.invokeMcpClient( - () => - this.client!.listResourceTemplates( - this.aggregateListParams(options?.metadata), - this.getCacheableRequestOptions(options?.cacheMode), - ), - { method: "resources/templates/list" }, - ); - return { resourceTemplates: [...response.resourceTemplates] }; + const resourceTemplates = await this.listAllSalvaging({ + method: "resources/templates/list", + itemsKey: "resourceTemplates", + itemSchema: ResourceTemplateSchema, + metadata: options?.metadata, + aggregate: async () => [ + ...( + await this.client!.listResourceTemplates( + this.aggregateListParams(options?.metadata), + this.getCacheableRequestOptions(options?.cacheMode), + ) + ).resourceTemplates, + ], + }); + return { resourceTemplates }; } /** @@ -4559,13 +4744,21 @@ export class InspectorClient extends InspectorClientEventTarget { if (!this.client) { throw new Error("Client is not connected"); } - const response = await this.invokeMcpClient(() => - this.client!.listPrompts( - this.aggregateListParams(options?.metadata), - this.getCacheableRequestOptions(options?.cacheMode), - ), - ); - return { prompts: [...response.prompts] }; + const prompts = await this.listAllSalvaging({ + method: "prompts/list", + itemsKey: "prompts", + itemSchema: PromptSchema, + metadata: options?.metadata, + aggregate: async () => [ + ...( + await this.client!.listPrompts( + this.aggregateListParams(options?.metadata), + this.getCacheableRequestOptions(options?.cacheMode), + ) + ).prompts, + ], + }); + return { prompts }; } /** diff --git a/core/mcp/inspectorClientEventTarget.ts b/core/mcp/inspectorClientEventTarget.ts index d4edee16..572909c8 100644 --- a/core/mcp/inspectorClientEventTarget.ts +++ b/core/mcp/inspectorClientEventTarget.ts @@ -26,6 +26,7 @@ import type { ResourceSubscriptionStreamState, ExcludedTool, } from "./types.js"; +import type { MalformedListItem } from "./listSalvage.js"; import type { Tool, ServerCapabilities, @@ -62,6 +63,12 @@ export interface InspectorClientEventMap { * connections (which don't exclude) and before connect (#1632). */ excludedToolsChange: ExcludedTool[]; + /** + * Entries dropped from a list result because they failed the spec schema for + * their primitive, across every list method. Empty against a conforming + * server; a non-empty set means the list rendered without them (#1909). + */ + malformedListItemsChange: MalformedListItem[]; capabilitiesChange: ServerCapabilities | undefined; serverInfoChange: Implementation | undefined; instructionsChange: string | undefined; diff --git a/core/mcp/inspectorClientProtocol.ts b/core/mcp/inspectorClientProtocol.ts index 6a5322ea..756b560f 100644 --- a/core/mcp/inspectorClientProtocol.ts +++ b/core/mcp/inspectorClientProtocol.ts @@ -37,6 +37,7 @@ import type { Tool, } from "@modelcontextprotocol/client"; import type { JsonValue } from "../json/jsonUtils.js"; +import type { MalformedListItem } from "./listSalvage.js"; import type { InspectorClientEventTarget } from "./inspectorClientEventTarget.js"; import type { SamplingCreateMessage } from "./samplingCreateMessage.js"; import type { ElicitationCreateMessage } from "./elicitationCreateMessage.js"; @@ -65,6 +66,9 @@ export interface InspectorClientProtocol extends InspectorClientEventTarget { /** Tools excluded from `tools/list` for invalid `x-mcp-header` annotations * (SEP-2243); empty on legacy/stdio connections (#1632). */ getExcludedTools(): ExcludedTool[]; + /** Entries dropped from a list result as malformed, across every list method; + * empty against a conforming server (#1909). */ + getMalformedListItems(): MalformedListItem[]; getResourceSubscriptionStreamState(): ResourceSubscriptionStreamState; getDiscoverResult(): DiscoverResult | undefined; getServerSettings(): InspectorServerSettings | undefined; diff --git a/core/mcp/listSalvage.ts b/core/mcp/listSalvage.ts new file mode 100644 index 00000000..a144c2bd --- /dev/null +++ b/core/mcp/listSalvage.ts @@ -0,0 +1,199 @@ +/** + * Per-item salvage for paginated list results (#1909). + * + * The SDK validates a list result as a whole: one non-conforming element + * rejects the entire response, so a server with a single malformed entry + * appears to have NO tools/resources/templates/prompts at all. That is exactly + * backwards for a debugging tool — the one thing the user needs to see is which + * entry is wrong and why, and the other entries are still perfectly usable. + * + * A real report: a PHP server whose empty `annotations` object reaches the wire + * as `[]` (`json_encode` of an empty array, or of `(array) new stdClass()`) + * instead of `{}` made every resource template vanish behind "Couldn't load + * resources" (#1909). + * + * The strict path is left alone. `InspectorClient.listAll*` still calls the + * SDK's cache-aware aggregate first, and only falls back to a lenient re-walk + * when that rejects a result it received — so a conforming server pays nothing, + * and the fallback is a pure recovery path. + * + * Salvaging does NOT mean accepting: a salvaged entry is dropped from the list + * and reported as {@link MalformedListItem}, and the Protocol entry is still + * marked rejected. The Inspector keeps telling the truth about the response; it + * just stops throwing away the valid part of it. + */ + +import { z } from "zod/v4"; +import { SdkError, SdkErrorCode } from "@modelcontextprotocol/client"; + +/** + * One entry the Inspector dropped from a list result because it failed the + * spec's schema for that primitive. + */ +export interface MalformedListItem { + /** The list method that carried it, e.g. `"resources/templates/list"`. */ + method: string; + /** + * Zero-based position in the aggregated list. An entry too broken to carry a + * name is still identifiable by where it sat in the response. + */ + index: number; + /** Best-effort label read off the raw entry, when it has a usable one. */ + label?: string; + /** Why it was rejected — the first validation issue, with its field path. */ + reason: string; +} + +/** + * Whether a failed fetch is the CLIENT refusing a response it received, rather + * than the request never producing one. + * + * This gates both `markResponseRejected` (#1953) and the salvage fallback + * (#1909), and the distinction is load-bearing for each. For the Protocol-entry + * marking: the correlation recovers the request id as "the last response + * received for this method", which is only the failing exchange when a response + * actually just arrived and was refused while decoding. A transport drop, a + * timeout, or an aborted request produces no response frame at all — the + * last-answered id then still points at some EARLIER, successful call, and + * marking it would stamp "Rejected by the Inspector" onto an exchange that + * succeeded. For the salvage fallback: re-walking the list only makes sense + * when there IS a result to salvage; re-issuing the request after a timeout + * would just double the wait before failing anyway. + * + * A server-sent JSON-RPC error is excluded for a different reason: it is a real + * response, so the id would be right, but the failure is the server's. Its + * entry already renders as an error from the error frame itself, and blaming + * the Inspector for it would misattribute the cause. + * + * `SdkErrorCode.InvalidResult` is exactly "a result arrived and failed + * validation for the negotiated era"; `UnsupportedResultType` is its sibling + * for a `resultType` the codec has no handling for. Both are decisions the + * client made about a frame in hand. + */ +export function isClientDecodeRejection(err: unknown): boolean { + return ( + SdkError.isInstance(err) && + (err.code === SdkErrorCode.InvalidResult || + err.code === SdkErrorCode.UnsupportedResultType) + ); +} + +/** + * A list page parsed with everything except pagination left unvalidated. + * + * `looseObject` keeps the unknown keys, so the primitive array (`tools`, + * `resources`, `resourceTemplates`, `prompts`) survives as `unknown` for + * per-item validation. Only `nextCursor` is typed, because the walk itself + * depends on it — if THAT is malformed the page is unusable and the strict + * error should stand. + */ +export const LenientListPageSchema = z.looseObject({ + nextCursor: z.string().optional(), +}); + +export type LenientListPage = z.infer; + +/** Read the primitive array out of a leniently-parsed page. */ +export function rawItemsOf(page: LenientListPage, itemsKey: string): unknown[] { + const value = (page as Record)[itemsKey]; + return Array.isArray(value) ? value : []; +} + +/** + * Best-effort human label for a raw entry: whichever identifying string field + * it happens to carry. Undefined when the entry is too broken to have one (a + * bare string, a number, a null) — the caller falls back to the index. + */ +export function labelForRawItem(value: unknown): string | undefined { + if (typeof value !== "object" || value === null) return undefined; + const record = value as Record; + for (const key of ["name", "uriTemplate", "uri", "title"]) { + const candidate = record[key]; + if (typeof candidate === "string" && candidate.length > 0) return candidate; + } + return undefined; +} + +/** + * One-line summary of why an entry failed: the first issue, prefixed with its + * field path so `annotations` is named rather than left for the reader to find + * in a wall of JSON. The path is relative to the entry, not the response, since + * the entry is what we're reporting on. + */ +export function describeIssues(error: z.ZodError): string { + const [issue] = error.issues; + /* v8 ignore next -- zod never produces a failed parse with zero issues; guards against an empty message if it ever did */ + if (!issue) return "did not match the expected shape"; + const path = issue.path.join("."); + const detail = expectedDetail(issue); + const message = detail ? `${issue.message} (${detail})` : issue.message; + return path.length > 0 ? `${path}: ${message}` : message; +} + +/** + * `expected object` — appended because the issue's own message cannot be relied + * on to carry it. zod's browser build reports a bare "Invalid input" where Node + * says "Invalid input: expected object, received array", and the expected type + * is the whole diagnostic: without it the warning says something is wrong but + * not what would be right. Skipped when the message already spells it out, so + * the Node text doesn't end up saying it twice. + */ +function expectedDetail(issue: z.core.$ZodIssue): string | undefined { + if (!("expected" in issue) || typeof issue.expected !== "string") { + return undefined; + } + const detail = `expected ${issue.expected}`; + return issue.message.includes(detail) ? undefined : detail; +} + +/** + * Validate a page's entries one at a time, keeping the ones that pass. + * + * `startIndex` is the running count across pages, so an index in the report + * refers to the aggregated list the user sees rather than resetting per page. + */ +export function salvageListItems({ + method, + items, + schema, + startIndex = 0, +}: { + method: string; + items: unknown[]; + schema: z.ZodType; + startIndex?: number; +}): { valid: T[]; malformed: MalformedListItem[] } { + const valid: T[] = []; + const malformed: MalformedListItem[] = []; + items.forEach((item, offset) => { + const parsed = schema.safeParse(item); + if (parsed.success) { + valid.push(parsed.data); + return; + } + const label = labelForRawItem(item); + malformed.push({ + method, + index: startIndex + offset, + ...(label !== undefined && { label }), + reason: describeIssues(parsed.error), + }); + }); + return { valid, malformed }; +} + +/** + * Summary line used for the Protocol entry's rejection reason, so the Network / + * Protocol view still says the response was non-conforming even though the list + * rendered (#1953 + #1909). + */ +export function summarizeMalformed(malformed: MalformedListItem[]): string { + const [first] = malformed; + /* v8 ignore next -- callers only summarize a non-empty set */ + if (!first) return ""; + const where = first.label ?? `index ${first.index}`; + const rest = malformed.length > 1 ? ` (+${malformed.length - 1} more)` : ""; + return `Dropped ${malformed.length} malformed ${ + malformed.length === 1 ? "entry" : "entries" + } — ${where}: ${first.reason}${rest}`; +} diff --git a/core/mcp/state/managedListState.ts b/core/mcp/state/managedListState.ts index ea5128b5..eaf51f5d 100644 --- a/core/mcp/state/managedListState.ts +++ b/core/mcp/state/managedListState.ts @@ -17,7 +17,7 @@ import type { CacheMode, ServerCapabilities, } from "@modelcontextprotocol/client"; -import { SdkError, SdkErrorCode } from "@modelcontextprotocol/client"; +import { isClientDecodeRejection } from "../listSalvage.js"; import { isTerminalStatus } from "../types.js"; import { TypedEventTarget } from "../typedEventTarget.js"; @@ -30,38 +30,6 @@ import { TypedEventTarget } from "../typedEventTarget.js"; */ export const DEFAULT_LIST_CHANGED_DEBOUNCE_MS = 250; -/** - * Whether a failed fetch is the CLIENT refusing a response it received, rather - * than the request never producing one. - * - * This gates `markResponseRejected`, and the distinction is load-bearing. That - * correlation recovers the request id as "the last response received for this - * method", which is only the failing exchange when a response actually just - * arrived and was refused while decoding. A transport drop, a timeout, or an - * aborted request produces no response frame at all — the last-answered id then - * still points at some EARLIER, successful call, and marking it would stamp - * "Rejected by the Inspector" onto an exchange that succeeded. That is the very - * class of lie this issue exists to remove, so it must not be traded for - * another one. - * - * A server-sent JSON-RPC error is excluded for a different reason: it is a real - * response, so the id would be right, but the failure is the server's. Its - * entry already renders as an error from the error frame itself, and blaming - * the Inspector for it would misattribute the cause. - * - * `SdkErrorCode.InvalidResult` is exactly "a result arrived and failed - * validation for the negotiated era"; `UnsupportedResultType` is its sibling - * for a `resultType` the codec has no handling for. Both are decisions the - * client made about a frame in hand. - */ -function isClientDecodeRejection(err: unknown): boolean { - return ( - SdkError.isInstance(err) && - (err.code === SdkErrorCode.InvalidResult || - err.code === SdkErrorCode.UnsupportedResultType) - ); -} - /** * Every managed-list event map carries the list-changed indicator event and the * last-fetch error (#1953). diff --git a/core/react/useInspectorClient.ts b/core/react/useInspectorClient.ts index db0910c3..5e9de64a 100644 --- a/core/react/useInspectorClient.ts +++ b/core/react/useInspectorClient.ts @@ -11,6 +11,7 @@ import type { DiscoverResult, } from "@modelcontextprotocol/client"; import type { ExcludedTool } from "../mcp/types.js"; +import type { MalformedListItem } from "../mcp/listSalvage.js"; // Module-scope frozen object so the `?? EMPTY_CLIENT_CAPABILITIES` // fallback below doesn't return a fresh literal on every render — @@ -44,6 +45,12 @@ export interface UseInspectorClientResult { * connections and before connect (#1632). */ excludedTools: ExcludedTool[]; + /** + * Entries the Inspector dropped from a list result because they failed the + * spec schema for their primitive. Empty against a conforming server; a + * non-empty set means that list rendered without them (#1909). + */ + malformedListItems: MalformedListItem[]; /** * Message from the most recent mid-session transport failure (the client's * `error` event — stdio crash, SSE drop, HTTP 5xx). Stays set until the next @@ -97,6 +104,9 @@ export function useInspectorClient( const [excludedTools, setExcludedTools] = useState( inspectorClient?.getExcludedTools() ?? [], ); + const [malformedListItems, setMalformedListItems] = useState< + MalformedListItem[] + >(inspectorClient?.getMalformedListItems() ?? []); const [lastError, setLastError] = useState(undefined); useEffect(() => { @@ -109,6 +119,7 @@ export function useInspectorClient( setProtocolEra(undefined); setDiscoverResult(undefined); setExcludedTools([]); + setMalformedListItems([]); setLastError(undefined); return; } @@ -121,6 +132,7 @@ export function useInspectorClient( setProtocolEra(inspectorClient.getProtocolEra()); setDiscoverResult(inspectorClient.getDiscoverResult()); setExcludedTools(inspectorClient.getExcludedTools()); + setMalformedListItems(inspectorClient.getMalformedListItems()); setLastError(undefined); const onStatusChange = (event: TypedEvent<"statusChange">) => { @@ -156,6 +168,11 @@ export function useInspectorClient( ) => { setDiscoverResult(event.detail); }; + const onMalformedListItemsChange = ( + event: TypedEvent<"malformedListItemsChange">, + ) => { + setMalformedListItems(event.detail); + }; const onExcludedToolsChange = ( event: TypedEvent<"excludedToolsChange">, ) => { @@ -186,6 +203,10 @@ export function useInspectorClient( "excludedToolsChange", onExcludedToolsChange, ); + inspectorClient.addEventListener( + "malformedListItemsChange", + onMalformedListItemsChange, + ); return () => { inspectorClient.removeEventListener("statusChange", onStatusChange); @@ -218,6 +239,10 @@ export function useInspectorClient( "excludedToolsChange", onExcludedToolsChange, ); + inspectorClient.removeEventListener( + "malformedListItemsChange", + onMalformedListItemsChange, + ); }; }, [inspectorClient]); @@ -247,6 +272,7 @@ export function useInspectorClient( protocolEra, discoverResult, excludedTools, + malformedListItems, lastError, appRendererClient: inspectorClient?.getAppRendererClient() ?? null, connect,