docs: bring comments and docs in line with -32021 and the Skills registry entry (#2374 review)

Round 2 found text the PR's own changes made stale: the fixture's
option doc, handler summary and repro step still promised -32601 on
modern; the core skills.ts header still said Skills must never be in
ADVERTISABLE_EXTENSIONS; the listSkills and getSkillResult comments
still said the codec validates none of a skills result, or that the
result arrives as the server sent it; and two tests and a docs step
still described Tasks as the only registry extension. The backfill
test also now asserts the Skills declaration is on by default.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q3RsrgCxVoFPokLB4vAkxi
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-09-14 09:07:58 -04:00
commit 2243849711
3 changed files with 49 additions and 28 deletions

View file

@ -5739,11 +5739,13 @@ export class InspectorClient extends InspectorClientEventTarget {
...(cursor !== undefined ? { cursor } : {}),
};
// Era-aware: a modern (2026-07-28+) `skills/list` result also carries the
// base list envelope (`resultType` / `ttlMs` / `cacheScope`). `skills/*` is
// consumer-owned, so the SDK codec validates none of it — without picking
// the schema here a modern server could answer `{ skills: [] }` and the
// conformance UI would show a clean list. Legacy stays permissive: those
// are 2026-era attributes.
// base list envelope (`resultType` / `ttlMs` / `cacheScope`). The two halves
// are checked in different places: the SDK codec checks `resultType` on
// every modern result and removes it before this schema runs (#2373), but
// `skills/*` is consumer-owned, so it checks neither caching attribute —
// without picking the schema here a modern server could answer
// `{ skills: [] }` and the conformance UI would show a clean list. Legacy
// stays permissive: those are 2026-era attributes.
const resultSchema = this.isModernEra()
? ModernListSkillsResultSchema
: ListSkillsResultSchema;
@ -5773,8 +5775,10 @@ export class InspectorClient extends InspectorClientEventTarget {
}
/**
* `skills/get` as the server sent it — the `{ skill }` envelope **and any
* other members it carried**.
* `skills/get` as the SDK decoded it — the `{ skill }` envelope **and any
* other members the server sent**, such as the caching attributes. On a
* modern connection `resultType` is not among them: the codec checks it and
* removes it before the result gets here (#2373).
*
* Separate from {@link getSkill} because the callers differ: the UIs want the
* entry, while the CLI prints the result and must not reshape it. SEP-2640

View file

@ -11,12 +11,15 @@
* shared by every client, and produce a structured finding list rather than a
* boolean — a report is useful, "invalid" is not.
*
* ⚠️ Skills is a **server-declared** extension, read off the connecting server's
* `capabilities.extensions`. It deliberately does NOT belong in
* `ADVERTISABLE_EXTENSIONS` (`core/mcp/extensions.ts`), which is the catalog of
* extensions the *Inspector* advertises and the user toggles in Server Settings.
* The precedent is `appElicitation.ts`, which reads the server side the same
* way; getting it backwards would put a meaningless toggle in Server Settings.
* ⚠️ Skills is negotiated from **both** sides (SEP-2133), and the two halves
* live in different places. This module reads the *server's* declaration off
* the connecting server's `capabilities.extensions` — that is what gates the
* Skills screen. The *Inspector's* own declaration is a separate entry in
* `ADVERTISABLE_EXTENSIONS` (`core/mcp/extensions.ts`): a strict server refuses
* `skills/*` to a client that did not declare it, and turning that toggle off
* in Server Settings is how to check a server's refusal path (#2373). An
* earlier revision of this comment said Skills must never be in that registry;
* that was the missing client declaration #2373 fixed.
*
* The Inspector is an inspector, not a host: a `resources/read` of a `SKILL.md`
* is explicitly not a load and confers no standing, so none of the SEP's host

View file

@ -181,7 +181,10 @@ export const GetSkillEnvelopeSchema = z.looseObject({
});
/**
* The `skills/get` result as the server sent it, envelope and all.
* The `skills/get` result envelope as the SDK decoded it: the `skill` wrapper
* plus any caching attributes the server sent. On a modern connection
* `resultType` is not in it — the codec checks it and removes it before this
* schema runs (#2373).
*
* Exported alongside the unwrapping schema below because the two callers want
* different things: the UIs want the entry, while the CLI's job is to print