fix: reconnect with fresh transport after OAuth completion#104
fix: reconnect with fresh transport after OAuth completion#104KentonYu wants to merge 2 commits intosteipete:mainfrom
Conversation
After a successful OAuth browser flow, `connectWithAuth` looped back to `client.connect(transport)` reusing the same StreamableHTTPClientTransport instance. The MCP SDK does not allow calling `start()` twice on a transport, so it threw "StreamableHTTPClientTransport already started!". Because this error was neither an `UnauthorizedError` nor an `OAuthTimeoutError`, the catch block in `createClientContext` fell through to the SSE fallback. Servers that only support Streamable HTTP (POST) then returned 404 on the SSE GET request, producing a confusing "SSE error: Non-200 status code (404)" — even though OAuth had already succeeded and tokens were persisted. The fix introduces `OAuthCompletedError`: after `finishAuth` succeeds, `connectWithAuth` throws this sentinel instead of retrying on the stale transport. `createClientContext` catches it and `continue`s the outer loop, which creates a brand-new Client + Transport with the now-cached tokens and connects cleanly.
|
@steipete Please help me review this PR. When we use mcporter auth, we can't use MCP services like notion normally |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c217031232
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The OAuthCompletedError handler was missing oauthSession.close(), leaking the callback server. The next loop iteration creates a new session; with a fixed oauthRedirectUrl port this causes EADDRINUSE.
|
@codex reivew |
|
I reproduced the same issue locally with Notion MCP on In my case, the browser OAuth flow completed successfully and tokens were saved, but the auth flow broke immediately after:
I first confirmed locally that recreating the transport after That said, after comparing approaches, the If you have a moment, I’d appreciate a review when you get the chance. 🙏 |
|
@KentonYu one follow-up from my testing: I reproduced this with Atlassian MCP when the initial HTTP path returned |
2 participants
After a successful OAuth browser flow,
connectWithAuthlooped back toclient.connect(transport)reusing the same StreamableHTTPClientTransport instance. The MCP SDK does not allow callingstart()twice on a transport, so it threw "StreamableHTTPClientTransport already started!".Because this error was neither an
UnauthorizedErrornor anOAuthTimeoutError, the catch block increateClientContextfell through to the SSE fallback. Servers that only support Streamable HTTP (POST) then returned 404 on the SSE GET request, producing a confusing "SSE error: Non-200 status code (404)" — even though OAuth had already succeeded and tokens were persisted.The fix introduces
OAuthCompletedError: afterfinishAuthsucceeds,connectWithAuththrows this sentinel instead of retrying on the stale transport.createClientContextcatches it andcontinues the outer loop, which creates a brand-new Client + Transport with the now-cached tokens and connects cleanly.