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