Skip to content

Commit 99f951b

Browse files
committed
feat: add new-sep scaffolding command
Related #243. This commit adds the new-sep <NNNN> subcommand, which writes the file src/scenarios/<target>/sep-<NNNN>.yaml. This file contains the "sep", "spec_url", and "requirements" rows in the same style as sep-2164.yaml, with single quotes and two spaces. The spec source resolves in order: --spec-url → --spec-path → GitHub API against modelcontextprotocol/modelcontextprotocol (explicit --pr or title search for "SEP-NNNN"). The target directory is inferred from the spec path (`server/`, `client/`, or `basic/authorization*`); the --target flag overrides this. It refuses to overwrite without the --force flag. Token resolution reuses the "gh auth token" fallback pattern from tier-check. It also ships the .claude/skills/new-sep directory, which drives the CLI. It fetches the spec diff via "gh api" and extracts RFC 2119 sentences. There is a warning against regex-only matching. Bullets inherit keywords from their lead-in sentence. It rewrites the placeholder rows per the severity rules in AGENTS.md. MUST/SHALL/REQUIRED are checked for FAILURE. SHOULD is checked for WARNING. MAY/OPTIONAL are excluded. Signed-off-by: Satoshi Ito <satoshi.ito.tf@hitachi.com>
1 parent 17f1f93 commit 99f951b

5 files changed

Lines changed: 582 additions & 0 deletions

File tree

.claude/skills/new-sep/SKILL.md

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
---
2+
name: new-sep
3+
description: >-
4+
Scaffold a sep-NNNN.yaml requirement-traceability file for the MCP
5+
conformance repo from a SEP PR's spec diff. Runs the new-sep CLI, then
6+
parses the modelcontextprotocol/modelcontextprotocol spec diff to populate
7+
`requirements[]` with the RFC 2119 sentences and proposed check IDs.
8+
argument-hint: '<sep-number> [--pr <num>] [--target client|server|authorization-server]'
9+
---
10+
11+
# new-sep: SEP traceability YAML scaffolding
12+
13+
You are bootstrapping a `sep-NNNN.yaml` file for a new SEP in the MCP conformance repo. The output is the requirement-traceability file specified by SEP-2484: a YAML that maps each normative sentence from the SEP's spec diff to a `check:` ID (testable) or an `excluded:` reason (not testable). The CLI gets the skeleton; you fill in the rows by reading the spec diff.
14+
15+
## Step 0: Pre-flight checks
16+
17+
Before doing anything else, verify GitHub CLI authentication:
18+
19+
```bash
20+
gh auth status 2>&1
21+
```
22+
23+
If this fails, stop immediately and tell the user:
24+
25+
> GitHub authentication is required for this skill. Please run `gh auth login` first, then re-run.
26+
27+
Verify you're running inside the conformance repo:
28+
29+
```bash
30+
test -f package.json && jq -r '.name' package.json
31+
```
32+
33+
The name should be `@modelcontextprotocol/conformance`. If not, stop and ask the user to `cd` into the conformance repo first.
34+
35+
## Step 1: Parse arguments
36+
37+
Extract from the user's input:
38+
39+
- **sep-number** (required): the SEP number, e.g. `2164`.
40+
- **--pr <num>** (optional): the PR number in `modelcontextprotocol/modelcontextprotocol`. If omitted, the CLI searches for a PR titled `SEP-<NNNN>` and fails loudly on 0 or >1 hits.
41+
- **--target client|server|authorization-server** (optional): which scenarios subdirectory to write to. Inferred from the spec path if omitted.
42+
43+
## Step 2: Generate the skeleton
44+
45+
Run the CLI:
46+
47+
```bash
48+
npm run --silent build
49+
node dist/index.js new-sep <NNNN> [--pr <num>] [--target <target>]
50+
```
51+
52+
(For development against a non-built source tree: `npx tsx src/index.ts new-sep ...`.)
53+
54+
The CLI writes `src/scenarios/<target>/sep-<NNNN>.yaml` with `sep`, `spec_url`, and two TODO `requirements[]` rows. Capture the output path from the CLI's `Wrote …` line and remember it as `$YAML`.
55+
56+
If the CLI errors with "No PRs match" or "Multiple PRs match", read the message, ask the user for the right `--pr <num>`, and rerun. Do not guess.
57+
58+
## Step 3: Fetch the spec diff
59+
60+
`AGENTS.md` (lines 64–72) is explicit that severity must come from the spec text itself, not the SEP markdown or the conformance PR description:
61+
62+
```bash
63+
PR=$(node dist/index.js new-sep <NNNN> --help >/dev/null 2>&1; echo <pr-from-step-2>)
64+
gh api "repos/modelcontextprotocol/modelcontextprotocol/pulls/$PR/files" \
65+
--jq '.[] | select(.filename | test("^docs/specification/draft/.*\\.mdx$")) | {filename, patch}'
66+
```
67+
68+
For each file, pull the added (`+`-prefixed) lines from `patch`. If `patch` is truncated for a large file, fall back to fetching the whole file at the PR's head ref:
69+
70+
```bash
71+
gh api "repos/modelcontextprotocol/modelcontextprotocol/contents/<path>?ref=<sep-branch>" \
72+
--jq '.content' | base64 -d
73+
```
74+
75+
## Step 4: Extract RFC 2119 requirements
76+
77+
Walk the added lines and identify sentences containing the keywords: **MUST**, **MUST NOT**, **SHOULD**, **SHOULD NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **MAY**, **OPTIONAL**.
78+
79+
**Quote the whole sentence**, not just the matched line. The matched word may sit inside a bullet point whose lead-in sentence supplies the keyword by inheritance — e.g.:
80+
81+
> Servers SHOULD return standard JSON-RPC errors for common failure cases:
82+
>
83+
> - Resource not found: -32602 (Invalid Params)
84+
85+
The bullet inherits `SHOULD`. The yaml row should quote the _combined_ obligation: `'Servers SHOULD return standard JSON-RPC errors for common failure cases: Resource not found: -32602 (Invalid Params)'` — see `src/scenarios/server/sep-2164.yaml` for the canonical example.
86+
87+
**Regex alone is insufficient** (this is called out in Issue #243). Read for context: pronouns, "the server", and "such cases" all refer back to the lead-in.
88+
89+
## Step 5: Map severity → check vs. excluded
90+
91+
From `AGENTS.md:50-56`:
92+
93+
| Keyword | Severity | YAML field |
94+
| ---------------------------------------------- | ------------------------- | -------------------------- |
95+
| MUST / MUST NOT / SHALL / SHALL NOT / REQUIRED | FAILURE | `check: sep-<NNNN>-<slug>` |
96+
| SHOULD / SHOULD NOT | WARNING | `check: sep-<NNNN>-<slug>` |
97+
| MAY / OPTIONAL | (not enforced as a check) | `excluded: '<reason>'` |
98+
99+
If a requirement is testable in principle but you can't see how to drive it from the harness, write a `check:` row anyway and leave it for the human to wire up — do **not** silently demote to `excluded:`.
100+
101+
Use `excluded:` only when the requirement genuinely can't be protocol-observed (e.g. "clients SHOULD also accept -32002" — the conformance harness tests servers, so client-side acceptance is not observable here). When you use `excluded:`, write the reason verbatim and add an `issue:` URL if there's a tracking issue.
102+
103+
Slug convention: lowercase-kebab, derived from the verb phrase. Examples from `sep-2164.yaml`: `no-empty-contents`, `error-code`. Same `id` is used for SUCCESS and FAILURE (`AGENTS.md:52`).
104+
105+
## Step 6: Rewrite the YAML
106+
107+
Replace the two TODO rows the CLI generated with one row per extracted requirement. Preserve the CLI's quoting style (single quotes, two-space indent — see `src/scenarios/server/sep-2164.yaml`).
108+
109+
If a requirement is ambiguous or you're not confident, leave it as a `TODO:` row rather than guessing — humans review this yaml before scenarios get written.
110+
111+
Also fix the `spec_url`: the CLI emits the page URL with no anchor. If the requirements you extracted live under a specific spec subsection (e.g. `#error-handling`), append it.
112+
113+
Write the result back to `$YAML`.
114+
115+
## Step 7: Hand-off
116+
117+
Report to the user, in this order:
118+
119+
1. Path to the generated yaml.
120+
2. Number of rows extracted (e.g. "3 `check:` rows, 1 `excluded:` row").
121+
3. Any requirements you marked TODO and why.
122+
4. Reminder of the next steps the user still owns:
123+
- implement the TypeScript scenario under `src/scenarios/<target>/`,
124+
- register it in the appropriate suite list in `src/scenarios/index.ts` (`AGENTS.md:48`),
125+
- add a passing example to the everything-client/server and a negative test, per `AGENTS.md:74-81`.
126+
127+
Do **not** generate the scenario `.ts` file or touch `src/scenarios/index.ts`. The skill's scope ends at the yaml.

AGENTS.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,16 @@ Verify requirement levels against the SEP's **spec diff** — the change to `doc
7171
gh api "repos/modelcontextprotocol/modelcontextprotocol/contents/docs/specification/draft/<path>?ref=<sep-branch>" --jq '.content' | base64 -d
7272
```
7373

74+
### Adding a new SEP
75+
76+
Scaffold the requirement-traceability YAML with:
77+
78+
```sh
79+
npx @modelcontextprotocol/conformance new-sep <NNNN>
80+
```
81+
82+
The command searches `modelcontextprotocol/modelcontextprotocol` for a PR titled `SEP-<NNNN>`, derives `spec_url` from the `docs/specification/draft/*.mdx` file it changes, picks `src/scenarios/{client,server,authorization-server}/` from the spec path, and writes `sep-<NNNN>.yaml` with TODO `requirements[]` rows. Use `--spec-url`, `--spec-path`, or `--pr` to override the lookup when title search is ambiguous. The `new-sep` Claude Code skill drives the same flow end-to-end, parses the spec diff, and fills in the requirement rows.
83+
7484
## Examples: prove it passes and fails
7585

7686
A new scenario should come with:

src/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import {
4545
printBaselineResults
4646
} from './expected-failures';
4747
import { createTierCheckCommand } from './tier-check';
48+
import { createNewSepCommand } from './new-sep';
4849
import packageJson from '../package.json';
4950

5051
// Note on naming: `command` refers to which CLI command is calling this.
@@ -540,6 +541,9 @@ program
540541
// Tier check command
541542
program.addCommand(createTierCheckCommand());
542543

544+
// New SEP scaffolding command
545+
program.addCommand(createNewSepCommand());
546+
543547
// List scenarios command
544548
program
545549
.command('list')

src/new-sep/index.test.ts

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
import { describe, it, expect } from 'vitest';
2+
import { specPathToUrl, inferTarget, renderYaml } from './index';
3+
4+
describe('specPathToUrl', () => {
5+
it('strips the docs/specification/draft/ prefix and .mdx suffix', () => {
6+
expect(specPathToUrl('docs/specification/draft/server/resources.mdx')).toBe(
7+
'https://modelcontextprotocol.io/specification/draft/server/resources'
8+
);
9+
});
10+
11+
it('handles nested paths', () => {
12+
expect(specPathToUrl('docs/specification/draft/basic/lifecycle.mdx')).toBe(
13+
'https://modelcontextprotocol.io/specification/draft/basic/lifecycle'
14+
);
15+
});
16+
17+
it('rejects paths outside docs/specification/draft/', () => {
18+
expect(() =>
19+
specPathToUrl('docs/specification/2025-11-25/server/x.mdx')
20+
).toThrow(/must start with/);
21+
});
22+
});
23+
24+
describe('inferTarget', () => {
25+
it('returns server for server/ paths', () => {
26+
expect(
27+
inferTarget('docs/specification/draft/server/resources.mdx')
28+
).toEqual({ target: 'server', inferred: false });
29+
});
30+
31+
it('returns client for client/ paths', () => {
32+
expect(inferTarget('docs/specification/draft/client/sampling.mdx')).toEqual(
33+
{ target: 'client', inferred: false }
34+
);
35+
});
36+
37+
it('returns authorization-server for basic/authorization* paths', () => {
38+
expect(
39+
inferTarget('docs/specification/draft/basic/authorization.mdx')
40+
).toEqual({ target: 'authorization-server', inferred: false });
41+
});
42+
43+
it('falls back to server (inferred) for unrecognized paths', () => {
44+
expect(inferTarget('docs/specification/draft/basic/lifecycle.mdx')).toEqual(
45+
{ target: 'server', inferred: true }
46+
);
47+
});
48+
49+
it('accepts paths already stripped of the prefix', () => {
50+
expect(inferTarget('client/elicitation.mdx')).toEqual({
51+
target: 'client',
52+
inferred: false
53+
});
54+
});
55+
});
56+
57+
describe('renderYaml', () => {
58+
it('emits placeholder yaml in the sep-2164.yaml style', () => {
59+
const out = renderYaml({
60+
sep: 9999,
61+
specUrl:
62+
'https://modelcontextprotocol.io/specification/draft/server/resources'
63+
});
64+
expect(out).toBe(
65+
`sep: 9999
66+
spec_url: https://modelcontextprotocol.io/specification/draft/server/resources
67+
requirements:
68+
- text: 'TODO: quote the normative sentence from the spec diff'
69+
check: sep-9999-todo
70+
- text: 'TODO: requirement that cannot be tested'
71+
excluded: 'TODO: reason'
72+
issue: https://github.com/modelcontextprotocol/conformance/issues/<NNNN>
73+
`
74+
);
75+
});
76+
77+
it('matches the byte-shape of the real sep-2164.yaml when given its rows', () => {
78+
const out = renderYaml({
79+
sep: 2164,
80+
specUrl:
81+
'https://modelcontextprotocol.io/specification/draft/server/resources#error-handling',
82+
requirements: [
83+
{
84+
text: 'Servers MUST NOT return an empty contents array for a non-existent resource',
85+
check: 'sep-2164-no-empty-contents'
86+
},
87+
{
88+
text: 'Servers SHOULD return standard JSON-RPC errors for common failure cases: Resource not found: -32602 (Invalid Params)',
89+
check: 'sep-2164-error-code'
90+
},
91+
{
92+
text: 'clients SHOULD also accept -32002 as a resource not found error',
93+
excluded:
94+
'Client-side error handling is implementation-defined; not protocol-observable'
95+
}
96+
]
97+
});
98+
expect(out).toBe(
99+
`sep: 2164
100+
spec_url: https://modelcontextprotocol.io/specification/draft/server/resources#error-handling
101+
requirements:
102+
- text: 'Servers MUST NOT return an empty contents array for a non-existent resource'
103+
check: sep-2164-no-empty-contents
104+
- text: 'Servers SHOULD return standard JSON-RPC errors for common failure cases: Resource not found: -32602 (Invalid Params)'
105+
check: sep-2164-error-code
106+
- text: 'clients SHOULD also accept -32002 as a resource not found error'
107+
excluded: 'Client-side error handling is implementation-defined; not protocol-observable'
108+
`
109+
);
110+
});
111+
112+
it('escapes single quotes by doubling them', () => {
113+
const out = renderYaml({
114+
sep: 1,
115+
specUrl: 'https://example.com/x',
116+
requirements: [{ text: "can't happen", check: 'sep-1-x' }]
117+
});
118+
expect(out).toContain("text: 'can''t happen'");
119+
});
120+
});

0 commit comments

Comments
 (0)