From efaa8bfe138bb71bc26d3d19a30d368eff3d712b Mon Sep 17 00:00:00 2001 From: naps62-yolo Date: Tue, 26 May 2026 09:24:51 +0000 Subject: [PATCH] feat(maestro): wire maestro-cli to unified config (NAP-233) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLI now reads `[shared].registry_url` and `[cli].cookie` from the unified file via maestro-config instead of `~/.config/maestro/cli.toml`. The two persistence paths are now distinct: - `maestro set-registry ` → writes `[shared].registry_url` - captured Set-Cookie on first contact → writes `[cli].cookie` Both go through read-modify-write so they preserve any `[daemon]` / `[registry]` content already in the file. Env-var renames on the CLI flag layer: - `--registry-url` reads `MAESTRO_SHARED_REGISTRY_URL` (was `MAESTRO_REGISTRY_URL`) - `--config` reads `MAESTRO_CONFIG` (was `MAESTRO_CLI_CONFIG`) Default config path is now `~/.config/maestro/config.toml`. --- Cargo.lock | 12 ++ crates/maestro-cli/Cargo.toml | 1 + crates/maestro-cli/src/cli.rs | 8 +- crates/maestro-cli/src/config.rs | 187 +++++++++++++++++++++---------- crates/maestro-cli/src/http.rs | 6 +- crates/maestro-cli/src/main.rs | 5 +- 6 files changed, 148 insertions(+), 71 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0e23b15..7c94808 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -909,6 +909,7 @@ dependencies = [ "dirs", "futures-util", "libc", + "maestro-config", "maestro-protocol", "nix", "reqwest", @@ -924,6 +925,17 @@ dependencies = [ "uuid", ] +[[package]] +name = "maestro-config" +version = "0.1.0" +dependencies = [ + "dirs", + "serde", + "tempfile", + "thiserror 1.0.69", + "toml", +] + [[package]] name = "maestro-daemon" version = "0.1.0" diff --git a/crates/maestro-cli/Cargo.toml b/crates/maestro-cli/Cargo.toml index e1f9846..87c8f89 100644 --- a/crates/maestro-cli/Cargo.toml +++ b/crates/maestro-cli/Cargo.toml @@ -20,6 +20,7 @@ clap.workspace = true dirs.workspace = true futures-util.workspace = true libc.workspace = true +maestro-config.workspace = true maestro-protocol.workspace = true nix.workspace = true reqwest.workspace = true diff --git a/crates/maestro-cli/src/cli.rs b/crates/maestro-cli/src/cli.rs index 9727735..c140dd1 100644 --- a/crates/maestro-cli/src/cli.rs +++ b/crates/maestro-cli/src/cli.rs @@ -14,12 +14,12 @@ use uuid::Uuid; )] pub struct Cli { /// Override the registry URL for this invocation (e.g. `https://maestro.example.com`). - /// If omitted, the URL persisted in `~/.config/maestro/cli.toml` is used. - #[arg(long, global = true, env = "MAESTRO_REGISTRY_URL")] + /// If omitted, `[shared].registry_url` from the unified config file is used. + #[arg(long, global = true, env = "MAESTRO_SHARED_REGISTRY_URL")] pub registry_url: Option, - /// Path to the CLI config file. Default `~/.config/maestro/cli.toml`. - #[arg(long, global = true, env = "MAESTRO_CLI_CONFIG")] + /// Path to the unified Maestro config file. Default `~/.config/maestro/config.toml`. + #[arg(long, global = true, env = "MAESTRO_CONFIG")] pub config: Option, /// Increase log verbosity. Repeat for more: `-v` = debug, `-vv` = trace. diff --git a/crates/maestro-cli/src/config.rs b/crates/maestro-cli/src/config.rs index 3e58b24..5905e39 100644 --- a/crates/maestro-cli/src/config.rs +++ b/crates/maestro-cli/src/config.rs @@ -1,103 +1,172 @@ -//! CLI config persisted at `~/.config/maestro/cli.toml`. +//! CLI view over the unified Maestro config at `~/.config/maestro/config.toml`. //! -//! Stores the default registry URL and the auth cookie issued by the -//! registry's cookie middleware (`crates/maestro-registry/src/auth.rs`). The -//! cookie is opaque to the CLI — we just round-trip it as a string. +//! Reads `[shared].registry_url` (registry HTTP base) and `[cli].cookie` +//! (persisted `maestro_session` JWT). All other sections in the file are +//! preserved on write. use std::path::{Path, PathBuf}; -use serde::{Deserialize, Serialize}; +use maestro_config::MaestroConfigFile; + +pub use maestro_config::CONFIG_PATH_ENV; + +/// `~/.config/maestro/config.toml` (XDG-aware). +pub fn default_config_path() -> Result { + Ok(maestro_config::default_config_path()?) +} + +/// `$MAESTRO_CONFIG` if set, else [`default_config_path`]. +pub fn resolve_config_path() -> Result { + Ok(maestro_config::resolve_config_path()?) +} #[derive(Debug, thiserror::Error)] pub enum ConfigError { - #[error("io: {0}")] - Io(#[from] std::io::Error), - #[error("toml decode: {0}")] - TomlDecode(#[from] toml::de::Error), - #[error("toml encode: {0}")] - TomlEncode(#[from] toml::ser::Error), - #[error("could not determine config directory (XDG_CONFIG_HOME / $HOME unset?)")] - NoConfigDir, + #[error(transparent)] + Inner(#[from] maestro_config::ConfigError), #[error( - "no registry URL configured — set MAESTRO_REGISTRY_URL or run `maestro set-registry `" + "no registry URL configured — pass --registry-url, set \ + MAESTRO_SHARED_REGISTRY_URL, or run `maestro set-registry `" )] NoRegistryUrl, } -#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +/// Resolved CLI config — the subset of the unified file that this binary +/// actually consumes. +#[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct CliConfig { - /// Default registry base URL (e.g. `https://maestro.example.com`). - #[serde(default, skip_serializing_if = "Option::is_none")] pub registry_url: Option, - - /// Persisted `maestro_session` cookie value (a JWT) — opaque blob. - #[serde(default, skip_serializing_if = "Option::is_none")] pub cookie: Option, } impl CliConfig { - /// Load from disk if present; return an empty config if not. + /// Load `[shared].registry_url` + `[cli].cookie` from the unified file. + /// Returns an empty config if the file is missing — the CLI tolerates a + /// missing file so `maestro set-registry ` can bootstrap it. pub fn load(path: &Path) -> Result { - if !path.exists() { - return Ok(Self::default()); - } - let bytes = std::fs::read(path)?; - let s = std::str::from_utf8(&bytes).unwrap_or(""); - Ok(toml::from_str(s)?) + let file = MaestroConfigFile::load_or_default(path)?; + Ok(Self { + registry_url: file.shared.registry_url, + cookie: file.cli.cookie, + }) } - /// Atomically write the config back to disk (write to tmp + rename). - pub fn save(&self, path: &Path) -> Result<(), ConfigError> { - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent)?; - } - let body = toml::to_string_pretty(self)?; - let tmp = path.with_extension("toml.tmp"); - std::fs::write(&tmp, body)?; - std::fs::rename(&tmp, path)?; + /// Persist `[shared].registry_url`. Read-modify-write — preserves + /// any other sections already in the file (`[daemon]`, `[registry]`, + /// existing `[cli].cookie`). + pub fn save_registry_url(path: &Path, url: &str) -> Result<(), ConfigError> { + let mut file = MaestroConfigFile::load_or_default(path)?; + file.shared.registry_url = Some(url.to_string()); + file.save(path)?; Ok(()) } -} -/// Default config path: `~/.config/maestro/cli.toml`. -pub fn default_config_path() -> Result { - let base = dirs::config_dir().ok_or(ConfigError::NoConfigDir)?; - Ok(base.join("maestro").join("cli.toml")) + /// Persist `[cli].cookie`. Same read-modify-write semantics as + /// [`save_registry_url`]; in particular this does not touch + /// `[shared].registry_url`. + pub fn save_cookie(path: &Path, cookie: &str) -> Result<(), ConfigError> { + let mut file = MaestroConfigFile::load_or_default(path)?; + file.cli.cookie = Some(cookie.to_string()); + file.save(path)?; + Ok(()) + } } #[cfg(test)] mod tests { use super::*; + use maestro_config::SharedSection; + + fn write_file(path: &Path, body: &str) { + std::fs::write(path, body).unwrap(); + } #[test] - fn missing_file_returns_default() { + fn missing_file_returns_empty() { let dir = tempfile::tempdir().unwrap(); - let cfg = CliConfig::load(&dir.path().join("cli.toml")).unwrap(); + let cfg = CliConfig::load(&dir.path().join("config.toml")).unwrap(); assert_eq!(cfg, CliConfig::default()); } #[test] - fn round_trip() { + fn load_reads_shared_registry_url_and_cli_cookie() { let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("cli.toml"); - let cfg = CliConfig { - registry_url: Some("https://r.example.com".into()), - cookie: Some("eyJ...".into()), - }; - cfg.save(&path).unwrap(); - assert_eq!(CliConfig::load(&path).unwrap(), cfg); + let path = dir.path().join("config.toml"); + write_file( + &path, + r#" +[shared] +registry_url = "https://r.example" + +[cli] +cookie = "eyJ..." +"#, + ); + let cfg = CliConfig::load(&path).unwrap(); + assert_eq!(cfg.registry_url.as_deref(), Some("https://r.example")); + assert_eq!(cfg.cookie.as_deref(), Some("eyJ...")); } #[test] - fn save_creates_parent_directory() { + fn save_registry_url_preserves_other_sections() { let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("nested/here/cli.toml"); - CliConfig { - registry_url: Some("https://x".into()), - cookie: None, - } - .save(&path) - .unwrap(); + let path = dir.path().join("config.toml"); + write_file( + &path, + r#" +[shared] +token = "abc" + +[daemon] +daemon_url = "ws://laptop:7070" +"#, + ); + CliConfig::save_registry_url(&path, "https://new.example/").unwrap(); + let after = MaestroConfigFile::load(&path).unwrap(); + assert_eq!( + after.shared.registry_url.as_deref(), + Some("https://new.example/") + ); + assert_eq!(after.shared.token.as_deref(), Some("abc")); + assert_eq!(after.daemon.daemon_url.as_deref(), Some("ws://laptop:7070")); + } + + #[test] + fn save_cookie_does_not_touch_registry_url() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + write_file( + &path, + r#" +[shared] +registry_url = "https://r.example" +"#, + ); + CliConfig::save_cookie(&path, "eyJ.new").unwrap(); + let after = MaestroConfigFile::load(&path).unwrap(); + assert_eq!( + after.shared.registry_url.as_deref(), + Some("https://r.example") + ); + assert_eq!(after.cli.cookie.as_deref(), Some("eyJ.new")); + } + + #[test] + fn save_creates_file_if_missing() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("nested/config.toml"); + CliConfig::save_registry_url(&path, "https://x").unwrap(); assert!(path.exists()); + let after = MaestroConfigFile::load(&path).unwrap(); + assert_eq!( + after, + MaestroConfigFile { + shared: SharedSection { + registry_url: Some("https://x".into()), + token: None, + }, + ..Default::default() + } + ); } } diff --git a/crates/maestro-cli/src/http.rs b/crates/maestro-cli/src/http.rs index 10068c4..8d424d4 100644 --- a/crates/maestro-cli/src/http.rs +++ b/crates/maestro-cli/src/http.rs @@ -101,11 +101,7 @@ impl RegistryClient { } *guard = Some(val.to_string()); // Persist; on failure, log but don't abort the request. - let cfg = CliConfig { - registry_url: Some(self.base.clone()), - cookie: Some(val.to_string()), - }; - if let Err(e) = cfg.save(&self.config_path) { + if let Err(e) = CliConfig::save_cookie(&self.config_path, val) { tracing::warn!(error = %e, path = %self.config_path.display(), "failed to persist cookie"); } Ok(()) diff --git a/crates/maestro-cli/src/main.rs b/crates/maestro-cli/src/main.rs index 063c1e8..eedf6f8 100644 --- a/crates/maestro-cli/src/main.rs +++ b/crates/maestro-cli/src/main.rs @@ -64,9 +64,8 @@ async fn dispatch(cli: Cli) -> Result<(), CliError> { // `set-registry` is the only command that writes config first and skips // the registry-required path below. if let Command::SetRegistry(args) = &cli.command { - let mut cfg = CliConfig::load(&config_path)?; - cfg.registry_url = Some(args.url.trim_end_matches('/').to_string()); - cfg.save(&config_path)?; + let url = args.url.trim_end_matches('/'); + CliConfig::save_registry_url(&config_path, url)?; println!("registry URL saved to {}", config_path.display()); return Ok(()); }