Skip to content

Fix: Add Session ID and Header Support to SSE Transport (#660)#661

Merged
deathbyknowledge merged 15 commits into
cloudflare:mainfrom
agentcathq:feat/mcp-sse-header-parity
Nov 25, 2025
Merged

Fix: Add Session ID and Header Support to SSE Transport (#660)#661
deathbyknowledge merged 15 commits into
cloudflare:mainfrom
agentcathq:feat/mcp-sse-header-parity

Conversation

@naji247

@naji247 naji247 commented Nov 18, 2025

Copy link
Copy Markdown
Contributor

Overview

This is a PR to fix issue #660. The SSE transport in MCP was missing critical request context that StreamableHTTP transport already provided. When using SSE, MCP servers couldn't access:

  • Session IDs for connection tracking
  • Authentication information from middleware
  • Request headers for custom metadata or authorization

This created a feature gap between the two transports, limiting SSE-based implementations. Even though SSE is technically deprecated, our analysis of major companies with MCP servers shows that many are still dependent on SSE. To disable SSE support for their users would require all of them to update their MCP JSON configs (which is quite a hassle).

This PR brings full feature parity to the SSE transport in the following ways:

  1. Session ID Support
  • Updated McpSSETransport to accept and store session IDs
  • Modified transport initialization to pass session ID from the agent name
  1. Authentication & Header Forwarding
  • Created buildMessageExtraInfo() helper to construct MessageExtraInfo with auth and headers
  • Updated onSSEMcpMessage() to accept and forward authentication context
  • Modified SSE handler in utils.ts to extract and pass request metadata

Key Changes

  • McpSSETransport: Now accepts optional sessionId in constructor
  • onSSEMcpMessage: Accepts extraInfo parameter with headers and auth
  • buildMessageExtraInfo: New helper that filters internal headers and builds proper context
  • Type definitions: Added RequestWithAuth type for cleaner code

Testing

I added a few tests to the existing SSE based test file. The build passes successfully and the changes have been validated to ensure:

  • Session IDs are properly transmitted
  • Authentication info from middleware is accessible
  • Request headers are forwarded correctly
  • Internal transport headers are filtered out

- Add MessageExtraInfo support to SSE transport matching StreamableHTTP behavior
- Pass request headers and auth info through SSE message handling
- Add RequestWithAuth type for consistent auth info passing
- Filter internal headers (content-type, content-length, upgrade) before passing to MCP server
- Add comprehensive tests for header and auth handling in both transports
- Ensure transport initialization before processing SSE messages

This ensures both SSE and StreamableHTTP transports have consistent header and authentication handling capabilities, allowing MCP servers to access request context regardless of transport type.
@changeset-bot

changeset-bot Bot commented Nov 18, 2025

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: f0e7d86

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
agents Patch

Not sure what this means? Click here to learn what changesets are.

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

@pkg-pr-new

pkg-pr-new Bot commented Nov 18, 2025

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/agents@661

commit: f0e7d86

@threepointone threepointone left a comment

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.

this looks good to me, but I'll let @deathbyknowledge have a look before we land it since he'd touched this part of the code

@naji247

naji247 commented Nov 18, 2025

Copy link
Copy Markdown
Contributor Author

this looks good to me, but I'll let @deathbyknowledge have a look before we land it since he'd touched this part of the code

Thank you @threepointone ! Please let us know if you want any modifications. I tried to keep it similar to how Streamable HTTP was implemented.

@deathbyknowledge deathbyknowledge left a comment

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.

Thanks for the PR! While we want to add support for the extra parameter for the SSE transport, there are a few open questions that I'd like to ask.

Specially regarding request.auth, I'd appreciate more context about your auth middleware implementation.
Most commonly MCPs have implemented auth with workers-oauth-provider through ctx.props (which is populated by the library). Docs that cover some of this can be found here

method: "POST"
});

// Spy on the transport to capture the extraInfo

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.

Doesn't seem like this is testing anything new here. What am I missing?

// Only custom headers like x-custom-header should be preserved in requestInfo.headers
const sseText = await readSSEEvent(response);
const result = parseSSEData(sseText);
expectValidToolsList(result);

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.

On a closer look it seems like the 3 tests are doing the same. They are either adding custom headers to the request or setting an auth property.
Then the test checks that the request was successful and that the tool list is correct but there are no assertions relevant to either the custom headers or the auth field.

One thing that can be done is update the MCP behind the worker to do/return differently in these scenarios.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I decided to create a tool in the workers.ts test utility that actually returns the requestInfo it receives to make it more thorough of a test.

Comment thread packages/agents/src/mcp/index.ts Outdated
}

// Ensure transport is initialized before processing messages
if (!this._transport) {

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.

This should not be needed, onStart is called every time a request is routed and the DO is initialized. Were you facing issues with this._transport not being initialized?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I did this just to make sure it was properly initialized. I will remove this :)

Comment thread packages/agents/src/mcp/index.ts Outdated
Comment on lines +101 to +102
// Remove internal headers that are not part of the original request
delete headers["content-type"];

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.

These headers are part of the original request, they should not be removed

Comment thread packages/agents/src/mcp/index.ts Outdated
return new McpSSETransport(() => this.getWebSocket());
return new McpSSETransport(
() => this.getWebSocket(),
this.getSessionId()

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.

In the StreamableHTTP we use the pattern here to get this, so we can avoid drilling fields through the constructor.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. Will do 🫡

Originally I was worried about breaking existing uses of this class, but it seems to be internal only and limited to this part of the codebase.

Comment thread packages/agents/src/mcp/utils.ts Outdated
const error = await agent.onSSEMcpMessage(sessionId, messageBody);
const error = await agent.onSSEMcpMessage(sessionId, messageBody, {
headers: Object.fromEntries(request.headers.entries()),
auth: (request as RequestWithAuth).auth

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.

Hmm this normally runs inside your Worker fetch handler, how did the request get the auth property?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right, for the purposes of the Cloudflare users we're aiding, we actually only needed the headers (and not auth explicitly) so our tests weren't covering the auth header. I added it in with the hopes of meeting parity with StreamableHTTP but I decided to instead reduce the scope of this PR to just include header and session management information.

@naji247

naji247 commented Nov 19, 2025

Copy link
Copy Markdown
Contributor Author

@deathbyknowledge I went ahead and updated the tests, reverted some of the changes I made to include auth information, and refactored the SSE constructor to use an options object paradigm.

@deathbyknowledge deathbyknowledge left a comment

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.

Looks good! There are a few nits and one type error breaking the CI but this should be ready to merge after those are addressed 👍

Comment thread packages/agents/src/mcp/index.ts Outdated
Comment thread packages/agents/src/mcp/utils.ts Outdated
Comment thread packages/agents/src/mcp/transport.ts Outdated
Comment thread packages/agents/src/mcp/index.ts Outdated
@naji247 naji247 changed the title Fix: Add Session ID, Auth, and Header Support to SSE Transport (#660) Fix: Add Session ID and Header Support to SSE Transport (#660) Nov 20, 2025
@naji247
naji247 force-pushed the feat/mcp-sse-header-parity branch 2 times, most recently from a910d78 to 65c36f5 Compare November 20, 2025 21:19
@naji247

naji247 commented Nov 20, 2025

Copy link
Copy Markdown
Contributor Author

@deathbyknowledge refactored to use the getCurrentAgent utility and removed the less necessary buildExtra method like you asked! Also, I think all type checks are passing locally now too.

@naji247

naji247 commented Nov 21, 2025

Copy link
Copy Markdown
Contributor Author

Ah looks like the TS issue still exists... fixing now!

@naji247

naji247 commented Nov 24, 2025

Copy link
Copy Markdown
Contributor Author

@deathbyknowledge sorry to trouble you again, but do you think we can merge this in sometime this week? :)

@deathbyknowledge deathbyknowledge left a comment

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.

Sorry I missed this. Just need to remove one unnecessary bit and we can get this merged 👍

Comment thread packages/agents/src/mcp/utils.ts Outdated
Resolve conflict in worker.ts by keeping imports needed for
echoRequestInfo tool (RequestHandlerExtra, CallToolResult,
IsomorphicHeaders, ServerNotification, ServerRequest).
@naji247

naji247 commented Nov 25, 2025

Copy link
Copy Markdown
Contributor Author

@deathbyknowledge should be good to go :)

@deathbyknowledge

Copy link
Copy Markdown
Contributor

CI is failing, seems like a silly type error. Can you check with npm run check please?

@whoiskatrin

Copy link
Copy Markdown
Contributor

@deathbyknowledge should be good to go :)

@naji247, sorry to bug you, but type checks are failing

@naji247

naji247 commented Nov 25, 2025

Copy link
Copy Markdown
Contributor Author

Thanks @whoiskatrin I swear they passed locally :( fixed now I think

@naji247

naji247 commented Nov 25, 2025

Copy link
Copy Markdown
Contributor Author

Woo 🥳 they passed.

@deathbyknowledge
deathbyknowledge merged commit 93589e5 into cloudflare:main Nov 25, 2025
3 of 5 checks passed
@deathbyknowledge

Copy link
Copy Markdown
Contributor

Merged O7

@github-actions github-actions Bot mentioned this pull request Nov 25, 2025
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.

4 participants