From 85c763d511cb4472716dfb3a9e5bc3a1396cd5a9 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 29 Apr 2026 08:02:50 -0400 Subject: [PATCH] fix(cli): persist auth login target When auth login succeeds and no CLI target is configured, write the resolved server target to settings.toml so later CLI commands can reuse it. --- Cargo.lock | 1 + lib/crates/fabro-cli/Cargo.toml | 1 + lib/crates/fabro-cli/src/command_context.rs | 4 + .../fabro-cli/src/commands/auth/login.rs | 14 ++ lib/crates/fabro-cli/src/user_config.rs | 127 ++++++++++++++++++ lib/crates/fabro-cli/tests/it/cmd/auth.rs | 7 + 6 files changed, 154 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index f88c821aa..a15db1858 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1717,6 +1717,7 @@ dependencies = [ "tokio", "tokio-util", "toml 0.8.23", + "toml_edit", "tracing", "tracing-appender", "tracing-subscriber", diff --git a/lib/crates/fabro-cli/Cargo.toml b/lib/crates/fabro-cli/Cargo.toml index d73982b86..63b369163 100644 --- a/lib/crates/fabro-cli/Cargo.toml +++ b/lib/crates/fabro-cli/Cargo.toml @@ -68,6 +68,7 @@ fs2.workspace = true serde.workspace = true thiserror.workspace = true toml.workspace = true +toml_edit.workspace = true futures.workspace = true regex.workspace = true semver.workspace = true diff --git a/lib/crates/fabro-cli/src/command_context.rs b/lib/crates/fabro-cli/src/command_context.rs index 7b55c393f..41e07cdde 100644 --- a/lib/crates/fabro-cli/src/command_context.rs +++ b/lib/crates/fabro-cli/src/command_context.rs @@ -111,6 +111,10 @@ impl CommandContext { &self.user_settings } + pub(crate) fn base_config_path(&self) -> &Path { + &self.base_config_path + } + pub(crate) fn json_output(&self) -> bool { self.user_settings.cli.output.format == OutputFormat::Json } diff --git a/lib/crates/fabro-cli/src/commands/auth/login.rs b/lib/crates/fabro-cli/src/commands/auth/login.rs index 9c582fdd1..df1a33154 100644 --- a/lib/crates/fabro-cli/src/commands/auth/login.rs +++ b/lib/crates/fabro-cli/src/commands/auth/login.rs @@ -49,6 +49,7 @@ pub(super) async fn login_command(args: AuthLoginArgs, base_ctx: &CommandContext logged_in_at: Utc::now(), }), )?; + configure_cli_target_after_login(base_ctx, &target)?; fabro_util::printerr!(printer, "Logged in to {} with dev-token", target); return Ok(()); } @@ -113,11 +114,24 @@ pub(super) async fn login_command(args: AuthLoginArgs, base_ctx: &CommandContext }; let summary = identity_summary(&entry.subject); AuthStore::default().put(&target, AuthEntry::OAuth(entry))?; + configure_cli_target_after_login(base_ctx, &target)?; fabro_util::printerr!(printer, "Logged in to {} as {}", target, summary); Ok(()) } } +fn configure_cli_target_after_login( + base_ctx: &CommandContext, + target: &ServerTarget, +) -> Result<()> { + user_config::configure_cli_target_if_missing( + base_ctx.base_config_path(), + base_ctx.user_settings(), + target, + )?; + Ok(()) +} + #[cfg(unix)] fn browser_origin(target: &ServerTarget) -> Result<&str> { if target.as_unix_socket_path().is_some() { diff --git a/lib/crates/fabro-cli/src/user_config.rs b/lib/crates/fabro-cli/src/user_config.rs index 55bb1cb7e..b6b373faf 100644 --- a/lib/crates/fabro-cli/src/user_config.rs +++ b/lib/crates/fabro-cli/src/user_config.rs @@ -14,6 +14,7 @@ use fabro_types::settings::server::LogDestination; use fabro_types::settings::{CliNamespace, InterpString, RunNamespace}; use fabro_types::{ServerSettings, UserSettings}; use fabro_util::version::FABRO_VERSION; +use toml_edit::{DocumentMut, Item, Table, value}; use tracing::debug; use crate::args::ServerTargetArgs; @@ -260,6 +261,80 @@ pub(crate) fn resolve_server_target( Ok(resolve_nondefault_server_target(args, settings)?.unwrap_or_else(default_server_target)) } +#[expect( + clippy::disallowed_methods, + reason = "CLI auth/login updates the user settings file synchronously after successful login." +)] +pub(crate) fn configure_cli_target_if_missing( + config_path: &Path, + settings: &UserSettings, + target: &ServerTarget, +) -> Result { + if settings.cli.target.is_some() { + return Ok(false); + } + + let mut document = read_settings_document_for_write(config_path)?; + if document + .get("cli") + .and_then(Item::as_table) + .is_some_and(|cli| cli.get("target").is_some_and(|target| !target.is_none())) + { + return Ok(false); + } + + if document.get("_version").is_none() { + document["_version"] = value(1); + } + + let cli = ensure_document_table(&mut document, "cli")?; + cli.insert("target", Item::Table(cli_target_table(target))); + + if let Some(parent) = config_path.parent() { + std::fs::create_dir_all(parent) + .with_context(|| format!("failed to create {}", parent.display()))?; + } + std::fs::write(config_path, document.to_string()) + .with_context(|| format!("failed to write {}", config_path.display()))?; + + Ok(true) +} + +fn read_settings_document_for_write(config_path: &Path) -> Result { + if !config_path.exists() { + return Ok(DocumentMut::new()); + } + + let contents = std::fs::read_to_string(config_path) + .with_context(|| format!("failed to read {}", config_path.display()))?; + contents + .parse::() + .with_context(|| format!("failed to parse {}", config_path.display())) +} + +fn ensure_document_table<'a>(document: &'a mut DocumentMut, key: &str) -> Result<&'a mut Table> { + if document.get(key).is_none() { + let mut table = Table::new(); + table.set_implicit(true); + document[key] = Item::Table(table); + } + document[key] + .as_table_mut() + .ok_or_else(|| anyhow!("settings.toml [{key}] is not a table")) +} + +fn cli_target_table(target: &ServerTarget) -> Table { + let mut table = Table::new(); + if let Some(url) = target.as_http_url() { + table.insert("type", value("http")); + table.insert("url", value(url)); + } else if let Some(path) = target.as_unix_socket_path() { + table.insert("type", value("unix")); + table.insert("path", value(path.display().to_string())); + } + table +} + pub(crate) fn exec_server_target(args: &ServerTargetArgs) -> Result> { let target = explicit_server_target(args)?; debug!(?target, "Resolved exec server target"); @@ -428,6 +503,58 @@ url = "https://config.example.com" ); } + #[test] + #[expect( + clippy::disallowed_methods, + reason = "unit test writes a temporary settings fixture with sync std::fs" + )] + fn configure_cli_target_creates_settings_file_when_missing() { + let dir = tempfile::tempdir().unwrap(); + let config_path = dir.path().join(".fabro").join("settings.toml"); + let settings = UserSettings::default(); + let target = ServerTarget::http_url("http://127.0.0.1:32276").unwrap(); + + assert!(configure_cli_target_if_missing(&config_path, &settings, &target).unwrap()); + + let contents = std::fs::read_to_string(&config_path).unwrap(); + insta::assert_snapshot!(contents, @r#" + _version = 1 + + [cli.target] + type = "http" + url = "http://127.0.0.1:32276" + "#); + let settings = UserSettingsBuilder::load_from(&config_path).unwrap(); + assert_eq!( + resolve_server_target(&server_target_args(None), &settings).unwrap(), + target + ); + } + + #[test] + #[expect( + clippy::disallowed_methods, + reason = "unit test writes a temporary settings fixture with sync std::fs" + )] + fn configure_cli_target_does_not_overwrite_existing_target() { + let dir = tempfile::tempdir().unwrap(); + let config_path = dir.path().join("settings.toml"); + let existing = r#" +_version = 1 + +[cli.target] +type = "http" +url = "https://configured.example.com" +"#; + std::fs::write(&config_path, existing).unwrap(); + let settings = UserSettingsBuilder::load_from(&config_path).unwrap(); + let target = ServerTarget::http_url("https://new.example.com").unwrap(); + + assert!(!configure_cli_target_if_missing(&config_path, &settings, &target).unwrap()); + + assert_eq!(std::fs::read_to_string(&config_path).unwrap(), existing); + } + #[test] fn storage_dir_defaults_without_server_auth_methods() { let document = toml::Value::Table(toml::Table::new()); diff --git a/lib/crates/fabro-cli/tests/it/cmd/auth.rs b/lib/crates/fabro-cli/tests/it/cmd/auth.rs index c5214bf1c..bcaa953c2 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/auth.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/auth.rs @@ -68,6 +68,9 @@ fn login_help() { )] fn login_with_dev_token_writes_auth_store_entry() { let context = test_context!(); + let settings_file = context.home_dir.join(".fabro").join("settings.toml"); + std::fs::write(&settings_file, "_version = 1\n").unwrap(); + let mut cmd = context.command(); cmd.args([ "auth", @@ -91,6 +94,10 @@ fn login_with_dev_token_writes_auth_store_entry() { let entry = &auth["servers"]["http://127.0.0.1:32276"]; assert_eq!(entry["kind"], "dev-token"); assert_eq!(entry["token"], DEV_TOKEN); + + let settings = std::fs::read_to_string(settings_file).unwrap(); + assert!(settings.contains("[cli.target]")); + assert!(settings.contains("http://127.0.0.1:32276")); } #[test]