Skip to content

fix(http-stream): default responseMode to batch to match docs#193

Open
crossi-dev wants to merge 1 commit into
QuantGeekDev:mainfrom
crossi-dev:fix/http-stream-default-response-mode
Open

fix(http-stream): default responseMode to batch to match docs#193
crossi-dev wants to merge 1 commit into
QuantGeekDev:mainfrom
crossi-dev:fix/http-stream-default-response-mode

Conversation

@crossi-dev

Copy link
Copy Markdown

Summary

DEFAULT_HTTP_STREAM_CONFIG.responseMode was set to 'stream', but every place that documents the default says 'batch':

  • README.md, "Response Modes" section: "Batch Mode (Default)", and the config example comment responseMode: "batch", // Optional (default: "batch")
  • This repo's own CLAUDE.md: "Response modes: "batch" (default) or "stream""
  • The responseMode field's own JSDoc comment

So any server built with the documented pattern:

new MCPServer({
  transport: {
    type: "http-stream",
    options: { port: 8080, endpoint: "/mcp" /* responseMode intentionally omitted */ },
  },
});

silently ran in stream (SSE) mode instead of the documented batch (JSON) mode, because MCPServer.createTransportFromConfig spreads DEFAULT_HTTP_STREAM_CONFIG first and the constant itself had the wrong value.

Fixes #184.

Changes

  • src/transports/http/types.ts: DEFAULT_HTTP_STREAM_CONFIG.responseMode 'stream''batch', and updated the field's doc comment to match.
  • tests/transports/host-binding.test.ts: added a regression test in the existing "Default config values" block (same pattern already used there for port/host) asserting DEFAULT_HTTP_STREAM_CONFIG.responseMode === 'batch'.

Why this direction (and not updating the docs instead)

The issue's "Suggested fix" section offered either direction. A prior PR (#185) took the "fix the docs to say stream" direction and was closed. Since the docs are consistent and repeated in three independent places (README, CLAUDE.md, and the field's own JSDoc) and none of them describe stream as the default, I fixed the runtime to match the documented contract instead.

Verification

  • npm run build (tsc) — passes, no errors.
  • npx jest tests/transports/host-binding.test.ts tests/transports/http/ tests/compat/ — 10 suites, 180 tests, all passing (includes the new test, plus the existing ticket-018-serverless-bc-* backward-compat suites that exercise HttpStreamTransport/MCPServer construction).
  • Verified the new test fails against unfixed code (responseMode: 'stream') and passes with the fix, confirming it actually pins the bug.
  • Full npx jest run: 851 tests passing. 5 unrelated suites under packages/docs/tests/ fail to compile on main as well (pre-existing TS2339: Property 'toolCall' does not exist errors, unrelated to this change) — confirmed by stashing this change and re-running.
  • npx prettier --check src/transports/http/types.ts tests/transports/host-binding.test.ts — clean. (Repo-wide npm run lint / prettier-check has pre-existing failures on main in unrelated files — confirmed also present without this change — left untouched per minimal-diff scope.)

No other behavior changes. Happy to adjust if you'd prefer the opposite direction (docs-to-match-runtime) after all, or want the standalone HttpStreamTransport constructor's own responseMode === 'batch' check aligned the same way (I kept this change scoped to the exact path in the issue's repro).

DEFAULT_HTTP_STREAM_CONFIG.responseMode was 'stream', but the README,
the repo's own CLAUDE.md architecture notes, and the type's own JSDoc
all document batch as the default response mode for the HTTP Stream
transport. Any MCPServer using type: 'http-stream' without an explicit
responseMode silently ran in stream (SSE) mode instead of the
documented batch (JSON) mode.

Fixes QuantGeekDev#184
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.

http-stream default responseMode is stream in runtime, but documented as batch

1 participant