Merge pull request #2367 from modelcontextprotocol/v2/feat/2294-skills-catalog-budget-settings

feat: make the skills verification catalog budget configurable per server (#2294)
This commit is contained in:
Cliff Hall 2026-09-14 01:40:52 -04:00 • committed by GitHub
commit 2f3099f764
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 613 additions and 7 deletions

View file

@ -130,6 +130,12 @@ function InteractiveRender(args: ServerSettingsFormProps) {
settings: { ...args.settings, maxFetchRequests: value },
});
}}
onSkillCatalogLimitChange={(field, value) => {
args.onSkillCatalogLimitChange(field, value);
updateArgs({
settings: { ...args.settings, [field]: value },
});
}}
onProtocolEraChange={(value) => {
args.onProtocolEraChange(value);
updateArgs({
@ -178,6 +184,7 @@ const meta: Meta<typeof ServerSettingsForm> = {
onPaginatedListsChange: fn(),
onAdvertisedExtensionChange: fn(),
onMaxFetchRequestsChange: fn(),
onSkillCatalogLimitChange: fn(),
onProtocolEraChange: fn(),
onModernLogLevelChange: fn(),
onOAuthChange: fn(),

View file

@ -70,6 +70,7 @@ const baseHandlers = {
onPaginatedListsChange: vi.fn(),
onAdvertisedExtensionChange: vi.fn(),
onMaxFetchRequestsChange: vi.fn(),
onSkillCatalogLimitChange: vi.fn(),
onProtocolEraChange: vi.fn(),
onModernLogLevelChange: vi.fn(),
onOAuthChange: vi.fn(),
@ -559,6 +560,96 @@ describe("ServerSettingsForm", () => {
expect(onMaxFetchRequestsChange).toHaveBeenLastCalledWith(2500);
});
describe("Skills section (#2294)", () => {
it("renders the default catalog budget when the server sets none", () => {
renderWithMantine(
<ServerSettingsForm
{...baseHandlers}
settings={emptySettings}
expandedSections={["skills"]}
/>,
);
expect(screen.getByText("Skills")).toBeInTheDocument();
expect(screen.getByLabelText(/Maximum Number of Skills/)).toHaveValue(
"256",
);
expect(screen.getByLabelText(/Maximum Catalog Size/)).toHaveValue(
"67,108,864 bytes",
);
});
it("renders configured limits", () => {
renderWithMantine(
<ServerSettingsForm
{...baseHandlers}
settings={{
...emptySettings,
skillCatalogMaxSkills: 10,
skillCatalogMaxBytes: 2048,
}}
expandedSections={["skills"]}
/>,
);
expect(screen.getByLabelText(/Maximum Number of Skills/)).toHaveValue(
"10",
);
expect(screen.getByLabelText(/Maximum Catalog Size/)).toHaveValue(
"2,048 bytes",
);
});
it("emits the typed number for each field", async () => {
const user = userEvent.setup();
const onSkillCatalogLimitChange = vi.fn();
renderWithMantine(
<ServerSettingsForm
{...baseHandlers}
onSkillCatalogLimitChange={onSkillCatalogLimitChange}
settings={{
...emptySettings,
skillCatalogMaxSkills: 1,
skillCatalogMaxBytes: 1,
}}
expandedSections={["skills"]}
/>,
);
await user.type(screen.getByLabelText(/Maximum Number of Skills/), "2");
expect(onSkillCatalogLimitChange).toHaveBeenLastCalledWith(
"skillCatalogMaxSkills",
12,
);
await user.type(screen.getByLabelText(/Maximum Catalog Size/), "5");
expect(onSkillCatalogLimitChange).toHaveBeenLastCalledWith(
"skillCatalogMaxBytes",
15,
);
});
it("keeps the current limit when a field is cleared", async () => {
// A cleared budget is not a usable setting, so it must not persist as 0.
const user = userEvent.setup();
const onSkillCatalogLimitChange = vi.fn();
renderWithMantine(
<ServerSettingsForm
{...baseHandlers}
onSkillCatalogLimitChange={onSkillCatalogLimitChange}
settings={{ ...emptySettings, skillCatalogMaxBytes: 4096 }}
expandedSections={["skills"]}
/>,
);
await user.clear(screen.getByLabelText(/Maximum Number of Skills/));
expect(onSkillCatalogLimitChange).toHaveBeenLastCalledWith(
"skillCatalogMaxSkills",
256,
);
await user.clear(screen.getByLabelText(/Maximum Catalog Size/));
expect(onSkillCatalogLimitChange).toHaveBeenLastCalledWith(
"skillCatalogMaxBytes",
4096,
);
});
});
describe("stdio Working Directory (Options) / Environment Variables section", () => {
it("hides the Working Directory field and the Environment Variables section for non-stdio servers", () => {
renderWithMantine(

View file

@ -38,6 +38,10 @@ import {
} from "@inspector/core/auth/authorizationParams.js";
import { oauthEndpointUrlError } from "@inspector/core/auth/endpointOverrides.js";
import { ADVERTISABLE_EXTENSIONS } from "@inspector/core/mcp/extensions.js";
import {
isSkillCatalogLimit,
resolveSkillCatalogBudget,
} from "@inspector/core/mcp/skills.js";
import type { Root } from "@modelcontextprotocol/client";
/**
@ -60,9 +64,15 @@ export type ServerSettingsSection =
| "headers"
| "metadata"
| "timeouts"
| "skills"
| "oauth"
| "roots";
/** The two per-server skills verification budget fields (#2294). */
export type SkillCatalogLimitField =
| "skillCatalogMaxSkills"
| "skillCatalogMaxBytes";
export interface ServerSettingsFormProps {
settings: InspectorServerSettings;
/** Transport type — EMA checkbox is hidden for stdio. Defaults to streamable-http. */
@ -101,6 +111,14 @@ export interface ServerSettingsFormProps {
*/
onAdvertisedExtensionChange: (key: string, checked: boolean) => void;
onMaxFetchRequestsChange: (value: number) => void;
/**
* Set one skills verification budget limit. Only ever called with a positive
* integer: a cleared or invalid entry re-emits the current value (#2294).
*/
onSkillCatalogLimitChange: (
field: SkillCatalogLimitField,
value: number,
) => void;
onProtocolEraChange: (value: ServerProtocolEra) => void;
onModernLogLevelChange: (value: ModernLogLevel) => void;
/**
@ -166,6 +184,22 @@ const TimeoutInput = NumberInput.withProps({
suffix: " ms",
});
// Positive integers only — `0` is not "unlimited" for a verification budget.
const SkillCountInput = NumberInput.withProps({
min: 1,
allowDecimal: false,
allowNegative: false,
thousandSeparator: ",",
});
const SkillBytesInput = NumberInput.withProps({
min: 1,
allowDecimal: false,
allowNegative: false,
thousandSeparator: ",",
suffix: " bytes",
});
const ExtensionsHint = Text.withProps({
size: "xs",
c: "var(--inspector-text-secondary)",
@ -447,6 +481,7 @@ export function ServerSettingsForm({
onPaginatedListsChange,
onAdvertisedExtensionChange,
onMaxFetchRequestsChange,
onSkillCatalogLimitChange,
onProtocolEraChange,
onModernLogLevelChange,
negotiatedEra,
@ -496,6 +531,20 @@ export function ServerSettingsForm({
Number.isNaN(parsed) ? settings.maxFetchRequests : parsed,
);
};
// Absent settings render as the defaults `verifySkills` will actually apply.
const skillCatalogBudget = resolveSkillCatalogBudget(settings);
const handleSkillCatalogLimitChange =
(field: SkillCatalogLimitField, current: number) =>
(value: number | string) => {
// Mantine emits "" on a clear. Like the Network Log Size field, keep the
// current value rather than persisting something `verifySkills` would
// discard — a cleared or zero budget is not a meaningful setting here.
const parsed = typeof value === "number" ? value : parseInt(value, 10);
onSkillCatalogLimitChange(
field,
isSkillCatalogLimit(parsed) ? parsed : current,
);
};
const handleTimeoutChange =
(field: "connectionTimeout" | "requestTimeout" | "taskTtl") =>
(value: number | string) => {
@ -816,6 +865,32 @@ export function ServerSettingsForm({
</Accordion.Panel>
</Accordion.Item>
<Accordion.Item value="skills">
<Accordion.Control>Skills</Accordion.Control>
<Accordion.Panel>
<Stack gap="xs">
<SkillCountInput
label="Maximum Number of Skills"
description="The maximum number of skills whose files are read in one verification run (the CLI's --verify, or the TUI's Skills pane). Skills past the limit are still listed, and reported as incomplete rather than checked."
value={skillCatalogBudget.maxSkills}
onChange={handleSkillCatalogLimitChange(
"skillCatalogMaxSkills",
skillCatalogBudget.maxSkills,
)}
/>
<SkillBytesInput
label="Maximum Catalog Size"
description="The maximum number of bytes read across all skills in one verification run. Once it is reached, no further skills are read, and the remaining ones are reported as incomplete."
value={skillCatalogBudget.maxBytes}
onChange={handleSkillCatalogLimitChange(
"skillCatalogMaxBytes",
skillCatalogBudget.maxBytes,
)}
/>
</Stack>
</Accordion.Panel>
</Accordion.Item>
<Accordion.Item value="roots">
<Accordion.Control>Roots</Accordion.Control>
<Accordion.Panel>

View file

@ -679,6 +679,27 @@ describe("ServerSettingsModal", () => {
expect(typeof call.maxFetchRequests).toBe("number");
});
it("calls onSettingsChange when changing a skills catalog limit (#2294)", async () => {
const user = userEvent.setup();
const onSettingsChange = vi.fn();
renderWithMantine(
<ServerSettingsModal
opened
settings={{ ...emptySettings, skillCatalogMaxSkills: 3 }}
serverType="streamable-http"
isStdio={false}
onClose={vi.fn()}
onSettingsChange={onSettingsChange}
/>,
);
await user.click(screen.getByRole("button", { name: "Skills" }));
await user.type(screen.getByLabelText(/Maximum Number of Skills/), "0");
expect(onSettingsChange).toHaveBeenLastCalledWith({
...emptySettings,
skillCatalogMaxSkills: 30,
});
});
it("preserves sibling rows when editing one of several headers", async () => {
const user = userEvent.setup();
const onSettingsChange = vi.fn();

View file

@ -17,6 +17,7 @@ import type { SecretStorageInfo } from "@inspector/core/auth/secret-storage-info
import {
ServerSettingsForm,
type ServerSettingsSection,
type SkillCatalogLimitField,
} from "../ServerSettingsForm/ServerSettingsForm";
// The "environment" section only renders for stdio servers and the "oauth"
@ -34,6 +35,7 @@ function allSectionsFor(
"headers",
"metadata",
"timeouts",
"skills",
...(isOAuthCapableServerType(serverType) ? (["oauth"] as const) : []),
"roots",
];
@ -226,6 +228,13 @@ export function ServerSettingsModal({
onSettingsChange({ ...settings, maxFetchRequests: value });
}
function handleSkillCatalogLimitChange(
field: SkillCatalogLimitField,
value: number,
) {
onSettingsChange({ ...settings, [field]: value });
}
function handleProtocolEraChange(value: ServerProtocolEra) {
onSettingsChange({ ...settings, protocolEra: value });
}
@ -299,6 +308,7 @@ export function ServerSettingsModal({
onPaginatedListsChange={handlePaginatedListsChange}
onAdvertisedExtensionChange={handleAdvertisedExtensionChange}
onMaxFetchRequestsChange={handleMaxFetchRequestsChange}
onSkillCatalogLimitChange={handleSkillCatalogLimitChange}
onProtocolEraChange={handleProtocolEraChange}
onModernLogLevelChange={handleModernLogLevelChange}
negotiatedEra={negotiatedEra}

View file

@ -456,6 +456,55 @@ describe("serverEntriesToMcpConfig", () => {
expect(round).toEqual(original);
});
it("round-trips the skills catalog budget (#2294)", () => {
const original: MCPConfig = {
mcpServers: {
sk: {
type: "streamable-http",
url: "https://x.test/mcp",
skillCatalogMaxSkills: 10,
skillCatalogMaxBytes: 2048,
},
},
};
const [entry] = mcpConfigToServerEntries(original);
expect(entry?.settings?.skillCatalogMaxSkills).toBe(10);
expect(entry?.settings?.skillCatalogMaxBytes).toBe(2048);
const round = serverEntriesToMcpConfig(mcpConfigToServerEntries(original));
expect(round).toEqual(original);
});
it("drops an unusable skills catalog limit on read and omits the default on write", () => {
const [entry] = mcpConfigToServerEntries({
mcpServers: {
sk: {
type: "streamable-http",
url: "https://x.test/mcp",
skillCatalogMaxSkills: 0,
skillCatalogMaxBytes: 1.5,
},
},
});
// Present on disk still materializes a settings node, but neither value
// is usable, so both read back as absent (the default budget).
expect(entry?.settings).toBeDefined();
expect(entry?.settings?.skillCatalogMaxSkills).toBeUndefined();
expect(entry?.settings?.skillCatalogMaxBytes).toBeUndefined();
const round = serverEntriesToMcpConfig([
{
...entry!,
settings: {
...entry!.settings!,
skillCatalogMaxSkills: 256,
skillCatalogMaxBytes: 64 * 1024 * 1024,
},
},
]);
expect("skillCatalogMaxSkills" in (round.mcpServers.sk ?? {})).toBe(false);
expect("skillCatalogMaxBytes" in (round.mcpServers.sk ?? {})).toBe(false);
});
it("round-trips protocolEra: lifts a non-default value to settings and back to disk", () => {
const original: MCPConfig = {
mcpServers: {

View file

@ -1,6 +1,9 @@
import { describe, it, expect, vi } from "vitest";
import type { InspectorClientProtocol } from "@inspector/core/mcp/inspectorClientProtocol.js";
import type { SkillEntry } from "@inspector/core/mcp/skillsSchemas.js";
import {
DYNAMIC_RESOURCES,
type SkillEntry,
} from "@inspector/core/mcp/skillsSchemas.js";
import {
SKILL_MAX_CATALOG_SKILLS,
sha256Digest,
@ -790,6 +793,181 @@ describe("verifySkills (#2248)", () => {
expect(reports[0].outcome).toBe("verified");
});
it("honors the server's configured catalog budget (#2294)", async () => {
// Read through `getServerSettings()`, so neither caller has to pass it.
const enc = new TextEncoder();
const mdFor = (i: number) =>
`---\nname: c${i}\ndescription: A demo\n---\n\n# c${i}\n`;
const skills = await Promise.all(
Array.from({ length: 4 }, async (_, i): Promise<SkillEntry> => {
const bytes = enc.encode(mdFor(i));
return entry({
uri: `skill://c${i}/SKILL.md`,
frontmatter: { name: `c${i}`, description: "A demo" },
resources: [
{
uri: `skill://c${i}/SKILL.md`,
digest: await sha256Digest(bytes),
size: bytes.byteLength,
},
],
});
}),
);
const readResource = vi.fn(async (uri: string) => ({
result: {
contents: [
{ uri, text: mdFor(Number(/c(\d+)/.exec(uri)?.[1] ?? "0")) },
],
},
}));
const client = {
readResource,
getServerSettings: () => ({ skillCatalogMaxSkills: 2 }),
} as unknown as InspectorClientProtocol;
const reports = await verifySkills(client, skills);
expect(readResource.mock.calls.length).toBe(2);
expect(reports.map((r) => r.outcome)).toEqual([
"verified",
"verified",
"incomplete",
"incomplete",
]);
// The reason names the limit that actually applied, not the default.
expect(reports[2].incomplete).toMatch(/budget of 2 skills \/ 67108864/);
});
it("honors a configured catalog BYTE budget (#2294)", async () => {
// Each skill's SKILL.md is ~50 bytes, so a 60-byte budget is spent by the
// second skill: the third is where reading stops.
const enc = new TextEncoder();
const mdFor = (i: number) =>
`---\nname: b${i}\ndescription: A demo\n---\n\n# b${i}\n`;
const skills = await Promise.all(
Array.from({ length: 4 }, async (_, i): Promise<SkillEntry> => {
const bytes = enc.encode(mdFor(i));
return entry({
uri: `skill://b${i}/SKILL.md`,
frontmatter: { name: `b${i}`, description: "A demo" },
resources: [
{
uri: `skill://b${i}/SKILL.md`,
digest: await sha256Digest(bytes),
size: bytes.byteLength,
},
],
});
}),
);
const readResource = vi.fn(async (uri: string) => ({
result: {
contents: [
{ uri, text: mdFor(Number(/b(\d+)/.exec(uri)?.[1] ?? "0")) },
],
},
}));
const client = {
readResource,
getServerSettings: () => ({ skillCatalogMaxBytes: 60 }),
} as unknown as InspectorClientProtocol;
const reports = await verifySkills(client, skills);
expect(readResource.mock.calls.length).toBe(2);
expect(reports.map((r) => r.outcome)).toEqual([
"verified",
"verified",
"incomplete",
"incomplete",
]);
expect(reports[2].incomplete).toMatch(/256 skills \/ 60 bytes/);
});
it("stops reading once the byte budget is exactly REACHED, not only exceeded", async () => {
// Every name is one digit, so every SKILL.md has the same length. With the
// budget set to exactly one file's charge, the first skill spends it and
// the second must not be read (Copilot, #2294).
const enc = new TextEncoder();
const mdFor = (i: number) =>
`---\nname: e${i}\ndescription: A demo\n---\n\n# e${i}\n`;
const oneFile = enc.encode(mdFor(0)).byteLength;
const skills = await Promise.all(
Array.from({ length: 3 }, async (_, i): Promise<SkillEntry> => {
const bytes = enc.encode(mdFor(i));
return entry({
uri: `skill://e${i}/SKILL.md`,
frontmatter: { name: `e${i}`, description: "A demo" },
resources: [
{
uri: `skill://e${i}/SKILL.md`,
digest: await sha256Digest(bytes),
size: bytes.byteLength,
},
],
});
}),
);
const readResource = vi.fn(async (uri: string) => ({
result: {
contents: [
{ uri, text: mdFor(Number(/e(\d+)/.exec(uri)?.[1] ?? "0")) },
],
},
}));
const client = {
readResource,
getServerSettings: () => ({ skillCatalogMaxBytes: oneFile }),
} as unknown as InspectorClientProtocol;
const reports = await verifySkills(client, skills);
expect(readResource.mock.calls.length).toBe(1);
expect(reports.map((r) => r.outcome)).toEqual([
"verified",
"incomplete",
"incomplete",
]);
});
it("charges a dynamic skill's SKILL.md read to the byte budget", async () => {
// A `"dynamic"` skill has no manifest, so its only read is the fallback
// SKILL.md fetch. Uncharged, `catalogBytes` stayed 0 and a byte budget of
// 1 read every dynamic skill in the catalog (Copilot).
const mdFor = (i: number) =>
`---\nname: d${i}\ndescription: A demo\n---\n\n# d${i}\n`;
const skills: SkillEntry[] = Array.from({ length: 3 }, (_, i) => ({
uri: `skill://d${i}/SKILL.md`,
frontmatter: { name: `d${i}`, description: "A demo" },
resources: DYNAMIC_RESOURCES,
}));
const readResource = vi.fn(async (uri: string) => ({
result: {
contents: [
{ uri, text: mdFor(Number(/d(\d+)/.exec(uri)?.[1] ?? "0")) },
],
},
}));
const client = {
readResource,
getServerSettings: () => ({ skillCatalogMaxBytes: 1 }),
} as unknown as InspectorClientProtocol;
const reports = await verifySkills(client, skills);
expect(readResource.mock.calls.length).toBe(1);
expect(reports[0].outcome).not.toBe("incomplete");
expect(reports[1].outcome).toBe("incomplete");
expect(reports[2].outcome).toBe("incomplete");
});
it("falls back to the default budget for an unusable configured limit", async () => {
const { skill, client } = await truncatable({ name: "fb", count: 2 });
const withSettings = {
...client,
readResource: client.readResource,
getServerSettings: () => ({
skillCatalogMaxSkills: 0,
skillCatalogMaxBytes: -1,
}),
} as unknown as InspectorClientProtocol;
const [report] = await verifySkills(withSettings, [skill]);
expect(report.outcome).toBe("verified");
});
it("is not incomplete when the budget is crossed by the LAST entry", async () => {
// Crossing the line on the final row stopped nothing: every manifest entry
// was fetched and checked. Reporting "Stopped after 4 of 4" there both

View file

@ -832,6 +832,13 @@ describe("server.ts supplemental coverage", () => {
expect((await res.json()).error).toMatch(/maxFetchRequests/);
});
it("rejects invalid skills catalog limits (#2294)", async () => {
const skills = await postSettings({ ...base, skillCatalogMaxSkills: 0 });
expect((await skills.json()).error).toMatch(/skillCatalogMaxSkills/);
const bytes = await postSettings({ ...base, skillCatalogMaxBytes: 1.5 });
expect((await bytes.json()).error).toMatch(/skillCatalogMaxBytes/);
});
it("rejects a non-string OAuth field", async () => {
const res = await postSettings({ ...base, oauthClientId: 7 });
expect((await res.json()).error).toMatch(/oauthClientId/);
@ -960,6 +967,8 @@ describe("server.ts supplemental coverage", () => {
paginatedLists: true,
advertisedExtensions: { "io.modelcontextprotocol/tasks": false },
maxFetchRequests: 5,
skillCatalogMaxSkills: 10,
skillCatalogMaxBytes: 2048,
protocolEra: "modern",
modernLogLevel: "off",
oauthClientId: "cid",
@ -999,6 +1008,8 @@ describe("server.ts supplemental coverage", () => {
requestTimeout: -1,
taskTtl: "x",
maxFetchRequests: -1,
skillCatalogMaxSkills: 0,
skillCatalogMaxBytes: "big",
// unknown era literal → isProtocolEra branch
protocolEra: "future",
// unknown modern log level → isModernLogLevel branch (#1629)
@ -1030,6 +1041,8 @@ describe("server.ts supplemental coverage", () => {
"requestTimeout",
"taskTtl",
"maxFetchRequests",
"skillCatalogMaxSkills",
"skillCatalogMaxBytes",
"protocolEra",
"modernLogLevel",
"oauth",