Merge pull request #2016 from modelcontextprotocol/v2/fix/1909-salvage-malformed-list-items
fix: salvage a list result's valid entries when one is malformed (#1909)
This commit is contained in:
Cliff Hall
GitHub
commit
aaca08aa0e
28 changed files with 3129 additions and 73 deletions
|
|
@ -75,7 +75,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
|
||||
|
|
|
|||
|
|
@ -598,6 +598,7 @@ const DEFAULT_USE_INSPECTOR_CLIENT: ReturnType<typeof useInspectorClient> = {
|
|||
serverInfo: undefined,
|
||||
instructions: undefined,
|
||||
excludedTools: [],
|
||||
malformedListItems: [],
|
||||
appRendererClient: null,
|
||||
connect: vi.fn().mockResolvedValue(undefined),
|
||||
disconnect: vi.fn().mockResolvedValue(undefined),
|
||||
|
|
|
|||
|
|
@ -164,6 +164,7 @@ import {
|
|||
import { buildExportFilename, downloadJsonFile } from "./lib/downloadFile";
|
||||
import { INSPECTOR_SERVERS_TAB } from "./utils/inspectorTabs";
|
||||
import { enrichProtocolEntries } from "./utils/correlateTransportErrors";
|
||||
import { visibleMalformedListItems } from "./utils/malformedListReport";
|
||||
import {
|
||||
parseDeepLink,
|
||||
deepLinkConfigEquals,
|
||||
|
|
@ -991,6 +992,7 @@ function App() {
|
|||
protocolEra,
|
||||
discoverResult,
|
||||
excludedTools,
|
||||
malformedListItems,
|
||||
lastError,
|
||||
} = useInspectorClient(inspectorClient);
|
||||
const {
|
||||
|
|
@ -1056,6 +1058,14 @@ function App() {
|
|||
setPaginatedListsOverride(null);
|
||||
}, [persistedPaginatedLists, activeServerId]);
|
||||
const paginatedLists = paginatedListsOverride ?? persistedPaginatedLists;
|
||||
// The malformed-entry report is written by the aggregate walk's salvage. In
|
||||
// paginated mode the tools/prompts/resources panels render the paged stores
|
||||
// instead, which never write or clear it — so it would linger above a page it
|
||||
// does not describe. See `visibleMalformedListItems` (#1909 + #1721).
|
||||
const shownMalformedListItems = useMemo(
|
||||
() => visibleMalformedListItems(malformedListItems, paginatedLists),
|
||||
[malformedListItems, paginatedLists],
|
||||
);
|
||||
const connected = connectionStatus === "connected";
|
||||
const toolsPagination = usePaginatedList({
|
||||
connected,
|
||||
|
|
@ -4384,6 +4394,7 @@ function App() {
|
|||
latencyMs={latencyMs}
|
||||
tools={tools}
|
||||
excludedTools={excludedTools}
|
||||
malformedListItems={shownMalformedListItems}
|
||||
prompts={prompts}
|
||||
resources={resources}
|
||||
resourceTemplates={resourceTemplates}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,89 @@
|
|||
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||
import { MalformedItemsWarning } from "./MalformedItemsWarning";
|
||||
|
||||
const meta: Meta<typeof MalformedItemsWarning> = {
|
||||
title: "Elements/MalformedItemsWarning",
|
||||
component: MalformedItemsWarning,
|
||||
};
|
||||
|
||||
export default meta;
|
||||
type Story = StoryObj<typeof MalformedItemsWarning>;
|
||||
|
||||
/** 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: [],
|
||||
},
|
||||
};
|
||||
|
|
@ -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(
|
||||
<MalformedItemsWarning
|
||||
items={[TEMPLATE_ENTRY]}
|
||||
method="resources/templates/list"
|
||||
what="resource templates"
|
||||
/>,
|
||||
);
|
||||
|
||||
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(
|
||||
<MalformedItemsWarning
|
||||
items={[]}
|
||||
method="resources/templates/list"
|
||||
what="resource templates"
|
||||
/>,
|
||||
);
|
||||
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(
|
||||
<MalformedItemsWarning
|
||||
items={[TOOL_ENTRY]}
|
||||
method="resources/templates/list"
|
||||
what="resource templates"
|
||||
/>,
|
||||
);
|
||||
expect(screen.queryByRole("alert")).not.toBeInTheDocument();
|
||||
expect(screen.queryByText("get_weather")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("pluralizes the count and lists every entry", () => {
|
||||
renderWithMantine(
|
||||
<MalformedItemsWarning
|
||||
items={[
|
||||
TOOL_ENTRY,
|
||||
{ ...TOOL_ENTRY, index: 4, label: "add", reason: "name: bad" },
|
||||
TEMPLATE_ENTRY,
|
||||
]}
|
||||
method="tools/list"
|
||||
what="tools"
|
||||
/>,
|
||||
);
|
||||
|
||||
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(
|
||||
<MalformedItemsWarning
|
||||
items={[
|
||||
{
|
||||
method: "prompts/list",
|
||||
index: 2,
|
||||
reason: "Invalid input: expected object, received null",
|
||||
},
|
||||
]}
|
||||
method="prompts/list"
|
||||
what="prompts"
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByText("entry 2")).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
@ -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 (
|
||||
<WarningAlert
|
||||
title={`${mine.length} malformed ${
|
||||
mine.length === 1 ? "entry" : "entries"
|
||||
} dropped`}
|
||||
>
|
||||
<SummaryText>
|
||||
The server returned {what} that don't match the MCP schema. The
|
||||
rest of the list is shown below.
|
||||
</SummaryText>
|
||||
<EntryScroll>
|
||||
<EntryList>
|
||||
{mine.map((item) => (
|
||||
<List.Item key={`${item.method}:${item.index}`}>
|
||||
<EntryLabel>{describeEntry(item)}</EntryLabel>{" "}
|
||||
<EntryReason>{item.reason}</EntryReason>
|
||||
</List.Item>
|
||||
))}
|
||||
</EntryList>
|
||||
</EntryScroll>
|
||||
</WarningAlert>
|
||||
);
|
||||
}
|
||||
|
|
@ -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({
|
|||
/>
|
||||
<ListPaginationControls {...pagination} />
|
||||
<ListLoadError error={loadError} what="prompts" onRetry={onRefreshList} />
|
||||
<MalformedItemsWarning
|
||||
items={malformedListItems}
|
||||
method="prompts/list"
|
||||
what="prompts"
|
||||
/>
|
||||
<ListScroll viewportRef={viewportRef}>
|
||||
<Stack gap="xs">
|
||||
{filteredPrompts.map((prompt) => (
|
||||
|
|
|
|||
|
|
@ -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}
|
||||
/>
|
||||
<MalformedItemsWarning
|
||||
items={malformedListItems}
|
||||
method="resources/list"
|
||||
what="resources"
|
||||
/>
|
||||
<MalformedItemsWarning
|
||||
items={malformedListItems}
|
||||
method="resources/templates/list"
|
||||
what="resource templates"
|
||||
/>
|
||||
{/* Stays inline: Accordion is a compound, `multiple`-discriminated generic,
|
||||
so `.withProps({ multiple: true, ... })` loses its JSX call signature
|
||||
(same tooling limit as Box). */}
|
||||
|
|
|
|||
|
|
@ -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({
|
|||
/>
|
||||
<ListPaginationControls {...pagination} />
|
||||
<ListLoadError error={loadError} what="tools" onRetry={onRefreshList} />
|
||||
<MalformedItemsWarning
|
||||
items={malformedListItems}
|
||||
method="tools/list"
|
||||
what="tools"
|
||||
/>
|
||||
<SidebarScroll viewportRef={viewportRef}>
|
||||
<Stack gap="xs">
|
||||
{filteredTools.map(({ tool, key }) => (
|
||||
|
|
|
|||
|
|
@ -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}
|
||||
|
|
|
|||
|
|
@ -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) =>
|
||||
|
|
|
|||
|
|
@ -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({
|
|||
<ToolControls
|
||||
tools={tools}
|
||||
excludedTools={excludedTools}
|
||||
malformedListItems={malformedListItems}
|
||||
selectedName={selectedToolName}
|
||||
searchText={search}
|
||||
listChanged={listChanged}
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import {
|
|||
type ReactNode,
|
||||
type Ref,
|
||||
} from "react";
|
||||
import type { MalformedListItem } from "@inspector/core/mcp";
|
||||
import type { DeepLink, DeepLinkParseStatus } from "../../../utils/deepLink";
|
||||
import {
|
||||
AppShell,
|
||||
|
|
@ -418,6 +419,11 @@ export interface InspectorViewProps {
|
|||
/** Tools excluded from `tools/list` for invalid `x-mcp-header` annotations
|
||||
* (SEP-2243); shown in the Tools 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[];
|
||||
prompts: Prompt[];
|
||||
resources: Resource[];
|
||||
resourceTemplates: ResourceTemplate[];
|
||||
|
|
@ -632,6 +638,7 @@ export function InspectorView({
|
|||
latencyMs,
|
||||
tools,
|
||||
excludedTools = [],
|
||||
malformedListItems = [],
|
||||
prompts,
|
||||
resources,
|
||||
resourceTemplates,
|
||||
|
|
@ -1365,6 +1372,7 @@ export function InspectorView({
|
|||
<ToolsScreen
|
||||
tools={tools}
|
||||
excludedTools={excludedTools}
|
||||
malformedListItems={malformedListItems}
|
||||
callState={toolCallState}
|
||||
ui={toolsUi}
|
||||
listChanged={toolsListChanged}
|
||||
|
|
@ -1405,6 +1413,7 @@ export function InspectorView({
|
|||
<ScreenStage active={activeTab === "Prompts"}>
|
||||
<PromptsScreen
|
||||
prompts={prompts}
|
||||
malformedListItems={malformedListItems}
|
||||
getPromptState={getPromptState}
|
||||
ui={promptsUi}
|
||||
listChanged={promptsListChanged}
|
||||
|
|
@ -1421,6 +1430,7 @@ export function InspectorView({
|
|||
<ScreenStage active={activeTab === "Resources"}>
|
||||
<ResourcesScreen
|
||||
resources={resources}
|
||||
malformedListItems={malformedListItems}
|
||||
templates={resourceTemplates}
|
||||
subscriptions={subscriptions}
|
||||
subscriptionStreamState={subscriptionStreamState}
|
||||
|
|
|
|||
339
clients/web/src/test/core/mcp/listSalvage.test.ts
Normal file
339
clients/web/src/test/core/mcp/listSalvage.test.ts
Normal file
|
|
@ -0,0 +1,339 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import { z } from "zod/v4";
|
||||
import { SdkError, SdkErrorCode } from "@modelcontextprotocol/client";
|
||||
import {
|
||||
LIST_MAX_PAGES,
|
||||
ModernResultEnvelopeSchema,
|
||||
describeIssues,
|
||||
listPaginationExceeded,
|
||||
isClientDecodeRejection,
|
||||
isSalvageableRejection,
|
||||
labelForRawItem,
|
||||
lenientListPageSchema,
|
||||
nextCursorOf,
|
||||
rawItemsOf,
|
||||
salvageListItems,
|
||||
summarizeMalformed,
|
||||
} from "@inspector/core/mcp/listSalvage.js";
|
||||
import { ListToolsResultSchema } from "@modelcontextprotocol/core";
|
||||
|
||||
/**
|
||||
* Per-item list salvage (#1909).
|
||||
*
|
||||
* These cover the pure half — deciding what is salvageable and how it is
|
||||
* described. The client-side half (fall back only on a decode rejection, keep
|
||||
* the strict path otherwise) is driven end to end against a real malformed
|
||||
* server in `src/test/integration/mcp/inspectorClient-malformed-list.test.ts`.
|
||||
*/
|
||||
|
||||
const ItemSchema = z.object({
|
||||
name: z.string(),
|
||||
annotations: z.object({ priority: z.number().optional() }).optional(),
|
||||
});
|
||||
|
||||
describe("isClientDecodeRejection", () => {
|
||||
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", () => {
|
||||
const ToolsPageSchema = lenientListPageSchema(ListToolsResultSchema, "tools");
|
||||
|
||||
it("leaves the entries raw while still validating every other field", () => {
|
||||
// The entries are this module's job to check one at a time; everything
|
||||
// else is the real schema's, so a second violation can't ride along.
|
||||
const parsed = ToolsPageSchema.parse({
|
||||
tools: [{ name: "ok" }, { name: 42 }],
|
||||
nextCursor: "page-2",
|
||||
});
|
||||
expect(nextCursorOf(parsed)).toBe("page-2");
|
||||
expect(rawItemsOf(parsed, "tools")).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(
|
||||
ToolsPageSchema.safeParse({ tools: [], nextCursor: 7 }).success,
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects a top-level violation that rides along with a bad entry", () => {
|
||||
// A hand-rolled `{ nextCursor? }` schema would accept this, report the bad
|
||||
// entry, and silently drop the `_meta` violation with the strict error.
|
||||
expect(
|
||||
ToolsPageSchema.safeParse({ tools: [{ name: 42 }], _meta: "nope" })
|
||||
.success,
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("distinguishes an empty page from one that is not a list at all", () => {
|
||||
expect(rawItemsOf(ToolsPageSchema.parse({ tools: [] }), "tools")).toEqual(
|
||||
[],
|
||||
);
|
||||
expect(rawItemsOf({}, "tools")).toBeUndefined();
|
||||
expect(rawItemsOf({ tools: "not-a-list" }, "tools")).toBeUndefined();
|
||||
expect(rawItemsOf(null, "tools")).toBeUndefined();
|
||||
});
|
||||
|
||||
it("reads no cursor from a non-object page", () => {
|
||||
expect(nextCursorOf(null)).toBeUndefined();
|
||||
expect(nextCursorOf({ nextCursor: 7 })).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("isSalvageableRejection", () => {
|
||||
it("salvages an invalid result but not an unrecognized result type", () => {
|
||||
// `UnsupportedResultType` means the codec doesn't know what KIND of frame
|
||||
// arrived — not a list with a bad entry, so there is nothing to salvage
|
||||
// and reporting an item-level reason would hide the real failure.
|
||||
expect(
|
||||
isSalvageableRejection(new SdkError(SdkErrorCode.InvalidResult, "bad")),
|
||||
).toBe(true);
|
||||
expect(
|
||||
isSalvageableRejection(
|
||||
new SdkError(SdkErrorCode.UnsupportedResultType, "unknown"),
|
||||
),
|
||||
).toBe(false);
|
||||
// Still marks the Protocol entry, which is why both predicates exist.
|
||||
expect(
|
||||
isClientDecodeRejection(
|
||||
new SdkError(SdkErrorCode.UnsupportedResultType, "unknown"),
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("ModernResultEnvelopeSchema", () => {
|
||||
it("accepts a complete modern result with its cache hints", () => {
|
||||
expect(
|
||||
ModernResultEnvelopeSchema.safeParse({
|
||||
resultType: "complete",
|
||||
ttlMs: 0,
|
||||
cacheScope: "public",
|
||||
tools: [],
|
||||
}).success,
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("requires ttlMs to be a non-negative integer, as the codec does", () => {
|
||||
// A looser `z.number()` would accept `-1` / `0.5`, so a response with a bad
|
||||
// envelope AND one bad entry would salvage and hide the envelope violation.
|
||||
for (const ttlMs of [-1, 0.5]) {
|
||||
expect(
|
||||
ModernResultEnvelopeSchema.safeParse({
|
||||
resultType: "complete",
|
||||
ttlMs,
|
||||
cacheScope: "public",
|
||||
}).success,
|
||||
).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects a modern result missing the envelope the codec would enforce", () => {
|
||||
// The raw-wire salvage path goes around that codec, so a response missing
|
||||
// its cache hints AND carrying a bad entry must not salvage.
|
||||
expect(
|
||||
ModernResultEnvelopeSchema.safeParse({
|
||||
resultType: "complete",
|
||||
tools: [],
|
||||
}).success,
|
||||
).toBe(false);
|
||||
expect(
|
||||
ModernResultEnvelopeSchema.safeParse({
|
||||
resultType: "task",
|
||||
ttlMs: 0,
|
||||
cacheScope: "public",
|
||||
}).success,
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
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)");
|
||||
});
|
||||
|
||||
describe("the page bound", () => {
|
||||
it("matches the cap the SDK client is configured with", () => {
|
||||
// The value is passed to the SDK as `listMaxPages`, so the strict
|
||||
// aggregate and the fallback walks are bounded by the same number.
|
||||
expect(LIST_MAX_PAGES).toBe(64);
|
||||
});
|
||||
|
||||
it("names the method and the cap it hit", () => {
|
||||
// The walk's failure is logged rather than surfaced (the original
|
||||
// validation error is what the caller sees), so the message is the only
|
||||
// record of why salvage gave up.
|
||||
const error = listPaginationExceeded("resources/templates/list");
|
||||
expect(error.message).toContain("resources/templates/list");
|
||||
expect(error.message).toContain("64 pages");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -144,6 +144,76 @@ describe("useInspectorClient", () => {
|
|||
expect(result.current.excludedTools).toEqual([]);
|
||||
});
|
||||
|
||||
it("subscribes to malformedListItemsChange and updates (#1909)", () => {
|
||||
const client = new FakeInspectorClient();
|
||||
const { result } = renderHook(() => useInspectorClient(client));
|
||||
expect(result.current.malformedListItems).toEqual([]);
|
||||
const malformed = [
|
||||
{
|
||||
method: "resources/templates/list",
|
||||
index: 1,
|
||||
label: "array_annotations",
|
||||
reason: "annotations: Invalid input (expected object)",
|
||||
},
|
||||
];
|
||||
act(() => {
|
||||
client.setMalformedListItems(malformed);
|
||||
});
|
||||
expect(result.current.malformedListItems).toEqual(malformed);
|
||||
});
|
||||
|
||||
it("seeds malformedListItems from the client it is given (#1909)", () => {
|
||||
// A client that already salvaged before this hook mounted (a screen opened
|
||||
// after the connect-time list load) must show the report immediately.
|
||||
const client = new FakeInspectorClient({ status: "connected" });
|
||||
const malformed = [
|
||||
{ method: "tools/list", index: 0, reason: "name: expected string" },
|
||||
];
|
||||
client.setMalformedListItems(malformed);
|
||||
const { result } = renderHook(() => useInspectorClient(client));
|
||||
expect(result.current.malformedListItems).toEqual(malformed);
|
||||
});
|
||||
|
||||
it("resets malformedListItems to [] when client becomes null (#1909)", () => {
|
||||
const client = new FakeInspectorClient({ status: "connected" });
|
||||
client.setMalformedListItems([
|
||||
{ method: "tools/list", index: 0, reason: "name: expected string" },
|
||||
]);
|
||||
const { result, rerender } = renderHook(({ c }) => useInspectorClient(c), {
|
||||
initialProps: { c: client as FakeInspectorClient | null },
|
||||
});
|
||||
expect(result.current.malformedListItems.length).toBe(1);
|
||||
rerender({ c: null });
|
||||
expect(result.current.malformedListItems).toEqual([]);
|
||||
});
|
||||
|
||||
it("re-seeds malformedListItems on a client swap, not the old server's (#1909)", () => {
|
||||
const first = new FakeInspectorClient({ status: "connected" });
|
||||
first.setMalformedListItems([
|
||||
{ method: "tools/list", index: 0, reason: "name: expected string" },
|
||||
]);
|
||||
const second = new FakeInspectorClient({ status: "connected" });
|
||||
const { result, rerender } = renderHook(({ c }) => useInspectorClient(c), {
|
||||
initialProps: { c: first as FakeInspectorClient },
|
||||
});
|
||||
expect(result.current.malformedListItems).toHaveLength(1);
|
||||
rerender({ c: second });
|
||||
expect(result.current.malformedListItems).toEqual([]);
|
||||
});
|
||||
|
||||
it("stops listening for malformedListItemsChange after unmount (#1909)", () => {
|
||||
const client = new FakeInspectorClient({ status: "connected" });
|
||||
const { result, unmount } = renderHook(() => useInspectorClient(client));
|
||||
unmount();
|
||||
act(() => {
|
||||
client.setMalformedListItems([
|
||||
{ method: "tools/list", index: 0, reason: "name: expected string" },
|
||||
]);
|
||||
});
|
||||
// A listener left attached would update state on an unmounted tree.
|
||||
expect(result.current.malformedListItems).toEqual([]);
|
||||
});
|
||||
|
||||
it("resets protocolEra / discoverResult to defaults when client becomes null", () => {
|
||||
const client = new FakeInspectorClient({
|
||||
status: "connected",
|
||||
|
|
|
|||
|
|
@ -120,6 +120,28 @@ describe("excluded tools (SEP-2243 x-mcp-header)", () => {
|
|||
expect(excluded.map((x) => x.tool.name)).toEqual(["invalid_header_tool"]);
|
||||
});
|
||||
|
||||
it("does not carry an excluded set into the next connection (#1909)", async () => {
|
||||
// `resetSessionState()` is the start-clean path, and it is the ONLY one the
|
||||
// route out that tears down nothing (`onerror` with no `onclose`) reaches.
|
||||
// A set left behind names tools of the PREVIOUS server — and would persist
|
||||
// indefinitely against a server that never lists tools again.
|
||||
const started = await start({});
|
||||
const connected = await connect(started.url, "modern");
|
||||
await connected.listAllTools();
|
||||
expect(connected.getExcludedTools()).toHaveLength(1);
|
||||
|
||||
const events: number[] = [];
|
||||
connected.addEventListener("excludedToolsChange", (e) => {
|
||||
events.push(e.detail.length);
|
||||
});
|
||||
// Reconnecting runs the start-clean reset; assert before anything re-lists.
|
||||
await connected.disconnect();
|
||||
await connected.connect();
|
||||
|
||||
expect(connected.getExcludedTools()).toEqual([]);
|
||||
expect(events).toContain(0);
|
||||
});
|
||||
|
||||
it("reports no excluded tools on a legacy connection (no exclusion there)", async () => {
|
||||
const started = await start(undefined);
|
||||
const connected = await connect(started.url, "legacy");
|
||||
|
|
|
|||
|
|
@ -0,0 +1,802 @@
|
|||
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";
|
||||
import { eraToVersionNegotiation } from "@inspector/core/mcp/types.js";
|
||||
import { LIST_MAX_PAGES } from "@inspector/core/mcp/listSalvage.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.
|
||||
*/
|
||||
|
||||
const MODERN_VERSION = "2026-07-28";
|
||||
|
||||
/**
|
||||
* One page exactly as the server will put it on the wire.
|
||||
*
|
||||
* Both fields are `unknown` on purpose: this server exists to serve shapes the
|
||||
* spec forbids (a non-array `items`, a numeric `nextCursor`), so the fixture
|
||||
* models raw wire data rather than the validated type. Declaring the spec type
|
||||
* and casting malformed values through it at each call site would assert the
|
||||
* opposite of what these tests are for.
|
||||
*/
|
||||
interface ListPage {
|
||||
items: unknown;
|
||||
nextCursor?: unknown;
|
||||
}
|
||||
|
||||
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[];
|
||||
},
|
||||
/** Negotiate the modern era, which is what turns the SEP-2243 gate on. */
|
||||
modern = false,
|
||||
): Promise<{
|
||||
url: string;
|
||||
stop: () => Promise<void>;
|
||||
calls: string[];
|
||||
/** Every `cursor` param the server was sent, in order. */
|
||||
cursors: (string | undefined)[];
|
||||
/** 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 cursors: (string | undefined)[] = [];
|
||||
// Cursor string -> the page index it addresses, learned as pages are served.
|
||||
// Modeling it this way (rather than parsing the cursor as a number) is what
|
||||
// makes an EMPTY-STRING cursor meaningful: a client that drops it sends no
|
||||
// cursor at all, lands back on page one, and duplicates its entries.
|
||||
const cursorToIndex = new Map<string, number>();
|
||||
const pageFor = (key: "resourceTemplates" | "tools", cursor?: string) => {
|
||||
cursors.push(cursor);
|
||||
const configured = pages[key] ?? [];
|
||||
const index = cursor === undefined ? 0 : (cursorToIndex.get(cursor) ?? 0);
|
||||
const page = configured[index] ?? { items: [] };
|
||||
// Only a string cursor can address a page; a malformed one is served
|
||||
// verbatim so the client can reject it, but it maps to nothing.
|
||||
if (typeof page.nextCursor === "string") {
|
||||
cursorToIndex.set(page.nextCursor, index + 1);
|
||||
}
|
||||
return page;
|
||||
};
|
||||
|
||||
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: Record<string, unknown>) => {
|
||||
res.writeHead(200, { "Content-Type": "application/json" });
|
||||
// The 2026-07-28 codec requires `resultType` on every result; the legacy
|
||||
// leg ignores it.
|
||||
res.end(
|
||||
JSON.stringify(
|
||||
jsonRpcResult(
|
||||
body.id,
|
||||
modern
|
||||
? {
|
||||
resultType: "complete",
|
||||
// The modern leg also requires the cache hints on a list
|
||||
// result; `ttlMs: 0` means "don't cache", which keeps each
|
||||
// call in these tests a real round trip.
|
||||
ttlMs: 0,
|
||||
cacheScope: "public",
|
||||
...result,
|
||||
}
|
||||
: result,
|
||||
),
|
||||
),
|
||||
);
|
||||
};
|
||||
|
||||
if (body.method === "initialize") {
|
||||
send({
|
||||
protocolVersion: modern ? MODERN_VERSION : "2025-06-18",
|
||||
capabilities: { resources: {}, tools: {} },
|
||||
serverInfo: { name: "malformed-list-server", version: "1.0.0" },
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (body.method === "server/discover") {
|
||||
send({
|
||||
supportedVersions: [MODERN_VERSION],
|
||||
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,
|
||||
cursors,
|
||||
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<void>) | null = null;
|
||||
|
||||
async function connectTo(url: string, era?: "legacy" | "modern") {
|
||||
client = new InspectorClient(
|
||||
{ type: "streamable-http", url },
|
||||
{
|
||||
environment: { transport: createTransportNode },
|
||||
...(era ? { versionNegotiation: eraToVersionNegotiation(era) } : {}),
|
||||
},
|
||||
);
|
||||
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 ORIGINAL rejected response, not a salvage page", async () => {
|
||||
// The re-walk answers the same method again, so a mark taken afterwards
|
||||
// would land on a page that succeeded and leave the invalid response
|
||||
// rendering clean — the exact lie #1953 exists to remove.
|
||||
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 templateCalls = log
|
||||
.getMessages()
|
||||
.filter(
|
||||
(entry) =>
|
||||
"method" in entry.message &&
|
||||
entry.message.method === "resources/templates/list",
|
||||
);
|
||||
// Two exchanges: the strict aggregate that was rejected, then the re-walk.
|
||||
expect(templateCalls.length).toBeGreaterThanOrEqual(2);
|
||||
expect(templateCalls[0]?.clientError).toContain(
|
||||
"Dropped 1 malformed entry",
|
||||
);
|
||||
for (const later of templateCalls.slice(1)) {
|
||||
expect(later.clientError).toBeUndefined();
|
||||
}
|
||||
log.destroy();
|
||||
});
|
||||
|
||||
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("does not readmit a SEP-2243-excluded tool while salvaging", async () => {
|
||||
// The strict aggregate is filtered by the SDK, so the salvage path has to
|
||||
// reapply the rule: otherwise one ordinary malformed tool anywhere in the
|
||||
// list quietly readmits every invalid-header tool beside it, turning a
|
||||
// rendering fix into a spec violation (#1632 + #1909).
|
||||
const server = await startMalformedServer(
|
||||
{
|
||||
tools: [
|
||||
{
|
||||
items: [
|
||||
{ name: "ok_tool", inputSchema: { type: "object" } },
|
||||
// Malformed: trips the salvage fallback.
|
||||
{ name: "broken", inputSchema: "not-a-schema" },
|
||||
// Schema-valid, but its x-mcp-header is not an RFC 9110 token.
|
||||
{
|
||||
name: "invalid_header_tool",
|
||||
inputSchema: {
|
||||
type: "object",
|
||||
properties: {
|
||||
value: {
|
||||
type: "string",
|
||||
"x-mcp-header": "Bad Header",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
true,
|
||||
);
|
||||
stopServer = server.stop;
|
||||
const connected = await connectTo(server.url, "modern");
|
||||
expect(connected.getProtocolEra()).toBe("modern");
|
||||
|
||||
const { tools } = await connected.listAllTools();
|
||||
expect(tools.map((t) => t.name)).toEqual(["ok_tool"]);
|
||||
// Still reported as excluded rather than silently gone.
|
||||
expect(connected.getExcludedTools().map((x) => x.tool.name)).toEqual([
|
||||
"invalid_header_tool",
|
||||
]);
|
||||
});
|
||||
|
||||
it("surfaces the ORIGINAL error when the re-walk itself fails", async () => {
|
||||
// A malformed `nextCursor` fails the lenient page schema too, so the walk
|
||||
// throws its own error. The original is what the caller's list actually
|
||||
// failed on — and what `rejectedResponseId` was captured for — so that is
|
||||
// the one that must surface.
|
||||
const server = await startMalformedServer({
|
||||
resourceTemplates: [
|
||||
{
|
||||
items: [PHP_EMPTY_ANNOTATIONS, VALID_TEMPLATE],
|
||||
nextCursor: 42,
|
||||
},
|
||||
],
|
||||
});
|
||||
stopServer = server.stop;
|
||||
const connected = await connectTo(server.url);
|
||||
|
||||
await expect(connected.listAllResourceTemplates()).rejects.toThrow(
|
||||
/annotations/,
|
||||
);
|
||||
expect(connected.getMalformedListItems()).toEqual([]);
|
||||
});
|
||||
|
||||
it("keeps the strict error when a page is not a list at all", async () => {
|
||||
// `{ resourceTemplates: "nope" }` is a top-level violation the per-item
|
||||
// pass cannot explain; treating it as an empty page would return a
|
||||
// silently truncated list and discard the error that was right about it.
|
||||
const server = await startMalformedServer({
|
||||
resourceTemplates: [{ items: "nope" }],
|
||||
});
|
||||
stopServer = server.stop;
|
||||
const connected = await connectTo(server.url);
|
||||
|
||||
await expect(connected.listAllResourceTemplates()).rejects.toThrow();
|
||||
expect(connected.getMalformedListItems()).toEqual([]);
|
||||
});
|
||||
|
||||
it("sends an empty-string cursor rather than re-fetching page one", async () => {
|
||||
// "" is a valid cursor. A truthiness check would drop it, re-request page
|
||||
// one, and duplicate its entries until the repeated-cursor guard fired.
|
||||
const server = await startMalformedServer({
|
||||
resourceTemplates: [
|
||||
{ items: [PHP_EMPTY_ANNOTATIONS, VALID_TEMPLATE], nextCursor: "" },
|
||||
{ items: [{ ...VALID_TEMPLATE, name: "page_two" }] },
|
||||
],
|
||||
});
|
||||
stopServer = server.stop;
|
||||
const connected = await connectTo(server.url);
|
||||
|
||||
const { resourceTemplates } = await connected.listAllResourceTemplates();
|
||||
expect(resourceTemplates.map((t) => t.name)).toEqual([
|
||||
"full_annotations",
|
||||
"page_two",
|
||||
]);
|
||||
expect(server.cursors).toContain("");
|
||||
});
|
||||
|
||||
it("marks the exclusion scan's own refused response too", async () => {
|
||||
// The scan is a separate exchange from the aggregate's, and its re-fetch
|
||||
// moves the method correlation off it — so without marking it here its
|
||||
// Protocol entry renders as a clean success the client actually refused.
|
||||
const server = await startMalformedServer(
|
||||
{
|
||||
tools: [
|
||||
{
|
||||
items: [
|
||||
{ name: "ok_tool", inputSchema: { type: "object" } },
|
||||
{ name: "broken", inputSchema: "not-a-schema" },
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
true,
|
||||
);
|
||||
stopServer = server.stop;
|
||||
const connected = await connectTo(server.url, "modern");
|
||||
const log = new MessageLogState(connected);
|
||||
|
||||
await connected.listAllTools();
|
||||
|
||||
const marked = log
|
||||
.getMessages()
|
||||
.filter(
|
||||
(entry) =>
|
||||
"method" in entry.message &&
|
||||
entry.message.method === "tools/list" &&
|
||||
entry.clientError !== undefined,
|
||||
);
|
||||
// Two refused responses: the aggregate's and the scan's.
|
||||
expect(marked).toHaveLength(2);
|
||||
for (const entry of marked) {
|
||||
expect(entry.clientError).toContain("Dropped 1 malformed entry");
|
||||
}
|
||||
log.destroy();
|
||||
});
|
||||
|
||||
it("indexes a dropped tool against the whole aggregate, across pages", async () => {
|
||||
// The reported index has to be offset by the RAW entries of PRIOR pages —
|
||||
// valid and malformed alike, not just the kept ones. An unlabeled entry's
|
||||
// index is all the user has to go on, so a wrong offset points at nothing.
|
||||
// This is the aggregate salvage's walk: it owns the `tools/list` report,
|
||||
// because it is the walk that produced the list being rendered.
|
||||
const server = await startMalformedServer(
|
||||
{
|
||||
tools: [
|
||||
{
|
||||
items: [
|
||||
{ name: "one", inputSchema: { type: "object" } },
|
||||
{ name: "two", inputSchema: { type: "object" } },
|
||||
],
|
||||
nextCursor: "p2",
|
||||
},
|
||||
// Page two, entry one (index 2 of the whole list) is unlabelable.
|
||||
{ items: [42, { name: "four", inputSchema: { type: "object" } }] },
|
||||
],
|
||||
},
|
||||
true,
|
||||
);
|
||||
stopServer = server.stop;
|
||||
const connected = await connectTo(server.url, "modern");
|
||||
|
||||
const { tools } = await connected.listAllTools();
|
||||
expect(tools.map((t) => t.name)).toEqual(["one", "two", "four"]);
|
||||
const reported = connected.getMalformedListItems();
|
||||
expect(reported).toHaveLength(1);
|
||||
expect(reported[0]?.index).toBe(2);
|
||||
expect(reported[0]?.label).toBeUndefined();
|
||||
});
|
||||
|
||||
it("leaves the correlation on the original response when it rethrows", async () => {
|
||||
// On a rethrow the re-walk has already answered the same method, so the
|
||||
// caller's own `markResponseRejected` would land on the lenient page that
|
||||
// succeeded. The correlation is restored so the outer mark still finds the
|
||||
// response that was actually refused.
|
||||
const server = await startMalformedServer({
|
||||
resourceTemplates: [{ items: [VALID_TEMPLATE], nextCursor: 42 }],
|
||||
});
|
||||
stopServer = server.stop;
|
||||
const connected = await connectTo(server.url);
|
||||
const log = new MessageLogState(connected);
|
||||
|
||||
await expect(connected.listAllResourceTemplates()).rejects.toThrow();
|
||||
// Simulates what ManagedListState.refresh() does in its catch.
|
||||
connected.markResponseRejected("resources/templates/list", "outer mark");
|
||||
|
||||
const calls = log
|
||||
.getMessages()
|
||||
.filter(
|
||||
(entry) =>
|
||||
"method" in entry.message &&
|
||||
entry.message.method === "resources/templates/list",
|
||||
);
|
||||
expect(calls[0]?.clientError).toBe("outer mark");
|
||||
for (const later of calls.slice(1)) {
|
||||
expect(later.clientError).toBeUndefined();
|
||||
}
|
||||
log.destroy();
|
||||
});
|
||||
|
||||
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 }],
|
||||
});
|
||||
stopServer = server.stop;
|
||||
const connected = await connectTo(server.url);
|
||||
|
||||
await expect(connected.listAllResourceTemplates()).rejects.toThrow();
|
||||
expect(connected.getMalformedListItems()).toEqual([]);
|
||||
});
|
||||
|
||||
it("keeps the aggregate's report when the scan sees a different list", async () => {
|
||||
// Both walks run on a modern connection and they are separate, uncached
|
||||
// exchanges. The aggregate is the list being RENDERED, so it owns the
|
||||
// report; the scan never writes it. Here the server answers the scan's
|
||||
// request with a different malformed shape — which, if the scan published,
|
||||
// would leave the warning pointing into a list nobody is looking at.
|
||||
const server = await startMalformedServer(
|
||||
{
|
||||
tools: [
|
||||
{
|
||||
items: [
|
||||
{ name: "ok_tool", inputSchema: { type: "object" } },
|
||||
{ name: "broken", inputSchema: "not-a-schema" },
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
true,
|
||||
);
|
||||
stopServer = server.stop;
|
||||
const connected = await connectTo(server.url, "modern");
|
||||
|
||||
// Serve a DIFFERENT malformed shape to the scan's walk, which runs after
|
||||
// the aggregate — so an overwrite is visible as a changed index/label.
|
||||
let swapped = false;
|
||||
const original = connected.listTools.bind(connected);
|
||||
connected.listTools = async (cursor?: string, meta?: object) => {
|
||||
if (!swapped) {
|
||||
swapped = true;
|
||||
server.setPages({
|
||||
tools: [
|
||||
{
|
||||
items: [
|
||||
{ name: "a", inputSchema: { type: "object" } },
|
||||
{ name: "b", inputSchema: { type: "object" } },
|
||||
{ name: "later_break", inputSchema: 42 },
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
}
|
||||
return original(cursor, meta as Record<string, string> | undefined);
|
||||
};
|
||||
|
||||
await connected.listAllTools();
|
||||
|
||||
// The aggregate's finding, at the aggregate's index — not the scan's.
|
||||
expect(connected.getMalformedListItems()).toEqual([
|
||||
{
|
||||
method: "tools/list",
|
||||
index: 1,
|
||||
label: "broken",
|
||||
reason: expect.stringMatching(/^inputSchema/),
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("marks the scan's refused response even when the re-fetch is clean", async () => {
|
||||
// The list can change between the strict page and the lenient re-fetch. A
|
||||
// conforming re-fetch says nothing about the response that was refused, so
|
||||
// leaving it unmarked would render a rejected exchange as a clean success.
|
||||
const server = await startMalformedServer(
|
||||
{ tools: [{ items: [{ name: "broken", inputSchema: 42 }] }] },
|
||||
true,
|
||||
);
|
||||
stopServer = server.stop;
|
||||
const connected = await connectTo(server.url, "modern");
|
||||
const log = new MessageLogState(connected);
|
||||
|
||||
// Let the strict call fail, then serve a conforming list to every later
|
||||
// request — so the scan's re-fetch finds nothing per-item wrong.
|
||||
const original = connected.listTools.bind(connected);
|
||||
connected.listTools = async (cursor?: string, meta?: object) => {
|
||||
try {
|
||||
return await original(
|
||||
cursor,
|
||||
meta as Record<string, string> | undefined,
|
||||
);
|
||||
} finally {
|
||||
server.setPages({
|
||||
tools: [
|
||||
{ items: [{ name: "fixed", inputSchema: { type: "object" } }] },
|
||||
],
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
await connected.refreshExcludedTools();
|
||||
|
||||
const marked = log
|
||||
.getMessages()
|
||||
.filter(
|
||||
(entry) =>
|
||||
"method" in entry.message &&
|
||||
entry.message.method === "tools/list" &&
|
||||
entry.clientError !== undefined,
|
||||
);
|
||||
expect(marked).toHaveLength(1);
|
||||
log.destroy();
|
||||
});
|
||||
|
||||
it("stops the salvage walk at the page cap instead of paging forever", async () => {
|
||||
// A server whose `nextCursor` never converges but never REPEATS slips the
|
||||
// repeated-cursor guard: every cursor is fresh, so the walk had no reason
|
||||
// to stop and would page (and accumulate) without bound. The bound is the
|
||||
// same `LIST_MAX_PAGES` the SDK aggregate is configured with.
|
||||
const server = await startMalformedServer({
|
||||
resourceTemplates: Array.from(
|
||||
{ length: LIST_MAX_PAGES + 40 },
|
||||
(_, i) => ({
|
||||
items: [PHP_EMPTY_ANNOTATIONS],
|
||||
nextCursor: `cursor-${i}`,
|
||||
}),
|
||||
),
|
||||
});
|
||||
stopServer = server.stop;
|
||||
const connected = await connectTo(server.url);
|
||||
|
||||
// Truncation is not salvage: with entries still unfetched, returning what
|
||||
// we have would present a partial list as complete, so the original
|
||||
// validation error stands.
|
||||
await expect(connected.listAllResourceTemplates()).rejects.toThrow();
|
||||
expect(connected.getMalformedListItems()).toEqual([]);
|
||||
|
||||
const listCalls = server.calls.filter(
|
||||
(method) => method === "resources/templates/list",
|
||||
);
|
||||
// One strict aggregate attempt (rejected on page one), then a salvage walk
|
||||
// bounded at exactly the cap. Without the bound this runs to 104 pages —
|
||||
// and against a truly endless server, forever.
|
||||
expect(listCalls).toHaveLength(1 + LIST_MAX_PAGES);
|
||||
});
|
||||
|
||||
it("stops the exclusion scan at the page cap too", async () => {
|
||||
// The SEP-2243 scan walks pages of its own, outside the SDK aggregate's
|
||||
// cap, so it needs the same bound. Driven directly because a non-converging
|
||||
// list trips `listAllTools`'s salvage walk first.
|
||||
const server = await startMalformedServer(
|
||||
{
|
||||
tools: Array.from({ length: LIST_MAX_PAGES + 40 }, (_, i) => ({
|
||||
items: [{ name: `tool_${i}`, inputSchema: { type: "object" } }],
|
||||
nextCursor: `cursor-${i}`,
|
||||
})),
|
||||
},
|
||||
true,
|
||||
);
|
||||
stopServer = server.stop;
|
||||
const connected = await connectTo(server.url, "modern");
|
||||
|
||||
await expect(connected.refreshExcludedTools()).rejects.toThrow(
|
||||
/exceeded 64 pages/,
|
||||
);
|
||||
expect(
|
||||
server.calls.filter((method) => method === "tools/list"),
|
||||
).toHaveLength(LIST_MAX_PAGES);
|
||||
});
|
||||
|
||||
it("marks the scan's refused response when its own fallback fails", async () => {
|
||||
// `tools` is not a list at all, so the scan's lenient re-fetch cannot
|
||||
// explain the rejection and rethrows. `listAllTools` catches a failing scan
|
||||
// by design (it must never fail the tools list), so nothing downstream
|
||||
// would mark the response the client demonstrably refused — it would render
|
||||
// as a clean exchange.
|
||||
const server = await startMalformedServer(
|
||||
{ tools: [{ items: "not-a-list" }] },
|
||||
true,
|
||||
);
|
||||
stopServer = server.stop;
|
||||
const connected = await connectTo(server.url, "modern");
|
||||
const log = new MessageLogState(connected);
|
||||
|
||||
await expect(connected.refreshExcludedTools()).rejects.toThrow();
|
||||
|
||||
const marked = log
|
||||
.getMessages()
|
||||
.filter(
|
||||
(entry) =>
|
||||
"method" in entry.message &&
|
||||
entry.message.method === "tools/list" &&
|
||||
entry.clientError !== undefined,
|
||||
);
|
||||
// The mark carries the ORIGINAL decode error: the fallback's own failure is
|
||||
// a different event from the response being marked.
|
||||
expect(marked).toHaveLength(1);
|
||||
expect(marked[0]?.clientError).toBeTruthy();
|
||||
log.destroy();
|
||||
});
|
||||
});
|
||||
366
clients/web/src/test/integration/mcp/listSalvage-era.test.ts
Normal file
366
clients/web/src/test/integration/mcp/listSalvage-era.test.ts
Normal file
|
|
@ -0,0 +1,366 @@
|
|||
import { describe, it, expect, afterEach } from "vitest";
|
||||
import {
|
||||
createServer,
|
||||
type IncomingMessage,
|
||||
type Server,
|
||||
type ServerResponse,
|
||||
} from "node:http";
|
||||
import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js";
|
||||
import { createTransportNode } from "@inspector/core/mcp/node/transport.js";
|
||||
import { eraToVersionNegotiation } from "@inspector/core/mcp/types.js";
|
||||
import { ToolSchema } from "@modelcontextprotocol/core";
|
||||
import {
|
||||
describeIssues,
|
||||
toolItemSchemaForEra,
|
||||
} from "@inspector/core/mcp/listSalvage.js";
|
||||
|
||||
/**
|
||||
* The era item contract the salvage fallback validates tools against (#1909).
|
||||
*
|
||||
* `toolItemSchemaForEra` replicates two rules that live in the SDK's internal
|
||||
* per-era wire schemas, which are not exported. That is a real cost, and these
|
||||
* tests are what makes it safe to pay: each one asserts the STRICT path's
|
||||
* observed behavior on a real connection, so if a future SDK closes or widens a
|
||||
* divergence, the test fails and the replicated rule gets revisited instead of
|
||||
* silently rotting.
|
||||
*
|
||||
* The failure being prevented: the fallback keeping an entry the strict path
|
||||
* refused, which the user would then see — and could call — on a connection
|
||||
* whose era says it isn't valid.
|
||||
*/
|
||||
|
||||
const ARRAY_OUTPUT_TOOL = {
|
||||
name: "array_output",
|
||||
inputSchema: { type: "object" },
|
||||
outputSchema: { type: "array" },
|
||||
};
|
||||
|
||||
const EXECUTION_TOOL = {
|
||||
name: "with_execution",
|
||||
inputSchema: { type: "object" },
|
||||
execution: { taskSupport: "optional" },
|
||||
};
|
||||
|
||||
function startToolsServer(
|
||||
tools: unknown[],
|
||||
modern: boolean,
|
||||
): Promise<{ url: string; stop: () => Promise<void> }> {
|
||||
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();
|
||||
if (raw.length === 0) {
|
||||
res.writeHead(405).end();
|
||||
return;
|
||||
}
|
||||
const body = JSON.parse(raw) as { id?: unknown; method: string };
|
||||
if (body.id === undefined) {
|
||||
res.writeHead(202).end();
|
||||
return;
|
||||
}
|
||||
const envelope = modern
|
||||
? { resultType: "complete", ttlMs: 0, cacheScope: "public" }
|
||||
: {};
|
||||
const send = (result: object) => {
|
||||
res.writeHead(200, { "Content-Type": "application/json" });
|
||||
res.end(
|
||||
JSON.stringify({
|
||||
jsonrpc: "2.0",
|
||||
id: body.id,
|
||||
result: { ...envelope, ...result },
|
||||
}),
|
||||
);
|
||||
};
|
||||
if (body.method === "initialize") {
|
||||
send({
|
||||
protocolVersion: modern ? "2026-07-28" : "2025-06-18",
|
||||
capabilities: { tools: {} },
|
||||
serverInfo: { name: "era-contract-server", version: "1.0.0" },
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (body.method === "server/discover") {
|
||||
send({
|
||||
supportedVersions: ["2026-07-28"],
|
||||
capabilities: { tools: {} },
|
||||
serverInfo: { name: "era-contract-server", version: "1.0.0" },
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (body.method === "tools/list") {
|
||||
send({ tools });
|
||||
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`,
|
||||
stop: () =>
|
||||
new Promise((done) => {
|
||||
server.close(() => done());
|
||||
}),
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
describe("salvage era item contract (#1909)", () => {
|
||||
let client: InspectorClient | null = null;
|
||||
let stop: (() => Promise<void>) | null = null;
|
||||
|
||||
afterEach(async () => {
|
||||
try {
|
||||
await client?.disconnect();
|
||||
} catch {
|
||||
// Teardown only.
|
||||
}
|
||||
client = null;
|
||||
await stop?.();
|
||||
stop = null;
|
||||
});
|
||||
|
||||
async function connect(url: string, era: "legacy" | "modern") {
|
||||
client = new InspectorClient(
|
||||
{ type: "streamable-http", url },
|
||||
{
|
||||
environment: { transport: createTransportNode },
|
||||
versionNegotiation: eraToVersionNegotiation(era),
|
||||
},
|
||||
);
|
||||
await client.connect();
|
||||
return client;
|
||||
}
|
||||
|
||||
it("legacy: the era rule is what rejects a non-object outputSchema (pins it)", async () => {
|
||||
// The rule lives in the SDK's high-level aggregate (the era codec), not in
|
||||
// the neutral schema the single-page call uses — which is precisely the
|
||||
// divergence `toolItemSchemaForEra` exists to close.
|
||||
//
|
||||
// This tool is the page's ONLY problem, so the observable outcome is
|
||||
// entirely the SDK's rule talking: the strict aggregate refuses the page,
|
||||
// the fallback runs, and our contract agrees the tool is not era-valid. If
|
||||
// a future SDK accepts it, the aggregate succeeds instead and this fails —
|
||||
// which is the signal to revisit the replicated rule.
|
||||
const server = await startToolsServer([ARRAY_OUTPUT_TOOL], false);
|
||||
stop = server.stop;
|
||||
const connected = await connect(server.url, "legacy");
|
||||
|
||||
const { tools } = await connected.listAllTools();
|
||||
expect(tools).toEqual([]);
|
||||
expect(
|
||||
connected.getMalformedListItems().map((entry) => entry.label),
|
||||
).toEqual(["array_output"]);
|
||||
});
|
||||
|
||||
it("modern: the era rule accepts a tool but strips execution (pins it)", async () => {
|
||||
// The mirror image: here the strict aggregate SUCCEEDS, so no salvage runs
|
||||
// and what comes back is the SDK's own normalization. If it ever stops
|
||||
// stripping, `execution` reappears here and this fails.
|
||||
const server = await startToolsServer([EXECUTION_TOOL], true);
|
||||
stop = server.stop;
|
||||
const connected = await connect(server.url, "modern");
|
||||
|
||||
const { tools } = await connected.listAllTools();
|
||||
expect(tools).toHaveLength(1);
|
||||
expect(tools[0]).not.toHaveProperty("execution");
|
||||
expect(connected.getMalformedListItems()).toEqual([]);
|
||||
});
|
||||
|
||||
it("the neutral ToolSchema is looser than both, which is why the contract exists", () => {
|
||||
expect(ToolSchema.safeParse(ARRAY_OUTPUT_TOOL).success).toBe(true);
|
||||
const neutral = ToolSchema.safeParse(EXECUTION_TOOL);
|
||||
expect(neutral.success).toBe(true);
|
||||
expect(neutral.success && neutral.data).toHaveProperty("execution");
|
||||
});
|
||||
|
||||
it("names the offending field, not just 'invalid tool'", () => {
|
||||
// The era contract validates through a wire schema and forwards its issues,
|
||||
// so the PATH has to survive that hop — it is the whole diagnostic value of
|
||||
// the report, and a collapsed "Invalid input" would still pass every
|
||||
// accept/reject assertion above.
|
||||
const parsed = toolItemSchemaForEra(false).safeParse({
|
||||
name: "t",
|
||||
inputSchema: { type: "array" },
|
||||
});
|
||||
expect(parsed.success).toBe(false);
|
||||
expect(parsed.success || describeIssues(parsed.error)).toMatch(
|
||||
/^inputSchema\.type: .*object/,
|
||||
);
|
||||
});
|
||||
|
||||
it("strips unknown fields the way the strict codec does", async () => {
|
||||
// The contract validates through a wire schema; returning the ORIGINAL
|
||||
// value rather than the parsed one would leave salvaged tools carrying
|
||||
// fields the strict path drops, so the two paths would hand the UI
|
||||
// differently-shaped tools for the same server.
|
||||
const strayTool = {
|
||||
name: "stray",
|
||||
inputSchema: { type: "object" },
|
||||
notAToolField: "should not survive",
|
||||
};
|
||||
const parsed = toolItemSchemaForEra(false).safeParse(strayTool);
|
||||
expect(parsed.success).toBe(true);
|
||||
expect(parsed.success && parsed.data).not.toHaveProperty("notAToolField");
|
||||
|
||||
// And the strict path agrees — this is the shape being matched, not an
|
||||
// invented rule.
|
||||
const server = await startToolsServer([strayTool], false);
|
||||
stop = server.stop;
|
||||
const connected = await connect(server.url, "legacy");
|
||||
const { tools } = await connected.listAllTools();
|
||||
expect(tools[0]).not.toHaveProperty("notAToolField");
|
||||
});
|
||||
|
||||
it("the era contract matches the strict path on both divergences", () => {
|
||||
// Legacy: rejected, exactly as the strict parse above rejected it.
|
||||
expect(
|
||||
toolItemSchemaForEra(false).safeParse(ARRAY_OUTPUT_TOOL).success,
|
||||
).toBe(false);
|
||||
// Modern: accepted, with `execution` stripped, exactly as above.
|
||||
const modern = toolItemSchemaForEra(true).safeParse(EXECUTION_TOOL);
|
||||
expect(modern.success).toBe(true);
|
||||
expect(modern.success && modern.data).not.toHaveProperty("execution");
|
||||
});
|
||||
|
||||
/**
|
||||
* What the STRICT path did with a tool, decided without consulting our own
|
||||
* contract: the SDK accepted it only if the tool came back and nothing was
|
||||
* reported malformed. (If the SDK rejects and our contract accepts, salvage
|
||||
* finds nothing per-item wrong and rethrows — also a "reject" observation.)
|
||||
*/
|
||||
async function strictVerdict(
|
||||
tool: unknown,
|
||||
era: "legacy" | "modern",
|
||||
): Promise<"accept" | "reject"> {
|
||||
const server = await startToolsServer([tool], era === "modern");
|
||||
stop = server.stop;
|
||||
const connected = await connect(server.url, era);
|
||||
try {
|
||||
const { tools } = await connected.listAllTools();
|
||||
return tools.length === 1 &&
|
||||
connected.getMalformedListItems().length === 0
|
||||
? "accept"
|
||||
: "reject";
|
||||
} catch {
|
||||
return "reject";
|
||||
}
|
||||
}
|
||||
|
||||
const withInput = (inputSchema: unknown) => ({ name: "t", inputSchema });
|
||||
const withOutput = (outputSchema: unknown) => ({
|
||||
name: "t",
|
||||
inputSchema: { type: "object" },
|
||||
outputSchema,
|
||||
});
|
||||
|
||||
/**
|
||||
* Every measured divergence between the two eras and the neutral schema.
|
||||
*
|
||||
* Each row is checked twice: that the SDK still behaves this way, and that
|
||||
* our replicated contract agrees with it. The two directions both matter —
|
||||
* an `accept` we reject means salvage would drop a tool the strict path kept
|
||||
* and report a conforming server as malformed; a `reject` we accept means
|
||||
* salvage readmits an entry the era refused.
|
||||
*/
|
||||
const ERA_CASES: {
|
||||
label: string;
|
||||
tool: unknown;
|
||||
legacy: "accept" | "reject";
|
||||
modern: "accept" | "reject";
|
||||
}[] = [
|
||||
{
|
||||
label: "inputSchema.type: array",
|
||||
tool: withInput({ type: "array" }),
|
||||
legacy: "reject",
|
||||
modern: "reject",
|
||||
},
|
||||
{
|
||||
label: "inputSchema.properties: 5",
|
||||
tool: withInput({ type: "object", properties: 5 }),
|
||||
legacy: "reject",
|
||||
modern: "accept",
|
||||
},
|
||||
{
|
||||
label: "inputSchema.required: [1]",
|
||||
tool: withInput({ type: "object", required: [1] }),
|
||||
legacy: "reject",
|
||||
modern: "accept",
|
||||
},
|
||||
{
|
||||
label: "inputSchema.$schema: 5",
|
||||
tool: withInput({ type: "object", $schema: 5 }),
|
||||
legacy: "accept",
|
||||
modern: "reject",
|
||||
},
|
||||
{
|
||||
label: "outputSchema.type: array",
|
||||
tool: withOutput({ type: "array" }),
|
||||
legacy: "reject",
|
||||
modern: "accept",
|
||||
},
|
||||
{
|
||||
label: "outputSchema.properties: 5",
|
||||
tool: withOutput({ type: "object", properties: 5 }),
|
||||
legacy: "reject",
|
||||
modern: "accept",
|
||||
},
|
||||
{
|
||||
label: "outputSchema.required: [1]",
|
||||
tool: withOutput({ type: "object", required: [1] }),
|
||||
legacy: "reject",
|
||||
modern: "accept",
|
||||
},
|
||||
{
|
||||
label: "outputSchema.$schema: 5",
|
||||
tool: withOutput({ type: "object", $schema: 5 }),
|
||||
legacy: "accept",
|
||||
modern: "reject",
|
||||
},
|
||||
];
|
||||
|
||||
for (const era of ["legacy", "modern"] as const) {
|
||||
for (const { label, tool, ...expected } of ERA_CASES) {
|
||||
it(`${era}: ${label} — contract agrees with the strict path`, async () => {
|
||||
const observed = await strictVerdict(tool, era);
|
||||
// Pins the SDK: if this fails, the era rule changed and the replicated
|
||||
// contract below needs revisiting rather than the test relaxing.
|
||||
expect(observed).toBe(expected[era]);
|
||||
// Pins our agreement with it, in whichever direction it differs.
|
||||
expect(
|
||||
toolItemSchemaForEra(era === "modern").safeParse(tool).success,
|
||||
).toBe(expected[era] === "accept");
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
it("keeps only era-valid tools when salvaging beside a malformed one", async () => {
|
||||
// The end-to-end shape of the defect: a malformed entry trips the
|
||||
// fallback, and the array-output tool must NOT ride along on the strength
|
||||
// of the neutral schema accepting it.
|
||||
const server = await startToolsServer(
|
||||
[
|
||||
{ name: "ok_tool", inputSchema: { type: "object" } },
|
||||
ARRAY_OUTPUT_TOOL,
|
||||
{ name: "broken", inputSchema: "not-a-schema" },
|
||||
],
|
||||
false,
|
||||
);
|
||||
stop = server.stop;
|
||||
const connected = await connect(server.url, "legacy");
|
||||
|
||||
const { tools } = await connected.listAllTools();
|
||||
expect(tools.map((tool) => tool.name)).toEqual(["ok_tool"]);
|
||||
expect(
|
||||
connected.getMalformedListItems().map((entry) => entry.label),
|
||||
).toEqual(["array_output", "broken"]);
|
||||
});
|
||||
});
|
||||
44
clients/web/src/utils/malformedListReport.test.ts
Normal file
44
clients/web/src/utils/malformedListReport.test.ts
Normal file
|
|
@ -0,0 +1,44 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import type { MalformedListItem } from "@inspector/core/mcp";
|
||||
import { visibleMalformedListItems } from "./malformedListReport";
|
||||
|
||||
const entry = (method: string): MalformedListItem => ({
|
||||
method,
|
||||
index: 0,
|
||||
reason: "annotations: expected object",
|
||||
});
|
||||
|
||||
const TOOLS = entry("tools/list");
|
||||
const PROMPTS = entry("prompts/list");
|
||||
const RESOURCES = entry("resources/list");
|
||||
const TEMPLATES = entry("resources/templates/list");
|
||||
|
||||
describe("visibleMalformedListItems", () => {
|
||||
it("shows every report in aggregate mode", () => {
|
||||
const items = [TOOLS, PROMPTS, RESOURCES, TEMPLATES];
|
||||
expect(visibleMalformedListItems(items, false)).toEqual(items);
|
||||
});
|
||||
|
||||
it("returns the same array in aggregate mode rather than a copy", () => {
|
||||
// Identity is load-bearing: the value feeds a memo, and a fresh array every
|
||||
// render would re-render the panels for a filter that removed nothing.
|
||||
const items = [TOOLS];
|
||||
expect(visibleMalformedListItems(items, false)).toBe(items);
|
||||
});
|
||||
|
||||
it("suppresses the paged lists' reports in paginated mode", () => {
|
||||
// Those panels render the paged store, which neither writes nor clears this
|
||||
// report — so the entry would describe a list that is no longer on screen.
|
||||
expect(
|
||||
visibleMalformedListItems([TOOLS, PROMPTS, RESOURCES], true),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it("keeps resource templates, which are aggregate-backed in both modes", () => {
|
||||
// Templates have no paged store, so their report always describes the list
|
||||
// actually being rendered.
|
||||
expect(
|
||||
visibleMalformedListItems([TOOLS, TEMPLATES, PROMPTS], true),
|
||||
).toEqual([TEMPLATES]);
|
||||
});
|
||||
});
|
||||
46
clients/web/src/utils/malformedListReport.ts
Normal file
46
clients/web/src/utils/malformedListReport.ts
Normal file
|
|
@ -0,0 +1,46 @@
|
|||
import type { MalformedListItem } from "@inspector/core/mcp";
|
||||
|
||||
/**
|
||||
* Which malformed-entry reports the UI may show, given the pagination mode
|
||||
* (#1909 + #1721).
|
||||
*
|
||||
* The report is produced by the salvage fallback inside the client's `listAll*`
|
||||
* aggregate walk. In **paginated** mode the tools, prompts, and resources
|
||||
* panels do not render that aggregate at all — they render the paged store,
|
||||
* which fetches one page at a time through a different path that neither writes
|
||||
* nor clears this report. So a report written while the list was aggregated
|
||||
* would simply persist, sitting above a freshly fetched page it does not
|
||||
* describe: the same warning over a page that may be perfectly conforming, with
|
||||
* indices that refer to positions in an aggregate the user is no longer
|
||||
* looking at.
|
||||
*
|
||||
* Suppressing is the honest option of the two available. The alternative —
|
||||
* giving the paged path its own per-page salvage and report — is a real feature
|
||||
* rather than a display fix, and showing a stale warning is worse than showing
|
||||
* none: this panel's whole purpose is to say which entry the server got wrong,
|
||||
* so pointing at the wrong entry costs more than staying quiet.
|
||||
*
|
||||
* Resource templates are deliberately NOT suppressed. That list has no paged
|
||||
* store — it is fetched through the aggregate in both modes — so its report is
|
||||
* always about the list actually on screen.
|
||||
*/
|
||||
const PAGED_LIST_METHODS: readonly string[] = [
|
||||
"tools/list",
|
||||
"prompts/list",
|
||||
"resources/list",
|
||||
];
|
||||
|
||||
/**
|
||||
* The reports to render for the current pagination mode.
|
||||
*
|
||||
* Returns the input untouched in aggregate mode — the common case — so the
|
||||
* memoized identity is preserved and nothing re-renders for a filter that
|
||||
* removes nothing.
|
||||
*/
|
||||
export function visibleMalformedListItems(
|
||||
items: MalformedListItem[],
|
||||
paginated: boolean,
|
||||
): MalformedListItem[] {
|
||||
if (!paginated) return items;
|
||||
return items.filter((item) => !PAGED_LIST_METHODS.includes(item.method));
|
||||
}
|
||||
Some files were not shown because too many files have changed in this diff Show more
Loading…
Reference in a new issue