Skip to content

Fix tools-name-format for spec Tool Names SHOULD rules on 2025-11-25+#380

Open
canardleteer wants to merge 4 commits into
modelcontextprotocol:mainfrom
canardleteer:fix/379-sep-986-tool-names
Open

Fix tools-name-format for spec Tool Names SHOULD rules on 2025-11-25+#380
canardleteer wants to merge 4 commits into
modelcontextprotocol:mainfrom
canardleteer:fix/379-sep-986-tool-names

Conversation

@canardleteer

Copy link
Copy Markdown

Closes #379. Completes the gaps left by #238 / #240.

Summary

  • Fixes tools-name-format on tools-list to enforce 2025-11-25 / draft spec prose (1–128 chars, [A-Za-z0-9_.-] only), not stale SEP-986 markdown (64 chars, / allowed).
  • Emits WARNING (not FAILURE) for SHOULD violations per AGENTS.md; gates the check to 2025-11-25+ via toolNameFormatCheckApplies() so 2025-03-26 / 2025-06-18 runs do not false-fail.
  • Adds negative fixture sep-986-invalid-tool-names.ts + vitest; specReferences list core spec URLs first, then SEP/history links (#986, PR #1603) for divergence context only. No sep-986.yaml — requirement is core spec, not SEP traceability.

Test plan

  • npm run build
  • npm test (336 tests, including all-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-formatWARNING against sep-986-invalid-tool-names.ts)
  • CLI smoke: node dist/index.js server --url http://127.0.0.1:<port>/mcp --scenario tools-list --spec-version 2025-11-25 against everything-servertools-list + tools-name-format SUCCESS
  • CLI run against typescript-sdk conformance server on 2025-11-25node dist/index.js sdk typescript-sdk-v1 --mode server --scenario tools-list --spec-version 2025-11-25 --skip-buildtools-list + tools-name-format both SUCCESS, baseline check passed
  • Confirm tools-name-format not emitted for --spec-version 2025-06-18 — CLI against everything-server emits only tools-list (1/1 checks); grep finds no tools-name-format

Acceptance criteria (#379)

  • tools-name-format uses WARNING for SHOULD violations on 2025-11-25 and draft
  • Validation rules match 2025-11-25 / draft prose (128 max, no /)
  • Check does not run on 2025-06-18 or 2025-03-26
  • Scenario description says SHOULD, not MUST
  • Negative vitest + broken-server fixture proves the check fires
  • Reference everything-server passes on applicable versions
  • No sep-986.yaml (core spec enforcement; SEP links are context in specReferences only)

Notes for reviewers

  • SEP vs spec: Validation follows PR #1603 integrated spec diff; block comment in tools.ts documents divergence from SEP markdown.
  • Out of scope (issue follow-ups): uniqueness / case-sensitivity checks; traceability.json refresh (workflow-driven).

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/scenarios/server/tools.test.ts Outdated
Comment thread src/scenarios/server/negative.test.ts
Comment thread examples/servers/typescript/invalid-tool-names.ts
@pkg-pr-new

pkg-pr-new Bot commented Jul 2, 2026

Copy link
Copy Markdown

Open in StackBlitz

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

commit: 0fd7828

@canardleteer canardleteer force-pushed the fix/379-sep-986-tool-names branch from 0fd7828 to 2d083ab Compare July 2, 2026 17:59
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.
@canardleteer canardleteer force-pushed the fix/379-sep-986-tool-names branch from 2d083ab to f1e4395 Compare July 2, 2026 18:02
@canardleteer

Copy link
Copy Markdown
Author

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.

@claude - Accepted all 3 changes and pushed after another local test run.

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.

Fix SEP-986(-ish) strict tool name checks: align with SHOULD semantics and 2025-11-25 spec

1 participant