fix(web): make the Connection Info extension sections say something
Three things the Skills work got wrong in that modal, all found by review of the PR screenshots. - The "Skills Extension" section repeated `io.modelcontextprotocol/skills` as its value — the identifier the "Server Extensions" section two rows above already lists, so the section added nothing. What a flat key list *cannot* show is the extension's sub-options, which is the fact a server author opens this modal to check. Renamed "Skills Extension Options" and rendered as a ✓/✗ row for `directoryRead`, in the same vocabulary as the capability columns, so it reads as the same kind of claim. - The extension sections' contents were bold (`ValueText`, the value half of a label/value pair) while sitting directly beneath the capability checklists, which are plain. They are lists of items, not values, so they now use the same `Text` the checklist rows do. - Those lists were comma-joined into one line, which wraps mid-identifier in a half-width column. One row per identifier. The `skills-directory-read` tests now assert `data-supported` rather than the copy: "Not supported" contains "Supported", so a text assertion passed for either answer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018kfrH3uRNUM4JAk1reKtyY Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
1cb05ca573
commit
27fafe6acb
2 changed files with 73 additions and 37 deletions
|
|
@ -427,11 +427,12 @@ describe("ConnectionInfoContent", () => {
|
|||
expect(
|
||||
screen.getByText("Client Advertised Extensions"),
|
||||
).toBeInTheDocument();
|
||||
// One row per identifier, not a comma-joined string: two ~30-character
|
||||
// ids wrap mid-name in a half-width column.
|
||||
expect(
|
||||
screen.getByText(
|
||||
"io.modelcontextprotocol/tasks, io.modelcontextprotocol/ui",
|
||||
),
|
||||
screen.getByText("io.modelcontextprotocol/tasks"),
|
||||
).toBeInTheDocument();
|
||||
expect(screen.getByText("io.modelcontextprotocol/ui")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders em-dashes for the extensions sections when neither side advertises any (#1740)", () => {
|
||||
|
|
@ -461,7 +462,9 @@ describe("ConnectionInfoContent", () => {
|
|||
protocolEra="legacy"
|
||||
/>,
|
||||
);
|
||||
expect(screen.queryByText("Skills Extension")).not.toBeInTheDocument();
|
||||
expect(
|
||||
screen.queryByText("Skills Extension Options"),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows the Skills extension and its directoryRead sub-flag (#2234)", () => {
|
||||
|
|
@ -484,10 +487,16 @@ describe("ConnectionInfoContent", () => {
|
|||
protocolEra="legacy"
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByText("Skills Extension")).toBeInTheDocument();
|
||||
expect(screen.getByTestId("skills-directory-read")).toHaveTextContent(
|
||||
"Supported",
|
||||
expect(screen.getByText("Skills Extension Options")).toBeInTheDocument();
|
||||
// Asserted on the attribute, not the copy: "Not supported" contains
|
||||
// "Supported", so a text check would pass for either answer.
|
||||
expect(screen.getByTestId("skills-directory-read")).toHaveAttribute(
|
||||
"data-supported",
|
||||
"true",
|
||||
);
|
||||
// The section states the sub-option, not the identifier — that is already
|
||||
// in "Server Extensions" and repeating it would add nothing.
|
||||
expect(screen.getByText("resources/directory/read")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("reports directory read as unsupported for a bare skills declaration (#2234)", () => {
|
||||
|
|
@ -505,8 +514,9 @@ describe("ConnectionInfoContent", () => {
|
|||
protocolEra="legacy"
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByTestId("skills-directory-read")).toHaveTextContent(
|
||||
"Not supported",
|
||||
expect(screen.getByTestId("skills-directory-read")).toHaveAttribute(
|
||||
"data-supported",
|
||||
"false",
|
||||
);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ import {
|
|||
Button,
|
||||
Code,
|
||||
Flex,
|
||||
Group,
|
||||
ScrollArea,
|
||||
SimpleGrid,
|
||||
Stack,
|
||||
|
|
@ -18,7 +19,6 @@ import type {
|
|||
} from "@modelcontextprotocol/client";
|
||||
import type { ServerType } from "@inspector/core/mcp/types.js";
|
||||
import { TASKS_EXTENSION_KEY } from "@inspector/core/mcp/modernTaskSchemas.js";
|
||||
import { SKILLS_EXTENSION_KEY } from "@inspector/core/mcp/skillsSchemas.js";
|
||||
import { getSkillsExtension } from "@inspector/core/mcp/skills.js";
|
||||
import type { OAuthClientRegistrationKind } from "@inspector/core/auth/types.js";
|
||||
import {
|
||||
|
|
@ -106,6 +106,15 @@ const SectionHeading = Title.withProps({
|
|||
// `scrollable-region-focusable`).
|
||||
const ValueCode = Code.withProps({ variant: "wrapping" });
|
||||
|
||||
// One declared sub-option of an extension. Mirrors `CapabilityItem`'s ✓/✗ row
|
||||
// rather than reusing it: that element's `capability` prop is the closed union
|
||||
// of spec capability keys, and widening it to accept an arbitrary extension
|
||||
// sub-option name would collapse it to `string` and lose the typo protection
|
||||
// the union buys every other caller.
|
||||
const SubOptionRow = Group.withProps({ gap: "xs", wrap: "nowrap" });
|
||||
|
||||
const SubOptionMark = Text.withProps({ fw: 600 });
|
||||
|
||||
const ClearOAuthButton = Button.withProps({
|
||||
variant: "subtle",
|
||||
color: "red",
|
||||
|
|
@ -161,16 +170,20 @@ function formatSession(
|
|||
return isModernEra(era) ? "Sessionless" : "Session-based";
|
||||
}
|
||||
|
||||
// Render an `extensions` capability map (SEP-2133) as a comma-separated list of
|
||||
// its extension identifiers, or an em dash when none are present. Works for
|
||||
// either side's map: the server's negotiated `capabilities.extensions` (present
|
||||
// on both eras via `getServerCapabilities()`) or the Inspector's own advertised
|
||||
// The extension identifiers in an `extensions` capability map (SEP-2133), one
|
||||
// per rendered row, or a single em dash when none are present. Works for either
|
||||
// side's map: the server's negotiated `capabilities.extensions` (present on both
|
||||
// eras via `getServerCapabilities()`) or the Inspector's own advertised
|
||||
// `clientCapabilities.extensions`. (#1740)
|
||||
//
|
||||
// A list rather than a comma-joined string (#2234): an identifier is ~30
|
||||
// characters and two of them wrap mid-name in a half-width column, which is
|
||||
// what made the joined form hard to read at a glance.
|
||||
function formatExtensions(
|
||||
extensions: Record<string, unknown> | undefined,
|
||||
): string {
|
||||
): string[] {
|
||||
const keys = extensions ? Object.keys(extensions) : [];
|
||||
return keys.length > 0 ? keys.join(", ") : "—";
|
||||
return keys.length > 0 ? keys : ["\u2014"];
|
||||
}
|
||||
|
||||
const SERVER_CAPABILITY_KEYS: CapabilityKey[] = [
|
||||
|
|
@ -357,34 +370,47 @@ export function ConnectionInfoContent({
|
|||
<SimpleGrid cols={2}>
|
||||
<Stack gap="xs">
|
||||
<SectionHeading>Server Extensions</SectionHeading>
|
||||
<ValueText>{formatExtensions(capabilities.extensions)}</ValueText>
|
||||
{/* A plain `Text`, not the bold `ValueText`: these sections list
|
||||
*items*, the way the capability columns above do, rather than
|
||||
giving the value half of a label/value pair. Bolding them made
|
||||
them read as emphasized answers to a question the section never
|
||||
asks, and set them in a different font from the checklist rows
|
||||
they sit directly beneath. */}
|
||||
{formatExtensions(capabilities.extensions).map((extension) => (
|
||||
<Text key={extension}>{extension}</Text>
|
||||
))}
|
||||
</Stack>
|
||||
<Stack gap="xs">
|
||||
<SectionHeading>Client Advertised Extensions</SectionHeading>
|
||||
<ValueText>
|
||||
{formatExtensions(clientCapabilities.extensions)}
|
||||
</ValueText>
|
||||
{formatExtensions(clientCapabilities.extensions).map((extension) => (
|
||||
<Text key={extension}>{extension}</Text>
|
||||
))}
|
||||
</Stack>
|
||||
</SimpleGrid>
|
||||
|
||||
{/* Skills (SEP-2640). The generic "Server Extensions" row above lists the
|
||||
identifier, but not the one sub-option the extension defines —
|
||||
`directoryRead`, which gates `resources/directory/read`. That flag is
|
||||
exactly what a server author opens this modal to confirm, so it gets a
|
||||
row of its own rather than being flattened into a key list (#2234). */}
|
||||
{/* Skills (SEP-2640). The "Server Extensions" row above already names the
|
||||
identifier, so repeating it here would say nothing: what this section
|
||||
adds is the extension's SUB-OPTIONS, which a flat list of keys cannot
|
||||
show. `directoryRead` is the only one SEP-2640 defines, and whether a
|
||||
server declared it is the fact a server author opens this modal to
|
||||
check — it gates `resources/directory/read` (#2234). Rendered with the
|
||||
same ✓/✗ vocabulary as the capability columns above so it reads as the
|
||||
same kind of claim. */}
|
||||
{skillsExtension && (
|
||||
<SimpleGrid cols={2}>
|
||||
<Stack gap="xs">
|
||||
<SectionHeading>Skills Extension</SectionHeading>
|
||||
<ValueText>{SKILLS_EXTENSION_KEY}</ValueText>
|
||||
</Stack>
|
||||
<Stack gap="xs">
|
||||
<SectionHeading>Directory Read</SectionHeading>
|
||||
<ValueText data-testid="skills-directory-read">
|
||||
{skillsExtension.directoryRead ? "Supported" : "Not supported"}
|
||||
</ValueText>
|
||||
</Stack>
|
||||
</SimpleGrid>
|
||||
<Stack gap="xs">
|
||||
<SectionHeading>Skills Extension Options</SectionHeading>
|
||||
<SubOptionRow
|
||||
data-testid="skills-directory-read"
|
||||
data-supported={skillsExtension.directoryRead}
|
||||
>
|
||||
<SubOptionMark c={skillsExtension.directoryRead ? "green" : "red"}>
|
||||
{skillsExtension.directoryRead ? "\u2713" : "\u2717"}
|
||||
</SubOptionMark>
|
||||
<Text>
|
||||
Directory read — <Code>resources/directory/read</Code>
|
||||
</Text>
|
||||
</SubOptionRow>
|
||||
</Stack>
|
||||
)}
|
||||
|
||||
{instructions && (
|
||||
|
|
|
|||
Loading…
Reference in a new issue