Cancel a running join session too (#224)

The FFmpegEngine half landed in ad2a75d; this is the same gap in
ConcatEngine. Between them, a real native session being asked to stop is
now covered on both FFmpeg paths.

The assertion is the session's return code again, and here that is not
merely the better choice but close to the only one: ConcatEngine does not
delete its output on cancellation at all. Its invokeOnCancellation is
FFmpegKit.cancel and nothing else, where FFmpegEngine's also deletes the
partial. Whether that asymmetry is deliberate is a separate question, so
this asserts what is true of both engines rather than depending on it.

The cancel triggers on SessionState.RUNNING rather than on progress.
ConcatWorker publishes no progress at all, so there is no callback to hang
it on even in principle -- and the conversion side already measured the
deeper reason, that the committed clips outrun a callback-triggered cancel.

The inputs are the mismatched pair on purpose, so ConcatPlanner chooses
REENCODE. A stream copy of two 2 s clips is close to instantaneous and
would leave nothing to interrupt; re-encoding is also the case where a user
would actually reach for Cancel.

Verified on a local API 34 emulator: three runs green at 64/0/0/3, and
replacing invokeOnCancellation's body with an empty block fails this test
and nothing else, with state=COMPLETED rc=0.

Media3Engine's transformer.cancel() is still uncovered and #224 stays open
for it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

app/src/androidTest/java/org/libremediaconverter/ffmpeg/FFmpegEngineTest.kt

@@ -4,7 +4,16 @@ import android.media.MediaExtractor
 import android.media.MediaFormat
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.platform.app.InstrumentationRegistry
+import com.arthenica.ffmpegkit.FFmpegKit
+import com.arthenica.ffmpegkit.FFmpegSession
+import com.arthenica.ffmpegkit.ReturnCode
+import com.arthenica.ffmpegkit.SessionState
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.cancelAndJoin
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withTimeout
 import org.junit.After
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
@@ -191,6 +200,84 @@ class FFmpegEngineTest {
         )
     }
 
+    /**
+     * Cancelling a *running* conversion actually stops the native session.
+     *
+     * Nothing on any source set did this before (#224). Every `cancel` in `app/src/androidTest` is
+     * `WorkManager.cancelWorkById` against work that is **queued or already finished** — the two in
+     * `ReattachOnLaunchTest` cancel a job carrying a one-hour initial delay, and one immediately
+     * after enqueue. On the JVM, `WorkerCancellationTest` and `HardwareFallbackTest`'s cancellation
+     * case drive a `SoftwareTranscoder` double that records the call. No test had ever asked a real
+     * native session to stop. This is `docs/defect-audit.md` **D10**'s forcing condition.
+     *
+     * It is the one path where cancelling wrong is silently expensive rather than loudly broken: a
+     * missed `FFmpegKit.cancel` leaves the native process encoding to completion while the UI says
+     * the job is cancelled, and nothing reports the battery and thermal cost.
+     *
+     * ## Why the assertion is the session's return code, not the output file
+     *
+     * The obvious assertion — the partial output is gone — **cannot fail**, so it would have been a
+     * vacuous test. `invokeOnCancellation` deletes the path, and on POSIX unlinking a file ffmpeg
+     * still holds open leaves ffmpeg writing to the unlinked inode; the path stays gone whether or
+     * not the cancel ever reached the session. Deleting `FFmpegKit.cancel` and keeping
+     * `output.delete()` passes that check every time.
+     *
+     * What distinguishes them is the session's own verdict: a cancelled session ends with the
+     * cancel return code, a completed one ends successfully. That is a fact about the session
+     * rather than about timing, so it is read *after* waiting for the session to leave
+     * [SessionState.RUNNING] rather than at a fixed delay.
+     *
+     * ## Why it cancels on RUNNING rather than on the first progress callback
+     *
+     * Measured, and this is the part worth keeping. Cancelling from the first `onProgress` was
+     * tried first and **failed on a local API 34 emulator with `state=COMPLETED rc=0`** — every
+     * committed fixture is 2-3 s at 320x240, and the encode finishes before the first statistics
+     * callback has been delivered and acted on. The progress callback is proof the session is
+     * running, but it arrives too late to interrupt anything.
+     *
+     * `FFmpegKit.listSessions` shows the session as [SessionState.RUNNING] far earlier, so that is
+     * what is waited on. `QualityTier.BEST` is deliberate for the same reason: `-preset medium`
+     * leaves more of the encode ahead of the cancel than `veryfast` would.
+     *
+     * The session is identified by diffing against the ids present before the run, because this
+     * class has already produced eight of them by the time this executes.
+     */
+    @Test
+    fun cancellingARunningConversionCancelsTheNativeSession(): Unit = runBlocking {
+        val before = FFmpegKit.listSessions().map { it.getSessionId() }.toSet()
+        val out = outputFor("out_cancelled.mp4")
+
+        val job = launch(Dispatchers.IO) {
+            engine.run(
+                request = ConversionRequest(spec = OutputFormat.MP4_H265.spec, quality = QualityTier.BEST),
+                inputPath = input.absolutePath,
+                output = out,
+                durationMs = 3_000,
+            )
+        }
+
+        // Interrupt as early as the session can be observed at all. See the KDoc: waiting for
+        // progress instead lost the race outright.
+        val ours = withTimeout(TIMEOUT_MS) {
+            var found: FFmpegSession? = null
+            while (found?.getState() != SessionState.RUNNING) {
+                found = FFmpegKit.listSessions().firstOrNull { it.getSessionId() !in before }
+                if (found?.getState() != SessionState.RUNNING) delay(POLL_MS)
+            }
+            found
+        }
+        job.cancelAndJoin()
+
+        withTimeout(TIMEOUT_MS) {
+            while (ours.getState() == SessionState.RUNNING) delay(POLL_MS)
+        }
+
+        assertTrue(
+            "the native session was not cancelled: state=${ours.getState()} rc=${ours.getReturnCode()}",
+            ReturnCode.isCancel(ours.getReturnCode()),
+        )
+    }
+
     // --- the quality tier the GPL licence was taken for --------------------
 
     @Test
@@ -228,4 +315,10 @@ class FFmpegEngineTest {
         }.exceptionOrNull()
         assertTrue("expected an FFmpegException, got $failure", failure is FFmpegEngine.FFmpegException)
     }
+
+    private companion object {
+        /** Generous: it bounds a hang, and every wait here normally settles in well under a second. */
+        const val TIMEOUT_MS = 30_000L
+        const val POLL_MS = 50L
+    }
 }

app/src/androidTest/java/org/libremediaconverter/join/ConcatEngineTest.kt

@@ -5,7 +5,16 @@ import android.media.MediaFormat
 import android.net.Uri
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.platform.app.InstrumentationRegistry
+import com.arthenica.ffmpegkit.FFmpegKit
+import com.arthenica.ffmpegkit.FFmpegSession
+import com.arthenica.ffmpegkit.ReturnCode
+import com.arthenica.ffmpegkit.SessionState
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.cancelAndJoin
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withTimeout
 import org.junit.After
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
@@ -16,6 +25,7 @@ import org.libremediaconverter.convert.MediaProbe
 import org.libremediaconverter.convert.StagingNames
 import org.libremediaconverter.ffmpeg.ConcatEngine
 import org.libremediaconverter.model.ConcatStrategy
+import org.libremediaconverter.work.ConcatWorker
 import java.io.File
 
 /**
@@ -50,6 +60,65 @@ class ConcatEngineTest {
         (staged + listOf(clipA, clipB, clipMismatched)).forEach { it.delete() }
     }
 
+    /**
+     * Cancelling a *running* join actually stops the native session.
+     *
+     * The `FFmpegEngine` half of #224 landed first (PR #236); this is the same gap in
+     * [ConcatEngine]. Before these two, no test on any source set had ever asked a real native
+     * session to stop — every `cancel` in `app/src/androidTest` targets WorkManager entries that
+     * are queued or already finished.
+     *
+     * ## Two things carried over from the conversion side, both measured there
+     *
+     * **The assertion is the session's return code.** A cancelled session ends with the cancel
+     * code, a completed one does not. The alternative — checking the output file — is even less
+     * available here than it was for conversions: [ConcatEngine] does not delete its output on
+     * cancellation at all. Its `invokeOnCancellation` is `FFmpegKit.cancel(...)` and nothing else,
+     * where [org.libremediaconverter.ffmpeg.FFmpegEngine]'s also deletes the partial. Whether that
+     * asymmetry is deliberate is a separate question from this test, which is why this asserts the
+     * thing that is true of both.
+     *
+     * **The cancel is triggered on [SessionState.RUNNING], not on progress.** `ConcatWorker`
+     * publishes no progress at all, so there is no callback to hang it on even in principle — but
+     * the conversion side established the deeper reason: the committed clips are 2 s at 320x240 and
+     * the encode outruns a callback-triggered cancel.
+     *
+     * The inputs are deliberately the **mismatched** pair, so [ConcatStrategy.REENCODE] is chosen.
+     * A stream copy of two short clips is close to instantaneous and would leave nothing to
+     * interrupt; re-encoding is the case where a user would actually reach for Cancel.
+     *
+     * *Mutation:* drop `FFmpegKit.cancel(session.getSessionId())` from `ConcatEngine`'s
+     * `invokeOnCancellation` — the session runs to completion and this fails.
+     */
+    @Test
+    fun cancellingARunningJoinCancelsTheNativeSession(): Unit = runBlocking {
+        val before = FFmpegKit.listSessions().map { it.getSessionId() }.toSet()
+        val out = output("cancelled_join.mp4")
+
+        val job = launch(Dispatchers.IO) {
+            engine.join(listOf(Uri.fromFile(clipA), Uri.fromFile(clipMismatched)), out, ConcatWorker.DEFAULT_FORMAT)
+        }
+
+        val ours = withTimeout(TIMEOUT_MS) {
+            var found: FFmpegSession? = null
+            while (found?.getState() != SessionState.RUNNING) {
+                found = FFmpegKit.listSessions().firstOrNull { it.getSessionId() !in before }
+                if (found?.getState() != SessionState.RUNNING) delay(POLL_MS)
+            }
+            found
+        }
+        job.cancelAndJoin()
+
+        withTimeout(TIMEOUT_MS) {
+            while (ours.getState() == SessionState.RUNNING) delay(POLL_MS)
+        }
+
+        assertTrue(
+            "the native join session was not cancelled: state=${ours.getState()} rc=${ours.getReturnCode()}",
+            ReturnCode.isCancel(ours.getReturnCode()),
+        )
+    }
+
     private fun copyAsset(name: String): File {
         val out = File(context.cacheDir, name)
         InstrumentationRegistry.getInstrumentation().context.assets
@@ -180,4 +249,10 @@ class ConcatEngineTest {
             a.width != mismatched.width || a.height != mismatched.height,
         )
     }
+
+    private companion object {
+        /** Generous: it bounds a hang, and both waits here normally settle in well under a second. */
+        const val TIMEOUT_MS = 30_000L
+        const val POLL_MS = 50L
+    }
 }

app/src/androidTest/java/org/libremediaconverter/work/NotificationCancelActionTest.kt

@@ -0,0 +1,129 @@
+package org.libremediaconverter.work
+
+import android.net.Uri
+import androidx.media3.common.util.UnstableApi
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.platform.app.InstrumentationRegistry
+import androidx.work.OneTimeWorkRequestBuilder
+import androidx.work.WorkInfo
+import androidx.work.WorkManager
+import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withTimeout
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotNull
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.libremediaconverter.model.OutputFormat
+import org.libremediaconverter.model.QualityTier
+import java.io.File
+import java.util.concurrent.TimeUnit
+
+/**
+ * The Cancel button in the notification shade actually cancels the job.
+ *
+ * `ConversionNotifications.build` attaches one action, wired to
+ * `WorkManager.createCancelPendingIntent(id)`. Before this test `createCancelPendingIntent` had
+ * **no references anywhere outside its own declaration** — no JVM test, no instrumented test
+ * (#227).
+ *
+ * That matters more than an ordinary uncovered line. A conversion runs in a foreground service and
+ * the user is invited to leave the app; once they do, this action is the only way to stop it. If
+ * the `PendingIntent` carries the wrong id, the button does nothing, the notification stays, and
+ * the job runs to completion — with no error, no log, and no screen to look at.
+ *
+ * ## Why this fires the intent rather than reading the shade
+ *
+ * The obvious version asks `NotificationManager.getActiveNotifications()` for id 1001 and taps what
+ * it finds. That was rejected: the instrumented suite grants no runtime permissions, so
+ * `POST_NOTIFICATIONS` is denied throughout, and whether a suppressed foreground-service
+ * notification is returned there is a platform detail that varies — the test would be asserting
+ * something about notification *visibility* rather than about cancellation.
+ *
+ * The `PendingIntent` is the subject; where it is read from is incidental. Building the
+ * notification for a real, live work id and firing its action exercises exactly the thing that can
+ * be wrong — a real `PendingIntent` dispatch reaching real `WorkManager` — and does it the same way
+ * on every API level.
+ *
+ * ## Why the job is delayed rather than running
+ *
+ * A conversion of the committed 3 s fixture finishes in well under a second on an emulator
+ * (`HardwareFallbackTest` completed one in 448 ms), so racing a cancel against a running job would
+ * be flaky in the direction that fails. An initial delay keeps the job reliably `ENQUEUED`, which
+ * is a state `cancelWorkById` acts on identically — what is under test is whether firing the action
+ * reaches WorkManager with the right id, not which state it interrupts.
+ *
+ * *Mutation:* build the `PendingIntent` from `UUID.randomUUID()` instead of the request's id. The
+ * notification looks identical and the job is never cancelled.
+ */
+@UnstableApi
+@RunWith(AndroidJUnit4::class)
+class NotificationCancelActionTest {
+
+    private val context = InstrumentationRegistry.getInstrumentation().targetContext
+    private val workManager = WorkManager.getInstance(context)
+    private lateinit var input: File
+
+    @Before
+    fun setUp() {
+        input = File(context.cacheDir, "cancel_action_sample.mp4")
+        InstrumentationRegistry.getInstrumentation().context.assets
+            .open("sample_h264.mp4")
+            .use { asset -> input.outputStream().use { asset.copyTo(it) } }
+    }
+
+    @After
+    fun tearDown() {
+        input.delete()
+        File(context.cacheDir, "conversions").listFiles()?.forEach { it.delete() }
+    }
+
+    @Test
+    fun theNotificationsCancelActionCancelsThatJob(): Unit = runBlocking {
+        val request = ConversionWorker.request(
+            inputUri = Uri.fromFile(input),
+            displayName = input.name,
+            sizeBytes = input.length(),
+            spec = OutputFormat.MP4_H264.spec,
+            quality = QualityTier.FAST,
+        ).let { base ->
+            // Rebuild with a delay so the job stays ENQUEUED for the whole test. See the KDoc.
+            OneTimeWorkRequestBuilder<ConversionWorker>()
+                .setInputData(base.workSpec.input)
+                .setInitialDelay(1, TimeUnit.HOURS)
+                .build()
+        }
+        workManager.enqueue(request).result.get()
+
+        // The job is queued and waiting, which is the state the cancel has to interrupt.
+        assertEquals(
+            WorkInfo.State.ENQUEUED,
+            withTimeout(TIMEOUT_MS) {
+                workManager.getWorkInfoByIdFlow(request.id).first { it != null }
+            }?.state,
+        )
+
+        val notification = ConversionNotifications(context)
+            .build(request.id, title = input.name, percent = 0, indeterminate = true)
+        val action = notification.actions?.firstOrNull()
+        assertNotNull("the progress notification carries no action to cancel with", action)
+
+        // The whole point: fire it the way the shade would, and see the job stop.
+        action!!.actionIntent.send()
+
+        val terminal = withTimeout(TIMEOUT_MS) {
+            workManager.getWorkInfoByIdFlow(request.id).first { it != null && it.state.isFinished }
+        }
+        assertEquals(
+            "firing the notification's Cancel action must cancel the job it was built for",
+            WorkInfo.State.CANCELLED,
+            terminal?.state,
+        )
+    }
+
+    private companion object {
+        const val TIMEOUT_MS = 30_000L
+    }
+}