Conversation
When a server returns 401 and OAuth promotion occurs, the server has already proven it speaks Streamable HTTP (POST). Falling back to SSE (GET) after OAuth fails with 405 Method Not Allowed or a fresh 401 that can never be resolved. Skip the SSE fallback path when the active definition has auth set to oauth, and re-throw the original error instead. Fixes steipete#38 Fixes steipete#47 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84ae561d9e
ℹ️ 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".
| if (activeDefinition.auth === 'oauth') { | ||
| await oauthSession?.close().catch(() => {}); | ||
| throw primaryError; |
There was a problem hiding this comment.
Avoid skipping SSE fallback based on auth alone
activeDefinition.auth === 'oauth' is used here to skip SSE fallback for any primary error, but auth only describes the auth mechanism, not the transport (there’s no transport field in ServerDefinition). That means an OAuth‑protected server that only exposes SSE (e.g., a POST returning 405 or a network error on the streamable endpoint) will now fail permanently instead of falling back to SSE, even though SSE may be the only supported transport. Consider gating this on a confirmed streamable‑HTTP 401 (or another capability signal) rather than the auth flag alone.
Useful? React with 👍 / 👎.
Summary
createClientContextto skip the SSE fallback path whenactiveDefinition.auth === 'oauth', re-throwing the original error instead.Fixes #38
Fixes #47
Test plan
does not fall back to SSE when OAuth is active— verifies only StreamableHTTP is attempted and the error propagatesfalls back to SSE when primary connect fails— still passes (non-OAuth path unchanged)🤖 Generated with Claude Code