From 881113d121d57433536a960d673ce2c69c5c08e2 Mon Sep 17 00:00:00 2001 From: naps62-yolo Date: Thu, 18 Jun 2026 14:35:30 +0000 Subject: [PATCH] refactor(maestro): consolidate bind_addr + state_dir config helpers (NAP-273 B/C) Add `parse_bind_addr`, `default_state_dir(subdir)`, and an `InvalidBindAddr` variant to `maestro-config`, and route the daemon, registry, and web config loaders through them. Each crate's hand-rolled `InvalidBindAddr` variant and duplicated XDG state-dir resolution are gone; their loaders now `?` the shared helper, which converts into their existing transparent `Inner` variant. Also adds the shared `BroadcastTuning` struct (used by the next commit). Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/maestro-config/src/lib.rs | 86 ++++++++++++++++++++++++++++ crates/maestro-daemon/src/config.rs | 21 ++----- crates/maestro-registry/src/token.rs | 16 ++---- crates/maestro-web/src/config.rs | 19 +----- 4 files changed, 97 insertions(+), 45 deletions(-) diff --git a/crates/maestro-config/src/lib.rs b/crates/maestro-config/src/lib.rs index 677767d..1b3ff57 100644 --- a/crates/maestro-config/src/lib.rs +++ b/crates/maestro-config/src/lib.rs @@ -60,6 +60,12 @@ pub enum ConfigError { MissingFile { path: PathBuf }, #[error("invalid registry_url {input:?}: {reason}")] InvalidRegistryUrl { input: String, reason: String }, + #[error("invalid {env} ({input:?}): {source}")] + InvalidBindAddr { + env: &'static str, + input: String, + source: std::net::AddrParseError, + }, } /// On-disk representation of the unified config. Every section is optional @@ -344,10 +350,90 @@ pub fn derive_daemon_ws_url(registry_url: &str) -> Result { } } +/// Parse a `SocketAddr` from a raw (usually env-sourced) string, tagging the +/// failure with the originating env-var name so every binary surfaces the same +/// `invalid (...)` diagnostic. Shared by the daemon, registry, and web +/// `bind_addr` resolution paths (NAP-273). +pub fn parse_bind_addr(raw: &str, env: &'static str) -> Result { + raw.parse().map_err(|source| ConfigError::InvalidBindAddr { + env, + input: raw.to_string(), + source, + }) +} + +/// XDG state directory for a Maestro component, e.g. `~/.local/state/` +/// (falling back to the data-local dir when `XDG_STATE_HOME`/`$HOME` are +/// unset). The daemon passes `"maestro"`, `maestro-web` passes `"maestro-web"` +/// (NAP-273). +pub fn default_state_dir(subdir: &str) -> Result { + let base = dirs::state_dir() + .or_else(dirs::data_local_dir) + .ok_or(ConfigError::NoConfigDir)?; + Ok(base.join(subdir)) +} + +/// Capacities for the daemon/registry broadcast channels (session events and +/// PTY byte fan-out). These were four independent `= 256` constants scattered +/// across the daemon and registry; centralizing them here makes the coincidence +/// explicit and gives a single home for per-deployment tuning later (NAP-273). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct BroadcastTuning { + /// Session/registry event channels (`SessionEvent`, `RegistryEvent`). + pub events_capacity: usize, + /// Per-session PTY byte broadcast channel. + pub pty_capacity: usize, +} + +impl BroadcastTuning { + /// The historical defaults — every channel was 256 by coincidence. + pub const DEFAULT: Self = Self { + events_capacity: 256, + pty_capacity: 256, + }; +} + +impl Default for BroadcastTuning { + fn default() -> Self { + Self::DEFAULT + } +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn parse_bind_addr_ok_and_err() { + assert_eq!( + parse_bind_addr("127.0.0.1:7070", "MAESTRO_X").unwrap(), + "127.0.0.1:7070".parse().unwrap() + ); + let err = parse_bind_addr("not-an-addr", "MAESTRO_X").unwrap_err(); + match err { + ConfigError::InvalidBindAddr { env, input, .. } => { + assert_eq!(env, "MAESTRO_X"); + assert_eq!(input, "not-an-addr"); + } + other => panic!("expected InvalidBindAddr, got {other:?}"), + } + } + + #[test] + fn default_state_dir_appends_subdir() { + // On any host with XDG_STATE_HOME / HOME set (CI included) this is Some. + if let Ok(p) = default_state_dir("maestro-xyz") { + assert!(p.ends_with("maestro-xyz")); + } + } + + #[test] + fn broadcast_tuning_defaults_to_256() { + assert_eq!(BroadcastTuning::default(), BroadcastTuning::DEFAULT); + assert_eq!(BroadcastTuning::DEFAULT.events_capacity, 256); + assert_eq!(BroadcastTuning::DEFAULT.pty_capacity, 256); + } + #[test] fn missing_file_strict_load_errors_with_pointer_to_configure() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/maestro-daemon/src/config.rs b/crates/maestro-daemon/src/config.rs index 9123242..07f955a 100644 --- a/crates/maestro-daemon/src/config.rs +++ b/crates/maestro-daemon/src/config.rs @@ -59,12 +59,6 @@ pub enum ConfigError { env: &'static str, path: PathBuf, }, - #[error("invalid {env} ({input:?}): {source}")] - InvalidBindAddr { - env: &'static str, - input: String, - source: std::net::AddrParseError, - }, } /// Resolved, validated daemon configuration ready to drive the binary. @@ -224,11 +218,7 @@ pub fn load_config(path: &Path, env: EnvOverrides) -> Result raw.parse().map_err(|source| ConfigError::InvalidBindAddr { - env: ENV_DAEMON_BIND_ADDR, - input: raw, - source, - })?, + Some(raw) => maestro_config::parse_bind_addr(&raw, ENV_DAEMON_BIND_ADDR)?, None => daemon.bind_addr.unwrap_or_else(default_bind_addr), }; @@ -275,10 +265,7 @@ fn env_has_all_required(env: &EnvOverrides) -> bool { } pub fn default_state_dir() -> Result { - let base = dirs::state_dir() - .or_else(dirs::data_local_dir) - .ok_or(maestro_config::ConfigError::NoConfigDir)?; - Ok(base.join("maestro")) + Ok(maestro_config::default_state_dir("maestro")?) } /// Read the kernel hostname via `gethostname(2)`. Falls back to `"unknown"` @@ -426,7 +413,9 @@ daemon_url = "ws://x" }; let err = load_config(&path, env).unwrap_err(); match err { - ConfigError::InvalidBindAddr { input, env, .. } => { + ConfigError::Inner(maestro_config::ConfigError::InvalidBindAddr { + input, env, .. + }) => { assert_eq!(input, "not-an-addr"); assert_eq!(env, ENV_DAEMON_BIND_ADDR); } diff --git a/crates/maestro-registry/src/token.rs b/crates/maestro-registry/src/token.rs index 847279e..299b9dd 100644 --- a/crates/maestro-registry/src/token.rs +++ b/crates/maestro-registry/src/token.rs @@ -44,12 +44,6 @@ pub enum TokenError { Inner(#[from] maestro_config::ConfigError), #[error("io: {0}")] Io(#[from] std::io::Error), - #[error("invalid {env} ({input:?}): {source}")] - InvalidBindAddr { - env: &'static str, - input: String, - source: std::net::AddrParseError, - }, } /// Resolved registry config: the token to sign JWTs with and the address @@ -109,11 +103,7 @@ pub fn load_config( }; if let Some(raw) = env_bind_addr { - cfg.bind_addr = raw.parse().map_err(|source| TokenError::InvalidBindAddr { - env: ENV_REGISTRY_BIND_ADDR, - input: raw.to_string(), - source, - })?; + cfg.bind_addr = maestro_config::parse_bind_addr(raw, ENV_REGISTRY_BIND_ADDR)?; } Ok(cfg) @@ -278,7 +268,9 @@ bind_addr = "127.0.0.1:1" let path = dir.path().join("config.toml"); let err = load_config(&path, Some("t".into()), Some("not-an-addr")).unwrap_err(); match err { - TokenError::InvalidBindAddr { input, env, .. } => { + TokenError::Inner(maestro_config::ConfigError::InvalidBindAddr { + input, env, .. + }) => { assert_eq!(input, "not-an-addr"); assert_eq!(env, ENV_REGISTRY_BIND_ADDR); } diff --git a/crates/maestro-web/src/config.rs b/crates/maestro-web/src/config.rs index 4cc11b1..b7050fb 100644 --- a/crates/maestro-web/src/config.rs +++ b/crates/maestro-web/src/config.rs @@ -24,12 +24,6 @@ fn default_bind_addr() -> SocketAddr { pub enum WebConfigError { #[error(transparent)] Inner(#[from] maestro_config::ConfigError), - #[error("invalid {env} ({input:?}): {source}")] - InvalidBindAddr { - env: &'static str, - input: String, - source: std::net::AddrParseError, - }, } /// Resolved runtime configuration for the `maestro-web` binary. @@ -75,13 +69,7 @@ pub fn load_config(path: &Path, env: EnvOverrides) -> Result Result { let bind_addr = match env.bind_addr { - Some(raw) => raw - .parse() - .map_err(|source| WebConfigError::InvalidBindAddr { - env: ENV_WEB_BIND_ADDR, - input: raw, - source, - })?, + Some(raw) => maestro_config::parse_bind_addr(&raw, ENV_WEB_BIND_ADDR)?, None => web.bind_addr.unwrap_or_else(default_bind_addr), }; @@ -107,10 +95,7 @@ fn resolve_with(web: WebSection, env: EnvOverrides) -> Result Result { - let base = dirs::state_dir() - .or_else(dirs::data_local_dir) - .ok_or(maestro_config::ConfigError::NoConfigDir)?; - Ok(base.join("maestro-web")) + Ok(maestro_config::default_state_dir("maestro-web")?) } #[cfg(test)]