* Extensions: first-class advertised-extensions map (Phase 1 of #1633) Collapse the two ad-hoc `capabilities.extensions` spreads in the InspectorClient constructor (EMA, gated on enterpriseManaged; the modern Tasks extension, always-on) into a single builder that is the source of truth for what the client advertises. - core/mcp/extensions.ts: `ADVERTISABLE_EXTENSIONS` registry (id + label + default-on flag), the `EMA_EXTENSION_KEY` constant, and `buildClientExtensions()`. The registry is the shared list the Server Settings toggle UI (Phase 2, #1739) will drive; EMA stays a conditional built-in (auth-mode driven, not a free toggle). - InspectorClientOptions.advertisedExtensions: per-extension overrides (present key wins over the registry default) so a future UI can toggle advertised extensions as a debugging knob. - Behavior preserved: tasks defaults to advertised, so the map is non-empty and rides every modern request envelope; EMA advertised iff enterpriseManaged. The raw modern-task envelope re-stamp is left untouched (it fires only on the raw tasks channel, which requires the extension anyway). Tests: extensions.test.ts covers the registry and every builder branch (defaults, EMA on/off, override enable/disable, unknown key) at 100%. The tasks-era and EMA integration suites still assert the same wire advertisement. `advertisedExtensions` is plumbed but not yet wired to the form (that lands in Phase 2). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Extensions: address Phase 1 review (EMA-override test + envelope comment) - extensions.test.ts: add an EMA-specific assertion that an override for EMA_EXTENSION_KEY cannot advertise it (auth-mode only) — documents intent and guards against EMA being added to ADVERTISABLE_EXTENSIONS by mistake. - inspectorClient.ts: rewrite the withModernTaskEnvelope tasks-stamp comment so it stays accurate once Phase 2 lets a user disable general tasks advertisement — the force-stamp is load-bearing (the raw tasks/* channel requires the extension), not a redundant re-add. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * TUI: de-flake step-up OAuth frame assertions (raise poll ceiling) The step-up authorize tests intermittently timed out on the final "Step-up authorization succeeded" frame. Root cause is a test-infra timing budget, not product code: waitForFrame/waitUntil polled only 25× (25ms tick ≈ 625ms), and the step-up flow adds an extra async OAuth-runner hop plus React commits that can exceed that under CI load with v8 coverage. Hoist the poll count into a named POLL_TRIES = 100 (~2.5s ceiling). A poll exits the instant its predicate is true, so a higher ceiling never slows a passing assertion — it only widens the margin for the slow path. Verified 5× green (3 plain + 2 under --coverage), including the previously-flaky step-up cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * TUI: prettier-format the wrapped waitForFrame signature The POLL_TRIES change pushed waitForFrame's signature past the print width; Prettier wraps it. (Root has no aggregate format script — format is per-client, so this was missed locally.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
e70a11360c
commit
2f80958d3f
5 changed files with 238 additions and 16 deletions
|
|
@ -516,8 +516,17 @@ async function press(r: RenderResult, keys: string[]) {
|
|||
* Poll until `predicate` is true (or the tries run out). React + ink schedule
|
||||
* renders across several macrotasks, so async state set by a flow can take more
|
||||
* than one fixed tick to land under coverage instrumentation.
|
||||
*
|
||||
* The default budget (POLL_TRIES × 25ms tick) is generous on purpose: a poll
|
||||
* exits the instant the predicate is true, so a high ceiling never slows a
|
||||
* passing assertion — it only widens the margin for the slow path. Flows with
|
||||
* an extra async hop (e.g. the step-up OAuth runner before the success frame)
|
||||
* plus React commits can exceed a tight budget under CI load with v8 coverage,
|
||||
* which is what made the step-up frame assertions intermittently time out.
|
||||
*/
|
||||
async function waitUntil(predicate: () => boolean, tries = 25) {
|
||||
const POLL_TRIES = 100;
|
||||
|
||||
async function waitUntil(predicate: () => boolean, tries = POLL_TRIES) {
|
||||
for (let i = 0; i < tries; i++) {
|
||||
if (predicate()) return;
|
||||
await tick();
|
||||
|
|
@ -529,7 +538,11 @@ async function waitUntil(predicate: () => boolean, tries = 25) {
|
|||
* settling races a single fixed tick under v8 coverage instrumentation, so
|
||||
* frame assertions that follow a mount/keypress use this instead of one tick.
|
||||
*/
|
||||
async function waitForFrame(r: RenderResult, substr: string, tries = 25) {
|
||||
async function waitForFrame(
|
||||
r: RenderResult,
|
||||
substr: string,
|
||||
tries = POLL_TRIES,
|
||||
) {
|
||||
await waitUntil(() => (r.lastFrame() ?? "").includes(substr), tries);
|
||||
}
|
||||
|
||||
|
|
|
|||
97
clients/web/src/test/core/mcp/extensions.test.ts
Normal file
97
clients/web/src/test/core/mcp/extensions.test.ts
Normal file
|
|
@ -0,0 +1,97 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import {
|
||||
ADVERTISABLE_EXTENSIONS,
|
||||
EMA_EXTENSION_KEY,
|
||||
buildClientExtensions,
|
||||
} from "@inspector/core/mcp/extensions.js";
|
||||
import { TASKS_EXTENSION_KEY } from "@inspector/core/mcp/modernTaskSchemas.js";
|
||||
|
||||
describe("extensions (#1738)", () => {
|
||||
describe("ADVERTISABLE_EXTENSIONS registry", () => {
|
||||
it("lists the Tasks extension, advertised by default", () => {
|
||||
const tasks = ADVERTISABLE_EXTENSIONS.find(
|
||||
(e) => e.key === TASKS_EXTENSION_KEY,
|
||||
);
|
||||
expect(tasks).toBeDefined();
|
||||
expect(tasks?.defaultAdvertised).toBe(true);
|
||||
expect(tasks?.label).toContain("Tasks");
|
||||
});
|
||||
|
||||
it("does not list EMA (it follows the auth mode, not a toggle)", () => {
|
||||
expect(
|
||||
ADVERTISABLE_EXTENSIONS.some((e) => e.key === EMA_EXTENSION_KEY),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("has unique keys and non-empty labels", () => {
|
||||
const keys = ADVERTISABLE_EXTENSIONS.map((e) => e.key);
|
||||
expect(new Set(keys).size).toBe(keys.length);
|
||||
for (const ext of ADVERTISABLE_EXTENSIONS) {
|
||||
expect(ext.label.length).toBeGreaterThan(0);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildClientExtensions", () => {
|
||||
it("advertises registry defaults with no overrides (tasks on)", () => {
|
||||
const map = buildClientExtensions({ enterpriseManaged: false });
|
||||
expect(map).toEqual({ [TASKS_EXTENSION_KEY]: {} });
|
||||
});
|
||||
|
||||
it("adds EMA when enterpriseManaged, alongside the registry defaults", () => {
|
||||
const map = buildClientExtensions({ enterpriseManaged: true });
|
||||
expect(map).toEqual({
|
||||
[TASKS_EXTENSION_KEY]: {},
|
||||
[EMA_EXTENSION_KEY]: {},
|
||||
});
|
||||
});
|
||||
|
||||
it("omits EMA when not enterpriseManaged", () => {
|
||||
const map = buildClientExtensions({ enterpriseManaged: false });
|
||||
expect(map).not.toHaveProperty(EMA_EXTENSION_KEY);
|
||||
});
|
||||
|
||||
it("honors a user override that disables a default-on extension", () => {
|
||||
const map = buildClientExtensions({
|
||||
enterpriseManaged: false,
|
||||
advertised: { [TASKS_EXTENSION_KEY]: false },
|
||||
});
|
||||
expect(map).toEqual({});
|
||||
});
|
||||
|
||||
it("honors a user override that keeps a default-on extension enabled", () => {
|
||||
const map = buildClientExtensions({
|
||||
enterpriseManaged: false,
|
||||
advertised: { [TASKS_EXTENSION_KEY]: true },
|
||||
});
|
||||
expect(map).toEqual({ [TASKS_EXTENSION_KEY]: {} });
|
||||
});
|
||||
|
||||
it("does not let an override advertise EMA (auth-mode only)", () => {
|
||||
// EMA is not a free toggle: it follows the auth mode, so an override for
|
||||
// its key must not be able to advertise it. Locks in intent and guards
|
||||
// against someone mistakenly adding EMA to ADVERTISABLE_EXTENSIONS.
|
||||
const map = buildClientExtensions({
|
||||
enterpriseManaged: false,
|
||||
advertised: { [EMA_EXTENSION_KEY]: true },
|
||||
});
|
||||
expect(map).not.toHaveProperty(EMA_EXTENSION_KEY);
|
||||
});
|
||||
|
||||
it("ignores override keys that are not in the registry", () => {
|
||||
const map = buildClientExtensions({
|
||||
enterpriseManaged: false,
|
||||
advertised: { "io.example/unknown": true },
|
||||
});
|
||||
expect(map).toEqual({ [TASKS_EXTENSION_KEY]: {} });
|
||||
});
|
||||
|
||||
it("layers EMA on even when all registry entries are disabled", () => {
|
||||
const map = buildClientExtensions({
|
||||
enterpriseManaged: true,
|
||||
advertised: { [TASKS_EXTENSION_KEY]: false },
|
||||
});
|
||||
expect(map).toEqual({ [EMA_EXTENSION_KEY]: {} });
|
||||
});
|
||||
});
|
||||
});
|
||||
93
core/mcp/extensions.ts
Normal file
93
core/mcp/extensions.ts
Normal file
|
|
@ -0,0 +1,93 @@
|
|||
import type { ClientCapabilities } from "@modelcontextprotocol/client";
|
||||
import { TASKS_EXTENSION_KEY } from "./modernTaskSchemas.js";
|
||||
|
||||
/**
|
||||
* Extension identifier for SEP-2350 enterprise-managed authorization. Advertised
|
||||
* when the connection routes through the enterprise IdP (EMA), which is a
|
||||
* property of the auth mode rather than a free debugging toggle — so it is a
|
||||
* conditional built-in in {@link buildClientExtensions}, not a registry entry.
|
||||
*/
|
||||
export const EMA_EXTENSION_KEY =
|
||||
"io.modelcontextprotocol/enterprise-managed-authorization";
|
||||
|
||||
/**
|
||||
* The value the client stamps for each advertised extension. The wire shape is
|
||||
* `{ [extensionId]: object }` (per `ClientCapabilities.extensions`); an empty
|
||||
* object is the standard "declared, no sub-options" advertisement.
|
||||
*/
|
||||
export type ExtensionAdvertisement = NonNullable<
|
||||
ClientCapabilities["extensions"]
|
||||
>[string];
|
||||
|
||||
/**
|
||||
* A single Inspector-advertisable extension. The registry of these is the shared
|
||||
* source of truth for both the capability builder here and the Server Settings
|
||||
* toggle UI (#1739), so the two never drift on which extensions exist or what
|
||||
* they are called.
|
||||
*/
|
||||
export interface AdvertisableExtension {
|
||||
/** Extension identifier stamped into `capabilities.extensions`. */
|
||||
key: string;
|
||||
/** Human-readable label for the Server Settings toggle. */
|
||||
label: string;
|
||||
/**
|
||||
* Whether the Inspector advertises this extension when the user has expressed
|
||||
* no explicit preference (the toggle's default position).
|
||||
*/
|
||||
defaultAdvertised: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Catalog of extensions the Inspector can advertise and the user can toggle.
|
||||
* EMA is deliberately absent — it is driven by the auth mode (see
|
||||
* {@link EMA_EXTENSION_KEY}), not a standalone toggle. The `io.modelcontextprotocol/ui`
|
||||
* Apps extension is added here in Phase 3 (#1740).
|
||||
*/
|
||||
export const ADVERTISABLE_EXTENSIONS: readonly AdvertisableExtension[] = [
|
||||
{
|
||||
key: TASKS_EXTENSION_KEY,
|
||||
label: "Tasks (io.modelcontextprotocol/tasks)",
|
||||
// The modern Tasks extension (SEP-2663). Advertised by default so the SDK
|
||||
// stamps it into every modern request envelope — the per-request
|
||||
// declaration a server requires before it may return a `CreateTaskResult`
|
||||
// (server-directed task creation). Harmless on legacy (extensions ignored).
|
||||
defaultAdvertised: true,
|
||||
},
|
||||
];
|
||||
|
||||
export interface BuildClientExtensionsInput {
|
||||
/** True when the connection routes through the enterprise IdP (EMA). */
|
||||
enterpriseManaged: boolean;
|
||||
/**
|
||||
* Per-extension advertise overrides keyed by extension id, from
|
||||
* {@link InspectorClientOptions.advertisedExtensions}. A key present here wins
|
||||
* over the registry's `defaultAdvertised`; an absent key falls back to it.
|
||||
*/
|
||||
advertised?: Record<string, boolean>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Assemble the `capabilities.extensions` map advertised at construction — the
|
||||
* single source of truth that replaces the previously ad-hoc, per-extension
|
||||
* spreads. Registry entries resolve to advertised/not via the user override with
|
||||
* a registry-default fallback; EMA is layered on top as an auth-mode-driven
|
||||
* built-in.
|
||||
*
|
||||
* With the Tasks entry defaulting to advertised, the map is non-empty for a
|
||||
* default config, so `capabilities.extensions` is always attached.
|
||||
*/
|
||||
export function buildClientExtensions(
|
||||
input: BuildClientExtensionsInput,
|
||||
): Record<string, ExtensionAdvertisement> {
|
||||
const map: Record<string, ExtensionAdvertisement> = {};
|
||||
for (const ext of ADVERTISABLE_EXTENSIONS) {
|
||||
const advertised = input.advertised?.[ext.key] ?? ext.defaultAdvertised;
|
||||
if (advertised) {
|
||||
map[ext.key] = {};
|
||||
}
|
||||
}
|
||||
if (input.enterpriseManaged) {
|
||||
map[EMA_EXTENSION_KEY] = {};
|
||||
}
|
||||
return map;
|
||||
}
|
||||
|
|
@ -127,6 +127,7 @@ import {
|
|||
isModernCreateTaskResult,
|
||||
type ModernDetailedTask,
|
||||
} from "./modernTaskSchemas.js";
|
||||
import { buildClientExtensions } from "./extensions.js";
|
||||
import {
|
||||
EmptyResultSchema,
|
||||
CallToolResultSchema,
|
||||
|
|
@ -430,6 +431,9 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
private activeToolCallAbortController?: AbortController;
|
||||
// Receiver tasks (server-initiated: server sends createMessage/elicit with params.task, server polls us)
|
||||
private receiverTasks: boolean;
|
||||
// Per-extension advertise overrides (#1738); undefined key falls back to the
|
||||
// registry default in ADVERTISABLE_EXTENSIONS.
|
||||
private advertisedExtensions?: Record<string, boolean>;
|
||||
private receiverTaskTtlMs: number | (() => number);
|
||||
private receiverTaskRecords: Map<string, ReceiverTaskRecord> = new Map();
|
||||
// OAuth support (config owned by oauthManager; client delegates and uses !!oauthManager for "is OAuth configured")
|
||||
|
|
@ -467,6 +471,7 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
this.sample = options.sample ?? true;
|
||||
this.elicit = options.elicit ?? true;
|
||||
this.receiverTasks = options.receiverTasks ?? false;
|
||||
this.advertisedExtensions = options.advertisedExtensions;
|
||||
this.receiverTaskTtlMs = options.receiverTaskTtlMs ?? 60_000;
|
||||
this.progress = options.progress ?? true;
|
||||
this.resetTimeoutOnProgress = options.resetTimeoutOnProgress ?? true;
|
||||
|
|
@ -594,22 +599,24 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
},
|
||||
};
|
||||
}
|
||||
if (options.oauth?.enterpriseManaged) {
|
||||
// Assemble the advertised-extensions map from one builder (the single
|
||||
// source of truth), instead of ad-hoc per-extension spreads. It layers the
|
||||
// registry defaults (with any user overrides from `advertisedExtensions`)
|
||||
// and the auth-mode-driven EMA extension. The Tasks entry defaults to
|
||||
// advertised, so the map is non-empty and `capabilities.extensions` is
|
||||
// always attached — the modern Tasks extension (SEP-2663) must ride every
|
||||
// modern request envelope for a server to legally return a `CreateTaskResult`
|
||||
// (harmless on legacy, where extensions are ignored). (#1738)
|
||||
const advertisedExtensions = buildClientExtensions({
|
||||
enterpriseManaged: options.oauth?.enterpriseManaged ?? false,
|
||||
advertised: this.advertisedExtensions,
|
||||
});
|
||||
if (Object.keys(advertisedExtensions).length > 0) {
|
||||
capabilities.extensions = {
|
||||
...capabilities.extensions,
|
||||
"io.modelcontextprotocol/enterprise-managed-authorization": {},
|
||||
...advertisedExtensions,
|
||||
};
|
||||
}
|
||||
// Advertise the modern Tasks extension (SEP-2663) so the SDK stamps it into
|
||||
// every modern request's `clientCapabilities` envelope — the per-request
|
||||
// declaration a server requires before it may return a `CreateTaskResult`.
|
||||
// Harmless on legacy (extensions are ignored there). This is what makes
|
||||
// server-directed ("unsolicited") task creation legal on modern, and it
|
||||
// makes `capabilities` always non-empty, so it's always attached.
|
||||
capabilities.extensions = {
|
||||
...capabilities.extensions,
|
||||
[TASKS_EXTENSION_KEY]: {},
|
||||
};
|
||||
clientOptions.capabilities = capabilities;
|
||||
this.clientCapabilities = capabilities;
|
||||
|
||||
|
|
@ -1806,8 +1813,10 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
(params._meta as Record<string, unknown> | undefined) ?? {};
|
||||
const clientCapabilities = {
|
||||
...this.clientCapabilities,
|
||||
// extensions always carries the tasks extension (advertised at
|
||||
// construction), so spreading it is never a no-op.
|
||||
// Force-stamp the tasks extension regardless of what the client
|
||||
// advertised at construction: the raw `tasks/*` channel requires it, and
|
||||
// a user may disable general tasks advertisement via `advertisedExtensions`
|
||||
// (#1738). So this stamp is load-bearing, not a redundant re-add.
|
||||
extensions: {
|
||||
...this.clientCapabilities.extensions,
|
||||
[TASKS_EXTENSION_KEY]: {},
|
||||
|
|
|
|||
|
|
@ -904,6 +904,16 @@ export interface InspectorClientOptions {
|
|||
*/
|
||||
roots?: Root[];
|
||||
|
||||
/**
|
||||
* Per-extension overrides for which extensions the Inspector advertises in
|
||||
* `capabilities.extensions`, keyed by extension id. A present key wins over
|
||||
* the registry default in `ADVERTISABLE_EXTENSIONS`; an absent key falls back
|
||||
* to it. Lets a user toggle advertised extensions as a debugging knob —
|
||||
* servers legitimately change tool registration on client-declared extensions
|
||||
* (#1633). EMA is not configured here (it follows the auth mode). (#1738)
|
||||
*/
|
||||
advertisedExtensions?: Record<string, boolean>;
|
||||
|
||||
/**
|
||||
* Whether to enable listChanged notification handlers (default: true)
|
||||
* If enabled, InspectorClient will subscribe to list_changed notifications and fire
|
||||
|
|
|
|||
Loading…
Reference in a new issue