Merge pull request #2251 from modelcontextprotocol/v2/feat/2234-skills-extension
feat: support the Skills extension (SEP-2640) with digest verification
This commit is contained in:
commit
ed2a188069
45 changed files with 6916 additions and 18 deletions
|
|
@ -93,10 +93,12 @@ Nothing _enforces_ the boundary — no path alias keys off it, and the coverage
|
|||
|
||||
## Core tab automation contract
|
||||
|
||||
The Tools, Resources, and Prompts screens each expose a `data-testid` plus a
|
||||
The Tools, Resources, Prompts and Skills screens each expose a `data-testid` plus a
|
||||
small set of `data-*` attributes, so a headless driver can `waitForSelector` on
|
||||
a deterministic signal rather than on visible copy. `scripts/smoke-web-tabs.mjs`
|
||||
drives all three against `test-servers/configs/web-tabs-http.json` ([#2148](https://github.com/modelcontextprotocol/inspector/issues/2148)).
|
||||
drives the first three of those four against `test-servers/configs/web-tabs-http.json`
|
||||
([#2148](https://github.com/modelcontextprotocol/inspector/issues/2148)); Skills
|
||||
publishes the same contract but is not smoked yet ([#2234](https://github.com/modelcontextprotocol/inspector/issues/2234)).
|
||||
Treat them as a public contract, for the same reason as the Apps ones below:
|
||||
|
||||
| Attribute | Where | Meaning |
|
||||
|
|
@ -114,6 +116,10 @@ Treat them as a public contract, for the same reason as the Apps ones below:
|
|||
| `data-prompt-count` | on `prompts-screen`| Entries from `prompts/list`. |
|
||||
| `data-get-status` | on `prompts-screen`| `idle` → `pending` → `ok` / `error` for the current `prompts/get`. |
|
||||
| `data-testid="prompt-messages"` | messages panel | The fetched prompt's **rendered** messages — the `prompts/get` counterpart of `resource-preview`, and asserted alongside `data-get-status` for the same reason. |
|
||||
| `data-testid="skills-screen"` | Skills root | The element carrying the two attributes below ([#2234](https://github.com/modelcontextprotocol/inspector/issues/2234)). Not driven by `smoke-web-tabs.mjs` yet — the attributes exist so it can be, and so a rename fails in the screen's unit test rather than later. |
|
||||
| `data-skill-count` | on `skills-screen` | Entries accumulated from `skills/list`. |
|
||||
| `data-skill-page-count` | on `skills-screen` | Pages the last `skills/list` walk took — a **separate** fact from the count, and the one that shows pagination actually happened. |
|
||||
| `data-testid="skill-manifest"` | detail pane | The selected skill's resource manifest. Absent for a `resources: "dynamic"` skill, which has no manifest to render. |
|
||||
|
||||
Why attributes rather than text: a smoke that waited on a label fails the next
|
||||
time the label is reworded, which is noise rather than signal — and it fails as
|
||||
|
|
|
|||
|
|
@ -75,6 +75,7 @@ import type {
|
|||
ResourcesPanelProps,
|
||||
ServerListProps,
|
||||
ShellProps,
|
||||
SkillsPanelProps,
|
||||
TasksPanelProps,
|
||||
ToolsPanelProps,
|
||||
} from "./components/views/InspectorView/types";
|
||||
|
|
@ -458,6 +459,11 @@ function App() {
|
|||
tasks,
|
||||
refreshTasks,
|
||||
clearCompletedTasks,
|
||||
sessionNonce,
|
||||
skills,
|
||||
skillsPageCount,
|
||||
skillsLoadError,
|
||||
refreshSkills,
|
||||
subscriptions,
|
||||
subscriptionStreamState,
|
||||
messages,
|
||||
|
|
@ -828,6 +834,9 @@ function App() {
|
|||
onRefreshTools,
|
||||
onRefreshPrompts,
|
||||
onRefreshResources,
|
||||
onRefreshSkills,
|
||||
onReadSkillFile,
|
||||
onGetSkill,
|
||||
onRefreshTasks,
|
||||
onTogglePaginatedLists,
|
||||
onLoadMoreTools,
|
||||
|
|
@ -851,6 +860,7 @@ function App() {
|
|||
activeToolCallTaskIdRef,
|
||||
clearCompletedTasks,
|
||||
refreshTasks,
|
||||
refreshSkills,
|
||||
paginatedLists,
|
||||
paginatedListsOverride,
|
||||
toolsPagination,
|
||||
|
|
@ -1816,6 +1826,21 @@ function App() {
|
|||
onRefreshApps: onRefreshTools,
|
||||
};
|
||||
|
||||
const skillsPanelProps: SkillsPanelProps = {
|
||||
// Server id AND per-connect nonce: the id alone would repeat on a
|
||||
// reconnect to the same server, which is one of the crossings this key
|
||||
// exists to prevent.
|
||||
skillsSessionKey: `${activeServerId ?? ""}:${sessionNonce}`,
|
||||
skills,
|
||||
skillsPageCount,
|
||||
skillsLoadError,
|
||||
skillsUi: ui.skillsUi,
|
||||
onSkillsUiChange: setUi.setSkillsUi,
|
||||
onRefreshSkills,
|
||||
onReadSkillFile,
|
||||
onGetSkill,
|
||||
};
|
||||
|
||||
const tasksPanelProps: TasksPanelProps = {
|
||||
tasks,
|
||||
progressByTaskId,
|
||||
|
|
@ -1889,6 +1914,7 @@ function App() {
|
|||
prompts={promptsPanelProps}
|
||||
resources={resourcesPanelProps}
|
||||
apps={appsPanelProps}
|
||||
skills={skillsPanelProps}
|
||||
tasks={tasksPanelProps}
|
||||
logs={logsPanelProps}
|
||||
protocol={protocolPanelProps}
|
||||
|
|
|
|||
|
|
@ -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)", () => {
|
||||
|
|
@ -452,6 +453,73 @@ describe("ConnectionInfoContent", () => {
|
|||
expect(screen.getAllByText("—")).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("hides the Skills section when the server declares no skills extension (#2234)", () => {
|
||||
renderWithMantine(
|
||||
<ConnectionInfoContent
|
||||
initializeResult={fullResult}
|
||||
clientCapabilities={fullClientCaps}
|
||||
transport="streamable-http"
|
||||
protocolEra="legacy"
|
||||
/>,
|
||||
);
|
||||
expect(
|
||||
screen.queryByText("Skills Extension Options"),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows the Skills extension and its directoryRead sub-flag (#2234)", () => {
|
||||
// The generic "Server Extensions" row lists the identifier; the sub-flag
|
||||
// that gates `resources/directory/read` is what this section adds, and it
|
||||
// is the fact a server author opens the modal to confirm.
|
||||
renderWithMantine(
|
||||
<ConnectionInfoContent
|
||||
initializeResult={{
|
||||
...fullResult,
|
||||
capabilities: {
|
||||
...fullResult.capabilities,
|
||||
extensions: {
|
||||
"io.modelcontextprotocol/skills": { directoryRead: true },
|
||||
},
|
||||
},
|
||||
}}
|
||||
clientCapabilities={fullClientCaps}
|
||||
transport="streamable-http"
|
||||
protocolEra="legacy"
|
||||
/>,
|
||||
);
|
||||
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)", () => {
|
||||
renderWithMantine(
|
||||
<ConnectionInfoContent
|
||||
initializeResult={{
|
||||
...fullResult,
|
||||
capabilities: {
|
||||
...fullResult.capabilities,
|
||||
extensions: { "io.modelcontextprotocol/skills": {} },
|
||||
},
|
||||
}}
|
||||
clientCapabilities={fullClientCaps}
|
||||
transport="streamable-http"
|
||||
protocolEra="legacy"
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByTestId("skills-directory-read")).toHaveAttribute(
|
||||
"data-supported",
|
||||
"false",
|
||||
);
|
||||
});
|
||||
|
||||
it("renders client registration kind when provided", () => {
|
||||
renderWithMantine(
|
||||
<ConnectionInfoContent
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ import {
|
|||
Button,
|
||||
Code,
|
||||
Flex,
|
||||
Group,
|
||||
ScrollArea,
|
||||
SimpleGrid,
|
||||
Stack,
|
||||
|
|
@ -18,6 +19,7 @@ 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 { getSkillsExtension } from "@inspector/core/mcp/skills.js";
|
||||
import type { OAuthClientRegistrationKind } from "@inspector/core/auth/types.js";
|
||||
import {
|
||||
CapabilityItem,
|
||||
|
|
@ -104,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",
|
||||
|
|
@ -159,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[] = [
|
||||
|
|
@ -259,6 +274,9 @@ export function ConnectionInfoContent({
|
|||
}: ConnectionInfoContentProps) {
|
||||
const { serverInfo, protocolVersion, capabilities, instructions } =
|
||||
initializeResult;
|
||||
// `undefined` when the server declared no Skills extension, which is what
|
||||
// hides the section below — an absent extension has no sub-flags to report.
|
||||
const skillsExtension = getSkillsExtension(capabilities);
|
||||
|
||||
// Only trust `serverInfo` when the server actually reported it; otherwise the
|
||||
// name is a catalog fallback. Both rows `?.trim()` before the `||` (not `??`)
|
||||
|
|
@ -352,16 +370,49 @@ 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 "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 && (
|
||||
<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 && (
|
||||
<Stack gap="xs">
|
||||
<SectionHeading>Server Instructions</SectionHeading>
|
||||
|
|
|
|||
|
|
@ -0,0 +1,159 @@
|
|||
import { useState } from "react";
|
||||
import type { ComponentProps } from "react";
|
||||
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||
import { expect, fn, userEvent, within } from "storybook/test";
|
||||
import type { SkillEntry } from "@inspector/core/mcp/skillsSchemas";
|
||||
import { SkillsScreen } from "./SkillsScreen";
|
||||
import type { SkillsUiState } from "./SkillsScreen";
|
||||
import { EMPTY_SKILLS_UI } from "../screenUiState";
|
||||
|
||||
// SkillsScreen is controlled (selection and search live in the parent as one
|
||||
// `ui` object — see #1417). This wrapper holds that state so the play-driven
|
||||
// clicks drive the detail pane, mirroring how App owns it in the real app.
|
||||
function StatefulSkillsScreen(args: ComponentProps<typeof SkillsScreen>) {
|
||||
const [ui, setUi] = useState<SkillsUiState>(args.ui ?? EMPTY_SKILLS_UI);
|
||||
return <SkillsScreen {...args} ui={ui} onUiChange={setUi} />;
|
||||
}
|
||||
|
||||
const REF_TEXT = "# Column rules\n";
|
||||
const SELF_TEXT = "# skill\n";
|
||||
// The real digests of those two strings, so the clean skill actually verifies
|
||||
// when the "Verify all" story runs — a placeholder would demo a false green.
|
||||
const REF_DIGEST =
|
||||
"sha256:e201429aa2684958ca1a0537ab4eb4b7eb3a81c71e7cc7a11397eb500738e015";
|
||||
const SELF_DIGEST =
|
||||
"sha256:6504f2de0a1febf7492c3b98f93d9ab49558eb364607a706f02fe9a75aa7f75b";
|
||||
|
||||
/** Every manifest lists the skill's own SKILL.md — a manifest is the complete
|
||||
* file set, so one that omits it is a `manifest-missing-self` error. */
|
||||
const selfEntry = (path: string) => ({
|
||||
uri: `skill://${path}/SKILL.md`,
|
||||
digest: SELF_DIGEST,
|
||||
size: 8,
|
||||
});
|
||||
|
||||
const sampleSkills: SkillEntry[] = [
|
||||
{
|
||||
uri: "skill://data-analysis/SKILL.md",
|
||||
frontmatter: {
|
||||
name: "data-analysis",
|
||||
description: "Analyze a CSV and summarize its columns",
|
||||
},
|
||||
resources: [
|
||||
selfEntry("data-analysis"),
|
||||
{
|
||||
uri: "skill://data-analysis/reference.md",
|
||||
digest: REF_DIGEST,
|
||||
size: 15,
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
uri: "skill://tampered-notes/SKILL.md",
|
||||
frontmatter: {
|
||||
name: "tampered-notes",
|
||||
description: "Advertises a digest its bytes do not match",
|
||||
},
|
||||
resources: [
|
||||
selfEntry("tampered-notes"),
|
||||
{
|
||||
// A well-formed digest of bytes the fake read does not return, with a
|
||||
// size that agrees — so the reported failure is a *digest* mismatch
|
||||
// rather than the cheaper size cross-check.
|
||||
uri: "skill://tampered-notes/notes.md",
|
||||
digest: `sha256:${"b".repeat(64)}`,
|
||||
size: 8,
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
uri: "skill://dynamic-report/SKILL.md",
|
||||
frontmatter: {
|
||||
name: "dynamic-report",
|
||||
description: "Generated files, so integrity cannot be verified",
|
||||
},
|
||||
resources: "dynamic",
|
||||
},
|
||||
{
|
||||
uri: "skill://wrong-folder/SKILL.md",
|
||||
frontmatter: {
|
||||
name: "right-name",
|
||||
description: "URI path segment disagrees with frontmatter.name",
|
||||
},
|
||||
resources: [selfEntry("wrong-folder")],
|
||||
},
|
||||
];
|
||||
|
||||
const meta: Meta<typeof SkillsScreen> = {
|
||||
title: "Screens/SkillsScreen",
|
||||
component: SkillsScreen,
|
||||
parameters: { layout: "fullscreen" },
|
||||
args: {
|
||||
skills: sampleSkills,
|
||||
pageCount: 2,
|
||||
ui: EMPTY_SKILLS_UI,
|
||||
onUiChange: fn(),
|
||||
onRefreshList: fn(),
|
||||
onReadSkillFile: fn(async (uri: string) =>
|
||||
uri.endsWith("reference.md")
|
||||
? { text: REF_TEXT }
|
||||
: { text: SELF_TEXT, mimeType: "text/markdown" },
|
||||
),
|
||||
// Echoes back the entry `skills/list` advertised, so "Fetch with
|
||||
// skills/get" demonstrates the matching case rather than throwing.
|
||||
onGetSkill: fn(async (uri: string) => {
|
||||
const found = sampleSkills.find((skill) => skill.uri === uri);
|
||||
if (!found) throw new Error(`Unknown skill uri: ${uri}`);
|
||||
return found;
|
||||
}),
|
||||
},
|
||||
render: (args) => <StatefulSkillsScreen {...args} />,
|
||||
};
|
||||
|
||||
export default meta;
|
||||
type Story = StoryObj<typeof SkillsScreen>;
|
||||
|
||||
export const Default: Story = {};
|
||||
|
||||
export const Empty: Story = {
|
||||
args: { skills: [], pageCount: 0 },
|
||||
};
|
||||
|
||||
export const LoadFailed: Story = {
|
||||
args: { loadError: new Error("skills/list failed: -32601 Method not found") },
|
||||
};
|
||||
|
||||
export const ConformingSkill: Story = {
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
await userEvent.click(canvas.getByText("data-analysis"));
|
||||
await expect(canvas.getByText("Conforms")).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
export const NameMismatch: Story = {
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
await userEvent.click(canvas.getByText("right-name"));
|
||||
await expect(canvas.getByText("name-path-mismatch")).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
export const DynamicResources: Story = {
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
await userEvent.click(canvas.getByText("dynamic-report"));
|
||||
await expect(canvas.getByText("Dynamic resources")).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
export const DigestMismatch: Story = {
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
await userEvent.click(canvas.getByText("tampered-notes"));
|
||||
await userEvent.click(canvas.getByRole("button", { name: /Verify all/ }));
|
||||
await expect(
|
||||
await canvas.findByText("Digest mismatch"),
|
||||
).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
File diff suppressed because it is too large
Load diff
990
clients/web/src/components/screens/SkillsScreen/SkillsScreen.tsx
Normal file
990
clients/web/src/components/screens/SkillsScreen/SkillsScreen.tsx
Normal file
|
|
@ -0,0 +1,990 @@
|
|||
import { useCallback, useMemo, useRef, useState } from "react";
|
||||
import {
|
||||
Alert,
|
||||
Badge,
|
||||
Button,
|
||||
Card,
|
||||
Code,
|
||||
Flex,
|
||||
Group,
|
||||
NavLink,
|
||||
ScrollArea,
|
||||
Stack,
|
||||
Table,
|
||||
Text,
|
||||
TextInput,
|
||||
} from "@mantine/core";
|
||||
import { MdRefresh, MdSearch, MdVerifiedUser } from "react-icons/md";
|
||||
import type {
|
||||
SkillEntry,
|
||||
SkillResource,
|
||||
} from "@inspector/core/mcp/skillsSchemas.js";
|
||||
import { DYNAMIC_RESOURCES } from "@inspector/core/mcp/skillsSchemas.js";
|
||||
import {
|
||||
checkSkillConformance,
|
||||
skillDisplayName,
|
||||
skillEntriesMatch,
|
||||
skillUriIdentity,
|
||||
totalSkillBytes,
|
||||
verifySkillResource,
|
||||
type SkillIssue,
|
||||
type SkillVerification,
|
||||
} from "@inspector/core/mcp/skills.js";
|
||||
import { ContentViewer } from "../../elements/ContentViewer/ContentViewer";
|
||||
import { useValueChange } from "../../../hooks/useValueChange";
|
||||
import {
|
||||
skillFileBytes,
|
||||
type SkillFileContents,
|
||||
} from "../../../utils/skillFileBytes";
|
||||
|
||||
/**
|
||||
* How many skill files are read at once by "Verify all". A conforming manifest
|
||||
* may hold 512 entries, so this is what keeps one click from becoming 512
|
||||
* simultaneous `resources/read` calls.
|
||||
*/
|
||||
const VERIFY_CONCURRENCY = 4;
|
||||
|
||||
/**
|
||||
* Per-row verification progress. `attempt` is the click that produced it: two
|
||||
* verifications of the SAME row in the SAME manifest (a double click, or a row
|
||||
* button pressed while "Verify all" is running) are not distinguished by the
|
||||
* manifest key, so without it an older read finishing last would overwrite the
|
||||
* newer verdict and leave the UI reporting bytes it no longer fetched.
|
||||
*/
|
||||
type FileState = { attempt: number } & (
|
||||
| { status: "pending" }
|
||||
| { status: "done"; verification: SkillVerification }
|
||||
| { status: "error"; message: string }
|
||||
);
|
||||
|
||||
/**
|
||||
* Verification verdicts plus the manifest they belong to. Rows are keyed by
|
||||
* their **index**, not their URI: the conformance checker deliberately tolerates
|
||||
* a duplicated URI so it can report `duplicate-resource`, and a URI key would
|
||||
* collide those two rows into one verdict.
|
||||
*/
|
||||
interface VerificationState {
|
||||
/**
|
||||
* The manifest these verdicts belong to, or `null` before anything has been
|
||||
* verified. `useValueChange` deliberately does not fire on the first render,
|
||||
* so `null` stands in for "the initial manifest, not yet adopted" — the first
|
||||
* write claims it. Once set it is only ever replaced by an invalidation, so a
|
||||
* stale continuation can never be mistaken for an initial one.
|
||||
*/
|
||||
key: string | null;
|
||||
files: Record<number, FileState>;
|
||||
}
|
||||
|
||||
/**
|
||||
* The SKILL.md preview, plus the manifest it belongs to (`null` as above) and
|
||||
* the click that produced it. The manifest key cannot order two reads issued
|
||||
* for the SAME manifest, so without `attempt` a double click whose older read
|
||||
* finishes last would replace the newer preview.
|
||||
*/
|
||||
interface PreviewState {
|
||||
key: string | null;
|
||||
attempt?: number;
|
||||
contents?: SkillFileContents;
|
||||
message?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* The result of the on-demand `skills/get`, plus the manifest it belongs to.
|
||||
* `matches` records whether the fetched entry describes the same skill as the
|
||||
* one `skills/list` returned — the reason for making the call at all.
|
||||
*/
|
||||
interface FetchedEntryState {
|
||||
key: string | null;
|
||||
/** The click this result belongs to — see {@link PreviewState.attempt}. */
|
||||
attempt?: number;
|
||||
entry?: SkillEntry;
|
||||
/** Conformance findings for the FETCHED entry, in its own right. */
|
||||
issues?: SkillIssue[];
|
||||
/** True when the fetched entry describes the same skill as the listed one. */
|
||||
matches?: boolean;
|
||||
/** True when the fetched entry is for a different URI than was asked for. */
|
||||
wrongUri?: boolean;
|
||||
message?: string;
|
||||
}
|
||||
|
||||
export interface SkillsScreenProps {
|
||||
/**
|
||||
* Identity of the connected session. Part of the invalidation key below, so
|
||||
* a verification still in flight when the user switches servers cannot land
|
||||
* afterwards and report a verdict for an identical-looking entry on the new
|
||||
* one — this screen stays mounted across a disconnect, so content alone does
|
||||
* not distinguish the two.
|
||||
*/
|
||||
sessionKey: string;
|
||||
skills: SkillEntry[];
|
||||
/** Pages the last `skills/list` walk took; shown so pagination is visible. */
|
||||
pageCount: number;
|
||||
/** A failed list walk, rendered above the sidebar list. */
|
||||
loadError?: Error | null;
|
||||
ui: SkillsUiState;
|
||||
onUiChange: (next: SkillsUiState) => void;
|
||||
onRefreshList: () => void;
|
||||
/** Fetch one skill file's contents via `resources/read`, on demand. */
|
||||
onReadSkillFile: (uri: string) => Promise<SkillFileContents>;
|
||||
/**
|
||||
* Re-fetch the selected entry through `skills/get` (SEP-2640) — the
|
||||
* extension's second required method, which nothing else in the app calls.
|
||||
*
|
||||
* It is a **fresh point-in-time snapshot**, so a conforming result may
|
||||
* legitimately differ from an older listing, and the screen presents a
|
||||
* difference as an updated snapshot rather than a fault. What it does treat
|
||||
* as a fault is the fetched entry being non-conforming in its own right, or
|
||||
* answering for a different URI than the one requested — neither of which a
|
||||
* fresh read excuses.
|
||||
*/
|
||||
onGetSkill: (uri: string) => Promise<SkillEntry>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Selection and the sidebar search — controlled by the parent (App) as one
|
||||
* object so they persist across tab navigation within a live session (#1417).
|
||||
* Verification results stay local to the screen: they are derived from a live
|
||||
* `resources/read` round trip that is torn down with the screen, so persisting
|
||||
* them would restore a verdict without the fetch that produced it.
|
||||
*/
|
||||
export interface SkillsUiState {
|
||||
selectedSkillUri?: string;
|
||||
search: string;
|
||||
}
|
||||
|
||||
const ScreenLayout = Flex.withProps({
|
||||
variant: "screen",
|
||||
h: "calc(100dvh - var(--app-shell-header-height, 0px) - var(--app-shell-footer-height, 0px))",
|
||||
gap: "md",
|
||||
p: "xl",
|
||||
align: "flex-start",
|
||||
});
|
||||
|
||||
const Sidebar = Stack.withProps({
|
||||
w: 340,
|
||||
flex: "0 0 auto",
|
||||
});
|
||||
|
||||
const SidebarCard = Card.withProps({
|
||||
withBorder: true,
|
||||
padding: "lg",
|
||||
});
|
||||
|
||||
const DetailCard = Card.withProps({
|
||||
withBorder: true,
|
||||
padding: "lg",
|
||||
flex: 1,
|
||||
h: "100%",
|
||||
});
|
||||
|
||||
const DetailScroll = ScrollArea.withProps({
|
||||
type: "auto",
|
||||
scrollbars: "y",
|
||||
offsetScrollbars: true,
|
||||
h: "100%",
|
||||
});
|
||||
|
||||
const EmptyState = Text.withProps({
|
||||
c: "dimmed",
|
||||
ta: "center",
|
||||
py: "xl",
|
||||
});
|
||||
|
||||
const ControlsRow = Group.withProps({
|
||||
justify: "space-between",
|
||||
wrap: "nowrap",
|
||||
gap: "sm",
|
||||
});
|
||||
|
||||
// The Resources header carries three action buttons beside its count badge, so
|
||||
// it wraps rather than truncating the badge on a narrow detail pane — unlike
|
||||
// the sidebar row above, where the search field is meant to absorb the space.
|
||||
const SectionControlsRow = Group.withProps({
|
||||
justify: "space-between",
|
||||
wrap: "wrap",
|
||||
gap: "sm",
|
||||
});
|
||||
|
||||
const SearchInput = TextInput.withProps({
|
||||
size: "xs",
|
||||
flex: 1,
|
||||
leftSection: <MdSearch aria-hidden size={14} />,
|
||||
});
|
||||
|
||||
const RefreshButton = Button.withProps({
|
||||
variant: "subtle",
|
||||
size: "compact-xs",
|
||||
leftSection: <MdRefresh aria-hidden size={14} />,
|
||||
});
|
||||
|
||||
const VerifyButton = Button.withProps({
|
||||
variant: "light",
|
||||
size: "compact-sm",
|
||||
leftSection: <MdVerifiedUser aria-hidden size={14} />,
|
||||
});
|
||||
|
||||
// A `Text` renders a `<p>`, so a section heading must never *wrap* the count
|
||||
// badge beside it — a `<div>` inside a `<p>` is invalid HTML that React reports
|
||||
// as a hydration error and the Storybook run fails on. Heading and badge sit
|
||||
// side by side in an `InlineRow` instead.
|
||||
const SectionHeading = Text.withProps({
|
||||
fw: 600,
|
||||
size: "sm",
|
||||
});
|
||||
|
||||
const MonoCaption = Text.withProps({
|
||||
size: "xs",
|
||||
c: "dimmed",
|
||||
ff: "monospace",
|
||||
});
|
||||
|
||||
const DetailStack = Stack.withProps({
|
||||
gap: "md",
|
||||
});
|
||||
|
||||
const IssueStack = Stack.withProps({
|
||||
gap: "xs",
|
||||
});
|
||||
|
||||
const ManifestTable = Table.withProps({
|
||||
striped: true,
|
||||
withTableBorder: true,
|
||||
fz: "xs",
|
||||
verticalSpacing: "xs",
|
||||
});
|
||||
|
||||
const CountBadge = Badge.withProps({
|
||||
size: "xs",
|
||||
variant: "light",
|
||||
});
|
||||
|
||||
// A tight, non-wrapping row — used for the detail-pane action pair and for the
|
||||
// badge + Verify button inside a manifest cell.
|
||||
const InlineRow = Group.withProps({
|
||||
gap: "xs",
|
||||
wrap: "nowrap",
|
||||
});
|
||||
|
||||
const RowVerifyButton = Button.withProps({
|
||||
variant: "subtle",
|
||||
size: "compact-xs",
|
||||
});
|
||||
|
||||
const SkillTitle = Text.withProps({
|
||||
fw: 600,
|
||||
size: "lg",
|
||||
truncate: true,
|
||||
});
|
||||
|
||||
/** Colour token for a finding's severity — errors read as failures. */
|
||||
function issueColor(issue: SkillIssue): string {
|
||||
return issue.severity === "error" ? "red" : "yellow";
|
||||
}
|
||||
|
||||
/** Colour token for a per-file verification verdict. */
|
||||
function verificationColor(status: SkillVerification["status"]): string {
|
||||
if (status === "verified") return "green";
|
||||
if (status === "mismatch") return "red";
|
||||
return "yellow";
|
||||
}
|
||||
|
||||
/**
|
||||
* The short label a manifest row shows for its verdict. `—` (not yet run) is
|
||||
* deliberately distinct from `unverifiable` (run, but nothing to compare
|
||||
* against): conflating them would hide the fact that a server advertised no
|
||||
* digest.
|
||||
*/
|
||||
function verificationLabel(state: FileState | undefined): string {
|
||||
if (!state) return "—";
|
||||
if (state.status === "pending") return "checking…";
|
||||
if (state.status === "error") return "read failed";
|
||||
return state.verification.status;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a settled request should be discarded: its manifest was invalidated
|
||||
* (a different key), or a later click for the same manifest already wrote (a
|
||||
* higher attempt). A `null` key is the un-adopted initial manifest, which the
|
||||
* first write claims.
|
||||
*/
|
||||
function isStale(
|
||||
held: { key: string | null; attempt?: number },
|
||||
key: string,
|
||||
attempt: number,
|
||||
): boolean {
|
||||
if (held.key !== null && held.key !== key) return true;
|
||||
return held.attempt !== undefined && held.attempt > attempt;
|
||||
}
|
||||
|
||||
/** `sha256:abcd…wxyz`, so a long digest stays readable in a table cell. */
|
||||
function shortDigest(digest: string | undefined): string {
|
||||
if (!digest) return "—";
|
||||
return digest.length <= 24 ? digest : `${digest.slice(0, 16)}…`;
|
||||
}
|
||||
|
||||
/**
|
||||
* The Skills screen (SEP-2640) — a conformance view, not just a list.
|
||||
*
|
||||
* The sidebar lists the skills the server enumerated; the detail pane shows the
|
||||
* entry's frontmatter, every conformance finding
|
||||
* (`checkSkillConformance`), and the resource manifest with a per-file
|
||||
* verification verdict. Verification is on demand: SEP-2640 says a
|
||||
* `resources/read` of a skill file is not a load and confers no standing, so
|
||||
* the Inspector fetches only what the user asks it to.
|
||||
*/
|
||||
export function SkillsScreen({
|
||||
sessionKey,
|
||||
skills,
|
||||
pageCount,
|
||||
loadError,
|
||||
ui,
|
||||
onUiChange,
|
||||
onRefreshList,
|
||||
onReadSkillFile,
|
||||
onGetSkill,
|
||||
}: SkillsScreenProps) {
|
||||
const { selectedSkillUri, search } = ui;
|
||||
// Both slices carry the manifest key they belong to, and every async
|
||||
// continuation writes through a functional update that compares it. That is
|
||||
// what discards a read still in flight when the selection changes or a
|
||||
// Refresh replaces the manifest — without it, a slow fetch lands afterwards
|
||||
// and writes a verdict for content nobody is looking at, or one that was
|
||||
// never checked. Storing the key IN the state (rather than bumping a ref
|
||||
// during render) keeps the `useValueChange` callback to `setState` calls
|
||||
// only, which is the purity that hook documents and requires.
|
||||
const [verification, setVerification] = useState<VerificationState>({
|
||||
key: null,
|
||||
files: {},
|
||||
});
|
||||
const [previewState, setPreviewState] = useState<PreviewState>({ key: null });
|
||||
const [fetchedEntry, setFetchedEntry] = useState<FetchedEntryState>({
|
||||
key: null,
|
||||
});
|
||||
// Every "Verify all" batch in flight, keyed by the manifest it belongs to.
|
||||
//
|
||||
// A **map**, not one slot, and the reason is a bug a single slot really had:
|
||||
// batches on different skills genuinely overlap, so a slot remembers only the
|
||||
// most recent one. Start A, switch to B and start B, return to A — the slot
|
||||
// now says B, A's button reads as free, and clicking it starts a SECOND pool
|
||||
// of workers for A on top of the first, doubling the concurrency cap. Keyed
|
||||
// by manifest, A stays disabled for exactly as long as A's batch runs.
|
||||
//
|
||||
// The value is the invocation's token, so a finalizer deletes only its own
|
||||
// entry; and a per-manifest entry is what keeps a hung batch on one skill
|
||||
// from disabling every other skill's button.
|
||||
const [batches, setBatches] = useState<ReadonlyMap<string, number>>(
|
||||
() => new Map(),
|
||||
);
|
||||
// Monotonic attempt token, shared by every on-demand action here: a manifest
|
||||
// row's verification, the SKILL.md preview, and the `skills/get` fetch. One
|
||||
// counter rather than three because it only has to be *increasing*, and each
|
||||
// consumer compares it against its own slot. A ref because it is claimed
|
||||
// inside an event handler, never during render.
|
||||
const nextAttempt = useRef(0);
|
||||
|
||||
const filtered = useMemo(() => {
|
||||
const needle = search.trim().toLowerCase();
|
||||
if (!needle) return skills;
|
||||
return skills.filter(
|
||||
(skill) =>
|
||||
skillDisplayName(skill).toLowerCase().includes(needle) ||
|
||||
skill.uri.toLowerCase().includes(needle),
|
||||
);
|
||||
}, [skills, search]);
|
||||
|
||||
// Matched by IDENTITY, like every other URI comparison here: a refresh that
|
||||
// canonicalizes `skill://demo/%53KILL.md` to `skill://demo/SKILL.md` names
|
||||
// the same skill, and the detail pane must not empty out because the server
|
||||
// changed its spelling.
|
||||
const selected = useMemo(() => {
|
||||
if (selectedSkillUri === undefined) return undefined;
|
||||
const wanted = skillUriIdentity(selectedSkillUri);
|
||||
return skills.find((skill) => skillUriIdentity(skill.uri) === wanted);
|
||||
}, [skills, selectedSkillUri]);
|
||||
|
||||
const issues = useMemo(
|
||||
() => (selected ? checkSkillConformance(selected) : []),
|
||||
[selected],
|
||||
);
|
||||
|
||||
const manifest: SkillResource[] = useMemo(
|
||||
() =>
|
||||
selected && selected.resources !== DYNAMIC_RESOURCES
|
||||
? selected.resources
|
||||
: [],
|
||||
[selected],
|
||||
);
|
||||
|
||||
// What every result on screen is a result *about*: the selected skill entry,
|
||||
// in full. Keying on the URI alone would leave a green `verified` badge
|
||||
// attached to a digest a Refresh replaced; keying on the manifest alone would
|
||||
// leave a stale "skills/get matches skills/list" verdict after a
|
||||
// metadata-only change, since that comparison covers `frontmatter` too.
|
||||
// Re-verifying after a metadata-only refresh is the cheap direction to be
|
||||
// wrong in; showing a match that was computed against a different entry is
|
||||
// not. A primitive string, because `useValueChange` compares with `Object.is`
|
||||
// and a fresh object every render would loop.
|
||||
const manifestKey = useMemo(
|
||||
() =>
|
||||
`${sessionKey}\n${selected ? JSON.stringify(selected) : (selectedSkillUri ?? "")}`,
|
||||
[selected, selectedSkillUri, sessionKey],
|
||||
);
|
||||
|
||||
// Adjusted DURING RENDER via `useValueChange` rather than in an effect, so a
|
||||
// new selection (or a refreshed manifest) never paints a frame carrying the
|
||||
// previous one's verification results. `setState` calls only — the hook
|
||||
// replays this callback whenever React replays the render.
|
||||
useValueChange(manifestKey, (next) => {
|
||||
setVerification({ key: next, files: {} });
|
||||
setPreviewState({ key: next });
|
||||
setFetchedEntry({ key: next });
|
||||
});
|
||||
|
||||
const fileStates = verification.key === manifestKey ? verification.files : {};
|
||||
|
||||
/**
|
||||
* Verify one manifest ROW. Keyed by row index, not by URI: the checker
|
||||
* deliberately tolerates a duplicated URI so it can report
|
||||
* `duplicate-resource`, and two rows sharing a key would share one verdict —
|
||||
* verifying either would update both, and "Verify all" would race two
|
||||
* different digest/size declarations into the same slot.
|
||||
*/
|
||||
const verifyRow = useCallback(
|
||||
async (index: number, resource: SkillResource, key: string) => {
|
||||
// Claimed synchronously, so two verifications of this row are ordered
|
||||
// before either read starts.
|
||||
const attempt = (nextAttempt.current += 1);
|
||||
const write = (state: FileState) =>
|
||||
setVerification((prev) => {
|
||||
// `null` is the un-adopted initial manifest; any other mismatch is a
|
||||
// continuation from a manifest that has since been invalidated.
|
||||
if (prev.key !== null && prev.key !== key) return prev;
|
||||
const files = prev.key === key ? prev.files : {};
|
||||
// A newer attempt for this row already wrote — an older read
|
||||
// finishing last must not overwrite it.
|
||||
const held = files[index];
|
||||
if (held !== undefined && held.attempt > attempt) return prev;
|
||||
return { key, files: { ...files, [index]: state } };
|
||||
});
|
||||
write({ attempt, status: "pending" });
|
||||
try {
|
||||
const contents = await onReadSkillFile(resource.uri);
|
||||
const result = await verifySkillResource(
|
||||
resource,
|
||||
skillFileBytes(contents),
|
||||
);
|
||||
write({ attempt, status: "done", verification: result });
|
||||
} catch (err) {
|
||||
write({
|
||||
attempt,
|
||||
status: "error",
|
||||
message: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
}
|
||||
},
|
||||
[onReadSkillFile],
|
||||
);
|
||||
|
||||
const verifyAll = useCallback(() => {
|
||||
// Bounded concurrency, not `Promise.all` over the whole manifest: a
|
||||
// conforming skill may declare 512 files, and firing 512 simultaneous
|
||||
// `resources/read` calls would bury the transport and the server for no
|
||||
// gain. Workers pull from a shared cursor so each row still flips to
|
||||
// `checking…` and then to its verdict as it lands, rather than all at once.
|
||||
//
|
||||
// Held rather than floated: each `verifyFile` owns its own failures (it
|
||||
// records them as per-row state), and this handler cannot be async, so the
|
||||
// settled promise is discarded explicitly at one place instead of per file.
|
||||
let next = 0;
|
||||
const key = manifestKey;
|
||||
const worker = async (): Promise<void> => {
|
||||
for (let i = next++; i < manifest.length; i = next++) {
|
||||
await verifyRow(i, manifest[i], key);
|
||||
}
|
||||
};
|
||||
const workers = Math.min(VERIFY_CONCURRENCY, manifest.length);
|
||||
const token = (nextAttempt.current += 1);
|
||||
setBatches((prev) => new Map(prev).set(key, token));
|
||||
// The concurrency cap is per invocation, so without the button being
|
||||
// disabled below, a second click would start a second pool of four and a
|
||||
// third would make it twelve — the flood the cap exists to prevent.
|
||||
void Promise.all(Array.from({ length: workers }, () => worker())).finally(
|
||||
// Clears only ITS OWN invocation: matched on the token as well as the
|
||||
// key, so an earlier batch settling cannot free a button a later one is
|
||||
// holding.
|
||||
() =>
|
||||
setBatches((prev) => {
|
||||
if (prev.get(key) !== token) return prev;
|
||||
const next = new Map(prev);
|
||||
next.delete(key);
|
||||
return next;
|
||||
}),
|
||||
);
|
||||
}, [manifest, manifestKey, verifyRow]);
|
||||
|
||||
const showSkillMd = useCallback(() => {
|
||||
if (!selected) return;
|
||||
const key = manifestKey;
|
||||
const attempt = (nextAttempt.current += 1);
|
||||
// A click handler cannot await, and this chain terminates in its own
|
||||
// `catch` that surfaces the message in the preview slot. Both arms go
|
||||
// through `writePreview`, which drops a result whose manifest has been
|
||||
// invalidated OR whose click has been superseded.
|
||||
const writePreview = (next: Omit<PreviewState, "key" | "attempt">) =>
|
||||
setPreviewState((prev) =>
|
||||
isStale(prev, key, attempt) ? prev : { key, attempt, ...next },
|
||||
);
|
||||
// Claimed BEFORE the request goes out, the way `verifyRow` claims its row.
|
||||
// Recording the attempt only on settle leaves a window where an older
|
||||
// request that happens to resolve first is still considered current, and
|
||||
// publishes its contents while a newer one is in flight. Clearing the
|
||||
// previous result at the same time also means the pane doesn't keep
|
||||
// showing the old file while the new read is running.
|
||||
writePreview({});
|
||||
void onReadSkillFile(selected.uri)
|
||||
.then((contents) => writePreview({ contents }))
|
||||
.catch((err: unknown) => {
|
||||
writePreview({
|
||||
message: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
});
|
||||
}, [manifestKey, onReadSkillFile, selected]);
|
||||
|
||||
const fetchEntry = useCallback(() => {
|
||||
if (!selected) return;
|
||||
const key = manifestKey;
|
||||
const attempt = (nextAttempt.current += 1);
|
||||
// Same shape as the SKILL.md read: a click handler cannot await, the chain
|
||||
// ends in its own `catch`, and both arms drop a result whose manifest has
|
||||
// been invalidated or whose click has been superseded.
|
||||
const writeFetched = (next: Omit<FetchedEntryState, "key" | "attempt">) =>
|
||||
setFetchedEntry((prev) =>
|
||||
isStale(prev, key, attempt) ? prev : { key, attempt, ...next },
|
||||
);
|
||||
// Claimed before the request goes out — see `showSkillMd` for why settling
|
||||
// is too late.
|
||||
writeFetched({});
|
||||
void onGetSkill(selected.uri)
|
||||
.then((entry) => {
|
||||
// The fetched entry is checked ON ITS OWN before being compared. A
|
||||
// snapshot is allowed to have moved on, but it is not allowed to be
|
||||
// non-conforming: an entry missing a digest is invalid whether or not
|
||||
// the skill changed, and an entry for a DIFFERENT uri is never a valid
|
||||
// refresh of the one that was asked for. Only a conforming entry with
|
||||
// the same identity gets the benign "the snapshot moved" reading.
|
||||
writeFetched({
|
||||
entry,
|
||||
issues: checkSkillConformance(entry),
|
||||
// Compared by identity, not raw string: a server that canonicalizes
|
||||
// an escape has answered for the same resource, and calling that
|
||||
// "a different URI" would be the tool disagreeing with the read
|
||||
// path, which accepts exactly that equivalence.
|
||||
wrongUri:
|
||||
skillUriIdentity(entry.uri) !== skillUriIdentity(selected.uri),
|
||||
// Compared semantically — see `skillEntriesMatch` for why a
|
||||
// `JSON.stringify` comparison would report key order and manifest
|
||||
// order as differences.
|
||||
matches: skillEntriesMatch(entry, selected),
|
||||
});
|
||||
})
|
||||
.catch((err: unknown) => {
|
||||
writeFetched({
|
||||
message: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
});
|
||||
}, [manifestKey, onGetSkill, selected]);
|
||||
|
||||
const fetched = fetchedEntry.key === manifestKey ? fetchedEntry : undefined;
|
||||
// `invalid` outranks the snapshot comparison: an entry that breaks a
|
||||
// requirement, or answers for a different URI, is wrong regardless of
|
||||
// whether the skill it describes has changed since the listing.
|
||||
const fetchedVerdict =
|
||||
fetched?.wrongUri ||
|
||||
(fetched?.issues ?? []).some((issue) => issue.severity === "error")
|
||||
? "invalid"
|
||||
: fetched?.matches
|
||||
? "matches"
|
||||
: "differs";
|
||||
const batchRunning = batches.has(manifestKey);
|
||||
|
||||
const preview =
|
||||
previewState.key === manifestKey ? previewState.contents : undefined;
|
||||
const previewError =
|
||||
previewState.key === manifestKey ? previewState.message : undefined;
|
||||
|
||||
const errorCount = issues.filter((i) => i.severity === "error").length;
|
||||
const warningCount = issues.length - errorCount;
|
||||
|
||||
return (
|
||||
// `data-*` readiness contract for the headless tab smoke (#2148); see
|
||||
// clients/web/README.md#core-tab-automation-contract.
|
||||
<ScreenLayout
|
||||
data-testid="skills-screen"
|
||||
data-skill-count={skills.length}
|
||||
data-skill-page-count={pageCount}
|
||||
>
|
||||
<Sidebar>
|
||||
<SidebarCard>
|
||||
<Stack gap="sm">
|
||||
<ControlsRow>
|
||||
<SearchInput
|
||||
aria-label="Search skills"
|
||||
placeholder="Search skills"
|
||||
value={search}
|
||||
onChange={(event) =>
|
||||
onUiChange({ ...ui, search: event.currentTarget.value })
|
||||
}
|
||||
/>
|
||||
<RefreshButton onClick={onRefreshList}>Refresh</RefreshButton>
|
||||
</ControlsRow>
|
||||
{loadError && (
|
||||
<Alert color="red" title="Could not load skills">
|
||||
{loadError.message}
|
||||
</Alert>
|
||||
)}
|
||||
{filtered.length === 0 ? (
|
||||
// Not "No skills": SEP-2640 lets a server return an empty or
|
||||
// partial catalog, and says an empty result must not be read as
|
||||
// proof it has none — an unlisted skill can still be fetched by
|
||||
// URI with `skills/get`. Claiming otherwise would be the tool
|
||||
// asserting something the protocol explicitly does not.
|
||||
<EmptyState>No skills listed</EmptyState>
|
||||
) : (
|
||||
filtered.map((skill) => {
|
||||
const skillIssues = checkSkillConformance(skill);
|
||||
const errors = skillIssues.filter(
|
||||
(i) => i.severity === "error",
|
||||
).length;
|
||||
return (
|
||||
<NavLink
|
||||
key={skill.uri}
|
||||
active={
|
||||
selectedSkillUri !== undefined &&
|
||||
skillUriIdentity(skill.uri) ===
|
||||
skillUriIdentity(selectedSkillUri)
|
||||
}
|
||||
label={skillDisplayName(skill)}
|
||||
description={skill.uri}
|
||||
onClick={() =>
|
||||
onUiChange({ ...ui, selectedSkillUri: skill.uri })
|
||||
}
|
||||
rightSection={
|
||||
skillIssues.length > 0 ? (
|
||||
<CountBadge color={errors > 0 ? "red" : "yellow"}>
|
||||
{skillIssues.length}
|
||||
</CountBadge>
|
||||
) : undefined
|
||||
}
|
||||
/>
|
||||
);
|
||||
})
|
||||
)}
|
||||
<MonoCaption>
|
||||
{skills.length} skill(s) over {pageCount} page(s)
|
||||
</MonoCaption>
|
||||
</Stack>
|
||||
</SidebarCard>
|
||||
</Sidebar>
|
||||
|
||||
<DetailCard>
|
||||
{!selected ? (
|
||||
<EmptyState>Select a skill to view details</EmptyState>
|
||||
) : (
|
||||
<DetailScroll>
|
||||
<DetailStack data-testid="skill-detail">
|
||||
<Stack gap={4}>
|
||||
<SkillTitle>{skillDisplayName(selected)}</SkillTitle>
|
||||
<MonoCaption>{selected.uri}</MonoCaption>
|
||||
</Stack>
|
||||
|
||||
{selected.frontmatter.description && (
|
||||
<Text size="sm">{selected.frontmatter.description}</Text>
|
||||
)}
|
||||
|
||||
<Stack gap="xs">
|
||||
<InlineRow>
|
||||
<SectionHeading>Conformance</SectionHeading>
|
||||
<CountBadge color={errorCount > 0 ? "red" : "green"}>
|
||||
{errorCount} error(s), {warningCount} warning(s)
|
||||
</CountBadge>
|
||||
</InlineRow>
|
||||
{issues.length === 0 ? (
|
||||
<Alert color="green" title="Conforms">
|
||||
No structural issues found in this entry.
|
||||
</Alert>
|
||||
) : (
|
||||
<IssueStack data-testid="skill-issues">
|
||||
{issues.map((issue, index) => (
|
||||
<Alert
|
||||
// The index is load-bearing, not decoration: a manifest
|
||||
// repeating one URI three times yields three
|
||||
// `duplicate-resource` findings with identical code and
|
||||
// URI, and a key built from those alone would make
|
||||
// React drop the extras — hiding findings in exactly
|
||||
// the malformed input this view exists to inspect.
|
||||
key={`${index}:${issue.code}:${issue.resourceUri ?? ""}`}
|
||||
color={issueColor(issue)}
|
||||
title={issue.code}
|
||||
>
|
||||
<Stack gap={2}>
|
||||
<Text size="sm">{issue.message}</Text>
|
||||
{issue.resourceUri && (
|
||||
<MonoCaption>{issue.resourceUri}</MonoCaption>
|
||||
)}
|
||||
</Stack>
|
||||
</Alert>
|
||||
))}
|
||||
</IssueStack>
|
||||
)}
|
||||
</Stack>
|
||||
|
||||
<Stack gap="xs">
|
||||
<SectionControlsRow>
|
||||
<InlineRow>
|
||||
<SectionHeading>Resources</SectionHeading>
|
||||
<CountBadge>
|
||||
{manifest.length} file(s), {totalSkillBytes(manifest)}{" "}
|
||||
bytes
|
||||
</CountBadge>
|
||||
</InlineRow>
|
||||
<InlineRow>
|
||||
<VerifyButton onClick={fetchEntry}>
|
||||
Fetch with skills/get
|
||||
</VerifyButton>
|
||||
<VerifyButton onClick={showSkillMd}>
|
||||
View SKILL.md
|
||||
</VerifyButton>
|
||||
<VerifyButton
|
||||
onClick={verifyAll}
|
||||
disabled={manifest.length === 0 || batchRunning}
|
||||
loading={batchRunning}
|
||||
>
|
||||
Verify all
|
||||
</VerifyButton>
|
||||
</InlineRow>
|
||||
</SectionControlsRow>
|
||||
{selected.resources === DYNAMIC_RESOURCES ? (
|
||||
<Alert color="yellow" title="Dynamic resources">
|
||||
This skill declares{" "}
|
||||
<Code>resources: "dynamic"</Code> — its files are
|
||||
generated, so no manifest is advertised and integrity cannot
|
||||
be verified.
|
||||
</Alert>
|
||||
) : (
|
||||
<ManifestTable data-testid="skill-manifest">
|
||||
<Table.Thead>
|
||||
<Table.Tr>
|
||||
<Table.Th>URI</Table.Th>
|
||||
<Table.Th>Size</Table.Th>
|
||||
<Table.Th>Digest</Table.Th>
|
||||
<Table.Th>Verification</Table.Th>
|
||||
</Table.Tr>
|
||||
</Table.Thead>
|
||||
<Table.Tbody>
|
||||
{manifest.map((resource, index) => {
|
||||
const state = fileStates[index];
|
||||
const color =
|
||||
state?.status === "done"
|
||||
? verificationColor(state.verification.status)
|
||||
: state?.status === "error"
|
||||
? "red"
|
||||
: "gray";
|
||||
return (
|
||||
// Index-keyed for the same reason the verdicts are:
|
||||
// a duplicated URI is a case this screen reports, so
|
||||
// it must not also collide two rows into one.
|
||||
<Table.Tr key={index}>
|
||||
<Table.Td>{resource.uri}</Table.Td>
|
||||
<Table.Td>{resource.size ?? "—"}</Table.Td>
|
||||
<Table.Td>{shortDigest(resource.digest)}</Table.Td>
|
||||
<Table.Td>
|
||||
<InlineRow>
|
||||
<CountBadge color={color}>
|
||||
{verificationLabel(state)}
|
||||
</CountBadge>
|
||||
<RowVerifyButton
|
||||
// Every row's button reads "Verify", so the
|
||||
// visible text alone gives a screen-reader
|
||||
// user no way to tell which file each one
|
||||
// checks; the URI cell is in the same row but
|
||||
// is not programmatically associated with it.
|
||||
aria-label={`Verify ${resource.uri}`}
|
||||
// A click handler cannot await, and
|
||||
// `verifyRow` owns its own failures — it
|
||||
// records them as this row's state.
|
||||
onClick={() =>
|
||||
void verifyRow(index, resource, manifestKey)
|
||||
}
|
||||
>
|
||||
Verify
|
||||
</RowVerifyButton>
|
||||
</InlineRow>
|
||||
</Table.Td>
|
||||
</Table.Tr>
|
||||
);
|
||||
})}
|
||||
</Table.Tbody>
|
||||
</ManifestTable>
|
||||
)}
|
||||
{manifest.map((resource, index) => {
|
||||
const state = fileStates[index];
|
||||
if (state?.status === "done") {
|
||||
const result = state.verification;
|
||||
if (result.status !== "mismatch") return null;
|
||||
// A size disagreement is caught BEFORE hashing, so it has
|
||||
// no `actualDigest` — titling it "Digest mismatch" and
|
||||
// rendering "actual undefined" would hide the real failure.
|
||||
const sizeFailure = result.actualDigest === undefined;
|
||||
return (
|
||||
<Alert
|
||||
key={`mismatch:${index}`}
|
||||
color="red"
|
||||
title={
|
||||
sizeFailure ? "Size mismatch" : "Digest mismatch"
|
||||
}
|
||||
>
|
||||
<Stack gap={2}>
|
||||
<MonoCaption>{resource.uri}</MonoCaption>
|
||||
{sizeFailure ? (
|
||||
<Text size="sm">{result.reason}</Text>
|
||||
) : (
|
||||
<>
|
||||
<MonoCaption>
|
||||
expected {result.expectedDigest}
|
||||
</MonoCaption>
|
||||
<MonoCaption>
|
||||
actual {result.actualDigest}
|
||||
</MonoCaption>
|
||||
</>
|
||||
)}
|
||||
</Stack>
|
||||
</Alert>
|
||||
);
|
||||
}
|
||||
if (state?.status === "error") {
|
||||
return (
|
||||
<Alert
|
||||
key={`read:${index}`}
|
||||
color="red"
|
||||
title="Could not read file"
|
||||
>
|
||||
<Stack gap={2}>
|
||||
<MonoCaption>{resource.uri}</MonoCaption>
|
||||
<Text size="sm">{state.message}</Text>
|
||||
</Stack>
|
||||
</Alert>
|
||||
);
|
||||
}
|
||||
return null;
|
||||
})}
|
||||
</Stack>
|
||||
|
||||
{fetched?.message !== undefined && (
|
||||
<Alert color="red" title="skills/get failed">
|
||||
{fetched.message}
|
||||
</Alert>
|
||||
)}
|
||||
{fetched?.entry !== undefined && (
|
||||
<Alert
|
||||
data-testid="skills-get-result"
|
||||
data-verdict={fetchedVerdict}
|
||||
// Red only for a genuine violation — a non-conforming entry,
|
||||
// or one answering with a different URI, neither of which a
|
||||
// fresh snapshot excuses. A conforming entry that merely
|
||||
// moved on is yellow: `skills/get` IS a point-in-time read,
|
||||
// so a changed skill legitimately differs.
|
||||
color={
|
||||
fetchedVerdict === "invalid"
|
||||
? "red"
|
||||
: fetchedVerdict === "matches"
|
||||
? "green"
|
||||
: "yellow"
|
||||
}
|
||||
title={
|
||||
fetchedVerdict === "invalid"
|
||||
? "skills/get returned a non-conforming entry"
|
||||
: fetchedVerdict === "matches"
|
||||
? "skills/get matches skills/list"
|
||||
: "skills/get returned a different snapshot"
|
||||
}
|
||||
>
|
||||
<Stack gap={2}>
|
||||
<Text size="sm">
|
||||
{fetchedVerdict === "invalid"
|
||||
? fetched.wrongUri
|
||||
? "This entry is for a different URI than the one requested, which is never a valid refresh of it."
|
||||
: "This entry breaks a requirement of its own, so the difference is not simply a newer snapshot."
|
||||
: fetchedVerdict === "matches"
|
||||
? "The entry this server returns for this URI describes the same skill it listed (compared ignoring key and manifest order)."
|
||||
: "The entry this server returns for this URI differs from the one it listed. `skills/get` is a fresh snapshot, so this is expected if the skill changed since the list was fetched — and a server inconsistency if it did not."}
|
||||
</Text>
|
||||
{(fetched.issues ?? [])
|
||||
.filter((issue) => issue.severity === "error")
|
||||
.map((issue, index) => (
|
||||
<MonoCaption key={`${index}:${issue.code}`}>
|
||||
{issue.code}: {issue.message}
|
||||
</MonoCaption>
|
||||
))}
|
||||
{fetchedVerdict !== "matches" && (
|
||||
<ContentViewer
|
||||
block={{
|
||||
type: "text",
|
||||
text: JSON.stringify(fetched.entry, null, 2),
|
||||
}}
|
||||
mimeType="application/json"
|
||||
copyable
|
||||
/>
|
||||
)}
|
||||
</Stack>
|
||||
</Alert>
|
||||
)}
|
||||
|
||||
{previewError && (
|
||||
<Alert color="red" title="Could not read SKILL.md">
|
||||
{previewError}
|
||||
</Alert>
|
||||
)}
|
||||
{preview && (
|
||||
<Stack gap="xs" data-testid="skill-md-preview">
|
||||
<SectionHeading>SKILL.md</SectionHeading>
|
||||
{/* `contents`, not a text `block`: a server may serve
|
||||
SKILL.md as a base64 `blob`, and the block form would
|
||||
substitute an empty string and paint a blank preview for
|
||||
a file verification just read correctly. */}
|
||||
<ContentViewer
|
||||
contents={
|
||||
typeof preview.text === "string"
|
||||
? {
|
||||
uri: selected.uri,
|
||||
text: preview.text,
|
||||
mimeType: preview.mimeType ?? "text/markdown",
|
||||
}
|
||||
: {
|
||||
uri: selected.uri,
|
||||
blob: preview.blob ?? "",
|
||||
mimeType: preview.mimeType ?? "text/markdown",
|
||||
}
|
||||
}
|
||||
copyable
|
||||
/>
|
||||
</Stack>
|
||||
)}
|
||||
|
||||
<Stack gap="xs">
|
||||
<SectionHeading>Frontmatter</SectionHeading>
|
||||
<ContentViewer
|
||||
block={{
|
||||
type: "text",
|
||||
text: JSON.stringify(selected.frontmatter, null, 2),
|
||||
}}
|
||||
mimeType="application/json"
|
||||
copyable
|
||||
/>
|
||||
</Stack>
|
||||
</DetailStack>
|
||||
</DetailScroll>
|
||||
)}
|
||||
</DetailCard>
|
||||
</ScreenLayout>
|
||||
);
|
||||
}
|
||||
|
|
@ -7,6 +7,7 @@ import type { ToolsUiState } from "./ToolsScreen/ToolsScreen";
|
|||
import type { PromptsUiState } from "./PromptsScreen/PromptsScreen";
|
||||
import type { ResourcesUiState } from "./ResourcesScreen/ResourcesScreen";
|
||||
import type { AppsUiState } from "./AppsScreen/AppsScreen";
|
||||
import type { SkillsUiState } from "./SkillsScreen/SkillsScreen";
|
||||
import type { TasksUiState } from "./TasksScreen/TasksScreen";
|
||||
import type { LogsUiState } from "./LoggingScreen/LoggingScreen";
|
||||
import type { ProtocolUiState } from "./ProtocolScreen/ProtocolScreen";
|
||||
|
|
@ -43,6 +44,11 @@ export const EMPTY_APPS_UI: AppsUiState = {
|
|||
search: "",
|
||||
};
|
||||
|
||||
export const EMPTY_SKILLS_UI: SkillsUiState = {
|
||||
selectedSkillUri: undefined,
|
||||
search: "",
|
||||
};
|
||||
|
||||
export const EMPTY_TASKS_UI: TasksUiState = {
|
||||
search: "",
|
||||
statusFilter: undefined,
|
||||
|
|
@ -74,6 +80,7 @@ export const TAB_UI_REGISTRY = {
|
|||
Tools: { empty: EMPTY_TOOLS_UI },
|
||||
Prompts: { empty: EMPTY_PROMPTS_UI },
|
||||
Resources: { empty: EMPTY_RESOURCES_UI },
|
||||
Skills: { empty: EMPTY_SKILLS_UI },
|
||||
Tasks: { empty: EMPTY_TASKS_UI },
|
||||
Logs: { empty: EMPTY_LOGS_UI },
|
||||
Protocol: { empty: EMPTY_PROTOCOL_UI },
|
||||
|
|
|
|||
|
|
@ -29,6 +29,7 @@ import type {
|
|||
ResourcesPanelProps,
|
||||
ServerListProps,
|
||||
ShellProps,
|
||||
SkillsPanelProps,
|
||||
TasksPanelProps,
|
||||
ToolsPanelProps,
|
||||
} from "./types";
|
||||
|
|
@ -38,6 +39,7 @@ import {
|
|||
EMPTY_APPS_UI,
|
||||
EMPTY_PROMPTS_UI,
|
||||
EMPTY_RESOURCES_UI,
|
||||
EMPTY_SKILLS_UI,
|
||||
EMPTY_TASKS_UI,
|
||||
EMPTY_LOGS_UI,
|
||||
EMPTY_PROTOCOL_UI,
|
||||
|
|
@ -424,6 +426,21 @@ const appsArgs: AppsPanelProps = {
|
|||
onRefreshApps: fn(),
|
||||
};
|
||||
|
||||
const skillsArgs: SkillsPanelProps = {
|
||||
skillsSessionKey: "story-session",
|
||||
skills: [],
|
||||
skillsPageCount: 0,
|
||||
skillsUi: EMPTY_SKILLS_UI,
|
||||
onSkillsUiChange: fn(),
|
||||
onRefreshSkills: fn(),
|
||||
onReadSkillFile: fn(async () => ({ text: "" })),
|
||||
onGetSkill: fn(async (uri: string) => ({
|
||||
uri,
|
||||
frontmatter: {},
|
||||
resources: [],
|
||||
})),
|
||||
};
|
||||
|
||||
const tasksArgs: TasksPanelProps = {
|
||||
tasks: demoTasks,
|
||||
progressByTaskId: demoProgressByTaskId,
|
||||
|
|
@ -484,6 +501,7 @@ const meta: Meta<typeof InspectorView> = {
|
|||
prompts: promptsArgs,
|
||||
resources: resourcesArgs,
|
||||
apps: appsArgs,
|
||||
skills: skillsArgs,
|
||||
tasks: tasksArgs,
|
||||
logs: logsArgs,
|
||||
protocol: protocolArgs,
|
||||
|
|
|
|||
|
|
@ -39,6 +39,7 @@ import {
|
|||
EMPTY_APPS_UI,
|
||||
EMPTY_PROMPTS_UI,
|
||||
EMPTY_RESOURCES_UI,
|
||||
EMPTY_SKILLS_UI,
|
||||
EMPTY_TASKS_UI,
|
||||
EMPTY_LOGS_UI,
|
||||
EMPTY_PROTOCOL_UI,
|
||||
|
|
@ -170,6 +171,17 @@ function makeProps(...overrides: PropOverrides[]): InspectorViewProps {
|
|||
onRefreshApps: vi.fn(),
|
||||
...mergeBundle("apps", overrides),
|
||||
},
|
||||
skills: {
|
||||
skillsSessionKey: "test-session",
|
||||
skills: [],
|
||||
skillsPageCount: 0,
|
||||
skillsUi: EMPTY_SKILLS_UI,
|
||||
onSkillsUiChange: vi.fn(),
|
||||
onRefreshSkills: vi.fn(),
|
||||
onReadSkillFile: vi.fn().mockResolvedValue({ text: "" }),
|
||||
onGetSkill: vi.fn(),
|
||||
...mergeBundle("skills", overrides),
|
||||
},
|
||||
tasks: {
|
||||
tasks: [],
|
||||
tasksUi: EMPTY_TASKS_UI,
|
||||
|
|
@ -1369,6 +1381,50 @@ describe("InspectorView", () => {
|
|||
expect(labels).not.toContain("Tasks");
|
||||
});
|
||||
|
||||
it("hides the Skills tab when the server declares no skills extension (#2234)", async () => {
|
||||
renderWithMantine(
|
||||
<StatefulInspectorViewHost
|
||||
{...makeProps({
|
||||
connection: {
|
||||
activeServer: "alpha",
|
||||
connectionStatus: "connected",
|
||||
initializeResult: initWithCapabilities({ tools: {} }),
|
||||
},
|
||||
servers: { servers: [sampleServer] },
|
||||
})}
|
||||
/>,
|
||||
);
|
||||
const radios = await screen.findAllByRole("radio");
|
||||
const labels = radios.map((r) => r.getAttribute("value"));
|
||||
expect(labels).toContain("Tools");
|
||||
expect(labels).not.toContain("Skills");
|
||||
});
|
||||
|
||||
it("shows the Skills tab on a LEGACY connection that declares the extension (#2234)", async () => {
|
||||
// Unlike Tasks, Skills is not era-gated: `skills/*` are not spec method
|
||||
// names in either codec, so a legacy-era server that declares the
|
||||
// extension is serving it and must get the tab.
|
||||
renderWithMantine(
|
||||
<StatefulInspectorViewHost
|
||||
{...makeProps({
|
||||
connection: {
|
||||
activeServer: "alpha",
|
||||
connectionStatus: "connected",
|
||||
protocolEra: "legacy",
|
||||
initializeResult: initWithCapabilities({
|
||||
tools: {},
|
||||
extensions: { "io.modelcontextprotocol/skills": {} },
|
||||
}),
|
||||
},
|
||||
servers: { servers: [sampleServer] },
|
||||
})}
|
||||
/>,
|
||||
);
|
||||
const radios = await screen.findAllByRole("radio");
|
||||
const labels = radios.map((r) => r.getAttribute("value"));
|
||||
expect(labels).toContain("Skills");
|
||||
});
|
||||
|
||||
it("shows the Tasks tab when the server advertises tasks even with no tasks yet", async () => {
|
||||
renderWithMantine(
|
||||
<StatefulInspectorViewHost
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import type { ServerEntry } from "@inspector/core/mcp/types.js";
|
|||
import { isTerminalStatus } from "@inspector/core/mcp/types.js";
|
||||
import { isAppTool } from "@inspector/core/mcp/apps.js";
|
||||
import { TASKS_EXTENSION_KEY } from "@inspector/core/mcp/modernTaskSchemas.js";
|
||||
import { isSkillsExtensionSupported } from "@inspector/core/mcp/skills.js";
|
||||
import { ViewHeader } from "../../groups/ViewHeader/ViewHeader";
|
||||
import { VersionBadge } from "../../elements/VersionBadge/VersionBadge";
|
||||
import { CopyrightBadge } from "../../elements/CopyrightBadge/CopyrightBadge";
|
||||
|
|
@ -22,6 +23,7 @@ import { AppsScreen } from "../../screens/AppsScreen/AppsScreen";
|
|||
import { PromptsScreen } from "../../screens/PromptsScreen/PromptsScreen";
|
||||
import { ResourcesScreen } from "../../screens/ResourcesScreen/ResourcesScreen";
|
||||
import { LoggingScreen } from "../../screens/LoggingScreen/LoggingScreen";
|
||||
import { SkillsScreen } from "../../screens/SkillsScreen/SkillsScreen";
|
||||
import { TasksScreen } from "../../screens/TasksScreen/TasksScreen";
|
||||
import { ProtocolScreen } from "../../screens/ProtocolScreen/ProtocolScreen";
|
||||
import { NetworkScreen } from "../../screens/NetworkScreen/NetworkScreen";
|
||||
|
|
@ -37,6 +39,7 @@ import type {
|
|||
ResourcesPanelProps,
|
||||
ServerListProps,
|
||||
ShellProps,
|
||||
SkillsPanelProps,
|
||||
TasksPanelProps,
|
||||
ToolsPanelProps,
|
||||
} from "./types";
|
||||
|
|
@ -118,6 +121,7 @@ function useListCompact(
|
|||
}
|
||||
|
||||
const SERVERS_TAB = INSPECTOR_SERVERS_TAB;
|
||||
const SKILLS_TAB = "Skills";
|
||||
const TASKS_TAB = "Tasks";
|
||||
const LOGS_TAB = "Logs";
|
||||
const PROTOCOL_TAB = "Protocol";
|
||||
|
|
@ -151,6 +155,7 @@ const ALL_TABS: string[] = [
|
|||
"Tools",
|
||||
"Prompts",
|
||||
"Resources",
|
||||
SKILLS_TAB,
|
||||
TASKS_TAB,
|
||||
LOGS_TAB,
|
||||
PROTOCOL_TAB,
|
||||
|
|
@ -328,6 +333,7 @@ export interface InspectorViewProps {
|
|||
prompts: PromptsPanelProps;
|
||||
resources: ResourcesPanelProps;
|
||||
apps: AppsPanelProps;
|
||||
skills: SkillsPanelProps;
|
||||
tasks: TasksPanelProps;
|
||||
logs: LogsPanelProps;
|
||||
protocol: ProtocolPanelProps;
|
||||
|
|
@ -343,6 +349,7 @@ export function InspectorView({
|
|||
prompts: promptsPanel,
|
||||
resources: resourcesPanel,
|
||||
apps: appsPanel,
|
||||
skills: skillsPanel,
|
||||
tasks: tasksPanel,
|
||||
logs: logsPanel,
|
||||
protocol: protocolPanel,
|
||||
|
|
@ -448,6 +455,17 @@ export function InspectorView({
|
|||
onAppError,
|
||||
onRefreshApps,
|
||||
} = appsPanel;
|
||||
const {
|
||||
skillsSessionKey,
|
||||
skills,
|
||||
skillsPageCount,
|
||||
skillsLoadError,
|
||||
skillsUi,
|
||||
onSkillsUiChange,
|
||||
onRefreshSkills,
|
||||
onReadSkillFile,
|
||||
onGetSkill,
|
||||
} = skillsPanel;
|
||||
const {
|
||||
tasks,
|
||||
progressByTaskId,
|
||||
|
|
@ -654,6 +672,10 @@ export function InspectorView({
|
|||
capabilities?.tasks !== undefined ||
|
||||
capabilities?.extensions?.[TASKS_EXTENSION_KEY] !== undefined;
|
||||
const hasLogging = capabilities?.logging !== undefined;
|
||||
// Skills (SEP-2640) is a server-declared *extension*, and unlike Tasks it
|
||||
// is not era-gated: `skills/*` are not spec method names in either codec,
|
||||
// so a legacy-era server that declares the extension is serving it.
|
||||
const hasSkills = isSkillsExtensionSupported(capabilities);
|
||||
return ALL_TABS.filter((t) => {
|
||||
if (t === NETWORK_TAB && isStdio) return false;
|
||||
// Console is the stdio process's stderr stream — shown only for stdio
|
||||
|
|
@ -663,6 +685,7 @@ export function InspectorView({
|
|||
if (t === "Apps" && !hasApps) return false;
|
||||
if (t === "Prompts" && !hasPrompts) return false;
|
||||
if (t === "Resources" && !hasResources) return false;
|
||||
if (t === SKILLS_TAB && !hasSkills) return false;
|
||||
if (t === "Tasks" && !hasTasks) return false;
|
||||
if (t === "Logs" && !hasLogging) return false;
|
||||
return true;
|
||||
|
|
@ -1032,6 +1055,17 @@ export function InspectorView({
|
|||
sortDirection: consoleSort,
|
||||
onSortChange: setConsoleSort,
|
||||
};
|
||||
const skillsScreenProps = {
|
||||
sessionKey: skillsSessionKey,
|
||||
skills,
|
||||
pageCount: skillsPageCount,
|
||||
loadError: skillsLoadError,
|
||||
ui: skillsUi,
|
||||
onUiChange: onSkillsUiChange,
|
||||
onRefreshList: onRefreshSkills,
|
||||
onReadSkillFile,
|
||||
onGetSkill,
|
||||
};
|
||||
const tasksScreenProps = {
|
||||
tasks,
|
||||
progressByTaskId,
|
||||
|
|
@ -1240,6 +1274,9 @@ export function InspectorView({
|
|||
onCompactChange={setResourcesCompact}
|
||||
/>
|
||||
</ScreenStage>
|
||||
<ScreenStage active={activeTab === SKILLS_TAB}>
|
||||
<SkillsScreen {...skillsScreenProps} />
|
||||
</ScreenStage>
|
||||
<ScreenStage active={activeTab === TASKS_TAB}>
|
||||
<TasksScreen {...tasksScreenProps} />
|
||||
</ScreenStage>
|
||||
|
|
|
|||
|
|
@ -59,6 +59,9 @@ import type {
|
|||
ResourcesUiState,
|
||||
} from "../../screens/ResourcesScreen/ResourcesScreen";
|
||||
import type { LogsUiState } from "../../screens/LoggingScreen/LoggingScreen";
|
||||
import type { SkillsUiState } from "../../screens/SkillsScreen/SkillsScreen";
|
||||
import type { SkillEntry } from "@inspector/core/mcp/skillsSchemas.js";
|
||||
import type { SkillFileContents } from "../../../utils/skillFileBytes";
|
||||
import type { TasksUiState } from "../../screens/TasksScreen/TasksScreen";
|
||||
import type { ProtocolUiState } from "../../screens/ProtocolScreen/ProtocolScreen";
|
||||
import type { NetworkUiState } from "../../screens/NetworkScreen/NetworkScreen";
|
||||
|
|
@ -308,6 +311,27 @@ export interface AppsPanelProps {
|
|||
onRefreshApps: () => void;
|
||||
}
|
||||
|
||||
/** The Skills screen (SEP-2640): the enumerated skills and their verification. */
|
||||
export interface SkillsPanelProps {
|
||||
/**
|
||||
* Identity of the connected session. Part of the screen's invalidation key,
|
||||
* so async verification state can never cross a connection — see
|
||||
* `UseInspectorStoresResult.sessionNonce`.
|
||||
*/
|
||||
skillsSessionKey: string;
|
||||
skills: SkillEntry[];
|
||||
/** Pages the last `skills/list` walk took. */
|
||||
skillsPageCount: number;
|
||||
skillsLoadError?: Error | null;
|
||||
skillsUi: SkillsUiState;
|
||||
onSkillsUiChange: (next: SkillsUiState) => void;
|
||||
onRefreshSkills: () => void;
|
||||
/** Read one skill file (`resources/read`) so its digest can be checked. */
|
||||
onReadSkillFile: (uri: string) => Promise<SkillFileContents>;
|
||||
/** Re-fetch the selected entry through `skills/get`. */
|
||||
onGetSkill: (uri: string) => Promise<SkillEntry>;
|
||||
}
|
||||
|
||||
/** The Tasks monitor: the task list, its progress map, and actions. */
|
||||
export interface TasksPanelProps {
|
||||
tasks: Task[];
|
||||
|
|
|
|||
|
|
@ -63,6 +63,9 @@ vi.mock("@inspector/core/mcp/state/managedResourceTemplatesState.js", () => ({
|
|||
vi.mock("@inspector/core/mcp/state/managedRequestorTasksState.js", () => ({
|
||||
ManagedRequestorTasksState: fakeState("managedRequestorTasksState"),
|
||||
}));
|
||||
vi.mock("@inspector/core/mcp/state/managedSkillsState.js", () => ({
|
||||
ManagedSkillsState: fakeState("managedSkillsState"),
|
||||
}));
|
||||
vi.mock("@inspector/core/mcp/state/resourceSubscriptionsState.js", () => ({
|
||||
ResourceSubscriptionsState: fakeState("resourceSubscriptionsState"),
|
||||
}));
|
||||
|
|
@ -111,6 +114,14 @@ vi.mock("@inspector/core/react/useManagedRequestorTasks.js", () => ({
|
|||
clearCompleted: vi.fn(),
|
||||
}),
|
||||
}));
|
||||
vi.mock("@inspector/core/react/useManagedSkills.js", () => ({
|
||||
useManagedSkills: () => ({
|
||||
skills: [{ uri: "skill://s/SKILL.md" }],
|
||||
pageCount: 1,
|
||||
error: null,
|
||||
refresh: vi.fn().mockResolvedValue(undefined),
|
||||
}),
|
||||
}));
|
||||
vi.mock("@inspector/core/react/useResourceSubscriptions.js", () => ({
|
||||
useResourceSubscriptions: () => ({
|
||||
subscriptions: [{ uri: "sub://r" }],
|
||||
|
|
@ -181,6 +192,7 @@ const STORE_NAMES = [
|
|||
"pagedResourcesState",
|
||||
"managedResourceTemplatesState",
|
||||
"managedRequestorTasksState",
|
||||
"managedSkillsState",
|
||||
"resourceSubscriptionsState",
|
||||
"messageLogState",
|
||||
"fetchRequestLogState",
|
||||
|
|
@ -201,7 +213,7 @@ describe("useInspectorStores", () => {
|
|||
expect(built).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("builds all twelve stores against the client", () => {
|
||||
it("builds all thirteen stores against the client", () => {
|
||||
const h = harness();
|
||||
const c = client();
|
||||
h.run((api) => api.createStores(c, fetchLogOptions));
|
||||
|
|
@ -245,6 +257,21 @@ describe("useInspectorStores", () => {
|
|||
expect(h.api().stores).not.toBe(first);
|
||||
});
|
||||
|
||||
it("advances the session nonce on both create and destroy", () => {
|
||||
// It names one connected session and must never repeat across a
|
||||
// reconnect — `SkillsScreen` keys async verification state on it, and a
|
||||
// repeated value would let one session's result land in another.
|
||||
const h = harness();
|
||||
const seen = new Set<number>([h.api().sessionNonce]);
|
||||
h.run((api) => api.createStores(client(), fetchLogOptions));
|
||||
seen.add(h.api().sessionNonce);
|
||||
h.run((api) => api.destroyStores());
|
||||
seen.add(h.api().sessionNonce);
|
||||
h.run((api) => api.createStores(client(), fetchLogOptions));
|
||||
seen.add(h.api().sessionNonce);
|
||||
expect(seen.size).toBe(4);
|
||||
});
|
||||
|
||||
it("destroys and clears on destroyStores", () => {
|
||||
const h = harness();
|
||||
h.run((api) => api.createStores(client(), fetchLogOptions));
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import type {
|
|||
ResourceSubscriptionStreamState,
|
||||
StderrLogEntry,
|
||||
} from "@inspector/core/mcp/types.js";
|
||||
import type { SkillEntry } from "@inspector/core/mcp/skillsSchemas.js";
|
||||
import type { InspectorClient } from "@inspector/core/mcp/index.js";
|
||||
import { ManagedToolsState } from "@inspector/core/mcp/state/managedToolsState.js";
|
||||
import { ManagedPromptsState } from "@inspector/core/mcp/state/managedPromptsState.js";
|
||||
|
|
@ -22,6 +23,7 @@ import { PagedPromptsState } from "@inspector/core/mcp/state/pagedPromptsState.j
|
|||
import { PagedResourcesState } from "@inspector/core/mcp/state/pagedResourcesState.js";
|
||||
import { ManagedResourceTemplatesState } from "@inspector/core/mcp/state/managedResourceTemplatesState.js";
|
||||
import { ManagedRequestorTasksState } from "@inspector/core/mcp/state/managedRequestorTasksState.js";
|
||||
import { ManagedSkillsState } from "@inspector/core/mcp/state/managedSkillsState.js";
|
||||
import { ResourceSubscriptionsState } from "@inspector/core/mcp/state/resourceSubscriptionsState.js";
|
||||
import { MessageLogState } from "@inspector/core/mcp/state/messageLogState.js";
|
||||
import {
|
||||
|
|
@ -37,6 +39,7 @@ import { usePagedPrompts } from "@inspector/core/react/usePagedPrompts.js";
|
|||
import { usePagedResources } from "@inspector/core/react/usePagedResources.js";
|
||||
import { useManagedResourceTemplates } from "@inspector/core/react/useManagedResourceTemplates.js";
|
||||
import { useManagedRequestorTasks } from "@inspector/core/react/useManagedRequestorTasks.js";
|
||||
import { useManagedSkills } from "@inspector/core/react/useManagedSkills.js";
|
||||
import { useResourceSubscriptions } from "@inspector/core/react/useResourceSubscriptions.js";
|
||||
import { useMessageLog } from "@inspector/core/react/useMessageLog.js";
|
||||
import { useFetchRequestLog } from "@inspector/core/react/useFetchRequestLog.js";
|
||||
|
|
@ -44,9 +47,9 @@ import { useStderrLog } from "@inspector/core/react/useStderrLog.js";
|
|||
import { usePaginatedList, type PaginatedListModel } from "./usePaginatedList";
|
||||
|
||||
/**
|
||||
* The twelve per-session state managers. They are created together (one
|
||||
* The thirteen per-session state managers. They are created together (one
|
||||
* `InspectorClient`, one set of stores) and torn down together, so they are
|
||||
* held as one slot rather than twelve — a partially-replaced set would leave
|
||||
* held as one slot rather than thirteen — a partially-replaced set would leave
|
||||
* some stores listening to a client the others had already left.
|
||||
*/
|
||||
export interface InspectorStores {
|
||||
|
|
@ -58,6 +61,7 @@ export interface InspectorStores {
|
|||
pagedResourcesState: PagedResourcesState;
|
||||
managedResourceTemplatesState: ManagedResourceTemplatesState;
|
||||
managedRequestorTasksState: ManagedRequestorTasksState;
|
||||
managedSkillsState: ManagedSkillsState;
|
||||
resourceSubscriptionsState: ResourceSubscriptionsState;
|
||||
messageLogState: MessageLogState;
|
||||
fetchRequestLogState: FetchRequestLogState;
|
||||
|
|
@ -82,6 +86,17 @@ export type FetchLogOptions = Pick<
|
|||
export interface UseInspectorStoresResult {
|
||||
/** The live stores, or `null` before the first connect / after teardown. */
|
||||
stores: InspectorStores | null;
|
||||
/**
|
||||
* Bumped on every `createStores` **and** every `destroyStores`, so it names
|
||||
* one connected session and never repeats across a reconnect.
|
||||
*
|
||||
* Screens that hold async state keyed by *content* need this in the key:
|
||||
* `SkillsScreen` stays mounted across a disconnect, so a verification still
|
||||
* in flight for server A could otherwise land after a switch to server B and
|
||||
* report a verdict for an identical-looking entry that was never read from B
|
||||
* (#2234).
|
||||
*/
|
||||
sessionNonce: number;
|
||||
/**
|
||||
* Build a fresh set of stores for `client`, tearing down whatever set is
|
||||
* live first. Stable, so callers need no dependency on the current stores.
|
||||
|
|
@ -131,6 +146,12 @@ export interface UseInspectorStoresResult {
|
|||
tasks: Task[];
|
||||
refreshTasks: () => Promise<unknown>;
|
||||
clearCompletedTasks: () => void;
|
||||
/** The server's skills (SEP-2640); empty when it declared no extension. */
|
||||
skills: SkillEntry[];
|
||||
/** Pages the last `skills/list` walk took. */
|
||||
skillsPageCount: number;
|
||||
skillsLoadError: Error | null;
|
||||
refreshSkills: () => Promise<unknown>;
|
||||
subscriptions: InspectorResourceSubscription[];
|
||||
subscriptionStreamState: ResourceSubscriptionStreamState;
|
||||
messages: MessageEntry[];
|
||||
|
|
@ -152,6 +173,9 @@ export function useInspectorStores({
|
|||
paginatedLists,
|
||||
}: UseInspectorStoresParams): UseInspectorStoresResult {
|
||||
const [stores, setStores] = useState<InspectorStores | null>(null);
|
||||
// See `sessionNonce` above. A counter rather than the store object's identity
|
||||
// because it has to be usable as part of a string key.
|
||||
const [sessionNonce, setSessionNonce] = useState(0);
|
||||
// Mirrors `stores` so `destroyStores` can read the live set without taking a
|
||||
// dependency on it — which is what keeps every caller's callback stable.
|
||||
const storesRef = useRef<InspectorStores | null>(null);
|
||||
|
|
@ -167,6 +191,7 @@ export function useInspectorStores({
|
|||
storesRef.current = null;
|
||||
fetchLogRef.current = null;
|
||||
setStores(null);
|
||||
setSessionNonce((n) => n + 1);
|
||||
}, []);
|
||||
|
||||
const createStores = useCallback(
|
||||
|
|
@ -192,6 +217,7 @@ export function useInspectorStores({
|
|||
client,
|
||||
),
|
||||
managedRequestorTasksState: new ManagedRequestorTasksState(client),
|
||||
managedSkillsState: new ManagedSkillsState(client),
|
||||
resourceSubscriptionsState: new ResourceSubscriptionsState(
|
||||
client,
|
||||
managedResourcesState,
|
||||
|
|
@ -203,6 +229,7 @@ export function useInspectorStores({
|
|||
storesRef.current = next;
|
||||
fetchLogRef.current = fetchRequestLogState;
|
||||
setStores(next);
|
||||
setSessionNonce((n) => n + 1);
|
||||
},
|
||||
[destroyStores],
|
||||
);
|
||||
|
|
@ -310,6 +337,15 @@ export function useInspectorStores({
|
|||
inspectorClient,
|
||||
stores?.managedRequestorTasksState ?? null,
|
||||
);
|
||||
// Skills (SEP-2640). The store no-ops when the server declared no extension,
|
||||
// so this hook is unconditional like the rest — the Skills *tab* is what is
|
||||
// gated, in `InspectorView`.
|
||||
const {
|
||||
skills,
|
||||
pageCount: skillsPageCount,
|
||||
error: skillsLoadError,
|
||||
refresh: refreshSkills,
|
||||
} = useManagedSkills(inspectorClient, stores?.managedSkillsState ?? null);
|
||||
const { subscriptions, streamState: subscriptionStreamState } =
|
||||
useResourceSubscriptions(stores?.resourceSubscriptionsState ?? null);
|
||||
const { messages } = useMessageLog(stores?.messageLogState ?? null);
|
||||
|
|
@ -320,6 +356,7 @@ export function useInspectorStores({
|
|||
|
||||
return {
|
||||
stores,
|
||||
sessionNonce,
|
||||
createStores,
|
||||
destroyStores,
|
||||
fetchLogRef,
|
||||
|
|
@ -344,6 +381,10 @@ export function useInspectorStores({
|
|||
tasks,
|
||||
refreshTasks,
|
||||
clearCompletedTasks,
|
||||
skills,
|
||||
skillsPageCount,
|
||||
skillsLoadError,
|
||||
refreshSkills,
|
||||
subscriptions,
|
||||
subscriptionStreamState,
|
||||
messages,
|
||||
|
|
|
|||
|
|
@ -162,6 +162,7 @@ function spies() {
|
|||
setModernLogLevel: vi.fn(),
|
||||
clearCompletedTasks: vi.fn(),
|
||||
refreshTasks: vi.fn().mockResolvedValue(undefined),
|
||||
refreshSkills: vi.fn().mockResolvedValue(undefined),
|
||||
refreshTools: vi.fn().mockResolvedValue(undefined),
|
||||
refreshPrompts: vi.fn().mockResolvedValue(undefined),
|
||||
refreshResources: vi.fn().mockResolvedValue(undefined),
|
||||
|
|
@ -271,6 +272,7 @@ function harness(initial: HarnessProps = {}): Harness {
|
|||
activeToolCallTaskIdRef,
|
||||
clearCompletedTasks: s.clearCompletedTasks,
|
||||
refreshTasks: s.refreshTasks,
|
||||
refreshSkills: s.refreshSkills,
|
||||
paginatedLists: p.paginatedLists ?? false,
|
||||
paginatedListsOverride: { record: s.record, valueFor: s.valueFor },
|
||||
toolsPagination,
|
||||
|
|
@ -840,6 +842,191 @@ describe("onReadResourceContents", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("onReadSkillFile (#2234)", () => {
|
||||
const skillUri = "skill://demo/reference.md";
|
||||
|
||||
it("returns the block whose uri matches exactly", async () => {
|
||||
const c = client({
|
||||
readResource: vi.fn().mockResolvedValue({
|
||||
result: {
|
||||
contents: [
|
||||
{ uri: "skill://demo/other.md", text: "wrong" },
|
||||
{ uri: skillUri, text: "right", mimeType: "text/markdown" },
|
||||
],
|
||||
},
|
||||
}),
|
||||
});
|
||||
const h = harness({ client: c });
|
||||
await expect(h.api().onReadSkillFile(skillUri)).resolves.toEqual({
|
||||
text: "right",
|
||||
mimeType: "text/markdown",
|
||||
});
|
||||
});
|
||||
|
||||
it("accepts a block whose uri the server echoed back in an equivalent form", async () => {
|
||||
// A resolved `..` is the same resource; `normalizeSkillUri` is what says so.
|
||||
const c = client({
|
||||
readResource: vi.fn().mockResolvedValue({
|
||||
result: {
|
||||
contents: [{ uri: "skill://demo/x/../reference.md", text: "x" }],
|
||||
},
|
||||
}),
|
||||
});
|
||||
const h = harness({ client: c });
|
||||
await expect(h.api().onReadSkillFile(skillUri)).resolves.toEqual({
|
||||
text: "x",
|
||||
});
|
||||
});
|
||||
|
||||
it("refuses a sole block for a DIFFERENT uri rather than verifying it", async () => {
|
||||
// The dangerous fallback: treating "the only block" as "the block we asked
|
||||
// for" would hash `other.md`'s bytes against `reference.md`'s advertised
|
||||
// digest — and could report that as `verified`.
|
||||
const c = client({
|
||||
readResource: vi.fn().mockResolvedValue({
|
||||
result: { contents: [{ uri: "skill://demo/other.md", text: "wrong" }] },
|
||||
}),
|
||||
});
|
||||
const h = harness({ client: c });
|
||||
await expect(h.api().onReadSkillFile(skillUri)).rejects.toThrow(
|
||||
/returned no content .*skill:\/\/demo\/other\.md/,
|
||||
);
|
||||
});
|
||||
|
||||
it("refuses an unparseable echoed uri rather than matching it to another", async () => {
|
||||
// Two unparseable URIs must not compare equal just because both normalize
|
||||
// to `undefined`.
|
||||
const c = client({
|
||||
readResource: vi.fn().mockResolvedValue({
|
||||
result: { contents: [{ uri: "not a uri", text: "wrong" }] },
|
||||
}),
|
||||
});
|
||||
const h = harness({ client: c });
|
||||
await expect(h.api().onReadSkillFile("also not a uri")).rejects.toThrow(
|
||||
/returned no content/,
|
||||
);
|
||||
});
|
||||
|
||||
it("passes a blob block through as a blob", async () => {
|
||||
const c = client({
|
||||
readResource: vi.fn().mockResolvedValue({
|
||||
result: { contents: [{ uri: skillUri, blob: "aGk=" }] },
|
||||
}),
|
||||
});
|
||||
const h = harness({ client: c });
|
||||
await expect(h.api().onReadSkillFile(skillUri)).resolves.toEqual({
|
||||
blob: "aGk=",
|
||||
});
|
||||
});
|
||||
|
||||
it("throws when no block answers the uri", async () => {
|
||||
// Returning an empty payload here would let the caller hash zero bytes and
|
||||
// report a confident digest *mismatch* for a response that carried nothing.
|
||||
const c = client({
|
||||
readResource: vi.fn().mockResolvedValue({
|
||||
result: {
|
||||
contents: [
|
||||
{ uri: "skill://demo/a.md", text: "a" },
|
||||
{ uri: "skill://demo/b.md", text: "b" },
|
||||
],
|
||||
},
|
||||
}),
|
||||
});
|
||||
const h = harness({ client: c });
|
||||
await expect(h.api().onReadSkillFile(skillUri)).rejects.toThrow(
|
||||
/returned no content/,
|
||||
);
|
||||
});
|
||||
|
||||
it("throws when there is no client", async () => {
|
||||
const h = harness();
|
||||
await expect(h.api().onReadSkillFile(skillUri)).rejects.toThrow(
|
||||
"Client is not connected",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("onGetSkill (#2234)", () => {
|
||||
it("routes the uri through the client's skills/get", async () => {
|
||||
const getSkill = vi.fn().mockResolvedValue({
|
||||
uri: "skill://demo/SKILL.md",
|
||||
frontmatter: {},
|
||||
resources: [],
|
||||
});
|
||||
const h = harness({ client: client({ getSkill }) });
|
||||
await expect(
|
||||
h.api().onGetSkill("skill://demo/SKILL.md"),
|
||||
).resolves.toMatchObject({ uri: "skill://demo/SKILL.md" });
|
||||
expect(getSkill).toHaveBeenCalledWith("skill://demo/SKILL.md");
|
||||
});
|
||||
|
||||
it("throws when there is no client", async () => {
|
||||
const h = harness();
|
||||
await expect(h.api().onGetSkill("skill://demo/SKILL.md")).rejects.toThrow(
|
||||
"Client is not connected",
|
||||
);
|
||||
});
|
||||
|
||||
it("retries once after a satisfied recovery", async () => {
|
||||
// Every server command routes through the shared recovery; without it an
|
||||
// expired authorization would render an error and stop there, with no
|
||||
// reauthorization and no retry.
|
||||
const recover = vi.fn().mockResolvedValue(true);
|
||||
const getSkill = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(authError())
|
||||
.mockResolvedValue({
|
||||
uri: "skill://demo/SKILL.md",
|
||||
frontmatter: {},
|
||||
resources: [],
|
||||
});
|
||||
const h = harness({
|
||||
client: client({ getSkill }),
|
||||
activeServerId: "a",
|
||||
recovery: { handleCommandScopedAuthRecovery: recover },
|
||||
});
|
||||
await expect(
|
||||
h.api().onGetSkill("skill://demo/SKILL.md"),
|
||||
).resolves.toMatchObject({ uri: "skill://demo/SKILL.md" });
|
||||
expect(getSkill).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("rethrows when the recovery was not satisfied", async () => {
|
||||
const recover = vi.fn().mockResolvedValue(false);
|
||||
const h = harness({
|
||||
client: client({ getSkill: vi.fn().mockRejectedValue(authError()) }),
|
||||
activeServerId: "a",
|
||||
recovery: { handleCommandScopedAuthRecovery: recover },
|
||||
});
|
||||
await expect(
|
||||
h.api().onGetSkill("skill://demo/SKILL.md"),
|
||||
).rejects.toBeInstanceOf(AuthRecoveryRequiredError);
|
||||
});
|
||||
|
||||
it("rethrows a non-auth failure untouched", async () => {
|
||||
const recover = vi.fn();
|
||||
const h = harness({
|
||||
client: client({
|
||||
getSkill: vi.fn().mockRejectedValue(new Error("-32602")),
|
||||
}),
|
||||
activeServerId: "a",
|
||||
recovery: { handleCommandScopedAuthRecovery: recover },
|
||||
});
|
||||
await expect(h.api().onGetSkill("skill://demo/SKILL.md")).rejects.toThrow(
|
||||
"-32602",
|
||||
);
|
||||
expect(recover).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("onRefreshSkills (#2234)", () => {
|
||||
it("drives the store refresh in the background", () => {
|
||||
const h = harness();
|
||||
h.api().onRefreshSkills();
|
||||
expect(h.spies.refreshSkills).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("subscriptions and completion", () => {
|
||||
it("subscribes and unsubscribes through the client", async () => {
|
||||
const c = client();
|
||||
|
|
|
|||
|
|
@ -34,6 +34,9 @@ import type {
|
|||
} from "../components/screens/ToolsScreen/ToolsScreen";
|
||||
import type { GetPromptState } from "../components/screens/PromptsScreen/PromptsScreen";
|
||||
import type { ReadResourceState } from "../components/screens/ResourcesScreen/ResourcesScreen";
|
||||
import type { SkillFileContents } from "../utils/skillFileBytes";
|
||||
import type { SkillEntry } from "@inspector/core/mcp/skillsSchemas.js";
|
||||
import { normalizeSkillUri } from "@inspector/core/mcp/skills.js";
|
||||
import { UrlElicitationErrorToastMessage } from "../components/elements/Toasts/UrlElicitationErrorToastMessage";
|
||||
import {
|
||||
errorCodeOf,
|
||||
|
|
@ -161,6 +164,8 @@ export interface UseServerCommandsOptions {
|
|||
activeToolCallTaskIdRef: { current: string | undefined };
|
||||
clearCompletedTasks: () => void;
|
||||
refreshTasks: () => Promise<unknown>;
|
||||
/** Re-walk `skills/list` (SEP-2640). */
|
||||
refreshSkills: () => Promise<unknown>;
|
||||
|
||||
// --- The list stores and their two fetch modes (#1721). ---
|
||||
paginatedLists: boolean;
|
||||
|
|
@ -210,6 +215,14 @@ export interface ServerCommands {
|
|||
onReadResourceContents: (
|
||||
uri: string,
|
||||
) => Promise<Awaited<ReturnType<InspectorClient["readResource"]>>["result"]>;
|
||||
/**
|
||||
* Read one skill file's contents (SEP-2640), for digest verification. Returns
|
||||
* the single content block that answers the URI, narrowed to the two fields
|
||||
* the digest is taken over.
|
||||
*/
|
||||
onReadSkillFile: (uri: string) => Promise<SkillFileContents>;
|
||||
/** Re-fetch one skill entry through `skills/get` (SEP-2640). */
|
||||
onGetSkill: (uri: string) => Promise<SkillEntry>;
|
||||
onSubscribeResource: (uri: string) => void;
|
||||
onUnsubscribeResource: (uri: string) => void;
|
||||
onCompleteArgument: (
|
||||
|
|
@ -228,6 +241,7 @@ export interface ServerCommands {
|
|||
onRefreshTools: () => void;
|
||||
onRefreshPrompts: () => void;
|
||||
onRefreshResources: () => void;
|
||||
onRefreshSkills: () => void;
|
||||
onRefreshTasks: () => void;
|
||||
onTogglePaginatedLists: (value: boolean) => void;
|
||||
onLoadMoreTools: () => void;
|
||||
|
|
@ -269,6 +283,7 @@ export function useServerCommands({
|
|||
activeToolCallTaskIdRef,
|
||||
clearCompletedTasks,
|
||||
refreshTasks,
|
||||
refreshSkills,
|
||||
paginatedLists,
|
||||
paginatedListsOverride,
|
||||
toolsPagination,
|
||||
|
|
@ -932,6 +947,88 @@ export function useServerCommands({
|
|||
runCommandInBackground(() => resourcesPagination.onLoadMore(), "ambient"),
|
||||
[resourcesPagination, runCommandInBackground],
|
||||
);
|
||||
// Skill files are fetched on demand, never pre-fetched: SEP-2640 is explicit
|
||||
// that a `resources/read` of a skill file is not a load and confers no
|
||||
// standing, so the Inspector reads only what the user asks it to verify.
|
||||
// Routed through `onReadResourceContents` so a skill read gets the same
|
||||
// auth-recovery retry every other read does.
|
||||
const onReadSkillFile = useCallback(
|
||||
async (uri: string): Promise<SkillFileContents> => {
|
||||
const result = await onReadResourceContents(uri);
|
||||
// The returned URI must be the one asked for. A tempting fallback —
|
||||
// "a single-block response must be the block we asked for" — is right
|
||||
// for a viewer and WRONG here: these bytes are about to be hashed
|
||||
// against `uri`'s advertised digest, so accepting a block the server
|
||||
// labelled `b.md` would verify one file's content against another
|
||||
// file's digest and could report that as `verified`. Knowing which
|
||||
// bytes were hashed is the whole point.
|
||||
//
|
||||
// A normalized match is still accepted, because a server may echo the
|
||||
// URI back in a different but equivalent form (a resolved `..`, a
|
||||
// percent-encoding difference); `normalizeSkillUri` returns `undefined`
|
||||
// for anything unparseable, and two `undefined`s must not compare equal.
|
||||
const wanted = normalizeSkillUri(uri);
|
||||
const block = result.contents.find((c) => {
|
||||
if (c.uri === uri) return true;
|
||||
const got = normalizeSkillUri(c.uri);
|
||||
return got !== undefined && got === wanted;
|
||||
});
|
||||
if (!block) {
|
||||
throw new Error(
|
||||
`resources/read returned no content for ${uri}` +
|
||||
(result.contents.length > 0
|
||||
? ` (got ${result.contents.map((c) => c.uri).join(", ")})`
|
||||
: ""),
|
||||
);
|
||||
}
|
||||
// `contents` is a union of the text and blob shapes, each with its own
|
||||
// payload field required — so `in` is what narrows it, not a `typeof` on
|
||||
// a property one arm does not declare.
|
||||
return {
|
||||
...("text" in block ? { text: block.text } : { blob: block.blob }),
|
||||
...(typeof block.mimeType === "string"
|
||||
? { mimeType: block.mimeType }
|
||||
: {}),
|
||||
};
|
||||
},
|
||||
[onReadResourceContents],
|
||||
);
|
||||
|
||||
// `skills/get` is the extension's second required method, and the Skills tab
|
||||
// calls it on demand so a server author can see their own handler answer —
|
||||
// and see whether it agrees with what their `skills/list` advertised.
|
||||
const onGetSkill = useCallback(
|
||||
async (uri: string): Promise<SkillEntry> => {
|
||||
if (!inspectorClient) throw new Error("Client is not connected");
|
||||
// Routed through the shared recovery like every other server command
|
||||
// (#2174). Without it an expired authorization renders an error in the
|
||||
// panel and stops there — no reauthorization, no retry — which is the
|
||||
// one thing this hook exists to make impossible.
|
||||
const get = () => inspectorClient.getSkill(uri);
|
||||
try {
|
||||
return await get();
|
||||
} catch (err) {
|
||||
if (err instanceof AuthRecoveryRequiredError && activeServerId) {
|
||||
const satisfied = await handleCommandScopedAuthRecovery(err, {
|
||||
serverId: activeServerId,
|
||||
source: "resource",
|
||||
});
|
||||
if (satisfied) return get();
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
},
|
||||
[inspectorClient, activeServerId, handleCommandScopedAuthRecovery],
|
||||
);
|
||||
|
||||
const onRefreshSkills = useCallback(() => {
|
||||
runCommandInBackground(
|
||||
() => refreshSkills(),
|
||||
"ambient",
|
||||
"Failed to refresh skills",
|
||||
);
|
||||
}, [refreshSkills, runCommandInBackground]);
|
||||
|
||||
const onRefreshTasks = useCallback(() => {
|
||||
runCommandInBackground(
|
||||
() => refreshTasks(),
|
||||
|
|
@ -947,6 +1044,8 @@ export function useServerCommands({
|
|||
onGetPrompt,
|
||||
onReadResource,
|
||||
onReadResourceContents,
|
||||
onReadSkillFile,
|
||||
onGetSkill,
|
||||
onSubscribeResource,
|
||||
onUnsubscribeResource,
|
||||
onCompleteArgument,
|
||||
|
|
@ -958,6 +1057,7 @@ export function useServerCommands({
|
|||
onRefreshTools,
|
||||
onRefreshPrompts,
|
||||
onRefreshResources,
|
||||
onRefreshSkills,
|
||||
onRefreshTasks,
|
||||
onTogglePaginatedLists,
|
||||
onLoadMoreTools,
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import {
|
|||
EMPTY_PROMPTS_UI,
|
||||
EMPTY_PROTOCOL_UI,
|
||||
EMPTY_RESOURCES_UI,
|
||||
EMPTY_SKILLS_UI,
|
||||
EMPTY_TASKS_UI,
|
||||
EMPTY_TOOLS_UI,
|
||||
} from "../components/screens/screenUiState";
|
||||
|
|
@ -67,6 +68,7 @@ export function useTabUiState(): TabUiStateResult {
|
|||
const [promptsUi, setPromptsUi] = useState(EMPTY_PROMPTS_UI);
|
||||
const [resourcesUi, setResourcesUi] = useState(EMPTY_RESOURCES_UI);
|
||||
const [appsUi, setAppsUi] = useState(EMPTY_APPS_UI);
|
||||
const [skillsUi, setSkillsUi] = useState(EMPTY_SKILLS_UI);
|
||||
const [tasksUi, setTasksUi] = useState(EMPTY_TASKS_UI);
|
||||
const [logsUi, setLogsUi] = useState(EMPTY_LOGS_UI);
|
||||
const [protocolUi, setProtocolUi] = useState(EMPTY_PROTOCOL_UI);
|
||||
|
|
@ -85,6 +87,7 @@ export function useTabUiState(): TabUiStateResult {
|
|||
promptsUi,
|
||||
resourcesUi,
|
||||
appsUi,
|
||||
skillsUi,
|
||||
tasksUi,
|
||||
logsUi,
|
||||
protocolUi,
|
||||
|
|
@ -96,6 +99,7 @@ export function useTabUiState(): TabUiStateResult {
|
|||
promptsUi,
|
||||
resourcesUi,
|
||||
appsUi,
|
||||
skillsUi,
|
||||
tasksUi,
|
||||
logsUi,
|
||||
protocolUi,
|
||||
|
|
@ -112,6 +116,7 @@ export function useTabUiState(): TabUiStateResult {
|
|||
setPromptsUi,
|
||||
setResourcesUi,
|
||||
setAppsUi,
|
||||
setSkillsUi,
|
||||
setTasksUi,
|
||||
setLogsUi,
|
||||
setProtocolUi,
|
||||
|
|
@ -139,6 +144,7 @@ export function useTabUiState(): TabUiStateResult {
|
|||
setPromptsUi(EMPTY_PROMPTS_UI);
|
||||
setResourcesUi(EMPTY_RESOURCES_UI);
|
||||
setAppsUi(EMPTY_APPS_UI);
|
||||
setSkillsUi(EMPTY_SKILLS_UI);
|
||||
setTasksUi(EMPTY_TASKS_UI);
|
||||
setLogsUi(EMPTY_LOGS_UI);
|
||||
setProtocolUi(EMPTY_PROTOCOL_UI);
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ import {
|
|||
EMPTY_PROMPTS_UI,
|
||||
EMPTY_RESOURCES_UI,
|
||||
EMPTY_APPS_UI,
|
||||
EMPTY_SKILLS_UI,
|
||||
EMPTY_TASKS_UI,
|
||||
EMPTY_LOGS_UI,
|
||||
EMPTY_PROTOCOL_UI,
|
||||
|
|
@ -107,6 +108,7 @@ describe("oauthResume", () => {
|
|||
promptsUi: EMPTY_PROMPTS_UI,
|
||||
resourcesUi: EMPTY_RESOURCES_UI,
|
||||
appsUi: EMPTY_APPS_UI,
|
||||
skillsUi: EMPTY_SKILLS_UI,
|
||||
tasksUi: EMPTY_TASKS_UI,
|
||||
logsUi: EMPTY_LOGS_UI,
|
||||
protocolUi: EMPTY_PROTOCOL_UI,
|
||||
|
|
@ -118,6 +120,7 @@ describe("oauthResume", () => {
|
|||
setPromptsUi: vi.fn(),
|
||||
setResourcesUi: vi.fn(),
|
||||
setAppsUi: vi.fn(),
|
||||
setSkillsUi: vi.fn(),
|
||||
setTasksUi: vi.fn(),
|
||||
setLogsUi: vi.fn(),
|
||||
setProtocolUi: vi.fn(),
|
||||
|
|
@ -147,6 +150,7 @@ describe("oauthResume", () => {
|
|||
setPromptsUi: vi.fn(),
|
||||
setResourcesUi: vi.fn(),
|
||||
setAppsUi: vi.fn(),
|
||||
setSkillsUi: vi.fn(),
|
||||
setTasksUi: vi.fn(),
|
||||
setLogsUi: vi.fn(),
|
||||
setProtocolUi: vi.fn(),
|
||||
|
|
@ -244,6 +248,7 @@ describe("oauthResume", () => {
|
|||
setPromptsUi: vi.fn(),
|
||||
setResourcesUi: vi.fn(),
|
||||
setAppsUi: vi.fn(),
|
||||
setSkillsUi: vi.fn(),
|
||||
setTasksUi: vi.fn(),
|
||||
setLogsUi: vi.fn(),
|
||||
setProtocolUi: vi.fn(),
|
||||
|
|
@ -553,6 +558,7 @@ describe("oauthResume", () => {
|
|||
setPromptsUi: vi.fn(),
|
||||
setResourcesUi: vi.fn(),
|
||||
setAppsUi: vi.fn(),
|
||||
setSkillsUi: vi.fn(),
|
||||
setTasksUi: vi.fn(),
|
||||
setLogsUi: vi.fn(),
|
||||
setProtocolUi: vi.fn(),
|
||||
|
|
@ -567,6 +573,7 @@ describe("oauthResume", () => {
|
|||
setPromptsUi: vi.fn(),
|
||||
setResourcesUi: vi.fn(),
|
||||
setAppsUi: vi.fn(),
|
||||
setSkillsUi: vi.fn(),
|
||||
setTasksUi: vi.fn(),
|
||||
setLogsUi: vi.fn(),
|
||||
setProtocolUi: vi.fn(),
|
||||
|
|
@ -587,6 +594,7 @@ describe("oauthResume", () => {
|
|||
setPromptsUi: vi.fn(),
|
||||
setResourcesUi: vi.fn(),
|
||||
setAppsUi: vi.fn(),
|
||||
setSkillsUi: vi.fn(),
|
||||
setTasksUi: vi.fn(),
|
||||
setLogsUi: vi.fn(),
|
||||
setProtocolUi: vi.fn(),
|
||||
|
|
@ -598,6 +606,7 @@ describe("oauthResume", () => {
|
|||
Prompts: EMPTY_PROMPTS_UI,
|
||||
Resources: EMPTY_RESOURCES_UI,
|
||||
Apps: EMPTY_APPS_UI,
|
||||
Skills: EMPTY_SKILLS_UI,
|
||||
Tasks: EMPTY_TASKS_UI,
|
||||
Logs: EMPTY_LOGS_UI,
|
||||
Protocol: EMPTY_PROTOCOL_UI,
|
||||
|
|
@ -609,18 +618,46 @@ describe("oauthResume", () => {
|
|||
expect(setters.setPromptsUi).toHaveBeenCalledWith(EMPTY_PROMPTS_UI);
|
||||
expect(setters.setResourcesUi).toHaveBeenCalledWith(EMPTY_RESOURCES_UI);
|
||||
expect(setters.setAppsUi).toHaveBeenCalledWith(EMPTY_APPS_UI);
|
||||
expect(setters.setSkillsUi).toHaveBeenCalledWith(EMPTY_SKILLS_UI);
|
||||
expect(setters.setTasksUi).toHaveBeenCalledWith(EMPTY_TASKS_UI);
|
||||
expect(setters.setLogsUi).toHaveBeenCalledWith(EMPTY_LOGS_UI);
|
||||
expect(setters.setProtocolUi).toHaveBeenCalledWith(EMPTY_PROTOCOL_UI);
|
||||
expect(setters.setNetworkUi).toHaveBeenCalledWith(EMPTY_NETWORK_UI);
|
||||
});
|
||||
|
||||
it("restoreTabUiFromSnapshot restores a SAVED Skills selection, not the default", () => {
|
||||
// The other cases restore each tab's EMPTY value, so a bug that always
|
||||
// wrote the default would pass them. This one saves a real selection.
|
||||
const saved = {
|
||||
...EMPTY_SKILLS_UI,
|
||||
selectedSkillUri: "skill://data-analysis/SKILL.md",
|
||||
search: "analysis",
|
||||
};
|
||||
const setSkillsUi = vi.fn();
|
||||
restoreTabUiFromSnapshot(
|
||||
{ Skills: saved },
|
||||
{
|
||||
setToolsUi: vi.fn(),
|
||||
setPromptsUi: vi.fn(),
|
||||
setResourcesUi: vi.fn(),
|
||||
setAppsUi: vi.fn(),
|
||||
setSkillsUi,
|
||||
setTasksUi: vi.fn(),
|
||||
setLogsUi: vi.fn(),
|
||||
setProtocolUi: vi.fn(),
|
||||
setNetworkUi: vi.fn(),
|
||||
},
|
||||
);
|
||||
expect(setSkillsUi).toHaveBeenCalledWith(saved);
|
||||
});
|
||||
|
||||
it("restoreTabUiFromSnapshot falls back to EMPTY state for undefined tab values", () => {
|
||||
const setters = {
|
||||
setToolsUi: vi.fn(),
|
||||
setPromptsUi: vi.fn(),
|
||||
setResourcesUi: vi.fn(),
|
||||
setAppsUi: vi.fn(),
|
||||
setSkillsUi: vi.fn(),
|
||||
setTasksUi: vi.fn(),
|
||||
setLogsUi: vi.fn(),
|
||||
setProtocolUi: vi.fn(),
|
||||
|
|
@ -632,6 +669,7 @@ describe("oauthResume", () => {
|
|||
Prompts: undefined,
|
||||
Resources: undefined,
|
||||
Apps: undefined,
|
||||
Skills: undefined,
|
||||
Tasks: undefined,
|
||||
Logs: undefined,
|
||||
Protocol: undefined,
|
||||
|
|
@ -643,6 +681,7 @@ describe("oauthResume", () => {
|
|||
expect(setters.setPromptsUi).toHaveBeenCalledWith(EMPTY_PROMPTS_UI);
|
||||
expect(setters.setResourcesUi).toHaveBeenCalledWith(EMPTY_RESOURCES_UI);
|
||||
expect(setters.setAppsUi).toHaveBeenCalledWith(EMPTY_APPS_UI);
|
||||
expect(setters.setSkillsUi).toHaveBeenCalledWith(EMPTY_SKILLS_UI);
|
||||
expect(setters.setTasksUi).toHaveBeenCalledWith(EMPTY_TASKS_UI);
|
||||
expect(setters.setLogsUi).toHaveBeenCalledWith(EMPTY_LOGS_UI);
|
||||
expect(setters.setProtocolUi).toHaveBeenCalledWith(EMPTY_PROTOCOL_UI);
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ import {
|
|||
EMPTY_NETWORK_UI,
|
||||
EMPTY_PROMPTS_UI,
|
||||
EMPTY_RESOURCES_UI,
|
||||
EMPTY_SKILLS_UI,
|
||||
EMPTY_TASKS_UI,
|
||||
EMPTY_TOOLS_UI,
|
||||
} from "../components/screens/screenUiState.js";
|
||||
|
|
@ -20,6 +21,7 @@ import type { LogsUiState } from "../components/screens/LoggingScreen/LoggingScr
|
|||
import type { NetworkUiState } from "../components/screens/NetworkScreen/NetworkScreen.js";
|
||||
import type { PromptsUiState } from "../components/screens/PromptsScreen/PromptsScreen.js";
|
||||
import type { ResourcesUiState } from "../components/screens/ResourcesScreen/ResourcesScreen.js";
|
||||
import type { SkillsUiState } from "../components/screens/SkillsScreen/SkillsScreen.js";
|
||||
import type { TasksUiState } from "../components/screens/TasksScreen/TasksScreen.js";
|
||||
import type { ToolsUiState } from "../components/screens/ToolsScreen/ToolsScreen.js";
|
||||
import {
|
||||
|
|
@ -77,6 +79,7 @@ export interface LiftedTabUiState {
|
|||
promptsUi: PromptsUiState;
|
||||
resourcesUi: ResourcesUiState;
|
||||
appsUi: AppsUiState;
|
||||
skillsUi: SkillsUiState;
|
||||
tasksUi: TasksUiState;
|
||||
logsUi: LogsUiState;
|
||||
protocolUi: ProtocolUiState;
|
||||
|
|
@ -88,6 +91,7 @@ export interface TabUiSetters {
|
|||
setPromptsUi: (next: PromptsUiState) => void;
|
||||
setResourcesUi: (next: ResourcesUiState) => void;
|
||||
setAppsUi: (next: AppsUiState) => void;
|
||||
setSkillsUi: (next: SkillsUiState) => void;
|
||||
setTasksUi: (next: TasksUiState) => void;
|
||||
setLogsUi: (next: LogsUiState) => void;
|
||||
setProtocolUi: (next: ProtocolUiState) => void;
|
||||
|
|
@ -102,6 +106,7 @@ export function buildTabUiSnapshot(
|
|||
Tools: state.toolsUi,
|
||||
Prompts: state.promptsUi,
|
||||
Resources: state.resourcesUi,
|
||||
Skills: state.skillsUi,
|
||||
Tasks: state.tasksUi,
|
||||
Logs: state.logsUi,
|
||||
Protocol: state.protocolUi,
|
||||
|
|
@ -155,6 +160,11 @@ export function restoreTabUiFromSnapshot(
|
|||
case "Apps":
|
||||
setters.setAppsUi((value as AppsUiState | undefined) ?? EMPTY_APPS_UI);
|
||||
break;
|
||||
case "Skills":
|
||||
setters.setSkillsUi(
|
||||
(value as SkillsUiState | undefined) ?? EMPTY_SKILLS_UI,
|
||||
);
|
||||
break;
|
||||
case "Tasks":
|
||||
setters.setTasksUi(
|
||||
(value as TasksUiState | undefined) ?? EMPTY_TASKS_UI,
|
||||
|
|
|
|||
239
clients/web/src/test/core/mcp/inspectorClient-skills.test.ts
Normal file
239
clients/web/src/test/core/mcp/inspectorClient-skills.test.ts
Normal file
|
|
@ -0,0 +1,239 @@
|
|||
import { describe, it, expect, vi } from "vitest";
|
||||
import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js";
|
||||
import type { ServerCapabilities } from "@modelcontextprotocol/client";
|
||||
import { SKILLS_EXTENSION_KEY } from "@inspector/core/mcp/skillsSchemas.js";
|
||||
import { SdkError, SdkErrorCode } from "@modelcontextprotocol/client";
|
||||
|
||||
/**
|
||||
* Unit coverage for the Skills extension methods (#2234, SEP-2640).
|
||||
*
|
||||
* The SDK client is stubbed rather than connected: what these assert is the
|
||||
* shape of the outbound request and the normalization of the result, both of
|
||||
* which are decided entirely in `InspectorClient` — and the point worth pinning
|
||||
* is that `skills/*` go out through the ordinary `client.request` path with an
|
||||
* explicit result schema, NOT through the raw-wire channel modern `tasks/*`
|
||||
* needs.
|
||||
*/
|
||||
describe("InspectorClient skills methods (#2234)", () => {
|
||||
const ENTRY = {
|
||||
uri: "skill://demo/SKILL.md",
|
||||
frontmatter: { name: "demo", description: "A demo skill" },
|
||||
resources: [
|
||||
{
|
||||
uri: "skill://demo/ref.md",
|
||||
digest: `sha256:${"a".repeat(64)}`,
|
||||
size: 3,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
interface SkillsInternals {
|
||||
protocolEra: string | undefined;
|
||||
client: {
|
||||
request: (
|
||||
req: { method: string; params: Record<string, unknown> },
|
||||
schema: { parse: (value: unknown) => unknown },
|
||||
) => Promise<unknown>;
|
||||
} | null;
|
||||
capabilities: ServerCapabilities | undefined;
|
||||
}
|
||||
|
||||
function makeClient(): InspectorClient {
|
||||
return new InspectorClient(
|
||||
{ type: "stdio", command: "noop", args: [] },
|
||||
// `environment.transport` is only used on connect(); these tests never
|
||||
// connect, they stub the SDK client directly.
|
||||
{ environment: { transport: () => ({}) as never } },
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* A structural view onto two private fields, so the tests can stub the SDK
|
||||
* client and set `capabilities` without connecting.
|
||||
*
|
||||
* The double cast is justified rather than incidental: `InspectorClient`
|
||||
* declares both members `private`, so no single `as` relates it to a type
|
||||
* that exposes them, and there is no public setter for either — the public
|
||||
* path is `connect()`, which needs a transport, a live server and a
|
||||
* handshake to reach the same state. It is safe because the shape asserted
|
||||
* here is exactly the shape the class declares (`client` is the SDK client;
|
||||
* `capabilities` is `ServerCapabilities | undefined`), so a rename or a type
|
||||
* change on either field breaks these tests at the first use rather than
|
||||
* silently passing. The same seam is used by
|
||||
* `inspectorClient-raw-wire.test.ts`.
|
||||
*/
|
||||
function internals(client: InspectorClient): SkillsInternals {
|
||||
return client as unknown as SkillsInternals;
|
||||
}
|
||||
|
||||
/** Stub the SDK client so `request` parses through the supplied schema. */
|
||||
function stubRequest(client: InspectorClient, result: unknown) {
|
||||
const request = vi.fn(
|
||||
async (
|
||||
_req: { method: string; params: Record<string, unknown> },
|
||||
schema: { parse: (value: unknown) => unknown },
|
||||
) => schema.parse(result),
|
||||
);
|
||||
internals(client).client = { request };
|
||||
return request;
|
||||
}
|
||||
|
||||
it("getSkillsExtension reads the server's declaration", () => {
|
||||
const client = makeClient();
|
||||
expect(client.getSkillsExtension()).toBeUndefined();
|
||||
internals(client).capabilities = {
|
||||
extensions: { [SKILLS_EXTENSION_KEY]: { directoryRead: true } },
|
||||
} as ServerCapabilities;
|
||||
expect(client.getSkillsExtension()).toEqual({ directoryRead: true });
|
||||
});
|
||||
|
||||
it("listSkills throws when not connected", async () => {
|
||||
await expect(makeClient().listSkills()).rejects.toThrow(/not connected/i);
|
||||
});
|
||||
|
||||
it("getSkill throws when not connected", async () => {
|
||||
await expect(makeClient().getSkill("skill://x/SKILL.md")).rejects.toThrow(
|
||||
/not connected/i,
|
||||
);
|
||||
});
|
||||
|
||||
it("sends skills/list with no cursor on the first page", async () => {
|
||||
const client = makeClient();
|
||||
const request = stubRequest(client, { skills: [ENTRY] });
|
||||
const page = await client.listSkills();
|
||||
expect(request.mock.calls[0][0].method).toBe("skills/list");
|
||||
expect(request.mock.calls[0][0].params).not.toHaveProperty("cursor");
|
||||
expect(page.skills).toEqual([ENTRY]);
|
||||
expect(page.nextCursor).toBeUndefined();
|
||||
});
|
||||
|
||||
it("forwards a cursor and returns the server's nextCursor", async () => {
|
||||
const client = makeClient();
|
||||
const request = stubRequest(client, { skills: [], nextCursor: "4" });
|
||||
const page = await client.listSkills("2");
|
||||
expect(request.mock.calls[0][0].params.cursor).toBe("2");
|
||||
expect(page.nextCursor).toBe("4");
|
||||
});
|
||||
|
||||
it("stamps call metadata onto skills/list as _meta", async () => {
|
||||
const client = makeClient();
|
||||
const request = stubRequest(client, { skills: [] });
|
||||
await client.listSkills(undefined, { trace: "abc" });
|
||||
expect(request.mock.calls[0][0].params._meta).toMatchObject({
|
||||
trace: "abc",
|
||||
});
|
||||
});
|
||||
|
||||
it("sends skills/get with the requested uri", async () => {
|
||||
const client = makeClient();
|
||||
const request = stubRequest(client, { skill: ENTRY });
|
||||
await client.getSkill("skill://demo/SKILL.md");
|
||||
expect(request.mock.calls[0][0].method).toBe("skills/get");
|
||||
expect(request.mock.calls[0][0].params.uri).toBe("skill://demo/SKILL.md");
|
||||
});
|
||||
|
||||
it("unwraps the skills/get envelope to the entry", async () => {
|
||||
const client = makeClient();
|
||||
stubRequest(client, { skill: ENTRY });
|
||||
expect(await client.getSkill("skill://demo/SKILL.md")).toEqual(ENTRY);
|
||||
});
|
||||
|
||||
it("rejects a skills/get result returned without its envelope", async () => {
|
||||
// Normalizing it would let a non-conforming server through the one place
|
||||
// that could have reported it.
|
||||
const client = makeClient();
|
||||
stubRequest(client, ENTRY);
|
||||
await expect(
|
||||
client.getSkill("skill://demo/SKILL.md"),
|
||||
).rejects.toBeDefined();
|
||||
});
|
||||
|
||||
it("requires the modern list envelope on a modern connection", async () => {
|
||||
// SEP-2640: "In protocol versions 2026-07-28 and later, the result also
|
||||
// carries … `ttlMs` and `cacheScope`." Nothing else validates it —
|
||||
// `skills/*` is consumer-owned, so the SDK codec never sees it.
|
||||
const client = makeClient();
|
||||
internals(client).protocolEra = "modern";
|
||||
stubRequest(client, { skills: [] });
|
||||
await expect(client.listSkills()).rejects.toBeDefined();
|
||||
});
|
||||
|
||||
it("accepts a modern result that carries the envelope", async () => {
|
||||
const client = makeClient();
|
||||
internals(client).protocolEra = "modern";
|
||||
stubRequest(client, {
|
||||
resultType: "complete",
|
||||
ttlMs: 0,
|
||||
cacheScope: "public",
|
||||
skills: [ENTRY],
|
||||
});
|
||||
await expect(client.listSkills()).resolves.toMatchObject({
|
||||
skills: [ENTRY],
|
||||
});
|
||||
});
|
||||
|
||||
it("does NOT require the envelope on a legacy connection", async () => {
|
||||
// Those are 2026-era attributes; failing a legacy server for their absence
|
||||
// would reject a conforming server.
|
||||
const client = makeClient();
|
||||
stubRequest(client, { skills: [ENTRY] });
|
||||
await expect(client.listSkills()).resolves.toMatchObject({
|
||||
skills: [ENTRY],
|
||||
});
|
||||
});
|
||||
|
||||
it("attributes a rejected skills/get envelope to its Protocol entry", async () => {
|
||||
// Without this the Skills screen shows an error while the Protocol tab
|
||||
// renders the same exchange as a clean success. Done in the client rather
|
||||
// than a store because `skills/get` has none — the screen calls it.
|
||||
const client = makeClient();
|
||||
const marked: [string, string][] = [];
|
||||
(
|
||||
client as unknown as {
|
||||
markResponseRejected: (m: string, r: string) => void;
|
||||
}
|
||||
).markResponseRejected = (method, reason) => {
|
||||
marked.push([method, reason]);
|
||||
};
|
||||
internals(client).client = {
|
||||
request: async () => {
|
||||
throw new SdkError(
|
||||
SdkErrorCode.InvalidResult,
|
||||
"Invalid result for skills/get",
|
||||
);
|
||||
},
|
||||
};
|
||||
await expect(client.getSkill("skill://demo/SKILL.md")).rejects.toThrow();
|
||||
expect(marked).toEqual([["skills/get", "Invalid result for skills/get"]]);
|
||||
});
|
||||
|
||||
it("does NOT attribute a transport failure on skills/get", async () => {
|
||||
// No response frame arrived, so the last-answered id still points at an
|
||||
// earlier, successful exchange; marking it would stamp that one.
|
||||
const client = makeClient();
|
||||
const marked: string[] = [];
|
||||
(
|
||||
client as unknown as {
|
||||
markResponseRejected: (m: string, r: string) => void;
|
||||
}
|
||||
).markResponseRejected = (method) => {
|
||||
marked.push(method);
|
||||
};
|
||||
internals(client).client = {
|
||||
request: async () => {
|
||||
throw new SdkError(SdkErrorCode.ConnectionClosed, "Connection closed");
|
||||
},
|
||||
};
|
||||
await expect(client.getSkill("skill://demo/SKILL.md")).rejects.toThrow();
|
||||
expect(marked).toEqual([]);
|
||||
});
|
||||
|
||||
it("rejects a skills/list result that is not a skills page", async () => {
|
||||
// The explicit result schema is the whole client-side mechanism for a
|
||||
// consumer-owned extension method, so a nonconforming result must fail
|
||||
// here rather than reaching the UI as a half-parsed shape.
|
||||
const client = makeClient();
|
||||
stubRequest(client, { notSkills: true });
|
||||
await expect(client.listSkills()).rejects.toBeDefined();
|
||||
});
|
||||
});
|
||||
Some files were not shown because too many files have changed in this diff Show more
Loading…
Reference in a new issue