modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

Merge pull request #2285 from modelcontextprotocol/v2/feat/2205-toggle-schema-warnings

Browse source 
feat(web): collapse the schema-portability section by default
This commit is contained in:
Cliff Hall 2026-09-07 00:16:02 -04:00 • committed by GitHub
commit b785bf3e69
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 708 additions and 134 deletions
Download patch file Download diff file Expand all files Collapse all files

17
clients/web/README.md
View file

@ -70,12 +70,27 @@ merely handled unevenly — and selecting the tool renders a **Schema
portability** section (`SchemaFindingsList`) above the argument form, one block
per finding with its path, the problem, and a concrete fix.
That section **opens collapsed**, behind a count badge reading
`N error(s), M warning(s)` (`#2205`). The findings address the *server author*
but render in the panel the *caller* fills in, so on a server whose schemas are
broadly unportable they used to put the same wall of text ahead of every tool's
first input —
`test-servers/configs/unportable-schemas-many-http.json` is 26 findings over
four tools, none of which a caller needs in order to fill the form. Expanding it
is one click, and the choice is **global rather than per tool**
(`useSchemaFindingsExpanded`, stored as `inspector.schemaFindings.expanded`):
this panel is reused across selections, so a per-tool disclosure would
re-collapse on every click and reproduce the same scrolling. The badge stays
visible either way, so nothing about a tool's standing is hidden by the closed
state.
Both read [`core/json/schemaLint.ts`](../../core/json/schemaLint.ts), which is
also what backs the TUI's detail pane and the CLI's `--strict` report — so the
three clients cannot disagree about whether a schema is portable. That module's
header explains why it is a portability lint rather than a JSON Schema
validator. `test-servers/configs/unportable-schemas-http.json` is a server that
exercises every rule.
exercises every rule, and `unportable-schemas-many-http.json` the same rules at
volume.
## Non-component code: `src/lib` vs `src/utils`

157
clients/web/src/components/elements/JsonEditor/JsonEditor.tsx
View file

@ -1,6 +1,6 @@
import { useEffect, useId, useRef } from "react";
import type { ReactNode } from "react";
import { Input, useComputedColorScheme } from "@mantine/core";
import { Input, Paper, useComputedColorScheme } from "@mantine/core";
import type { Ace } from "ace-builds";
import AceEditor from "react-ace";
import ace from "ace-builds/src-noconflict/ace";
@ -16,6 +16,25 @@ import jsonWorkerUrl from "ace-builds/src-noconflict/worker-json.js?url";
// it must not be repeated per mount.
ace.config.setModuleUrl("ace/mode/json_worker", jsonWorkerUrl);
/**
* The editor's frame.
*
* Ace paints its own background edge to edge with nothing around it, so an
* editor dropped into a panel reads as a discoloured patch rather than as a
* field — most visibly on the Tools tab, where "Edit as JSON" replaces a column
* of bordered inputs with one borderless slab. The border gives it the same
* edge every Mantine input beside it has.
*
* `variant="contained"` supplies the `overflow: hidden` that makes the rounded
* corners actually clip Ace's square background; without it the corners are
* painted over and the radius is invisible.
*/
const EditorFrame = Paper.withProps({
variant: "contained",
withBorder: true,
radius: "sm",
});
export interface JsonEditorProps {
/**
* The text the editor displays. This is a **text** contract, not a JSON one:
@ -228,73 +247,75 @@ export function JsonEditor({
opacity={disabled ? 0.6 : undefined}
w="100%"
>
<AceEditor
mode="json"
theme={colorScheme === "dark" ? "github_dark" : "github"}
// react-ace uses `name` as the editor container's DOM id, so a fixed
// value would collide the moment two of these render — and it must also
// differ from the id given to the textarea below, which is the one the
// wrapper's `<label for>` points at.
name={`${wrapperId}-editor`}
// A class, not a style: Ace renders its own DOM and paints a caret even
// when read-only, which reads as an editable field whose keystrokes are
// being swallowed. Hiding it needs a selector into that DOM, which is
// the same reason the gutter override in App.css exists.
//
// `""` rather than `undefined` for the off case: react-ace's
// `componentDidUpdate` reads `prevProps.className.trim()` whenever the
// class changes, with no guard — so going *to* a class from `undefined`
// throws, and going *from* one to `undefined` writes the literal class
// name "undefined" onto the element. Both are reachable here, because
// read-only is derived state: a tool form disables itself while a call
// is in flight, which flips this on an editor already mounted.
className={isReadOnly ? "json-editor-readonly" : ""}
value={value}
onChange={handleChange}
readOnly={isReadOnly}
width="100%"
minLines={minLines}
maxLines={maxLines}
tabSize={2}
showPrintMargin={false}
// A read-only editor is a rendering of someone else's payload, so it
// carries none of the caret furniture an editable one does.
highlightActiveLine={!isReadOnly}
editorProps={{ $blockScrolling: Infinity }}
onLoad={(editor) => {
editorRef.current = editor;
// `textInputAriaLabel` alone is not enough: Ace composes the hidden
// textarea's label as "<label>, Cursor at row N" inside
// `TextInput.setAriaLabel`, and only recomputes it when the cursor
// moves — so an option applied at mount does not reach the DOM until
// the user clicks into the editor. Until then the control's only
// accessible name is the position readout, which names nothing.
// Setting it here and recomputing once is what makes the editor
// identifiable from first paint.
editor.setOption("textInputAriaLabel", ariaLabel);
editor.textInput.setAriaLabel();
}}
setOptions={{
// The JSON worker is what puts a marker on the offending line rather
// than one message for the whole document. Pointless on a read-only
// document — the reader cannot act on what it flags — and actively
// misleading where the text is a *fragment* rendered for display.
useWorker: !isReadOnly,
// Closes a brace/bracket/quote when you open one, and skips over the
// closing one you then type. On by default; named because it is half
// of why this component exists.
behavioursEnabled: true,
// Roving-tabindex arrow-key navigation plus an Esc-to-exit-trap, so
// the editor does not swallow Tab for keyboard users.
enableKeyboardAccessibility: true,
textInputAriaLabel: ariaLabel,
showFoldWidgets: true,
displayIndentGuides: true,
useSoftTabs: true,
scrollPastEnd: false,
highlightGutterLine: !isReadOnly,
}}
/>
<EditorFrame>
<AceEditor
mode="json"
theme={colorScheme === "dark" ? "github_dark" : "github"}
// react-ace uses `name` as the editor container's DOM id, so a fixed
// value would collide the moment two of these render — and it must also
// differ from the id given to the textarea below, which is the one the
// wrapper's `<label for>` points at.
name={`${wrapperId}-editor`}
// A class, not a style: Ace renders its own DOM and paints a caret even
// when read-only, which reads as an editable field whose keystrokes are
// being swallowed. Hiding it needs a selector into that DOM, which is
// the same reason the gutter override in App.css exists.
//
// `""` rather than `undefined` for the off case: react-ace's
// `componentDidUpdate` reads `prevProps.className.trim()` whenever the
// class changes, with no guard — so going *to* a class from `undefined`
// throws, and going *from* one to `undefined` writes the literal class
// name "undefined" onto the element. Both are reachable here, because
// read-only is derived state: a tool form disables itself while a call
// is in flight, which flips this on an editor already mounted.
className={isReadOnly ? "json-editor-readonly" : ""}
value={value}
onChange={handleChange}
readOnly={isReadOnly}
width="100%"
minLines={minLines}
maxLines={maxLines}
tabSize={2}
showPrintMargin={false}
// A read-only editor is a rendering of someone else's payload, so it
// carries none of the caret furniture an editable one does.
highlightActiveLine={!isReadOnly}
editorProps={{ $blockScrolling: Infinity }}
onLoad={(editor) => {
editorRef.current = editor;
// `textInputAriaLabel` alone is not enough: Ace composes the hidden
// textarea's label as "<label>, Cursor at row N" inside
// `TextInput.setAriaLabel`, and only recomputes it when the cursor
// moves — so an option applied at mount does not reach the DOM until
// the user clicks into the editor. Until then the control's only
// accessible name is the position readout, which names nothing.
// Setting it here and recomputing once is what makes the editor
// identifiable from first paint.
editor.setOption("textInputAriaLabel", ariaLabel);
editor.textInput.setAriaLabel();
}}
setOptions={{
// The JSON worker is what puts a marker on the offending line rather
// than one message for the whole document. Pointless on a read-only
// document — the reader cannot act on what it flags — and actively
// misleading where the text is a *fragment* rendered for display.
useWorker: !isReadOnly,
// Closes a brace/bracket/quote when you open one, and skips over the
// closing one you then type. On by default; named because it is half
// of why this component exists.
behavioursEnabled: true,
// Roving-tabindex arrow-key navigation plus an Esc-to-exit-trap, so
// the editor does not swallow Tab for keyboard users.
enableKeyboardAccessibility: true,
textInputAriaLabel: ariaLabel,
showFoldWidgets: true,
displayIndentGuides: true,
useSoftTabs: true,
scrollPastEnd: false,
highlightGutterLine: !isReadOnly,
}}
/>
</EditorFrame>
</Input.Wrapper>
);
}

66
clients/web/src/components/elements/SchemaFindingsList/SchemaFindingsList.stories.tsx
View file

@ -1,5 +1,6 @@
import { useState } from "react";
import type { Meta, StoryObj } from "@storybook/react-vite";
import { expect, within } from "storybook/test";
import { expect, userEvent, waitFor, within } from "storybook/test";
import type { Tool } from "@modelcontextprotocol/client";
import { lintToolSchemas } from "@inspector/core/json/schemaLint.js";
import { SchemaFindingsList } from "./SchemaFindingsList";
@ -13,6 +14,19 @@ function findingsFor(tool: Partial<Tool>) {
const meta: Meta<typeof SchemaFindingsList> = {
title: "Elements/SchemaFindingsList",
component: SchemaFindingsList,
// The component is controlled (the expand preference is global — see
// `useSchemaFindingsExpanded`), so the stories own the state that the real
// caller keeps in localStorage. `args.expanded` seeds it.
render: function Render(args) {
const [expanded, setExpanded] = useState(args.expanded);
return (
<SchemaFindingsList
{...args}
expanded={expanded}
onExpandedChange={setExpanded}
/>
);
},
};
export default meta;
@ -21,6 +35,7 @@ type Story = StoryObj<typeof SchemaFindingsList>;
/** The reported case: Go's `jsonschema` emits `true` for an `interface{}`. */
export const BareTrueProperty: Story = {
args: {
expanded: true,
findings: findingsFor({
outputSchema: {
type: "object",
@ -30,7 +45,8 @@ export const BareTrueProperty: Story = {
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await expect(canvas.getByText("Schema portability (1)")).toBeVisible();
await expect(canvas.getByText("Schema portability")).toBeVisible();
await expect(canvas.getByText("1 error(s), 0 warning(s)")).toBeVisible();
await expect(canvas.getByText("error")).toBeVisible();
await expect(
canvas.getByText("outputSchema.properties.data"),
@ -41,6 +57,7 @@ export const BareTrueProperty: Story = {
/** Warning-only: an array-form `type`, legal but read unevenly. */
export const TypeUnionWarning: Story = {
args: {
expanded: true,
findings: findingsFor({
inputSchema: {
type: "object",
@ -58,6 +75,7 @@ export const TypeUnionWarning: Story = {
/** Both severities on one tool, which is what a real problem server looks like. */
export const Mixed: Story = {
args: {
expanded: true,
findings: findingsFor({
inputSchema: {
type: "object",
@ -72,13 +90,53 @@ export const Mixed: Story = {
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await expect(canvas.getByText("Schema portability (4)")).toBeVisible();
await expect(canvas.getByText("1 error(s), 3 warning(s)")).toBeVisible();
},
};
/**
* How the panel actually opens (#2205): one summary line, findings behind it.
* The counts stay on screen, so collapsing does not hide that the tool has
* findings — and one click brings the detail back.
*/
export const Collapsed: Story = {
args: {
expanded: false,
findings: findingsFor({
inputSchema: {
type: "object",
properties: {
show_ids: { type: ["null", "boolean"] },
extra: {},
ref: { $ref: "https://example.com/s.json" },
},
},
outputSchema: { type: "object", properties: { data: true } },
} as Partial<Tool>),
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await expect(canvas.getByText("1 error(s), 3 warning(s)")).toBeVisible();
const toggle = canvas.getByRole("button", { name: /Schema portability/ });
await expect(toggle).toHaveAttribute("aria-expanded", "false");
await userEvent.click(toggle);
await expect(toggle).toHaveAttribute("aria-expanded", "true");
// The panel animates open, so the reveal is only true once the collapse
// transition has run — asserting straight after the click races it.
await waitFor(async () => {
await expect(
canvas.getByText("outputSchema.properties.data"),
).toBeVisible();
});
},
};
/** The common case — nothing to say, so the section renders nothing at all. */
export const Clean: Story = {
args: { findings: [] },
args: { expanded: true, findings: [] },
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await expect(canvas.queryByTestId("schema-findings")).toBeNull();

147
clients/web/src/components/elements/SchemaFindingsList/SchemaFindingsList.test.tsx
View file

@ -1,5 +1,7 @@
import { describe, it, expect } from "vitest";
import { useState } from "react";
import { describe, it, expect, vi } from "vitest";
import { screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import type { Tool } from "@modelcontextprotocol/client";
import { lintToolSchemas } from "@inspector/core/json/schemaLint.js";
import { renderWithMantine } from "../../../test/renderWithMantine";
@ -9,21 +11,56 @@ function findingsFor(tool: Partial<Tool>) {
return lintToolSchemas({ name: "info", ...tool } as Tool);
}
/**
* Expanded is the default in these tests, so the assertions read against the
* findings themselves; the collapsed default belongs to the *caller*
* (`useSchemaFindingsExpanded`), which has its own tests. `onExpandedChange`
* is required, so a noop keeps the uncontrolled cases honest about it.
*/
function renderExpanded(findings: ReturnType<typeof findingsFor>) {
return renderWithMantine(
<SchemaFindingsList
findings={findings}
expanded
onExpandedChange={vi.fn()}
/>,
);
}
/** Drives the controlled `expanded` prop the way `ToolDetailPanel` does. */
function Controlled({
findings,
initial,
}: {
findings: ReturnType<typeof findingsFor>;
initial: boolean;
}) {
const [expanded, setExpanded] = useState(initial);
return (
<SchemaFindingsList
findings={findings}
expanded={expanded}
onExpandedChange={setExpanded}
/>
);
}
describe("SchemaFindingsList", () => {
it("renders nothing when there are no findings", () => {
renderWithMantine(<SchemaFindingsList findings={[]} />);
renderWithMantine(
<SchemaFindingsList findings={[]} expanded onExpandedChange={vi.fn()} />,
);
expect(screen.queryByTestId("schema-findings")).not.toBeInTheDocument();
});
it("shows the path, severity, issue and fix for an error finding", () => {
renderWithMantine(
<SchemaFindingsList
findings={findingsFor({
outputSchema: { type: "object", properties: { data: true } },
} as Partial<Tool>)}
/>,
renderExpanded(
findingsFor({
outputSchema: { type: "object", properties: { data: true } },
} as Partial<Tool>),
);
expect(screen.getByText("Schema portability (1)")).toBeInTheDocument();
expect(screen.getByText("Schema portability")).toBeInTheDocument();
expect(screen.getByText("1 error(s), 0 warning(s)")).toBeInTheDocument();
expect(screen.getByText("error")).toBeInTheDocument();
expect(
screen.getByText("outputSchema.properties.data"),
@ -33,29 +70,97 @@ describe("SchemaFindingsList", () => {
});
it("labels a warning finding as such", () => {
renderWithMantine(
<SchemaFindingsList
findings={findingsFor({
inputSchema: {
type: "object",
properties: { a: { type: ["null", "boolean"] } },
},
} as Partial<Tool>)}
/>,
renderExpanded(
findingsFor({
inputSchema: {
type: "object",
properties: { a: { type: ["null", "boolean"] } },
},
} as Partial<Tool>),
);
expect(screen.getByText("warning")).toBeInTheDocument();
expect(screen.queryByText("error")).not.toBeInTheDocument();
});
it("renders one block per finding", () => {
renderExpanded(
findingsFor({
inputSchema: { type: "object", properties: { a: true, b: true } },
} as Partial<Tool>),
);
expect(screen.getByText("2 error(s), 0 warning(s)")).toBeInTheDocument();
expect(screen.getAllByText("error")).toHaveLength(2);
});
// The point of #2205: the counts have to survive collapsing, or the closed
// state hides the fact that the tool has findings at all.
it("keeps the severity counts visible while collapsed", () => {
renderWithMantine(
<SchemaFindingsList
findings={findingsFor({
inputSchema: { type: "object", properties: { a: true, b: true } },
inputSchema: {
type: "object",
properties: { a: true, b: { type: ["null", "boolean"] } },
},
} as Partial<Tool>)}
expanded={false}
onExpandedChange={vi.fn()}
/>,
);
expect(screen.getByText("Schema portability (2)")).toBeInTheDocument();
expect(screen.getAllByText("error")).toHaveLength(2);
expect(screen.getByText("Schema portability")).toBeVisible();
expect(screen.getByText("1 error(s), 1 warning(s)")).toBeVisible();
expect(screen.getByText(/Bare `true`/)).not.toBeVisible();
});
it("reports its state on the toggle and reveals the findings when opened", async () => {
const user = userEvent.setup();
renderWithMantine(
<Controlled
findings={findingsFor({
outputSchema: { type: "object", properties: { data: true } },
} as Partial<Tool>)}
initial={false}
/>,
);
const toggle = screen.getByRole("button", { name: /Schema portability/ });
expect(toggle).toHaveAttribute("aria-expanded", "false");
expect(screen.getByText(/Bare `true`/)).not.toBeVisible();
await user.click(toggle);
expect(toggle).toHaveAttribute("aria-expanded", "true");
expect(screen.getByText(/Bare `true`/)).toBeVisible();
});
it("collapses again on a second click", async () => {
const user = userEvent.setup();
renderWithMantine(
<Controlled
findings={findingsFor({
outputSchema: { type: "object", properties: { data: true } },
} as Partial<Tool>)}
initial
/>,
);
const toggle = screen.getByRole("button", { name: /Schema portability/ });
expect(toggle).toHaveAttribute("aria-expanded", "true");
await user.click(toggle);
expect(toggle).toHaveAttribute("aria-expanded", "false");
});
it("points the toggle at the region it reveals", () => {
renderExpanded(
findingsFor({
outputSchema: { type: "object", properties: { data: true } },
} as Partial<Tool>),
);
const toggle = screen.getByRole("button", { name: /Schema portability/ });
const regionId = toggle.getAttribute("aria-controls");
expect(regionId).toBeTruthy();
expect(document.getElementById(regionId as string)).toBeInTheDocument();
});
});

145
clients/web/src/components/elements/SchemaFindingsList/SchemaFindingsList.tsx
View file

@ -1,22 +1,34 @@
import { Code, Group, Stack, Text } from "@mantine/core";
import { Accordion, Badge, Code, Group, Stack, Text } from "@mantine/core";
import { RiArrowRightSLine } from "react-icons/ri";
import {
describeSchemaPath,
type SchemaFinding,
} from "@inspector/core/json/schemaLint.js";
// Section wrapper: heading + one block per finding.
const FindingsSection = Stack.withProps({
gap: "xs",
/** The one accordion item, so the controlled value has a stable name. */
const SECTION_VALUE = "schema-portability";
// Section heading + count badge, side by side. A `Text` renders a `<p>`, so the
// heading must never *wrap* the badge — a `<div>` inside a `<p>` is invalid
// HTML that React reports as a hydration error. Same arrangement the Skills
// pane's Conformance header uses.
const SectionHeading = Text.withProps({
variant: "sectionHeading",
});
const FindingsTitle = Text.withProps({
size: "sm",
fw: 600,
});
const FindingsNote = Text.withProps({
const CountBadge = Badge.withProps({
size: "xs",
c: "var(--inspector-text-secondary)",
variant: "light",
});
const InlineRow = Group.withProps({
gap: "xs",
wrap: "nowrap",
});
// The findings themselves, inside the panel.
const FindingsBody = Stack.withProps({
gap: "xs",
});
// One finding: severity badge + path on the first row, then issue and fix.
@ -35,6 +47,11 @@ const FindingText = Text.withProps({
c: "var(--inspector-text-secondary)",
});
const FindingsNote = Text.withProps({
size: "xs",
c: "var(--inspector-text-secondary)",
});
/**
* Severity label.
*
@ -62,9 +79,24 @@ function severityColor(severity: SchemaFinding["severity"]): string {
: "var(--inspector-warning-text)";
}
/**
* Colour for the count badge, which summarises the whole list rather than one
* finding. There is deliberately no green case: the section renders nothing at
* all for a tool with no findings, so a clean badge could never appear.
*/
function summaryColor(errorCount: number): string {
return errorCount > 0 ? "red" : "yellow";
}
export interface SchemaFindingsListProps {
/** Findings for one tool, in walk order. Renders nothing when empty. */
findings: readonly SchemaFinding[];
/**
* Whether the findings are revealed. Controlled by the caller because the
* preference is global rather than per tool — see `useSchemaFindingsExpanded`.
*/
expanded: boolean;
onExpandedChange: (expanded: boolean) => void;
}
/**
@ -73,31 +105,78 @@ export interface SchemaFindingsListProps {
* The same verdict the CLI's `--strict` report and the TUI's detail pane show
* — all three read `core/json/schemaLint`, so they cannot disagree about
* whether a schema is portable, only about how much room they have to say so.
*
* Collapsed behind its count badge by default (#2205). The findings address the
* *server author*, but they render in the panel the *caller* fills in, above
* the argument form; on a server with broadly unportable schemas that put the
* same wall of text ahead of every tool's first input. The badge stays visible
* either way, so nothing about the tool's standing is hidden by the closed
* state.
*/
export function SchemaFindingsList({ findings }: SchemaFindingsListProps) {
export function SchemaFindingsList({
findings,
expanded,
onExpandedChange,
}: SchemaFindingsListProps) {
if (findings.length === 0) return null;
const errorCount = findings.filter((f) => f.severity === "error").length;
const warningCount = findings.length - errorCount;
return (
<FindingsSection data-testid="schema-findings">
<FindingsTitle>Schema portability ({findings.length})</FindingsTitle>
{findings.map((finding, index) => (
<FindingBlock
key={`${finding.schema}-${finding.path}-${finding.rule}-${index}`}
>
<FindingHeadRow>
<SeverityLabel c={severityColor(finding.severity)}>
{finding.severity}
</SeverityLabel>
<Code>{describeSchemaPath(finding.schema, finding.path)}</Code>
</FindingHeadRow>
<FindingText>{finding.issue}</FindingText>
<FindingText>Fix: {finding.suggestion}</FindingText>
</FindingBlock>
))}
<FindingsNote>
These constructs are legal JSON Schema but are refused or mishandled by
some MCP clients, so a tool can work here and fail there.
</FindingsNote>
</FindingsSection>
<Stack gap="xs" data-testid="schema-findings">
{/* Inline, not a `.withProps()` subcomponent: `Accordion` is a compound,
`multiple`-discriminated generic, and baking props into it loses the
JSX call signature (see AGENTS.md).
`variant="disclosure"` is the app's existing collapsible-section look
(#1462) — the same one the Skills pane's Conformance section uses, so
a section heading with a severity badge reads the same wherever it
appears. `multiple` only so the controlled value is an array; there is
one item. */}
<Accordion
multiple
variant="disclosure"
chevron={<RiArrowRightSLine />}
value={expanded ? [SECTION_VALUE] : []}
onChange={(value) => onExpandedChange(value.includes(SECTION_VALUE))}
>
<Accordion.Item value={SECTION_VALUE}>
<Accordion.Control>
<InlineRow>
<SectionHeading>Schema portability</SectionHeading>
<CountBadge color={summaryColor(errorCount)}>
{errorCount} error(s), {warningCount} warning(s)
</CountBadge>
</InlineRow>
</Accordion.Control>
<Accordion.Panel>
<FindingsBody>
{findings.map((finding, index) => (
<FindingBlock
key={`${finding.schema}-${finding.path}-${finding.rule}-${index}`}
>
<FindingHeadRow>
<SeverityLabel c={severityColor(finding.severity)}>
{finding.severity}
</SeverityLabel>
<Code>
{describeSchemaPath(finding.schema, finding.path)}
</Code>
</FindingHeadRow>
<FindingText>{finding.issue}</FindingText>
<FindingText>Fix: {finding.suggestion}</FindingText>
</FindingBlock>
))}
<FindingsNote>
These constructs are legal JSON Schema but are refused or
mishandled by some MCP clients, so a tool can work here and fail
there.
</FindingsNote>
</FindingsBody>
</Accordion.Panel>
</Accordion.Item>
</Accordion>
</Stack>
);
}

69
clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.test.tsx
View file

@ -1,4 +1,4 @@
import { describe, it, expect, vi } from "vitest";
import { describe, it, expect, vi, beforeEach } from "vitest";
import userEvent from "@testing-library/user-event";
import type { Tool } from "@modelcontextprotocol/client";
import { renderWithMantine, screen } from "../../../test/renderWithMantine";
@ -536,21 +536,66 @@ describe("ToolDetailPanel", () => {
});
describe("schema portability findings (#1005)", () => {
it("lists a finding with its path when a schema is unportable", () => {
const unportableTool: Tool = {
name: "info",
inputSchema: { type: "object", properties: {} },
outputSchema: { type: "object", properties: { data: true } },
};
// The findings address the server author but render above the argument
// form the caller fills in, so they open collapsed behind their counts
// (#2205). The preference is global, hence the cleared localStorage.
beforeEach(() => {
window.localStorage.clear();
});
it("summarizes the findings without unfurling them", () => {
renderWithMantine(
<ToolDetailPanel
{...baseProps}
tool={{
name: "info",
inputSchema: { type: "object", properties: {} },
outputSchema: { type: "object", properties: { data: true } },
}}
/>,
<ToolDetailPanel {...baseProps} tool={unportableTool} />,
);
expect(screen.getByText("Schema portability (1)")).toBeInTheDocument();
expect(screen.getByText("Schema portability")).toBeVisible();
expect(screen.getByText("1 error(s), 0 warning(s)")).toBeVisible();
expect(
screen.getByText("outputSchema.properties.data"),
).toBeInTheDocument();
).not.toBeVisible();
});
it("lists a finding with its path once expanded", async () => {
const user = userEvent.setup();
renderWithMantine(
<ToolDetailPanel {...baseProps} tool={unportableTool} />,
);
await user.click(
screen.getByRole("button", { name: /Schema portability/ }),
);
expect(screen.getByText("outputSchema.properties.data")).toBeVisible();
});
// Global rather than per tool: the panel is reused across selections, so a
// per-tool disclosure would re-collapse on every click — the scrolling
// #2205 is about, by another route.
it("keeps the expanded choice across a tool switch", async () => {
const user = userEvent.setup();
const { rerender } = renderWithMantine(
<ToolDetailPanel {...baseProps} tool={unportableTool} />,
);
await user.click(
screen.getByRole("button", { name: /Schema portability/ }),
);
rerender(
<ToolDetailPanel
{...baseProps}
tool={{ ...unportableTool, name: "other" }}
/>,
);
expect(
screen.getByRole("button", { name: /Schema portability/ }),
).toHaveAttribute("aria-expanded", "true");
});
it("omits the section for a portable tool", () => {

20
clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.tsx
View file

@ -27,6 +27,7 @@ import { getMirroredHeaderParams } from "@inspector/core/json/xMcpHeader.js";
import { lintToolSchemas } from "@inspector/core/json/schemaLint.js";
import { AnnotationBadge } from "../../elements/AnnotationBadge/AnnotationBadge";
import { SchemaFindingsList } from "../../elements/SchemaFindingsList/SchemaFindingsList";
import { useSchemaFindingsExpanded } from "../../../hooks/useSchemaFindingsExpanded";
import { ProgressDisplay } from "../../elements/ProgressDisplay/ProgressDisplay";
import { SchemaForm } from "../SchemaForm/SchemaForm";
@ -88,7 +89,13 @@ const BodyScroll = ScrollArea.withProps({
flex: "0 1 auto",
miw: 0,
mih: 0,
type: "auto",
// No `type` override: the app-wide default is `type="scroll"` (see
// `src/theme/ScrollArea.ts`), which shows the bar only while the user is
// actually scrolling. `type="auto"` parked a permanent bar down the side of
// every tool whose form is taller than the panel — which, now that the
// schema-portability section opens collapsed, is the ordinary case rather
// than the exception. `offsetScrollbars` stays: it reserves the gutter, so
// the form does not shift sideways when the bar fades in.
scrollbars: "y",
offsetScrollbars: true,
});
@ -224,6 +231,11 @@ export function ToolDetailPanel({
// Memoized on the tool: this panel re-renders on every keystroke in the
// argument form, and the walk depends on nothing that changes in between.
const schemaFindings = useMemo(() => lintToolSchemas(tool), [tool]);
// Global rather than per tool, so the choice survives a tool switch — see
// the hook. This panel is reused across selections, so per-tool state here
// would re-open the wall on every click anyway.
const [schemaFindingsExpanded, setSchemaFindingsExpanded] =
useSchemaFindingsExpanded();
// Descriptions are shown by default (most are short); the chevron lets the
// user hide a long one to keep the form and Execute footer in view. Reset to
@ -343,7 +355,11 @@ export function ToolDetailPanel({
</HeaderParamsSection>
)}
<SchemaFindingsList findings={schemaFindings} />
<SchemaFindingsList
findings={schemaFindings}
expanded={schemaFindingsExpanded}
onExpandedChange={setSchemaFindingsExpanded}
/>
<SchemaForm
schema={formSchema}

76
clients/web/src/hooks/useSchemaFindingsExpanded.test.tsx Normal file
View file

@ -0,0 +1,76 @@
import { describe, it, expect, beforeEach, afterEach } from "vitest";
import { act, renderHook } from "@testing-library/react";
import {
SCHEMA_FINDINGS_EXPANDED_KEY,
useSchemaFindingsExpanded,
} from "./useSchemaFindingsExpanded";
describe("useSchemaFindingsExpanded", () => {
beforeEach(() => {
window.localStorage.clear();
});
afterEach(() => {
window.localStorage.clear();
});
// Collapsed by default is the whole fix (#2205): the wall of findings must
// not sit between the caller and the argument form on a fresh install.
it("starts collapsed when nothing is stored", () => {
const { result } = renderHook(() => useSchemaFindingsExpanded());
expect(result.current[0]).toBe(false);
});
it("reads a stored preference synchronously on first render", () => {
window.localStorage.setItem(SCHEMA_FINDINGS_EXPANDED_KEY, "true");
const { result } = renderHook(() => useSchemaFindingsExpanded());
expect(result.current[0]).toBe(true);
});
it("reads a stored collapsed preference back as collapsed", () => {
window.localStorage.setItem(SCHEMA_FINDINGS_EXPANDED_KEY, "false");
const { result } = renderHook(() => useSchemaFindingsExpanded());
expect(result.current[0]).toBe(false);
});
// A manual edit or a value written by another build must land on the default
// rather than silently coercing to `false` — the same clamp-on-read shape the
// sort/compact preferences in InspectorView use.
it("clamps an unrecognized stored value back to the default", () => {
window.localStorage.setItem(SCHEMA_FINDINGS_EXPANDED_KEY, "yes please");
const { result } = renderHook(() => useSchemaFindingsExpanded());
expect(result.current[0]).toBe(false);
});
it("persists the choice as a human-readable boolean", () => {
const { result } = renderHook(() => useSchemaFindingsExpanded());
act(() => {
result.current[1](true);
});
expect(result.current[0]).toBe(true);
expect(window.localStorage.getItem(SCHEMA_FINDINGS_EXPANDED_KEY)).toBe(
"true",
);
act(() => {
result.current[1](false);
});
expect(result.current[0]).toBe(false);
expect(window.localStorage.getItem(SCHEMA_FINDINGS_EXPANDED_KEY)).toBe(
"false",
);
});
// Global, not per tool: a second consumer mounted later sees the choice the
// first one made, which is what makes the preference survive a tool switch.
it("hands the stored choice to a later consumer", () => {
const { result: first } = renderHook(() => useSchemaFindingsExpanded());
act(() => {
first.current[1](true);
});
const { result: second } = renderHook(() => useSchemaFindingsExpanded());
expect(second.current[0]).toBe(true);
});
});

54
clients/web/src/hooks/useSchemaFindingsExpanded.ts Normal file
View file

@ -0,0 +1,54 @@
import { useLocalStorage } from "@mantine/hooks";
/**
* Whether the tool detail panel's schema-portability section is expanded
* (#2205).
*
* The preference is **global rather than per tool**, and that is the whole
* point of the issue: the findings sit above the argument form, so on a server
* whose schemas are broadly unportable every tool selection put a wall of
* identical text between the user and the first input — 26 findings across
* four tools in `unportable-schemas-many-http.json`, none of which the caller
* needs in order to fill the form. A per-tool disclosure would have to be
* re-collapsed on every selection, which is the same scrolling by another
* route.
*
* Collapsed is the default. Nothing is lost by it: the summary line still
* names the counts, the tool list still carries its per-tool severity icon,
* and one click re-opens the detail — for the whole session and every later
* one, since the choice persists.
*/
const SCHEMA_FINDINGS_EXPANDED_DEFAULT = false;
/** localStorage key. Shares the `inspector.<kind>.<scope>` namespace the other
* UI preferences use, so the whole group is easy to inspect or clear in bulk. */
export const SCHEMA_FINDINGS_EXPANDED_KEY = "inspector.schemaFindings.expanded";
/**
* Stores the boolean as `"true"` / `"false"` rather than Mantine's default
* `JSON.stringify`, matching the sort/compact adapters in `InspectorView`:
* the persisted value stays human-readable, and anything else — a manual edit,
* a value written by an older build — clamps back to the default instead of
* silently coercing to `false`.
*/
function deserialize(raw: string | undefined): boolean {
if (raw === "true") return true;
if (raw === "false") return false;
return SCHEMA_FINDINGS_EXPANDED_DEFAULT;
}
function serialize(value: boolean): string {
return value ? "true" : "false";
}
export function useSchemaFindingsExpanded() {
return useLocalStorage<boolean>({
key: SCHEMA_FINDINGS_EXPANDED_KEY,
defaultValue: SCHEMA_FINDINGS_EXPANDED_DEFAULT,
deserialize,
serialize,
// Read synchronously on first render — SPA only, no SSR — so a persisted
// "expanded" does not flash through the collapsed default.
getInitialValueInEffect: false,
});
}