feat(web): collapse the schema-portability section by default (#2205)

The Tools tab rendered every schema-portability finding, always open, above
the argument form. On a server whose schemas are broadly unportable that put
the same wall of text ahead of every tool's first input — and the findings
address the server author, not the caller trying to fill the form.

The section now opens collapsed behind the same disclosure heading the Skills
pane's Conformance section uses, with an `N error(s), M warning(s)` badge. The
expand 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 while collapsed and the
tool-list severity icon is untouched, so nothing about a tool's standing is
hidden by the closed state.

Two neighbouring fixes on the same panel:

- `JsonEditor` frames Ace in a bordered `Paper`. Ace paints its background edge
  to edge with nothing around it, so "Edit as JSON" replaced a column of
  bordered inputs with one borderless slab.
- `ToolDetailPanel`'s body `ScrollArea` overrode the app-wide `type="scroll"`
  default with `type="auto"`, parking a permanent bar down the side of any tool
  whose form is taller than the panel. `offsetScrollbars` stays, so the form
  does not shift sideways when the bar fades in.

`test-servers/configs/unportable-schemas-many-http.json` reproduces the
original complaint: the same four presets as `unportable-schemas-http.json`
carrying 26 findings rather than 5.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DU3QYnc95g6EM4vo3hYNet
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-09-06 23:52:34 -04:00
commit db122f53db
11 changed files with 708 additions and 134 deletions

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`

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>
);
}

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();

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();
});
});

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>
);
}

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", () => {

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}

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);
});
});

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,
});
}

View file

@ -48,6 +48,7 @@ as a missing capability rather than an error.
| `nullable-fields-http.json` **(legacy era)** | Tools tab: nullable (`anyOf` + `null`) arguments | [#1928](https://github.com/modelcontextprotocol/inspector/issues/1928) |
| `root-union-schemas-http.json` **(legacy era)** | Tool schemas whose arguments are a root `anyOf` / `oneOf`, including one no branch of which can be offered | [#2123](https://github.com/modelcontextprotocol/inspector/issues/2123), [#2224](https://github.com/modelcontextprotocol/inspector/issues/2224) |
| `unportable-schemas-http.json` **(legacy era)** | Tool schemas a real client rejects, flagged in all three clients | [#1005](https://github.com/modelcontextprotocol/inspector/issues/1005) |
| `unportable-schemas-many-http.json` **(legacy era)** | The same constructs at **volume** — 26 findings over four tools, enough to bury the argument form | [#2205](https://github.com/modelcontextprotocol/inspector/issues/2205) |
| `rfc6570-templates-http.json` **(legacy era)** | Resources tab: RFC 6570 resource-template expansion | [#1919](https://github.com/modelcontextprotocol/inspector/issues/1919) |
| `advertised-extensions-http.json` **(legacy era)** | Tool registration gated on advertised extensions | [#1739](https://github.com/modelcontextprotocol/inspector/issues/1739) |
| `oauth-custom-resource-metadata-http.json` **(legacy era)** | OAuth discovery driven by the challenge's `resource_metadata` | [#2071](https://github.com/modelcontextprotocol/inspector/issues/2071) |
@ -349,6 +350,24 @@ has:
mcp-inspector --cli http://127.0.0.1:6603/mcp --method tools/list --strict # exits 6
```
### The same rules at volume
`unportable-schemas-many-http.json` (port 6613, legacy era) is the same four
presets carrying **26 findings** — `echo` 11, `add` 6, `get_temp` 5 on its
output schema, `get_weather` 4 — which is what a server generated from a
codebase that spells every nullable field `"type": ["string", "null"]` actually
looks like.
It exists for [#2205](https://github.com/modelcontextprotocol/inspector/issues/2205)
rather than for the rules themselves. Broken, selecting any tool here filled the
whole detail panel with findings and pushed the argument form off the bottom —
not one input field was reachable without scrolling, on every tool switch, and
the findings address the server author rather than the caller who is trying to
fill the form. Fixed, the section opens collapsed behind its
`N error(s), M warning(s)` badge and the form is on screen immediately; the
expand choice is global, so opening it once keeps it open across tools.
- **CLI** — `--strict` prints the full report (path, issue, suggested fix) on
stderr and exits `6` on an error-severity finding; without it, one summary
line. See [Schema portability](../clients/cli/README.md#schema-portability---strict).

View file

@ -0,0 +1,86 @@
{
"serverInfo": {
"name": "unportable-schemas-volume-showcase",
"version": "1.0.0"
},
"tools": [
{ "preset": "echo" },
{ "preset": "add" },
{ "preset": "get_weather" },
{ "preset": "get_temp" }
],
"rawToolSchemas": {
"echo": {
"inputSchema": {
"type": "object",
"properties": {
"message": { "type": "string" },
"prefix": { "type": ["string", "null"] },
"suffix": { "type": ["string", "null"] },
"repeat": { "type": ["integer", "null"] },
"uppercase": { "type": ["boolean", "null"] },
"separator": { "type": ["string", "null"] },
"locale": { "type": ["string", "null"] },
"metadata": {},
"context": { "description": "Anything the caller wants to attach." },
"passthrough": true,
"envelope": { "$ref": "https://example.com/schemas/envelope.json" },
"tags": {
"type": "array",
"items": { "type": ["string", "null"] }
}
},
"required": ["message"]
}
},
"add": {
"inputSchema": {
"type": "object",
"properties": {
"a": {
"type": "number",
"$ref": "https://example.com/schemas/number.json"
},
"b": {
"type": "number",
"$ref": "https://example.com/schemas/number.json"
},
"precision": { "type": ["integer", "null"] },
"rounding": { "type": ["string", "null"] },
"trace": {},
"audit": true
},
"required": ["a", "b"]
}
},
"get_weather": {
"inputSchema": {
"type": "object",
"properties": {
"city": { "type": "string" },
"units": { "type": ["string", "null"] },
"at": { "type": ["string", "null"] },
"hints": {},
"provider": { "$ref": "https://example.com/schemas/provider.json" }
},
"required": ["city"]
}
},
"get_temp": {
"outputSchema": {
"type": "object",
"properties": {
"temperature": { "type": ["number", "null"] },
"unit": { "type": ["string", "null"] },
"city": { "type": ["string", "null"] },
"data": true,
"diagnostics": {}
}
}
}
},
"transport": {
"type": "streamable-http",
"port": 6613
}
}