Skip to content

chore(sep-2640): re-extract against the Final text - #511

Open
panyam wants to merge 3 commits into
modelcontextprotocol:mainfrom
panyam:chore/sep-2640-final-refresh
Open

panyam wants to merge 3 commits into
modelcontextprotocol:mainfrom
panyam:chore/sep-2640-final-refresh

Conversation

@panyam

@panyam panyam commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

What changes

Re-extracts src/seps/sep-2640.yaml against the SEP at Final. The file was extracted at a3e147ca (2026-08-25) while the CM vote was still running, and its own header says to expect one more pass at the CM-stamped head. This is that pass.

One normative addition, two rows restated after a section was reworded, twelve quotes re-synced after a copy-editing pass, and the header's counts recomputed. No check ID moves, so no scenario changes.

Prerequisite knowledge

  • AGENTS.md § Traceability manifest — what a check: row is for and why traceability.json is generated rather than edited.
  • SEP-2640 at Final, particularly Skill Format and Capability Declaration, which are the two sections that moved.
  • src/seps/sep-2640.yaml's own provenance header — it records why the file held at an older revision and what the previous re-extraction changed.

Reviewer's guide

A requirements yaml is a set of bookmarks pressed into a document. Each row quotes a sentence and hangs a check ID on it, and the checks are what scenarios emit. Move the sentence and the bookmark still points at a page, so nothing breaks and nothing complains; it just quotes words that are no longer there. That is most of this diff. A copy-editing pass swapped em-dashes for commas and parentheses across the SEP, and twelve rows kept the old punctuation, which makes them silently unquotable against the published text. Nothing in CI compares a row's text: to the spec, so this drift is invisible until somebody reads both side by side. The part that is not bookkeeping is one new paragraph in Skill Format, which adds a real client obligation that no row covered.

Read in this order:

  1. src/seps/sep-2640.yaml, the header — start here. It records what moved between a3e147ca and f56f204f, and which of it was normative.
  2. src/seps/sep-2640.yaml, the two new excluded rows under # === Excluded: not observable on the MCP wire === — the only new requirement. Worth arguing about; see the decision log.
  3. src/seps/sep-2640.yaml, sep-2640-capability-declaration-inline and sep-2640-capability-requires-resources — restated from the reworded Capability Declaration section. Same IDs, same scenarios.
  4. Everything else in the diff is re-punctuation of quoted sentences. The bookmarks now match the page again.

Decision log

  • Both halves of the new Backward Compatibility requirement are excluded: rather than declared checks. They condition on the Agent Skills specification shipping a breaking change, which has not happened. Until it does there is nothing for a harness to present and nothing for a client to honor, and a client that would ignore a compatibility mechanism is indistinguishable from one that honors it. Declaring them keeps the requirement tracked rather than lost. If a reviewer would rather see them as untested checks than as excluded rows, that is a reasonable different call and an easy change.
  • Re-synced the twelve drifted quotes rather than leaving them. They changed only in punctuation, so no meaning moved and no reviewer is misled today. Left alone they rot further with each editing pass, and the file's value is that its rows are quotable.
  • Did not touch src/seps/traceability.json. Per AGENTS.md it is generated against the reference SDK and refreshed by the traceability workflow. Regenerating it locally against a single suite's results drops every other SEP's tested status, which is exactly what happened on the first attempt here before it was reverted.
  • Did not add coverage for the 48 untested rows. They are host obligations that need a host-side scenario set, which is a separate piece of work.
  • Corrected the header's counts rather than carrying them forward. They were already wrong before this pass: the file said 89 checks and 7 excluded rows when it held 90 and 8, and said the scenarios emit 40 when they emit 42. Nothing recomputes them, so the header now says so and carries a date.

Risk / blast radius

  • Affects: requirement rows and provenance only. No scenario, no runner, no check ID.
  • Tests covering this: npm run lint, npm run typecheck, and the three server scenarios run against a second implementation (mcpkit examples/skills), unchanged at 45 pass / 0 fail / 0 warn / 0 skip. A traceability run reports 48 untested, which is what the recomputed header claims.
  • Verified the suite still grades something. A green suite is not proof it graded the implementation under test, so the fixture was broken deliberately (name: in a SKILL.md no longer matching its URI's final segment) and the suite went red, then green again on restore.
  • Be paranoid about: whether the two new rows should be excluded or untested checks. That is the only judgment call in the diff; the rest is mechanical and verifiable against the published SEP.

Before / after

Rows quoting text the Final SEP no longer contains, counted by substring match against seps/2640-skills-extension.md at f56f204f:

before:  12 rows quote sentences absent from the Final text
after:    0

Header counts, before and after, against a traceability run:

header said actual now says
declared checks 89 90 90
excluded rows 7 8, now 10 10
emitted by server scenarios 40 42 42
untested 49 48 48

Suite against mcpkit examples/skills, before and after this change:

testconf-skills: 45 pass / 0 fail / 0 warn / 0 skip across 3 scenarios

And with the fixture deliberately broken, confirming the suite grades the implementation rather than passing by default:

make[1]: *** [Makefile:89: testconf-skills] Error 1

Out of scope

  • Host-side scenarios for the 48 untested rows.
  • The five client-side skills scenarios this repo already registers, which are unaffected.

Refs #513.

The header said to expect one more pass at the CM-stamped head. This is it.
SEP-2640 was marked Accepted on 2026-09-03 (d6b31a03) and Final on 2026-09-11
(8075db4a), and the source moved off the sep/skills-extension branch to main.

Four commits touched the text after this file's extraction point, one of them
normative. The Skill Format section gained a paragraph binding clients to the
Agent Skills specification's own backward-compatibility mechanisms; it is
declared here as two excluded rows, since both halves condition on a breaking
revision of an external spec that does not yet exist, so there is nothing for
a harness to present and nothing for a client to honor.

The Capability Declaration section was reworded to locate the declaration in
the `extensions` field of the server's capabilities rather than loosely in the
`initialize` response, and now states the `resources`-capability requirement
outright instead of leaving it to be derived from Dependencies. Both rows are
restated from the Final text; neither check ID moves, so no scenario changes.

A copy-editing pass (582d814a) swapped em-dashes for commas, colons and
parentheses throughout, which left twelve rows quoting sentences the SEP no
longer contains. Those quotes are re-synced. No requirement changed meaning.

The header's own counts were wrong before this pass and are recomputed: 90
checks and 10 excluded rows, of which the three server scenarios emit 42,
confirmed against a traceability run rather than asserted.

Suite unchanged at 45 pass / 0 fail / 0 warn / 0 skip.
@pkg-pr-new

pkg-pr-new Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

npx https://pkg.pr.new/@modelcontextprotocol/conformance@511

commit: 4b6ff1a

@dfedoryshchev

Copy link
Copy Markdown

Checked the yaml against seps/2640-skills-extension.md at f56f204f.

Nine rows still quote wording that is not in the Final text.

For example, host-nested-fresh-consent quotes:

Activating a nested skill - loading it as a skill in its own right - requires...

while the Final reads:

Activating a nested skill (loading it as a skill in its own right, whether through the host's skill-loading machinery or by giving effect to its frontmatter) requires...

The others:

  • nested-publication-flat
  • skills-list-cache-attributes
  • entry-frontmatter-required
  • host-skill-identity-pair
  • host-content-bound-approval
  • intermediary-meta-prefix
  • sdk-convenience-wrappers
  • directory-read-not-manifest-extension

The drift check from #513 only flags rows that were exact quotes of the old revision, and these were already trimmed there, so it skips them.

skills-get-cache-attributes quotes a source the header does not name.

It quotes "ttlMs and cacheScope are REQUIRED" from the ext-skills stable page (modelcontextprotocol/ext-skills#139). The Final SEP says this "is left open", and the header names the SEP as the only source.

Minor: the resources-capability MUST is new text in fd72d3f1, not derived from Dependencies, so the header undercounts the normative changes.

…nst the Final text

Addresses the review on modelcontextprotocol#511 (2026-10-08).

- Nine rows trimmed in the first pass still carried pre-copy-edit wording
  (em-dash asides the Final text turned into commas, colons and
  parentheses). modelcontextprotocol#513's drift check only catches whole-sentence quotes, so
  it could not see them. All nine now quote f56f204f verbatim.
- A sentence-by-sentence check against f56f204f found four more rows that
  paraphrased rather than quoted: host-origin-tag-visible,
  host-cache-write-isolation, host-cache-excluded-from-discovery and
  host-ignore-allowed-tools. The last had also dropped the "unless the user
  has explicitly approved that grant" exception, which changed its meaning.
- capability-declaration-inline and host-frontmatter-comparison now quote
  the Final sentence instead of a gloss on it.
- skills-get-cache-attributes quotes the ext-skills stable page, because
  the SEP leaves that question open. The header now names that second
  source, and the row's url points at the stable page.
- The header now counts fd72d3f1 as normative: it added the
  resources-capability MUST as new text, not as a restatement of
  Dependencies.

mcpkit examples/skills: 45 pass / 0 fail on this branch.
@panyam

panyam commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @dfedoryshchev, good catch.
You're right that #513's check couldn't see these, it only flags rows that were whole-sentence quotes of the old revision and these had been trimmed.
I re-quoted all nine against f56f204f in 4b6ff1a.
I also did a sentence-by-sentence pass and found four more that paraphrased rather than quoted: host-origin-tag-visible, host-cache-write-isolation, host-cache-excluded-from-discovery and host-ignore-allowed-tools.
host-ignore-allowed-tools had also lost the "unless the user has explicitly approved that grant" exception, so it was stricter than the SEP.
For skills-get-cache-attributes I kept the row, but the header now names the ext-skills stable page as a second source and the row's url points there, since the SEP leaves it open and ext-skills#139 closed it on stable.
I wonder if longer term this file should extract from skills.mdx instead, since that's what the WG maintains now and ext-skills#157 is about to move it again.
Also counted fd72d3f1 as normative in the header, you're right it's new text and not derived from Dependencies.
mcpkit is still 45/0 on the branch.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants