From 6e8618a6fc0761124b8ac052305854300b8b36f6 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Thu, 20 Aug 2026 10:38:08 -0300 Subject: [PATCH 1/9] feat(infrastructure): add read_dir to FileSystemTrait This commit extends the file-system abstraction with directory listing so callers can discover kernel images under arch boot directories. Entry kind stays on the existing is_dir/is_file methods, keeping the new surface minimal. This commit is part of the kw integration's step 3. Signed-off-by: lorenzoberts --- src/infrastructure/file_system/mod.rs | 10 ++++++++- src/infrastructure/file_system/tests.rs | 28 +++++++++++++++++++++++++ src/infrastructure/file_system/trait.rs | 13 +++++++++++- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/infrastructure/file_system/mod.rs b/src/infrastructure/file_system/mod.rs index 48ede4e4..41d2c66e 100644 --- a/src/infrastructure/file_system/mod.rs +++ b/src/infrastructure/file_system/mod.rs @@ -10,7 +10,7 @@ pub use r#trait::MockFileSystemTrait; use std::{ fs::{self, File}, io::{self, BufReader}, - path::Path, + path::{Path, PathBuf}, }; #[cfg(test)] @@ -43,6 +43,14 @@ impl FileSystemTrait for OsFileSystem { path.is_dir() } + fn read_dir(&self, path: &Path) -> Result, FileSystemError> { + let mut entries = fs::read_dir(path)? + .map(|entry| entry.map(|e| e.path())) + .collect::, io::Error>>()?; + entries.sort(); + Ok(entries) + } + fn rename(&self, from: &Path, to: &Path) -> Result<(), FileSystemError> { Ok(fs::rename(from, to)?) } diff --git a/src/infrastructure/file_system/tests.rs b/src/infrastructure/file_system/tests.rs index a864663b..e1ae3dc4 100644 --- a/src/infrastructure/file_system/tests.rs +++ b/src/infrastructure/file_system/tests.rs @@ -101,6 +101,34 @@ fn is_dir_distinguishes_dirs_from_files() { assert!(!fs.is_dir(&file_path)); } +#[test] +fn read_dir_lists_immediate_children_sorted() { + let dir = TempDir::new("read_dir"); + std::fs::write(dir.path().join("b.txt"), "").unwrap(); + std::fs::create_dir(dir.path().join("a_sub")).unwrap(); + std::fs::create_dir(dir.path().join("a_sub/nested")).unwrap(); + std::fs::write(dir.path().join("c.txt"), "").unwrap(); + + let fs = OsFileSystem; + let entries = fs.read_dir(dir.path()).unwrap(); + + assert_eq!( + entries, + vec![ + dir.path().join("a_sub"), + dir.path().join("b.txt"), + dir.path().join("c.txt"), + ] + ); +} + +#[test] +fn read_dir_returns_error_for_missing_dir() { + let fs = OsFileSystem; + let result = fs.read_dir(Path::new("/nonexistent/path")); + assert!(result.is_err()); +} + #[test] fn rename_moves_file() { let dir = TempDir::new("rename"); diff --git a/src/infrastructure/file_system/trait.rs b/src/infrastructure/file_system/trait.rs index c1a1fb51..077eb200 100644 --- a/src/infrastructure/file_system/trait.rs +++ b/src/infrastructure/file_system/trait.rs @@ -1,7 +1,11 @@ use mockall::automock; use thiserror::Error; -use std::{fs::Metadata, io, path::Path}; +use std::{ + fs::Metadata, + io, + path::{Path, PathBuf}, +}; #[derive(Debug, Error)] pub enum FileSystemError { @@ -17,6 +21,13 @@ pub trait FileSystemTrait: Send + Sync { fn exists(&self, path: &Path) -> bool; fn is_file(&self, path: &Path) -> bool; fn is_dir(&self, path: &Path) -> bool; + /// Returns the immediate children of directory `path` as full paths, + /// sorted for determinism. Entry kind and metadata are queried + /// separately via `is_dir`/`is_file`/`metadata`. + // No production caller until the kw readiness probes land; kept per the + // CachePolicy precedent (src/lore/application/cache.rs). + #[allow(dead_code)] + fn read_dir(&self, path: &Path) -> Result, FileSystemError>; fn rename(&self, from: &Path, to: &Path) -> Result<(), FileSystemError>; fn create_writer(&self, path: &Path) -> Result, FileSystemError>; fn open_bufreader(&self, path: &Path) -> Result, FileSystemError>; From 9d7f6de1350273119f60fd075d04460a73a23ce0 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Thu, 20 Aug 2026 10:45:39 -0300 Subject: [PATCH 2/9] feat(kw): add kernel tree probes and kw config parser This commit adds the first tree-side readiness probes: a parser for kw's key=value config files, an is_kernel_root check matching kw's own file set, and a build-arch reader that returns the literal arch= value. An unset arch maps to None so callers glob rather than guess an architecture. This commit is part of the kw integration's step 3. Signed-off-by: lorenzoberts --- src/kw/mod.rs | 4 +- src/kw/readiness.rs | 388 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 391 insertions(+), 1 deletion(-) create mode 100644 src/kw/readiness.rs diff --git a/src/kw/mod.rs b/src/kw/mod.rs index 68c49c5e..a01a8244 100644 --- a/src/kw/mod.rs +++ b/src/kw/mod.rs @@ -1,3 +1,5 @@ -//! kw integration: persistence for apply history. +//! kw integration: persistence, readiness probes, and (in later steps) the +//! actor orchestrating `kw build` / `kw deploy` jobs. pub mod history; +pub mod readiness; diff --git a/src/kw/readiness.rs b/src/kw/readiness.rs new file mode 100644 index 00000000..ef58ed24 --- /dev/null +++ b/src/kw/readiness.rs @@ -0,0 +1,388 @@ +//! Readiness probes for running `kw build` / `kw deploy` on a configured +//! kernel tree. +//! +//! Each probe mirrors the corresponding discovery logic in kw itself +//! (`src/lib/kwlib.sh`, `src/lib/kw_config_loader.sh`, `src/deploy.sh` at +//! kw 0.10) so patch-hub's idea of "ready" matches what kw will actually +//! do, instead of being a parallel interpretation that can silently drift +//! from it. The probes are pure functions over injected infrastructure +//! traits; KwActor composes them into the `GetReadiness` snapshot. + +use std::{collections::HashMap, path::Path}; + +use crate::infrastructure::file_system::FileSystemTrait; + +/// Readiness of a configured kernel tree for kw operations. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum TreeReadiness { + /// Kernel root, kw-initialized, with a `.config`. `arch` is the literal + /// `arch=` value from `.kw/build.config`; `None` means image discovery + /// must glob `arch/*/boot/`, the fallback kw's own `arch=` resolution + /// effectively produces when the key is unset. + Ready { arch: Option }, + /// The configured path is not a directory. + Missing, + /// Directory exists but lacks the files and directories kw's + /// `is_kernel_root` expects at a kernel tree root. + NotAKernelRoot, + /// No `.kw/` directory: `kw init` was never run in this tree. + MissingKwDir, + /// No `.config` at the build root (the tree itself, or the kw env's + /// output dir when one is active). + MissingKernelConfig, +} + +/// Parses kw's `key=value` config format (`.kw/build.config`, +/// `.kw/deploy.config`, ...), mirroring kw's own `parse_configuration`: +/// blank lines and lines starting with `#` are skipped, everything from the +/// last `#` on is stripped as a trailing comment, the key has all +/// whitespace removed, and the value is trimmed. Lines without `=` are +/// ignored, and a final line without a trailing newline still counts. +pub fn parse_kw_config(content: &str) -> HashMap { + let mut entries = HashMap::new(); + for line in content.lines() { + if line.starts_with('#') || line.is_empty() { + continue; + } + let uncommented = match line.rfind('#') { + Some(index) => &line[..index], + None => line, + }; + let Some((key, value)) = uncommented.split_once('=') else { + continue; + }; + let key: String = key.chars().filter(|c| !c.is_whitespace()).collect(); + entries.insert(key, value.trim().to_string()); + } + entries +} + +/// Mirrors kw's `is_kernel_root`: the same files and directories kw checks +/// (also the set `get_maintainer.pl` relies on). `MAINTAINERS` is checked +/// with `exists` because kw uses `-e` on it and `-f` on the other files. +pub fn is_kernel_root(fs: &dyn FileSystemTrait, path: &Path) -> bool { + const FILES: [&str; 5] = ["COPYING", "CREDITS", "Kbuild", "Makefile", "README"]; + const DIRS: [&str; 10] = [ + "Documentation", + "arch", + "include", + "drivers", + "fs", + "init", + "ipc", + "kernel", + "lib", + "scripts", + ]; + + FILES.iter().all(|file| fs.is_file(&path.join(file))) + && fs.exists(&path.join("MAINTAINERS")) + && DIRS.iter().all(|dir| fs.is_dir(&path.join(dir))) +} + +/// Probes whether `tree_path` is a kernel tree ready for kw operations. +/// `output_dir` is the resolved kw-env `O=` path when an env is active: kw +/// then keeps the `.config` there instead of in the tree root. +// No production caller until the readiness aggregation lands; kept per the +// CachePolicy precedent (src/lore/application/cache.rs). +#[allow(dead_code)] +pub fn probe_tree( + fs: &dyn FileSystemTrait, + tree_path: &Path, + output_dir: Option<&Path>, +) -> TreeReadiness { + if !fs.is_dir(tree_path) { + return TreeReadiness::Missing; + } + if !is_kernel_root(fs, tree_path) { + return TreeReadiness::NotAKernelRoot; + } + if !fs.is_dir(&tree_path.join(".kw")) { + return TreeReadiness::MissingKwDir; + } + let build_root = output_dir.unwrap_or(tree_path); + if !fs.is_file(&build_root.join(".config")) { + return TreeReadiness::MissingKernelConfig; + } + TreeReadiness::Ready { + arch: read_build_arch(fs, tree_path), + } +} + +/// Reads the literal `arch=` value from `/.kw/build.config`, the same +/// value kw's image discovery globs under `arch//boot/`. Returns +/// `None` when the file or the key is absent or empty — kw's +/// `${build_config[arch]:-...}` expansion treats empty as unset — meaning +/// the caller should fall back to globbing `arch/*/boot/`. +pub fn read_build_arch(fs: &dyn FileSystemTrait, tree_path: &Path) -> Option { + let content = fs + .read_to_string(&tree_path.join(".kw").join("build.config")) + .ok()?; + let arch = parse_kw_config(&content).remove("arch")?; + if arch.is_empty() { None } else { Some(arch) } +} + +#[cfg(test)] +mod tests { + use std::{ + fs, + path::PathBuf, + sync::atomic::{AtomicU64, Ordering}, + }; + + use crate::infrastructure::file_system::OsFileSystem; + + use super::*; + + static TEST_SEQ: AtomicU64 = AtomicU64::new(0); + + struct TempDir(PathBuf); + + impl TempDir { + fn new(test_name: &str) -> Self { + let n = TEST_SEQ.fetch_add(1, Ordering::SeqCst); + let dir = std::env::temp_dir().join(format!( + "patch-hub-kw-readiness-{}-{}-{}", + test_name, + std::process::id(), + n + )); + // A leftover from a failed previous run must not poison this one. + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&dir).unwrap(); + Self(dir) + } + + fn path(&self) -> &Path { + &self.0 + } + } + + impl Drop for TempDir { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } + } + + const KERNEL_ROOT_FILES: [&str; 6] = [ + "COPYING", + "CREDITS", + "Kbuild", + "Makefile", + "README", + "MAINTAINERS", + ]; + const KERNEL_ROOT_DIRS: [&str; 10] = [ + "Documentation", + "arch", + "include", + "drivers", + "fs", + "init", + "ipc", + "kernel", + "lib", + "scripts", + ]; + + /// Creates the exact file/dir set kw's `is_kernel_root` expects. + fn make_kernel_root(dir: &Path) { + for file in KERNEL_ROOT_FILES { + fs::write(dir.join(file), "").unwrap(); + } + for sub in KERNEL_ROOT_DIRS { + fs::create_dir(dir.join(sub)).unwrap(); + } + } + + /// A kernel root with `.kw/` and an in-tree `.config`: the fully ready + /// fixture most probes start from. + fn make_ready_tree(test_name: &str) -> TempDir { + let dir = TempDir::new(test_name); + make_kernel_root(dir.path()); + fs::create_dir(dir.path().join(".kw")).unwrap(); + fs::write(dir.path().join(".config"), "").unwrap(); + dir + } + + #[test] + fn parse_kw_config_mirrors_kw_semantics() { + // Deliberately no trailing newline: kw's read loop handles a final + // unterminated line, and so must we. + let content = "\ +# a comment line +arch=x86_64 + + cpu_scaling_factor = 100 +kernel_img_name=bzImage # trailing comment +dtb_copy_pattern={broadcom,rockchip}#kept +no_equals_line +cross_compile= +last_line_without_newline=yes"; + let parsed = parse_kw_config(content); + + assert_eq!(parsed.get("arch"), Some(&"x86_64".to_string())); + assert_eq!(parsed.get("cpu_scaling_factor"), Some(&"100".to_string())); + assert_eq!(parsed.get("kernel_img_name"), Some(&"bzImage".to_string())); + // kw strips from the last '#', so an earlier one stays in the value. + assert_eq!( + parsed.get("dtb_copy_pattern"), + Some(&"{broadcom,rockchip}".to_string()) + ); + assert_eq!(parsed.get("cross_compile"), Some(&String::new())); + assert_eq!( + parsed.get("last_line_without_newline"), + Some(&"yes".to_string()) + ); + assert!(!parsed.contains_key("no_equals_line")); + assert_eq!(6, parsed.len()); + } + + #[test] + fn is_kernel_root_accepts_complete_fixture() { + let dir = TempDir::new("kernel-root-complete"); + make_kernel_root(dir.path()); + + assert!(is_kernel_root(&OsFileSystem, dir.path())); + } + + #[test] + fn is_kernel_root_rejects_any_missing_member() { + for member in KERNEL_ROOT_FILES.into_iter().chain(KERNEL_ROOT_DIRS) { + let dir = TempDir::new("kernel-root-incomplete"); + make_kernel_root(dir.path()); + fs::remove_dir_all(dir.path().join(member)) + .or_else(|_| fs::remove_file(dir.path().join(member))) + .unwrap(); + + assert!( + !is_kernel_root(&OsFileSystem, dir.path()), + "missing {member} should fail the check" + ); + } + } + + #[test] + fn probe_tree_reports_missing_path() { + let dir = TempDir::new("probe-missing"); + + assert_eq!( + TreeReadiness::Missing, + probe_tree(&OsFileSystem, &dir.path().join("nope"), None) + ); + } + + #[test] + fn probe_tree_reports_non_kernel_root() { + let dir = TempDir::new("probe-not-root"); + + assert_eq!( + TreeReadiness::NotAKernelRoot, + probe_tree(&OsFileSystem, dir.path(), None) + ); + } + + #[test] + fn probe_tree_reports_missing_kw_dir() { + let dir = TempDir::new("probe-no-kw"); + make_kernel_root(dir.path()); + + assert_eq!( + TreeReadiness::MissingKwDir, + probe_tree(&OsFileSystem, dir.path(), None) + ); + } + + #[test] + fn probe_tree_reports_missing_kernel_config() { + let dir = TempDir::new("probe-no-config"); + make_kernel_root(dir.path()); + fs::create_dir(dir.path().join(".kw")).unwrap(); + + assert_eq!( + TreeReadiness::MissingKernelConfig, + probe_tree(&OsFileSystem, dir.path(), None) + ); + } + + #[test] + fn probe_tree_ready_without_build_config_means_glob_fallback() { + let dir = make_ready_tree("probe-ready"); + + assert_eq!( + TreeReadiness::Ready { arch: None }, + probe_tree(&OsFileSystem, dir.path(), None) + ); + } + + #[test] + fn probe_tree_ready_reads_arch_from_build_config() { + let dir = make_ready_tree("probe-ready-arch"); + fs::write(dir.path().join(".kw/build.config"), "arch=arm64\n").unwrap(); + + assert_eq!( + TreeReadiness::Ready { + arch: Some("arm64".to_string()) + }, + probe_tree(&OsFileSystem, dir.path(), None) + ); + } + + #[test] + fn probe_tree_with_active_env_checks_config_in_output_dir() { + let dir = make_ready_tree("probe-env"); + let out = TempDir::new("probe-env-output"); + // With a kw env active, kw moves the .config into the env's O= dir. + fs::remove_file(dir.path().join(".config")).unwrap(); + + assert_eq!( + TreeReadiness::MissingKernelConfig, + probe_tree(&OsFileSystem, dir.path(), Some(out.path())) + ); + + fs::write(out.path().join(".config"), "").unwrap(); + assert!(matches!( + probe_tree(&OsFileSystem, dir.path(), Some(out.path())), + TreeReadiness::Ready { .. } + )); + } + + #[test] + fn read_build_arch_reads_literal_value() { + let dir = make_ready_tree("arch-literal"); + fs::write(dir.path().join(".kw/build.config"), "arch=x86\n").unwrap(); + + assert_eq!( + Some("x86".to_string()), + read_build_arch(&OsFileSystem, dir.path()) + ); + } + + #[test] + fn read_build_arch_unset_means_glob_fallback() { + // Missing file, missing key, commented key, and empty value all map + // to kw's "unset" semantics. + let no_file = make_ready_tree("arch-no-file"); + assert_eq!(None, read_build_arch(&OsFileSystem, no_file.path())); + + let no_key = make_ready_tree("arch-no-key"); + fs::write( + no_key.path().join(".kw/build.config"), + "cpu_scaling_factor=100\n", + ) + .unwrap(); + assert_eq!(None, read_build_arch(&OsFileSystem, no_key.path())); + + let commented = make_ready_tree("arch-commented"); + fs::write( + commented.path().join(".kw/build.config"), + "#arch=riscv\n", + ) + .unwrap(); + assert_eq!(None, read_build_arch(&OsFileSystem, commented.path())); + + let empty = make_ready_tree("arch-empty"); + fs::write(empty.path().join(".kw/build.config"), "arch=\n").unwrap(); + assert_eq!(None, read_build_arch(&OsFileSystem, empty.path())); + } +} From 7de6845c63d4bb5c5d6a3afb0c7257797c116c1e Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Thu, 20 Aug 2026 10:51:14 -0300 Subject: [PATCH 3/9] feat(kw): resolve kw env output dir and discover kernel images This commit adds env-aware image discovery. The active kw env's O= path is resolved from .kw/env.current using kw's cache layout, and the newest *Image under the matching arch boot directory is selected. Trees without an arch= line glob every arch boot dir so they are not false-negative. This commit is part of the kw integration's step 3. Signed-off-by: lorenzoberts --- Cargo.lock | 1 + Cargo.toml | 1 + src/kw/readiness.rs | 351 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 349 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 76265be0..b1363bf7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -855,6 +855,7 @@ version = "0.1.7" dependencies = [ "ansi-to-tui", "async-trait", + "base64", "chrono", "clap", "color-eyre", diff --git a/Cargo.toml b/Cargo.toml index b27eb8e8..d9ffd029 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,7 @@ ureq = { version = "3.0.12", features = ["rustls"] } tokio = { version = "1.52.3", features = ["rt-multi-thread", "macros", "sync", "process", "time"] } async-trait = "0.1" nix = { version = "0.31", features = ["signal"] } +base64 = "0.22" [dev-dependencies] ctor = "0.2" diff --git a/src/kw/readiness.rs b/src/kw/readiness.rs index ef58ed24..3e6197ee 100644 --- a/src/kw/readiness.rs +++ b/src/kw/readiness.rs @@ -8,9 +8,29 @@ //! from it. The probes are pure functions over injected infrastructure //! traits; KwActor composes them into the `GetReadiness` snapshot. -use std::{collections::HashMap, path::Path}; - -use crate::infrastructure::file_system::FileSystemTrait; +use base64::{Engine as _, engine::general_purpose::STANDARD as BASE64}; +use thiserror::Error; + +use std::{ + collections::HashMap, + path::{Path, PathBuf}, + time::SystemTime, +}; + +use crate::infrastructure::{ + env::{EnvError, EnvTrait}, + file_system::{FileSystemError, FileSystemTrait}, +}; + +/// Errors from readiness probes for states where "absent" is not a normal +/// situation (unlike a missing `.kw` dir, which is a readiness verdict). +#[derive(Debug, Error)] +pub enum KwReadinessError { + #[error("filesystem error: {0}")] + Fs(#[from] FileSystemError), + #[error("cannot resolve the kw cache dir: {0}")] + Env(#[from] EnvError), +} /// Readiness of a configured kernel tree for kw operations. #[derive(Debug, Clone, PartialEq, Eq)] @@ -122,15 +142,138 @@ 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 +/// 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. +/// +/// kw's launcher recomputes the cache dir unconditionally, so a +/// user-exported `KW_CACHE_DIR` is intentionally ignored here too. The +/// `KWORKFLOW` rename knob is not honored: it exists for kw development. +/// +/// Returns `Ok(None)` when no env is active. The resolved dir is not +/// required to exist: kw/make create it on first build. An unreadable +/// `env.current` or an unresolvable cache base (neither `XDG_CACHE_HOME` +/// nor `HOME` set) is an error, since the env state is then unknown — +/// kw's "active but unresolvable" case. +// No production caller until the readiness aggregation lands; kept per the +// CachePolicy precedent (src/lore/application/cache.rs). +#[allow(dead_code)] +pub fn resolve_output_dir( + fs: &dyn FileSystemTrait, + env: &dyn EnvTrait, + tree_path: &Path, +) -> Result, KwReadinessError> { + let env_file = tree_path.join(".kw").join("env.current"); + if !fs.is_file(&env_file) { + return Ok(None); + } + let env_name = fs + .read_to_string(&env_file)? + .trim_end_matches('\n') + .to_string(); + // An empty env.current names no env; kw's $(<) read yields the same + // empty string and kw then behaves as if no env were active. + if env_name.is_empty() { + return Ok(None); + } + + let cache_base = match env.var("XDG_CACHE_HOME") { + Ok(xdg) => xdg, + Err(_) => format!("{}/.cache", env.var("HOME")?), + }; + let trimmed = tree_path.to_string_lossy(); + let normalized = match trimmed.trim_end_matches('/') { + "" => "/", + path => path, + }; + let encoded = BASE64.encode(normalized); + Ok(Some( + Path::new(&cache_base) + .join("kw") + .join("envs") + .join(encoded) + .join(env_name), + )) +} + +/// Finds the newest kernel image under `/arch/`, mirroring kw's +/// `get_kernel_binary_name`: a candidate's basename must end with `Image` +/// (find's `-name '*Image'` is case-sensitive, so `Image.gz` and `image` +/// are excluded) and the most recently modified one wins, with ties broken +/// by descending path (kw's `sort -r | head -1`). With `arch`, only +/// `arch//boot/` is probed; without, every `arch/*/boot/` is globbed. +/// +/// Deliberate deviation: kw's `find` recurses into boot/ subdirectories, +/// while this scans only the top level. Kernel images for every arch kw +/// supports are produced directly in boot/ (subdirs like compressed/ or +/// dts/ never hold `*Image` files), and find does not descend into symlinked +/// dirs either, so the behaviors agree on real trees. +// No production caller until the readiness aggregation lands; kept per the +// CachePolicy precedent (src/lore/application/cache.rs). +#[allow(dead_code)] +pub fn find_newest_kernel_image( + fs: &dyn FileSystemTrait, + build_root: &Path, + arch: Option<&str>, +) -> Option { + match arch { + Some(arch) => newest_image_in(fs, &build_root.join("arch").join(arch).join("boot")), + None => fs + .read_dir(&build_root.join("arch")) + .ok()? + .into_iter() + .filter(|entry| fs.is_dir(entry)) + .filter_map(|arch_dir| { + newest_image_in(fs, &arch_dir.join("boot")) + .map(|image| (image_mtime(fs, &image), image)) + }) + .max_by_key(|(mtime, _)| *mtime) + .map(|(_, image)| image), + } +} + +/// Newest `*Image` file directly inside `boot_dir`, if any. +fn newest_image_in(fs: &dyn FileSystemTrait, boot_dir: &Path) -> Option { + fs.read_dir(boot_dir) + .ok()? + .into_iter() + .filter(|entry| { + entry + .file_name() + .is_some_and(|name| name.to_string_lossy().ends_with("Image")) + && fs.is_file(entry) + }) + .map(|entry| (image_mtime(fs, &entry), entry)) + .max_by_key(|(mtime, _)| *mtime) + .map(|(_, entry)| entry) +} + +fn image_mtime(fs: &dyn FileSystemTrait, path: &Path) -> SystemTime { + fs.metadata(path) + .and_then(|meta| meta.modified().map_err(FileSystemError::from)) + .unwrap_or(SystemTime::UNIX_EPOCH) +} + #[cfg(test)] mod tests { use std::{ fs, path::PathBuf, sync::atomic::{AtomicU64, Ordering}, + time::{Duration, SystemTime}, }; - use crate::infrastructure::file_system::OsFileSystem; + use crate::infrastructure::{ + env::MockEnvTrait, + file_system::{MockFileSystemTrait, OsFileSystem}, + }; use super::*; @@ -385,4 +528,204 @@ last_line_without_newline=yes"; fs::write(empty.path().join(".kw/build.config"), "arch=\n").unwrap(); assert_eq!(None, read_build_arch(&OsFileSystem, empty.path())); } + + /// Creates a file whose mtime is `mtime_secs` seconds after the epoch, + /// so newest-wins ordering is fully deterministic. + fn write_file_with_mtime(path: &Path, mtime_secs: u64) { + let file = fs::File::create(path).unwrap(); + file.set_times( + fs::FileTimes::new() + .set_modified(SystemTime::UNIX_EPOCH + Duration::from_secs(mtime_secs)), + ) + .unwrap(); + } + + #[test] + fn resolve_output_dir_without_env_file_is_inactive() { + let dir = make_ready_tree("env-inactive"); + let env = MockEnvTrait::new(); + + assert_eq!( + None, + resolve_output_dir(&OsFileSystem, &env, dir.path()).unwrap() + ); + } + + #[test] + fn resolve_output_dir_encodes_tree_path_like_kw() { + // Known vector: `printf '%s' /home/user/linux | base64 --wrap=0`. + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_file() + .returning(|p| p == Path::new("/home/user/linux/.kw/env.current")); + fs.expect_read_to_string() + .returning(|_| Ok("minix\n".to_string())); + let mut env = MockEnvTrait::new(); + env.expect_var() + .withf(|key| key == "XDG_CACHE_HOME") + .returning(|_| Ok("/xdg".to_string())); + + let resolved = + resolve_output_dir(&fs, &env, Path::new("/home/user/linux")).unwrap(); + + assert_eq!( + Some(PathBuf::from("/xdg/kw/envs/L2hvbWUvdXNlci9saW51eA==/minix")), + resolved + ); + } + + #[test] + fn resolve_output_dir_falls_back_to_home_cache() { + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_file().returning(|_| true); + fs.expect_read_to_string() + .returning(|_| Ok("minix\n".to_string())); + let mut env = MockEnvTrait::new(); + env.expect_var() + .withf(|key| key == "XDG_CACHE_HOME") + .returning(|_| Err(std::env::VarError::NotPresent.into())); + env.expect_var() + .withf(|key| key == "HOME") + .returning(|_| Ok("/home/user".to_string())); + + let resolved = resolve_output_dir(&fs, &env, Path::new("/kernel")).unwrap(); + + assert_eq!( + Some( + Path::new("/home/user/.cache/kw/envs") + .join(BASE64.encode("/kernel")) + .join("minix") + ), + resolved + ); + } + + #[test] + fn resolve_output_dir_trims_trailing_slashes_before_encoding() { + // kw encodes $PWD after cd-ing into the tree, where the path no + // longer carries a trailing slash. + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_file().returning(|_| true); + fs.expect_read_to_string() + .returning(|_| Ok("minix\n".to_string())); + let mut env = MockEnvTrait::new(); + env.expect_var() + .withf(|key| key == "XDG_CACHE_HOME") + .returning(|_| Ok("/xdg".to_string())); + + let resolved = + resolve_output_dir(&fs, &env, Path::new("/home/user/linux/")).unwrap(); + + assert_eq!( + Some(PathBuf::from("/xdg/kw/envs/L2hvbWUvdXNlci9saW51eA==/minix")), + resolved + ); + } + + #[test] + fn resolve_output_dir_empty_env_file_is_inactive() { + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_file().returning(|_| true); + fs.expect_read_to_string().returning(|_| Ok("\n".to_string())); + let env = MockEnvTrait::new(); + + assert_eq!(None, resolve_output_dir(&fs, &env, Path::new("/kernel")).unwrap()); + } + + #[test] + fn resolve_output_dir_unreadable_env_file_errors() { + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_file().returning(|_| true); + fs.expect_read_to_string().returning(|_| { + Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied, "denied").into()) + }); + let env = MockEnvTrait::new(); + + assert!(resolve_output_dir(&fs, &env, Path::new("/kernel")).is_err()); + } + + #[test] + fn resolve_output_dir_errors_without_any_cache_base() { + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_file().returning(|_| true); + fs.expect_read_to_string() + .returning(|_| Ok("minix\n".to_string())); + let mut env = MockEnvTrait::new(); + env.expect_var() + .returning(|_| Err(std::env::VarError::NotPresent.into())); + + assert!(resolve_output_dir(&fs, &env, Path::new("/kernel")).is_err()); + } + + #[test] + fn find_image_with_arch_picks_newest_image_only() { + let dir = make_ready_tree("image-arch"); + let boot = dir.path().join("arch/x86/boot"); + fs::create_dir_all(&boot).unwrap(); + write_file_with_mtime(&boot.join("bzImage"), 100); + write_file_with_mtime(&boot.join("Image"), 200); + // Neither name matches find's case-sensitive `*Image`. + write_file_with_mtime(&boot.join("vmlinux"), 300); + write_file_with_mtime(&boot.join("Image.gz"), 400); + + assert_eq!( + Some(boot.join("Image")), + find_newest_kernel_image(&OsFileSystem, dir.path(), Some("x86")) + ); + } + + #[test] + fn find_image_without_arch_globs_every_boot_dir() { + // The no-false-negative regression: with no arch= hint, an image + // under any arch/*/boot/ must still be found. + let dir = make_ready_tree("image-glob"); + let x86_boot = dir.path().join("arch/x86/boot"); + let arm64_boot = dir.path().join("arch/arm64/boot"); + fs::create_dir_all(&x86_boot).unwrap(); + fs::create_dir_all(&arm64_boot).unwrap(); + write_file_with_mtime(&x86_boot.join("bzImage"), 100); + write_file_with_mtime(&arm64_boot.join("Image"), 200); + + assert_eq!( + Some(arm64_boot.join("Image")), + find_newest_kernel_image(&OsFileSystem, dir.path(), None) + ); + } + + #[test] + fn find_image_tie_breaks_by_path_descending_like_kw() { + // Same mtime: kw's `sort -r | head -1` on `%T+ %p` picks the + // lexicographically larger path. + let dir = make_ready_tree("image-tie"); + let boot = dir.path().join("arch/x86/boot"); + fs::create_dir_all(&boot).unwrap(); + write_file_with_mtime(&boot.join("bzImage"), 100); + write_file_with_mtime(&boot.join("zImage"), 100); + + assert_eq!( + Some(boot.join("zImage")), + find_newest_kernel_image(&OsFileSystem, dir.path(), Some("x86")) + ); + } + + #[test] + fn find_image_missing_or_empty_dirs_return_none() { + let dir = make_ready_tree("image-none"); + // No image anywhere yet: the fixture has an empty arch/ dir. + assert_eq!(None, find_newest_kernel_image(&OsFileSystem, dir.path(), None)); + assert_eq!( + None, + find_newest_kernel_image(&OsFileSystem, dir.path(), Some("x86")) + ); + + let boot = dir.path().join("arch/x86/boot"); + fs::create_dir_all(&boot).unwrap(); + write_file_with_mtime(&boot.join("image"), 100); // lowercase: no match + write_file_with_mtime(&boot.join("Image.gz"), 200); // suffix: no match + + assert_eq!(None, find_newest_kernel_image(&OsFileSystem, dir.path(), None)); + assert_eq!( + None, + find_newest_kernel_image(&OsFileSystem, dir.path(), Some("x86")) + ); + } } From 9f77a5a959eace0fc4f25bdaae0962b86760c462 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Thu, 20 Aug 2026 10:56:22 -0300 Subject: [PATCH 4/9] feat(kw): add build records to KwHistoryStore This commit extends the history store with one build record per tree and branch, including failed attempts. A failed build on one branch does not clobber another branch's successful record, and a stored failure can be distinguished from having no record at all. This commit is part of the kw integration's step 3. Signed-off-by: lorenzoberts --- src/kw/history.rs | 344 ++++++++++++++++++++++++++++++++++++++++++---- src/main.rs | 4 +- 2 files changed, 316 insertions(+), 32 deletions(-) diff --git a/src/kw/history.rs b/src/kw/history.rs index a823c295..35154925 100644 --- a/src/kw/history.rs +++ b/src/kw/history.rs @@ -1,7 +1,9 @@ -//! User-local history of patchset applies, stored as JSON under the -//! configured `data_dir`. +//! User-local history of patchset applies and kw builds, stored as JSON +//! under the configured `data_dir`. //! -//! Records are user state — not a cache — and are never refreshed from lore. +//! Apply records feed kw build/deploy readiness and the KwOps branch +//! prefill, and build records feed deploy-alone readiness, so they are +//! user state — not a cache — and are never refreshed from lore. use mockall::automock; use serde::{Deserialize, Serialize}; @@ -12,6 +14,7 @@ use std::{collections::HashMap, io, path::Path, sync::Arc}; use crate::infrastructure::file_system::{FileSystemError, FileSystemTrait, JsonUtils}; pub const APPLY_HISTORY_FILENAME: &str = "kw_apply_history.json"; +pub const BUILD_HISTORY_FILENAME: &str = "kw_build_history.json"; /// One recorded `git am` application of a lore patchset to a kernel tree. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] @@ -26,10 +29,44 @@ pub struct KwApplyRecord { pub applied_at: String, } +/// One recorded `kw build` attempt on a kernel tree branch, whether it +/// succeeded or not: storing failures lets KwOps show "last build failed" +/// instead of "no build recorded", and deploy-alone readiness requires +/// `success == true` on the matching record. +/// +/// `message_id`, `arch`, `image_path`, and `kernelrelease` are optional: a +/// build can target a branch no patchset was applied to, `arch` is unknown +/// when image discovery had to glob, and a failed build may never have +/// produced an image or a kernelrelease. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct KwBuildRecord { + pub kernel_tree_id: String, + /// Snapshot of `KernelTree.path` when the record was written, so later + /// readiness checks can detect the tree being repointed or moved. + pub tree_path: String, + pub message_id: Option, + pub branch: String, + pub arch: Option, + pub image_path: Option, + /// Resolved kw-env `O=` dir at build time, if an env was active. + pub output_dir: Option, + pub kernelrelease: Option, + pub log_path: String, + /// RFC3339 timestamp. Readers compare parsed timestamps; records with + /// unparseable `built_at` values sort oldest. + pub built_at: String, + pub success: bool, +} + /// message id → kernel tree id → record: applying the same patchset to /// several trees keeps one record per tree. type ApplyRecords = HashMap>; +/// kernel tree id → branch → record: building several branches of the same +/// tree keeps one record per branch, so a failed build on one branch does +/// not clobber another branch's successful record. +type BuildRecords = HashMap>; + #[automock] pub trait KwHistoryStore: Send + Sync { /// Inserts or replaces the apply record for the record's @@ -45,36 +82,73 @@ pub trait KwHistoryStore: Send + Sync { message_id: &str, kernel_tree_id: &str, ) -> Result, FileSystemError>; + + /// Inserts or replaces the build record for the record's + /// `(kernel_tree_id, branch)` pair. + // Written by KwActor once builds land; kept per the CachePolicy + // precedent (src/lore/application/cache.rs). + #[allow(dead_code)] + fn record_build(&self, record: KwBuildRecord) -> Result<(), FileSystemError>; + + /// Returns the build record for the `(kernel_tree_id, branch)` pair, or + /// `None` if it was never recorded. A missing history file is a normal + /// state, not an error. + // Read by the kw readiness checks in a later step; kept per the + // CachePolicy precedent (src/lore/application/cache.rs). + #[allow(dead_code)] + fn build_record( + &self, + kernel_tree_id: &str, + branch: &str, + ) -> Result, FileSystemError>; + + /// Returns the chronologically newest build record for the tree, across + /// branches, or `None` if none was recorded. + // Read by the kw readiness checks in a later step; kept per the + // CachePolicy precedent (src/lore/application/cache.rs). + #[allow(dead_code)] + fn latest_build_record( + &self, + kernel_tree_id: &str, + ) -> Result, FileSystemError>; } pub struct FileKwHistoryStore { fs: Arc, apply_history_path: String, + build_history_path: String, } impl FileKwHistoryStore { - pub fn new(fs: Arc, apply_history_path: String) -> Self { + /// Creates a store keeping both history files ([`APPLY_HISTORY_FILENAME`] + /// and [`BUILD_HISTORY_FILENAME`]) directly under `data_dir`. + pub fn new(fs: Arc, data_dir: String) -> Self { FileKwHistoryStore { fs, - apply_history_path, + apply_history_path: format!("{data_dir}/{APPLY_HISTORY_FILENAME}"), + build_history_path: format!("{data_dir}/{BUILD_HISTORY_FILENAME}"), } } - fn load_apply_records(&self) -> Result { - let path = Path::new(&self.apply_history_path); - if !self.fs.is_file(path) { - return Ok(HashMap::new()); + /// Loads a history file, or its empty default when the file does not + /// exist. A corrupt file is an error rather than an empty map: history + /// must never be silently clobbered by the next write. + fn load_records(&self, path: &str) -> Result + where + T: serde::de::DeserializeOwned + Default, + { + let path_ref = Path::new(path); + if !self.fs.is_file(path_ref) { + return Ok(T::default()); } - let reader = self.fs.open_bufreader(path)?; - // A corrupt file is an error rather than an empty map: history must - // never be silently clobbered by the next write. + let reader = self.fs.open_bufreader(path_ref)?; from_reader(reader) .map_err(io::Error::from) .map_err(FileSystemError::from) } fn store_apply_record(&self, record: KwApplyRecord) -> Result<(), FileSystemError> { - let mut records = self.load_apply_records()?; + let mut records: ApplyRecords = self.load_records(&self.apply_history_path)?; records .entry(record.message_id.clone()) .or_default() @@ -82,20 +156,26 @@ impl FileKwHistoryStore { JsonUtils::atomic_write_json(&*self.fs, &records, &self.apply_history_path) } + fn store_build_record(&self, record: KwBuildRecord) -> Result<(), FileSystemError> { + let mut records: BuildRecords = self.load_records(&self.build_history_path)?; + records + .entry(record.kernel_tree_id.clone()) + .or_default() + .insert(record.branch.clone(), record); + JsonUtils::atomic_write_json(&*self.fs, &records, &self.build_history_path) + } + /// Makes store errors self-describing so the apply hook's warning popup /// can point the user at the file to inspect or delete. - fn error_with_path(&self, error: FileSystemError) -> FileSystemError { - FileSystemError::IoError(io::Error::other(format!( - "{}: {error}", - self.apply_history_path - ))) + fn error_with_path(&self, path: &str, error: FileSystemError) -> FileSystemError { + FileSystemError::IoError(io::Error::other(format!("{path}: {error}"))) } } impl KwHistoryStore for FileKwHistoryStore { fn record_apply(&self, record: KwApplyRecord) -> Result<(), FileSystemError> { self.store_apply_record(record) - .map_err(|e| self.error_with_path(e)) + .map_err(|e| self.error_with_path(&self.apply_history_path, e)) } fn apply_record( @@ -103,14 +183,51 @@ impl KwHistoryStore for FileKwHistoryStore { message_id: &str, kernel_tree_id: &str, ) -> Result, FileSystemError> { - self.load_apply_records() - .map(|records| { + self.load_records(&self.apply_history_path) + .map(|records: ApplyRecords| { records .get(message_id) .and_then(|by_tree| by_tree.get(kernel_tree_id)) .cloned() }) - .map_err(|e| self.error_with_path(e)) + .map_err(|e| self.error_with_path(&self.apply_history_path, e)) + } + + fn record_build(&self, record: KwBuildRecord) -> Result<(), FileSystemError> { + self.store_build_record(record) + .map_err(|e| self.error_with_path(&self.build_history_path, e)) + } + + fn build_record( + &self, + kernel_tree_id: &str, + branch: &str, + ) -> Result, FileSystemError> { + self.load_records(&self.build_history_path) + .map(|records: BuildRecords| { + records + .get(kernel_tree_id) + .and_then(|by_branch| by_branch.get(branch)) + .cloned() + }) + .map_err(|e| self.error_with_path(&self.build_history_path, e)) + } + + fn latest_build_record( + &self, + kernel_tree_id: &str, + ) -> Result, FileSystemError> { + self.load_records(&self.build_history_path) + .map(|records: BuildRecords| { + records + .get(kernel_tree_id)? + .values() + .max_by_key(|record| { + chrono::DateTime::parse_from_rfc3339(&record.built_at).ok() + }) + .cloned() + }) + .map_err(|e| self.error_with_path(&self.build_history_path, e)) } } @@ -142,13 +259,7 @@ mod tests { } fn store_at(dir: &Path) -> FileKwHistoryStore { - FileKwHistoryStore::new( - Arc::new(OsFileSystem), - dir.join(APPLY_HISTORY_FILENAME) - .to_str() - .unwrap() - .to_string(), - ) + FileKwHistoryStore::new(Arc::new(OsFileSystem), dir.to_str().unwrap().to_string()) } fn record(message_id: &str, kernel_tree_id: &str, branch: &str) -> KwApplyRecord { @@ -162,6 +273,24 @@ mod tests { } } + fn build(kernel_tree_id: &str, branch: &str, built_at: &str) -> KwBuildRecord { + KwBuildRecord { + kernel_tree_id: kernel_tree_id.to_string(), + tree_path: format!("/home/user/{kernel_tree_id}"), + message_id: None, + branch: branch.to_string(), + arch: Some("x86".to_string()), + image_path: Some(format!( + "/home/user/{kernel_tree_id}/arch/x86/boot/bzImage" + )), + output_dir: None, + kernelrelease: Some("6.17.0".to_string()), + log_path: "/home/user/.cache/patch_hub/kw_logs/build-1.log".to_string(), + built_at: built_at.to_string(), + success: true, + } + } + #[test] fn record_and_read_round_trip() { let dir = tmp_dir("round-trip"); @@ -248,7 +377,6 @@ mod tests { Arc::new(OsFileSystem), dir.join("nested") .join("deeper") - .join(APPLY_HISTORY_FILENAME) .to_str() .unwrap() .to_string(), @@ -304,4 +432,160 @@ mod tests { fs::remove_dir_all(&dir).unwrap(); } + + #[test] + fn build_record_round_trip_including_failures() { + let dir = tmp_dir("build-round-trip"); + let store = store_at(&dir); + + store.record_build(build("mainline", "for-next", "2026-08-01T18:10:00Z")).unwrap(); + let mut failed = build("mainline", "patchset-x", "2026-08-02T09:00:00Z"); + failed.success = false; + failed.image_path = None; + failed.kernelrelease = None; + store.record_build(failed.clone()).unwrap(); + + // A failed attempt is stored, not dropped: KwOps can show "last + // build failed" and deploy-alone readiness refuses it. + assert_eq!( + Some(build("mainline", "for-next", "2026-08-01T18:10:00Z")), + store.build_record("mainline", "for-next").unwrap() + ); + assert_eq!( + Some(failed), + store.build_record("mainline", "patchset-x").unwrap() + ); + assert_eq!(None, store.build_record("mainline", "master").unwrap()); + + fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn build_record_with_same_tree_and_branch_overwrites() { + let dir = tmp_dir("build-overwrite"); + let store = store_at(&dir); + + store.record_build(build("mainline", "for-next", "2026-08-01T18:10:00Z")).unwrap(); + store.record_build(build("mainline", "for-next", "2026-08-02T18:10:00Z")).unwrap(); + + assert_eq!( + Some(build("mainline", "for-next", "2026-08-02T18:10:00Z")), + store.build_record("mainline", "for-next").unwrap() + ); + + fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn build_records_coexist_across_branches_and_trees() { + let dir = tmp_dir("build-multi"); + let store = store_at(&dir); + + store.record_build(build("mainline", "for-next", "2026-08-01T18:10:00Z")).unwrap(); + store.record_build(build("mainline", "patchset-x", "2026-08-02T18:10:00Z")).unwrap(); + store.record_build(build("stable", "for-next", "2026-08-03T18:10:00Z")).unwrap(); + + assert_eq!( + Some(build("mainline", "for-next", "2026-08-01T18:10:00Z")), + store.build_record("mainline", "for-next").unwrap() + ); + assert_eq!( + Some(build("mainline", "patchset-x", "2026-08-02T18:10:00Z")), + store.build_record("mainline", "patchset-x").unwrap() + ); + assert_eq!( + Some(build("stable", "for-next", "2026-08-03T18:10:00Z")), + store.build_record("stable", "for-next").unwrap() + ); + + fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn latest_build_record_picks_newest_across_branches() { + let dir = tmp_dir("build-latest"); + let store = store_at(&dir); + + store.record_build(build("mainline", "for-next", "2026-08-01T18:10:00Z")).unwrap(); + store.record_build(build("mainline", "patchset-x", "2026-08-03T18:10:00Z")).unwrap(); + store.record_build(build("mainline", "master", "2026-08-02T18:10:00Z")).unwrap(); + // Unparseable timestamps sort oldest. + store.record_build(build("mainline", "broken-ts", "not a timestamp")).unwrap(); + + assert_eq!( + Some(build("mainline", "patchset-x", "2026-08-03T18:10:00Z")), + store.latest_build_record("mainline").unwrap() + ); + assert_eq!(None, store.latest_build_record("amd-gfx").unwrap()); + + fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn missing_build_history_reads_as_empty() { + let dir = tmp_dir("build-missing"); + let store = store_at(&dir); + + assert_eq!(None, store.build_record("mainline", "for-next").unwrap()); + assert_eq!(None, store.latest_build_record("mainline").unwrap()); + + fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn corrupt_build_history_errors_instead_of_clobbering() { + let dir = tmp_dir("build-corrupt"); + let store = store_at(&dir); + fs::write(dir.join(BUILD_HISTORY_FILENAME), b"not json").unwrap(); + + let err = store.build_record("mainline", "for-next").unwrap_err(); + assert!(err.to_string().contains(BUILD_HISTORY_FILENAME)); + assert!(store + .record_build(build("mainline", "for-next", "2026-08-01T18:10:00Z")) + .is_err()); + assert_eq!( + "not json", + fs::read_to_string(dir.join(BUILD_HISTORY_FILENAME)).unwrap() + ); + + fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn build_atomic_write_leaves_no_tmp_file() { + let dir = tmp_dir("build-atomic"); + let store = store_at(&dir); + + store.record_build(build("mainline", "for-next", "2026-08-01T18:10:00Z")).unwrap(); + + let tmp_left = fs::read_dir(&dir).unwrap().any(|e| { + e.ok() + .is_some_and(|x| x.file_name().to_string_lossy().ends_with(".tmp")) + }); + assert!(!tmp_left, "atomic write should rename away .tmp"); + + fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn apply_and_build_histories_are_independent_files() { + let dir = tmp_dir("independent"); + let store = store_at(&dir); + + store + .record_apply(record("msg-1", "mainline", "patchset-x")) + .unwrap(); + assert!(!dir.join(BUILD_HISTORY_FILENAME).exists()); + assert_eq!(None, store.build_record("mainline", "patchset-x").unwrap()); + + store.record_build(build("mainline", "patchset-x", "2026-08-01T18:10:00Z")).unwrap(); + assert!(dir.join(APPLY_HISTORY_FILENAME).exists()); + assert!(dir.join(BUILD_HISTORY_FILENAME).exists()); + assert_eq!( + Some(record("msg-1", "mainline", "patchset-x")), + store.apply_record("msg-1", "mainline").unwrap() + ); + + fs::remove_dir_all(&dir).unwrap(); + } } diff --git a/src/main.rs b/src/main.rs index 8accbadb..90c880be 100644 --- a/src/main.rs +++ b/src/main.rs @@ -25,7 +25,7 @@ use infrastructure::{ terminal::init, }; use input::{actor::InputActor, event::InputEvent}; -use kw::history::{FileKwHistoryStore, KwHistoryStore, APPLY_HISTORY_FILENAME}; +use kw::history::{FileKwHistoryStore, KwHistoryStore}; use lore::{ application::{actor::LoreApiActor, cache::CacheTtl, service::LoreService}, infrastructure::{ @@ -98,7 +98,7 @@ async fn main() -> Result<()> { let parser = Arc::new(MboxPatchsetParser::new(fs_arc.clone())); let kw_history: Arc = Arc::new(FileKwHistoryStore::new( fs_arc.clone(), - format!("{}/{}", config.data_dir(), APPLY_HISTORY_FILENAME), + config.data_dir().to_string(), )); let render = RenderActor::spawn(Box::new(ShellRenderService::new(shell_arc.clone()))); From 52a3b3e6a7607ceb5c268d18237dd4c29bf50cf1 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Thu, 20 Aug 2026 11:11:12 -0300 Subject: [PATCH 5/9] feat(kw): add kw binary probe and readiness aggregation This commit composes the tree, image, history, and kw-binary probes into a single readiness snapshot. The version floor is advisory because kw's shipped VERSION file is stale; deploy-alone refuses unless a successful build record still matches the current tree, HEAD, env, and image. This commit is part of the kw integration's step 3. Signed-off-by: lorenzoberts --- src/kw/readiness.rs | 510 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 510 insertions(+) diff --git a/src/kw/readiness.rs b/src/kw/readiness.rs index 3e6197ee..9b3852e3 100644 --- a/src/kw/readiness.rs +++ b/src/kw/readiness.rs @@ -20,7 +20,9 @@ use std::{ use crate::infrastructure::{ env::{EnvError, EnvTrait}, file_system::{FileSystemError, FileSystemTrait}, + shell::{ShellCommand, ShellTrait}, }; +use crate::{config::KernelTree, kw::history::{KwBuildRecord, KwHistoryStore}}; /// Errors from readiness probes for states where "absent" is not a normal /// situation (unlike a missing `.kw` dir, which is a readiness verdict). @@ -261,6 +263,217 @@ fn image_mtime(fs: &dyn FileSystemTrait, path: &Path) -> SystemTime { .unwrap_or(SystemTime::UNIX_EPOCH) } +/// Minimum kw version this integration is verified against. +pub const KW_MIN_VERSION: (u32, u32) = (0, 10); + +/// Result of comparing the version kw reports against [`KW_MIN_VERSION`]. +/// +/// Advisory only: kw's shipped VERSION file is stale (it reads `beta-0.9` +/// even at the 0.10 tag), so `Below` can fire on a genuinely recent kw and +/// must never gate functionality — the raw line is carried verbatim so the +/// UI can show exactly what kw reported. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum KwVersionCheck { + Meets, + Below(String), + /// The version output could not be obtained or parsed. + Unknown, +} + +/// Probe of the kw binary on `PATH`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct KwBinaryProbe { + pub available: bool, + /// First line of `kw --version` output, verbatim. + pub version_line: Option, + pub check: KwVersionCheck, +} + +/// Probes for the kw binary (`which kw`) and, when present, its version +/// (`kw --version`, whose first line is the version string; repo-mode and +/// installed kw both print `Branch:`/`Commit:` lines after it). +// Wired into startup checks in a later step; kept per the CachePolicy +// precedent (src/lore/application/cache.rs). +#[allow(dead_code)] +pub fn probe_kw_binary(env: &dyn EnvTrait, shell: &dyn ShellTrait) -> KwBinaryProbe { + if !env.which("kw") { + return KwBinaryProbe { + available: false, + version_line: None, + check: KwVersionCheck::Unknown, + }; + } + let version_line = shell + .execute(&ShellCommand::new("kw").arg("--version")) + .ok() + .filter(|out| out.success) + .and_then(|out| String::from_utf8(out.stdout).ok()) + .and_then(|stdout| stdout.lines().next().map(str::to_string)) + .filter(|line| !line.is_empty()); + let check = match &version_line { + Some(line) => check_kw_version(line), + None => KwVersionCheck::Unknown, + }; + KwBinaryProbe { + available: true, + version_line, + check, + } +} + +fn check_kw_version(version_line: &str) -> KwVersionCheck { + match parse_kw_version(version_line) { + Some(version) if version >= KW_MIN_VERSION => KwVersionCheck::Meets, + Some(_) => KwVersionCheck::Below(version_line.to_string()), + None => KwVersionCheck::Unknown, + } +} + +/// Extracts the first `X.Y[.Z]` pair from a version line: `0.10.0` and the +/// stale `beta-0.9` kw currently ships both parse. +fn parse_kw_version(line: &str) -> Option<(u32, u32)> { + let start = line.find(|c: char| c.is_ascii_digit())?; + let mut parts = line[start..].splitn(3, '.'); + let major = parts.next()?.parse().ok()?; + let minor: String = parts + .next()? + .chars() + .take_while(|c| c.is_ascii_digit()) + .collect(); + Some((major, minor.parse().ok()?)) +} + +/// Why a deploy-without-build was refused (integration plan §2.1d). Each +/// variant's message is the actionable explanation KwOps shows. +#[derive(Debug, Clone, PartialEq, Eq, Error)] +pub enum DeployAloneRefusal { + #[error("no build recorded for this tree and branch; run a build first")] + NoBuildRecord, + #[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" + )] + HeadMismatch { recorded: String, current: String }, + #[error( + "the kernel tree moved from '{recorded}' to '{current}' since the last build; \ + rebuild before deploying" + )] + TreePathDrift { recorded: String, current: String }, + #[error( + "the last build ran with a different kw env (O=) than the active one; \ + rebuild in the active env before deploying" + )] + OutputDirMismatch, + #[error("no kernel image (*Image) found under arch/*/boot; rebuild before deploying")] + ImageMissing, +} + +/// Deploy-alone readiness gate (integration plan §2.1d steps 1–4): 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. +// Consumed by KwActor deploy in a later step; kept per the CachePolicy +// precedent (src/lore/application/cache.rs). +#[allow(dead_code)] +pub fn check_deploy_alone( + record: Option<&KwBuildRecord>, + tree: &KernelTree, + head_branch: &str, + output_dir: Option<&Path>, + image: Option<&Path>, +) -> Result<(), DeployAloneRefusal> { + let record = record.ok_or(DeployAloneRefusal::NoBuildRecord)?; + if !record.success { + return Err(DeployAloneRefusal::LastBuildFailed); + } + if record.branch != head_branch { + return Err(DeployAloneRefusal::HeadMismatch { + recorded: record.branch.clone(), + current: head_branch.to_string(), + }); + } + if record.tree_path.as_str() != tree.path().as_str() { + return Err(DeployAloneRefusal::TreePathDrift { + recorded: record.tree_path.clone(), + current: tree.path().to_string(), + }); + } + let current_output_dir = output_dir.map(|p| p.to_string_lossy().into_owned()); + if record.output_dir != current_output_dir { + return Err(DeployAloneRefusal::OutputDirMismatch); + } + if image.is_none() { + return Err(DeployAloneRefusal::ImageMissing); + } + Ok(()) +} + +/// Snapshot of everything KwOps needs to decide whether build/deploy can +/// start, and why not — returned by KwActor's `GetReadiness` (§2.3). +// Assembled by KwActor in a later step; kept per the CachePolicy precedent +// (src/lore/application/cache.rs). +#[allow(dead_code)] +#[derive(Debug, Clone)] +pub struct KwReadiness { + pub kw_binary: KwBinaryProbe, + pub tree: TreeReadiness, + /// Active kw env's `O=` dir, if any. + 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. + pub build_record: Option, + pub deploy_alone: Result<(), DeployAloneRefusal>, +} + +/// 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. +// Composed by KwActor in a later step; kept per the CachePolicy precedent +// (src/lore/application/cache.rs). +#[allow(dead_code)] +pub fn evaluate_readiness( + fs: &dyn FileSystemTrait, + env: &dyn EnvTrait, + shell: &dyn ShellTrait, + history: &dyn KwHistoryStore, + kernel_tree_id: &str, + tree: &KernelTree, + head_branch: &str, +) -> Result { + let tree_path = Path::new(tree.path()); + let kw_binary = probe_kw_binary(env, shell); + let output_dir = resolve_output_dir(fs, env, tree_path)?; + let tree_status = probe_tree(fs, tree_path, output_dir.as_deref()); + let arch = match &tree_status { + TreeReadiness::Ready { arch } => arch.clone(), + _ => None, + }; + let kernel_image = find_newest_kernel_image( + fs, + output_dir.as_deref().unwrap_or(tree_path), + arch.as_deref(), + ); + let build_record = history.build_record(kernel_tree_id, head_branch)?; + let deploy_alone = check_deploy_alone( + build_record.as_ref(), + tree, + head_branch, + output_dir.as_deref(), + kernel_image.as_deref(), + ); + Ok(KwReadiness { + kw_binary, + tree: tree_status, + output_dir, + kernel_image, + build_record, + deploy_alone, + }) +} + #[cfg(test)] mod tests { use std::{ @@ -273,10 +486,14 @@ mod tests { use crate::infrastructure::{ env::MockEnvTrait, file_system::{MockFileSystemTrait, OsFileSystem}, + shell::{MockShellTrait, ShellOutput}, }; + use crate::kw::history::FileKwHistoryStore; use super::*; + use std::sync::Arc; + static TEST_SEQ: AtomicU64 = AtomicU64::new(0); struct TempDir(PathBuf); @@ -728,4 +945,297 @@ last_line_without_newline=yes"; find_newest_kernel_image(&OsFileSystem, dir.path(), Some("x86")) ); } + + fn shell_output(stdout: &str, success: bool) -> ShellOutput { + ShellOutput { + stdout: stdout.as_bytes().to_vec(), + stderr: Vec::new(), + success, + } + } + + fn kernel_tree(path: &Path) -> KernelTree { + serde_json::from_value(serde_json::json!({ + "path": path.to_str().unwrap(), + "branch": "master" + })) + .unwrap() + } + + fn built_record(tree: &Path, branch: &str) -> KwBuildRecord { + KwBuildRecord { + kernel_tree_id: "mainline".to_string(), + tree_path: tree.to_str().unwrap().to_string(), + message_id: None, + branch: branch.to_string(), + arch: Some("x86".to_string()), + image_path: None, + output_dir: None, + kernelrelease: None, + log_path: String::new(), + built_at: "2026-08-01T18:10:00Z".to_string(), + success: true, + } + } + + #[test] + fn kw_probe_missing_binary_never_spawns() { + let mut env = MockEnvTrait::new(); + env.expect_which() + .withf(|name| name == "kw") + .returning(|_| false); + let mut shell = MockShellTrait::new(); + shell.expect_execute().times(0); + + let probe = probe_kw_binary(&env, &shell); + + assert!(!probe.available); + assert_eq!(None, probe.version_line); + assert_eq!(KwVersionCheck::Unknown, probe.check); + } + + #[test] + fn kw_probe_compares_version_against_floor() { + for (stdout, expected) in [ + ("0.10.0\n", KwVersionCheck::Meets), + ("0.10\n", KwVersionCheck::Meets), + ("1.0\n", KwVersionCheck::Meets), + ("0.9.9\n", KwVersionCheck::Below("0.9.9".to_string())), + // What a real 0.10 install prints: kw's shipped VERSION is stale. + ( + "beta-0.9\nBranch: master\nCommit: 3575d38\n", + KwVersionCheck::Below("beta-0.9".to_string()), + ), + ("not a version\n", KwVersionCheck::Unknown), + ] { + let mut env = MockEnvTrait::new(); + env.expect_which().returning(|_| true); + let mut shell = MockShellTrait::new(); + let stdout_bytes = stdout.as_bytes().to_vec(); + shell + .expect_execute() + .withf(|cmd| cmd.program == "kw" && cmd.args == ["--version"]) + .returning(move |_| Ok(shell_output(str::from_utf8(&stdout_bytes).unwrap(), true))); + + let probe = probe_kw_binary(&env, &shell); + + assert!(probe.available, "for version output {stdout:?}"); + assert_eq!(expected, probe.check, "for version output {stdout:?}"); + } + } + + #[test] + fn kw_probe_keeps_the_raw_first_line_verbatim() { + let mut env = MockEnvTrait::new(); + env.expect_which().returning(|_| true); + let mut shell = MockShellTrait::new(); + shell.expect_execute().returning(|_| { + Ok(shell_output( + "beta-0.9\nBranch: master\nCommit: 3575d38\n", + true, + )) + }); + + let probe = probe_kw_binary(&env, &shell); + + assert_eq!(Some("beta-0.9".to_string()), probe.version_line); + } + + #[test] + fn kw_probe_unknown_when_version_is_unreadable() { + // kw is on PATH but its --version output cannot be trusted. + let mut spawn_fails = MockShellTrait::new(); + spawn_fails + .expect_execute() + .returning(|_| Err(std::io::Error::other("spawn failed").into())); + + let mut empty_stdout = MockShellTrait::new(); + empty_stdout + .expect_execute() + .returning(|_| Ok(shell_output("", true))); + + let mut kw_fails = MockShellTrait::new(); + kw_fails + .expect_execute() + .returning(|_| Ok(shell_output("0.10.0\n", false))); + + for shell in [spawn_fails, empty_stdout, kw_fails] { + let mut env = MockEnvTrait::new(); + env.expect_which().returning(|_| true); + + let probe = probe_kw_binary(&env, &shell); + + assert!(probe.available); + assert_eq!(None, probe.version_line); + assert_eq!(KwVersionCheck::Unknown, probe.check); + } + } + + #[test] + fn deploy_alone_requires_a_successful_build_record() { + let dir = make_ready_tree("deploy-gate"); + let tree = kernel_tree(dir.path()); + let image = dir.path().join("arch/x86/boot/bzImage"); + + assert_eq!( + Err(DeployAloneRefusal::NoBuildRecord), + check_deploy_alone(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)) + ); + } + + #[test] + fn deploy_alone_refuses_frankenstein_combinations() { + let dir = make_ready_tree("deploy-frankenstein"); + let tree = kernel_tree(dir.path()); + 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. + assert_eq!( + Err(DeployAloneRefusal::HeadMismatch { + recorded: "patchset-x".to_string(), + current: "master".to_string(), + }), + check_deploy_alone(Some(&record), &tree, "master", None, Some(&image)) + ); + + // The config repointed the same tree id at another path. + let moved_tree = kernel_tree(Path::new("/elsewhere/linux")); + assert_eq!( + Err(DeployAloneRefusal::TreePathDrift { + 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)) + ); + + // The active kw env changed since the build. + assert_eq!( + Err(DeployAloneRefusal::OutputDirMismatch), + check_deploy_alone( + Some(&record), + &tree, + "patchset-x", + Some(Path::new("/cache/kw/envs/xyz/minix")), + Some(&image), + ) + ); + + // The image the build produced is gone. + assert_eq!( + Err(DeployAloneRefusal::ImageMissing), + check_deploy_alone(Some(&record), &tree, "patchset-x", None, None) + ); + + assert_eq!( + Ok(()), + check_deploy_alone(Some(&record), &tree, "patchset-x", None, Some(&image)) + ); + } + + #[test] + fn deploy_alone_checks_failure_before_branch_mismatch() { + let dir = make_ready_tree("deploy-order"); + let tree = kernel_tree(dir.path()); + let mut failed = built_record(dir.path(), "patchset-x"); + failed.success = false; + + assert_eq!( + Err(DeployAloneRefusal::LastBuildFailed), + check_deploy_alone(Some(&failed), &tree, "master", None, None) + ); + } + + #[test] + fn evaluate_readiness_composes_all_probes() { + let dir = make_ready_tree("evaluate"); + fs::write(dir.path().join(".kw/build.config"), "arch=x86\n").unwrap(); + let boot = dir.path().join("arch/x86/boot"); + fs::create_dir_all(&boot).unwrap(); + write_file_with_mtime(&boot.join("bzImage"), 100); + + let data = TempDir::new("evaluate-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 mut env = MockEnvTrait::new(); + env.expect_which().returning(|_| true); + let mut shell = MockShellTrait::new(); + shell + .expect_execute() + .returning(|_| Ok(shell_output("0.10.0\n", true))); + + let tree = kernel_tree(dir.path()); + let readiness = evaluate_readiness( + &OsFileSystem, + &env, + &shell, + &history, + "mainline", + &tree, + "patchset-x", + ) + .unwrap(); + + assert_eq!( + TreeReadiness::Ready { + arch: Some("x86".to_string()) + }, + readiness.tree + ); + assert_eq!(Some(boot.join("bzImage")), readiness.kernel_image); + assert_eq!(Some(record), readiness.build_record); + assert_eq!(Ok(()), readiness.deploy_alone); + assert_eq!(None, readiness.output_dir); + assert!(readiness.kw_binary.available); + assert_eq!(KwVersionCheck::Meets, readiness.kw_binary.check); + } + + #[test] + fn evaluate_readiness_without_build_refuses_deploy_alone() { + let dir = make_ready_tree("evaluate-nobuild"); + let data = TempDir::new("evaluate-nobuild-data"); + let history = FileKwHistoryStore::new( + Arc::new(OsFileSystem), + data.path().to_str().unwrap().to_string(), + ); + + let mut env = MockEnvTrait::new(); + env.expect_which().returning(|_| false); + 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, + "patchset-x", + ) + .unwrap(); + + // No build.config: arch stays None (glob fallback); no images exist. + assert_eq!(TreeReadiness::Ready { arch: None }, readiness.tree); + assert_eq!(None, readiness.kernel_image); + assert_eq!(None, readiness.build_record); + assert_eq!( + Err(DeployAloneRefusal::NoBuildRecord), + readiness.deploy_alone + ); + assert!(!readiness.kw_binary.available); + } } From 2de4921c8136ffec7994aba7988942dbf3de2780 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Thu, 20 Aug 2026 11:15:27 -0300 Subject: [PATCH 6/9] feat(app): warn at startup when kw is missing or unverifiable This commit adds a soft startup warning when kw is missing or its version cannot be confirmed as recent enough. Lore review still works without kw; the check never blocks launch, matching the advisory version policy. This commit is part of the kw integration's step 3. Signed-off-by: lorenzoberts --- src/app/dependencies.rs | 101 +++++++++++++++++++++++++++++++++------- src/kw/readiness.rs | 3 -- src/main.rs | 2 +- 3 files changed, 85 insertions(+), 21 deletions(-) diff --git a/src/app/dependencies.rs b/src/app/dependencies.rs index 3a074fc6..1a9f10f1 100644 --- a/src/app/dependencies.rs +++ b/src/app/dependencies.rs @@ -1,16 +1,23 @@ use tracing::{event, Level}; use crate::{ - app::errors::AppError, config::ConfigSnapshot, infrastructure::env::EnvTrait, + app::errors::AppError, + config::ConfigSnapshot, + infrastructure::{env::EnvTrait, shell::ShellTrait}, + kw::readiness::{KwVersionCheck, probe_kw_binary}, render_prefs::PatchRenderer, }; /// Verifies required and optional external binaries before the terminal starts. /// /// A missing `b4` is a hard failure; all other missing binaries only emit -/// warnings. This keeps fatal startup failures out of terminal raw mode. +/// warnings — including `kw`, and including an unverifiable kw version, +/// since kw's own VERSION file is stale upstream (it reports `beta-0.9` +/// even at the 0.10 tag). This keeps fatal startup failures out of +/// terminal raw mode. pub(crate) fn check_external_deps( env: &dyn EnvTrait, + shell: &dyn ShellTrait, config: &ConfigSnapshot, ) -> Result<(), AppError> { if !env.which("b4") { @@ -55,6 +62,21 @@ pub(crate) fn check_external_deps( _ => {} } + let kw = probe_kw_binary(env, shell); + if !kw.available { + event!( + Level::WARN, + "kw is not installed, kernel build/deploy won't work" + ); + } else if !matches!(kw.check, KwVersionCheck::Meets) { + event!( + Level::WARN, + version = kw.version_line.as_deref().unwrap_or("unknown"), + "could not confirm kw >= 0.10; the build/deploy integration is \ + verified against kw 0.10 (kw's own VERSION file may be stale)" + ); + } + Ok(()) } @@ -62,35 +84,54 @@ pub(crate) fn check_external_deps( mod tests { use crate::{ config::{ConfigState, ValidatedConfigUpdate}, - infrastructure::env::MockEnvTrait, + infrastructure::{ + env::MockEnvTrait, + shell::{MockShellTrait, ShellOutput}, + }, render_prefs::PatchRenderer, }; use super::*; + /// An env where every binary is present and kw reports a current + /// version, so individual tests only need to override their own case. + fn happy_env() -> (MockEnvTrait, MockShellTrait) { + let mut env = MockEnvTrait::new(); + env.expect_which().returning(|_| true); + let mut shell = MockShellTrait::new(); + shell.expect_execute().returning(|_| { + Ok(ShellOutput { + stdout: b"0.10.0\n".to_vec(), + stderr: Vec::new(), + success: true, + }) + }); + (env, shell) + } + #[test] fn missing_b4_returns_dependencies_error() { let mut env = MockEnvTrait::new(); env.expect_which() .withf(|name| name == "b4") .returning(|_| false); + let mut shell = MockShellTrait::new(); + shell.expect_execute().times(0); - let err = check_external_deps(&env, &ConfigState::default().to_snapshot()).unwrap_err(); + let err = check_external_deps(&env, &shell, &ConfigState::default().to_snapshot()) + .unwrap_err(); assert!(matches!(err, AppError::Dependencies(_))); } #[test] fn missing_git_is_not_fatal() { - let mut env = MockEnvTrait::new(); - env.expect_which() - .withf(|name| name == "b4") - .returning(|_| true); + let (mut env, shell) = happy_env(); env.expect_which() .withf(|name| name == "git") .returning(|_| false); - let result = check_external_deps(&env, &ConfigState::default().to_snapshot()); + let result = check_external_deps(&env, &shell, &ConfigState::default().to_snapshot()); assert!(result.is_ok()); } @@ -103,18 +144,44 @@ mod tests { ..Default::default() }); - let mut env = MockEnvTrait::new(); - env.expect_which() - .withf(|name| name == "b4") - .returning(|_| true); - env.expect_which() - .withf(|name| name == "git") - .returning(|_| true); + let (mut env, shell) = happy_env(); env.expect_which() .withf(|name| name == "bat") .returning(|_| false); - let result = check_external_deps(&env, &state.to_snapshot()); + let result = check_external_deps(&env, &shell, &state.to_snapshot()); + + assert!(result.is_ok()); + } + + #[test] + fn missing_kw_is_not_fatal() { + let (mut env, mut shell) = happy_env(); + env.expect_which() + .withf(|name| name == "kw") + .returning(|_| false); + // No kw on PATH: the version probe must not spawn anything. + shell.expect_execute().times(0); + + let result = check_external_deps(&env, &shell, &ConfigState::default().to_snapshot()); + + assert!(result.is_ok()); + } + + #[test] + fn unverifiable_kw_version_is_not_fatal() { + let (env, mut shell) = happy_env(); + // Real 0.10 installs can still report the stale beta-0.9: the floor + // check stays a warning regardless of what kw answers. + shell.expect_execute().returning(|_| { + Ok(ShellOutput { + stdout: b"beta-0.9\nBranch: master\nCommit: 3575d38\n".to_vec(), + stderr: Vec::new(), + success: true, + }) + }); + + let result = check_external_deps(&env, &shell, &ConfigState::default().to_snapshot()); assert!(result.is_ok()); } diff --git a/src/kw/readiness.rs b/src/kw/readiness.rs index 9b3852e3..b2fed846 100644 --- a/src/kw/readiness.rs +++ b/src/kw/readiness.rs @@ -292,9 +292,6 @@ pub struct KwBinaryProbe { /// Probes for the kw binary (`which kw`) and, when present, its version /// (`kw --version`, whose first line is the version string; repo-mode and /// installed kw both print `Branch:`/`Commit:` lines after it). -// Wired into startup checks in a later step; kept per the CachePolicy -// precedent (src/lore/application/cache.rs). -#[allow(dead_code)] pub fn probe_kw_binary(env: &dyn EnvTrait, shell: &dyn ShellTrait) -> KwBinaryProbe { if !env.which("kw") { return KwBinaryProbe { diff --git a/src/main.rs b/src/main.rs index 90c880be..fb3e07b4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -72,7 +72,7 @@ async fn main() -> Result<()> { ControlFlow::Continue(()) => {} } - check_external_deps(&env, &config)?; + check_external_deps(&env, &OsShell, &config)?; let config_handle = ConfigActor::spawn(config_state, config_repo); let terminal_handle = TerminalActor::spawn(Box::new(CrosstermTerminalSession::new(init()?))); From 7ac3294c76df824e7f02b82d24571179da24f4fc Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Thu, 20 Aug 2026 11:49:06 -0300 Subject: [PATCH 7/9] fix(kw): treat empty XDG_CACHE_HOME as unset and close deploy-alone gaps This commit fixes env output-dir resolution when XDG_CACHE_HOME is set but empty, which previously produced a cwd-relative path and poisoned the following probes. Deploy-alone now also requires the tree to still be ready, and the snapshot carries the latest build across branches. This commit is part of the kw integration's step 3. Signed-off-by: lorenzoberts --- src/app/dependencies.rs | 6 +- src/kw/history.rs | 52 ++++++--- src/kw/readiness.rs | 245 ++++++++++++++++++++++++++++++++-------- 3 files changed, 239 insertions(+), 64 deletions(-) diff --git a/src/app/dependencies.rs b/src/app/dependencies.rs index 1a9f10f1..7402e4cb 100644 --- a/src/app/dependencies.rs +++ b/src/app/dependencies.rs @@ -4,7 +4,7 @@ use crate::{ app::errors::AppError, config::ConfigSnapshot, infrastructure::{env::EnvTrait, shell::ShellTrait}, - kw::readiness::{KwVersionCheck, probe_kw_binary}, + kw::readiness::{probe_kw_binary, KwVersionCheck}, render_prefs::PatchRenderer, }; @@ -118,8 +118,8 @@ mod tests { let mut shell = MockShellTrait::new(); shell.expect_execute().times(0); - let err = check_external_deps(&env, &shell, &ConfigState::default().to_snapshot()) - .unwrap_err(); + let err = + check_external_deps(&env, &shell, &ConfigState::default().to_snapshot()).unwrap_err(); assert!(matches!(err, AppError::Dependencies(_))); } diff --git a/src/kw/history.rs b/src/kw/history.rs index 35154925..2a8b9ea4 100644 --- a/src/kw/history.rs +++ b/src/kw/history.rs @@ -280,9 +280,7 @@ mod tests { message_id: None, branch: branch.to_string(), arch: Some("x86".to_string()), - image_path: Some(format!( - "/home/user/{kernel_tree_id}/arch/x86/boot/bzImage" - )), + image_path: Some(format!("/home/user/{kernel_tree_id}/arch/x86/boot/bzImage")), output_dir: None, kernelrelease: Some("6.17.0".to_string()), log_path: "/home/user/.cache/patch_hub/kw_logs/build-1.log".to_string(), @@ -438,7 +436,9 @@ mod tests { let dir = tmp_dir("build-round-trip"); let store = store_at(&dir); - store.record_build(build("mainline", "for-next", "2026-08-01T18:10:00Z")).unwrap(); + store + .record_build(build("mainline", "for-next", "2026-08-01T18:10:00Z")) + .unwrap(); let mut failed = build("mainline", "patchset-x", "2026-08-02T09:00:00Z"); failed.success = false; failed.image_path = None; @@ -465,8 +465,12 @@ mod tests { let dir = tmp_dir("build-overwrite"); let store = store_at(&dir); - store.record_build(build("mainline", "for-next", "2026-08-01T18:10:00Z")).unwrap(); - store.record_build(build("mainline", "for-next", "2026-08-02T18:10:00Z")).unwrap(); + store + .record_build(build("mainline", "for-next", "2026-08-01T18:10:00Z")) + .unwrap(); + store + .record_build(build("mainline", "for-next", "2026-08-02T18:10:00Z")) + .unwrap(); assert_eq!( Some(build("mainline", "for-next", "2026-08-02T18:10:00Z")), @@ -481,9 +485,15 @@ mod tests { let dir = tmp_dir("build-multi"); let store = store_at(&dir); - store.record_build(build("mainline", "for-next", "2026-08-01T18:10:00Z")).unwrap(); - store.record_build(build("mainline", "patchset-x", "2026-08-02T18:10:00Z")).unwrap(); - store.record_build(build("stable", "for-next", "2026-08-03T18:10:00Z")).unwrap(); + store + .record_build(build("mainline", "for-next", "2026-08-01T18:10:00Z")) + .unwrap(); + store + .record_build(build("mainline", "patchset-x", "2026-08-02T18:10:00Z")) + .unwrap(); + store + .record_build(build("stable", "for-next", "2026-08-03T18:10:00Z")) + .unwrap(); assert_eq!( Some(build("mainline", "for-next", "2026-08-01T18:10:00Z")), @@ -506,11 +516,19 @@ mod tests { let dir = tmp_dir("build-latest"); let store = store_at(&dir); - store.record_build(build("mainline", "for-next", "2026-08-01T18:10:00Z")).unwrap(); - store.record_build(build("mainline", "patchset-x", "2026-08-03T18:10:00Z")).unwrap(); - store.record_build(build("mainline", "master", "2026-08-02T18:10:00Z")).unwrap(); + store + .record_build(build("mainline", "for-next", "2026-08-01T18:10:00Z")) + .unwrap(); + store + .record_build(build("mainline", "patchset-x", "2026-08-03T18:10:00Z")) + .unwrap(); + store + .record_build(build("mainline", "master", "2026-08-02T18:10:00Z")) + .unwrap(); // Unparseable timestamps sort oldest. - store.record_build(build("mainline", "broken-ts", "not a timestamp")).unwrap(); + store + .record_build(build("mainline", "broken-ts", "not a timestamp")) + .unwrap(); assert_eq!( Some(build("mainline", "patchset-x", "2026-08-03T18:10:00Z")), @@ -556,7 +574,9 @@ mod tests { let dir = tmp_dir("build-atomic"); let store = store_at(&dir); - store.record_build(build("mainline", "for-next", "2026-08-01T18:10:00Z")).unwrap(); + store + .record_build(build("mainline", "for-next", "2026-08-01T18:10:00Z")) + .unwrap(); let tmp_left = fs::read_dir(&dir).unwrap().any(|e| { e.ok() @@ -578,7 +598,9 @@ mod tests { assert!(!dir.join(BUILD_HISTORY_FILENAME).exists()); assert_eq!(None, store.build_record("mainline", "patchset-x").unwrap()); - store.record_build(build("mainline", "patchset-x", "2026-08-01T18:10:00Z")).unwrap(); + store + .record_build(build("mainline", "patchset-x", "2026-08-01T18:10:00Z")) + .unwrap(); assert!(dir.join(APPLY_HISTORY_FILENAME).exists()); assert!(dir.join(BUILD_HISTORY_FILENAME).exists()); assert_eq!( diff --git a/src/kw/readiness.rs b/src/kw/readiness.rs index b2fed846..f8652613 100644 --- a/src/kw/readiness.rs +++ b/src/kw/readiness.rs @@ -1,14 +1,17 @@ //! Readiness probes for running `kw build` / `kw deploy` on a configured //! kernel tree. //! -//! Each probe mirrors the corresponding discovery logic in kw itself +//! Most probes mirror the corresponding discovery logic in kw itself //! (`src/lib/kwlib.sh`, `src/lib/kw_config_loader.sh`, `src/deploy.sh` at //! kw 0.10) so patch-hub's idea of "ready" matches what kw will actually //! do, instead of being a parallel interpretation that can silently drift -//! from it. The probes are pure functions over injected infrastructure -//! traits; KwActor composes them into the `GetReadiness` snapshot. +//! from it. The deliberate divergences — the `arch`-unset glob fallback +//! and the non-recursive boot-dir scan — are documented on +//! [`find_newest_kernel_image`]. The probes are pure functions over +//! injected infrastructure traits; KwActor composes them into the +//! `GetReadiness` snapshot. -use base64::{Engine as _, engine::general_purpose::STANDARD as BASE64}; +use base64::{engine::general_purpose::STANDARD as BASE64, Engine as _}; use thiserror::Error; use std::{ @@ -22,7 +25,10 @@ use crate::infrastructure::{ file_system::{FileSystemError, FileSystemTrait}, shell::{ShellCommand, ShellTrait}, }; -use crate::{config::KernelTree, kw::history::{KwBuildRecord, KwHistoryStore}}; +use crate::{ + config::KernelTree, + kw::history::{KwBuildRecord, KwHistoryStore}, +}; /// Errors from readiness probes for states where "absent" is not a normal /// situation (unlike a missing `.kw` dir, which is a readiness verdict). @@ -38,9 +44,10 @@ pub enum KwReadinessError { #[derive(Debug, Clone, PartialEq, Eq)] pub enum TreeReadiness { /// Kernel root, kw-initialized, with a `.config`. `arch` is the literal - /// `arch=` value from `.kw/build.config`; `None` means image discovery - /// must glob `arch/*/boot/`, the fallback kw's own `arch=` resolution - /// effectively produces when the key is unset. + /// `arch=` value from `.kw/build.config`; `None` means the key is unset + /// and image discovery will glob `arch/*/boot/` instead — a deliberate + /// divergence from kw, whose own fallback is the merged kw-config + /// `arch` (see [`find_newest_kernel_image`]). Ready { arch: Option }, /// The configured path is not a directory. Missing, @@ -104,7 +111,9 @@ pub fn is_kernel_root(fs: &dyn FileSystemTrait, path: &Path) -> bool { /// Probes whether `tree_path` is a kernel tree ready for kw operations. /// `output_dir` is the resolved kw-env `O=` path when an env is active: kw -/// then keeps the `.config` there instead of in the tree root. +/// refuses to activate an env while an in-tree `.config` exists +/// (`kw_env.sh::validate_env_before_switch`), so with an env active the +/// `.config` lives only at the env's `O=` dir. // No production caller until the readiness aggregation lands; kept per the // CachePolicy precedent (src/lore/application/cache.rs). #[allow(dead_code)] @@ -135,13 +144,19 @@ pub fn probe_tree( /// value kw's image discovery globs under `arch//boot/`. Returns /// `None` when the file or the key is absent or empty — kw's /// `${build_config[arch]:-...}` expansion treats empty as unset — meaning -/// the caller should fall back to globbing `arch/*/boot/`. +/// the caller falls back to globbing `arch/*/boot/`, a deliberate +/// divergence from kw's merged-config fallback (see +/// [`find_newest_kernel_image`]). pub fn read_build_arch(fs: &dyn FileSystemTrait, tree_path: &Path) -> Option { let content = fs .read_to_string(&tree_path.join(".kw").join("build.config")) .ok()?; let arch = parse_kw_config(&content).remove("arch")?; - if arch.is_empty() { None } else { Some(arch) } + if arch.is_empty() { + None + } else { + Some(arch) + } } /// Resolves kw's active build output dir (`O=`) for `tree_path`, mirroring @@ -187,8 +202,10 @@ pub fn resolve_output_dir( } let cache_base = match env.var("XDG_CACHE_HOME") { - Ok(xdg) => xdg, - Err(_) => format!("{}/.cache", env.var("HOME")?), + // bash's `:-` (and the XDG spec) treat a set-but-empty value as + // unset; env::var would happily return it as Ok(""). + Ok(xdg) if !xdg.is_empty() => xdg, + _ => format!("{}/.cache", env.var("HOME")?), }; let trimmed = tree_path.to_string_lossy(); let normalized = match trimmed.trim_end_matches('/') { @@ -205,18 +222,29 @@ pub fn resolve_output_dir( )) } -/// Finds the newest kernel image under `/arch/`, mirroring kw's -/// `get_kernel_binary_name`: a candidate's basename must end with `Image` -/// (find's `-name '*Image'` is case-sensitive, so `Image.gz` and `image` +/// Finds the newest kernel image under `/arch/`. Candidate +/// basenames must end with `Image` (the `-name '*Image'` in kw's +/// `get_kernel_binary_name` is case-sensitive, so `Image.gz` and `image` /// are excluded) and the most recently modified one wins, with ties broken -/// by descending path (kw's `sort -r | head -1`). With `arch`, only -/// `arch//boot/` is probed; without, every `arch/*/boot/` is globbed. +/// by descending path (kw's `sort -r | head -1`). +/// +/// With `arch`, only `arch//boot/` is probed — exactly kw's behavior. +/// Without `arch` this is a **deliberate divergence**, not a mirror: kw +/// falls back to the merged kw-config `arch` (packaged default `x86_64`, a +/// directory that does not exist in kernel trees, so `kw deploy` then fails +/// with exit 125), and patch-hub does not read kw's global config layers. +/// Globbing every `arch/*/boot/` gives a more useful readiness signal than +/// probing a directory that is never there — at the cost of possibly +/// reporting an image kw would not find. A green image probe with `arch=` +/// unset is therefore not a guarantee kw deploy will locate one; setting +/// `arch=` in `.kw/build.config` makes the two agree. /// -/// Deliberate deviation: kw's `find` recurses into boot/ subdirectories, -/// while this scans only the top level. Kernel images for every arch kw -/// supports are produced directly in boot/ (subdirs like compressed/ or -/// dts/ never hold `*Image` files), and find does not descend into symlinked -/// dirs either, so the behaviors agree on real trees. +/// Second deliberate deviation: kw's `find` recurses into boot/ +/// subdirectories, while this scans only the top level. Kernel images for +/// every arch kw supports are produced directly in boot/ (subdirs like +/// compressed/ or dts/ never hold `*Image` files), and find does not +/// descend into symlinked dirs either, so the behaviors agree on real +/// trees. // No production caller until the readiness aggregation lands; kept per the // CachePolicy precedent (src/lore/application/cache.rs). #[allow(dead_code)] @@ -340,10 +368,35 @@ fn parse_kw_version(line: &str) -> Option<(u32, u32)> { Some((major, minor.parse().ok()?)) } +impl std::fmt::Display for TreeReadiness { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + TreeReadiness::Ready { .. } => write!(f, "ready"), + TreeReadiness::Missing => write!(f, "the configured path is not a directory"), + TreeReadiness::NotAKernelRoot => write!( + f, + "the directory is not a kernel tree root (missing files like \ + Makefile or dirs like arch/)" + ), + TreeReadiness::MissingKwDir => { + write!(f, "kw init was never run in this tree (no .kw/ directory)") + } + TreeReadiness::MissingKernelConfig => { + write!( + f, + "no .config at the build root (the tree, or the kw env's O=)" + ) + } + } + } +} + /// Why a deploy-without-build was refused (integration plan §2.1d). Each /// variant's message is the actionable explanation KwOps shows. #[derive(Debug, Clone, PartialEq, Eq, Error)] pub enum DeployAloneRefusal { + #[error("the kernel tree is not ready: {0}")] + TreeNotReady(TreeReadiness), #[error("no build recorded for this tree and branch; run a build first")] NoBuildRecord, #[error("the last build of this branch failed; rebuild before deploying")] @@ -363,7 +416,11 @@ pub enum DeployAloneRefusal { rebuild in the active env before deploying" )] OutputDirMismatch, - #[error("no kernel image (*Image) found under arch/*/boot; rebuild before deploying")] + #[error( + "no kernel image (*Image) found under arch/*/boot; rebuild before deploying \ + (with no arch= in .kw/build.config, kw probes arch/x86_64/boot/, which \ + does not exist in kernel trees — set arch= explicitly)" + )] ImageMissing, } @@ -391,7 +448,9 @@ pub fn check_deploy_alone( current: head_branch.to_string(), }); } - if record.tree_path.as_str() != tree.path().as_str() { + // Trailing slashes are normalized away: a config edit that only adds + // or drops one does not move the tree. + if record.tree_path.trim_end_matches('/') != tree.path().trim_end_matches('/') { return Err(DeployAloneRefusal::TreePathDrift { recorded: record.tree_path.clone(), current: tree.path().to_string(), @@ -422,6 +481,11 @@ pub struct KwReadiness { pub kernel_image: Option, /// Build record for `(kernel_tree_id, head_branch)`, if any. pub build_record: Option, + /// Newest build record for the tree across branches, so KwOps can show + /// "last build was on branch X" copy even when HEAD has no record. + pub latest_build: Option, + /// `Ok(())` is a self-sufficient verdict: tree readiness is already + /// conjoined in, so a caller cannot forget to check `tree` as well. pub deploy_alone: Result<(), DeployAloneRefusal>, } @@ -454,19 +518,27 @@ pub fn evaluate_readiness( arch.as_deref(), ); let build_record = history.build_record(kernel_tree_id, head_branch)?; - let deploy_alone = check_deploy_alone( - build_record.as_ref(), - tree, - head_branch, - output_dir.as_deref(), - kernel_image.as_deref(), - ); + let latest_build = history.latest_build_record(kernel_tree_id)?; + // 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. + let deploy_alone = match &tree_status { + TreeReadiness::Ready { .. } => check_deploy_alone( + build_record.as_ref(), + tree, + head_branch, + output_dir.as_deref(), + kernel_image.as_deref(), + ), + other => Err(DeployAloneRefusal::TreeNotReady(other.clone())), + }; Ok(KwReadiness { kw_binary, tree: tree_status, output_dir, kernel_image, build_record, + latest_build, deploy_alone, }) } @@ -731,11 +803,7 @@ last_line_without_newline=yes"; assert_eq!(None, read_build_arch(&OsFileSystem, no_key.path())); let commented = make_ready_tree("arch-commented"); - fs::write( - commented.path().join(".kw/build.config"), - "#arch=riscv\n", - ) - .unwrap(); + fs::write(commented.path().join(".kw/build.config"), "#arch=riscv\n").unwrap(); assert_eq!(None, read_build_arch(&OsFileSystem, commented.path())); let empty = make_ready_tree("arch-empty"); @@ -778,8 +846,7 @@ last_line_without_newline=yes"; .withf(|key| key == "XDG_CACHE_HOME") .returning(|_| Ok("/xdg".to_string())); - let resolved = - resolve_output_dir(&fs, &env, Path::new("/home/user/linux")).unwrap(); + let resolved = resolve_output_dir(&fs, &env, Path::new("/home/user/linux")).unwrap(); assert_eq!( Some(PathBuf::from("/xdg/kw/envs/L2hvbWUvdXNlci9saW51eA==/minix")), @@ -813,6 +880,34 @@ last_line_without_newline=yes"; ); } + #[test] + fn resolve_output_dir_treats_empty_xdg_cache_home_as_unset() { + // bash's `:-` (and the XDG spec) treat set-but-empty as unset; + // otherwise the resolved path would be relative to cwd. + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_file().returning(|_| true); + fs.expect_read_to_string() + .returning(|_| Ok("minix\n".to_string())); + let mut env = MockEnvTrait::new(); + env.expect_var() + .withf(|key| key == "XDG_CACHE_HOME") + .returning(|_| Ok(String::new())); + env.expect_var() + .withf(|key| key == "HOME") + .returning(|_| Ok("/home/user".to_string())); + + let resolved = resolve_output_dir(&fs, &env, Path::new("/kernel")).unwrap(); + + assert_eq!( + Some( + Path::new("/home/user/.cache/kw/envs") + .join(BASE64.encode("/kernel")) + .join("minix") + ), + resolved + ); + } + #[test] fn resolve_output_dir_trims_trailing_slashes_before_encoding() { // kw encodes $PWD after cd-ing into the tree, where the path no @@ -826,8 +921,7 @@ last_line_without_newline=yes"; .withf(|key| key == "XDG_CACHE_HOME") .returning(|_| Ok("/xdg".to_string())); - let resolved = - resolve_output_dir(&fs, &env, Path::new("/home/user/linux/")).unwrap(); + let resolved = resolve_output_dir(&fs, &env, Path::new("/home/user/linux/")).unwrap(); assert_eq!( Some(PathBuf::from("/xdg/kw/envs/L2hvbWUvdXNlci9saW51eA==/minix")), @@ -839,10 +933,14 @@ last_line_without_newline=yes"; fn resolve_output_dir_empty_env_file_is_inactive() { let mut fs = MockFileSystemTrait::new(); fs.expect_is_file().returning(|_| true); - fs.expect_read_to_string().returning(|_| Ok("\n".to_string())); + fs.expect_read_to_string() + .returning(|_| Ok("\n".to_string())); let env = MockEnvTrait::new(); - assert_eq!(None, resolve_output_dir(&fs, &env, Path::new("/kernel")).unwrap()); + assert_eq!( + None, + resolve_output_dir(&fs, &env, Path::new("/kernel")).unwrap() + ); } #[test] @@ -925,7 +1023,10 @@ last_line_without_newline=yes"; fn find_image_missing_or_empty_dirs_return_none() { let dir = make_ready_tree("image-none"); // No image anywhere yet: the fixture has an empty arch/ dir. - assert_eq!(None, find_newest_kernel_image(&OsFileSystem, dir.path(), None)); + assert_eq!( + None, + find_newest_kernel_image(&OsFileSystem, dir.path(), None) + ); assert_eq!( None, find_newest_kernel_image(&OsFileSystem, dir.path(), Some("x86")) @@ -936,7 +1037,10 @@ last_line_without_newline=yes"; write_file_with_mtime(&boot.join("image"), 100); // lowercase: no match write_file_with_mtime(&boot.join("Image.gz"), 200); // suffix: no match - assert_eq!(None, find_newest_kernel_image(&OsFileSystem, dir.path(), None)); + assert_eq!( + None, + find_newest_kernel_image(&OsFileSystem, dir.path(), None) + ); assert_eq!( None, find_newest_kernel_image(&OsFileSystem, dir.path(), Some("x86")) @@ -1135,6 +1239,14 @@ last_line_without_newline=yes"; Ok(()), check_deploy_alone(Some(&record), &tree, "patchset-x", None, Some(&image)) ); + + // A trailing-slash-only difference is the same tree, not drift. + let mut slashed = built_record(dir.path(), "patchset-x"); + slashed.tree_path = format!("{}/", dir.path().to_str().unwrap()); + assert_eq!( + Ok(()), + check_deploy_alone(Some(&slashed), &tree, "patchset-x", None, Some(&image)) + ); } #[test] @@ -1192,13 +1304,53 @@ last_line_without_newline=yes"; readiness.tree ); assert_eq!(Some(boot.join("bzImage")), readiness.kernel_image); - assert_eq!(Some(record), readiness.build_record); + assert_eq!(Some(record.clone()), readiness.build_record); + assert_eq!(Some(record), readiness.latest_build); assert_eq!(Ok(()), readiness.deploy_alone); assert_eq!(None, readiness.output_dir); assert!(readiness.kw_binary.available); assert_eq!(KwVersionCheck::Meets, readiness.kw_binary.check); } + #[test] + fn evaluate_readiness_missing_tree_short_circuits() { + let dir = TempDir::new("evaluate-missing"); + let missing = dir.path().join("nope"); + let data = TempDir::new("evaluate-missing-data"); + let history = FileKwHistoryStore::new( + Arc::new(OsFileSystem), + data.path().to_str().unwrap().to_string(), + ); + + let mut env = MockEnvTrait::new(); + env.expect_which().returning(|_| false); + let mut shell = MockShellTrait::new(); + shell.expect_execute().times(0); + + let tree = kernel_tree(&missing); + let readiness = evaluate_readiness( + &OsFileSystem, + &env, + &shell, + &history, + "mainline", + &tree, + "patchset-x", + ) + .unwrap(); + + assert_eq!(TreeReadiness::Missing, readiness.tree); + assert_eq!(None, readiness.kernel_image); + assert_eq!(None, readiness.build_record); + assert_eq!(None, readiness.latest_build); + // Tree readiness is conjoined into the deploy-alone verdict, so + // Ok(()) can never describe a tree that is not build-ready. + assert_eq!( + Err(DeployAloneRefusal::TreeNotReady(TreeReadiness::Missing)), + readiness.deploy_alone + ); + } + #[test] fn evaluate_readiness_without_build_refuses_deploy_alone() { let dir = make_ready_tree("evaluate-nobuild"); @@ -1229,6 +1381,7 @@ last_line_without_newline=yes"; assert_eq!(TreeReadiness::Ready { arch: None }, readiness.tree); assert_eq!(None, readiness.kernel_image); assert_eq!(None, readiness.build_record); + assert_eq!(None, readiness.latest_build); assert_eq!( Err(DeployAloneRefusal::NoBuildRecord), readiness.deploy_alone From 7aaa6493f9c61244a92564b72151d32e8b2500fa Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Thu, 20 Aug 2026 12:29:22 -0300 Subject: [PATCH 8/9] refactor(kw): read build history once per readiness snapshot This commit loads kw_build_history.json once when composing a readiness snapshot, then derives the branch-matched and latest-across-branches records from that load, so the file is not parsed twice per snapshot. This commit completes the kw integration's step 3. Signed-off-by: lorenzoberts --- src/kw/history.rs | 87 +++++++++++++++++++++++++++++++++++++-------- src/kw/readiness.rs | 8 +++-- 2 files changed, 78 insertions(+), 17 deletions(-) diff --git a/src/kw/history.rs b/src/kw/history.rs index 2a8b9ea4..89555c40 100644 --- a/src/kw/history.rs +++ b/src/kw/history.rs @@ -111,6 +111,18 @@ pub trait KwHistoryStore: Send + Sync { &self, kernel_tree_id: &str, ) -> Result, FileSystemError>; + + /// Returns the record for `(kernel_tree_id, branch)` and the newest + /// record for the tree across branches from a single load of the + /// history file — the pair a readiness snapshot is computed from. + // Read by the kw readiness aggregation in a later step; kept per the + // CachePolicy precedent (src/lore/application/cache.rs). + #[allow(dead_code)] + fn build_records( + &self, + kernel_tree_id: &str, + branch: &str, + ) -> Result<(Option, Option), FileSystemError>; } pub struct FileKwHistoryStore { @@ -203,29 +215,39 @@ impl KwHistoryStore for FileKwHistoryStore { kernel_tree_id: &str, branch: &str, ) -> Result, FileSystemError> { - self.load_records(&self.build_history_path) - .map(|records: BuildRecords| { - records - .get(kernel_tree_id) - .and_then(|by_branch| by_branch.get(branch)) - .cloned() - }) - .map_err(|e| self.error_with_path(&self.build_history_path, e)) + self.build_records(kernel_tree_id, branch) + .map(|(record, _)| record) } fn latest_build_record( &self, kernel_tree_id: &str, ) -> Result, FileSystemError> { + // The branch half of the pair is unused here. + self.build_records(kernel_tree_id, "") + .map(|(_, latest)| latest) + } + + fn build_records( + &self, + kernel_tree_id: &str, + branch: &str, + ) -> Result<(Option, Option), FileSystemError> { self.load_records(&self.build_history_path) .map(|records: BuildRecords| { - records - .get(kernel_tree_id)? - .values() - .max_by_key(|record| { - chrono::DateTime::parse_from_rfc3339(&record.built_at).ok() - }) - .cloned() + let by_branch = records.get(kernel_tree_id); + let record = by_branch + .and_then(|by_branch| by_branch.get(branch)) + .cloned(); + let latest = by_branch.and_then(|by_branch| { + by_branch + .values() + .max_by_key(|record| { + chrono::DateTime::parse_from_rfc3339(&record.built_at).ok() + }) + .cloned() + }); + (record, latest) }) .map_err(|e| self.error_with_path(&self.build_history_path, e)) } @@ -539,6 +561,41 @@ mod tests { fs::remove_dir_all(&dir).unwrap(); } + #[test] + fn build_records_returns_branch_match_and_latest_from_one_load() { + let dir = tmp_dir("build-pair"); + let store = store_at(&dir); + + store + .record_build(build("mainline", "for-next", "2026-08-01T18:10:00Z")) + .unwrap(); + store + .record_build(build("mainline", "patchset-x", "2026-08-03T18:10:00Z")) + .unwrap(); + + assert_eq!( + ( + Some(build("mainline", "for-next", "2026-08-01T18:10:00Z")), + Some(build("mainline", "patchset-x", "2026-08-03T18:10:00Z")), + ), + store.build_records("mainline", "for-next").unwrap() + ); + // An unbuilt branch still reports the tree's latest record. + assert_eq!( + ( + None, + Some(build("mainline", "patchset-x", "2026-08-03T18:10:00Z")), + ), + store.build_records("mainline", "never-built").unwrap() + ); + assert_eq!( + (None, None), + store.build_records("amd-gfx", "for-next").unwrap() + ); + + fs::remove_dir_all(&dir).unwrap(); + } + #[test] fn missing_build_history_reads_as_empty() { let dir = tmp_dir("build-missing"); diff --git a/src/kw/readiness.rs b/src/kw/readiness.rs index f8652613..99495596 100644 --- a/src/kw/readiness.rs +++ b/src/kw/readiness.rs @@ -428,6 +428,11 @@ pub enum DeployAloneRefusal { /// 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. +/// +/// 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. // Consumed by KwActor deploy in a later step; kept per the CachePolicy // precedent (src/lore/application/cache.rs). #[allow(dead_code)] @@ -517,8 +522,7 @@ pub fn evaluate_readiness( output_dir.as_deref().unwrap_or(tree_path), arch.as_deref(), ); - let build_record = history.build_record(kernel_tree_id, head_branch)?; - let latest_build = history.latest_build_record(kernel_tree_id)?; + let (build_record, latest_build) = history.build_records(kernel_tree_id, head_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. From f80952c9fbe2189b8b26850fe6077fae0ee11834 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Mon, 24 Aug 2026 14:21:46 -0300 Subject: [PATCH 9/9] docs(kw): describe readiness probes without the integration plan This commit drops plan section numbers, KwOps, and "later step" comments. The dead_code allowances stay where this change still has no production caller; the comments around them now describe the code as it is. This commit is part of the kw integration's step 3. Signed-off-by: lorenzoberts Co-authored-by: Cursor --- src/infrastructure/file_system/trait.rs | 3 --- src/kw/history.rs | 25 ++++++------------- src/kw/mod.rs | 4 ++-- src/kw/readiness.rs | 32 ++++++++----------------- 4 files changed, 19 insertions(+), 45 deletions(-) diff --git a/src/infrastructure/file_system/trait.rs b/src/infrastructure/file_system/trait.rs index 077eb200..5a97c372 100644 --- a/src/infrastructure/file_system/trait.rs +++ b/src/infrastructure/file_system/trait.rs @@ -24,9 +24,6 @@ pub trait FileSystemTrait: Send + Sync { /// Returns the immediate children of directory `path` as full paths, /// sorted for determinism. Entry kind and metadata are queried /// separately via `is_dir`/`is_file`/`metadata`. - // No production caller until the kw readiness probes land; kept per the - // CachePolicy precedent (src/lore/application/cache.rs). - #[allow(dead_code)] fn read_dir(&self, path: &Path) -> Result, FileSystemError>; fn rename(&self, from: &Path, to: &Path) -> Result<(), FileSystemError>; fn create_writer(&self, path: &Path) -> Result, FileSystemError>; diff --git a/src/kw/history.rs b/src/kw/history.rs index 89555c40..ed95037c 100644 --- a/src/kw/history.rs +++ b/src/kw/history.rs @@ -1,9 +1,8 @@ //! User-local history of patchset applies and kw builds, stored as JSON //! under the configured `data_dir`. //! -//! Apply records feed kw build/deploy readiness and the KwOps branch -//! prefill, and build records feed deploy-alone readiness, so they are -//! user state — not a cache — and are never refreshed from lore. +//! Apply and build records are user state — not a cache — and are never +//! refreshed from lore. use mockall::automock; use serde::{Deserialize, Serialize}; @@ -30,9 +29,8 @@ pub struct KwApplyRecord { } /// One recorded `kw build` attempt on a kernel tree branch, whether it -/// succeeded or not: storing failures lets KwOps show "last build failed" -/// instead of "no build recorded", and deploy-alone readiness requires -/// `success == true` on the matching record. +/// succeeded or not. Failed attempts are stored so deploy-alone can refuse +/// them instead of treating the tree as never built. /// /// `message_id`, `arch`, `image_path`, and `kernelrelease` are optional: a /// build can target a branch no patchset was applied to, `arch` is unknown @@ -41,8 +39,7 @@ pub struct KwApplyRecord { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct KwBuildRecord { pub kernel_tree_id: String, - /// Snapshot of `KernelTree.path` when the record was written, so later - /// readiness checks can detect the tree being repointed or moved. + /// Snapshot of `KernelTree.path` when the record was written. pub tree_path: String, pub message_id: Option, pub branch: String, @@ -85,16 +82,12 @@ pub trait KwHistoryStore: Send + Sync { /// Inserts or replaces the build record for the record's /// `(kernel_tree_id, branch)` pair. - // Written by KwActor once builds land; kept per the CachePolicy - // precedent (src/lore/application/cache.rs). #[allow(dead_code)] fn record_build(&self, record: KwBuildRecord) -> Result<(), FileSystemError>; /// Returns the build record for the `(kernel_tree_id, branch)` pair, or /// `None` if it was never recorded. A missing history file is a normal /// state, not an error. - // Read by the kw readiness checks in a later step; kept per the - // CachePolicy precedent (src/lore/application/cache.rs). #[allow(dead_code)] fn build_record( &self, @@ -104,8 +97,6 @@ pub trait KwHistoryStore: Send + Sync { /// Returns the chronologically newest build record for the tree, across /// branches, or `None` if none was recorded. - // Read by the kw readiness checks in a later step; kept per the - // CachePolicy precedent (src/lore/application/cache.rs). #[allow(dead_code)] fn latest_build_record( &self, @@ -115,8 +106,6 @@ pub trait KwHistoryStore: Send + Sync { /// Returns the record for `(kernel_tree_id, branch)` and the newest /// record for the tree across branches from a single load of the /// history file — the pair a readiness snapshot is computed from. - // Read by the kw readiness aggregation in a later step; kept per the - // CachePolicy precedent (src/lore/application/cache.rs). #[allow(dead_code)] fn build_records( &self, @@ -467,8 +456,8 @@ mod tests { failed.kernelrelease = None; store.record_build(failed.clone()).unwrap(); - // A failed attempt is stored, not dropped: KwOps can show "last - // build failed" and deploy-alone readiness refuses it. + // A failed attempt is stored, not dropped: deploy-alone readiness + // refuses it. assert_eq!( Some(build("mainline", "for-next", "2026-08-01T18:10:00Z")), store.build_record("mainline", "for-next").unwrap() diff --git a/src/kw/mod.rs b/src/kw/mod.rs index a01a8244..2e5b1b99 100644 --- a/src/kw/mod.rs +++ b/src/kw/mod.rs @@ -1,5 +1,5 @@ -//! kw integration: persistence, readiness probes, and (in later steps) the -//! actor orchestrating `kw build` / `kw deploy` jobs. +//! kw integration: persistence and readiness probes for `kw build` / +//! `kw deploy` jobs. pub mod history; pub mod readiness; diff --git a/src/kw/readiness.rs b/src/kw/readiness.rs index 99495596..28f3d468 100644 --- a/src/kw/readiness.rs +++ b/src/kw/readiness.rs @@ -114,8 +114,6 @@ pub fn is_kernel_root(fs: &dyn FileSystemTrait, path: &Path) -> bool { /// refuses to activate an env while an in-tree `.config` exists /// (`kw_env.sh::validate_env_before_switch`), so with an env active the /// `.config` lives only at the env's `O=` dir. -// No production caller until the readiness aggregation lands; kept per the -// CachePolicy precedent (src/lore/application/cache.rs). #[allow(dead_code)] pub fn probe_tree( fs: &dyn FileSystemTrait, @@ -179,8 +177,6 @@ pub fn read_build_arch(fs: &dyn FileSystemTrait, tree_path: &Path) -> Option, @@ -471,10 +463,8 @@ pub fn check_deploy_alone( Ok(()) } -/// Snapshot of everything KwOps needs to decide whether build/deploy can -/// start, and why not — returned by KwActor's `GetReadiness` (§2.3). -// Assembled by KwActor in a later step; kept per the CachePolicy precedent -// (src/lore/application/cache.rs). +/// Snapshot of tree, kw binary, and history probes used to decide whether +/// a job can start, and why not. #[allow(dead_code)] #[derive(Debug, Clone)] pub struct KwReadiness { @@ -486,8 +476,8 @@ pub struct KwReadiness { pub kernel_image: Option, /// Build record for `(kernel_tree_id, head_branch)`, if any. pub build_record: Option, - /// Newest build record for the tree across branches, so KwOps can show - /// "last build was on branch X" copy even when HEAD has no record. + /// Newest build record for the tree across branches, even when HEAD + /// has none. pub latest_build: Option, /// `Ok(())` is a self-sufficient verdict: tree readiness is already /// conjoined in, so a caller cannot forget to check `tree` as well. @@ -497,8 +487,6 @@ pub struct KwReadiness { /// 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. -// Composed by KwActor in a later step; kept per the CachePolicy precedent -// (src/lore/application/cache.rs). #[allow(dead_code)] pub fn evaluate_readiness( fs: &dyn FileSystemTrait,