From 6a33152a7de4084114c6e1d2700dd14d80a978fe Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Tue, 19 May 2026 15:03:10 -0500 Subject: [PATCH 01/17] vm: stream downloads with progress (fix 458 MB var.btrfs timeout) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `avocado vm update` was failing mid-download on real-world links: Error: error decoding response body Caused by: operation timed out Two underlying causes: 1. `ClientBuilder::timeout(Duration::from_secs(30))` set a global request timeout that fires across the body download too — 30s is hopeless for a 458 MB artifact on any link slower than ~16 MB/s. Fix: switch to `connect_timeout` (still 30s — fail fast on a stalled handshake) but leave the overall timeout unset so large downloads can run to completion. 2. `Response::bytes()` buffered the entire body in memory before writing — 458 MB of heap per artifact, plus the wasted RTT before disk writes started. Fix: stream via `Response::bytes_stream()` and write each chunk straight to `std::fs::File`. UX: - Human mode now renders an indicatif progress bar per artifact: [1/4] downloading avocado-image-rootfs-qemuarm64.erofs-lz4 (...) ===>... 65.0 MB/65.0 MB 10.2 MB/s ETA 0s - JSON mode (--output json) emits structured NDJSON events throttled to ~10 Hz so a polling consumer (e.g. avocado-desktop's VMInstallController) can drive its own progress UI without being flooded: {"event":"download_started","file":"...","size":N,"index":i,"total":n} {"event":"download_progress","file":"...","bytes":N,"total":N} {"event":"download_completed","file":"...","bytes":N,"index":i,"total":n} Verified locally end-to-end: clean cache + state, ran `avocado vm update --yes` against the live v0.1.0 channel, all four artifacts downloaded, sha256-verified, atomic-swapped into ~/.avocado/vm/install/. Sha hashing still happens after write (unchanged from before — keeps the staging-dir verify path the same). --- src/commands/vm/update.rs | 140 ++++++++++++++++++++++++++++++++------ 1 file changed, 120 insertions(+), 20 deletions(-) diff --git a/src/commands/vm/update.rs b/src/commands/vm/update.rs index 6cbc713..e047d16 100644 --- a/src/commands/vm/update.rs +++ b/src/commands/vm/update.rs @@ -19,10 +19,13 @@ use anyhow::{bail, Context, Result}; use clap::ValueEnum; +use futures_util::StreamExt; +use indicatif::{ProgressBar, ProgressStyle}; use reqwest::ClientBuilder; use serde_json::json; +use std::io::Write; use std::path::Path; -use std::time::Duration; +use std::time::{Duration, Instant}; use crate::utils::output_format::OutputFormat; use crate::utils::user_config::UserConfig; @@ -115,9 +118,16 @@ impl UpdateCommand { confirm(&avail.pointer, installed_version.as_deref())?; } - // Fetch the per-platform manifest for the new version. + // HTTP client — `connect_timeout` is bounded so a stalled DNS / + // TCP handshake fails fast, but the overall request timeout is + // unset because artifact downloads can run several minutes on + // slow links (the var.btrfs alone is ~450 MB). A global + // `.timeout(Duration::from_secs(30))` is what previously caused + // `Error: operation timed out` mid-download on real-world + // connections. let http = ClientBuilder::new() - .timeout(Duration::from_secs(30)) + .connect_timeout(Duration::from_secs(30)) + .pool_idle_timeout(Some(Duration::from_secs(60))) .build()?; let new_manifest: Manifest = serde_json::from_str( &http @@ -154,28 +164,24 @@ impl UpdateCommand { let stage = StagingDir::create(&install_dir, &version)?; stage.record_was_running(was_running)?; - for item in &downloads { - println!( - "downloading {} ({} bytes)", - item.file, - item.size - .map(|n| n.to_string()) - .unwrap_or_else(|| "?".into()), - ); + let json_mode = self.output.is_json(); + for (idx, item) in downloads.iter().enumerate() { let url = format!( "{}/{}", platform_entry.base_url.trim_end_matches('/'), item.file, ); - let bytes = http - .get(&url) - .send() - .await? - .error_for_status()? - .bytes() - .await?; - std::fs::write(stage.slot(&item.file), &bytes) - .with_context(|| format!("writing staged {}", item.file))?; + download_artifact( + &http, + &url, + &stage.slot(&item.file), + item, + idx + 1, + downloads.len(), + json_mode, + ) + .await + .with_context(|| format!("downloading {}", item.file))?; stage .verify_sha256(&item.file, &item.sha256) .context("staged artifact sha256 mismatch")?; @@ -235,6 +241,7 @@ impl UpdateCommand { cmdline_extra: None, workspace: None, var_size: None, + dns_override: None, }; crate::utils::vm::lifecycle::start(opts).await?; } @@ -367,3 +374,96 @@ fn print_update_available( } Ok(()) } + +/// Stream-download one artifact to `dest`. +/// +/// - Writes chunks straight to disk via std::fs::File. Doesn't buffer +/// the full body in memory — important for the var.btrfs which is +/// ~450 MB. +/// - In human mode shows an indicatif progress bar with bytes / total / +/// rate / ETA. +/// - In `--output json` mode emits NDJSON progress events throttled to +/// ~10 Hz so the desktop app can drive a progress bar without being +/// flooded. +async fn download_artifact( + http: &reqwest::Client, + url: &str, + dest: &std::path::Path, + item: &PlannedDownload, + idx: usize, + total_items: usize, + json_mode: bool, +) -> Result<()> { + let resp = http + .get(url) + .send() + .await + .with_context(|| format!("GET {url}"))? + .error_for_status()?; + let total_bytes = resp.content_length().or(item.size).unwrap_or(0); + + if !json_mode { + println!( + "[{idx}/{total_items}] downloading {} ({} bytes)", + item.file, total_bytes, + ); + } + let pb = if !json_mode { + let pb = ProgressBar::new(total_bytes); + pb.set_style( + ProgressStyle::with_template( + " {bar:30.green/black} {decimal_bytes:>10}/{decimal_total_bytes:<10} {decimal_bytes_per_sec:>12} ETA {eta:>5}", + ) + .unwrap() + .progress_chars("=> "), + ); + Some(pb) + } else { + crate::utils::output_format::emit_json_object(&json!({ + "event": "download_started", + "file": item.file, + "size": total_bytes, + "index": idx, + "total": total_items, + })); + None + }; + + let mut file = std::fs::File::create(dest) + .with_context(|| format!("creating {}", dest.display()))?; + let mut stream = resp.bytes_stream(); + let mut written: u64 = 0; + let mut last_emit = Instant::now(); + while let Some(chunk) = stream.next().await { + let chunk = chunk.with_context(|| format!("reading body of {url}"))?; + file.write_all(&chunk) + .with_context(|| format!("writing to {}", dest.display()))?; + written += chunk.len() as u64; + if let Some(pb) = &pb { + pb.set_position(written); + } else if json_mode && last_emit.elapsed() >= Duration::from_millis(100) { + crate::utils::output_format::emit_json_object(&json!({ + "event": "download_progress", + "file": item.file, + "bytes": written, + "total": total_bytes, + })); + last_emit = Instant::now(); + } + } + file.sync_all().ok(); + drop(file); + if let Some(pb) = pb { + pb.finish_and_clear(); + } + if json_mode { + crate::utils::output_format::emit_json_object(&json!({ + "event": "download_completed", + "file": item.file, + "bytes": written, + "index": idx, + "total": total_items, + })); + } + Ok(()) +} From 44e53e6a51bd7789433ec8ec7b3a4f37f938491b Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Tue, 19 May 2026 15:06:21 -0500 Subject: [PATCH 02/17] vm: cargo fmt sweep on update.rs --- src/commands/vm/update.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/commands/vm/update.rs b/src/commands/vm/update.rs index e047d16..a5f7cf7 100644 --- a/src/commands/vm/update.rs +++ b/src/commands/vm/update.rs @@ -429,8 +429,8 @@ async fn download_artifact( None }; - let mut file = std::fs::File::create(dest) - .with_context(|| format!("creating {}", dest.display()))?; + let mut file = + std::fs::File::create(dest).with_context(|| format!("creating {}", dest.display()))?; let mut stream = resp.bytes_stream(); let mut written: u64 = 0; let mut last_emit = Instant::now(); From 8e8dd50d924c659a85a505d2bd1db8a49e2ae9bd Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Tue, 19 May 2026 15:29:46 -0500 Subject: [PATCH 03/17] vm: `vm start` defaults to ~/.avocado/vm/install/ when nothing else set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User flow that hit this: $ avocado vm update # downloads to ~/.avocado/vm/install/ $ avocado vm start Error: --vm-source not given and AVOCADO_VM_DIR is unset… After `avocado vm update` the artifacts ARE on disk in the canonical location — `vm start` shouldn't refuse to find them. Resolution order is now: 1. --vm-source flag 2. $AVOCADO_VM_DIR env var 3. ~/.avocado/vm/install/ (managed install, populated by vm update) 4. ~/.avocado/vm/artifact-dir (last `vm start --vm-source ` or `vm rebuild`, dev workflow) 5. Helpful error pointing at `avocado vm update` VmPaths::default_vm_source() centralises the layered fallback. The existing `vm rebuild` flow still writes the artifact-dir pointer for backwards compat with the dev workflow, but end-users who only ever `avocado vm update` + `avocado vm start` no longer need to think about env vars. main.rs help text + start.rs error message both updated to reflect the new behaviour. --- src/commands/vm/start.rs | 28 +++++++++++++-- src/main.rs | 75 +++++++++++++++++++++++++++++++++++++++- src/utils/vm/state.rs | 37 ++++++++++++++++++++ 3 files changed, 136 insertions(+), 4 deletions(-) diff --git a/src/commands/vm/start.rs b/src/commands/vm/start.rs index 891fd5e..efdc344 100644 --- a/src/commands/vm/start.rs +++ b/src/commands/vm/start.rs @@ -16,6 +16,8 @@ pub struct StartCommand { pub cmdline_extra: Option, pub workspace: Option, pub var_size: Option, + /// One-shot DNS override; empty means "fall through to persisted config". + pub dns: Vec, pub watch: bool, #[allow(dead_code)] pub foreground: bool, @@ -23,11 +25,26 @@ pub struct StartCommand { impl StartCommand { pub async fn execute(self) -> Result<()> { + // Resolution order: explicit CLI flag → AVOCADO_VM_DIR → + // managed install dir (populated by `avocado vm update`) → + // last `vm start` / `vm rebuild` artifact-dir pointer. Helps + // the common end-user flow where you just `avocado vm update` + // and then `avocado vm start` without juggling flags or env. + let paths_default = VmPaths::resolve()?; let vm_source = match self.vm_source { Some(p) => p, - None => std::env::var("AVOCADO_VM_DIR") - .map(PathBuf::from) - .context("--vm-source not given and AVOCADO_VM_DIR is unset; point at a directory containing `direct` profile output (manifest.json + kernel/initramfs/rootfs/var)")?, + None => { + if let Ok(raw) = std::env::var("AVOCADO_VM_DIR") { + PathBuf::from(raw) + } else if let Some(p) = paths_default.default_vm_source() { + p + } else { + anyhow::bail!( + "no avocado-vm install found. Run `avocado vm update` to install \ + the latest release, or pass `--vm-source ` for a local build." + ); + } + } }; print_info( @@ -68,6 +85,11 @@ impl StartCommand { cmdline_extra: self.cmdline_extra, workspace: self.workspace, var_size: self.var_size, + dns_override: if self.dns.is_empty() { + None + } else { + Some(self.dns) + }, }; let result = lifecycle::start(opts).await; diff --git a/src/main.rs b/src/main.rs index af7e86c..ee79606 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2938,6 +2938,7 @@ async fn main() -> Result<()> { cmdline_extra, workspace, var_size, + dns, watch, foreground, } => { @@ -2949,6 +2950,7 @@ async fn main() -> Result<()> { cmdline_extra, workspace, var_size, + dns, watch, foreground, }; @@ -2995,6 +2997,28 @@ async fn main() -> Result<()> { .execute() .await } + VmCommands::Config { command } => match command { + VmConfigCommands::Get { key, output } => { + commands::vm::config::ConfigGetCommand { key, output } + .execute() + .await + } + VmConfigCommands::Set { key, values } => { + commands::vm::config::ConfigSetCommand { key, values } + .execute() + .await + } + VmConfigCommands::Unset { key } => { + commands::vm::config::ConfigUnsetCommand { key } + .execute() + .await + } + VmConfigCommands::List { output } => { + commands::vm::config::ConfigListCommand { output } + .execute() + .await + } + }, }, Commands::Hitl { command } => match command { HitlCommands::Server { @@ -4160,7 +4184,9 @@ enum VmCommands { /// Boot the avocado-vm (no-op if already running). Start { /// Directory containing `direct` profile output (manifest.json + artifacts). - /// Falls back to $AVOCADO_VM_DIR if unset. + /// Resolution order when unset: $AVOCADO_VM_DIR → ~/.avocado/vm/install/ + /// (populated by `avocado vm update`) → last `vm start`/`vm rebuild` + /// dir → error. #[arg(long)] vm_source: Option, /// Memory in MiB. @@ -4186,6 +4212,12 @@ enum VmCommands { /// Default 50G — comfortable for SDK image + several container images. #[arg(long)] var_size: Option, + /// One-shot DNS override applied to this start only. Repeatable — + /// `--dns 1.1.1.1 --dns 8.8.8.8` sets both. Wins over any value + /// persisted via `vm config set network.dns`; the persisted value + /// is unchanged. Useful when a VPN's slirp DNS proxy is broken. + #[arg(long = "dns")] + dns: Vec, /// Tail the serial log live while waiting for boot-sync. On failure, /// the tail of the log is printed automatically even without this flag. #[arg(short = 'w', long)] @@ -4230,6 +4262,13 @@ enum VmCommands { #[arg(short = 'y', long)] yes: bool, }, + /// Read/write persistent VM configuration at `~/.avocado/vm/config.yaml`. + /// Same file the Avocado.app settings UI edits — every knob shipped in + /// the desktop is reachable here. + Config { + #[command(subcommand)] + command: VmConfigCommands, + }, /// Check for and apply VM image updates from the release channel. /// Stops + restarts the VM if it was running. Preserves the existing /// `var` partition; use `vm reset` to wipe state. @@ -4250,6 +4289,40 @@ enum VmCommands { }, } +#[derive(Subcommand)] +enum VmConfigCommands { + /// Print the value of a dotted key (e.g. `network.dns`). Silent on + /// missing keys; use `--output json` to disambiguate missing vs empty. + Get { + /// Dotted key path, e.g. `network.dns` or `network.dns_search`. + key: String, + /// Output format (human plain text or JSON `{key, value}`). + #[arg(long, value_enum, default_value_t = crate::utils::output_format::OutputFormat::Human)] + output: crate::utils::output_format::OutputFormat, + }, + /// Set a dotted key. Multiple values become a list (e.g. + /// `vm config set network.dns 1.1.1.1 8.8.8.8`). + Set { + /// Dotted key path. + key: String, + /// One or more values. A single value is stored as a scalar; two + /// or more are stored as a list. Use `vm config unset` to remove. + #[arg(required = true, num_args = 1..)] + values: Vec, + }, + /// Remove a dotted key. No-op if it doesn't exist. + Unset { + /// Dotted key path. + key: String, + }, + /// Print the entire config (YAML by default, JSON with `--output json`). + /// The same JSON shape is what avocado-desktop reads to render its UI. + List { + #[arg(long, value_enum, default_value_t = crate::utils::output_format::OutputFormat::Human)] + output: crate::utils::output_format::OutputFormat, + }, +} + #[derive(Subcommand)] enum HitlCommands { /// Start a HITL server container with preconfigured settings diff --git a/src/utils/vm/state.rs b/src/utils/vm/state.rs index d790978..ab34475 100644 --- a/src/utils/vm/state.rs +++ b/src/utils/vm/state.rs @@ -54,6 +54,37 @@ impl VmPaths { pub fn install_manifest(&self) -> PathBuf { self.install_dir().join("manifest.json") } + + /// Default artifact directory to boot from when the user didn't + /// pass `--vm-source` and `AVOCADO_VM_DIR` is unset. Layered: + /// + /// 1. [`install_dir`] if it has a manifest (i.e. the user ran + /// `avocado vm update` — the common path for end-users). + /// 2. The `artifact-dir` pointer file written by previous + /// `vm start` / `vm rebuild` runs, if it still points at + /// an extant directory with a manifest (dev workflow). + /// 3. `None` — the caller surfaces an error pointing the user + /// at `avocado vm update`. + pub fn default_vm_source(&self) -> Option { + // Managed install first — that's what the user gets after + // `avocado vm update`. + let install = self.install_dir(); + if install.join("manifest.json").is_file() { + return Some(install); + } + // Fallback: artifact-dir pointer file from the last explicit + // `vm start --vm-source ` / `vm rebuild`. + if let Ok(raw) = std::fs::read_to_string(self.artifact_dir_file()) { + let trimmed = raw.trim(); + if !trimmed.is_empty() { + let p = PathBuf::from(trimmed); + if p.join("manifest.json").is_file() { + return Some(p); + } + } + } + None + } /// Reserved for future artifact caching under ~/.avocado/vm/. #[allow(dead_code)] pub fn rootfs(&self) -> PathBuf { @@ -125,6 +156,12 @@ impl VmPaths { pub fn artifact_dir_file(&self) -> PathBuf { self.root.join("artifact-dir") } + /// Persistent VM configuration (DNS, future network knobs, etc.). Both + /// avocado-cli (`vm config set/get`) and Avocado.app's settings UI + /// read/write this file. + pub fn config_file(&self) -> PathBuf { + self.root.join("config.yaml") + } } /// Find the user's home directory. Wraps the `directories` crate so callers From f864d8428fe0d858badf866229101975bdb32ecf Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Tue, 19 May 2026 15:36:09 -0500 Subject: [PATCH 04/17] vm: match avocado connect upload's progress bar style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-allocate a ProgressBar per artifact via MultiProgress (same as connect upload at src/commands/connect/upload.rs:563-585), and reuse the exact template: " {msg} [{wide_bar:.cyan/blue}] {bytes}/{total_bytes} ({bytes_per_sec})" with progress_chars "#>-". Bars finish_with_message("X (done)") so the queue stays visible at 100% after each download — matches the connect upload completion behaviour. The earlier per-file style (green/black bar + ETA, drawn one at a time after a "[1/4] downloading…" header) is replaced; users now see all four artifacts queued from the start, filling sequentially. One visual idiom across the CLI. JSON mode unchanged — emits the same three download_* events (started / progress / completed). --- src/commands/vm/update.rs | 68 ++++++++++++++++++++++++++------------- 1 file changed, 46 insertions(+), 22 deletions(-) diff --git a/src/commands/vm/update.rs b/src/commands/vm/update.rs index a5f7cf7..76ad6e2 100644 --- a/src/commands/vm/update.rs +++ b/src/commands/vm/update.rs @@ -20,7 +20,7 @@ use anyhow::{bail, Context, Result}; use clap::ValueEnum; use futures_util::StreamExt; -use indicatif::{ProgressBar, ProgressStyle}; +use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; use reqwest::ClientBuilder; use serde_json::json; use std::io::Write; @@ -165,6 +165,35 @@ impl UpdateCommand { stage.record_was_running(was_running)?; let json_mode = self.output.is_json(); + + // Pre-create the MultiProgress + one ProgressBar per artifact + // so the user sees the whole queue from the start (bars at 0% + // for not-yet-started files, filling sequentially as each + // download runs). Matches `avocado connect upload`'s rendering + // for a consistent look across the CLI. + let multi = if !json_mode { + Some(MultiProgress::new()) + } else { + None + }; + let bars: Vec> = downloads + .iter() + .map(|item| { + multi.as_ref().map(|m| { + let pb = m.add(ProgressBar::new(item.size.unwrap_or(0))); + pb.set_style( + ProgressStyle::with_template( + " {msg} [{wide_bar:.cyan/blue}] {bytes}/{total_bytes} ({bytes_per_sec})", + ) + .expect("static template parses") + .progress_chars("#>-"), + ); + pb.set_message(item.file.clone()); + pb + }) + }) + .collect(); + for (idx, item) in downloads.iter().enumerate() { let url = format!( "{}/{}", @@ -179,6 +208,7 @@ impl UpdateCommand { idx + 1, downloads.len(), json_mode, + bars[idx].as_ref(), ) .await .with_context(|| format!("downloading {}", item.file))?; @@ -385,6 +415,7 @@ fn print_update_available( /// - In `--output json` mode emits NDJSON progress events throttled to /// ~10 Hz so the desktop app can drive a progress bar without being /// flooded. +#[allow(clippy::too_many_arguments)] async fn download_artifact( http: &reqwest::Client, url: &str, @@ -393,6 +424,7 @@ async fn download_artifact( idx: usize, total_items: usize, json_mode: bool, + pb: Option<&ProgressBar>, ) -> Result<()> { let resp = http .get(url) @@ -402,23 +434,14 @@ async fn download_artifact( .error_for_status()?; let total_bytes = resp.content_length().or(item.size).unwrap_or(0); - if !json_mode { - println!( - "[{idx}/{total_items}] downloading {} ({} bytes)", - item.file, total_bytes, - ); - } - let pb = if !json_mode { - let pb = ProgressBar::new(total_bytes); - pb.set_style( - ProgressStyle::with_template( - " {bar:30.green/black} {decimal_bytes:>10}/{decimal_total_bytes:<10} {decimal_bytes_per_sec:>12} ETA {eta:>5}", - ) - .unwrap() - .progress_chars("=> "), - ); - Some(pb) - } else { + // Bar was pre-created in the caller with the manifest's `size`. The + // HTTP response's content_length is the authoritative figure once + // the request lands — adjust the length if it differs. + if let Some(pb) = pb { + if total_bytes > 0 { + pb.set_length(total_bytes); + } + } else if json_mode { crate::utils::output_format::emit_json_object(&json!({ "event": "download_started", "file": item.file, @@ -426,8 +449,7 @@ async fn download_artifact( "index": idx, "total": total_items, })); - None - }; + } let mut file = std::fs::File::create(dest).with_context(|| format!("creating {}", dest.display()))?; @@ -439,7 +461,7 @@ async fn download_artifact( file.write_all(&chunk) .with_context(|| format!("writing to {}", dest.display()))?; written += chunk.len() as u64; - if let Some(pb) = &pb { + if let Some(pb) = pb { pb.set_position(written); } else if json_mode && last_emit.elapsed() >= Duration::from_millis(100) { crate::utils::output_format::emit_json_object(&json!({ @@ -454,7 +476,9 @@ async fn download_artifact( file.sync_all().ok(); drop(file); if let Some(pb) = pb { - pb.finish_and_clear(); + // Leave the bar visible at 100% with a "(done)" tail — matches + // `avocado connect upload`'s finish-with-message style. + pb.finish_with_message(format!("{} (done)", item.file)); } if json_mode { crate::utils::output_format::emit_json_object(&json!({ From 2759150c3c3020d42c10a30789fd8f15ae82bcbf Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Wed, 20 May 2026 10:48:32 -0500 Subject: [PATCH 05/17] vm: add `vm config` subcommand + persistent guest network config Adds `~/.avocado/vm/config.yaml` as a shared host/guest config file with a typed `VmConfig` reader and a `vm config get/set/unset/list` CLI surface. avocado-desktop reads/writes the same file so settings stay in sync across the two clients. First consumer is DNS: persisted `network.dns` (+ optional one-shot `--dns` override on `vm start`) is pushed into the guest via `resolvectl` once SSH comes up. Falls back to SLIRP's 10.0.2.3 when nothing is configured. Resolves the macOS-VPN case where scoped resolvers on the host are invisible to the slirp DNS proxy. --- src/commands/vm/config.rs | 118 ++++++++++++++ src/commands/vm/mod.rs | 1 + src/commands/vm/reset.rs | 1 + src/utils/vm/config.rs | 326 ++++++++++++++++++++++++++++++++++++++ src/utils/vm/lifecycle.rs | 102 ++++++++++++ src/utils/vm/mod.rs | 1 + src/utils/vm/route.rs | 1 + 7 files changed, 550 insertions(+) create mode 100644 src/commands/vm/config.rs create mode 100644 src/utils/vm/config.rs diff --git a/src/commands/vm/config.rs b/src/commands/vm/config.rs new file mode 100644 index 0000000..d7f2f39 --- /dev/null +++ b/src/commands/vm/config.rs @@ -0,0 +1,118 @@ +//! `avocado vm config` — read/write the persistent VM config at +//! `~/.avocado/vm/config.yaml`. The file is shared with avocado-desktop; +//! anything desktop exposes should round-trip through these commands. +//! +//! Subcommands: +//! - `get ` print the value (YAML or JSON) +//! - `set ` write one (scalar) or many (list) values +//! - `unset ` remove a key +//! - `list` print the whole config (YAML or JSON) + +use anyhow::{Context, Result}; + +use crate::utils::output_format::{emit_json_object, OutputFormat}; +use crate::utils::vm::config::VmConfig; +use crate::utils::vm::state::VmPaths; + +pub struct ConfigGetCommand { + pub key: String, + pub output: OutputFormat, +} + +impl ConfigGetCommand { + pub async fn execute(self) -> Result<()> { + let paths = VmPaths::resolve()?; + let cfg = VmConfig::load(&paths)?; + let value = cfg.get(&self.key)?; + match (value, self.output) { + (None, OutputFormat::Json) => { + emit_json_object(&serde_json::json!({ "key": self.key, "value": null })); + } + (None, OutputFormat::Human) => { + // Stay silent on stdout so `vm config get` is grep-safe in + // scripts. Exit code stays 0 — missing != error. + } + (Some(v), OutputFormat::Json) => { + let json_v: serde_json::Value = + serde_json::to_value(&v).context("yaml→json convert")?; + emit_json_object(&serde_json::json!({ "key": self.key, "value": json_v })); + } + (Some(v), OutputFormat::Human) => print_yaml_value(&v), + } + Ok(()) + } +} + +pub struct ConfigSetCommand { + pub key: String, + pub values: Vec, +} + +impl ConfigSetCommand { + pub async fn execute(self) -> Result<()> { + let paths = VmPaths::resolve()?; + let mut cfg = VmConfig::load(&paths)?; + cfg.set(&self.key, &self.values)?; + cfg.save(&paths)?; + Ok(()) + } +} + +pub struct ConfigUnsetCommand { + pub key: String, +} + +impl ConfigUnsetCommand { + pub async fn execute(self) -> Result<()> { + let paths = VmPaths::resolve()?; + let mut cfg = VmConfig::load(&paths)?; + cfg.unset(&self.key)?; + cfg.save(&paths)?; + Ok(()) + } +} + +pub struct ConfigListCommand { + pub output: OutputFormat, +} + +impl ConfigListCommand { + pub async fn execute(self) -> Result<()> { + let paths = VmPaths::resolve()?; + let cfg = VmConfig::load(&paths)?; + match self.output { + OutputFormat::Json => { + let v: serde_json::Value = + serde_json::to_value(&cfg).context("yaml→json convert")?; + emit_json_object(&v); + } + OutputFormat::Human => { + let yaml = serde_yaml::to_string(&cfg).context("serialize config")?; + // Default-constructed config serializes to "{}\n" which is + // confusing to read; print a friendlier hint instead. + let trimmed = yaml.trim(); + if trimmed.is_empty() || trimmed == "{}" { + println!("(no config set; use `avocado vm config set `)"); + } else { + print!("{yaml}"); + } + } + } + Ok(()) + } +} + +fn print_yaml_value(v: &serde_yaml::Value) { + // Render scalars as bare values (so callers can `vm config get ... | xargs`) + // and structured values as YAML. + match v { + serde_yaml::Value::String(s) => println!("{s}"), + serde_yaml::Value::Number(n) => println!("{n}"), + serde_yaml::Value::Bool(b) => println!("{b}"), + serde_yaml::Value::Null => {} + other => { + let s = serde_yaml::to_string(other).unwrap_or_default(); + print!("{s}"); + } + } +} diff --git a/src/commands/vm/mod.rs b/src/commands/vm/mod.rs index 0604f0d..daf97cb 100644 --- a/src/commands/vm/mod.rs +++ b/src/commands/vm/mod.rs @@ -4,6 +4,7 @@ //! work lives in the utils submodules; these modules just present the user- //! facing flags and pretty-print results. +pub mod config; pub mod logs; pub mod rebuild; pub mod reset; diff --git a/src/commands/vm/reset.rs b/src/commands/vm/reset.rs index d073dd6..b6a58c2 100644 --- a/src/commands/vm/reset.rs +++ b/src/commands/vm/reset.rs @@ -106,6 +106,7 @@ impl ResetCommand { cmdline_extra: None, workspace: None, var_size: None, + dns_override: None, }; lifecycle::start(opts).await?; } diff --git a/src/utils/vm/config.rs b/src/utils/vm/config.rs new file mode 100644 index 0000000..f9333bc --- /dev/null +++ b/src/utils/vm/config.rs @@ -0,0 +1,326 @@ +//! Per-VM persistent configuration at `~/.avocado/vm/config.yaml`. +//! +//! Read by both avocado-cli and avocado-desktop. The CLI surface +//! (`avocado vm config get/set/list/unset`) is the source of truth; the +//! desktop UI edits this same file so the two stay in lockstep. +//! +//! Schema is intentionally small to start — network DNS only — and grows +//! by adding keys. Unknown keys round-trip via `serde(flatten)` on a +//! `BTreeMap` so an older CLI doesn't drop +//! settings a newer desktop wrote. + +use anyhow::{anyhow, bail, Context, Result}; +use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; +use std::path::Path; + +use super::state::VmPaths; + +/// Top-level config. Every section is optional so a partial file is valid +/// and a freshly-written one only contains keys the user actually set. +#[derive(Debug, Default, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct VmConfig { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub network: Option, + + /// Forward-compat bucket for keys this CLI version doesn't know about. + /// Preserved verbatim on save so a newer desktop's settings survive an + /// older CLI's round-trip. + #[serde(flatten)] + pub extra: BTreeMap, +} + +#[derive(Debug, Default, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct NetworkConfig { + /// Override the guest's DNS resolvers. Applied post-boot via + /// `resolvectl dns eth0 …` so it survives `vm stop` / `vm start`. + /// `None` keeps slirp's DHCP-provided 10.0.2.3 (the default). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub dns: Option>, + + /// DNS search domains. Applied as `resolvectl domain eth0 …`. When + /// `dns` is set without `dns_search`, the apply step also installs + /// `~.` so the public resolvers handle all suffixes. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub dns_search: Option>, + + #[serde(flatten)] + pub extra: BTreeMap, +} + +impl VmConfig { + /// Load the config from `paths.config_file()`. A missing file is *not* + /// an error — returns `Self::default()` so callers can treat "no config" + /// and "empty config" identically. + pub fn load(paths: &VmPaths) -> Result { + Self::load_from(&paths.config_file()) + } + + pub fn load_from(path: &Path) -> Result { + if !path.exists() { + return Ok(Self::default()); + } + let raw = + std::fs::read_to_string(path).with_context(|| format!("reading {}", path.display()))?; + if raw.trim().is_empty() { + return Ok(Self::default()); + } + serde_yaml::from_str(&raw).with_context(|| format!("parsing {}", path.display())) + } + + /// Save atomically via a sibling tempfile + rename. The directory is + /// created if missing so first-write doesn't require a prior `vm start`. + pub fn save(&self, paths: &VmPaths) -> Result<()> { + paths.ensure()?; + self.save_to(&paths.config_file()) + } + + pub fn save_to(&self, path: &Path) -> Result<()> { + let parent = path + .parent() + .ok_or_else(|| anyhow!("config path {} has no parent", path.display()))?; + std::fs::create_dir_all(parent) + .with_context(|| format!("creating {}", parent.display()))?; + let yaml = serde_yaml::to_string(self).context("serializing vm config")?; + let mut tmp = tempfile::NamedTempFile::new_in(parent).context("temp file for vm config")?; + use std::io::Write; + tmp.write_all(yaml.as_bytes()) + .context("write vm config temp file")?; + tmp.flush().context("flush vm config temp file")?; + tmp.persist(path) + .with_context(|| format!("persist {}", path.display()))?; + Ok(()) + } + + /// Read a dotted key (e.g. `network.dns`) as a YAML value. Unknown + /// sections / keys return `Ok(None)`. + pub fn get(&self, key: &str) -> Result> { + let value = serde_yaml::to_value(self).context("snapshot config as yaml")?; + Ok(walk(&value, key)) + } + + /// Set a dotted key. Strings that parse as YAML scalars (numbers, bools, + /// lists) are coerced; everything else is stored as a string. If the + /// resulting config fails to deserialize because the schema expects a + /// sequence at this key (e.g. `network.dns` with a single value), the + /// scalar is automatically promoted to a single-element list — so + /// `vm config set network.dns 1.1.1.1` does the obvious thing without + /// the caller having to know the schema. + pub fn set(&mut self, key: &str, raw_values: &[String]) -> Result<()> { + let value = parse_value(raw_values)?; + let base = serde_yaml::to_value(&*self).context("snapshot config as yaml")?; + + let try_value = |v: serde_yaml::Value| -> Result { + let mut snap = base.clone(); + place(&mut snap, key, v)?; + serde_yaml::from_value(snap).map_err(anyhow::Error::from) + }; + + match try_value(value.clone()) { + Ok(updated) => { + *self = updated; + Ok(()) + } + Err(first) => { + // Retry by wrapping in a one-element list — covers the + // common case `set network.dns 1.1.1.1` where the schema + // is `Vec` but the user only passed one value. + if !matches!(value, serde_yaml::Value::Sequence(_)) { + let wrapped = serde_yaml::Value::Sequence(vec![value]); + if let Ok(updated) = try_value(wrapped) { + *self = updated; + return Ok(()); + } + } + Err(first).with_context(|| format!("invalid value for {key}")) + } + } + } + + /// Remove a dotted key. No-op if it doesn't exist. + pub fn unset(&mut self, key: &str) -> Result<()> { + let mut snap = serde_yaml::to_value(&*self).context("snapshot config as yaml")?; + remove(&mut snap, key)?; + let updated: VmConfig = serde_yaml::from_value(snap) + .with_context(|| format!("invalid config after unset {key}"))?; + *self = updated; + Ok(()) + } +} + +/// Parse one-or-many strings into a YAML value. Single value → scalar (or +/// list/bool/number if it parses as YAML); multiple values → sequence of +/// strings. Empty input is an error so `vm config set k` with no value +/// doesn't silently clear the key (use `unset` for that). +fn parse_value(raw: &[String]) -> Result { + match raw.len() { + 0 => bail!("missing value (use `vm config unset` to remove)"), + 1 => { + // Accept YAML scalars (numbers, bools, JSON-ish lists). Fall back + // to plain string if the parse is ambiguous (e.g. "8.8.8.8" parses + // as a string already, but "1.1.1.1" should remain a string too). + let s = &raw[0]; + match serde_yaml::from_str::(s) { + Ok(serde_yaml::Value::Bool(b)) => Ok(serde_yaml::Value::Bool(b)), + Ok(serde_yaml::Value::Number(n)) => Ok(serde_yaml::Value::Number(n)), + Ok(serde_yaml::Value::Sequence(seq)) => Ok(serde_yaml::Value::Sequence(seq)), + _ => Ok(serde_yaml::Value::String(s.clone())), + } + } + _ => Ok(serde_yaml::Value::Sequence( + raw.iter() + .map(|s| serde_yaml::Value::String(s.clone())) + .collect(), + )), + } +} + +fn walk(value: &serde_yaml::Value, key: &str) -> Option { + let mut cur = value; + for part in key.split('.') { + let map = cur.as_mapping()?; + cur = map.get(serde_yaml::Value::String(part.into()))?; + } + Some(cur.clone()) +} + +fn place(root: &mut serde_yaml::Value, key: &str, new_value: serde_yaml::Value) -> Result<()> { + let parts: Vec<&str> = key.split('.').collect(); + if parts.iter().any(|p| p.is_empty()) { + bail!("invalid key {key:?}: empty path segment"); + } + if !root.is_mapping() { + *root = serde_yaml::Value::Mapping(serde_yaml::Mapping::new()); + } + let mut cur = root; + for part in &parts[..parts.len() - 1] { + let map = cur + .as_mapping_mut() + .ok_or_else(|| anyhow!("cannot descend into non-mapping at {part}"))?; + let entry = map + .entry(serde_yaml::Value::String((*part).into())) + .or_insert_with(|| serde_yaml::Value::Mapping(serde_yaml::Mapping::new())); + if !entry.is_mapping() { + *entry = serde_yaml::Value::Mapping(serde_yaml::Mapping::new()); + } + cur = entry; + } + let last = parts[parts.len() - 1]; + let map = cur + .as_mapping_mut() + .ok_or_else(|| anyhow!("cannot set {key}: not a mapping"))?; + map.insert(serde_yaml::Value::String(last.into()), new_value); + Ok(()) +} + +fn remove(root: &mut serde_yaml::Value, key: &str) -> Result<()> { + let parts: Vec<&str> = key.split('.').collect(); + if parts.iter().any(|p| p.is_empty()) { + bail!("invalid key {key:?}: empty path segment"); + } + let mut cur = root; + for part in &parts[..parts.len() - 1] { + let Some(map) = cur.as_mapping_mut() else { + return Ok(()); + }; + let Some(next) = map.get_mut(serde_yaml::Value::String((*part).into())) else { + return Ok(()); + }; + cur = next; + } + if let Some(map) = cur.as_mapping_mut() { + map.remove(serde_yaml::Value::String(parts[parts.len() - 1].into())); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::tempdir; + + #[test] + fn missing_file_is_default() { + let tmp = tempdir().unwrap(); + let cfg = VmConfig::load_from(&tmp.path().join("config.yaml")).unwrap(); + assert_eq!(cfg, VmConfig::default()); + } + + #[test] + fn round_trip_through_yaml() { + let tmp = tempdir().unwrap(); + let path = tmp.path().join("config.yaml"); + let cfg = VmConfig { + network: Some(NetworkConfig { + dns: Some(vec!["1.1.1.1".into(), "8.8.8.8".into()]), + dns_search: None, + extra: Default::default(), + }), + ..Default::default() + }; + cfg.save_to(&path).unwrap(); + let loaded = VmConfig::load_from(&path).unwrap(); + assert_eq!(loaded, cfg); + } + + #[test] + fn set_then_get_dns() { + let mut cfg = VmConfig::default(); + cfg.set("network.dns", &["1.1.1.1".into(), "8.8.8.8".into()]) + .unwrap(); + let got = cfg.get("network.dns").unwrap().unwrap(); + let seq = got.as_sequence().unwrap(); + assert_eq!(seq.len(), 2); + assert_eq!(seq[0].as_str(), Some("1.1.1.1")); + assert_eq!(seq[1].as_str(), Some("8.8.8.8")); + } + + #[test] + fn unset_removes_key() { + let mut cfg = VmConfig::default(); + cfg.set("network.dns", &["1.1.1.1".into()]).unwrap(); + cfg.unset("network.dns").unwrap(); + assert!(cfg.get("network.dns").unwrap().is_none()); + } + + #[test] + fn unknown_keys_round_trip() { + // Simulate a newer desktop writing a key this CLI doesn't model. + let tmp = tempdir().unwrap(); + let path = tmp.path().join("config.yaml"); + let yaml = + "network:\n dns:\n - 1.1.1.1\n future_knob: 42\nother_section:\n foo: bar\n"; + std::fs::write(&path, yaml).unwrap(); + let loaded = VmConfig::load_from(&path).unwrap(); + loaded.save_to(&path).unwrap(); + let reread = std::fs::read_to_string(&path).unwrap(); + assert!( + reread.contains("future_knob"), + "lost forward-compat key: {reread}" + ); + assert!( + reread.contains("other_section"), + "lost unknown section: {reread}" + ); + } + + #[test] + fn set_rejects_empty_value() { + let mut cfg = VmConfig::default(); + let err = cfg.set("network.dns", &[]).unwrap_err(); + assert!(err.to_string().contains("missing value")); + } + + #[test] + fn empty_segment_in_key_rejected() { + let mut cfg = VmConfig::default(); + let err = cfg.set("network..dns", &["1.1.1.1".into()]).unwrap_err(); + // Use alternate-format ({:#}) so the underlying `place` error + // (which `set` wraps in "invalid value for …" context) is included. + let full = format!("{err:#}"); + assert!( + full.contains("empty path segment"), + "unexpected error chain: {full}" + ); + } +} diff --git a/src/utils/vm/lifecycle.rs b/src/utils/vm/lifecycle.rs index 185ac11..b0b254b 100644 --- a/src/utils/vm/lifecycle.rs +++ b/src/utils/vm/lifecycle.rs @@ -35,6 +35,11 @@ pub struct StartOptions { /// until written), then btrfs is resized to fill it inside the VM. /// Shrinking is refused. `None` → default ([`DEFAULT_VAR_SIZE`]). pub var_size: Option, + /// One-shot DNS override for this start only. Wins over the persisted + /// `network.dns` in `~/.avocado/vm/config.yaml`; the persisted value + /// is unchanged. `None` → fall through to the persisted config (or + /// SLIRP's DHCP-supplied 10.0.2.3 if neither is set). + pub dns_override: Option>, } /// Default target size for the persistent var.btrfs. 50 GiB sparse — large @@ -189,6 +194,17 @@ pub async fn start(opts: StartOptions) -> Result { ); } + // Apply persisted network config (+ any one-shot --dns override). The + // most common reason this matters: macOS host on a VPN that pushes DNS + // via scoped resolvers, which QEMU's slirp DNS proxy (10.0.2.3) can't + // see. Pointing the guest at public resolvers via SLIRP's NAT works. + if let Err(e) = apply_network_config(&target, opts.dns_override.as_deref()).await { + crate::utils::output::print_warning( + &format!("applying network config in guest failed: {e:#}. Falling back to slirp's default DNS (10.0.2.3)."), + crate::utils::output::OutputLevel::Normal, + ); + } + // Bring up the docker-socket SSH forward so DOCKER_HOST=unix://… works // from the host without touching the user's ~/.ssh/config. Non-fatal // on error: a working VM with just SSH access is still useful for @@ -324,6 +340,92 @@ pub fn ssh_target_for_running() -> Result { Ok(SshTarget::local(&paths, port)) } +/// Apply the persisted [`config::VmConfig`] (+ optional `--dns` one-shot +/// override) inside the running guest. Currently only DNS is implemented; +/// future knobs (MTU, http_proxy, …) hang off the same entry point. +/// +/// Resolution order for DNS: `dns_override` if `Some(non-empty)`, else +/// the persisted `network.dns`, else no-op (leave SLIRP's 10.0.2.3). +async fn apply_network_config( + target: &super::ssh::SshTarget, + dns_override: Option<&[String]>, +) -> Result<()> { + use super::config::VmConfig; + + let paths = VmPaths::resolve()?; + let persisted = VmConfig::load(&paths).unwrap_or_default(); + let persisted_dns = persisted + .network + .as_ref() + .and_then(|n| n.dns.as_ref()) + .map(|v| v.as_slice()); + let persisted_search = persisted + .network + .as_ref() + .and_then(|n| n.dns_search.as_ref()) + .map(|v| v.as_slice()); + + let effective_dns: Option<&[String]> = match dns_override { + Some(v) if !v.is_empty() => Some(v), + _ => persisted_dns, + }; + + let Some(dns_list) = effective_dns else { + return Ok(()); // nothing to apply + }; + + for s in dns_list { + if !looks_like_ip(s) { + bail!("network.dns entry {s:?} is not an IPv4/IPv6 literal"); + } + } + let dns_args = dns_list.join(" "); + target + .exec(&format!("resolvectl dns eth0 {dns_args}")) + .await + .with_context(|| format!("resolvectl dns eth0 {dns_args}"))?; + + // If the user set explicit search domains, honor them verbatim; else + // install `~.` so the user-supplied resolvers handle every suffix + // (otherwise systemd-resolved still routes some queries to 10.0.2.3 + // via per-link defaults). + let domains = match persisted_search { + Some(list) if !list.is_empty() => list + .iter() + .map(|d| shell_quote(d)) + .collect::>() + .join(" "), + _ => "'~.'".to_string(), + }; + target + .exec(&format!("resolvectl domain eth0 {domains}")) + .await + .with_context(|| format!("resolvectl domain eth0 {domains}"))?; + + crate::utils::output::print_info( + &format!("applied guest DNS: {}", dns_list.join(", ")), + crate::utils::output::OutputLevel::Normal, + ); + Ok(()) +} + +/// Very lax IP literal check — we just want to refuse obvious shell-injection +/// attempts before passing user input into a remote command. Anything that +/// isn't [0-9a-fA-F.:%/_] gets rejected; the kernel will catch malformed +/// IPs later. +fn looks_like_ip(s: &str) -> bool { + !s.is_empty() + && s.chars() + .all(|c| c.is_ascii_hexdigit() || matches!(c, '.' | ':' | '%' | '/' | '_')) +} + +/// Single-quote a search-domain string for shell. Search domains are +/// constrained (RFC 1035 label charset), but be defensive anyway. +fn shell_quote(s: &str) -> String { + let escaped = s.replace('\'', "'\\''"); + format!("'{escaped}'") +} + /// Parse a human-readable size string ("50G", "10M", "1024K", "12345" bytes). /// Suffixes are powers of 1024 (KiB/MiB/GiB), matching what `truncate(1)` /// and `btrfs filesystem resize` accept on Linux. diff --git a/src/utils/vm/mod.rs b/src/utils/vm/mod.rs index 4fde6c1..bc4a7c3 100644 --- a/src/utils/vm/mod.rs +++ b/src/utils/vm/mod.rs @@ -22,6 +22,7 @@ pub mod boot_sync; pub mod channel; #[cfg(target_os = "macos")] pub mod client; +pub mod config; pub mod forward; pub mod lifecycle; pub mod manifest; diff --git a/src/utils/vm/route.rs b/src/utils/vm/route.rs index 5a897a7..9f26705 100644 --- a/src/utils/vm/route.rs +++ b/src/utils/vm/route.rs @@ -112,6 +112,7 @@ pub async fn ensure_routed_for_process( cmdline_extra: None, workspace: None, var_size: None, + dns_override: None, }) .await?; status From 1a505d86217b1cad8bce3756c710a2a2991440fd Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Wed, 20 May 2026 10:49:01 -0500 Subject: [PATCH 06/17] ci: add windows-msvc compile check + gate unix-only modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New `windows-check` PR job that runs `cargo check --target x86_64-pc-windows-msvc`. To make that build pass without functional regressions on unix targets: - `qga` (AF_UNIX socket client) and `qmp` (QEMU monitor over AF_UNIX) are now `#[cfg(unix)]` on the module declarations; their call sites in `lifecycle::stop` and `boot_sync::wait_for_guest_ready` get the same gating with a non-unix fallback path that waits for SSH only. - `libc::SIGTERM` / `libc::SIGKILL` are not exposed on Windows — replace the call-site uses in `lifecycle` and `forward` with local POSIX-value constants (`send_signal` itself is already `#[cfg(unix)]`). No behavior change on macOS/Linux. --- .github/workflows/pr.yml | 20 ++++++++++++++++ src/utils/vm/boot_sync.rs | 48 ++++++++++++++++++++++++--------------- src/utils/vm/forward.rs | 12 +++++++--- src/utils/vm/lifecycle.rs | 14 +++++++++--- src/utils/vm/mod.rs | 2 ++ 5 files changed, 72 insertions(+), 24 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 54c32be..5d20f80 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -12,3 +12,23 @@ jobs: security: name: Security Audit uses: ./.github/workflows/security-reusable.yml + + windows-check: + name: Windows compile check + runs-on: windows-latest + steps: + - name: Install build pre-reqs + run: | + choco install nasm cmake -y + echo "C:\Program Files\NASM" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append + + - uses: actions/checkout@v4 + + - uses: dtolnay/rust-toolchain@stable + with: + target: x86_64-pc-windows-msvc + + - uses: Swatinem/rust-cache@v2 + + - name: cargo check (windows-msvc) + run: cargo check --target x86_64-pc-windows-msvc --all-targets diff --git a/src/utils/vm/boot_sync.rs b/src/utils/vm/boot_sync.rs index c69adaa..1ce7531 100644 --- a/src/utils/vm/boot_sync.rs +++ b/src/utils/vm/boot_sync.rs @@ -12,6 +12,7 @@ use tokio::io::AsyncReadExt; use tokio::net::TcpStream; use tokio::time::{sleep, timeout, Duration, Instant}; +#[cfg(unix)] use super::qga::QgaClient; // 120s covers cold-boot pessimism on first run (kernel ramdisk extract + @@ -34,29 +35,39 @@ pub async fn wait_for_guest_ready( ) -> Result<&'static str> { let timeout = timeout.unwrap_or(DEFAULT_BOOT_TIMEOUT); - let qga = wait_qga(qga_socket.to_path_buf(), timeout); - let ssh = wait_ssh(ssh_port, timeout); + #[cfg(unix)] + { + let qga = wait_qga(qga_socket.to_path_buf(), timeout); + let ssh = wait_ssh(ssh_port, timeout); - // Race for the first sign of life. qga winning is fine — it just means - // we'll lean on the SSH wait that follows. SSH winning is enough by - // itself (it's the same thing the caller's about to use). - let winner = tokio::select! { - r = qga => r.map(|_| "qga"), - r = ssh => r.map(|_| "ssh"), - }?; + // Race for the first sign of life. qga winning is fine — it just means + // we'll lean on the SSH wait that follows. SSH winning is enough by + // itself (it's the same thing the caller's about to use). + let winner = tokio::select! { + r = qga => r.map(|_| "qga"), + r = ssh => r.map(|_| "ssh"), + }?; - // If qga won, sshd may still be in its accept-but-reset window. Do a - // final SSH-banner check before returning so the next operation - // doesn't immediately hit "kex_exchange_identification: Connection - // reset by peer". - if winner == "qga" { - wait_ssh(ssh_port, timeout) - .await - .context("qga came up but sshd never advanced past kex-init")?; + // If qga won, sshd may still be in its accept-but-reset window. Do a + // final SSH-banner check before returning so the next operation + // doesn't immediately hit "kex_exchange_identification: Connection + // reset by peer". + if winner == "qga" { + wait_ssh(ssh_port, timeout) + .await + .context("qga came up but sshd never advanced past kex-init")?; + } + Ok(winner) + } + #[cfg(not(unix))] + { + let _ = qga_socket; + wait_ssh(ssh_port, timeout).await?; + Ok("ssh") } - Ok(winner) } +#[cfg(unix)] async fn wait_qga(socket: std::path::PathBuf, timeout: Duration) -> Result<()> { let deadline = Instant::now() + timeout; while !socket.exists() { @@ -118,6 +129,7 @@ async fn wait_ssh(port: u16, deadline_window: Duration) -> Result<()> { /// Same as [`wait_for_guest_ready`] but tries to surface a final error from /// the last attempt, useful for `avocado vm status` style introspection. +#[cfg(unix)] #[allow(dead_code)] pub async fn last_boot_error(socket: &Path) -> Result<()> { let mut client = QgaClient::connect(socket).await.context("qga connect")?; diff --git a/src/utils/vm/forward.rs b/src/utils/vm/forward.rs index 3a5fb2e..472ed2c 100644 --- a/src/utils/vm/forward.rs +++ b/src/utils/vm/forward.rs @@ -25,6 +25,12 @@ use super::state::{self, VmPaths}; const REMOTE_DOCKER_SOCK: &str = "/run/docker.sock"; +// POSIX signal numbers, declared here so call sites compile on Windows +// where `libc::SIGTERM` / `libc::SIGKILL` are not defined. `send_signal` +// itself is `#[cfg(unix)]` and is a no-op elsewhere. +const SIGTERM: libc::c_int = 15; +const SIGKILL: libc::c_int = 9; + /// Spawn the SSH forward in the background. The returned pid is the ssh /// process; killing it cleans up the forwarded socket. pub async fn start(paths: &VmPaths, ssh_port: u16) -> Result { @@ -100,7 +106,7 @@ pub async fn start(paths: &VmPaths, ssh_port: u16) -> Result { } if std::time::Instant::now() >= deadline { // Best-effort kill so we don't leak the process. - send_signal(pid, libc::SIGTERM); + send_signal(pid, SIGTERM); let _ = std::fs::remove_file(paths.forwarder_pid()); bail!( "timed out waiting for {} to appear (ssh forward not established)", @@ -115,7 +121,7 @@ pub async fn start(paths: &VmPaths, ssh_port: u16) -> Result { pub async fn stop(paths: &VmPaths) -> Result<()> { if let Ok(pid) = read_pid(&paths.forwarder_pid()) { if state::pid_alive(pid) { - send_signal(pid, libc::SIGTERM); + send_signal(pid, SIGTERM); for _ in 0..20 { if !state::pid_alive(pid) { break; @@ -123,7 +129,7 @@ pub async fn stop(paths: &VmPaths) -> Result<()> { tokio::time::sleep(Duration::from_millis(100)).await; } if state::pid_alive(pid) { - send_signal(pid, libc::SIGKILL); + send_signal(pid, SIGKILL); } } } diff --git a/src/utils/vm/lifecycle.rs b/src/utils/vm/lifecycle.rs index b0b254b..fe5a5a8 100644 --- a/src/utils/vm/lifecycle.rs +++ b/src/utils/vm/lifecycle.rs @@ -10,10 +10,17 @@ use std::time::Duration; use super::manifest::Manifest; use super::qemu::{self, QemuConfig}; +#[cfg(unix)] use super::qmp::QmpClient; use super::ssh::SshTarget; use super::state::{self, VmPaths}; +// POSIX signal numbers, declared here so call sites compile on Windows +// where `libc::SIGTERM` / `libc::SIGKILL` are not defined. `send_signal` +// itself is `#[cfg(unix)]` and is a no-op elsewhere. +const SIGTERM: libc::c_int = 15; +const SIGKILL: libc::c_int = 9; + /// Knobs the `avocado vm start` command resolves from args + env. #[derive(Debug, Clone)] pub struct StartOptions { @@ -268,6 +275,7 @@ pub async fn stop(force: bool) -> Result<()> { } // Try QMP quit first if the socket is around. + #[cfg(unix)] if paths.qmp_socket().exists() { if let Ok(mut q) = QmpClient::connect(&paths.qmp_socket()).await { let _ = q.quit().await; @@ -283,9 +291,9 @@ pub async fn stop(force: bool) -> Result<()> { } if force { - send_signal(pid, libc::SIGKILL); + send_signal(pid, SIGKILL); } else { - send_signal(pid, libc::SIGTERM); + send_signal(pid, SIGTERM); for _ in 0..20 { if !state::pid_alive(pid) { state::cleanup_transient(&paths); @@ -293,7 +301,7 @@ pub async fn stop(force: bool) -> Result<()> { } tokio::time::sleep(Duration::from_millis(250)).await; } - send_signal(pid, libc::SIGKILL); + send_signal(pid, SIGKILL); } // Final cleanup diff --git a/src/utils/vm/mod.rs b/src/utils/vm/mod.rs index bc4a7c3..34ff0cd 100644 --- a/src/utils/vm/mod.rs +++ b/src/utils/vm/mod.rs @@ -27,7 +27,9 @@ pub mod forward; pub mod lifecycle; pub mod manifest; pub mod qemu; +#[cfg(unix)] pub mod qga; +#[cfg(unix)] pub mod qmp; pub mod route; pub mod share; From abc9dba613d1f5d67d1b2e013c6084c46d004b01 Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Wed, 20 May 2026 10:49:43 -0500 Subject: [PATCH 07/17] runtime: support `extensions: - foo: {enabled: false}` map form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `utils::runtime_extension::RuntimeExtensionSpec::parse_entry` as the one place that knows how to read a runtime config's `extensions:` list. The list previously accepted only string entries; now it also accepts a single-key map whose value carries per-extension flags (initial flag: `enabled`). Every iterator that walks the list — `Config::extension_deps`, `Config::find_active_extensions`, runtime/build, runtime/deps, build, install — now funnels through the same parser, so any future per-extension knob (`merge_index`, `runs_on`, …) lands in one spot without silently dropping entries. `runtime build` propagates the `enabled: false` flag into the runtime manifest's `AVOCADO_EXT_DISABLED` env var so avocadoctl skips activation at refresh time. --- src/commands/build.rs | 5 +- src/commands/install.rs | 9 +- src/commands/runtime/build.rs | 66 ++++++++++++-- src/commands/runtime/deps.rs | 6 +- src/utils/config.rs | 23 +++-- src/utils/mod.rs | 1 + src/utils/runtime_extension.rs | 154 +++++++++++++++++++++++++++++++++ 7 files changed, 243 insertions(+), 21 deletions(-) create mode 100644 src/utils/runtime_extension.rs diff --git a/src/commands/build.rs b/src/commands/build.rs index 7161a8b..30cfde0 100644 --- a/src/commands/build.rs +++ b/src/commands/build.rs @@ -545,7 +545,10 @@ impl BuildCommand { merged_value.get("extensions").and_then(|e| e.as_sequence()) { for ext in extensions { - if let Some(ext_name) = ext.as_str() { + if let Some(spec) = + crate::utils::runtime_extension::RuntimeExtensionSpec::parse_entry(ext) + { + let ext_name = spec.name.as_str(); // Check if this extension has a source: field (remote extension) if let Some(Some(source)) = ext_sources.get(ext_name) { // Remote extension with source field diff --git a/src/commands/install.rs b/src/commands/install.rs index 29b4d4a..cf75796 100644 --- a/src/commands/install.rs +++ b/src/commands/install.rs @@ -446,10 +446,11 @@ impl InstallCommand { merged_value.get("extensions").and_then(|e| e.as_sequence()) { for ext_val in extensions_list { - if let Some(ext_name) = ext_val.as_str() { - required_extensions.insert(ExtensionDependency::Local( - ext_name.to_string(), - )); + if let Some(spec) = + crate::utils::runtime_extension::RuntimeExtensionSpec::parse_entry(ext_val) + { + required_extensions + .insert(ExtensionDependency::Local(spec.name)); } } } diff --git a/src/commands/runtime/build.rs b/src/commands/runtime/build.rs index 2f8cb19..1adc6c8 100644 --- a/src/commands/runtime/build.rs +++ b/src/commands/runtime/build.rs @@ -1189,14 +1189,28 @@ cp /opt/src/.tuf-staging-tmp/delegations/runtime-{runtime_uuid}.json \ let mut required_extensions = HashSet::new(); let _extension_type_overrides: HashMap> = HashMap::new(); + // Names the runtime config explicitly marked `enabled: false`, + // stored as interpolated names so they match the entries in + // `resolved_extensions` (used to look up manifest entries below). + // Flows through into the manifest's `enabled` field so avocadoctl + // skips activation at refresh time. + let mut disabled_extensions: HashSet = HashSet::new(); + // Collect extensions from the new `extensions` array format if let Some(extensions) = merged_runtime .get("extensions") .and_then(|e| e.as_sequence()) { for ext in extensions { - if let Some(ext_name) = ext.as_str() { - required_extensions.insert(ext_name.to_string()); + if let Some(spec) = + crate::utils::runtime_extension::RuntimeExtensionSpec::parse_entry(ext) + { + if !spec.enabled { + let interpolated = + crate::utils::interpolation::interpolate_name(&spec.name, target_arch); + disabled_extensions.insert(interpolated); + } + required_extensions.insert(spec.name); } } } @@ -1349,6 +1363,19 @@ fi"# .collect(); let ext_info_str = ext_info_pairs.join(" "); + // Space-separated, interpolated names that should land as + // `"enabled": false` in the manifest. Empty when no extension is + // disabled — Python reads this into a set, default-empty. + let ext_disabled_str: String = { + let mut names: Vec<&String> = disabled_extensions.iter().collect(); + names.sort(); + names + .into_iter() + .map(String::as_str) + .collect::>() + .join(" ") + }; + // Resolve `image:` config for rootfs / initramfs / kernel. Same // dynamic accessors as extensions — the helpers don't care about // the parent yaml node's identity. @@ -1430,6 +1457,7 @@ export AVOCADO_BUILT_AT="$BUILT_AT" export AVOCADO_RUNTIME_NAME="{runtime_name}" export AVOCADO_RUNTIME_VERSION="$RUNTIME_VERSION" export AVOCADO_EXT_PAIRS="{ext_info_str}" +export AVOCADO_EXT_DISABLED="{ext_disabled_str}" export AVOCADO_ROOTFS_IMAGE_TYPE="{rootfs_image_type}" export AVOCADO_INITRAMFS_IMAGE_TYPE="{initramfs_image_type}" export AVOCADO_KERNEL_IMAGE_TYPE="{kernel_image_type}" @@ -1559,6 +1587,19 @@ kos_amf = os.environ.get("AVOCADO_AMF_KOS") == "1" # MINI_HASH_PROTO = "1" MINI_HASH_BLOCK = 4096 +# Streaming SHA256. Images can be tens of GB (customer extensions in +# particular); the previous hashlib.sha256(f.read()) form OOM-killed the +# python3 child on a 20 GB extension. Chunk size is a balance: large +# enough to keep syscall overhead negligible, small enough that resident +# set stays flat regardless of file size. +HASH_CHUNK = 1024 * 1024 +def sha256_file(filepath): + h = hashlib.sha256() + with open(filepath, "rb") as f: + for chunk in iter(lambda: f.read(HASH_CHUNK), b""): + h.update(chunk) + return h.hexdigest() + def mini_hash(filepath): file_size = os.path.getsize(filepath) n = min(MINI_HASH_BLOCK, file_size) @@ -1578,6 +1619,11 @@ def mini_hash(filepath): ext_pairs = ext_pairs_str.split() if ext_pairs_str else [] +# Names the runtime config marked `enabled: false`. Emitted into the +# manifest entry as `"enabled": false` (omitted when default-true) so +# avocadoctl skips activation at refresh time. +ext_disabled = set(os.environ.get("AVOCADO_EXT_DISABLED", "").split()) + extensions = [] for pair in ext_pairs: parts = pair.split(":", 2) @@ -1588,8 +1634,7 @@ for pair in ext_pairs: if not os.path.isfile(img_file): print("WARNING: Extension image not found: " + img_file) continue - with open(img_file, "rb") as f: - sha256 = hashlib.sha256(f.read()).hexdigest() + sha256 = sha256_file(img_file) size = os.path.getsize(img_file) image_id = str(uuid.uuid5(namespace, sha256)) dest = os.path.join(images_dir, image_id + ext_suffix) @@ -1598,6 +1643,8 @@ for pair in ext_pairs: entry = dict(name=name, version=version, image_id=image_id, sha256=sha256) if image_type != "raw": entry["image_type"] = image_type + if name in ext_disabled: + entry["enabled"] = False if kos_amf: entry["size"] = size entry["mini_hash"] = mini_hash(img_file) @@ -1611,8 +1658,7 @@ for pair in ext_pairs: def add_image_entry(img_path, version, image_type): if not (img_path and os.path.isfile(img_path)): return None - with open(img_path, "rb") as f: - sha256 = hashlib.sha256(f.read()).hexdigest() + sha256 = sha256_file(img_path) size = os.path.getsize(img_path) image_id = str(uuid.uuid5(namespace, sha256)) suffix = ".kab" if image_type == "kab" else ".raw" @@ -2344,8 +2390,14 @@ namespace = uuid.UUID(os.environ["AVOCADO_NS_UUID"]) images_dir = os.environ["AVOCADO_IMAGES_DIR"] manifest_path = os.environ["AVOCADO_MANIFEST_PATH"] +# Streaming SHA256 — see HASH_CHUNK rationale in the manifest builder +# above. .aos bundles routinely exceed available RAM on builders. +HASH_CHUNK = 1024 * 1024 +aos_h = hashlib.sha256() with open(aos_path, "rb") as f: - aos_sha256 = hashlib.sha256(f.read()).hexdigest() + for chunk in iter(lambda: f.read(HASH_CHUNK), b""): + aos_h.update(chunk) +aos_sha256 = aos_h.hexdigest() aos_image_id = str(uuid.uuid5(namespace, aos_sha256)) dest = os.path.join(images_dir, aos_image_id + ".raw") shutil.copy2(aos_path, dest) diff --git a/src/commands/runtime/deps.rs b/src/commands/runtime/deps.rs index f31e838..53812b6 100644 --- a/src/commands/runtime/deps.rs +++ b/src/commands/runtime/deps.rs @@ -84,8 +84,10 @@ impl RuntimeDepsCommand { // New way: Read extensions from the `extensions` array if let Some(extensions) = runtime_spec.get("extensions").and_then(|e| e.as_sequence()) { for ext in extensions { - if let Some(ext_name) = ext.as_str() { - dependencies.push(self.resolve_extension_dependency(parsed, ext_name)); + if let Some(spec) = + crate::utils::runtime_extension::RuntimeExtensionSpec::parse_entry(ext) + { + dependencies.push(self.resolve_extension_dependency(parsed, &spec.name)); } } } diff --git a/src/utils/config.rs b/src/utils/config.rs index a64c2ec..c5bf169 100644 --- a/src/utils/config.rs +++ b/src/utils/config.rs @@ -3154,8 +3154,10 @@ impl Config { .and_then(|e| e.as_sequence()) { for ext in extensions { - if let Some(ext_name) = ext.as_str() { - ext_deps.push(RuntimeExtDep::Local(ext_name.to_string())); + if let Some(spec) = + crate::utils::runtime_extension::RuntimeExtensionSpec::parse_entry(ext) + { + ext_deps.push(RuntimeExtDep::Local(spec.name)); } } } @@ -3940,9 +3942,12 @@ impl Config { .with_context(|| format!("Failed to load composed config: {config_path_str}"))?; for ext_val in extensions { - let Some(ext_name) = ext_val.as_str() else { + let Some(spec) = + crate::utils::runtime_extension::RuntimeExtensionSpec::parse_entry(ext_val) + else { continue; }; + let ext_name = spec.name.as_str(); // Look up the extension's content in the composed merged // value. Handles template keys like @@ -4310,8 +4315,10 @@ impl Config { merged_value.get("extensions").and_then(|e| e.as_sequence()) { for ext in extensions { - if let Some(ext_name) = ext.as_str() { - if ext_name == extension_name { + if let Some(spec) = + crate::utils::runtime_extension::RuntimeExtensionSpec::parse_entry(ext) + { + if spec.name == extension_name { if let Some(ext_section) = parsed.get("extensions") { if let Some(ext_map) = ext_section.as_mapping() { for (ext_key, ext_config) in ext_map { @@ -5310,8 +5317,10 @@ pub fn find_active_extensions( merged_value.get("extensions").and_then(|e| e.as_sequence()) { for ext_val in extensions_list { - if let Some(ext_name) = ext_val.as_str() { - active_extensions.insert(ext_name.to_string()); + if let Some(spec) = + crate::utils::runtime_extension::RuntimeExtensionSpec::parse_entry(ext_val) + { + active_extensions.insert(spec.name); } } } diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 1759ea7..ebd176f 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -22,6 +22,7 @@ pub mod provision_result; pub mod remote; pub mod runs_on; pub mod runtime; +pub mod runtime_extension; pub mod scheduler; pub mod signing_keys; #[cfg(unix)] diff --git a/src/utils/runtime_extension.rs b/src/utils/runtime_extension.rs new file mode 100644 index 0000000..e4bea76 --- /dev/null +++ b/src/utils/runtime_extension.rs @@ -0,0 +1,154 @@ +//! Centralized parsing for runtime extension list entries. +//! +//! A `runtimes..extensions:` list accepts two shapes: +//! +//! 1. Plain string — just the extension name; all options take defaults. +//! +//! ```yaml +//! extensions: +//! - microclaw +//! - vm-agent +//! ``` +//! +//! 2. Single-key map — the key is the extension name, the value is a +//! sub-mapping carrying per-extension knobs: +//! +//! ```yaml +//! extensions: +//! - microclaw: { enabled: false } +//! - avocado-ext-experimental: +//! enabled: false +//! ``` +//! +//! Every iterator that walks `runtime.extensions` must funnel through +//! [`parse_entry`] so a missed call site can never silently drop the new +//! map form (`as_str()` returns `None` on a map → the extension +//! disappears from `ext_deps`, `active_extensions`, etc., with no error). +//! Centralizing the parse also gives one place to grow new per-extension +//! flags (`merge_index:`, `runs_on:`, …) without touching the iterators. + +use serde_yaml::Value; + +/// Parsed view of one entry in a runtime's `extensions:` list. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RuntimeExtensionSpec { + /// The extension name (possibly still containing `{{ avocado.target }}` + /// template placeholders — interpolation happens elsewhere). + pub name: String, + /// Whether this extension should be auto-activated when the runtime + /// is provisioned. `false` ships the image but requires + /// `avocadoctl ext enable ` to flip it on. + pub enabled: bool, +} + +impl RuntimeExtensionSpec { + /// Parse a single list entry. Returns `None` for malformed shapes + /// (multi-key map, empty list, non-string key, …) — the caller treats + /// `None` the same way it used to treat a non-string entry: skip it. + pub fn parse_entry(value: &Value) -> Option { + // Plain string: `- microclaw` + if let Some(s) = value.as_str() { + return Some(Self { + name: s.to_string(), + enabled: true, + }); + } + // Single-key map: `- microclaw: { enabled: false }` + let mapping = value.as_mapping()?; + if mapping.len() != 1 { + return None; + } + let (key, opts) = mapping.iter().next()?; + let name = key.as_str()?.to_string(); + let enabled = parse_enabled(opts).unwrap_or(true); + Some(Self { name, enabled }) + } +} + +/// Extract `enabled` from the options sub-mapping. A null value +/// (`microclaw:`) is treated as "no options provided" → defaults to +/// enabled. Anything other than a boolean is ignored. +fn parse_enabled(opts: &Value) -> Option { + if opts.is_null() { + return None; + } + opts.as_mapping()?.get("enabled").and_then(|v| v.as_bool()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn parse(yaml: &str) -> Vec { + let v: Value = serde_yaml::from_str(yaml).unwrap(); + v.as_sequence() + .unwrap() + .iter() + .filter_map(RuntimeExtensionSpec::parse_entry) + .collect() + } + + #[test] + fn plain_string_entry() { + let r = parse("- microclaw\n"); + assert_eq!(r.len(), 1); + assert_eq!(r[0].name, "microclaw"); + assert!(r[0].enabled); + } + + #[test] + fn map_with_enabled_false() { + let r = parse("- microclaw: { enabled: false }\n"); + assert_eq!(r[0].name, "microclaw"); + assert!(!r[0].enabled); + } + + #[test] + fn map_block_form() { + let r = parse("- microclaw:\n enabled: false\n"); + assert_eq!(r[0].name, "microclaw"); + assert!(!r[0].enabled); + } + + #[test] + fn map_without_options_value_is_enabled() { + // `- microclaw:` parses as `{microclaw: null}` — treat as default. + let r = parse("- microclaw:\n"); + assert_eq!(r[0].name, "microclaw"); + assert!(r[0].enabled); + } + + #[test] + fn templated_name_works_as_string_and_key() { + let r = parse("- \"avocado-bsp-{{ avocado.target }}\"\n"); + assert_eq!(r[0].name, "avocado-bsp-{{ avocado.target }}"); + let r = parse("- \"avocado-bsp-{{ avocado.target }}\": { enabled: false }\n"); + assert_eq!(r[0].name, "avocado-bsp-{{ avocado.target }}"); + assert!(!r[0].enabled); + } + + #[test] + fn mixed_entries() { + let r = parse( + "- a\n\ + - b: { enabled: false }\n\ + - c\n", + ); + assert_eq!(r.len(), 3); + assert_eq!(r[0].name, "a"); + assert!(r[0].enabled); + assert_eq!(r[1].name, "b"); + assert!(!r[1].enabled); + assert_eq!(r[2].name, "c"); + assert!(r[2].enabled); + } + + #[test] + fn multi_key_map_is_rejected() { + let v: Value = + serde_yaml::from_str("{ a: { enabled: false }, b: { enabled: true } }").unwrap(); + // Wrapped in a one-element list to feed parse_entry directly. + let entry: Value = serde_yaml::to_value(&v).unwrap(); + assert!(RuntimeExtensionSpec::parse_entry(&entry).is_none()); + } +} From 6ef0b6dadbd083576da03c1a46714870d9aac28d Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Wed, 20 May 2026 10:49:52 -0500 Subject: [PATCH 08/17] ext: emit AVOCADO_ON_MERGE=systemd-tmpfiles --create when tmpfiles.d present Mirror the existing sysusers.d / ld.so.conf.d detection. When an extension ships config under `usr/(local/)lib/tmpfiles.d/` or `etc/tmpfiles.d/`, the generated release file gets `AVOCADO_ON_MERGE="systemd-tmpfiles --create"` so the extension's tmpfiles take effect after merge without a reboot. --- src/commands/ext/build.rs | 41 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/commands/ext/build.rs b/src/commands/ext/build.rs index 5a11eb3..82a2e51 100644 --- a/src/commands/ext/build.rs +++ b/src/commands/ext/build.rs @@ -1024,6 +1024,15 @@ if ([ -d "$sysusers_dir1" ] && [ -n "$(find "$sysusers_dir1" -name "*.conf" 2>/d echo "[INFO] Found sysusers.d config files in extension '{}', added AVOCADO_ON_MERGE=\"systemd-sysusers\" to release file" fi +# Check if extension includes tmpfiles.d config files and add systemd-tmpfiles --create to AVOCADO_ON_MERGE if needed +tmpfiles_dir1="$AVOCADO_EXT_SYSROOTS/{}/usr/local/lib/tmpfiles.d" +tmpfiles_dir2="$AVOCADO_EXT_SYSROOTS/{}/usr/lib/tmpfiles.d" +if ([ -d "$tmpfiles_dir1" ] && [ -n "$(find "$tmpfiles_dir1" -name "*.conf" 2>/dev/null | head -n 1)" ]) || \ + ([ -d "$tmpfiles_dir2" ] && [ -n "$(find "$tmpfiles_dir2" -name "*.conf" 2>/dev/null | head -n 1)" ]); then + echo "AVOCADO_ON_MERGE=\"systemd-tmpfiles --create\"" >> "$release_file" + echo "[INFO] Found tmpfiles.d config files in extension '{}', added AVOCADO_ON_MERGE=\"systemd-tmpfiles --create\" to release file" +fi + # Add custom AVOCADO_ON_MERGE commands if specified {} @@ -1043,6 +1052,9 @@ fi self.extension, self.extension, self.extension, + self.extension, + self.extension, + self.extension, custom_on_merge_section, custom_on_unmerge_section ) @@ -1234,6 +1246,13 @@ if [ -d "$ldso_etc_dir" ] && [ -n "$(find "$ldso_etc_dir" -name "*.conf" 2>/dev/ echo "[INFO] Found ld.so.conf.d config files in extension '{}', added AVOCADO_ON_MERGE=\"ldconfig\" to release file" fi +# Check if extension includes tmpfiles.d config files and add systemd-tmpfiles --create to AVOCADO_ON_MERGE if needed +tmpfiles_etc_dir="$AVOCADO_EXT_SYSROOTS/{}/etc/tmpfiles.d" +if [ -d "$tmpfiles_etc_dir" ] && [ -n "$(find "$tmpfiles_etc_dir" -name "*.conf" 2>/dev/null | head -n 1)" ]; then + echo "AVOCADO_ON_MERGE=\"systemd-tmpfiles --create\"" >> "$release_file" + echo "[INFO] Found tmpfiles.d config files in extension '{}', added AVOCADO_ON_MERGE=\"systemd-tmpfiles --create\" to release file" +fi + # Add custom AVOCADO_ON_MERGE commands if specified {} @@ -1255,6 +1274,8 @@ fi self.extension, self.extension, self.extension, + self.extension, + self.extension, custom_on_merge_section, custom_on_unmerge_section, enable_services_section, @@ -2030,6 +2051,17 @@ mod tests { .contains("echo \"AVOCADO_ON_MERGE=\\\"systemd-sysusers\\\"\" >> \"$release_file\"")); assert!(script.contains("Found sysusers.d config files in extension 'test-ext'")); + // Check for tmpfiles.d functionality + assert!(script + .contains("tmpfiles_dir1=\"$AVOCADO_EXT_SYSROOTS/test-ext/usr/local/lib/tmpfiles.d\"")); + assert!( + script.contains("tmpfiles_dir2=\"$AVOCADO_EXT_SYSROOTS/test-ext/usr/lib/tmpfiles.d\"") + ); + assert!(script.contains( + "echo \"AVOCADO_ON_MERGE=\\\"systemd-tmpfiles --create\\\"\" >> \"$release_file\"" + )); + assert!(script.contains("Found tmpfiles.d config files in extension 'test-ext'")); + // Check for custom on_merge functionality (should be present but not activated) assert!(script.contains("# Add custom AVOCADO_ON_MERGE commands if specified")); // With no custom commands, no AVOCADO_ON_MERGE entries should be added for custom commands @@ -2086,6 +2118,15 @@ mod tests { assert!(script.contains("echo \"AVOCADO_ON_MERGE=\\\"ldconfig\\\"\" >> \"$release_file\"")); assert!(script.contains("Found ld.so.conf.d config files in extension 'test-ext'")); + // Check for tmpfiles.d functionality in confext + assert!( + script.contains("tmpfiles_etc_dir=\"$AVOCADO_EXT_SYSROOTS/test-ext/etc/tmpfiles.d\"") + ); + assert!(script.contains( + "echo \"AVOCADO_ON_MERGE=\\\"systemd-tmpfiles --create\\\"\" >> \"$release_file\"" + )); + assert!(script.contains("Found tmpfiles.d config files in extension 'test-ext'")); + // Check for custom on_merge functionality (should be present but not activated) assert!(script.contains("# Add custom AVOCADO_ON_MERGE commands if specified")); // With no custom commands, no AVOCADO_ON_MERGE entries should be added for custom commands From 75610cda7b8230be01fdb46247db601049f70e6d Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Wed, 20 May 2026 11:01:54 -0500 Subject: [PATCH 09/17] image-signing: stream large files through the hasher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `compute_file_hash` previously read the whole file into memory before feeding it to the hasher. Image artifacts can be tens of GB (`var.btrfs` alone is ~458 MB now and growing), so reading them fully into RAM is a footgun waiting to happen — and on a 16 GB Mac it can push the process into swap thrash before any sign of progress. Replace with a fixed-size 1 MiB-chunk streaming loop for both Sha256 and Blake3. No behavior change for callers; the resulting digest is identical. --- src/utils/image_signing.rs | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/src/utils/image_signing.rs b/src/utils/image_signing.rs index aa510ff..5b11a12 100644 --- a/src/utils/image_signing.rs +++ b/src/utils/image_signing.rs @@ -15,6 +15,7 @@ use ed25519_compact::{SecretKey, Signature}; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; use std::fs; +use std::io::Read; use std::path::Path; use std::str::FromStr; @@ -80,19 +81,44 @@ impl ChecksumAlgorithm { } } -/// Compute checksum of a file +/// Compute checksum of a file by streaming it through the hasher in +/// fixed-size chunks. Image files can be tens of GB, so reading them +/// fully into memory is not safe. #[allow(dead_code)] // Public API for future use pub fn compute_file_hash(file_path: &Path, algorithm: &ChecksumAlgorithm) -> Result> { - let data = fs::read(file_path) - .with_context(|| format!("Failed to read file for hashing: {}", file_path.display()))?; + const HASH_CHUNK: usize = 1024 * 1024; + + let mut file = fs::File::open(file_path) + .with_context(|| format!("Failed to open file for hashing: {}", file_path.display()))?; + let mut buf = vec![0u8; HASH_CHUNK]; Ok(match algorithm { ChecksumAlgorithm::Sha256 => { let mut hasher = Sha256::new(); - hasher.update(&data); + loop { + let n = file.read(&mut buf).with_context(|| { + format!("Failed to read file for hashing: {}", file_path.display()) + })?; + if n == 0 { + break; + } + hasher.update(&buf[..n]); + } hasher.finalize().to_vec() } - ChecksumAlgorithm::Blake3 => blake3::hash(&data).as_bytes().to_vec(), + ChecksumAlgorithm::Blake3 => { + let mut hasher = blake3::Hasher::new(); + loop { + let n = file.read(&mut buf).with_context(|| { + format!("Failed to read file for hashing: {}", file_path.display()) + })?; + if n == 0 { + break; + } + hasher.update(&buf[..n]); + } + hasher.finalize().as_bytes().to_vec() + } }) } From c8a017c380cdcd5616cbf11321d99d540a520575 Mon Sep 17 00:00:00 2001 From: nicksinas Date: Wed, 20 May 2026 11:38:57 -0500 Subject: [PATCH 10/17] docs link update --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 464151d..6d06e85 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ Command line interface for Avocado. -- [Documentation](https://docs.peridio.com/avocado-cli) +- [Documentation](https://docs.peridio.com/developer-reference/avocado-cli/overview) ## Install From 124b613ab4999b0409331b777c746e625e93dbeb Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Thu, 21 May 2026 06:44:17 -0500 Subject: [PATCH 11/17] runtime: parse map-form extension entries in build deps + stamps Two remaining call sites in build.rs (collect_extension_dependencies' nested loop and the per-extension dep-resolution loop) and the runtime input-hash compute in stamps.rs were still treating each entry as a bare string. After 3a7c528 added support for `extensions: - foo: {enabled: false}` map syntax, those paths silently skipped any map-form entry, so disabled extensions could still pull deps via the nested traversal and the input hash didn't account for them. --- src/commands/runtime/build.rs | 13 ++++++++++--- src/utils/stamps.rs | 5 ++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/commands/runtime/build.rs b/src/commands/runtime/build.rs index 1adc6c8..9de590e 100644 --- a/src/commands/runtime/build.rs +++ b/src/commands/runtime/build.rs @@ -2535,10 +2535,14 @@ sign_amf "$AVOCADO_MANIFEST_PATH" ext_config.get("extensions").and_then(|e| e.as_sequence()) { for nested_ext in nested_extensions { - if let Some(nested_ext_name) = nested_ext.as_str() { + if let Some(spec) = + crate::utils::runtime_extension::RuntimeExtensionSpec::parse_entry( + nested_ext, + ) + { self.collect_extension_dependencies( _config, - nested_ext_name, + &spec.name, all_extensions, visited, _target_arch, @@ -2586,7 +2590,10 @@ sign_amf "$AVOCADO_MANIFEST_PATH" if let Some(ext_seq) = ext_list { for ext in ext_seq { - if let Some(ext_name) = ext.as_str() { + if let Some(spec) = + crate::utils::runtime_extension::RuntimeExtensionSpec::parse_entry(ext) + { + let ext_name = spec.name.as_str(); let version = self .resolve_extension_version( parsed, diff --git a/src/utils/stamps.rs b/src/utils/stamps.rs index 4405ca6..f082875 100644 --- a/src/utils/stamps.rs +++ b/src/utils/stamps.rs @@ -1106,7 +1106,10 @@ pub fn compute_runtime_input_hash( .and_then(|e| e.as_sequence()) { for ext_val in ext_list { - if let Some(ext_name) = ext_val.as_str() { + if let Some(spec) = + crate::utils::runtime_extension::RuntimeExtensionSpec::parse_entry(ext_val) + { + let ext_name = spec.name.as_str(); if let Some(docker_images) = parsed .get("extensions") .and_then(|e| e.get(ext_name)) From 04e4e9d1578492a1be0b8786d2fbc84dc2b42d98 Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Thu, 21 May 2026 06:44:28 -0500 Subject: [PATCH 12/17] vm: CLI owns qemu lifecycle on macOS; IPC is best-effort notify only Previously the macOS path delegated `vm start` / `vm stop` to Avocado.app via a synchronous JSON-line client, which made the CLI unusable on its own and stalled `vm stop` whenever the app's main actor was busy. Now the CLI spawns and signals qemu directly on every platform; Avocado.app (when installed) adopts the pid via its existing pidfile reconciler. The remaining IPC (`vm.notify.starting` / `.running` / `.stopping` / `.stopped`) is pure dashboard hinting: 100 ms timeouts, silent no-op when the desktop isn't reachable, and self-heals via the reconciler within ~2 s if a notification is dropped. `Client::connect_or_launch` and `delegate_start_to_app` are gone along with their wait loops. Also adds a second virtio-serial port (`avocado.control` -> control.sock) so Avocado.app's USBHostBridge / ControlPlane can talk to avocado-vm-agent without waiting for the app to spawn qemu itself. --- src/utils/vm/client.rs | 167 ++++++++++++-------------------------- src/utils/vm/lifecycle.rs | 126 +++++++++++----------------- src/utils/vm/mod.rs | 4 +- src/utils/vm/qemu.rs | 22 +++-- src/utils/vm/state.rs | 10 +++ 5 files changed, 127 insertions(+), 202 deletions(-) diff --git a/src/utils/vm/client.rs b/src/utils/vm/client.rs index 7b9ece0..1f75ccd 100644 --- a/src/utils/vm/client.rs +++ b/src/utils/vm/client.rs @@ -1,134 +1,67 @@ -//! JSON-line client for talking to Avocado.app's IPC socket. +//! Fire-and-forget IPC notifications to Avocado.app. //! -//! On macOS, `avocado vm *` delegates VM lifecycle to a long-lived -//! `Avocado.app` (com.peridio.avocadodesktop) that owns the qemu process. -//! This module is the CLI side of that conversation: it connects to the -//! app's Unix socket, sends one-shot requests, and reads one-line responses. +//! The CLI is authoritative for the qemu lifecycle (spawn, signal, pidfile). +//! These notifications exist purely to keep the desktop app's dashboard in +//! sync — `.starting` / `.stopping` are visible the instant the CLI begins +//! the transition rather than waiting for the desktop's pidfile reconciler +//! to notice ~2 s later. //! -//! The app auto-launches if it isn't already running (Finder/Dock-style via -//! `open`). On Linux this whole module is unused. +//! Crucially this is **never load-bearing**: +//! - All timeouts are aggressive (100 ms). If the desktop is bogged down +//! rendering, the CLI moves on without blocking. +//! - Connect failure (app not running, app not installed, socket missing) +//! is a silent no-op. +//! - The desktop has a pidfile reconciler as a backstop, so a dropped +//! notification self-heals within ~2 s. //! -//! Protocol (mirrors `macos/AvocadoApp/IPCServer.swift`): +//! Wire format (mirrors `macos/AvocadoApp/IPCServer.swift`): //! Request: `{"method": "...", "params": {...}, "id": }\n` -//! Response: `{"result": {...}, "id": }\n` or -//! `{"error": "...", "id": }\n` +//! Response: `{"result": {...}, "id": }\n` //! -//! Module-level `#[cfg(target_os = "macos")]` lives on the `pub mod` -//! declaration in `super::mod.rs`; we don't repeat it here. +//! `#[cfg(target_os = "macos")]` lives on `pub mod client;` in `mod.rs`. -use anyhow::{bail, Context, Result}; use serde_json::{json, Value}; use std::io::{BufRead, BufReader, Write}; use std::os::unix::net::UnixStream; use std::path::PathBuf; -use std::time::{Duration, Instant}; +use std::time::Duration; -/// Connected JSON-line client. One round trip per `request()` call. -pub struct Client { - stream: UnixStream, -} - -impl Client { - /// Where Avocado.app advertises its IPC socket. - pub fn socket_path() -> Result { - let home = directories::BaseDirs::new() - .context("could not determine home directory")? - .home_dir() - .to_path_buf(); - Ok(home.join("Library/Application Support/Avocado/avocado.sock")) - } - - /// Connect once. Errors if Avocado.app isn't running. Use - /// [`Self::connect_or_launch`] for the user-facing path. - pub fn connect() -> Result { - let path = Self::socket_path()?; - let stream = - UnixStream::connect(&path).with_context(|| format!("connect to {}", path.display()))?; - // Snug, not heroic — these requests are tiny and the app responds - // synchronously. `vm.wait_ready` is the only long-runner and it sets - // its own per-request timeout via the `timeout_sec` param. - let _ = stream.set_read_timeout(Some(Duration::from_secs(120))); - let _ = stream.set_write_timeout(Some(Duration::from_secs(10))); - Ok(Self { stream }) - } +/// Aggressive cap on the total time any one notification can steal from +/// the CLI. Connect, write, and the brief response wait are each bounded +/// by this; the user-visible worst case is ~3× this when the socket +/// exists but the desktop's main actor is stuck. +const NOTIFY_TIMEOUT: Duration = Duration::from_millis(100); - /// Connect, auto-launching Avocado.app if its socket isn't there yet. - /// Polls for up to ~6 s after launching for the socket to appear. - pub fn connect_or_launch() -> Result { - if let Ok(c) = Self::connect() { - return Ok(c); - } - Self::launch_app().context("launching Avocado.app")?; - let deadline = Instant::now() + Duration::from_secs(6); - let mut last_err: Option = None; - while Instant::now() < deadline { - std::thread::sleep(Duration::from_millis(200)); - match Self::connect() { - Ok(c) => return Ok(c), - Err(e) => last_err = Some(e), - } - } - bail!( - "Avocado.app did not become reachable on {} within 6s ({})", - Self::socket_path()?.display(), - last_err.map(|e| format!("{e:#}")).unwrap_or_default() - ) - } +/// Where Avocado.app advertises its IPC socket. +fn socket_path() -> Option { + let home = directories::BaseDirs::new()?.home_dir().to_path_buf(); + Some(home.join("Library/Application Support/Avocado/avocado.sock")) +} - /// Locate Avocado.app on disk and launch it via `open -gja`. Search order: - /// 1. `$AVOCADO_APP_PATH` env override - /// 2. `/Applications/Avocado.app` - /// 3. dev build under `avocado-vm/macos/build/Build/Products/Debug/Avocado.app` - /// (relative to this crate's manifest dir) - fn launch_app() -> Result<()> { - let mut candidates: Vec = Vec::new(); - if let Ok(p) = std::env::var("AVOCADO_APP_PATH") { - candidates.push(PathBuf::from(p)); - } - candidates.push(PathBuf::from("/Applications/Avocado.app")); - if let Some(crate_parent) = PathBuf::from(env!("CARGO_MANIFEST_DIR")).parent() { - candidates - .push(crate_parent.join("avocado-vm/macos/build/Build/Products/Debug/Avocado.app")); - } - let app = candidates.iter().find(|p| p.exists()).cloned() - .with_context(|| format!( - "could not find Avocado.app (checked {candidates:?}). Build it with `make -C avocado-vm/macos build` or set AVOCADO_APP_PATH." - ))?; - // -g: don't bring to foreground. -j: hide. -a: target by bundle path. - let status = std::process::Command::new("open") - .args(["-gja"]) - .arg(&app) - .status() - .context("spawn `open`")?; - if !status.success() { - bail!("`open -gja {}` exited {status}", app.display()); - } - Ok(()) - } +/// Best-effort fire-and-forget notification to the desktop app. Never +/// blocks the CLI for more than ~3× [`NOTIFY_TIMEOUT`]; never returns an +/// error (failure modes are all logged-and-swallowed). Safe to call +/// regardless of whether the desktop is installed or reachable. +pub fn notify(method: &str, params: Value) { + let Some(path) = socket_path() else { return }; + let Ok(stream) = UnixStream::connect(&path) else { return }; + // Apply timeouts up-front; without these `write_all` and `read_line` + // can stall on a hung peer for far longer than we're willing to wait. + let _ = stream.set_write_timeout(Some(NOTIFY_TIMEOUT)); + let _ = stream.set_read_timeout(Some(NOTIFY_TIMEOUT)); - /// One JSON-line request → one JSON-line response. Returns the `result` - /// payload; surfaces server-side `error` strings as anyhow errors. - pub fn request(&mut self, method: &str, params: Value) -> Result { - let req = json!({ - "method": method, - "params": params, - "id": 1, - }); - let mut buf = serde_json::to_vec(&req).context("encode request")?; - buf.push(b'\n'); - self.stream.write_all(&buf).context("write request")?; + let req = json!({ "method": method, "params": params, "id": 1 }); + let Ok(mut buf) = serde_json::to_vec(&req) else { return }; + buf.push(b'\n'); - let mut reader = BufReader::new(&self.stream); - let mut line = String::new(); - let n = reader.read_line(&mut line).context("read response line")?; - if n == 0 { - bail!("app closed the connection without responding"); - } - let resp: Value = serde_json::from_str(line.trim()) - .with_context(|| format!("parse response: {line:?}"))?; - if let Some(err) = resp.get("error").and_then(|v| v.as_str()) { - bail!("Avocado.app error: {err}"); - } - Ok(resp.get("result").cloned().unwrap_or(Value::Null)) + let mut stream = stream; + if stream.write_all(&buf).is_err() { + return; } + // Drain the response if the desktop is quick enough. We don't care + // about the contents; this just prevents the desktop from logging a + // "peer closed mid-response" warning when we drop the stream below. + // If it times out, fine — the request was already written. + let mut line = String::new(); + let _ = BufReader::new(&stream).read_line(&mut line); } diff --git a/src/utils/vm/lifecycle.rs b/src/utils/vm/lifecycle.rs index fe5a5a8..553f7ca 100644 --- a/src/utils/vm/lifecycle.rs +++ b/src/utils/vm/lifecycle.rs @@ -21,6 +21,20 @@ use super::state::{self, VmPaths}; const SIGTERM: libc::c_int = 15; const SIGKILL: libc::c_int = 9; +/// Best-effort one-way notification to Avocado.app (macOS only). All call +/// sites are fire-and-forget: never blocks the CLI on the desktop's +/// responsiveness, silently no-ops when the desktop isn't running or +/// installed. The desktop has a pidfile reconciler as a backstop, so a +/// dropped notification self-heals within ~2 s. +fn notify_desktop(method: &str, params: serde_json::Value) { + #[cfg(target_os = "macos")] + super::client::notify(method, params); + #[cfg(not(target_os = "macos"))] + { + let _ = (method, params); + } +} + /// Knobs the `avocado vm start` command resolves from args + env. #[derive(Debug, Clone)] pub struct StartOptions { @@ -82,6 +96,11 @@ pub async fn start(opts: StartOptions) -> Result { state::cleanup_transient(&paths); } + // Announce intent so the desktop dashboard can flip to "Starting…" + // before the qemu pid even exists. Backstopped by the pidfile poller + // in case the notification doesn't get through. + notify_desktop("vm.notify.starting", serde_json::json!({})); + // Resolve manifest + verify artifacts let artifact_dir = opts.vm_source; if !artifact_dir.is_dir() { @@ -148,29 +167,20 @@ pub async fn start(opts: StartOptions) -> Result { workspace: workspace.clone(), }; - // On macOS, Avocado.app owns the QEMU process so its dashboard / - // USB bridge / virtio-serial control plane can observe lifecycle and - // wire up the rest of the supervisor stack. The CLI keeps its - // pre-flight (ssh keys, ssh config, var disk seeding) and post-boot - // wiring (workspace mount, docker forward); only the spawn step is - // delegated. Linux keeps the direct spawn. + // The CLI is authoritative for the qemu lifecycle on every platform. + // Avocado.app (when installed) observes via pidfile adoption rather + // than owning the process — that decoupling keeps the CLI usable on + // its own and avoids the IPC stall we used to hit when `vm stop` had + // to round-trip through the app. let pid = { - #[cfg(target_os = "macos")] - { - let _ = (&manifest, &cfg); // suppress unused-var warnings on this arm - delegate_start_to_app(&artifact_dir, opts.memory_mib, opts.cpus, ssh_port).await? - } - #[cfg(not(target_os = "macos"))] - { - let p = qemu::spawn_detached(&manifest, &paths, &cfg).await?; - // qemu's -pidfile flag will (re)write the pid; give it a moment, then - // also write our spawn-time pid as a fallback if the file doesn't appear. - tokio::time::sleep(Duration::from_millis(200)).await; - if !paths.pid_file().exists() { - let _ = std::fs::write(paths.pid_file(), p.to_string()); - } - p + let p = qemu::spawn_detached(&manifest, &paths, &cfg).await?; + // qemu's -pidfile flag will (re)write the pid; give it a moment, then + // also write our spawn-time pid as a fallback if the file doesn't appear. + tokio::time::sleep(Duration::from_millis(200)).await; + if !paths.pid_file().exists() { + let _ = std::fs::write(paths.pid_file(), p.to_string()); } + p }; // Wait for the guest to become ready — first signal wins (qga vs SSH). @@ -226,6 +236,11 @@ pub async fn start(opts: StartOptions) -> Result { ); } + notify_desktop( + "vm.notify.running", + serde_json::json!({ "pid": pid, "ssh_port": ssh_port }), + ); + Ok(VmStatus { running: true, pid: Some(pid), @@ -238,6 +253,15 @@ pub async fn start(opts: StartOptions) -> Result { /// Stop the VM gracefully. Sends QMP `quit`; falls back to SIGTERM/SIGKILL. pub async fn stop(force: bool) -> Result<()> { + let result = stop_inner(force).await; + // Announce regardless of how stop_inner exited (graceful, signal, or + // already-dead) — the desktop's pidfile reconciler reaches the same + // conclusion eventually, this just shortens the latency. + notify_desktop("vm.notify.stopped", serde_json::json!({})); + result +} + +async fn stop_inner(force: bool) -> Result<()> { let paths = VmPaths::resolve()?; // Always try to tear down the docker socket forward first — the SSH @@ -254,25 +278,10 @@ pub async fn stop(force: bool) -> Result<()> { } }; - // On macOS, ask Avocado.app to stop its supervised qemu so the - // dashboard + lifecycle observers see the transition. Falls through to - // the pidfile-signal path if the app isn't reachable. - #[cfg(target_os = "macos")] - { - if let Ok(mut client) = super::client::Client::connect() { - if client.request("vm.stop", serde_json::json!({})).is_ok() { - // Wait for the app's monitored process to exit. - for _ in 0..40 { - if !state::pid_alive(pid) { - state::cleanup_transient(&paths); - return Ok(()); - } - tokio::time::sleep(Duration::from_millis(250)).await; - } - // Fall through to forceful path below. - } - } - } + // Now that we know there's actually a live VM to take down, flip the + // dashboard to "Stopping…" right away rather than wait for the + // reconciler to notice the pid is gone. + notify_desktop("vm.notify.stopping", serde_json::json!({ "pid": pid })); // Try QMP quit first if the socket is around. #[cfg(unix)] @@ -665,40 +674,3 @@ fn write_ssh_config(paths: &VmPaths, ssh_port: u16) -> Result<()> { Ok(()) } -/// Ask Avocado.app to spawn qemu and wait for ready. Auto-launches the app -/// if it isn't running yet. Returns the qemu pid the app reports. -/// -/// Pre-flight (ssh keys, ssh-config, var.btrfs seeding, manifest sha256 -/// verify) has already happened by the time this is called — both sides -/// trust ~/.avocado/vm/ for the supporting state. -#[cfg(target_os = "macos")] -async fn delegate_start_to_app( - artifact_dir: &Path, - memory_mib: u32, - cpus: u32, - ssh_port: u16, -) -> Result { - let mut client = super::client::Client::connect_or_launch() - .context("failed to reach Avocado.app for vm.start")?; - - let params = serde_json::json!({ - "vm_dir": artifact_dir.display().to_string(), - "memory_mib": memory_mib, - "cpus": cpus, - "ssh_port": ssh_port, - }); - let _ = client - .request("vm.start", params) - .context("vm.start dispatch")?; - - // Block until the app reports running (or errors). The app's own - // monitor + pidfile sync gives us the authoritative pid. - let ready = client - .request("vm.wait_ready", serde_json::json!({ "timeout_sec": 120.0 })) - .context("vm.wait_ready")?; - let pid = ready - .get("pid") - .and_then(|v| v.as_i64()) - .ok_or_else(|| anyhow::anyhow!("vm.wait_ready returned no pid: {ready}"))?; - Ok(pid as u32) -} diff --git a/src/utils/vm/mod.rs b/src/utils/vm/mod.rs index 34ff0cd..261e353 100644 --- a/src/utils/vm/mod.rs +++ b/src/utils/vm/mod.rs @@ -2,7 +2,9 @@ //! //! On macOS dev hosts (and eventually Windows), avocado-cli runs a single //! user-level QEMU guest that hosts its own `dockerd` and exposes a 9p -//! source mount. USB passthrough is handled out-of-band by Avocado.app's +//! source mount. The CLI owns the qemu process directly (spawn, signal, +//! pidfile); Avocado.app, when installed, observes by adopting whatever +//! pidfile shows. USB passthrough is handled out-of-band by Avocado.app's //! IOUSBHost → USB/IP → vhci_hcd bridge (see avocado-vm/macos/). This //! module manages the VM's lifecycle and the control channels avocado-cli //! uses to drive it (QMP for QEMU-level ops, qga for guest-side ops diff --git a/src/utils/vm/qemu.rs b/src/utils/vm/qemu.rs index 67931fd..7e0910a 100644 --- a/src/utils/vm/qemu.rs +++ b/src/utils/vm/qemu.rs @@ -4,13 +4,6 @@ //! [`state::VmPaths`], spawns it daemonized (via `setsid` + redirected //! stdio), records the pid, and surfaces a [`QemuHandle`] the lifecycle //! layer can stop later. -//! -//! On macOS the spawn step is delegated to Avocado.app (see -//! `utils/vm/client.rs` + `lifecycle::delegate_start_to_app`), so this -//! module's spawn helpers are unused there — the cfg_attr below silences -//! the dead-code lint specifically for that platform without hiding -//! actual bugs on Linux. -#![cfg_attr(target_os = "macos", allow(dead_code))] use anyhow::{bail, Context, Result}; use std::path::PathBuf; @@ -205,6 +198,21 @@ pub fn build_qemu_args( args.push("-device".into()); args.push("virtserialport,chardev=qga,name=org.qemu.guest_agent.0".into()); + // Avocado control plane — second virtserialport on the same + // virtio-serial bus. The guest side opens + // `/dev/virtio-ports/avocado.control`; the host side gets a Unix + // socket at `paths.control_socket()` that Avocado.app connects to + // from `USBHostBridge`/`ControlPlane`. Without this, the desktop + // app's "Attach to VM" flow stalls on "agent hasn't responded" + // (the helper waits forever for `control.sock` to materialize). + args.push("-chardev".into()); + args.push(format!( + "socket,id=avocadoctl,path={},server=on,wait=off", + paths.control_socket().display() + )); + args.push("-device".into()); + args.push("virtserialport,chardev=avocadoctl,name=avocado.control".into()); + // Serial console -> logfile args.push("-serial".into()); args.push(format!("file:{}", paths.serial_log().display())); diff --git a/src/utils/vm/state.rs b/src/utils/vm/state.rs index ab34475..bcab64f 100644 --- a/src/utils/vm/state.rs +++ b/src/utils/vm/state.rs @@ -128,6 +128,15 @@ impl VmPaths { pub fn qga_socket(&self) -> PathBuf { self.root.join("qga.sock") } + /// Unix socket for the avocado control plane — a second virtio-serial + /// port exposed inside the guest as `/dev/virtio-ports/avocado.control`. + /// Avocado.app (USBHostBridge / ControlPlane) connects to this from + /// the host side; the in-guest avocado-vm-agent opens the matching + /// guest-side device for messages like `device_available` / + /// `device_gone` / `request_twiddle`. + pub fn control_socket(&self) -> PathBuf { + self.root.join("control.sock") + } pub fn serial_log(&self) -> PathBuf { self.root.join("serial.log") } @@ -234,6 +243,7 @@ pub fn cleanup_transient(paths: &VmPaths) { for p in [ paths.qmp_socket(), paths.qga_socket(), + paths.control_socket(), paths.pid_file(), paths.ssh_port_file(), paths.lock_file(), From ad94379d39a1bd0e5b9b3ab1192a7fbd789dd54c Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Thu, 21 May 2026 10:01:55 -0500 Subject: [PATCH 13/17] config: add --detail to surface extensions + runtime cross-refs avocado config show --detail emits a `detail` block alongside the existing narrow projection: per-runtime extension references (with defined/enabled flags and node_paths for cross-ref navigation), per-extension types/packages/services/used_by_runtimes, and an SDK image+packages summary. Default output is byte-stable so the desktop app's project-list scan keeps working unchanged; the new payload only appears when --detail is passed. Six unit tests cover map/list package shapes, broken runtime->extension refs, used_by inversion, and empty configs. The desktop app consumes this to render a config tree per project without having to parse avocado.yaml itself. --- src/commands/config_show.rs | 598 +++++++++++++++++++++++++++++++----- src/main.rs | 13 +- 2 files changed, 535 insertions(+), 76 deletions(-) diff --git a/src/commands/config_show.rs b/src/commands/config_show.rs index feb2e9d..9b44eea 100644 --- a/src/commands/config_show.rs +++ b/src/commands/config_show.rs @@ -4,16 +4,22 @@ //! Exists primarily so the desktop app doesn't have to parse YAML //! itself. Returns ONLY the fields the UI consumes today; growing this //! schema is a deliberate, additive operation. +//! +//! `--detail` opts into a richer `detail` sub-object with nested +//! extensions / packages / SDK info and runtime↔extension cross-refs. +//! The default (non-`--detail`) output is byte-stable. use anyhow::{Context, Result}; use serde_json::json; -use crate::utils::config::{load_config, SupportedTargets}; +use crate::utils::config::{load_config, Config, SupportedTargets}; use crate::utils::output_format::{emit_json_object, OutputFormat}; +use crate::utils::runtime_extension::RuntimeExtensionSpec; pub struct ConfigShowCommand { pub config_path: String, pub output: OutputFormat, + pub detail: bool, } impl ConfigShowCommand { @@ -21,81 +27,25 @@ impl ConfigShowCommand { let cfg = load_config(&self.config_path) .with_context(|| format!("Failed to load config at '{}'", self.config_path))?; - // Stable, narrow projection. The full Config struct contains - // many internal fields the UI doesn't need; baking them all - // in would couple the wire format to the CLI's internals. - let distro = cfg.distro.as_ref().map(|d| { - json!({ - "release": d.release, - "channel": d.channel, - }) - }); - - let supported_targets = match &cfg.supported_targets { - Some(SupportedTargets::All(s)) if s == "*" => json!("*"), - Some(SupportedTargets::List(list)) => json!(list), - Some(SupportedTargets::All(other)) => json!(other), - None => serde_json::Value::Null, - }; - - let runtimes: Vec<_> = cfg - .runtimes - .as_ref() - .map(|m| { - let mut list: Vec<_> = m - .iter() - .map(|(name, r)| { - json!({ - "name": name, - "target": r.target, - "target_board": r.target_board, - "version": r.version, - }) - }) - .collect(); - // Stable ordering so the UI doesn't dance around. - list.sort_by(|a, b| { - a["name"] - .as_str() - .unwrap_or("") - .cmp(b["name"].as_str().unwrap_or("")) - }); - list - }) - .unwrap_or_default(); - - let provision_profiles: Vec = cfg - .provision_profiles - .as_ref() - .map(|m| { - let mut names: Vec = m.keys().cloned().collect(); - names.sort(); - names - }) - .unwrap_or_default(); + let mut payload = build_base_payload(&self.config_path, &cfg); - // Connect mirror info — the desktop "Delete project" flow uses - // this to decide whether to offer "also delete from Avocado - // Connect" and what IDs to pass to `connect projects delete`. - let connect = cfg.connect.as_ref().map(|c| { - json!({ - "org": c.org, - "project": c.project, - }) - }); - - let payload = json!({ - "config_path": self.config_path, - "distro": distro, - "default_target": cfg.default_target, - "default_target_board": cfg.default_target_board, - "default_runtime": cfg.default_runtime, - "supported_targets": supported_targets, - "src_dir": cfg.src_dir, - "runtimes": runtimes, - "provision_profiles": provision_profiles, - "connect": connect, - }); + if self.detail { + // `load_composed` resolves remote/external extension configs + // into one merged YAML view, so extension definitions + // declared outside the main file still show up here. + let composed = Config::load_composed(&self.config_path, cfg.default_target.as_deref()) + .with_context(|| { + format!( + "Failed to load composed config at '{}' for --detail", + self.config_path + ) + })?; + let detail = build_detail(&composed.merged_value); + payload + .as_object_mut() + .expect("base payload is an object") + .insert("detail".to_string(), detail); + } if self.output.is_json() { emit_json_object(&payload); @@ -109,6 +59,272 @@ impl ConfigShowCommand { } } +/// The original, narrow projection. Kept byte-identical to the +/// pre-`--detail` output so existing consumers don't have to special-case. +fn build_base_payload(config_path: &str, cfg: &Config) -> serde_json::Value { + let distro = cfg.distro.as_ref().map(|d| { + json!({ + "release": d.release, + "channel": d.channel, + }) + }); + + let supported_targets = match &cfg.supported_targets { + Some(SupportedTargets::All(s)) if s == "*" => json!("*"), + Some(SupportedTargets::List(list)) => json!(list), + Some(SupportedTargets::All(other)) => json!(other), + None => serde_json::Value::Null, + }; + + let runtimes: Vec<_> = cfg + .runtimes + .as_ref() + .map(|m| { + let mut list: Vec<_> = m + .iter() + .map(|(name, r)| { + json!({ + "name": name, + "target": r.target, + "target_board": r.target_board, + "version": r.version, + }) + }) + .collect(); + // Stable ordering so the UI doesn't dance around. + list.sort_by(|a, b| { + a["name"] + .as_str() + .unwrap_or("") + .cmp(b["name"].as_str().unwrap_or("")) + }); + list + }) + .unwrap_or_default(); + + let provision_profiles: Vec = cfg + .provision_profiles + .as_ref() + .map(|m| { + let mut names: Vec = m.keys().cloned().collect(); + names.sort(); + names + }) + .unwrap_or_default(); + + // Connect mirror info — the desktop "Delete project" flow uses + // this to decide whether to offer "also delete from Avocado + // Connect" and what IDs to pass to `connect projects delete`. + let connect = cfg.connect.as_ref().map(|c| { + json!({ + "org": c.org, + "project": c.project, + }) + }); + + json!({ + "config_path": config_path, + "distro": distro, + "default_target": cfg.default_target, + "default_target_board": cfg.default_target_board, + "default_runtime": cfg.default_runtime, + "supported_targets": supported_targets, + "src_dir": cfg.src_dir, + "runtimes": runtimes, + "provision_profiles": provision_profiles, + "connect": connect, + }) +} + +/// Build the `detail` sub-object from the composed (post-include, +/// post-interpolation) YAML view. The UI consumes this to render the +/// per-project config tree: runtimes → extensions → packages. +/// +/// Shape: +/// ```jsonc +/// { +/// "runtimes": [{ name, node_path, extensions:[{name,enabled,defined,node_path}], packages:[…] }], +/// "extensions": [{ name, node_path, types:[…], packages:[…], enable_services:[…], used_by_runtimes:[…] }], +/// "sdk": { "image": "…", "packages": […] } // null when no sdk block +/// } +/// ``` +fn build_detail(merged: &serde_yaml::Value) -> serde_json::Value { + let extension_names: std::collections::HashSet = merged + .get("extensions") + .and_then(|e| e.as_mapping()) + .map(|m| { + m.keys() + .filter_map(|k| k.as_str().map(String::from)) + .collect() + }) + .unwrap_or_default(); + + // Walk runtimes first so we can compute `used_by_runtimes` for each + // extension in a single pass. + let mut runtimes_out: Vec = Vec::new(); + // extension name → sorted set of runtime names that reference it. + let mut used_by: std::collections::BTreeMap> = + std::collections::BTreeMap::new(); + + if let Some(rt_map) = merged.get("runtimes").and_then(|r| r.as_mapping()) { + let mut entries: Vec<(&str, &serde_yaml::Value)> = rt_map + .iter() + .filter_map(|(k, v)| k.as_str().map(|n| (n, v))) + .collect(); + entries.sort_by_key(|(n, _)| *n); + + for (rt_name, rt_value) in entries { + let exts = extract_runtime_extensions(rt_value, &extension_names); + for ext in &exts { + if let Some(name) = ext["name"].as_str() { + if ext["defined"].as_bool().unwrap_or(false) { + used_by + .entry(name.to_string()) + .or_default() + .insert(rt_name.to_string()); + } + } + } + + let packages = extract_package_names(rt_value.get("packages")); + + runtimes_out.push(json!({ + "name": rt_name, + "node_path": format!("/runtimes/{rt_name}"), + "extensions": exts, + "packages": packages, + })); + } + } + + // Now walk extensions, decorating each with the runtimes that pull it in. + let mut extensions_out: Vec = Vec::new(); + if let Some(ext_map) = merged.get("extensions").and_then(|e| e.as_mapping()) { + let mut entries: Vec<(&str, &serde_yaml::Value)> = ext_map + .iter() + .filter_map(|(k, v)| k.as_str().map(|n| (n, v))) + .collect(); + entries.sort_by_key(|(n, _)| *n); + + for (ext_name, ext_value) in entries { + let types = extract_string_list(ext_value.get("types")); + let packages = extract_package_names(ext_value.get("packages")); + let enable_services = extract_string_list(ext_value.get("enable_services")); + let used: Vec = used_by + .get(ext_name) + .map(|s| s.iter().cloned().collect()) + .unwrap_or_default(); + + extensions_out.push(json!({ + "name": ext_name, + "node_path": format!("/extensions/{ext_name}"), + "types": types, + "packages": packages, + "enable_services": enable_services, + "used_by_runtimes": used, + })); + } + } + + let sdk = merged.get("sdk").and_then(|s| s.as_mapping()).map(|m| { + let image = m + .get(serde_yaml::Value::String("image".to_string())) + .and_then(|v| v.as_str()) + .map(String::from); + let packages = extract_package_names( + m.get(serde_yaml::Value::String("packages".to_string())) + // `dependencies` is the deprecated alias the typed + // SdkConfig also accepts. + .or_else(|| m.get(serde_yaml::Value::String("dependencies".to_string()))), + ); + json!({ + "image": image, + "packages": packages, + }) + }); + + json!({ + "runtimes": runtimes_out, + "extensions": extensions_out, + "sdk": sdk, + }) +} + +/// Extract a runtime's `extensions:` list as structured entries with +/// resolution flags. Each entry: +/// - `name` — the extension reference name (post-interpolation). +/// - `enabled` — auto-activate when the runtime is provisioned (the +/// `- foo: { enabled: false }` map form sets this to false). +/// - `defined` — true iff the name resolves to a top-level +/// `extensions.` entry. False flags broken references that +/// the UI should mark with a warning. +/// - `node_path` — pointer to the extension definition, or null when +/// undefined. +fn extract_runtime_extensions( + rt_value: &serde_yaml::Value, + extension_names: &std::collections::HashSet, +) -> Vec { + let Some(list) = rt_value.get("extensions").and_then(|e| e.as_sequence()) else { + return Vec::new(); + }; + list.iter() + .filter_map(RuntimeExtensionSpec::parse_entry) + .map(|spec| { + let defined = extension_names.contains(&spec.name); + let node_path = if defined { + Some(format!("/extensions/{}", spec.name)) + } else { + None + }; + json!({ + "name": spec.name, + "enabled": spec.enabled, + "defined": defined, + "node_path": node_path, + }) + }) + .collect() +} + +/// Extract package names from either grammar: +/// - Map form: `packages: { nginx: '*', curl: '1.0' }` → keys. +/// - List form: `packages: [nginx, curl]` → items. +/// - Anything else (missing, scalar, …) → empty. +/// +/// Version specs are intentionally dropped — the project-detail tree +/// shows what's in the runtime, not at which version. Add a separate +/// detailed projection later if a use-case appears. +fn extract_package_names(value: Option<&serde_yaml::Value>) -> Vec { + let Some(value) = value else { + return Vec::new(); + }; + let mut names: Vec = if let Some(map) = value.as_mapping() { + map.keys() + .filter_map(|k| k.as_str().map(String::from)) + .collect() + } else if let Some(list) = value.as_sequence() { + list.iter() + .filter_map(|v| v.as_str().map(String::from)) + .collect() + } else { + Vec::new() + }; + names.sort(); + names.dedup(); + names +} + +/// Extract a sequence of strings from a YAML field, tolerating +/// missing/non-sequence values (return empty). +fn extract_string_list(value: Option<&serde_yaml::Value>) -> Vec { + let Some(seq) = value.and_then(|v| v.as_sequence()) else { + return Vec::new(); + }; + seq.iter() + .filter_map(|v| v.as_str().map(String::from)) + .collect() +} + fn print_human_summary(payload: &serde_json::Value) { println!("Config: {}", payload["config_path"].as_str().unwrap_or("?")); if let Some(distro) = payload["distro"].as_object() { @@ -166,4 +382,236 @@ fn print_human_summary(payload: &serde_json::Value) { ); } } + if let Some(detail) = payload.get("detail") { + print_human_detail(detail); + } +} + +fn print_human_detail(detail: &serde_json::Value) { + if let Some(rs) = detail.get("runtimes").and_then(|v| v.as_array()) { + if !rs.is_empty() { + println!(" detail.runtimes:"); + for r in rs { + let name = r["name"].as_str().unwrap_or("?"); + println!(" - {name}:"); + if let Some(exts) = r.get("extensions").and_then(|v| v.as_array()) { + if !exts.is_empty() { + let parts: Vec = exts + .iter() + .map(|e| { + let n = e["name"].as_str().unwrap_or("?"); + let mark = if e["defined"].as_bool().unwrap_or(false) { + "" + } else { + "!" + }; + format!("{n}{mark}") + }) + .collect(); + println!(" extensions: [{}]", parts.join(", ")); + } + } + if let Some(pkgs) = r.get("packages").and_then(|v| v.as_array()) { + if !pkgs.is_empty() { + let names: Vec = pkgs + .iter() + .filter_map(|v| v.as_str().map(String::from)) + .collect(); + println!(" packages: [{}]", names.join(", ")); + } + } + } + } + } + if let Some(es) = detail.get("extensions").and_then(|v| v.as_array()) { + if !es.is_empty() { + println!(" detail.extensions:"); + for e in es { + let name = e["name"].as_str().unwrap_or("?"); + let used: Vec = e + .get("used_by_runtimes") + .and_then(|v| v.as_array()) + .map(|a| { + a.iter() + .filter_map(|v| v.as_str().map(String::from)) + .collect() + }) + .unwrap_or_default(); + let pkg_count = e + .get("packages") + .and_then(|v| v.as_array()) + .map(|a| a.len()) + .unwrap_or(0); + let used_s = if used.is_empty() { + "(unused)".to_string() + } else { + format!("used by [{}]", used.join(", ")) + }; + println!(" - {name}: {pkg_count} packages, {used_s}"); + } + } + } + if let Some(sdk) = detail.get("sdk").and_then(|v| v.as_object()) { + let image = sdk.get("image").and_then(|v| v.as_str()).unwrap_or("?"); + let pkg_count = sdk + .get("packages") + .and_then(|v| v.as_array()) + .map(|a| a.len()) + .unwrap_or(0); + println!(" detail.sdk: image={image}, {pkg_count} packages"); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_yaml::Value; + + fn yaml(s: &str) -> Value { + serde_yaml::from_str(s).unwrap() + } + + #[test] + fn extracts_package_names_from_map_form() { + let v = yaml("nginx: '*'\ncurl: '1.0'\n"); + let names = extract_package_names(Some(&v)); + assert_eq!(names, vec!["curl".to_string(), "nginx".to_string()]); + } + + #[test] + fn extracts_package_names_from_list_form() { + let v = yaml("- nginx\n- curl\n"); + let names = extract_package_names(Some(&v)); + assert_eq!(names, vec!["curl".to_string(), "nginx".to_string()]); + } + + #[test] + fn package_names_missing_returns_empty() { + assert!(extract_package_names(None).is_empty()); + } + + #[test] + fn detail_links_runtimes_to_extensions_and_flags_broken_refs() { + let merged = yaml( + r#" +runtimes: + dev: + extensions: + - real-ext + - missing-ext + - disabled-ext: { enabled: false } + packages: + avocado-runtime: '*' +extensions: + real-ext: + types: [sysext] + packages: + nginx: '*' + curl: '*' + disabled-ext: + types: [confext] + enable_services: + - foo.service +sdk: + image: docker.io/x/y:1 + packages: + nativesdk-foo: '*' +"#, + ); + + let detail = build_detail(&merged); + + let dev = &detail["runtimes"][0]; + assert_eq!(dev["name"], "dev"); + assert_eq!(dev["node_path"], "/runtimes/dev"); + + let exts = dev["extensions"].as_array().unwrap(); + assert_eq!(exts.len(), 3); + + let real = exts.iter().find(|e| e["name"] == "real-ext").unwrap(); + assert_eq!(real["defined"], true); + assert_eq!(real["enabled"], true); + assert_eq!(real["node_path"], "/extensions/real-ext"); + + let missing = exts.iter().find(|e| e["name"] == "missing-ext").unwrap(); + assert_eq!(missing["defined"], false); + assert!(missing["node_path"].is_null()); + + let disabled = exts.iter().find(|e| e["name"] == "disabled-ext").unwrap(); + assert_eq!(disabled["enabled"], false); + assert_eq!(disabled["defined"], true); + + // Runtime packages list (just names, no versions). + let pkgs: Vec<&str> = dev["packages"] + .as_array() + .unwrap() + .iter() + .filter_map(|v| v.as_str()) + .collect(); + assert_eq!(pkgs, vec!["avocado-runtime"]); + + // Extensions sorted alphabetically; only defined ones appear here + // (missing-ext was a runtime ref, not a definition). + let ext_list = detail["extensions"].as_array().unwrap(); + assert_eq!(ext_list.len(), 2); + assert_eq!(ext_list[0]["name"], "disabled-ext"); + assert_eq!(ext_list[1]["name"], "real-ext"); + + // used_by_runtimes is computed by inversion from runtime extension lists. + assert_eq!(ext_list[1]["used_by_runtimes"], json!(["dev"])); + let real_pkgs: Vec<&str> = ext_list[1]["packages"] + .as_array() + .unwrap() + .iter() + .filter_map(|v| v.as_str()) + .collect(); + assert_eq!(real_pkgs, vec!["curl", "nginx"]); + + // enable_services round-trips. + let services: Vec<&str> = ext_list[0]["enable_services"] + .as_array() + .unwrap() + .iter() + .filter_map(|v| v.as_str()) + .collect(); + assert_eq!(services, vec!["foo.service"]); + + // SDK summary present and stripped to image + package names. + assert_eq!(detail["sdk"]["image"], "docker.io/x/y:1"); + let sdk_pkgs: Vec<&str> = detail["sdk"]["packages"] + .as_array() + .unwrap() + .iter() + .filter_map(|v| v.as_str()) + .collect(); + assert_eq!(sdk_pkgs, vec!["nativesdk-foo"]); + } + + #[test] + fn detail_handles_empty_config() { + let merged = yaml("default_target: qemux86-64\n"); + let detail = build_detail(&merged); + assert!(detail["runtimes"].as_array().unwrap().is_empty()); + assert!(detail["extensions"].as_array().unwrap().is_empty()); + assert!(detail["sdk"].is_null()); + } + + #[test] + fn extension_unused_by_any_runtime() { + let merged = yaml( + r#" +runtimes: + dev: + extensions: [] +extensions: + orphan: + types: [sysext] +"#, + ); + let detail = build_detail(&merged); + let exts = detail["extensions"].as_array().unwrap(); + assert_eq!(exts.len(), 1); + assert_eq!(exts[0]["name"], "orphan"); + assert!(exts[0]["used_by_runtimes"].as_array().unwrap().is_empty()); + } } diff --git a/src/main.rs b/src/main.rs index ee79606..f9ed288 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1206,6 +1206,12 @@ enum ConfigCommands { /// Output format #[arg(long, value_enum, default_value_t = OutputFormat::Human)] output: OutputFormat, + /// Include nested detail (extensions, packages, SDK summary, + /// runtime↔extension cross-references) under a `detail` key. + /// Default output is unchanged when this flag is absent so + /// existing consumers keep working byte-for-byte. + #[arg(long)] + detail: bool, }, } @@ -2920,10 +2926,15 @@ async fn main() -> Result<()> { } }, Commands::Config { command } => match command { - ConfigCommands::Show { config, output } => { + ConfigCommands::Show { + config, + output, + detail, + } => { let cmd = ConfigShowCommand { config_path: config, output, + detail, }; cmd.execute().await?; Ok(()) From d20e1dcabad59aca63a1551f286281e378534a65 Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Thu, 21 May 2026 14:28:08 -0500 Subject: [PATCH 14/17] clean: use save_replacing on --unlock so cleared entries stick The previous code called `lock_file.save()` after clearing entries, which merges with the on-disk state and re-adds them. The desktop app's Unlock button hit this and silently no-op'd. Add a regression test. --- src/commands/clean.rs | 67 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 65 insertions(+), 2 deletions(-) diff --git a/src/commands/clean.rs b/src/commands/clean.rs index 97f0ebc..2fef24f 100644 --- a/src/commands/clean.rs +++ b/src/commands/clean.rs @@ -185,9 +185,12 @@ impl CleanCommand { } lock_file.clear_all(&target); - // Save updated lock file + // Save updated lock file. `save_replacing` writes verbatim + // without merging from disk — required for unlock semantics + // since the regular `save` would re-add the cleared entries + // through merge. lock_file - .save(&src_dir) + .save_replacing(&src_dir) .with_context(|| "Failed to save lock file")?; print_success( @@ -313,6 +316,7 @@ fi #[cfg(test)] mod tests { use super::*; + use crate::utils::lockfile::SysrootType; use std::fs; use tempfile::TempDir; @@ -381,6 +385,65 @@ mod tests { assert!(result.is_ok()); } + #[tokio::test] + async fn test_clean_unlock_actually_clears_entries() { + // Regression: `clean --unlock` must use `save_replacing` so + // cleared entries don't get merged back from disk. Previously + // it called `save`, which merged with on-disk state and made + // unlock a no-op — the desktop app's Unlock button hit this. + let temp_dir = TempDir::new().unwrap(); + let temp_path = temp_dir.path(); + + let config_content = r#" +default_target: "qemux86-64" +sdk: + image: "test-image" +"#; + let config_path = temp_path.join("avocado.yaml"); + fs::write(&config_path, config_content).unwrap(); + + let mut lock = LockFile::new(); + lock.set_locked_version( + "qemux86-64", + &SysrootType::Sdk("x86_64".to_string()), + "avocado-sdk-bootstrap", + "2024.0.129-r0.0", + ); + lock.save(temp_path).unwrap(); + + assert!(lock + .get_locked_version( + "qemux86-64", + &SysrootType::Sdk("x86_64".to_string()), + "avocado-sdk-bootstrap" + ) + .is_some()); + + let clean_cmd = CleanCommand::new( + Some(temp_path.to_str().unwrap().to_string()), + false, + None, + false, + ) + .with_config_path(Some(config_path.to_string_lossy().to_string())) + .with_target(Some("qemux86-64".to_string())) + .with_unlock(true); + let result = clean_cmd.execute().await; + assert!(result.is_ok(), "clean --unlock failed: {result:?}"); + + let reloaded = LockFile::load(temp_path).unwrap(); + assert!( + reloaded + .get_locked_version( + "qemux86-64", + &SysrootType::Sdk("x86_64".to_string()), + "avocado-sdk-bootstrap" + ) + .is_none(), + "lock entry should be cleared after `clean --unlock`" + ); + } + #[tokio::test] async fn test_clean_skip_volumes_keeps_state_file() { // `--skip-volumes` means "leave both the volume and its From c71bd83a57bdd9b5d3db879f134c915e12431ea7 Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Thu, 21 May 2026 14:31:09 -0500 Subject: [PATCH 15/17] vm: persist cpu/memory overrides to ~/.avocado/vm/config.yaml `avocado vm start --memory-mib` / `--cpus` now make the flags optional, resolve to `runtime.{cpus,memory_mib}` in the VM config (also written by Avocado.app's settings UI), and fall back to DEFAULT_CPUS / DEFAULT_MEMORY_MIB. When the user passes a flag, the value is written back to the config so the next flag-less `vm start` (and the desktop app) converge on the same value. `vm reset` / `vm update` / route-on-demand callers pass `None` so they pick up the persisted settings instead of hardcoded 4096 MiB / 4 CPUs. --- src/commands/vm/reset.rs | 4 +-- src/commands/vm/start.rs | 4 +-- src/commands/vm/update.rs | 15 +++++---- src/main.rs | 16 +++++---- src/utils/vm/client.rs | 8 +++-- src/utils/vm/config.rs | 44 ++++++++++++++++++++++++ src/utils/vm/lifecycle.rs | 71 +++++++++++++++++++++++++++++++++++---- src/utils/vm/route.rs | 4 +-- 8 files changed, 138 insertions(+), 28 deletions(-) diff --git a/src/commands/vm/reset.rs b/src/commands/vm/reset.rs index b6a58c2..3203492 100644 --- a/src/commands/vm/reset.rs +++ b/src/commands/vm/reset.rs @@ -100,8 +100,8 @@ impl ResetCommand { println!("avocado vm reset: restarting VM…"); let opts = lifecycle::StartOptions { vm_source: artifact_dir, - memory_mib: 4096, - cpus: 4, + memory_mib: None, + cpus: None, ssh_port: None, cmdline_extra: None, workspace: None, diff --git a/src/commands/vm/start.rs b/src/commands/vm/start.rs index efdc344..73d10b5 100644 --- a/src/commands/vm/start.rs +++ b/src/commands/vm/start.rs @@ -10,8 +10,8 @@ use crate::utils::vm::state::VmPaths; pub struct StartCommand { pub vm_source: Option, - pub memory_mib: u32, - pub cpus: u32, + pub memory_mib: Option, + pub cpus: Option, pub ssh_port: Option, pub cmdline_extra: Option, pub workspace: Option, diff --git a/src/commands/vm/update.rs b/src/commands/vm/update.rs index 76ad6e2..c03ab45 100644 --- a/src/commands/vm/update.rs +++ b/src/commands/vm/update.rs @@ -258,15 +258,16 @@ impl UpdateCommand { if was_running { println!("avocado vm update: restarting VM…"); - // Minimal start opts — pick defaults consistent with prior - // start. We deliberately don't try to reconstruct every - // flag the user originally used; this is "restart with - // defaults", and `avocado vm start --foo=...` is the path - // when the user wants to re-customise. + // Minimal start opts — None for cpus/memory means lifecycle::start + // reads `runtime.*` from ~/.avocado/vm/config.yaml (or falls back + // to DEFAULT_CPUS / DEFAULT_MEMORY_MIB). Other knobs we deliberately + // don't try to reconstruct from the user's original flags; this is + // "restart with persisted/default settings", and `vm start --foo=…` + // is the path when the user wants to re-customise. let opts = crate::utils::vm::lifecycle::StartOptions { vm_source: install_dir.clone(), - memory_mib: 4096, - cpus: 4, + memory_mib: None, + cpus: None, ssh_port: None, cmdline_extra: None, workspace: None, diff --git a/src/main.rs b/src/main.rs index f9ed288..6483d77 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4200,12 +4200,16 @@ enum VmCommands { /// dir → error. #[arg(long)] vm_source: Option, - /// Memory in MiB. - #[arg(long, default_value_t = 4096)] - memory_mib: u32, - /// vCPU count. - #[arg(long, default_value_t = 4)] - cpus: u32, + /// Memory in MiB. Resolution order: this flag → `runtime.memory_mib` + /// in `~/.avocado/vm/config.yaml` (also written by Avocado.app's + /// settings UI) → 4096. When passed, the value is persisted back to + /// the config so the next flag-less `vm start` reuses it. + #[arg(long)] + memory_mib: Option, + /// vCPU count. Same resolution + persistence as `--memory-mib`, + /// falling back to `runtime.cpus` or 4. + #[arg(long)] + cpus: Option, /// Bind SSH on this host port (default: pick a free high port). #[arg(long)] ssh_port: Option, diff --git a/src/utils/vm/client.rs b/src/utils/vm/client.rs index 1f75ccd..d14ad4d 100644 --- a/src/utils/vm/client.rs +++ b/src/utils/vm/client.rs @@ -44,14 +44,18 @@ fn socket_path() -> Option { /// regardless of whether the desktop is installed or reachable. pub fn notify(method: &str, params: Value) { let Some(path) = socket_path() else { return }; - let Ok(stream) = UnixStream::connect(&path) else { return }; + let Ok(stream) = UnixStream::connect(&path) else { + return; + }; // Apply timeouts up-front; without these `write_all` and `read_line` // can stall on a hung peer for far longer than we're willing to wait. let _ = stream.set_write_timeout(Some(NOTIFY_TIMEOUT)); let _ = stream.set_read_timeout(Some(NOTIFY_TIMEOUT)); let req = json!({ "method": method, "params": params, "id": 1 }); - let Ok(mut buf) = serde_json::to_vec(&req) else { return }; + let Ok(mut buf) = serde_json::to_vec(&req) else { + return; + }; buf.push(b'\n'); let mut stream = stream; diff --git a/src/utils/vm/config.rs b/src/utils/vm/config.rs index f9333bc..a01c9aa 100644 --- a/src/utils/vm/config.rs +++ b/src/utils/vm/config.rs @@ -23,6 +23,9 @@ pub struct VmConfig { #[serde(default, skip_serializing_if = "Option::is_none")] pub network: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub runtime: Option, + /// Forward-compat bucket for keys this CLI version doesn't know about. /// Preserved verbatim on save so a newer desktop's settings survive an /// older CLI's round-trip. @@ -30,6 +33,21 @@ pub struct VmConfig { pub extra: BTreeMap, } +/// QEMU resource knobs the user can override. Persisted so `avocado vm start` +/// (without flags) and Avocado.app's settings UI converge on the same values. +/// `None` for a field means "fall back to the built-in default". +#[derive(Debug, Default, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct RuntimeConfig { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub cpus: Option, + + #[serde(default, skip_serializing_if = "Option::is_none")] + pub memory_mib: Option, + + #[serde(flatten)] + pub extra: BTreeMap, +} + #[derive(Debug, Default, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct NetworkConfig { /// Override the guest's DNS resolvers. Applied post-boot via @@ -283,6 +301,32 @@ mod tests { assert!(cfg.get("network.dns").unwrap().is_none()); } + #[test] + fn runtime_round_trip() { + let tmp = tempdir().unwrap(); + let path = tmp.path().join("config.yaml"); + let cfg = VmConfig { + runtime: Some(RuntimeConfig { + cpus: Some(6), + memory_mib: Some(8192), + extra: Default::default(), + }), + ..Default::default() + }; + cfg.save_to(&path).unwrap(); + let loaded = VmConfig::load_from(&path).unwrap(); + assert_eq!(loaded, cfg); + } + + #[test] + fn set_runtime_via_dotted_key() { + let mut cfg = VmConfig::default(); + cfg.set("runtime.cpus", &["6".into()]).unwrap(); + cfg.set("runtime.memory_mib", &["8192".into()]).unwrap(); + assert_eq!(cfg.runtime.as_ref().and_then(|r| r.cpus), Some(6)); + assert_eq!(cfg.runtime.as_ref().and_then(|r| r.memory_mib), Some(8192)); + } + #[test] fn unknown_keys_round_trip() { // Simulate a newer desktop writing a key this CLI doesn't model. diff --git a/src/utils/vm/lifecycle.rs b/src/utils/vm/lifecycle.rs index 553f7ca..921af48 100644 --- a/src/utils/vm/lifecycle.rs +++ b/src/utils/vm/lifecycle.rs @@ -40,10 +40,13 @@ fn notify_desktop(method: &str, params: serde_json::Value) { pub struct StartOptions { /// Directory containing the `direct` profile output (manifest.json + artifacts). pub vm_source: PathBuf, - /// Memory in MiB. - pub memory_mib: u32, - /// vCPU count. - pub cpus: u32, + /// Memory in MiB. `None` → read from `runtime.memory_mib` in + /// ~/.avocado/vm/config.yaml, else fall back to [`DEFAULT_MEMORY_MIB`]. + /// When `Some(_)`, the value is also persisted back to the config so + /// the next flag-less `vm start` (and Avocado.app) sees it. + pub memory_mib: Option, + /// vCPU count. Same resolution / persistence rules as [`memory_mib`]. + pub cpus: Option, /// Host port for ssh-into-VM. `None` → pick a free port. pub ssh_port: Option, /// Extra kernel cmdline. @@ -69,6 +72,14 @@ pub struct StartOptions { /// what's actually written). pub const DEFAULT_VAR_SIZE: &str = "50G"; +/// Fallback CPU count when neither the `--cpus` flag nor `runtime.cpus` in +/// `~/.avocado/vm/config.yaml` is set. +pub const DEFAULT_CPUS: u32 = 4; + +/// Fallback memory size (MiB) when neither the `--memory-mib` flag nor +/// `runtime.memory_mib` in `~/.avocado/vm/config.yaml` is set. +pub const DEFAULT_MEMORY_MIB: u32 = 4096; + /// What the `status` command needs to render. #[derive(Debug, Clone)] pub struct VmStatus { @@ -158,9 +169,15 @@ pub async fn start(opts: StartOptions) -> Result { // ~/.ssh/config but has no env hook for our key/known_hosts. write_ssh_config(&paths, ssh_port)?; + // Resolve cpu/memory: explicit flag wins, else persisted config, else + // the built-in defaults. When the user passed a flag, also persist it + // back to ~/.avocado/vm/config.yaml so the next flag-less `vm start` + // and Avocado.app's settings UI both see the same value. + let (cpus, memory_mib) = resolve_and_persist_runtime(&paths, opts.cpus, opts.memory_mib)?; + let cfg = QemuConfig { - memory_mib: opts.memory_mib, - cpus: opts.cpus, + memory_mib, + cpus, ssh_port, cmdline_extra: opts.cmdline_extra, artifact_dir: artifact_dir.clone(), @@ -357,6 +374,47 @@ pub fn ssh_target_for_running() -> Result { Ok(SshTarget::local(&paths, port)) } +/// Resolve effective cpu/memory and, if either was explicitly passed via +/// CLI flags, persist them back to ~/.avocado/vm/config.yaml so subsequent +/// flag-less starts (and Avocado.app's settings UI) converge. +/// +/// Returns `(cpus, memory_mib)` actually used for the launch. +fn resolve_and_persist_runtime( + paths: &VmPaths, + flag_cpus: Option, + flag_memory_mib: Option, +) -> Result<(u32, u32)> { + use super::config::{RuntimeConfig, VmConfig}; + + let mut cfg = VmConfig::load(paths).unwrap_or_default(); + let persisted_cpus = cfg.runtime.as_ref().and_then(|r| r.cpus); + let persisted_memory = cfg.runtime.as_ref().and_then(|r| r.memory_mib); + + let cpus = flag_cpus.or(persisted_cpus).unwrap_or(DEFAULT_CPUS); + let memory_mib = flag_memory_mib + .or(persisted_memory) + .unwrap_or(DEFAULT_MEMORY_MIB); + + // Only write back when the user actually passed a flag AND it + // differs from what's already on disk. Avoids touching the file + // (and growing its mtime) on every routine `vm start`. + let cpus_changed = flag_cpus.is_some() && persisted_cpus != Some(cpus); + let memory_changed = flag_memory_mib.is_some() && persisted_memory != Some(memory_mib); + if cpus_changed || memory_changed { + let runtime = cfg.runtime.get_or_insert_with(RuntimeConfig::default); + if cpus_changed { + runtime.cpus = Some(cpus); + } + if memory_changed { + runtime.memory_mib = Some(memory_mib); + } + cfg.save(paths) + .context("persisting runtime overrides to vm config")?; + } + + Ok((cpus, memory_mib)) +} + /// Apply the persisted [`config::VmConfig`] (+ optional `--dns` one-shot /// override) inside the running guest. Currently only DNS is implemented; /// future knobs (MTU, http_proxy, …) hang off the same entry point. @@ -673,4 +731,3 @@ fn write_ssh_config(paths: &VmPaths, ssh_port: u16) -> Result<()> { .with_context(|| format!("writing {}", paths.ssh_config().display()))?; Ok(()) } - diff --git a/src/utils/vm/route.rs b/src/utils/vm/route.rs index 9f26705..01c5674 100644 --- a/src/utils/vm/route.rs +++ b/src/utils/vm/route.rs @@ -106,8 +106,8 @@ pub async fn ensure_routed_for_process( ); let status = lifecycle::start(StartOptions { vm_source, - memory_mib: 4096, - cpus: 4, + memory_mib: None, + cpus: None, ssh_port: None, cmdline_extra: None, workspace: None, From 8fa050ef9c23cec9ae96fbdedff819f99a006b3e Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Thu, 21 May 2026 14:32:32 -0500 Subject: [PATCH 16/17] runtime deploy: add --output json for desktop UI integration When `--output json` is set, deploy skips TUI rendering and emits NDJSON `task_registered` / `step` / `step_error` events for each phase (stamps, hash-collection, metadata-sign, deploy). Names mirror the human-facing labels so the desktop can render them directly. --- src/commands/runtime/deploy.rs | 255 ++++++++++++++++++++++++--------- src/main.rs | 12 ++ 2 files changed, 203 insertions(+), 64 deletions(-) diff --git a/src/commands/runtime/deploy.rs b/src/commands/runtime/deploy.rs index c414d3f..8e08b1f 100644 --- a/src/commands/runtime/deploy.rs +++ b/src/commands/runtime/deploy.rs @@ -3,6 +3,7 @@ use crate::utils::{ container::{RunConfig, SdkContainer}, lockfile::LockFile, output::{print_info, print_success, print_warning, OutputLevel}, + output_format::{emit_json_event, is_json_output_active}, stamps::{generate_batch_read_stamps_script, validate_stamps_batch, StampRequirement}, target::resolve_target_required, update_repo::{self, HashCollectionOutput}, @@ -86,6 +87,14 @@ pub struct RuntimeDeployCommand { composed_config: Option>, } +/// Deploy phase identifiers used for NDJSON `task_registered` / `step` events +/// when `--output json` is active. Names mirror the human-facing phase +/// labels in `execute()` so the desktop app can render them directly. +const PHASE_STAMPS: &str = "stamps"; +const PHASE_HASH: &str = "hash-collection"; +const PHASE_METADATA: &str = "metadata-sign"; +const PHASE_DEPLOY: &str = "deploy"; + impl RuntimeDeployCommand { pub fn new( runtime_name: String, @@ -129,6 +138,42 @@ impl RuntimeDeployCommand { self } + /// Register a phase as a known step so the desktop UI can render it + /// before it starts running. No-op in human-output mode. + fn emit_phase_register(name: &str, label: &str) { + if is_json_output_active() { + emit_json_event(&serde_json::json!({ + "event": "task_registered", + "name": name, + "label": label, + })); + } + } + + /// Transition a phase to the given status (`running`, `success`, `failed`). + /// No-op in human-output mode. + fn emit_phase_status(name: &str, status: &str) { + if is_json_output_active() { + emit_json_event(&serde_json::json!({ + "event": "step", + "name": name, + "status": status, + })); + } + } + + /// Annotate the most recent failed phase with an error message so the + /// desktop UI can surface it inline next to the step. No-op in human mode. + fn emit_phase_error(name: &str, message: &str) { + if is_json_output_active() { + emit_json_event(&serde_json::json!({ + "event": "step_error", + "name": name, + "message": message, + })); + } + } + pub async fn execute(&self) -> Result<()> { let composed = match &self.composed_config { Some(cc) => Arc::clone(cc), @@ -180,10 +225,21 @@ impl RuntimeDeployCommand { let container_helper = SdkContainer::from_config(&self.config_path, config)?.verbose(self.verbose); + // Pre-register all phases so the desktop UI can render the + // step list before any work starts. Order here matches the + // execution order below. + if !self.no_stamps { + Self::emit_phase_register(PHASE_STAMPS, "Verify build stamps"); + } + Self::emit_phase_register(PHASE_HASH, "Collect artifact hashes"); + Self::emit_phase_register(PHASE_METADATA, "Sign TUF metadata"); + Self::emit_phase_register(PHASE_DEPLOY, "Deploy to device"); + // Validate stamps before proceeding (unless --no-stamps). // Deploy needs the runtime to be built; provision is not required — // deploy and provision are independent consumers of the build output. if !self.no_stamps { + Self::emit_phase_status(PHASE_STAMPS, "running"); let required = vec![StampRequirement::new( crate::utils::stamps::StampCommand::Build, crate::utils::stamps::StampComponent::Runtime, @@ -202,18 +258,28 @@ impl RuntimeDeployCommand { ..Default::default() }; - let output = container_helper + let output = match container_helper .run_in_container_with_output(run_config) - .await?; + .await + { + Ok(o) => o, + Err(e) => { + Self::emit_phase_error(PHASE_STAMPS, &e.to_string()); + Self::emit_phase_status(PHASE_STAMPS, "failed"); + return Err(e); + } + }; let validation = validate_stamps_batch(&required, output.as_deref().unwrap_or(""), None); if !validation.is_satisfied() { - validation - .into_error(&format!("Cannot deploy runtime '{}'", self.runtime_name)) - .print_and_exit(); + let msg = format!("Cannot deploy runtime '{}'", self.runtime_name); + Self::emit_phase_error(PHASE_STAMPS, &msg); + Self::emit_phase_status(PHASE_STAMPS, "failed"); + validation.into_error(&msg).print_and_exit(); } + Self::emit_phase_status(PHASE_STAMPS, "success"); } print_info( @@ -229,6 +295,7 @@ impl RuntimeDeployCommand { "Phase 1: Collecting artifact hashes...", OutputLevel::Normal, ); + Self::emit_phase_status(PHASE_HASH, "running"); let hash_script = self.create_hash_collection_script(&target_arch); let hash_run_config = RunConfig { @@ -243,13 +310,33 @@ impl RuntimeDeployCommand { ..Default::default() }; - let hash_output = container_helper + let hash_output = match container_helper .run_in_container_with_output(hash_run_config) - .await? - .context("Hash collection script produced no output")?; + .await + { + Ok(Some(out)) => out, + Ok(None) => { + let msg = "Hash collection script produced no output"; + Self::emit_phase_error(PHASE_HASH, msg); + Self::emit_phase_status(PHASE_HASH, "failed"); + return Err(anyhow::anyhow!(msg)); + } + Err(e) => { + Self::emit_phase_error(PHASE_HASH, &e.to_string()); + Self::emit_phase_status(PHASE_HASH, "failed"); + return Err(e); + } + }; - let collection: HashCollectionOutput = - serde_json::from_str(&hash_output).context("Failed to parse hash collection output")?; + let collection: HashCollectionOutput = match serde_json::from_str(&hash_output) { + Ok(c) => c, + Err(e) => { + let msg = format!("Failed to parse hash collection output: {e}"); + Self::emit_phase_error(PHASE_HASH, &msg); + Self::emit_phase_status(PHASE_HASH, "failed"); + return Err(anyhow::anyhow!(msg)); + } + }; if self.verbose { print_info( @@ -260,22 +347,31 @@ impl RuntimeDeployCommand { OutputLevel::Normal, ); } + Self::emit_phase_status(PHASE_HASH, "success"); // --- Phase 2: Generate and sign TUF metadata --- print_info( "Phase 2: Generating signed TUF metadata...", OutputLevel::Normal, ); + Self::emit_phase_status(PHASE_METADATA, "running"); - let signing_key_name = config.get_runtime_signing_key_name(&self.runtime_name); - let content_key_name = config.get_runtime_content_key_name(&self.runtime_name); let project_dir = std::path::Path::new(&self.config_path) .parent() .unwrap_or(std::path::Path::new(".")); + let staging_dir = project_dir.join(DEPLOY_STAGING_DIR); - let mut resolved_signing_key_name = signing_key_name.clone(); - let signer = - match crate::utils::update_signing::resolve_signing_key(signing_key_name.as_deref())? { + // Group all metadata work in a single fallible block so any + // failure is attributed to PHASE_METADATA without wrapping every + // `?` site individually. + let metadata_result: Result<()> = (|| { + let signing_key_name = config.get_runtime_signing_key_name(&self.runtime_name); + let content_key_name = config.get_runtime_content_key_name(&self.runtime_name); + + let mut resolved_signing_key_name = signing_key_name.clone(); + let signer = match crate::utils::update_signing::resolve_signing_key( + signing_key_name.as_deref(), + )? { Some(s) => s, None => { // Auto-generate a development signing key for local deploys @@ -285,52 +381,59 @@ impl RuntimeDeployCommand { .expect("dev signing key was just created") } }; - let content_signer = crate::utils::update_signing::resolve_content_key( - content_key_name.as_deref(), - resolved_signing_key_name.as_deref(), - )? - .expect("signing key is set, so content key resolution should return Some"); - - let repo_metadata = update_repo::generate_repo_metadata( - &collection.targets, - &collection.runtime_uuid, - &signer, - &content_signer, - )?; - - let staging_dir = project_dir.join(DEPLOY_STAGING_DIR); - let delegations_dir = staging_dir.join("delegations"); - std::fs::create_dir_all(&delegations_dir).with_context(|| { - format!( - "Failed to create deploy staging directory: {}", - delegations_dir.display() + let content_signer = crate::utils::update_signing::resolve_content_key( + content_key_name.as_deref(), + resolved_signing_key_name.as_deref(), + )? + .expect("signing key is set, so content key resolution should return Some"); + + let repo_metadata = update_repo::generate_repo_metadata( + &collection.targets, + &collection.runtime_uuid, + &signer, + &content_signer, + )?; + + let delegations_dir = staging_dir.join("delegations"); + std::fs::create_dir_all(&delegations_dir).with_context(|| { + format!( + "Failed to create deploy staging directory: {}", + delegations_dir.display() + ) + })?; + + std::fs::write( + staging_dir.join("targets.json"), + &repo_metadata.targets_json, ) - })?; - - std::fs::write( - staging_dir.join("targets.json"), - &repo_metadata.targets_json, - ) - .context("Failed to write targets.json")?; - std::fs::write( - staging_dir.join("snapshot.json"), - &repo_metadata.snapshot_json, - ) - .context("Failed to write snapshot.json")?; - std::fs::write( - staging_dir.join("timestamp.json"), - &repo_metadata.timestamp_json, - ) - .context("Failed to write timestamp.json")?; - if let Some(root_json) = &collection.root_json { - std::fs::write(staging_dir.join("root.json"), root_json) - .context("Failed to write root.json to staging")?; + .context("Failed to write targets.json")?; + std::fs::write( + staging_dir.join("snapshot.json"), + &repo_metadata.snapshot_json, + ) + .context("Failed to write snapshot.json")?; + std::fs::write( + staging_dir.join("timestamp.json"), + &repo_metadata.timestamp_json, + ) + .context("Failed to write timestamp.json")?; + if let Some(root_json) = &collection.root_json { + std::fs::write(staging_dir.join("root.json"), root_json) + .context("Failed to write root.json to staging")?; + } + std::fs::write( + delegations_dir.join(format!("runtime-{}.json", collection.runtime_uuid)), + &repo_metadata.delegated_targets_json, + ) + .context("Failed to write delegated targets json")?; + Ok(()) + })(); + + if let Err(e) = metadata_result { + Self::emit_phase_error(PHASE_METADATA, &e.to_string()); + Self::emit_phase_status(PHASE_METADATA, "failed"); + return Err(e); } - std::fs::write( - delegations_dir.join(format!("runtime-{}.json", collection.runtime_uuid)), - &repo_metadata.delegated_targets_json, - ) - .context("Failed to write delegated targets json")?; if self.verbose { print_info( @@ -338,14 +441,24 @@ impl RuntimeDeployCommand { OutputLevel::Normal, ); } + Self::emit_phase_status(PHASE_METADATA, "success"); // --- Phase 3: Serve repo and trigger update --- print_info( "Phase 3: Serving update repository and triggering device update...", OutputLevel::Normal, ); - - let deploy_script = self.create_deploy_script(&target_arch, &collection.runtime_uuid)?; + Self::emit_phase_status(PHASE_DEPLOY, "running"); + + let deploy_script = match self.create_deploy_script(&target_arch, &collection.runtime_uuid) + { + Ok(s) => s, + Err(e) => { + Self::emit_phase_error(PHASE_DEPLOY, &e.to_string()); + Self::emit_phase_status(PHASE_DEPLOY, "failed"); + return Err(e); + } + }; let mut env_vars = HashMap::new(); env_vars.insert("AVOCADO_TARGET".to_string(), target_arch.clone()); @@ -373,18 +486,32 @@ impl RuntimeDeployCommand { sdk_arch: self.sdk_arch.clone(), ..Default::default() }; - let deploy_result = container_helper + let deploy_result = match container_helper .run_in_container(run_config) .await - .context("Failed to deploy runtime")?; + .context("Failed to deploy runtime") + { + Ok(r) => r, + Err(e) => { + let _ = std::fs::remove_dir_all(&staging_dir); + Self::emit_phase_error(PHASE_DEPLOY, &e.to_string()); + Self::emit_phase_status(PHASE_DEPLOY, "failed"); + return Err(e); + } + }; // Clean up staging directory let _ = std::fs::remove_dir_all(&staging_dir); if !deploy_result { - return Err(anyhow::anyhow!("Failed to deploy runtime")); + let msg = "Failed to deploy runtime"; + Self::emit_phase_error(PHASE_DEPLOY, msg); + Self::emit_phase_status(PHASE_DEPLOY, "failed"); + return Err(anyhow::anyhow!(msg)); } + Self::emit_phase_status(PHASE_DEPLOY, "success"); + print_success( &format!( "Successfully deployed runtime '{}' to device '{}'", diff --git a/src/main.rs b/src/main.rs index 6483d77..d622ccb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -439,6 +439,9 @@ enum Commands { /// Additional arguments to pass to DNF commands #[arg(long = "dnf-arg", num_args = 1, allow_hyphen_values = true, action = clap::ArgAction::Append)] dnf_args: Option>, + /// Output format. JSON skips TUI rendering and emits NDJSON events. + #[arg(long, value_enum, default_value_t = OutputFormat::Human)] + output: OutputFormat, }, /// Manage signing keys for extension and image signing #[command(name = "signing-keys")] @@ -1644,6 +1647,9 @@ enum RuntimeCommands { /// Additional arguments to pass to DNF commands #[arg(long = "dnf-arg", num_args = 1, allow_hyphen_values = true, action = clap::ArgAction::Append)] dnf_args: Option>, + /// Output format. JSON skips TUI rendering and emits NDJSON events. + #[arg(long, value_enum, default_value_t = OutputFormat::Human)] + output: OutputFormat, }, /// Sign runtime images Sign { @@ -2187,8 +2193,11 @@ async fn main() -> Result<()> { device, container_args, dnf_args, + output, } => { let runtime = resolve_runtime_at_path(&config, name.as_deref().or(runtime.as_deref()))?; + let _json_guard = + run_with_json_lifecycle(output, "deploy", target.as_deref(), Some(&runtime)); let deploy_cmd = RuntimeDeployCommand::new( runtime, @@ -2497,9 +2506,12 @@ async fn main() -> Result<()> { device, container_args, dnf_args, + output, } => { let runtime = resolve_runtime_at_path(&config, name.as_deref().or(runtime.as_deref()))?; + let _json_guard = + run_with_json_lifecycle(output, "deploy", target.as_deref(), Some(&runtime)); let deploy_cmd = RuntimeDeployCommand::new( runtime, From 6d4c265bcbb13283489c3c1216532a00622e1042 Mon Sep 17 00:00:00 2001 From: Justin Schneck Date: Thu, 21 May 2026 14:28:00 -0500 Subject: [PATCH 17/17] release: bump to 0.40.0 --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9eea002..e403ed1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -130,7 +130,7 @@ checksum = "c08606f8c3cbf4ce6ec8e28fb0014a2c086708fe954eaa885384a6165172e7e8" [[package]] name = "avocado-cli" -version = "0.39.0" +version = "0.40.0" dependencies = [ "anyhow", "base64", diff --git a/Cargo.toml b/Cargo.toml index 260b910..7bf418e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "avocado-cli" -version = "0.39.0" +version = "0.40.0" edition = "2021" description = "Command line interface for Avocado." authors = ["Avocado"]