From 6596104789500bd861c7f37aa89b524bf639358e Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Sat, 22 Aug 2026 14:39:01 +0100 Subject: [PATCH] refactor(maestro): delete the config projections and the dead knobs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit maestro-cli/src/config.rs was two pass-throughs, a newtype error, a two-field projection of MaestroConfigFile, and one four-line or_else with a single caller. `resolve_daemon_url` moves onto MaestroConfigFile, next to the schema it reads, and the module goes; its tests move with it. `[web].dev_proxy_target` is deleted end to end — schema, env var, normalisation, three tests, README line, and the startup warning that admitted the server ignores it. MaestroConfigFile::save had no callers outside its own tests. Deleting it also removes every skip_serializing_if, the five is_empty impls, and the Serialize derives that existed only to make its output tidy. BroadcastTuning was a struct that was never instantiated: all three uses were `::DEFAULT.field` in const position and there is no [broadcast] section. Now two plain consts. $MAESTRO_CONFIG was honoured twice, by resolve_config_path and by clap's `env =`. Every binary already gets it through clap, so resolve_config_path and CONFIG_PATH_ENV go and the TUI falls back to default_config_path like everything else. Also collapses three layers of 1:1 forwarding: the umbrella's cli_dispatch shim, and four near-identical 8-line functions in cli/dispatch.rs that differed only in which run_* they called. Co-Authored-By: Claude Opus 5 (1M context) --- crates/maestro-cli/src/config.rs | 128 ------------ crates/maestro-cli/src/dispatch.rs | 94 +++------ crates/maestro-cli/src/lib.rs | 1 - crates/maestro-config/src/lib.rs | 278 +++++++-------------------- crates/maestro-daemon/src/config.rs | 4 +- crates/maestro-daemon/src/pty.rs | 4 +- crates/maestro-daemon/src/session.rs | 10 +- crates/maestro-tui/src/lib.rs | 4 +- crates/maestro-web/README.md | 1 - crates/maestro-web/src/config.rs | 24 +-- crates/maestro-web/src/main.rs | 8 - crates/maestro-web/tests/proxy.rs | 1 - crates/maestro/src/main.rs | 26 +-- 13 files changed, 123 insertions(+), 460 deletions(-) delete mode 100644 crates/maestro-cli/src/config.rs diff --git a/crates/maestro-cli/src/config.rs b/crates/maestro-cli/src/config.rs deleted file mode 100644 index 7eaeba4..0000000 --- a/crates/maestro-cli/src/config.rs +++ /dev/null @@ -1,128 +0,0 @@ -//! CLI view over the unified Maestro config at `~/.config/maestro/config.toml`. -//! -//! Reads `[cli].daemon_url` and, as a fallback, `[daemon].bind_addr`. - -use std::net::SocketAddr; -use std::path::{Path, PathBuf}; - -use maestro_config::MaestroConfigFile; - -pub use maestro_config::CONFIG_PATH_ENV; - -/// Last-resort daemon HTTP base when neither config nor a local -/// `[daemon].bind_addr` tells us otherwise. Matches the daemon's default -/// `bind_addr`. -pub const DEFAULT_DAEMON_URL: &str = "http://127.0.0.1:7070"; - -/// `~/.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(transparent)] - Inner(#[from] maestro_config::ConfigError), -} - -/// Resolved CLI config — the subset of the unified file that this binary -/// actually consumes. -#[derive(Debug, Clone, Default, PartialEq, Eq)] -pub struct CliConfig { - /// `[cli].daemon_url` — explicit daemon HTTP base. - pub daemon_url: Option, - /// `[daemon].bind_addr` if a daemon is configured on this host. Used to - /// derive the default daemon URL when `daemon_url` is unset. - pub daemon_bind_addr: Option, -} - -impl CliConfig { - /// Load the subset of the unified file the CLI consumes. A missing file is - /// fine — every field has a default. - pub fn load(path: &Path) -> Result { - let file = MaestroConfigFile::load(path)?; - Ok(Self { - daemon_url: file.cli.daemon_url, - daemon_bind_addr: file.daemon.bind_addr, - }) - } - - /// Resolve the daemon HTTP base to talk to. - /// - /// Precedence: explicit `override_url` (CLI flag / env) → `[cli].daemon_url` - /// → `http://<[daemon].bind_addr>` → [`DEFAULT_DAEMON_URL`]. Always returns - /// a value; reachability is decided by whether the connection succeeds. - pub fn resolve_daemon_url(&self, override_url: Option) -> String { - override_url - .or_else(|| self.daemon_url.clone()) - .or_else(|| self.daemon_bind_addr.map(|a| format!("http://{a}"))) - .unwrap_or_else(|| DEFAULT_DAEMON_URL.to_string()) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - fn write_file(path: &Path, body: &str) { - std::fs::write(path, body).unwrap(); - } - - #[test] - fn missing_file_returns_empty() { - let dir = tempfile::tempdir().unwrap(); - let cfg = CliConfig::load(&dir.path().join("config.toml")).unwrap(); - assert_eq!(cfg, CliConfig::default()); - } - - #[test] - fn resolve_daemon_url_precedence() { - // override wins over everything - let cfg = CliConfig { - daemon_url: Some("http://from-cli-section:7070".into()), - daemon_bind_addr: Some("127.0.0.1:9999".parse().unwrap()), - }; - assert_eq!( - cfg.resolve_daemon_url(Some("http://override:1".into())), - "http://override:1" - ); - // [cli].daemon_url next - assert_eq!(cfg.resolve_daemon_url(None), "http://from-cli-section:7070"); - // derive from [daemon].bind_addr when no explicit url - let cfg = CliConfig { - daemon_bind_addr: Some("10.0.0.7:7070".parse().unwrap()), - ..Default::default() - }; - assert_eq!(cfg.resolve_daemon_url(None), "http://10.0.0.7:7070"); - // built-in default when nothing is configured - let cfg = CliConfig::default(); - assert_eq!(cfg.resolve_daemon_url(None), DEFAULT_DAEMON_URL); - } - - #[test] - fn load_reads_daemon_url_and_daemon_bind_addr() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("config.toml"); - write_file( - &path, - r#" -[cli] -daemon_url = "http://laptop:7070" - -[daemon] -bind_addr = "127.0.0.1:7071" -"#, - ); - let cfg = CliConfig::load(&path).unwrap(); - assert_eq!(cfg.daemon_url.as_deref(), Some("http://laptop:7070")); - assert_eq!( - cfg.daemon_bind_addr, - Some("127.0.0.1:7071".parse().unwrap()) - ); - } -} diff --git a/crates/maestro-cli/src/dispatch.rs b/crates/maestro-cli/src/dispatch.rs index 16f19e6..b1a6fed 100644 --- a/crates/maestro-cli/src/dispatch.rs +++ b/crates/maestro-cli/src/dispatch.rs @@ -9,15 +9,15 @@ use std::path::{Path, PathBuf}; use maestro_protocol::DaemonClient; use crate::attach::{parse_detach_key, run_attach_url}; -use crate::cli::{AttachArgs, Command, KillArgs, ListArgs, SpawnArgs}; -use crate::config::{default_config_path, CliConfig, ConfigError}; +use crate::cli::Command; use crate::sessions::{run_kill, run_list, run_spawn}; +use maestro_config::{default_config_path, MaestroConfigFile}; #[derive(Debug, thiserror::Error)] #[allow(clippy::result_large_err)] pub enum CliError { #[error("config: {0}")] - Config(#[from] ConfigError), + Config(#[from] maestro_config::ConfigError), #[error("{0}")] Client(#[from] maestro_protocol::ClientError), #[error("{0}")] @@ -39,76 +39,42 @@ pub async fn dispatch( None => default_config_path()?, }; + let daemon = client(&config_path, daemon_url_override)?; + match command { - Command::List(args) => list(&config_path, daemon_url_override, args).await, - Command::Spawn(args) => spawn(&config_path, daemon_url_override, args).await, - Command::Attach(args) => attach(&config_path, daemon_url_override, args).await, - Command::Kill(args) => kill(&config_path, daemon_url_override, args).await, - } -} - -#[allow(clippy::result_large_err)] -async fn list( - config_path: &Path, - daemon_url_override: Option, - args: ListArgs, -) -> Result<(), CliError> { - let daemon = client(config_path, daemon_url_override)?; - let mut stdout = std::io::stdout().lock(); - run_list(&daemon, &args, &mut stdout).await?; - Ok(()) -} - -#[allow(clippy::result_large_err)] -async fn spawn( - config_path: &Path, - daemon_url_override: Option, - args: SpawnArgs, -) -> Result<(), CliError> { - let daemon = client(config_path, daemon_url_override)?; - let attach_after = args.attach; - // `spawn --attach` has no detach-key flag; use the attach default (Ctrl-q). - let detach_key = 0x11; - - let session = run_spawn(&daemon, args).await?; - println!("spawned {}", session.id); - - if attach_after { - let url = daemon.attach_url(session.id)?; - run_attach_url(&url, detach_key).await?; + Command::List(args) => { + let mut stdout = std::io::stdout().lock(); + run_list(&daemon, &args, &mut stdout).await?; + } + Command::Spawn(args) => { + let attach_after = args.attach; + let session = run_spawn(&daemon, args).await?; + println!("spawned {}", session.id); + if attach_after { + // `spawn --attach` has no detach-key flag; use the attach + // default (Ctrl-q). + run_attach_url(&daemon.attach_url(session.id)?, CTRL_Q).await?; + } + } + Command::Attach(args) => { + let detach = parse_detach_key(&args.detach_key)?; + run_attach_url(&daemon.attach_url(args.session_id)?, detach).await?; + } + Command::Kill(args) => { + run_kill(&daemon, &args).await?; + println!("killed {}", args.session_id); + } } Ok(()) } -#[allow(clippy::result_large_err)] -async fn attach( - config_path: &Path, - daemon_url_override: Option, - args: AttachArgs, -) -> Result<(), CliError> { - let daemon = client(config_path, daemon_url_override)?; - let detach = parse_detach_key(&args.detach_key)?; - let url = daemon.attach_url(args.session_id)?; - run_attach_url(&url, detach).await?; - Ok(()) -} - -#[allow(clippy::result_large_err)] -async fn kill( - config_path: &Path, - daemon_url_override: Option, - args: KillArgs, -) -> Result<(), CliError> { - let daemon = client(config_path, daemon_url_override)?; - run_kill(&daemon, &args).await?; - println!("killed {}", args.session_id); - Ok(()) -} +/// Default detach byte, matching `maestro attach --detach-key`'s `^q`. +const CTRL_Q: u8 = 0x11; /// HTTP client pointed at the daemon's API. That surface needs no auth — /// reaching `bind_addr` is the authorization. #[allow(clippy::result_large_err)] fn client(config_path: &Path, override_url: Option) -> Result { - let cfg = CliConfig::load(config_path)?; + let cfg = MaestroConfigFile::load(config_path)?; Ok(DaemonClient::new(cfg.resolve_daemon_url(override_url))) } diff --git a/crates/maestro-cli/src/lib.rs b/crates/maestro-cli/src/lib.rs index 4e8d943..a6a9cc9 100644 --- a/crates/maestro-cli/src/lib.rs +++ b/crates/maestro-cli/src/lib.rs @@ -4,7 +4,6 @@ pub mod attach; pub mod cli; -pub mod config; pub mod dispatch; pub mod sessions; diff --git a/crates/maestro-config/src/lib.rs b/crates/maestro-config/src/lib.rs index 5b34a3e..57b2a6d 100644 --- a/crates/maestro-config/src/lib.rs +++ b/crates/maestro-config/src/lib.rs @@ -22,14 +22,15 @@ use std::collections::BTreeMap; use std::net::SocketAddr; use std::path::{Path, PathBuf}; -use serde::{Deserialize, Serialize}; +use serde::Deserialize; /// Directory name under `$XDG_CONFIG_HOME` (default `~/.config`). pub const DIR_NAME: &str = "maestro"; /// File name inside [`DIR_NAME`]. pub const FILE_NAME: &str = "config.toml"; -/// Env var that, when set, overrides the resolved config-file path. -pub const CONFIG_PATH_ENV: &str = "MAESTRO_CONFIG"; + +/// Last-resort daemon HTTP base, matching the daemon's own default `bind_addr`. +pub const DEFAULT_DAEMON_URL: &str = "http://127.0.0.1:7070"; #[derive(Debug, thiserror::Error)] pub enum ConfigError { @@ -51,55 +52,49 @@ pub enum ConfigError { /// On-disk representation of the unified config. Every section is optional /// so a partial file (e.g. just `[daemon]`) is valid. -#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize)] pub struct MaestroConfigFile { - #[serde(default, skip_serializing_if = "CliSection::is_empty")] + #[serde(default)] pub cli: CliSection, - #[serde(default, skip_serializing_if = "DaemonSection::is_empty")] + #[serde(default)] pub daemon: DaemonSection, - #[serde(default, skip_serializing_if = "WebSection::is_empty")] + #[serde(default)] pub web: WebSection, - #[serde(default, skip_serializing_if = "TuiSection::is_empty")] + #[serde(default)] pub tui: TuiSection, - #[serde(default, skip_serializing_if = "SpawnSection::is_empty")] + #[serde(default)] pub spawn: SpawnSection, } -#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize)] pub struct CliSection { /// HTTP base URL of the daemon the CLI talks to, e.g. /// `http://127.0.0.1:7070`. When unset, the CLI derives it from /// `[daemon].bind_addr` if a daemon is configured locally, else falls /// back to `http://127.0.0.1:7070`. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub daemon_url: Option, } -impl CliSection { - fn is_empty(&self) -> bool { - self.daemon_url.is_none() - } -} - -#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize)] pub struct DaemonSection { /// Hostname advertised to clients, purely for display. Defaults to /// `gethostname(2)`. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub hostname: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub bind_addr: Option, /// Root for everything daemon-local: sidecars, FIFOs, etc. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub state_dir: Option, /// Root for ephemeral worktrees the daemon creates on `CreateSession` /// with a `worktree` spec. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub worktree_root: Option, /// Per-session PTY replay buffer cap in bytes (NAP-229). Each new attach /// receives this much of the prior PTY output before the live stream. /// `None` ⇒ daemon default (~64 KB); `Some(0)` disables replay. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub pty_replay_bytes: Option, /// `TMUX_TMPDIR` the daemon exports for every `tmux` invocation (NAP-234). /// When unset, the daemon defaults to `$XDG_RUNTIME_DIR` so that bare @@ -107,50 +102,27 @@ pub struct DaemonSection { /// maestro sessions. Set to `/tmp` to opt back into tmux's own default — /// useful if the daemon outlives the user's login session and you can't /// enable `loginctl enable-linger`. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub tmux_tmpdir: Option, /// Recency cap in days for the durable spawn history that backs the spawn /// form's "recent dirs" picker (#24). `None` ⇒ daemon default (30 days); /// `Some(0)` disables history entirely (no log written, no recents read). - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub spawn_history_days: Option, } -impl DaemonSection { - fn is_empty(&self) -> bool { - self.hostname.is_none() - && self.bind_addr.is_none() - && self.state_dir.is_none() - && self.worktree_root.is_none() - && self.pty_replay_bytes.is_none() - && self.tmux_tmpdir.is_none() - && self.spawn_history_days.is_none() - } -} - /// `[web]` — `maestro-web` binary settings (NAP-241). -#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize)] pub struct WebSection { /// Address `maestro-web` binds. Default `127.0.0.1:8080`. Bind a LAN/VPN /// address to share with your own devices on a trusted segment; there is no /// auth layer, so never bind a public address. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub bind_addr: Option, /// HTTP base URL of the co-located daemon `maestro-web` proxies to. /// Default `http://127.0.0.1:7070`. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub daemon_url: Option, - /// When set, requests for `/` are proxied to a Vite dev server at this - /// URL instead of being served from the embedded bundle. Use it during - /// frontend development so live reload works. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub dev_proxy_target: Option, -} - -impl WebSection { - fn is_empty(&self) -> bool { - self.bind_addr.is_none() && self.daemon_url.is_none() && self.dev_proxy_target.is_none() - } } /// `[tui]` — `maestro tui` settings (NAP-275 epic, phase 2 NAP-277). @@ -158,53 +130,40 @@ impl WebSection { /// Leader key, chord timeout, and per-mode keymap overrides. The TUI ships /// with a default keymap; entries in `keymap_normal` / `keymap_insert` are /// applied **over** the defaults, so a partial override is enough. -#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize)] pub struct TuiSection { /// Canonical key form, e.g. `"ctrl-a"`. Default `ctrl-a`. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub leader: Option, /// Maximum interval between chord keys before a held prefix is dropped. /// Default `1000`. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub chord_timeout_ms: Option, /// Normal-mode bindings: canonical input → action string. Applied over /// the shipped defaults. - #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + #[serde(default)] pub keymap_normal: BTreeMap, /// Insert-mode bindings, same semantics as `keymap_normal`. - #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + #[serde(default)] pub keymap_insert: BTreeMap, } -impl TuiSection { - pub fn is_empty(&self) -> bool { - self.leader.is_none() - && self.chord_timeout_ms.is_none() - && self.keymap_normal.is_empty() - && self.keymap_insert.is_empty() - } -} - /// `[spawn]` — command presets for the spawn form (#22, part of #21). /// /// Defined in shared config so every binary sees the same list: the web reads /// it via the daemon's catalog advertisement (#26), the TUI adopts it later. -#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize)] pub struct SpawnSection { /// Command presets offered as one-click tiles. When present, this list /// **replaces** the [`builtin_presets`] defaults (the implicit `custom` /// escape hatch is still appended by [`SpawnSection::resolve`]); when /// absent, the built-ins are used. `presets = []` is "present but empty" — /// it resolves to just the `custom` tile. - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default)] pub presets: Option>, } impl SpawnSection { - fn is_empty(&self) -> bool { - self.presets.is_none() - } - /// The presets to show, in order, with the implicit `custom` tile appended. /// /// Resolution: configured `presets` if present, else [`builtin_presets`]; @@ -317,27 +276,20 @@ impl MaestroConfigFile { Ok(toml::from_str(s)?) } - /// Atomic write (tmp + rename). Creates parent dirs on demand. - 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)?; - Ok(()) + /// Daemon HTTP base a client should talk to. + /// + /// Precedence: explicit `override_url` (flag or env), then + /// `[cli].daemon_url`, then `http://<[daemon].bind_addr>`, then + /// [`DEFAULT_DAEMON_URL`]. Always yields a value — reachability is decided + /// by whether the connection succeeds. + pub fn resolve_daemon_url(&self, override_url: Option) -> String { + override_url + .or_else(|| self.cli.daemon_url.clone()) + .or_else(|| self.daemon.bind_addr.map(|a| format!("http://{a}"))) + .unwrap_or_else(|| DEFAULT_DAEMON_URL.to_string()) } } -/// `$MAESTRO_CONFIG` if set, else [`default_config_path`]. -pub fn resolve_config_path() -> Result { - if let Some(p) = std::env::var_os(CONFIG_PATH_ENV) { - return Ok(PathBuf::from(p)); - } - default_config_path() -} - /// `~/.config/maestro/config.toml` (XDG-aware). pub fn default_config_path() -> Result { let base = dirs::config_dir().ok_or(ConfigError::NoConfigDir)?; @@ -366,32 +318,10 @@ pub fn default_state_dir(subdir: &str) -> Result { .ok_or(ConfigError::NoConfigDir)?; Ok(base.join(subdir)) } - -/// Capacities for the daemon's broadcast channels (session events and PTY byte -/// fan-out). These were independent `= 256` constants scattered across the -/// daemon; 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 event channels (`SessionEvent`, `CatalogEvent`). - 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 - } -} +/// Capacity of the session-event broadcast channel (`CatalogEvent`). +pub const EVENTS_CAPACITY: usize = 256; +/// Capacity of the per-session PTY byte broadcast channel. +pub const PTY_CAPACITY: usize = 256; #[cfg(test)] mod tests { @@ -422,10 +352,9 @@ mod tests { } #[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); + fn broadcast_capacities_are_the_historical_defaults() { + assert_eq!(EVENTS_CAPACITY, 256); + assert_eq!(PTY_CAPACITY, 256); } #[test] @@ -436,46 +365,6 @@ mod tests { assert_eq!(cfg, MaestroConfigFile::default()); } - #[test] - fn full_file_round_trips() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("config.toml"); - let cfg = MaestroConfigFile { - cli: CliSection { - daemon_url: Some("http://127.0.0.1:7070".into()), - }, - daemon: DaemonSection { - hostname: Some("laptop.lan".into()), - bind_addr: Some("127.0.0.1:7070".parse().unwrap()), - state_dir: Some(PathBuf::from("/var/lib/maestrod")), - worktree_root: Some(PathBuf::from("/var/lib/maestrod/wt")), - pty_replay_bytes: Some(131_072), - tmux_tmpdir: Some(PathBuf::from("/run/user/1000")), - spawn_history_days: Some(14), - }, - web: WebSection { - bind_addr: Some("127.0.0.1:8080".parse().unwrap()), - daemon_url: Some("http://127.0.0.1:7070".into()), - dev_proxy_target: Some("http://127.0.0.1:5173".into()), - }, - tui: TuiSection { - leader: Some("ctrl-b".into()), - chord_timeout_ms: Some(750), - keymap_normal: BTreeMap::from([("i".into(), "ShowHelp".into())]), - keymap_insert: BTreeMap::new(), - }, - spawn: SpawnSection { - presets: Some(vec![SpawnPreset { - id: "claude".into(), - label: "Claude".into(), - command: "claude --resume".into(), - }]), - }, - }; - cfg.save(&path).unwrap(); - assert_eq!(MaestroConfigFile::load(&path).unwrap(), cfg); - } - #[test] fn spawn_presets_default_to_builtins_plus_custom() { // Absent [spawn].presets → built-ins, with the implicit custom tile last. @@ -607,23 +496,6 @@ presets = [ assert_eq!(ids, ["claude", "zsh", "custom"]); } - #[test] - fn save_omits_empty_spawn_section() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("config.toml"); - MaestroConfigFile { - daemon: DaemonSection { - hostname: Some("laptop".into()), - ..Default::default() - }, - ..Default::default() - } - .save(&path) - .unwrap(); - let body = std::fs::read_to_string(&path).unwrap(); - assert!(!body.contains("[spawn]"), "body: {body}"); - } - #[test] fn partial_file_loads_with_section_defaults() { // Only [daemon] present (typical single-host install). @@ -646,40 +518,36 @@ bind_addr = "127.0.0.1:9090" ); } - #[test] - fn save_omits_empty_sections() { - // Avoid littering the file with empty `[cli]` / `[daemon]` tables - // when only one component is configured. - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("config.toml"); - let cfg = MaestroConfigFile { - daemon: DaemonSection { - hostname: Some("laptop".into()), - ..Default::default() - }, - ..Default::default() - }; - cfg.save(&path).unwrap(); - let body = std::fs::read_to_string(&path).unwrap(); - assert!(body.contains("[daemon]"), "body: {body}"); - assert!(!body.contains("[cli]"), "body: {body}"); - assert!(!body.contains("[web]"), "body: {body}"); + fn file_with(body: &str) -> MaestroConfigFile { + toml::from_str(body).expect("parse") } #[test] - fn save_atomic_creates_parent_dirs() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("nested/here/config.toml"); - MaestroConfigFile::default().save(&path).unwrap(); - assert!(path.exists()); + fn daemon_url_prefers_an_explicit_override() { + let cfg = file_with("[cli]\ndaemon_url = \"http://from-file:1\"\n"); + assert_eq!( + cfg.resolve_daemon_url(Some("http://override:2".into())), + "http://override:2" + ); } #[test] - fn resolve_config_path_respects_env_var() { - // We can't safely tweak the process env in a unit test (it races with - // other tests). Exercise the default path branch instead and rely on - // an integration-level smoke that env-override works. - let p = default_config_path().unwrap(); - assert!(p.ends_with(format!("{DIR_NAME}/{FILE_NAME}"))); + fn daemon_url_falls_back_through_cli_then_bind_addr_then_default() { + let from_cli = file_with("[cli]\ndaemon_url = \"http://from-cli:1\"\n"); + assert_eq!(from_cli.resolve_daemon_url(None), "http://from-cli:1"); + + let from_bind = file_with("[daemon]\nbind_addr = \"10.0.0.1:7070\"\n"); + assert_eq!(from_bind.resolve_daemon_url(None), "http://10.0.0.1:7070"); + + let empty = MaestroConfigFile::default(); + assert_eq!(empty.resolve_daemon_url(None), DEFAULT_DAEMON_URL); + } + + #[test] + fn cli_daemon_url_beats_a_locally_configured_bind_addr() { + let cfg = file_with( + "[cli]\ndaemon_url = \"http://explicit:1\"\n[daemon]\nbind_addr = \"10.0.0.1:7070\"\n", + ); + assert_eq!(cfg.resolve_daemon_url(None), "http://explicit:1"); } } diff --git a/crates/maestro-daemon/src/config.rs b/crates/maestro-daemon/src/config.rs index 6678479..88ded4b 100644 --- a/crates/maestro-daemon/src/config.rs +++ b/crates/maestro-daemon/src/config.rs @@ -14,9 +14,7 @@ use std::path::{Path, PathBuf}; use maestro_config::MaestroConfigFile; -pub use maestro_config::{ - default_config_path, resolve_config_path, DaemonSection, CONFIG_PATH_ENV, -}; +pub use maestro_config::default_config_path; /// Default bind address when neither env nor file specify one. pub const DEFAULT_BIND_ADDR: &str = "127.0.0.1:7070"; diff --git a/crates/maestro-daemon/src/pty.rs b/crates/maestro-daemon/src/pty.rs index e903e84..bc4855d 100644 --- a/crates/maestro-daemon/src/pty.rs +++ b/crates/maestro-daemon/src/pty.rs @@ -39,9 +39,9 @@ use tokio::task::JoinHandle; /// How many recent PTY frames each new subscriber inherits before starting to /// receive live frames. A small buffer is fine — clients render a fresh -/// terminal anyway. Sourced from the shared [`maestro_config::BroadcastTuning`] +/// terminal anyway. Sourced from [`maestro_config::PTY_CAPACITY`] /// (NAP-273). -pub const PTY_BROADCAST_CAPACITY: usize = maestro_config::BroadcastTuning::DEFAULT.pty_capacity; +pub const PTY_BROADCAST_CAPACITY: usize = maestro_config::PTY_CAPACITY; /// Default replay-buffer size — roughly one screenful for a typical 24×80 /// terminal plus reasonable scrollback. Override via diff --git a/crates/maestro-daemon/src/session.rs b/crates/maestro-daemon/src/session.rs index f57fbb8..2860861 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -25,12 +25,10 @@ use crate::spawn_history::{RecentDir, SpawnHistory, SpawnHistoryRecord}; use crate::tmux::TmuxError; use crate::worktree::{WorktreeError, WorktreeManager}; -/// Capacity of the session-events broadcast channel. The `/api/events` -/// publisher listens; if it lags by this much we just drop frames (clients -/// reconciles on `Hello` via the snapshot). Sourced from the shared -/// [`maestro_config::BroadcastTuning`] so all broadcast channels track one -/// definition (NAP-273). -pub const EVENTS_CAPACITY: usize = maestro_config::BroadcastTuning::DEFAULT.events_capacity; +/// Capacity of the session-events broadcast channel. A subscriber that falls +/// this far behind gets `RecvError::Lagged`, which `/api/events` forwards as +/// [`maestro_protocol::CatalogEvent::Lagged`] so that client re-fetches. +pub const EVENTS_CAPACITY: usize = maestro_config::EVENTS_CAPACITY; /// How often the liveness watcher polls tmux for sessions that disappeared. pub const DEFAULT_LIVENESS_INTERVAL: Duration = Duration::from_secs(2); diff --git a/crates/maestro-tui/src/lib.rs b/crates/maestro-tui/src/lib.rs index 4736fa5..bc9ed0c 100644 --- a/crates/maestro-tui/src/lib.rs +++ b/crates/maestro-tui/src/lib.rs @@ -38,7 +38,7 @@ pub struct RunConfig { /// No trailing slash. pub daemon_url: String, /// Explicit `--config ` override from the umbrella binary's global - /// flag. When `None`, [`maestro_config::resolve_config_path`] picks the + /// flag. When `None`, [`maestro_config::default_config_path`] picks the /// default (`$MAESTRO_CONFIG` or `~/.config/maestro/config.toml`). pub config_path: Option, } @@ -76,7 +76,7 @@ pub async fn run(cfg: RunConfig) -> Result<(), TuiError> { // `$MAESTRO_CONFIG`, then the platform default. Missing file is fine — // we fall back to shipped defaults. let cfg_path = cfg.config_path.clone().map(Ok).unwrap_or_else(|| { - maestro_config::resolve_config_path().map_err(|e| TuiError::Config(e.to_string())) + maestro_config::default_config_path().map_err(|e| TuiError::Config(e.to_string())) }); let cfg_file = match cfg_path { Ok(path) => maestro_config::MaestroConfigFile::load(&path) diff --git a/crates/maestro-web/README.md b/crates/maestro-web/README.md index dfd1a47..c6876b6 100644 --- a/crates/maestro-web/README.md +++ b/crates/maestro-web/README.md @@ -31,7 +31,6 @@ The binary reads `~/.config/maestro/config.toml`. The `[web]` section: [web] bind_addr = "127.0.0.1:8080" # default daemon_url = "http://127.0.0.1:7070" # default -# dev_proxy_target = "http://127.0.0.1:5173" # only when developing the frontend ``` Env-var overrides (all optional): diff --git a/crates/maestro-web/src/config.rs b/crates/maestro-web/src/config.rs index 15be39c..dc1ae7f 100644 --- a/crates/maestro-web/src/config.rs +++ b/crates/maestro-web/src/config.rs @@ -5,7 +5,9 @@ use std::path::{Path, PathBuf}; use maestro_config::MaestroConfigFile; -pub use maestro_config::{default_config_path, resolve_config_path, WebSection, CONFIG_PATH_ENV}; +pub use maestro_config::default_config_path; + +use maestro_config::WebSection; /// Default bind address when neither env nor file specify one. pub const DEFAULT_BIND_ADDR: &str = "127.0.0.1:8080"; @@ -14,7 +16,6 @@ pub const DEFAULT_DAEMON_URL: &str = "http://127.0.0.1:7070"; pub const ENV_WEB_BIND_ADDR: &str = "MAESTRO_WEB_BIND_ADDR"; pub const ENV_WEB_DAEMON_URL: &str = "MAESTRO_WEB_DAEMON_URL"; -pub const ENV_WEB_DEV_PROXY_TARGET: &str = "MAESTRO_WEB_DEV_PROXY_TARGET"; fn default_bind_addr() -> SocketAddr { DEFAULT_BIND_ADDR.parse().expect("valid default bind_addr") @@ -33,17 +34,12 @@ pub struct WebConfig { /// Daemon HTTP base URL — e.g. `http://127.0.0.1:7070`. Always a base /// without trailing slash and without a path. pub daemon_url: String, - /// Optional dev override: when set, `/` (and only `/`) is proxied here - /// instead of being served from the embedded bundle. Set to the Vite dev - /// server URL during frontend development. - pub dev_proxy_target: Option, } #[derive(Debug, Clone, Default)] pub struct EnvOverrides { pub bind_addr: Option, pub daemon_url: Option, - pub dev_proxy_target: Option, } impl EnvOverrides { @@ -51,7 +47,6 @@ impl EnvOverrides { Self { bind_addr: std::env::var(ENV_WEB_BIND_ADDR).ok(), daemon_url: std::env::var(ENV_WEB_DAEMON_URL).ok(), - dev_proxy_target: std::env::var(ENV_WEB_DEV_PROXY_TARGET).ok(), } } } @@ -76,15 +71,9 @@ fn resolve_with(web: WebSection, env: EnvOverrides) -> Result) -> Result<(), Box> { tracing::info!( bind = %cfg.bind_addr, daemon = %cfg.daemon_url, - dev_proxy = ?cfg.dev_proxy_target, "loaded config", ); - if cfg.dev_proxy_target.is_some() { - tracing::warn!( - "dev_proxy_target is set in config but not yet honored by the server \ - (NAP-241 v1 ships only the embedded-bundle path)" - ); - } - let router = router(&cfg)?; let listener = TcpListener::bind(cfg.bind_addr).await?; tracing::info!(addr = %listener.local_addr()?, "serving"); diff --git a/crates/maestro-web/tests/proxy.rs b/crates/maestro-web/tests/proxy.rs index 347a10c..407780a 100644 --- a/crates/maestro-web/tests/proxy.rs +++ b/crates/maestro-web/tests/proxy.rs @@ -64,7 +64,6 @@ async fn spawn_web(daemon_url: String) -> (SocketAddr, oneshot::Sender<()>) { let cfg = WebConfig { bind_addr: SocketAddr::from((Ipv4Addr::LOCALHOST, 0)), daemon_url, - dev_proxy_target: None, }; let router = maestro_web::router(&cfg).expect("router"); let listener = TcpListener::bind(cfg.bind_addr).await.unwrap(); diff --git a/crates/maestro/src/main.rs b/crates/maestro/src/main.rs index 8a97966..3df2e23 100644 --- a/crates/maestro/src/main.rs +++ b/crates/maestro/src/main.rs @@ -89,37 +89,37 @@ async fn dispatch(cli: Cli) -> Result<(), Box> { match cli.command { #[cfg(feature = "cli")] Command::List(args) => { - cli_dispatch( + maestro_cli::dispatch( maestro_cli::cli::Command::List(args), cli.config, - cli.daemon_url.clone(), + cli.daemon_url, ) .await? } #[cfg(feature = "cli")] Command::Spawn(args) => { - cli_dispatch( + maestro_cli::dispatch( maestro_cli::cli::Command::Spawn(args), cli.config, - cli.daemon_url.clone(), + cli.daemon_url, ) .await? } #[cfg(feature = "cli")] Command::Attach(args) => { - cli_dispatch( + maestro_cli::dispatch( maestro_cli::cli::Command::Attach(args), cli.config, - cli.daemon_url.clone(), + cli.daemon_url, ) .await? } #[cfg(feature = "cli")] Command::Kill(args) => { - cli_dispatch( + maestro_cli::dispatch( maestro_cli::cli::Command::Kill(args), cli.config, - cli.daemon_url.clone(), + cli.daemon_url, ) .await? } @@ -141,16 +141,6 @@ async fn dispatch(cli: Cli) -> Result<(), Box> { Ok(()) } -#[cfg(feature = "cli")] -#[allow(clippy::result_large_err)] -async fn cli_dispatch( - command: maestro_cli::cli::Command, - config: Option, - daemon_url: Option, -) -> Result<(), maestro_cli::CliError> { - maestro_cli::dispatch(command, config, daemon_url).await -} - fn init_tracing(cli: &Cli) { let filter = if is_cli_subcommand(&cli.command) { cli_filter(cli.verbose)