Skip to content

Conversation

@naji247
Copy link

@naji247 naji247 commented Nov 18, 2025

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
Copy link

changeset-bot bot commented Nov 18, 2025

🦋 Changeset detected

Latest commit: 2eb4334

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
Copy link

pkg-pr-new bot commented Nov 18, 2025

Open in StackBlitz

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

commit: 2eb4334

Copy link
Contributor

@threepointone threepointone left a comment

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
Copy link
Author

naji247 commented Nov 18, 2025

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.

Copy link
Contributor

@deathbyknowledge deathbyknowledge left a comment

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
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
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
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.

}

// Ensure transport is initialized before processing messages
if (!this._transport) {
Copy link
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
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 on lines 101 to 102
// Remove internal headers that are not part of the original request
delete headers["content-type"];
Copy link
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

return new McpSSETransport(() => this.getWebSocket());
return new McpSSETransport(
() => this.getWebSocket(),
this.getSessionId()
Copy link
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
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.

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
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
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
Copy link
Author

naji247 commented Nov 19, 2025

@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.

Copy link
Contributor

@deathbyknowledge deathbyknowledge left a comment

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 👍

@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
Copy link
Author

naji247 commented Nov 20, 2025

@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
Copy link
Author

naji247 commented Nov 21, 2025

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

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.

3 participants