[MCP] Fix the bearer token authorization header#4846
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting
| } else { | ||
| let mut http_config = StreamableHttpClientTransportConfig::with_uri(url.to_string()); | ||
| if let Some(bearer_token) = bearer_token { | ||
| http_config = http_config.auth_header(format!("Bearer {bearer_token}")); | ||
| http_config = http_config.auth_header(bearer_token); |
There was a problem hiding this comment.
Don’t move bearer_token before building HTTP config
In new_streamable_http_client the initial OAuth tokens are computed with match bearer_token { … } and later the same bearer_token is matched again to populate the auth header. Because Option<String> is not Copy, the first match moves bearer_token and the later if let Some(bearer_token) = bearer_token cannot compile (value used after move). This commit therefore fails to build. Consider matching on a reference (bearer_token.as_ref()) or cloning before the first match.
Useful? React with 👍 / 👎.
http_config.auth_headerautomatically addedBearer. By adding it ourselves, we were sendingBearer Bearer <token>.I confirmed that the GitHub MCP initialization 400s before and works now.
I also optimized the oauth flow to not check the keyring if you explicitly pass in a bearer token.