Skip to content

Commit db6052e

Browse files
committed
dropped --pr arg, list changed files, support multi-file seps w url override
1 parent 99f951b commit db6052e

4 files changed

Lines changed: 64 additions & 52 deletions

File tree

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

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ description: >-
55
conformance repo from a SEP PR's spec diff. Runs the new-sep CLI, then
66
parses the modelcontextprotocol/modelcontextprotocol spec diff to populate
77
`requirements[]` with the RFC 2119 sentences and proposed check IDs.
8-
argument-hint: '<sep-number> [--pr <num>] [--target client|server|authorization-server]'
8+
argument-hint: '<sep-number> [--target client|server|authorization-server]'
99
---
1010

1111
# new-sep: SEP traceability YAML scaffolding
@@ -36,8 +36,7 @@ The name should be `@modelcontextprotocol/conformance`. If not, stop and ask the
3636

3737
Extract from the user's input:
3838

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.
39+
- **sep-number** (required): the SEP number, e.g. `2164`. This is also the PR number in `modelcontextprotocol/modelcontextprotocol` by convention.
4140
- **--target client|server|authorization-server** (optional): which scenarios subdirectory to write to. Inferred from the spec path if omitted.
4241

4342
## Step 2: Generate the skeleton
@@ -46,22 +45,21 @@ Run the CLI:
4645

4746
```bash
4847
npm run --silent build
49-
node dist/index.js new-sep <NNNN> [--pr <num>] [--target <target>]
48+
node dist/index.js new-sep <NNNN> [--target <target>]
5049
```
5150

5251
(For development against a non-built source tree: `npx tsx src/index.ts new-sep ...`.)
5352

5453
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`.
5554

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.
55+
If the CLI errors with "does not change any docs/specification/draft/\*.mdx", the SEP's spec changes landed in a separate PR — ask the user for the spec file path and rerun with `--spec-path docs/specification/draft/<path>`. Do not guess.
5756

5857
## Step 3: Fetch the spec diff
5958

6059
`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:
6160

6261
```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" \
62+
gh api "repos/modelcontextprotocol/modelcontextprotocol/pulls/<NNNN>/files" \
6563
--jq '.[] | select(.filename | test("^docs/specification/draft/.*\\.mdx$")) | {filename, patch}'
6664
```
6765

@@ -110,6 +108,16 @@ If a requirement is ambiguous or you're not confident, leave it as a `TODO:` row
110108

111109
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.
112110

111+
If a requirement comes from a **different spec page** than `spec_url` (the SEP touched multiple `.mdx` files — the CLI prints these as "PR also changes N other spec file(s)"), give that row a full `url:` override:
112+
113+
```yaml
114+
- text: '...'
115+
check: sep-NNNN-slug
116+
url: https://modelcontextprotocol.io/specification/draft/other/page#anchor
117+
```
118+
119+
A row's effective spec reference is `row.url ?? file.spec_url`.
120+
113121
Write the result back to `$YAML`.
114122

115123
## Step 7: Hand-off

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ Scaffold the requirement-traceability YAML with:
7979
npx @modelcontextprotocol/conformance new-sep <NNNN>
8080
```
8181

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.
82+
The command looks up PR #`<NNNN>` in `modelcontextprotocol/modelcontextprotocol` (SEP numbers are PR numbers), 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-path` or `--spec-url` to skip the lookup. The `new-sep` Claude Code skill drives the same flow end-to-end, parses the spec diff, and fills in the requirement rows.
8383

8484
## Examples: prove it passes and fails
8585

src/new-sep/index.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,32 @@ requirements:
109109
);
110110
});
111111

112+
it('emits per-row url: overrides', () => {
113+
const out = renderYaml({
114+
sep: 1234,
115+
specUrl: 'https://modelcontextprotocol.io/specification/draft/server/a',
116+
requirements: [
117+
{ text: 'from primary file', check: 'sep-1234-a' },
118+
{
119+
text: 'from secondary file',
120+
check: 'sep-1234-b',
121+
url: 'https://modelcontextprotocol.io/specification/draft/server/b#x'
122+
}
123+
]
124+
});
125+
expect(out).toBe(
126+
`sep: 1234
127+
spec_url: https://modelcontextprotocol.io/specification/draft/server/a
128+
requirements:
129+
- text: 'from primary file'
130+
check: sep-1234-a
131+
- text: 'from secondary file'
132+
check: sep-1234-b
133+
url: https://modelcontextprotocol.io/specification/draft/server/b#x
134+
`
135+
);
136+
});
137+
112138
it('escapes single quotes by doubling them', () => {
113139
const out = renderYaml({
114140
sep: 1,

src/new-sep/index.ts

Lines changed: 22 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ export interface RequirementRow {
1010
check?: string;
1111
excluded?: string;
1212
issue?: string;
13+
/** Full spec URL for this requirement; overrides the file-level spec_url. */
14+
url?: string;
1315
}
1416

1517
const TARGET_DIRS: Record<Target, string> = {
@@ -81,6 +83,7 @@ export function renderYaml(input: {
8183
lines.push(` excluded: '${escapeSingleQuoted(r.excluded)}'`);
8284
}
8385
if (r.issue) lines.push(` issue: ${r.issue}`);
86+
if (r.url) lines.push(` url: ${r.url}`);
8487
}
8588
return lines.join('\n') + '\n';
8689
}
@@ -105,38 +108,17 @@ interface SpecCandidate {
105108

106109
async function lookupSpecPath(args: {
107110
sep: number;
108-
pr?: number;
109111
repo: string;
110112
token: string;
111-
}): Promise<string> {
113+
}): Promise<string[]> {
112114
const [owner, repoName] = args.repo.split('/');
113115
if (!owner || !repoName) {
114116
throw new Error(`Invalid --repo: ${args.repo} (expected owner/repo)`);
115117
}
116118
const octokit = new Octokit({ auth: args.token });
117119

118-
let prNumber = args.pr;
119-
if (!prNumber) {
120-
const q = `repo:${args.repo} type:pr SEP-${args.sep} in:title`;
121-
const res = await octokit.search.issuesAndPullRequests({ q });
122-
if (res.data.total_count === 0) {
123-
throw new Error(
124-
`No PRs in ${args.repo} matching "SEP-${args.sep}" in title. ` +
125-
`Pass --pr <num> to disambiguate.`
126-
);
127-
}
128-
if (res.data.total_count > 1) {
129-
const candidates = res.data.items
130-
.slice(0, 5)
131-
.map((i) => ` #${i.number} ${i.title}`)
132-
.join('\n');
133-
throw new Error(
134-
`Multiple PRs in ${args.repo} match "SEP-${args.sep}":\n${candidates}\n` +
135-
`Pass --pr <num> to pick one.`
136-
);
137-
}
138-
prNumber = res.data.items[0].number;
139-
}
120+
// SEP numbers are PR numbers in the spec repo by convention.
121+
const prNumber = args.sep;
140122

141123
const files = await octokit.paginate(octokit.pulls.listFiles, {
142124
owner,
@@ -158,24 +140,8 @@ async function lookupSpecPath(args: {
158140
`${SPEC_PATH_PREFIX}*.mdx file. Pass --spec-path <path> to override.`
159141
);
160142
}
161-
if (candidates.length === 1) {
162-
return candidates[0].filename;
163-
}
164143
candidates.sort((a, b) => b.additions - a.additions);
165-
if (candidates[0].additions > candidates[1].additions) {
166-
console.error(
167-
`Multiple spec files changed; picking ${candidates[0].filename} ` +
168-
`(most additions).`
169-
);
170-
return candidates[0].filename;
171-
}
172-
const list = candidates
173-
.map((c) => ` ${c.filename} (+${c.additions})`)
174-
.join('\n');
175-
throw new Error(
176-
`Multiple spec files changed with equal weight in PR #${prNumber}:\n${list}\n` +
177-
`Pass --spec-path <path> to pick one.`
178-
);
144+
return candidates.map((c) => c.filename);
179145
}
180146

181147
export function createNewSepCommand(): Command {
@@ -196,7 +162,6 @@ export function createNewSepCommand(): Command {
196162
'--spec-path <path>',
197163
`${SPEC_PATH_PREFIX}... path to derive spec_url from (skips GitHub lookup)`
198164
)
199-
.option('--pr <num>', 'PR number in the spec repo (skips title search)')
200165
.option(
201166
'--repo <owner/repo>',
202167
'Spec repo to query for the SEP PR',
@@ -228,6 +193,7 @@ export function createNewSepCommand(): Command {
228193

229194
let specUrl: string | undefined = options.specUrl;
230195
let specPath: string | undefined = options.specPath;
196+
let otherSpecPaths: string[] = [];
231197

232198
if (!specUrl && !specPath) {
233199
const token = await resolveToken(options.token);
@@ -242,12 +208,13 @@ export function createNewSepCommand(): Command {
242208
process.exit(1);
243209
}
244210
try {
245-
specPath = await lookupSpecPath({
211+
const specPaths = await lookupSpecPath({
246212
sep,
247-
pr: options.pr ? parseInt(options.pr, 10) : undefined,
248213
repo: options.repo,
249214
token
250215
});
216+
specPath = specPaths[0];
217+
otherSpecPaths = specPaths.slice(1);
251218
console.error(`Resolved spec path: ${specPath}`);
252219
} catch (err) {
253220
console.error((err as Error).message);
@@ -306,6 +273,17 @@ export function createNewSepCommand(): Command {
306273
await fs.writeFile(outPath, yaml, 'utf-8');
307274

308275
console.error(`Wrote ${outPath}`);
276+
if (otherSpecPaths.length > 0) {
277+
console.error(
278+
`Note: PR also changes ${otherSpecPaths.length} other spec file(s):`
279+
);
280+
for (const p of otherSpecPaths) {
281+
console.error(` ${specPathToUrl(p)}`);
282+
}
283+
console.error(
284+
` Use a per-row "url:" for requirements from those files.`
285+
);
286+
}
309287
console.error('Next steps:');
310288
console.error(
311289
' 1. Edit the file to quote real normative sentences from the spec diff'

0 commit comments

Comments
 (0)