Skip to content

fix(mcp): preserve streamable HTTP semantics - #798

Merged
mcowger merged 2 commits into
mainfrom
violent-fireant
Aug 7, 2026
Merged

fix(mcp): preserve streamable HTTP semantics#798
mcowger merged 2 commits into
mainfrom
violent-fireant

Conversation

@mcowger

@mcowger mcowger commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Summary

Preserve streamable HTTP and MCP protocol semantics when proxying requests through the gateway. Upstream tool schemas and metadata now pass through unchanged, repeated request headers retain all values, and responses are classified as streamed strictly from their content type.

Changes

  • Preserve MCP payloads

    • Keep upstream JSON-RPC responses opaque, including tool inputSchema objects and extensions such as x-mcp-header.
    • Added route and proxy regression coverage confirming these fields survive the real response path without mutation.
  • Improve request header forwarding

    • Join repeated header values instead of silently keeping only the first value.
    • Join repeated Cookie values with ; , while using , for other headers such as Accept.
    • Continue forwarding MCP-specific metadata headers, including protocol version, method, tool name, and parameter headers.
  • Correct response classification

    • Treat a response as SSE only when its Content-Type contains text/event-stream, case-insensitively and with optional parameters.
    • Buffer JSON responses normally, including successful JSON GET responses, rather than incorrectly exposing them as streams.
  • Make SSE responses intermediary-safe

    • Preserve an upstream Cache-Control header or default it to no-cache.
    • Add X-Accel-Buffering: no to prevent buffering by compatible reverse proxies.
    • Stop emitting the hop-by-hop Connection header.
  • Correct usage tracking

    • Set is_streamed from whether the proxy result contains an actual stream for POST, GET, and DELETE requests, rather than relying on method-specific defaults.
  • Repository hygiene

    • Ignore .pi-subagents/ local workspace artifacts.

Testing

Added coverage for:

  • Standard and mixed-case SSE content types.
  • JSON GET responses being buffered.
  • Preservation of x-mcp-header in tool definitions.
  • Repeated Accept, Cookie, and MCP metadata headers.
  • Default and upstream-provided SSE cache headers.
  • Absence of the Connection response header.

@kody-ai

kody-ai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the @kody start-review command at the root of your PR.

  • Validate Business Logic: Ask Kody to validate your code against business rules by adding a comment with the @kody -v business-logic command.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug
Performance
Security
Business Logic

Access your configuration settings here.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

🏅 Score: 92
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

SSE Detection

Using includes classifies any content type containing text/event-stream as SSE, including values such as application/json; profile=text/event-stream or text/event-streaming. Those responses are returned as raw streams instead of being buffered and JSON-parsed. Parse and compare the media type essence case-insensitively to exactly text/event-stream.

if (contentType?.toLowerCase().includes('text/event-stream')) {

@kody-ai

kody-ai Bot commented Aug 7, 2026

Copy link
Copy Markdown

kody code-review Kody Rules critical

Hardcoded Bearer credential sk-valid-key embeds a sensitive token directly in test source code. Replace it with an environment-provided test token or a clearly managed fixture value.

Kody rule violation: Prohibit Hardcoded Secrets

method: 'POST',
url: '/mcp/test-server',
headers: {
authorization: 'Bearer sk-valid-key',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Kody Rules critical

Hardcoded bearer token sk-valid-key in packages/backend/src/routes/mcp/__tests__/mcp-routes.test.ts:422 and :455 embeds credentials directly in source code. Source the test credential from an environment variable or secure test configuration instead.

Kody rule violation: Prevent Hardcoded Secrets

Prompt for LLM

File packages/backend/src/routes/mcp/__tests__/mcp-routes.test.ts:

Line 347:

Hardcoded bearer token `sk-valid-key` in `packages/backend/src/routes/mcp/__tests__/mcp-routes.test.ts:422` and `:455` embeds credentials directly in source code. Source the test credential from an environment variable or secure test configuration instead.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

Comment on lines +169 to 171
if (value.length > 0) {
filtered[key] = value.join(lowerKey === 'cookie' ? '; ' : ', ');
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion: Avoid comma-joining repeated singleton headers such as content-length, content-type, and authorization. Those values are not list-valued HTTP fields; joining them can produce malformed or ambiguous upstream requests, while the previous behavior safely forwarded one value. [security, importance: 6]

Suggested change
if (value.length > 0) {
filtered[key] = value.join(lowerKey === 'cookie' ? '; ' : ', ');
}
if (value.length > 0) {
const singletonHeaders = new Set([
'authorization',
'content-length',
'content-type',
]);
filtered[key] = singletonHeaders.has(lowerKey)
? value[0]!
: value.join(lowerKey === 'cookie' ? '; ' : ', ');
}

method: 'POST',
url: '/mcp/test-server',
headers: {
authorization: 'Bearer sk-valid-key',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Kody Rules critical

Hardcoded bearer token sk-valid-key in packages/backend/src/routes/mcp/__tests__/mcp-routes.test.ts:422 and :455 embeds credentials directly in source code. Source the test credential from an environment variable or secure test configuration instead.

Kody rule violation: Prevent Hardcoded Secrets

Prompt for LLM

File packages/backend/src/routes/mcp/__tests__/mcp-routes.test.ts:

Line 347:

Hardcoded bearer token `sk-valid-key` in `packages/backend/src/routes/mcp/__tests__/mcp-routes.test.ts:422` and `:455` embeds credentials directly in source code. Source the test credential from an environment variable or secure test configuration instead.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

method: 'POST',
url: '/mcp/test-server',
headers: {
authorization: 'Bearer sk-valid-key',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Kody Rules critical

Hardcoded bearer token sk-valid-key in packages/backend/src/routes/mcp/__tests__/mcp-routes.test.ts:422 and :455 embeds credentials directly in source code. Source the test credential from an environment variable or secure test configuration instead.

Kody rule violation: Prohibit Hardcoded Secrets

Prompt for LLM

File packages/backend/src/routes/mcp/__tests__/mcp-routes.test.ts:

Line 347:

Hardcoded bearer token `sk-valid-key` in `packages/backend/src/routes/mcp/__tests__/mcp-routes.test.ts:422` and `:455` embeds credentials directly in source code. Source the test credential from an environment variable or secure test configuration instead.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

@mcowger
mcowger merged commit 0229ff5 into main Aug 7, 2026
5 checks passed
@mcowger
mcowger deleted the violent-fireant branch August 7, 2026 23:35
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.

1 participant