lorenzo/start kw integration pr 4

.github/workflows/build_and_unit_test.yml

@@ -10,9 +10,13 @@ on:
       - labeled
     paths:
       - src/**
+      - Cargo.toml
+      - Cargo.lock
   push:
     paths:
       - src/**
+      - Cargo.toml
+      - Cargo.lock
     branches:
       - master
       - unstable
@@ -50,3 +54,37 @@ jobs:
           printf '\e[1;33m\tPLEASE, SOLVE THEM LOCALLY W/ `cargo test`\e[0m\n'
           printf '\e[1;33m\t==========================================\n\e[0m'
           exit 1
+
+  # The kw actor and the process infrastructure it uses are unix-only
+  # (#[cfg(unix)]). This job exists so a cfg leak — non-gated code
+  # referencing them — fails mechanically instead of in review.
+  check-non-unix:
+    runs-on: ubuntu-latest
+    timeout-minutes: 4
+    if: '!github.event.pull_request.draft'
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          ref: ${{ github.event.pull_request.head.sha }}
+
+      - name: Update rustup and install rustc and cargo
+        shell: bash
+        run: |
+          rustup update
+          rustup install stable
+
+      # rustls/ring needs a C toolchain for the target; windows-gnu builds
+      # with mingw on Ubuntu, unlike the MSVC target.
+      - name: Install the mingw cross toolchain
+        shell: bash
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y gcc-mingw-w64-x86-64
+
+      - name: Check non-unix compilation
+        shell: bash
+        env:
+          CARGO_TARGET_X86_64_PC_WINDOWS_GNU_LINKER: x86_64-w64-mingw32-gcc
+        run: |
+          rustup target add x86_64-pc-windows-gnu
+          cargo check --target x86_64-pc-windows-gnu --verbose

Cargo.toml

@@ -27,7 +27,7 @@ chrono = "0.4.41"
 ansi-to-tui = "7.0.0"
 which = "8.0.0"
 ureq = { version = "3.0.12", features = ["rustls"] }
-tokio = { version = "1.52.3", features = ["rt-multi-thread", "macros", "sync", "process", "time"] }
+tokio = { version = "1.52.3", features = ["rt-multi-thread", "macros", "sync", "process", "time", "test-util"] }
 async-trait = "0.1"
 nix = { version = "0.31", features = ["signal"] }
 base64 = "0.22"

src/app/actor.rs

@@ -234,6 +234,7 @@ mod tests {
                 fs: Box::new(MockFileSystemTrait::new()),
                 config: dummy_config_handle(),
                 kw_history: Arc::new(MockKwHistoryStore::new()),
+                kw: None,
             },
         }
     }
@@ -336,6 +337,7 @@ mod tests {
             lore_api.clone(),
             render.clone(),
             Arc::new(MockKwHistoryStore::new()),
+            None,
         )
         .expect("App::new must succeed");
 

src/app/integration_tests/helpers/app_harness.rs

@@ -56,6 +56,7 @@ pub(crate) fn app_with_bootstrap_and_handles(
         lore_api,
         render,
         Arc::new(MockKwHistoryStore::new()),
+        None,
     )
     .expect("minimal app should build")
 }

src/app/integration_tests/patchset_actions.rs

@@ -1,5 +1,6 @@
 use std::{
     collections::{HashMap, HashSet, VecDeque},
+    path::PathBuf,
     sync::{Arc, Mutex},
 };
 
@@ -16,10 +17,12 @@ use crate::{
     },
     config::{ConfigSnapshot, ConfigState},
     infrastructure::{
+        env::MockEnvTrait,
         file_system::{FileSystemError, MockFileSystemTrait},
+        process::FakeProcess,
         shell::{MockShellTrait, ShellCommand, ShellOutput},
     },
-    kw::history::MockKwHistoryStore,
+    kw::{actor::KwActor, history::MockKwHistoryStore},
     lore::application::{
         cache::BootstrapLoreData, handle::LoreApiHandle, messages::LoreApiMessage,
     },
@@ -64,6 +67,7 @@ async fn apply_success_sets_success_popup_and_resets_apply_action() {
             "Current branch: 'patchset-",
         ],
     );
+    shutdown_kw(&app).await;
 }
 
 #[tokio::test]
@@ -94,11 +98,14 @@ async fn apply_success_switches_back_when_stay_disabled() {
         "Patchset Apply Success",
         &["Current branch: 'feature'"],
     );
-    let calls = calls.lock().unwrap();
-    assert_eq!(
-        command(&["git", "-C", KERNEL_TREE_PATH, "switch", "feature"]),
-        calls[6]
-    );
+    {
+        let calls = calls.lock().unwrap();
+        assert_eq!(
+            command(&["git", "-C", KERNEL_TREE_PATH, "switch", "feature"]),
+            calls[6]
+        );
+    }
+    shutdown_kw(&app).await;
 }
 
 #[tokio::test]
@@ -123,11 +130,14 @@ async fn apply_failure_sets_failure_popup_and_resets_apply_action() {
         "Patchset Apply Fail",
         &["`git am` failed", "feature", "apply failed"],
     );
-    let calls = calls.lock().unwrap();
-    assert_eq!(
-        command(&["git", "-C", KERNEL_TREE_PATH, "am", "--abort"]),
-        calls[6]
-    );
+    {
+        let calls = calls.lock().unwrap();
+        assert_eq!(
+            command(&["git", "-C", KERNEL_TREE_PATH, "am", "--abort"]),
+            calls[6]
+        );
+    }
+    shutdown_kw(&app).await;
 }
 
 #[tokio::test]
@@ -169,6 +179,7 @@ async fn apply_success_records_apply_history() {
         "Patchset Apply Success",
         &["applied successfully"],
     );
+    shutdown_kw(&app).await;
 }
 
 #[tokio::test]
@@ -208,6 +219,7 @@ async fn apply_success_with_history_write_failure_keeps_success_popup() {
             "inspect or delete that file",
         ],
     );
+    shutdown_kw(&app).await;
 }
 
 #[tokio::test]
@@ -236,6 +248,7 @@ async fn apply_failure_does_not_record_history() {
     app.consolidate_patchset_actions().await.unwrap();
 
     assert_info_popup_contains(app.state.popup.as_ref(), "Patchset Apply Fail", &[]);
+    shutdown_kw(&app).await;
 }
 
 #[tokio::test]
@@ -268,6 +281,7 @@ async fn reviewed_reply_success_records_persists_and_resets_reply_action() {
         .clone()
         .expect("reviewed state should be persisted");
     assert_eq!(HashSet::from([0]), saved[&message_id]);
+    shutdown_kw(&app).await;
 }
 
 #[tokio::test]
@@ -297,6 +311,7 @@ async fn reviewed_reply_failure_does_not_record_failed_index() {
         .clone()
         .expect("reviewed state should be persisted");
     assert!(saved[&message_id].is_empty());
+    shutdown_kw(&app).await;
 }
 
 fn app_with_apply_details(fs: MockFileSystemTrait, shell: MockShellTrait) -> App {
@@ -310,6 +325,14 @@ fn app_with_apply_details(fs: MockFileSystemTrait, shell: MockShellTrait) -> App
     )
 }
 
+/// Shuts down the KwActor the app was wired with, instead of relying on
+/// the test runtime aborting it at drop.
+async fn shutdown_kw(app: &App) {
+    if let Some(kw) = &app.services.kw {
+        kw.shutdown().await;
+    }
+}
+
 fn app_with_reviewed_reply_details(shell: MockShellTrait, lore_api: LoreApiHandle) -> App {
     app_with_details(
         MockFileSystemTrait::new(),
@@ -329,6 +352,16 @@ fn app_with_details(
     config: ConfigSnapshot,
     kw_history: MockKwHistoryStore,
 ) -> App {
+    // Apply history is recorded through the real actor wrapping the mock
+    // store, mirroring production wiring.
+    let kw = KwActor::spawn(
+        Arc::new(kw_history),
+        Arc::new(FakeProcess::new()),
+        Arc::new(MockShellTrait::new()),
+        Arc::new(MockFileSystemTrait::new()),
+        Arc::new(MockEnvTrait::new()),
+        PathBuf::from("/tmp/patch-hub-test-kw-logs"),
+    );
     let mut app = App::new(
         config,
         dummy_config_handle(),
@@ -341,7 +374,8 @@ fn app_with_details(
         Box::new(shell),
         lore_api,
         dummy_render_handle(),
-        Arc::new(kw_history),
+        Arc::new(MockKwHistoryStore::new()),
+        Some(kw),
     )
     .expect("app should build");
 

src/app/mod.rs

@@ -47,7 +47,10 @@ use crate::{
         file_system::FileSystemTrait, monitoring::logging::garbage_collector::collect_garbage,
         shell::ShellTrait,
     },
-    kw::history::{KwApplyRecord, KwHistoryStore},
+    kw::{
+        handle::KwHandle,
+        history::{KwApplyRecord, KwHistoryStore},
+    },
     lore::{
         application::{
             cache::{BootstrapLoreData, CacheMode},
@@ -76,8 +79,12 @@ pub struct AppServices {
     pub shell: Box<dyn ShellTrait>,
     pub fs: Box<dyn FileSystemTrait>,
     pub config: ConfigHandle,
-    /// Shared with KwActor once it exists (the actor adopts the same store).
+    /// Direct access to the store, for the non-unix fallback below.
     pub kw_history: Arc<dyn KwHistoryStore>,
+    /// `None` on non-unix builds, where ProcessTrait (and thus KwActor)
+    /// does not exist; apply-history writes then go to `kw_history`
+    /// directly, as they did before the actor landed.
+    pub kw: Option<KwHandle>,
 }
 
 /// Result type signalling whether a patchset was successfully loaded.
@@ -110,6 +117,7 @@ impl App {
         lore_api: LoreApiHandle,
         render: RenderHandle,
         kw_history: Arc<dyn KwHistoryStore>,
+        kw: Option<KwHandle>,
     ) -> Result<Self> {
         event!(Level::INFO, "patch-hub started");
         collect_garbage(&config);
@@ -147,6 +155,7 @@ impl App {
                 fs,
                 config: config_handle,
                 kw_history,
+                kw,
             },
         })
     }
@@ -310,7 +319,7 @@ impl App {
         debug!("consolidating patchset actions");
         self.sync_patchset_bookmark().await?;
         self.execute_reviewed_reply().await?;
-        self.execute_apply_patchset();
+        self.execute_apply_patchset().await;
         debug!("patchset actions consolidated");
         Ok(())
     }
@@ -408,7 +417,7 @@ impl App {
         Ok(())
     }
 
-    fn execute_apply_patchset(&mut self) {
+    async fn execute_apply_patchset(&mut self) {
         let details = self
             .state
             .lore
@@ -437,18 +446,33 @@ impl App {
                                 applied.message
                             )
                         }
-                        // The git apply itself succeeded; a history-write
-                        // failure must not turn it into a reported failure.
-                        Some(record) => match self.services.kw_history.record_apply(record) {
-                            Ok(()) => applied.message,
-                            Err(e) => {
-                                warn!(error = %e, "failed to record kw apply history");
-                                format!(
-                                    "{}\n\nWarning: the apply was not recorded in the kw history: {e}\nIf this warning keeps appearing, inspect or delete that file.",
-                                    applied.message
-                                )
+                        Some(record) => {
+                            // History writes go through KwActor so apply
+                            // recording serializes with job state; without an
+                            // actor (non-unix), write the store directly.
+                            let recorded = match &self.services.kw {
+                                Some(kw) => {
+                                    kw.record_apply(record).await.map_err(|e| e.to_string())
+                                }
+                                None => self
+                                    .services
+                                    .kw_history
+                                    .record_apply(record)
+                                    .map_err(|e| e.to_string()),
+                            };
+                            // The git apply itself succeeded; a history-write
+                            // failure must not turn it into a reported failure.
+                            match recorded {
+                                Ok(()) => applied.message,
+                                Err(e) => {
+                                    warn!(error = %e, "failed to record kw apply history");
+                                    format!(
+                                        "{}\n\nWarning: the apply was not recorded in the kw history: {e}\nIf this warning keeps appearing, inspect or delete that file.",
+                                        applied.message
+                                    )
+                                }
                             }
-                        },
+                        }
                     };
                     popup::AppPopup::info("Patchset Apply Success", popup_body)
                 }