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