From f15d5b077c798630e57c4093e8db8f28573c3cd8 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sat, 19 Sep 2026 20:11:00 -0300 Subject: [PATCH 01/12] feat(kw): parse remote.config and resolve the deploy remote MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit reads kw's ssh-config-like remote.config directly instead of scraping `kw remote --list`, so deploy can target a stable host:port. The tree's .kw/remote.config wins over the XDG/home copy, and a present local file is never skipped — falling through would deploy to a different machine than the tree is configured for. This commit is part of the kw integration's step 7. Signed-off-by: lorenzoberts --- src/kw/mod.rs | 1 + src/kw/remote.rs | 632 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 633 insertions(+) create mode 100644 src/kw/remote.rs diff --git a/src/kw/mod.rs b/src/kw/mod.rs index d5f0da2..97ac2bb 100644 --- a/src/kw/mod.rs +++ b/src/kw/mod.rs @@ -9,4 +9,5 @@ pub mod handle; pub mod history; pub mod messages; pub mod readiness; +pub mod remote; pub mod status; diff --git a/src/kw/remote.rs b/src/kw/remote.rs new file mode 100644 index 0000000..566016e --- /dev/null +++ b/src/kw/remote.rs @@ -0,0 +1,632 @@ +//! Resolve the remote kw deploy target from kw's `remote.config`. +//! +//! The file is a small ssh-config-like text file written by `kw remote` +//! (`src/kw_remote.sh` at kw 0.10): an optional `#kw-default=` +//! sentinel plus `Host` stanzas with `Hostname` / `Port` / `User`. This +//! parser reads that file directly rather than scraping `kw remote --list`, +//! whose output is colorized terminal prose with no machine-readable mode. +//! +//! Resolution prefers `/.kw/remote.config`, then +//! `${XDG_CONFIG_HOME:-$HOME/.config}/kw/remote.config`. A present local +//! file is authoritative even when empty or unreadable — falling through +//! to the home copy would silently deploy to a different machine than the +//! tree is configured for. +//! +//! Production callers land with the deploy start path; until then this +//! module is exercised by its tests. + +#![cfg_attr(not(test), allow(dead_code))] + +use std::path::{Path, PathBuf}; + +use thiserror::Error; + +use crate::infrastructure::{env::EnvTrait, file_system::FileSystemTrait}; + +/// A Host stanza from `remote.config` that has a usable Hostname. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct KwRemote { + pub name: String, + pub hostname: String, + pub port: u16, + pub user: Option, +} + +impl KwRemote { + /// The `--remote` token kw's deploy parser accepts: + /// `[user@]host:port`. + pub fn endpoint(&self) -> String { + match &self.user { + Some(user) => format!("{}@{}:{}", user, self.hostname, self.port), + None => format!("{}:{}", self.hostname, self.port), + } + } +} + +/// Why a deploy remote could not be resolved. Each variant's message is +/// the actionable explanation. +#[derive(Debug, Clone, PartialEq, Eq, Error)] +pub enum RemoteRefusal { + #[error( + "no remotes configured; configure a remote with `kw remote --set-default` \ + or edit `.kw/remote.config`" + )] + NoRemotesConfigured, + #[error( + "{count} remotes configured with no default; set one with \ + `kw remote --set-default` or edit `.kw/remote.config`" + )] + NoDefault { count: usize }, + #[error( + "default remote '{name}' is not a Host in remote.config; set a valid \ + default with `kw remote --set-default`" + )] + DefaultNotFound { name: String }, +} + +/// Parsed contents of a `remote.config` file. Incomplete Host stanzas +/// (no Hostname, or an unparseable Port) are dropped rather than guessed. +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub struct ParsedRemoteConfig { + pub default: Option, + pub hosts: Vec, +} + +/// Parses kw's ssh-config-like `remote.config`. Blank lines and comments +/// are skipped; `#kw-default=` (anywhere) names the default Host, +/// last occurrence winning; `Host ` starts a stanza; `Hostname`, +/// `Port`, and `User` are read case-insensitively; `IdentityFile` and +/// other keys are tolerated and ignored. Port defaults to 22 when unset. +/// Duplicate Host names keep the last complete stanza. +pub fn parse_remote_config(content: &str) -> ParsedRemoteConfig { + let mut parsed = ParsedRemoteConfig::default(); + let mut current: Option = None; + + for line in content.lines() { + let trimmed = line.trim(); + if trimmed.is_empty() { + continue; + } + if let Some(name) = trimmed.strip_prefix("#kw-default=") { + let name = name.trim(); + if !name.is_empty() { + parsed.default = Some(name.to_string()); + } + continue; + } + if trimmed.starts_with('#') { + continue; + } + + let Some((keyword, rest)) = split_keyword(trimmed) else { + continue; + }; + if keyword.eq_ignore_ascii_case("host") { + if let Some(host) = current.take().and_then(HostBuilder::finish) { + upsert_host(&mut parsed.hosts, host); + } + let name = rest.split_whitespace().next().unwrap_or(""); + if !name.is_empty() { + current = Some(HostBuilder::new(name)); + } + continue; + } + let Some(builder) = current.as_mut() else { + continue; + }; + if keyword.eq_ignore_ascii_case("hostname") { + if !rest.is_empty() { + builder.hostname = Some(rest.to_string()); + } + } else if keyword.eq_ignore_ascii_case("port") { + if !rest.is_empty() { + builder.port = Some(rest.to_string()); + } + } else if keyword.eq_ignore_ascii_case("user") { + if !rest.is_empty() { + builder.user = Some(rest.to_string()); + } + } + } + if let Some(host) = current.and_then(HostBuilder::finish) { + upsert_host(&mut parsed.hosts, host); + } + parsed +} + +/// Picks the deploy remote from a parsed file: the `#kw-default=` Host +/// when set, otherwise the only Host. Multiple hosts with no default, or +/// a default that names no complete Host, are refusals. +pub fn select_deploy_remote(parsed: &ParsedRemoteConfig) -> Result { + if let Some(name) = parsed.default.as_deref() { + return parsed + .hosts + .iter() + .find(|host| host.name == name) + .cloned() + .ok_or_else(|| RemoteRefusal::DefaultNotFound { + name: name.to_string(), + }); + } + match parsed.hosts.as_slice() { + [] => Err(RemoteRefusal::NoRemotesConfigured), + [only] => Ok(only.clone()), + hosts => Err(RemoteRefusal::NoDefault { count: hosts.len() }), + } +} + +/// Resolves the remote kw deploy will be pointed at. See the module docs +/// for the file lookup order. +pub fn resolve_deploy_remote( + fs: &dyn FileSystemTrait, + env: &dyn EnvTrait, + tree_path: &Path, +) -> Result { + let local = tree_path.join(".kw").join("remote.config"); + if fs.is_file(&local) { + return remote_from_file(fs, &local); + } + let Some(global) = xdg_kw_remote_config(env) else { + return Err(RemoteRefusal::NoRemotesConfigured); + }; + if fs.is_file(&global) { + return remote_from_file(fs, &global); + } + Err(RemoteRefusal::NoRemotesConfigured) +} + +fn remote_from_file(fs: &dyn FileSystemTrait, path: &Path) -> Result { + let content = fs + .read_to_string(path) + .map_err(|_| RemoteRefusal::NoRemotesConfigured)?; + select_deploy_remote(&parse_remote_config(&content)) +} + +/// `${XDG_CONFIG_HOME:-$HOME/.config}/kw/remote.config`. A set-but-empty +/// `XDG_CONFIG_HOME` is treated as unset, matching bash `:-` and the XDG +/// spec — otherwise the path would be relative to cwd. +fn xdg_kw_remote_config(env: &dyn EnvTrait) -> Option { + let config_home = match env.var("XDG_CONFIG_HOME") { + Ok(xdg) if !xdg.is_empty() => xdg, + _ => format!("{}/.config", env.var("HOME").ok()?), + }; + Some(Path::new(&config_home).join("kw").join("remote.config")) +} + +struct HostBuilder { + name: String, + hostname: Option, + port: Option, + user: Option, +} + +impl HostBuilder { + fn new(name: &str) -> Self { + Self { + name: name.to_string(), + hostname: None, + port: None, + user: None, + } + } + + fn finish(self) -> Option { + let hostname = self.hostname.filter(|value| !value.is_empty())?; + let port = match self.port.as_deref() { + None | Some("") => 22, + Some(value) => value.parse().ok().filter(|port| *port != 0)?, + }; + let user = self.user.filter(|value| !value.is_empty()); + Some(KwRemote { + name: self.name, + hostname, + port, + user, + }) + } +} + +fn split_keyword(line: &str) -> Option<(&str, &str)> { + let keyword_end = line.find(|c: char| c.is_whitespace()).unwrap_or(line.len()); + let keyword = &line[..keyword_end]; + if keyword.is_empty() { + return None; + } + Some((keyword, line[keyword_end..].trim())) +} + +fn upsert_host(hosts: &mut Vec, host: KwRemote) { + if let Some(existing) = hosts.iter_mut().find(|entry| entry.name == host.name) { + *existing = host; + } else { + hosts.push(host); + } +} + +#[cfg(test)] +mod tests { + use std::{collections::HashMap, path::PathBuf, sync::Arc}; + + use crate::infrastructure::{ + env::{EnvError, MockEnvTrait}, + file_system::{FileSystemError, MockFileSystemTrait}, + }; + + use super::*; + + /// The example from kw's own writer (`src/kw_remote.sh`) / the + /// integration plan: two hosts, default on the second, fields indented. + const PLAN_FIXTURE: &str = "\ +#kw-default=arch-test +Host steamos + Hostname steamdeck + Port 8888 + User jozzi +Host arch-test + Hostname arch-tm + Port 22 + User abc +"; + + /// Lab deploy-smoke file: IdentityFile must not break parsing, and + /// User root must land on the endpoint. + const LAB_FIXTURE: &str = "\ +#kw-default=ph-dut +Host ph-dut + Hostname lima-ph-dut.internal + Port 22 + User root + IdentityFile /opt/ph-lab/keys/lab_ed25519 +"; + + fn choose(content: &str) -> Result { + select_deploy_remote(&parse_remote_config(content)) + } + + fn remote(name: &str, hostname: &str, port: u16, user: Option<&str>) -> KwRemote { + KwRemote { + name: name.to_string(), + hostname: hostname.to_string(), + port, + user: user.map(str::to_string), + } + } + + #[test] + fn plan_fixture_picks_the_default_host() { + let parsed = parse_remote_config(PLAN_FIXTURE); + assert_eq!(Some("arch-test"), parsed.default.as_deref()); + assert_eq!( + vec![ + remote("steamos", "steamdeck", 8888, Some("jozzi")), + remote("arch-test", "arch-tm", 22, Some("abc")), + ], + parsed.hosts + ); + assert_eq!("abc@arch-tm:22", choose(PLAN_FIXTURE).unwrap().endpoint()); + } + + #[test] + fn lab_fixture_ignores_identity_file() { + let chosen = choose(LAB_FIXTURE).unwrap(); + assert_eq!( + remote("ph-dut", "lima-ph-dut.internal", 22, Some("root")), + chosen + ); + assert_eq!("root@lima-ph-dut.internal:22", chosen.endpoint()); + } + + #[test] + fn single_host_without_default_is_used() { + let content = "\ +Host dut + Hostname 192.0.2.10 + User root +"; + let chosen = choose(content).unwrap(); + assert_eq!(remote("dut", "192.0.2.10", 22, Some("root")), chosen); + assert_eq!("root@192.0.2.10:22", chosen.endpoint()); + } + + #[test] + fn port_defaults_to_22_when_unset() { + let content = "\ +Host dut + Hostname box +"; + let chosen = choose(content).unwrap(); + assert_eq!(22, chosen.port); + assert_eq!(None, chosen.user); + assert_eq!("box:22", chosen.endpoint()); + } + + #[test] + fn multiple_hosts_without_default_are_refused() { + let content = "\ +Host a + Hostname one +Host b + Hostname two +"; + assert_eq!(Err(RemoteRefusal::NoDefault { count: 2 }), choose(content)); + } + + #[test] + fn missing_default_host_is_refused() { + let content = "\ +#kw-default=missing +Host dut + Hostname box +"; + assert_eq!( + Err(RemoteRefusal::DefaultNotFound { + name: "missing".to_string() + }), + choose(content) + ); + } + + #[test] + fn empty_file_is_no_remotes() { + assert_eq!(Err(RemoteRefusal::NoRemotesConfigured), choose("")); + assert_eq!( + Err(RemoteRefusal::NoRemotesConfigured), + choose("# just a comment\n") + ); + } + + #[test] + fn reordered_fields_and_comments_inside_a_stanza_are_tolerated() { + // kw used to assume Hostname/Port/User on the three lines after + // Host; that broke on comments and reordering. We key-match. + let content = "\ +#kw-default=origin +Host origin +# Port 33 + User root + Port 2222 + Hostname 192.0.2.1 +Host other + Hostname 192.0.2.2 + Port 22 + User other +"; + let chosen = choose(content).unwrap(); + assert_eq!(remote("origin", "192.0.2.1", 2222, Some("root")), chosen); + } + + #[test] + fn keys_are_matched_case_insensitively() { + let content = "\ +Host dut + hostname Box + PORT 2222 + user Root +"; + assert_eq!( + remote("dut", "Box", 2222, Some("Root")), + choose(content).unwrap() + ); + } + + #[test] + fn last_kw_default_and_duplicate_host_win() { + let content = "\ +#kw-default=first +Host dut + Hostname old +#kw-default=dut +Host dut + Hostname new + Port 2222 +"; + let parsed = parse_remote_config(content); + assert_eq!(Some("dut"), parsed.default.as_deref()); + assert_eq!(vec![remote("dut", "new", 2222, None)], parsed.hosts); + } + + #[test] + fn incomplete_hosts_are_dropped() { + let no_hostname = "\ +Host dut + Port 22 + User root +"; + assert_eq!(Err(RemoteRefusal::NoRemotesConfigured), choose(no_hostname)); + + let bad_port = "\ +Host dut + Hostname box + Port not-a-port +"; + assert_eq!(Err(RemoteRefusal::NoRemotesConfigured), choose(bad_port)); + + let zero_port = "\ +Host dut + Hostname box + Port 0 +"; + assert_eq!(Err(RemoteRefusal::NoRemotesConfigured), choose(zero_port)); + } + + #[test] + fn default_pointing_at_an_incomplete_host_is_not_found() { + let content = "\ +#kw-default=broken +Host broken + Port 22 +Host ok + Hostname box +"; + assert_eq!( + Err(RemoteRefusal::DefaultNotFound { + name: "broken".to_string() + }), + choose(content) + ); + } + + #[test] + fn last_line_without_newline_still_counts() { + let content = "Host dut\n Hostname box\n User root"; + assert_eq!( + remote("dut", "box", 22, Some("root")), + choose(content).unwrap() + ); + } + + #[test] + fn host_name_is_the_first_token_like_kw() { + // kw's `cut -d ' ' -f2` keeps only the first word after Host. + let content = "\ +Host dut extra + Hostname box +"; + assert_eq!("dut", choose(content).unwrap().name); + } + + #[test] + fn refusal_messages_are_actionable() { + assert!(RemoteRefusal::NoRemotesConfigured + .to_string() + .contains("kw remote --set-default")); + assert!(RemoteRefusal::NoDefault { count: 2 } + .to_string() + .contains("2 remotes")); + assert!(RemoteRefusal::DefaultNotFound { + name: "gone".to_string() + } + .to_string() + .contains("gone")); + } + + fn fs_with_files(files: &[(&str, &str)]) -> MockFileSystemTrait { + let map: HashMap = files + .iter() + .map(|(path, content)| (PathBuf::from(path), content.to_string())) + .collect(); + let is_file = Arc::new(map.clone()); + let read = Arc::new(map); + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_file() + .returning(move |path| is_file.contains_key(path)); + fs.expect_read_to_string().returning(move |path| { + read.get(path).cloned().ok_or_else(|| { + FileSystemError::IoError(std::io::Error::new( + std::io::ErrorKind::NotFound, + "missing", + )) + }) + }); + fs + } + + fn env_with(xdg: Option<&str>, home: Option<&str>) -> MockEnvTrait { + let xdg = xdg.map(str::to_string); + let home = home.map(str::to_string); + let mut env = MockEnvTrait::new(); + env.expect_var().returning(move |key| match key { + "XDG_CONFIG_HOME" => xdg.clone().ok_or_else(|| missing_var()), + "HOME" => home.clone().ok_or_else(|| missing_var()), + _ => Err(missing_var()), + }); + env + } + + fn missing_var() -> EnvError { + std::env::VarError::NotPresent.into() + } + + #[test] + fn resolve_prefers_the_tree_remote_config() { + let fs = fs_with_files(&[ + ("/home/user/linux/.kw/remote.config", LAB_FIXTURE), + ("/xdg/kw/remote.config", "Host other\n Hostname elsewhere\n"), + ]); + let env = env_with(Some("/xdg"), Some("/home/user")); + + let chosen = resolve_deploy_remote(&fs, &env, Path::new("/home/user/linux")).unwrap(); + assert_eq!("ph-dut", chosen.name); + } + + #[test] + fn resolve_falls_back_to_xdg_config_when_the_tree_has_none() { + let fs = fs_with_files(&[( + "/xdg/kw/remote.config", + "Host dut\n Hostname box\n User root\n", + )]); + let env = env_with(Some("/xdg"), Some("/home/user")); + + let chosen = resolve_deploy_remote(&fs, &env, Path::new("/home/user/linux")).unwrap(); + assert_eq!("root@box:22", chosen.endpoint()); + } + + #[test] + fn resolve_treats_empty_xdg_config_home_as_unset() { + let fs = fs_with_files(&[( + "/home/user/.config/kw/remote.config", + "Host dut\n Hostname box\n", + )]); + let env = env_with(Some(""), Some("/home/user")); + + let chosen = resolve_deploy_remote(&fs, &env, Path::new("/kernel")).unwrap(); + assert_eq!("box:22", chosen.endpoint()); + } + + #[test] + fn resolve_with_no_files_is_no_remotes() { + let fs = fs_with_files(&[]); + let env = env_with(Some("/xdg"), Some("/home/user")); + + assert_eq!( + Err(RemoteRefusal::NoRemotesConfigured), + resolve_deploy_remote(&fs, &env, Path::new("/kernel")) + ); + } + + #[test] + fn local_file_does_not_fall_through_when_empty() { + // A present local file is the tree's config even if it names no + // hosts: guessing the home copy would deploy somewhere else. + let fs = fs_with_files(&[ + ("/kernel/.kw/remote.config", "# nothing yet\n"), + ("/xdg/kw/remote.config", "Host dut\n Hostname box\n"), + ]); + let env = env_with(Some("/xdg"), Some("/home/user")); + + assert_eq!( + Err(RemoteRefusal::NoRemotesConfigured), + resolve_deploy_remote(&fs, &env, Path::new("/kernel")) + ); + } + + #[test] + fn unreadable_local_file_does_not_fall_through() { + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_file() + .returning(|path| path == Path::new("/kernel/.kw/remote.config")); + fs.expect_read_to_string().returning(|_| { + Err(FileSystemError::IoError(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "denied", + ))) + }); + let env = env_with(Some("/xdg"), Some("/home/user")); + + assert_eq!( + Err(RemoteRefusal::NoRemotesConfigured), + resolve_deploy_remote(&fs, &env, Path::new("/kernel")) + ); + } + + #[test] + fn resolve_without_home_or_xdg_and_no_local_file_is_no_remotes() { + let fs = fs_with_files(&[]); + let env = env_with(None, None); + + assert_eq!( + Err(RemoteRefusal::NoRemotesConfigured), + resolve_deploy_remote(&fs, &env, Path::new("/kernel")) + ); + } +} From afc31e41b0496435cd4ff1d12eff876ec46ac183 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sat, 19 Sep 2026 20:18:20 -0300 Subject: [PATCH 02/12] feat(kw): build deploy argv with reserved remote/reboot/force flags This commit constructs the kw deploy command line so patch-hub's resolved --remote, reboot, and force flags always win. User extras that would switch to local, list or uninstall kernels, run interactive setup, or flip boot-into-new-kernel-once are stripped; --alert is stripped too, because it is not a deploy option and older kw rejects it as invalid. This commit is part of the kw integration's step 7. Signed-off-by: lorenzoberts --- src/kw/argv.rs | 190 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 190 insertions(+) diff --git a/src/kw/argv.rs b/src/kw/argv.rs index 1ec076a..0cc5fcd 100644 --- a/src/kw/argv.rs +++ b/src/kw/argv.rs @@ -48,11 +48,66 @@ const BUILD_RESERVED: &[ReservedOption] = &[ ReservedOption::new(&["--from-sha"], true), ]; +/// Reserved for `kw deploy`. Patch-hub always injects a resolved +/// `--remote host:port` and the reboot/force knobs; user extras that +/// would override those, switch to local, list/uninstall kernels, run +/// interactive `--setup`, or flip `boot_into_new_kernel_once` via `-n` +/// are stripped. +/// +/// Short-flag collisions with ssh-config intuition matter here: `-l` is +/// `--list`, not `--local` (`--local` has no short form); `-r` is +/// `--reboot`, not `--remote`. `--uninstall`/`-u` takes an optional +/// value in kw (`uninstall::`) but is reserved as a boolean so `-u` +/// does not eat the following token. `--alert` is not a deploy option +/// at all — injecting or forwarding it hard-fails on kw beta-0.9 +/// (`Invalid option`), the version this lab still reports. +const DEPLOY_RESERVED: &[ReservedOption] = &[ + ReservedOption::new(&["--remote"], true), + ReservedOption::new(&["--local"], false), + ReservedOption::new(&["--reboot", "-r"], false), + ReservedOption::new(&["--no-reboot"], false), + ReservedOption::new(&["--force", "-f"], false), + ReservedOption::new(&["--list", "-l"], false), + ReservedOption::new(&["--ls-line", "-s"], false), + ReservedOption::new(&["--list-all", "-a"], false), + ReservedOption::new(&["--setup"], false), + ReservedOption::new(&["--uninstall", "-u"], false), + ReservedOption::new(&["--from-package", "-F"], true), + ReservedOption::new(&["--create-package", "-p"], false), + ReservedOption::new(&["--boot-into-new-kernel-once", "-n"], false), + ReservedOption::new(&["--save-log-to"], true), + ReservedOption::new(&["--alert"], true), +]; + /// The argv for a build job: `kw build ` (reserved extras stripped). pub fn build_argv(extra_args: &[String]) -> Vec { merge_extra_args(&["build"], BUILD_RESERVED, extra_args) } +/// The argv for a deploy job: `kw deploy --remote +/// --no-reboot|--reboot [--force] `. Reserved extras are +/// stripped so the injected remote, reboot, and force flags win. +/// `--force` is omitted entirely when `force` is false rather than +/// passing a no-op, because kw has no `--no-force`. +#[cfg_attr(not(test), allow(dead_code))] +pub fn deploy_argv( + endpoint: &str, + reboot: bool, + force: bool, + extra_args: &[String], +) -> Vec { + let mut base = vec![ + "deploy", + "--remote", + endpoint, + if reboot { "--reboot" } else { "--no-reboot" }, + ]; + if force { + base.push("--force"); + } + merge_extra_args(&base, DEPLOY_RESERVED, extra_args) +} + /// Appends user-supplied extra args to `base`, stripping every token that /// would override a reserved option: `--name=value` is stripped whole, /// `--name value` consumes the following token too, and a boolean @@ -210,4 +265,139 @@ mod tests { ) ); } + + const ENDPOINT: &str = "root@lima-ph-dut.internal:22"; + + fn deploy(extra: &[&str]) -> Vec { + deploy_argv(ENDPOINT, false, true, &extras(extra)) + } + + #[test] + fn deploy_argv_without_extras_is_remote_no_reboot_force() { + assert_eq!( + vec!["deploy", "--remote", ENDPOINT, "--no-reboot", "--force",], + deploy_argv(ENDPOINT, false, true, &[]) + ); + } + + #[test] + fn deploy_argv_reboot_and_unforced_swap_the_injected_flags() { + assert_eq!( + vec!["deploy", "--remote", ENDPOINT, "--reboot"], + deploy_argv(ENDPOINT, true, false, &[]) + ); + } + + #[test] + fn deploy_remote_always_wins_over_user_remote_and_local() { + assert_eq!( + vec![ + "deploy", + "--remote", + ENDPOINT, + "--no-reboot", + "--force", + "--verbose", + ], + deploy(&[ + "--remote", + "other:22", + "--local", + "--remote=evil:1", + "--verbose", + ]) + ); + } + + #[test] + fn deploy_strips_query_modes_setup_and_package_flags() { + assert_eq!( + vec![ + "deploy", + "--remote", + ENDPOINT, + "--no-reboot", + "--force", + "--verbose", + ], + deploy(&[ + "--list", + "-l", + "--ls-line", + "-s", + "--list-all", + "-a", + "--setup", + "--from-package", + "kernel.kw.tar", + "--from-package=other.kw.tar", + "-F", + "also.kw.tar", + "--create-package", + "-p", + "--verbose", + ]) + ); + } + + #[test] + fn deploy_strips_boot_once_alert_save_log_and_reboot_overrides() { + assert_eq!( + vec![ + "deploy", + "--remote", + ENDPOINT, + "--no-reboot", + "--force", + "--modules", + ], + deploy(&[ + "--boot-into-new-kernel-once", + "-n", + "--alert=n", + "--alert", + "v", + "--save-log-to", + "/tmp/x.log", + "--reboot", + "-r", + "--no-reboot", + "--force", + "-f", + "--modules", + ]) + ); + } + + #[test] + fn uninstall_short_flag_does_not_eat_the_next_token() { + // kw's `-u` takes an optional value (`uninstall::`). Treating it as + // a boolean reserved option keeps a following extra from vanishing. + assert_eq!( + vec![ + "deploy", + "--remote", + ENDPOINT, + "--no-reboot", + "--force", + "--verbose", + ], + deploy(&["-u", "--verbose"]) + ); + assert_eq!( + vec![ + "deploy", + "--remote", + ENDPOINT, + "--no-reboot", + "--force", + "--verbose", + ], + deploy(&["--uninstall", "--verbose"]) + ); + assert_eq!( + vec!["deploy", "--remote", ENDPOINT, "--no-reboot", "--force",], + deploy(&["--uninstall=old-kernel"]) + ); + } } From 87f7aa3af9b053c268717be663ca4dd931247c5a Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sat, 19 Sep 2026 20:20:06 -0300 Subject: [PATCH 03/12] feat(kw): name known kw deploy exit codes This commit maps the deploy exit codes verified in kw 0.10 so KwOps can explain a failed deploy instead of showing only a number. Unknown codes, including the stale 30/EROFS entry, stay unnamed so the raw exit is still visible. This commit is part of the kw integration's step 7. Signed-off-by: lorenzoberts --- src/kw/status.rs | 53 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/kw/status.rs b/src/kw/status.rs index 324ac97..13eda64 100644 --- a/src/kw/status.rs +++ b/src/kw/status.rs @@ -100,6 +100,28 @@ impl KwJobStatus { } } +/// Human-readable hint for a `kw deploy` exit code, when we know it. +/// +/// Codes are the ones verified in kw 0.10's deploy path (`src/deploy.sh`, +/// `install.sh`, `bootloader.sh`). Unknown codes — including errno 30 +/// (EROFS), which is not used there — return `None` so the UI can still +/// show the raw number. 68 can still surface even with `--force`: force +/// only skips the interactive prompt, not the underlying initramfs +/// errors. +#[cfg_attr(not(test), allow(dead_code))] +pub fn deploy_exit_hint(code: i32) -> Option<&'static str> { + Some(match code { + 2 => "kernel image not found", + 22 => "invalid option or kernel name", + 68 => "initramfs generation reported errors", + 95 => "unsupported bootloader", + 101 => "SSH unreachable after setup", + 103 => "passwordless root SSH setup failed", + 124 | 125 => "deploy cancelled, no valid kernel image, or not a kernel root", + _ => return None, + }) +} + #[cfg(test)] mod tests { use std::path::PathBuf; @@ -165,4 +187,35 @@ mod tests { .running_indicator() ); } + + #[test] + fn deploy_exit_hint_names_the_known_deploy_codes() { + let cases = [ + (2, "kernel image not found"), + (22, "invalid option or kernel name"), + (68, "initramfs generation reported errors"), + (95, "unsupported bootloader"), + (101, "SSH unreachable after setup"), + (103, "passwordless root SSH setup failed"), + ( + 124, + "deploy cancelled, no valid kernel image, or not a kernel root", + ), + ( + 125, + "deploy cancelled, no valid kernel image, or not a kernel root", + ), + ]; + for (code, hint) in cases { + assert_eq!(Some(hint), deploy_exit_hint(code), "code {code}"); + } + } + + #[test] + fn deploy_exit_hint_leaves_unknown_codes_unnamed() { + // 30/EROFS was a stale map entry: kw's deploy path never returns it. + assert_eq!(None, deploy_exit_hint(30)); + assert_eq!(None, deploy_exit_hint(1)); + assert_eq!(None, deploy_exit_hint(0)); + } } From 92b0aba508dcab5f2253a7371a32e2337eb70a11 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sat, 19 Sep 2026 20:28:40 -0300 Subject: [PATCH 04/12] feat(config): add kw deploy reboot and force knobs This commit adds kw_reboot_after_deploy (default false) and kw_deploy_force (default true) so deploy argv can follow user config instead of hard-coded flags. Missing keys in existing JSON keep the compiled-in defaults, matching the other optional config fields. This commit is part of the kw integration's step 7. Signed-off-by: lorenzoberts --- src/app/screens/edit_config.rs | 57 +++++++++++++++++++++++++++++++ src/config/errors.rs | 4 +++ src/config/service.rs | 26 +++++++++++++++ src/config/state.rs | 24 +++++++++++++ src/config/tests.rs | 61 ++++++++++++++++++++++++++++++++++ src/config/update.rs | 4 +++ 6 files changed, 176 insertions(+) diff --git a/src/app/screens/edit_config.rs b/src/app/screens/edit_config.rs index 65b50ad..0f9905b 100644 --- a/src/app/screens/edit_config.rs +++ b/src/app/screens/edit_config.rs @@ -45,6 +45,14 @@ impl EditConfigState { EditableConfig::StayOnAppliedBranch, config.stay_on_applied_branch().to_string(), ); + config_buffer.insert( + EditableConfig::KwRebootAfterDeploy, + config.kw_reboot_after_deploy().to_string(), + ); + config_buffer.insert( + EditableConfig::KwDeployForce, + config.kw_deploy_force().to_string(), + ); EditConfigState { config_buffer, @@ -145,6 +153,14 @@ impl EditConfigState { .config_buffer .get(&EditableConfig::StayOnAppliedBranch) .cloned(), + kw_reboot_after_deploy: self + .config_buffer + .get(&EditableConfig::KwRebootAfterDeploy) + .cloned(), + kw_deploy_force: self + .config_buffer + .get(&EditableConfig::KwDeployForce) + .cloned(), } } } @@ -160,6 +176,8 @@ enum EditableConfig { CoverRenderer, MaxLogAge, StayOnAppliedBranch, + KwRebootAfterDeploy, + KwDeployForce, } impl TryFrom for EditableConfig { @@ -176,6 +194,8 @@ impl TryFrom for EditableConfig { 6 => Ok(EditableConfig::CoverRenderer), 7 => Ok(EditableConfig::MaxLogAge), 8 => Ok(EditableConfig::StayOnAppliedBranch), + 9 => Ok(EditableConfig::KwRebootAfterDeploy), + 10 => Ok(EditableConfig::KwDeployForce), _ => bail!("Invalid index {} for EditableConfig", value), // Handle out of bounds } } @@ -199,6 +219,43 @@ impl Display for EditableConfig { EditableConfig::StayOnAppliedBranch => { write!(f, "Stay On Applied Branch (true/false)") } + EditableConfig::KwRebootAfterDeploy => { + write!(f, "Reboot After kw Deploy (true/false)") + } + EditableConfig::KwDeployForce => { + write!(f, "Force kw Deploy (true/false)") + } } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::ConfigState; + + #[test] + fn draft_includes_deploy_knobs_with_compiled_in_defaults() { + let snapshot = ConfigState::default().to_snapshot(); + let edit = EditConfigState::new(&snapshot); + let draft = edit.to_update_draft(); + + assert_eq!(Some("false".to_string()), draft.kw_reboot_after_deploy); + assert_eq!(Some("true".to_string()), draft.kw_deploy_force); + assert_eq!(11, edit.config_count()); + assert_eq!( + Some(( + "Reboot After kw Deploy (true/false)".to_string(), + "false".to_string() + )), + edit.config(9) + ); + assert_eq!( + Some(( + "Force kw Deploy (true/false)".to_string(), + "true".to_string() + )), + edit.config(10) + ); + } +} diff --git a/src/config/errors.rs b/src/config/errors.rs index 39c698b..7c8dbe0 100644 --- a/src/config/errors.rs +++ b/src/config/errors.rs @@ -18,6 +18,10 @@ pub enum ConfigError { InvalidMaxLogAge(String), #[error("invalid stay-on-applied-branch value: {0}")] InvalidStayOnAppliedBranch(String), + #[error("invalid kw reboot-after-deploy value: {0}")] + InvalidKwRebootAfterDeploy(String), + #[error("invalid kw deploy-force value: {0}")] + InvalidKwDeployForce(String), #[error("filesystem error: {0}")] Fs(#[from] FileSystemError), } diff --git a/src/config/service.rs b/src/config/service.rs index bad6ab7..b5c0ab6 100644 --- a/src/config/service.rs +++ b/src/config/service.rs @@ -145,6 +145,30 @@ pub(crate) fn validate_update( ), }; + let kw_reboot_after_deploy = match &draft.kw_reboot_after_deploy { + None => None, + Some(s) if s.trim().is_empty() => { + return Err(ConfigError::InvalidKwRebootAfterDeploy(s.clone())); + } + Some(s) => Some( + s.trim() + .parse::() + .map_err(|_| ConfigError::InvalidKwRebootAfterDeploy(s.clone()))?, + ), + }; + + let kw_deploy_force = match &draft.kw_deploy_force { + None => None, + Some(s) if s.trim().is_empty() => { + return Err(ConfigError::InvalidKwDeployForce(s.clone())); + } + Some(s) => Some( + s.trim() + .parse::() + .map_err(|_| ConfigError::InvalidKwDeployForce(s.clone()))?, + ), + }; + Ok(ValidatedConfigUpdate { page_size, cache_dir, @@ -155,6 +179,8 @@ pub(crate) fn validate_update( cover_renderer, max_log_age, stay_on_applied_branch, + kw_reboot_after_deploy, + kw_deploy_force, }) } diff --git a/src/config/state.rs b/src/config/state.rs index 89341ab..b5e7c5f 100644 --- a/src/config/state.rs +++ b/src/config/state.rs @@ -39,6 +39,10 @@ pub struct ConfigState { pub(crate) git_am_options: String, pub(crate) git_am_branch_prefix: String, pub(crate) stay_on_applied_branch: bool, + /// When false (the default), `kw deploy` is invoked with `--no-reboot`. + pub(crate) kw_reboot_after_deploy: bool, + /// When true (the default), `kw deploy` is invoked with `--force`. + pub(crate) kw_deploy_force: bool, } impl Default for ConfigState { @@ -81,6 +85,8 @@ impl ConfigState { git_am_options: String::new(), git_am_branch_prefix: String::from("patchset-"), stay_on_applied_branch: true, + kw_reboot_after_deploy: false, + kw_deploy_force: true, } } @@ -130,6 +136,14 @@ impl ConfigState { self.stay_on_applied_branch = stay_on_applied_branch; } + fn set_kw_reboot_after_deploy(&mut self, kw_reboot_after_deploy: bool) { + self.kw_reboot_after_deploy = kw_reboot_after_deploy; + } + + fn set_kw_deploy_force(&mut self, kw_deploy_force: bool) { + self.kw_deploy_force = kw_deploy_force; + } + /// Merges validated field updates from the edit-config flow. pub fn apply_update(&mut self, u: &ValidatedConfigUpdate) { if let Some(page_size) = u.page_size { @@ -159,6 +173,12 @@ impl ConfigState { if let Some(stay_on_applied_branch) = u.stay_on_applied_branch { self.set_stay_on_applied_branch(stay_on_applied_branch); } + if let Some(kw_reboot_after_deploy) = u.kw_reboot_after_deploy { + self.set_kw_reboot_after_deploy(kw_reboot_after_deploy); + } + if let Some(kw_deploy_force) = u.kw_deploy_force { + self.set_kw_deploy_force(kw_deploy_force); + } } pub fn to_snapshot(&self) -> ConfigSnapshot { @@ -199,6 +219,8 @@ pub struct ConfigSnapshot { git_am_options: String, git_am_branch_prefix: String, stay_on_applied_branch: bool, + kw_reboot_after_deploy: bool, + kw_deploy_force: bool, } impl ConfigSnapshot { @@ -221,6 +243,8 @@ impl ConfigSnapshot { git_am_options: s.git_am_options.clone(), git_am_branch_prefix: s.git_am_branch_prefix.clone(), stay_on_applied_branch: s.stay_on_applied_branch, + kw_reboot_after_deploy: s.kw_reboot_after_deploy, + kw_deploy_force: s.kw_deploy_force, } } diff --git a/src/config/tests.rs b/src/config/tests.rs index d60eb82..220db04 100644 --- a/src/config/tests.rs +++ b/src/config/tests.rs @@ -144,6 +144,8 @@ fn bootstrap_with_default_values() { assert_eq!("", config.git_am_options().as_str()); assert_eq!("patchset-", config.git_am_branch_prefix().as_str()); assert!(config.stay_on_applied_branch()); + assert!(!config.kw_reboot_after_deploy()); + assert!(config.kw_deploy_force()); } #[test] @@ -230,6 +232,9 @@ fn bootstrap_with_config_file() { config.git_am_branch_prefix().as_str() ); assert!(!config.stay_on_applied_branch()); + // The fixture predates these knobs: missing keys keep the compiled-in defaults. + assert!(!config.kw_reboot_after_deploy()); + assert!(config.kw_deploy_force()); } #[test] @@ -375,6 +380,8 @@ fn deserialize_config_state_with_missing_field() { // Missing fields fall back to the compiled-in defaults; in particular the // kw-integration apply toggle defaults to staying on the applied branch. assert!(state.stay_on_applied_branch()); + assert!(!state.kw_reboot_after_deploy()); + assert!(state.kw_deploy_force()); } #[test] @@ -575,6 +582,60 @@ fn apply_update_toggles_stay_on_applied_branch() { assert!(!state.stay_on_applied_branch()); } +#[test] +fn validate_update_rejects_invalid_kw_deploy_bools() { + for (reboot, force, expect_reboot_err) in [ + (Some("not-a-bool"), None, true), + (Some(""), None, true), + (None, Some("not-a-bool"), false), + (None, Some(""), false), + ] { + let err = validate_update( + ConfigUpdateDraft { + kw_reboot_after_deploy: reboot.map(str::to_string), + kw_deploy_force: force.map(str::to_string), + ..Default::default() + }, + &os_fs(), + ) + .unwrap_err(); + if expect_reboot_err { + let raw = reboot.unwrap(); + assert!( + matches!(err, ConfigError::InvalidKwRebootAfterDeploy(ref s) if s == raw), + "unexpected error for reboot={reboot:?}: {err:?}" + ); + } else { + let raw = force.unwrap(); + assert!( + matches!(err, ConfigError::InvalidKwDeployForce(ref s) if s == raw), + "unexpected error for force={force:?}: {err:?}" + ); + } + } +} + +#[test] +fn apply_update_toggles_kw_deploy_knobs() { + let (env, _home) = default_env(); + let mut state = ConfigState::new_with_defaults(&env); + assert!(!state.kw_reboot_after_deploy()); + assert!(state.kw_deploy_force()); + + state.apply_update(&ValidatedConfigUpdate { + kw_reboot_after_deploy: Some(true), + kw_deploy_force: Some(false), + ..Default::default() + }); + assert!(state.kw_reboot_after_deploy()); + assert!(!state.kw_deploy_force()); + + // A draft that omits the fields leaves the current values untouched. + state.apply_update(&ValidatedConfigUpdate::default()); + assert!(state.kw_reboot_after_deploy()); + assert!(!state.kw_deploy_force()); +} + #[test] fn validate_update_rejects_cache_dir_that_is_existing_file() { let root = unique_test_dir("not-a-dir"); diff --git a/src/config/update.rs b/src/config/update.rs index 995a9c1..44b14dc 100644 --- a/src/config/update.rs +++ b/src/config/update.rs @@ -12,6 +12,8 @@ pub struct ConfigUpdateDraft { pub cover_renderer: Option, pub max_log_age: Option, pub stay_on_applied_branch: Option, + pub kw_reboot_after_deploy: Option, + pub kw_deploy_force: Option, } /// Parsed and validated update ready to merge into [`crate::config::ConfigState`](super::state::ConfigState). @@ -26,4 +28,6 @@ pub struct ValidatedConfigUpdate { pub cover_renderer: Option, pub max_log_age: Option, pub stay_on_applied_branch: Option, + pub kw_reboot_after_deploy: Option, + pub kw_deploy_force: Option, } From a508a5a44484464891a1ea1d3694800cf540bee3 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sat, 19 Sep 2026 20:40:35 -0300 Subject: [PATCH 05/12] feat(kw): probe deploy remote and boot-once state in readiness This commit extends GetReadiness so KwOps can judge deploy-alone against the typed branch while still reporting real HEAD, and snapshots the resolved --remote target plus boot-into-new-kernel-once. Only literal `no` is Off; a missing or unknown value is a confirm gate, because kw has no CLI off-switch. This commit is part of the kw integration's step 7. Signed-off-by: lorenzoberts --- src/app/flows/kw_ops.rs | 8 +- src/app/screens/kw_ops.rs | 6 +- src/app/view_model.rs | 4 + src/kw/actor.rs | 71 ++++++++- src/kw/handle.rs | 2 + src/kw/messages.rs | 3 + src/kw/readiness.rs | 309 ++++++++++++++++++++++++++++++++++++-- src/kw/remote.rs | 6 +- 8 files changed, 390 insertions(+), 19 deletions(-) diff --git a/src/app/flows/kw_ops.rs b/src/app/flows/kw_ops.rs index 8897ccf..fdd96f4 100644 --- a/src/app/flows/kw_ops.rs +++ b/src/app/flows/kw_ops.rs @@ -197,7 +197,7 @@ pub async fn open_kw_ops(app: &mut App) -> Result<()> { return Ok(()); }; - match kw.get_readiness(&kernel_tree_id, &tree).await { + match kw.get_readiness(&kernel_tree_id, &tree, None).await { Ok(readiness) => { let reuse = app.state.kw.ops.as_ref().is_some_and(|ops| { ops.message_id == message_id && ops.kernel_tree_id == kernel_tree_id @@ -371,8 +371,10 @@ mod tests { use super::*; use crate::app::screens::kw_ops::KwOpsState; use crate::kw::readiness::{ - DeployAloneRefusal, KwBinaryProbe, KwReadiness, KwVersionCheck, TreeReadiness, + BootOnceState, DeployAloneRefusal, KwBinaryProbe, KwReadiness, KwVersionCheck, + TreeReadiness, }; + use crate::kw::remote::RemoteRefusal; fn sample_ops() -> KwOpsState { KwOpsState::new( @@ -399,6 +401,8 @@ mod tests { latest_build: None, deploy_alone: Err(DeployAloneRefusal::NoBuildRecord), current_branch: Some("main".to_string()), + deploy_remote: Err(RemoteRefusal::NoRemotesConfigured), + boot_once: BootOnceState::Unknown, }, ) } diff --git a/src/app/screens/kw_ops.rs b/src/app/screens/kw_ops.rs index 21be8e2..ef1d7ab 100644 --- a/src/app/screens/kw_ops.rs +++ b/src/app/screens/kw_ops.rs @@ -138,8 +138,10 @@ fn split_extra_args(raw: &str) -> Vec { mod tests { use super::*; use crate::kw::readiness::{ - DeployAloneRefusal, KwBinaryProbe, KwReadiness, KwVersionCheck, TreeReadiness, + BootOnceState, DeployAloneRefusal, KwBinaryProbe, KwReadiness, KwVersionCheck, + TreeReadiness, }; + use crate::kw::remote::RemoteRefusal; fn sample_tree() -> KernelTree { serde_json::from_value(serde_json::json!({ @@ -165,6 +167,8 @@ mod tests { latest_build: None, deploy_alone: Err(DeployAloneRefusal::NoBuildRecord), current_branch: branch.map(str::to_string), + deploy_remote: Err(RemoteRefusal::NoRemotesConfigured), + boot_once: BootOnceState::Unknown, } } diff --git a/src/app/view_model.rs b/src/app/view_model.rs index 840b279..ad1a42d 100644 --- a/src/app/view_model.rs +++ b/src/app/view_model.rs @@ -761,6 +761,8 @@ mod tests { latest_build: None, deploy_alone: Err(crate::kw::readiness::DeployAloneRefusal::NoBuildRecord), current_branch: Some("feature".to_string()), + deploy_remote: Err(crate::kw::remote::RemoteRefusal::NoRemotesConfigured), + boot_once: crate::kw::readiness::BootOnceState::Unknown, }, ); ops.extra_args = "--verbose --clean --from-sha abc --doc".to_string(); @@ -800,6 +802,8 @@ mod tests { latest_build: None, deploy_alone: Err(crate::kw::readiness::DeployAloneRefusal::NoBuildRecord), current_branch: branch.map(str::to_string), + deploy_remote: Err(crate::kw::remote::RemoteRefusal::NoRemotesConfigured), + boot_once: crate::kw::readiness::BootOnceState::Unknown, }, ) } diff --git a/src/kw/actor.rs b/src/kw/actor.rs index 5415e58..d9eef8a 100644 --- a/src/kw/actor.rs +++ b/src/kw/actor.rs @@ -217,12 +217,14 @@ impl KwActor { KwMessage::GetReadiness { kernel_tree_id, tree, + for_branch, reply, } => { send_kw_reply( message_name, reply, - self.evaluate_readiness(&kernel_tree_id, &tree).await, + self.evaluate_readiness(&kernel_tree_id, &tree, for_branch) + .await, ); ControlFlow::Continue(()) } @@ -613,6 +615,7 @@ impl KwActor { &self, kernel_tree_id: &str, tree: &KernelTree, + for_branch: Option, ) -> Result { let fs = Arc::clone(&self.fs); let env = Arc::clone(&self.env); @@ -630,6 +633,7 @@ impl KwActor { &kernel_tree_id, &tree, &head, + for_branch.as_deref(), ) .map_err(KwError::from) }) @@ -906,7 +910,8 @@ mod tests { errors::KwStartError, history::{KwApplyRecord, KwBuildRecord, MockKwHistoryStore}, messages::StartRequest, - readiness::{DeployAloneRefusal, TreeReadiness}, + readiness::{BootOnceState, DeployAloneRefusal, TreeReadiness}, + remote::RemoteRefusal, status::{KwJobKind, KwJobStatus, KwPhase}, }, }; @@ -1329,6 +1334,8 @@ mod tests { env.expect_which() .withf(|name| name == "kw") .returning(|_| false); + env.expect_var() + .returning(|_| Err(std::env::VarError::NotPresent.into())); let mut fs = MockFileSystemTrait::new(); fs.expect_is_file().returning(|_| false); fs.expect_is_dir().returning(|_| false); @@ -1361,7 +1368,7 @@ mod tests { }); let handle = spawn_test_actor("readiness", history, shell, fs, env); - let readiness = handle.get_readiness("mainline", &tree).await.unwrap(); + let readiness = handle.get_readiness("mainline", &tree, None).await.unwrap(); assert!(!readiness.kw_binary.available); assert_eq!(TreeReadiness::Missing, readiness.tree); @@ -1369,6 +1376,64 @@ mod tests { Err(DeployAloneRefusal::TreeNotReady(TreeReadiness::Missing)), readiness.deploy_alone ); + assert_eq!(Some("for-next".to_string()), readiness.current_branch); + assert_eq!( + Err(RemoteRefusal::NoRemotesConfigured), + readiness.deploy_remote + ); + assert_eq!(BootOnceState::Unknown, readiness.boot_once); + handle.shutdown().await; + } + + #[tokio::test] + async fn get_readiness_for_branch_looks_up_that_branch_not_head() { + let tree = kernel_tree(Path::new("/home/user/linux")); + + let mut env = MockEnvTrait::new(); + env.expect_which() + .withf(|name| name == "kw") + .returning(|_| false); + env.expect_var() + .returning(|_| Err(std::env::VarError::NotPresent.into())); + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_file().returning(|_| false); + fs.expect_is_dir().returning(|_| false); + fs.expect_read_dir().returning(|_| { + Err(FileSystemError::IoError(io::Error::new( + io::ErrorKind::NotFound, + "missing", + ))) + }); + let mut history = MockKwHistoryStore::new(); + history + .expect_build_records() + .withf(|kernel_tree_id, branch| kernel_tree_id == "mainline" && branch == "patchset-x") + .times(1) + .returning(|_, _| Ok((None, None))); + let mut shell = MockShellTrait::new(); + shell + .expect_execute() + .withf(|cmd| { + cmd.program == "git" + && cmd.args == ["-C", "/home/user/linux", "branch", "--show-current"] + }) + .times(1) + .returning(|_| { + Ok(ShellOutput { + stdout: b"master\n".to_vec(), + stderr: Vec::new(), + success: true, + }) + }); + let handle = spawn_test_actor("readiness-for-branch", history, shell, fs, env); + + let readiness = handle + .get_readiness("mainline", &tree, Some("patchset-x")) + .await + .unwrap(); + + assert_eq!(Some("master".to_string()), readiness.current_branch); + assert_eq!(None, readiness.build_record); handle.shutdown().await; } diff --git a/src/kw/handle.rs b/src/kw/handle.rs index 9530f5e..ad6ed04 100644 --- a/src/kw/handle.rs +++ b/src/kw/handle.rs @@ -61,10 +61,12 @@ impl KwHandle { &self, kernel_tree_id: &str, tree: &KernelTree, + for_branch: Option<&str>, ) -> Result { self.request_result(|reply| KwMessage::GetReadiness { kernel_tree_id: kernel_tree_id.to_string(), tree: tree.clone(), + for_branch: for_branch.map(str::to_string), reply, }) .await diff --git a/src/kw/messages.rs b/src/kw/messages.rs index 44a067f..5409dba 100644 --- a/src/kw/messages.rs +++ b/src/kw/messages.rs @@ -64,6 +64,9 @@ pub enum KwMessage { GetReadiness { kernel_tree_id: String, tree: KernelTree, + /// When set, deploy-alone is judged against this branch instead of + /// HEAD. `KwReadiness::current_branch` still reports the real HEAD. + for_branch: Option, reply: oneshot::Sender>, }, RestorePreviousBranch { diff --git a/src/kw/readiness.rs b/src/kw/readiness.rs index cc8d248..09fd725 100644 --- a/src/kw/readiness.rs +++ b/src/kw/readiness.rs @@ -27,7 +27,10 @@ use crate::infrastructure::{ }; use crate::{ config::KernelTree, - kw::history::{KwBuildRecord, KwHistoryStore}, + kw::{ + history::{KwBuildRecord, KwHistoryStore}, + remote::{self, KwRemote, RemoteRefusal}, + }, }; /// Errors from readiness probes for states where "absent" is not a normal @@ -439,10 +442,76 @@ pub enum DeployAloneRefusal { ImageMissing, } +/// Whether `.kw/deploy.config` (then the user-level copy) sets +/// `boot_into_new_kernel_once=no`. +/// +/// kw only treats the literal value `no` as off (`src/deploy.sh`); any other +/// value, including a missing key, leaves the option on. [`Unknown`] is +/// therefore a confirm-to-proceed gate, same as [`On`]: patch-hub cannot +/// pass a CLI off-switch. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BootOnceState { + Off, + On, + Unknown, +} + +/// Reads `boot_into_new_kernel_once` from `/.kw/deploy.config`, then +/// `${XDG_CONFIG_HOME:-$HOME/.config}/kw/deploy.config`. A present but +/// unreadable tree file is [`BootOnceState::Unknown`] rather than a guess +/// at the home copy. A readable tree file that simply omits the key still +/// falls through, matching kw's merged-config lookup. +pub fn probe_boot_once( + fs: &dyn FileSystemTrait, + env: &dyn EnvTrait, + tree_path: &Path, +) -> BootOnceState { + let local = tree_path.join(".kw").join("deploy.config"); + if fs.is_file(&local) { + match boot_once_from_file(fs, &local) { + Err(()) => return BootOnceState::Unknown, + Ok(Some(state)) => return state, + Ok(None) => {} + } + } + let Some(global) = xdg_kw_config_file(env, "deploy.config") else { + return BootOnceState::Unknown; + }; + if !fs.is_file(&global) { + return BootOnceState::Unknown; + } + match boot_once_from_file(fs, &global) { + Ok(Some(state)) => state, + Ok(None) | Err(()) => BootOnceState::Unknown, + } +} + +fn boot_once_from_file(fs: &dyn FileSystemTrait, path: &Path) -> Result, ()> { + let content = fs.read_to_string(path).map_err(|_| ())?; + Ok(parse_kw_config(&content) + .remove("boot_into_new_kernel_once") + .map(|value| match value.as_str() { + "no" => BootOnceState::Off, + "yes" => BootOnceState::On, + _ => BootOnceState::Unknown, + })) +} + +/// `${XDG_CONFIG_HOME:-$HOME/.config}/kw/`. A set-but-empty +/// `XDG_CONFIG_HOME` is treated as unset, matching bash `:-` and the XDG +/// spec. +fn xdg_kw_config_file(env: &dyn EnvTrait, filename: &str) -> Option { + let config_home = match env.var("XDG_CONFIG_HOME") { + Ok(xdg) if !xdg.is_empty() => xdg, + _ => format!("{}/.config", env.var("HOME").ok()?), + }; + Some(Path::new(&config_home).join("kw").join(filename)) +} + /// Deploy-alone readiness gate: a deploy without a preceding build is only -/// allowed when a successful build record exists for the tree and current -/// HEAD, written against the same tree path and kw env, and a kernel image -/// is still discoverable. +/// allowed when a successful build record exists for the tree and the +/// lookup branch, written against the same tree path and kw env, and a +/// kernel image is still discoverable. /// /// This is only the record-matching half of the gate — it says nothing /// about the tree's *current* state. [`evaluate_readiness`] conjoins @@ -494,7 +563,7 @@ pub struct KwReadiness { pub output_dir: Option, /// Newest discoverable kernel image under the build root, if any. pub kernel_image: Option, - /// Build record for `(kernel_tree_id, head_branch)`, if any. + /// Build record for `(kernel_tree_id, lookup_branch)`, if any. pub build_record: Option, /// Newest build record for the tree across branches, even when HEAD /// has none. @@ -505,11 +574,18 @@ pub struct KwReadiness { /// Currently checked-out branch. `None` when HEAD is detached or /// `git branch --show-current` could not be read. pub current_branch: Option, + /// Resolved `--remote` target, or why none could be chosen. + pub deploy_remote: Result, + pub boot_once: BootOnceState, } /// Runs all readiness probes for `tree` and composes them into a /// [`KwReadiness`] snapshot. `head_branch` is the tree's current branch — /// resolving it (via git) is the caller's job, keeping these probes pure. +/// +/// `for_branch`, when set, is the branch deploy-alone should be judged +/// against (the branch typed on KwOps). `current_branch` still reports +/// the real HEAD so the UI can show both. // The only caller is the unix-only actor's GetReadiness. #[cfg_attr(not(unix), allow(dead_code))] pub fn evaluate_readiness( @@ -520,6 +596,7 @@ pub fn evaluate_readiness( kernel_tree_id: &str, tree: &KernelTree, head_branch: &str, + for_branch: Option<&str>, ) -> Result { let tree_path = Path::new(tree.path()); let kw_binary = probe_kw_binary(env, shell); @@ -534,7 +611,14 @@ pub fn evaluate_readiness( output_dir.as_deref().unwrap_or(tree_path), arch.as_deref(), ); - let (build_record, latest_build) = history.build_records(kernel_tree_id, head_branch)?; + let lookup_branch = match for_branch + .map(str::trim) + .filter(|branch| !branch.is_empty()) + { + Some(branch) => branch, + None => head_branch, + }; + let (build_record, latest_build) = history.build_records(kernel_tree_id, lookup_branch)?; // The tree's current state is part of the verdict: a stale image and a // matching record must not green-light a deploy on a tree that has // since lost its .config, .kw/, or kernel-root files. @@ -542,7 +626,7 @@ pub fn evaluate_readiness( TreeReadiness::Ready { .. } => check_deploy_alone( build_record.as_ref(), tree, - head_branch, + lookup_branch, output_dir.as_deref(), kernel_image.as_deref(), ), @@ -564,6 +648,8 @@ pub fn evaluate_readiness( Some(trimmed.to_string()) } }, + deploy_remote: remote::resolve_deploy_remote(fs, env, tree_path), + boot_once: probe_boot_once(fs, env, tree_path), }) } @@ -1315,6 +1401,16 @@ last_line_without_newline=yes"; fn evaluate_readiness_composes_all_probes() { let dir = make_ready_tree("evaluate"); fs::write(dir.path().join(".kw/build.config"), "arch=x86\n").unwrap(); + fs::write( + dir.path().join(".kw/deploy.config"), + "boot_into_new_kernel_once=no\n", + ) + .unwrap(); + fs::write( + dir.path().join(".kw/remote.config"), + "#kw-default=dut\nHost dut\n Hostname box\n Port 22\n User root\n", + ) + .unwrap(); let boot = dir.path().join("arch/x86/boot"); fs::create_dir_all(&boot).unwrap(); write_file_with_mtime(&boot.join("bzImage"), 100); @@ -1343,6 +1439,7 @@ last_line_without_newline=yes"; "mainline", &tree, "patchset-x", + None, ) .unwrap(); @@ -1360,6 +1457,8 @@ last_line_without_newline=yes"; assert!(readiness.kw_binary.available); assert_eq!(KwVersionCheck::Meets, readiness.kw_binary.check); assert_eq!(Some("patchset-x".to_string()), readiness.current_branch); + assert_eq!("root@box:22", readiness.deploy_remote.unwrap().endpoint()); + assert_eq!(BootOnceState::Off, readiness.boot_once); } #[test] @@ -1374,6 +1473,8 @@ last_line_without_newline=yes"; let mut env = MockEnvTrait::new(); env.expect_which().returning(|_| false); + env.expect_var() + .returning(|_| Err(std::env::VarError::NotPresent.into())); let mut shell = MockShellTrait::new(); shell.expect_execute().times(0); @@ -1386,6 +1487,7 @@ last_line_without_newline=yes"; "mainline", &tree, "patchset-x", + None, ) .unwrap(); @@ -1399,6 +1501,11 @@ last_line_without_newline=yes"; Err(DeployAloneRefusal::TreeNotReady(TreeReadiness::Missing)), readiness.deploy_alone ); + assert_eq!( + Err(RemoteRefusal::NoRemotesConfigured), + readiness.deploy_remote + ); + assert_eq!(BootOnceState::Unknown, readiness.boot_once); } #[test] @@ -1412,6 +1519,8 @@ last_line_without_newline=yes"; let mut env = MockEnvTrait::new(); env.expect_which().returning(|_| false); + env.expect_var() + .returning(|_| Err(std::env::VarError::NotPresent.into())); let mut shell = MockShellTrait::new(); shell.expect_execute().times(0); @@ -1424,6 +1533,7 @@ last_line_without_newline=yes"; "mainline", &tree, "patchset-x", + None, ) .unwrap(); @@ -1438,6 +1548,7 @@ last_line_without_newline=yes"; ); assert!(!readiness.kw_binary.available); assert_eq!(Some("patchset-x".to_string()), readiness.current_branch); + assert_eq!(BootOnceState::Unknown, readiness.boot_once); } #[test] @@ -1450,12 +1561,190 @@ last_line_without_newline=yes"; ); let mut env = MockEnvTrait::new(); env.expect_which().returning(|_| false); + env.expect_var() + .returning(|_| Err(std::env::VarError::NotPresent.into())); let mut shell = MockShellTrait::new(); shell.expect_execute().times(0); let tree = kernel_tree(dir.path()); - let readiness = - evaluate_readiness(&OsFileSystem, &env, &shell, &history, "mainline", &tree, "") - .unwrap(); + let readiness = evaluate_readiness( + &OsFileSystem, + &env, + &shell, + &history, + "mainline", + &tree, + "", + None, + ) + .unwrap(); assert_eq!(None, readiness.current_branch); } + + #[test] + fn evaluate_readiness_for_branch_looks_up_that_branch_not_head() { + let dir = make_ready_tree("evaluate-for-branch"); + let data = TempDir::new("evaluate-for-branch-data"); + let history = FileKwHistoryStore::new( + Arc::new(OsFileSystem), + data.path().to_str().unwrap().to_string(), + ); + let record = built_record(dir.path(), "patchset-x"); + history.record_build(record.clone()).unwrap(); + let boot = dir.path().join("arch/x86/boot"); + fs::create_dir_all(&boot).unwrap(); + write_file_with_mtime(&boot.join("bzImage"), 100); + fs::write(dir.path().join(".kw/build.config"), "arch=x86\n").unwrap(); + + let mut env = MockEnvTrait::new(); + env.expect_which().returning(|_| false); + env.expect_var() + .returning(|_| Err(std::env::VarError::NotPresent.into())); + let mut shell = MockShellTrait::new(); + shell.expect_execute().times(0); + let tree = kernel_tree(dir.path()); + + let on_head = evaluate_readiness( + &OsFileSystem, + &env, + &shell, + &history, + "mainline", + &tree, + "master", + None, + ) + .unwrap(); + assert_eq!(Some("master".to_string()), on_head.current_branch); + assert_eq!(None, on_head.build_record); + assert_eq!(Err(DeployAloneRefusal::NoBuildRecord), on_head.deploy_alone); + + let for_typed = evaluate_readiness( + &OsFileSystem, + &env, + &shell, + &history, + "mainline", + &tree, + "master", + Some("patchset-x"), + ) + .unwrap(); + // HEAD is still master; deploy-alone is judged against the typed branch. + assert_eq!(Some("master".to_string()), for_typed.current_branch); + assert_eq!(Some(record), for_typed.build_record); + assert_eq!(Ok(()), for_typed.deploy_alone); + } + + #[test] + fn probe_boot_once_reads_literal_no_and_yes() { + let off = make_ready_tree("boot-once-off"); + fs::write( + off.path().join(".kw/deploy.config"), + "boot_into_new_kernel_once=no\n", + ) + .unwrap(); + let env = MockEnvTrait::new(); + assert_eq!( + BootOnceState::Off, + probe_boot_once(&OsFileSystem, &env, off.path()) + ); + + let on = make_ready_tree("boot-once-on"); + fs::write( + on.path().join(".kw/deploy.config"), + "boot_into_new_kernel_once=yes\n", + ) + .unwrap(); + assert_eq!( + BootOnceState::On, + probe_boot_once(&OsFileSystem, &env, on.path()) + ); + } + + #[test] + fn probe_boot_once_unknown_values_and_missing_files_gate() { + let empty = make_ready_tree("boot-once-missing"); + let mut env = MockEnvTrait::new(); + env.expect_var() + .returning(|_| Err(std::env::VarError::NotPresent.into())); + assert_eq!( + BootOnceState::Unknown, + probe_boot_once(&OsFileSystem, &env, empty.path()) + ); + + let weird = make_ready_tree("boot-once-weird"); + fs::write( + weird.path().join(".kw/deploy.config"), + "boot_into_new_kernel_once=true\n", + ) + .unwrap(); + assert_eq!( + BootOnceState::Unknown, + probe_boot_once(&OsFileSystem, &MockEnvTrait::new(), weird.path()) + ); + } + + #[test] + fn probe_boot_once_falls_through_to_xdg_when_the_tree_omits_the_key() { + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_file().returning(|path| { + path.ends_with(".kw/deploy.config") || path == Path::new("/xdg/kw/deploy.config") + }); + fs.expect_read_to_string().returning(|path| { + if path.ends_with(".kw/deploy.config") { + Ok("reboot=no\n".to_string()) + } else { + Ok("boot_into_new_kernel_once=no\n".to_string()) + } + }); + let mut env = MockEnvTrait::new(); + env.expect_var() + .withf(|key| key == "XDG_CONFIG_HOME") + .returning(|_| Ok("/xdg".to_string())); + + assert_eq!( + BootOnceState::Off, + probe_boot_once(&fs, &env, Path::new("/kernel")) + ); + } + + #[test] + fn probe_boot_once_unreadable_tree_file_does_not_fall_through() { + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_file() + .returning(|path| path.ends_with(".kw/deploy.config")); + fs.expect_read_to_string().returning(|_| { + Err(FileSystemError::IoError(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "denied", + ))) + }); + let env = MockEnvTrait::new(); + + assert_eq!( + BootOnceState::Unknown, + probe_boot_once(&fs, &env, Path::new("/kernel")) + ); + } + + #[test] + fn probe_boot_once_treats_empty_xdg_config_home_as_unset() { + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_file() + .returning(|path| path == Path::new("/home/user/.config/kw/deploy.config")); + fs.expect_read_to_string() + .returning(|_| Ok("boot_into_new_kernel_once=no\n".to_string())); + let mut env = MockEnvTrait::new(); + env.expect_var() + .withf(|key| key == "XDG_CONFIG_HOME") + .returning(|_| Ok(String::new())); + env.expect_var() + .withf(|key| key == "HOME") + .returning(|_| Ok("/home/user".to_string())); + + assert_eq!( + BootOnceState::Off, + probe_boot_once(&fs, &env, Path::new("/kernel")) + ); + } } diff --git a/src/kw/remote.rs b/src/kw/remote.rs index 566016e..93a04b9 100644 --- a/src/kw/remote.rs +++ b/src/kw/remote.rs @@ -12,10 +12,10 @@ //! to the home copy would silently deploy to a different machine than the //! tree is configured for. //! -//! Production callers land with the deploy start path; until then this -//! module is exercised by its tests. +//! Production callers are readiness probes and the unix-only deploy start +//! path. -#![cfg_attr(not(test), allow(dead_code))] +#![cfg_attr(not(unix), allow(dead_code))] use std::path::{Path, PathBuf}; From 9729606e5e246f3e207705116f358abdefa7e115 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sat, 19 Sep 2026 20:47:19 -0300 Subject: [PATCH 06/12] feat(kw): run deploy-only jobs gated on build history This commit starts kw deploy through the same checkout policy as build, then refuses unless a remote resolves, boot-once is off or acknowledged, and a matching successful build record still has an image. A deploy never writes a build record; a post-switch refusal rolls HEAD back so a failed start cannot leave the tree on the typed branch. This commit is part of the kw integration's step 7. Signed-off-by: lorenzoberts --- src/app/flows/kw_ops.rs | 1 + src/app/integration_tests/kw_status.rs | 1 + src/app/integration_tests/patchset_actions.rs | 1 + src/kw/actor.rs | 567 +++++++++++++++++- src/kw/argv.rs | 2 +- src/kw/errors.rs | 13 +- src/kw/messages.rs | 16 +- src/kw/status.rs | 2 - 8 files changed, 577 insertions(+), 26 deletions(-) diff --git a/src/app/flows/kw_ops.rs b/src/app/flows/kw_ops.rs index fdd96f4..34f08ec 100644 --- a/src/app/flows/kw_ops.rs +++ b/src/app/flows/kw_ops.rs @@ -257,6 +257,7 @@ async fn start_build(app: &mut App) -> Result<()> { tree: ops.tree.clone(), branch, extra_args: ops.extra_arg_tokens(), + deploy: None, }; match kw.start_build(request).await { Ok(()) => { diff --git a/src/app/integration_tests/kw_status.rs b/src/app/integration_tests/kw_status.rs index 1616fde..5026b12 100644 --- a/src/app/integration_tests/kw_status.rs +++ b/src/app/integration_tests/kw_status.rs @@ -367,6 +367,7 @@ fn start_request() -> StartRequest { .expect("kernel tree should deserialize"), branch: BUILD_BRANCH.to_string(), extra_args: Vec::new(), + deploy: None, } } diff --git a/src/app/integration_tests/patchset_actions.rs b/src/app/integration_tests/patchset_actions.rs index caeeeed..e5fd293 100644 --- a/src/app/integration_tests/patchset_actions.rs +++ b/src/app/integration_tests/patchset_actions.rs @@ -739,6 +739,7 @@ fn kw_start_request() -> StartRequest { .expect("kernel tree should deserialize"), branch: "patchset-2026-08-20-15-00-00".to_string(), extra_args: Vec::new(), + deploy: None, } } diff --git a/src/kw/actor.rs b/src/kw/actor.rs index d9eef8a..cb888a8 100644 --- a/src/kw/actor.rs +++ b/src/kw/actor.rs @@ -38,8 +38,9 @@ use crate::{ errors::{KwError, KwStartError, TreeGitError}, handle::KwHandle, history::{KwBuildRecord, KwHistoryStore}, - messages::{KwMessage, StartRequest}, - readiness::{self, KwReadiness, KwVersionCheck, TreeReadiness}, + messages::{DeployOptions, KwMessage, StartRequest}, + readiness::{self, BootOnceState, KwReadiness, KwVersionCheck, TreeReadiness}, + remote::{self, KwRemote}, status::{KwJobKind, KwJobStatus, KwPhase, KwStatusSnapshot}, }, }; @@ -196,9 +197,16 @@ impl KwActor { ); ControlFlow::Continue(()) } + KwMessage::StartDeploy { request, reply } => { + send_start_reply( + message_name, + reply, + self.start_job(KwJobKind::Deploy, request).await, + ); + ControlFlow::Continue(()) + } // Not implemented: reply immediately with NotImplemented. - KwMessage::StartDeploy { reply, .. } - | KwMessage::StartBuildThenDeploy { reply, .. } => { + KwMessage::StartBuildThenDeploy { reply, .. } => { send_start_reply(message_name, reply, Err(KwStartError::NotImplemented)); ControlFlow::Continue(()) } @@ -260,13 +268,15 @@ impl KwActor { } } - /// Accepts and starts a build job, or refuses. The reply is sent by the + /// Accepts and starts a job, or refuses. The reply is sent by the /// caller right after this returns: the job itself keeps running in a /// detached task and is observed via the status snapshot. /// /// Refusals, in order: a job already running, no kw binary on PATH, /// unresolvable kw-env state, a tree that fails the readiness probes, - /// a dirty worktree, or a failed branch switch. Reserved flags on + /// a dirty worktree, or a failed branch switch. Deploy jobs then also + /// refuse on an unresolved remote, an unacknowledged boot-once + /// setting, or a deploy-alone record mismatch. Reserved flags on /// patch-hub's own argv win over the request's extra args. A start /// refused after the branch switch rolls the switch back: a refused /// start never leaves the tree on a branch the user did not check out. @@ -308,7 +318,29 @@ impl KwActor { let pre_job_branch = self.checkout_build_branch(&request).await?; - let (process, log_path) = match self.spawn_build_process(&request) { + let remote = if kind == KwJobKind::Deploy { + match self.gate_deploy(&request, &tree_path, output_dir.as_deref(), arch.as_deref()) { + Ok(remote) => Some(remote), + Err(error) => { + self.rollback_switch(request.tree.path(), pre_job_branch.as_deref()) + .await; + return Err(error); + } + } + } else { + None + }; + + let spawned = match kind { + KwJobKind::Deploy => self.spawn_deploy_process( + &request, + remote + .as_ref() + .expect("gate_deploy returns a remote for Deploy"), + ), + KwJobKind::Build | KwJobKind::BuildThenDeploy => self.spawn_build_process(&request), + }; + let (process, log_path) = match spawned { Ok(spawned) => spawned, Err(error) => { self.rollback_switch(request.tree.path(), pre_job_branch.as_deref()) @@ -320,13 +352,24 @@ impl KwActor { let (cancel_tx, cancel_rx) = oneshot::channel(); spawn(run_job(process, cancel_rx, self.job_event_tx.clone())); - let phase = KwPhase::Building; - tracing::info!( - kernel_tree_id = request.kernel_tree_id, - branch = request.branch, - log_path = %log_path.display(), - "kw build job started" - ); + let phase = match kind { + KwJobKind::Deploy => KwPhase::Deploying, + KwJobKind::Build | KwJobKind::BuildThenDeploy => KwPhase::Building, + }; + match kind { + KwJobKind::Deploy => tracing::info!( + kernel_tree_id = request.kernel_tree_id, + branch = request.branch, + log_path = %log_path.display(), + "kw deploy job started" + ), + KwJobKind::Build | KwJobKind::BuildThenDeploy => tracing::info!( + kernel_tree_id = request.kernel_tree_id, + branch = request.branch, + log_path = %log_path.display(), + "kw build job started" + ), + } // Recorded only on accept. Refused starts never reach here, and — // with their switch rolled back — can neither clobber a previous // job's restore target nor strand the tree on a branch the user @@ -357,6 +400,46 @@ impl KwActor { Ok(()) } + /// Deploy-only gates after the branch switch: resolved remote, + /// boot-once confirm, then the deploy-alone record match against the + /// requested (now checked-out) branch. Any refusal is rolled back by + /// the caller. + fn gate_deploy( + &self, + request: &StartRequest, + tree_path: &Path, + output_dir: Option<&Path>, + arch: Option<&str>, + ) -> Result { + let options = request.deploy.clone().unwrap_or(DeployOptions { + reboot: false, + force: true, + boot_once_acknowledged: false, + }); + let remote = remote::resolve_deploy_remote(&*self.fs, &*self.env, tree_path) + .map_err(KwStartError::RemoteUnresolved)?; + let boot_once = readiness::probe_boot_once(&*self.fs, &*self.env, tree_path); + if matches!(boot_once, BootOnceState::On | BootOnceState::Unknown) + && !options.boot_once_acknowledged + { + return Err(KwStartError::BootOnceNotAcknowledged); + } + let (record, _) = self + .history + .build_records(&request.kernel_tree_id, &request.branch)?; + let image = + readiness::find_newest_kernel_image(&*self.fs, output_dir.unwrap_or(tree_path), arch); + readiness::check_deploy_alone( + record.as_ref(), + &request.tree, + &request.branch, + output_dir, + image.as_deref(), + ) + .map_err(KwStartError::DeployAloneRefused)?; + Ok(remote) + } + /// Refuse a dirty worktree, record HEAD, then `git switch` to the /// requested branch. The git calls run on the blocking pool so the /// accept reply stays immediate. @@ -445,6 +528,34 @@ impl KwActor { Ok((process, log_path)) } + /// Creates the job's log dir and spawns `kw deploy`. Split from + /// `start_job` so a failure here can roll the branch switch back. + fn spawn_deploy_process( + &self, + request: &StartRequest, + remote: &KwRemote, + ) -> Result<(Box, PathBuf), KwStartError> { + self.fs.create_dir_all(&self.kw_log_dir)?; + let log_path = self.kw_log_dir.join(format!( + "deploy-{}.log", + chrono::Utc::now().format("%Y%m%d-%H%M%S-%3f") + )); + let options = request.deploy.clone().unwrap_or(DeployOptions { + reboot: false, + force: true, + boot_once_acknowledged: false, + }); + let cmd = ShellCommand::new("kw").args(argv::deploy_argv( + &remote.endpoint(), + options.reboot, + options.force, + &request.extra_args, + )); + let cwd = PathBuf::from(request.tree.path()); + let process = self.process.spawn(&cmd, &cwd, &log_path)?; + Ok((process, log_path)) + } + fn request_cancel(&mut self) -> Result<(), KwError> { match self.job.as_mut() { Some(job) => { @@ -523,6 +634,9 @@ impl KwActor { /// nothing. History and patchset-link errors are logged, not folded /// into job status — the build's real outcome already reached the user. fn record_build_outcome(&self, job: &JobState, outcome: &JobOutcome) { + if job.kind == KwJobKind::Deploy { + return; + } let success = match outcome { JobOutcome::Exited(exit) => exit.success(), // The exit status is lost: record an honest failure rather @@ -909,7 +1023,7 @@ mod tests { kw::{ errors::KwStartError, history::{KwApplyRecord, KwBuildRecord, MockKwHistoryStore}, - messages::StartRequest, + messages::{DeployOptions, StartRequest}, readiness::{BootOnceState, DeployAloneRefusal, TreeReadiness}, remote::RemoteRefusal, status::{KwJobKind, KwJobStatus, KwPhase}, @@ -947,6 +1061,7 @@ mod tests { tree: kernel_tree(Path::new("/home/user/linux")), branch: "patchset-2026-08-01-17-30-00".to_string(), extra_args: Vec::new(), + deploy: None, } } @@ -1170,6 +1285,118 @@ mod tests { fs } + const DEPLOY_REMOTE_CONFIG: &str = + "#kw-default=dut\nHost dut\n Hostname box\n Port 22\n User root\n"; + const DEPLOY_BOOT_ONCE_OFF: &str = "boot_into_new_kernel_once=no\n"; + const DEPLOY_BOOT_ONCE_ON: &str = "boot_into_new_kernel_once=yes\n"; + + fn deploy_ready_fs() -> MockFileSystemTrait { + deploy_fs(DEPLOY_REMOTE_CONFIG, DEPLOY_BOOT_ONCE_OFF, true) + } + + fn deploy_fs( + remote_config: &'static str, + deploy_config: &'static str, + has_image: bool, + ) -> MockFileSystemTrait { + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_dir().returning(|_| true); + fs.expect_is_file() + .returning(|path| !path.ends_with(".kw/env.current")); + fs.expect_exists().returning(|_| true); + fs.expect_read_to_string().returning(move |path| { + if path.ends_with("build.config") { + Ok("arch=x86\n".to_string()) + } else if path.ends_with("kernel.release") { + Ok("6.17.0\n".to_string()) + } else if path.ends_with("remote.config") { + Ok(remote_config.to_string()) + } else if path.ends_with("deploy.config") { + Ok(deploy_config.to_string()) + } else { + Err(FileSystemError::IoError(io::Error::new( + io::ErrorKind::NotFound, + "missing", + ))) + } + }); + fs.expect_read_dir().returning(move |path| { + if has_image && path.ends_with("arch/x86/boot") { + Ok(vec![PathBuf::from( + "/home/user/linux/arch/x86/boot/bzImage", + )]) + } else { + Err(FileSystemError::IoError(io::Error::new( + io::ErrorKind::NotFound, + "missing", + ))) + } + }); + fs.expect_metadata() + .returning(|_| Err(FileSystemError::IoError(io::Error::other("no metadata")))); + fs.expect_create_dir_all().returning(|_| Ok(())); + fs + } + + fn matching_build_record() -> KwBuildRecord { + KwBuildRecord { + kernel_tree_id: "mainline".to_string(), + tree_path: "/home/user/linux".to_string(), + message_id: None, + branch: "patchset-2026-08-01-17-30-00".to_string(), + arch: Some("x86".to_string()), + image_path: Some("/home/user/linux/arch/x86/boot/bzImage".to_string()), + output_dir: None, + kernelrelease: Some("6.17.0".to_string()), + log_path: String::new(), + built_at: "2026-08-01T18:10:00Z".to_string(), + success: true, + } + } + + fn deploy_options(acknowledged: bool) -> DeployOptions { + DeployOptions { + reboot: false, + force: true, + boot_once_acknowledged: acknowledged, + } + } + + fn deploy_request() -> StartRequest { + let mut request = start_request(); + request.deploy = Some(deploy_options(false)); + request + } + + /// History for a deploy-alone start: answers the record lookup for the + /// requested branch and panics if a deploy writes a build record. + fn deploy_history(record: Option) -> MockKwHistoryStore { + let mut history = MockKwHistoryStore::new(); + history + .expect_build_records() + .withf(|kernel_tree_id, branch| { + kernel_tree_id == "mainline" && branch == "patchset-2026-08-01-17-30-00" + }) + .returning(move |_, _| Ok((record.clone(), record.clone()))); + history.expect_record_build().times(0); + history + } + + fn env_with_kw() -> MockEnvTrait { + let mut env = MockEnvTrait::new(); + env.expect_which().returning(|_| true); + env + } + + fn spawn_deploy_actor( + test_name: &str, + history: MockKwHistoryStore, + fs: MockFileSystemTrait, + ) -> (KwHandle, Arc, PathBuf) { + let (shell, _calls) = recording_shell(KW_VERSION_OK, CLEAN_STATUS, SWITCH_OK); + spawn_full_actor(test_name, history, shell, fs, env_with_kw()) + } + /// History answers for an actor whose jobs complete: no patchset /// link, build-record writes accepted and dropped. fn quiet_history() -> MockKwHistoryStore { @@ -1862,24 +2089,322 @@ mod tests { } #[tokio::test] - async fn start_deploy_still_refused_until_the_deploy_step() { + async fn start_build_then_deploy_still_refused_until_the_chain_step() { let handle = spawn_test_actor( - "deploy-refused", + "chain-refused", MockKwHistoryStore::new(), MockShellTrait::new(), MockFileSystemTrait::new(), MockEnvTrait::new(), ); - let result = - tokio::time::timeout(Duration::from_secs(1), handle.start_deploy(start_request())) - .await - .expect("start_deploy must reply immediately"); + let result = tokio::time::timeout( + Duration::from_secs(1), + handle.start_build_then_deploy(start_request()), + ) + .await + .expect("start_build_then_deploy must reply immediately"); assert!(matches!(result, Err(KwStartError::NotImplemented))); handle.shutdown().await; } + #[tokio::test] + async fn start_deploy_replies_immediately_and_runs_in_background() { + let (handle, process, log_dir) = spawn_deploy_actor( + "deploy-immediate", + deploy_history(Some(matching_build_record())), + deploy_ready_fs(), + ); + + let result = tokio::time::timeout( + Duration::from_secs(1), + handle.start_deploy(deploy_request()), + ) + .await + .expect("start_deploy must reply immediately"); + result.unwrap(); + + let spawned = process.spawned(); + assert_eq!(1, spawned.len()); + assert_eq!("kw", spawned[0].program); + assert_eq!( + [ + "deploy", + "--remote", + "root@box:22", + "--no-reboot", + "--force" + ] + .as_slice(), + spawned[0].args.as_slice() + ); + assert_eq!(Path::new("/home/user/linux"), spawned[0].cwd); + assert!(spawned[0].log_path.starts_with(&log_dir)); + assert!(spawned[0] + .log_path + .file_name() + .unwrap() + .to_string_lossy() + .starts_with("deploy-")); + + let snapshot = handle.get_status().await.unwrap(); + assert!( + matches!( + snapshot.job, + KwJobStatus::Running { + kind: KwJobKind::Deploy, + phase: KwPhase::Deploying, + .. + } + ), + "unexpected status: {:?}", + snapshot.job + ); + + process.last_child().finish(0); + let mut watch = handle.watch_status().await.unwrap(); + let status = wait_for_terminal_status(&mut watch).await; + assert!( + matches!( + status, + KwJobStatus::Succeeded { + kind: KwJobKind::Deploy, + .. + } + ), + "unexpected status: {status:?}" + ); + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_deploy_merges_extras_and_follows_reboot_force_options() { + let (handle, process, log_dir) = spawn_deploy_actor( + "deploy-argv", + deploy_history(Some(matching_build_record())), + deploy_ready_fs(), + ); + + let mut request = deploy_request(); + request.deploy = Some(DeployOptions { + reboot: true, + force: false, + boot_once_acknowledged: false, + }); + request.extra_args = [ + "--verbose", + "--local", + "--alert=n", + "--force", + "--remote", + "other:22", + ] + .into_iter() + .map(String::from) + .collect(); + handle.start_deploy(request).await.unwrap(); + + let spawned = process.spawned(); + assert_eq!( + ["deploy", "--remote", "root@box:22", "--reboot", "--verbose"].as_slice(), + spawned[0].args.as_slice() + ); + + process.last_child().finish(0); + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_deploy_refused_without_a_build_record() { + let (handle, process, log_dir) = + spawn_deploy_actor("deploy-no-record", deploy_history(None), deploy_ready_fs()); + + let err = handle.start_deploy(deploy_request()).await.unwrap_err(); + + assert!(matches!( + err, + KwStartError::DeployAloneRefused(DeployAloneRefusal::NoBuildRecord) + )); + assert_eq!(KwJobStatus::Idle, handle.get_status().await.unwrap().job); + assert!(process.spawned().is_empty()); + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_deploy_refused_when_last_build_failed() { + let mut record = matching_build_record(); + record.success = false; + let (handle, process, log_dir) = spawn_deploy_actor( + "deploy-failed-build", + deploy_history(Some(record)), + deploy_ready_fs(), + ); + + let err = handle.start_deploy(deploy_request()).await.unwrap_err(); + + assert!(matches!( + err, + KwStartError::DeployAloneRefused(DeployAloneRefusal::LastBuildFailed) + )); + assert!(process.spawned().is_empty()); + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_deploy_refused_on_tree_path_drift() { + let mut record = matching_build_record(); + record.tree_path = "/other/linux".to_string(); + let (handle, process, log_dir) = spawn_deploy_actor( + "deploy-drift", + deploy_history(Some(record)), + deploy_ready_fs(), + ); + + let err = handle.start_deploy(deploy_request()).await.unwrap_err(); + + assert!(matches!( + err, + KwStartError::DeployAloneRefused(DeployAloneRefusal::TreePathDrift { .. }) + )); + assert!(process.spawned().is_empty()); + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_deploy_refused_on_output_dir_mismatch() { + let mut record = matching_build_record(); + record.output_dir = Some("/cache/kw/envs/testenv".to_string()); + let (handle, process, log_dir) = spawn_deploy_actor( + "deploy-env", + deploy_history(Some(record)), + deploy_ready_fs(), + ); + + let err = handle.start_deploy(deploy_request()).await.unwrap_err(); + + assert!(matches!( + err, + KwStartError::DeployAloneRefused(DeployAloneRefusal::OutputDirMismatch) + )); + assert!(process.spawned().is_empty()); + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_deploy_refused_when_image_is_missing() { + let (handle, process, log_dir) = spawn_deploy_actor( + "deploy-no-image", + deploy_history(Some(matching_build_record())), + deploy_fs(DEPLOY_REMOTE_CONFIG, DEPLOY_BOOT_ONCE_OFF, false), + ); + + let err = handle.start_deploy(deploy_request()).await.unwrap_err(); + + assert!(matches!( + err, + KwStartError::DeployAloneRefused(DeployAloneRefusal::ImageMissing) + )); + assert!(process.spawned().is_empty()); + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_deploy_refused_when_remote_is_unresolved() { + let (handle, process, log_dir) = spawn_deploy_actor( + "deploy-no-remote", + MockKwHistoryStore::new(), + deploy_fs("", DEPLOY_BOOT_ONCE_OFF, true), + ); + + let err = handle.start_deploy(deploy_request()).await.unwrap_err(); + + assert!(matches!( + err, + KwStartError::RemoteUnresolved(RemoteRefusal::NoRemotesConfigured) + )); + assert!(process.spawned().is_empty()); + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_deploy_refused_when_boot_once_is_on_and_unacked() { + let (handle, process, log_dir) = spawn_deploy_actor( + "deploy-boot-once", + MockKwHistoryStore::new(), + deploy_fs(DEPLOY_REMOTE_CONFIG, DEPLOY_BOOT_ONCE_ON, true), + ); + + let err = handle.start_deploy(deploy_request()).await.unwrap_err(); + + assert!(matches!(err, KwStartError::BootOnceNotAcknowledged)); + assert!(process.spawned().is_empty()); + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_deploy_proceeds_when_boot_once_is_on_and_acked() { + let (handle, process, log_dir) = spawn_deploy_actor( + "deploy-boot-once-acked", + deploy_history(Some(matching_build_record())), + deploy_fs(DEPLOY_REMOTE_CONFIG, DEPLOY_BOOT_ONCE_ON, true), + ); + + let mut request = deploy_request(); + request.deploy = Some(deploy_options(true)); + handle.start_deploy(request).await.unwrap(); + assert_eq!(1, process.spawned().len()); + + process.last_child().finish(0); + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_deploy_post_switch_refusal_rolls_the_switch_back() { + let git = GitStub::on_branch("master"); + let (handle, process, log_dir) = spawn_full_actor( + "deploy-rollback", + deploy_history(None), + git.shell(), + deploy_ready_fs(), + env_with_kw(), + ); + + let err = handle.start_deploy(deploy_request()).await.unwrap_err(); + + assert!(matches!( + err, + KwStartError::DeployAloneRefused(DeployAloneRefusal::NoBuildRecord) + )); + assert!(process.spawned().is_empty()); + assert_eq!(git.head(), "master"); + assert!(matches!( + handle.restore_previous_branch().await, + Err(KwError::NoRecordedBranch) + )); + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + #[tokio::test] async fn cancel_and_restore_without_job_are_immediate_errors() { let handle = spawn_test_actor( diff --git a/src/kw/argv.rs b/src/kw/argv.rs index 0cc5fcd..4d87f49 100644 --- a/src/kw/argv.rs +++ b/src/kw/argv.rs @@ -89,7 +89,7 @@ pub fn build_argv(extra_args: &[String]) -> Vec { /// stripped so the injected remote, reboot, and force flags win. /// `--force` is omitted entirely when `force` is false rather than /// passing a no-op, because kw has no `--no-force`. -#[cfg_attr(not(test), allow(dead_code))] +#[cfg_attr(not(unix), allow(dead_code))] pub fn deploy_argv( endpoint: &str, reboot: bool, diff --git a/src/kw/errors.rs b/src/kw/errors.rs index 1b6f434..e370e22 100644 --- a/src/kw/errors.rs +++ b/src/kw/errors.rs @@ -7,7 +7,10 @@ use thiserror::Error; use crate::infrastructure::process::ProcessError; use crate::{ infrastructure::{file_system::FileSystemError, shell::ShellError}, - kw::readiness::{KwReadinessError, TreeReadiness}, + kw::{ + readiness::{DeployAloneRefusal, KwReadinessError, TreeReadiness}, + remote::RemoteRefusal, + }, }; #[derive(Debug, Error)] @@ -55,6 +58,14 @@ pub enum KwStartError { GitStateProbe(String), #[error("failed to switch the kernel tree to the requested branch: {0}")] CheckoutFailed(String), + #[error("{0}")] + RemoteUnresolved(RemoteRefusal), + #[error( + "boot-into-new-kernel-once is on; confirm before deploying — kw has no CLI off-switch" + )] + BootOnceNotAcknowledged, + #[error("{0}")] + DeployAloneRefused(DeployAloneRefusal), #[error("kw jobs are not supported yet")] NotImplemented, // Spawning a process is unix-only (ProcessTrait is cfg(unix)). diff --git a/src/kw/messages.rs b/src/kw/messages.rs index 5409dba..0ba564d 100644 --- a/src/kw/messages.rs +++ b/src/kw/messages.rs @@ -13,6 +13,17 @@ use crate::{ }, }; +/// Deploy knobs the actor injects onto `kw deploy`. Build-only starts +/// leave [`StartRequest::deploy`] unset. +#[derive(Debug, Clone)] +pub struct DeployOptions { + pub reboot: bool, + pub force: bool, + /// True after the user confirmed boot-into-new-kernel-once. The actor + /// still refuses when the option is on or unknown and this is false. + pub boot_once_acknowledged: bool, +} + /// Everything the actor needs to start a job. The tree context is resolved /// by the caller from its config snapshot, keeping KwActor decoupled from /// ConfigActor. @@ -27,6 +38,10 @@ pub struct StartRequest { /// Reserved options (`--alert`, `--save-log-to`) are stripped — /// patch-hub's own argv wins. pub extra_args: Vec, + /// Required for deploy kinds so the actor can inject reboot/force and + /// honor the boot-once confirm gate. Build-only starts leave this + /// `None`. + pub deploy: Option, } pub enum KwMessage { @@ -39,7 +54,6 @@ pub enum KwMessage { reply: oneshot::Sender>, }, StartDeploy { - #[allow(dead_code)] request: StartRequest, reply: oneshot::Sender>, }, diff --git a/src/kw/status.rs b/src/kw/status.rs index 13eda64..a2934f1 100644 --- a/src/kw/status.rs +++ b/src/kw/status.rs @@ -11,7 +11,6 @@ use std::path::PathBuf; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum KwJobKind { Build, - #[allow(dead_code)] Deploy, #[allow(dead_code)] BuildThenDeploy, @@ -21,7 +20,6 @@ pub enum KwJobKind { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum KwPhase { Building, - #[allow(dead_code)] Deploying, } From c8b826f3995caa1a697e95e3b67867e305902baa Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sat, 19 Sep 2026 20:54:59 -0300 Subject: [PATCH 07/12] feat(kw): chain deploy after a successful build This commit starts BuildThenDeploy as a build, then on a successful exit writes the build record and spawns kw deploy without a second Start. A failed or cancelled build never deploys; a cancel or spawn failure during Deploying keeps the record so deploy-alone can reuse it. This commit is part of the kw integration's step 7. Signed-off-by: lorenzoberts --- src/kw/actor.rs | 510 +++++++++++++++++++++++++++++++++++++++------ src/kw/errors.rs | 2 - src/kw/messages.rs | 1 - src/kw/status.rs | 1 - 4 files changed, 441 insertions(+), 73 deletions(-) diff --git a/src/kw/actor.rs b/src/kw/actor.rs index cb888a8..b749f49 100644 --- a/src/kw/actor.rs +++ b/src/kw/actor.rs @@ -92,11 +92,25 @@ struct JobState { output_dir: Option, arch: Option, log_path: PathBuf, + /// Present for deploy kinds so a BuildThenDeploy job can spawn the + /// deploy process at the build/deploy boundary without the original + /// StartRequest. + deploy: Option, /// `None` once a cancel has been requested; a second `Cancel` is an /// idempotent ack. cancel_tx: Option>, } +/// Deploy argv inputs snapshotted at accept. The remote and options are +/// resolved before the job starts so a chain cannot silently retarget +/// mid-build. +#[derive(Clone)] +struct JobDeploy { + remote: KwRemote, + options: DeployOptions, + extra_args: Vec, +} + pub struct KwActor { rx: mpsc::Receiver, job_event_rx: mpsc::Receiver, @@ -205,9 +219,12 @@ impl KwActor { ); ControlFlow::Continue(()) } - // Not implemented: reply immediately with NotImplemented. - KwMessage::StartBuildThenDeploy { reply, .. } => { - send_start_reply(message_name, reply, Err(KwStartError::NotImplemented)); + KwMessage::StartBuildThenDeploy { request, reply } => { + send_start_reply( + message_name, + reply, + self.start_job(KwJobKind::BuildThenDeploy, request).await, + ); ControlFlow::Continue(()) } KwMessage::Cancel { reply } => { @@ -274,12 +291,13 @@ impl KwActor { /// /// Refusals, in order: a job already running, no kw binary on PATH, /// unresolvable kw-env state, a tree that fails the readiness probes, - /// a dirty worktree, or a failed branch switch. Deploy jobs then also - /// refuse on an unresolved remote, an unacknowledged boot-once - /// setting, or a deploy-alone record mismatch. Reserved flags on - /// patch-hub's own argv win over the request's extra args. A start - /// refused after the branch switch rolls the switch back: a refused - /// start never leaves the tree on a branch the user did not check out. + /// a dirty worktree, or a failed branch switch. Deploy kinds then also + /// refuse on an unresolved remote or an unacknowledged boot-once + /// setting; StartDeploy additionally requires a matching build record + /// and image. Reserved flags on patch-hub's own argv win over the + /// request's extra args. A start refused after the branch switch rolls + /// the switch back: a refused start never leaves the tree on a branch + /// the user did not check out. async fn start_job( &mut self, kind: KwJobKind, @@ -318,9 +336,15 @@ impl KwActor { let pre_job_branch = self.checkout_build_branch(&request).await?; - let remote = if kind == KwJobKind::Deploy { - match self.gate_deploy(&request, &tree_path, output_dir.as_deref(), arch.as_deref()) { - Ok(remote) => Some(remote), + let deploy = if matches!(kind, KwJobKind::Deploy | KwJobKind::BuildThenDeploy) { + match self.prepare_deploy( + kind, + &request, + &tree_path, + output_dir.as_deref(), + arch.as_deref(), + ) { + Ok(deploy) => Some(deploy), Err(error) => { self.rollback_switch(request.tree.path(), pre_job_branch.as_deref()) .await; @@ -333,10 +357,10 @@ impl KwActor { let spawned = match kind { KwJobKind::Deploy => self.spawn_deploy_process( - &request, - remote + request.tree.path(), + deploy .as_ref() - .expect("gate_deploy returns a remote for Deploy"), + .expect("prepare_deploy returns context for Deploy"), ), KwJobKind::Build | KwJobKind::BuildThenDeploy => self.spawn_build_process(&request), }; @@ -388,6 +412,7 @@ impl KwActor { output_dir, arch, log_path: log_path.clone(), + deploy, cancel_tx: Some(cancel_tx), }); self.set_status(KwJobStatus::Running { @@ -400,17 +425,19 @@ impl KwActor { Ok(()) } - /// Deploy-only gates after the branch switch: resolved remote, - /// boot-once confirm, then the deploy-alone record match against the - /// requested (now checked-out) branch. Any refusal is rolled back by - /// the caller. - fn gate_deploy( + /// Deploy-kind gates after the branch switch: resolved remote and + /// boot-once confirm for every deploy kind; the deploy-alone record + /// match only for StartDeploy, against the requested (now checked-out) + /// branch. Any refusal is rolled back by the caller. BuildThenDeploy + /// skips the record gate because the build has not run yet. + fn prepare_deploy( &self, + kind: KwJobKind, request: &StartRequest, tree_path: &Path, output_dir: Option<&Path>, arch: Option<&str>, - ) -> Result { + ) -> Result { let options = request.deploy.clone().unwrap_or(DeployOptions { reboot: false, force: true, @@ -424,20 +451,29 @@ impl KwActor { { return Err(KwStartError::BootOnceNotAcknowledged); } - let (record, _) = self - .history - .build_records(&request.kernel_tree_id, &request.branch)?; - let image = - readiness::find_newest_kernel_image(&*self.fs, output_dir.unwrap_or(tree_path), arch); - readiness::check_deploy_alone( - record.as_ref(), - &request.tree, - &request.branch, - output_dir, - image.as_deref(), - ) - .map_err(KwStartError::DeployAloneRefused)?; - Ok(remote) + if kind == KwJobKind::Deploy { + let (record, _) = self + .history + .build_records(&request.kernel_tree_id, &request.branch)?; + let image = readiness::find_newest_kernel_image( + &*self.fs, + output_dir.unwrap_or(tree_path), + arch, + ); + readiness::check_deploy_alone( + record.as_ref(), + &request.tree, + &request.branch, + output_dir, + image.as_deref(), + ) + .map_err(KwStartError::DeployAloneRefused)?; + } + Ok(JobDeploy { + remote, + options, + extra_args: request.extra_args.clone(), + }) } /// Refuse a dirty worktree, record HEAD, then `git switch` to the @@ -532,26 +568,21 @@ impl KwActor { /// `start_job` so a failure here can roll the branch switch back. fn spawn_deploy_process( &self, - request: &StartRequest, - remote: &KwRemote, + tree_path: &str, + deploy: &JobDeploy, ) -> Result<(Box, PathBuf), KwStartError> { self.fs.create_dir_all(&self.kw_log_dir)?; let log_path = self.kw_log_dir.join(format!( "deploy-{}.log", chrono::Utc::now().format("%Y%m%d-%H%M%S-%3f") )); - let options = request.deploy.clone().unwrap_or(DeployOptions { - reboot: false, - force: true, - boot_once_acknowledged: false, - }); let cmd = ShellCommand::new("kw").args(argv::deploy_argv( - &remote.endpoint(), - options.reboot, - options.force, - &request.extra_args, + &deploy.remote.endpoint(), + deploy.options.reboot, + deploy.options.force, + &deploy.extra_args, )); - let cwd = PathBuf::from(request.tree.path()); + let cwd = PathBuf::from(tree_path); let process = self.process.spawn(&cmd, &cwd, &log_path)?; Ok((process, log_path)) } @@ -575,6 +606,10 @@ impl KwActor { tracing::warn!("kw job finished with no job state recorded"); return; }; + let job = match self.advance_build_then_deploy(job, &outcome) { + ControlFlow::Break(()) => return, + ControlFlow::Continue(job) => job, + }; // The record is written before the status flips: watchers // that react to the terminal status find the history // already durable. @@ -630,11 +665,84 @@ impl KwActor { } } + /// After a successful BuildThenDeploy build, write the build record + /// and spawn the deploy process. `Break` means this Finished was + /// consumed (job still running, or failed at the deploy spawn + /// boundary). `Continue` hands the job back for a normal terminal. + fn advance_build_then_deploy( + &mut self, + job: JobState, + outcome: &JobOutcome, + ) -> ControlFlow<(), JobState> { + let JobOutcome::Exited(exit) = outcome else { + return ControlFlow::Continue(job); + }; + if job.kind != KwJobKind::BuildThenDeploy + || job.phase != KwPhase::Building + || !exit.success() + { + return ControlFlow::Continue(job); + } + // Durable before the phase flips: a deploy spawn failure must not + // lose the successful build, and KwOps watching Deploying must + // already see the record. + self.record_build_outcome(&job, outcome); + let Some(deploy) = job.deploy.clone() else { + tracing::error!("BuildThenDeploy missing deploy context after a successful build"); + self.set_status(KwJobStatus::Failed { + kind: job.kind, + phase: KwPhase::Deploying, + exit_code: None, + log_path: job.log_path, + }); + return ControlFlow::Break(()); + }; + match self.spawn_deploy_process(&job.tree_path, &deploy) { + Ok((process, log_path)) => { + let (cancel_tx, cancel_rx) = oneshot::channel(); + spawn(run_job(process, cancel_rx, self.job_event_tx.clone())); + tracing::info!( + kernel_tree_id = job.kernel_tree_id, + branch = job.branch, + log_path = %log_path.display(), + "kw deploy phase started" + ); + self.set_status(KwJobStatus::Running { + kind: job.kind, + phase: KwPhase::Deploying, + kernel_tree_id: job.kernel_tree_id.clone(), + branch: job.branch.clone(), + log_path: log_path.clone(), + }); + let mut job = job; + job.phase = KwPhase::Deploying; + job.log_path = log_path; + job.cancel_tx = Some(cancel_tx); + self.job = Some(job); + ControlFlow::Break(()) + } + Err(error) => { + tracing::warn!( + %error, + branch = job.branch, + "failed to spawn kw deploy after a successful build" + ); + self.set_status(KwJobStatus::Failed { + kind: job.kind, + phase: KwPhase::Deploying, + exit_code: None, + log_path: job.log_path, + }); + ControlFlow::Break(()) + } + } + } + /// Persist a finished build (success or failure). Cancel writes /// nothing. History and patchset-link errors are logged, not folded /// into job status — the build's real outcome already reached the user. fn record_build_outcome(&self, job: &JobState, outcome: &JobOutcome) { - if job.kind == KwJobKind::Deploy { + if job.kind == KwJobKind::Deploy || job.phase == KwPhase::Deploying { return; } let success = match outcome { @@ -1685,6 +1793,26 @@ mod tests { .expect("status must reach a terminal state") } + async fn wait_for_running_phase( + watch: &mut watch::Receiver, + phase: KwPhase, + ) -> KwJobStatus { + tokio::time::timeout(Duration::from_secs(10), async { + loop { + let status = watch.borrow().job.clone(); + if matches!( + status, + KwJobStatus::Running { phase: running, .. } if running == phase + ) { + return status; + } + watch.changed().await.unwrap(); + } + }) + .await + .expect("status must reach the expected running phase") + } + #[tokio::test] async fn start_build_replies_immediately_and_runs_in_background() { let (handle, process, log_dir) = spawn_job_actor("start-immediate"); @@ -2088,27 +2216,6 @@ mod tests { std::fs::remove_dir_all(&log_dir).unwrap(); } - #[tokio::test] - async fn start_build_then_deploy_still_refused_until_the_chain_step() { - let handle = spawn_test_actor( - "chain-refused", - MockKwHistoryStore::new(), - MockShellTrait::new(), - MockFileSystemTrait::new(), - MockEnvTrait::new(), - ); - - let result = tokio::time::timeout( - Duration::from_secs(1), - handle.start_build_then_deploy(start_request()), - ) - .await - .expect("start_build_then_deploy must reply immediately"); - - assert!(matches!(result, Err(KwStartError::NotImplemented))); - handle.shutdown().await; - } - #[tokio::test] async fn start_deploy_replies_immediately_and_runs_in_background() { let (handle, process, log_dir) = spawn_deploy_actor( @@ -2405,6 +2512,271 @@ mod tests { std::fs::remove_dir_all(&log_dir).unwrap(); } + #[tokio::test] + async fn start_build_then_deploy_chains_deploy_after_a_successful_build() { + let (history, builds) = recording_history(None); + let (handle, process, log_dir) = + spawn_deploy_actor("chain-success", history, deploy_ready_fs()); + let mut watch = handle.watch_status().await.unwrap(); + + let mut request = deploy_request(); + request.extra_args = ["--verbose", "--ccache", "--alert=n"] + .into_iter() + .map(String::from) + .collect(); + tokio::time::timeout( + Duration::from_secs(1), + handle.start_build_then_deploy(request), + ) + .await + .expect("start_build_then_deploy must reply immediately") + .unwrap(); + + assert_eq!(1, process.spawned().len()); + assert_eq!( + ["build", "--verbose", "--ccache"].as_slice(), + process.spawned()[0].args.as_slice() + ); + assert!( + matches!( + handle.get_status().await.unwrap().job, + KwJobStatus::Running { + kind: KwJobKind::BuildThenDeploy, + phase: KwPhase::Building, + .. + } + ), + "unexpected status: {:?}", + handle.get_status().await.unwrap().job + ); + + let build = process.last_child(); + build.finish(0); + let deploying = wait_for_running_phase(&mut watch, KwPhase::Deploying).await; + match deploying { + KwJobStatus::Running { + kind, + phase, + log_path, + .. + } => { + assert_eq!(KwJobKind::BuildThenDeploy, kind); + assert_eq!(KwPhase::Deploying, phase); + assert!(log_path + .file_name() + .unwrap() + .to_string_lossy() + .starts_with("deploy-")); + } + other => panic!("expected Running Deploying, got {other:?}"), + } + + let spawned = process.spawned(); + assert_eq!(2, spawned.len()); + assert_eq!( + [ + "deploy", + "--remote", + "root@box:22", + "--no-reboot", + "--force", + "--verbose", + "--ccache" + ] + .as_slice(), + spawned[1].args.as_slice() + ); + assert_eq!(Path::new("/home/user/linux"), spawned[1].cwd); + { + let builds = builds.lock().unwrap(); + assert_eq!(1, builds.len()); + assert!(builds[0].success); + } + + process.last_child().finish(0); + let status = wait_for_terminal_status(&mut watch).await; + assert!( + matches!( + status, + KwJobStatus::Succeeded { + kind: KwJobKind::BuildThenDeploy, + .. + } + ), + "unexpected status: {status:?}" + ); + assert_eq!(1, builds.lock().unwrap().len()); + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_build_then_deploy_skips_deploy_when_the_build_fails() { + let (history, builds) = recording_history(None); + let (handle, process, log_dir) = + spawn_deploy_actor("chain-build-fail", history, deploy_ready_fs()); + let mut watch = handle.watch_status().await.unwrap(); + + handle + .start_build_then_deploy(deploy_request()) + .await + .unwrap(); + process.last_child().finish(2); + + let status = wait_for_terminal_status(&mut watch).await; + match status { + KwJobStatus::Failed { + kind, + phase, + exit_code, + .. + } => { + assert_eq!(KwJobKind::BuildThenDeploy, kind); + assert_eq!(KwPhase::Building, phase); + assert_eq!(Some(2), exit_code); + } + other => panic!("expected Failed Building, got {other:?}"), + } + assert_eq!(1, process.spawned().len()); + { + let builds = builds.lock().unwrap(); + assert_eq!(1, builds.len()); + assert!(!builds[0].success); + } + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_build_then_deploy_cancel_in_building_skips_deploy_and_record() { + let (history, builds) = recording_history(None); + let (handle, process, log_dir) = + spawn_deploy_actor("chain-cancel-build", history, deploy_ready_fs()); + let mut watch = handle.watch_status().await.unwrap(); + + handle + .start_build_then_deploy(deploy_request()) + .await + .unwrap(); + handle.cancel().await.unwrap(); + + let status = wait_for_terminal_status(&mut watch).await; + match status { + KwJobStatus::Cancelled { kind, phase, .. } => { + assert_eq!(KwJobKind::BuildThenDeploy, kind); + assert_eq!(KwPhase::Building, phase); + } + other => panic!("expected Cancelled Building, got {other:?}"), + } + assert_eq!(1, process.spawned().len()); + assert!(process.last_child().was_killed()); + assert!(builds.lock().unwrap().is_empty()); + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_build_then_deploy_cancel_in_deploying_keeps_the_build_record() { + let (history, builds) = recording_history(None); + let (handle, process, log_dir) = + spawn_deploy_actor("chain-cancel-deploy", history, deploy_ready_fs()); + let mut watch = handle.watch_status().await.unwrap(); + + handle + .start_build_then_deploy(deploy_request()) + .await + .unwrap(); + process.last_child().finish(0); + let _ = wait_for_running_phase(&mut watch, KwPhase::Deploying).await; + assert_eq!(1, builds.lock().unwrap().len()); + + handle.cancel().await.unwrap(); + let status = wait_for_terminal_status(&mut watch).await; + match status { + KwJobStatus::Cancelled { kind, phase, .. } => { + assert_eq!(KwJobKind::BuildThenDeploy, kind); + assert_eq!(KwPhase::Deploying, phase); + } + other => panic!("expected Cancelled Deploying, got {other:?}"), + } + assert_eq!(2, process.spawned().len()); + assert!(process.last_child().was_killed()); + { + let builds = builds.lock().unwrap(); + assert_eq!(1, builds.len()); + assert!(builds[0].success); + } + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_build_then_deploy_spawn_failure_at_boundary_keeps_the_build_record() { + let (history, builds) = recording_history(None); + let (handle, process, log_dir) = + spawn_deploy_actor("chain-spawn-fail", history, deploy_ready_fs()); + let mut watch = handle.watch_status().await.unwrap(); + + handle + .start_build_then_deploy(deploy_request()) + .await + .unwrap(); + let build = process.last_child(); + process.refuse_spawns(true); + build.finish(0); + + let status = wait_for_terminal_status(&mut watch).await; + match status { + KwJobStatus::Failed { + kind, + phase, + exit_code, + .. + } => { + assert_eq!(KwJobKind::BuildThenDeploy, kind); + assert_eq!(KwPhase::Deploying, phase); + assert_eq!(None, exit_code); + } + other => panic!("expected Failed Deploying with no exit, got {other:?}"), + } + assert_eq!(1, process.spawned().len()); + { + let builds = builds.lock().unwrap(); + assert_eq!(1, builds.len()); + assert!(builds[0].success); + } + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_build_then_deploy_refused_when_remote_is_unresolved() { + let (handle, process, log_dir) = spawn_deploy_actor( + "chain-no-remote", + MockKwHistoryStore::new(), + deploy_fs("", DEPLOY_BOOT_ONCE_OFF, true), + ); + + let err = handle + .start_build_then_deploy(deploy_request()) + .await + .unwrap_err(); + + assert!(matches!( + err, + KwStartError::RemoteUnresolved(RemoteRefusal::NoRemotesConfigured) + )); + assert!(process.spawned().is_empty()); + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + #[tokio::test] async fn cancel_and_restore_without_job_are_immediate_errors() { let handle = spawn_test_actor( diff --git a/src/kw/errors.rs b/src/kw/errors.rs index e370e22..cfedc71 100644 --- a/src/kw/errors.rs +++ b/src/kw/errors.rs @@ -66,8 +66,6 @@ pub enum KwStartError { BootOnceNotAcknowledged, #[error("{0}")] DeployAloneRefused(DeployAloneRefusal), - #[error("kw jobs are not supported yet")] - NotImplemented, // Spawning a process is unix-only (ProcessTrait is cfg(unix)). #[cfg(unix)] #[error("failed to spawn the kw process: {0}")] diff --git a/src/kw/messages.rs b/src/kw/messages.rs index 0ba564d..efc31d0 100644 --- a/src/kw/messages.rs +++ b/src/kw/messages.rs @@ -58,7 +58,6 @@ pub enum KwMessage { reply: oneshot::Sender>, }, StartBuildThenDeploy { - #[allow(dead_code)] request: StartRequest, reply: oneshot::Sender>, }, diff --git a/src/kw/status.rs b/src/kw/status.rs index a2934f1..42401cb 100644 --- a/src/kw/status.rs +++ b/src/kw/status.rs @@ -12,7 +12,6 @@ use std::path::PathBuf; pub enum KwJobKind { Build, Deploy, - #[allow(dead_code)] BuildThenDeploy, } From 69194633c02b1ffa8b23fdbc7fff4853d5c97702 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sat, 19 Sep 2026 21:01:17 -0300 Subject: [PATCH 08/12] feat(app): gate deploys on boot-into-new-kernel-once confirmation This commit adds a proceed/back-out confirm popup for kw's one-shot boot-into-new-kernel-once, because there is no CLI off-switch. Back out is the default; proceeding stores the acknowledgement and consumes the pending deploy kind. The quit prompt now says "job" so a running deploy is not described as a build. This commit is part of the kw integration's step 7. Signed-off-by: lorenzoberts --- src/app/actor.rs | 157 +++++++++++++++++++++++-- src/app/flows/kw_ops.rs | 19 +++ src/app/integration_tests/kw_status.rs | 2 +- src/app/popup.rs | 60 +++++++++- src/app/screens/kw_ops.rs | 39 +++++- src/app/view_model.rs | 19 ++- 6 files changed, 278 insertions(+), 18 deletions(-) diff --git a/src/app/actor.rs b/src/app/actor.rs index 6f68c24..b4c1558 100644 --- a/src/app/actor.rs +++ b/src/app/actor.rs @@ -21,8 +21,8 @@ use crate::{ details_actions::handle_patchset_details, edit_config::handle_edit_config, kw_ops::{ - apply_kw_snapshot, fallback_kw_status, handle_kw_ops, poll_kw_status, - refresh_kw_ops_log_tail, + apply_kw_snapshot, clear_pending_deploy, fallback_kw_status, handle_kw_ops, + poll_kw_status, refresh_kw_ops_log_tail, resume_pending_deploy, }, latest::handle_latest_patchsets, mail_list::handle_mailing_list_selection, @@ -242,22 +242,26 @@ async fn on_input( terminal_handle: &TerminalHandle, loading: &mut TerminalLoadingIndicator, ) -> Result> { - if let Some(popup) = app.state.popup.as_mut() { + if app.state.popup.is_some() { match input { InputEvent::ClosePopup => { - app.state.popup = None; + dismiss_open_popup(app); } - InputEvent::ConfirmPopup => match popup.selected_confirm_action() { - Some(ConfirmAction::CancelKwAndQuit) => { - app.state.popup = None; - return Ok(cancel_kw_and_quit(app).await); + InputEvent::ConfirmPopup => { + if let Some(action) = app + .state + .popup + .as_ref() + .and_then(AppPopup::selected_confirm_action) + { + return apply_confirm_action(app, action).await; } - Some(ConfirmAction::Wait) => { - app.state.popup = None; + } + _ => { + if let Some(popup) = app.state.popup.as_mut() { + popup.handle_input(input); } - None => {} - }, - _ => popup.handle_input(input), + } } } else if input == InputEvent::Quit && kw_job_is_running(app) { app.state.popup = Some(AppPopup::quit_while_job_running()); @@ -290,6 +294,36 @@ async fn on_input( Ok(ControlFlow::Continue(())) } +fn is_boot_once_confirm(popup: &AppPopup) -> bool { + matches!( + popup.selected_confirm_action(), + Some(ConfirmAction::ProceedWithBootOnce | ConfirmAction::BackOut) + ) +} + +fn dismiss_open_popup(app: &mut App) { + if app.state.popup.as_ref().is_some_and(is_boot_once_confirm) { + clear_pending_deploy(app); + } + app.state.popup = None; +} + +async fn apply_confirm_action(app: &mut App, action: ConfirmAction) -> Result> { + app.state.popup = None; + match action { + ConfirmAction::CancelKwAndQuit => Ok(cancel_kw_and_quit(app).await), + ConfirmAction::Wait => Ok(ControlFlow::Continue(())), + ConfirmAction::ProceedWithBootOnce => { + resume_pending_deploy(app).await?; + Ok(ControlFlow::Continue(())) + } + ConfirmAction::BackOut => { + clear_pending_deploy(app); + Ok(ControlFlow::Continue(())) + } + } +} + fn kw_job_is_running(app: &App) -> bool { matches!( app.state.kw.status.as_ref().map(|status| &status.job), @@ -520,4 +554,101 @@ mod tests { lore_api.shutdown().await; render.shutdown().await; } + + fn sample_kw_ops() -> crate::app::screens::kw_ops::KwOpsState { + crate::app::screens::kw_ops::KwOpsState::new( + "title".to_string(), + "mid".to_string(), + "linux".to_string(), + serde_json::from_value(serde_json::json!({ + "path": "/kernel", + "branch": "main" + })) + .unwrap(), + crate::kw::readiness::KwReadiness { + kw_binary: crate::kw::readiness::KwBinaryProbe { + available: true, + version_line: Some("kw, version 0.10.0".to_string()), + check: crate::kw::readiness::KwVersionCheck::Meets, + }, + tree: crate::kw::readiness::TreeReadiness::Ready { + arch: Some("x86_64".to_string()), + }, + output_dir: None, + kernel_image: None, + build_record: None, + latest_build: None, + deploy_alone: Err(crate::kw::readiness::DeployAloneRefusal::NoBuildRecord), + current_branch: Some("main".to_string()), + deploy_remote: Err(crate::kw::remote::RemoteRefusal::NoRemotesConfigured), + boot_once: crate::kw::readiness::BootOnceState::Unknown, + }, + ) + } + + #[tokio::test] + async fn proceed_with_boot_once_acks_and_clears_pending() { + let mut app = minimal_app(); + let mut ops = sample_kw_ops(); + ops.pending_deploy = Some(crate::app::screens::kw_ops::DeployStartKind::Deploy); + app.state.kw.ops = Some(ops); + app.state.popup = Some(AppPopup::boot_once_warning()); + + let flow = apply_confirm_action(&mut app, ConfirmAction::ProceedWithBootOnce) + .await + .unwrap(); + assert_eq!(ControlFlow::Continue(()), flow); + assert!(app.state.popup.is_none()); + let ops = app.state.kw.ops.as_ref().unwrap(); + assert!(ops.boot_once_acknowledged); + assert_eq!(None, ops.pending_deploy); + } + + #[tokio::test] + async fn back_out_clears_pending_without_acknowledging() { + let mut app = minimal_app(); + let mut ops = sample_kw_ops(); + ops.pending_deploy = Some(crate::app::screens::kw_ops::DeployStartKind::BuildThenDeploy); + app.state.kw.ops = Some(ops); + app.state.popup = Some(AppPopup::boot_once_warning()); + + let flow = apply_confirm_action(&mut app, ConfirmAction::BackOut) + .await + .unwrap(); + assert_eq!(ControlFlow::Continue(()), flow); + assert!(app.state.popup.is_none()); + let ops = app.state.kw.ops.as_ref().unwrap(); + assert!(!ops.boot_once_acknowledged); + assert_eq!(None, ops.pending_deploy); + } + + #[test] + fn closing_the_boot_once_popup_clears_pending() { + let mut app = minimal_app(); + let mut ops = sample_kw_ops(); + ops.pending_deploy = Some(crate::app::screens::kw_ops::DeployStartKind::Deploy); + app.state.kw.ops = Some(ops); + app.state.popup = Some(AppPopup::boot_once_warning()); + + dismiss_open_popup(&mut app); + assert!(app.state.popup.is_none()); + assert_eq!(None, app.state.kw.ops.as_ref().unwrap().pending_deploy); + assert!(!app.state.kw.ops.as_ref().unwrap().boot_once_acknowledged); + } + + #[test] + fn closing_the_quit_popup_does_not_touch_pending_deploy() { + let mut app = minimal_app(); + let mut ops = sample_kw_ops(); + ops.pending_deploy = Some(crate::app::screens::kw_ops::DeployStartKind::Deploy); + app.state.kw.ops = Some(ops); + app.state.popup = Some(AppPopup::quit_while_job_running()); + + dismiss_open_popup(&mut app); + assert!(app.state.popup.is_none()); + assert_eq!( + Some(crate::app::screens::kw_ops::DeployStartKind::Deploy), + app.state.kw.ops.as_ref().unwrap().pending_deploy + ); + } } diff --git a/src/app/flows/kw_ops.rs b/src/app/flows/kw_ops.rs index 34f08ec..fe8b4bd 100644 --- a/src/app/flows/kw_ops.rs +++ b/src/app/flows/kw_ops.rs @@ -351,6 +351,25 @@ fn job_is_busy(app: &App) -> bool { .is_some_and(|ops| ops.start_requested) } +/// Records that the user confirmed boot-once and consumes the pending +/// deploy kind. The KwOps input commit re-issues Start against this +/// acknowledgement; until then Proceed only stores the ack. +pub(crate) async fn resume_pending_deploy(app: &mut App) -> Result<()> { + let Some(ops) = app.state.kw.ops.as_mut() else { + return Ok(()); + }; + ops.boot_once_acknowledged = true; + let _kind = ops.pending_deploy.take(); + Ok(()) +} + +/// Drops a deploy start that was waiting on the boot-once confirm popup. +pub(crate) fn clear_pending_deploy(app: &mut App) { + if let Some(ops) = app.state.kw.ops.as_mut() { + ops.pending_deploy = None; + } +} + pub fn generate_help_popup() -> AppPopup { AppPopup::help() .title("Kw operations") diff --git a/src/app/integration_tests/kw_status.rs b/src/app/integration_tests/kw_status.rs index 5026b12..cd974bf 100644 --- a/src/app/integration_tests/kw_status.rs +++ b/src/app/integration_tests/kw_status.rs @@ -141,7 +141,7 @@ async fn quit_while_job_running_opens_confirm_and_wait_keeps_app_alive() { event_tx.send(InputEvent::Quit).await.unwrap(); wait_for_latest_popup(&scenes, |popup| { - popup.is_some_and(|popup| popup.title == "Cancel build and quit?") + popup.is_some_and(|popup| popup.title == "Cancel job and quit?") }) .await; let latest = scenes.lock().unwrap().last().cloned().unwrap(); diff --git a/src/app/popup.rs b/src/app/popup.rs index dbc8169..1eb7c3f 100644 --- a/src/app/popup.rs +++ b/src/app/popup.rs @@ -16,6 +16,10 @@ use crate::{ pub enum ConfirmAction { CancelKwAndQuit, Wait, + /// User accepted kw's one-shot boot into the new kernel. + ProceedWithBootOnce, + /// User declined the boot-once gate; the pending deploy is dropped. + BackOut, } /// Concrete, cloneable popup state stored in `AppState`. @@ -124,7 +128,7 @@ impl AppPopup { /// Esc, stays in the app unless the user explicitly picks cancel. pub fn quit_while_job_running() -> Self { AppPopup::Confirm { - title: "Cancel build and quit?".to_string(), + title: "Cancel job and quit?".to_string(), body: "A kw job is still running. Cancel it and quit, or wait and stay in the app?" .to_string(), options: vec![ @@ -139,6 +143,26 @@ impl AppPopup { } } + /// Choice popup shown when a deploy would boot the new kernel once. + /// + /// kw has no CLI off-switch for `boot_into_new_kernel_once`; this is + /// only a proceed/back-out gate. [`ConfirmAction::BackOut`] is the + /// highlighted default so Enter, like Esc, does not deploy. + pub fn boot_once_warning() -> Self { + AppPopup::Confirm { + title: "Boot into new kernel once?".to_string(), + body: "kw will set a one-shot boot into the new kernel on the target. \ +There is no CLI off-switch; this popup only lets you proceed or back out." + .to_string(), + options: vec![ + ("Back out".to_string(), ConfirmAction::BackOut), + ("Proceed".to_string(), ConfirmAction::ProceedWithBootOnce), + ], + selected: 0, + dimensions: (50, 30), + } + } + /// The currently highlighted confirm action, if this is a choice popup. pub fn selected_confirm_action(&self) -> Option { match self { @@ -299,6 +323,40 @@ mod tests { assert_eq!(Some(ConfirmAction::Wait), popup.selected_confirm_action()); } + #[test] + fn boot_once_warning_defaults_to_back_out() { + let popup = AppPopup::boot_once_warning(); + assert_eq!( + Some(ConfirmAction::BackOut), + popup.selected_confirm_action() + ); + } + + #[test] + fn boot_once_warning_right_selects_proceed() { + let mut popup = AppPopup::boot_once_warning(); + popup.handle_input(InputEvent::NavigateRight); + assert_eq!( + Some(ConfirmAction::ProceedWithBootOnce), + popup.selected_confirm_action() + ); + popup.handle_input(InputEvent::NavigateRight); + assert_eq!( + Some(ConfirmAction::ProceedWithBootOnce), + popup.selected_confirm_action() + ); + } + + #[test] + fn boot_once_warning_left_stays_on_back_out() { + let mut popup = AppPopup::boot_once_warning(); + popup.handle_input(InputEvent::NavigateLeft); + assert_eq!( + Some(ConfirmAction::BackOut), + popup.selected_confirm_action() + ); + } + #[test] fn info_popup_still_scrolls() { let mut popup = AppPopup::info("Title", "line 1\nline 2\nline 3"); diff --git a/src/app/screens/kw_ops.rs b/src/app/screens/kw_ops.rs index ef1d7ab..2a7069d 100644 --- a/src/app/screens/kw_ops.rs +++ b/src/app/screens/kw_ops.rs @@ -8,6 +8,13 @@ pub enum KwOpsFocus { ExtraArgs, } +/// Which deploy start was interrupted by the boot-once confirm popup. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum DeployStartKind { + Deploy, + BuildThenDeploy, +} + /// Form state on the KwOps screen. Job status lives in [`crate::app::state::KwUiState::status`]. #[derive(Clone, Debug)] pub struct KwOpsState { @@ -31,6 +38,12 @@ pub struct KwOpsState { /// Optimistic lock so a second Start before the watch snapshot /// arrives is ignored instead of refused with an error popup. pub start_requested: bool, + /// True after the user confirmed boot-into-new-kernel-once for this + /// KwOps visit. Preserved across `reenter` so a later Start does not + /// re-prompt in the same session. + pub boot_once_acknowledged: bool, + /// Deploy start waiting on the boot-once confirm popup. + pub pending_deploy: Option, } impl KwOpsState { @@ -58,11 +71,13 @@ impl KwOpsState { log_tail: String::new(), cancel_requested: false, start_requested: false, + boot_once_acknowledged: false, + pending_deploy: None, } } - /// Re-open KwOps for the same patchset/tree without dropping extras - /// or an in-flight cancel/start indication. + /// Re-open KwOps for the same patchset/tree without dropping extras, + /// an in-flight cancel/start indication, or a boot-once acknowledgement. /// /// Branch and `head_unreadable` stay as the user last edited them. /// An external HEAD change while away is not applied, so a stale @@ -248,6 +263,26 @@ mod tests { assert_eq!("main", ops.branch); } + #[test] + fn reenter_keeps_boot_once_ack_and_pending_deploy() { + let mut ops = KwOpsState::new( + "title".to_string(), + "mid".to_string(), + "linux".to_string(), + sample_tree(), + readiness(Some("main")), + ); + ops.boot_once_acknowledged = true; + ops.pending_deploy = Some(DeployStartKind::BuildThenDeploy); + ops.reenter( + "new title".to_string(), + sample_tree(), + readiness(Some("feature")), + ); + assert!(ops.boot_once_acknowledged); + assert_eq!(Some(DeployStartKind::BuildThenDeploy), ops.pending_deploy); + } + #[test] fn extra_arg_preview_tokens_follow_the_edit_buffer() { let mut ops = KwOpsState::new( diff --git a/src/app/view_model.rs b/src/app/view_model.rs index ad1a42d..25568a3 100644 --- a/src/app/view_model.rs +++ b/src/app/view_model.rs @@ -719,7 +719,7 @@ mod tests { state.popup = Some(AppPopup::quit_while_job_running()); let vm = project_state(&state); let popup = vm.popup.expect("confirm popup should project"); - assert_eq!("Cancel build and quit?", popup.title); + assert_eq!("Cancel job and quit?", popup.title); let PopupViewBody::Confirm { options, selected, .. } = popup.body @@ -733,6 +733,23 @@ mod tests { assert_eq!(1, selected); } + #[test] + fn boot_once_popup_projects_back_out_as_default() { + let mut state = app_state_with_kw(None); + state.popup = Some(AppPopup::boot_once_warning()); + let vm = project_state(&state); + let popup = vm.popup.expect("boot-once popup should project"); + assert_eq!("Boot into new kernel once?", popup.title); + let PopupViewBody::Confirm { + options, selected, .. + } = popup.body + else { + panic!("expected Confirm projection"); + }; + assert_eq!(vec!["Back out".to_string(), "Proceed".to_string()], options); + assert_eq!(0, selected); + } + #[test] fn kw_ops_command_strips_reserved_extras() { let mut state = app_state_with_kw(None); From c78089a00cd516593ec9462652d69b2d171199f8 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sat, 19 Sep 2026 21:06:39 -0300 Subject: [PATCH 09/12] feat(app): project deploy target, gates, and command previews on KwOps This commit replaces the KwOps deploy placeholder with the resolved remote, boot-once state, deploy argv, and (d)/(D) availability so the dashboard shows why deploy is blocked. Deploy failures append a known exit hint; boot-once On/Unknown does not disable the labels, because the confirm popup is still the gate. This commit is part of the kw integration's step 7. Signed-off-by: lorenzoberts --- src/app/view_model.rs | 281 +++++++++++++++++++++++++++++++++++++-- src/kw/status.rs | 2 +- src/ui/scene.rs | 6 +- src/ui/screens/kw_ops.rs | 24 +++- 4 files changed, 292 insertions(+), 21 deletions(-) diff --git a/src/app/view_model.rs b/src/app/view_model.rs index 25568a3..1c296db 100644 --- a/src/app/view_model.rs +++ b/src/app/view_model.rs @@ -17,8 +17,9 @@ use super::{ }; use crate::kw::{ argv, - readiness::TreeReadiness, - status::{KwJobStatus, KwStatusSnapshot}, + readiness::{BootOnceState, DeployAloneRefusal, TreeReadiness}, + remote::{KwRemote, RemoteRefusal}, + status::{deploy_exit_hint, KwJobStatus, KwPhase, KwStatusSnapshot}, }; /// One mailing list entry shown in the selection list. @@ -146,7 +147,11 @@ pub struct KwOpsViewModel { pub start_label: String, pub cancel_label: String, pub restore_label: String, - pub deploy_placeholder: String, + pub remote: String, + pub boot_once: String, + pub deploy_command: String, + pub deploy_label: String, + pub build_deploy_label: String, pub branch_guidance: Option, pub log_tail: String, } @@ -466,13 +471,25 @@ fn project_kw_ops(state: &AppState) -> KwOpsViewModel { } else { ops.extra_args.clone() }; - let start_label = if running || start_requested { - "unavailable (a job is already running)".to_string() - } else if ops.branch.trim().is_empty() { - "unavailable (set a branch first)".to_string() - } else { - "available (b)".to_string() + let branch_empty = ops.branch.trim().is_empty(); + let start_block = start_block_reason(running, start_requested, branch_empty); + let start_label = action_label(start_block.clone(), 'b'); + let remote_block = match &ops.readiness.deploy_remote { + Err(reason) => Some(format!("unavailable ({})", compact_remote_refusal(reason))), + Ok(_) => None, }; + let deploy_block = start_block + .clone() + .or_else(|| remote_block.clone()) + .or_else(|| match &ops.readiness.deploy_alone { + Err(reason) => Some(format!( + "unavailable ({})", + compact_deploy_alone_refusal(reason) + )), + Ok(()) => None, + }); + let deploy_label = action_label(deploy_block, 'd'); + let build_deploy_label = action_label(start_block.or(remote_block), 'D'); let cancel_label = if running { if ops.cancel_requested { "requested; waiting for the job to stop".to_string() @@ -529,10 +546,19 @@ fn project_kw_ops(state: &AppState) -> KwOpsViewModel { start_label, cancel_label, restore_label, - deploy_placeholder: "not available yet".to_string(), + remote: format_deploy_remote(&ops.readiness.deploy_remote), + boot_once: format_boot_once(ops.readiness.boot_once, ops.boot_once_acknowledged), + deploy_command: format_deploy_command( + &ops.readiness.deploy_remote, + state.config.kw_reboot_after_deploy(), + state.config.kw_deploy_force(), + &ops.extra_arg_tokens_for_preview(), + ), + deploy_label, + build_deploy_label, branch_guidance: if ops.head_unreadable && ops.branch.trim().is_empty() { Some( - "HEAD is detached or unverifiable; type a branch before starting a build." + "HEAD is detached or unverifiable; type a branch before starting a job." .to_string(), ) } else { @@ -542,6 +568,72 @@ fn project_kw_ops(state: &AppState) -> KwOpsViewModel { } } +fn start_block_reason(running: bool, start_requested: bool, branch_empty: bool) -> Option { + if running || start_requested { + Some("unavailable (a job is already running)".to_string()) + } else if branch_empty { + Some("unavailable (set a branch first)".to_string()) + } else { + None + } +} + +fn action_label(blocked: Option, key: char) -> String { + blocked.unwrap_or_else(|| format!("available ({key})")) +} + +fn format_deploy_remote(remote: &Result) -> String { + match remote { + Ok(remote) => remote.endpoint(), + Err(reason) => reason.to_string(), + } +} + +fn format_boot_once(state: BootOnceState, acknowledged: bool) -> String { + match (state, acknowledged) { + (BootOnceState::Off, _) => "off".to_string(), + (BootOnceState::On, true) => "on (confirmed)".to_string(), + (BootOnceState::Unknown, true) => "unknown (confirmed)".to_string(), + (BootOnceState::On, false) => "on (confirm before deploy)".to_string(), + (BootOnceState::Unknown, false) => "unknown (confirm before deploy)".to_string(), + } +} + +fn format_deploy_command( + remote: &Result, + reboot: bool, + force: bool, + extra_args: &[String], +) -> String { + match remote { + Ok(remote) => format!( + "kw {}", + argv::deploy_argv(&remote.endpoint(), reboot, force, extra_args).join(" ") + ), + Err(_) => "(no remote)".to_string(), + } +} + +fn compact_remote_refusal(reason: &RemoteRefusal) -> &'static str { + match reason { + RemoteRefusal::NoRemotesConfigured => "no remotes configured", + RemoteRefusal::NoDefault { .. } => "no default remote", + RemoteRefusal::DefaultNotFound { .. } => "default remote missing", + } +} + +fn compact_deploy_alone_refusal(reason: &DeployAloneRefusal) -> &'static str { + match reason { + DeployAloneRefusal::TreeNotReady(_) => "tree not ready", + DeployAloneRefusal::NoBuildRecord => "no build recorded", + DeployAloneRefusal::LastBuildFailed => "last build failed", + DeployAloneRefusal::HeadMismatch { .. } => "build was on another branch", + DeployAloneRefusal::TreePathDrift { .. } => "tree path changed", + DeployAloneRefusal::OutputDirMismatch => "kw env changed", + DeployAloneRefusal::ImageMissing => "kernel image missing", + } +} + fn format_kw_binary(probe: &crate::kw::readiness::KwBinaryProbe) -> String { if !probe.available { return "not on PATH".to_string(); @@ -565,8 +657,8 @@ fn format_job_status(job: Option<&KwJobStatus>, cancel_requested: bool) -> Strin None | Some(KwJobStatus::Idle) => "idle".to_string(), Some(KwJobStatus::Running { phase, branch, .. }) => { let phase = match phase { - crate::kw::status::KwPhase::Building => "building", - crate::kw::status::KwPhase::Deploying => "deploying", + KwPhase::Building => "building", + KwPhase::Deploying => "deploying", }; if cancel_requested { format!("cancelling {phase} {branch}") @@ -575,6 +667,17 @@ fn format_job_status(job: Option<&KwJobStatus>, cancel_requested: bool) -> Strin } } Some(KwJobStatus::Succeeded { branch, .. }) => format!("succeeded on {branch}"), + Some(KwJobStatus::Failed { + phase: KwPhase::Deploying, + exit_code, + .. + }) => match exit_code { + Some(code) => match deploy_exit_hint(*code) { + Some(hint) => format!("failed during deploy (exit {code}: {hint})"), + None => format!("failed during deploy (exit {code})"), + }, + None => "failed during deploy (exit unknown)".to_string(), + }, Some(KwJobStatus::Failed { exit_code, .. }) => match exit_code { Some(code) => format!("failed (exit {code})"), None => "failed (exit unknown)".to_string(), @@ -791,7 +894,14 @@ mod tests { assert_eq!("kw build --verbose", vm.command); assert_eq!("feature", vm.branch); assert_eq!("available (b)", vm.start_label); - assert_eq!("not available yet", vm.deploy_placeholder); + assert_eq!( + "no remotes configured; configure a remote with `kw remote --set-default` or edit `.kw/remote.config`", + vm.remote + ); + assert_eq!("unknown (confirm before deploy)", vm.boot_once); + assert_eq!("(no remote)", vm.deploy_command); + assert_eq!("unavailable (no remotes configured)", vm.deploy_label); + assert_eq!("unavailable (no remotes configured)", vm.build_deploy_label); } fn sample_kw_ops(branch: Option<&str>) -> crate::app::screens::kw_ops::KwOpsState { @@ -838,6 +948,8 @@ mod tests { }; assert_eq!(None, vm.branch_guidance); assert_eq!("unavailable (set a branch first)", vm.start_label); + assert_eq!("unavailable (set a branch first)", vm.deploy_label); + assert_eq!("unavailable (set a branch first)", vm.build_deploy_label); } #[test] @@ -889,6 +1001,147 @@ mod tests { }; assert_eq!("starting…", vm.job_status); assert_eq!("unavailable (a job is already running)", vm.start_label); + assert_eq!("unavailable (a job is already running)", vm.deploy_label); + assert_eq!( + "unavailable (a job is already running)", + vm.build_deploy_label + ); + } + + fn sample_remote() -> crate::kw::remote::KwRemote { + crate::kw::remote::KwRemote { + name: "dut".to_string(), + hostname: "box".to_string(), + port: 22, + user: Some("root".to_string()), + } + } + + #[test] + fn deploy_alone_refusal_disables_deploy_but_not_build_then_deploy() { + let mut state = app_state_with_kw(None); + state.navigation.current_screen = CurrentScreen::KwOps; + let mut ops = sample_kw_ops(Some("feature")); + ops.readiness.deploy_remote = Ok(sample_remote()); + ops.readiness.deploy_alone = Err(crate::kw::readiness::DeployAloneRefusal::NoBuildRecord); + ops.readiness.boot_once = crate::kw::readiness::BootOnceState::On; + state.kw.ops = Some(ops); + + let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { + panic!("expected KwOps projection"); + }; + assert_eq!("root@box:22", vm.remote); + assert_eq!("on (confirm before deploy)", vm.boot_once); + assert_eq!( + "kw deploy --remote root@box:22 --no-reboot --force", + vm.deploy_command + ); + assert_eq!("unavailable (no build recorded)", vm.deploy_label); + assert_eq!("available (D)", vm.build_deploy_label); + } + + #[test] + fn matching_record_and_remote_enable_deploy_actions() { + let mut state = app_state_with_kw(None); + state.navigation.current_screen = CurrentScreen::KwOps; + let mut ops = sample_kw_ops(Some("feature")); + ops.readiness.deploy_remote = Ok(sample_remote()); + ops.readiness.deploy_alone = Ok(()); + ops.readiness.boot_once = crate::kw::readiness::BootOnceState::Off; + ops.extra_args = "--verbose --local".to_string(); + state.kw.ops = Some(ops); + + let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { + panic!("expected KwOps projection"); + }; + assert_eq!("off", vm.boot_once); + assert_eq!("available (d)", vm.deploy_label); + assert_eq!("available (D)", vm.build_deploy_label); + assert_eq!( + "kw deploy --remote root@box:22 --no-reboot --force --verbose", + vm.deploy_command + ); + } + + #[test] + fn boot_once_acknowledgement_does_not_change_deploy_availability() { + let mut state = app_state_with_kw(None); + state.navigation.current_screen = CurrentScreen::KwOps; + let mut ops = sample_kw_ops(Some("feature")); + ops.readiness.deploy_remote = Ok(sample_remote()); + ops.readiness.deploy_alone = Ok(()); + ops.readiness.boot_once = crate::kw::readiness::BootOnceState::Unknown; + ops.boot_once_acknowledged = true; + state.kw.ops = Some(ops); + + let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { + panic!("expected KwOps projection"); + }; + assert_eq!("unknown (confirmed)", vm.boot_once); + assert_eq!("available (d)", vm.deploy_label); + } + + #[test] + fn deploy_command_follows_reboot_and_force_config() { + let mut state = app_state_with_kw(None); + state.navigation.current_screen = CurrentScreen::KwOps; + let mut config = ConfigState::default(); + config.kw_reboot_after_deploy = true; + config.kw_deploy_force = false; + state.config = config.to_snapshot(); + let mut ops = sample_kw_ops(Some("feature")); + ops.readiness.deploy_remote = Ok(sample_remote()); + state.kw.ops = Some(ops); + + let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { + panic!("expected KwOps projection"); + }; + assert_eq!("kw deploy --remote root@box:22 --reboot", vm.deploy_command); + } + + #[test] + fn failed_deploy_projects_known_exit_hint() { + let mut state = app_state_with_kw(Some(KwStatusSnapshot { + job: KwJobStatus::Failed { + kind: KwJobKind::Deploy, + phase: KwPhase::Deploying, + exit_code: Some(101), + log_path: PathBuf::from("/tmp/deploy.log"), + }, + restore_branch: None, + })); + state.navigation.current_screen = CurrentScreen::KwOps; + let mut ops = sample_kw_ops(Some("feature")); + ops.readiness.deploy_remote = Ok(sample_remote()); + state.kw.ops = Some(ops); + + let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { + panic!("expected KwOps projection"); + }; + assert_eq!( + "failed during deploy (exit 101: SSH unreachable after setup)", + vm.job_status + ); + } + + #[test] + fn failed_build_keeps_the_generic_exit_line() { + let mut state = app_state_with_kw(Some(KwStatusSnapshot { + job: KwJobStatus::Failed { + kind: KwJobKind::Build, + phase: KwPhase::Building, + exit_code: Some(1), + log_path: PathBuf::from("/tmp/build.log"), + }, + restore_branch: None, + })); + state.navigation.current_screen = CurrentScreen::KwOps; + state.kw.ops = Some(sample_kw_ops(Some("feature"))); + + let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { + panic!("expected KwOps projection"); + }; + assert_eq!("failed (exit 1)", vm.job_status); } #[test] diff --git a/src/kw/status.rs b/src/kw/status.rs index 42401cb..d55d4a0 100644 --- a/src/kw/status.rs +++ b/src/kw/status.rs @@ -105,7 +105,7 @@ impl KwJobStatus { /// show the raw number. 68 can still surface even with `--force`: force /// only skips the interactive prompt, not the underlying initramfs /// errors. -#[cfg_attr(not(test), allow(dead_code))] +#[cfg_attr(not(unix), allow(dead_code))] pub fn deploy_exit_hint(code: i32) -> Option<&'static str> { Some(match code { 2 => "kernel image not found", diff --git a/src/ui/scene.rs b/src/ui/scene.rs index 92d0b66..63eb479 100644 --- a/src/ui/scene.rs +++ b/src/ui/scene.rs @@ -88,7 +88,11 @@ pub struct KwOpsScene { pub start_label: String, pub cancel_label: String, pub restore_label: String, - pub deploy_placeholder: String, + pub remote: String, + pub boot_once: String, + pub deploy_command: String, + pub deploy_label: String, + pub build_deploy_label: String, pub branch_guidance: Option, pub log_tail: String, } diff --git a/src/ui/screens/kw_ops.rs b/src/ui/screens/kw_ops.rs index 0d89d3b..0331a2d 100644 --- a/src/ui/screens/kw_ops.rs +++ b/src/ui/screens/kw_ops.rs @@ -27,7 +27,11 @@ pub fn build_scene(vm: &KwOpsViewModel) -> KwOpsScene { start_label: vm.start_label.clone(), cancel_label: vm.cancel_label.clone(), restore_label: vm.restore_label.clone(), - deploy_placeholder: vm.deploy_placeholder.clone(), + remote: vm.remote.clone(), + boot_once: vm.boot_once.clone(), + deploy_command: vm.deploy_command.clone(), + deploy_label: vm.deploy_label.clone(), + build_deploy_label: vm.build_deploy_label.clone(), branch_guidance: vm.branch_guidance.clone(), log_tail: vm.log_tail.clone(), } @@ -66,16 +70,19 @@ fn paint_form(f: &mut Frame, scene: &KwOpsScene, chunk: Rect) { ))); } lines.extend([ - Line::from(""), labeled("kw", &scene.kw_binary), labeled("Tree status", &scene.tree_readiness), labeled("Output dir", &scene.output_dir), + labeled("Remote", &scene.remote), + labeled("Boot once", &scene.boot_once), labeled("Job", &scene.job_status), labeled("Command", &scene.command), + labeled("Deploy command", &scene.deploy_command), labeled("Start", &scene.start_label), + labeled("Deploy", &scene.deploy_label), + labeled("Build+deploy", &scene.build_deploy_label), labeled("Cancel", &scene.cancel_label), labeled("Restore", &scene.restore_label), - labeled("Deploy", &scene.deploy_placeholder), ]); let paragraph = Paragraph::new(lines) @@ -93,7 +100,7 @@ fn paint_log(f: &mut Frame, scene: &KwOpsScene, chunk: Rect) { let inner_height = chunk.height.saturating_sub(2); let offset = log_scroll_offset(&scene.log_tail, inner_width, inner_height); let paragraph = Paragraph::new(scene.log_tail.clone()) - .block(Block::default().borders(Borders::ALL).title(" Build log ")) + .block(Block::default().borders(Borders::ALL).title(" Job log ")) .wrap(Wrap { trim: false }) .scroll((offset, 0)); f.render_widget(paragraph, chunk); @@ -158,7 +165,7 @@ pub fn keys_hint_span(editing: bool) -> Span<'static> { ) } else { Span::styled( - "(ESC / q) back | (e) edit | (b) build | (c) cancel | (r) restore | (?) help", + "(ESC / q) back | (e) edit | (b) build | (d) deploy | (D) build+deploy | (c) cancel | (r) restore | (?) help", Style::default().fg(Color::Red), ) } @@ -206,4 +213,11 @@ mod tests { assert_eq!(0, log_scroll_offset("line\nline\n", 0, 10)); assert_eq!(0, log_scroll_offset("line\nline\n", 10, 0)); } + + #[test] + fn keys_hint_lists_deploy_bindings() { + let hint = keys_hint_span(false); + assert!(hint.content.contains("(d) deploy")); + assert!(hint.content.contains("(D) build+deploy")); + } } From f6db6c4a4399fcb1afae565d277e203539569011 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 20 Sep 2026 09:37:27 -0300 Subject: [PATCH 10/12] feat(app): start deploys from KwOps This commit wires d/D on KwOps to StartDeploy and StartBuildThenDeploy so the dashboard can actually launch the jobs it already previews. Boot-once On/Unknown opens the confirm popup instead of starting; Proceed re-issues the pending kind with the acknowledgement, and a branch-field commit re-probes deploy-alone for the typed branch. This commit is part of the kw integration's step 7. Signed-off-by: lorenzoberts --- src/app/actor.rs | 6 +- src/app/flows/kw_ops.rs | 216 +++++++++++-- src/app/integration_tests/kw_ops.rs | 457 +++++++++++++++++++++++++++- src/input/event.rs | 2 + src/input/mapper.rs | 10 + 5 files changed, 646 insertions(+), 45 deletions(-) diff --git a/src/app/actor.rs b/src/app/actor.rs index b4c1558..66e6f27 100644 --- a/src/app/actor.rs +++ b/src/app/actor.rs @@ -598,10 +598,14 @@ mod tests { .await .unwrap(); assert_eq!(ControlFlow::Continue(()), flow); - assert!(app.state.popup.is_none()); let ops = app.state.kw.ops.as_ref().unwrap(); assert!(ops.boot_once_acknowledged); assert_eq!(None, ops.pending_deploy); + let Some(AppPopup::Info { title, body, .. }) = &app.state.popup else { + panic!("resume without a kw actor should explain that deploy cannot start"); + }; + assert_eq!("Cannot start deploy", title); + assert!(body.contains("not attached")); } #[tokio::test] diff --git a/src/app/flows/kw_ops.rs b/src/app/flows/kw_ops.rs index fe8b4bd..d1412b7 100644 --- a/src/app/flows/kw_ops.rs +++ b/src/app/flows/kw_ops.rs @@ -5,14 +5,18 @@ use color_eyre::Result; use crate::{ app::{ popup::AppPopup, - screens::{kw_ops::KwOpsState, CurrentScreen}, + screens::{ + kw_ops::{DeployStartKind, KwOpsFocus, KwOpsState}, + CurrentScreen, + }, App, }, infrastructure::file_system::FileSystemError, input::event::InputEvent, kw::{ - errors::KwError, - messages::StartRequest, + errors::{KwError, KwStartError}, + messages::{DeployOptions, StartRequest}, + readiness::BootOnceState, status::{KwJobStatus, KwStatusSnapshot}, }, }; @@ -121,14 +125,31 @@ pub(crate) async fn fallback_kw_status(app: &mut App) { pub async fn handle_kw_ops(app: &mut App, input: InputEvent) -> Result<()> { let editing = app.state.kw.ops.as_ref().is_some_and(|ops| ops.editing); if editing { - let Some(ops) = app.state.kw.ops.as_mut() else { - return Ok(()); - }; match input { - InputEvent::CancelKwOpsEdit => ops.cancel_edit(), - InputEvent::Backspace => ops.backspace_edit(), - InputEvent::TextInput(ch) => ops.append_edit(ch), - InputEvent::StageKwOpsEdit => ops.commit_edit(), + InputEvent::CancelKwOpsEdit => { + if let Some(ops) = app.state.kw.ops.as_mut() { + ops.cancel_edit(); + } + } + InputEvent::Backspace => { + if let Some(ops) = app.state.kw.ops.as_mut() { + ops.backspace_edit(); + } + } + InputEvent::TextInput(ch) => { + if let Some(ops) = app.state.kw.ops.as_mut() { + ops.append_edit(ch); + } + } + InputEvent::StageKwOpsEdit => { + let focus = app.state.kw.ops.as_ref().map(|ops| ops.focus); + if let Some(ops) = app.state.kw.ops.as_mut() { + ops.commit_edit(); + } + if focus == Some(KwOpsFocus::Branch) { + refresh_kw_ops_readiness(app).await; + } + } _ => {} } return Ok(()); @@ -158,7 +179,9 @@ pub async fn handle_kw_ops(app: &mut App, input: InputEvent) -> Result<()> { } } } - InputEvent::StartKwBuild => start_build(app).await?, + InputEvent::StartKwBuild => start_job(app, KwStartKind::Build).await?, + InputEvent::StartKwDeploy => start_job(app, KwStartKind::Deploy).await?, + InputEvent::StartKwBuildThenDeploy => start_job(app, KwStartKind::BuildThenDeploy).await?, InputEvent::CancelKwJob => cancel_job(app).await?, InputEvent::RestoreKwBranch => restore_branch(app).await?, _ => {} @@ -228,7 +251,39 @@ pub async fn open_kw_ops(app: &mut App) -> Result<()> { Ok(()) } -async fn start_build(app: &mut App) -> Result<()> { +#[derive(Clone, Copy)] +enum KwStartKind { + Build, + Deploy, + BuildThenDeploy, +} + +impl KwStartKind { + fn title(self) -> &'static str { + match self { + Self::Build => "Cannot start build", + Self::Deploy => "Cannot start deploy", + Self::BuildThenDeploy => "Cannot start build+deploy", + } + } + + fn action_word(self) -> &'static str { + match self { + Self::Build => "build", + Self::Deploy | Self::BuildThenDeploy => "job", + } + } + + fn pending_kind(self) -> Option { + match self { + Self::Build => None, + Self::Deploy => Some(DeployStartKind::Deploy), + Self::BuildThenDeploy => Some(DeployStartKind::BuildThenDeploy), + } + } +} + +async fn start_job(app: &mut App, kind: KwStartKind) -> Result<()> { if job_is_busy(app) { return Ok(()); } @@ -238,18 +293,27 @@ async fn start_build(app: &mut App) -> Result<()> { let branch = ops.branch.trim().to_string(); if branch.is_empty() { let body = if ops.head_unreadable { - "HEAD is detached or unverifiable. Type a branch name before starting a build." + format!( + "HEAD is detached or unverifiable. Type a branch name before starting a {}.", + kind.action_word() + ) } else { - "Set a branch before starting a build." + format!("Set a branch before starting a {}.", kind.action_word()) }; - app.state.popup = Some(AppPopup::info("Cannot start build", body)); + app.state.popup = Some(AppPopup::info(kind.title(), body)); return Ok(()); } - let Some(kw) = app.services.kw.clone() else { - app.state.popup = Some(AppPopup::info( - "Cannot start build", - "kw jobs require a Unix kw actor, which is not attached.", - )); + if kind.pending_kind().is_some() && needs_boot_once_confirm(ops) { + if let Some(ops) = app.state.kw.ops.as_mut() { + ops.pending_deploy = kind.pending_kind(); + } + app.state.popup = Some(AppPopup::boot_once_warning()); + return Ok(()); + } + + let reboot = app.state.config.kw_reboot_after_deploy(); + let force = app.state.config.kw_deploy_force(); + let Some(ops) = app.state.kw.ops.as_ref() else { return Ok(()); }; let request = StartRequest { @@ -257,9 +321,25 @@ async fn start_build(app: &mut App) -> Result<()> { tree: ops.tree.clone(), branch, extra_args: ops.extra_arg_tokens(), - deploy: None, + deploy: kind.pending_kind().map(|_| DeployOptions { + reboot, + force, + boot_once_acknowledged: ops.boot_once_acknowledged, + }), }; - match kw.start_build(request).await { + let Some(kw) = app.services.kw.clone() else { + app.state.popup = Some(AppPopup::info( + kind.title(), + "kw jobs require a Unix kw actor, which is not attached.", + )); + return Ok(()); + }; + let result = match kind { + KwStartKind::Build => kw.start_build(request).await, + KwStartKind::Deploy => kw.start_deploy(request).await, + KwStartKind::BuildThenDeploy => kw.start_build_then_deploy(request).await, + }; + match result { Ok(()) => { if let Some(ops) = app.state.kw.ops.as_mut() { ops.cancel_requested = false; @@ -272,13 +352,65 @@ async fn start_build(app: &mut App) -> Result<()> { apply_kw_snapshot(app, snapshot); } } + Err(KwStartError::BootOnceNotAcknowledged) => { + if let Some(ops) = app.state.kw.ops.as_mut() { + ops.pending_deploy = kind.pending_kind(); + } + app.state.popup = Some(AppPopup::boot_once_warning()); + } Err(error) => { - app.state.popup = Some(AppPopup::info("Cannot start build", error.to_string())); + app.state.popup = Some(AppPopup::info(kind.title(), error.to_string())); } } Ok(()) } +fn needs_boot_once_confirm(ops: &KwOpsState) -> bool { + matches!( + ops.readiness.boot_once, + BootOnceState::On | BootOnceState::Unknown + ) && !ops.boot_once_acknowledged +} + +async fn refresh_kw_ops_readiness(app: &mut App) { + let Some(kw) = app.services.kw.clone() else { + return; + }; + let Some((kernel_tree_id, tree, branch)) = app.state.kw.ops.as_ref().map(|ops| { + ( + ops.kernel_tree_id.clone(), + ops.tree.clone(), + ops.branch.clone(), + ) + }) else { + return; + }; + let for_branch = { + let trimmed = branch.trim(); + if trimmed.is_empty() { + None + } else { + Some(trimmed.to_string()) + } + }; + match kw + .get_readiness(&kernel_tree_id, &tree, for_branch.as_deref()) + .await + { + Ok(readiness) => { + if let Some(ops) = app.state.kw.ops.as_mut() { + ops.readiness = readiness; + } + } + Err(error) => { + app.state.popup = Some(AppPopup::info( + "Cannot refresh kw status", + error.to_string(), + )); + } + } +} + async fn cancel_job(app: &mut App) -> Result<()> { if !job_is_running(app) { return Ok(()); @@ -351,16 +483,20 @@ fn job_is_busy(app: &App) -> bool { .is_some_and(|ops| ops.start_requested) } -/// Records that the user confirmed boot-once and consumes the pending -/// deploy kind. The KwOps input commit re-issues Start against this -/// acknowledgement; until then Proceed only stores the ack. +/// Records that the user confirmed boot-once and re-issues the pending +/// StartDeploy / StartBuildThenDeploy against that acknowledgement. pub(crate) async fn resume_pending_deploy(app: &mut App) -> Result<()> { let Some(ops) = app.state.kw.ops.as_mut() else { return Ok(()); }; ops.boot_once_acknowledged = true; - let _kind = ops.pending_deploy.take(); - Ok(()) + let Some(kind) = ops.pending_deploy.take() else { + return Ok(()); + }; + match kind { + DeployStartKind::Deploy => start_job(app, KwStartKind::Deploy).await, + DeployStartKind::BuildThenDeploy => start_job(app, KwStartKind::BuildThenDeploy).await, + } } /// Drops a deploy start that was waiting on the boot-once confirm popup. @@ -373,13 +509,13 @@ pub(crate) fn clear_pending_deploy(app: &mut App) { pub fn generate_help_popup() -> AppPopup { AppPopup::help() .title("Kw operations") - .description( - "Start a kw build on the configured target kernel tree. Deploy is not available yet.", - ) + .description("Start a kw build and/or remote deploy on the configured target kernel tree.") .keybind("ESC / q", "Return to patchset details") .keybind("j/k", "Move between branch and extra arguments") .keybind("e / ENTER", "Edit the focused field") .keybind("b", "Start build") + .keybind("d", "Start deploy") + .keybind("D", "Start build then deploy") .keybind("c", "Cancel the running job") .keybind("r", "Restore the previous branch") .keybind("?", "Show this help screen") @@ -453,4 +589,22 @@ mod tests { assert!(assign_log_tail(&mut ops, "done".to_string())); assert_eq!("done", ops.log_tail); } + + #[test] + fn help_lists_deploy_keys() { + let AppPopup::Help { + description, + formatted_keybinds, + .. + } = generate_help_popup() + else { + panic!("expected help popup"); + }; + assert!(description + .as_deref() + .is_some_and(|text| text.contains("remote deploy"))); + assert!(formatted_keybinds.contains("d: Start deploy")); + assert!(formatted_keybinds.contains("D: Start build then deploy")); + assert!(!formatted_keybinds.contains("not available yet")); + } } diff --git a/src/app/integration_tests/kw_ops.rs b/src/app/integration_tests/kw_ops.rs index 28d701d..865ce3d 100644 --- a/src/app/integration_tests/kw_ops.rs +++ b/src/app/integration_tests/kw_ops.rs @@ -90,7 +90,8 @@ mod unix { input::{event::InputEvent, handle::InputHandle, messages::InputMessage}, kw::{ actor::KwActor, - history::MockKwHistoryStore, + history::{KwBuildRecord, MockKwHistoryStore}, + readiness::{BootOnceState, DeployAloneRefusal}, status::{KwJobKind, KwJobStatus, KwPhase, KwStatusSnapshot}, }, lore::application::cache::BootstrapLoreData, @@ -104,9 +105,13 @@ mod unix { }; use super::{assert_info_popup, details_state}; - use crate::app::integration_tests::helpers::{ - app_harness::{dummy_config_handle, dummy_render_handle, dummy_terminal_handle}, - lore::{lore_handle_with_persistence, sample_mailing_list}, + use crate::app::{ + integration_tests::helpers::{ + app_harness::{dummy_config_handle, dummy_render_handle, dummy_terminal_handle}, + lore::{lore_handle_with_persistence, sample_mailing_list}, + }, + popup::AppPopup, + screens::kw_ops::DeployStartKind, }; static LOG_DIR_SEQ: AtomicU64 = AtomicU64::new(0); @@ -349,6 +354,304 @@ mod unix { std::fs::remove_dir_all(&log_dir).unwrap(); } + #[tokio::test] + async fn open_kw_ops_projects_the_resolved_remote() { + let log_dir = kw_log_dir("remote-display"); + let mut app = app_with_kw( + &log_dir, + head_branch_shell("feature"), + Arc::new(FakeProcess::new()), + Arc::new(MockFileSystemTrait::new()), + deploy_kw_fs(true), + default_kw_history(), + ); + handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) + .await + .unwrap(); + + let ops = app.state.kw.ops.as_ref().expect("KwOps state"); + assert_eq!( + "root@box:22", + ops.readiness.deploy_remote.as_ref().unwrap().endpoint() + ); + assert_eq!(BootOnceState::Off, ops.readiness.boot_once); + + shutdown_kw(&app).await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_deploy_opens_the_boot_once_gate() { + let log_dir = kw_log_dir("deploy-gate"); + let process = Arc::new(FakeProcess::new()); + let mut app = app_with_details_and_kw_process( + &log_dir, + head_branch_shell("feature"), + process.clone(), + Arc::new(MockFileSystemTrait::new()), + ); + handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) + .await + .unwrap(); + + handle_kw_ops(&mut app, InputEvent::StartKwDeploy) + .await + .unwrap(); + + let Some(AppPopup::Confirm { title, .. }) = app.state.popup.as_ref() else { + panic!("expected boot-once confirm popup"); + }; + assert_eq!("Boot into new kernel once?", title); + assert_eq!( + Some(DeployStartKind::Deploy), + app.state.kw.ops.as_ref().unwrap().pending_deploy + ); + assert!(process.spawned().is_empty()); + + shutdown_kw(&app).await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_deploy_refused_without_a_build_record() { + let log_dir = kw_log_dir("deploy-no-record"); + let process = Arc::new(FakeProcess::new()); + let mut app = app_with_kw( + &log_dir, + head_branch_shell("feature"), + process.clone(), + Arc::new(MockFileSystemTrait::new()), + deploy_kw_fs(true), + default_kw_history(), + ); + handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) + .await + .unwrap(); + + handle_kw_ops(&mut app, InputEvent::StartKwDeploy) + .await + .unwrap(); + assert_info_popup( + app.state.popup.as_ref(), + "Cannot start deploy", + "no build recorded", + ); + assert!(process.spawned().is_empty()); + + shutdown_kw(&app).await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_deploy_accepts_when_ready() { + let log_dir = kw_log_dir("deploy-ready"); + let process = Arc::new(FakeProcess::new()); + let mut app = app_with_kw( + &log_dir, + head_branch_shell("feature"), + process.clone(), + Arc::new(MockFileSystemTrait::new()), + deploy_kw_fs(true), + feature_build_history(Some(matching_feature_build_record())), + ); + handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) + .await + .unwrap(); + + handle_kw_ops(&mut app, InputEvent::StartKwDeploy) + .await + .unwrap(); + assert!(app.state.popup.is_none()); + let spawned = process.spawned(); + assert_eq!(1, spawned.len()); + assert_eq!( + [ + "deploy", + "--remote", + "root@box:22", + "--no-reboot", + "--force" + ] + .as_slice(), + spawned[0].args.as_slice() + ); + assert!(matches!( + app.state.kw.status.as_ref().map(|s| &s.job), + Some(KwJobStatus::Running { + kind: KwJobKind::Deploy, + phase: KwPhase::Deploying, + .. + }) + )); + + process.last_child().finish(0); + shutdown_kw(&app).await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_build_then_deploy_chains_two_processes() { + let log_dir = kw_log_dir("build-then-deploy"); + let process = Arc::new(FakeProcess::new()); + let mut app = app_with_kw( + &log_dir, + head_branch_shell("feature"), + process.clone(), + Arc::new(MockFileSystemTrait::new()), + deploy_kw_fs(true), + default_kw_history(), + ); + handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) + .await + .unwrap(); + + handle_kw_ops(&mut app, InputEvent::StartKwBuildThenDeploy) + .await + .unwrap(); + assert!(app.state.popup.is_none()); + assert_eq!(vec!["build"], process.spawned()[0].args); + + process.last_child().finish(0); + tokio::time::timeout(Duration::from_secs(5), async { + loop { + if process.spawned().len() >= 2 { + return; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("deploy should spawn after a successful build"); + + let spawned = process.spawned(); + assert_eq!("deploy", spawned[1].args[0]); + assert!(spawned[1].args.iter().any(|arg| arg == "root@box:22")); + + process.last_child().finish(0); + shutdown_kw(&app).await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn committing_a_branch_reprobes_deploy_alone() { + let log_dir = kw_log_dir("branch-reprobe"); + let mut history = MockKwHistoryStore::new(); + history + .expect_apply_record_for_branch() + .returning(|_, _| Ok(None)); + history.expect_record_build().returning(|_| Ok(())); + history.expect_build_records().returning(|_, branch| { + if branch == "built" { + Ok(( + Some(matching_build_record("built")), + Some(matching_build_record("built")), + )) + } else { + Ok((None, None)) + } + }); + let mut app = app_with_kw( + &log_dir, + head_branch_shell("feature"), + Arc::new(FakeProcess::new()), + Arc::new(MockFileSystemTrait::new()), + deploy_kw_fs(true), + history, + ); + handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) + .await + .unwrap(); + assert!(matches!( + app.state.kw.ops.as_ref().unwrap().readiness.deploy_alone, + Err(DeployAloneRefusal::NoBuildRecord) + )); + + handle_kw_ops(&mut app, InputEvent::EditKwOpsField) + .await + .unwrap(); + app.state.kw.ops.as_mut().unwrap().edit_buffer = "built".to_string(); + handle_kw_ops(&mut app, InputEvent::StageKwOpsEdit) + .await + .unwrap(); + + let ops = app.state.kw.ops.as_ref().unwrap(); + assert_eq!("built", ops.branch); + assert_eq!(Ok(()), ops.readiness.deploy_alone); + assert_eq!(Some("feature".to_string()), ops.readiness.current_branch); + + shutdown_kw(&app).await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test(flavor = "multi_thread")] + async fn boot_once_enter_backs_out_without_starting() { + let log_dir = kw_log_dir("boot-once-back-out"); + let process = Arc::new(FakeProcess::new()); + let app = app_with_kw( + &log_dir, + head_branch_shell("feature"), + process.clone(), + Arc::new(MockFileSystemTrait::new()), + deploy_kw_fs(false), + feature_build_history(Some(matching_feature_build_record())), + ); + let kw = app.services.kw.as_ref().unwrap().clone(); + let (scenes, event_tx, handle) = spawn_app_actor(app); + + event_tx.send(InputEvent::OpenKwOps).await.unwrap(); + wait_for_kw_ops(&scenes, |_| true).await; + event_tx.send(InputEvent::StartKwDeploy).await.unwrap(); + wait_for_latest_popup(&scenes, |popup| { + popup.is_some_and(|popup| popup.title == "Boot into new kernel once?") + }) + .await; + + event_tx.send(InputEvent::ConfirmPopup).await.unwrap(); + wait_for_latest_popup(&scenes, |popup| popup.is_none()).await; + assert!(process.spawned().is_empty()); + + drop(event_tx); + handle.run_until_done().await.unwrap(); + kw.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test(flavor = "multi_thread")] + async fn boot_once_proceed_starts_deploy() { + let log_dir = kw_log_dir("boot-once-proceed"); + let process = Arc::new(FakeProcess::new()); + let app = app_with_kw( + &log_dir, + head_branch_shell("feature"), + process.clone(), + Arc::new(MockFileSystemTrait::new()), + deploy_kw_fs(false), + feature_build_history(Some(matching_feature_build_record())), + ); + let kw = app.services.kw.as_ref().unwrap().clone(); + let (scenes, event_tx, handle) = spawn_app_actor(app); + + event_tx.send(InputEvent::OpenKwOps).await.unwrap(); + wait_for_kw_ops(&scenes, |_| true).await; + event_tx.send(InputEvent::StartKwDeploy).await.unwrap(); + wait_for_latest_popup(&scenes, |popup| popup.is_some()).await; + + event_tx.send(InputEvent::NavigateRight).await.unwrap(); + event_tx.send(InputEvent::ConfirmPopup).await.unwrap(); + wait_for_nav(&scenes, |text| text.contains("kw: deploying feature")).await; + wait_for_latest_popup(&scenes, |popup| popup.is_none()).await; + + let spawned = process.spawned(); + assert_eq!(1, spawned.len()); + assert_eq!("deploy", spawned[0].args[0]); + + process.last_child().finish(0); + drop(event_tx); + handle.run_until_done().await.unwrap(); + kw.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + #[tokio::test(flavor = "multi_thread")] async fn live_tail_updates_without_input_and_pauses_off_screen() { let log_dir = kw_log_dir("live-tail"); @@ -445,19 +748,29 @@ mod unix { process: Arc, app_fs: Arc, ) -> App { - let mut history = MockKwHistoryStore::new(); - history - .expect_apply_record_for_branch() - .returning(|_, _| Ok(None)); - history.expect_record_build().returning(|_| Ok(())); - history - .expect_build_records() - .returning(|_, _| Ok((None, None))); + app_with_kw( + log_dir, + kw_shell, + process, + app_fs, + kw_actor_fs(), + default_kw_history(), + ) + } + + fn app_with_kw( + log_dir: &std::path::Path, + kw_shell: MockShellTrait, + process: Arc, + app_fs: Arc, + kw_fs: MockFileSystemTrait, + history: MockKwHistoryStore, + ) -> App { let kw = KwActor::spawn( Arc::new(history), process, Arc::new(kw_shell), - Arc::new(kw_actor_fs()), + Arc::new(kw_fs), Arc::new(kw_actor_env()), log_dir.to_path_buf(), ); @@ -482,6 +795,103 @@ mod unix { app } + fn default_kw_history() -> MockKwHistoryStore { + let mut history = MockKwHistoryStore::new(); + history + .expect_apply_record_for_branch() + .returning(|_, _| Ok(None)); + history.expect_record_build().returning(|_| Ok(())); + history + .expect_build_records() + .returning(|_, _| Ok((None, None))); + history + } + + fn feature_build_history(record: Option) -> MockKwHistoryStore { + let mut history = MockKwHistoryStore::new(); + history + .expect_apply_record_for_branch() + .returning(|_, _| Ok(None)); + history.expect_record_build().returning(|_| Ok(())); + history.expect_build_records().returning(move |_, branch| { + if branch == "feature" { + Ok((record.clone(), record.clone())) + } else { + Ok((None, None)) + } + }); + history + } + + fn matching_feature_build_record() -> KwBuildRecord { + matching_build_record("feature") + } + + fn matching_build_record(branch: &str) -> KwBuildRecord { + KwBuildRecord { + kernel_tree_id: "linux".to_string(), + tree_path: "/kernel".to_string(), + message_id: None, + branch: branch.to_string(), + arch: Some("x86".to_string()), + image_path: Some("/kernel/arch/x86/boot/bzImage".to_string()), + output_dir: None, + kernelrelease: Some("6.17.0".to_string()), + log_path: String::new(), + built_at: "2026-08-01T18:10:00Z".to_string(), + success: true, + } + } + + fn deploy_kw_fs(boot_once_off: bool) -> MockFileSystemTrait { + let deploy_config = if boot_once_off { + "boot_into_new_kernel_once=no\n".to_string() + } else { + "boot_into_new_kernel_once=yes\n".to_string() + }; + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_dir().returning(|_| true); + fs.expect_is_file() + .returning(|path| !path.ends_with(".kw/env.current")); + fs.expect_exists().returning(|_| true); + fs.expect_read_to_string().returning(move |path| { + if path.ends_with("build.config") { + Ok("arch=x86\n".to_string()) + } else if path.ends_with("kernel.release") { + Ok("6.17.0\n".to_string()) + } else if path.ends_with("remote.config") { + Ok( + "#kw-default=dut\nHost dut\n Hostname box\n Port 22\n User root\n" + .to_string(), + ) + } else if path.ends_with("deploy.config") { + Ok(deploy_config.clone()) + } else { + Err(FileSystemError::IoError(std::io::Error::new( + std::io::ErrorKind::NotFound, + "missing", + ))) + } + }); + fs.expect_read_dir().returning(|path| { + if path.ends_with("arch/x86/boot") { + Ok(vec![PathBuf::from("/kernel/arch/x86/boot/bzImage")]) + } else { + Err(FileSystemError::IoError(std::io::Error::new( + std::io::ErrorKind::NotFound, + "missing", + ))) + } + }); + fs.expect_metadata().returning(|_| { + Err(FileSystemError::IoError(std::io::Error::other( + "no metadata", + ))) + }); + fs.expect_create_dir_all().returning(|_| Ok(())); + fs + } + fn head_branch_shell(branch: &str) -> MockShellTrait { let branch = branch.to_string(); let mut shell = MockShellTrait::new(); @@ -627,6 +1037,27 @@ mod unix { .expect("expected KwOps scene did not appear"); } + async fn wait_for_latest_popup( + scenes: &Arc>>, + predicate: impl Fn(Option<&crate::ui::scene::PopupScene>) -> bool, + ) { + tokio::time::timeout(Duration::from_secs(5), async { + loop { + let matches = scenes + .lock() + .unwrap() + .last() + .is_some_and(|scene| predicate(scene.popup.as_ref())); + if matches { + return; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("expected popup state did not appear"); + } + async fn wait_for_details(scenes: &Arc>>) { tokio::time::timeout(Duration::from_secs(5), async { loop { diff --git a/src/input/event.rs b/src/input/event.rs index af54bf6..cf18872 100644 --- a/src/input/event.rs +++ b/src/input/event.rs @@ -56,6 +56,8 @@ pub enum InputEvent { OpenHelp, OpenKwOps, StartKwBuild, + StartKwDeploy, + StartKwBuildThenDeploy, CancelKwJob, RestoreKwBranch, EditKwOpsField, diff --git a/src/input/mapper.rs b/src/input/mapper.rs index 908786a..0120eea 100644 --- a/src/input/mapper.rs +++ b/src/input/mapper.rs @@ -203,6 +203,8 @@ impl InputMapper { KeyCode::Char('k') | KeyCode::Up => Some(InputEvent::NavigateUp), KeyCode::Char('e') | KeyCode::Enter => Some(InputEvent::EditKwOpsField), KeyCode::Char('b') => Some(InputEvent::StartKwBuild), + KeyCode::Char('d') => Some(InputEvent::StartKwDeploy), + KeyCode::Char('D') => Some(InputEvent::StartKwBuildThenDeploy), KeyCode::Char('c') => Some(InputEvent::CancelKwJob), KeyCode::Char('r') => Some(InputEvent::RestoreKwBranch), _ => None, @@ -428,6 +430,14 @@ mod tests { mapper.map_terminal_event(key(KeyCode::Char('b')), &context), Some(InputEvent::StartKwBuild) ); + assert_eq!( + mapper.map_terminal_event(key(KeyCode::Char('d')), &context), + Some(InputEvent::StartKwDeploy) + ); + assert_eq!( + mapper.map_terminal_event(key(KeyCode::Char('D')), &context), + Some(InputEvent::StartKwBuildThenDeploy) + ); assert_eq!( mapper.map_terminal_event(key(KeyCode::Char('c')), &context), Some(InputEvent::CancelKwJob) From 1fd62680f942950f465641019812595ccac831f8 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 20 Sep 2026 10:32:58 -0300 Subject: [PATCH 11/12] fix(kw): stop cancelled builds from deploying and tighten deploy gates This commit keeps a cancelled build from chaining into deploy, and makes deploy extras safe for kw's getopt: shared flags are filtered, and reserved options are stripped in GNU long-option forms. Deploy-alone refuses when the latest successful build is on another branch, and start-path probes run off the actor task so they cannot stall replies. KwOps labels and gate order match what the actor accepts. This commit completes the kw integration's step 7. Signed-off-by: lorenzoberts --- src/app/actor.rs | 12 +- src/app/flows/kw_ops.rs | 33 ++- src/app/integration_tests/kw_ops.rs | 4 +- src/app/view_model.rs | 31 +- src/kw/actor.rs | 257 ++++++++++++----- src/kw/argv.rs | 424 +++++++++++++++++++++++++--- src/kw/readiness.rs | 83 +++++- 7 files changed, 714 insertions(+), 130 deletions(-) diff --git a/src/app/actor.rs b/src/app/actor.rs index 66e6f27..e3e0458 100644 --- a/src/app/actor.rs +++ b/src/app/actor.rs @@ -21,8 +21,9 @@ use crate::{ details_actions::handle_patchset_details, edit_config::handle_edit_config, kw_ops::{ - apply_kw_snapshot, clear_pending_deploy, fallback_kw_status, handle_kw_ops, - poll_kw_status, refresh_kw_ops_log_tail, resume_pending_deploy, + apply_kw_snapshot, apply_kw_snapshot_refreshing_readiness, clear_pending_deploy, + fallback_kw_status, handle_kw_ops, poll_kw_status, refresh_kw_ops_log_tail, + resume_pending_deploy, }, latest::handle_latest_patchsets, mail_list::handle_mailing_list_selection, @@ -144,7 +145,7 @@ impl AppActor { watch_event = kw_status_changed(&mut kw_status_rx) => { match watch_event { KwWatchEvent::Updated(snapshot) => { - apply_kw_snapshot(&mut self.app, snapshot); + apply_kw_snapshot_refreshing_readiness(&mut self.app, snapshot).await; if self.app.state.navigation.current_screen == CurrentScreen::KwOps { refresh_kw_ops_log_tail(&mut self.app).await; @@ -605,7 +606,10 @@ mod tests { panic!("resume without a kw actor should explain that deploy cannot start"); }; assert_eq!("Cannot start deploy", title); - assert!(body.contains("not attached")); + assert!( + body.contains("no build recorded"), + "resume with a stale deploy-alone snapshot should refuse before the missing-actor path, got {body:?}" + ); } #[tokio::test] diff --git a/src/app/flows/kw_ops.rs b/src/app/flows/kw_ops.rs index d1412b7..a597c7f 100644 --- a/src/app/flows/kw_ops.rs +++ b/src/app/flows/kw_ops.rs @@ -92,6 +92,27 @@ pub(crate) fn apply_kw_snapshot(app: &mut App, snapshot: KwStatusSnapshot) { app.state.kw.status = Some(snapshot); } +fn snapshot_job_is_terminal(job: &KwJobStatus) -> bool { + matches!( + job, + KwJobStatus::Succeeded { .. } | KwJobStatus::Failed { .. } | KwJobStatus::Cancelled { .. } + ) +} + +/// Re-probe deploy-alone after a job ends so the (d) label tracks the +/// in-session build record instead of staying on the pre-build snapshot. +pub(crate) async fn apply_kw_snapshot_refreshing_readiness( + app: &mut App, + snapshot: KwStatusSnapshot, +) { + let refresh = snapshot_job_is_terminal(&snapshot.job) + && app.state.navigation.current_screen == CurrentScreen::KwOps; + apply_kw_snapshot(app, snapshot); + if refresh { + refresh_kw_ops_readiness(app).await; + } +} + /// Keyboard-only fallback: pull status when the watch is unavailable. pub(crate) async fn poll_kw_status(app: &mut App) -> bool { let Some(kw) = app.services.kw.clone() else { @@ -101,7 +122,7 @@ pub(crate) async fn poll_kw_status(app: &mut App) -> bool { return false; }; let changed = app.state.kw.status.as_ref() != Some(&snapshot); - apply_kw_snapshot(app, snapshot); + apply_kw_snapshot_refreshing_readiness(app, snapshot).await; changed } @@ -117,7 +138,7 @@ pub(crate) async fn fallback_kw_status(app: &mut App) { return; }; match kw.get_status().await { - Ok(snapshot) => apply_kw_snapshot(app, snapshot), + Ok(snapshot) => apply_kw_snapshot_refreshing_readiness(app, snapshot).await, Err(_) => app.state.kw.status = None, } } @@ -303,6 +324,14 @@ async fn start_job(app: &mut App, kind: KwStartKind) -> Result<()> { app.state.popup = Some(AppPopup::info(kind.title(), body)); return Ok(()); } + // Record/tree refusals before the boot-once confirm so a deploy + // without a build does not ask the user to proceed, then refuse. + if matches!(kind, KwStartKind::Deploy) { + if let Err(reason) = &ops.readiness.deploy_alone { + app.state.popup = Some(AppPopup::info(kind.title(), reason.to_string())); + return Ok(()); + } + } if kind.pending_kind().is_some() && needs_boot_once_confirm(ops) { if let Some(ops) = app.state.kw.ops.as_mut() { ops.pending_deploy = kind.pending_kind(); diff --git a/src/app/integration_tests/kw_ops.rs b/src/app/integration_tests/kw_ops.rs index 865ce3d..0195f4e 100644 --- a/src/app/integration_tests/kw_ops.rs +++ b/src/app/integration_tests/kw_ops.rs @@ -384,11 +384,13 @@ mod unix { async fn start_deploy_opens_the_boot_once_gate() { let log_dir = kw_log_dir("deploy-gate"); let process = Arc::new(FakeProcess::new()); - let mut app = app_with_details_and_kw_process( + let mut app = app_with_kw( &log_dir, head_branch_shell("feature"), process.clone(), Arc::new(MockFileSystemTrait::new()), + deploy_kw_fs(false), + feature_build_history(Some(matching_feature_build_record())), ); handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) .await diff --git a/src/app/view_model.rs b/src/app/view_model.rs index 1c296db..eddf272 100644 --- a/src/app/view_model.rs +++ b/src/app/view_model.rs @@ -472,7 +472,8 @@ fn project_kw_ops(state: &AppState) -> KwOpsViewModel { ops.extra_args.clone() }; let branch_empty = ops.branch.trim().is_empty(); - let start_block = start_block_reason(running, start_requested, branch_empty); + let kw_available = ops.readiness.kw_binary.available; + let start_block = start_block_reason(running, start_requested, branch_empty, kw_available); let start_label = action_label(start_block.clone(), 'b'); let remote_block = match &ops.readiness.deploy_remote { Err(reason) => Some(format!("unavailable ({})", compact_remote_refusal(reason))), @@ -568,9 +569,16 @@ fn project_kw_ops(state: &AppState) -> KwOpsViewModel { } } -fn start_block_reason(running: bool, start_requested: bool, branch_empty: bool) -> Option { +fn start_block_reason( + running: bool, + start_requested: bool, + branch_empty: bool, + kw_available: bool, +) -> Option { if running || start_requested { Some("unavailable (a job is already running)".to_string()) + } else if !kw_available { + Some("unavailable (kw not on PATH)".to_string()) } else if branch_empty { Some("unavailable (set a branch first)".to_string()) } else { @@ -1048,7 +1056,7 @@ mod tests { ops.readiness.deploy_remote = Ok(sample_remote()); ops.readiness.deploy_alone = Ok(()); ops.readiness.boot_once = crate::kw::readiness::BootOnceState::Off; - ops.extra_args = "--verbose --local".to_string(); + ops.extra_args = "--verbose --local --ccache".to_string(); state.kw.ops = Some(ops); let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { @@ -1165,4 +1173,21 @@ mod tests { .kw_running ); } + + #[test] + fn missing_kw_binary_disables_start_labels() { + let mut state = app_state_with_kw(None); + state.navigation.current_screen = CurrentScreen::KwOps; + let mut ops = sample_kw_ops(Some("feature")); + ops.readiness.kw_binary.available = false; + ops.readiness.kw_binary.version_line = None; + state.kw.ops = Some(ops); + + let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { + panic!("expected KwOps projection"); + }; + assert_eq!("unavailable (kw not on PATH)", vm.start_label); + assert_eq!("unavailable (kw not on PATH)", vm.deploy_label); + assert_eq!("unavailable (kw not on PATH)", vm.build_deploy_label); + } } diff --git a/src/kw/actor.rs b/src/kw/actor.rs index b749f49..63703ae 100644 --- a/src/kw/actor.rs +++ b/src/kw/actor.rs @@ -174,8 +174,12 @@ impl KwActor { tracing::info!("kw actor started"); // The job-event sender is held by the actor itself, so that arm of // the select never closes while the actor is alive. + // Prefer control messages over job events so a Cancel that races a + // successful build exit is observed before BuildThenDeploy would + // chain into deploy. loop { tokio::select! { + biased; message = self.rx.recv() => { let Some(message) = message else { break }; if let ControlFlow::Break(()) = self.handle_message(message).await { @@ -292,9 +296,9 @@ impl KwActor { /// Refusals, in order: a job already running, no kw binary on PATH, /// unresolvable kw-env state, a tree that fails the readiness probes, /// a dirty worktree, or a failed branch switch. Deploy kinds then also - /// refuse on an unresolved remote or an unacknowledged boot-once - /// setting; StartDeploy additionally requires a matching build record - /// and image. Reserved flags on patch-hub's own argv win over the + /// refuse on an unresolved remote; StartDeploy additionally requires a + /// matching build record and image, then an acknowledged boot-once + /// setting. Reserved flags on patch-hub's own argv win over the /// request's extra args. A start refused after the branch switch rolls /// the switch back: a refused start never leaves the tree on a branch /// the user did not check out. @@ -307,13 +311,29 @@ impl KwActor { return Err(KwStartError::JobAlreadyRunning); } + let tree_path = PathBuf::from(request.tree.path()); // Hard fail on invoke: with no kw binary on PATH no job can run. // The version check is advisory only — kw's shipped VERSION file // is stale, so Below/Unknown are logged, never gated. - let kw_binary = readiness::probe_kw_binary(&*self.env, &*self.shell); - if !kw_binary.available { - return Err(KwStartError::KwBinaryMissing); - } + // Probes run on the blocking pool so a slow tree or cold + // `kw --version` cannot stall the Tokio worker (or delay Cancel + // past this Start's await). + let fs = Arc::clone(&self.fs); + let env = Arc::clone(&self.env); + let shell = Arc::clone(&self.shell); + let tree_path_for_probe = tree_path.clone(); + let (kw_binary, output_dir, tree_readiness) = tokio::task::spawn_blocking(move || { + let kw_binary = readiness::probe_kw_binary(&*env, &*shell); + if !kw_binary.available { + return Err(KwStartError::KwBinaryMissing); + } + let output_dir = readiness::resolve_output_dir(&*fs, &*env, &tree_path_for_probe)?; + let tree_readiness = + readiness::probe_tree(&*fs, &tree_path_for_probe, output_dir.as_deref()); + Ok((kw_binary, output_dir, tree_readiness)) + }) + .await + .map_err(|error| KwStartError::GitStateProbe(error.to_string()))??; if let KwVersionCheck::Below(version_line) = &kw_binary.check { tracing::warn!( version = %version_line, @@ -322,14 +342,11 @@ impl KwActor { ); } - let tree_path = PathBuf::from(request.tree.path()); // Unresolvable env state refuses the start: the build record this // job writes at completion must know whether it ran under an O=. // Both values are then snapshotted onto the job — the build runs // under them, so the completion record describes them, not the // tree's configuration at whatever time the job ends. - let output_dir = readiness::resolve_output_dir(&*self.fs, &*self.env, &tree_path)?; - let tree_readiness = readiness::probe_tree(&*self.fs, &tree_path, output_dir.as_deref()); let TreeReadiness::Ready { arch } = tree_readiness else { return Err(KwStartError::TreeNotReady(tree_readiness)); }; @@ -337,13 +354,16 @@ impl KwActor { let pre_job_branch = self.checkout_build_branch(&request).await?; let deploy = if matches!(kind, KwJobKind::Deploy | KwJobKind::BuildThenDeploy) { - match self.prepare_deploy( - kind, - &request, - &tree_path, - output_dir.as_deref(), - arch.as_deref(), - ) { + match self + .prepare_deploy( + kind, + &request, + &tree_path, + output_dir.as_deref(), + arch.as_deref(), + ) + .await + { Ok(deploy) => Some(deploy), Err(error) => { self.rollback_switch(request.tree.path(), pre_job_branch.as_deref()) @@ -425,12 +445,14 @@ impl KwActor { Ok(()) } - /// Deploy-kind gates after the branch switch: resolved remote and - /// boot-once confirm for every deploy kind; the deploy-alone record - /// match only for StartDeploy, against the requested (now checked-out) - /// branch. Any refusal is rolled back by the caller. BuildThenDeploy - /// skips the record gate because the build has not run yet. - fn prepare_deploy( + /// Deploy-kind gates after the branch switch: resolved remote for + /// every deploy kind; the deploy-alone record match only for + /// StartDeploy, against the requested (now checked-out) branch; + /// boot-once confirm last so a missing record refuses before the + /// confirm popup. Any refusal is rolled back by the caller. + /// BuildThenDeploy skips the record gate because the build has not + /// run yet. Probes run on the blocking pool. + async fn prepare_deploy( &self, kind: KwJobKind, request: &StartRequest, @@ -438,42 +460,27 @@ impl KwActor { output_dir: Option<&Path>, arch: Option<&str>, ) -> Result { - let options = request.deploy.clone().unwrap_or(DeployOptions { - reboot: false, - force: true, - boot_once_acknowledged: false, - }); - let remote = remote::resolve_deploy_remote(&*self.fs, &*self.env, tree_path) - .map_err(KwStartError::RemoteUnresolved)?; - let boot_once = readiness::probe_boot_once(&*self.fs, &*self.env, tree_path); - if matches!(boot_once, BootOnceState::On | BootOnceState::Unknown) - && !options.boot_once_acknowledged - { - return Err(KwStartError::BootOnceNotAcknowledged); - } - if kind == KwJobKind::Deploy { - let (record, _) = self - .history - .build_records(&request.kernel_tree_id, &request.branch)?; - let image = readiness::find_newest_kernel_image( - &*self.fs, - output_dir.unwrap_or(tree_path), - arch, - ); - readiness::check_deploy_alone( - record.as_ref(), - &request.tree, - &request.branch, - output_dir, - image.as_deref(), + let fs = Arc::clone(&self.fs); + let env = Arc::clone(&self.env); + let history = Arc::clone(&self.history); + let request = request.clone(); + let tree_path = tree_path.to_path_buf(); + let output_dir = output_dir.map(Path::to_path_buf); + let arch = arch.map(str::to_string); + tokio::task::spawn_blocking(move || { + prepare_deploy_blocking( + kind, + &request, + &*fs, + &*env, + &*history, + &tree_path, + output_dir.as_deref(), + arch.as_deref(), ) - .map_err(KwStartError::DeployAloneRefused)?; - } - Ok(JobDeploy { - remote, - options, - extra_args: request.extra_args.clone(), }) + .await + .map_err(|error| KwStartError::GitStateProbe(error.to_string()))? } /// Refuse a dirty worktree, record HEAD, then `git switch` to the @@ -606,6 +613,7 @@ impl KwActor { tracing::warn!("kw job finished with no job state recorded"); return; }; + let outcome = outcome_after_building_cancel(&job, outcome); let job = match self.advance_build_then_deploy(job, &outcome) { ControlFlow::Break(()) => return, ControlFlow::Continue(job) => job, @@ -680,6 +688,10 @@ impl KwActor { if job.kind != KwJobKind::BuildThenDeploy || job.phase != KwPhase::Building || !exit.success() + // A cancel during Building must not deploy, even if the build + // process exited 0 (wait racing the cancel arm, or a trapped + // SIGTERM). `cancel_tx` is taken when Cancel is requested. + || job.cancel_tx.is_none() { return ControlFlow::Continue(job); } @@ -905,6 +917,52 @@ impl KwActor { } } +/// Deploy gates that must not run on the actor task: remote.config, +/// history JSON, image globs, and boot-once files. Record match runs +/// before the boot-once confirm so a missing build refuses cheaper. +fn prepare_deploy_blocking( + kind: KwJobKind, + request: &StartRequest, + fs: &dyn FileSystemTrait, + env: &dyn EnvTrait, + history: &dyn KwHistoryStore, + tree_path: &Path, + output_dir: Option<&Path>, + arch: Option<&str>, +) -> Result { + let options = request.deploy.clone().unwrap_or(DeployOptions { + reboot: false, + force: true, + boot_once_acknowledged: false, + }); + let remote = remote::resolve_deploy_remote(fs, env, tree_path) + .map_err(KwStartError::RemoteUnresolved)?; + if kind == KwJobKind::Deploy { + let (record, latest) = history.build_records(&request.kernel_tree_id, &request.branch)?; + let image = readiness::find_newest_kernel_image(fs, output_dir.unwrap_or(tree_path), arch); + readiness::check_deploy_alone( + record.as_ref(), + latest.as_ref(), + &request.tree, + &request.branch, + output_dir, + image.as_deref(), + ) + .map_err(KwStartError::DeployAloneRefused)?; + } + let boot_once = readiness::probe_boot_once(fs, env, tree_path); + if matches!(boot_once, BootOnceState::On | BootOnceState::Unknown) + && !options.boot_once_acknowledged + { + return Err(KwStartError::BootOnceNotAcknowledged); + } + Ok(JobDeploy { + remote, + options, + extra_args: request.extra_args.clone(), + }) +} + /// Fails unless the tree's git state verifies clean. Untracked files /// don't count: kernel trees accumulate local scratch files, and only /// tracked changes can corrupt the branch a job builds. A probe that @@ -1026,6 +1084,22 @@ async fn cancel_job(process: &mut dyn RunningProcess) -> JobOutcome { } } +/// A cancel during Building of a BuildThenDeploy job must not chain into +/// deploy, even when the build process reports exit 0. Rewriting the +/// outcome to [`JobOutcome::Cancelled`] also skips the build record, matching +/// the "cancel in Building writes nothing" rule. +fn outcome_after_building_cancel(job: &JobState, outcome: JobOutcome) -> JobOutcome { + if job.cancel_tx.is_none() + && job.kind == KwJobKind::BuildThenDeploy + && job.phase == KwPhase::Building + && matches!(&outcome, JobOutcome::Exited(exit) if exit.success()) + { + JobOutcome::Cancelled + } else { + outcome + } +} + /// Maps a reap result observed after a cancel request. A signal-terminated /// process means our SIGTERM/SIGKILL landed — the job was really cancelled. /// A plain exit means the process finished on its own before the signal: @@ -1033,11 +1107,13 @@ async fn cancel_job(process: &mut dyn RunningProcess) -> JobOutcome { /// build history (and deploy-alone readiness) needs to see. /// /// Known, accepted edges: a process that *traps* our SIGTERM and exits 0 -/// counts as success (kw is bash, so this is possible in principle), and an -/// external signal racing a cancel (e.g. the OOM killer) reads as -/// Cancelled. Both are indistinguishable from the honest cases without -/// comparing who signaled first, and both favor showing the user real -/// output over inventing failures. +/// counts as success for a standalone build (kw is bash, so this is +/// possible in principle). BuildThenDeploy does not chain that exit into +/// deploy — see [`outcome_after_building_cancel`]. An external signal +/// racing a cancel (e.g. the OOM killer) reads as Cancelled. Those cases +/// are indistinguishable from the honest ones without comparing who +/// signaled first, and both favor showing the user real output over +/// inventing failures. fn outcome_after_cancel(result: Result) -> JobOutcome { use std::os::unix::process::ExitStatusExt; @@ -2453,7 +2529,7 @@ mod tests { async fn start_deploy_refused_when_boot_once_is_on_and_unacked() { let (handle, process, log_dir) = spawn_deploy_actor( "deploy-boot-once", - MockKwHistoryStore::new(), + deploy_history(Some(matching_build_record())), deploy_fs(DEPLOY_REMOTE_CONFIG, DEPLOY_BOOT_ONCE_ON, true), ); @@ -2466,6 +2542,30 @@ mod tests { std::fs::remove_dir_all(&log_dir).unwrap(); } + #[tokio::test] + async fn start_deploy_refused_when_latest_build_is_on_another_branch() { + let mut latest = matching_build_record(); + latest.branch = "other".to_string(); + let mut history = MockKwHistoryStore::new(); + history + .expect_build_records() + .returning(move |_, _| Ok((None, Some(latest.clone())))); + history.expect_record_build().times(0); + let (handle, process, log_dir) = + spawn_deploy_actor("deploy-head-mismatch", history, deploy_ready_fs()); + + let err = handle.start_deploy(deploy_request()).await.unwrap_err(); + + assert!(matches!( + err, + KwStartError::DeployAloneRefused(DeployAloneRefusal::HeadMismatch { .. }) + )); + assert!(process.spawned().is_empty()); + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + #[tokio::test] async fn start_deploy_proceeds_when_boot_once_is_on_and_acked() { let (handle, process, log_dir) = spawn_deploy_actor( @@ -2581,7 +2681,6 @@ mod tests { "--no-reboot", "--force", "--verbose", - "--ccache" ] .as_slice(), spawned[1].args.as_slice() @@ -2678,6 +2777,36 @@ mod tests { std::fs::remove_dir_all(&log_dir).unwrap(); } + #[tokio::test] + async fn start_build_then_deploy_cancel_then_exit_zero_skips_deploy() { + let (history, builds) = recording_history(None); + let (handle, process, log_dir) = + spawn_deploy_actor("chain-cancel-exit-zero", history, deploy_ready_fs()); + let mut watch = handle.watch_status().await.unwrap(); + + process.ignore_sigterm(true); + handle + .start_build_then_deploy(deploy_request()) + .await + .unwrap(); + handle.cancel().await.unwrap(); + process.last_child().finish(0); + + let status = wait_for_terminal_status(&mut watch).await; + match status { + KwJobStatus::Cancelled { kind, phase, .. } => { + assert_eq!(KwJobKind::BuildThenDeploy, kind); + assert_eq!(KwPhase::Building, phase); + } + other => panic!("expected Cancelled Building, got {other:?}"), + } + assert_eq!(1, process.spawned().len()); + assert!(builds.lock().unwrap().is_empty()); + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + #[tokio::test] async fn start_build_then_deploy_cancel_in_deploying_keeps_the_build_record() { let (history, builds) = recording_history(None); diff --git a/src/kw/argv.rs b/src/kw/argv.rs index 4d87f49..5c2b8af 100644 --- a/src/kw/argv.rs +++ b/src/kw/argv.rs @@ -12,6 +12,12 @@ pub struct ReservedOption { /// Whether the option takes a value (`--name=value`, or `--name /// value` as a separate token). takes_value: bool, + /// When true, GNU getopt unique prefixes of this option's long + /// spellings are stripped too (`--boot` → `--boot-into-new-kernel-once`). + /// Only for flags the target kw command actually honors, so a + /// build-only `--menu` on the deploy table cannot steal `--m` from + /// `--modules`. + match_abbrev: bool, } impl ReservedOption { @@ -19,6 +25,17 @@ impl ReservedOption { Self { spellings, takes_value, + match_abbrev: false, + } + } + + /// Like [`Self::new`], and also strip unambiguous GNU getopt long + /// abbreviations of this option. + pub const fn gnu_abbrev(spellings: &'static [&'static str], takes_value: bool) -> Self { + Self { + spellings, + takes_value, + match_abbrev: true, } } } @@ -38,14 +55,15 @@ impl ReservedOption { /// `--info` hang a redirected job, and `--from-sha` mutates git. They are /// stripped from extras and never injected. const BUILD_RESERVED: &[ReservedOption] = &[ - ReservedOption::new(&["--alert"], true), - ReservedOption::new(&["--save-log-to"], true), - ReservedOption::new(&["--menu"], false), - ReservedOption::new(&["--clean"], false), - ReservedOption::new(&["--full-cleanup"], false), - ReservedOption::new(&["--doc"], false), - ReservedOption::new(&["--info"], false), - ReservedOption::new(&["--from-sha"], true), + ReservedOption::gnu_abbrev(&["--help", "-h"], false), + ReservedOption::gnu_abbrev(&["--alert"], true), + ReservedOption::gnu_abbrev(&["--save-log-to"], true), + ReservedOption::gnu_abbrev(&["--menu"], false), + ReservedOption::gnu_abbrev(&["--clean"], false), + ReservedOption::gnu_abbrev(&["--full-cleanup"], false), + ReservedOption::gnu_abbrev(&["--doc"], false), + ReservedOption::gnu_abbrev(&["--info"], false), + ReservedOption::gnu_abbrev(&["--from-sha"], true), ]; /// Reserved for `kw deploy`. Patch-hub always injects a resolved @@ -61,22 +79,61 @@ const BUILD_RESERVED: &[ReservedOption] = &[ /// does not eat the following token. `--alert` is not a deploy option /// at all — injecting or forwarding it hard-fails on kw beta-0.9 /// (`Invalid option`), the version this lab still reports. +/// +/// Build-only flags are stripped too: KwOps has one extras field, and +/// kw deploy's getopt (`src/deploy.sh` at 0.10: `remote:,local,reboot, +/// no-reboot,modules,list,ls-line,uninstall::,list-all,force,setup, +/// verbose,create-package,from-package:,boot-into-new-kernel-once`) +/// rejects unrecognized options with exit 22. Without this, `D` with +/// `--ccache` would build successfully then die at the deploy boundary. +/// Short spellings that collide with deploy's own flags (`-n` menu vs +/// boot-once, `-f` full-cleanup vs force, `-s` save-log-to vs ls-line) +/// stay on the deploy meaning already reserved above. +/// +/// kw_parse is GNU `getopt -q`, so exact-token stripping is not enough: +/// `-rf` bundles `--reboot --force`, `-Fpkg.kw.tar` attaches a +/// from-package value, and `--boot` uniquely abbreviates +/// `--boot-into-new-kernel-once`. Those forms are stripped too. +/// `gnu_abbrev` is only set on flags deploy's getopt actually honors, +/// so `--m` still reaches `--modules` instead of matching build-only +/// `--menu`. Ambiguous prefixes (`--re` → remote and reboot) pass +/// through and fail getopt rather than eating the next token. +/// Abbreviation uniqueness is among this reserved table, not kw's full +/// option set. For deploy those coincide (the only passthrough longs are +/// `--modules`/`--verbose`). For build, `--c` is ambiguous in kw +/// (`ccache`/`cpu-scaling`/`clean`/`cflags`) but uniquely matches +/// `--clean` here, so it is stripped instead of failing getopt. const DEPLOY_RESERVED: &[ReservedOption] = &[ - ReservedOption::new(&["--remote"], true), - ReservedOption::new(&["--local"], false), - ReservedOption::new(&["--reboot", "-r"], false), - ReservedOption::new(&["--no-reboot"], false), - ReservedOption::new(&["--force", "-f"], false), - ReservedOption::new(&["--list", "-l"], false), - ReservedOption::new(&["--ls-line", "-s"], false), - ReservedOption::new(&["--list-all", "-a"], false), - ReservedOption::new(&["--setup"], false), - ReservedOption::new(&["--uninstall", "-u"], false), - ReservedOption::new(&["--from-package", "-F"], true), - ReservedOption::new(&["--create-package", "-p"], false), - ReservedOption::new(&["--boot-into-new-kernel-once", "-n"], false), + ReservedOption::gnu_abbrev(&["--remote"], true), + ReservedOption::gnu_abbrev(&["--local"], false), + ReservedOption::gnu_abbrev(&["--reboot", "-r"], false), + ReservedOption::gnu_abbrev(&["--no-reboot"], false), + ReservedOption::gnu_abbrev(&["--force", "-f"], false), + ReservedOption::gnu_abbrev(&["--list", "-l"], false), + ReservedOption::gnu_abbrev(&["--ls-line", "-s"], false), + ReservedOption::gnu_abbrev(&["--list-all", "-a"], false), + ReservedOption::gnu_abbrev(&["--setup"], false), + ReservedOption::gnu_abbrev(&["--uninstall", "-u"], false), + ReservedOption::gnu_abbrev(&["--from-package", "-F"], true), + ReservedOption::gnu_abbrev(&["--create-package", "-p"], false), + ReservedOption::gnu_abbrev(&["--boot-into-new-kernel-once", "-n"], false), + ReservedOption::new(&["--help", "-h"], false), ReservedOption::new(&["--save-log-to"], true), ReservedOption::new(&["--alert"], true), + ReservedOption::new(&["--ccache"], false), + ReservedOption::new(&["--llvm"], false), + ReservedOption::new(&["--cpu-scaling", "-S"], true), + // kw declares `warnings::` (optional value): GNU getopt only consumes + // an attached `--warnings=1` / `-w1`. Treating this as required would + // eat the following token (`--warnings --verbose`). + ReservedOption::new(&["--warnings", "-w"], false), + ReservedOption::new(&["--cflags"], true), + ReservedOption::new(&["--menu"], false), + ReservedOption::new(&["--doc", "-d"], false), + ReservedOption::new(&["--info", "-i"], false), + ReservedOption::new(&["--clean", "-c"], false), + ReservedOption::new(&["--full-cleanup"], false), + ReservedOption::new(&["--from-sha"], true), ]; /// The argv for a build job: `kw build ` (reserved extras stripped). @@ -86,9 +143,10 @@ pub fn build_argv(extra_args: &[String]) -> Vec { /// The argv for a deploy job: `kw deploy --remote /// --no-reboot|--reboot [--force] `. Reserved extras are -/// stripped so the injected remote, reboot, and force flags win. -/// `--force` is omitted entirely when `force` is false rather than -/// passing a no-op, because kw has no `--no-force`. +/// stripped so the injected remote, reboot, and force flags win, and +/// so build-only extras (shared KwOps field) cannot fail kw deploy's +/// getopt. `--force` is omitted entirely when `force` is false rather +/// than passing a no-op, because kw has no `--no-force`. #[cfg_attr(not(unix), allow(dead_code))] pub fn deploy_argv( endpoint: &str, @@ -109,10 +167,12 @@ pub fn deploy_argv( } /// Appends user-supplied extra args to `base`, stripping every token that -/// would override a reserved option: `--name=value` is stripped whole, -/// `--name value` consumes the following token too, and a boolean -/// reserved option strips only itself. All other extras pass through in -/// order. +/// would override a reserved option. GNU getopt forms are stripped too: +/// `--name=value`, `--name value`, unique long abbreviations, bundled +/// shorts (`-rf`), and attached short values (`-Fpkg.kw.tar`). A mixed +/// short cluster that contains any reserved flag is dropped whole, so +/// `-uVALUE` cannot be rewritten into a leftover `-VALUE`. All other +/// extras pass through in order. pub fn merge_extra_args( base: &[&str], reserved: &[ReservedOption], @@ -123,8 +183,8 @@ pub fn merge_extra_args( while let Some(token) = extras.next() { match reserved_option_for(reserved, token) { Some(option) => { - if option.takes_value && !token.contains('=') { - // `--name value`: the separate value token goes too. + if option.takes_value && !value_is_attached(token, option) { + // `--name value` / `-F value`: the separate value token goes too. extras.next(); } } @@ -134,22 +194,131 @@ pub fn merge_extra_args( argv } -/// Finds the reserved option a token sets, if any: an exact spelling -/// match, or `--name=value` for a long spelling. The `=` boundary keeps -/// `--alertness` from matching `--alert`. +/// Finds the reserved option a token sets, if any: an exact spelling, +/// `--name=value` for a long spelling, a unique GNU getopt abbreviation +/// of a `match_abbrev` long, a bundled reserved short, or an attached +/// short value. The `=` boundary keeps `--alertness` from matching +/// `--alert`; an abbreviation must be a prefix of the reserved spelling, +/// not the other way around. fn reserved_option_for<'a>( reserved: &'a [ReservedOption], token: &str, ) -> Option<&'a ReservedOption> { - reserved.iter().find(|option| { - option.spellings.iter().any(|spelling| { - token == *spelling - || spelling.starts_with("--") - && token - .strip_prefix(spelling) - .is_some_and(|rest| rest.starts_with('=')) + if token.starts_with("--") { + return reserved_long_option(reserved, token); + } + reserved_short_cluster(reserved, token) +} + +fn reserved_long_option<'a>( + reserved: &'a [ReservedOption], + token: &str, +) -> Option<&'a ReservedOption> { + let name = token.split_once('=').map_or(token, |(name, _)| name); + reserved + .iter() + .find(|option| { + option + .spellings + .iter() + .any(|spelling| spelling.starts_with("--") && name == *spelling) }) - }) + .or_else(|| unique_long_abbrev(reserved, name)) +} + +/// GNU getopt unique-prefix match among options that opted into +/// `match_abbrev`. Uniqueness is against this reserved table, not kw's +/// full option list — a prefix kw would reject as ambiguous can still +/// strip here when only one reserved long matches. Ambiguous prefixes +/// among reserved longs are left alone so a following value token is +/// not eaten. +fn unique_long_abbrev<'a>( + reserved: &'a [ReservedOption], + name: &str, +) -> Option<&'a ReservedOption> { + if name.len() < 3 || !name.starts_with("--") { + return None; + } + let mut found: Option<&ReservedOption> = None; + for option in reserved { + if !option.match_abbrev { + continue; + } + let matches = option + .spellings + .iter() + .any(|spelling| spelling.starts_with("--") && spelling.starts_with(name)); + if !matches { + continue; + } + match found { + None => found = Some(option), + Some(previous) if std::ptr::eq(previous, option) => {} + Some(_) => return None, + } + } + found +} + +/// A short token that contains any reserved flag: `-f`, `-rf`, `-Fpkg`. +/// Walking leftover letters would turn `-ukernel` into `-kernel`, so a +/// hit drops the whole token. +fn reserved_short_cluster<'a>( + reserved: &'a [ReservedOption], + token: &str, +) -> Option<&'a ReservedOption> { + let body = token.strip_prefix('-')?; + if body.is_empty() || body.starts_with('-') { + return None; + } + let chars: Vec = body.chars().collect(); + let mut i = 0; + let mut hit = None; + while i < chars.len() { + let spelling = format!("-{}", chars[i]); + match exact_short(reserved, &spelling) { + Some(option) => { + hit = Some(option); + if option.takes_value { + // Remainder is the attached value; stop so + // `value_is_attached` can see those extra chars. + break; + } + i += 1; + } + None => i += 1, + } + } + hit +} + +fn exact_short<'a>(reserved: &'a [ReservedOption], spelling: &str) -> Option<&'a ReservedOption> { + reserved + .iter() + .find(|option| option.spellings.iter().any(|s| *s == spelling)) +} + +fn value_is_attached(token: &str, option: &ReservedOption) -> bool { + if token.contains('=') { + return true; + } + if !option.takes_value { + return false; + } + let Some(body) = token + .strip_prefix('-') + .filter(|body| !body.starts_with('-')) + else { + return false; + }; + let mut chars = body.chars(); + while let Some(ch) = chars.next() { + let spelling = format!("-{ch}"); + if option.spellings.iter().any(|s| *s == spelling) { + return chars.next().is_some(); + } + } + false } #[cfg(test)] @@ -400,4 +569,177 @@ mod tests { deploy(&["--uninstall=old-kernel"]) ); } + + #[test] + fn deploy_strips_build_only_flags_that_kw_deploy_would_reject() { + // Shared KwOps extras: --verbose is a real deploy option; the rest + // are kw build-only (`src/build.sh` 0.10) and would exit 22. + assert_eq!( + vec![ + "deploy", + "--remote", + ENDPOINT, + "--no-reboot", + "--force", + "--verbose", + ], + deploy(&[ + "--ccache", + "--llvm", + "--cpu-scaling", + "50", + "--warnings=1", + "--cflags", + "-O2", + "--menu", + "--doc", + "--info", + "--clean", + "--full-cleanup", + "--from-sha", + "abc123", + "--verbose", + ]) + ); + assert_eq!( + vec![ + "deploy", + "--remote", + ENDPOINT, + "--no-reboot", + "--force", + "--verbose", + ], + deploy(&[ + "--cpu-scaling=50", + "--warnings=1", + "--cflags=-O2", + "--from-sha=abc123", + "-S", + "25", + "-w2", + "-d", + "-i", + "-c", + "--verbose", + ]) + ); + } + + #[test] + fn deploy_strips_bundled_reserved_shorts_and_attached_values() { + // GNU getopt: `-rf` is `--reboot --force`; `-Fpkg` is from-package. + assert_eq!( + vec![ + "deploy", + "--remote", + ENDPOINT, + "--no-reboot", + "--force", + "--verbose", + ], + deploy(&["-rf", "-Fpkg.kw.tar", "-S25", "-w12", "--verbose"]) + ); + // Mixed cluster containing a reserved flag is dropped whole so + // `-ukernel` cannot be rewritten into leftover `-kernel`. + assert_eq!( + vec![ + "deploy", + "--remote", + ENDPOINT, + "--no-reboot", + "--force", + "--verbose", + ], + deploy(&["-mr", "-ukernel", "--verbose"]) + ); + } + + #[test] + fn deploy_strips_unique_gnu_getopt_long_abbreviations() { + assert_eq!( + vec![ + "deploy", + "--remote", + ENDPOINT, + "--no-reboot", + "--force", + "--verbose", + ], + deploy(&[ + "--rem", + "evil:1", + "--rem=other:1", + "--reb", + "--no-r", + "--boot", + "--verbose", + ]) + ); + } + + #[test] + fn deploy_keeps_modules_abbrev_and_ambiguous_long_prefixes() { + // `--m` uniquely matches `--modules` on deploy; `--menu` is + // build-only and must not steal it. `--re` is ambiguous between + // `--remote` and `--reboot`, so it is left for getopt to reject + // rather than eating the next token. + assert_eq!( + vec![ + "deploy", + "--remote", + ENDPOINT, + "--no-reboot", + "--force", + "--m", + "--re", + "--verbose", + ], + deploy(&["--m", "--re", "--verbose"]) + ); + } + + #[test] + fn build_strips_unique_long_abbreviations_of_reserved_flags() { + assert_eq!( + vec!["build", "--verbose"], + build_argv(&extras(&["--men", "--from", "abc123", "--verbose"])) + ); + } + + #[test] + fn help_is_stripped_from_build_and_deploy_extras() { + // `kw build --help` exits 0 without compiling, so a D job would + // otherwise chain into `kw deploy --help` (exit 22). + assert_eq!( + vec!["build", "--verbose"], + build_argv(&extras(&["--help", "-h", "--verbose"])) + ); + assert_eq!( + vec![ + "deploy", + "--remote", + ENDPOINT, + "--no-reboot", + "--force", + "--verbose", + ], + deploy(&["--help", "-h", "--verbose"]) + ); + } + + #[test] + fn warnings_optional_value_does_not_eat_the_next_token() { + assert_eq!( + vec![ + "deploy", + "--remote", + ENDPOINT, + "--no-reboot", + "--force", + "--verbose", + ], + deploy(&["--warnings", "--verbose"]) + ); + } } diff --git a/src/kw/readiness.rs b/src/kw/readiness.rs index 09fd725..6a01c29 100644 --- a/src/kw/readiness.rs +++ b/src/kw/readiness.rs @@ -164,11 +164,12 @@ pub fn read_build_arch(fs: &dyn FileSystemTrait, tree_path: &Path) -> Option/.kw/env.current` (trailing newlines stripped, like bash's /// `$(< ...)`), and the output dir is /// `{XDG_CACHE_HOME | ~/.cache}/kw/envs//`. -/// The tree path is encoded exactly like kw's `get_encoded_pwd` — standard +/// The tree path is encoded like kw 0.10's `get_encoded_pwd` — standard /// base64 with padding, no wrapping — after trimming trailing slashes, /// since kw encodes `$PWD` after changing into the tree. The encoded path /// may contain `/` (standard alphabet), producing nested directories; kw -/// has the same behavior. +/// has the same behavior. kw master later strips padding (`tr --delete +/// '='`); that is an upstream-watch item, not this floor. /// /// kw's launcher recomputes the cache dir unconditionally, so a /// user-exported `KW_CACHE_DIR` is intentionally ignored here too. The @@ -420,8 +421,8 @@ pub enum DeployAloneRefusal { #[error("the last build of this branch failed; rebuild before deploying")] LastBuildFailed, #[error( - "the last build was on branch '{recorded}', but HEAD is '{current}'; \ - rebuild on the current branch before deploying" + "the last build was on branch '{recorded}', but the deploy target is '{current}'; \ + rebuild on the target branch before deploying" )] HeadMismatch { recorded: String, current: String }, #[error( @@ -513,18 +514,37 @@ fn xdg_kw_config_file(env: &dyn EnvTrait, filename: &str) -> Option { /// lookup branch, written against the same tree path and kw env, and a /// kernel image is still discoverable. /// +/// `record` is the lookup keyed by the deploy target branch. `latest` is +/// the newest record for the tree across branches. When the target has +/// no keyed record but another branch does, that is +/// [`DeployAloneRefusal::HeadMismatch`] (M18), not "no build recorded". +/// /// This is only the record-matching half of the gate — it says nothing /// about the tree's *current* state. [`evaluate_readiness`] conjoins /// [`TreeReadiness`] into its `deploy_alone` verdict; prefer it over /// calling this directly. pub fn check_deploy_alone( record: Option<&KwBuildRecord>, + latest: Option<&KwBuildRecord>, tree: &KernelTree, head_branch: &str, output_dir: Option<&Path>, image: Option<&Path>, ) -> Result<(), DeployAloneRefusal> { - let record = record.ok_or(DeployAloneRefusal::NoBuildRecord)?; + let record = match record { + Some(record) => record, + None => { + if let Some(latest) = latest { + if latest.branch != head_branch { + return Err(DeployAloneRefusal::HeadMismatch { + recorded: latest.branch.clone(), + current: head_branch.to_string(), + }); + } + } + return Err(DeployAloneRefusal::NoBuildRecord); + } + }; if !record.success { return Err(DeployAloneRefusal::LastBuildFailed); } @@ -625,6 +645,7 @@ pub fn evaluate_readiness( let deploy_alone = match &tree_status { TreeReadiness::Ready { .. } => check_deploy_alone( build_record.as_ref(), + latest_build.as_ref(), tree, lookup_branch, output_dir.as_deref(), @@ -1315,14 +1336,14 @@ last_line_without_newline=yes"; assert_eq!( Err(DeployAloneRefusal::NoBuildRecord), - check_deploy_alone(None, &tree, "patchset-x", None, Some(&image)) + check_deploy_alone(None, None, &tree, "patchset-x", None, Some(&image)) ); let mut failed = built_record(dir.path(), "patchset-x"); failed.success = false; assert_eq!( Err(DeployAloneRefusal::LastBuildFailed), - check_deploy_alone(Some(&failed), &tree, "patchset-x", None, Some(&image)) + check_deploy_alone(Some(&failed), None, &tree, "patchset-x", None, Some(&image)) ); } @@ -1333,13 +1354,24 @@ last_line_without_newline=yes"; let image = dir.path().join("arch/x86/boot/bzImage"); let record = built_record(dir.path(), "patchset-x"); - // HEAD moved to another branch since the build. + // Keyed-record sanity: the store returned a row whose branch + // field does not match the lookup key. assert_eq!( Err(DeployAloneRefusal::HeadMismatch { recorded: "patchset-x".to_string(), current: "master".to_string(), }), - check_deploy_alone(Some(&record), &tree, "master", None, Some(&image)) + check_deploy_alone(Some(&record), None, &tree, "master", None, Some(&image)) + ); + + // Production M18: no keyed row for the target, but the tree has + // a latest build on another branch. + assert_eq!( + Err(DeployAloneRefusal::HeadMismatch { + recorded: "patchset-x".to_string(), + current: "master".to_string(), + }), + check_deploy_alone(None, Some(&record), &tree, "master", None, Some(&image)) ); // The config repointed the same tree id at another path. @@ -1349,7 +1381,14 @@ last_line_without_newline=yes"; recorded: dir.path().to_str().unwrap().to_string(), current: "/elsewhere/linux".to_string(), }), - check_deploy_alone(Some(&record), &moved_tree, "patchset-x", None, Some(&image)) + check_deploy_alone( + Some(&record), + None, + &moved_tree, + "patchset-x", + None, + Some(&image) + ) ); // The active kw env changed since the build. @@ -1357,6 +1396,7 @@ last_line_without_newline=yes"; Err(DeployAloneRefusal::OutputDirMismatch), check_deploy_alone( Some(&record), + None, &tree, "patchset-x", Some(Path::new("/cache/kw/envs/xyz/minix")), @@ -1367,12 +1407,12 @@ last_line_without_newline=yes"; // The image the build produced is gone. assert_eq!( Err(DeployAloneRefusal::ImageMissing), - check_deploy_alone(Some(&record), &tree, "patchset-x", None, None) + check_deploy_alone(Some(&record), None, &tree, "patchset-x", None, None) ); assert_eq!( Ok(()), - check_deploy_alone(Some(&record), &tree, "patchset-x", None, Some(&image)) + check_deploy_alone(Some(&record), None, &tree, "patchset-x", None, Some(&image)) ); // A trailing-slash-only difference is the same tree, not drift. @@ -1380,7 +1420,14 @@ last_line_without_newline=yes"; slashed.tree_path = format!("{}/", dir.path().to_str().unwrap()); assert_eq!( Ok(()), - check_deploy_alone(Some(&slashed), &tree, "patchset-x", None, Some(&image)) + check_deploy_alone( + Some(&slashed), + None, + &tree, + "patchset-x", + None, + Some(&image) + ) ); } @@ -1393,7 +1440,7 @@ last_line_without_newline=yes"; assert_eq!( Err(DeployAloneRefusal::LastBuildFailed), - check_deploy_alone(Some(&failed), &tree, "master", None, None) + check_deploy_alone(Some(&failed), None, &tree, "master", None, None) ); } @@ -1616,7 +1663,13 @@ last_line_without_newline=yes"; .unwrap(); assert_eq!(Some("master".to_string()), on_head.current_branch); assert_eq!(None, on_head.build_record); - assert_eq!(Err(DeployAloneRefusal::NoBuildRecord), on_head.deploy_alone); + assert_eq!( + Err(DeployAloneRefusal::HeadMismatch { + recorded: "patchset-x".to_string(), + current: "master".to_string(), + }), + on_head.deploy_alone + ); let for_typed = evaluate_readiness( &OsFileSystem, From c25dd45600a3e2f5539628761763aeb66feeb2bb Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 20 Sep 2026 10:47:37 -0300 Subject: [PATCH 12/12] docs(kw): describe deploy without the integration plan This commit drops plan and lab-scenario citations, comments about unmapped exit codes and future kw master, and long reserved-flag essays. What remains describes the remote parser, argv merge, and deploy-alone gate as they stand. This commit is part of the kw integration's step 7. Signed-off-by: lorenzoberts --- src/kw/actor.rs | 15 +++------------ src/kw/argv.rs | 44 ++++++++------------------------------------ src/kw/readiness.rs | 9 ++++----- src/kw/remote.rs | 9 ++++----- src/kw/status.rs | 14 +++++--------- 5 files changed, 24 insertions(+), 67 deletions(-) diff --git a/src/kw/actor.rs b/src/kw/actor.rs index 63703ae..674bc05 100644 --- a/src/kw/actor.rs +++ b/src/kw/actor.rs @@ -290,18 +290,9 @@ impl KwActor { } /// Accepts and starts a job, or refuses. The reply is sent by the - /// caller right after this returns: the job itself keeps running in a - /// detached task and is observed via the status snapshot. - /// - /// Refusals, in order: a job already running, no kw binary on PATH, - /// unresolvable kw-env state, a tree that fails the readiness probes, - /// a dirty worktree, or a failed branch switch. Deploy kinds then also - /// refuse on an unresolved remote; StartDeploy additionally requires a - /// matching build record and image, then an acknowledged boot-once - /// setting. Reserved flags on patch-hub's own argv win over the - /// request's extra args. A start refused after the branch switch rolls - /// the switch back: a refused start never leaves the tree on a branch - /// the user did not check out. + /// caller right after this returns; the job keeps running in a + /// detached task. A start refused after the branch switch rolls + /// the switch back. async fn start_job( &mut self, kind: KwJobKind, diff --git a/src/kw/argv.rs b/src/kw/argv.rs index 5c2b8af..d2ce4b5 100644 --- a/src/kw/argv.rs +++ b/src/kw/argv.rs @@ -66,43 +66,15 @@ const BUILD_RESERVED: &[ReservedOption] = &[ ReservedOption::gnu_abbrev(&["--from-sha"], true), ]; -/// Reserved for `kw deploy`. Patch-hub always injects a resolved -/// `--remote host:port` and the reboot/force knobs; user extras that -/// would override those, switch to local, list/uninstall kernels, run -/// interactive `--setup`, or flip `boot_into_new_kernel_once` via `-n` -/// are stripped. +/// Reserved for `kw deploy`. Injected `--remote` / reboot / force win. +/// User extras that override those, switch to local, list/uninstall, +/// run `--setup`, or pass `-n` are stripped. Build-only extras are +/// stripped too (one KwOps field; unknown flags are exit 22). +/// GNU getopt forms (`-rf`, `-Fpkg`, unique `--boot`) are stripped as well. /// -/// Short-flag collisions with ssh-config intuition matter here: `-l` is -/// `--list`, not `--local` (`--local` has no short form); `-r` is -/// `--reboot`, not `--remote`. `--uninstall`/`-u` takes an optional -/// value in kw (`uninstall::`) but is reserved as a boolean so `-u` -/// does not eat the following token. `--alert` is not a deploy option -/// at all — injecting or forwarding it hard-fails on kw beta-0.9 -/// (`Invalid option`), the version this lab still reports. -/// -/// Build-only flags are stripped too: KwOps has one extras field, and -/// kw deploy's getopt (`src/deploy.sh` at 0.10: `remote:,local,reboot, -/// no-reboot,modules,list,ls-line,uninstall::,list-all,force,setup, -/// verbose,create-package,from-package:,boot-into-new-kernel-once`) -/// rejects unrecognized options with exit 22. Without this, `D` with -/// `--ccache` would build successfully then die at the deploy boundary. -/// Short spellings that collide with deploy's own flags (`-n` menu vs -/// boot-once, `-f` full-cleanup vs force, `-s` save-log-to vs ls-line) -/// stay on the deploy meaning already reserved above. -/// -/// kw_parse is GNU `getopt -q`, so exact-token stripping is not enough: -/// `-rf` bundles `--reboot --force`, `-Fpkg.kw.tar` attaches a -/// from-package value, and `--boot` uniquely abbreviates -/// `--boot-into-new-kernel-once`. Those forms are stripped too. -/// `gnu_abbrev` is only set on flags deploy's getopt actually honors, -/// so `--m` still reaches `--modules` instead of matching build-only -/// `--menu`. Ambiguous prefixes (`--re` → remote and reboot) pass -/// through and fail getopt rather than eating the next token. -/// Abbreviation uniqueness is among this reserved table, not kw's full -/// option set. For deploy those coincide (the only passthrough longs are -/// `--modules`/`--verbose`). For build, `--c` is ambiguous in kw -/// (`ccache`/`cpu-scaling`/`clean`/`cflags`) but uniquely matches -/// `--clean` here, so it is stripped instead of failing getopt. +/// `-l` is `--list`, not `--local`; `-r` is `--reboot`, not `--remote`. +/// `--uninstall`/`-u` is reserved as a boolean so `-u` does not eat the +/// next token. `--alert` is not a deploy option. const DEPLOY_RESERVED: &[ReservedOption] = &[ ReservedOption::gnu_abbrev(&["--remote"], true), ReservedOption::gnu_abbrev(&["--local"], false), diff --git a/src/kw/readiness.rs b/src/kw/readiness.rs index 6a01c29..ee569ca 100644 --- a/src/kw/readiness.rs +++ b/src/kw/readiness.rs @@ -168,8 +168,7 @@ pub fn read_build_arch(fs: &dyn FileSystemTrait, tree_path: &Path) -> Option Option { /// `record` is the lookup keyed by the deploy target branch. `latest` is /// the newest record for the tree across branches. When the target has /// no keyed record but another branch does, that is -/// [`DeployAloneRefusal::HeadMismatch`] (M18), not "no build recorded". +/// [`DeployAloneRefusal::HeadMismatch`], not "no build recorded". /// /// This is only the record-matching half of the gate — it says nothing /// about the tree's *current* state. [`evaluate_readiness`] conjoins @@ -1364,8 +1363,8 @@ last_line_without_newline=yes"; check_deploy_alone(Some(&record), None, &tree, "master", None, Some(&image)) ); - // Production M18: no keyed row for the target, but the tree has - // a latest build on another branch. + // No keyed row for the target, but the tree has a latest build + // on another branch. assert_eq!( Err(DeployAloneRefusal::HeadMismatch { recorded: "patchset-x".to_string(), diff --git a/src/kw/remote.rs b/src/kw/remote.rs index 93a04b9..0df8238 100644 --- a/src/kw/remote.rs +++ b/src/kw/remote.rs @@ -254,9 +254,8 @@ mod tests { use super::*; - /// The example from kw's own writer (`src/kw_remote.sh`) / the - /// integration plan: two hosts, default on the second, fields indented. - const PLAN_FIXTURE: &str = "\ + /// Example remote.config: two hosts, default on the second, fields indented. + const SAMPLE_REMOTE_CONFIG: &str = "\ #kw-default=arch-test Host steamos Hostname steamdeck @@ -294,7 +293,7 @@ Host ph-dut #[test] fn plan_fixture_picks_the_default_host() { - let parsed = parse_remote_config(PLAN_FIXTURE); + let parsed = parse_remote_config(SAMPLE_REMOTE_CONFIG); assert_eq!(Some("arch-test"), parsed.default.as_deref()); assert_eq!( vec![ @@ -303,7 +302,7 @@ Host ph-dut ], parsed.hosts ); - assert_eq!("abc@arch-tm:22", choose(PLAN_FIXTURE).unwrap().endpoint()); + assert_eq!("abc@arch-tm:22", choose(SAMPLE_REMOTE_CONFIG).unwrap().endpoint()); } #[test] diff --git a/src/kw/status.rs b/src/kw/status.rs index d55d4a0..ccc1860 100644 --- a/src/kw/status.rs +++ b/src/kw/status.rs @@ -97,14 +97,10 @@ impl KwJobStatus { } } -/// Human-readable hint for a `kw deploy` exit code, when we know it. -/// -/// Codes are the ones verified in kw 0.10's deploy path (`src/deploy.sh`, -/// `install.sh`, `bootloader.sh`). Unknown codes — including errno 30 -/// (EROFS), which is not used there — return `None` so the UI can still -/// show the raw number. 68 can still surface even with `--force`: force -/// only skips the interactive prompt, not the underlying initramfs -/// errors. +/// Human-readable hint for a known `kw deploy` exit code. +/// Unknown codes return `None` so the UI can still show the raw number. +/// 68 can still surface with `--force`: force skips the prompt, not the +/// initramfs errors. #[cfg_attr(not(unix), allow(dead_code))] pub fn deploy_exit_hint(code: i32) -> Option<&'static str> { Some(match code { @@ -210,7 +206,7 @@ mod tests { #[test] fn deploy_exit_hint_leaves_unknown_codes_unnamed() { - // 30/EROFS was a stale map entry: kw's deploy path never returns it. + // Unknown codes have no hint. assert_eq!(None, deploy_exit_hint(30)); assert_eq!(None, deploy_exit_hint(1)); assert_eq!(None, deploy_exit_hint(0));