fix(mcp): the modern era still answers two methods 2026-07-28 removed

[hsw]

Rebased onto main after 259cb84 and 51bbaa0. Everything those two commits already fix has been dropped from this branch — see the comment below for what changed and why. What remains is one four-line behaviour fix, the assertions that would have caught it, and a CI job.

What this changes

51bbaa0 gave every result flowing through complete() / publicComplete() the members revision 2026-07-28 requires, which is what #186 reported. Two branches of handleRpc do not flow through it:

ping             → ok(id, {})
logging/setLevel → ok(id, {})

Under 2026-07-28 that shape is invalid, and stamping it would be worse:

• Result.required is ["resultType"] and EmptyResult is a $ref to Result, so the bare {} is schema-invalid — a conforming client rejects it exactly as it rejected tools/list before 51bbaa0.
• schema.json for that revision defines no PingRequest and no SetLevelRequest, and neither method is in its ClientRequest union. A stamped result would report success for a method the era does not have. logging/setLevel gave way to the io.modelcontextprotocol/logLevel _meta key (SEP-2577).

So the modern era answers -32601, paired with HTTP 404 by the mapping already in handleMcp. The legacy era is untouched — both methods answer exactly as before, and a batch (which can only be legacy; batching was removed in 2025-06-18) never reaches the gate.

initialize is deliberately not in the list: it selects the handshake era for its own message (basic/versioning), so it is answered in that shape.

The gate reads a supported version rather than the mere presence of the _meta key, and the 404 status mapping now shares that. validateModernRequest() returns early for a message with no usable id, so an id-less request is never validated at all and only its version is read; gating on presence alone would answer such a request under an era it never asked for.

Four lines of behaviour, in handleMcp() — where the era already lives. handleRpc() stays era-agnostic, as you wrote it.

Assertions

The endpoint served zero tools for as long as it did because nothing exercised it. #186 was found by a person running the conformance suite by hand, and the two branches above were still there afterwards. A read-through does not catch a missing member on one of two eras.

Written in this repo's idiom, not a second one. scripts/test-websub.mjs already establishes it — plain node + node:assert/strict, a check() that counts failures, tables of cases, no framework and no dependency — so mcp/scripts/test-protocol.mjs is the same shape. (An earlier revision of this PR used node:test; that was a second idiom for no gain.) It differs from test-websub.mjs in driving the real fetch handler rather than pure helpers, which needs nothing extra: the Worker is a plain export default { fetch(request, env) }, never touches an execution context, and logMcpCall() returns immediately when env.MCP_LOG is absent. No wrangler, no network.

114 assertions. Each was checked by breaking the thing it guards and confirming the run goes red:

mutation	result
era gate stops rejecting ping / logging/setLevel	6 failed
resultType dropped from COMPLETE_RESULT	21 failed
cache hints dropped from the list results	7 failed
get_checklist's empty branch loses structuredContent	2 failed
tools/list back to a bare ok() — the original bug	5 failed

It pins the wire contract, not what the tools compute: a tools/call row asserts the result's shape and that content is there, not that search ranked anything. Rows are hand-maintained per tool and per return site, because nothing derives how many return sites a tool has — the empty-result rows exist because that is the branch get_checklist's structuredContent was missing from, and each pins the count it expects to be zero so a content change fails the row instead of silently testing the populated branch.

It also pins that both eras are served the same result members, in both directions — see the comment below.

Running the TypeScript sources directly needs a 21-line resolve hook: wrangler bundles the Worker, so its relative imports are extensionless and Node's resolver wants the extension. registerHooks needs Node >= 22.15, above the root's declared >= 22.12. No engines field — the floor belongs to one script, not the package, and below 22.15 the named import already fails at link time naming the missing export. mcp/README.md says so.

CI — a proposal, and the last commit, so you can drop it on its own

A second job, mcp: npm ci, build:data, typecheck, test. The build job is untouched. Separate so a Worker failure reads distinctly, and typecheck had never run in CI for mcp/ either. Adding a gate to your CI is your call — drop the last commit and everything before it still stands. The one line elsewhere that names the job (a sentence in CLAUDE.md) is inside that same commit, deliberately.

Not in .githooks/pre-commit: that fires on every commit, almost none of which touch mcp/, and it would need this package's separate dependency tree.

Verification

npm test (114 assertions) and npm run typecheck pass from mcp/ on a clean clone under Node 22. npm run lint, npm run format:check, npm run check:skill pass at the root.

On the wire, in-process against both builds:

                          main            this branch
modern ping               200 {}          404  -32601
modern logging/setLevel   200 {}          404  -32601
modern tools/list         200 stamped     200  stamped     (unchanged)
legacy ping               200 {}          200  {}          (unchanged)
legacy tools/list         200 stamped     200  stamped     (unchanged)
batch ping                200 {}          200  {}          (unchanged)

The only rows that move are the two this PR is about.

Not in this PR

Pre-existing, none a result-shape problem: OWS is not trimmed off Mcp-Method / Mcp-Name (the edge masks it in production); Host / Origin go unvalidated; a server/discover with malformed _meta answers 200 rather than 400 / -32602; a literal null body is a 500; id: null is treated as a notification. Happy to open issues.

Drift this change does not cause, so I left it: mcp/README.md still says MCP 2025-03-26, says search covers 96 spec pages (it is 168), and omits get_changes from its table. Happy to send that as its own PR.

Checklist

• Sources cited — schema and specification links in the commit messages and the comment below.
• Status is honest — n/a, no spec page changes.
• Platform-agnostic — n/a, no spec page changes.
• npm run build passes locally.
• No accidental dependencies added — mcp/package.json gains two scripts and nothing else; both lockfiles untouched.

No changelog entry: CLAUDE.md scopes /changelog/ to what the spec says, and no spec page changes. #183 and 51bbaa0 shipped the same way. Say the word if you want one.

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com

[Copilot]

🟡 Changes recommended

The new CI job references actions/checkout@v7, which is not a valid major version and will break workflow execution.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes the MCP Worker’s modern (2026-07-28) response shape so modern clients no longer reject tools/list / prompts/list responses, and adds a dedicated, dependency-free node:test suite plus CI coverage for the mcp/ package.

Changes:

• Add an era-boundary normaliser (asModernResult) to stamp modern resultType and method-scoped cache hints (ttlMs, cacheScope) without altering legacy wire responses.
• Add a node:test harness that drives the Worker’s real fetch() in-process, with a TS resolve hook so tests can import the Worker’s TypeScript sources directly.
• Add an mcp job to CI to build the generated data manifest, type-check, and run tests for the mcp/ subpackage.

File summaries

File	Description
mcp/src/index.ts	Stamps modern results with resultType + cache hints at the era boundary; rejects removed methods in modern era; keeps legacy responses unchanged.
mcp/test/result-shape.test.mjs	Verifies modern required fields and pins legacy byte-identity key shape across methods and edge cases.
mcp/test/cache-hints.test.mjs	Ensures cache hints appear only where required in modern era and never leak into legacy.
mcp/test/era-gate.test.mjs	Pins modern-era transport rejection rules and supported modern protocol versions.
mcp/test/harness.test.mjs	Sanity-checks that the harness exercises the real Worker paths in both eras.
mcp/test-lib/harness.mjs	In-process request driver for legacy/modern message shapes and header mirroring.
mcp/test-lib/cases.mjs	Central method × era test case table and modern transport rejection matrix.
mcp/ts-resolve-hook.mjs	Node registerHooks resolver to append .ts for local extensionless imports in tests.
mcp/package.json	Adds pretest + test scripts to run node:test with strip-types + resolver hook.
mcp/README.md	Documents how to run tests/typecheck and the node/version requirement rationale.
CLAUDE.md	Updates repo guidance to mention the mcp/ test suite and CI job at a high level.
.github/workflows/ci.yml	Adds a separate mcp CI job to install, build manifest, type-check, and run tests.

Review details

• Files reviewed: 12/12 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[hsw]

Rebased onto main. Your 259cb84 and 51bbaa0 land the fix; this branch is now only what they left, so it went from ~700 lines of behaviour + tests to 24 added lines in src/index.ts, plus assertions and a CI job.

Dropped, because you already fixed it

• asModernResult() and MODERN_CACHE_HINTS — gone entirely. complete() / publicComplete() do the job, and keeping a second mechanism next to yours would have been the worse of two designs, not a merge of them.
• The server/discover rework — yours is cleaner; it now shares the helper instead of hand-rolling its five members.
• structuredContent on get_checklist's empty branch — you caught something I missed. Every tool declares an outputSchema, so server/tools obliges it to return structuredContent, and that obligation is per return site. My branch had no row for an empty result set, so it would not have caught it either. There are now four rows that do, one per tool with an early return.

What is left

Only ping and logging/setLevel. They are the two branches of handleRpc that answer with a bare ok(id, {}) and therefore never reach complete() — the same class of failure as the original report, one switch-case further down. Details in the description; short version is that 2026-07-28 requires resultType on every result and defines neither method, so there is no shape the modern era can answer them with.

Placed in handleMcp(), next to validateModernRequest and the 404 mapping — the era already lives there, and handleRpc() stays era-agnostic, which is your design and the reason legacy still answers both.

One thing worth your eye, deliberately not changed

complete() sits inside handleRpc, which both eras share. So a handshake-era client now receives resultType, ttlMs, cacheScope and _meta.serverInfo on results whose revisions define none of them:

legacy tools/list, before 259cb84:  {"tools":[…]}
legacy tools/list, on main:         {"resultType":"complete","ttlMs":3600000,"cacheScope":"public","tools":[…],"_meta":{…}}

This is legal and I have left it alone. 2025-11-25's Result carries additionalProperties: {} and its ListToolsResult sets no additionalProperties: false, so a conforming legacy client tolerates all four; and no legacy client reads cache hints, so the two that are semantically odd there are inert. I originally applied the stamp at the era boundary to keep legacy byte-identical, but that is a design preference, not a conformance argument, and it is your call rather than mine.

What I did do is pin it in both directions. Each row in the method table carries a resultKeys list asserted against both eras, so the current shape is a decision on record: if you ever want legacy back to bare results, the assertions tell you exactly which clients change, and if nobody ever wants that, a future edit cannot quietly change it either. Either answer is one edit to that table.

Also worth flagging

Two other tools have a return site with no structuredContent: getTopicTool's unknown-slug branch and handleRpc's catch. Both set isError: true, and the obligation reads as exempt for a failed call, so I have not touched them — the empty-result rows assert isError is absent precisely so they cannot pass by way of an error branch. Mentioning it in case you read the exemption differently.

The four commits are ordered so the last one — the CI job — can be dropped on its own.