| 1 | //! `codewhale doctor --fix` repair planning and application (#5552, v1). |
| 2 | //! |
| 3 | //! The doctor is a read-only diagnostic by default. `--fix` computes a |
| 4 | //! concrete repair plan first — pure detection, no mutation — shows it, and |
| 5 | //! applies it only after explicit consent (or `--yes`). Every v1 action is |
| 6 | //! narrowly scoped and reversible: |
| 7 | //! |
| 8 | //! - delete stale `.tmp*` files left behind by interrupted atomic writes in |
| 9 | //! the Codewhale home; |
| 10 | //! - tighten secret-store file permissions to `0600` on Unix when group or |
| 11 | //! world bits are set (the secret store writes private files, but a file |
| 12 | //! restored from backup or moved from another machine may have drifted); |
| 13 | //! - disable MCP entries whose structural check reports `Error` (no command |
| 14 | //! and no URL, or an empty command) by writing `enabled: false` through the |
| 15 | //! same atomic save path the MCP editor uses; |
| 16 | //! - scaffold the user-global `skills`, `tools`, and `plugins` directories |
| 17 | //! when they are missing so discovery surfaces stop reporting them absent. |
| 18 | //! |
| 19 | //! Explicitly out of scope for v1: completion registration, launch-record |
| 20 | //! repair, and config.toml credential scrubbing (each needs its own consent |
| 21 | //! story; the doctor still *reports* the credential-shaped keys). |
| 22 | |
| 23 | use std::path::{Path, PathBuf}; |
| 24 | |
| 25 | use anyhow::Result; |
| 26 | |
| 27 | use crate::McpServerDoctorStatus; |
| 28 | use crate::mcp; |
| 29 | |
| 30 | /// One concrete, reversible repair the doctor can apply. |
| 31 | #[derive(Debug, Clone, PartialEq, Eq)] |
| 32 | pub(crate) enum DoctorFixAction { |
| 33 | /// Delete an interrupted-atomic-write leftover. Only ever a regular file |
| 34 | /// whose name starts with `.tmp` directly inside the Codewhale home. |
| 35 | DeleteStaleTempFile { path: PathBuf }, |
| 36 | /// Restrict a secret-store file to owner-only on Unix. |
| 37 | #[cfg(unix)] |
| 38 | TightenSecretPermissions { path: PathBuf, from_mode: u32 }, |
| 39 | /// Disable a structurally broken MCP entry by name in the given config. |
| 40 | DisableMcpServer { |
| 41 | config_path: PathBuf, |
| 42 | server: String, |
| 43 | }, |
| 44 | /// Create a missing user-global discovery directory. |
| 45 | ScaffoldDirectory { path: PathBuf }, |
| 46 | } |
| 47 | |
| 48 | /// The full repair plan for one doctor run. |
| 49 | #[derive(Debug, Clone, Default, PartialEq, Eq)] |
| 50 | pub(crate) struct DoctorFixPlan { |
| 51 | pub(crate) actions: Vec<DoctorFixAction>, |
| 52 | } |
| 53 | |
| 54 | impl DoctorFixPlan { |
| 55 | pub(crate) fn is_empty(&self) -> bool { |
| 56 | self.actions.is_empty() |
| 57 | } |
| 58 | |
| 59 | pub(crate) fn len(&self) -> usize { |
| 60 | self.actions.len() |
| 61 | } |
| 62 | |
| 63 | /// One human-readable line per action, in a stable order. |
| 64 | pub(crate) fn describe(&self) -> Vec<String> { |
| 65 | self.actions |
| 66 | .iter() |
| 67 | .map(|action| match action { |
| 68 | DoctorFixAction::DeleteStaleTempFile { path } => { |
| 69 | format!( |
| 70 | "delete stale temp file {}", |
| 71 | crate::utils::display_path(path) |
| 72 | ) |
| 73 | } |
| 74 | #[cfg(unix)] |
| 75 | DoctorFixAction::TightenSecretPermissions { path, from_mode } => format!( |
| 76 | "restrict {} to 0600 (currently {:o})", |
| 77 | crate::utils::display_path(path), |
| 78 | from_mode & 0o7777 |
| 79 | ), |
| 80 | DoctorFixAction::DisableMcpServer { |
| 81 | config_path, |
| 82 | server, |
| 83 | } => format!( |
| 84 | "disable broken MCP server entry '{server}' in {}", |
| 85 | crate::utils::display_path(config_path) |
| 86 | ), |
| 87 | DoctorFixAction::ScaffoldDirectory { path } => { |
| 88 | format!( |
| 89 | "create missing directory {}", |
| 90 | crate::utils::display_path(path) |
| 91 | ) |
| 92 | } |
| 93 | }) |
| 94 | .collect() |
| 95 | } |
| 96 | } |
| 97 | |
| 98 | /// Stale `.tmp*` regular files directly inside the Codewhale home. |
| 99 | fn stale_temp_files() -> Vec<PathBuf> { |
| 100 | let Ok(home) = codewhale_config::codewhale_home() else { |
| 101 | return Vec::new(); |
| 102 | }; |
| 103 | let Ok(entries) = std::fs::read_dir(&home) else { |
| 104 | return Vec::new(); |
| 105 | }; |
| 106 | let mut files = entries |
| 107 | .flatten() |
| 108 | .filter(|entry| { |
| 109 | entry.file_name().to_string_lossy().starts_with(".tmp") |
| 110 | && entry.file_type().is_ok_and(|kind| kind.is_file()) |
| 111 | }) |
| 112 | .map(|entry| entry.path()) |
| 113 | .collect::<Vec<_>>(); |
| 114 | files.sort(); |
| 115 | files |
| 116 | } |
| 117 | |
| 118 | /// Secret-store files whose Unix permissions are looser than 0600. |
| 119 | #[cfg(unix)] |
| 120 | fn secret_files_needing_tightening() -> Vec<(PathBuf, u32)> { |
| 121 | use std::os::unix::fs::PermissionsExt; |
| 122 | |
| 123 | let Ok(paths) = codewhale_secrets::FileKeyringStore::default_paths_read_only() else { |
| 124 | return Vec::new(); |
| 125 | }; |
| 126 | let candidates = std::iter::once(paths.0).chain(paths.1); |
| 127 | candidates |
| 128 | .filter_map(|path| { |
| 129 | let metadata = std::fs::metadata(&path).ok()?; |
| 130 | if !metadata.is_file() { |
| 131 | return None; |
| 132 | } |
| 133 | let mode = metadata.permissions().mode(); |
| 134 | (mode & 0o077 != 0).then_some((path, mode)) |
| 135 | }) |
| 136 | .collect() |
| 137 | } |
| 138 | |
| 139 | /// MCP entries whose structural check reports `Error`, by config file. |
| 140 | fn broken_mcp_entries( |
| 141 | config: &crate::config::Config, |
| 142 | workspace: &Path, |
| 143 | plugins: &crate::plugins::PluginRegistry, |
| 144 | ) -> Vec<(PathBuf, Vec<String>)> { |
| 145 | let global_path = config.mcp_config_path(); |
| 146 | let project_path = mcp::workspace_mcp_config_path(workspace); |
| 147 | let mut broken = Vec::new(); |
| 148 | for path in [global_path, project_path] { |
| 149 | let Ok(cfg) = mcp::load_config_with_workspace_and_plugins(&path, workspace, plugins) else { |
| 150 | continue; |
| 151 | }; |
| 152 | let names = cfg |
| 153 | .servers |
| 154 | .iter() |
| 155 | .filter(|(_, server)| { |
| 156 | server.is_enabled() |
| 157 | && matches!( |
| 158 | crate::doctor_check_mcp_server(server), |
| 159 | McpServerDoctorStatus::Error(_) |
| 160 | ) |
| 161 | }) |
| 162 | .map(|(name, _)| name.clone()) |
| 163 | .collect::<Vec<_>>(); |
| 164 | if !names.is_empty() { |
| 165 | broken.push((path, names)); |
| 166 | } |
| 167 | } |
| 168 | broken |
| 169 | } |
| 170 | |
| 171 | /// User-global discovery directories the product expects to exist. |
| 172 | fn missing_user_directories(config: &crate::config::Config) -> Vec<PathBuf> { |
| 173 | let candidates = [ |
| 174 | config.skills_dir(), |
| 175 | crate::default_tools_dir(), |
| 176 | crate::default_plugins_dir(), |
| 177 | ]; |
| 178 | let mut missing = candidates |
| 179 | .into_iter() |
| 180 | .filter(|dir| !dir.exists()) |
| 181 | .collect::<Vec<_>>(); |
| 182 | missing.sort(); |
| 183 | missing |
| 184 | } |
| 185 | |
| 186 | /// Compute the repair plan. Pure: reads state, mutates nothing. |
| 187 | pub(crate) fn plan_fixes( |
| 188 | config: &crate::config::Config, |
| 189 | workspace: &Path, |
| 190 | plugins: &crate::plugins::PluginRegistry, |
| 191 | ) -> DoctorFixPlan { |
| 192 | let mut actions = Vec::new(); |
| 193 | actions.extend( |
| 194 | stale_temp_files() |
| 195 | .into_iter() |
| 196 | .map(|path| DoctorFixAction::DeleteStaleTempFile { path }), |
| 197 | ); |
| 198 | #[cfg(unix)] |
| 199 | actions.extend( |
| 200 | secret_files_needing_tightening() |
| 201 | .into_iter() |
| 202 | .map(|(path, from_mode)| DoctorFixAction::TightenSecretPermissions { path, from_mode }), |
| 203 | ); |
| 204 | for (config_path, servers) in broken_mcp_entries(config, workspace, plugins) { |
| 205 | actions.extend( |
| 206 | servers |
| 207 | .into_iter() |
| 208 | .map(|server| DoctorFixAction::DisableMcpServer { |
| 209 | config_path: config_path.clone(), |
| 210 | server, |
| 211 | }), |
| 212 | ); |
| 213 | } |
| 214 | actions.extend( |
| 215 | missing_user_directories(config) |
| 216 | .into_iter() |
| 217 | .map(|path| DoctorFixAction::ScaffoldDirectory { path }), |
| 218 | ); |
| 219 | DoctorFixPlan { actions } |
| 220 | } |
| 221 | |
| 222 | /// Outcome of applying one action. |
| 223 | #[derive(Debug, Clone, PartialEq, Eq)] |
| 224 | pub(crate) enum DoctorFixOutcome { |
| 225 | Applied, |
| 226 | Failed(String), |
| 227 | } |
| 228 | |
| 229 | /// Apply a plan. Each action reports its own outcome; one failure never |
| 230 | /// blocks the rest. Actions are idempotent, so a re-run converges. |
| 231 | pub(crate) fn apply_fixes(plan: &DoctorFixPlan) -> Vec<(DoctorFixAction, DoctorFixOutcome)> { |
| 232 | plan.actions |
| 233 | .iter() |
| 234 | .cloned() |
| 235 | .map(|action| { |
| 236 | let outcome = apply_one(&action); |
| 237 | (action, outcome) |
| 238 | }) |
| 239 | .collect() |
| 240 | } |
| 241 | |
| 242 | fn apply_one(action: &DoctorFixAction) -> DoctorFixOutcome { |
| 243 | match action { |
| 244 | DoctorFixAction::DeleteStaleTempFile { path } => { |
| 245 | // Re-verify the deletion guard at apply time: the plan was |
| 246 | // computed before consent, so the file may have changed. |
| 247 | let still_stale = path |
| 248 | .file_name() |
| 249 | .and_then(|name| name.to_str()) |
| 250 | .is_some_and(|name| name.starts_with(".tmp")) |
| 251 | && std::fs::symlink_metadata(path) |
| 252 | .ok() |
| 253 | .is_some_and(|meta| meta.is_file()); |
| 254 | if !still_stale { |
| 255 | return DoctorFixOutcome::Applied; |
| 256 | } |
| 257 | match std::fs::remove_file(path) { |
| 258 | Ok(()) => DoctorFixOutcome::Applied, |
| 259 | Err(error) => DoctorFixOutcome::Failed(error.to_string()), |
| 260 | } |
| 261 | } |
| 262 | #[cfg(unix)] |
| 263 | DoctorFixAction::TightenSecretPermissions { path, .. } => { |
| 264 | use std::os::unix::fs::PermissionsExt; |
| 265 | match std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600)) { |
| 266 | Ok(()) => DoctorFixOutcome::Applied, |
| 267 | Err(error) => DoctorFixOutcome::Failed(error.to_string()), |
| 268 | } |
| 269 | } |
| 270 | DoctorFixAction::DisableMcpServer { |
| 271 | config_path, |
| 272 | server, |
| 273 | } => match disable_mcp_server(config_path, server) { |
| 274 | Ok(()) => DoctorFixOutcome::Applied, |
| 275 | Err(error) => DoctorFixOutcome::Failed(format!("{error:#}")), |
| 276 | }, |
| 277 | DoctorFixAction::ScaffoldDirectory { path } => match std::fs::create_dir_all(path) { |
| 278 | Ok(()) => DoctorFixOutcome::Applied, |
| 279 | Err(error) => DoctorFixOutcome::Failed(error.to_string()), |
| 280 | }, |
| 281 | } |
| 282 | } |
| 283 | |
| 284 | /// Disable one server entry in an MCP config file through the atomic save |
| 285 | /// path the MCP editor uses. A missing file or missing server is a no-op |
| 286 | /// (the plan may be stale after consent). |
| 287 | fn disable_mcp_server(config_path: &Path, server_name: &str) -> Result<()> { |
| 288 | mcp::mutate_config(config_path, None, |cfg| { |
| 289 | if let Some(server) = cfg.servers.get_mut(server_name) { |
| 290 | server.enabled = false; |
| 291 | server.disabled = true; |
| 292 | } |
| 293 | Ok(()) |
| 294 | }) |
| 295 | .map(|_| ()) |
| 296 | } |
| 297 | |
| 298 | /// Print the repair plan the way the human doctor report presents it. |
| 299 | pub(crate) fn print_fix_plan(plan: &DoctorFixPlan) { |
| 300 | use colored::Colorize; |
| 301 | |
| 302 | let (sky_r, sky_g, sky_b) = codewhale_palette::WHALE_ACTION_RGB; |
| 303 | println!("{}", "Repair plan (--fix):".bold()); |
| 304 | if plan.is_empty() { |
| 305 | println!(" {} nothing to repair", "✓".truecolor(sky_r, sky_g, sky_b)); |
| 306 | return; |
| 307 | } |
| 308 | for line in plan.describe() { |
| 309 | println!(" · {line}"); |
| 310 | } |
| 311 | println!( |
| 312 | " {} pass --yes to apply without prompting", |
| 313 | "!".truecolor(sky_r, sky_g, sky_b) |
| 314 | ); |
| 315 | } |
| 316 | |
| 317 | /// Ask for consent on stdin. Only used by the human (non-JSON) doctor path. |
| 318 | pub(crate) fn confirm_fix(plan: &DoctorFixPlan) -> bool { |
| 319 | use std::io::{BufRead, Write}; |
| 320 | |
| 321 | println!(); |
| 322 | println!("Apply these {} repair(s) now? [y/N] ", plan.len()); |
| 323 | let mut answer = String::new(); |
| 324 | let _ = std::io::stdout().flush(); |
| 325 | if std::io::stdin().lock().read_line(&mut answer).is_err() { |
| 326 | return false; |
| 327 | } |
| 328 | matches!(answer.trim().to_ascii_lowercase().as_str(), "y" | "yes") |
| 329 | } |
| 330 | |
| 331 | /// Print apply results and return whether every action succeeded. |
| 332 | pub(crate) fn print_apply_results(results: &[(DoctorFixAction, DoctorFixOutcome)]) -> bool { |
| 333 | use colored::Colorize; |
| 334 | |
| 335 | let (aqua_r, aqua_g, aqua_b) = codewhale_palette::WHALE_ACTION_RGB; |
| 336 | let (red_r, red_g, red_b) = codewhale_palette::WHALE_ERROR_RGB; |
| 337 | println!("{}", "Repair results:".bold()); |
| 338 | let mut all_applied = true; |
| 339 | for (action, outcome) in results { |
| 340 | match outcome { |
| 341 | DoctorFixOutcome::Applied => println!( |
| 342 | " {} {}", |
| 343 | "✓".truecolor(aqua_r, aqua_g, aqua_b), |
| 344 | plan_action_line(action) |
| 345 | ), |
| 346 | DoctorFixOutcome::Failed(error) => { |
| 347 | all_applied = false; |
| 348 | println!( |
| 349 | " {} {} — {error}", |
| 350 | "✗".truecolor(red_r, red_g, red_b), |
| 351 | plan_action_line(action) |
| 352 | ); |
| 353 | } |
| 354 | } |
| 355 | } |
| 356 | all_applied |
| 357 | } |
| 358 | |
| 359 | fn plan_action_line(action: &DoctorFixAction) -> String { |
| 360 | DoctorFixPlan { |
| 361 | actions: vec![action.clone()], |
| 362 | } |
| 363 | .describe() |
| 364 | .pop() |
| 365 | .unwrap_or_else(|| "repair".to_string()) |
| 366 | } |
| 367 | |
| 368 | #[cfg(test)] |
| 369 | mod tests { |
| 370 | use super::*; |
| 371 | |
| 372 | /// Scoped `CODEWHALE_HOME` override under the process-wide env barrier. |
| 373 | /// `EnvVarGuard` restores the prior value even on panic. |
| 374 | struct ScratchHome { |
| 375 | dir: tempfile::TempDir, |
| 376 | _env: crate::test_support::EnvVarGuard, |
| 377 | _lock: crate::test_support::TestEnvLock, |
| 378 | } |
| 379 | |
| 380 | impl ScratchHome { |
| 381 | fn new() -> (Self, crate::config::Config) { |
| 382 | let lock = crate::test_support::lock_test_env(); |
| 383 | let dir = tempfile::tempdir().expect("home tempdir"); |
| 384 | let env = crate::test_support::EnvVarGuard::set("CODEWHALE_HOME", dir.path()); |
| 385 | ( |
| 386 | Self { |
| 387 | dir, |
| 388 | _env: env, |
| 389 | _lock: lock, |
| 390 | }, |
| 391 | crate::config::Config::default(), |
| 392 | ) |
| 393 | } |
| 394 | |
| 395 | fn path(&self) -> &Path { |
| 396 | self.dir.path() |
| 397 | } |
| 398 | } |
| 399 | |
| 400 | #[test] |
| 401 | fn stale_temp_files_only_names_regular_dot_tmp_files_in_home() { |
| 402 | let (home, config) = ScratchHome::new(); |
| 403 | std::fs::write(home.path().join(".tmpAbC123"), b"orphaned").expect("write stale"); |
| 404 | std::fs::write(home.path().join(".tmpOther"), b"orphaned 2").expect("write stale 2"); |
| 405 | std::fs::write(home.path().join("keep.txt"), b"keep").expect("write keep"); |
| 406 | std::fs::create_dir_all(home.path().join(".tmpDir")).expect("mkdir .tmpDir"); |
| 407 | |
| 408 | let workspace = tempfile::tempdir().expect("workspace"); |
| 409 | let plan = plan_fixes( |
| 410 | &config, |
| 411 | workspace.path(), |
| 412 | &crate::plugins::PluginRegistry::default(), |
| 413 | ); |
| 414 | let temp_deletions = plan |
| 415 | .actions |
| 416 | .iter() |
| 417 | .filter(|a| matches!(a, DoctorFixAction::DeleteStaleTempFile { .. })) |
| 418 | .count(); |
| 419 | assert_eq!(temp_deletions, 2, "{:?}", plan.actions); |
| 420 | |
| 421 | let results = apply_fixes(&DoctorFixPlan { |
| 422 | actions: plan |
| 423 | .actions |
| 424 | .iter() |
| 425 | .filter(|a| matches!(a, DoctorFixAction::DeleteStaleTempFile { .. })) |
| 426 | .cloned() |
| 427 | .collect(), |
| 428 | }); |
| 429 | assert!(results.iter().all(|(_, o)| *o == DoctorFixOutcome::Applied)); |
| 430 | assert!(!home.path().join(".tmpAbC123").exists()); |
| 431 | assert!(home.path().join("keep.txt").exists()); |
| 432 | } |
| 433 | |
| 434 | #[test] |
| 435 | fn missing_user_directories_are_planned_and_scaffolding_is_idempotent() { |
| 436 | let (home, config) = ScratchHome::new(); |
| 437 | let workspace = tempfile::tempdir().expect("workspace"); |
| 438 | let registry = crate::plugins::PluginRegistry::default(); |
| 439 | let plan = plan_fixes(&config, workspace.path(), ®istry); |
| 440 | assert!( |
| 441 | plan.actions |
| 442 | .iter() |
| 443 | .any(|a| matches!(a, DoctorFixAction::ScaffoldDirectory { .. })), |
| 444 | "scratch home plans scaffolding: {:?}", |
| 445 | plan.actions |
| 446 | ); |
| 447 | let results = apply_fixes(&plan); |
| 448 | assert!( |
| 449 | results.iter().all(|(_, o)| *o == DoctorFixOutcome::Applied), |
| 450 | "{results:?}" |
| 451 | ); |
| 452 | assert!(home.path().join("skills").exists()); |
| 453 | assert!(home.path().join("tools").exists()); |
| 454 | assert!(home.path().join("plugins").exists()); |
| 455 | let replan = plan_fixes(&config, workspace.path(), ®istry); |
| 456 | assert!( |
| 457 | !replan |
| 458 | .actions |
| 459 | .iter() |
| 460 | .any(|a| matches!(a, DoctorFixAction::ScaffoldDirectory { .. })), |
| 461 | "scaffolding converges" |
| 462 | ); |
| 463 | } |
| 464 | |
| 465 | #[cfg(unix)] |
| 466 | #[test] |
| 467 | fn loose_secret_file_permissions_are_planned_and_tightened() { |
| 468 | use std::os::unix::fs::PermissionsExt; |
| 469 | |
| 470 | let (home, config) = ScratchHome::new(); |
| 471 | let secrets_dir = home.path().join("secrets"); |
| 472 | std::fs::create_dir_all(&secrets_dir).expect("secrets dir"); |
| 473 | let secrets_file = secrets_dir.join("secrets.json"); |
| 474 | std::fs::write(&secrets_file, b"{}").expect("secrets file"); |
| 475 | std::fs::set_permissions(&secrets_file, std::fs::Permissions::from_mode(0o644)) |
| 476 | .expect("loosen"); |
| 477 | |
| 478 | let workspace = tempfile::tempdir().expect("workspace"); |
| 479 | let registry = crate::plugins::PluginRegistry::default(); |
| 480 | let plan = plan_fixes(&config, workspace.path(), ®istry); |
| 481 | let tighten = plan |
| 482 | .actions |
| 483 | .iter() |
| 484 | .find(|a| matches!(a, DoctorFixAction::TightenSecretPermissions { .. })) |
| 485 | .expect("loose secret file is planned"); |
| 486 | let results = apply_fixes(&DoctorFixPlan { |
| 487 | actions: vec![tighten.clone()], |
| 488 | }); |
| 489 | assert_eq!(results[0].1, DoctorFixOutcome::Applied); |
| 490 | let mode = std::fs::metadata(&secrets_file) |
| 491 | .expect("metadata") |
| 492 | .permissions() |
| 493 | .mode(); |
| 494 | assert_eq!(mode & 0o777, 0o600); |
| 495 | } |
| 496 | |
| 497 | #[test] |
| 498 | fn fix_flags_parse_and_conflict_with_json_modes() { |
| 499 | use clap::Parser; |
| 500 | |
| 501 | let cli = crate::Cli::try_parse_from(["codewhale", "doctor", "--fix", "--yes"]) |
| 502 | .expect("--fix --yes parses"); |
| 503 | let Some(crate::Commands::Doctor(args)) = cli.command else { |
| 504 | panic!("expected doctor command"); |
| 505 | }; |
| 506 | assert!(args.fix); |
| 507 | assert!(args.yes); |
| 508 | |
| 509 | crate::Cli::try_parse_from(["codewhale", "doctor", "--fix", "--json"]) |
| 510 | .expect_err("--fix conflicts with --json"); |
| 511 | crate::Cli::try_parse_from(["codewhale", "doctor", "--fix", "--context-json"]) |
| 512 | .expect_err("--fix conflicts with --context-json"); |
| 513 | crate::Cli::try_parse_from(["codewhale", "doctor", "--yes"]) |
| 514 | .expect_err("--yes requires --fix"); |
| 515 | } |
| 516 | |
| 517 | #[test] |
| 518 | fn broken_mcp_entry_is_disabled_through_the_atomic_save_path() { |
| 519 | let (home, config) = ScratchHome::new(); |
| 520 | let workspace = tempfile::tempdir().expect("workspace"); |
| 521 | let mcp_path = home.path().join("mcp.json"); |
| 522 | std::fs::write( |
| 523 | &mcp_path, |
| 524 | serde_json::json!({ |
| 525 | "mcpServers": { |
| 526 | "broken": { "command": "", "args": [] }, |
| 527 | "healthy": { "command": "node", "args": ["server.js"] } |
| 528 | } |
| 529 | }) |
| 530 | .to_string(), |
| 531 | ) |
| 532 | .expect("mcp config"); |
| 533 | |
| 534 | let mut config = config; |
| 535 | config.mcp_config_path = Some(mcp_path.display().to_string()); |
| 536 | |
| 537 | let registry = crate::plugins::PluginRegistry::default(); |
| 538 | let plan = plan_fixes(&config, workspace.path(), ®istry); |
| 539 | let disable = plan |
| 540 | .actions |
| 541 | .iter() |
| 542 | .find_map(|a| match a { |
| 543 | DoctorFixAction::DisableMcpServer { |
| 544 | config_path, |
| 545 | server, |
| 546 | } if server == "broken" => Some(config_path.clone()), |
| 547 | _ => None, |
| 548 | }) |
| 549 | .expect("broken entry is planned"); |
| 550 | assert_eq!(disable, mcp_path); |
| 551 | |
| 552 | let results = apply_fixes(&DoctorFixPlan { |
| 553 | actions: plan |
| 554 | .actions |
| 555 | .iter() |
| 556 | .filter(|a| matches!(a, DoctorFixAction::DisableMcpServer { .. })) |
| 557 | .cloned() |
| 558 | .collect(), |
| 559 | }); |
| 560 | assert!( |
| 561 | results.iter().all(|(_, o)| *o == DoctorFixOutcome::Applied), |
| 562 | "{results:?}" |
| 563 | ); |
| 564 | let raw: serde_json::Value = |
| 565 | serde_json::from_str(&std::fs::read_to_string(&mcp_path).expect("reread")) |
| 566 | .expect("json"); |
| 567 | assert!( |
| 568 | raw.get("servers").is_none(), |
| 569 | "preserve the original mcpServers spelling" |
| 570 | ); |
| 571 | assert_eq!(raw["mcpServers"]["broken"]["enabled"], false); |
| 572 | assert_eq!(raw["mcpServers"]["broken"]["disabled"], true); |
| 573 | assert_eq!( |
| 574 | raw["mcpServers"]["healthy"], |
| 575 | serde_json::json!({"command":"node", "args":["server.js"]}), |
| 576 | "healthy entry remains unchanged" |
| 577 | ); |
| 578 | assert!(mcp::load_config(&mcp_path).unwrap().servers["healthy"].enabled); |
| 579 | } |
| 580 | } |
| 581 |