Skip to content

Commit 4d99512

Browse files
authored
[MCP] Dedicated error message for GitHub MCPs missing a personal access token (openai#5393)
Because the GitHub MCP is one of the most popular MCPs and it confusingly doesn't support OAuth, we should make it more clear how to make it work so people don't think Codex is broken.
1 parent 346b27d commit 4d99512

File tree

3 files changed

+133
-24
lines changed

3 files changed

+133
-24
lines changed

codex-rs/cli/src/mcp_cmd.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
402402
.map(|(name, cfg)| {
403403
let auth_status = auth_statuses
404404
.get(name.as_str())
405-
.copied()
405+
.map(|entry| entry.auth_status)
406406
.unwrap_or(McpAuthStatus::Unsupported);
407407
let transport = match &cfg.transport {
408408
McpServerTransportConfig::Stdio {
@@ -489,7 +489,7 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
489489
};
490490
let auth_status = auth_statuses
491491
.get(name.as_str())
492-
.copied()
492+
.map(|entry| entry.auth_status)
493493
.unwrap_or(McpAuthStatus::Unsupported)
494494
.to_string();
495495
stdio_rows.push([
@@ -514,7 +514,7 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
514514
};
515515
let auth_status = auth_statuses
516516
.get(name.as_str())
517-
.copied()
517+
.map(|entry| entry.auth_status)
518518
.unwrap_or(McpAuthStatus::Unsupported)
519519
.to_string();
520520
http_rows.push([

codex-rs/core/src/codex.rs

Lines changed: 117 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::borrow::Cow;
2+
use std::collections::HashMap;
23
use std::fmt::Debug;
34
use std::path::PathBuf;
45
use std::sync::Arc;
@@ -8,6 +9,7 @@ use crate::AuthManager;
89
use crate::client_common::REVIEW_PROMPT;
910
use crate::event_mapping::map_response_item_to_event_messages;
1011
use crate::function_tool::FunctionCallError;
12+
use crate::mcp::auth::McpAuthStatusEntry;
1113
use crate::parse_command::parse_command;
1214
use crate::review_format::format_review_findings_block;
1315
use crate::state::ItemCollector;
@@ -21,7 +23,6 @@ use codex_protocol::items::TurnItem;
2123
use codex_protocol::items::UserMessageItem;
2224
use codex_protocol::protocol::ConversationPathResponseEvent;
2325
use codex_protocol::protocol::ExitedReviewModeEvent;
24-
use codex_protocol::protocol::McpAuthStatus;
2526
use codex_protocol::protocol::ReviewRequest;
2627
use codex_protocol::protocol::RolloutItem;
2728
use codex_protocol::protocol::SessionSource;
@@ -55,6 +56,7 @@ use crate::client::ModelClient;
5556
use crate::client_common::Prompt;
5657
use crate::client_common::ResponseEvent;
5758
use crate::config::Config;
59+
use crate::config_types::McpServerTransportConfig;
5860
use crate::config_types::ShellEnvironmentPolicy;
5961
use crate::conversation_history::ConversationHistory;
6062
use crate::environment_context::EnvironmentContext;
@@ -506,22 +508,9 @@ impl Session {
506508
// Surface individual client start-up failures to the user.
507509
if !failed_clients.is_empty() {
508510
for (server_name, err) in failed_clients {
509-
let auth_status = auth_statuses.get(&server_name);
510-
let requires_login = match auth_status {
511-
Some(McpAuthStatus::NotLoggedIn) => true,
512-
Some(McpAuthStatus::OAuth) => is_mcp_client_auth_required_error(&err),
513-
_ => false,
514-
};
515-
let log_message =
516-
format!("MCP client for `{server_name}` failed to start: {err:#}");
517-
error!("{log_message}");
518-
let display_message = if requires_login {
519-
format!(
520-
"The {server_name} MCP server is not logged in. Run `codex mcp login {server_name}` to log in."
521-
)
522-
} else {
523-
log_message
524-
};
511+
let auth_entry = auth_statuses.get(&server_name);
512+
let display_message = mcp_init_error_display(&server_name, auth_entry, &err);
513+
warn!("MCP client for `{server_name}` failed to start: {err:#}");
525514
post_session_configured_error_events.push(Event {
526515
id: INITIAL_SUBMIT_ID.to_owned(),
527516
msg: EventMsg::Error(ErrorEvent {
@@ -1235,7 +1224,7 @@ async fn submission_loop(sess: Arc<Session>, config: Arc<Config>, rx_sub: Receiv
12351224

12361225
// This is a cheap lookup from the connection manager's cache.
12371226
let tools = sess.services.mcp_connection_manager.list_all_tools();
1238-
let (auth_statuses, resources, resource_templates) = tokio::join!(
1227+
let (auth_status_entries, resources, resource_templates) = tokio::join!(
12391228
compute_auth_statuses(
12401229
config.mcp_servers.iter(),
12411230
config.mcp_oauth_credentials_store_mode,
@@ -1245,6 +1234,10 @@ async fn submission_loop(sess: Arc<Session>, config: Arc<Config>, rx_sub: Receiv
12451234
.mcp_connection_manager
12461235
.list_all_resource_templates()
12471236
);
1237+
let auth_statuses = auth_status_entries
1238+
.iter()
1239+
.map(|(name, entry)| (name.clone(), entry.auth_status))
1240+
.collect();
12481241
let event = Event {
12491242
id: sub_id,
12501243
msg: EventMsg::McpListToolsResponse(
@@ -2312,6 +2305,36 @@ pub(crate) async fn exit_review_mode(
23122305
.await;
23132306
}
23142307

2308+
fn mcp_init_error_display(
2309+
server_name: &str,
2310+
entry: Option<&McpAuthStatusEntry>,
2311+
err: &anyhow::Error,
2312+
) -> String {
2313+
if let Some(McpServerTransportConfig::StreamableHttp {
2314+
url,
2315+
bearer_token_env_var,
2316+
http_headers,
2317+
..
2318+
}) = &entry.map(|entry| &entry.config.transport)
2319+
&& url == "https://api.githubcopilot.com/mcp/"
2320+
&& bearer_token_env_var.is_none()
2321+
&& http_headers.as_ref().map(HashMap::is_empty).unwrap_or(true)
2322+
{
2323+
// GitHub only supports OAUth for first party MCP clients.
2324+
// That means that the user has to specify a personal access token either via bearer_token_env_var or http_headers.
2325+
// https://github.com/github/github-mcp-server/issues/921#issuecomment-3221026448
2326+
format!(
2327+
"GitHub MCP does not support OAuth. Log in by adding `bearer_token_env_var = CODEX_GITHUB_PAT` in the `mcp_servers.{server_name}` section of your config.toml"
2328+
)
2329+
} else if is_mcp_client_auth_required_error(err) {
2330+
format!(
2331+
"The {server_name} MCP server is not logged in. Run `codex mcp login {server_name}`."
2332+
)
2333+
} else {
2334+
format!("MCP client for `{server_name}` failed to start: {err:#}")
2335+
}
2336+
}
2337+
23152338
fn is_mcp_client_auth_required_error(error: &anyhow::Error) -> bool {
23162339
// StreamableHttpError::AuthRequired from the MCP SDK.
23172340
error.to_string().contains("Auth required")
@@ -2325,7 +2348,10 @@ mod tests {
23252348
use super::*;
23262349
use crate::config::ConfigOverrides;
23272350
use crate::config::ConfigToml;
2351+
use crate::config_types::McpServerConfig;
2352+
use crate::config_types::McpServerTransportConfig;
23282353
use crate::exec::ExecToolCallOutput;
2354+
use crate::mcp::auth::McpAuthStatusEntry;
23292355
use crate::tools::format_exec_output_str;
23302356

23312357
use crate::protocol::CompactedItem;
@@ -2344,6 +2370,7 @@ mod tests {
23442370
use codex_app_server_protocol::AuthMode;
23452371
use codex_protocol::models::ContentItem;
23462372
use codex_protocol::models::ResponseItem;
2373+
use codex_protocol::protocol::McpAuthStatus;
23472374
use std::time::Duration;
23482375
use tokio::time::sleep;
23492376

@@ -3128,4 +3155,76 @@ mod tests {
31283155
pretty_assertions::assert_eq!(exec_output.metadata, ResponseExecMetadata { exit_code: 0 });
31293156
assert!(exec_output.output.contains("hi"));
31303157
}
3158+
3159+
#[test]
3160+
fn mcp_init_error_display_prompts_for_github_pat() {
3161+
let server_name = "github";
3162+
let entry = McpAuthStatusEntry {
3163+
config: McpServerConfig {
3164+
transport: McpServerTransportConfig::StreamableHttp {
3165+
url: "https://api.githubcopilot.com/mcp/".to_string(),
3166+
bearer_token_env_var: None,
3167+
http_headers: None,
3168+
env_http_headers: None,
3169+
},
3170+
enabled: true,
3171+
startup_timeout_sec: None,
3172+
tool_timeout_sec: None,
3173+
enabled_tools: None,
3174+
disabled_tools: None,
3175+
},
3176+
auth_status: McpAuthStatus::Unsupported,
3177+
};
3178+
let err = anyhow::anyhow!("OAuth is unsupported");
3179+
3180+
let display = mcp_init_error_display(server_name, Some(&entry), &err);
3181+
3182+
let expected = format!(
3183+
"GitHub MCP does not support OAuth. Log in by adding `bearer_token_env_var = CODEX_GITHUB_PAT` in the `mcp_servers.{server_name}` section of your config.toml"
3184+
);
3185+
3186+
assert_eq!(expected, display);
3187+
}
3188+
3189+
#[test]
3190+
fn mcp_init_error_display_prompts_for_login_when_auth_required() {
3191+
let server_name = "example";
3192+
let err = anyhow::anyhow!("Auth required for server");
3193+
3194+
let display = mcp_init_error_display(server_name, None, &err);
3195+
3196+
let expected = format!(
3197+
"The {server_name} MCP server is not logged in. Run `codex mcp login {server_name}`."
3198+
);
3199+
3200+
assert_eq!(expected, display);
3201+
}
3202+
3203+
#[test]
3204+
fn mcp_init_error_display_reports_generic_errors() {
3205+
let server_name = "custom";
3206+
let entry = McpAuthStatusEntry {
3207+
config: McpServerConfig {
3208+
transport: McpServerTransportConfig::StreamableHttp {
3209+
url: "https://example.com".to_string(),
3210+
bearer_token_env_var: Some("TOKEN".to_string()),
3211+
http_headers: None,
3212+
env_http_headers: None,
3213+
},
3214+
enabled: true,
3215+
startup_timeout_sec: None,
3216+
tool_timeout_sec: None,
3217+
enabled_tools: None,
3218+
disabled_tools: None,
3219+
},
3220+
auth_status: McpAuthStatus::Unsupported,
3221+
};
3222+
let err = anyhow::anyhow!("boom");
3223+
3224+
let display = mcp_init_error_display(server_name, Some(&entry), &err);
3225+
3226+
let expected = format!("MCP client for `{server_name}` failed to start: {err:#}");
3227+
3228+
assert_eq!(expected, display);
3229+
}
31313230
}

codex-rs/core/src/mcp/auth.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,25 +10,35 @@ use tracing::warn;
1010
use crate::config_types::McpServerConfig;
1111
use crate::config_types::McpServerTransportConfig;
1212

13+
#[derive(Debug, Clone)]
14+
pub struct McpAuthStatusEntry {
15+
pub config: McpServerConfig,
16+
pub auth_status: McpAuthStatus,
17+
}
18+
1319
pub async fn compute_auth_statuses<'a, I>(
1420
servers: I,
1521
store_mode: OAuthCredentialsStoreMode,
16-
) -> HashMap<String, McpAuthStatus>
22+
) -> HashMap<String, McpAuthStatusEntry>
1723
where
1824
I: IntoIterator<Item = (&'a String, &'a McpServerConfig)>,
1925
{
2026
let futures = servers.into_iter().map(|(name, config)| {
2127
let name = name.clone();
2228
let config = config.clone();
2329
async move {
24-
let status = match compute_auth_status(&name, &config, store_mode).await {
30+
let auth_status = match compute_auth_status(&name, &config, store_mode).await {
2531
Ok(status) => status,
2632
Err(error) => {
2733
warn!("failed to determine auth status for MCP server `{name}`: {error:?}");
2834
McpAuthStatus::Unsupported
2935
}
3036
};
31-
(name, status)
37+
let entry = McpAuthStatusEntry {
38+
config,
39+
auth_status,
40+
};
41+
(name, entry)
3242
}
3343
});
3444

0 commit comments

Comments
 (0)