From bbf92935b61cd248d0aa8687a4c6098092cb944d Mon Sep 17 00:00:00 2001
From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com>
Date: Sat, 8 Aug 2026 10:08:01 +0800
Subject: [PATCH] fix(windows): tighten the install-dir clearing per review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- An image that fails to canonicalize stays in the lock check instead of
  being silently skipped; only a positive match against the caller's own
  running image is excluded.
- reap_recorded_daemon shares one deadline across the whole tree via a
  new winproc::terminate_and_wait_all, which stop_for_update and
  reap_descendants_of now use too — one implementation of "terminate,
  then wait, bounded overall" instead of three.
- [UninstallRun] passes --update-install-dir "{app}" like
  PrepareToInstall, so uninstalling after a daemon crash gets the same
  orphaned-ConPTY-host cleanup as upgrading.
---
 .github/scripts/windows-installer.iss  | 16 ++++++++------
 crates/tty7-core/src/daemon/spawn.rs   | 25 +++++++++++----------
 crates/tty7-core/src/daemon/winproc.rs | 30 ++++++++++++++++----------
 3 files changed, 41 insertions(+), 30 deletions(-)

diff --git a/.github/scripts/windows-installer.iss b/.github/scripts/windows-installer.iss
index 26a4919d..5aad047a 100644
--- a/.github/scripts/windows-installer.iss
+++ b/.github/scripts/windows-installer.iss
@@ -130,12 +130,16 @@ Filename: "{app}\tty7-app.exe"; Description: "{cm:LaunchProgram,tty7}"; Flags: n
 
 [UninstallRun]
 ; Stop the daemon before the uninstaller deletes tty7-app.exe — the running daemon
-; is the locked image of that file, so removing it fails otherwise. This runs at
-; the start of uninstallation, before any files are removed. The installed binary
-; is this version, which understands the flag; runhidden suppresses any flash and
-; the call returns without opening a window. RunOnceId keys the entry so a repeated
-; uninstall doesn't run it twice.
-Filename: "{app}\tty7-app.exe"; Parameters: "--stop-daemon"; Flags: runhidden waituntilterminated; RunOnceId: "StopDaemon"
+; is the locked image of that file, so removing it fails otherwise. Naming {app}
+; widens the stop the same way PrepareToInstall's does: ConPTY hosts orphaned by
+; a daemon that never got to shut down keep the installed images open, and their
+; DeleteFile fails an uninstall exactly as it fails an upgrade. (The stop excludes
+; the calling process itself, so the binary running this step is safe.) This runs
+; at the start of uninstallation, before any files are removed. The installed
+; binary is this version, which understands the flags; runhidden suppresses any
+; flash and the call returns without opening a window. RunOnceId keys the entry
+; so a repeated uninstall doesn't run it twice.
+Filename: "{app}\tty7-app.exe"; Parameters: "--stop-daemon --update-install-dir ""{app}"""; Flags: runhidden waituntilterminated; RunOnceId: "StopDaemon"
 ; Unconditional, and deliberately not gated on the task: an install that had the
 ; menu registered and was later upgraded without the box ticked still holds the
 ; keys, and verbs pointing at a deleted exe are worse than a no-op. Removing keys
diff --git a/crates/tty7-core/src/daemon/spawn.rs b/crates/tty7-core/src/daemon/spawn.rs
index ef130fb1..0a478246 100644
--- a/crates/tty7-core/src/daemon/spawn.rs
+++ b/crates/tty7-core/src/daemon/spawn.rs
@@ -419,12 +419,12 @@ fn reap_recorded_daemon() {
         .is_some_and(|entry| is_reapable_daemon_name(&entry.name));
     if matches {
         log::warn!("reaping unreachable daemon (pid {pid}); its sessions will be hung up");
-        for descendant in winproc::descendants(&procs, pid) {
-            winproc::terminate(descendant);
-            winproc::wait_for_exit(descendant, REAP_WAIT_TIMEOUT);
-        }
-        winproc::terminate(pid);
-        winproc::wait_for_exit(pid, REAP_WAIT_TIMEOUT);
+        // One deadline across the whole tree: this runs synchronously before
+        // the first window exists, and a crash that left many hosts behind
+        // must not multiply the wait by their count.
+        let mut targets = winproc::descendants(&procs, pid);
+        targets.push(pid);
+        winproc::terminate_and_wait_all(&targets, Instant::now() + REAP_WAIT_TIMEOUT);
     }
     pidfile::remove();
 }
@@ -455,12 +455,8 @@ pub fn stop_for_update(install_dir: &Path) -> Result<(), String> {
             "terminating pid {pid} still running from {}",
             install_dir.display()
         );
-        winproc::terminate(pid);
-    }
-    for pid in holdouts {
-        let left = deadline.saturating_duration_since(Instant::now());
-        winproc::wait_for_exit(pid, left);
     }
+    winproc::terminate_and_wait_all(&holdouts, deadline);
 
     wait_until_images_unlocked(install_dir, deadline)
 }
@@ -491,8 +487,11 @@ fn wait_until_images_unlocked(dir: &Path, deadline: Instant) -> Result<(), Strin
                 })
         })
         .filter(|path| {
-            own.as_deref()
-                .is_none_or(|own| std::fs::canonicalize(path).is_ok_and(|path| path != own))
+            // Excluding takes a positive identification: a candidate that
+            // cannot be canonicalized (delete-pending, held by a scanner) is
+            // a lock to wait out, not ours to skip.
+            !own.as_deref()
+                .is_some_and(|own| std::fs::canonicalize(path).is_ok_and(|path| path == own))
         })
         .collect();
 
diff --git a/crates/tty7-core/src/daemon/winproc.rs b/crates/tty7-core/src/daemon/winproc.rs
index 4e9eb3a0..fa28fb2c 100644
--- a/crates/tty7-core/src/daemon/winproc.rs
+++ b/crates/tty7-core/src/daemon/winproc.rs
@@ -135,21 +135,29 @@ pub(crate) fn wait_for_exit(pid: u32, timeout: std::time::Duration) -> bool {
     }
 }
 
+/// Terminates every pid in order and waits for each to release its image,
+/// all bounded by the one `deadline` — never a fresh timeout per process.
+/// Callers pass parents after children when a parent could respawn one.
+pub(crate) fn terminate_and_wait_all(pids: &[u32], deadline: std::time::Instant) {
+    for &pid in pids {
+        terminate(pid);
+    }
+    for &pid in pids {
+        let left = deadline.saturating_duration_since(std::time::Instant::now());
+        if !wait_for_exit(pid, left) {
+            log::warn!("process {pid} did not exit in time");
+        }
+    }
+}
+
 /// Terminates every descendant of `root` and waits for them to release their
 /// images, bounded by `timeout` overall. Deepest-first, like the per-pane
 /// kill, so a parent never respawns a child we already visited.
 pub(crate) fn reap_descendants_of(root: u32, timeout: std::time::Duration) {
-    let deadline = std::time::Instant::now() + timeout;
-    let targets = descendants(&snapshot(), root);
-    for &pid in &targets {
-        terminate(pid);
-    }
-    for pid in targets {
-        let left = deadline.saturating_duration_since(std::time::Instant::now());
-        if !wait_for_exit(pid, left) {
-            log::warn!("descendant process {pid} did not exit in time");
-        }
-    }
+    terminate_and_wait_all(
+        &descendants(&snapshot(), root),
+        std::time::Instant::now() + timeout,
+    );
 }
 
 /// Pids (other than the caller's own) whose executable image lives under