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) <noreply@anthropic.com>
This commit is contained in:
@@ -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<String, ConfigError> {
|
||||
}
|
||||
}
|
||||
|
||||
/// 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 <ENV> (...)` 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<SocketAddr, ConfigError> {
|
||||
raw.parse().map_err(|source| ConfigError::InvalidBindAddr {
|
||||
env,
|
||||
input: raw.to_string(),
|
||||
source,
|
||||
})
|
||||
}
|
||||
|
||||
/// XDG state directory for a Maestro component, e.g. `~/.local/state/<subdir>`
|
||||
/// (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<PathBuf, ConfigError> {
|
||||
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();
|
||||
|
||||
@@ -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<DaemonConfig, Confi
|
||||
})?;
|
||||
|
||||
let bind_addr = match env.bind_addr {
|
||||
Some(raw) => 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<PathBuf, ConfigError> {
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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<WebConfig, WebConfi
|
||||
|
||||
fn resolve_with(web: WebSection, env: EnvOverrides) -> Result<WebConfig, WebConfigError> {
|
||||
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<WebConfig, WebConf
|
||||
/// Standard state dir (mostly so callers have a place to put logs — the
|
||||
/// binary itself is stateless).
|
||||
pub fn default_state_dir() -> Result<PathBuf, WebConfigError> {
|
||||
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)]
|
||||
|
||||
Reference in New Issue
Block a user