fix(cancel): report a cancelled job as cancelled, not as a failure

app/lib/services/worker_manager.dart

@@ -120,20 +120,27 @@ class WorkerManager {
         // down its plumbing here would strand it silently.
         if (generation != _generation) return;
 
-        // The worker sends a `complete` message on both success and failure, so
-        // _pendingCompletion is normally set. If it isn't, the exit still has to
-        // be reported — see [_completionEmitted].
-        _emitCompletion(_pendingCompletion ??
-            CompletionResult(
-              success: false,
-              cancelled: _cancelRequested,
-              errorMessage: _cancelRequested
-                  ? 'Job cancelled by user'
-                  : _lastErrorMessage ??
-                      (exitCode == 0
-                          ? 'Worker exited without reporting a result'
-                          : 'Worker exited with code $exitCode'),
-            ));
+        // A cancelled job MUST report as cancelled, and that has to override
+        // `_pendingCompletion` rather than merely fill in for it.
+        //
+        // The worker sends `complete(false)` on its way out of a cancellation
+        // too (main.rs), and `_handleStdoutLine` builds that into a
+        // CompletionResult with no `cancelled` flag — so it defaults to false.
+        // Since `_pendingCompletion` is therefore always set, putting the flag
+        // in a `??` fallback left it permanently unreachable: cancelling looked
+        // exactly like a failure.
+        //
+        // That is not cosmetic. `_handleQueueItemCompletion` stops the queue on
+        // `cancelled`, but on a plain failure it marks the item failed and calls
+        // `_processNextItem()` — so a cancel silently started the next job and
+        // left `_isQueueProcessing` true, and the UI sat on "processing" with a
+        // spinner and no progress.
+        _emitCompletion(completionFor(
+          cancelRequested: _cancelRequested,
+          pending: _pendingCompletion,
+          lastError: _lastErrorMessage,
+          exitCode: exitCode,
+        ));
 
         _cleanup();
       });
@@ -277,6 +284,51 @@ class WorkerManager {
     ));
   }
 
+  /// What to report when the worker exits.
+  ///
+  /// Pulled out as a pure function because getting it wrong is invisible from
+  /// inside the app: every value is plausible, nothing throws, and the only
+  /// symptom is the queue behaving as though a cancelled job had failed.
+  ///
+  /// A cancelled job MUST report `cancelled`, and that has to override [pending]
+  /// rather than merely fill in for it. The worker sends `complete(false)` on its
+  /// way out of a cancellation too, and `_handleStdoutLine` turns that into a
+  /// CompletionResult with no `cancelled` flag — so it defaults to false. Since
+  /// [pending] is therefore always set, an earlier attempt at this put the flag
+  /// in a `??` fallback, where it was permanently unreachable and cancelling
+  /// still looked exactly like a failure.
+  ///
+  /// The consequence is in `_handleQueueItemCompletion`: it stops the queue on
+  /// `cancelled`, but on a plain failure it marks the item failed and calls
+  /// `_processNextItem()`. So a cancel silently started the next job, left
+  /// `_isQueueProcessing` true, and the UI sat on "processing" with a spinner
+  /// and no progress.
+  // Public so it can be unit-tested directly; not part of the intended API.
+  static CompletionResult completionFor({
+    required bool cancelRequested,
+    required CompletionResult? pending,
+    required String? lastError,
+    required int exitCode,
+  }) {
+    if (cancelRequested) {
+      return const CompletionResult(
+        success: false,
+        cancelled: true,
+        errorMessage: 'Job cancelled by user',
+      );
+    }
+    // The worker reports `complete` on success and failure alike, so `pending`
+    // is normally set. If it isn't, the exit still has to be reported.
+    return pending ??
+        CompletionResult(
+          success: false,
+          errorMessage: lastError ??
+              (exitCode == 0
+                  ? 'Worker exited without reporting a result'
+                  : 'Worker exited with code $exitCode'),
+        );
+  }
+
   /// Emit [result] unless this job has already reported one.
   void _emitCompletion(CompletionResult result) {
     if (_completionEmitted || _completionController.isClosed) return;

app/test/worker_completion_result_test.dart

@@ -0,0 +1,100 @@
+// What the worker's exit reports, and why a cancellation must win.
+//
+// The bug these pin was invisible from inside the app: every value was
+// plausible, nothing threw, and the only symptom was the queue behaving as
+// though a cancelled job had failed — marking the item failed, starting the next
+// job, and leaving the UI on "processing" with a spinner and no progress.
+//
+// Two earlier attempts missed it. The first reasoned about process teardown and
+// never touched the reporting. The second put `cancelled: _cancelRequested` in a
+// `??` fallback behind `_pendingCompletion` — which the worker always sets,
+// because it sends `complete(false)` on its way out of a cancellation too. So
+// the flag was unreachable and the behaviour was unchanged.
+//
+// Hence a direct test of the decision rather than a scan of the source: the
+// previous tests read the file, passed, and the bug shipped anyway.
+
+import 'package:test/test.dart';
+import 'package:vapourbox/services/worker_manager.dart';
+
+void main() {
+  group('completionFor', () {
+    test('a cancelled job reports cancelled even though the worker sent complete',
+        () {
+      // Exactly the shipped case: SIGTERM, the worker emits complete(false) on
+      // its way out, and _handleStdoutLine builds it with no cancelled flag.
+      const pending = CompletionResult(success: false, errorMessage: 'whatever');
+
+      final result = WorkerManager.completionFor(
+        cancelRequested: true,
+        pending: pending,
+        lastError: null,
+        exitCode: 130,
+      );
+
+      expect(result.cancelled, isTrue,
+          reason: 'the queue stops only on cancelled; reported as a plain '
+              'failure it marks the item failed and starts the next job');
+      expect(result.success, isFalse);
+    });
+
+    test('a cancelled job reports cancelled when the worker sent nothing', () {
+      final result = WorkerManager.completionFor(
+        cancelRequested: true,
+        pending: null,
+        lastError: null,
+        exitCode: 143,
+      );
+      expect(result.cancelled, isTrue);
+    });
+
+    test('a successful job is reported from the worker, untouched', () {
+      const pending = CompletionResult(success: true, outputPath: '/tmp/out.mkv');
+      final result = WorkerManager.completionFor(
+        cancelRequested: false,
+        pending: pending,
+        lastError: null,
+        exitCode: 0,
+      );
+      expect(result.success, isTrue);
+      expect(result.cancelled, isFalse);
+      expect(result.outputPath, '/tmp/out.mkv');
+    });
+
+    test('a genuine failure stays a failure, not a cancellation', () {
+      const pending = CompletionResult(success: false, errorMessage: 'ffmpeg died');
+      final result = WorkerManager.completionFor(
+        cancelRequested: false,
+        pending: pending,
+        lastError: 'ffmpeg died',
+        exitCode: 1,
+      );
+      expect(result.cancelled, isFalse,
+          reason: 'a crash must not be swallowed as a cancellation — the queue '
+              'would stop silently instead of recording the failure');
+      expect(result.errorMessage, 'ffmpeg died');
+    });
+
+    test('an exit with no report at all is still reported', () {
+      // The hang from #50: without this the UI waits forever on a dead worker.
+      final result = WorkerManager.completionFor(
+        cancelRequested: false,
+        pending: null,
+        lastError: null,
+        exitCode: 3,
+      );
+      expect(result.success, isFalse);
+      expect(result.errorMessage, contains('3'));
+    });
+
+    test('a clean exit with no report is distinguished from a crash', () {
+      final result = WorkerManager.completionFor(
+        cancelRequested: false,
+        pending: null,
+        lastError: null,
+        exitCode: 0,
+      );
+      expect(result.errorMessage, contains('without reporting'));
+    });
+  });
+}

app/test/worker_manager_generation_test.dart

@@ -72,17 +72,18 @@ void main() {
           reason: 'the guard must precede the _cleanup() call');
     });
 
-    test('a cancelled job is reported as cancelled, not as an exit code', () {
-      // The exit handler is registered on exitCode before cancel() awaits it, so
-      // it reports first. Without _cancelRequested it describes a deliberate
-      // cancellation as "Worker exited with code 143", and the UI cannot tell a
-      // cancellation from a crash.
+    test('the exit handler delegates the decision, passing the cancel flag', () {
+      // What gets reported is decided by WorkerManager.completionFor, which is
+      // unit-tested directly in worker_completion_result_test.dart. All that
+      // matters here is that the handler actually routes through it and hands
+      // over _cancelRequested — an earlier version computed the result inline
+      // and put the cancelled flag in an unreachable branch.
       final start = source.indexOf('_process!.exitCode.then(');
       final body = source.substring(start, source.indexOf('} catch (e) {', start));
-      expect(body, contains('cancelled: _cancelRequested'),
-          reason: 'the exit handler must mark a cancelled job as cancelled');
-      expect(body, contains("_cancelRequested\n"),
-          reason: 'and use it for the message too');
+      expect(body, contains('completionFor('),
+          reason: 'the exit handler must use the tested decision, not its own');
+      expect(body, contains('cancelRequested: _cancelRequested'),
+          reason: 'a cancelled job is only distinguishable if the flag is passed');
     });
 
     test('startJob resets both flags so state cannot leak between jobs', () {