pmaxhogan
diff --git a/‎src-tauri/installer-hooks.nsh‎
Lines changed: 29 additions & 0 deletions b/‎src-tauri/installer-hooks.nsh‎
Lines changed: 29 additions & 0 deletions
diff --git a/‎src-tauri/src/app_state.rs‎
Lines changed: 100 additions & 0 deletions b/‎src-tauri/src/app_state.rs‎
Lines changed: 100 additions & 0 deletions
diff --git a/‎src-tauri/src/updater.rs‎
Lines changed: 34 additions & 0 deletions b/‎src-tauri/src/updater.rs‎
Lines changed: 34 additions & 0 deletions
diff --git a/‎src-tauri/src/vss_helper.rs‎
Lines changed: 42 additions & 0 deletions b/‎src-tauri/src/vss_helper.rs‎
Lines changed: 42 additions & 0 deletions
diff --git a/‎src-tauri/tauri.conf.json‎
Lines changed: 3 additions & 0 deletions b/‎src-tauri/tauri.conf.json‎
Lines changed: 3 additions & 0 deletions
@@ -0,0 +1,29 @@
+; NSIS installer hooks for the Driven Windows bundle (issue #125).
+;
+; Tauri's NSIS installer template inserts NSIS_HOOK_PREINSTALL at the very top of
+; the Install section - before SetOutPath and the first File write, and before
+; its own CheckIfAppIsRunning step. That built-in kill targets ONLY the main
+; binary (driven-app.exe); it never touches the bundled driven-vss-helper.exe
+; sidecar. If an elevated VSS helper broker is still running (a same-session
+; broker, or an orphan from a crashed / force-restarted prior session), it holds
+; an open handle to its own exe and the install aborts with:
+;
+;   Error opening file for writing: ...\driven-vss-helper.exe   [Abort/Retry/Ignore]
+;
+; Belt-and-braces: force-terminate any lingering helper before files are copied.
+; Error tolerated - taskkill exits non-zero when no such process exists, which is
+; the normal (no helper running) case; we discard the exit code with `Pop $0`.
+;
+; LIMITATION (see the PR for #125): the helper always runs ELEVATED (it refuses to
+; start un-elevated), and the default NSIS install mode is `currentUser`, so the
+; updater-spawned installer runs UN-elevated. A medium-integrity taskkill cannot
+; terminate the high-integrity helper (access-denied), so this hook only bites
+; when the installer itself is elevated (an admin-run / perMachine install). The
+; primary fix for the same-session case is the app-side pipe Shutdown before
+; `download_and_install`; the durable fix for a crash-orphaned elevated helper is
+; tracked as a follow-up (a parent-death watchdog in the helper).
+
+!macro NSIS_HOOK_PREINSTALL
+  nsExec::Exec '"$SYSDIR\taskkill.exe" /F /IM driven-vss-helper.exe'
+  Pop $0
+!macroend
@@ -570,6 +570,53 @@ impl AppState {
         }
     }
 
+    /// Shut the VSS helper broker down BEFORE an in-app update installer runs
+    /// (issue #125), returning whether a manager was disabled (so the caller can
+    /// RE-ARM it if the install then fails - see
+    /// [`Self::rearm_vss_helper_after_failed_update`]).
+    ///
+    /// The Windows NSIS updater overwrites the bundled `driven-vss-helper.exe`
+    /// sidecar, but its stock process-kill only targets the MAIN binary
+    /// (`driven-app.exe`) - never the sidecar. A running elevated broker holds an
+    /// open handle to its own exe, so the install fails with "Error opening file
+    /// for writing: ...driven-vss-helper.exe". `download_and_install` runs the
+    /// NSIS installer synchronously (`/P /R`), so the broker must be gone BEFORE
+    /// that call.
+    ///
+    /// Unlike the app-quit [`Self::shutdown_vss_helper`] (a bare `shutdown()`),
+    /// this uses `set_enabled(false)`, which is a SUPERSET: it performs the same
+    /// Shutdown+reap (including abandoning + reaping a `Pending` launch per the
+    /// #113 generation semantics) AND disables the manager so a still-running
+    /// sync that hits a locked file cannot RE-LAUNCH the elevated broker (and
+    /// re-lock the exe) during the potentially-long `download_and_install`
+    /// window. A memoised session decline is reset to `NotAttempted` by the
+    /// underlying shutdown; that is inherent to any shutdown-based sweep and
+    /// harmless here (an update is user-consented and a successful install
+    /// restarts the app anyway).
+    ///
+    /// Best-effort + idempotent: a no-op (returns `false`) when no manager is in
+    /// play (off Windows / elevated / setting off).
+    pub fn shutdown_vss_helper_for_update(&self) -> bool {
+        if let Some(manager) = self.vss_helper_manager() {
+            manager.set_enabled(false);
+            true
+        } else {
+            false
+        }
+    }
+
+    /// Re-arm the VSS helper broker after a FAILED update install (issue #125):
+    /// the app keeps running, so undo the
+    /// [`Self::shutdown_vss_helper_for_update`] disable so locked-file backup is
+    /// available again on demand (a LAZY re-launch on the next locked file - no
+    /// forced UAC prompt), rather than staying silently degraded until the next
+    /// app restart. Best-effort + idempotent; a no-op when no manager is in play.
+    pub fn rearm_vss_helper_after_failed_update(&self) {
+        if let Some(manager) = self.vss_helper_manager() {
+            manager.set_enabled(true);
+        }
+    }
+
     // --- M9c D4: recovery-phrase ACK gate (M6 R4-P1-1, DATA-SAFETY) ---------
 
     /// Lock the recovery-ack map, recovering a poisoned lock (house rule: never
@@ -1659,6 +1706,59 @@ mod tests {
         let _ = std::fs::remove_dir_all(dir);
     }
 
+    #[tokio::test]
+    async fn shutdown_vss_helper_for_update_disables_then_rearm_restores() {
+        // Issue #125: the updater path disables the broker BEFORE
+        // `download_and_install` so a live/relaunching elevated helper cannot hold
+        // its own exe open while the NSIS installer overwrites it. On a FAILED
+        // install (app keeps running) the caller re-arms it so locked-file backup
+        // is not left silently degraded.
+        use driven_vss_helper::HelperLauncher; // brings `is_available` into scope
+        let (state, dir) = temp_state().await;
+        let app_state = AppState::new(
+            state,
+            HashMap::new(),
+            RemoteMode::Fake,
+            default_fake_registry(),
+        );
+
+        // No manager: both calls are safe no-ops; the pre-install sweep reports it
+        // disabled nothing.
+        assert!(!app_state.shutdown_vss_helper_for_update());
+        app_state.rearm_vss_helper_after_failed_update();
+
+        // Install an ENABLED manager with an injected launch (no real UAC /
+        // process). Not launched yet -> NotAttempted + enabled == launchable.
+        let manager = Arc::new(crate::vss_helper::VssHelperManager::with_launch_fn(
+            std::env::temp_dir().join("driven-vss-helper.exe"),
+            std::env::temp_dir(),
+            true,
+            Box::new(|| Ok(())),
+        ));
+        app_state.set_vss_helper_manager(manager.clone());
+        assert!(
+            manager.is_available(),
+            "an enabled, not-yet-tried broker is available on demand"
+        );
+
+        // The pre-install sweep disables it (so a mid-download locked file cannot
+        // re-launch the broker) and reports it acted.
+        assert!(app_state.shutdown_vss_helper_for_update());
+        assert!(
+            !manager.is_available(),
+            "disabled broker is not available -> cannot re-launch during the install"
+        );
+
+        // A failed install re-arms it so backup is available again on demand.
+        app_state.rearm_vss_helper_after_failed_update();
+        assert!(
+            manager.is_available(),
+            "re-arm after a failed install restores on-demand launchability"
+        );
+
+        let _ = std::fs::remove_dir_all(dir);
+    }
+
     #[tokio::test]
     async fn recovery_ack_gate_requires_a_recorded_backend_reveal() {
         // M9c D4 (M6 R4-P1-1, DATA-SAFETY): the ack gate `ack_recovery_phrase_saved`
 
@@ -604,12 +604,28 @@ async fn silent_install_dev_update(
         }
     };
 
+    // Issue #125: shut the elevated VSS helper broker down BEFORE the installer
+    // runs (same reason as the manual `install_update` path). The dev-silent path
+    // ran `download_and_install` from the periodic task WITHOUT the app-quit
+    // sweep, so a live broker either blocked the install or was left orphaned when
+    // the installer restarted the app - the exact QA repro. Disabling also blocks
+    // a mid-download re-launch; re-armed on failure since the app keeps running.
+    let vss_disabled = app
+        .try_state::<AppState>()
+        .map(|s| s.shutdown_vss_helper_for_update())
+        .unwrap_or(false);
+
     match update.download_and_install(on_chunk, on_done).await {
         Ok(()) => {
             tracing::info!(target: TARGET, version = %info.version, "dev update installed silently; applies on next restart");
             crate::tray::notify_dev_update_installed(app, &info.version);
         }
         Err(e) => {
+            if vss_disabled {
+                if let Some(s) = app.try_state::<AppState>() {
+                    s.rearm_vss_helper_after_failed_update();
+                }
+            }
             tracing::warn!(target: TARGET, error = %e, version = %info.version, "dev silent update install failed (will retry next interval)");
         }
     }
@@ -688,6 +704,18 @@ pub async fn install_update(app: AppHandle, state: State<'_, AppState>) -> Comma
     // The display channel for the downloaded event (R1-P2-3).
     let channel = Channel::from_str_lenient(&downloaded_channel(&channel_str));
 
+    // Issue #125: shut the elevated VSS helper broker down BEFORE the installer
+    // runs. `download_and_install` executes the NSIS installer synchronously
+    // (`/P /R`), which overwrites the bundled `driven-vss-helper.exe` sidecar -
+    // but the stock installer only terminates the MAIN binary, so a live broker
+    // holds its own exe open and the install fails with "Error opening file for
+    // writing: ...driven-vss-helper.exe". Disabling (not a bare shutdown) also
+    // blocks a still-running sync from re-launching the broker mid-download. This
+    // ALSO stops the update path itself from seeding an orphan: the pre-fix path
+    // let `/R` restart the app WITHOUT ever sweeping the helper. Re-armed below
+    // if the install fails (the app keeps running).
+    let vss_disabled = state.shutdown_vss_helper_for_update();
+
     // The progress callback accumulates downloaded bytes and emits
     // `updater:download_progress`. `content_length` arrives once the server
     // reports it; until then `total` is None.
@@ -718,6 +746,12 @@ pub async fn install_update(app: AppHandle, state: State<'_, AppState>) -> Comma
         // R1-P2-2: restore the pending update (with its channel) so the user can
         // retry without re-checking.
         state.set_pending_update(Some((update, channel_str)));
+        // Issue #125: the install failed and the app keeps running - re-arm the
+        // VSS helper we disabled above so locked-file backup is not left silently
+        // degraded for the rest of the session.
+        if vss_disabled {
+            state.rearm_vss_helper_after_failed_update();
+        }
     }
     install_result.map_err(map_install_error)?;
 
 
@@ -734,6 +734,48 @@ mod tests {
         );
     }
 
+    #[test]
+    fn update_sweep_reaps_ready_helper_and_blocks_relaunch() {
+        // Issue #125: the updater's pre-install sweep (AppState
+        // `shutdown_vss_helper_for_update`) calls `set_enabled(false)`. On a Ready
+        // broker this must (1) reap the running helper - directly, like the quit
+        // sweep - so it releases its exe handle before the NSIS installer
+        // overwrites it, AND (2) DISABLE the manager so a still-running sync that
+        // hits a locked file cannot RE-LAUNCH the elevated broker (re-locking the
+        // exe) during the `download_and_install` window.
+        let (launch, calls) = counting_launch(Ok(()));
+        let mgr = manager(true, launch);
+        mgr.launch_now();
+        mgr.join_launch_thread();
+        assert!(
+            mgr.helper_alive(),
+            "the broker is up before the update sweep"
+        );
+
+        // The pre-install sweep: disable + reap.
+        mgr.set_enabled(false);
+        assert!(
+            !mgr.helper_alive(),
+            "the broker is shut down before the installer runs"
+        );
+        assert!(!mgr.is_available(), "a disabled broker is not available");
+        assert_eq!(mgr.launch_status(), LaunchStatus::Disabled);
+        // A Ready broker is reaped by the shutdown path directly (not the
+        // abandoned-launch resolver), so no reap is counted there.
+        assert_eq!(mgr.reap_count(), 0);
+
+        // A locked file mid-download consults the launcher: it must NOT re-launch
+        // the elevated broker while the install is in flight.
+        let before = calls.load(Ordering::SeqCst);
+        mgr.launch_now();
+        mgr.join_launch_thread();
+        assert_eq!(
+            calls.load(Ordering::SeqCst),
+            before,
+            "a disabled broker must not re-launch during the install window"
+        );
+    }
+
     /// Exercise the PRODUCTION constructor + launch path (`new` -> `production_launch`
     /// -> `launch_elevated`) without a real UAC prompt. Gated to non-Windows: there
     /// `launch_elevated` reports "Windows only" immediately (no `runas`, no prompt),
 
@@ -76,6 +76,9 @@
     "windows": {
       "wix": {
         "language": "en-US"
+      },
+      "nsis": {
+        "installerHooks": "installer-hooks.nsh"
       }
     }
   }
Original file line number	Diff line number	Diff line change
`@@ -76,6 +76,9 @@`
`76`	`76`	`"windows": {`
`77`	`77`	`"wix": {`
`78`	`78`	`"language": "en-US"`
	`79`	`+ },`
	`80`	`+ "nsis": {`
	`81`	`+ "installerHooks": "installer-hooks.nsh"`
`79`	`82`	`}`
`80`	`83`	`}`
`81`	`84`	`}`