Skip to content

feat(@mcp/hono): mcpHonoHandler mount helper (handleHttp-backed)#2074

Draft
felixweinberger wants to merge 1 commit into
fweinberger/r4-backchannelfrom
fweinberger/m3-hono
Draft

feat(@mcp/hono): mcpHonoHandler mount helper (handleHttp-backed)#2074
felixweinberger wants to merge 1 commit into
fweinberger/r4-backchannelfrom
fweinberger/m3-hono

Conversation

@felixweinberger
Copy link
Copy Markdown
Contributor


@modelcontextprotocol/hono adapter switches internals to handleHttp(server). Public API unchanged.

Motivation and Context

Same as M1, for hono.

How Has This Been Tested?

Existing @modelcontextprotocol/hono tests pass unchanged.

Breaking Changes

None.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Depends on R4. Hold until R/S/F have soaked. Review guide: https://gist.github.com/felixweinberger/5a48e0f14d5aced39ed6a91b61940711.


@felixweinberger felixweinberger added the v2-stateless 2026-06 SDK: Protocol decomposition + SEP alignment (request-first / stateless) label May 12, 2026
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 12, 2026

⚠️ No Changeset found

Latest commit: bdf79c7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 12, 2026

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2074

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2074

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2074

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2074

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2074

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2074

commit: bdf79c7

@felixweinberger
Copy link
Copy Markdown
Contributor Author

@claude review

1 similar comment
@felixweinberger
Copy link
Copy Markdown
Contributor Author

@claude review

* app.all('/mcp', mcpHonoHandler(mcp, { session: new SessionCompat() }));
* ```
*/
export function mcpHonoHandler(mcp: Dispatchable, options?: HandleHttpOptions): Handler<{ Variables: McpHonoVariables }> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 This PR adds two new public exports (mcpHonoHandler and McpHonoVariables, surfaced via export * from './hono.js' in index.ts), but the PR body says "Public API unchanged" / "Documentation update" only, no .changeset is included (changeset-bot flagged this — should be a minor for @modelcontextprotocol/hono), and packages/middleware/hono/README.md still lists only createMcpHonoApp / hostHeaderValidation / localhostHostValidation and demonstrates the legacy WebStandardStreamableHTTPServerTransport + server.connect() pattern. Please add a changeset, correct the PR description, and update the README's Exports + Usage sections — the JSDoc snippet on mcpHonoHandler (hono.ts:23-30) is ready to copy in.

Extended reasoning...

What the issue is. packages/middleware/hono/src/index.ts does export * from './hono.js', so the newly added export function mcpHonoHandler(...) and export type McpHonoVariables become part of @modelcontextprotocol/hono's public API surface. CLAUDE.md § Public API Exports is explicit: "Adding a symbol to a package index.ts makes it public API — do so intentionally." The PR title itself agrees — it's feat(@mcp/hono): mcpHonoHandler mount helper. Yet three release-hygiene artifacts were not updated to match.

Step-by-step proof.

  1. src/index.ts:1export * from './hono.js' re-exports everything from hono.ts.
  2. The diff adds export type McpHonoVariables (hono.ts:13) and export function mcpHonoHandler (hono.ts:32) — both are therefore public.
  3. git diff HEAD~1..HEAD --name-only shows only hono.ts and hono.test.ts — no .changeset/*.md and no README.md change. changeset-bot in the PR timeline confirms "No Changeset found" for commit 20b17c0.
  4. The PR description states "Public API unchanged" and ticks only "[x] Documentation update", contradicting both the diff and the PR title.
  5. packages/middleware/hono/README.md lines 19–23 list only createMcpHonoApp, hostHeaderValidation, localhostHostValidation under ExportsmcpHonoHandler and McpHonoVariables are absent.
  6. README lines 29–39 still show WebStandardStreamableHTTPServerTransport + server.connect(transport) + manual transport.handleRequest(c.req.raw, { parsedBody: c.get('parsedBody') }) — exactly the pattern mcpHonoHandler is meant to supersede.

Why nothing currently catches it. The repo's review checklist says "New feature: verify prose documentation is added (not just JSDoc)" and the Documentation & Changesets recurring catch says to flag prose that the diff doesn't back. JSDoc was added (hono.ts:15–31), but neither the package README nor a changeset was, and the PR body actively claims no API change. Tests and typecheck pass because none of this is enforced by CI beyond changeset-bot's non-blocking warning.

Impact. Without a changeset, the next @modelcontextprotocol/hono release won't get a version bump or changelog entry for the new mount helper, so consumers reading the changelog/README won't discover mcpHonoHandler and will keep wiring the legacy transport manually. The misleading "Public API unchanged" claim also makes the PR harder to audit later. This is process/release hygiene rather than a runtime defect, hence nit severity. (Noted: this is part of the stacked v2-stateless series — "Depends on R4. Hold until R/S/F have soaked" — so the author may intend to batch changesets, but the PR-body claim and README should still be corrected here.)

How to fix.

  • Add a changeset, e.g. .changeset/lucky-mangos-rescue.md:
    ---
    "@modelcontextprotocol/hono": minor
    ---
    Add `mcpHonoHandler(mcp, options?)` mount helper and `McpHonoVariables` type, backed by `handleHttp`.
  • Update the PR description: tick New feature, drop "Public API unchanged".
  • Update packages/middleware/hono/README.md: add mcpHonoHandler(mcp, options?) and McpHonoVariables to the Exports list, and replace the Usage example with the snippet already in the JSDoc:
    import { McpServer, SessionCompat } from '@modelcontextprotocol/server';
    import { createMcpHonoApp, mcpHonoHandler } from '@modelcontextprotocol/hono';
    
    const mcp = new McpServer({ name: 's', version: '1.0.0' });
    const app = createMcpHonoApp();
    app.all('/mcp', mcpHonoHandler(mcp, { session: new SessionCompat() }));

Comment on lines +23 to +30
* ```ts
* import { McpServer, SessionCompat } from '@modelcontextprotocol/server';
* import { createMcpHonoApp, mcpHonoHandler } from '@modelcontextprotocol/hono';
*
* const mcp = new McpServer({ name: 's', version: '1.0.0' });
* const app = createMcpHonoApp();
* app.all('/mcp', mcpHonoHandler(mcp, { session: new SessionCompat() }));
* ```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 nit: per CLAUDE.md § "JSDoc @example Code Snippets", this code block should be an @example with a ts source="./hono.examples.ts#…" fence pulling from a type-checked companion file (as express.ts, fastify.ts, and node/streamableHttp.ts all do). Right now there's no packages/middleware/hono/src/hono.examples.ts, so this inline snippet isn't type-checked and will silently rot if mcpHonoHandler / SessionCompat signatures change. Consider adding a hono.examples.ts with e.g. a #region mcpHonoHandler_basicUsage and referencing it via source=.

Extended reasoning...

What the issue is. CLAUDE.md (§ JSDoc @example Code Snippets, lines ~44–48) states that JSDoc example code should pull type-checked code from companion .examples.ts files using fences of the form ts source="./file.examples.ts#regionName", kept in sync via pnpm sync:snippets. The new mcpHonoHandler JSDoc instead embeds a plain ts fence with an inline, hand-written snippet (lines 23–30), with no @example tag and no source= attribute.

Why the convention exists / why it matters here. The point of the source= + .examples.ts mechanism is that the snippet is real, compiled TypeScript — if mcpHonoHandler's signature changes, or SessionCompat is renamed, or createMcpHonoApp gains a required option, tsc fails on hono.examples.ts and the breakage is caught in CI. An inline snippet has none of that protection: it can drift from the actual API and nothing in the build will notice. This is the first JSDoc code block in hono.ts, so it also sets the precedent for the package.

Sibling-package precedent. Every other middleware adapter already follows the convention:

  • packages/middleware/express/src/express.ts@example + source="./express.examples.ts#…", backed by express.examples.ts
  • packages/middleware/fastify/src/fastify.ts@example + source="./fastify.examples.ts#…", backed by fastify.examples.ts
  • packages/middleware/node/src/streamableHttp.tssource="./streamableHttp.examples.ts#…", backed by streamableHttp.examples.ts

@modelcontextprotocol/hono is the only middleware package with no *.examples.ts file at all (verified via glob), so this PR is a natural place to add one rather than diverge further.

Step-by-step illustration of the rot risk.

  1. Today the snippet reads mcpHonoHandler(mcp, { session: new SessionCompat() }).
  2. Suppose a later PR renames SessionCompatSessionManager, or changes the option key from session to sessions.
  3. pnpm build / pnpm typecheck run cleanly because nothing compiles this string — it's just a comment.
  4. pnpm sync:snippets also does nothing, because there's no source= attribute pointing at a region to re-sync.
  5. The published JSDoc / generated docs now show code that doesn't compile, and the only way to catch it is a human re-reading the comment. With a hono.examples.ts region, step 3 would instead fail the build.

Suggested fix. Add packages/middleware/hono/src/hono.examples.ts containing the snippet inside // #region mcpHonoHandler_basicUsage / // #endregion, then change the JSDoc to:

 * @example
 * ```ts source="./hono.examples.ts#mcpHonoHandler_basicUsage"
 * import { McpServer, SessionCompat } from '@modelcontextprotocol/server';
 * import { createMcpHonoApp, mcpHonoHandler } from '@modelcontextprotocol/hono';
 *
 * const mcp = new McpServer({ name: 's', version: '1.0.0' });
 * const app = createMcpHonoApp();
 * app.all('/mcp', mcpHonoHandler(mcp, { session: new SessionCompat() }));
 * ```

and run pnpm sync:snippets.

Severity. This is a docs/style convention nit, not a correctness defect — the handler itself is fine. Filing as nit. (Minor caveat: a couple of pre-existing bare ts fences exist elsewhere in the repo, e.g. node/streamableHttp.ts:34 and express/auth/metadataRouter.ts:108, so enforcement isn't absolute — but the dominant pattern across the middleware adapters is clearly source= + .examples.ts, and CLAUDE.md documents it as the expected approach.)

@felixweinberger felixweinberger force-pushed the fweinberger/r4-backchannel branch from 3eebc7f to 2358111 Compare May 12, 2026 14:38
@felixweinberger
Copy link
Copy Markdown
Contributor Author

@claude review

@felixweinberger felixweinberger force-pushed the fweinberger/r4-backchannel branch from 2358111 to 8493aac Compare May 12, 2026 15:56
@felixweinberger
Copy link
Copy Markdown
Contributor Author

@claude review

@felixweinberger felixweinberger force-pushed the fweinberger/r4-backchannel branch from 8493aac to 716039a Compare May 12, 2026 17:28
@felixweinberger
Copy link
Copy Markdown
Contributor Author

@claude review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2-stateless 2026-06 SDK: Protocol decomposition + SEP alignment (request-first / stateless)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant