From d7bf980bff72ce27bd8543bf8a8f98ec472398aa Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Fri, 18 Sep 2026 17:22:50 -0300 Subject: [PATCH 1/6] feat(app): project kw status and show global running state This commit subscribes AppActor to KwHandle::watch_status once at startup and redraws when the snapshot changes, so a running job is visible without waiting for a keypress. The nav bar shows a compact running indicator; restore availability and cancelled log paths are published on the snapshot so the UI does not have to infer them. This commit is part of the kw integration's step 6. Signed-off-by: lorenzoberts --- src/app/actor.rs | 93 ++++++-- src/app/integration_tests/kw_status.rs | 299 +++++++++++++++++++++++++ src/app/integration_tests/mod.rs | 2 + src/app/mod.rs | 1 + src/app/state.rs | 11 + src/app/view_model.rs | 102 ++++++++- src/kw/actor.rs | 53 ++++- src/kw/status.rs | 79 +++++++ src/ui/actor.rs | 1 + src/ui/core.rs | 68 +++++- 10 files changed, 680 insertions(+), 29 deletions(-) create mode 100644 src/app/integration_tests/kw_status.rs diff --git a/src/app/actor.rs b/src/app/actor.rs index c0ee9e22..0bcf8acf 100644 --- a/src/app/actor.rs +++ b/src/app/actor.rs @@ -3,8 +3,7 @@ //! Each frame: process system updates → project state to [`AppViewModel`] via //! [`UiHandle`](crate::ui::handle::UiHandle) → draw through //! [`TerminalHandle`](crate::terminal::handle::TerminalHandle) → await the next -//! [`InputEvent`](crate::input::event::InputEvent) from the channel registered -//! with [`InputHandle`](crate::input::handle::InputHandle). +//! [`InputEvent`](crate::input::event::InputEvent) or a kw-status change. //! //! The actor stops when the input event channel closes (user quit) or when I/O //! returns an unrecoverable error. Startup dependency checks run before this @@ -12,7 +11,7 @@ use std::ops::ControlFlow; use color_eyre::{eyre::eyre, Result}; -use tokio::{spawn, sync::mpsc}; +use tokio::{spawn, sync::mpsc, sync::watch}; use crate::{ app::{ @@ -27,6 +26,7 @@ use crate::{ App, }, input::{event::InputEvent, handle::InputHandle}, + kw::status::KwStatusSnapshot, terminal::{handle::TerminalHandle, messages::TerminalFrame}, ui::handle::UiHandle, }; @@ -46,6 +46,11 @@ pub struct AppActor { event_rx: mpsc::Receiver, } +enum KwWatchEvent { + Updated(KwStatusSnapshot), + Closed, +} + impl AppActor { /// Moves all resources into a new `AppActor`, spawns it on the Tokio /// runtime, and returns an [`AppHandle`] to wait on it. @@ -71,6 +76,7 @@ impl AppActor { tracing::info!("app actor started"); tracing::info!("app actor initialized"); + let mut kw_status_rx = self.subscribe_kw_status().await; let mut loading = TerminalLoadingIndicator::new(self.terminal_handle.clone()); loop { @@ -86,23 +92,40 @@ impl AppActor { .await .map_err(terminal_error)?; - match self.event_rx.recv().await { - Some(event) => { - match on_input(&mut self.app, event, &self.terminal_handle, &mut loading) - .await? - { - ControlFlow::Continue(()) => { - self.input_handle - .update_context(self.app.input_context()) - .await - .ok(); + tokio::select! { + event = self.event_rx.recv() => { + match event { + Some(event) => { + match on_input(&mut self.app, event, &self.terminal_handle, &mut loading) + .await? + { + ControlFlow::Continue(()) => { + self.input_handle + .update_context(self.app.input_context()) + .await + .ok(); + } + ControlFlow::Break(()) => break, + } + } + None => { + tracing::info!("input channel closed; app actor stopping"); + break; } - ControlFlow::Break(()) => break, } } - None => { - tracing::info!("input channel closed; app actor stopping"); - break; + watch_event = kw_status_changed(&mut kw_status_rx) => { + match watch_event { + KwWatchEvent::Updated(snapshot) => { + self.app.state.kw.status = Some(snapshot); + } + KwWatchEvent::Closed => { + tracing::warn!( + "kw status watch closed; falling back to keyboard-only redraws" + ); + kw_status_rx = None; + } + } } } } @@ -110,6 +133,41 @@ impl AppActor { tracing::info!("app actor stopped"); Ok(()) } + + /// Subscribes once at startup. A missing handle or a failed + /// subscription leaves the app on keyboard-only redraws rather than + /// failing to start. + async fn subscribe_kw_status(&mut self) -> Option> { + let Some(kw) = self.app.services.kw.as_ref() else { + return None; + }; + match kw.watch_status().await { + Ok(mut rx) => { + self.app.state.kw.status = Some(rx.borrow_and_update().clone()); + Some(rx) + } + Err(error) => { + tracing::warn!( + %error, + "failed to subscribe to kw status; keyboard-only redraws" + ); + None + } + } + } +} + +/// Waits for the next kw-status change. With no receiver this future +/// never completes, so `select!` stays on the input arm instead of +/// spinning. +async fn kw_status_changed(rx: &mut Option>) -> KwWatchEvent { + match rx.as_mut() { + Some(rx) => match rx.changed().await { + Ok(()) => KwWatchEvent::Updated(rx.borrow_and_update().clone()), + Err(_) => KwWatchEvent::Closed, + }, + None => std::future::pending().await, + } } async fn on_input( @@ -226,6 +284,7 @@ mod tests { config_state: ConfigUiState { edit_config: None }, config: ConfigState::default().to_snapshot(), popup: None, + kw: Default::default(), }, services: AppServices { lore_api: LoreApiHandle::new(lore_tx), diff --git a/src/app/integration_tests/kw_status.rs b/src/app/integration_tests/kw_status.rs new file mode 100644 index 00000000..de5141a5 --- /dev/null +++ b/src/app/integration_tests/kw_status.rs @@ -0,0 +1,299 @@ +use std::{ + path::PathBuf, + sync::{ + atomic::{AtomicU64, Ordering}, + Arc, Mutex, + }, + time::Duration, +}; + +use crate::{ + app::actor::AppActor, + config::{ConfigSnapshot, ConfigState}, + infrastructure::{ + env::MockEnvTrait, + file_system::{FileSystemError, MockFileSystemTrait}, + process::FakeProcess, + shell::{MockShellTrait, ShellOutput}, + }, + input::{event::InputEvent, handle::InputHandle, messages::InputMessage}, + kw::{actor::KwActor, history::MockKwHistoryStore, messages::StartRequest}, + lore::application::cache::BootstrapLoreData, + terminal::{actor::TerminalActor, messages::TerminalFrame, session::MockTerminalSessionApi}, + ui::{actor::UiActor, scene::UiScene}, +}; + +use super::helpers::{ + app_harness::{dummy_config_handle, dummy_render_handle}, + lore::{lore_handle_with_persistence, sample_mailing_list}, +}; + +const KERNEL_TREE_PATH: &str = "/kernel"; +const BUILD_BRANCH: &str = "patchset-2026-08-20-15-00-00"; + +static LOG_DIR_SEQ: AtomicU64 = AtomicU64::new(0); + +#[tokio::test(flavor = "multi_thread")] +async fn first_frame_shows_a_job_that_started_before_attach() { + let log_dir = kw_log_dir("attach-running"); + let (app, kw, process) = app_with_kw(&log_dir); + kw.start_build(start_request()).await.unwrap(); + + let (scenes, event_tx, handle) = spawn_app_actor(app); + wait_for_nav(&scenes, |text| text.contains("kw: building")).await; + + process.last_child().finish(0); + drop(event_tx); + handle.run_until_done().await.unwrap(); + kw.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); +} + +#[tokio::test(flavor = "multi_thread")] +async fn status_change_redraws_without_input() { + let log_dir = kw_log_dir("redraw-no-input"); + let (app, kw, process) = app_with_kw(&log_dir); + let (scenes, event_tx, handle) = spawn_app_actor(app); + + wait_for_nav(&scenes, |text| !text.contains("kw:")).await; + let draws_before_start = scene_count(&scenes); + + kw.start_build(start_request()).await.unwrap(); + wait_for_nav(&scenes, |text| { + text.contains(&format!("kw: building {BUILD_BRANCH}")) + }) + .await; + assert!( + scene_count(&scenes) > draws_before_start, + "a running job must trigger a redraw without an input event" + ); + + process.last_child().finish(0); + wait_for_nav(&scenes, |text| !text.contains("kw:")).await; + + drop(event_tx); + handle.run_until_done().await.unwrap(); + kw.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); +} + +#[tokio::test(flavor = "multi_thread")] +async fn closed_watch_does_not_busy_loop_and_input_still_works() { + let log_dir = kw_log_dir("watch-closed"); + let (app, kw, _process) = app_with_kw(&log_dir); + let (scenes, event_tx, handle) = spawn_app_actor(app); + + wait_for_nav(&scenes, |_| true).await; + kw.shutdown().await; + + // Give the actor a moment to observe the closed watch and redraw once. + tokio::time::sleep(Duration::from_millis(50)).await; + let draws_after_close = scene_count(&scenes); + tokio::time::sleep(Duration::from_millis(150)).await; + assert_eq!( + draws_after_close, + scene_count(&scenes), + "a closed kw watch must not spin the render loop" + ); + + event_tx.send(InputEvent::NavigateDown).await.unwrap(); + tokio::time::timeout(Duration::from_secs(5), async { + loop { + if scene_count(&scenes) > draws_after_close { + break; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("input must still redraw after the kw watch closes"); + + drop(event_tx); + handle.run_until_done().await.unwrap(); + std::fs::remove_dir_all(&log_dir).unwrap(); +} + +fn spawn_app_actor( + app: crate::app::App, +) -> ( + Arc>>, + tokio::sync::mpsc::Sender, + crate::app::handle::AppHandle, +) { + let scenes = Arc::new(Mutex::new(Vec::new())); + let scenes_for_draw = Arc::clone(&scenes); + let mut session = MockTerminalSessionApi::new(); + session + .expect_draw() + .withf(|frame| matches!(frame, TerminalFrame::Main(_))) + .times(1..) + .returning(move |frame| { + if let TerminalFrame::Main(scene) = frame { + scenes_for_draw.lock().unwrap().push(*scene); + } + Ok(()) + }); + + let terminal_handle = TerminalActor::spawn(Box::new(session)); + let ui_handle = UiActor::spawn(); + let (event_tx, event_rx) = tokio::sync::mpsc::channel::(8); + let (input_tx, _input_rx) = tokio::sync::mpsc::channel::(1); + let input_handle = InputHandle::new(input_tx); + let handle = AppActor::spawn(app, terminal_handle, ui_handle, input_handle, event_rx); + (scenes, event_tx, handle) +} + +fn app_with_kw( + log_dir: &std::path::Path, +) -> ( + crate::app::App, + crate::kw::handle::KwHandle, + Arc, +) { + let process = Arc::new(FakeProcess::new()); + let mut history = MockKwHistoryStore::new(); + history + .expect_apply_record_for_branch() + .returning(|_, _| Ok(None)); + history.expect_record_build().returning(|_| Ok(())); + let kw = KwActor::spawn( + Arc::new(history), + process.clone(), + Arc::new(kw_actor_shell()), + Arc::new(kw_actor_fs()), + Arc::new(kw_actor_env()), + log_dir.to_path_buf(), + ); + let app = crate::app::App::new( + apply_config(), + dummy_config_handle(), + BootstrapLoreData { + mailing_lists: vec![sample_mailing_list()], + bookmarks: vec![], + reviewed: Default::default(), + }, + Box::new(MockFileSystemTrait::new()), + Box::new(MockShellTrait::new()), + lore_handle_with_persistence(), + dummy_render_handle(), + Arc::new(MockKwHistoryStore::new()), + Some(kw.clone()), + ) + .expect("app should build"); + (app, kw, process) +} + +async fn wait_for_nav(scenes: &Arc>>, predicate: impl Fn(&str) -> bool) { + tokio::time::timeout(Duration::from_secs(5), async { + loop { + if scenes + .lock() + .unwrap() + .iter() + .any(|scene| predicate(&nav_text(scene))) + { + return; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("expected navigation text did not appear"); +} + +fn scene_count(scenes: &Arc>>) -> usize { + scenes.lock().unwrap().len() +} + +fn nav_text(scene: &UiScene) -> String { + scene + .navigation + .mode_spans + .iter() + .map(|span| span.content.to_string()) + .collect() +} + +fn kw_log_dir(test_name: &str) -> PathBuf { + let n = LOG_DIR_SEQ.fetch_add(1, Ordering::SeqCst); + let dir = std::env::temp_dir().join(format!( + "patch-hub-app-kw-status-{}-{}-{n}", + test_name, + std::process::id() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + dir +} + +fn start_request() -> StartRequest { + StartRequest { + kernel_tree_id: "linux".to_string(), + tree: serde_json::from_value(serde_json::json!({ + "path": KERNEL_TREE_PATH, + "branch": "main" + })) + .expect("kernel tree should deserialize"), + branch: BUILD_BRANCH.to_string(), + extra_args: Vec::new(), + } +} + +fn apply_config() -> ConfigSnapshot { + serde_json::from_value::(serde_json::json!({ + "kernel_trees": { + "linux": { + "path": KERNEL_TREE_PATH, + "branch": "main" + } + }, + "target_kernel_tree": "linux" + })) + .expect("test config should deserialize") + .to_snapshot() +} + +fn kw_actor_shell() -> MockShellTrait { + let mut shell = MockShellTrait::new(); + shell.expect_execute().returning(|cmd| { + let stdout = match cmd.program.as_str() { + "kw" => b"kw, version 0.10.0\n".to_vec(), + _ if cmd.args.iter().any(|arg| arg == "status") => Vec::new(), + _ => b"main\n".to_vec(), + }; + Ok(ShellOutput { + stdout, + stderr: Vec::new(), + success: true, + }) + }); + shell +} + +fn kw_actor_fs() -> MockFileSystemTrait { + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_dir().returning(|_| true); + fs.expect_is_file() + .returning(|path| !path.ends_with(".kw/env.current")); + fs.expect_exists().returning(|_| true); + fs.expect_read_to_string().returning(|_| { + Err(FileSystemError::IoError(std::io::Error::new( + std::io::ErrorKind::NotFound, + "missing", + ))) + }); + fs.expect_read_dir().returning(|_| { + Err(FileSystemError::IoError(std::io::Error::new( + std::io::ErrorKind::NotFound, + "missing", + ))) + }); + fs.expect_create_dir_all().returning(|_| Ok(())); + fs +} + +fn kw_actor_env() -> MockEnvTrait { + let mut env = MockEnvTrait::new(); + env.expect_which().returning(|_| true); + env +} diff --git a/src/app/integration_tests/mod.rs b/src/app/integration_tests/mod.rs index 65db2996..e4aa0260 100644 --- a/src/app/integration_tests/mod.rs +++ b/src/app/integration_tests/mod.rs @@ -2,6 +2,8 @@ mod actor_lifecycle; mod flow_errors; mod flow_navigation; mod helpers; +#[cfg(unix)] +mod kw_status; mod patchset_actions; use crate::app::{loading::LoadingIndicator, screens::CurrentScreen}; diff --git a/src/app/mod.rs b/src/app/mod.rs index 7d1a0bfb..7d466bcc 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -148,6 +148,7 @@ impl App { config_state: ConfigUiState { edit_config: None }, config, popup: None, + kw: Default::default(), }, services: AppServices { lore_api, diff --git a/src/app/state.rs b/src/app/state.rs index d7418790..62b457c4 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -10,6 +10,7 @@ use crate::{ }, }, config::ConfigSnapshot, + kw::status::KwStatusSnapshot, }; /// Navigation-only state: which screen is active. @@ -39,6 +40,15 @@ pub struct ConfigUiState { pub edit_config: Option, } +/// Projection of kw job status owned by AppActor, not the job itself. +/// +/// `status` is `None` when no KwActor is attached (non-unix, or the +/// watch subscription failed) rather than a fabricated idle job. +#[derive(Clone, Debug, Default)] +pub struct KwUiState { + pub status: Option, +} + /// All application state grouped as the App actor's state. #[derive(Clone)] pub struct AppState { @@ -48,4 +58,5 @@ pub struct AppState { pub config_state: ConfigUiState, pub config: ConfigSnapshot, pub popup: Option, + pub kw: KwUiState, } diff --git a/src/app/view_model.rs b/src/app/view_model.rs index 1a5ecfc5..1b6eb3cb 100644 --- a/src/app/view_model.rs +++ b/src/app/view_model.rs @@ -15,6 +15,7 @@ use super::{ screens::{details_actions::PatchsetAction, CurrentScreen}, state::AppState, }; +use crate::kw::status::KwStatusSnapshot; /// One mailing list entry shown in the selection list. #[derive(Clone, Debug)] @@ -160,6 +161,9 @@ pub enum ScreenViewModel { pub struct AppViewModel { pub screen: ScreenViewModel, pub popup: Option, + /// Compact running-job copy for the nav bar. `None` when idle or + /// after a terminal outcome — those belong on KwOps, not globally. + pub kw_running: Option, } /// Projects `state` into an owned [`AppViewModel`]. @@ -168,7 +172,16 @@ pub struct AppViewModel { pub fn project_state(state: &AppState) -> AppViewModel { let screen = project_screen(state); let popup = state.popup.as_ref().map(project_popup); - AppViewModel { screen, popup } + let kw_running = state + .kw + .status + .as_ref() + .and_then(KwStatusSnapshot::running_indicator); + AppViewModel { + screen, + popup, + kw_running, + } } fn project_screen(state: &AppState) -> ScreenViewModel { match state.navigation.current_screen { @@ -439,3 +452,90 @@ fn project_popup(popup: &AppPopup) -> PopupViewModel { }, } } + +#[cfg(test)] +mod tests { + use std::{collections::HashMap, path::PathBuf}; + + use crate::{ + app::{ + screens::{bookmarked::BookmarkedPatchsetsState, mail_list::MailingListSelectionState}, + state::{ + AppState, ConfigUiState, KwUiState, LoreUiState, NavigationState, UserLoreState, + }, + }, + config::ConfigState, + kw::status::{KwJobKind, KwJobStatus, KwPhase, KwStatusSnapshot}, + lore::domain::mailing_list::MailingList, + }; + + use super::*; + + fn app_state_with_kw(status: Option) -> AppState { + let dummy_list = MailingList::new("test-list", "Test list"); + AppState { + navigation: NavigationState { + current_screen: CurrentScreen::MailingListSelection, + }, + 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: ConfigState::default().to_snapshot(), + popup: None, + kw: KwUiState { status }, + } + } + + #[test] + fn running_job_projects_a_global_indicator() { + let vm = project_state(&app_state_with_kw(Some(KwStatusSnapshot { + job: KwJobStatus::Running { + kind: KwJobKind::Build, + phase: KwPhase::Building, + kernel_tree_id: "mainline".to_string(), + branch: "patchset-x".to_string(), + log_path: PathBuf::from("/tmp/build.log"), + }, + restore_branch: Some("master".to_string()), + }))); + + assert_eq!(Some("kw: building patchset-x".to_string()), vm.kw_running); + } + + #[test] + fn idle_succeeded_and_missing_status_have_no_global_indicator() { + assert_eq!(None, project_state(&app_state_with_kw(None)).kw_running); + assert_eq!( + None, + project_state(&app_state_with_kw(Some(KwStatusSnapshot::idle()))).kw_running + ); + assert_eq!( + None, + project_state(&app_state_with_kw(Some(KwStatusSnapshot { + job: KwJobStatus::Succeeded { + kind: KwJobKind::Build, + kernel_tree_id: "mainline".to_string(), + branch: "patchset-x".to_string(), + log_path: PathBuf::from("/tmp/build.log"), + }, + restore_branch: Some("master".to_string()), + }))) + .kw_running + ); + } +} diff --git a/src/kw/actor.rs b/src/kw/actor.rs index 3a82131f..5d36d0c6 100644 --- a/src/kw/actor.rs +++ b/src/kw/actor.rs @@ -507,6 +507,7 @@ impl KwActor { KwJobStatus::Cancelled { kind: job.kind, phase: job.phase, + log_path: job.log_path, } } }; @@ -585,8 +586,24 @@ impl KwActor { fn set_status(&mut self, job: KwJobStatus) { // send_replace, not send: no receiver (nobody called WatchStatus - // yet) is a normal state, not an error. - self.status_tx.send_replace(KwStatusSnapshot { job }); + // yet) is a normal state, not an error. Restore availability is + // published with every snapshot so the UI does not have to guess + // whether RestorePreviousBranch would succeed. + self.status_tx.send_replace(KwStatusSnapshot { + job, + restore_branch: self + .last_restore + .as_ref() + .map(|restore| restore.branch.clone()), + }); + } + + /// Re-broadcasts the current job with the current restore projection. + /// Used after restore succeeds (the branch is gone) without changing + /// the job's own status. + fn republish_status(&mut self) { + let job = self.status_tx.borrow().job.clone(); + self.set_status(job); } fn evaluate_readiness( @@ -637,6 +654,7 @@ impl KwActor { branch = restore.branch, "restored pre-job branch" ); + self.republish_status(); Ok(()) } Err(error) => { @@ -1269,6 +1287,7 @@ mod tests { let snapshot = handle.get_status().await.unwrap(); assert_eq!(KwJobStatus::Idle, snapshot.job); + assert_eq!(None, snapshot.restore_branch); handle.shutdown().await; } @@ -1499,13 +1518,18 @@ mod tests { assert!(process.last_child().was_killed()); let status = wait_for_terminal_status(&mut watch).await; - assert_eq!( + match status { KwJobStatus::Cancelled { - kind: KwJobKind::Build, - phase: KwPhase::Building, - }, - status - ); + kind, + phase, + log_path, + } => { + assert_eq!(KwJobKind::Build, kind); + assert_eq!(KwPhase::Building, phase); + assert!(log_path.starts_with(&log_dir)); + } + other => panic!("expected Cancelled, got {other:?}"), + } handle.shutdown().await; std::fs::remove_dir_all(&log_dir).unwrap(); @@ -1810,12 +1834,21 @@ mod tests { handle.start_build(start_request()).await.unwrap(); // The checkout policy left HEAD on the build branch. assert_eq!(git.head(), "patchset-2026-08-01-17-30-00"); + assert_eq!( + Some("master"), + handle.get_status().await.unwrap().restore_branch.as_deref() + ); process.last_child().finish(0); let status = wait_for_terminal_status(&mut watch).await; assert!(matches!(status, KwJobStatus::Succeeded { .. })); + assert_eq!( + Some("master"), + handle.get_status().await.unwrap().restore_branch.as_deref() + ); handle.restore_previous_branch().await.unwrap(); assert_eq!(git.head(), "master"); + assert_eq!(None, handle.get_status().await.unwrap().restore_branch); // A successful restore consumes the context: a second restore has // nothing to do. @@ -1864,6 +1897,10 @@ mod tests { assert!(matches!(err, KwError::DirtyWorktree)); // The refused restore did not touch the tree. assert_eq!(git.head(), "patchset-2026-08-01-17-30-00"); + assert_eq!( + Some("master"), + handle.get_status().await.unwrap().restore_branch.as_deref() + ); // The context survives: clean the tree and retry. git.set_dirty(false); diff --git a/src/kw/status.rs b/src/kw/status.rs index 3f638aa9..b2c6f1be 100644 --- a/src/kw/status.rs +++ b/src/kw/status.rs @@ -50,18 +50,97 @@ pub enum KwJobStatus { Cancelled { kind: KwJobKind, phase: KwPhase, + log_path: PathBuf, }, } #[derive(Debug, Clone, PartialEq, Eq)] pub struct KwStatusSnapshot { pub job: KwJobStatus, + /// Pre-job branch the last accepted job can restore to, if any. + /// Session-only; `None` after a successful restore or when nothing + /// was recorded. + pub restore_branch: Option, } impl KwStatusSnapshot { pub fn idle() -> Self { Self { job: KwJobStatus::Idle, + restore_branch: None, + } + } + + /// Compact nav-bar copy for a job that is still running. Terminal + /// success/failure is not shown globally — it belongs on KwOps. + pub fn running_indicator(&self) -> Option { + match &self.job { + KwJobStatus::Running { phase, branch, .. } => { + let phase = match phase { + KwPhase::Building => "building", + KwPhase::Deploying => "deploying", + }; + Some(format!("kw: {phase} {branch}")) + } + _ => None, + } + } +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use super::*; + + fn running(branch: &str) -> KwStatusSnapshot { + KwStatusSnapshot { + job: KwJobStatus::Running { + kind: KwJobKind::Build, + phase: KwPhase::Building, + kernel_tree_id: "mainline".to_string(), + branch: branch.to_string(), + log_path: PathBuf::from("/tmp/build.log"), + }, + restore_branch: Some("master".to_string()), } } + + #[test] + fn running_indicator_names_the_phase_and_branch() { + assert_eq!( + Some("kw: building patchset-x".to_string()), + running("patchset-x").running_indicator() + ); + } + + #[test] + fn terminal_and_idle_status_have_no_global_indicator() { + assert_eq!(None, KwStatusSnapshot::idle().running_indicator()); + assert_eq!( + None, + KwStatusSnapshot { + job: KwJobStatus::Succeeded { + kind: KwJobKind::Build, + kernel_tree_id: "mainline".to_string(), + branch: "patchset-x".to_string(), + log_path: PathBuf::from("/tmp/build.log"), + }, + restore_branch: Some("master".to_string()), + } + .running_indicator() + ); + assert_eq!( + None, + KwStatusSnapshot { + job: KwJobStatus::Cancelled { + kind: KwJobKind::Build, + phase: KwPhase::Building, + log_path: PathBuf::from("/tmp/build.log"), + }, + restore_branch: Some("master".to_string()), + } + .running_indicator() + ); + } } diff --git a/src/ui/actor.rs b/src/ui/actor.rs index 0a170145..c6aab859 100644 --- a/src/ui/actor.rs +++ b/src/ui/actor.rs @@ -120,6 +120,7 @@ mod tests { target_list_status: TargetListStatus::Empty, }), popup: None, + kw_running: None, } } diff --git a/src/ui/core.rs b/src/ui/core.rs index 4240fd48..c0fdcaaf 100644 --- a/src/ui/core.rs +++ b/src/ui/core.rs @@ -10,6 +10,10 @@ use crate::ui::{ scene::{NavigationBarScene, UiBody, UiScene}, screens, }; +use ratatui::{ + style::{Color, Style}, + text::Span, +}; pub struct UiCore; @@ -36,7 +40,7 @@ impl UiCore { } }; - let navigation = self.build_navigation(&vm.screen); + let navigation = self.build_navigation(&vm.screen, vm.kw_running.as_deref()); let popup = vm.popup.as_ref().map(screens::popup::build_scene); Ok(UiScene { @@ -46,8 +50,12 @@ impl UiCore { }) } - fn build_navigation(&self, screen: &ScreenViewModel) -> NavigationBarScene { - let (mode_spans, keys_hint) = match screen { + fn build_navigation( + &self, + screen: &ScreenViewModel, + kw_running: Option<&str>, + ) -> NavigationBarScene { + let (mut mode_spans, keys_hint) = match screen { ScreenViewModel::MailingListSelection(vm) => ( screens::mailing_list::mode_spans(vm), screens::mailing_list::keys_hint_span(), @@ -69,6 +77,13 @@ impl UiCore { screens::edit_config::keys_hint_span(vm), ), }; + if let Some(indicator) = kw_running { + mode_spans.push(Span::styled(" | ", Style::default().fg(Color::DarkGray))); + mode_spans.push(Span::styled( + indicator.to_string(), + Style::default().fg(Color::Yellow), + )); + } NavigationBarScene { mode_spans, keys_hint, @@ -81,3 +96,50 @@ impl Default for UiCore { Self::new() } } + +#[cfg(test)] +mod tests { + use crate::app::view_model::{ + AppViewModel, MailingListSelectionViewModel, ScreenViewModel, TargetListStatus, + }; + + use super::UiCore; + + fn mailing_list_vm(kw_running: Option) -> AppViewModel { + AppViewModel { + screen: ScreenViewModel::MailingListSelection(MailingListSelectionViewModel { + entries: vec![], + highlighted_index: 0, + target_list: String::new(), + target_list_status: TargetListStatus::Empty, + }), + popup: None, + kw_running, + } + } + + fn nav_text(vm: AppViewModel) -> String { + UiCore::new() + .build_scene(&vm) + .unwrap() + .navigation + .mode_spans + .into_iter() + .map(|span| span.content.to_string()) + .collect() + } + + #[test] + fn running_indicator_is_appended_to_the_nav_bar() { + let text = nav_text(mailing_list_vm(Some("kw: building patchset-x".to_string()))); + assert!(text.contains("Target List:")); + assert!(text.contains(" | kw: building patchset-x")); + } + + #[test] + fn idle_nav_bar_has_no_kw_indicator() { + let text = nav_text(mailing_list_vm(None)); + assert!(text.contains("Target List:")); + assert!(!text.contains("kw:")); + } +} From fb12bda970f55dcd3e1f4c26ee85a8303999c064 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Fri, 18 Sep 2026 17:29:01 -0300 Subject: [PATCH 2/6] feat(app): confirm cancellation before quitting active kw job This commit adds a typed confirmation popup so Esc on the mailing list cannot tear down a running kw job by accident. Wait is the highlighted default; cancel-and-quit requests KwHandle::cancel and then leaves, including the harmless race where the job already finished. This commit is part of the kw integration's step 6. Signed-off-by: lorenzoberts --- src/app/actor.rs | 51 +++++++++-- src/app/integration_tests/kw_status.rs | 114 +++++++++++++++++++++++ src/app/popup.rs | 119 ++++++++++++++++++++++++- src/app/view_model.rs | 42 +++++++++ src/input/event.rs | 1 + src/input/mapper.rs | 11 +++ src/ui/scene.rs | 6 ++ src/ui/screens/popup.rs | 64 ++++++++++++- 8 files changed, 401 insertions(+), 7 deletions(-) diff --git a/src/app/actor.rs b/src/app/actor.rs index 0bcf8acf..905814d4 100644 --- a/src/app/actor.rs +++ b/src/app/actor.rs @@ -22,11 +22,15 @@ use crate::{ }, handle::AppHandle, loading::{terminal_error, TerminalLoadingIndicator}, + popup::{AppPopup, ConfirmAction}, screens::CurrentScreen, App, }, input::{event::InputEvent, handle::InputHandle}, - kw::status::KwStatusSnapshot, + kw::{ + errors::KwError, + status::{KwJobStatus, KwStatusSnapshot}, + }, terminal::{handle::TerminalHandle, messages::TerminalFrame}, ui::handle::UiHandle, }; @@ -177,11 +181,24 @@ async fn on_input( loading: &mut TerminalLoadingIndicator, ) -> Result> { if let Some(popup) = app.state.popup.as_mut() { - if input == InputEvent::ClosePopup { - app.state.popup = None; - } else { - popup.handle_scroll(input); + match input { + InputEvent::ClosePopup => { + app.state.popup = None; + } + InputEvent::ConfirmPopup => match popup.selected_confirm_action() { + Some(ConfirmAction::CancelKwAndQuit) => { + app.state.popup = None; + return Ok(cancel_kw_and_quit(app).await); + } + Some(ConfirmAction::Wait) => { + app.state.popup = None; + } + None => {} + }, + _ => popup.handle_input(input), } + } else if input == InputEvent::Quit && kw_job_is_running(app) { + app.state.popup = Some(AppPopup::quit_while_job_running()); } else { tracing::debug!(screen = ?app.state.navigation.current_screen, "dispatching input to screen handler"); match app.state.navigation.current_screen { @@ -208,6 +225,30 @@ async fn on_input( Ok(ControlFlow::Continue(())) } +fn kw_job_is_running(app: &App) -> bool { + matches!( + app.state.kw.status.as_ref().map(|status| &status.job), + Some(KwJobStatus::Running { .. }) + ) +} + +/// Cancel then leave. A job that finished while the confirm popup was +/// open is `NoJobRunning`; that race is harmless and still quits. +async fn cancel_kw_and_quit(app: &App) -> ControlFlow<()> { + if let Some(kw) = app.services.kw.as_ref() { + match kw.cancel().await { + Ok(()) => {} + Err(KwError::NoJobRunning) => { + tracing::debug!("kw job already finished before cancel-and-quit"); + } + Err(error) => { + tracing::warn!(%error, "kw cancel failed while quitting"); + } + } + } + ControlFlow::Break(()) +} + #[cfg(test)] mod tests { use std::{collections::HashMap, sync::Arc}; diff --git a/src/app/integration_tests/kw_status.rs b/src/app/integration_tests/kw_status.rs index de5141a5..094cb7ff 100644 --- a/src/app/integration_tests/kw_status.rs +++ b/src/app/integration_tests/kw_status.rs @@ -113,6 +113,99 @@ async fn closed_watch_does_not_busy_loop_and_input_still_works() { std::fs::remove_dir_all(&log_dir).unwrap(); } +#[tokio::test(flavor = "multi_thread")] +async fn quit_while_job_running_opens_confirm_and_wait_keeps_app_alive() { + let log_dir = kw_log_dir("quit-wait"); + let (app, kw, process) = app_with_kw(&log_dir); + kw.start_build(start_request()).await.unwrap(); + + let (scenes, event_tx, handle) = spawn_app_actor(app); + wait_for_nav(&scenes, |text| text.contains("kw: building")).await; + + event_tx.send(InputEvent::Quit).await.unwrap(); + wait_for_latest_popup(&scenes, |popup| { + popup.is_some_and(|popup| popup.title == "Cancel build and quit?") + }) + .await; + let latest = scenes.lock().unwrap().last().cloned().unwrap(); + let popup = latest.popup.expect("confirm popup"); + match popup.body { + crate::ui::scene::PopupBody::Confirm { + options, selected, .. + } => { + assert_eq!( + vec!["Cancel and quit".to_string(), "Wait".to_string()], + options + ); + assert_eq!(1, selected); + } + other => panic!("expected Confirm scene, got {other:?}"), + } + + event_tx.send(InputEvent::ConfirmPopup).await.unwrap(); + wait_for_latest_popup(&scenes, |popup| popup.is_none()).await; + wait_for_nav(&scenes, |text| text.contains("kw: building")).await; + + event_tx.send(InputEvent::NavigateDown).await.unwrap(); + process.last_child().finish(0); + drop(event_tx); + handle.run_until_done().await.unwrap(); + kw.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); +} + +#[tokio::test(flavor = "multi_thread")] +async fn quit_confirm_esc_is_wait() { + let log_dir = kw_log_dir("quit-esc-wait"); + let (app, kw, process) = app_with_kw(&log_dir); + kw.start_build(start_request()).await.unwrap(); + + let (scenes, event_tx, handle) = spawn_app_actor(app); + wait_for_nav(&scenes, |text| text.contains("kw: building")).await; + + event_tx.send(InputEvent::Quit).await.unwrap(); + wait_for_latest_popup(&scenes, |popup| popup.is_some()).await; + event_tx.send(InputEvent::ClosePopup).await.unwrap(); + wait_for_latest_popup(&scenes, |popup| popup.is_none()).await; + wait_for_nav(&scenes, |text| text.contains("kw: building")).await; + + process.last_child().finish(0); + drop(event_tx); + handle.run_until_done().await.unwrap(); + kw.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); +} + +#[tokio::test(flavor = "multi_thread")] +async fn cancel_and_quit_requests_cancellation() { + let log_dir = kw_log_dir("quit-cancel"); + let (app, kw, process) = app_with_kw(&log_dir); + kw.start_build(start_request()).await.unwrap(); + + let (scenes, event_tx, handle) = spawn_app_actor(app); + wait_for_nav(&scenes, |text| text.contains("kw: building")).await; + + event_tx.send(InputEvent::Quit).await.unwrap(); + wait_for_latest_popup(&scenes, |popup| popup.is_some()).await; + event_tx.send(InputEvent::NavigateLeft).await.unwrap(); + event_tx.send(InputEvent::ConfirmPopup).await.unwrap(); + + handle.run_until_done().await.unwrap(); + tokio::time::timeout(Duration::from_secs(5), async { + loop { + if process.last_child().was_killed() { + break; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("cancel-and-quit must request kw cancellation"); + drop(event_tx); + kw.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); +} + fn spawn_app_actor( app: crate::app::App, ) -> ( @@ -201,6 +294,27 @@ async fn wait_for_nav(scenes: &Arc>>, predicate: impl Fn(&str .expect("expected navigation text did not appear"); } +async fn wait_for_latest_popup( + scenes: &Arc>>, + predicate: impl Fn(Option<&crate::ui::scene::PopupScene>) -> bool, +) { + tokio::time::timeout(Duration::from_secs(5), async { + loop { + let matches = scenes + .lock() + .unwrap() + .last() + .is_some_and(|scene| predicate(scene.popup.as_ref())); + if matches { + return; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("expected popup state did not appear"); +} + fn scene_count(scenes: &Arc>>) -> usize { scenes.lock().unwrap().len() } diff --git a/src/app/popup.rs b/src/app/popup.rs index 75157e4f..abf31e56 100644 --- a/src/app/popup.rs +++ b/src/app/popup.rs @@ -11,6 +11,16 @@ use crate::{ lore::domain::patch::Author, }; +/// Action taken when the user confirms a choice popup. +/// +/// Step 7's boot-once gate will add its own variant; do not reuse +/// [`ConfirmAction::Wait`] or [`ConfirmAction::CancelKwAndQuit`] for that. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ConfirmAction { + CancelKwAndQuit, + Wait, +} + /// Concrete, cloneable popup state stored in `AppState`. #[derive(Clone, Debug)] pub enum AppPopup { @@ -37,6 +47,13 @@ pub enum AppPopup { max_scroll: (u16, u16), dimensions: (u16, u16), }, + Confirm { + title: String, + body: String, + options: Vec<(String, ConfirmAction)>, + selected: usize, + dimensions: (u16, u16), + }, } impl AppPopup { @@ -103,9 +120,64 @@ impl AppPopup { dimensions: (50, 40), } } + + /// Choice popup shown when the user tries to quit while a kw job runs. + /// + /// [`ConfirmAction::Wait`] is the highlighted default so Enter, like + /// Esc, stays in the app unless the user explicitly picks cancel. + pub fn quit_while_job_running() -> Self { + AppPopup::Confirm { + title: "Cancel build and quit?".to_string(), + body: "A kw job is still running. Cancel it and quit, or wait and stay in the app?" + .to_string(), + options: vec![ + ( + "Cancel and quit".to_string(), + ConfirmAction::CancelKwAndQuit, + ), + ("Wait".to_string(), ConfirmAction::Wait), + ], + selected: 1, + dimensions: (40, 60), + } + } + + /// The currently highlighted confirm action, if this is a choice popup. + pub fn selected_confirm_action(&self) -> Option { + match self { + AppPopup::Confirm { + options, selected, .. + } => options.get(*selected).map(|(_, action)| *action), + _ => None, + } + } + + /// Handle input for whichever popup is open. + /// + /// Info/Help/ReviewTrailers scroll. Confirm moves the highlighted + /// option; Enter is handled by the caller via [`InputEvent::ConfirmPopup`]. + pub fn handle_input(&mut self, input: InputEvent) { + match self { + AppPopup::Confirm { + options, selected, .. + } => match input { + InputEvent::NavigateLeft | InputEvent::NavigateUp => { + *selected = selected.saturating_sub(1); + } + InputEvent::NavigateRight | InputEvent::NavigateDown => { + if *selected + 1 < options.len() { + *selected += 1; + } + } + _ => {} + }, + _ => self.handle_scroll(input), + } + } + /// Advance scroll position in response to a navigation input. /// - /// All popup variants share identical two-axis scroll semantics. + /// Scrollable popup variants share identical two-axis scroll semantics. pub fn handle_scroll(&mut self, input: InputEvent) { let (scroll, max_scroll) = match self { AppPopup::Info { @@ -117,6 +189,7 @@ impl AppPopup { | AppPopup::ReviewTrailers { scroll, max_scroll, .. } => (scroll, max_scroll), + AppPopup::Confirm { .. } => return, }; match input { @@ -142,6 +215,7 @@ impl AppPopup { } } } + /// Fluent builder for `AppPopup::Help`. /// /// Mirrors the API of the old `HelpPopUpBuilder` so handler call sites change @@ -195,3 +269,46 @@ impl AppHelpBuilder { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn quit_confirm_defaults_to_wait() { + let popup = AppPopup::quit_while_job_running(); + assert_eq!(Some(ConfirmAction::Wait), popup.selected_confirm_action()); + } + + #[test] + fn quit_confirm_left_selects_cancel_and_quit() { + let mut popup = AppPopup::quit_while_job_running(); + popup.handle_input(InputEvent::NavigateLeft); + assert_eq!( + Some(ConfirmAction::CancelKwAndQuit), + popup.selected_confirm_action() + ); + popup.handle_input(InputEvent::NavigateLeft); + assert_eq!( + Some(ConfirmAction::CancelKwAndQuit), + popup.selected_confirm_action() + ); + } + + #[test] + fn quit_confirm_right_stays_on_wait() { + let mut popup = AppPopup::quit_while_job_running(); + popup.handle_input(InputEvent::NavigateRight); + assert_eq!(Some(ConfirmAction::Wait), popup.selected_confirm_action()); + } + + #[test] + fn info_popup_still_scrolls() { + let mut popup = AppPopup::info("Title", "line 1\nline 2\nline 3"); + popup.handle_input(InputEvent::NavigateDown); + let AppPopup::Info { scroll, .. } = popup else { + panic!("expected Info popup"); + }; + assert_eq!((1, 0), scroll); + } +} diff --git a/src/app/view_model.rs b/src/app/view_model.rs index 1b6eb3cb..e873365f 100644 --- a/src/app/view_model.rs +++ b/src/app/view_model.rs @@ -134,6 +134,12 @@ pub enum PopupViewBody { tested_by: String, acked_by: String, }, + /// Choice labels only; the selected action stays in `AppPopup`. + Confirm { + body: String, + options: Vec, + selected: usize, + }, } #[derive(Clone, Debug)] @@ -450,6 +456,22 @@ fn project_popup(popup: &AppPopup) -> PopupViewModel { scroll_offset: *scroll, dimensions: *dimensions, }, + AppPopup::Confirm { + title, + body, + options, + selected, + dimensions, + } => PopupViewModel { + title: title.clone(), + body: PopupViewBody::Confirm { + body: body.clone(), + options: options.iter().map(|(label, _)| label.clone()).collect(), + selected: *selected, + }, + scroll_offset: (0, 0), + dimensions: *dimensions, + }, } } @@ -517,6 +539,26 @@ mod tests { assert_eq!(Some("kw: building patchset-x".to_string()), vm.kw_running); } + #[test] + fn confirm_popup_projects_labels_without_actions() { + let mut state = app_state_with_kw(None); + state.popup = Some(AppPopup::quit_while_job_running()); + let vm = project_state(&state); + let popup = vm.popup.expect("confirm popup should project"); + assert_eq!("Cancel build and quit?", popup.title); + let PopupViewBody::Confirm { + options, selected, .. + } = popup.body + else { + panic!("expected Confirm projection"); + }; + assert_eq!( + vec!["Cancel and quit".to_string(), "Wait".to_string()], + options + ); + assert_eq!(1, selected); + } + #[test] fn idle_succeeded_and_missing_status_have_no_global_indicator() { assert_eq!(None, project_state(&app_state_with_kw(None)).kw_running); diff --git a/src/input/event.rs b/src/input/event.rs index b887e16a..be03ec2d 100644 --- a/src/input/event.rs +++ b/src/input/event.rs @@ -52,6 +52,7 @@ pub enum InputEvent { Quit, Back, ClosePopup, + ConfirmPopup, OpenHelp, TextInput(char), Backspace, diff --git a/src/input/mapper.rs b/src/input/mapper.rs index 5166dc7b..325bb516 100644 --- a/src/input/mapper.rs +++ b/src/input/mapper.rs @@ -65,6 +65,7 @@ impl InputMapper { fn map_popup_key(&mut self, key: &KeyInput) -> Option { match key.code { KeyCode::Esc | KeyCode::Char('q') => Some(InputEvent::ClosePopup), + KeyCode::Enter => Some(InputEvent::ConfirmPopup), KeyCode::Char('j') | KeyCode::Down => Some(InputEvent::NavigateDown), KeyCode::Char('k') | KeyCode::Up => Some(InputEvent::NavigateUp), KeyCode::Char('h') | KeyCode::Left => Some(InputEvent::NavigateLeft), @@ -251,6 +252,16 @@ mod tests { assert_eq!(event, Some(InputEvent::ClosePopup)); } + #[test] + fn maps_enter_to_confirm_popup_when_popup_is_open() { + let mut mapper = InputMapper::default(); + let context = context(CurrentScreen::MailingListSelection).with_popup_open(true); + + let event = mapper.map_terminal_event(key(KeyCode::Enter), &context); + + assert_eq!(event, Some(InputEvent::ConfirmPopup)); + } + #[test] fn maps_escape_to_back_on_details_without_popup() { let mut mapper = InputMapper::default(); diff --git a/src/ui/scene.rs b/src/ui/scene.rs index b6c69988..63a01df9 100644 --- a/src/ui/scene.rs +++ b/src/ui/scene.rs @@ -105,6 +105,12 @@ pub enum PopupBody { tested_by: String, acked_by: String, }, + /// Confirmation choices. Labels only; the application owns the action. + Confirm { + body: String, + options: Vec, + selected: usize, + }, } /// Fully projected popup ready to be painted. diff --git a/src/ui/screens/popup.rs b/src/ui/screens/popup.rs index 41a0e014..08fed1c0 100644 --- a/src/ui/screens/popup.rs +++ b/src/ui/screens/popup.rs @@ -1,7 +1,7 @@ use ratatui::{ layout::{Alignment, Rect}, style::{Color, Modifier, Style, Stylize}, - text::Line, + text::{Line, Span}, widgets::{Block, BorderType, Borders, Clear, Paragraph, Wrap}, Frame, }; @@ -10,6 +10,7 @@ use crate::{ app::view_model::{PopupViewBody, PopupViewModel}, ui::scene::{PopupBody, PopupScene}, }; + pub fn build_scene(vm: &PopupViewModel) -> PopupScene { let body = match &vm.body { PopupViewBody::Text(text) => PopupBody::Text(text.clone()), @@ -29,6 +30,15 @@ pub fn build_scene(vm: &PopupViewModel) -> PopupScene { tested_by: tested_by.clone(), acked_by: acked_by.clone(), }, + PopupViewBody::Confirm { + body, + options, + selected, + } => PopupBody::Confirm { + body: body.clone(), + options: options.clone(), + selected: *selected, + }, }; PopupScene { @@ -65,6 +75,11 @@ pub fn paint(f: &mut Frame, scene: &PopupScene, chunk: Rect) { scene.scroll_offset, chunk, ), + PopupBody::Confirm { + body, + options, + selected, + } => paint_confirm(f, &scene.title, body, options, *selected, chunk), } } @@ -178,3 +193,50 @@ fn paint_review_trailers( f.render_widget(Clear, chunk); f.render_widget(paragraph, chunk); } + +fn paint_confirm( + f: &mut Frame, + title: &str, + body: &str, + options: &[String], + selected: usize, + chunk: Rect, +) { + let bold_blue = Style::default() + .add_modifier(Modifier::BOLD) + .fg(Color::Blue); + let selected_style = Style::default() + .fg(Color::Yellow) + .add_modifier(Modifier::BOLD) + .add_modifier(Modifier::REVERSED); + let block = Block::default() + .title(title.to_string()) + .title_alignment(Alignment::Center) + .title_style(bold_blue) + .title_bottom(Line::styled("(ENTER) confirm | (ESC / q) wait", bold_blue)) + .borders(Borders::ALL) + .border_type(BorderType::Double) + .style(Style::default()); + + let mut lines = vec![Line::from(body.to_string()), Line::from("")]; + let mut option_spans = Vec::new(); + for (index, label) in options.iter().enumerate() { + if index > 0 { + option_spans.push(Span::raw(" ")); + } + if index == selected { + option_spans.push(Span::styled(format!("[{label}]"), selected_style)); + } else { + option_spans.push(Span::raw(format!(" {label} "))); + } + } + lines.push(Line::from(option_spans)); + + let paragraph = Paragraph::new(lines) + .block(block) + .alignment(Alignment::Left) + .wrap(Wrap { trim: true }); + + f.render_widget(Clear, chunk); + f.render_widget(paragraph, chunk); +} From 34936d4bdd29496c778b8f32629f2ce49ce781d1 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Fri, 18 Sep 2026 17:42:24 -0300 Subject: [PATCH 3/6] feat(app): add KwOps build screen and controls This commit adds an explicit KwOps build dashboard from Patchset Details so a running job can be started, cancelled, and restored without blocking the TUI. The branch field is prefilled from HEAD, and extras that would hang, wipe the tree, or mutate git are stripped rather than forwarded to kw. This commit is part of the kw integration's step 6. Signed-off-by: lorenzoberts --- src/app/actor.rs | 7 +- src/app/flows/details_actions.rs | 5 + src/app/flows/kw_ops.rs | 232 ++++++++++++++++++++ src/app/flows/mod.rs | 1 + src/app/input.rs | 1 + src/app/integration_tests/kw_ops.rs | 329 ++++++++++++++++++++++++++++ src/app/integration_tests/mod.rs | 1 + src/app/screens/kw_ops.rs | 192 ++++++++++++++++ src/app/screens/mod.rs | 2 + src/app/state.rs | 3 +- src/app/view_model.rs | 215 +++++++++++++++++- src/input/context.rs | 8 + src/input/event.rs | 7 + src/input/mapper.rs | 70 ++++++ src/kw/argv.rs | 26 +++ src/kw/readiness.rs | 32 +++ src/ui/core.rs | 5 + src/ui/painter.rs | 1 + src/ui/scene.rs | 26 +++ src/ui/screens/details.rs | 13 +- src/ui/screens/kw_ops.rs | 145 ++++++++++++ src/ui/screens/mod.rs | 1 + 22 files changed, 1315 insertions(+), 7 deletions(-) create mode 100644 src/app/flows/kw_ops.rs create mode 100644 src/app/integration_tests/kw_ops.rs create mode 100644 src/app/screens/kw_ops.rs create mode 100644 src/ui/screens/kw_ops.rs diff --git a/src/app/actor.rs b/src/app/actor.rs index 905814d4..0045ae4a 100644 --- a/src/app/actor.rs +++ b/src/app/actor.rs @@ -17,8 +17,8 @@ use crate::{ app::{ flows::{ bookmarked::handle_bookmarked_patchsets, details_actions::handle_patchset_details, - edit_config::handle_edit_config, latest::handle_latest_patchsets, - mail_list::handle_mailing_list_selection, + edit_config::handle_edit_config, kw_ops::handle_kw_ops, + latest::handle_latest_patchsets, mail_list::handle_mailing_list_selection, }, handle::AppHandle, loading::{terminal_error, TerminalLoadingIndicator}, @@ -220,6 +220,9 @@ async fn on_input( CurrentScreen::LatestPatchsets => { handle_latest_patchsets(app, input, loading).await?; } + CurrentScreen::KwOps => { + handle_kw_ops(app, input).await?; + } } } Ok(ControlFlow::Continue(())) diff --git a/src/app/flows/details_actions.rs b/src/app/flows/details_actions.rs index 15a101c8..62e3714f 100644 --- a/src/app/flows/details_actions.rs +++ b/src/app/flows/details_actions.rs @@ -17,6 +17,10 @@ pub async fn handle_patchset_details( input: InputEvent, terminal_handle: &TerminalHandle, ) -> Result<()> { + if input == InputEvent::OpenKwOps { + return crate::app::flows::kw_ops::open_kw_ops(app).await; + } + let patchset_details_and_actions = app .state .lore @@ -139,5 +143,6 @@ pub fn generate_help_popup() -> AppPopup { .keybind("r", "Toggle reply with Reviewed-by action") .keybind("Shift+r", "Toggle reply with Reviewed-by action for all patches") .keybind("Ctrl+t", "Show code-review trailers details") + .keybind("w", "Kw operations") .build() } diff --git a/src/app/flows/kw_ops.rs b/src/app/flows/kw_ops.rs new file mode 100644 index 00000000..134d215d --- /dev/null +++ b/src/app/flows/kw_ops.rs @@ -0,0 +1,232 @@ +use color_eyre::Result; + +use crate::{ + app::{ + popup::AppPopup, + screens::{kw_ops::KwOpsState, CurrentScreen}, + App, + }, + input::event::InputEvent, + kw::{errors::KwError, messages::StartRequest, status::KwJobStatus}, +}; + +pub async fn handle_kw_ops(app: &mut App, input: InputEvent) -> Result<()> { + let editing = app.state.kw.ops.as_ref().is_some_and(|ops| ops.editing); + if editing { + let Some(ops) = app.state.kw.ops.as_mut() else { + return Ok(()); + }; + match input { + InputEvent::CancelKwOpsEdit => ops.cancel_edit(), + InputEvent::Backspace => ops.backspace_edit(), + InputEvent::TextInput(ch) => ops.append_edit(ch), + InputEvent::StageKwOpsEdit => ops.commit_edit(), + _ => {} + } + return Ok(()); + } + + match input { + InputEvent::OpenHelp => { + app.state.popup = Some(generate_help_popup()); + } + InputEvent::Back => { + app.set_current_screen(CurrentScreen::PatchsetDetails); + } + InputEvent::NavigateDown => { + if let Some(ops) = app.state.kw.ops.as_mut() { + ops.highlight_next(); + } + } + InputEvent::NavigateUp => { + if let Some(ops) = app.state.kw.ops.as_mut() { + ops.highlight_prev(); + } + } + InputEvent::EditKwOpsField => { + if !job_is_running(app) { + if let Some(ops) = app.state.kw.ops.as_mut() { + ops.begin_edit(); + } + } + } + InputEvent::StartKwBuild => start_build(app).await?, + InputEvent::CancelKwJob => cancel_job(app).await?, + InputEvent::RestoreKwBranch => restore_branch(app).await?, + _ => {} + } + Ok(()) +} + +/// Opens KwOps from Patchset Details. Failures keep Details active. +pub async fn open_kw_ops(app: &mut App) -> Result<()> { + let Some(details) = app.state.lore.details.as_ref() else { + return Ok(()); + }; + let patchset_title = details.representative_patch.title().clone(); + let message_id = details.representative_patch.message_id().href.clone(); + + let Some(kw) = app.services.kw.clone() else { + app.state.popup = Some(AppPopup::info( + "Kw operations unavailable", + "kw jobs require a Unix kw actor, which is not attached.", + )); + return Ok(()); + }; + + let Some(kernel_tree_id) = app.state.config.target_kernel_tree().clone() else { + app.state.popup = Some(AppPopup::info( + "Kw operations unavailable", + "No target kernel tree is configured. Set target_kernel_tree in the config.", + )); + return Ok(()); + }; + let Some(tree) = app.state.config.get_kernel_tree(&kernel_tree_id).cloned() else { + app.state.popup = Some(AppPopup::info( + "Kw operations unavailable", + format!("Target kernel tree '{kernel_tree_id}' is not in the configuration."), + )); + return Ok(()); + }; + + match kw.get_readiness(&kernel_tree_id, &tree).await { + Ok(readiness) => { + app.state.kw.ops = Some(KwOpsState::new( + patchset_title, + message_id, + kernel_tree_id, + tree, + readiness, + )); + app.set_current_screen(CurrentScreen::KwOps); + } + Err(error) => { + app.state.popup = Some(AppPopup::info( + "Kw operations unavailable", + error.to_string(), + )); + } + } + Ok(()) +} + +async fn start_build(app: &mut App) -> Result<()> { + if job_is_running(app) { + return Ok(()); + } + let Some(ops) = app.state.kw.ops.as_ref() else { + return Ok(()); + }; + let branch = ops.branch.trim().to_string(); + if branch.is_empty() { + let body = if ops.head_unreadable { + "HEAD is detached or unverifiable. Type a branch name before starting a build." + } else { + "Set a branch before starting a build." + }; + app.state.popup = Some(AppPopup::info("Cannot start build", body)); + return Ok(()); + } + let Some(kw) = app.services.kw.clone() else { + app.state.popup = Some(AppPopup::info( + "Cannot start build", + "kw jobs require a Unix kw actor, which is not attached.", + )); + return Ok(()); + }; + let request = StartRequest { + kernel_tree_id: ops.kernel_tree_id.clone(), + tree: ops.tree.clone(), + branch, + extra_args: ops.extra_arg_tokens(), + }; + match kw.start_build(request).await { + Ok(()) => { + if let Some(ops) = app.state.kw.ops.as_mut() { + ops.cancel_requested = false; + } + } + Err(error) => { + app.state.popup = Some(AppPopup::info("Cannot start build", error.to_string())); + } + } + Ok(()) +} + +async fn cancel_job(app: &mut App) -> Result<()> { + if !job_is_running(app) { + return Ok(()); + } + let Some(kw) = app.services.kw.clone() else { + return Ok(()); + }; + match kw.cancel().await { + Ok(()) => { + if let Some(ops) = app.state.kw.ops.as_mut() { + ops.cancel_requested = true; + } + } + Err(KwError::NoJobRunning) => { + if let Some(ops) = app.state.kw.ops.as_mut() { + ops.cancel_requested = false; + } + } + Err(error) => { + app.state.popup = Some(AppPopup::info("Cannot cancel job", error.to_string())); + } + } + Ok(()) +} + +async fn restore_branch(app: &mut App) -> Result<()> { + if job_is_running(app) { + return Ok(()); + } + let Some(restore) = app + .state + .kw + .status + .as_ref() + .and_then(|status| status.restore_branch.clone()) + else { + return Ok(()); + }; + let Some(kw) = app.services.kw.clone() else { + return Ok(()); + }; + match kw.restore_previous_branch().await { + Ok(()) => { + if let Some(ops) = app.state.kw.ops.as_mut() { + ops.branch = restore; + ops.head_unreadable = false; + } + } + Err(error) => { + app.state.popup = Some(AppPopup::info("Cannot restore branch", error.to_string())); + } + } + Ok(()) +} + +fn job_is_running(app: &App) -> bool { + matches!( + app.state.kw.status.as_ref().map(|status| &status.job), + Some(KwJobStatus::Running { .. }) + ) +} + +pub fn generate_help_popup() -> AppPopup { + AppPopup::help() + .title("Kw operations") + .description( + "Start a kw build on the configured target kernel tree. Deploy is not available yet.", + ) + .keybind("ESC / q", "Return to patchset details") + .keybind("j/k", "Move between branch and extra arguments") + .keybind("e / ENTER", "Edit the focused field") + .keybind("b", "Start build") + .keybind("c", "Cancel the running job") + .keybind("r", "Restore the previous branch") + .keybind("?", "Show this help screen") + .build() +} diff --git a/src/app/flows/mod.rs b/src/app/flows/mod.rs index ae496ccd..b074bd64 100644 --- a/src/app/flows/mod.rs +++ b/src/app/flows/mod.rs @@ -1,6 +1,7 @@ pub(crate) mod bookmarked; pub(crate) mod details_actions; pub(crate) mod edit_config; +pub(crate) mod kw_ops; pub(crate) mod latest; pub(crate) mod mail_list; mod open_patchset; diff --git a/src/app/input.rs b/src/app/input.rs index 70119a1a..e212d75f 100644 --- a/src/app/input.rs +++ b/src/app/input.rs @@ -12,6 +12,7 @@ impl App { .edit_config .as_ref() .is_some_and(|edit_config| edit_config.is_editing()), + kw_ops_editing: self.state.kw.ops.as_ref().is_some_and(|ops| ops.editing), preview_fullscreen: self .state .lore diff --git a/src/app/integration_tests/kw_ops.rs b/src/app/integration_tests/kw_ops.rs new file mode 100644 index 00000000..977fa3c2 --- /dev/null +++ b/src/app/integration_tests/kw_ops.rs @@ -0,0 +1,329 @@ +use crate::{ + app::{ + flows::details_actions::handle_patchset_details, + popup::AppPopup, + screens::{details_actions::PatchsetDetailsState, CurrentScreen}, + }, + input::event::InputEvent, +}; + +use super::helpers::{ + app_harness::{dummy_terminal_handle, AppHarness}, + lore::{sample_patch, sample_patchset_details}, + render::sample_rendered_preview, +}; + +#[tokio::test] +async fn open_kw_ops_without_actor_stays_on_details() { + let mut harness = AppHarness::new(); + harness.app.state.navigation.current_screen = CurrentScreen::PatchsetDetails; + harness.app.state.lore.details = Some(details_state()); + + handle_patchset_details( + &mut harness.app, + InputEvent::OpenKwOps, + &dummy_terminal_handle(), + ) + .await + .unwrap(); + + assert_eq!( + CurrentScreen::PatchsetDetails, + harness.app.state.navigation.current_screen + ); + assert_info_popup( + harness.app.state.popup.as_ref(), + "Kw operations unavailable", + "not attached", + ); +} + +fn details_state() -> PatchsetDetailsState { + PatchsetDetailsState::from_rendered_preview( + sample_patch(), + sample_patchset_details(), + sample_rendered_preview(), + false, + CurrentScreen::LatestPatchsets, + ) +} + +fn assert_info_popup(popup: Option<&AppPopup>, expected_title: &str, fragment: &str) { + let Some(AppPopup::Info { title, body, .. }) = popup else { + panic!("expected info popup"); + }; + assert_eq!(expected_title, title); + assert!( + body.contains(fragment), + "expected popup body to contain {fragment:?}, got {body:?}" + ); +} + +#[cfg(unix)] +mod unix { + use std::{ + path::PathBuf, + sync::{ + atomic::{AtomicU64, Ordering}, + Arc, + }, + }; + + use crate::{ + app::{ + flows::{details_actions::handle_patchset_details, kw_ops::handle_kw_ops}, + screens::CurrentScreen, + App, + }, + config::{ConfigSnapshot, ConfigState}, + infrastructure::{ + env::MockEnvTrait, + file_system::{FileSystemError, MockFileSystemTrait}, + process::FakeProcess, + shell::{MockShellTrait, ShellOutput}, + }, + input::event::InputEvent, + kw::{actor::KwActor, history::MockKwHistoryStore, status::KwJobStatus}, + lore::application::cache::BootstrapLoreData, + }; + + use super::{assert_info_popup, details_state}; + use crate::app::integration_tests::helpers::{ + app_harness::{dummy_config_handle, dummy_render_handle, dummy_terminal_handle}, + lore::{lore_handle_with_persistence, sample_mailing_list}, + }; + + static LOG_DIR_SEQ: AtomicU64 = AtomicU64::new(0); + + #[tokio::test] + async fn open_kw_ops_prefills_branch_from_head_not_tree_config() { + let log_dir = kw_log_dir("open-head"); + let mut app = app_with_details_and_kw(&log_dir, head_branch_shell("feature")); + + handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) + .await + .unwrap(); + + assert_eq!(CurrentScreen::KwOps, app.state.navigation.current_screen); + let ops = app.state.kw.ops.as_ref().expect("KwOps state"); + assert_eq!("feature", ops.branch); + assert_eq!("main", ops.tree.branch()); + assert!(!ops.head_unreadable); + + shutdown_kw(&app).await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_rejects_empty_branch() { + let log_dir = kw_log_dir("empty-branch"); + let mut app = app_with_details_and_kw(&log_dir, head_branch_shell("")); + handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) + .await + .unwrap(); + assert!(app + .state + .kw + .ops + .as_ref() + .is_some_and(|ops| ops.branch.is_empty())); + + handle_kw_ops(&mut app, InputEvent::StartKwBuild) + .await + .unwrap(); + assert_info_popup( + app.state.popup.as_ref(), + "Cannot start build", + "detached or unverifiable", + ); + + shutdown_kw(&app).await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn start_build_accepts_and_cancel_sets_requested() { + let log_dir = kw_log_dir("start-cancel"); + let process = Arc::new(FakeProcess::new()); + let mut app = app_with_details_and_kw_process( + &log_dir, + head_branch_shell("feature"), + process.clone(), + ); + handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) + .await + .unwrap(); + + handle_kw_ops(&mut app, InputEvent::StartKwBuild) + .await + .unwrap(); + assert!(app.state.popup.is_none()); + let spawned = process.spawned(); + assert_eq!(1, spawned.len()); + assert_eq!(vec!["build"], spawned[0].args); + + let kw = app.services.kw.as_ref().unwrap().clone(); + let status = kw.get_status().await.unwrap(); + app.state.kw.status = Some(status); + assert!(matches!( + app.state.kw.status.as_ref().map(|s| &s.job), + Some(KwJobStatus::Running { .. }) + )); + + handle_kw_ops(&mut app, InputEvent::CancelKwJob) + .await + .unwrap(); + assert!(app + .state + .kw + .ops + .as_ref() + .is_some_and(|ops| ops.cancel_requested)); + + process.last_child().finish(0); + shutdown_kw(&app).await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn back_returns_to_details() { + let log_dir = kw_log_dir("back"); + let mut app = app_with_details_and_kw(&log_dir, head_branch_shell("feature")); + handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) + .await + .unwrap(); + handle_kw_ops(&mut app, InputEvent::Back).await.unwrap(); + assert_eq!( + CurrentScreen::PatchsetDetails, + app.state.navigation.current_screen + ); + assert!(app.state.lore.details.is_some()); + + shutdown_kw(&app).await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + fn app_with_details_and_kw(log_dir: &std::path::Path, kw_shell: MockShellTrait) -> App { + app_with_details_and_kw_process(log_dir, kw_shell, Arc::new(FakeProcess::new())) + } + + fn app_with_details_and_kw_process( + log_dir: &std::path::Path, + kw_shell: MockShellTrait, + process: Arc, + ) -> App { + let mut history = MockKwHistoryStore::new(); + history + .expect_apply_record_for_branch() + .returning(|_, _| Ok(None)); + history.expect_record_build().returning(|_| Ok(())); + history + .expect_build_records() + .returning(|_, _| Ok((None, None))); + let kw = KwActor::spawn( + Arc::new(history), + process, + Arc::new(kw_shell), + Arc::new(kw_actor_fs()), + Arc::new(kw_actor_env()), + log_dir.to_path_buf(), + ); + let mut app = App::new( + apply_config(), + dummy_config_handle(), + BootstrapLoreData { + mailing_lists: vec![sample_mailing_list()], + bookmarks: vec![], + reviewed: Default::default(), + }, + Box::new(MockFileSystemTrait::new()), + Box::new(MockShellTrait::new()), + lore_handle_with_persistence(), + dummy_render_handle(), + Arc::new(MockKwHistoryStore::new()), + Some(kw), + ) + .expect("app should build"); + app.state.navigation.current_screen = CurrentScreen::PatchsetDetails; + app.state.lore.details = Some(details_state()); + app + } + + fn head_branch_shell(branch: &str) -> MockShellTrait { + let branch = branch.to_string(); + let mut shell = MockShellTrait::new(); + shell.expect_execute().returning(move |cmd| { + let stdout = match cmd.program.as_str() { + "kw" => b"kw, version 0.10.0\n".to_vec(), + _ if cmd.args.iter().any(|arg| arg == "status") => Vec::new(), + _ => format!("{branch}\n").into_bytes(), + }; + Ok(ShellOutput { + stdout, + stderr: Vec::new(), + success: true, + }) + }); + shell + } + + fn kw_actor_fs() -> MockFileSystemTrait { + let mut fs = MockFileSystemTrait::new(); + fs.expect_is_dir().returning(|_| true); + fs.expect_is_file() + .returning(|path| !path.ends_with(".kw/env.current")); + fs.expect_exists().returning(|_| true); + fs.expect_read_to_string().returning(|_| { + Err(FileSystemError::IoError(std::io::Error::new( + std::io::ErrorKind::NotFound, + "missing", + ))) + }); + fs.expect_read_dir().returning(|_| { + Err(FileSystemError::IoError(std::io::Error::new( + std::io::ErrorKind::NotFound, + "missing", + ))) + }); + fs.expect_create_dir_all().returning(|_| Ok(())); + fs + } + + fn kw_actor_env() -> MockEnvTrait { + let mut env = MockEnvTrait::new(); + env.expect_which().returning(|_| true); + env + } + + fn apply_config() -> ConfigSnapshot { + serde_json::from_value::(serde_json::json!({ + "kernel_trees": { + "linux": { + "path": "/kernel", + "branch": "main" + } + }, + "target_kernel_tree": "linux" + })) + .expect("test config should deserialize") + .to_snapshot() + } + + async fn shutdown_kw(app: &App) { + if let Some(kw) = &app.services.kw { + kw.shutdown().await; + } + } + + fn kw_log_dir(test_name: &str) -> PathBuf { + let n = LOG_DIR_SEQ.fetch_add(1, Ordering::SeqCst); + let dir = std::env::temp_dir().join(format!( + "patch-hub-app-kw-ops-{}-{}-{n}", + test_name, + std::process::id() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + dir + } +} diff --git a/src/app/integration_tests/mod.rs b/src/app/integration_tests/mod.rs index e4aa0260..6f65a510 100644 --- a/src/app/integration_tests/mod.rs +++ b/src/app/integration_tests/mod.rs @@ -2,6 +2,7 @@ mod actor_lifecycle; mod flow_errors; mod flow_navigation; mod helpers; +mod kw_ops; #[cfg(unix)] mod kw_status; mod patchset_actions; diff --git a/src/app/screens/kw_ops.rs b/src/app/screens/kw_ops.rs new file mode 100644 index 00000000..4a720f45 --- /dev/null +++ b/src/app/screens/kw_ops.rs @@ -0,0 +1,192 @@ +use crate::{config::KernelTree, kw::readiness::KwReadiness}; + +/// Which editable KwOps field is focused. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub enum KwOpsFocus { + #[default] + Branch, + ExtraArgs, +} + +/// Build-path form on the KwOps screen. Job status lives in [`crate::app::state::KwUiState::status`]. +#[derive(Clone, Debug)] +pub struct KwOpsState { + pub patchset_title: String, + pub message_id: String, + pub kernel_tree_id: String, + pub tree: KernelTree, + pub branch: String, + pub extra_args: String, + pub focus: KwOpsFocus, + pub editing: bool, + pub edit_buffer: String, + pub readiness: KwReadiness, + /// True when readiness could not name HEAD; Start stays disabled until + /// the user types a branch (we never guess from `KernelTree.branch`). + pub head_unreadable: bool, + /// Presentation-only; the next commit fills this from a bounded tail. + pub log_tail: String, + pub cancel_requested: bool, +} + +impl KwOpsState { + pub fn new( + patchset_title: String, + message_id: String, + kernel_tree_id: String, + tree: KernelTree, + readiness: KwReadiness, + ) -> Self { + let head_unreadable = readiness.current_branch.is_none(); + let branch = readiness.current_branch.clone().unwrap_or_default(); + Self { + patchset_title, + message_id, + kernel_tree_id, + tree, + branch, + extra_args: String::new(), + focus: KwOpsFocus::Branch, + editing: false, + edit_buffer: String::new(), + readiness, + head_unreadable, + log_tail: String::new(), + cancel_requested: false, + } + } + + pub fn extra_arg_tokens(&self) -> Vec { + self.extra_args + .split_whitespace() + .map(str::to_string) + .collect() + } + + pub fn highlight_prev(&mut self) { + self.focus = KwOpsFocus::Branch; + } + + pub fn highlight_next(&mut self) { + self.focus = KwOpsFocus::ExtraArgs; + } + + pub fn begin_edit(&mut self) { + self.edit_buffer = match self.focus { + KwOpsFocus::Branch => self.branch.clone(), + KwOpsFocus::ExtraArgs => self.extra_args.clone(), + }; + self.editing = true; + } + + pub fn commit_edit(&mut self) { + match self.focus { + KwOpsFocus::Branch => self.branch = self.edit_buffer.trim().to_string(), + KwOpsFocus::ExtraArgs => self.extra_args = self.edit_buffer.clone(), + } + self.editing = false; + self.edit_buffer.clear(); + } + + pub fn cancel_edit(&mut self) { + self.editing = false; + self.edit_buffer.clear(); + } + + pub fn backspace_edit(&mut self) { + self.edit_buffer.pop(); + } + + pub fn append_edit(&mut self, ch: char) { + self.edit_buffer.push(ch); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::kw::readiness::{ + DeployAloneRefusal, KwBinaryProbe, KwReadiness, KwVersionCheck, TreeReadiness, + }; + + fn sample_tree() -> KernelTree { + serde_json::from_value(serde_json::json!({ + "path": "/kernel", + "branch": "main" + })) + .expect("kernel tree should deserialize") + } + + fn readiness(branch: Option<&str>) -> KwReadiness { + KwReadiness { + kw_binary: KwBinaryProbe { + available: true, + version_line: Some("kw, version 0.10.0".to_string()), + check: KwVersionCheck::Meets, + }, + tree: TreeReadiness::Ready { + arch: Some("x86_64".to_string()), + }, + output_dir: None, + kernel_image: None, + build_record: None, + latest_build: None, + deploy_alone: Err(DeployAloneRefusal::NoBuildRecord), + current_branch: branch.map(str::to_string), + } + } + + #[test] + fn prefills_branch_from_readiness_not_from_tree_config() { + let ops = KwOpsState::new( + "title".to_string(), + "mid".to_string(), + "linux".to_string(), + sample_tree(), + readiness(Some("feature")), + ); + assert_eq!("feature", ops.branch); + assert!(!ops.head_unreadable); + assert_eq!("main", ops.tree.branch()); + } + + #[test] + fn detached_head_leaves_branch_blank() { + let ops = KwOpsState::new( + "title".to_string(), + "mid".to_string(), + "linux".to_string(), + sample_tree(), + readiness(None), + ); + assert!(ops.branch.is_empty()); + assert!(ops.head_unreadable); + } + + #[test] + fn focus_and_edit_commit_the_active_field() { + let mut ops = KwOpsState::new( + "title".to_string(), + "mid".to_string(), + "linux".to_string(), + sample_tree(), + readiness(Some("main")), + ); + ops.highlight_next(); + ops.begin_edit(); + ops.append_edit('-'); + ops.append_edit('j'); + ops.append_edit('8'); + ops.commit_edit(); + assert_eq!("-j8", ops.extra_args); + assert_eq!(vec!["-j8"], ops.extra_arg_tokens()); + assert!(!ops.editing); + + ops.highlight_prev(); + ops.begin_edit(); + ops.append_edit('x'); + ops.cancel_edit(); + assert_eq!("main", ops.branch); + assert!(!ops.editing); + } +} diff --git a/src/app/screens/mod.rs b/src/app/screens/mod.rs index 4fb235bf..4659f6f1 100644 --- a/src/app/screens/mod.rs +++ b/src/app/screens/mod.rs @@ -1,6 +1,7 @@ pub mod bookmarked; pub mod details_actions; pub mod edit_config; +pub mod kw_ops; pub mod latest; pub mod mail_list; @@ -11,4 +12,5 @@ pub enum CurrentScreen { LatestPatchsets, PatchsetDetails, EditConfig, + KwOps, } diff --git a/src/app/state.rs b/src/app/state.rs index 62b457c4..56b84203 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -5,7 +5,7 @@ use crate::{ popup::AppPopup, screens::{ bookmarked::BookmarkedPatchsetsState, details_actions::PatchsetDetailsState, - edit_config::EditConfigState, latest::LatestPatchsetsState, + edit_config::EditConfigState, kw_ops::KwOpsState, latest::LatestPatchsetsState, mail_list::MailingListSelectionState, CurrentScreen, }, }, @@ -47,6 +47,7 @@ pub struct ConfigUiState { #[derive(Clone, Debug, Default)] pub struct KwUiState { pub status: Option, + pub ops: Option, } /// All application state grouped as the App actor's state. diff --git a/src/app/view_model.rs b/src/app/view_model.rs index e873365f..31e90bb2 100644 --- a/src/app/view_model.rs +++ b/src/app/view_model.rs @@ -12,10 +12,14 @@ use ratatui::text::Text; use super::{ popup::AppPopup, - screens::{details_actions::PatchsetAction, CurrentScreen}, + screens::{details_actions::PatchsetAction, kw_ops::KwOpsFocus, CurrentScreen}, state::AppState, }; -use crate::kw::status::KwStatusSnapshot; +use crate::kw::{ + argv, + readiness::TreeReadiness, + status::{KwJobStatus, KwStatusSnapshot}, +}; /// One mailing list entry shown in the selection list. #[derive(Clone, Debug)] @@ -122,6 +126,31 @@ pub struct EditConfigViewModel { pub is_editing_mode: bool, } +/// Build-only KwOps dashboard. Labels only; actions stay in AppState. +#[derive(Clone, Debug)] +pub struct KwOpsViewModel { + pub patchset_title: String, + pub message_id: String, + pub kernel_tree_id: String, + pub tree_path: String, + pub branch: String, + pub extra_args: String, + pub branch_focused: bool, + pub extras_focused: bool, + pub editing: bool, + pub kw_binary: String, + pub tree_readiness: String, + pub output_dir: String, + pub job_status: String, + pub command: String, + pub start_label: String, + pub cancel_label: String, + pub restore_label: String, + pub deploy_placeholder: String, + pub branch_guidance: Option, + pub log_tail: String, +} + #[derive(Clone, Debug)] pub enum PopupViewBody { Text(String), @@ -158,6 +187,7 @@ pub enum ScreenViewModel { Latest(LatestPatchsetsViewModel), PatchsetDetails(PatchsetDetailsViewModel), EditConfig(EditConfigViewModel), + KwOps(KwOpsViewModel), } /// Owned, typed projection of [`AppState`] for one TUI frame. @@ -200,6 +230,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)), } } @@ -409,6 +440,142 @@ fn project_edit_config(state: &AppState) -> EditConfigViewModel { } } +fn project_kw_ops(state: &AppState) -> KwOpsViewModel { + let ops = state + .kw + .ops + .as_ref() + .expect("KwOps must be initialised before projecting"); + let running = matches!( + state.kw.status.as_ref().map(|status| &status.job), + Some(KwJobStatus::Running { .. }) + ); + let restore_branch = state + .kw + .status + .as_ref() + .and_then(|status| status.restore_branch.clone()); + let branch = if ops.editing && ops.focus == KwOpsFocus::Branch { + ops.edit_buffer.clone() + } else { + ops.branch.clone() + }; + let extra_args = if ops.editing && ops.focus == KwOpsFocus::ExtraArgs { + ops.edit_buffer.clone() + } else { + ops.extra_args.clone() + }; + let start_label = if running { + "unavailable (a job is already running)".to_string() + } else if ops.branch.trim().is_empty() { + "unavailable (set a branch first)".to_string() + } else { + "available (b)".to_string() + }; + let cancel_label = if running { + if ops.cancel_requested { + "requested; waiting for the job to stop".to_string() + } else { + "available (c)".to_string() + } + } else { + "unavailable".to_string() + }; + let restore_label = match (running, restore_branch.as_deref()) { + (true, _) => "unavailable (a job is running)".to_string(), + (false, Some(branch)) => format!("available (r) to {branch}"), + (false, None) => "unavailable".to_string(), + }; + let log_tail = if ops.log_tail.is_empty() { + if running { + "Waiting for kw output…".to_string() + } else { + "(no log yet)".to_string() + } + } else { + ops.log_tail.clone() + }; + + KwOpsViewModel { + patchset_title: ops.patchset_title.clone(), + message_id: ops.message_id.clone(), + kernel_tree_id: ops.kernel_tree_id.clone(), + tree_path: ops.tree.path().clone(), + branch, + extra_args, + branch_focused: ops.focus == KwOpsFocus::Branch, + extras_focused: ops.focus == KwOpsFocus::ExtraArgs, + editing: ops.editing, + kw_binary: format_kw_binary(&ops.readiness.kw_binary), + tree_readiness: format_tree_readiness(&ops.readiness.tree), + output_dir: ops + .readiness + .output_dir + .as_ref() + .map(|path| path.display().to_string()) + .unwrap_or_else(|| "(none)".to_string()), + job_status: format_job_status( + state.kw.status.as_ref().map(|status| &status.job), + ops.cancel_requested, + ), + command: format!("kw {}", argv::build_argv(&ops.extra_arg_tokens()).join(" ")), + start_label, + cancel_label, + restore_label, + deploy_placeholder: "not available yet".to_string(), + branch_guidance: if ops.branch.trim().is_empty() { + Some( + "HEAD is detached or unverifiable; type a branch before starting a build." + .to_string(), + ) + } else { + None + }, + log_tail, + } +} + +fn format_kw_binary(probe: &crate::kw::readiness::KwBinaryProbe) -> String { + if !probe.available { + return "not on PATH".to_string(); + } + match &probe.version_line { + Some(line) => line.clone(), + None => "available (version unknown)".to_string(), + } +} + +fn format_tree_readiness(tree: &TreeReadiness) -> String { + match tree { + TreeReadiness::Ready { arch: Some(arch) } => format!("ready (arch={arch})"), + TreeReadiness::Ready { arch: None } => "ready (arch unset)".to_string(), + other => other.to_string(), + } +} + +fn format_job_status(job: Option<&KwJobStatus>, cancel_requested: bool) -> String { + match job { + None | Some(KwJobStatus::Idle) => "idle".to_string(), + Some(KwJobStatus::Running { phase, branch, .. }) => { + let phase = match phase { + crate::kw::status::KwPhase::Building => "building", + crate::kw::status::KwPhase::Deploying => "deploying", + }; + if cancel_requested { + format!("cancelling {phase} {branch}") + } else { + format!("{phase} {branch}") + } + } + Some(KwJobStatus::Succeeded { branch, .. }) => format!("succeeded on {branch}"), + Some(KwJobStatus::Failed { exit_code, .. }) => match exit_code { + Some(code) => format!("failed (exit {code})"), + None => "failed (exit unknown)".to_string(), + }, + Some(KwJobStatus::Cancelled { .. }) => "cancelled".to_string(), + } +} + fn project_popup(popup: &AppPopup) -> PopupViewModel { match popup { AppPopup::Info { @@ -519,7 +686,7 @@ mod tests { config_state: ConfigUiState { edit_config: None }, config: ConfigState::default().to_snapshot(), popup: None, - kw: KwUiState { status }, + kw: KwUiState { status, ops: None }, } } @@ -559,6 +726,48 @@ mod tests { assert_eq!(1, selected); } + #[test] + fn kw_ops_command_strips_reserved_extras() { + let mut state = app_state_with_kw(None); + state.navigation.current_screen = CurrentScreen::KwOps; + let mut ops = crate::app::screens::kw_ops::KwOpsState::new( + "[PATCH] test".to_string(), + "http://lore.example/123".to_string(), + "linux".to_string(), + serde_json::from_value(serde_json::json!({ + "path": "/kernel", + "branch": "main" + })) + .unwrap(), + crate::kw::readiness::KwReadiness { + kw_binary: crate::kw::readiness::KwBinaryProbe { + available: true, + version_line: Some("kw, version 0.10.0".to_string()), + check: crate::kw::readiness::KwVersionCheck::Meets, + }, + tree: crate::kw::readiness::TreeReadiness::Ready { + arch: Some("x86_64".to_string()), + }, + output_dir: None, + kernel_image: None, + build_record: None, + latest_build: None, + deploy_alone: Err(crate::kw::readiness::DeployAloneRefusal::NoBuildRecord), + current_branch: Some("feature".to_string()), + }, + ); + ops.extra_args = "--verbose --clean --from-sha abc --doc".to_string(); + state.kw.ops = Some(ops); + + let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { + panic!("expected KwOps projection"); + }; + assert_eq!("kw build --verbose", vm.command); + assert_eq!("feature", vm.branch); + assert_eq!("available (b)", vm.start_label); + assert_eq!("not available yet", vm.deploy_placeholder); + } + #[test] fn idle_succeeded_and_missing_status_have_no_global_indicator() { assert_eq!(None, project_state(&app_state_with_kw(None)).kw_running); diff --git a/src/input/context.rs b/src/input/context.rs index 41618767..d6f09ee7 100644 --- a/src/input/context.rs +++ b/src/input/context.rs @@ -6,6 +6,7 @@ pub struct InputContext { pub current_screen: CurrentScreen, pub popup_open: bool, pub edit_config_editing: bool, + pub kw_ops_editing: bool, pub preview_fullscreen: bool, } @@ -16,6 +17,7 @@ impl InputContext { current_screen, popup_open: false, edit_config_editing: false, + kw_ops_editing: false, preview_fullscreen: false, } } @@ -31,4 +33,10 @@ impl InputContext { self.edit_config_editing = edit_config_editing; self } + + #[cfg(test)] + pub fn with_kw_ops_editing(mut self, kw_ops_editing: bool) -> Self { + self.kw_ops_editing = kw_ops_editing; + self + } } diff --git a/src/input/event.rs b/src/input/event.rs index be03ec2d..af54bf6e 100644 --- a/src/input/event.rs +++ b/src/input/event.rs @@ -54,6 +54,13 @@ pub enum InputEvent { ClosePopup, ConfirmPopup, OpenHelp, + OpenKwOps, + StartKwBuild, + CancelKwJob, + RestoreKwBranch, + EditKwOpsField, + StageKwOpsEdit, + CancelKwOpsEdit, TextInput(char), Backspace, NavigateUp, diff --git a/src/input/mapper.rs b/src/input/mapper.rs index 325bb516..8a320a5c 100644 --- a/src/input/mapper.rs +++ b/src/input/mapper.rs @@ -59,6 +59,7 @@ impl InputMapper { CurrentScreen::LatestPatchsets => self.map_latest_key(&key), CurrentScreen::PatchsetDetails => self.map_details_key(&key), CurrentScreen::EditConfig => self.map_edit_config_key(&key, context), + CurrentScreen::KwOps => self.map_kw_ops_key(&key, context), } } @@ -178,11 +179,36 @@ impl InputMapper { KeyCode::Char('p') => Some(InputEvent::PreviewPrevious), KeyCode::Char('b') => Some(InputEvent::ToggleBookmark), KeyCode::Char('r') => Some(InputEvent::ToggleReplyWithReviewedBy), + KeyCode::Char('w') => Some(InputEvent::OpenKwOps), KeyCode::Enter => Some(InputEvent::ConsolidatePatchsetActions), _ => None, } } + fn map_kw_ops_key(&mut self, key: &KeyInput, context: &InputContext) -> Option { + if context.kw_ops_editing { + return match key.code { + KeyCode::Esc => Some(InputEvent::CancelKwOpsEdit), + KeyCode::Backspace => Some(InputEvent::Backspace), + KeyCode::Enter => Some(InputEvent::StageKwOpsEdit), + KeyCode::Char(ch) => Some(InputEvent::TextInput(ch)), + _ => None, + }; + } + + match key.code { + KeyCode::Char('?') => Some(InputEvent::OpenHelp), + KeyCode::Esc | KeyCode::Char('q') => Some(InputEvent::Back), + KeyCode::Char('j') | KeyCode::Down => Some(InputEvent::NavigateDown), + KeyCode::Char('k') | KeyCode::Up => Some(InputEvent::NavigateUp), + KeyCode::Char('e') | KeyCode::Enter => Some(InputEvent::EditKwOpsField), + KeyCode::Char('b') => Some(InputEvent::StartKwBuild), + KeyCode::Char('c') => Some(InputEvent::CancelKwJob), + KeyCode::Char('r') => Some(InputEvent::RestoreKwBranch), + _ => None, + } + } + fn map_details_go_to_first_line_chord(&mut self) -> Option { match self.pending { Some(PendingInput::DetailsGoToFirstLine { started_at }) @@ -379,6 +405,50 @@ mod tests { ); } + #[test] + fn maps_w_on_details_to_open_kw_ops() { + let mut mapper = InputMapper::default(); + let context = context(CurrentScreen::PatchsetDetails); + + assert_eq!( + mapper.map_terminal_event(key(KeyCode::Char('w')), &context), + Some(InputEvent::OpenKwOps) + ); + } + + #[test] + fn maps_kw_ops_keys_and_treats_letters_as_text_while_editing() { + let mut mapper = InputMapper::default(); + let context = context(CurrentScreen::KwOps); + + assert_eq!( + mapper.map_terminal_event(key(KeyCode::Char('b')), &context), + Some(InputEvent::StartKwBuild) + ); + assert_eq!( + mapper.map_terminal_event(key(KeyCode::Char('c')), &context), + Some(InputEvent::CancelKwJob) + ); + assert_eq!( + mapper.map_terminal_event(key(KeyCode::Char('r')), &context), + Some(InputEvent::RestoreKwBranch) + ); + assert_eq!( + mapper.map_terminal_event(key(KeyCode::Char('e')), &context), + Some(InputEvent::EditKwOpsField) + ); + + let editing = context.with_kw_ops_editing(true); + assert_eq!( + mapper.map_terminal_event(key(KeyCode::Char('b')), &editing), + Some(InputEvent::TextInput('b')) + ); + assert_eq!( + mapper.map_terminal_event(key(KeyCode::Esc), &editing), + Some(InputEvent::CancelKwOpsEdit) + ); + } + #[test] fn maps_resize_and_tick_events() { let mut mapper = InputMapper::default(); diff --git a/src/kw/argv.rs b/src/kw/argv.rs index 1870364c..1ec076a0 100644 --- a/src/kw/argv.rs +++ b/src/kw/argv.rs @@ -33,10 +33,19 @@ impl ReservedOption { /// kw beta-0.9 (still what many installs report, including this lab) treats /// unrecognized options as hard failures (`Invalid option`), and the /// unattended default is already `alert=n` in kw's own config. +/// +/// `--clean` / `--full-cleanup` can wipe the tree, `--menu` / `--doc` / +/// `--info` hang a redirected job, and `--from-sha` mutates git. They are +/// stripped from extras and never injected. const BUILD_RESERVED: &[ReservedOption] = &[ ReservedOption::new(&["--alert"], true), ReservedOption::new(&["--save-log-to"], true), ReservedOption::new(&["--menu"], false), + ReservedOption::new(&["--clean"], false), + ReservedOption::new(&["--full-cleanup"], false), + ReservedOption::new(&["--doc"], false), + ReservedOption::new(&["--info"], false), + ReservedOption::new(&["--from-sha"], true), ]; /// The argv for a build job: `kw build ` (reserved extras stripped). @@ -158,6 +167,23 @@ mod tests { ); } + #[test] + fn tree_mutating_and_hanging_build_flags_are_stripped() { + assert_eq!( + vec!["build", "--verbose"], + build_argv(&extras(&[ + "--clean", + "--full-cleanup", + "--doc", + "--info", + "--from-sha", + "abc123", + "--verbose", + ])) + ); + assert_eq!(vec!["build"], build_argv(&extras(&["--from-sha=abc123"]))); + } + #[test] fn boolean_reserved_strips_only_itself_in_all_spellings() { // A boolean reserved option must not eat the following token. diff --git a/src/kw/readiness.rs b/src/kw/readiness.rs index e577c0f2..cc8d2480 100644 --- a/src/kw/readiness.rs +++ b/src/kw/readiness.rs @@ -502,6 +502,9 @@ pub struct KwReadiness { /// `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>, + /// Currently checked-out branch. `None` when HEAD is detached or + /// `git branch --show-current` could not be read. + pub current_branch: Option, } /// Runs all readiness probes for `tree` and composes them into a @@ -553,6 +556,14 @@ pub fn evaluate_readiness( build_record, latest_build, deploy_alone, + current_branch: { + let trimmed = head_branch.trim(); + if trimmed.is_empty() { + None + } else { + Some(trimmed.to_string()) + } + }, }) } @@ -1348,6 +1359,7 @@ last_line_without_newline=yes"; assert_eq!(None, readiness.output_dir); assert!(readiness.kw_binary.available); assert_eq!(KwVersionCheck::Meets, readiness.kw_binary.check); + assert_eq!(Some("patchset-x".to_string()), readiness.current_branch); } #[test] @@ -1425,5 +1437,25 @@ last_line_without_newline=yes"; readiness.deploy_alone ); assert!(!readiness.kw_binary.available); + assert_eq!(Some("patchset-x".to_string()), readiness.current_branch); + } + + #[test] + fn evaluate_readiness_empty_head_has_no_current_branch() { + let dir = make_ready_tree("evaluate-detached"); + let data = TempDir::new("evaluate-detached-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, "") + .unwrap(); + assert_eq!(None, readiness.current_branch); } } diff --git a/src/ui/core.rs b/src/ui/core.rs index c0fdcaaf..bedb5bbb 100644 --- a/src/ui/core.rs +++ b/src/ui/core.rs @@ -38,6 +38,7 @@ 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)), }; let navigation = self.build_navigation(&vm.screen, vm.kw_running.as_deref()); @@ -76,6 +77,10 @@ impl UiCore { screens::edit_config::mode_spans(vm), screens::edit_config::keys_hint_span(vm), ), + ScreenViewModel::KwOps(vm) => ( + screens::kw_ops::mode_spans(), + screens::kw_ops::keys_hint_span(vm.editing), + ), }; if let Some(indicator) = kw_running { mode_spans.push(Span::styled(" | ", Style::default().fg(Color::DarkGray))); diff --git a/src/ui/painter.rs b/src/ui/painter.rs index 2deaee43..e0ba5de1 100644 --- a/src/ui/painter.rs +++ b/src/ui/painter.rs @@ -38,6 +38,7 @@ pub fn paint(f: &mut Frame, scene: &UiScene) { UiBody::Latest(s) => screens::latest::paint(f, s, chunks[1]), UiBody::PatchsetDetails(s) => screens::details::paint(f, s, chunks[1]), UiBody::EditConfig(s) => screens::edit_config::paint(f, s, chunks[1]), + UiBody::KwOps(s) => screens::kw_ops::paint(f, s, chunks[1]), } screens::navigation_bar::paint(f, &scene.navigation, chunks[2]); diff --git a/src/ui/scene.rs b/src/ui/scene.rs index 63a01df9..c5a0cfb5 100644 --- a/src/ui/scene.rs +++ b/src/ui/scene.rs @@ -68,6 +68,31 @@ pub struct EditConfigScene { pub entries: Vec, } +/// Scene for the KwOps build dashboard. +#[derive(Clone, Debug)] +pub struct KwOpsScene { + pub patchset_title: String, + pub message_id: String, + pub kernel_tree_id: String, + pub tree_path: String, + pub branch: String, + pub extra_args: String, + pub branch_focused: bool, + pub extras_focused: bool, + pub editing: bool, + pub kw_binary: String, + pub tree_readiness: String, + pub output_dir: String, + pub job_status: String, + pub command: String, + pub start_label: String, + pub cancel_label: String, + pub restore_label: String, + pub deploy_placeholder: String, + pub branch_guidance: Option, + pub log_tail: String, +} + /// Which screen's scene the body carries. #[derive(Clone, Debug)] pub enum UiBody { @@ -76,6 +101,7 @@ pub enum UiBody { Latest(LatestScene), PatchsetDetails(PatchsetDetailsScene), EditConfig(EditConfigScene), + KwOps(KwOpsScene), } /// Pre-computed navigation-bar content. diff --git a/src/ui/screens/details.rs b/src/ui/screens/details.rs index cf1dc237..5aeaa586 100644 --- a/src/ui/screens/details.rs +++ b/src/ui/screens/details.rs @@ -186,6 +186,17 @@ fn paint_details_and_actions( ), Span::styled("eviewed-by", Style::default().fg(Color::Cyan)), ]), + Line::from(vec![ + Span::styled(" ", Style::default()), + Span::styled( + "w", + Style::default() + .fg(Color::Cyan) + .add_modifier(Modifier::UNDERLINED) + .add_modifier(Modifier::BOLD), + ), + Span::styled(" Kw operations", Style::default().fg(Color::Cyan)), + ]), ]; let patchset_actions = Paragraph::new(patchset_actions) .block( @@ -232,7 +243,7 @@ pub fn mode_spans() -> Vec> { pub fn keys_hint_span() -> Span<'static> { Span::styled( - "(ESC / q) to return | (ENTER) run actions | (?) help", + "(ESC / q) to return | (ENTER) run actions | (w) kw | (?) help", Style::default().fg(Color::Red), ) } diff --git a/src/ui/screens/kw_ops.rs b/src/ui/screens/kw_ops.rs new file mode 100644 index 00000000..d04d242d --- /dev/null +++ b/src/ui/screens/kw_ops.rs @@ -0,0 +1,145 @@ +use ratatui::{ + layout::{Constraint, Direction, Layout, Rect}, + style::{Color, Modifier, Style}, + text::{Line, Span}, + widgets::{Block, Borders, Paragraph, Wrap}, + Frame, +}; + +use crate::{app::view_model::KwOpsViewModel, ui::scene::KwOpsScene}; + +pub fn build_scene(vm: &KwOpsViewModel) -> KwOpsScene { + KwOpsScene { + patchset_title: vm.patchset_title.clone(), + message_id: vm.message_id.clone(), + kernel_tree_id: vm.kernel_tree_id.clone(), + tree_path: vm.tree_path.clone(), + branch: vm.branch.clone(), + extra_args: vm.extra_args.clone(), + branch_focused: vm.branch_focused, + extras_focused: vm.extras_focused, + editing: vm.editing, + kw_binary: vm.kw_binary.clone(), + tree_readiness: vm.tree_readiness.clone(), + output_dir: vm.output_dir.clone(), + job_status: vm.job_status.clone(), + command: vm.command.clone(), + start_label: vm.start_label.clone(), + cancel_label: vm.cancel_label.clone(), + restore_label: vm.restore_label.clone(), + deploy_placeholder: vm.deploy_placeholder.clone(), + branch_guidance: vm.branch_guidance.clone(), + log_tail: vm.log_tail.clone(), + } +} + +pub fn paint(f: &mut Frame, scene: &KwOpsScene, chunk: Rect) { + let chunks = Layout::default() + .direction(Direction::Vertical) + .constraints([Constraint::Percentage(65), Constraint::Percentage(35)]) + .split(chunk); + + paint_form(f, scene, chunks[0]); + paint_log(f, scene, chunks[1]); +} + +fn paint_form(f: &mut Frame, scene: &KwOpsScene, chunk: Rect) { + let mut lines = vec![ + labeled("Patchset", &scene.patchset_title), + labeled("Message-ID", &scene.message_id), + labeled( + "Tree", + &format!("{} {}", scene.kernel_tree_id, scene.tree_path), + ), + field_line("Branch", &scene.branch, scene.branch_focused, scene.editing), + field_line( + "Extra args", + &scene.extra_args, + scene.extras_focused, + scene.editing, + ), + ]; + if let Some(guidance) = &scene.branch_guidance { + lines.push(Line::from(Span::styled( + guidance.clone(), + Style::default().fg(Color::Yellow), + ))); + } + lines.extend([ + Line::from(""), + labeled("kw", &scene.kw_binary), + labeled("Tree status", &scene.tree_readiness), + labeled("Output dir", &scene.output_dir), + labeled("Job", &scene.job_status), + labeled("Command", &scene.command), + labeled("Start", &scene.start_label), + labeled("Cancel", &scene.cancel_label), + labeled("Restore", &scene.restore_label), + labeled("Deploy", &scene.deploy_placeholder), + ]); + + let paragraph = Paragraph::new(lines) + .block( + Block::default() + .borders(Borders::ALL) + .title(" Kw operations "), + ) + .wrap(Wrap { trim: true }); + f.render_widget(paragraph, chunk); +} + +fn paint_log(f: &mut Frame, scene: &KwOpsScene, chunk: Rect) { + let paragraph = Paragraph::new(scene.log_tail.clone()) + .block(Block::default().borders(Borders::ALL).title(" Build log ")) + .wrap(Wrap { trim: false }); + f.render_widget(paragraph, chunk); +} + +fn labeled(label: &str, value: &str) -> Line<'static> { + Line::from(vec![ + Span::styled(format!("{label}: "), Style::default().fg(Color::Cyan)), + Span::styled(value.to_string(), Style::default().fg(Color::White)), + ]) +} + +fn field_line(label: &str, value: &str, focused: bool, editing: bool) -> Line<'static> { + let display = if value.is_empty() { " " } else { value }; + let mut style = Style::default().fg(Color::White); + if focused { + style = style.add_modifier(Modifier::BOLD); + if editing { + style = style.fg(Color::LightYellow); + } else { + style = style.fg(Color::Yellow); + } + } + let mut spans = vec![ + Span::styled(format!("{label}: "), Style::default().fg(Color::Cyan)), + Span::styled(display.to_string(), style), + ]; + if focused && editing { + spans.push(Span::styled(" ", Style::default().bg(Color::White))); + } + Line::from(spans) +} + +pub fn mode_spans() -> Vec> { + vec![Span::styled( + "Kw operations", + Style::default().fg(Color::Green), + )] +} + +pub fn keys_hint_span(editing: bool) -> Span<'static> { + if editing { + Span::styled( + "(ESC) cancel edit | (ENTER) confirm", + Style::default().fg(Color::Red), + ) + } else { + Span::styled( + "(ESC / q) back | (e) edit | (b) build | (c) cancel | (r) restore | (?) help", + Style::default().fg(Color::Red), + ) + } +} diff --git a/src/ui/screens/mod.rs b/src/ui/screens/mod.rs index b348635f..9ff1946f 100644 --- a/src/ui/screens/mod.rs +++ b/src/ui/screens/mod.rs @@ -1,6 +1,7 @@ pub mod bookmarked; pub mod details; pub mod edit_config; +pub mod kw_ops; pub mod latest; pub mod mailing_list; pub mod navigation_bar; From e790aca595098327aeadc56bc5fc9a46223dfe03 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Fri, 18 Sep 2026 17:49:43 -0300 Subject: [PATCH 4/6] feat(app): refresh KwOps log tail while builds run This commit reads a bounded tail of the kw job log so KwOps can show live build output without loading the whole file or blocking the TUI. The redraw tick runs only while KwOps is visible and a job is running, and a missing log is treated as "waiting" rather than an error. This commit is part of the kw integration's step 6. Signed-off-by: lorenzoberts --- src/app/actor.rs | 61 ++++++-- src/app/flows/kw_ops.rs | 121 +++++++++++++++ src/app/integration_tests/kw_ops.rs | 197 +++++++++++++++++++++++- src/app/screens/kw_ops.rs | 3 +- src/infrastructure/file_system/mod.rs | 25 ++- src/infrastructure/file_system/tests.rs | 54 +++++++ src/infrastructure/file_system/trait.rs | 7 + src/kw/status.rs | 22 +++ 8 files changed, 468 insertions(+), 22 deletions(-) diff --git a/src/app/actor.rs b/src/app/actor.rs index 0045ae4a..7280a059 100644 --- a/src/app/actor.rs +++ b/src/app/actor.rs @@ -3,22 +3,26 @@ //! Each frame: process system updates → project state to [`AppViewModel`] via //! [`UiHandle`](crate::ui::handle::UiHandle) → draw through //! [`TerminalHandle`](crate::terminal::handle::TerminalHandle) → await the next -//! [`InputEvent`](crate::input::event::InputEvent) or a kw-status change. +//! [`InputEvent`](crate::input::event::InputEvent), a kw-status change, or a +//! KwOps log-tail tick while a build is running on that screen. //! //! The actor stops when the input event channel closes (user quit) or when I/O //! returns an unrecoverable error. Startup dependency checks run before this //! actor is spawned. -use std::ops::ControlFlow; +use std::{ops::ControlFlow, time::Duration}; use color_eyre::{eyre::eyre, Result}; -use tokio::{spawn, sync::mpsc, sync::watch}; +use tokio::{spawn, sync::mpsc, sync::watch, time::MissedTickBehavior}; use crate::{ app::{ flows::{ - bookmarked::handle_bookmarked_patchsets, details_actions::handle_patchset_details, - edit_config::handle_edit_config, kw_ops::handle_kw_ops, - latest::handle_latest_patchsets, mail_list::handle_mailing_list_selection, + bookmarked::handle_bookmarked_patchsets, + details_actions::handle_patchset_details, + edit_config::handle_edit_config, + kw_ops::{handle_kw_ops, refresh_kw_ops_log_tail}, + latest::handle_latest_patchsets, + mail_list::handle_mailing_list_selection, }, handle::AppHandle, loading::{terminal_error, TerminalLoadingIndicator}, @@ -55,6 +59,8 @@ enum KwWatchEvent { Closed, } +const KW_OPS_LOG_TICK: Duration = Duration::from_millis(350); + impl AppActor { /// Moves all resources into a new `AppActor`, spawns it on the Tokio /// runtime, and returns an [`AppHandle`] to wait on it. @@ -82,20 +88,29 @@ impl AppActor { let mut kw_status_rx = self.subscribe_kw_status().await; let mut loading = TerminalLoadingIndicator::new(self.terminal_handle.clone()); + let mut log_interval = tokio::time::interval_at( + tokio::time::Instant::now() + KW_OPS_LOG_TICK, + KW_OPS_LOG_TICK, + ); + log_interval.set_missed_tick_behavior(MissedTickBehavior::Skip); + let mut redraw = true; loop { self.app.process_system_updates(&mut loading).await?; - let scene = self - .ui_handle - .build_scene(self.app.present()) - .await - .map_err(|e| eyre!("{e}"))?; - self.terminal_handle - .draw(TerminalFrame::Main(Box::new(scene))) - .await - .map_err(terminal_error)?; + if redraw { + let scene = self + .ui_handle + .build_scene(self.app.present()) + .await + .map_err(|e| eyre!("{e}"))?; + self.terminal_handle + .draw(TerminalFrame::Main(Box::new(scene))) + .await + .map_err(terminal_error)?; + } + let tail_while_running = kw_ops_should_tail(&self.app); tokio::select! { event = self.event_rx.recv() => { match event { @@ -108,6 +123,7 @@ impl AppActor { .update_context(self.app.input_context()) .await .ok(); + redraw = true; } ControlFlow::Break(()) => break, } @@ -122,6 +138,9 @@ impl AppActor { match watch_event { KwWatchEvent::Updated(snapshot) => { self.app.state.kw.status = Some(snapshot); + if self.app.state.kw.ops.is_some() { + refresh_kw_ops_log_tail(&mut self.app); + } } KwWatchEvent::Closed => { tracing::warn!( @@ -130,6 +149,10 @@ impl AppActor { kw_status_rx = None; } } + redraw = true; + } + _ = log_interval.tick(), if tail_while_running => { + redraw = refresh_kw_ops_log_tail(&mut self.app); } } } @@ -174,6 +197,14 @@ async fn kw_status_changed(rx: &mut Option>) - } } +fn kw_ops_should_tail(app: &App) -> bool { + app.state.navigation.current_screen == CurrentScreen::KwOps + && matches!( + app.state.kw.status.as_ref().map(|status| &status.job), + Some(KwJobStatus::Running { .. }) + ) +} + async fn on_input( app: &mut App, input: InputEvent, diff --git a/src/app/flows/kw_ops.rs b/src/app/flows/kw_ops.rs index 134d215d..c361d07b 100644 --- a/src/app/flows/kw_ops.rs +++ b/src/app/flows/kw_ops.rs @@ -6,10 +6,65 @@ use crate::{ screens::{kw_ops::KwOpsState, CurrentScreen}, App, }, + infrastructure::file_system::FileSystemError, input::event::InputEvent, kw::{errors::KwError, messages::StartRequest, status::KwJobStatus}, }; +/// Bytes read from the end of a kw job log. Kernel build logs can be +/// far larger than this; the TUI only needs a bounded tail. +pub(crate) const LOG_TAIL_MAX_BYTES: usize = 64 * 1024; + +/// Refresh the KwOps log panel from the current job's log file. +/// +/// A missing file is normal just after accept (`Waiting for kw output…` +/// is projected from an empty tail). Other read errors become a +/// non-fatal diagnostic in the panel and do not change job status. +/// Returns whether the displayed text changed. +pub(crate) fn refresh_kw_ops_log_tail(app: &mut App) -> bool { + let Some(path) = app + .state + .kw + .status + .as_ref() + .and_then(|status| status.job.log_path()) + .map(std::path::PathBuf::from) + else { + return false; + }; + if app.state.kw.ops.is_none() { + return false; + } + let text = tail_text_from_read( + app.services + .fs + .read_tail_to_string(&path, LOG_TAIL_MAX_BYTES), + ); + let Some(ops) = app.state.kw.ops.as_mut() else { + return false; + }; + assign_log_tail(ops, text) +} + +fn tail_text_from_read(result: Result) -> String { + match result { + Ok(text) => text, + Err(FileSystemError::IoError(error)) if error.kind() == std::io::ErrorKind::NotFound => { + String::new() + } + Err(error) => format!("(could not read log: {error})"), + } +} + +fn assign_log_tail(ops: &mut KwOpsState, text: String) -> bool { + if ops.log_tail == text { + false + } else { + ops.log_tail = text; + true + } +} + pub async fn handle_kw_ops(app: &mut App, input: InputEvent) -> Result<()> { let editing = app.state.kw.ops.as_ref().is_some_and(|ops| ops.editing); if editing { @@ -99,6 +154,7 @@ pub async fn open_kw_ops(app: &mut App) -> Result<()> { readiness, )); app.set_current_screen(CurrentScreen::KwOps); + refresh_kw_ops_log_tail(app); } Err(error) => { app.state.popup = Some(AppPopup::info( @@ -230,3 +286,68 @@ pub fn generate_help_popup() -> AppPopup { .keybind("?", "Show this help screen") .build() } + +#[cfg(test)] +mod tests { + use super::*; + use crate::app::screens::kw_ops::KwOpsState; + use crate::kw::readiness::{ + DeployAloneRefusal, KwBinaryProbe, KwReadiness, KwVersionCheck, TreeReadiness, + }; + + fn sample_ops() -> KwOpsState { + KwOpsState::new( + "title".to_string(), + "mid".to_string(), + "linux".to_string(), + serde_json::from_value(serde_json::json!({ + "path": "/kernel", + "branch": "main" + })) + .unwrap(), + KwReadiness { + kw_binary: KwBinaryProbe { + available: true, + version_line: Some("kw, version 0.10.0".to_string()), + check: KwVersionCheck::Meets, + }, + tree: TreeReadiness::Ready { + arch: Some("x86_64".to_string()), + }, + output_dir: None, + kernel_image: None, + build_record: None, + latest_build: None, + deploy_alone: Err(DeployAloneRefusal::NoBuildRecord), + current_branch: Some("main".to_string()), + }, + ) + } + + #[test] + fn missing_log_is_an_empty_tail_not_a_diagnostic() { + let missing = + FileSystemError::IoError(std::io::Error::new(std::io::ErrorKind::NotFound, "missing")); + assert_eq!("", tail_text_from_read(Err(missing))); + } + + #[test] + fn other_read_errors_become_a_panel_diagnostic() { + let error = FileSystemError::IoError(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "denied", + )); + let text = tail_text_from_read(Err(error)); + assert!(text.contains("could not read log")); + assert!(text.contains("denied")); + } + + #[test] + fn assign_log_tail_reports_whether_text_changed() { + let mut ops = sample_ops(); + assert!(assign_log_tail(&mut ops, "cc1: compiling".to_string())); + assert!(!assign_log_tail(&mut ops, "cc1: compiling".to_string())); + assert!(assign_log_tail(&mut ops, "done".to_string())); + assert_eq!("done", ops.log_tail); + } +} diff --git a/src/app/integration_tests/kw_ops.rs b/src/app/integration_tests/kw_ops.rs index 977fa3c2..070f6126 100644 --- a/src/app/integration_tests/kw_ops.rs +++ b/src/app/integration_tests/kw_ops.rs @@ -65,12 +65,14 @@ mod unix { path::PathBuf, sync::{ atomic::{AtomicU64, Ordering}, - Arc, + Arc, Mutex, }, + time::Duration, }; use crate::{ app::{ + actor::AppActor, flows::{details_actions::handle_patchset_details, kw_ops::handle_kw_ops}, screens::CurrentScreen, App, @@ -78,13 +80,20 @@ mod unix { config::{ConfigSnapshot, ConfigState}, infrastructure::{ env::MockEnvTrait, - file_system::{FileSystemError, MockFileSystemTrait}, + file_system::{FileSystemError, FileSystemTrait, MockFileSystemTrait, OsFileSystem}, process::FakeProcess, shell::{MockShellTrait, ShellOutput}, }, - input::event::InputEvent, + input::{event::InputEvent, handle::InputHandle, messages::InputMessage}, kw::{actor::KwActor, history::MockKwHistoryStore, status::KwJobStatus}, lore::application::cache::BootstrapLoreData, + terminal::{ + actor::TerminalActor, messages::TerminalFrame, session::MockTerminalSessionApi, + }, + ui::{ + actor::UiActor, + scene::{UiBody, UiScene}, + }, }; use super::{assert_info_popup, details_state}; @@ -149,6 +158,7 @@ mod unix { &log_dir, head_branch_shell("feature"), process.clone(), + Box::new(MockFileSystemTrait::new()), ); handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) .await @@ -203,14 +213,93 @@ mod unix { std::fs::remove_dir_all(&log_dir).unwrap(); } + #[tokio::test(flavor = "multi_thread")] + async fn live_tail_updates_without_input_and_pauses_off_screen() { + let log_dir = kw_log_dir("live-tail"); + let process = Arc::new(FakeProcess::new()); + let app = app_with_details_and_kw_process( + &log_dir, + head_branch_shell("feature"), + process.clone(), + Box::new(OsFileSystem), + ); + let kw = app.services.kw.as_ref().unwrap().clone(); + let (scenes, event_tx, handle) = spawn_app_actor(app); + + event_tx.send(InputEvent::OpenKwOps).await.unwrap(); + wait_for_kw_ops(&scenes, |_| true).await; + + event_tx.send(InputEvent::StartKwBuild).await.unwrap(); + wait_for_nav(&scenes, |text| text.contains("kw: building feature")).await; + wait_for_kw_ops(&scenes, |ops| { + ops.log_tail.contains("Waiting for kw output") + }) + .await; + + process.last_child().write_log(b"cc1: compiling foo.c\n"); + wait_for_kw_ops(&scenes, |ops| ops.log_tail.contains("cc1: compiling foo.c")).await; + + event_tx.send(InputEvent::Back).await.unwrap(); + wait_for_details(&scenes).await; + wait_for_nav(&scenes, |text| text.contains("kw: building feature")).await; + + let draws_on_details = scene_count(&scenes); + tokio::time::sleep(Duration::from_millis(800)).await; + process.last_child().write_log(b"ld: linking vmlinux\n"); + tokio::time::sleep(Duration::from_millis(800)).await; + assert_eq!( + draws_on_details, + scene_count(&scenes), + "log ticks must not redraw while away from KwOps" + ); + + event_tx.send(InputEvent::ToggleBookmark).await.unwrap(); + tokio::time::timeout(Duration::from_secs(5), async { + loop { + if scene_count(&scenes) > draws_on_details { + break; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("input must still redraw off KwOps"); + + event_tx.send(InputEvent::OpenKwOps).await.unwrap(); + wait_for_kw_ops(&scenes, |ops| { + ops.log_tail.contains("cc1: compiling foo.c") + && ops.log_tail.contains("ld: linking vmlinux") + }) + .await; + + process.last_child().finish(0); + wait_for_kw_ops(&scenes, |ops| ops.job_status.contains("succeeded")).await; + wait_for_kw_ops(&scenes, |ops| { + ops.log_tail.contains("cc1: compiling foo.c") + && ops.log_tail.contains("ld: linking vmlinux") + }) + .await; + + drop(event_tx); + handle.run_until_done().await.unwrap(); + kw.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + fn app_with_details_and_kw(log_dir: &std::path::Path, kw_shell: MockShellTrait) -> App { - app_with_details_and_kw_process(log_dir, kw_shell, Arc::new(FakeProcess::new())) + app_with_details_and_kw_process( + log_dir, + kw_shell, + Arc::new(FakeProcess::new()), + Box::new(MockFileSystemTrait::new()), + ) } fn app_with_details_and_kw_process( log_dir: &std::path::Path, kw_shell: MockShellTrait, process: Arc, + app_fs: Box, ) -> App { let mut history = MockKwHistoryStore::new(); history @@ -236,7 +325,7 @@ mod unix { bookmarks: vec![], reviewed: Default::default(), }, - Box::new(MockFileSystemTrait::new()), + app_fs, Box::new(MockShellTrait::new()), lore_handle_with_persistence(), dummy_render_handle(), @@ -326,4 +415,102 @@ mod unix { std::fs::create_dir_all(&dir).unwrap(); dir } + + fn spawn_app_actor( + app: App, + ) -> ( + Arc>>, + tokio::sync::mpsc::Sender, + crate::app::handle::AppHandle, + ) { + let scenes = Arc::new(Mutex::new(Vec::new())); + let scenes_for_draw = Arc::clone(&scenes); + let mut session = MockTerminalSessionApi::new(); + session + .expect_draw() + .withf(|frame| matches!(frame, TerminalFrame::Main(_))) + .times(1..) + .returning(move |frame| { + if let TerminalFrame::Main(scene) = frame { + scenes_for_draw.lock().unwrap().push(*scene); + } + Ok(()) + }); + + let terminal_handle = TerminalActor::spawn(Box::new(session)); + let ui_handle = UiActor::spawn(); + let (event_tx, event_rx) = tokio::sync::mpsc::channel::(8); + let (input_tx, _input_rx) = tokio::sync::mpsc::channel::(1); + let input_handle = InputHandle::new(input_tx); + let handle = AppActor::spawn(app, terminal_handle, ui_handle, input_handle, event_rx); + (scenes, event_tx, handle) + } + + async fn wait_for_nav(scenes: &Arc>>, predicate: impl Fn(&str) -> bool) { + tokio::time::timeout(Duration::from_secs(5), async { + loop { + if scenes + .lock() + .unwrap() + .iter() + .any(|scene| predicate(&nav_text(scene))) + { + return; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("expected navigation text did not appear"); + } + + async fn wait_for_kw_ops( + scenes: &Arc>>, + predicate: impl Fn(&crate::ui::scene::KwOpsScene) -> bool, + ) { + tokio::time::timeout(Duration::from_secs(5), async { + loop { + let matches = scenes.lock().unwrap().last().is_some_and( + |scene| matches!(&scene.body, UiBody::KwOps(ops) if predicate(ops)), + ); + if matches { + return; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("expected KwOps scene did not appear"); + } + + async fn wait_for_details(scenes: &Arc>>) { + tokio::time::timeout(Duration::from_secs(5), async { + loop { + if scenes + .lock() + .unwrap() + .last() + .is_some_and(|scene| matches!(scene.body, UiBody::PatchsetDetails(_))) + { + return; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("expected Patchset Details scene did not appear"); + } + + fn scene_count(scenes: &Arc>>) -> usize { + scenes.lock().unwrap().len() + } + + fn nav_text(scene: &UiScene) -> String { + scene + .navigation + .mode_spans + .iter() + .map(|span| span.content.to_string()) + .collect() + } } diff --git a/src/app/screens/kw_ops.rs b/src/app/screens/kw_ops.rs index 4a720f45..2e4239e2 100644 --- a/src/app/screens/kw_ops.rs +++ b/src/app/screens/kw_ops.rs @@ -24,7 +24,8 @@ pub struct KwOpsState { /// True when readiness could not name HEAD; Start stays disabled until /// the user types a branch (we never guess from `KernelTree.branch`). pub head_unreadable: bool, - /// Presentation-only; the next commit fills this from a bounded tail. + /// Bounded tail of the job log, refreshed by AppActor while KwOps is + /// visible and a job is running. pub log_tail: String, pub cancel_requested: bool, } diff --git a/src/infrastructure/file_system/mod.rs b/src/infrastructure/file_system/mod.rs index 41d2c66e..288f333f 100644 --- a/src/infrastructure/file_system/mod.rs +++ b/src/infrastructure/file_system/mod.rs @@ -9,7 +9,7 @@ pub use r#trait::MockFileSystemTrait; use std::{ fs::{self, File}, - io::{self, BufReader}, + io::{self, BufReader, Read, Seek, SeekFrom}, path::{Path, PathBuf}, }; @@ -68,4 +68,27 @@ impl FileSystemTrait for OsFileSystem { fn metadata(&self, path: &Path) -> Result { Ok(fs::metadata(path)?) } + + fn read_tail_to_string( + &self, + path: &Path, + max_bytes: usize, + ) -> Result { + let mut file = File::open(path)?; + let len = file.metadata()?.len(); + let window = max_bytes as u64; + let start = len.saturating_sub(window); + file.seek(SeekFrom::Start(start))?; + let mut buf = vec![0u8; (len - start) as usize]; + file.read_exact(&mut buf)?; + let bytes = if start > 0 { + match buf.iter().position(|&b| b == b'\n') { + Some(index) => &buf[index + 1..], + None => buf.as_slice(), + } + } else { + buf.as_slice() + }; + Ok(String::from_utf8_lossy(bytes).into_owned()) + } } diff --git a/src/infrastructure/file_system/tests.rs b/src/infrastructure/file_system/tests.rs index e1ae3dc4..6c3d1b73 100644 --- a/src/infrastructure/file_system/tests.rs +++ b/src/infrastructure/file_system/tests.rs @@ -192,3 +192,57 @@ fn metadata_returns_error_for_missing_path() { let result = fs.metadata(Path::new("/nonexistent/path")); assert!(result.is_err()); } + +#[test] +fn read_tail_returns_the_whole_file_when_it_fits() { + let dir = TempDir::new("read_tail_fits"); + let file_path = dir.path().join("log.txt"); + std::fs::write(&file_path, "line1\nline2\n").unwrap(); + + let fs = OsFileSystem; + assert_eq!( + "line1\nline2\n", + fs.read_tail_to_string(&file_path, 64).unwrap() + ); +} + +#[test] +fn read_tail_drops_a_partial_first_line_when_seeking() { + let dir = TempDir::new("read_tail_partial"); + let file_path = dir.path().join("log.txt"); + // 14 bytes; an 8-byte window starts mid-"BBBB". + std::fs::write(&file_path, "AAAA\nBBBB\nCCCC").unwrap(); + + let fs = OsFileSystem; + assert_eq!("CCCC", fs.read_tail_to_string(&file_path, 8).unwrap()); +} + +#[test] +fn read_tail_keeps_a_window_with_no_newline() { + let dir = TempDir::new("read_tail_nonewline"); + let file_path = dir.path().join("log.txt"); + std::fs::write(&file_path, "abcdefghijklmnopqrst").unwrap(); + + let fs = OsFileSystem; + assert_eq!("mnopqrst", fs.read_tail_to_string(&file_path, 8).unwrap()); +} + +#[test] +fn read_tail_decodes_invalid_utf8_lossily() { + let dir = TempDir::new("read_tail_utf8"); + let file_path = dir.path().join("log.txt"); + std::fs::write(&file_path, b"ok\n\xff\xfeworld").unwrap(); + + let fs = OsFileSystem; + let text = fs.read_tail_to_string(&file_path, 64).unwrap(); + assert!(text.starts_with("ok\n")); + assert!(text.contains('\u{FFFD}')); + assert!(text.ends_with("world")); +} + +#[test] +fn read_tail_errors_for_a_missing_file() { + let fs = OsFileSystem; + let result = fs.read_tail_to_string(Path::new("/nonexistent/path/log.txt"), 64); + assert!(result.is_err()); +} diff --git a/src/infrastructure/file_system/trait.rs b/src/infrastructure/file_system/trait.rs index 5a97c372..03ab2c37 100644 --- a/src/infrastructure/file_system/trait.rs +++ b/src/infrastructure/file_system/trait.rs @@ -29,4 +29,11 @@ pub trait FileSystemTrait: Send + Sync { fn create_writer(&self, path: &Path) -> Result, FileSystemError>; fn open_bufreader(&self, path: &Path) -> Result, FileSystemError>; fn metadata(&self, path: &Path) -> Result; + /// Read at most `max_bytes` from the end of `path`. + /// + /// When the read does not start at byte zero, an initial partial line + /// is discarded so the returned text begins on a line boundary. + /// Invalid UTF-8 is decoded lossily. + fn read_tail_to_string(&self, path: &Path, max_bytes: usize) + -> Result; } diff --git a/src/kw/status.rs b/src/kw/status.rs index b2c6f1be..324ac97d 100644 --- a/src/kw/status.rs +++ b/src/kw/status.rs @@ -87,6 +87,19 @@ impl KwStatusSnapshot { } } +impl KwJobStatus { + /// Log file for the current or last job, if the actor has opened one. + pub fn log_path(&self) -> Option<&std::path::Path> { + match self { + Self::Idle => None, + Self::Running { log_path, .. } + | Self::Succeeded { log_path, .. } + | Self::Failed { log_path, .. } + | Self::Cancelled { log_path, .. } => Some(log_path), + } + } +} + #[cfg(test)] mod tests { use std::path::PathBuf; @@ -114,6 +127,15 @@ mod tests { ); } + #[test] + fn log_path_is_present_for_jobs_with_a_log() { + assert_eq!( + Some(PathBuf::from("/tmp/build.log").as_path()), + running("patchset-x").job.log_path() + ); + assert_eq!(None, KwJobStatus::Idle.log_path()); + } + #[test] fn terminal_and_idle_status_have_no_global_indicator() { assert_eq!(None, KwStatusSnapshot::idle().running_indicator()); From 67131baf9f1653d4a2fb1306e03a55f51b104afd Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Fri, 18 Sep 2026 18:32:26 -0300 Subject: [PATCH 5/6] fix(kw): pin the KwOps log to newest output and unstick fallback This commit keeps the visible log on the newest output so a failed build's error is on screen, and moves log-tail and readiness work off the actor task so they cannot stall the TUI. Start no longer races a second keypress or stays stuck on "starting" when the status watch is missing: accept applies the current status immediately, and keyboard-only mode polls while a job is in flight without rereading the log off-screen. This commit completes the kw integration's step 6. Signed-off-by: lorenzoberts --- Cargo.toml | 2 +- src/app/actor.rs | 59 +++++-- src/app/flows/kw_ops.rs | 119 ++++++++++--- src/app/input.rs | 3 +- .../integration_tests/helpers/app_harness.rs | 2 +- src/app/integration_tests/kw_ops.rs | 156 +++++++++++++++++- src/app/integration_tests/kw_status.rs | 19 ++- src/app/integration_tests/patchset_actions.rs | 2 +- src/app/mod.rs | 4 +- src/app/popup.rs | 2 +- src/app/screens/kw_ops.rs | 81 ++++++++- src/app/updates.rs | 15 +- src/app/view_model.rs | 118 ++++++++++++- src/input/context.rs | 9 + src/input/mapper.rs | 19 ++- src/kw/actor.rs | 42 +++-- src/main.rs | 2 +- src/ui/screens/kw_ops.rs | 66 +++++++- 18 files changed, 632 insertions(+), 88 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 008a7053..16d15968 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,7 @@ mockall = "0.13.1" derive-getters = { version = "0.5.0", features = ["auto_copy_getters"] } lazy_static = "1.5.0" patch-hub-proc-macros = "0.1.0" -ratatui = "0.29.0" +ratatui = { version = "0.29.0", features = ["unstable-rendered-line-info"] } regex = "1.11.1" serde = { version = "1.0.219", features = ["derive"] } serde-xml-rs = "0.8.1" diff --git a/src/app/actor.rs b/src/app/actor.rs index 7280a059..33994cc7 100644 --- a/src/app/actor.rs +++ b/src/app/actor.rs @@ -20,7 +20,10 @@ use crate::{ bookmarked::handle_bookmarked_patchsets, details_actions::handle_patchset_details, edit_config::handle_edit_config, - kw_ops::{handle_kw_ops, refresh_kw_ops_log_tail}, + kw_ops::{ + apply_kw_snapshot, fallback_kw_status, handle_kw_ops, poll_kw_status, + refresh_kw_ops_log_tail, + }, latest::handle_latest_patchsets, mail_list::handle_mailing_list_selection, }, @@ -96,7 +99,10 @@ impl AppActor { let mut redraw = true; loop { - self.app.process_system_updates(&mut loading).await?; + let system_changed = self.app.process_system_updates(&mut loading).await?; + if system_changed { + redraw = true; + } if redraw { let scene = self @@ -111,6 +117,7 @@ impl AppActor { } let tail_while_running = kw_ops_should_tail(&self.app); + let poll_status = kw_status_rx.is_none() && should_poll_kw_status(&self.app); tokio::select! { event = self.event_rx.recv() => { match event { @@ -137,9 +144,10 @@ impl AppActor { watch_event = kw_status_changed(&mut kw_status_rx) => { match watch_event { KwWatchEvent::Updated(snapshot) => { - self.app.state.kw.status = Some(snapshot); - if self.app.state.kw.ops.is_some() { - refresh_kw_ops_log_tail(&mut self.app); + apply_kw_snapshot(&mut self.app, snapshot); + if self.app.state.navigation.current_screen == CurrentScreen::KwOps + { + refresh_kw_ops_log_tail(&mut self.app).await; } } KwWatchEvent::Closed => { @@ -147,12 +155,20 @@ impl AppActor { "kw status watch closed; falling back to keyboard-only redraws" ); kw_status_rx = None; + fallback_kw_status(&mut self.app).await; } } redraw = true; } - _ = log_interval.tick(), if tail_while_running => { - redraw = refresh_kw_ops_log_tail(&mut self.app); + _ = log_interval.tick(), if tail_while_running || poll_status => { + let mut changed = false; + if poll_status { + changed |= poll_kw_status(&mut self.app).await; + } + if tail_while_running { + changed |= refresh_kw_ops_log_tail(&mut self.app).await; + } + redraw = changed; } } } @@ -163,14 +179,14 @@ impl AppActor { /// Subscribes once at startup. A missing handle or a failed /// subscription leaves the app on keyboard-only redraws rather than - /// failing to start. + /// failing to start. A failed subscribe still seeds the projection + /// from GetStatus so a job already running is visible and the poll + /// arm can engage. async fn subscribe_kw_status(&mut self) -> Option> { - let Some(kw) = self.app.services.kw.as_ref() else { - return None; - }; + let kw = self.app.services.kw.clone()?; match kw.watch_status().await { Ok(mut rx) => { - self.app.state.kw.status = Some(rx.borrow_and_update().clone()); + apply_kw_snapshot(&mut self.app, rx.borrow_and_update().clone()); Some(rx) } Err(error) => { @@ -178,6 +194,7 @@ impl AppActor { %error, "failed to subscribe to kw status; keyboard-only redraws" ); + fallback_kw_status(&mut self.app).await; None } } @@ -205,6 +222,20 @@ fn kw_ops_should_tail(app: &App) -> bool { ) } +/// When the status watch is missing, poll GetStatus while a start is +/// in flight or a job is running so `start_requested` cannot latch and +/// the nav indicator can leave "building". +fn should_poll_kw_status(app: &App) -> bool { + app.services.kw.is_some() + && (kw_job_is_running(app) + || app + .state + .kw + .ops + .as_ref() + .is_some_and(|ops| ops.start_requested)) +} + async fn on_input( app: &mut App, input: InputEvent, @@ -365,7 +396,7 @@ mod tests { lore_api: LoreApiHandle::new(lore_tx), render: RenderHandle::new(render_tx), shell: Box::new(MockShellTrait::new()), - fs: Box::new(MockFileSystemTrait::new()), + fs: Arc::new(MockFileSystemTrait::new()), config: dummy_config_handle(), kw_history: Arc::new(MockKwHistoryStore::new()), kw: None, @@ -466,7 +497,7 @@ mod tests { ConfigState::default().to_snapshot(), dummy_config_handle(), bootstrap, - Box::new(MockFileSystemTrait::new()), + Arc::new(MockFileSystemTrait::new()), Box::new(MockShellTrait::new()), lore_api.clone(), render.clone(), diff --git a/src/app/flows/kw_ops.rs b/src/app/flows/kw_ops.rs index c361d07b..8897ccfd 100644 --- a/src/app/flows/kw_ops.rs +++ b/src/app/flows/kw_ops.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use color_eyre::Result; use crate::{ @@ -8,7 +10,11 @@ use crate::{ }, infrastructure::file_system::FileSystemError, input::event::InputEvent, - kw::{errors::KwError, messages::StartRequest, status::KwJobStatus}, + kw::{ + errors::KwError, + messages::StartRequest, + status::{KwJobStatus, KwStatusSnapshot}, + }, }; /// Bytes read from the end of a kw job log. Kernel build logs can be @@ -21,7 +27,13 @@ pub(crate) const LOG_TAIL_MAX_BYTES: usize = 64 * 1024; /// is projected from an empty tail). Other read errors become a /// non-fatal diagnostic in the panel and do not change job status. /// Returns whether the displayed text changed. -pub(crate) fn refresh_kw_ops_log_tail(app: &mut App) -> bool { +/// +/// The read runs on the blocking pool: the log is a real file, and +/// AppActor's loop must not stall on disk I/O. +pub(crate) async fn refresh_kw_ops_log_tail(app: &mut App) -> bool { + if app.state.kw.ops.is_none() { + return false; + } let Some(path) = app .state .kw @@ -32,14 +44,14 @@ pub(crate) fn refresh_kw_ops_log_tail(app: &mut App) -> bool { else { return false; }; - if app.state.kw.ops.is_none() { - return false; - } - let text = tail_text_from_read( - app.services - .fs - .read_tail_to_string(&path, LOG_TAIL_MAX_BYTES), - ); + let fs = Arc::clone(&app.services.fs); + let result = + tokio::task::spawn_blocking(move || fs.read_tail_to_string(&path, LOG_TAIL_MAX_BYTES)) + .await; + let text = match result { + Ok(read) => tail_text_from_read(read), + Err(error) => format!("(could not read log: {error})"), + }; let Some(ops) = app.state.kw.ops.as_mut() else { return false; }; @@ -65,6 +77,47 @@ fn assign_log_tail(ops: &mut KwOpsState, text: String) -> bool { } } +/// Projects a kw snapshot into App state and releases the optimistic +/// start lock once the actor has left Idle. +pub(crate) fn apply_kw_snapshot(app: &mut App, snapshot: KwStatusSnapshot) { + if let Some(ops) = app.state.kw.ops.as_mut() { + if !matches!(snapshot.job, KwJobStatus::Idle) { + ops.start_requested = false; + } + } + app.state.kw.status = Some(snapshot); +} + +/// Keyboard-only fallback: pull status when the watch is unavailable. +pub(crate) async fn poll_kw_status(app: &mut App) -> bool { + let Some(kw) = app.services.kw.clone() else { + return false; + }; + let Ok(snapshot) = kw.get_status().await else { + return false; + }; + let changed = app.state.kw.status.as_ref() != Some(&snapshot); + apply_kw_snapshot(app, snapshot); + changed +} + +/// Seed the projection from GetStatus after the watch is gone. +/// +/// A successful query lets the poll arm engage if a job is already +/// running (the subscribe-failed-with-live-job corner). A failed query +/// clears status so a dead actor cannot leave a stale "building" +/// indicator. +pub(crate) async fn fallback_kw_status(app: &mut App) { + let Some(kw) = app.services.kw.clone() else { + app.state.kw.status = None; + return; + }; + match kw.get_status().await { + Ok(snapshot) => apply_kw_snapshot(app, snapshot), + Err(_) => app.state.kw.status = None, + } +} + pub async fn handle_kw_ops(app: &mut App, input: InputEvent) -> Result<()> { let editing = app.state.kw.ops.as_ref().is_some_and(|ops| ops.editing); if editing { @@ -99,7 +152,7 @@ pub async fn handle_kw_ops(app: &mut App, input: InputEvent) -> Result<()> { } } InputEvent::EditKwOpsField => { - if !job_is_running(app) { + if !job_is_busy(app) { if let Some(ops) = app.state.kw.ops.as_mut() { ops.begin_edit(); } @@ -146,15 +199,24 @@ pub async fn open_kw_ops(app: &mut App) -> Result<()> { match kw.get_readiness(&kernel_tree_id, &tree).await { Ok(readiness) => { - app.state.kw.ops = Some(KwOpsState::new( - patchset_title, - message_id, - kernel_tree_id, - tree, - readiness, - )); + let reuse = app.state.kw.ops.as_ref().is_some_and(|ops| { + ops.message_id == message_id && ops.kernel_tree_id == kernel_tree_id + }); + if reuse { + if let Some(ops) = app.state.kw.ops.as_mut() { + ops.reenter(patchset_title, tree, readiness); + } + } else { + app.state.kw.ops = Some(KwOpsState::new( + patchset_title, + message_id, + kernel_tree_id, + tree, + readiness, + )); + } app.set_current_screen(CurrentScreen::KwOps); - refresh_kw_ops_log_tail(app); + refresh_kw_ops_log_tail(app).await; } Err(error) => { app.state.popup = Some(AppPopup::info( @@ -167,7 +229,7 @@ pub async fn open_kw_ops(app: &mut App) -> Result<()> { } async fn start_build(app: &mut App) -> Result<()> { - if job_is_running(app) { + if job_is_busy(app) { return Ok(()); } let Some(ops) = app.state.kw.ops.as_ref() else { @@ -200,6 +262,13 @@ async fn start_build(app: &mut App) -> Result<()> { Ok(()) => { if let Some(ops) = app.state.kw.ops.as_mut() { ops.cancel_requested = false; + ops.start_requested = true; + } + // Apply the actor's snapshot immediately so keyboard-only + // mode (no watch) cannot latch on start_requested, and so a + // second Start in the same loop turn sees the job as busy. + if let Ok(snapshot) = kw.get_status().await { + apply_kw_snapshot(app, snapshot); } } Err(error) => { @@ -271,6 +340,16 @@ fn job_is_running(app: &App) -> bool { ) } +fn job_is_busy(app: &App) -> bool { + job_is_running(app) + || app + .state + .kw + .ops + .as_ref() + .is_some_and(|ops| ops.start_requested) +} + pub fn generate_help_popup() -> AppPopup { AppPopup::help() .title("Kw operations") diff --git a/src/app/input.rs b/src/app/input.rs index e212d75f..3bd425dc 100644 --- a/src/app/input.rs +++ b/src/app/input.rs @@ -1,4 +1,4 @@ -use crate::{app::App, input::context::InputContext}; +use crate::{app::popup::AppPopup, app::App, input::context::InputContext}; impl App { /// Projects App state into the context needed by the input mapper. @@ -6,6 +6,7 @@ impl App { InputContext { current_screen: self.state.navigation.current_screen.clone(), popup_open: self.state.popup.is_some(), + confirm_popup_open: matches!(self.state.popup, Some(AppPopup::Confirm { .. })), edit_config_editing: self .state .config_state diff --git a/src/app/integration_tests/helpers/app_harness.rs b/src/app/integration_tests/helpers/app_harness.rs index 6969eac1..dcf38dd2 100644 --- a/src/app/integration_tests/helpers/app_harness.rs +++ b/src/app/integration_tests/helpers/app_harness.rs @@ -51,7 +51,7 @@ pub(crate) fn app_with_bootstrap_and_handles( ConfigState::default().to_snapshot(), dummy_config_handle(), bootstrap, - Box::new(MockFileSystemTrait::new()), + Arc::new(MockFileSystemTrait::new()), Box::new(MockShellTrait::new()), lore_api, render, diff --git a/src/app/integration_tests/kw_ops.rs b/src/app/integration_tests/kw_ops.rs index 070f6126..28d701d2 100644 --- a/src/app/integration_tests/kw_ops.rs +++ b/src/app/integration_tests/kw_ops.rs @@ -73,7 +73,10 @@ mod unix { use crate::{ app::{ actor::AppActor, - flows::{details_actions::handle_patchset_details, kw_ops::handle_kw_ops}, + flows::{ + details_actions::handle_patchset_details, + kw_ops::{fallback_kw_status, handle_kw_ops, refresh_kw_ops_log_tail}, + }, screens::CurrentScreen, App, }, @@ -85,7 +88,11 @@ mod unix { shell::{MockShellTrait, ShellOutput}, }, input::{event::InputEvent, handle::InputHandle, messages::InputMessage}, - kw::{actor::KwActor, history::MockKwHistoryStore, status::KwJobStatus}, + kw::{ + actor::KwActor, + history::MockKwHistoryStore, + status::{KwJobKind, KwJobStatus, KwPhase, KwStatusSnapshot}, + }, lore::application::cache::BootstrapLoreData, terminal::{ actor::TerminalActor, messages::TerminalFrame, session::MockTerminalSessionApi, @@ -158,7 +165,7 @@ mod unix { &log_dir, head_branch_shell("feature"), process.clone(), - Box::new(MockFileSystemTrait::new()), + Arc::new(MockFileSystemTrait::new()), ); handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) .await @@ -195,6 +202,135 @@ mod unix { std::fs::remove_dir_all(&log_dir).unwrap(); } + #[tokio::test] + async fn second_start_while_busy_is_absorbed() { + let log_dir = kw_log_dir("double-start"); + let process = Arc::new(FakeProcess::new()); + let mut app = app_with_details_and_kw_process( + &log_dir, + head_branch_shell("feature"), + process.clone(), + Arc::new(MockFileSystemTrait::new()), + ); + handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) + .await + .unwrap(); + + handle_kw_ops(&mut app, InputEvent::StartKwBuild) + .await + .unwrap(); + assert!(app.state.popup.is_none()); + assert!(matches!( + app.state.kw.status.as_ref().map(|s| &s.job), + Some(KwJobStatus::Running { .. }) + )); + assert!(!app + .state + .kw + .ops + .as_ref() + .is_some_and(|ops| ops.start_requested)); + + handle_kw_ops(&mut app, InputEvent::StartKwBuild) + .await + .unwrap(); + assert!(app.state.popup.is_none()); + assert_eq!(1, process.spawned().len()); + + process.last_child().finish(0); + shutdown_kw(&app).await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn fallback_seeds_a_running_job_when_projection_is_empty() { + let log_dir = kw_log_dir("fallback-seed"); + let process = Arc::new(FakeProcess::new()); + let mut app = app_with_details_and_kw_process( + &log_dir, + head_branch_shell("feature"), + process.clone(), + Arc::new(MockFileSystemTrait::new()), + ); + handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) + .await + .unwrap(); + handle_kw_ops(&mut app, InputEvent::StartKwBuild) + .await + .unwrap(); + app.state.kw.status = None; + + fallback_kw_status(&mut app).await; + assert!(matches!( + app.state.kw.status.as_ref().map(|s| &s.job), + Some(KwJobStatus::Running { .. }) + )); + + process.last_child().finish(0); + shutdown_kw(&app).await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn refresh_log_tail_goes_through_injected_fs() { + let log_dir = kw_log_dir("injected-fs"); + let mut fs = MockFileSystemTrait::new(); + fs.expect_read_tail_to_string() + .times(1) + .returning(|_, _| Ok("cc1: compiling from mock\n".to_string())); + let mut app = app_with_details_and_kw_process( + &log_dir, + head_branch_shell("feature"), + Arc::new(FakeProcess::new()), + Arc::new(fs), + ); + handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) + .await + .unwrap(); + app.state.kw.status = Some(KwStatusSnapshot { + job: KwJobStatus::Running { + kind: KwJobKind::Build, + phase: KwPhase::Building, + kernel_tree_id: "linux".to_string(), + branch: "feature".to_string(), + log_path: log_dir.join("build.log"), + }, + restore_branch: None, + }); + + refresh_kw_ops_log_tail(&mut app).await; + assert!(app + .state + .kw + .ops + .as_ref() + .is_some_and(|ops| ops.log_tail.contains("from mock"))); + + shutdown_kw(&app).await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + + #[tokio::test] + async fn reenter_keeps_typed_extras() { + let log_dir = kw_log_dir("reenter-extras"); + let mut app = app_with_details_and_kw(&log_dir, head_branch_shell("feature")); + handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) + .await + .unwrap(); + app.state.kw.ops.as_mut().unwrap().extra_args = "--verbose".to_string(); + + handle_kw_ops(&mut app, InputEvent::Back).await.unwrap(); + handle_patchset_details(&mut app, InputEvent::OpenKwOps, &dummy_terminal_handle()) + .await + .unwrap(); + + assert_eq!("--verbose", app.state.kw.ops.as_ref().unwrap().extra_args); + assert_eq!(CurrentScreen::KwOps, app.state.navigation.current_screen); + + shutdown_kw(&app).await; + std::fs::remove_dir_all(&log_dir).unwrap(); + } + #[tokio::test] async fn back_returns_to_details() { let log_dir = kw_log_dir("back"); @@ -221,7 +357,7 @@ mod unix { &log_dir, head_branch_shell("feature"), process.clone(), - Box::new(OsFileSystem), + Arc::new(OsFileSystem), ); let kw = app.services.kw.as_ref().unwrap().clone(); let (scenes, event_tx, handle) = spawn_app_actor(app); @@ -280,6 +416,14 @@ mod unix { }) .await; + let draws_after_success = scene_count(&scenes); + tokio::time::sleep(Duration::from_millis(800)).await; + assert_eq!( + draws_after_success, + scene_count(&scenes), + "log ticks must stop after the job finishes" + ); + drop(event_tx); handle.run_until_done().await.unwrap(); kw.shutdown().await; @@ -291,7 +435,7 @@ mod unix { log_dir, kw_shell, Arc::new(FakeProcess::new()), - Box::new(MockFileSystemTrait::new()), + Arc::new(MockFileSystemTrait::new()), ) } @@ -299,7 +443,7 @@ mod unix { log_dir: &std::path::Path, kw_shell: MockShellTrait, process: Arc, - app_fs: Box, + app_fs: Arc, ) -> App { let mut history = MockKwHistoryStore::new(); history diff --git a/src/app/integration_tests/kw_status.rs b/src/app/integration_tests/kw_status.rs index 094cb7ff..1616fde3 100644 --- a/src/app/integration_tests/kw_status.rs +++ b/src/app/integration_tests/kw_status.rs @@ -113,6 +113,23 @@ async fn closed_watch_does_not_busy_loop_and_input_still_works() { std::fs::remove_dir_all(&log_dir).unwrap(); } +#[tokio::test(flavor = "multi_thread")] +async fn quit_with_no_job_exits_without_confirm() { + let log_dir = kw_log_dir("quit-idle"); + let (app, kw, _process) = app_with_kw(&log_dir); + let (scenes, event_tx, handle) = spawn_app_actor(app); + + wait_for_nav(&scenes, |_| true).await; + event_tx.send(InputEvent::Quit).await.unwrap(); + tokio::time::timeout(Duration::from_secs(5), handle.run_until_done()) + .await + .expect("quit with no running job must exit without a confirm popup") + .unwrap(); + + kw.shutdown().await; + std::fs::remove_dir_all(&log_dir).unwrap(); +} + #[tokio::test(flavor = "multi_thread")] async fn quit_while_job_running_opens_confirm_and_wait_keeps_app_alive() { let log_dir = kw_log_dir("quit-wait"); @@ -265,7 +282,7 @@ fn app_with_kw( bookmarks: vec![], reviewed: Default::default(), }, - Box::new(MockFileSystemTrait::new()), + Arc::new(MockFileSystemTrait::new()), Box::new(MockShellTrait::new()), lore_handle_with_persistence(), dummy_render_handle(), diff --git a/src/app/integration_tests/patchset_actions.rs b/src/app/integration_tests/patchset_actions.rs index 5105f996..caeeeed2 100644 --- a/src/app/integration_tests/patchset_actions.rs +++ b/src/app/integration_tests/patchset_actions.rs @@ -515,7 +515,7 @@ fn app_with_details_and_kw( bookmarks: vec![], reviewed: Default::default(), }, - Box::new(fs), + Arc::new(fs), Box::new(shell), lore_api, dummy_render_handle(), diff --git a/src/app/mod.rs b/src/app/mod.rs index 7d466bcc..0aa9eee7 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -78,7 +78,7 @@ pub struct AppServices { pub lore_api: LoreApiHandle, pub render: RenderHandle, pub shell: Box, - pub fs: Box, + pub fs: Arc, pub config: ConfigHandle, /// Direct access to the store, for the non-unix fallback below. pub kw_history: Arc, @@ -113,7 +113,7 @@ impl App { config: ConfigSnapshot, config_handle: ConfigHandle, bootstrap: BootstrapLoreData, - fs: Box, + fs: Arc, shell: Box, lore_api: LoreApiHandle, render: RenderHandle, diff --git a/src/app/popup.rs b/src/app/popup.rs index abf31e56..853c348e 100644 --- a/src/app/popup.rs +++ b/src/app/popup.rs @@ -138,7 +138,7 @@ impl AppPopup { ("Wait".to_string(), ConfirmAction::Wait), ], selected: 1, - dimensions: (40, 60), + dimensions: (50, 30), } } diff --git a/src/app/screens/kw_ops.rs b/src/app/screens/kw_ops.rs index 2e4239e2..0018a8b0 100644 --- a/src/app/screens/kw_ops.rs +++ b/src/app/screens/kw_ops.rs @@ -28,6 +28,9 @@ pub struct KwOpsState { /// visible and a job is running. pub log_tail: String, pub cancel_requested: bool, + /// Optimistic lock so a second Start before the watch snapshot + /// arrives is ignored instead of refused with an error popup. + pub start_requested: bool, } impl KwOpsState { @@ -54,14 +57,38 @@ impl KwOpsState { head_unreadable, log_tail: String::new(), cancel_requested: false, + start_requested: false, } } + /// Re-open KwOps for the same patchset/tree without dropping extras + /// or an in-flight cancel/start indication. + /// + /// Branch and `head_unreadable` stay as the user last edited them. + /// An external HEAD change while away is not applied, so a stale + /// detached-HEAD flag can survive a tree that has since become + /// readable. + pub fn reenter(&mut self, patchset_title: String, tree: KernelTree, readiness: KwReadiness) { + self.patchset_title = patchset_title; + self.tree = tree; + self.readiness = readiness; + self.editing = false; + self.edit_buffer.clear(); + } + pub fn extra_arg_tokens(&self) -> Vec { - self.extra_args - .split_whitespace() - .map(str::to_string) - .collect() + split_extra_args(&self.extra_args) + } + + /// Extra args shown in the command preview, including in-progress + /// edits so the Command line tracks the Extra args field. + pub fn extra_arg_tokens_for_preview(&self) -> Vec { + let raw = if self.editing && self.focus == KwOpsFocus::ExtraArgs { + self.edit_buffer.as_str() + } else { + self.extra_args.as_str() + }; + split_extra_args(raw) } pub fn highlight_prev(&mut self) { @@ -103,6 +130,10 @@ impl KwOpsState { } } +fn split_extra_args(raw: &str) -> Vec { + raw.split_whitespace().map(str::to_string).collect() +} + #[cfg(test)] mod tests { use super::*; @@ -190,4 +221,46 @@ mod tests { assert_eq!("main", ops.branch); assert!(!ops.editing); } + + #[test] + fn reenter_keeps_extras_and_cancel_requested() { + let mut ops = KwOpsState::new( + "title".to_string(), + "mid".to_string(), + "linux".to_string(), + sample_tree(), + readiness(Some("main")), + ); + ops.extra_args = "--verbose".to_string(); + ops.cancel_requested = true; + ops.reenter( + "new title".to_string(), + sample_tree(), + readiness(Some("feature")), + ); + assert_eq!("--verbose", ops.extra_args); + assert!(ops.cancel_requested); + assert_eq!("new title", ops.patchset_title); + assert_eq!("main", ops.branch); + } + + #[test] + fn extra_arg_preview_tokens_follow_the_edit_buffer() { + let mut ops = KwOpsState::new( + "title".to_string(), + "mid".to_string(), + "linux".to_string(), + sample_tree(), + readiness(Some("main")), + ); + ops.extra_args = "--verbose".to_string(); + ops.highlight_next(); + ops.begin_edit(); + ops.append_edit(' '); + ops.append_edit('-'); + ops.append_edit('j'); + ops.append_edit('8'); + assert_eq!(vec!["--verbose", "-j8"], ops.extra_arg_tokens_for_preview()); + assert_eq!(vec!["--verbose"], ops.extra_arg_tokens()); + } } diff --git a/src/app/updates.rs b/src/app/updates.rs index 826ee224..9b0687fa 100644 --- a/src/app/updates.rs +++ b/src/app/updates.rs @@ -4,10 +4,13 @@ use crate::app::{loading::LoadingIndicator, screens::CurrentScreen, App}; impl App { /// Processes app-driven updates that are not direct user input. + /// + /// Returns whether any screen state changed, so the render loop can + /// draw even when no input/watch/tick arm requested a redraw. pub async fn process_system_updates( &mut self, loading: &mut (dyn LoadingIndicator + Send), - ) -> Result<()> { + ) -> Result { match self.state.navigation.current_screen { CurrentScreen::MailingListSelection => { if self @@ -21,7 +24,9 @@ impl App { let result = self.refresh_mailing_lists().await; loading.stop()?; result?; + return Ok(true); } + Ok(false) } CurrentScreen::LatestPatchsets => { let patchsets_state = self.state.lore.latest_patchsets.as_ref().expect( @@ -36,7 +41,9 @@ impl App { result?; self.state.lore.mailing_list_selection.clear_target_list(); + return Ok(true); } + Ok(false) } CurrentScreen::BookmarkedPatchsets => { if self @@ -47,11 +54,11 @@ impl App { .is_empty() { self.set_current_screen(CurrentScreen::MailingListSelection); + return Ok(true); } + Ok(false) } - _ => {} + _ => Ok(false), } - - Ok(()) } } diff --git a/src/app/view_model.rs b/src/app/view_model.rs index 31e90bb2..29abdf2d 100644 --- a/src/app/view_model.rs +++ b/src/app/view_model.rs @@ -450,6 +450,7 @@ fn project_kw_ops(state: &AppState) -> KwOpsViewModel { state.kw.status.as_ref().map(|status| &status.job), Some(KwJobStatus::Running { .. }) ); + let start_requested = ops.start_requested; let restore_branch = state .kw .status @@ -465,7 +466,7 @@ fn project_kw_ops(state: &AppState) -> KwOpsViewModel { } else { ops.extra_args.clone() }; - let start_label = if running { + let start_label = if running || start_requested { "unavailable (a job is already running)".to_string() } else if ops.branch.trim().is_empty() { "unavailable (set a branch first)".to_string() @@ -512,18 +513,24 @@ fn project_kw_ops(state: &AppState) -> KwOpsViewModel { .readiness .output_dir .as_ref() - .map(|path| path.display().to_string()) - .unwrap_or_else(|| "(none)".to_string()), - job_status: format_job_status( - state.kw.status.as_ref().map(|status| &status.job), - ops.cancel_requested, + .map_or_else(|| "(none)".to_string(), |path| path.display().to_string()), + 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, + ) + }, + command: format!( + "kw {}", + argv::build_argv(&ops.extra_arg_tokens_for_preview()).join(" ") ), - command: format!("kw {}", argv::build_argv(&ops.extra_arg_tokens()).join(" ")), start_label, cancel_label, restore_label, deploy_placeholder: "not available yet".to_string(), - branch_guidance: if ops.branch.trim().is_empty() { + branch_guidance: if ops.head_unreadable && ops.branch.trim().is_empty() { Some( "HEAD is detached or unverifiable; type a branch before starting a build." .to_string(), @@ -768,6 +775,101 @@ mod tests { assert_eq!("not available yet", vm.deploy_placeholder); } + fn sample_kw_ops(branch: Option<&str>) -> crate::app::screens::kw_ops::KwOpsState { + crate::app::screens::kw_ops::KwOpsState::new( + "[PATCH] test".to_string(), + "http://lore.example/123".to_string(), + "linux".to_string(), + serde_json::from_value(serde_json::json!({ + "path": "/kernel", + "branch": "main" + })) + .unwrap(), + crate::kw::readiness::KwReadiness { + kw_binary: crate::kw::readiness::KwBinaryProbe { + available: true, + version_line: Some("kw, version 0.10.0".to_string()), + check: crate::kw::readiness::KwVersionCheck::Meets, + }, + tree: crate::kw::readiness::TreeReadiness::Ready { + arch: Some("x86_64".to_string()), + }, + output_dir: None, + kernel_image: None, + build_record: None, + latest_build: None, + deploy_alone: Err(crate::kw::readiness::DeployAloneRefusal::NoBuildRecord), + current_branch: branch.map(str::to_string), + }, + ) + } + + #[test] + fn cleared_readable_branch_does_not_claim_detached_head() { + let mut state = app_state_with_kw(None); + state.navigation.current_screen = CurrentScreen::KwOps; + let mut ops = sample_kw_ops(Some("feature")); + ops.branch.clear(); + state.kw.ops = Some(ops); + + let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { + panic!("expected KwOps projection"); + }; + assert_eq!(None, vm.branch_guidance); + assert_eq!("unavailable (set a branch first)", vm.start_label); + } + + #[test] + fn detached_head_projects_branch_guidance() { + let mut state = app_state_with_kw(None); + state.navigation.current_screen = CurrentScreen::KwOps; + state.kw.ops = Some(sample_kw_ops(None)); + + let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { + panic!("expected KwOps projection"); + }; + assert!(vm + .branch_guidance + .as_deref() + .is_some_and(|text| text.contains("detached or unverifiable"))); + } + + #[test] + fn command_preview_tracks_in_progress_extra_args() { + let mut state = app_state_with_kw(None); + state.navigation.current_screen = CurrentScreen::KwOps; + let mut ops = sample_kw_ops(Some("feature")); + ops.extra_args = "--verbose".to_string(); + ops.highlight_next(); + ops.begin_edit(); + ops.append_edit(' '); + ops.append_edit('-'); + ops.append_edit('j'); + ops.append_edit('8'); + state.kw.ops = Some(ops); + + let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { + panic!("expected KwOps projection"); + }; + assert_eq!("kw build --verbose -j8", vm.command); + assert_eq!("--verbose -j8", vm.extra_args); + } + + #[test] + fn start_requested_projects_as_busy() { + let mut state = app_state_with_kw(None); + state.navigation.current_screen = CurrentScreen::KwOps; + let mut ops = sample_kw_ops(Some("feature")); + ops.start_requested = true; + state.kw.ops = Some(ops); + + let ScreenViewModel::KwOps(vm) = project_state(&state).screen else { + panic!("expected KwOps projection"); + }; + assert_eq!("starting…", vm.job_status); + assert_eq!("unavailable (a job is already running)", vm.start_label); + } + #[test] fn idle_succeeded_and_missing_status_have_no_global_indicator() { assert_eq!(None, project_state(&app_state_with_kw(None)).kw_running); diff --git a/src/input/context.rs b/src/input/context.rs index d6f09ee7..dc75591a 100644 --- a/src/input/context.rs +++ b/src/input/context.rs @@ -5,6 +5,7 @@ use crate::app::screens::CurrentScreen; pub struct InputContext { pub current_screen: CurrentScreen, pub popup_open: bool, + pub confirm_popup_open: bool, pub edit_config_editing: bool, pub kw_ops_editing: bool, pub preview_fullscreen: bool, @@ -16,6 +17,7 @@ impl InputContext { Self { current_screen, popup_open: false, + confirm_popup_open: false, edit_config_editing: false, kw_ops_editing: false, preview_fullscreen: false, @@ -28,6 +30,13 @@ impl InputContext { self } + #[cfg(test)] + pub fn with_confirm_popup_open(mut self) -> Self { + self.popup_open = true; + self.confirm_popup_open = true; + self + } + #[cfg(test)] pub fn with_edit_config_editing(mut self, edit_config_editing: bool) -> Self { self.edit_config_editing = edit_config_editing; diff --git a/src/input/mapper.rs b/src/input/mapper.rs index 8a320a5c..908786a0 100644 --- a/src/input/mapper.rs +++ b/src/input/mapper.rs @@ -50,7 +50,7 @@ impl InputMapper { } if context.popup_open { - return self.map_popup_key(&key); + return self.map_popup_key(&key, context); } match &context.current_screen { @@ -63,10 +63,10 @@ impl InputMapper { } } - fn map_popup_key(&mut self, key: &KeyInput) -> Option { + fn map_popup_key(&mut self, key: &KeyInput, context: &InputContext) -> Option { match key.code { KeyCode::Esc | KeyCode::Char('q') => Some(InputEvent::ClosePopup), - KeyCode::Enter => Some(InputEvent::ConfirmPopup), + KeyCode::Enter if context.confirm_popup_open => Some(InputEvent::ConfirmPopup), KeyCode::Char('j') | KeyCode::Down => Some(InputEvent::NavigateDown), KeyCode::Char('k') | KeyCode::Up => Some(InputEvent::NavigateUp), KeyCode::Char('h') | KeyCode::Left => Some(InputEvent::NavigateLeft), @@ -279,13 +279,16 @@ mod tests { } #[test] - fn maps_enter_to_confirm_popup_when_popup_is_open() { + fn maps_enter_to_confirm_popup_only_for_confirm_popups() { let mut mapper = InputMapper::default(); - let context = context(CurrentScreen::MailingListSelection).with_popup_open(true); - - let event = mapper.map_terminal_event(key(KeyCode::Enter), &context); + let confirm = context(CurrentScreen::MailingListSelection).with_confirm_popup_open(); + assert_eq!( + mapper.map_terminal_event(key(KeyCode::Enter), &confirm), + Some(InputEvent::ConfirmPopup) + ); - assert_eq!(event, Some(InputEvent::ConfirmPopup)); + let info = context(CurrentScreen::MailingListSelection).with_popup_open(true); + assert_eq!(mapper.map_terminal_event(key(KeyCode::Enter), &info), None); } #[test] diff --git a/src/kw/actor.rs b/src/kw/actor.rs index 5d36d0c6..5415e588 100644 --- a/src/kw/actor.rs +++ b/src/kw/actor.rs @@ -8,8 +8,9 @@ //! [`crate::kw::readiness`] and records applies through the shared //! [`KwHistoryStore`](crate::kw::history::KwHistoryStore). //! -//! Git checkout runs on the blocking pool. `create_dir_all`, history -//! writes, and completion probes run inline on the actor task. +//! Git checkout and readiness probes run on the blocking pool. +//! `create_dir_all`, history writes, and completion probes run inline +//! on the actor task. use std::{ ops::ControlFlow, @@ -221,7 +222,7 @@ impl KwActor { send_kw_reply( message_name, reply, - self.evaluate_readiness(&kernel_tree_id, &tree), + self.evaluate_readiness(&kernel_tree_id, &tree).await, ); ControlFlow::Continue(()) } @@ -606,21 +607,34 @@ impl KwActor { self.set_status(job); } - fn evaluate_readiness( + /// Git probes and history I/O run on the blocking pool so a slow + /// tree cannot stall Cancel (or any other message) for the duration. + async fn evaluate_readiness( &self, kernel_tree_id: &str, tree: &KernelTree, ) -> Result { - let head = head_branch(&*self.shell, tree.path()); - Ok(readiness::evaluate_readiness( - &*self.fs, - &*self.env, - &*self.shell, - &*self.history, - kernel_tree_id, - tree, - &head, - )?) + let fs = Arc::clone(&self.fs); + let env = Arc::clone(&self.env); + let shell = Arc::clone(&self.shell); + let history = Arc::clone(&self.history); + let kernel_tree_id = kernel_tree_id.to_string(); + let tree = tree.clone(); + tokio::task::spawn_blocking(move || { + let head = head_branch(&*shell, tree.path()); + readiness::evaluate_readiness( + &*fs, + &*env, + &*shell, + &*history, + &kernel_tree_id, + &tree, + &head, + ) + .map_err(KwError::from) + }) + .await + .map_err(|error| KwError::GitStateProbe(error.to_string()))? } /// Switches the tree that ran the last job back to the branch HEAD was diff --git a/src/main.rs b/src/main.rs index 32f67777..97a6dfe0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -146,7 +146,7 @@ async fn main() -> Result<()> { .map_err(|error| eyre!("{error}"))?, config_handle.clone(), bootstrap, - Box::new(OsFileSystem), + fs_arc.clone(), Box::new(OsShell), lore_api.clone(), render.clone(), diff --git a/src/ui/screens/kw_ops.rs b/src/ui/screens/kw_ops.rs index d04d242d..0d89d3b3 100644 --- a/src/ui/screens/kw_ops.rs +++ b/src/ui/screens/kw_ops.rs @@ -89,12 +89,32 @@ fn paint_form(f: &mut Frame, scene: &KwOpsScene, chunk: Rect) { } fn paint_log(f: &mut Frame, scene: &KwOpsScene, chunk: Rect) { + let inner_width = chunk.width.saturating_sub(2); + let inner_height = chunk.height.saturating_sub(2); + let offset = log_scroll_offset(&scene.log_tail, inner_width, inner_height); let paragraph = Paragraph::new(scene.log_tail.clone()) .block(Block::default().borders(Borders::ALL).title(" Build log ")) - .wrap(Wrap { trim: false }); + .wrap(Wrap { trim: false }) + .scroll((offset, 0)); f.render_widget(paragraph, chunk); } +/// Scroll so the newest wrapped rows sit at the bottom of the pane. +/// +/// Uses ratatui's wrap-aware [`Paragraph::line_count`] so the offset +/// matches what the painter actually renders (word wrap, tabs, wide +/// chars). Counted without a [`Block`] and with the inner width, so +/// border rows are not mixed into the text height. +pub(crate) fn log_scroll_offset(text: &str, inner_width: u16, inner_height: u16) -> u16 { + if inner_height == 0 || inner_width == 0 { + return 0; + } + let rows = Paragraph::new(text) + .wrap(Wrap { trim: false }) + .line_count(inner_width); + rows.saturating_sub(inner_height as usize) as u16 +} + fn labeled(label: &str, value: &str) -> Line<'static> { Line::from(vec![ Span::styled(format!("{label}: "), Style::default().fg(Color::Cyan)), @@ -143,3 +163,47 @@ pub fn keys_hint_span(editing: bool) -> Span<'static> { ) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn short_log_does_not_scroll() { + assert_eq!(0, log_scroll_offset("cc1: compiling\n", 40, 10)); + } + + #[test] + fn tall_log_pins_the_newest_rows() { + let text = (0..20) + .map(|i| format!("line {i}")) + .collect::>() + .join("\n"); + assert_eq!(15, log_scroll_offset(&text, 40, 5)); + } + + #[test] + fn hard_wrapped_lines_count_toward_the_offset() { + assert_eq!(1, log_scroll_offset("abcdefghij", 4, 2)); + } + + #[test] + fn word_wrapped_lines_pin_the_newest_rows() { + // Width 10, "aaaaa aaaaa aaaaa": char-ceil would be 2 rows; ratatui + // word-wraps to 3, so height 2 must scroll by 1 to keep the end visible. + assert_eq!(1, log_scroll_offset("aaaaa aaaaa aaaaa", 10, 2)); + } + + #[test] + fn trailing_newline_does_not_invent_an_extra_row() { + // ratatui's line_count does not treat a trailing newline as a + // blank wrapped row, so a one-line pane shows "a" with no scroll. + assert_eq!(0, log_scroll_offset("a\n", 10, 1)); + } + + #[test] + fn zero_inner_area_does_not_scroll() { + assert_eq!(0, log_scroll_offset("line\nline\n", 0, 10)); + assert_eq!(0, log_scroll_offset("line\nline\n", 10, 0)); + } +} From 20000b739db2db6ee8f69b530684205c1fbfeb79 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 20 Sep 2026 10:46:32 -0300 Subject: [PATCH 6/6] docs(app): describe KwOps without later steps This commit drops the Step 7 boot-once note and "build-only" wording from comments that describe the screen as it exists in this change. This commit is part of the kw integration's step 6. Signed-off-by: lorenzoberts --- src/app/actor.rs | 2 +- src/app/popup.rs | 3 --- src/app/screens/kw_ops.rs | 2 +- src/app/view_model.rs | 2 +- src/ui/scene.rs | 2 +- 5 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/app/actor.rs b/src/app/actor.rs index 33994cc7..6f68c24f 100644 --- a/src/app/actor.rs +++ b/src/app/actor.rs @@ -4,7 +4,7 @@ //! [`UiHandle`](crate::ui::handle::UiHandle) → draw through //! [`TerminalHandle`](crate::terminal::handle::TerminalHandle) → await the next //! [`InputEvent`](crate::input::event::InputEvent), a kw-status change, or a -//! KwOps log-tail tick while a build is running on that screen. +//! KwOps log-tail tick while a job is running on that screen. //! //! The actor stops when the input event channel closes (user quit) or when I/O //! returns an unrecoverable error. Startup dependency checks run before this diff --git a/src/app/popup.rs b/src/app/popup.rs index 853c348e..dbc81692 100644 --- a/src/app/popup.rs +++ b/src/app/popup.rs @@ -12,9 +12,6 @@ use crate::{ }; /// Action taken when the user confirms a choice popup. -/// -/// Step 7's boot-once gate will add its own variant; do not reuse -/// [`ConfirmAction::Wait`] or [`ConfirmAction::CancelKwAndQuit`] for that. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum ConfirmAction { CancelKwAndQuit, diff --git a/src/app/screens/kw_ops.rs b/src/app/screens/kw_ops.rs index 0018a8b0..21be8e29 100644 --- a/src/app/screens/kw_ops.rs +++ b/src/app/screens/kw_ops.rs @@ -8,7 +8,7 @@ pub enum KwOpsFocus { ExtraArgs, } -/// Build-path form on the KwOps screen. Job status lives in [`crate::app::state::KwUiState::status`]. +/// Form state on the KwOps screen. Job status lives in [`crate::app::state::KwUiState::status`]. #[derive(Clone, Debug)] pub struct KwOpsState { pub patchset_title: String, diff --git a/src/app/view_model.rs b/src/app/view_model.rs index 29abdf2d..840b2795 100644 --- a/src/app/view_model.rs +++ b/src/app/view_model.rs @@ -126,7 +126,7 @@ pub struct EditConfigViewModel { pub is_editing_mode: bool, } -/// Build-only KwOps dashboard. Labels only; actions stay in AppState. +/// KwOps dashboard. Labels only; actions stay in AppState. #[derive(Clone, Debug)] pub struct KwOpsViewModel { pub patchset_title: String, diff --git a/src/ui/scene.rs b/src/ui/scene.rs index c5a0cfb5..92d0b664 100644 --- a/src/ui/scene.rs +++ b/src/ui/scene.rs @@ -68,7 +68,7 @@ pub struct EditConfigScene { pub entries: Vec, } -/// Scene for the KwOps build dashboard. +/// Scene for the KwOps dashboard. #[derive(Clone, Debug)] pub struct KwOpsScene { pub patchset_title: String,