Skip to content

test(integration): un-skip mcp-protocol (closes #3317)#3347

Merged
bokelley merged 1 commit into
mainfrom
bokelley/mcp-protocol-sse
Apr 27, 2026
Merged

test(integration): un-skip mcp-protocol (closes #3317)#3347
bokelley merged 1 commit into
mainfrom
bokelley/mcp-protocol-sse

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

Closes #3317. Last item in the #3289 umbrella.

The triage agent's directional pick (Option B — force `Accept: application/json`) didn't survive contact: the MCP StreamableHTTP transport returns 406 Not Acceptable when the Accept header doesn't include `text/event-stream`. Switched to Option A (parse the SSE framing back into JSON), which is also more accurate to how real MCP clients negotiate.

Three tests had to move off JSON-RPC `error` envelopes too — the MCP spec carries tool-execution errors in `result.isError` + content, not in the `error` channel (which is reserved for protocol-layer failures like unknown methods). Realigned to the spec-correct shape.

What's in the PR

  • `callMcp` SSE-aware helper — sets `Accept: application/json, text/event-stream`, strips the `event: message\ndata: {...}` framing back to JSON, attaches it as `response.body` so existing assertions stay readable.
  • `vi.mock('express-rate-limit')` — `/mcp` has its own inline `mcpRateLimiter` (`max: 10/min`) that isn't exported from `middleware/rate-limit.ts`. Stubbing the underlying package as passthrough disables every rate limiter for the test, including the inline one. Without this, 22 sequential calls hit the limit and start coming back as `-32000 Rate limit exceeded`.
  • Three tests realigned to spec shape — `get_agent` for a missing URL and `tools/call` for an unknown tool name return `result` (not `error`); structured content now asserts `result` shape and absence of `error`.
  • `toHaveLength(32)` → `length > 0` — tool registry grows; presence checks for specific tools live in their own tests.
  • `server['app']` → `server.app` — drops the private-property bracket-escape.
  • `afterAll(() => server.stop())` — closes the open HTTP handle the previous file leaked.

Test plan

  • CI `Server integration tests` job goes green.
  • `DATABASE_URL=... npm run test:server-integration` locally → mcp-protocol shows 21 passed.

Combined with PR #3326, #3327, #3328, #3333 (all merged or pending), this completes the #3289 umbrella.

🤖 Generated with Claude Code

…, real spec shapes (#3317)

The MCP StreamableHTTP transport requires `Accept: text/event-stream` (a
JSON-only Accept gets 406 Not Acceptable) and frames JSON-RPC responses
as `event: message\ndata: {...}` even for unary calls — so plain
response.body parsing in supertest never sees a body. callMcp now sets
the right Accept and unwraps the SSE framing back into response.body.

Three tests asserted JSON-RPC `error` envelopes for tool-execution
failures (missing agent, unknown tool name). The MCP spec puts those in
`result.isError` + content; the JSON-RPC `error` channel is reserved for
protocol-layer failures. Realigned to the spec.

Sequential calls were tripping the inline mcpRateLimiter (max: 10/min);
mocked express-rate-limit to passthrough.

Drive-bys flagged in code review:
- toHaveLength(32) → length > 0 so the file doesn't break on tool count drift
- server['app'] private-property access → server.app public getter
- afterAll(() => server.stop()) closes the open HTTP handle

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit 3d1ae63 into main Apr 27, 2026
12 checks passed
@bokelley bokelley deleted the bokelley/mcp-protocol-sse branch April 27, 2026 11:29
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.

test(integration): mcp-protocol — handle SSE-framed responses

1 participant