From 06b1a3a56ac6ddc4a227af37017ab0d7b10aaa8b Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 20 Sep 2026 11:18:10 -0300 Subject: [PATCH 1/6] feat(config): validate target_kernel_tree against existing trees This commit adds target_kernel_tree to the config update pipeline so Edit Config can later select among existing kernel_trees keys without hand-editing JSON. An empty string unsets the target; an unknown key is refused and leaves the current value unchanged. Trees themselves are still added by editing the config file. This commit is part of the kw integration's step 8. Signed-off-by: lorenzoberts --- src/app/screens/edit_config.rs | 1 + src/config/actor.rs | 2 +- src/config/errors.rs | 2 + src/config/service.rs | 15 +++ src/config/state.rs | 7 ++ src/config/tests.rs | 175 ++++++++++++++++++++++++++++++++- src/config/update.rs | 5 + 7 files changed, 205 insertions(+), 2 deletions(-) diff --git a/src/app/screens/edit_config.rs b/src/app/screens/edit_config.rs index 0f9905b..3804bc7 100644 --- a/src/app/screens/edit_config.rs +++ b/src/app/screens/edit_config.rs @@ -161,6 +161,7 @@ impl EditConfigState { .config_buffer .get(&EditableConfig::KwDeployForce) .cloned(), + target_kernel_tree: None, } } } diff --git a/src/config/actor.rs b/src/config/actor.rs index e321fc3..2eb64bb 100644 --- a/src/config/actor.rs +++ b/src/config/actor.rs @@ -84,7 +84,7 @@ where } fn apply(&mut self, draft: ConfigUpdateDraft) -> ConfigResult { - let update = validate_update(draft, self.repo.fs())?; + let update = validate_update(draft, self.repo.fs(), &self.state)?; self.state.apply_update(&update); normalize_derived_paths(&mut self.state); ensure_directories(&self.state, self.repo.fs())?; diff --git a/src/config/errors.rs b/src/config/errors.rs index 7c8dbe0..735c18e 100644 --- a/src/config/errors.rs +++ b/src/config/errors.rs @@ -22,6 +22,8 @@ pub enum ConfigError { InvalidKwRebootAfterDeploy(String), #[error("invalid kw deploy-force value: {0}")] InvalidKwDeployForce(String), + #[error("unknown kernel tree key '{0}'; edit the config file to add trees")] + InvalidTargetKernelTree(String), #[error("filesystem error: {0}")] Fs(#[from] FileSystemError), } diff --git a/src/config/service.rs b/src/config/service.rs index b5c0ab6..4a8ef30 100644 --- a/src/config/service.rs +++ b/src/config/service.rs @@ -67,6 +67,7 @@ pub(crate) fn ensure_directories( pub(crate) fn validate_update( draft: ConfigUpdateDraft, fs: &dyn FileSystemTrait, + current: &ConfigState, ) -> Result { let page_size = match &draft.page_size { None => None, @@ -169,6 +170,19 @@ pub(crate) fn validate_update( ), }; + let target_kernel_tree = match &draft.target_kernel_tree { + None => None, + Some(s) if s.trim().is_empty() => Some(None), + Some(s) => { + let key = s.trim(); + if current.kernel_trees.contains_key(key) { + Some(Some(key.to_string())) + } else { + return Err(ConfigError::InvalidTargetKernelTree(s.clone())); + } + } + }; + Ok(ValidatedConfigUpdate { page_size, cache_dir, @@ -181,6 +195,7 @@ pub(crate) fn validate_update( stay_on_applied_branch, kw_reboot_after_deploy, kw_deploy_force, + target_kernel_tree, }) } diff --git a/src/config/state.rs b/src/config/state.rs index b5e7c5f..e7b466f 100644 --- a/src/config/state.rs +++ b/src/config/state.rs @@ -144,6 +144,10 @@ impl ConfigState { self.kw_deploy_force = kw_deploy_force; } + fn set_target_kernel_tree(&mut self, target_kernel_tree: Option) { + self.target_kernel_tree = target_kernel_tree; + } + /// Merges validated field updates from the edit-config flow. pub fn apply_update(&mut self, u: &ValidatedConfigUpdate) { if let Some(page_size) = u.page_size { @@ -179,6 +183,9 @@ impl ConfigState { if let Some(kw_deploy_force) = u.kw_deploy_force { self.set_kw_deploy_force(kw_deploy_force); } + if let Some(ref target_kernel_tree) = u.target_kernel_tree { + self.set_target_kernel_tree(target_kernel_tree.clone()); + } } pub fn to_snapshot(&self) -> ConfigSnapshot { diff --git a/src/config/tests.rs b/src/config/tests.rs index 220db04..486625e 100644 --- a/src/config/tests.rs +++ b/src/config/tests.rs @@ -12,7 +12,7 @@ use crate::config::repository::{ConfigRepository, JsonConfigRepository}; use crate::config::service::{bootstrap_parts, validate_update}; use crate::config::state::{normalize_derived_paths, ConfigState}; use crate::config::{ - ConfigError, ConfigSnapshot, ConfigUpdateDraft, ValidatedConfigUpdate, + ConfigError, ConfigSnapshot, ConfigUpdateDraft, KernelTree, ValidatedConfigUpdate, DEFAULT_CONFIG_PATH_SUFFIX, }; use crate::infrastructure::{ @@ -26,6 +26,27 @@ fn os_fs() -> OsFileSystem { OsFileSystem } +fn sample_kernel_tree(path: &str, branch: &str) -> KernelTree { + serde_json::from_value(json!({ + "path": path, + "branch": branch + })) + .unwrap() +} + +fn state_with_trees(env: &dyn EnvTrait) -> ConfigState { + let mut state = ConfigState::new_with_defaults(env); + state.kernel_trees.insert( + "linux".into(), + sample_kernel_tree("/home/user/linux", "master"), + ); + state.kernel_trees.insert( + "amd-gfx".into(), + sample_kernel_tree("/home/user/amd-gfx", "amd-staging-drm-next"), + ); + state +} + fn unique_test_dir(prefix: &str) -> PathBuf { let n = TEST_SEQ.fetch_add(1, Ordering::SeqCst); let p = std::env::temp_dir().join(format!("patch-hub-{prefix}-{}-{}", std::process::id(), n)); @@ -494,6 +515,7 @@ fn validate_update_rejects_invalid_page_size() { ..Default::default() }, &os_fs(), + &ConfigState::default(), ) .unwrap_err(); assert!(matches!(err, ConfigError::InvalidPageSize(ref s) if s == "xyz")); @@ -507,6 +529,7 @@ fn validate_update_rejects_invalid_patch_renderer() { ..Default::default() }, &os_fs(), + &ConfigState::default(), ) .unwrap_err(); assert!(matches!( @@ -523,6 +546,7 @@ fn validate_update_rejects_invalid_cover_renderer() { ..Default::default() }, &os_fs(), + &ConfigState::default(), ) .unwrap_err(); assert!(matches!( @@ -539,6 +563,7 @@ fn validate_update_rejects_invalid_max_log_age() { ..Default::default() }, &os_fs(), + &ConfigState::default(), ) .unwrap_err(); assert!(matches!( @@ -556,6 +581,7 @@ fn validate_update_rejects_invalid_stay_on_applied_branch() { ..Default::default() }, &os_fs(), + &ConfigState::default(), ) .unwrap_err(); assert!(matches!( @@ -597,6 +623,7 @@ fn validate_update_rejects_invalid_kw_deploy_bools() { ..Default::default() }, &os_fs(), + &ConfigState::default(), ) .unwrap_err(); if expect_reboot_err { @@ -636,6 +663,80 @@ fn apply_update_toggles_kw_deploy_knobs() { assert!(!state.kw_deploy_force()); } +#[test] +fn validate_update_accepts_existing_target_kernel_tree() { + let (env, _home) = default_env(); + let state = state_with_trees(&env); + let update = validate_update( + ConfigUpdateDraft { + target_kernel_tree: Some("linux".into()), + ..Default::default() + }, + &os_fs(), + &state, + ) + .unwrap(); + assert_eq!(Some(Some("linux".to_string())), update.target_kernel_tree); +} + +#[test] +fn validate_update_unsets_target_kernel_tree_on_empty_string() { + let (env, _home) = default_env(); + let state = state_with_trees(&env); + let update = validate_update( + ConfigUpdateDraft { + target_kernel_tree: Some("".into()), + ..Default::default() + }, + &os_fs(), + &state, + ) + .unwrap(); + assert_eq!(Some(None), update.target_kernel_tree); +} + +#[test] +fn validate_update_rejects_unknown_target_kernel_tree() { + let (env, _home) = default_env(); + let state = state_with_trees(&env); + let err = validate_update( + ConfigUpdateDraft { + target_kernel_tree: Some("missing".into()), + ..Default::default() + }, + &os_fs(), + &state, + ) + .unwrap_err(); + assert!(matches!( + err, + ConfigError::InvalidTargetKernelTree(ref s) if s == "missing" + )); +} + +#[test] +fn apply_update_sets_and_unsets_target_kernel_tree() { + let (env, _home) = default_env(); + let mut state = state_with_trees(&env); + assert!(state.target_kernel_tree().is_none()); + + state.apply_update(&ValidatedConfigUpdate { + target_kernel_tree: Some(Some("linux".into())), + ..Default::default() + }); + assert_eq!(Some("linux"), state.target_kernel_tree().as_deref()); + + state.apply_update(&ValidatedConfigUpdate { + target_kernel_tree: Some(None), + ..Default::default() + }); + assert!(state.target_kernel_tree().is_none()); + + // A draft that omits the field leaves the current value untouched. + state.apply_update(&ValidatedConfigUpdate::default()); + assert!(state.target_kernel_tree().is_none()); +} + #[test] fn validate_update_rejects_cache_dir_that_is_existing_file() { let root = unique_test_dir("not-a-dir"); @@ -647,6 +748,7 @@ fn validate_update_rejects_cache_dir_that_is_existing_file() { ..Default::default() }, &os_fs(), + &ConfigState::default(), ) .unwrap_err(); assert!(matches!(err, ConfigError::InvalidDirectory(_))); @@ -705,3 +807,74 @@ async fn validate_and_apply_persists_to_config_file() { assert_eq!(parsed["page_size"], 77); handle.shutdown().await; } + +#[tokio::test] +async fn validate_and_apply_persists_target_kernel_tree() { + let (env, home) = default_env(); + let (mut state, repo) = bootstrap_parts(&env, os_fs()).unwrap(); + state.kernel_trees.insert( + "linux".into(), + sample_kernel_tree("/home/user/linux", "master"), + ); + let handle = ConfigActor::spawn(state, repo); + let cfg_path = home.join(DEFAULT_CONFIG_PATH_SUFFIX); + + let snapshot = handle + .validate_and_apply(ConfigUpdateDraft { + target_kernel_tree: Some("linux".into()), + ..Default::default() + }) + .await + .unwrap(); + assert_eq!(Some("linux"), snapshot.target_kernel_tree().as_deref()); + + let raw = fs::read_to_string(&cfg_path).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&raw).unwrap(); + assert_eq!(parsed["target_kernel_tree"], "linux"); + + handle + .validate_and_apply(ConfigUpdateDraft { + target_kernel_tree: Some("".into()), + ..Default::default() + }) + .await + .unwrap(); + let raw = fs::read_to_string(&cfg_path).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&raw).unwrap(); + assert!(parsed["target_kernel_tree"].is_null()); + handle.shutdown().await; +} + +#[tokio::test] +async fn invalid_target_kernel_tree_keeps_existing_state() { + let (env, _home) = default_env(); + let (mut state, repo) = bootstrap_parts(&env, os_fs()).unwrap(); + state.kernel_trees.insert( + "linux".into(), + sample_kernel_tree("/home/user/linux", "master"), + ); + state.target_kernel_tree = Some("linux".into()); + let handle = ConfigActor::spawn(state, repo); + + let err = handle + .validate_and_apply(ConfigUpdateDraft { + target_kernel_tree: Some("missing".into()), + ..Default::default() + }) + .await + .unwrap_err(); + assert!(matches!( + err, + ConfigError::InvalidTargetKernelTree(ref s) if s == "missing" + )); + assert_eq!( + Some("linux"), + handle + .get_snapshot() + .await + .unwrap() + .target_kernel_tree() + .as_deref() + ); + handle.shutdown().await; +} diff --git a/src/config/update.rs b/src/config/update.rs index 44b14dc..2ee3b47 100644 --- a/src/config/update.rs +++ b/src/config/update.rs @@ -14,6 +14,8 @@ pub struct ConfigUpdateDraft { pub stay_on_applied_branch: Option, pub kw_reboot_after_deploy: Option, pub kw_deploy_force: Option, + /// Raw `kernel_trees` key. `None` omits the field; empty string unsets. + pub target_kernel_tree: Option, } /// Parsed and validated update ready to merge into [`crate::config::ConfigState`](super::state::ConfigState). @@ -30,4 +32,7 @@ pub struct ValidatedConfigUpdate { pub stay_on_applied_branch: Option, pub kw_reboot_after_deploy: Option, pub kw_deploy_force: Option, + /// `None` leaves the current target unchanged; `Some(None)` unsets it; + /// `Some(Some(key))` selects an existing `kernel_trees` key. + pub target_kernel_tree: Option>, } From 6ea2f0930b019dc36e0904a3c72f32f8bf3ab1e2 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 20 Sep 2026 11:20:57 -0300 Subject: [PATCH 2/6] feat(app): cycle target kernel tree in Edit Config state This commit adds a Target Kernel Tree row to Edit Config so the user can pick among existing kernel_trees keys, including to unset. Cycling is a state-level operation for now; input wiring comes next. Keys are sorted so the cycle order does not depend on HashMap iteration. This commit is part of the kw integration's step 8. Signed-off-by: lorenzoberts --- src/app/screens/edit_config.rs | 189 ++++++++++++++++++++++++++++++++- src/config/state.rs | 1 - src/kw/remote.rs | 5 +- 3 files changed, 188 insertions(+), 7 deletions(-) diff --git a/src/app/screens/edit_config.rs b/src/app/screens/edit_config.rs index 3804bc7..06423e8 100644 --- a/src/app/screens/edit_config.rs +++ b/src/app/screens/edit_config.rs @@ -13,6 +13,8 @@ use crate::config::{ConfigSnapshot, ConfigUpdateDraft}; pub struct EditConfigState { #[getter(skip)] config_buffer: HashMap, + #[getter(skip)] + tree_options: Vec, highlighted: usize, is_editing: bool, curr_edit: String, @@ -53,9 +55,17 @@ impl EditConfigState { EditableConfig::KwDeployForce, config.kw_deploy_force().to_string(), ); + config_buffer.insert( + EditableConfig::TargetKernelTree, + config.target_kernel_tree().clone().unwrap_or_default(), + ); + + let mut tree_options: Vec = config.kernel_trees().into_iter().cloned().collect(); + tree_options.sort(); EditConfigState { config_buffer, + tree_options, highlighted: 0, is_editing: false, curr_edit: String::new(), @@ -72,12 +82,51 @@ impl EditConfigState { EditableConfig::try_from(i) .ok() .and_then(|editable_config| { - self.config_buffer - .get(&editable_config) - .map(|value| (editable_config.to_string(), value.clone())) + self.config_buffer.get(&editable_config).map(|value| { + let display = if editable_config == EditableConfig::TargetKernelTree + && value.is_empty() + { + "".to_string() + } else { + value.clone() + }; + (editable_config.to_string(), display) + }) }) } + pub fn highlighted_is_tree_selector(&self) -> bool { + matches!( + EditableConfig::try_from(self.highlighted), + Ok(EditableConfig::TargetKernelTree) + ) + } + + /// Cycles the tree selector through `` and the sorted tree keys. + /// No-op unless the highlighted row is the tree selector and editing. + pub fn cycle_edit(&mut self, forward: bool) { + if !self.is_editing || !self.highlighted_is_tree_selector() { + return; + } + + let mut options = Vec::with_capacity(self.tree_options.len() + 1); + options.push(String::new()); + options.extend(self.tree_options.iter().cloned()); + + let current = options + .iter() + .position(|key| key == &self.curr_edit) + .unwrap_or(0); + let next = if forward { + (current + 1) % options.len() + } else if current == 0 { + options.len() - 1 + } else { + current - 1 + }; + self.curr_edit = options[next].clone(); + } + /// Toggle editing mode pub fn toggle_editing(&mut self) { if !self.is_editing { @@ -106,6 +155,9 @@ impl EditConfigState { /// Remove the last char from the current editing value if not empty pub fn backspace_edit(&mut self) { + if self.highlighted_is_tree_selector() { + return; + } if !self.curr_edit.is_empty() { self.curr_edit.pop(); } @@ -113,6 +165,9 @@ impl EditConfigState { /// Appends a new char to the current editing value pub fn append_edit(&mut self, ch: char) { + if self.highlighted_is_tree_selector() { + return; + } self.curr_edit.push(ch); } @@ -161,7 +216,10 @@ impl EditConfigState { .config_buffer .get(&EditableConfig::KwDeployForce) .cloned(), - target_kernel_tree: None, + target_kernel_tree: self + .config_buffer + .get(&EditableConfig::TargetKernelTree) + .cloned(), } } } @@ -179,6 +237,7 @@ enum EditableConfig { StayOnAppliedBranch, KwRebootAfterDeploy, KwDeployForce, + TargetKernelTree, } impl TryFrom for EditableConfig { @@ -197,6 +256,7 @@ impl TryFrom for EditableConfig { 8 => Ok(EditableConfig::StayOnAppliedBranch), 9 => Ok(EditableConfig::KwRebootAfterDeploy), 10 => Ok(EditableConfig::KwDeployForce), + 11 => Ok(EditableConfig::TargetKernelTree), _ => bail!("Invalid index {} for EditableConfig", value), // Handle out of bounds } } @@ -226,6 +286,9 @@ impl Display for EditableConfig { EditableConfig::KwDeployForce => { write!(f, "Force kw Deploy (true/false)") } + EditableConfig::TargetKernelTree => { + write!(f, "Target Kernel Tree (ENTER, then ←/→ to cycle)") + } } } } @@ -235,6 +298,28 @@ mod tests { use super::*; use crate::config::ConfigState; + fn snapshot_with_trees(keys: &[&str], target: Option<&str>) -> ConfigSnapshot { + let mut state = ConfigState::default(); + for key in keys { + state.kernel_trees.insert( + (*key).to_string(), + serde_json::from_value(serde_json::json!({ + "path": format!("/{key}"), + "branch": "master" + })) + .unwrap(), + ); + } + state.target_kernel_tree = target.map(str::to_string); + state.to_snapshot() + } + + fn tree_row(edit: &mut EditConfigState) { + while edit.highlighted() != 11 { + edit.highlight_next(); + } + } + #[test] fn draft_includes_deploy_knobs_with_compiled_in_defaults() { let snapshot = ConfigState::default().to_snapshot(); @@ -243,7 +328,8 @@ mod tests { assert_eq!(Some("false".to_string()), draft.kw_reboot_after_deploy); assert_eq!(Some("true".to_string()), draft.kw_deploy_force); - assert_eq!(11, edit.config_count()); + assert_eq!(Some(String::new()), draft.target_kernel_tree); + assert_eq!(12, edit.config_count()); assert_eq!( Some(( "Reboot After kw Deploy (true/false)".to_string(), @@ -258,5 +344,98 @@ mod tests { )), edit.config(10) ); + assert_eq!( + Some(( + "Target Kernel Tree (ENTER, then ←/→ to cycle)".to_string(), + "".to_string() + )), + edit.config(11) + ); + } + + #[test] + fn cycle_edit_wraps_across_none_and_sorted_keys() { + let snapshot = snapshot_with_trees(&["zebra", "linux"], Some("linux")); + let mut edit = EditConfigState::new(&snapshot); + tree_row(&mut edit); + edit.toggle_editing(); + assert_eq!("linux", edit.curr_edit()); + + edit.cycle_edit(true); + assert_eq!("zebra", edit.curr_edit()); + edit.cycle_edit(true); + assert_eq!("", edit.curr_edit()); + edit.cycle_edit(true); + assert_eq!("linux", edit.curr_edit()); + + edit.cycle_edit(false); + assert_eq!("", edit.curr_edit()); + edit.cycle_edit(false); + assert_eq!("zebra", edit.curr_edit()); + } + + #[test] + fn cycle_edit_is_noop_when_not_editing_or_on_other_rows() { + let snapshot = snapshot_with_trees(&["linux"], Some("linux")); + let mut edit = EditConfigState::new(&snapshot); + tree_row(&mut edit); + edit.cycle_edit(true); + assert_eq!("", edit.curr_edit()); + assert_eq!( + Some(( + "Target Kernel Tree (ENTER, then ←/→ to cycle)".to_string(), + "linux".to_string() + )), + edit.config(11) + ); + + edit.highlighted = 0; + edit.toggle_editing(); + let page_size = edit.curr_edit().to_string(); + edit.cycle_edit(true); + assert_eq!(page_size, edit.curr_edit().as_str()); + } + + #[test] + fn text_input_is_ignored_on_the_tree_selector() { + let snapshot = snapshot_with_trees(&["linux"], Some("linux")); + let mut edit = EditConfigState::new(&snapshot); + tree_row(&mut edit); + edit.toggle_editing(); + edit.append_edit('x'); + edit.backspace_edit(); + assert_eq!("linux", edit.curr_edit()); + } + + #[test] + fn staged_tree_selection_reaches_the_draft() { + let snapshot = snapshot_with_trees(&["linux", "zebra"], Some("linux")); + let mut edit = EditConfigState::new(&snapshot); + tree_row(&mut edit); + edit.toggle_editing(); + edit.cycle_edit(true); + edit.stage_edit(); + edit.toggle_editing(); + assert_eq!( + Some("zebra".to_string()), + edit.to_update_draft().target_kernel_tree + ); + + tree_row(&mut edit); + edit.toggle_editing(); + edit.cycle_edit(true); + edit.stage_edit(); + edit.toggle_editing(); + assert_eq!( + Some(String::new()), + edit.to_update_draft().target_kernel_tree + ); + assert_eq!( + Some(( + "Target Kernel Tree (ENTER, then ←/→ to cycle)".to_string(), + "".to_string() + )), + edit.config(11) + ); } } diff --git a/src/config/state.rs b/src/config/state.rs index e7b466f..b0f7ff3 100644 --- a/src/config/state.rs +++ b/src/config/state.rs @@ -259,7 +259,6 @@ impl ConfigSnapshot { self.page_size } - #[allow(dead_code)] pub fn kernel_trees(&self) -> HashSet<&String> { self.kernel_trees.keys().collect::>() } diff --git a/src/kw/remote.rs b/src/kw/remote.rs index 0df8238..685cf49 100644 --- a/src/kw/remote.rs +++ b/src/kw/remote.rs @@ -302,7 +302,10 @@ Host ph-dut ], parsed.hosts ); - assert_eq!("abc@arch-tm:22", choose(SAMPLE_REMOTE_CONFIG).unwrap().endpoint()); + assert_eq!( + "abc@arch-tm:22", + choose(SAMPLE_REMOTE_CONFIG).unwrap().endpoint() + ); } #[test] From b628c3f9574011e026e8a63a2b259479efe1a115 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 20 Sep 2026 11:24:56 -0300 Subject: [PATCH 3/6] feat(app): wire Edit Config tree cycling into the TUI This commit maps Left/Right while editing so the target kernel tree row can be cycled from the keyboard, shows in the live edit cursor, and documents the interaction in help and the key hint. ESC/q already saved and exited; the help text now matches that. This commit completes the kw integration's step 8. Signed-off-by: lorenzoberts --- src/app/flows/edit_config.rs | 211 +++++++++++++++++++++++++++++++++- src/app/view_model.rs | 38 +++++- src/input/mapper.rs | 35 ++++++ src/ui/screens/edit_config.rs | 35 +++++- 4 files changed, 313 insertions(+), 6 deletions(-) diff --git a/src/app/flows/edit_config.rs b/src/app/flows/edit_config.rs index 73a04b7..b7b4f26 100644 --- a/src/app/flows/edit_config.rs +++ b/src/app/flows/edit_config.rs @@ -36,6 +36,12 @@ pub async fn handle_edit_config(app: &mut App, input: InputEvent) -> Result<()> edit_config_state.clear_edit(); edit_config_state.toggle_editing(); } + InputEvent::NavigateLeft => { + edit_config_state.cycle_edit(false); + } + InputEvent::NavigateRight => { + edit_config_state.cycle_edit(true); + } _ => {} } } @@ -75,12 +81,209 @@ pub async fn handle_edit_config(app: &mut App, input: InputEvent) -> Result<()> pub fn generate_help_popup() -> AppPopup { AppPopup::help() .title("Edit Config") - .description("This screen allows you to edit the configuration options for patch-hub.\nMore configurations may be available in the configuration file.") - .keybind("ESC", "Exit") - .keybind("ENTER", "Save changes") + .description("This screen allows you to edit the configuration options for patch-hub.\nKernel trees are added by editing the config file; this screen selects among existing keys.") + .keybind("ESC / q", "Save and exit") + .keybind("ENTER", "Edit the highlighted option") .keybind("?", "Show this help screen") .keybind("j/🡇", "Down") .keybind("k/🡅", "Up") - .keybind("e", "Toggle editing for a configuration option") + .keybind("←/→", "Cycle the target kernel tree while editing that row") .build() } + +#[cfg(test)] +mod tests { + use std::{ + collections::HashMap, + env::VarError, + fs, + path::PathBuf, + sync::{ + atomic::{AtomicU64, Ordering}, + Arc, + }, + }; + + use tokio::sync::mpsc; + + use crate::{ + app::{ + screens::{ + bookmarked::BookmarkedPatchsetsState, mail_list::MailingListSelectionState, + CurrentScreen, + }, + state::{AppState, ConfigUiState, LoreUiState, NavigationState, UserLoreState}, + App, AppServices, + }, + config::{bootstrap_parts, ConfigActor, ConfigHandle, DEFAULT_CONFIG_PATH_SUFFIX}, + infrastructure::{ + env::MockEnvTrait, file_system::MockFileSystemTrait, file_system::OsFileSystem, + shell::MockShellTrait, + }, + kw::history::MockKwHistoryStore, + lore::{application::handle::LoreApiHandle, domain::mailing_list::MailingList}, + render::handle::RenderHandle, + }; + + use super::*; + + static TEST_SEQ: AtomicU64 = AtomicU64::new(0); + + fn unique_test_dir(prefix: &str) -> PathBuf { + let n = TEST_SEQ.fetch_add(1, Ordering::SeqCst); + let p = std::env::temp_dir().join(format!( + "patch-hub-edit-config-{prefix}-{}-{n}", + std::process::id() + )); + fs::create_dir_all(&p).unwrap(); + p + } + + fn default_env() -> (MockEnvTrait, PathBuf) { + let home = unique_test_dir("home"); + let home_s = home.to_string_lossy().into_owned(); + let mut mock = MockEnvTrait::new(); + mock.expect_var() + .withf(|key| key == "PATCH_HUB_CONFIG_PATH") + .returning(|_| Err(VarError::NotPresent.into())); + mock.expect_var() + .withf(move |key| key == "HOME") + .returning(move |_| Ok(home_s.clone())); + mock.expect_var() + .withf(|key| { + matches!( + key, + "PATCH_HUB_PAGE_SIZE" + | "PATCH_HUB_CACHE_DIR" + | "PATCH_HUB_DATA_DIR" + | "PATCH_HUB_GIT_SEND_EMAIL_OPTIONS" + | "PATCH_HUB_PATCH_RENDERER" + ) + }) + .returning(|_| Err(VarError::NotPresent.into())); + (mock, home) + } + + fn app_with_trees() -> (App, ConfigHandle, PathBuf) { + let (env, home) = default_env(); + let (mut state, repo) = bootstrap_parts(&env, OsFileSystem).unwrap(); + state.kernel_trees.insert( + "linux".into(), + serde_json::from_value(serde_json::json!({ + "path": "/linux", + "branch": "master" + })) + .unwrap(), + ); + state.kernel_trees.insert( + "zebra".into(), + serde_json::from_value(serde_json::json!({ + "path": "/zebra", + "branch": "master" + })) + .unwrap(), + ); + let snapshot = state.to_snapshot(); + let config = ConfigActor::spawn(state, repo); + + let dummy_list = MailingList::new("test-list", "Test list"); + let (lore_tx, _lore_rx) = mpsc::channel(1); + let (render_tx, _render_rx) = mpsc::channel(1); + let app = App { + state: AppState { + navigation: NavigationState { + current_screen: CurrentScreen::EditConfig, + }, + lore: LoreUiState { + mailing_list_selection: MailingListSelectionState { + mailing_lists: vec![dummy_list.clone()], + target_list: String::new(), + possible_mailing_lists: vec![dummy_list], + highlighted_list_index: 0, + }, + latest_patchsets: None, + details: None, + }, + user_state: UserLoreState { + bookmarked_patchsets: BookmarkedPatchsetsState { + bookmarked_patchsets: vec![], + patchset_index: 0, + }, + reviewed_patchsets: HashMap::new(), + }, + config_state: ConfigUiState { edit_config: None }, + config: snapshot, + popup: None, + kw: Default::default(), + }, + services: AppServices { + lore_api: LoreApiHandle::new(lore_tx), + render: RenderHandle::new(render_tx), + shell: Box::new(MockShellTrait::new()), + fs: Arc::new(MockFileSystemTrait::new()), + config: config.clone(), + kw_history: Arc::new(MockKwHistoryStore::new()), + kw: None, + }, + }; + (app, config, home) + } + + #[test] + fn help_documents_tree_cycle_and_save_keys() { + let AppPopup::Help { + description, + formatted_keybinds, + .. + } = generate_help_popup() + else { + panic!("expected help popup"); + }; + assert!(description + .as_deref() + .is_some_and(|text| text.contains("selects among existing keys"))); + assert!(formatted_keybinds.contains("ESC / q: Save and exit")); + assert!(formatted_keybinds.contains("ENTER: Edit the highlighted option")); + assert!(formatted_keybinds.contains("←/→: Cycle the target kernel tree")); + assert!(!formatted_keybinds.contains("Save changes")); + } + + #[tokio::test] + async fn cycling_the_tree_row_and_saving_updates_the_snapshot() { + let (mut app, config, home) = app_with_trees(); + app.init_edit_config(); + + for _ in 0..11 { + handle_edit_config(&mut app, InputEvent::NavigateDown) + .await + .unwrap(); + } + handle_edit_config(&mut app, InputEvent::EditConfigField) + .await + .unwrap(); + handle_edit_config(&mut app, InputEvent::NavigateRight) + .await + .unwrap(); + handle_edit_config(&mut app, InputEvent::StageConfigEdit) + .await + .unwrap(); + handle_edit_config(&mut app, InputEvent::SaveConfig) + .await + .unwrap(); + + assert_eq!( + Some("linux"), + app.state.config.target_kernel_tree().as_deref() + ); + assert!(app.state.config_state.edit_config.is_none()); + assert_eq!( + CurrentScreen::MailingListSelection, + app.state.navigation.current_screen + ); + + let raw = fs::read_to_string(home.join(DEFAULT_CONFIG_PATH_SUFFIX)).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&raw).unwrap(); + assert_eq!(parsed["target_kernel_tree"], "linux"); + config.shutdown().await; + } +} diff --git a/src/app/view_model.rs b/src/app/view_model.rs index eddf272..7f48623 100644 --- a/src/app/view_model.rs +++ b/src/app/view_model.rs @@ -125,6 +125,7 @@ pub struct PatchsetDetailsViewModel { pub struct EditConfigViewModel { pub entries: Vec, pub is_editing_mode: bool, + pub editing_tree_selector: bool, } /// KwOps dashboard. Labels only; actions stay in AppState. @@ -417,6 +418,7 @@ fn project_edit_config(state: &AppState) -> EditConfigViewModel { let is_editing_mode = ec.is_editing(); let highlighted = ec.highlighted(); + let editing_tree_selector = is_editing_mode && ec.highlighted_is_tree_selector(); let entries = (0..ec.config_count()) .filter_map(|i| { @@ -424,7 +426,11 @@ fn project_edit_config(state: &AppState) -> EditConfigViewModel { let is_highlighted = i == highlighted; let is_editing = is_editing_mode && is_highlighted; let edit_cursor_value = if is_editing { - ec.curr_edit().to_string() + if editing_tree_selector && ec.curr_edit().is_empty() { + "".to_string() + } else { + ec.curr_edit().to_string() + } } else { String::new() }; @@ -442,6 +448,7 @@ fn project_edit_config(state: &AppState) -> EditConfigViewModel { EditConfigViewModel { entries, is_editing_mode, + editing_tree_selector, } } @@ -844,6 +851,35 @@ mod tests { assert_eq!(1, selected); } + #[test] + fn edit_config_projects_none_placeholder_on_the_tree_row() { + let mut state = app_state_with_kw(None); + state.navigation.current_screen = CurrentScreen::EditConfig; + let mut config = ConfigState::default(); + config.kernel_trees.insert( + "linux".into(), + serde_json::from_value(serde_json::json!({ + "path": "/linux", + "branch": "master" + })) + .unwrap(), + ); + state.config = config.to_snapshot(); + let mut edit = crate::app::screens::edit_config::EditConfigState::new(&state.config); + while edit.highlighted() != 11 { + edit.highlight_next(); + } + edit.toggle_editing(); + state.config_state.edit_config = Some(edit); + + let ScreenViewModel::EditConfig(vm) = project_state(&state).screen else { + panic!("expected EditConfig projection"); + }; + assert!(vm.editing_tree_selector); + assert_eq!("", vm.entries[11].edit_cursor_value); + assert_eq!("", vm.entries[11].value); + } + #[test] fn boot_once_popup_projects_back_out_as_default() { let mut state = app_state_with_kw(None); diff --git a/src/input/mapper.rs b/src/input/mapper.rs index 0120eea..4ea219a 100644 --- a/src/input/mapper.rs +++ b/src/input/mapper.rs @@ -124,6 +124,8 @@ impl InputMapper { return match key.code { KeyCode::Esc => Some(InputEvent::CancelConfigEdit), KeyCode::Backspace => Some(InputEvent::Backspace), + KeyCode::Left => Some(InputEvent::NavigateLeft), + KeyCode::Right => Some(InputEvent::NavigateRight), KeyCode::Char(ch) => Some(InputEvent::TextInput(ch)), KeyCode::Enter => Some(InputEvent::StageConfigEdit), _ => None, @@ -352,6 +354,39 @@ mod tests { assert_eq!(event, Some(InputEvent::EditConfigField)); } + #[test] + fn maps_arrows_to_navigate_while_editing_config_and_keeps_hl_as_text() { + let mut mapper = InputMapper::default(); + let editing = context(CurrentScreen::EditConfig).with_edit_config_editing(true); + + assert_eq!( + mapper.map_terminal_event(key(KeyCode::Left), &editing), + Some(InputEvent::NavigateLeft) + ); + assert_eq!( + mapper.map_terminal_event(key(KeyCode::Right), &editing), + Some(InputEvent::NavigateRight) + ); + assert_eq!( + mapper.map_terminal_event(key(KeyCode::Char('h')), &editing), + Some(InputEvent::TextInput('h')) + ); + assert_eq!( + mapper.map_terminal_event(key(KeyCode::Char('l')), &editing), + Some(InputEvent::TextInput('l')) + ); + + let browsing = context(CurrentScreen::EditConfig); + assert_eq!( + mapper.map_terminal_event(key(KeyCode::Left), &browsing), + None + ); + assert_eq!( + mapper.map_terminal_event(key(KeyCode::Right), &browsing), + None + ); + } + #[test] fn ignores_key_release_events() { let mut mapper = InputMapper::default(); diff --git a/src/ui/screens/edit_config.rs b/src/ui/screens/edit_config.rs index fff8895..455d7d2 100644 --- a/src/ui/screens/edit_config.rs +++ b/src/ui/screens/edit_config.rs @@ -72,7 +72,12 @@ pub fn mode_spans(vm: &EditConfigViewModel) -> Vec> { } pub fn keys_hint_span(vm: &EditConfigViewModel) -> Span<'static> { - if vm.is_editing_mode { + if vm.editing_tree_selector { + Span::styled( + "(←/→) cycle | (ENTER) confirm | (ESC) cancel", + Style::default().fg(Color::Red), + ) + } else if vm.is_editing_mode { Span::styled( "(ESC) cancel | (ENTER) confirm", Style::default().fg(Color::Red), @@ -84,3 +89,31 @@ pub fn keys_hint_span(vm: &EditConfigViewModel) -> Span<'static> { ) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::app::view_model::EditConfigViewModel; + + fn vm(is_editing_mode: bool, editing_tree_selector: bool) -> EditConfigViewModel { + EditConfigViewModel { + entries: vec![], + is_editing_mode, + editing_tree_selector, + } + } + + #[test] + fn keys_hint_shows_cycle_bindings_on_the_tree_row() { + let hint = keys_hint_span(&vm(true, true)); + assert!(hint.content.contains("(←/→) cycle")); + assert!(hint.content.contains("(ENTER) confirm")); + } + + #[test] + fn keys_hint_keeps_text_edit_bindings_on_other_rows() { + let hint = keys_hint_span(&vm(true, false)); + assert!(hint.content.contains("(ESC) cancel")); + assert!(!hint.content.contains("cycle")); + } +} From 6aadc3f9929b8974269f1e2294e9686e7fdd5aae Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 20 Sep 2026 11:46:56 -0300 Subject: [PATCH 4/6] fix(app): do not crash Edit Config on a dangling tree target This commit treats a target_kernel_tree whose key is missing from kernel_trees as unset when opening Edit Config, so saving an unrelated field cannot abort the app. Trees are JSON-only, so that on-disk state is reachable. The repair is logged; surrounding whitespace is trimmed so it matches validation. Unknown-key errors now list known keys, and ESC/q is described as save-and-exit in the nav hint. This commit is part of the kw integration's step 8. Signed-off-by: lorenzoberts --- src/app/flows/edit_config.rs | 51 +++++++++++++++--------- src/app/screens/edit_config.rs | 72 ++++++++++++++++++++++++++-------- src/config/errors.rs | 4 +- src/config/service.rs | 20 +++++++++- src/config/tests.rs | 7 +++- src/ui/screens/edit_config.rs | 8 +++- 6 files changed, 122 insertions(+), 40 deletions(-) diff --git a/src/app/flows/edit_config.rs b/src/app/flows/edit_config.rs index b7b4f26..2c491d0 100644 --- a/src/app/flows/edit_config.rs +++ b/src/app/flows/edit_config.rs @@ -164,25 +164,20 @@ mod tests { (mock, home) } - fn app_with_trees() -> (App, ConfigHandle, PathBuf) { + fn app_with_kernel_trees(keys: &[&str], target: Option<&str>) -> (App, ConfigHandle, PathBuf) { let (env, home) = default_env(); let (mut state, repo) = bootstrap_parts(&env, OsFileSystem).unwrap(); - state.kernel_trees.insert( - "linux".into(), - serde_json::from_value(serde_json::json!({ - "path": "/linux", - "branch": "master" - })) - .unwrap(), - ); - state.kernel_trees.insert( - "zebra".into(), - serde_json::from_value(serde_json::json!({ - "path": "/zebra", - "branch": "master" - })) - .unwrap(), - ); + for key in keys { + state.kernel_trees.insert( + (*key).to_string(), + serde_json::from_value(serde_json::json!({ + "path": format!("/{key}"), + "branch": "master" + })) + .unwrap(), + ); + } + state.target_kernel_tree = target.map(str::to_string); let snapshot = state.to_snapshot(); let config = ConfigActor::spawn(state, repo); @@ -250,7 +245,7 @@ mod tests { #[tokio::test] async fn cycling_the_tree_row_and_saving_updates_the_snapshot() { - let (mut app, config, home) = app_with_trees(); + let (mut app, config, home) = app_with_kernel_trees(&["linux", "zebra"], None); app.init_edit_config(); for _ in 0..11 { @@ -286,4 +281,24 @@ mod tests { assert_eq!(parsed["target_kernel_tree"], "linux"); config.shutdown().await; } + + #[tokio::test] + async fn saving_with_a_dangling_target_unsets_it_instead_of_crashing() { + let (mut app, config, home) = app_with_kernel_trees(&["zebra"], Some("linux")); + assert_eq!( + Some("linux"), + app.state.config.target_kernel_tree().as_deref() + ); + app.init_edit_config(); + + handle_edit_config(&mut app, InputEvent::SaveConfig) + .await + .unwrap(); + + assert!(app.state.config.target_kernel_tree().is_none()); + let raw = fs::read_to_string(home.join(DEFAULT_CONFIG_PATH_SUFFIX)).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&raw).unwrap(); + assert!(parsed["target_kernel_tree"].is_null()); + config.shutdown().await; + } } diff --git a/src/app/screens/edit_config.rs b/src/app/screens/edit_config.rs index 06423e8..d4ac283 100644 --- a/src/app/screens/edit_config.rs +++ b/src/app/screens/edit_config.rs @@ -14,7 +14,7 @@ pub struct EditConfigState { #[getter(skip)] config_buffer: HashMap, #[getter(skip)] - tree_options: Vec, + cycle_options: Vec, highlighted: usize, is_editing: bool, curr_edit: String, @@ -55,17 +55,34 @@ impl EditConfigState { EditableConfig::KwDeployForce, config.kw_deploy_force().to_string(), ); - config_buffer.insert( - EditableConfig::TargetKernelTree, - config.target_kernel_tree().clone().unwrap_or_default(), - ); - let mut tree_options: Vec = config.kernel_trees().into_iter().cloned().collect(); - tree_options.sort(); + let mut keys: Vec = config.kernel_trees().into_iter().cloned().collect(); + keys.sort(); + let raw_target = config + .target_kernel_tree() + .as_ref() + .map(|target| target.trim().to_string()) + .unwrap_or_default(); + let target = if raw_target.is_empty() { + String::new() + } else if keys.iter().any(|key| key == &raw_target) { + raw_target + } else { + tracing::warn!( + target = %raw_target, + "configured target is not a known kernel tree; treating as unset" + ); + String::new() + }; + config_buffer.insert(EditableConfig::TargetKernelTree, target); + + let mut cycle_options = Vec::with_capacity(keys.len() + 1); + cycle_options.push(String::new()); + cycle_options.extend(keys); EditConfigState { config_buffer, - tree_options, + cycle_options, highlighted: 0, is_editing: false, curr_edit: String::new(), @@ -109,22 +126,19 @@ impl EditConfigState { return; } - let mut options = Vec::with_capacity(self.tree_options.len() + 1); - options.push(String::new()); - options.extend(self.tree_options.iter().cloned()); - - let current = options + let current = self + .cycle_options .iter() .position(|key| key == &self.curr_edit) .unwrap_or(0); let next = if forward { - (current + 1) % options.len() + (current + 1) % self.cycle_options.len() } else if current == 0 { - options.len() - 1 + self.cycle_options.len() - 1 } else { current - 1 }; - self.curr_edit = options[next].clone(); + self.curr_edit = self.cycle_options[next].clone(); } /// Toggle editing mode @@ -438,4 +452,30 @@ mod tests { edit.config(11) ); } + + #[test] + fn dangling_target_is_seeded_as_unset() { + let snapshot = snapshot_with_trees(&["zebra"], Some("linux")); + let edit = EditConfigState::new(&snapshot); + let draft = edit.to_update_draft(); + + assert_eq!(Some(String::new()), draft.target_kernel_tree); + assert_eq!( + Some(( + "Target Kernel Tree (ENTER, then ←/→ to cycle)".to_string(), + "".to_string() + )), + edit.config(11) + ); + } + + #[test] + fn padded_target_is_trimmed_to_a_known_key() { + let snapshot = snapshot_with_trees(&["linux"], Some(" linux ")); + let edit = EditConfigState::new(&snapshot); + assert_eq!( + Some("linux".to_string()), + edit.to_update_draft().target_kernel_tree + ); + } } diff --git a/src/config/errors.rs b/src/config/errors.rs index 735c18e..97a0cf9 100644 --- a/src/config/errors.rs +++ b/src/config/errors.rs @@ -22,8 +22,8 @@ pub enum ConfigError { InvalidKwRebootAfterDeploy(String), #[error("invalid kw deploy-force value: {0}")] InvalidKwDeployForce(String), - #[error("unknown kernel tree key '{0}'; edit the config file to add trees")] - InvalidTargetKernelTree(String), + #[error("unknown kernel tree key '{key}'; {hint}")] + InvalidTargetKernelTree { key: String, hint: String }, #[error("filesystem error: {0}")] Fs(#[from] FileSystemError), } diff --git a/src/config/service.rs b/src/config/service.rs index 4a8ef30..cbf38e9 100644 --- a/src/config/service.rs +++ b/src/config/service.rs @@ -178,7 +178,7 @@ pub(crate) fn validate_update( if current.kernel_trees.contains_key(key) { Some(Some(key.to_string())) } else { - return Err(ConfigError::InvalidTargetKernelTree(s.clone())); + return Err(unknown_target_kernel_tree(s, current)); } } }; @@ -199,6 +199,24 @@ pub(crate) fn validate_update( }) } +fn unknown_target_kernel_tree(raw: &str, current: &ConfigState) -> ConfigError { + let mut keys: Vec = current.kernel_trees.keys().cloned().collect(); + keys.sort(); + let hint = if keys.is_empty() { + "no kernel trees are configured; unset the target or add trees in the config file" + .to_string() + } else { + format!( + "known keys: {}; unset the target or pick one of these", + keys.join(", ") + ) + }; + ConfigError::InvalidTargetKernelTree { + key: raw.to_string(), + hint, + } +} + fn validate_dir(fs: &dyn FileSystemTrait, dir_path: &str) -> Result<(), ConfigError> { let path = Path::new(dir_path); if fs.exists(path) && fs.is_dir(path) { diff --git a/src/config/tests.rs b/src/config/tests.rs index 486625e..33ce226 100644 --- a/src/config/tests.rs +++ b/src/config/tests.rs @@ -710,8 +710,11 @@ fn validate_update_rejects_unknown_target_kernel_tree() { .unwrap_err(); assert!(matches!( err, - ConfigError::InvalidTargetKernelTree(ref s) if s == "missing" + ConfigError::InvalidTargetKernelTree { ref key, .. } if key == "missing" )); + let message = err.to_string(); + assert!(message.contains("known keys: amd-gfx, linux")); + assert!(message.contains("unset the target")); } #[test] @@ -865,7 +868,7 @@ async fn invalid_target_kernel_tree_keeps_existing_state() { .unwrap_err(); assert!(matches!( err, - ConfigError::InvalidTargetKernelTree(ref s) if s == "missing" + ConfigError::InvalidTargetKernelTree { ref key, .. } if key == "missing" )); assert_eq!( Some("linux"), diff --git a/src/ui/screens/edit_config.rs b/src/ui/screens/edit_config.rs index 455d7d2..db72efd 100644 --- a/src/ui/screens/edit_config.rs +++ b/src/ui/screens/edit_config.rs @@ -84,7 +84,7 @@ pub fn keys_hint_span(vm: &EditConfigViewModel) -> Span<'static> { ) } else { Span::styled( - "(ESC / q) exit | (ENTER) edit | (jk| 🡇 🡅 ) down up", + "(ESC / q) save and exit | (ENTER) edit | (jk| 🡇 🡅 ) down up", Style::default().fg(Color::Red), ) } @@ -116,4 +116,10 @@ mod tests { assert!(hint.content.contains("(ESC) cancel")); assert!(!hint.content.contains("cycle")); } + + #[test] + fn keys_hint_says_save_and_exit_when_browsing() { + let hint = keys_hint_span(&vm(false, false)); + assert!(hint.content.contains("(ESC / q) save and exit")); + } } From 1a1ef524d7eb63b04dfa4e2eb0cb852d165755d3 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sat, 3 Oct 2026 11:13:20 -0300 Subject: [PATCH 5/6] docs: update README mentioning the kw integration Signed-off-by: lorenzoberts --- README.md | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 4e4f232..53fa05e 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,7 @@ alt="patch-hub-demo-v0.1.0"> - **Apply patch(set)** to your local kernel tree. - **Bookmark** important patches - **Reply with `Reviewed-by` tags** to the series. + - **KwOps** (`w`) — build and remotely deploy the applied tree with `kw`. - **Bookmarking System** — Bookmark patchsets for easy reference later. @@ -98,12 +99,25 @@ alt="patch-hub-demo-v0.1.0"> visualization or use the built-in vanilla renderer (`default`) for a dependency-free experience. -### More Features Coming! +### KwOps (build and deploy with kw) -Future updates will introduce deeper integration with kw, including: +From a patchset's Details screen, press `w` to open **Kw operations** and +build or remotely deploy the applied tree with `kw`. -- Seamlessly compile and deploy patchset versions of the kernel to target - machines. +- **Build** (`b`), **Deploy** (`d`), and **Build then deploy** (`D`). +- Live logs while a job runs — you can leave the screen without freezing the + TUI. +- **Cancel** (`c`) a running job, or **Restore previous branch** (`r`) after + a job switched `HEAD`. Press `?` for in-screen help. + +KwOps uses your existing `kw` setup: [`kw`](https://github.com/kworkflow/kworkflow/) +0.10 or newer on `PATH`, a `.kw/` directory in the kernel tree (`kw init` / +`kw env`), and for deploy a remote plus `kw deploy --setup`. Local deploy is +not available from the UI. Extra `kw` arguments can be typed per run; they +are not saved. + +Related settings in Edit Config / `config.json`: `stay_on_applied_branch`, +`kw_reboot_after_deploy`, and `kw_deploy_force`. ## :package: How To Install From 9412b2e89340b2aa846dfb2c0d853014ae84c988 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sat, 3 Oct 2026 16:18:01 -0300 Subject: [PATCH 6/6] fix(kw): surface failures kw and git hide behind their exit codes This commit reports what kw deploy exits 0 through: initramfs and mkinitcpio failures, and a GRUB update that never listed the new kernel. These show as "succeeded ... with N warnings" on KwOps, and build records and deploy-alone are unchanged. A failed job shows the first error from its log, and every job shows the full log path. With a kw env active, an in-tree .config or include/config/ now makes the tree not ready, since kbuild refuses that O= build. A failed git am deletes the empty branch it created, and the popup names the branch the tree is back on. Signed-off-by: lorenzoberts Co-authored-by: Cursor --- src/app/actions/apply.rs | 65 +++- src/app/integration_tests/patchset_actions.rs | 8 +- src/app/view_model.rs | 152 ++++++++- src/kw/actor.rs | 174 +++++++++- src/kw/argv.rs | 2 +- src/kw/log_scan.rs | 318 ++++++++++++++++++ src/kw/mod.rs | 1 + src/kw/readiness.rs | 52 +++ src/kw/remote.rs | 10 +- src/kw/status.rs | 7 + src/ui/core.rs | 4 +- src/ui/scene.rs | 7 +- src/ui/screens/kw_ops.rs | 20 +- 13 files changed, 780 insertions(+), 40 deletions(-) create mode 100644 src/kw/log_scan.rs diff --git a/src/app/actions/apply.rs b/src/app/actions/apply.rs index bca3849..85c2d32 100644 --- a/src/app/actions/apply.rs +++ b/src/app/actions/apply.rs @@ -58,7 +58,13 @@ pub(crate) fn apply_patchset( } Err(e) => { switch_to_branch(shell, kernel_tree, &original_branch)?; - Err(format!(" `git am` failed\n{}{}", &original_branch, e)) + let cleanup_note = match delete_branch(shell, kernel_tree, &target_branch) { + Ok(()) => String::new(), + Err(error) => format!("\n\n(could not delete '{target_branch}': {error})"), + }; + Err(format!( + " `git am` failed (back on branch '{original_branch}')\n{e}{cleanup_note}" + )) } } } @@ -218,6 +224,30 @@ fn create_target_branch( Ok(target_branch_name) } +/// Deletes a branch `create_target_branch` made for an apply that failed. +/// `-D` because `git am --abort` leaves it at the base commit, which is not +/// necessarily merged into the branch HEAD switched back to. +fn delete_branch( + shell: &dyn ShellTrait, + kernel_tree: &KernelTree, + branch: &str, +) -> Result<(), String> { + let out = shell + .execute( + &ShellCommand::new("git") + .arg("-C") + .arg(kernel_tree.path()) + .args(["branch", "-D", branch]), + ) + .map_err(|e| e.to_string())?; + + if !out.success { + return Err(String::from_utf8_lossy(&out.stderr).trim().to_string()); + } + + Ok(()) +} + fn run_git_am( request: &ApplyPatchsetRequest, shell: &dyn ShellTrait, @@ -515,14 +545,15 @@ mod tests { output("", "apply failed", false), output("", "", true), output("", "", true), + output("", "", true), ]); let result = apply_patchset(&request(), &fs, &shell, &config).unwrap_err(); - assert!(result.contains("`git am` failed")); - assert!(result.contains("feature")); - assert!(result.contains("apply failed")); + assert!(result.starts_with(" `git am` failed (back on branch 'feature')\napply failed")); + assert!(!result.contains("could not delete")); let calls = calls.lock().unwrap(); + assert_eq!(9, calls.len()); assert_eq!( &calls[6], &command(&["git", "-C", KERNEL_TREE_PATH, "am", "--abort"]) @@ -531,6 +562,32 @@ mod tests { &calls[7], &command(&["git", "-C", KERNEL_TREE_PATH, "switch", "feature"]) ); + assert_eq!( + &calls[8], + &command(&["git", "-C", KERNEL_TREE_PATH, "branch", "-D", &calls[4][5]]) + ); } } + + #[test] + fn failed_branch_cleanup_is_noted_without_hiding_the_git_am_error() { + let fs = clean_fs(); + let (shell, _calls) = shell_with_outputs(vec![ + output("", "", true), + output("", "", true), + output("feature\n", "", true), + output("", "", true), + output("", "", true), + output("", "apply failed", false), + output("", "", true), + output("", "", true), + output("", "error: branch is locked\n", false), + ]); + + let result = apply_patchset(&request(), &fs, &shell, &config()).unwrap_err(); + + assert!(result.starts_with(" `git am` failed (back on branch 'feature')\napply failed")); + assert!(result.contains("could not delete 'patchset-")); + assert!(result.contains("error: branch is locked)")); + } } diff --git a/src/app/integration_tests/patchset_actions.rs b/src/app/integration_tests/patchset_actions.rs index e5fd293..881d1b8 100644 --- a/src/app/integration_tests/patchset_actions.rs +++ b/src/app/integration_tests/patchset_actions.rs @@ -121,6 +121,7 @@ async fn apply_failure_sets_failure_popup_and_resets_apply_action() { output("", "apply failed", false), output("", "", true), output("", "", true), + output("", "", true), ]); let mut app = app_with_apply_details(clean_fs(), shell); @@ -130,7 +131,7 @@ async fn apply_failure_sets_failure_popup_and_resets_apply_action() { assert_info_popup_contains( app.state.popup.as_ref(), "Patchset Apply Fail", - &["`git am` failed", "feature", "apply failed"], + &["`git am` failed (back on branch 'feature')", "apply failed"], ); { let calls = calls.lock().unwrap(); @@ -138,6 +139,10 @@ async fn apply_failure_sets_failure_popup_and_resets_apply_action() { command(&["git", "-C", KERNEL_TREE_PATH, "am", "--abort"]), calls[6] ); + assert_eq!( + &calls[8][..5], + command(&["git", "-C", KERNEL_TREE_PATH, "branch", "-D"]) + ); } shutdown_kw(&app).await; } @@ -235,6 +240,7 @@ async fn apply_failure_does_not_record_history() { output("", "apply failed", false), output("", "", true), output("", "", true), + output("", "", true), ]); let mut kw_history = MockKwHistoryStore::new(); kw_history.expect_record_apply().times(0); diff --git a/src/app/view_model.rs b/src/app/view_model.rs index 7f48623..eb2a37a 100644 --- a/src/app/view_model.rs +++ b/src/app/view_model.rs @@ -154,6 +154,12 @@ pub struct KwOpsViewModel { pub deploy_label: String, pub build_deploy_label: String, pub branch_guidance: Option, + /// Deploy warnings of the last job, joined with ` | `. + pub warnings: Option, + /// First error line of the last failed job's log. + pub first_error: Option, + /// Full log of the current or last job; the pane only shows its tail. + pub log_path: Option, pub log_tail: String, } @@ -193,7 +199,9 @@ pub enum ScreenViewModel { Latest(LatestPatchsetsViewModel), PatchsetDetails(PatchsetDetailsViewModel), EditConfig(EditConfigViewModel), - KwOps(KwOpsViewModel), + // Boxed to keep the enum small (clippy::large_enum_variant): KwOps + // carries one owned string per dashboard row. + KwOps(Box), } /// Owned, typed projection of [`AppState`] for one TUI frame. @@ -236,7 +244,7 @@ fn project_screen(state: &AppState) -> ScreenViewModel { CurrentScreen::LatestPatchsets => ScreenViewModel::Latest(project_latest(state)), CurrentScreen::PatchsetDetails => ScreenViewModel::PatchsetDetails(project_details(state)), CurrentScreen::EditConfig => ScreenViewModel::EditConfig(project_edit_config(state)), - CurrentScreen::KwOps => ScreenViewModel::KwOps(project_kw_ops(state)), + CurrentScreen::KwOps => ScreenViewModel::KwOps(Box::new(project_kw_ops(state))), } } @@ -512,6 +520,17 @@ fn project_kw_ops(state: &AppState) -> KwOpsViewModel { (false, Some(branch)) => format!("available (r) to {branch}"), (false, None) => "unavailable".to_string(), }; + let job = state.kw.status.as_ref().map(|status| &status.job); + let (warnings, first_error) = match job { + Some(KwJobStatus::Succeeded { warnings, .. }) if !warnings.is_empty() => { + (Some(warnings.join(" | ")), None) + } + Some(KwJobStatus::Failed { first_error, .. }) => (None, first_error.clone()), + _ => (None, None), + }; + let log_path = job + .and_then(KwJobStatus::log_path) + .map(|path| path.display().to_string()); let log_tail = if ops.log_tail.is_empty() { if running { "Waiting for kw output…".to_string() @@ -542,10 +561,7 @@ fn project_kw_ops(state: &AppState) -> KwOpsViewModel { job_status: if start_requested && !running { "starting…".to_string() } else { - format_job_status( - state.kw.status.as_ref().map(|status| &status.job), - ops.cancel_requested, - ) + format_job_status(job, ops.cancel_requested) }, command: format!( "kw {}", @@ -572,6 +588,9 @@ fn project_kw_ops(state: &AppState) -> KwOpsViewModel { } else { None }, + warnings, + first_error, + log_path, log_tail, } } @@ -681,7 +700,13 @@ fn format_job_status(job: Option<&KwJobStatus>, cancel_requested: bool) -> Strin format!("{phase} {branch}") } } - Some(KwJobStatus::Succeeded { branch, .. }) => format!("succeeded on {branch}"), + Some(KwJobStatus::Succeeded { + branch, warnings, .. + }) => match warnings.len() { + 0 => format!("succeeded on {branch}"), + 1 => format!("succeeded on {branch} with 1 warning"), + count => format!("succeeded on {branch} with {count} warnings"), + }, Some(KwJobStatus::Failed { phase: KwPhase::Deploying, exit_code, @@ -1129,10 +1154,12 @@ mod tests { fn deploy_command_follows_reboot_and_force_config() { let mut state = app_state_with_kw(None); state.navigation.current_screen = CurrentScreen::KwOps; - let mut config = ConfigState::default(); - config.kw_reboot_after_deploy = true; - config.kw_deploy_force = false; - state.config = config.to_snapshot(); + state.config = ConfigState { + kw_reboot_after_deploy: true, + kw_deploy_force: false, + ..Default::default() + } + .to_snapshot(); let mut ops = sample_kw_ops(Some("feature")); ops.readiness.deploy_remote = Ok(sample_remote()); state.kw.ops = Some(ops); @@ -1151,6 +1178,7 @@ mod tests { phase: KwPhase::Deploying, exit_code: Some(101), log_path: PathBuf::from("/tmp/deploy.log"), + first_error: None, }, restore_branch: None, })); @@ -1176,6 +1204,7 @@ mod tests { phase: KwPhase::Building, exit_code: Some(1), log_path: PathBuf::from("/tmp/build.log"), + first_error: None, }, restore_branch: None, })); @@ -1186,6 +1215,106 @@ mod tests { panic!("expected KwOps projection"); }; assert_eq!("failed (exit 1)", vm.job_status); + assert_eq!(None, vm.first_error); + assert_eq!(Some("/tmp/build.log".to_string()), vm.log_path); + } + + #[test] + fn failed_build_projects_first_error_and_log_path() { + let mut state = app_state_with_kw(Some(KwStatusSnapshot { + job: KwJobStatus::Failed { + kind: KwJobKind::Build, + phase: KwPhase::Building, + exit_code: Some(2), + log_path: PathBuf::from("/tmp/build.log"), + first_error: Some("init/main.c:1:2: error: #error broken".to_string()), + }, + restore_branch: None, + })); + state.navigation.current_screen = CurrentScreen::KwOps; + state.kw.ops = Some(sample_kw_ops(Some("feature"))); + + let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { + panic!("expected KwOps projection"); + }; + assert_eq!("failed (exit 2)", vm.job_status); + assert_eq!( + Some("init/main.c:1:2: error: #error broken".to_string()), + vm.first_error + ); + assert_eq!(Some("/tmp/build.log".to_string()), vm.log_path); + assert_eq!(None, vm.warnings); + } + + fn succeeded_deploy(warnings: &[&str]) -> KwStatusSnapshot { + KwStatusSnapshot { + job: KwJobStatus::Succeeded { + kind: KwJobKind::Deploy, + kernel_tree_id: "mainline".to_string(), + branch: "patchset-x".to_string(), + log_path: PathBuf::from("/tmp/deploy.log"), + warnings: warnings.iter().map(|w| w.to_string()).collect(), + }, + restore_branch: None, + } + } + + #[test] + fn deploy_with_warnings_projects_count_and_joined_warnings() { + let mut state = app_state_with_kw(Some(succeeded_deploy(&[ + "update-initramfs: failed for /boot/initrd.img-7.2.0 with 1.", + "GRUB did not list kernel 7.2.0", + ]))); + state.navigation.current_screen = CurrentScreen::KwOps; + state.kw.ops = Some(sample_kw_ops(Some("feature"))); + + let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { + panic!("expected KwOps projection"); + }; + assert_eq!("succeeded on patchset-x with 2 warnings", vm.job_status); + assert_eq!( + Some( + "update-initramfs: failed for /boot/initrd.img-7.2.0 with 1. | \ + GRUB did not list kernel 7.2.0" + .to_string() + ), + vm.warnings + ); + assert_eq!(None, vm.first_error); + assert_eq!(Some("/tmp/deploy.log".to_string()), vm.log_path); + } + + #[test] + fn clean_success_projects_no_warning_rows() { + let mut state = app_state_with_kw(Some(succeeded_deploy(&[]))); + state.navigation.current_screen = CurrentScreen::KwOps; + state.kw.ops = Some(sample_kw_ops(Some("feature"))); + + let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { + panic!("expected KwOps projection"); + }; + assert_eq!("succeeded on patchset-x", vm.job_status); + assert_eq!(None, vm.warnings); + + state.kw.status = Some(succeeded_deploy(&["GRUB did not list kernel 7.2.0"])); + let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { + panic!("expected KwOps projection"); + }; + assert_eq!("succeeded on patchset-x with 1 warning", vm.job_status); + } + + #[test] + fn idle_job_projects_no_log_path() { + let mut state = app_state_with_kw(Some(KwStatusSnapshot::idle())); + state.navigation.current_screen = CurrentScreen::KwOps; + state.kw.ops = Some(sample_kw_ops(Some("feature"))); + + let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { + panic!("expected KwOps projection"); + }; + assert_eq!(None, vm.log_path); + assert_eq!(None, vm.warnings); + assert_eq!(None, vm.first_error); } #[test] @@ -1203,6 +1332,7 @@ mod tests { kernel_tree_id: "mainline".to_string(), branch: "patchset-x".to_string(), log_path: PathBuf::from("/tmp/build.log"), + warnings: Vec::new(), }, restore_branch: Some("master".to_string()), }))) diff --git a/src/kw/actor.rs b/src/kw/actor.rs index 674bc05..e2bfa2b 100644 --- a/src/kw/actor.rs +++ b/src/kw/actor.rs @@ -38,6 +38,7 @@ use crate::{ errors::{KwError, KwStartError, TreeGitError}, handle::KwHandle, history::{KwBuildRecord, KwHistoryStore}, + log_scan, messages::{DeployOptions, KwMessage, StartRequest}, readiness::{self, BootOnceState, KwReadiness, KwVersionCheck, TreeReadiness}, remote::{self, KwRemote}, @@ -109,6 +110,10 @@ struct JobDeploy { remote: KwRemote, options: DeployOptions, extra_args: Vec, + /// Release of the kernel being deployed, from the build record, so + /// the deploy log can be checked for a GRUB menu that never listed + /// it. `None` until a BuildThenDeploy build has been recorded. + kernelrelease: Option, } pub struct KwActor { @@ -615,9 +620,23 @@ impl KwActor { self.record_build_outcome(&job, &outcome); let status = match outcome { JobOutcome::Exited(exit) if exit.success() => { + // kw deploy exits 0 through initramfs and GRUB + // failures; only its log tells. + let warnings = if job.phase == KwPhase::Deploying { + let kernelrelease = job + .deploy + .as_ref() + .and_then(|deploy| deploy.kernelrelease.as_deref()); + self.read_job_log(&job.log_path) + .map(|log| log_scan::deploy_warnings(&log, kernelrelease)) + .unwrap_or_default() + } else { + Vec::new() + }; tracing::info!( kernel_tree_id = job.kernel_tree_id, branch = job.branch, + warnings = warnings.len(), "kw job succeeded" ); KwJobStatus::Succeeded { @@ -625,6 +644,7 @@ impl KwActor { kernel_tree_id: job.kernel_tree_id, branch: job.branch, log_path: job.log_path, + warnings, } } JobOutcome::Exited(exit) => { @@ -638,6 +658,9 @@ impl KwActor { kind: job.kind, phase: job.phase, exit_code: exit.code(), + first_error: self + .read_job_log(&job.log_path) + .and_then(|log| log_scan::first_error(&log)), log_path: job.log_path, } } @@ -647,6 +670,9 @@ impl KwActor { kind: job.kind, phase: job.phase, exit_code: None, + first_error: self + .read_job_log(&job.log_path) + .and_then(|log| log_scan::first_error(&log)), log_path: job.log_path, } } @@ -689,17 +715,19 @@ impl KwActor { // Durable before the phase flips: a deploy spawn failure must not // lose the successful build, and KwOps watching Deploying must // already see the record. - self.record_build_outcome(&job, outcome); - let Some(deploy) = job.deploy.clone() else { + let kernelrelease = self.record_build_outcome(&job, outcome); + let Some(mut deploy) = job.deploy.clone() else { tracing::error!("BuildThenDeploy missing deploy context after a successful build"); self.set_status(KwJobStatus::Failed { kind: job.kind, phase: KwPhase::Deploying, exit_code: None, log_path: job.log_path, + first_error: None, }); return ControlFlow::Break(()); }; + deploy.kernelrelease = kernelrelease; match self.spawn_deploy_process(&job.tree_path, &deploy) { Ok((process, log_path)) => { let (cancel_tx, cancel_rx) = oneshot::channel(); @@ -720,6 +748,7 @@ impl KwActor { let mut job = job; job.phase = KwPhase::Deploying; job.log_path = log_path; + job.deploy = Some(deploy); job.cancel_tx = Some(cancel_tx); self.job = Some(job); ControlFlow::Break(()) @@ -735,6 +764,7 @@ impl KwActor { phase: KwPhase::Deploying, exit_code: None, log_path: job.log_path, + first_error: None, }); ControlFlow::Break(()) } @@ -744,16 +774,17 @@ impl KwActor { /// Persist a finished build (success or failure). Cancel writes /// nothing. History and patchset-link errors are logged, not folded /// into job status — the build's real outcome already reached the user. - fn record_build_outcome(&self, job: &JobState, outcome: &JobOutcome) { + /// Returns the recorded kernelrelease, if the build produced one. + fn record_build_outcome(&self, job: &JobState, outcome: &JobOutcome) -> Option { if job.kind == KwJobKind::Deploy || job.phase == KwPhase::Deploying { - return; + return None; } let success = match outcome { JobOutcome::Exited(exit) => exit.success(), // The exit status is lost: record an honest failure rather // than guess, so deploy-alone readiness cannot trust it. JobOutcome::WaitFailed(_) => false, - JobOutcome::Cancelled => return, + JobOutcome::Cancelled => return None, }; // The record describes the accept-time snapshot: the build ran @@ -802,7 +833,7 @@ impl KwActor { .output_dir .as_ref() .map(|path| path.to_string_lossy().into_owned()), - kernelrelease, + kernelrelease: kernelrelease.clone(), log_path: job.log_path.to_string_lossy().into_owned(), built_at: chrono::Utc::now().to_rfc3339(), success, @@ -810,6 +841,20 @@ impl KwActor { if let Err(error) = self.history.record_build(record) { tracing::warn!(%error, branch = job.branch, "failed to record kw build history"); } + kernelrelease + } + + /// Reads a finished job's log for the findings its terminal status + /// carries. An unreadable log only loses those findings: the exit code + /// is still the verdict. + fn read_job_log(&self, log_path: &Path) -> Option { + match self.fs.read_to_string(log_path) { + Ok(log) => Some(log), + Err(error) => { + tracing::warn!(%error, log_path = %log_path.display(), "failed to read kw job log"); + None + } + } } fn set_status(&mut self, job: KwJobStatus) { @@ -928,6 +973,7 @@ fn prepare_deploy_blocking( }); let remote = remote::resolve_deploy_remote(fs, env, tree_path) .map_err(KwStartError::RemoteUnresolved)?; + let mut kernelrelease = None; if kind == KwJobKind::Deploy { let (record, latest) = history.build_records(&request.kernel_tree_id, &request.branch)?; let image = readiness::find_newest_kernel_image(fs, output_dir.unwrap_or(tree_path), arch); @@ -940,6 +986,7 @@ fn prepare_deploy_blocking( image.as_deref(), ) .map_err(KwStartError::DeployAloneRefused)?; + kernelrelease = record.and_then(|record| record.kernelrelease); } let boot_once = readiness::probe_boot_once(fs, env, tree_path); if matches!(boot_once, BootOnceState::On | BootOnceState::Unknown) @@ -951,6 +998,7 @@ fn prepare_deploy_blocking( remote, options, extra_args: request.extra_args.clone(), + kernelrelease, }) } @@ -1397,11 +1445,13 @@ mod tests { fs.expect_is_file() .returning(|path| !path.ends_with(".kw/env.current")); fs.expect_exists().returning(|_| true); - fs.expect_read_to_string().returning(|_| { - Err(FileSystemError::IoError(io::Error::new( - io::ErrorKind::NotFound, - "missing", - ))) + fs.expect_read_to_string().returning(|path| { + read_real_job_log(path).unwrap_or_else(|| { + Err(FileSystemError::IoError(io::Error::new( + io::ErrorKind::NotFound, + "missing", + ))) + }) }); fs.expect_read_dir().returning(|_| { Err(FileSystemError::IoError(io::Error::new( @@ -1411,6 +1461,14 @@ mod tests { }); } + /// Job logs are real files ([`FakeProcess`] creates them and + /// `write_log` appends to them), so fs doubles read `*.log` paths from + /// disk. + fn read_real_job_log(path: &Path) -> Option> { + (path.extension() == Some("log".as_ref())) + .then(|| std::fs::read_to_string(path).map_err(FileSystemError::from)) + } + /// A ready kernel tree whose log dir can be created. fn ready_fs() -> MockFileSystemTrait { let mut fs = MockFileSystemTrait::new(); @@ -1480,7 +1538,9 @@ mod tests { .returning(|path| !path.ends_with(".kw/env.current")); fs.expect_exists().returning(|_| true); fs.expect_read_to_string().returning(move |path| { - if path.ends_with("build.config") { + if let Some(log) = read_real_job_log(path) { + log + } else if path.ends_with("build.config") { Ok("arch=x86\n".to_string()) } else if path.ends_with("kernel.release") { Ok("6.17.0\n".to_string()) @@ -1981,6 +2041,11 @@ mod tests { let mut watch = handle.watch_status().await.unwrap(); handle.start_build(start_request()).await.unwrap(); + process.last_child().write_log( + b" CC init/main.o\n\ + init/main.c:1691:2: error: #error broken\n\ + make: *** [Makefile:248: __sub-make] Error 2\n", + ); process.last_child().finish(2); let status = wait_for_terminal_status(&mut watch).await; @@ -1991,11 +2056,16 @@ mod tests { phase, exit_code, log_path, + first_error, } => { assert_eq!(KwJobKind::Build, kind); assert_eq!(KwPhase::Building, phase); assert_eq!(Some(2), exit_code); assert!(log_path.starts_with(&log_dir)); + assert_eq!( + Some("init/main.c:1691:2: error: #error broken"), + first_error.as_deref() + ); } other => panic!("expected Failed, got {other:?}"), } @@ -2354,6 +2424,78 @@ mod tests { std::fs::remove_dir_all(&log_dir).unwrap(); } + #[tokio::test] + async fn deploy_exit_zero_with_initramfs_failure_succeeds_with_warnings() { + let (handle, process, log_dir) = spawn_deploy_actor( + "deploy-warnings", + deploy_history(Some(matching_build_record())), + deploy_ready_fs(), + ); + let mut watch = handle.watch_status().await.unwrap(); + + handle.start_deploy(deploy_request()).await.unwrap(); + process.last_child().write_log( + b"update-initramfs: Generating /boot/initrd.img-6.17.0\n\ + E: gzip compression (CONFIG_RD_GZIP) not supported by kernel\n\ + update-initramfs: failed for /boot/initrd.img-6.17.0 with 1.\n\ + Generating grub configuration file ...\n\ + Found linux image: /boot/vmlinuz-6.8.0-generic\n\ + done\n", + ); + process.last_child().finish(0); + + match wait_for_terminal_status(&mut watch).await { + KwJobStatus::Succeeded { kind, warnings, .. } => { + assert_eq!(KwJobKind::Deploy, kind); + assert_eq!( + vec![ + "update-initramfs: failed for /boot/initrd.img-6.17.0 with 1.".to_string(), + "GRUB did not list kernel 6.17.0".to_string(), + ], + warnings + ); + } + other => panic!("expected Succeeded, got {other:?}"), + } + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn build_then_deploy_checks_grub_for_the_release_it_just_built() { + let (handle, process, log_dir) = + spawn_deploy_actor("chain-warnings", quiet_history(), deploy_ready_fs()); + let mut watch = handle.watch_status().await.unwrap(); + + handle + .start_build_then_deploy(deploy_request()) + .await + .unwrap(); + process.last_child().finish(0); + wait_for_running_phase(&mut watch, KwPhase::Deploying).await; + process.last_child().write_log( + b"Generating grub configuration file ...\n\ + Found linux image: /boot/vmlinuz-6.8.0-generic\n\ + Found linux image: /boot/Image-6.17.0-rc1\n\ + done\n", + ); + process.last_child().finish(0); + + match wait_for_terminal_status(&mut watch).await { + KwJobStatus::Succeeded { warnings, .. } => { + assert_eq!( + vec!["GRUB did not list kernel 6.17.0".to_string()], + warnings + ); + } + other => panic!("expected Succeeded, got {other:?}"), + } + + handle.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + #[tokio::test] async fn start_deploy_merges_extras_and_follows_reboot_force_options() { let (handle, process, log_dir) = spawn_deploy_actor( @@ -3428,8 +3570,12 @@ mod tests { let env_current_reads = Arc::new(AtomicU64::new(0)); let env_current_reads_in_fs = Arc::clone(&env_current_reads); let mut fs = MockFileSystemTrait::new(); - fs.expect_is_dir().returning(|_| true); - fs.expect_is_file().returning(|_| true); + // With an env active the build artifacts live only under O=; the + // source tree is clean. + fs.expect_is_dir() + .returning(|path| path != Path::new("/home/user/linux/include/config")); + fs.expect_is_file() + .returning(|path| path != Path::new("/home/user/linux/.config")); fs.expect_exists().returning(|_| true); fs.expect_read_to_string().returning(move |path| { if path.ends_with("env.current") { diff --git a/src/kw/argv.rs b/src/kw/argv.rs index d2ce4b5..6b398f9 100644 --- a/src/kw/argv.rs +++ b/src/kw/argv.rs @@ -267,7 +267,7 @@ fn reserved_short_cluster<'a>( fn exact_short<'a>(reserved: &'a [ReservedOption], spelling: &str) -> Option<&'a ReservedOption> { reserved .iter() - .find(|option| option.spellings.iter().any(|s| *s == spelling)) + .find(|option| option.spellings.contains(&spelling)) } fn value_is_attached(token: &str, option: &ReservedOption) -> bool { diff --git a/src/kw/log_scan.rs b/src/kw/log_scan.rs new file mode 100644 index 0000000..bc2fc69 --- /dev/null +++ b/src/kw/log_scan.rs @@ -0,0 +1,318 @@ +//! Scans a finished job's log for what the exit code alone does not say: +//! the first real error of a failed job, and the known non-fatal failures +//! `kw deploy` exits 0 through. +//! +//! Pure functions over the log text; the actor reads the file. + +// The only production caller is the unix-only actor. +#![cfg_attr(not(unix), allow(dead_code))] + +/// Longer lines (deep include paths, long make targets) are cut so one +/// finding stays a single KwOps row. +const MAX_LINE_CHARS: usize = 200; +const MAX_DEPLOY_WARNINGS: usize = 5; + +/// Lines `kw deploy` prints for a failure it does not turn into a non-zero +/// exit: initramfs generation (Debian `update-initramfs`, Arch +/// `mkinitcpio`) and kw's own bootloader/boot-once setup. +const DEPLOY_WARNING_MARKERS: [&str; 4] = [ + "update-initramfs: failed", + "==> ERROR:", + "kw was unable to set up the first boot", + "kw did not find grub.cfg", +]; + +/// The first line of a failed job's log that names what went wrong: a +/// compiler/linker `error:`, a modpost `ERROR:`, a kbuild `*** ...` +/// banner (e.g. "The source tree is not clean"), or a git/kw `error:`. +/// Falls back to the last `make: *** ... Error N` line, which only names +/// the failing target. Kbuild prints the real cause first and a cascade +/// of make errors after it, so the first match is the useful one. +pub fn first_error(log: &str) -> Option { + log.lines() + .map(str::trim) + .find(|line| is_error_line(line)) + .or_else(|| log.lines().map(str::trim).rfind(|line| is_make_error(line))) + .map(cap_line) +} + +/// Known failures in a deploy log that `kw deploy` exits 0 through, at +/// most [`MAX_DEPLOY_WARNINGS`]. With `kernelrelease`, a GRUB update that +/// did not list that kernel is reported too: kw installs arm64 kernels as +/// `Image-`, which Debian's `10_linux` does not pick up, so the +/// deployed kernel is never bootable from the menu. +pub fn deploy_warnings(log: &str, kernelrelease: Option<&str>) -> Vec { + let mut warnings: Vec = Vec::new(); + for line in log.lines().map(str::trim) { + if DEPLOY_WARNING_MARKERS + .iter() + .any(|marker| line.contains(marker)) + { + let line = cap_line(line); + if !warnings.contains(&line) { + warnings.push(line); + } + } + } + let unlisted = kernelrelease.filter(|release| grub_missed_release(log, release)); + warnings.truncate(MAX_DEPLOY_WARNINGS - usize::from(unlisted.is_some())); + if let Some(release) = unlisted { + warnings.push(format!("GRUB did not list kernel {release}")); + } + warnings +} + +fn is_error_line(line: &str) -> bool { + line.contains("fatal error:") + || line.contains(" error:") + || line.starts_with("error:") + || line.starts_with("ERROR:") + || line + .strip_prefix("*** ") + .is_some_and(|banner| !banner.trim().is_empty()) +} + +/// `make[N]: *** [target] Error N`, the summary make prints per failing +/// level. +fn is_make_error(line: &str) -> bool { + line.starts_with("make") + && line.contains(": *** ") + && line + .rsplit_once("Error ") + .is_some_and(|(_, code)| !code.is_empty() && code.chars().all(|c| c.is_ascii_digit())) +} + +/// GRUB ran (`Generating grub configuration file`) but no `Found linux +/// image:` line names an image for `release`. Matched on the `-` +/// suffix so `7.2.0-rc6` is not satisfied by `Image-7.2.0-rc6-phboot`. +fn grub_missed_release(log: &str, release: &str) -> bool { + if !log.contains("Generating grub configuration file") { + return false; + } + let suffix = format!("-{release}"); + !log.lines().map(str::trim).any(|line| { + line.strip_prefix("Found linux image:") + .is_some_and(|image| image.trim().ends_with(&suffix)) + }) +} + +fn cap_line(line: &str) -> String { + match line.char_indices().nth(MAX_LINE_CHARS) { + Some((index, _)) => format!("{}…", &line[..index]), + None => line.to_string(), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// `kw build` under an env `O=` with in-tree leftovers in the source + /// tree (lab/logs/20260920-122005). + const UNCLEAN_TREE_BUILD_LOG: &str = "\ +/boot/config-6.8.0-138-generic:883:warning: symbol value '0' invalid for BASE_SMALL + SYNC include/config/auto.conf.cmd +*** +*** The source tree is not clean, please run 'make ARCH=arm64 mrproper' +*** in /opt/ph-lab/work/linux +*** +make[3]: *** [/opt/ph-lab/work/linux/Makefile:709: outputmakefile] Error 1 +make[2]: *** [/opt/ph-lab/work/linux/Makefile:906: include/config/auto.conf.cmd] Error 2 +make[2]: *** [include/config/auto.conf.cmd] Deleting file 'include/generated/rustc_cfg' +make[2]: *** [include/config/auto.conf.cmd] Deleting file 'include/generated/autoconf.h' +make[1]: *** [/opt/ph-lab/work/linux/Makefile:248: __sub-make] Error 2 +make[1]: Leaving directory '/opt/ph-lab/work/out' +"; + + /// The M4 fixture's compile break (lab/logs/20260920-151635/M18). + const COMPILE_ERROR_BUILD_LOG: &str = "\ + CC mm/filemap.o + AS arch/arm64/kernel/entry.o +/opt/ph-lab/work/linux/init/main.c:1691:2: error: #error M4 broken-build fixture + 1691 | #error M4 broken-build fixture + | ^~~~~ +make[4]: *** [/opt/ph-lab/work/linux/scripts/Makefile.build:289: init/main.o] Error 1 +make[3]: *** [/opt/ph-lab/work/linux/scripts/Makefile.build:549: init] Error 2 +make[3]: *** Waiting for unfinished jobs.... + CC kernel/exec_domain.o +make: *** [Makefile:248: __sub-make] Error 2 +"; + + /// tinyconfig deploy to the Ubuntu DUT: kw exits 0 although + /// initramfs generation failed and GRUB never listed the kernel + /// (lab/logs/20261003-111949). + const TINYCONFIG_DEPLOY_LOG: &str = "\ +cp /tmp/kw/kw_pkg/Image-7.2.0-rc6+ /boot/ +generate_debian_temporary_root_file_system VERBOSE 7.2.0-rc6+ remote GRUB +update-initramfs -c -k 7.2.0-rc6+ +update-initramfs: Generating /boot/initrd.img-7.2.0-rc6+ +W: zstd compression (CONFIG_RD_ZSTD) not supported by kernel, using gzip +E: gzip compression (CONFIG_RD_GZIP) not supported by kernel +update-initramfs: failed for /boot/initrd.img-7.2.0-rc6+ with 1. +update-grub +Sourcing file `/etc/default/grub' +Generating grub configuration file ... +Found linux image: /boot/vmlinuz-6.8.0-137-generic +Found initrd image: /boot/initrd.img-6.8.0-137-generic +Found linux image: /boot/vmlinuz-6.8.0-136-generic +Found initrd image: /boot/initrd.img-6.8.0-136-generic +Adding boot menu entry for UEFI Firmware Settings ... +done +"; + + /// The real-boot deploy of the tipc patch: everything worked + /// (lab/logs/20261003-154745-real-boot-tui-tipc). + const CLEAN_DEPLOY_LOG: &str = "\ +* Preparing modules +cp: cannot stat '/home/lima.guest/.cache/kw/envs/L29wdC9waC1sYWIvd29yay9saW51eA==/ph-boot/arch/arm64/boot/dts/*.dtb': No such file or directory +* Sending kernel package (7.2.0-rc6-phboot-g2a475abe5df2.kw.tar) to the remote +update-initramfs: Generating /boot/initrd.img-7.2.0-rc6-phboot-g2a475abe5df2 +Ignoring old or unknown version 7.2.0-rc6-phboot-g2a475abe5df2 (latest is 6.8.0-137-generic) +Generating grub configuration file ... +Found linux image: /boot/vmlinuz-6.8.0-137-generic +Found initrd image: /boot/initrd.img-6.8.0-137-generic +Found linux image: /boot/Image-7.2.0-rc6-phboot-g2a475abe5df2 +Found initrd image: /boot/initrd.img-7.2.0-rc6-phboot-g2a475abe5df2 +Found linux image: /boot/Image-7.2.0-rc6-phboot+ +Found linux image: /boot/Image-7.2.0-rc6+ +done +"; + + #[test] + fn first_error_names_the_kbuild_banner_not_the_make_cascade() { + assert_eq!( + Some( + "*** The source tree is not clean, please run 'make ARCH=arm64 mrproper'" + .to_string() + ), + first_error(UNCLEAN_TREE_BUILD_LOG) + ); + } + + #[test] + fn first_error_names_the_compiler_error() { + assert_eq!( + Some( + "/opt/ph-lab/work/linux/init/main.c:1691:2: error: #error M4 broken-build fixture" + .to_string() + ), + first_error(COMPILE_ERROR_BUILD_LOG) + ); + } + + #[test] + fn first_error_recognizes_modpost_and_git_errors() { + assert_eq!( + Some("ERROR: modpost: \"foo\" [drivers/bar.ko] undefined!".to_string()), + first_error( + " MODPOST Module.symvers\nERROR: modpost: \"foo\" [drivers/bar.ko] undefined!\n" + ) + ); + assert_eq!( + Some("error: pathspec 'nope' did not match any file(s) known to git".to_string()), + first_error("error: pathspec 'nope' did not match any file(s) known to git\n") + ); + assert_eq!( + Some("drivers/foo.c:1:10: fatal error: bar.h: No such file or directory".to_string()), + first_error("drivers/foo.c:1:10: fatal error: bar.h: No such file or directory\n") + ); + } + + #[test] + fn first_error_falls_back_to_the_last_make_error() { + let log = "\ + CC init/main.o +make[2]: *** [scripts/Makefile.build:549: certs] Error 2 +make[2]: *** Waiting for unfinished jobs.... +make: *** [Makefile:248: __sub-make] Error 2 +"; + assert_eq!( + Some("make: *** [Makefile:248: __sub-make] Error 2".to_string()), + first_error(log) + ); + } + + #[test] + fn first_error_is_none_without_any_error_line() { + assert_eq!(None, first_error("")); + assert_eq!(None, first_error(" CC init/main.o\n***\n")); + } + + #[test] + fn first_error_caps_long_lines() { + let line = format!("drivers/x.c:1:1: error: {}", "y".repeat(400)); + let found = first_error(&line).unwrap(); + assert_eq!(MAX_LINE_CHARS + 1, found.chars().count()); + assert!(found.ends_with('…')); + } + + #[test] + fn deploy_warnings_catch_initramfs_failure_and_unlisted_kernel() { + assert_eq!( + vec![ + "update-initramfs: failed for /boot/initrd.img-7.2.0-rc6+ with 1.".to_string(), + "GRUB did not list kernel 7.2.0-rc6+".to_string(), + ], + deploy_warnings(TINYCONFIG_DEPLOY_LOG, Some("7.2.0-rc6+")) + ); + } + + #[test] + fn deploy_warnings_skip_the_grub_check_without_a_release() { + assert_eq!( + vec!["update-initramfs: failed for /boot/initrd.img-7.2.0-rc6+ with 1.".to_string()], + deploy_warnings(TINYCONFIG_DEPLOY_LOG, None) + ); + } + + #[test] + fn clean_deploy_has_no_warnings() { + assert!( + deploy_warnings(CLEAN_DEPLOY_LOG, Some("7.2.0-rc6-phboot-g2a475abe5df2")).is_empty() + ); + } + + #[test] + fn grub_check_needs_an_exact_release_suffix() { + // `Image-7.2.0-rc6-phboot+` must not satisfy release `7.2.0-rc6`. + let log = "\ +Generating grub configuration file ... +Found linux image: /boot/Image-7.2.0-rc6-phboot+ +done +"; + assert_eq!( + vec!["GRUB did not list kernel 7.2.0-rc6".to_string()], + deploy_warnings(log, Some("7.2.0-rc6")) + ); + } + + #[test] + fn grub_check_is_silent_when_grub_did_not_run() { + // systemd-boot / non-GRUB DUTs never print the GRUB banner. + assert!(deploy_warnings("==> Starting build: '7.2.0'\n", Some("7.2.0")).is_empty()); + } + + #[test] + fn deploy_warnings_dedup_and_cap() { + let log = "\ +==> ERROR: module not found: 'a' +==> ERROR: module not found: 'a' +==> ERROR: module not found: 'b' +==> ERROR: module not found: 'c' +==> ERROR: module not found: 'd' +==> ERROR: module not found: 'e' +==> ERROR: module not found: 'f' +kw was unable to set up the first boot +"; + let warnings = deploy_warnings(log, None); + assert_eq!(MAX_DEPLOY_WARNINGS, warnings.len()); + assert_eq!("==> ERROR: module not found: 'a'", warnings[0]); + assert_eq!("==> ERROR: module not found: 'e'", warnings[4]); + + // The unlisted-kernel warning survives the cap. + let log = format!("{log}Generating grub configuration file ...\n"); + let warnings = deploy_warnings(&log, Some("7.2.0")); + assert_eq!(MAX_DEPLOY_WARNINGS, warnings.len()); + assert_eq!("GRUB did not list kernel 7.2.0", warnings[4]); + } +} diff --git a/src/kw/mod.rs b/src/kw/mod.rs index 97ac2bb..f6b2176 100644 --- a/src/kw/mod.rs +++ b/src/kw/mod.rs @@ -7,6 +7,7 @@ pub mod argv; pub mod errors; pub mod handle; pub mod history; +pub mod log_scan; pub mod messages; pub mod readiness; pub mod remote; diff --git a/src/kw/readiness.rs b/src/kw/readiness.rs index ee569ca..d412180 100644 --- a/src/kw/readiness.rs +++ b/src/kw/readiness.rs @@ -62,6 +62,10 @@ pub enum TreeReadiness { /// No `.config` at the build root (the tree itself, or the kw env's /// output dir when one is active). MissingKernelConfig, + /// A kw env is active but the source tree still holds an in-tree + /// `.config` or `include/config/`: kbuild refuses an `O=` build of an + /// unclean source tree, so every build fails within seconds. + InTreeBuildArtifacts, } /// Parses kw's `key=value` config format (`.kw/build.config`, @@ -131,6 +135,12 @@ pub fn probe_tree( if !fs.is_dir(&tree_path.join(".kw")) { return TreeReadiness::MissingKwDir; } + if output_dir.is_some() + && (fs.is_file(&tree_path.join(".config")) + || fs.is_dir(&tree_path.join("include").join("config"))) + { + return TreeReadiness::InTreeBuildArtifacts; + } let build_root = output_dir.unwrap_or(tree_path); if !fs.is_file(&build_root.join(".config")) { return TreeReadiness::MissingKernelConfig; @@ -405,6 +415,11 @@ impl std::fmt::Display for TreeReadiness { "no .config at the build root (the tree, or the kw env's O=)" ) } + TreeReadiness::InTreeBuildArtifacts => write!( + f, + "in-tree .config / include/config block kw env (O=) builds; run \ + `make mrproper` in the tree (the env's O= is untouched)" + ), } } } @@ -906,6 +921,43 @@ last_line_without_newline=yes"; )); } + #[test] + fn probe_tree_with_active_env_refuses_in_tree_config() { + let dir = make_ready_tree("probe-env-in-tree-config"); + let out = TempDir::new("probe-env-in-tree-config-output"); + fs::write(out.path().join(".config"), "").unwrap(); + + assert_eq!( + TreeReadiness::InTreeBuildArtifacts, + probe_tree(&OsFileSystem, dir.path(), Some(out.path())) + ); + } + + #[test] + fn probe_tree_with_active_env_refuses_in_tree_include_config() { + let dir = make_ready_tree("probe-env-in-tree-include-config"); + fs::remove_file(dir.path().join(".config")).unwrap(); + fs::create_dir(dir.path().join("include/config")).unwrap(); + let out = TempDir::new("probe-env-in-tree-include-config-output"); + fs::write(out.path().join(".config"), "").unwrap(); + + assert_eq!( + TreeReadiness::InTreeBuildArtifacts, + probe_tree(&OsFileSystem, dir.path(), Some(out.path())) + ); + } + + #[test] + fn probe_tree_without_env_accepts_in_tree_build_artifacts() { + let dir = make_ready_tree("probe-no-env-in-tree-artifacts"); + fs::create_dir(dir.path().join("include/config")).unwrap(); + + assert_eq!( + TreeReadiness::Ready { arch: None }, + probe_tree(&OsFileSystem, dir.path(), None) + ); + } + #[test] fn read_build_arch_reads_literal_value() { let dir = make_ready_tree("arch-literal"); diff --git a/src/kw/remote.rs b/src/kw/remote.rs index 685cf49..24a0c22 100644 --- a/src/kw/remote.rs +++ b/src/kw/remote.rs @@ -122,10 +122,8 @@ pub fn parse_remote_config(content: &str) -> ParsedRemoteConfig { if !rest.is_empty() { builder.port = Some(rest.to_string()); } - } else if keyword.eq_ignore_ascii_case("user") { - if !rest.is_empty() { - builder.user = Some(rest.to_string()); - } + } else if keyword.eq_ignore_ascii_case("user") && !rest.is_empty() { + builder.user = Some(rest.to_string()); } } if let Some(host) = current.and_then(HostBuilder::finish) { @@ -528,8 +526,8 @@ Host dut extra let home = home.map(str::to_string); let mut env = MockEnvTrait::new(); env.expect_var().returning(move |key| match key { - "XDG_CONFIG_HOME" => xdg.clone().ok_or_else(|| missing_var()), - "HOME" => home.clone().ok_or_else(|| missing_var()), + "XDG_CONFIG_HOME" => xdg.clone().ok_or_else(missing_var), + "HOME" => home.clone().ok_or_else(missing_var), _ => Err(missing_var()), }); env diff --git a/src/kw/status.rs b/src/kw/status.rs index ccc1860..26eab61 100644 --- a/src/kw/status.rs +++ b/src/kw/status.rs @@ -37,12 +37,18 @@ pub enum KwJobStatus { kernel_tree_id: String, branch: String, log_path: PathBuf, + /// Known deploy failures kw exited 0 through (see + /// [`crate::kw::log_scan::deploy_warnings`]). Always empty for a + /// build-only job. + warnings: Vec, }, Failed { kind: KwJobKind, phase: KwPhase, exit_code: Option, log_path: PathBuf, + /// First error line found in the log, if any. + first_error: Option, }, Cancelled { kind: KwJobKind, @@ -162,6 +168,7 @@ mod tests { kernel_tree_id: "mainline".to_string(), branch: "patchset-x".to_string(), log_path: PathBuf::from("/tmp/build.log"), + warnings: Vec::new(), }, restore_branch: Some("master".to_string()), } diff --git a/src/ui/core.rs b/src/ui/core.rs index bedb5bb..fcc83ac 100644 --- a/src/ui/core.rs +++ b/src/ui/core.rs @@ -38,7 +38,9 @@ impl UiCore { ScreenViewModel::EditConfig(ec_vm) => { UiBody::EditConfig(screens::edit_config::build_scene(ec_vm)) } - ScreenViewModel::KwOps(kw_vm) => UiBody::KwOps(screens::kw_ops::build_scene(kw_vm)), + ScreenViewModel::KwOps(kw_vm) => { + UiBody::KwOps(Box::new(screens::kw_ops::build_scene(kw_vm))) + } }; let navigation = self.build_navigation(&vm.screen, vm.kw_running.as_deref()); diff --git a/src/ui/scene.rs b/src/ui/scene.rs index 63eb479..f420261 100644 --- a/src/ui/scene.rs +++ b/src/ui/scene.rs @@ -94,6 +94,9 @@ pub struct KwOpsScene { pub deploy_label: String, pub build_deploy_label: String, pub branch_guidance: Option, + pub warnings: Option, + pub first_error: Option, + pub log_path: Option, pub log_tail: String, } @@ -105,7 +108,9 @@ pub enum UiBody { Latest(LatestScene), PatchsetDetails(PatchsetDetailsScene), EditConfig(EditConfigScene), - KwOps(KwOpsScene), + // Boxed to keep the enum small (clippy::large_enum_variant): KwOps + // carries one owned string per dashboard row. + KwOps(Box), } /// Pre-computed navigation-bar content. diff --git a/src/ui/screens/kw_ops.rs b/src/ui/screens/kw_ops.rs index 0331a2d..69d3fe7 100644 --- a/src/ui/screens/kw_ops.rs +++ b/src/ui/screens/kw_ops.rs @@ -33,6 +33,9 @@ pub fn build_scene(vm: &KwOpsViewModel) -> KwOpsScene { deploy_label: vm.deploy_label.clone(), build_deploy_label: vm.build_deploy_label.clone(), branch_guidance: vm.branch_guidance.clone(), + warnings: vm.warnings.clone(), + first_error: vm.first_error.clone(), + log_path: vm.log_path.clone(), log_tail: vm.log_tail.clone(), } } @@ -76,6 +79,17 @@ fn paint_form(f: &mut Frame, scene: &KwOpsScene, chunk: Rect) { labeled("Remote", &scene.remote), labeled("Boot once", &scene.boot_once), labeled("Job", &scene.job_status), + ]); + if let Some(warnings) = &scene.warnings { + lines.push(labeled_colored("Warnings", warnings, Color::Yellow)); + } + if let Some(first_error) = &scene.first_error { + lines.push(labeled_colored("First error", first_error, Color::Red)); + } + if let Some(log_path) = &scene.log_path { + lines.push(labeled("Log", log_path)); + } + lines.extend([ labeled("Command", &scene.command), labeled("Deploy command", &scene.deploy_command), labeled("Start", &scene.start_label), @@ -123,9 +137,13 @@ pub(crate) fn log_scroll_offset(text: &str, inner_width: u16, inner_height: u16) } fn labeled(label: &str, value: &str) -> Line<'static> { + labeled_colored(label, value, Color::White) +} + +fn labeled_colored(label: &str, value: &str, color: Color) -> Line<'static> { Line::from(vec![ Span::styled(format!("{label}: "), Style::default().fg(Color::Cyan)), - Span::styled(value.to_string(), Style::default().fg(Color::White)), + Span::styled(value.to_string(), Style::default().fg(color)), ]) }