diff --git a/lib/crates/fabro-cli/src/command_context.rs b/lib/crates/fabro-cli/src/command_context.rs index ce1741bb1..60348841d 100644 --- a/lib/crates/fabro-cli/src/command_context.rs +++ b/lib/crates/fabro-cli/src/command_context.rs @@ -43,7 +43,37 @@ impl CommandContext { cli_layer: &CliLayer, process_local_json: bool, ) -> Result { - Self::new(printer, process_local_json, ServerMode::None, cli_layer) + let (machine_settings, user_settings) = load_merged_settings(cli_layer, &ServerMode::None)?; + Self::base_with_settings( + printer, + cli_layer, + process_local_json, + machine_settings, + user_settings, + ) + } + + pub(crate) fn base_with_settings( + printer: Printer, + cli_layer: &CliLayer, + process_local_json: bool, + machine_settings: SettingsLayer, + user_settings: UserSettings, + ) -> Result { + let cwd = std::env::current_dir().context("Failed to get current directory")?; + let base_config_path = user_config::active_settings_path(None); + + Ok(Self { + printer, + process_local_json, + cwd, + base_config_path, + cli_layer: cli_layer.clone(), + machine_settings, + user_settings, + server_mode: ServerMode::None, + server: OnceCell::new(), + }) } pub(crate) fn with_target(&self, args: &ServerTargetArgs) -> Result { @@ -59,29 +89,6 @@ impl CommandContext { }) } - fn new( - printer: Printer, - process_local_json: bool, - server_mode: ServerMode, - cli_layer: &CliLayer, - ) -> Result { - let cwd = std::env::current_dir().context("Failed to get current directory")?; - let base_config_path = user_config::active_settings_path(None); - let (machine_settings, user_settings) = load_merged_settings(cli_layer, &server_mode)?; - - Ok(Self { - printer, - process_local_json, - cwd, - base_config_path, - cli_layer: cli_layer.clone(), - machine_settings, - user_settings, - server_mode, - server: OnceCell::new(), - }) - } - pub(crate) fn printer(&self) -> Printer { self.printer } @@ -137,14 +144,28 @@ impl CommandContext { } fn with_server_mode(&self, server_mode: ServerMode) -> Result { - let (machine_settings, user_settings) = match &server_mode { - ServerMode::ByStorageDir { .. } => load_merged_settings(&self.cli_layer, &server_mode)?, - ServerMode::None | ServerMode::ByTarget { .. } => { - (self.machine_settings.clone(), self.user_settings.clone()) - } - }; + // Always reload settings for the requested derivation mode so the result + // depends only on the requested mode, not on whichever derived context + // happened to call into this helper. + let (machine_settings, user_settings) = + load_merged_settings(&self.cli_layer, &server_mode)?; - Ok(Self { + Ok( + self.with_server_mode_from_loaded_settings( + server_mode, + machine_settings, + user_settings, + ), + ) + } + + fn with_server_mode_from_loaded_settings( + &self, + server_mode: ServerMode, + machine_settings: SettingsLayer, + user_settings: UserSettings, + ) -> Self { + Self { printer: self.printer, process_local_json: self.process_local_json, cwd: self.cwd.clone(), @@ -154,7 +175,7 @@ impl CommandContext { user_settings, server_mode, server: OnceCell::new(), - }) + } } } @@ -188,13 +209,14 @@ fn merge_settings_layer( mod tests { use std::path::PathBuf; - use fabro_config::parse_settings_layer; + use fabro_config::user::apply_storage_dir_override; + use fabro_config::{UserSettings, parse_settings_layer}; + use fabro_types::settings::SettingsLayer; use fabro_types::settings::cli::{CliLayer, CliOutputLayer, OutputFormat, OutputVerbosity}; use fabro_util::printer::Printer; use tokio::sync::OnceCell; use super::{CommandContext, ServerMode, merge_settings_layer}; - use crate::args::ServerTargetArgs; fn cli_layer_with_json_and_verbose() -> CliLayer { CliLayer { @@ -225,6 +247,27 @@ mod tests { } } + fn synthetic_context_with_settings( + process_local_json: bool, + printer: Printer, + cli_layer: CliLayer, + machine_settings: SettingsLayer, + user_settings: UserSettings, + server_mode: ServerMode, + ) -> CommandContext { + CommandContext { + printer, + process_local_json, + cwd: PathBuf::from("/tmp/workspace"), + base_config_path: PathBuf::from("/tmp/settings.toml"), + cli_layer, + machine_settings, + user_settings, + server_mode, + server: OnceCell::new(), + } + } + #[test] fn context_exposes_resolved_output_and_explicit_json_state() { let ctx = synthetic_context(true, Printer::Default); @@ -241,11 +284,13 @@ mod tests { #[test] fn deriving_target_context_preserves_invocation_state() { let base = synthetic_context(true, Printer::Verbose); - let derived = base - .with_target(&ServerTargetArgs { - server: Some("https://fabro.example.com".to_string()), - }) - .expect("target context should derive"); + let derived = base.with_server_mode_from_loaded_settings( + ServerMode::ByTarget { + target_override: Some("https://fabro.example.com".to_string()), + }, + base.machine_settings().clone(), + base.user_settings().clone(), + ); assert_eq!(derived.printer(), Printer::Verbose); assert!(derived.explicit_json_requested()); @@ -271,7 +316,7 @@ root = "/srv/fabro/default" "#, ) .expect("settings fixture should parse"); - let override_disk_settings = fabro_config::user::apply_storage_dir_override( + let override_disk_settings = apply_storage_dir_override( base_disk_settings.clone(), Some(std::path::Path::new("/srv/fabro/override")), ); @@ -305,6 +350,61 @@ root = "/srv/fabro/default" ); } + #[test] + fn deriving_target_from_connection_context_discards_storage_override() { + let cli_layer = cli_layer_with_json_and_verbose(); + let base_disk_settings = parse_settings_layer( + r#" +_version = 1 + +[server.storage] +root = "/srv/fabro/default" +"#, + ) + .expect("settings fixture should parse"); + let override_disk_settings = apply_storage_dir_override( + base_disk_settings.clone(), + Some(std::path::Path::new("/srv/fabro/override")), + ); + + let (base_settings, base_user_settings) = + merge_settings_layer(base_disk_settings, &cli_layer) + .expect("base settings should merge"); + let (connection_settings, connection_user_settings) = + merge_settings_layer(override_disk_settings, &cli_layer) + .expect("connection settings should merge"); + let connection_ctx = synthetic_context_with_settings( + false, + Printer::Default, + cli_layer, + connection_settings, + connection_user_settings, + ServerMode::ByStorageDir { + target_override: None, + storage_dir_override: Some(PathBuf::from("/srv/fabro/override")), + }, + ); + + let derived = connection_ctx.with_server_mode_from_loaded_settings( + ServerMode::ByTarget { + target_override: Some("https://fabro.example.com".to_string()), + }, + base_settings, + base_user_settings, + ); + + assert_eq!( + derived + .machine_settings() + .server + .as_ref() + .and_then(|server| server.storage.as_ref()) + .and_then(|storage| storage.root.as_ref()) + .map(|root| root.as_source()), + Some("/srv/fabro/default".to_string()) + ); + } + #[test] fn explicit_json_guard_uses_invocation_flag_not_resolved_output_format() { let json_ctx = synthetic_context(true, Printer::Default); @@ -316,11 +416,14 @@ root = "/srv/fabro/default" #[test] fn target_resolution_errors_remain_deferred() { - let ctx = synthetic_context(false, Printer::Default) - .with_target(&ServerTargetArgs { - server: Some("not-a-valid-target".to_string()), - }) - .expect("target derivation should not resolve the target eagerly"); + let base = synthetic_context(false, Printer::Default); + let ctx = base.with_server_mode_from_loaded_settings( + ServerMode::ByTarget { + target_override: Some("not-a-valid-target".to_string()), + }, + base.machine_settings().clone(), + base.user_settings().clone(), + ); assert!(matches!(ctx.server_mode, ServerMode::ByTarget { .. })); } diff --git a/lib/crates/fabro-cli/src/commands/run/command.rs b/lib/crates/fabro-cli/src/commands/run/command.rs index 663cad207..471edc8e7 100644 --- a/lib/crates/fabro-cli/src/commands/run/command.rs +++ b/lib/crates/fabro-cli/src/commands/run/command.rs @@ -5,27 +5,17 @@ use fabro_util::terminal::Styles; use crate::args::RunArgs; use crate::command_context::CommandContext; use crate::shared::print_json_pretty; -use crate::user_config::load_settings_with_storage_dir; pub(crate) async fn execute(mut args: RunArgs, base_ctx: &CommandContext) -> Result<()> { let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr())); let printer = base_ctx.printer(); let ctx = base_ctx.with_target(&args.target)?; - let cli_defaults = load_settings_with_storage_dir(None)?; args.verbose = args.verbose || ctx.user_settings().cli.output.verbosity == OutputVerbosity::Verbose; let quiet = args.detach; let prevent_idle_sleep = ctx.user_settings().cli.exec.prevent_idle_sleep; - let created_run = Box::pin(super::create::create_run( - &ctx, - &args, - cli_defaults, - styles, - quiet, - printer, - )) - .await?; + let created_run = Box::pin(super::create::create_run(&ctx, &args, styles, quiet)).await?; if !quiet { fabro_util::printerr!( diff --git a/lib/crates/fabro-cli/src/commands/run/create.rs b/lib/crates/fabro-cli/src/commands/run/create.rs index 4c34ef861..9f1e5a105 100644 --- a/lib/crates/fabro-cli/src/commands/run/create.rs +++ b/lib/crates/fabro-cli/src/commands/run/create.rs @@ -1,8 +1,6 @@ use fabro_config::load::load_settings_user; use fabro_config::user::active_settings_path; use fabro_types::RunId; -use fabro_types::settings::SettingsLayer; -use fabro_util::printer::Printer; use fabro_util::terminal::Styles; use super::output::{api_diagnostics_to_local, print_preflight_workflow_summary}; @@ -22,10 +20,8 @@ pub(crate) struct CreatedRun { pub(crate) async fn create_run( ctx: &CommandContext, args: &RunArgs, - _cli_defaults: SettingsLayer, styles: &Styles, quiet: bool, - printer: Printer, ) -> anyhow::Result { let workflow_path = args .workflow @@ -51,6 +47,7 @@ pub(crate) async fn create_run( })?; let client = ctx.server().await?; if !quiet { + let printer = ctx.printer(); let preflight = client.run_preflight(built.manifest.clone()).await?; let diagnostics = api_diagnostics_to_local(&preflight.workflow.diagnostics); if !diagnostics diff --git a/lib/crates/fabro-cli/src/commands/run/mod.rs b/lib/crates/fabro-cli/src/commands/run/mod.rs index 0256ca566..000e8fc3d 100644 --- a/lib/crates/fabro-cli/src/commands/run/mod.rs +++ b/lib/crates/fabro-cli/src/commands/run/mod.rs @@ -5,7 +5,6 @@ use fabro_util::terminal::Styles; use crate::args::{AttachArgs, RunCommands, RunWorkerArgs, StartArgs}; use crate::command_context::CommandContext; use crate::shared::print_json_pretty; -use crate::user_config::load_settings_with_storage_dir; pub(crate) mod attach; pub(crate) mod command; @@ -32,17 +31,8 @@ pub(crate) async fn dispatch(cmd: RunCommands, base_ctx: &CommandContext) -> Res RunCommands::Run(args) => Box::pin(command::execute(args, base_ctx)).await, RunCommands::Create(args) => { let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr())); - let cli_defaults = load_settings_with_storage_dir(None)?; let ctx = base_ctx.with_target(&args.target)?; - let created_run = Box::pin(create::create_run( - &ctx, - &args, - cli_defaults, - styles, - true, - printer, - )) - .await?; + let created_run = Box::pin(create::create_run(&ctx, &args, styles, true)).await?; if ctx.user_settings().cli.output.format == OutputFormat::Json { print_json_pretty(&serde_json::json!({ "run_id": created_run.run_id }))?; } else { diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index 754e38243..a163ea8b1 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -166,18 +166,19 @@ async fn main_inner() -> (String, Result<()>) { Err(err) => return (command_name, Err(err)), }; - let user_settings = match user_config::load_settings() { + let disk_settings = match user_config::load_settings() { Ok(settings) => settings, Err(err) => return (command_name, Err(err)), }; - let combined_settings = combine_files(user_settings, SettingsLayer { + let machine_settings = combine_files(disk_settings, SettingsLayer { cli: Some(cli_layer.clone()), ..SettingsLayer::default() }); - let cli_settings = match fabro_config::UserSettings::from_layer(&combined_settings) { - Ok(settings) => settings.cli, + let user_settings = match fabro_config::UserSettings::from_layer(&machine_settings) { + Ok(settings) => settings, Err(err) => return (command_name, Err(err.into())), }; + let cli_settings = user_settings.cli.clone(); let printer = printer_from_verbosity(cli_settings.output.verbosity); let config_log_level = match &pre_tracing_bootstrap.sink { @@ -211,7 +212,15 @@ async fn main_inner() -> (String, Result<()>) { }; let result = Box::pin(async move { - let build_base_ctx = || CommandContext::base(printer, &cli_layer, process_local_json); + let build_base_ctx = || { + CommandContext::base_with_settings( + printer, + &cli_layer, + process_local_json, + machine_settings.clone(), + user_settings.clone(), + ) + }; match *command { Commands::Exec(args) => commands::exec::execute(args, &cli_settings, printer).await?, diff --git a/lib/crates/fabro-cli/tests/it/cmd/json_global.rs b/lib/crates/fabro-cli/tests/it/cmd/json_global.rs index ac22067bd..32e7d4dd7 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/json_global.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/json_global.rs @@ -48,6 +48,30 @@ fn settings_uses_json_output_format_from_home_config() { assert!(value.is_object()); } +#[test] +fn auth_status_ignores_json_output_format_from_home_config() { + let context = test_context!(); + context.write_home( + ".fabro/settings.toml", + "_version = 1\n\n[cli.output]\nformat = \"json\"\n", + ); + + let output = context + .command() + .args(["auth", "status"]) + .output() + .expect("command should run"); + + assert!(output.status.success()); + assert!( + output.stdout.is_empty(), + "stdout should stay empty in text mode" + ); + let stderr = output_stderr(&output); + assert!(stderr.contains("Not logged in to any servers.")); + assert!(stderr.contains("Dev token:")); +} + #[test] fn secret_list_uses_json_output_format_from_home_config() { let context = test_context!(); diff --git a/lib/crates/fabro-cli/tests/it/cmd/system_events.rs b/lib/crates/fabro-cli/tests/it/cmd/system_events.rs index 26c793e96..2950aeaae 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/system_events.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/system_events.rs @@ -1,4 +1,5 @@ use fabro_test::{fabro_snapshot, test_context}; +use httpmock::MockServer; #[test] fn help() { @@ -26,3 +27,87 @@ fn help() { ----- stderr ----- "); } + +#[test] +fn system_events_renders_text_lines_from_sse_payloads() { + let context = test_context!(); + let server = MockServer::start(); + let run_id = crate::support::unique_run_id(); + let payload = serde_json::json!({ + "payload": { + "ts": "2026-04-05T12:00:00Z", + "run_id": run_id, + "event": "run.completed", + } + }); + let attach_mock = server.mock(|when, then| { + when.method("GET") + .path("/api/v1/attach") + .query_param("run_id", run_id.as_str()); + then.status(200) + .header("Content-Type", "text/event-stream") + .body(format!("data: {payload}\n\n")); + }); + + let output = context + .command() + .args([ + "system", + "events", + "--server", + &format!("{}/api/v1", server.base_url()), + "--run-id", + &run_id, + ]) + .output() + .expect("command should run"); + + assert!(output.status.success(), "system events failed"); + let stdout = String::from_utf8(output.stdout).expect("stdout should be UTF-8"); + assert_eq!( + stdout.trim(), + format!("2026-04-05T12:00:00Z {} run.completed", &run_id[..12]) + ); + attach_mock.assert(); +} + +#[test] +fn system_events_json_emits_raw_sse_payloads() { + let context = test_context!(); + let server = MockServer::start(); + let run_id = crate::support::unique_run_id(); + let payload = serde_json::json!({ + "payload": { + "ts": "2026-04-05T12:00:00Z", + "run_id": run_id, + "event": "run.completed", + } + }); + let attach_mock = server.mock(|when, then| { + when.method("GET") + .path("/api/v1/attach") + .query_param("run_id", run_id.as_str()); + then.status(200) + .header("Content-Type", "text/event-stream") + .body(format!("data: {payload}\n\n")); + }); + + let output = context + .command() + .args([ + "--json", + "system", + "events", + "--server", + &format!("{}/api/v1", server.base_url()), + "--run-id", + &run_id, + ]) + .output() + .expect("command should run"); + + assert!(output.status.success(), "system events failed"); + let stdout = String::from_utf8(output.stdout).expect("stdout should be UTF-8"); + assert_eq!(stdout.trim(), payload.to_string()); + attach_mock.assert(); +} diff --git a/lib/crates/fabro-cli/tests/it/cmd/system_info.rs b/lib/crates/fabro-cli/tests/it/cmd/system_info.rs index b7cd24027..ce218247f 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/system_info.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/system_info.rs @@ -48,3 +48,41 @@ fn system_info_json_reports_runtime_fields() { assert!(value["uptime_secs"].is_number()); assert!(value["runs"]["total"].is_number()); } + +#[test] +fn system_info_uses_explicit_storage_dir_override() { + let mut context = test_context!(); + let storage_dir = context.temp_dir.join("alternate-storage"); + std::fs::create_dir_all(&storage_dir).unwrap(); + context.write_home( + ".fabro/settings.toml", + format!( + "_version = 1\n\n[server.storage]\nroot = {:?}\n", + context.storage_dir.display().to_string() + ), + ); + context.ensure_home_server_auth_methods(); + context.manage_storage_dir(&storage_dir); + + let output = context + .command() + .args([ + "--json", + "system", + "info", + "--storage-dir", + storage_dir.to_str().unwrap(), + ]) + .output() + .expect("command should run"); + + assert!( + output.status.success(), + "system info failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + let value: Value = + serde_json::from_slice(&output.stdout).expect("system info JSON should parse"); + assert_eq!(value["storage_dir"], storage_dir.display().to_string()); +}