diff --git a/crates/cli/src/commands/diagnostics.rs b/crates/cli/src/commands/diagnostics.rs index 18e557c2e..35034c671 100644 --- a/crates/cli/src/commands/diagnostics.rs +++ b/crates/cli/src/commands/diagnostics.rs @@ -32,6 +32,7 @@ pub(crate) struct AgentsCommand { pub(super) async fn execute( command: DoctorCommand, server: &super::serve::ServerArgs, + logging_fallback_error: Option<&CliError>, ) -> Result { if let Some(plugin) = command.plugin { return execute_plugin_doctor(plugin, command.install_dir, command.json); @@ -41,6 +42,7 @@ pub(super) async fn execute( command.agent.map(Into::into), command.json, &gateway_overrides, + logging_fallback_error, ) .await } diff --git a/crates/cli/src/commands/mod.rs b/crates/cli/src/commands/mod.rs index 8ce414493..79f9f19d4 100644 --- a/crates/cli/src/commands/mod.rs +++ b/crates/cli/src/commands/mod.rs @@ -53,9 +53,12 @@ pub(crate) async fn run(bootstrap_shutdown_token: Option) -> ExitCode { // Dispatches CLI subcommands while keeping the no-subcommand path as server mode. `run` inherits // top-level server flags so transparent launch can share config parsing with daemon startup. -fn configure_logging( - cli: &Cli, -) -> Result, error::CliError> { +struct LoggingSetup { + _runtime: Option, + fallback_error: Option, +} + +fn configure_logging(cli: &Cli) -> Result { let initialize = match cli.command.as_ref() { Some(command) => !command.skips_logging(), None => { @@ -64,7 +67,10 @@ fn configure_logging( } }; if !initialize { - return Ok(None); + return Ok(LoggingSetup { + _runtime: None, + fallback_error: None, + }); } let user_only = matches!(cli.command.as_ref(), Some(Command::Mcp)); @@ -85,7 +91,7 @@ fn configure_logging( Err(error) => return Err(error), }; let runtime = nemo_relay::logging::LoggingRuntime::configure(config)?; - if let Some(error) = fallback_error { + if let Some(error) = fallback_error.as_ref() { log::warn!( target: "nemo_relay.cli", event = "doctor_logging_fallback", @@ -93,7 +99,10 @@ fn configure_logging( "Doctor fell back to default logging after resolution failure" ); } - Ok(Some(runtime)) + Ok(LoggingSetup { + _runtime: Some(runtime), + fallback_error, + }) } async fn dispatch(bootstrap_shutdown_token: Option) -> Result { @@ -104,7 +113,7 @@ async fn dispatch(bootstrap_shutdown_token: Option) -> Result) -> Result run_command(command, &cli.server).await, + Some(command) => run_command(command, &cli.server, logging.fallback_error.as_ref()).await, None => run_default(&cli.server, bootstrap_shutdown_token).await, }; match &result { @@ -150,7 +159,11 @@ async fn dispatch(bootstrap_shutdown_token: Option) -> Result Result { +async fn run_command( + command: Command, + server: &ServerArgs, + logging_fallback_error: Option<&error::CliError>, +) -> Result { match command { Command::HookForward(command) => { hook_forward::execute(command).await?; @@ -166,7 +179,9 @@ async fn run_command(command: Command, server: &ServerArgs) -> Result configure::execute(command, server).await, Command::Plugins(command) => plugins::execute(command, server), Command::ModelPricing(command) => model_pricing::execute(command), - Command::Doctor(command) => diagnostics::execute(command, server).await, + Command::Doctor(command) => { + diagnostics::execute(command, server, logging_fallback_error).await + } Command::Agents(command) => runtime_diagnostics::run_agents(command.json).await, Command::Completions(command) => completions::execute(command), } @@ -218,7 +233,7 @@ async fn run_default( .await?; Ok(ExitCode::SUCCESS) } else if runtime_configuration::any_config_file_exists() { - runtime_diagnostics::run_doctor(None, false, &runtime_args).await + runtime_diagnostics::run_doctor(None, false, &runtime_args, None).await } else { configure::run(None, None).await?; Ok(ExitCode::SUCCESS) diff --git a/crates/cli/src/diagnostics/mod.rs b/crates/cli/src/diagnostics/mod.rs index 6df3026ae..ea2485c79 100644 --- a/crates/cli/src/diagnostics/mod.rs +++ b/crates/cli/src/diagnostics/mod.rs @@ -1368,8 +1368,24 @@ pub(crate) async fn run_doctor( target_agent: Option, json: bool, gateway_overrides: &GatewayOverrides, + logging_fallback_error: Option<&CliError>, ) -> Result { - let report = collect_report(target_agent, gateway_overrides).await?; + let mut report = collect_report(target_agent, gateway_overrides).await?; + if let Some(error) = logging_fallback_error { + let logging_details = format!( + "could not resolve logging configuration: {error}; repair or recreate the named logging configuration file" + ); + if report.configuration.resolution.status == Status::Pass { + report.configuration.resolution.status = Status::Fail; + report.configuration.resolution.details = logging_details; + } else { + report + .configuration + .resolution + .details + .push_str(&format!("; additionally, {logging_details}")); + } + } log::info!( target: "nemo_relay.diagnostics", event = "diagnostics_completed", diff --git a/crates/cli/tests/cli_tests.rs b/crates/cli/tests/cli_tests.rs index 6ae8c4421..a13a9962d 100644 --- a/crates/cli/tests/cli_tests.rs +++ b/crates/cli/tests/cli_tests.rs @@ -3266,6 +3266,128 @@ fn cli_doctor_json_reports_a_missing_explicit_config() { ); } +#[test] +fn cli_doctor_reports_a_missing_explicit_logging_config() { + let temp = tempfile::tempdir().unwrap(); + let xdg = temp.path().join("xdg"); + let cwd = temp.path().join("workdir"); + let config = temp.path().join("missing/logging.toml"); + std::fs::create_dir_all(&xdg).unwrap(); + std::fs::create_dir_all(&cwd).unwrap(); + + let output = Command::new(gateway_bin()) + .current_dir(&cwd) + .env("XDG_CONFIG_HOME", &xdg) + .env("HOME", temp.path()) + .args(["--log-config-path"]) + .arg(&config) + .arg("doctor") + .output() + .unwrap(); + + assert!(!output.status.success()); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("Configuration")); + assert!(stdout.contains("Resolution")); + assert!(stdout.contains("could not resolve logging configuration")); + assert!(stdout.contains(config.to_str().unwrap())); + assert!(stdout.contains("Agents detected")); + assert!(stdout.contains("Some checks FAILED")); +} + +#[test] +fn cli_doctor_json_reports_a_missing_explicit_logging_config() { + let temp = tempfile::tempdir().unwrap(); + let xdg = temp.path().join("xdg"); + let cwd = temp.path().join("workdir"); + let config = temp.path().join("missing/logging.toml"); + std::fs::create_dir_all(&xdg).unwrap(); + std::fs::create_dir_all(&cwd).unwrap(); + + let output = Command::new(gateway_bin()) + .current_dir(&cwd) + .env("XDG_CONFIG_HOME", &xdg) + .env("HOME", temp.path()) + .args(["--log-config-path"]) + .arg(&config) + .args(["doctor", "--json"]) + .output() + .unwrap(); + + assert!(!output.status.success()); + let report: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let resolution = &report["configuration"]["resolution"]; + assert_eq!(resolution["status"], "fail"); + assert!( + resolution["details"] + .as_str() + .unwrap() + .contains(config.to_str().unwrap()) + ); +} + +#[test] +fn cli_doctor_reports_invalid_explicit_logging_config_paths() { + let temp = tempfile::tempdir().unwrap(); + let xdg = temp.path().join("xdg"); + let cwd = temp.path().join("workdir"); + std::fs::create_dir_all(&xdg).unwrap(); + std::fs::create_dir_all(&cwd).unwrap(); + + for (path, expected) in [ + (PathBuf::from("logging.toml"), "must be absolute"), + ( + temp.path().join("logging.json"), + "must identify a .toml file", + ), + ] { + let output = Command::new(gateway_bin()) + .current_dir(&cwd) + .env("XDG_CONFIG_HOME", &xdg) + .env("HOME", temp.path()) + .args(["--log-config-path"]) + .arg(&path) + .args(["doctor", "--json"]) + .output() + .unwrap(); + + assert!(!output.status.success(), "path: {}", path.display()); + let report: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let resolution = &report["configuration"]["resolution"]; + assert_eq!(resolution["status"], "fail"); + assert!( + resolution["details"].as_str().unwrap().contains(expected), + "path: {}, details: {}", + path.display(), + resolution["details"] + ); + } +} + +#[test] +fn cli_doctor_accepts_a_valid_explicit_logging_config() { + let temp = tempfile::tempdir().unwrap(); + let xdg = temp.path().join("xdg"); + let cwd = temp.path().join("workdir"); + std::fs::create_dir_all(&xdg).unwrap(); + std::fs::create_dir_all(&cwd).unwrap(); + let (config, _log_path) = write_jsonl_logging_config(temp.path()); + + let output = Command::new(gateway_bin()) + .current_dir(&cwd) + .env("XDG_CONFIG_HOME", &xdg) + .env("HOME", temp.path()) + .args(["--log-config-path"]) + .arg(&config) + .args(["doctor", "--json"]) + .output() + .unwrap(); + + assert!(output.status.success()); + let report: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(report["configuration"]["resolution"]["status"], "pass"); +} + #[test] fn cli_doctor_reports_the_nearest_ancestor_workspace_config() { let temp = tempfile::tempdir().unwrap(); diff --git a/crates/cli/tests/coverage/commands/main_tests.rs b/crates/cli/tests/coverage/commands/main_tests.rs index b9f88c27e..2dfec86c5 100644 --- a/crates/cli/tests/coverage/commands/main_tests.rs +++ b/crates/cli/tests/coverage/commands/main_tests.rs @@ -501,7 +501,7 @@ async fn run_command_dispatches_safe_plugin_and_install_paths() { ]) .unwrap(); assert_eq!( - run_command(cli.command.unwrap(), &cli.server) + run_command(cli.command.unwrap(), &cli.server, None) .await .unwrap(), ExitCode::SUCCESS @@ -517,7 +517,7 @@ async fn run_command_dispatches_safe_plugin_and_install_paths() { ]) .unwrap(); assert_eq!( - run_command(cli.command.unwrap(), &cli.server) + run_command(cli.command.unwrap(), &cli.server, None) .await .unwrap(), ExitCode::SUCCESS