Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion packages/middleware/hono/src/hono.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,44 @@
import type { Context } from 'hono';
import type { AuthInfo, Dispatchable, HandleHttpOptions } from '@modelcontextprotocol/server';
import { handleHttp } from '@modelcontextprotocol/server';
import type { Context, Handler } from 'hono';
import { Hono } from 'hono';

import { hostHeaderValidation, localhostHostValidation } from './middleware/hostHeaderValidation.js';

/**
* Hono context variables {@linkcode mcpHonoHandler} reads. Upstream middleware
* may set `parsedBody` ({@linkcode createMcpHonoApp} does this for JSON requests)
* and `authInfo` (e.g. a bearer-token verifier) before this handler runs.
*/
export type McpHonoVariables = { parsedBody?: unknown; authInfo?: AuthInfo };

/**
* Mounts an `McpServer` (or any `Protocol` subclass) as a Hono `Handler`.
* Each request flows through `mcp.dispatch()` directly via {@linkcode handleHttp};
* `mcp.connect()` and a transport instance are not used.
*
* Reads `c.get('parsedBody')` (set by {@linkcode createMcpHonoApp}'s JSON middleware)
* and `c.get('authInfo')` (set by an upstream auth middleware) when present.
*
* ```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() }));
* ```
Comment on lines +23 to +30
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.)

*/
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() }));

const handler = handleHttp(mcp, options);
return c => {
const parsedBody = c.get('parsedBody');
const authInfo = c.get('authInfo');
const extra = authInfo !== undefined || parsedBody !== undefined ? { authInfo, parsedBody } : undefined;
return handler(c.req.raw, extra);
};
}

/**
* Options for creating an MCP Hono application.
*/
Expand Down
55 changes: 54 additions & 1 deletion packages/middleware/hono/test/hono.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,18 @@ import type { Context } from 'hono';
import { Hono } from 'hono';
import { vi } from 'vitest';

import { createMcpHonoApp } from '../src/hono.js';
import { McpServer, SessionCompat } from '@modelcontextprotocol/server';

import { createMcpHonoApp, mcpHonoHandler } from '../src/hono.js';
import { hostHeaderValidation } from '../src/middleware/hostHeaderValidation.js';

const INIT_MESSAGE = {
jsonrpc: '2.0',
method: 'initialize',
params: { clientInfo: { name: 'test-client', version: '1.0' }, protocolVersion: '2025-11-25', capabilities: {} },
id: 'init-1'
};

describe('@modelcontextprotocol/hono', () => {
test('hostHeaderValidation blocks invalid Host and allows valid Host', async () => {
const app = new Hono();
Expand Down Expand Up @@ -106,4 +115,48 @@ describe('@modelcontextprotocol/hono', () => {
expect(res.status).toBe(200);
expect(await res.json()).toEqual({ preset: true });
});

describe('mcpHonoHandler', () => {
function makeApp(options?: Parameters<typeof mcpHonoHandler>[1]) {
const mcp = new McpServer({ name: 'test-server', version: '1.0.0' });
const app = createMcpHonoApp({ host: '0.0.0.0', allowedHosts: ['localhost'] });
app.all('/mcp', mcpHonoHandler(mcp, { enableJsonResponse: true, ...options }));
return app;
}

async function postJson(app: Hono, body: unknown, headers: Record<string, string> = {}): Promise<Response> {
return app.request('http://localhost/mcp', {
method: 'POST',
headers: {
Host: 'localhost',
'Content-Type': 'application/json',
Accept: 'application/json, text/event-stream',
...headers
},
body: JSON.stringify(body)
});
}

test('serves initialize via mcp.dispatch() (stateless, no transport class)', async () => {
const res = await postJson(makeApp(), INIT_MESSAGE);
expect(res.status).toBe(200);
const body = (await res.json()) as { result: { serverInfo: { name: string } } };
expect(body.result).toMatchObject({ serverInfo: { name: 'test-server' } });
expect(res.headers.get('mcp-session-id')).toBeNull();
});

test('serves session lifecycle via SessionCompat', async () => {
const app = makeApp({ session: new SessionCompat() });
const initRes = await postJson(app, INIT_MESSAGE);
const sid = initRes.headers.get('mcp-session-id');
expect(sid).toBeTruthy();
const pingRes = await postJson(
app,
{ jsonrpc: '2.0', method: 'ping', params: {}, id: 'p-1' },
{ 'mcp-session-id': sid as string, 'mcp-protocol-version': '2025-11-25' }
);
expect(pingRes.status).toBe(200);
expect(await pingRes.json()).toMatchObject({ jsonrpc: '2.0', id: 'p-1', result: {} });
});
});
});
Loading