external-agent-migration: avoid mixed MCP transport configs

codex-rs/app-server/src/config/external_agent_config.rs

@@ -316,9 +316,8 @@ impl ExternalAgentConfigService {
             Some(self.external_agent_home.as_path()),
             mcp_settings.as_ref(),
         )?;
-        let mcp_server_names = migrated_mcp_server_names(&migrated_mcp);
+        let mut mcp_server_names = migrated_mcp_server_names(&migrated_mcp);
         if !is_empty_toml_table(&migrated_mcp) {
-            let mut should_include = true;
             if target_config.exists() {
                 let existing_raw = fs::read_to_string(&target_config)?;
                 let mut existing = if existing_raw.trim().is_empty() {
@@ -328,10 +327,10 @@ impl ExternalAgentConfigService {
                         invalid_data_error(format!("invalid existing config.toml: {err}"))
                     })?
                 };
-                should_include = merge_missing_toml_values(&mut existing, &migrated_mcp)?;
+                mcp_server_names = merge_missing_mcp_servers(&mut existing, &migrated_mcp)?;
             }
 
-            if should_include {
+            if !mcp_server_names.is_empty() {
                 items.push(ExternalAgentConfigMigrationItem {
                     item_type: ExternalAgentConfigMigrationItemType::McpServerConfig,
                     description: format!(
@@ -341,7 +340,7 @@ impl ExternalAgentConfigService {
                     ),
                     cwd: cwd.clone(),
                     details: Some(MigrationDetails {
-                        mcp_servers: named_migrations(mcp_server_names.clone()),
+                        mcp_servers: named_migrations(mcp_server_names),
                         ..Default::default()
                     }),
                 });
@@ -863,7 +862,7 @@ impl ExternalAgentConfigService {
             toml::from_str::<TomlValue>(&existing_raw)
                 .map_err(|err| invalid_data_error(format!("invalid existing config.toml: {err}")))?
         };
-        if merge_missing_toml_values(&mut existing, &migrated)? {
+        if !merge_missing_mcp_servers(&mut existing, &migrated)?.is_empty() {
             write_toml_file(&target_config, &existing)?;
         }
         Ok(())
@@ -1538,6 +1537,43 @@ fn merge_missing_toml_values(existing: &mut TomlValue, incoming: &TomlValue) ->
     }
 }
 
+fn merge_missing_mcp_servers(
+    existing: &mut TomlValue,
+    incoming: &TomlValue,
+) -> io::Result<Vec<String>> {
+    let existing_root = existing
+        .as_table_mut()
+        .ok_or_else(|| invalid_data_error("expected existing config to be a TOML table"))?;
+    let incoming_root = incoming
+        .as_table()
+        .ok_or_else(|| invalid_data_error("expected migrated MCP config to be a TOML table"))?;
+    let Some(incoming_servers) = incoming_root.get("mcp_servers") else {
+        return Ok(Vec::new());
+    };
+    let incoming_servers = incoming_servers
+        .as_table()
+        .ok_or_else(|| invalid_data_error("expected migrated MCP servers to be a TOML table"))?;
+    let Some(existing_servers) = existing_root.get_mut("mcp_servers") else {
+        existing_root.insert(
+            "mcp_servers".to_string(),
+            TomlValue::Table(incoming_servers.clone()),
+        );
+        return Ok(incoming_servers.keys().cloned().collect());
+    };
+    let Some(existing_servers) = existing_servers.as_table_mut() else {
+        return Ok(Vec::new());
+    };
+
+    let mut merged_server_names = Vec::new();
+    for (server_name, incoming_server) in incoming_servers {
+        if !existing_servers.contains_key(server_name) {
+            existing_servers.insert(server_name.clone(), incoming_server.clone());
+            merged_server_names.push(server_name.clone());
+        }
+    }
+    Ok(merged_server_names)
+}
+
 fn write_toml_file(path: &Path, value: &TomlValue) -> io::Result<()> {
     let serialized = toml::to_string_pretty(value)
         .map_err(|err| invalid_data_error(format!("failed to serialize config.toml: {err}")))?;

codex-rs/app-server/src/config/external_agent_config_tests.rs

@@ -638,6 +638,124 @@ Research with Codex carefully."""
     assert_eq!(agent, expected_agent);
 }
 
+#[tokio::test]
+async fn import_repo_mcp_preserves_existing_same_named_server() {
+    let root = TempDir::new().expect("create tempdir");
+    let repo_root = root.path().join("repo");
+    fs::create_dir_all(repo_root.join(".git")).expect("create git dir");
+    fs::write(
+        repo_root.join(".mcp.json"),
+        r#"{
+          "mcpServers": {
+            "mixedTransport": {
+              "command": "mcp-remote-proxy",
+              "args": [
+                "https://example.com/mixed-transport",
+                "--transport",
+                "http"
+              ],
+              "url": "https://example.com/mixed-transport"
+            }
+          }
+        }"#,
+    )
+    .expect("write mcp");
+    fs::create_dir_all(repo_root.join(".codex")).expect("create codex dir");
+    let existing_config = r#"[mcp_servers.mixedTransport]
+url = "https://example.com/mixed-transport"
+"#;
+    fs::write(
+        repo_root.join(".codex").join("config.toml"),
+        existing_config,
+    )
+    .expect("write config");
+
+    let service = service_for_paths(
+        root.path().join(EXTERNAL_AGENT_DIR),
+        root.path().join(".codex"),
+    );
+    assert_eq!(
+        service
+            .detect(ExternalAgentConfigDetectOptions {
+                include_home: false,
+                cwds: Some(vec![repo_root.clone()]),
+            })
+            .await
+            .expect("detect"),
+        Vec::<ExternalAgentConfigMigrationItem>::new()
+    );
+
+    service
+        .import(vec![ExternalAgentConfigMigrationItem {
+            item_type: ExternalAgentConfigMigrationItemType::McpServerConfig,
+            description: String::new(),
+            cwd: Some(repo_root.clone()),
+            details: None,
+        }])
+        .await
+        .expect("import");
+
+    assert_eq!(
+        fs::read_to_string(repo_root.join(".codex").join("config.toml")).expect("read config"),
+        existing_config
+    );
+}
+
+#[tokio::test]
+async fn detect_repo_mcp_lists_only_missing_servers() {
+    let root = TempDir::new().expect("create tempdir");
+    let repo_root = root.path().join("repo");
+    fs::create_dir_all(repo_root.join(".git")).expect("create git dir");
+    fs::write(
+        repo_root.join(".mcp.json"),
+        r#"{
+          "mcpServers": {
+            "docs": {"command": "docs-server"},
+            "mixedTransport": {"command": "mcp-remote-proxy"}
+          }
+        }"#,
+    )
+    .expect("write mcp");
+    fs::create_dir_all(repo_root.join(".codex")).expect("create codex dir");
+    fs::write(
+        repo_root.join(".codex").join("config.toml"),
+        r#"[mcp_servers.mixedTransport]
+url = "https://example.com/mixed-transport"
+"#,
+    )
+    .expect("write config");
+
+    let items = service_for_paths(
+        root.path().join(EXTERNAL_AGENT_DIR),
+        root.path().join(".codex"),
+    )
+    .detect(ExternalAgentConfigDetectOptions {
+        include_home: false,
+        cwds: Some(vec![repo_root.clone()]),
+    })
+    .await
+    .expect("detect");
+
+    assert_eq!(
+        items,
+        vec![ExternalAgentConfigMigrationItem {
+            item_type: ExternalAgentConfigMigrationItemType::McpServerConfig,
+            description: format!(
+                "Migrate MCP servers from {} into {}",
+                repo_root.display(),
+                repo_root.join(".codex").join("config.toml").display()
+            ),
+            cwd: Some(repo_root),
+            details: Some(MigrationDetails {
+                mcp_servers: vec![NamedMigration {
+                    name: "docs".to_string(),
+                }],
+                ..Default::default()
+            }),
+        }]
+    );
+}
+
 #[tokio::test]
 async fn import_home_migrates_supported_config_fields_skills_and_agents_md() {
     let (_root, external_agent_home, codex_home) = fixture_paths();

codex-rs/external-agent-migration/src/lib.rs

@@ -1451,6 +1451,49 @@ mod tests {
         );
     }
 
+    #[test]
+    fn mcp_migration_prefers_command_transport_for_mixed_server_config() {
+        let root = tempfile::TempDir::new().expect("tempdir");
+        fs::write(
+            root.path().join(".mcp.json"),
+            r#"{
+              "mcpServers": {
+                "mixedTransport": {
+                  "command": "mcp-remote-proxy",
+                  "args": [
+                    "https://example.com/mixed-transport",
+                    "--transport",
+                    "http"
+                  ],
+                  "url": "https://example.com/mixed-transport"
+                }
+              }
+            }"#,
+        )
+        .expect("write mcp");
+
+        assert_eq!(
+            build_mcp_config_from_external(
+                root.path(),
+                /*external_agent_home*/ None,
+                /*settings*/ None,
+            )
+            .unwrap(),
+            toml::from_str(
+                r#"
+[mcp_servers.mixedTransport]
+command = "mcp-remote-proxy"
+args = [
+  "https://example.com/mixed-transport",
+  "--transport",
+  "http",
+]
+"#
+            )
+            .unwrap()
+        );
+    }
+
     #[test]
     fn mcp_migration_skips_unsupported_transports() {
         let root = tempfile::TempDir::new().expect("tempdir");