openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-05 20:10:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

Tailor MCP doctor remediation to detected problems (#59057)

Browse Source 
## Why

MCP checks recommended setting missing environment variables even when the problem was an invalid server configuration or an unreachable URL.

## What changed

Separate configuration errors from missing environment variables and build remediation guidance from all detected issue types. Include server reachability advice for failed HTTP probes, while retaining the option to disable affected servers.

## Testing

Add remediation snapshots for unreachable optional HTTP servers, missing required stdio commands, and a required HTTP server with both a missing token and an unreachable URL.

GitOrigin-RevId: 8e8eb743414297561793be51cf3a25e5bd664f27
This commit is contained in:
Eric Traut
2026-11-03 04:20:25 +00:00
committed by copyberry
parent 3f958ac2fe
commit c5122161fe
1 changed files with 50 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/cli/src/doctor.rs
+50 -16
View File
@@ -1543,6 +1543,7 @@ async fn mcp_check_from_servers(servers: &HashMap<String, McpServerConfig>) -> D
let mut details = Vec::new();
let mut transport_counts: BTreeMap<&'static str, usize> = BTreeMap::new();
let mut disabled = 0usize;
let mut invalid_config = Vec::new();
let mut missing_env = Vec::new();
let mut unreachable_required_http = Vec::new();
let mut unreachable_optional_http = Vec::new();
@@ -1566,43 +1567,44 @@ async fn mcp_check_from_servers(servers: &HashMap<String, McpServerConfig>) -> D
}
let command_is_empty = command.trim().is_empty();
if command_is_empty {
missing_env.push(format!("{name}: stdio command is empty"));
invalid_config.push(format!("{name}: stdio command is empty"));
}
if server.is_local_environment() {
let host_native_cwd = cwd.as_ref().map(|cwd| Path::new(cwd.as_str()));
if let Some(cwd) = host_native_cwd
&& !cwd.exists()
{
missing_env.push(format!("{name}: cwd does not exist ({})", cwd.display()));
invalid_config
.push(format!("{name}: cwd does not exist ({})", cwd.display()));
}
if !command_is_empty
&& let Err(err) =
stdio_command_resolves(command, host_native_cwd, env.as_ref())
{
missing_env.push(format!(
invalid_config.push(format!(
"{name}: stdio command {command:?} is not resolvable ({err})"
));
}
} else {
match cwd {
Some(cwd) if cwd.to_inferred_path_uri().is_none() => {
missing_env
invalid_config
.push(format!("{name}: remote stdio cwd is not absolute ({cwd})"));
}
None => missing_env
None => invalid_config
.push(format!("{name}: remote stdio requires an explicit cwd")),
Some(_) => {}
}
}
if let Some(env) = env {
for key in env.keys().filter(|key| key.trim().is_empty()) {
missing_env.push(format!("{name}: empty env key {key}"));
invalid_config.push(format!("{name}: empty env key {key}"));
}
}
for env_var in env_vars {
if env_var.is_remote_source() {
if server.is_local_environment() {
missing_env.push(format!(
invalid_config.push(format!(
"{name}: env_vars entry `{}` uses source `remote`, which requires remote MCP stdio",
env_var.name()
));
@@ -1652,7 +1654,25 @@ async fn mcp_check_from_servers(servers: &HashMap<String, McpServerConfig>) -> D
for (transport, count) in transport_counts {
details.push(format!("{transport} servers: {count}"));
}
details.extend(missing_env.iter().cloned());
let mut hints = Vec::new();
if !invalid_config.is_empty() {
hints.push("Correct the listed MCP server configuration errors.");
}
if !missing_env.is_empty() {
hints.push("Set the missing MCP env vars.");
}
if !unreachable_required_http.is_empty() || !unreachable_optional_http.is_empty() {
hints.push("Check that the MCP server is running and its URL is reachable.");
}
let required_missing = servers.iter().any(|(name, server)| {
server.required
&& invalid_config
.iter()
.chain(&missing_env)
.any(|detail| detail.starts_with(&format!("{name}:")))
});
details.extend(invalid_config);
details.extend(missing_env);
details.extend(
unreachable_required_http
.iter()
@@ -1664,15 +1684,9 @@ async fn mcp_check_from_servers(servers: &HashMap<String, McpServerConfig>) -> D
.map(|detail| format!("optional reachability failed: {detail}")),
);
let required_missing = servers.iter().any(|(name, server)| {
server.required
&& missing_env
.iter()
.any(|missing| missing.starts_with(&format!("{name}:")))
});
let status = if required_missing || !unreachable_required_http.is_empty() {
CheckStatus::Fail
} else if !missing_env.is_empty() || !unreachable_optional_http.is_empty() {
} else if !hints.is_empty() {
CheckStatus::Warning
} else {
CheckStatus::Ok
@@ -1685,7 +1699,8 @@ async fn mcp_check_from_servers(servers: &HashMap<String, McpServerConfig>) -> D
let mut check = DoctorCheck::new("mcp.config", "mcp", status, summary).details(details);
if status != CheckStatus::Ok {
check = check.remediation("Set the missing MCP env vars or disable the affected server.");
hints.push("Alternatively, disable the affected server.");
check = check.remediation(hints.join(" "));
}
check
}
@@ -3612,6 +3627,7 @@ mod tests {
assert_eq!(check.status, CheckStatus::Warning);
assert_eq!(check.summary, "MCP configuration has optional issues");
insta::assert_snapshot!(check.remediation.unwrap(), @"Check that the MCP server is running and its URL is reachable. Alternatively, disable the affected server.");
assert!(
check
.details
@@ -3620,6 +3636,23 @@ mod tests {
);
}
#[tokio::test]
async fn mcp_check_reports_both_missing_token_and_unreachable_url() {
let server = toml::from_str(
r#"
url = "http://127.0.0.1:9/mcp"
bearer_token_env_var = "CODEX_DOCTOR_MISSING_MCP_TOKEN"
required = true
"#,
)
.expect("required MCP config");
let check =
mcp_check_from_servers(&HashMap::from([("required".to_string(), server)])).await;
assert_eq!(check.status, CheckStatus::Fail);
insta::assert_snapshot!(check.remediation.unwrap(), @"Set the missing MCP env vars. Check that the MCP server is running and its URL is reachable. Alternatively, disable the affected server.");
}
#[tokio::test]
async fn mcp_check_fails_required_remote_stdio_env_var() {
let command = toml::Value::String(
@@ -4195,6 +4228,7 @@ mod tests {
check.summary,
"MCP configuration has failing required inputs or reachability"
);
insta::assert_snapshot!(check.remediation.unwrap(), @"Correct the listed MCP server configuration errors. Alternatively, disable the affected server.");
assert!(check.details.iter().any(|detail| {
detail.contains(
"required: stdio command \"definitely-missing-codex-doctor-mcp\" is not resolvable",