Skip to content

fix: infer HTTP for URL-only Codex MCP imports #85

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
brushax wants to merge 1 commit into
base: main
Choose a base branch
from
+61 −1
Open

fix: infer HTTP for URL-only Codex MCP imports #85

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src-tauri/src/mcp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,10 +440,18 @@ pub fn import_from_codex(config: &mut MultiAppConfig) -> Result<usize, AppError>
continue;
};

// type 缺省为 stdio
// Codex 的远程 MCP 可以只写 `url`，不显式提供 `type`。
// 这种情况下应按 HTTP 服务器导入，而不是回退到 stdio。
let typ = entry_tbl
.get("type")
.and_then(|v| v.as_str())
.or_else(|| {
entry_tbl
.get("url")
.and_then(|v| v.as_str())
.filter(|url| !url.trim().is_empty())
.map(|_| "http")
})
.unwrap_or("stdio");
Comment on lines 445 to 455
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new inference uses .get("type").and_then(|v| v.as_str()).or_else(...), which will infer http not only when type is absent, but also when type is present but not a string (e.g. type = 1). That diverges from the PR intent (“type is absent”) and can silently import misconfigured entries. Consider checking entry_tbl.contains_key("type") first (or filtering out non-string/empty values as a validation error) so inference only happens when the key is truly missing.

Suggested change
let typ = entry_tbl
.get("type")
.and_then(|v| v.as_str())
.or_else(|| {
entry_tbl
.get("url")
.and_then(|v| v.as_str())
.filter(|url| !url.trim().is_empty())
.map(|_| "http")
})
.unwrap_or("stdio");
let typ = if entry_tbl.contains_key("type") {
// 如果显式提供了 type，则仅使用该字段（缺失或类型不匹配时回退到 stdio），
// 不再根据 url 推断为 http，避免静默掩盖配置错误。
entry_tbl
.get("type")
.and_then(|v| v.as_str())
.unwrap_or("stdio")
} else {
// 仅在完全省略 type 时，才允许通过非空 url 推断为 http。
entry_tbl
.get("url")
.and_then(|v| v.as_str())
.filter(|url| !url.trim().is_empty())
.map(|_| "http")
.unwrap_or("stdio")
};

Copilot uses AI. Check for mistakes.
Comment on lines +443 to 455
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: if typ ends up unknown (including the case where type is an empty string), the later match typ { _ => { ... return changed; } } returns from the import_servers_tbl closure, which stops processing remaining servers in the same table. Since the intent is to “skip” invalid items, this should likely continue the per-item loop instead, and/or filter out empty/whitespace type values up front to avoid hitting the unknown-type path.

Copilot uses AI. Check for mistakes.

// 构建 JSON 规范
Expand Down
52 changes: 52 additions & 0 deletions src-tauri/tests/import_export_sync.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,58 @@ url = "https://example.com"
);
}

#[test]
fn import_from_codex_infers_http_when_url_is_present_without_type() {
let _guard = lock_test_mutex();
reset_test_fs();
let path = cc_switch_lib::get_codex_config_path();
if let Some(parent) = path.parent() {
fs::create_dir_all(parent).expect("create codex dir");
}
fs::write(
&path,
r#"[mcp_servers.cloudflare_api]
url = "https://mcp.cloudflare.com/mcp"

[mcp_servers.github]
url = "https://api.githubcopilot.com/mcp/"
bearer_token_env_var = "GITHUB_MCP_TOKEN"
"#,
)
.expect("write codex config");

let mut config = MultiAppConfig::default();
let changed = cc_switch_lib::import_from_codex(&mut config).expect("import codex");
assert_eq!(changed, 2, "should import both URL-based servers");

let servers = config
.mcp
.servers
.as_ref()
.expect("unified servers should exist");

let cloudflare = servers
.get("cloudflare_api")
.expect("cloudflare server should be imported");
assert!(
cloudflare.apps.codex,
"Codex app should be enabled for cloudflare_api"
);
assert_eq!(cloudflare.server["type"], "http");
assert_eq!(cloudflare.server["url"], "https://mcp.cloudflare.com/mcp");

let github = servers
.get("github")
.expect("github server should be imported");
assert!(github.apps.codex, "Codex app should be enabled for github");
assert_eq!(github.server["type"], "http");
assert_eq!(github.server["url"], "https://api.githubcopilot.com/mcp/");
assert_eq!(
github.server["bearer_token_env_var"], "GITHUB_MCP_TOKEN",
"custom Codex HTTP fields should be preserved during import"
);
}

#[test]
fn mcp_env_import_from_codex_preserves_env_table() {
let _guard = lock_test_mutex();
Expand Down
Loading