diff --git a/crates/maestro-cli/src/attach.rs b/crates/maestro-cli/src/attach.rs index 6c9df3f..4fd922c 100644 --- a/crates/maestro-cli/src/attach.rs +++ b/crates/maestro-cli/src/attach.rs @@ -32,6 +32,28 @@ pub enum AttachError { NotATty, } +impl AttachError { + /// True when the failure was an inability to reach the WS endpoint at the + /// transport level (connection refused). Used to fall back from the home + /// daemon broker to the legacy registry direct-attach path (NAP-244). + /// An HTTP-level rejection from a *reachable* daemon (e.g. the broker + /// returns 404 / 502) is deliberately NOT treated as unreachable — the + /// daemon answered, so we surface its error instead of masking it. + pub fn is_unreachable(&self) -> bool { + use std::io::ErrorKind; + use tokio_tungstenite::tungstenite::Error as WsError; + match self { + AttachError::WebSocket(WsError::Io(e)) => matches!( + e.kind(), + ErrorKind::ConnectionRefused + | ErrorKind::ConnectionReset + | ErrorKind::ConnectionAborted + ), + _ => false, + } + } +} + /// Parse a detach key spec. Accepts `^X` for Ctrl-X (case-insensitive) or a /// single literal character. Returns the resulting byte. #[allow(clippy::result_large_err)] @@ -57,6 +79,33 @@ pub fn attach_url(info: &ConnectInfo, session_id: Uuid) -> Result Result { + let base = home_daemon_http_base.trim_end_matches('/'); + let ws_base = if let Some(rest) = base.strip_prefix("https://") { + format!("wss://{rest}") + } else if let Some(rest) = base.strip_prefix("http://") { + format!("ws://{rest}") + } else if base.starts_with("ws://") || base.starts_with("wss://") { + base.to_string() + } else { + return Err(AttachError::InvalidDaemonUrl( + home_daemon_http_base.to_string(), + )); + }; + Ok(format!("{ws_base}/api/attach/{session_id}")) +} + /// Minimal percent-encoder for the token in a query string. JWTs only contain /// `A-Z a-z 0-9 - _ .` (URL-safe base64 + dots) so this is paranoia, not need. fn urlencoding(s: &str) -> String { @@ -131,7 +180,15 @@ pub async fn run_attach( detach_byte: u8, ) -> Result<(), AttachError> { let url = attach_url(&connect_info, session_id)?; - let (ws, _resp) = tokio_tungstenite::connect_async(&url).await?; + run_attach_url(&url, detach_byte).await +} + +/// Drive the attach loop against an already-built WS `url`. Shared by the +/// home-daemon broker path ([`broker_attach_url`]) and the legacy registry +/// direct-attach path ([`run_attach`]) — both speak the same PTY wire format. +#[allow(clippy::result_large_err)] +pub async fn run_attach_url(url: &str, detach_byte: u8) -> Result<(), AttachError> { + let (ws, _resp) = tokio_tungstenite::connect_async(url).await?; let (mut sink, mut stream) = ws.split(); let raw = RawTty::enable()?; @@ -281,6 +338,32 @@ mod tests { )); } + #[test] + fn broker_attach_url_swaps_scheme_and_appends_route() { + let id = Uuid::from_u128(0xabcd); + assert_eq!( + broker_attach_url("http://127.0.0.1:7070", id).unwrap(), + format!("ws://127.0.0.1:7070/api/attach/{id}") + ); + assert_eq!( + broker_attach_url("https://laptop.lan:7070/", id).unwrap(), + format!("wss://laptop.lan:7070/api/attach/{id}") + ); + // already-ws bases pass through + assert_eq!( + broker_attach_url("ws://10.0.0.1:7070", id).unwrap(), + format!("ws://10.0.0.1:7070/api/attach/{id}") + ); + } + + #[test] + fn broker_attach_url_rejects_unknown_scheme() { + assert!(matches!( + broker_attach_url("ftp://x", Uuid::nil()), + Err(AttachError::InvalidDaemonUrl(_)) + )); + } + #[test] fn attach_url_trims_trailing_slash() { let info = ConnectInfo { diff --git a/crates/maestro-cli/src/cli.rs b/crates/maestro-cli/src/cli.rs index 934e975..5867be0 100644 --- a/crates/maestro-cli/src/cli.rs +++ b/crates/maestro-cli/src/cli.rs @@ -71,7 +71,7 @@ pub struct ListArgs { pub json: bool, } -#[derive(Debug, clap::Args)] +#[derive(Debug, Clone, clap::Args)] pub struct SpawnArgs { /// Daemon ID to spawn on. #[arg(long)] diff --git a/crates/maestro-cli/src/config.rs b/crates/maestro-cli/src/config.rs index 803a14b..c97bd60 100644 --- a/crates/maestro-cli/src/config.rs +++ b/crates/maestro-cli/src/config.rs @@ -4,12 +4,18 @@ //! (persisted `maestro_session` JWT). All other sections in the file are //! preserved on write. +use std::net::SocketAddr; use std::path::{Path, PathBuf}; use maestro_config::MaestroConfigFile; pub use maestro_config::CONFIG_PATH_ENV; +/// Last-resort home-daemon HTTP base when neither config nor a local +/// `[daemon].bind_addr` tells us otherwise. Matches the daemon's default +/// `bind_addr` (`crates/maestro-daemon` defaults). +pub const DEFAULT_HOME_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()?) @@ -37,20 +43,42 @@ pub enum ConfigError { pub struct CliConfig { pub registry_url: Option, pub cookie: Option, + /// `[cli].home_daemon_url` — explicit home-daemon HTTP base (NAP-244). + pub home_daemon_url: Option, + /// `[daemon].bind_addr` if a daemon is configured on this host. Used to + /// derive the default home-daemon URL when `home_daemon_url` is unset. + pub daemon_bind_addr: Option, } impl CliConfig { - /// 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. + /// Load the subset of the unified file the CLI consumes: + /// `[shared].registry_url`, `[cli].cookie`, `[cli].home_daemon_url`, and + /// `[daemon].bind_addr` (for default home-daemon derivation). 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 { let file = MaestroConfigFile::load_or_default(path)?; Ok(Self { registry_url: file.shared.registry_url, cookie: file.cli.cookie, + home_daemon_url: file.cli.home_daemon_url, + daemon_bind_addr: file.daemon.bind_addr, }) } + /// Resolve the home-daemon HTTP base the broker path should target. + /// + /// Precedence: explicit `override_url` (CLI flag / env) → `[cli] + /// .home_daemon_url` → `http://<[daemon].bind_addr>` → the built-in + /// [`DEFAULT_HOME_DAEMON_URL`]. Always returns a value; reachability is + /// decided later by whether the connection actually succeeds. + pub fn resolve_home_daemon_url(&self, override_url: Option) -> String { + override_url + .or_else(|| self.home_daemon_url.clone()) + .or_else(|| self.daemon_bind_addr.map(|a| format!("http://{a}"))) + .unwrap_or_else(|| DEFAULT_HOME_DAEMON_URL.to_string()) + } + /// Persist `[shared].registry_url`. Read-modify-write — preserves /// any other sections already in the file (`[daemon]`, `[registry]`, /// existing `[cli].cookie`). @@ -151,6 +179,56 @@ registry_url = "https://r.example" assert_eq!(after.cli.cookie.as_deref(), Some("eyJ.new")); } + #[test] + fn resolve_home_daemon_url_precedence() { + // override wins over everything + let cfg = CliConfig { + home_daemon_url: Some("http://from-cli-section:7070".into()), + daemon_bind_addr: Some("127.0.0.1:9999".parse().unwrap()), + ..Default::default() + }; + assert_eq!( + cfg.resolve_home_daemon_url(Some("http://override:1".into())), + "http://override:1" + ); + // [cli].home_daemon_url next + assert_eq!( + cfg.resolve_home_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_home_daemon_url(None), "http://10.0.0.7:7070"); + // built-in default when nothing is configured + let cfg = CliConfig::default(); + assert_eq!(cfg.resolve_home_daemon_url(None), DEFAULT_HOME_DAEMON_URL); + } + + #[test] + fn load_reads_home_daemon_url_and_daemon_bind_addr() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + write_file( + &path, + r#" +[cli] +home_daemon_url = "http://laptop:7070" + +[daemon] +bind_addr = "127.0.0.1:7071" +"#, + ); + let cfg = CliConfig::load(&path).unwrap(); + assert_eq!(cfg.home_daemon_url.as_deref(), Some("http://laptop:7070")); + assert_eq!( + cfg.daemon_bind_addr, + Some("127.0.0.1:7071".parse().unwrap()) + ); + } + #[test] fn save_creates_file_if_missing() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/maestro-cli/src/dispatch.rs b/crates/maestro-cli/src/dispatch.rs index dba6b93..b1ab2cd 100644 --- a/crates/maestro-cli/src/dispatch.rs +++ b/crates/maestro-cli/src/dispatch.rs @@ -2,13 +2,30 @@ //! //! Extracted from the per-binary `main.rs` (NAP-245) so the umbrella `maestro` //! binary can dispatch to the same logic. +//! +//! ## Transport (NAP-244) +//! +//! The CLI talks to its **home daemon** (`[cli].home_daemon_url`, default +//! `http://127.0.0.1:7070`) and lets the daemon broker the rest: +//! +//! * `list` reads the daemon's local catalog mirror (own ∪ peer sessions). +//! * `spawn` / `kill` hit the daemon's local session endpoints. +//! * `attach` opens the daemon's `WS /api/attach/:id` broker, which picks the +//! transport and enforces `allow_inbound` on the target side. +//! +//! If the home daemon is **unreachable** (connection refused / not running), +//! every command falls back to the legacy registry path (registry HTTP + +//! direct WS) with a one-line stderr warning, so migration is painless. A few +//! operations the daemon can't yet satisfy locally (cross-daemon spawn → 501, +//! cross-daemon kill → 404) also fall back to the registry, which forwards +//! them the old way. The fallback is removed in a future ticket. use std::path::{Path, PathBuf}; -use crate::attach::{parse_detach_key, run_attach}; +use crate::attach::{broker_attach_url, parse_detach_key, run_attach, run_attach_url}; use crate::cli::{AttachArgs, Command, KillArgs, ListArgs, PolicyCmd, SetRegistryArgs, SpawnArgs}; use crate::config::{default_config_path, CliConfig, ConfigError}; -use crate::http::RegistryClient; +use crate::http::{HttpError, RegistryClient}; use crate::policy::{run_reload as run_policy_reload, run_show as run_policy_show}; use crate::sessions::{fetch_connect_info, run_kill, run_list, run_spawn}; @@ -28,12 +45,14 @@ pub enum CliError { } /// Dispatch a parsed top-level subcommand. The umbrella binary passes the -/// global flags (`--config`, `--registry-url`) in alongside the variant. +/// global flags (`--config`, `--registry-url`, `--home-daemon-url`) in +/// alongside the variant. #[allow(clippy::result_large_err)] pub async fn dispatch( command: Command, config: Option, registry_url_override: Option, + home_daemon_url_override: Option, ) -> Result<(), CliError> { let config_path = match config { Some(p) => p, @@ -43,10 +62,42 @@ pub async fn dispatch( match command { Command::SetRegistry(args) => set_registry(&config_path, args), Command::Policy(sub) => policy(&config_path, sub).await, - Command::List(args) => list(&config_path, registry_url_override, args).await, - Command::Spawn(args) => spawn(&config_path, registry_url_override, args).await, - Command::Attach(args) => attach(&config_path, registry_url_override, args).await, - Command::Kill(args) => kill(&config_path, registry_url_override, args).await, + Command::List(args) => { + list( + &config_path, + registry_url_override, + home_daemon_url_override, + args, + ) + .await + } + Command::Spawn(args) => { + spawn( + &config_path, + registry_url_override, + home_daemon_url_override, + args, + ) + .await + } + Command::Attach(args) => { + attach( + &config_path, + registry_url_override, + home_daemon_url_override, + args, + ) + .await + } + Command::Kill(args) => { + kill( + &config_path, + registry_url_override, + home_daemon_url_override, + args, + ) + .await + } } } @@ -72,28 +123,63 @@ async fn policy(config_path: &Path, sub: PolicyCmd) -> Result<(), CliError> { async fn list( config_path: &Path, registry_url_override: Option, + home_daemon_url_override: Option, args: ListArgs, ) -> Result<(), CliError> { - let client = build_client(registry_url_override, config_path)?; + let cfg = CliConfig::load(config_path)?; + let home_url = cfg.resolve_home_daemon_url(home_daemon_url_override); + let daemon = daemon_client(&home_url, config_path); let mut stdout = std::io::stdout().lock(); - run_list(&client, &args, &mut stdout).await?; - Ok(()) + + match run_list(&daemon, &args, &mut stdout).await { + Ok(()) => Ok(()), + Err(e) if e.is_unreachable() => { + warn_fallback(&home_url, &e); + let registry = registry_client(registry_url_override, &cfg, config_path)?; + run_list(®istry, &args, &mut stdout).await?; + Ok(()) + } + Err(e) => Err(e.into()), + } } #[allow(clippy::result_large_err)] async fn spawn( config_path: &Path, registry_url_override: Option, + home_daemon_url_override: Option, args: SpawnArgs, ) -> Result<(), CliError> { - let client = build_client(registry_url_override, config_path)?; + let cfg = CliConfig::load(config_path)?; + let home_url = cfg.resolve_home_daemon_url(home_daemon_url_override.clone()); + let daemon = daemon_client(&home_url, config_path); let attach_after = args.attach; - let session = run_spawn(&client, args).await?; + // `spawn --attach` has no detach-key flag; use the attach default (Ctrl-q). + let detach_key = 0x11; + + // Cross-daemon create isn't brokered by the local daemon yet (501), so we + // fall back to the registry on that as well as on unreachable. + let session = match run_spawn(&daemon, args.clone()).await { + Ok(s) => s, + Err(e) if e.is_unreachable() || e.is_remote_op_unsupported() => { + warn_fallback(&home_url, &e); + let registry = registry_client(registry_url_override.clone(), &cfg, config_path)?; + run_spawn(®istry, args).await? + } + Err(e) => return Err(e.into()), + }; println!("spawned {}", session.id); + if attach_after { - let info = fetch_connect_info(&client, session.id).await?; - let detach = 0x11; // Ctrl-q - run_attach(info, session.id, detach).await?; + attach_session( + &home_url, + registry_url_override, + &cfg, + config_path, + session.id, + detach_key, + ) + .await?; } Ok(()) } @@ -102,39 +188,103 @@ async fn spawn( async fn attach( config_path: &Path, registry_url_override: Option, + home_daemon_url_override: Option, args: AttachArgs, ) -> Result<(), CliError> { - let client = build_client(registry_url_override, config_path)?; - let info = fetch_connect_info(&client, args.session_id).await?; + let cfg = CliConfig::load(config_path)?; + let home_url = cfg.resolve_home_daemon_url(home_daemon_url_override); let detach = parse_detach_key(&args.detach_key)?; - run_attach(info, args.session_id, detach).await?; - Ok(()) + attach_session( + &home_url, + registry_url_override, + &cfg, + config_path, + args.session_id, + detach, + ) + .await } #[allow(clippy::result_large_err)] async fn kill( config_path: &Path, registry_url_override: Option, + home_daemon_url_override: Option, args: KillArgs, ) -> Result<(), CliError> { - let client = build_client(registry_url_override, config_path)?; - run_kill(&client, &args).await?; + let cfg = CliConfig::load(config_path)?; + let home_url = cfg.resolve_home_daemon_url(home_daemon_url_override); + let daemon = daemon_client(&home_url, config_path); + + // The local daemon only kills sessions it owns (peer sessions → 404); the + // registry forwards cross-daemon kills, so fall back on 404 too. + match run_kill(&daemon, &args).await { + Ok(()) => {} + Err(e) if e.is_unreachable() || e.is_not_found() => { + warn_fallback(&home_url, &e); + let registry = registry_client(registry_url_override, &cfg, config_path)?; + run_kill(®istry, &args).await?; + } + Err(e) => return Err(e.into()), + } println!("killed {}", args.session_id); Ok(()) } +/// Attach to `session_id` through the home-daemon broker, falling back to the +/// registry's connect-info + direct WS path if the daemon is unreachable. #[allow(clippy::result_large_err)] -fn build_client( +async fn attach_session( + home_url: &str, + registry_url_override: Option, + cfg: &CliConfig, + config_path: &Path, + session_id: uuid::Uuid, + detach: u8, +) -> Result<(), CliError> { + let broker_url = broker_attach_url(home_url, session_id)?; + match run_attach_url(&broker_url, detach).await { + Ok(()) => Ok(()), + Err(e) if e.is_unreachable() => { + eprintln!( + "[maestro] home daemon at {home_url} unreachable; \ + falling back to the registry (legacy direct attach)" + ); + let registry = registry_client(registry_url_override, cfg, config_path)?; + let info = fetch_connect_info(®istry, session_id).await?; + run_attach(info, session_id, detach).await?; + Ok(()) + } + Err(e) => Err(e.into()), + } +} + +/// HTTP client pointed at the home daemon's local API. The daemon's local +/// surface needs no auth (same-trust as `bind_addr`), so no cookie is sent. +fn daemon_client(home_url: &str, config_path: &Path) -> RegistryClient { + RegistryClient::new(home_url.to_string(), None, config_path.to_path_buf()) +} + +/// HTTP client pointed at the registry (legacy / fallback path). +#[allow(clippy::result_large_err)] +fn registry_client( override_url: Option, + cfg: &CliConfig, config_path: &Path, ) -> Result { - let cfg = CliConfig::load(config_path)?; let url = override_url - .or(cfg.registry_url.clone()) + .or_else(|| cfg.registry_url.clone()) .ok_or(ConfigError::NoRegistryUrl)?; Ok(RegistryClient::new( url, - cfg.cookie, + cfg.cookie.clone(), config_path.to_path_buf(), )) } + +fn warn_fallback(home_url: &str, err: &HttpError) { + eprintln!( + "[maestro] home daemon at {home_url} unavailable ({err}); \ + falling back to the registry" + ); +} diff --git a/crates/maestro-cli/src/http.rs b/crates/maestro-cli/src/http.rs index 8d424d4..e6b9f46 100644 --- a/crates/maestro-cli/src/http.rs +++ b/crates/maestro-cli/src/http.rs @@ -27,6 +27,36 @@ pub enum HttpError { InvalidHeader(#[from] reqwest::header::InvalidHeaderValue), } +impl HttpError { + /// True when the error is a transport-level failure to reach the host + /// (connection refused / connect error) rather than an HTTP-level + /// rejection. The CLI uses this to decide whether to fall back from the + /// home daemon to the legacy registry path (NAP-244). + pub fn is_unreachable(&self) -> bool { + matches!(self, HttpError::Reqwest(e) if e.is_connect()) + } + + /// True when a *reachable* home daemon replied with the given HTTP status. + pub fn has_status(&self, want: StatusCode) -> bool { + matches!(self, HttpError::ServerError { status, .. } if *status == want) + } + + /// The home daemon's local `POST /api/sessions` only creates sessions it + /// owns; cross-daemon creates return `501 remote_create_unsupported` + /// (broker create-forwarding is future work). `maestro spawn --daemon + /// ` falls back to the registry on this so it keeps working. + pub fn is_remote_op_unsupported(&self) -> bool { + self.has_status(StatusCode::NOT_IMPLEMENTED) + } + + /// The home daemon's local `DELETE /api/sessions/:id` only kills sessions + /// it owns; a peer-owned session yields `404 unknown_session`. `maestro + /// kill` falls back to the registry on this so cross-daemon kill works. + pub fn is_not_found(&self) -> bool { + self.has_status(StatusCode::NOT_FOUND) + } +} + /// HTTP client bound to a single registry. Persists the auth cookie back to /// disk when the registry sets one (first-contact bootstrap from /// `crates/maestro-registry/src/auth.rs`). diff --git a/crates/maestro-config/src/lib.rs b/crates/maestro-config/src/lib.rs index 0b2e300..677767d 100644 --- a/crates/maestro-config/src/lib.rs +++ b/crates/maestro-config/src/lib.rs @@ -106,11 +106,18 @@ pub struct CliSection { /// Persisted `maestro_session` cookie value (a JWT). Opaque to the CLI. #[serde(default, skip_serializing_if = "Option::is_none")] pub cookie: Option, + /// HTTP base URL of the home daemon the CLI brokers through (NAP-244), + /// 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`. The legacy registry path stays + /// available as a compat fallback via `[shared].registry_url`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub home_daemon_url: Option, } impl CliSection { fn is_empty(&self) -> bool { - self.cookie.is_none() + self.cookie.is_none() && self.home_daemon_url.is_none() } } @@ -371,6 +378,7 @@ mod tests { }, cli: CliSection { cookie: Some("eyJ...".into()), + home_daemon_url: Some("http://127.0.0.1:7070".into()), }, daemon: DaemonSection { daemon_id: Some("laptop".into()), diff --git a/crates/maestro/src/main.rs b/crates/maestro/src/main.rs index ca0ab42..250f4ce 100644 --- a/crates/maestro/src/main.rs +++ b/crates/maestro/src/main.rs @@ -34,6 +34,12 @@ struct Cli { #[arg(long, global = true, env = "MAESTRO_REGISTRY_URL")] registry_url: Option, + /// Override the home-daemon HTTP base the CLI brokers through (e.g. + /// `http://127.0.0.1:7070`). Defaults to `[cli].home_daemon_url`, then the + /// local `[daemon].bind_addr`, then `http://127.0.0.1:7070`. CLI only. + #[arg(long, global = true, env = "MAESTRO_HOME_DAEMON_URL")] + home_daemon_url: Option, + /// Path to the unified Maestro config file. Default `~/.config/maestro/config.toml`. #[arg(long, global = true, env = "MAESTRO_CONFIG")] config: Option, @@ -110,6 +116,7 @@ async fn dispatch(cli: Cli) -> Result<(), Box> { maestro_cli::cli::Command::List(args), cli.config, cli.registry_url, + cli.home_daemon_url, ) .await? } @@ -119,6 +126,7 @@ async fn dispatch(cli: Cli) -> Result<(), Box> { maestro_cli::cli::Command::Spawn(args), cli.config, cli.registry_url, + cli.home_daemon_url, ) .await? } @@ -128,6 +136,7 @@ async fn dispatch(cli: Cli) -> Result<(), Box> { maestro_cli::cli::Command::Attach(args), cli.config, cli.registry_url, + cli.home_daemon_url, ) .await? } @@ -137,6 +146,7 @@ async fn dispatch(cli: Cli) -> Result<(), Box> { maestro_cli::cli::Command::Kill(args), cli.config, cli.registry_url, + cli.home_daemon_url, ) .await? } @@ -146,6 +156,7 @@ async fn dispatch(cli: Cli) -> Result<(), Box> { maestro_cli::cli::Command::SetRegistry(args), cli.config, cli.registry_url, + cli.home_daemon_url, ) .await? } @@ -155,6 +166,7 @@ async fn dispatch(cli: Cli) -> Result<(), Box> { maestro_cli::cli::Command::Policy(sub), cli.config, cli.registry_url, + cli.home_daemon_url, ) .await? } @@ -182,8 +194,9 @@ async fn cli_dispatch( command: maestro_cli::cli::Command, config: Option, registry_url: Option, + home_daemon_url: Option, ) -> Result<(), maestro_cli::CliError> { - maestro_cli::dispatch(command, config, registry_url).await + maestro_cli::dispatch(command, config, registry_url, home_daemon_url).await } fn init_tracing(cli: &Cli) {