From 93f56198da3c05d85e4dc9eb1c065f4a7f6a6f3f Mon Sep 17 00:00:00 2001 From: Jason Date: Wed, 8 Jul 2026 16:28:53 +0800 Subject: [PATCH] fix(codex): strip synced [mcp_servers] from provider snapshots on backfill MCP servers are owned by the DB mcp_servers table; the [mcp_servers] section in live config.toml is only a projection re-synced after every live write. When switching away baked that projection into the stored provider snapshot, servers deleted in the app were resurrected the next time that provider was activated -- per-entry reconcile only knows rows that still exist in the DB, so it can never clean up such orphans. Strip [mcp_servers] (and the legacy [mcp.servers] form) from the live settings during switch-away backfill. Previously polluted snapshots self-heal the next time the user switches away from them. --- src-tauri/src/codex_config.rs | 74 +++++++++++++++++++++++++ src-tauri/src/services/provider/live.rs | 45 +++++++++++++++ 2 files changed, 119 insertions(+) diff --git a/src-tauri/src/codex_config.rs b/src-tauri/src/codex_config.rs index c1da0e4c3..ddd2b4eb1 100644 --- a/src-tauri/src/codex_config.rs +++ b/src-tauri/src/codex_config.rs @@ -1473,6 +1473,45 @@ pub fn strip_codex_unified_session_bucket_from_settings( Ok(()) } +/// Backfill helper: strip `[mcp_servers]` from a live `{ auth, config }` +/// settings object before it is stored back to the DB. +/// +/// MCP 服务器的 SSOT 是 DB 的 mcp_servers 表，live `config.toml` 里的 +/// `[mcp_servers]` 只是每次写 live 之后由 MCP 同步重新投影的产物。若回填时 +/// 烙进供应商存储配置，已在应用里删除的服务器会随下次激活该供应商被写回 +/// live，而逐条 reconcile 只认识 DB 现存条目、永远清不掉这种孤儿。 +pub fn strip_codex_mcp_servers_from_settings(settings: &mut Value) -> Result<(), AppError> { + let Some(config_text) = settings + .get("config") + .and_then(|value| value.as_str()) + .map(str::to_string) + else { + return Ok(()); + }; + if !config_text.contains("mcp") { + return Ok(()); + } + let mut doc = config_text + .parse::() + .map_err(|e| AppError::Message(format!("Invalid Codex config.toml: {e}")))?; + let mut changed = doc.as_table_mut().remove("mcp_servers").is_some(); + // 历史错误格式 [mcp.servers] 一并清理（live 侧 MCP 同步也做同样迁移） + if let Some(mcp_tbl) = doc.get_mut("mcp").and_then(|item| item.as_table_like_mut()) { + if mcp_tbl.remove("servers").is_some() { + changed = true; + } + if mcp_tbl.is_empty() { + doc.as_table_mut().remove("mcp"); + } + } + if changed { + if let Some(obj) = settings.as_object_mut() { + obj.insert("config".to_string(), Value::String(doc.to_string())); + } + } + Ok(()) +} + /// Route a Codex live write between full auth+config or config-only. /// /// Official providers with usable login material own `auth.json`. Third-party @@ -1803,6 +1842,41 @@ requires_openai_auth = true assert!(settings.pointer("/auth/tokens/access_token").is_some()); } + #[test] + fn strip_mcp_servers_from_settings_removes_table_and_legacy_form() { + let mut settings = json!({ + "auth": { "OPENAI_API_KEY": "sk-test" }, + "config": "# user comment\nmodel = \"gpt-5.5\"\n\n[mcp_servers.echo]\ntype = \"stdio\"\ncommand = \"echo\"\n\n[mcp.servers.legacy]\ncommand = \"noop\"\n", + }); + strip_codex_mcp_servers_from_settings(&mut settings).expect("strip mcp"); + let config = settings + .get("config") + .and_then(|v| v.as_str()) + .expect("config text"); + assert!(!config.contains("mcp_servers"), "got: {config}"); + assert!( + !config.contains("[mcp"), + "legacy [mcp.servers] gone: {config}" + ); + assert!(config.contains("# user comment"), "comments preserved"); + assert!(config.contains("model = \"gpt-5.5\"")); + } + + #[test] + fn strip_mcp_servers_from_settings_is_noop_without_mcp() { + let original = "# comment\nmodel = \"gpt-5.5\"\n"; + let mut settings = json!({ + "auth": {}, + "config": original, + }); + strip_codex_mcp_servers_from_settings(&mut settings).expect("strip mcp"); + assert_eq!( + settings.get("config").and_then(|v| v.as_str()), + Some(original), + "config text must be byte-identical when nothing is stripped" + ); + } + #[test] fn extract_base_url_prefers_active_provider_section() { let input = r#"model_provider = "azure" diff --git a/src-tauri/src/services/provider/live.rs b/src-tauri/src/services/provider/live.rs index 961df149c..4fe2c8cf4 100644 --- a/src-tauri/src/services/provider/live.rs +++ b/src-tauri/src/services/provider/live.rs @@ -596,6 +596,15 @@ fn restore_live_settings_for_provider_backfill( ); } + // MCP 服务器归 DB mcp_servers 表所有，live 里的 [mcp_servers] 是同步投影； + // 回填时剥掉，否则已删除的服务器会随供应商快照复活（逐条 reconcile 清不掉孤儿）。 + if let Err(err) = crate::codex_config::strip_codex_mcp_servers_from_settings(&mut settings) { + log::warn!( + "Failed to strip mcp_servers while backfilling '{}': {err}", + provider.id + ); + } + // 统一会话开关注入的共享 `custom` 路由只属于 live 配置；切换回填时 // 必须剥掉，否则官方供应商的存储配置被污染，关闭开关后无法还原。 if provider.category.as_deref() == Some("official") { @@ -1860,4 +1869,40 @@ mod tests { "backfill must keep the Live-reconstructed catalog when the DB has none" ); } + + #[test] + fn codex_switch_backfill_strips_synced_mcp_servers() { + // Live 里的 [mcp_servers] 是 MCP 同步的投影（SSOT 在 DB 表）， + // 回填进供应商存储配置会让已删除的服务器随快照复活。 + let provider = Provider::with_id( + "prov".to_string(), + "Prov".to_string(), + json!({ + "auth": { "OPENAI_API_KEY": "sk-test" }, + "config": "model = \"gpt-5.5\"\n" + }), + None, + ); + + let live_settings = json!({ + "auth": { "OPENAI_API_KEY": "sk-test" }, + "config": "model = \"gpt-5.5\"\n\n[mcp_servers.echo]\ntype = \"stdio\"\ncommand = \"echo\"\n" + }); + + let result = + restore_live_settings_for_provider_backfill(&AppType::Codex, &provider, live_settings); + + let config_text = result + .get("config") + .and_then(|v| v.as_str()) + .expect("config text"); + assert!( + !config_text.contains("mcp_servers"), + "backfill must strip synced [mcp_servers] from the stored provider config, got: {config_text}" + ); + assert!( + config_text.contains("model = \"gpt-5.5\""), + "non-MCP content must survive the strip" + ); + } }