Fix tools-name-format for spec Tool Names SHOULD rules on 2025-11-25+ - #380
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for the thorough fix and the detailed test plan! I verified the substance against the spec source: the validation rules (1–128 chars, ^[A-Za-z0-9_.-]+$), WARNING severity for the SHOULD-level rules, and the version gating to 2025-11-25+ all match the dated spec prose, and all 336 tests pass locally.
One blocking item: npm run check (typecheck via tsgo --noEmit) fails with a TS2322 in tools.test.ts — one-click suggestion attached inline, verified locally (typecheck clean, tests still pass). The other two comments are optional nits — take or leave.
commit: |
0fd7828 to
2d083ab
Compare
Enforce dated spec Tool Names SHOULD rules (128 max, no slash), WARNING severity, 2025-11-25+ gate, negative fixture, and core-spec specReferences. Fixes modelcontextprotocol#379
Document SEP-986 markdown vs integrated spec divergence in fixture header.
2d083ab to
f1e4395
Compare
@claude - Accepted all 3 changes and pushed after another local test run. |
Keep the rule statement and the note that the dated spec (not the SEP-986 markdown) is authoritative; drop the process narration. Co-Authored-By: Claude <noreply@anthropic.com>
|
Pushed a small commit trimming the explanatory comments down to the rule + the "dated spec is authoritative" not, no logic changes. Merging; thanks for the fix and the fixture. |
pcarleton
left a comment
pcarleton
left a comment
There was a problem hiding this comment.
Verified against the 2025-11-25 tool-names prose (SHOULD, 1–128, no slash) — main had the stale SEP-986 numbers. Thanks for the fixture and the version gate.
Resolve conflicts with modelcontextprotocol#380 (tools-name-format): keep both the Tool Names description line and the deterministic-order line in tools-list, both test imports, and both negative-test describe blocks.
2 participants
Closes #379. Completes the gaps left by #238 / #240.
Summary
tools-name-formatontools-listto enforce 2025-11-25 / draft spec prose (1–128 chars,[A-Za-z0-9_.-]only), not stale SEP-986 markdown (64 chars,/allowed).WARNING(notFAILURE) for SHOULD violations per AGENTS.md; gates the check to2025-11-25+viatoolNameFormatCheckApplies()so2025-03-26/2025-06-18runs do not false-fail.sep-986-invalid-tool-names.ts+ vitest;specReferenceslist core spec URLs first, then SEP/history links (#986, PR #1603) for divergence context only. Nosep-986.yaml— requirement is core spec, not SEP traceability.Test plan
npm run buildnpm test(336 tests, includingall-scenarios.test.ts/tools-list)npm test -- src/scenarios/server/tools.test.ts(validation rules, version gate, mocked scenario gate)npm test -- src/scenarios/server/negative.test.ts(tools-name-format→WARNINGagainstsep-986-invalid-tool-names.ts)node dist/index.js server --url http://127.0.0.1:<port>/mcp --scenario tools-list --spec-version 2025-11-25againsteverything-server→tools-list+tools-name-formatSUCCESS2025-11-25—node dist/index.js sdk typescript-sdk-v1 --mode server --scenario tools-list --spec-version 2025-11-25 --skip-build→tools-list+tools-name-formatboth SUCCESS, baseline check passedtools-name-formatnot emitted for--spec-version 2025-06-18— CLI against everything-server emits onlytools-list(1/1 checks); grep finds notools-name-formatAcceptance criteria (#379)
tools-name-formatusesWARNINGfor SHOULD violations on 2025-11-25 and draft/)sep-986.yaml(core spec enforcement; SEP links are context inspecReferencesonly)Notes for reviewers
tools.tsdocuments divergence from SEP markdown.traceability.jsonrefresh (workflow-driven).