Skip to content

Conversation

@howardjohn
Copy link
Contributor

Motivation and Context

This was broken by
#197.

The URL join behaves like this:

$ let baseUrl = "https://example.com/sse";
$ new URL("?sessionId=x", baseUrl).href
'https://example.com/sse?sessionId=x'
$ new URL("/?sessionId=x", baseUrl).href
'https://example.com/?sessionId=x'

The PR #197 did not take into account the relative URL

Fixes #252

How Has This Been Tested?

Yes, tested with agentgateway.dev which was broken by this change.

Breaking Changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@github-actions github-actions bot added T-core Core library changes T-transport Transport layer changes labels Jun 17, 2025
Copy link
Collaborator

@jokemanfire jokemanfire left a comment

Choose a reason for hiding this comment

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

I think wrap it as a func , It will be more conducive to maintenance.

@jokemanfire
Copy link
Collaborator

cc @4t145

This was broken by
modelcontextprotocol#197.

The URL join behaves like this:

```
$ let baseUrl = "https://example.com/sse";
$ new URL("?sessionId=x", baseUrl).href
'https://example.com/sse?sessionId=x'
$ new URL("/?sessionId=x", baseUrl).href
'https://example.com/?sessionId=x'
```

The PR modelcontextprotocol#197 did not take into account the relative URL

Fixes modelcontextprotocol#252
@4t145 4t145 merged commit 13e72ec into modelcontextprotocol:main Jun 19, 2025
10 of 11 checks passed
@github-actions github-actions bot mentioned this pull request Jul 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-core Core library changes T-transport Transport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Recent changes break some SSE serves due to URL parsing

3 participants