Fix Windows preview process leak, and make the nightly heavy tests able to fail honestly

.github/workflows/nightly.yml

@@ -101,6 +101,13 @@ jobs:
           dart run build_runner build --delete-conflicting-outputs
 
       - name: Heavy integration tests
+        # ToolLocator resolves the worker relative to the running executable,
+        # which under `flutter test` is the test runner, not the app bundle. The
+        # tests that drive WorkerManager for real therefore need this pointed at
+        # the built binary; without it they fail with "Worker executable not
+        # found" and prove nothing.
+        env:
+          VAPOURBOX_WORKER: ${{ github.workspace }}/worker/target/debug/vapourbox-worker
         run: cd app && flutter test --tags heavy
 
   windows:
@@ -171,6 +178,10 @@ jobs:
           dart run build_runner build --delete-conflicting-outputs
 
       - name: Heavy integration tests
+        # See the macOS job: ToolLocator needs the worker path under
+        # `flutter test`, where the running executable is the test runner.
+        env:
+          VAPOURBOX_WORKER: ${{ github.workspace }}\worker\target\debug\vapourbox-worker.exe
         run: cd app && flutter test --tags heavy
 
   linux:
@@ -248,4 +259,11 @@ jobs:
           dart run build_runner build --delete-conflicting-outputs
 
       - name: Heavy integration tests
+        # ToolLocator resolves the worker relative to the running executable,
+        # which under `flutter test` is the test runner, not the app bundle. The
+        # tests that drive WorkerManager for real therefore need this pointed at
+        # the built binary; without it they fail with "Worker executable not
+        # found" and prove nothing.
+        env:
+          VAPOURBOX_WORKER: ${{ github.workspace }}/worker/target/debug/vapourbox-worker
         run: cd app && flutter test --tags heavy

app/lib/services/preview_generator.dart

@@ -301,8 +301,8 @@ class PreviewGenerator {
       _livePreviews.add(process);
 
       if (cancelToken?.isCancelled ?? false) {
-        // Whole group: killing the worker alone strands vspipe/ffmpeg.
-        ProcessTree.killTree(process);
+        // Whole tree: killing the worker alone strands vspipe/ffmpeg.
+        await ProcessTree.killTree(process);
         _previewProcess = null;
         _livePreviews.remove(process);
         await ProcessTree.waitForExit(process);
@@ -329,10 +329,9 @@ class PreviewGenerator {
       await for (final chunk in process.stdout) {
         if (cancelToken?.isCancelled ?? false) {
           // Whole group: killing the worker alone strands vspipe/ffmpeg.
-          ProcessTree.killTree(process);
+          await ProcessTree.killTree(process);
           _previewProcess = null;
           _livePreviews.remove(process);
-        _livePreviews.remove(process);
           await ProcessTree.waitForExit(process);
           return null;
         }
@@ -416,7 +415,7 @@ class PreviewGenerator {
     for (final p in doomed) {
       // The whole group: signalling the worker alone strands vspipe/ffmpeg,
       // and preview mode has no handler that would clean up after itself.
-      ProcessTree.killTree(p);
+      await ProcessTree.killTree(p);
       // Reap in the background so a slow shutdown cannot stall the next seek.
       unawaited(ProcessTree.waitForExit(p));
     }

app/lib/services/process_tree.dart

@@ -13,6 +13,13 @@ import 'dart:io';
 /// The worker puts itself in its own process group at startup (`setpgid` in
 /// `worker/src/main.rs`), so its whole tree can be signalled at once by sending
 /// to the negated pid. That is a deliberate teardown rather than a hopeful one.
+///
+/// Windows has no process groups, so the equivalent there is `taskkill /T`,
+/// which walks the child tree. That belongs **here** rather than in each caller:
+/// it used to be the caller's job, `worker_manager` did it, `preview_generator`
+/// did not, and preview cancellation therefore stranded vspipe and ffmpeg on
+/// Windows every time the scrubber moved. One implementation, so the next caller
+/// cannot forget.
 class ProcessTree {
   /// Signal [process] and everything it spawned.
   ///
@@ -21,11 +28,29 @@ class ProcessTree {
   /// without process groups. That fallback is exactly the previous behaviour, so
   /// this is never worse than what it replaced.
   ///
-  /// Returns true if the group signal landed.
-  static bool killTree(Process process, [ProcessSignal signal = ProcessSignal.sigterm]) {
+  /// **[signal] is advisory on Windows.** `taskkill /F` is unconditionally
+  /// forceful, so a `sigterm` request kills as hard as a `sigkill` one. Nothing
+  /// gentler reaches a child tree there, and leaving the tree alive is worse.
+  ///
+  /// Returns true if the whole tree was addressed (a group signal on Unix, a
+  /// successful `taskkill /T` on Windows), false if only the leader was.
+  static Future<bool> killTree(Process process,
+      [ProcessSignal signal = ProcessSignal.sigterm]) async {
     if (Platform.isWindows) {
-      // No process groups; taskkill /T walks the tree instead. Callers on
-      // Windows use that directly.
+      try {
+        final r = await Process.run(
+          'taskkill',
+          ['/PID', '${process.pid}', '/T', '/F'],
+        );
+        // 128 (and 255) mean it had already exited, which is success for our
+        // purposes — there is no tree left to strand.
+        if (r.exitCode == 0 || r.exitCode == 128 || r.exitCode == 255) {
+          return true;
+        }
+      } on ProcessException {
+        // taskkill missing or unrunnable — fall through to the leader-only kill
+        // so this is never worse than the old behaviour.
+      }
       return process.kill(signal);
     }
     // A negative pid addresses the process group. Dart forwards this to kill(2)
@@ -54,7 +79,7 @@ class ProcessTree {
       // Still alive. Forcing it here can orphan the children, which is the very
       // thing this class exists to avoid — so force the whole group, not just
       // the leader.
-      killTree(process, ProcessSignal.sigkill);
+      await killTree(process, ProcessSignal.sigkill);
       try {
         await process.exitCode.timeout(forceGrace);
       } on Object {

app/lib/services/worker_manager.dart

@@ -273,18 +273,11 @@ class WorkerManager implements JobRunner {
       return;
     }
 
-    if (Platform.isWindows) {
-      // No SIGTERM on Windows, and Process.kill maps to TerminateProcess, which
-      // does not touch children. taskkill /T walks the tree, so nothing is
-      // orphaned; /F is unavoidable there.
-      await Process.run('taskkill', ['/PID', '${process.pid}', '/T', '/F']);
-    } else {
-      // Signal the whole process group. The worker still tears its own pipeline
-      // down when it gets the chance, but that only covers the children it
-      // tracks, and it cannot run at all if it is forced below — so do not rely
-      // on it alone. See ProcessTree.
-      ProcessTree.killTree(process);
-    }
+    // The whole tree: a process group on Unix, taskkill /T on Windows, both
+    // inside ProcessTree. The worker still tears its own pipeline down when it
+    // gets the chance, but that only covers the children it tracks, and it
+    // cannot run at all if it is forced below — so do not rely on it alone.
+    await ProcessTree.killTree(process);
 
     // Wait for the process to actually exit. `exitCode` completes once it has
     // been reaped, so this is a real observation rather than a guess.
@@ -299,7 +292,7 @@ class WorkerManager implements JobRunner {
       // Genuinely wedged. Forcing it here orphans the children — the same
       // failure described above — but by now the alternative is a job that
       // never stops at all, so take the lesser problem and say so.
-      ProcessTree.killTree(process, ProcessSignal.sigkill);
+      await ProcessTree.killTree(process, ProcessSignal.sigkill);
       try {
         await process.exitCode.timeout(_forceKillGrace);
       } on TimeoutException {