Merge pull request #189 from JMR-dev/docs/coverage-wave3

Re-measure after wave 3, and write down the two kinds of gap it had to separate

CLAUDE.md

@@ -130,8 +130,9 @@ install for code that can never run — and on API 37 the full APK does not fit
 - The `model` package is excluded from `ReturnCount` and `CyclomaticComplexMethod` only. It is the
   decision layer, where one branch is one documented user-visible outcome and the metric counts
   answers rather than complexity. Every other rule still applies there.
-- **Coverage is reported, not gated** — **84.9% of lines (1971/2321), 63.8% of branches**,
-  measured 2026-08-26 with `./gradlew :app:jacocoTestReport`, against 454 JVM tests in 67 classes.
+- **Coverage is reported, not gated** — **92.8% of lines (2183/2352), 81.3% of branches
+  (1091/1342)**, measured 2026-09-02 with `./gradlew :app:jacocoTestReport`, against 584 JVM tests
+  in 87 classes.
 
   **Every figure this file carried before 2026-08-24 was an artifact, roughly half the real one.**
   Robolectric loads classes through its own sandbox classloader with no source location, JaCoCo
@@ -147,12 +148,94 @@ install for code that can never run — and on API 37 the full APK does not fit
   disproportionately Robolectric, so each one added denominator and no numerator — the measurement
   was punishing exactly the tests that were hardest to write.
 
-  Two things still hold. A floor needs a baseline that has settled, and this one has not: it moved
-  39 points in a single build change on 2026-08-24, then another 16 as the #52 test push and the
+  Two things still hold. A floor needs a baseline that has settled, and this one has not. It moved
+  39 points in a single build change on 2026-08-24; then another 16 as the #52 test push and the
   fixes it turned up landed — 69.2% -> 84.9% line, 53.2% -> 63.8% branch — while the denominator
-  grew 2194 -> 2321, because that work added production code of its own. And **re-measure before
-  quoting**: this entry was written quoting 81.4%, measured four hours earlier, and was already
-  three points stale by the time it was ready to merge.
+  grew 2194 -> 2321, because that work added production code of its own; then again on 2026-08-27
+  as #132 and #133's ten children landed — 84.9% -> 87.1% line, 63.8% -> **69.1%** branch, 454 ->
+  502 tests. **Branch moved four times as far as line that last time**, and that is the shape to
+  expect from this kind of work rather than a surprise: those children targeted decision code —
+  enum fallbacks, refusal arms, cursor shapes, a `when` over container rules — where one test
+  chooses a branch the suite had never taken. Line coverage barely notices; branch coverage is the
+  whole point.
+
+  Then #153's five children on 2026-08-29 — 87.1% -> 88.9% line, 69.1% -> **75.4%** branch, 502 ->
+  546 tests.
+
+  **That last branch figure moved for two reasons, and only one of them is new tests.** The
+  numerator rose 974 -> 1011; the denominator *fell* 1410 -> 1340. Both are the seam work. Pulling
+  a `when` out of a lambda inside a `collect` deletes the coroutine state machine's synthesized
+  branches around it, and what is left is a plain function whose branches a test can choose:
+  `ConversionViewModel$observe$1$1` went from carrying the whole mapping to 6 branches, while the
+  extracted `ConversionViewModelKt` covers 41 of 42 and `JoinViewModelKt` 38 of 39. So a seam is
+  worth more than the tests it enables — it also stops the measurement counting scaffolding.
+
+  Be careful quoting a branch move on its own for that reason. A percentage that rises because the
+  denominator shrank is not the same claim as one that rises because more branches are tested, and
+  this entry has a history of explaining its own numbers wrongly.
+
+  Then wave 3 (#167-#178) on 2026-09-02 — 88.9% -> 92.8% line, 75.4% -> **81.3%** branch, 546 ->
+  584 tests, in ten PRs from #179 to #188.
+
+  **Its shape is different from the two before it, and the difference is the thing to carry
+  forward.** Waves 1 and 2 were finding uncovered code. By wave 3 there was not much of that left,
+  so the gaps were sorted into two kinds before any test was written:
+
+  - **coverage gaps** — the line never executes. Filtered to sites where JaCoCo reports `mi > 0`, a
+    concrete instruction no test runs, which is what separates a real gap from a partial branch on
+    a compound condition. That filter cut the candidate list roughly in half and was right to.
+  - **assertion gaps** — JaCoCo is green and nothing checks the answer. `MainActivity`'s rail and
+    bottom bar were both *executed* by `AppRootRestorationTest` and **transposing them passed the
+    entire suite**; so did swapping the two progress-notification strings, and swapping `Content`'s
+    two destinations. No coverage number would ever have found any of the three.
+
+  So **every ticket named the mutation that had to go red, and that was its acceptance criterion
+  rather than a coverage delta**. It caught **two vacuous tests written in the same session**,
+  before either shipped:
+
+  - a `firstContainerHolding` test asserting a refusal still offered *something*. True, and
+    useless: the source container is a candidate in its own right, so the list stays non-empty
+    whatever the fallback does. What it actually buys is the codec the user asked for.
+  - a staged-delete test scanning for a `"join-"` prefix `StagingNames.forJob` does not produce —
+    it names files `<jobId>.<ext>`, so the assertion was true of everything.
+
+  It also corrected a *third* test that was not vacuous: `probeForConcat`'s KDoc claimed to drive
+  the `catch` arm, and rethrowing from that catch left it green. That is how the arm turned out to
+  be unreachable — see the next paragraph. A passing test with a wrong explanation is its own
+  failure mode.
+
+  **A green mutation is only evidence when the mutation is a real change**, which is the mirror
+  trap: one `classify` mutation stayed green because reordering two arms was semantically
+  equivalent for every reachable input. A bad mutation and a weak test look identical in the output.
+
+  Three things came back **not as the ticket described them**, which is a result rather than a
+  shortfall:
+
+  - `ContainerCapabilities:282`'s `exclude` filter **cannot drop anything**. `repair` always
+    changes a codec on the shared container — a codec it left alone is one `validate` would not
+    have refused — and the one non-default `exclude` carries `COPY` while every candidate carries
+    `NONE`. F4-shaped.
+  - `probeForConcat`'s catch arm is **unreachable on this runtime**. Robolectric's `MediaExtractor`
+    never throws from `setDataSource`, measured across an unregistered `content://` authority, a
+    missing `file://`, a file of garbage bytes and an `http://` URL — all four returned with
+    `trackCount = 0`. It stays device-only.
+  - `JobSnapshots:31`'s missed arm was **not** the `!isFile` one the ticket named — that is already
+    covered by the `reclaimed` fixture. It was `path == null`: a job carrying no output path at
+    all. Read the report, not the ticket, when the two disagree.
+
+  One item was **included against** the F4 rule rather than exempted by it, and the distinction is
+  worth having written down since both live in the same function: `ContainerCapabilities:94`
+  (`accepts(container, VideoCodec.NONE, mode)`) is dead in production today — every caller guards
+  `NONE` first — and was tested anyway, because its audio twin at `:101` has had a test since #136
+  and the asymmetry was the argument. The `COPY -> error(...)` arms beside it stay exempt, because
+  a second line of defence that can be provoked is not one.
+
+  Denominators moved here too, in both directions and for two different reasons: 1340 -> 1342
+  branches from `MediaProbe.merge`, 2348 -> 2352 lines from the `ConcatJoiner` interface. Neither
+  is new untested code.
+
+  And **re-measure before quoting**: this entry was once written quoting 81.4%, measured four hours
+  earlier, and was already three points stale by the time it was ready to merge.
 - **Testable code is not done until it is tested.** If a piece is unit testable, it gets unit
   tests before it counts as done. If it is e2e testable, it gets e2e tests. Both clauses apply —
   a change that is both needs both.
@@ -174,6 +257,32 @@ install for code that can never run — and on API 37 the full APK does not fit
 
 - `kotlin.code.style=official`. Gradle stays Kotlin DSL.
 
+- **A stacked PR does not merge with `gh pr merge`, and `MERGED` is not proof it reached `main`.**
+  Two separate traps, both measured on 2026-08-27 while landing #144-#151.
+
+  `gh pr merge` uses the GraphQL mutation, which refuses a stacked PR outright: *"This pull request
+  is part of a stack and must be merged using the asynchronous merge REST API."* So does
+  `PUT .../pulls/{n}/merge`. The one that works is
+  `gh api -X PUT repos/OWNER/REPO/pulls/N/merge-async -f merge_method=merge`, which returns a uuid
+  to poll at `.../merge-async/{uuid}` until `status` is `merged` or `failed`.
+
+  **The second trap is worse, because nothing looks wrong.** GitHub retargets a stacked PR's base
+  to `main` when the PR below it merges, but *asynchronously*. Merge a stack faster than that
+  settles — five PRs about thirty seconds apart, in the case that found this — and each one merges
+  into its own base branch, which has itself already been merged and left behind. Every call
+  returns `status: merged` and every one is true. `gh pr list --state open` comes back empty, every
+  PR shows `MERGED`, and **none of the content is on `main`**.
+
+  What caught it was a coverage re-measure reading two points lower than the same tree had measured
+  an hour earlier; a fresh `git pull` changed nothing, which is what turned it into a question.
+  `git merge-base --is-ancestor <merge-sha> origin/main` answers it in one line. Do that after
+  merging a stack, or merge one at a time and re-read `baseRefName` between. #160 is what the
+  recovery cost.
+
+  The auto-retarget belongs to the stacking feature specifically. A PR opened with a plain
+  `gh pr create --base some-branch` does **not** retarget when that branch merges — it is left
+  pointing at a dead base and has to be moved by hand.
+
 - **File one-off issues with `tools/github/file-issue.sh`, not `gh issue create`.** `gh issue
   create` does not touch the project board, so the issue exists, carries its labels, and is
   invisible in the Kanban — indistinguishable from never having been filed. Measured 2026-08-24:

app/src/main/java/org/libremediaconverter/MainActivity.kt

@@ -24,9 +24,11 @@ import androidx.compose.runtime.saveable.Saver
 import androidx.compose.runtime.saveable.rememberSaveable
 import androidx.compose.runtime.setValue
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.platform.testTag
 import androidx.media3.common.util.UnstableApi
 import org.libremediaconverter.convert.ConverterScreen
 import org.libremediaconverter.join.JoinScreen
+import org.libremediaconverter.ui.TestTags
 import org.libremediaconverter.ui.theme.LibreMediaConverterTheme
 
 /**
@@ -114,7 +116,7 @@ internal fun AppRoot(
 
     if (useRail) {
         Row(modifier = Modifier.fillMaxSize()) {
-            NavigationRail {
+            NavigationRail(modifier = Modifier.testTag(TestTags.Shell.NAVIGATION_RAIL)) {
                 Destination.entries.forEach { item ->
                     NavigationRailItem(
                         selected = destination == item,
@@ -132,7 +134,7 @@ internal fun AppRoot(
         Scaffold(
             modifier = Modifier.fillMaxSize(),
             bottomBar = {
-                NavigationBar {
+                NavigationBar(modifier = Modifier.testTag(TestTags.Shell.NAVIGATION_BAR)) {
                     Destination.entries.forEach { item ->
                         NavigationBarItem(
                             selected = destination == item,

app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt

@@ -6,6 +6,7 @@ import android.util.Log
 import androidx.lifecycle.AndroidViewModel
 import androidx.lifecycle.viewModelScope
 import androidx.media3.common.util.UnstableApi
+import androidx.work.Data
 import androidx.work.WorkInfo
 import androidx.work.WorkManager
 import kotlinx.coroutines.CoroutineDispatcher
@@ -71,6 +72,119 @@ data class InputFile(
     val probe: InputProbe? = null,
 )
 
+/**
+ * One update about a running conversion, as WorkManager last reported it.
+ *
+ * Only the fields [conversionStateFrom] reads — the same shape, and for the same reason, as
+ * `JobSnapshot` beside `Reattachment.choose`: the rule stays testable on the JVM because nothing
+ * in it needs a `WorkInfo`, which a test cannot readily build.
+ *
+ * [outputData] stays a `Data` rather than being unpacked into five nullable strings. It is what a
+ * test already builds with `workDataOf` everywhere in this suite, so unpacking would move the same
+ * reads without making anything easier to drive.
+ */
+internal data class ConversionUpdate(
+    val state: WorkInfo.State,
+    val progressPercent: Int,
+    val runAttemptCount: Int,
+    val outputData: Data,
+)
+
+/**
+ * What the screen should show, given what WorkManager last said about the job.
+ *
+ * ## Why this is a function rather than the body of a `collect`
+ *
+ * It was the body of one. `workManager` is built in the constructor from `WorkManager.getInstance`,
+ * `observe` is private, and nothing could hand either a chosen `WorkInfo` — so every arm below ran
+ * only when a real worker happened to produce it. A real worker produces a terminal state with
+ * well-formed output, which meant six of these arms had never been chosen by any test: the progress
+ * read, both sides of the retry check, a success with no file, a failure with nothing to say, and
+ * the two that map to a state the user cannot otherwise reach.
+ *
+ * That is the argument #141 made for `MediaProbe`'s track walk, against `WorkManager` instead of a
+ * media fixture, and it takes the same answer: the branch matrix is a pure function, and what is
+ * left needing the framework — the flow, the null check, the ownership check — is the thin edge.
+ *
+ * ## What is deliberately *not* in here
+ *
+ * The ownership check stays at the call site. Its comment is explicit that it guards the file
+ * ownership the `SUCCEEDED` arm takes, not merely the assignment, so moving it inside would change
+ * what it protects. And this function takes no responsibility for the staged file: it returns the
+ * state, and the caller reads the file off it. A pure function that deletes files is not a seam.
+ *
+ * @param cancelled where a cancellation lands, which differs for a reattached job — see [observe].
+ * @param fallbackSpec the current settings, read only when finished work predates the worker
+ *   reporting its own name and MIME type.
+ */
+@UnstableApi
+internal fun conversionStateFrom(
+    update: ConversionUpdate,
+    input: InputFile,
+    cancelled: ConversionState,
+    fallbackSpec: OutputSpec,
+): ConversionState = when (update.state) {
+    WorkInfo.State.RUNNING -> ConversionState.Converting(input, update.progressPercent)
+
+    // ENQUEUED after a run means a retry is pending. Either the six-hour foreground budget ran out
+    // mid-job, or the system refused to let the job start again while the app was in the background
+    // — the second being the likelier of the two, since it needs only a process restart. Nothing
+    // here can tell them apart, and nothing needs to: the answer is the same.
+    WorkInfo.State.ENQUEUED ->
+        if (update.runAttemptCount > 0) {
+            ConversionState.Waiting(input)
+        } else {
+            ConversionState.Converting(input, 0)
+        }
+
+    WorkInfo.State.SUCCEEDED -> convertedFrom(update.outputData, input, fallbackSpec)
+
+    // A worker that dies before it can report anything leaves no output data at all — a
+    // foreground-service start refused after a process restart is one way — and an exception's
+    // message can be an empty string. Both would read as a failure with nothing said, so blank
+    // falls back like missing does.
+    WorkInfo.State.FAILED -> ConversionState.Failed(
+        update.outputData.getString(ConversionWorker.KEY_ERROR)
+            ?.takeIf { it.isNotBlank() }
+            ?: ConversionWorker.GENERIC_FAILURE_MESSAGE,
+    )
+
+    WorkInfo.State.CANCELLED -> cancelled
+    WorkInfo.State.BLOCKED -> ConversionState.Converting(input, 0)
+}
+
+/**
+ * The `SUCCEEDED` arm, which is the only one that reads more than one field.
+ *
+ * Split out so [conversionStateFrom] stays a table of one line per state. A success with no output
+ * path is a failure: the job said it finished and named nothing, and there is no file to offer.
+ */
+@UnstableApi
+private fun convertedFrom(outputData: Data, input: InputFile, fallbackSpec: OutputSpec): ConversionState {
+    val path = outputData.getString(ConversionWorker.KEY_OUTPUT_PATH)
+        ?: return ConversionState.Failed(SUCCEEDED_WITHOUT_A_FILE_MESSAGE)
+    return ConversionState.Converted(
+        input = input,
+        staged = File(path),
+        engineUsed = outputData.getString(ConversionWorker.KEY_ENGINE_USED).orEmpty(),
+        routeReason = outputData.getString(ConversionWorker.KEY_ROUTE_REASON).orEmpty(),
+        // Work enqueued before the worker reported this carries nothing, and WorkManager keeps
+        // finished work for about a week -- so this branch is ordinary for a few days rather than a
+        // corner. It is the old derivation, kept because it is the same guess the app already made
+        // and there is genuinely nothing better available for such a job. New work never reaches it.
+        suggestedName = outputData.getString(ConversionWorker.KEY_SUGGESTED_NAME)
+            ?.takeIf { it.isNotBlank() }
+            ?: ConversionWorker.outputNameFor(input.displayName, fallbackSpec),
+        mimeType = outputData.getString(ConversionWorker.KEY_MIME_TYPE)
+            ?.takeIf { it.isNotBlank() }
+            ?: fallbackSpec.mimeType,
+    )
+}
+
+/** A job that reported success and named no file. There is nothing to offer the user to save. */
+internal const val SUCCEEDED_WITHOUT_A_FILE_MESSAGE: String =
+    "Conversion reported success but produced no file."
+
 sealed interface ConversionState {
     data object Idle : ConversionState
     data class Ready(val input: InputFile) : ConversionState
@@ -442,75 +556,24 @@ class ConversionViewModel @JvmOverloads constructor(
                 // that either. The state and `pendingStaged` are meant to refer to the same file
                 // or to no file, and this is where that stays true.
                 if (!ownership.stillHeldBy(token)) return@collect
-                _state.value = when (info.state) {
-                    WorkInfo.State.RUNNING -> ConversionState.Converting(
-                        input,
-                        info.progress.getInt(ConversionWorker.KEY_PROGRESS, 0),
-                    )
-
-                    // ENQUEUED after a run means a retry is pending. Either the six-hour
-                    // foreground budget ran out mid-job, or the system refused to let the job
-                    // start again while the app was in the background — the second being the
-                    // likelier of the two, since it needs only a process restart. Nothing here
-                    // can tell them apart, and nothing needs to: the answer is the same.
-                    WorkInfo.State.ENQUEUED ->
-                        if (info.runAttemptCount > 0) {
-                            ConversionState.Waiting(input)
-                        } else {
-                            ConversionState.Converting(input, 0)
-                        }
-
-                    WorkInfo.State.SUCCEEDED -> {
-                        val path = info.outputData.getString(ConversionWorker.KEY_OUTPUT_PATH)
-                        if (path == null) {
-                            ConversionState.Failed("Conversion reported success but produced no file.")
-                        } else {
-                            val staged = File(path)
-                            // Take responsibility for the file at the same moment the state
-                            // starts referring to it, so the two cannot disagree.
-                            pendingStaged = staged
-                            ConversionState.Converted(
-                                input = input,
-                                staged = staged,
-                                engineUsed = info.outputData
-                                    .getString(ConversionWorker.KEY_ENGINE_USED).orEmpty(),
-                                routeReason = info.outputData
-                                    .getString(ConversionWorker.KEY_ROUTE_REASON).orEmpty(),
-                                suggestedName = info.outputData
-                                    .getString(ConversionWorker.KEY_SUGGESTED_NAME)
-                                    ?.takeIf { it.isNotBlank() }
-                                    // Work enqueued before the worker reported this carries
-                                    // nothing, and WorkManager keeps finished work for about a
-                                    // week -- so this branch is ordinary for a few days rather
-                                    // than a corner. It is the old derivation, kept because it is
-                                    // the same guess the app already made and there is genuinely
-                                    // nothing better available for such a job. New work never
-                                    // reaches it.
-                                    ?: ConversionWorker.outputNameFor(
-                                        input.displayName,
-                                        _settings.value.spec,
-                                    ),
-                                mimeType = info.outputData
-                                    .getString(ConversionWorker.KEY_MIME_TYPE)
-                                    ?.takeIf { it.isNotBlank() }
-                                    ?: _settings.value.spec.mimeType,
-                            )
-                        }
-                    }
-
-                    // A worker that dies before it can report anything leaves no output data at
-                    // all — a foreground-service start refused after a process restart is one
-                    // way — and an exception's message can be an empty string. Both would read
-                    // as a failure with nothing said, so blank falls back like missing does.
-                    WorkInfo.State.FAILED -> ConversionState.Failed(
-                        info.outputData.getString(ConversionWorker.KEY_ERROR)
-                            ?.takeIf { it.isNotBlank() }
-                            ?: "Conversion failed.",
-                    )
-
-                    WorkInfo.State.CANCELLED -> cancelled
-                    WorkInfo.State.BLOCKED -> ConversionState.Converting(input, 0)
-                }
+                val next = conversionStateFrom(
+                    ConversionUpdate(
+                        state = info.state,
+                        progressPercent = info.progress.getInt(ConversionWorker.KEY_PROGRESS, 0),
+                        runAttemptCount = info.runAttemptCount,
+                        outputData = info.outputData,
+                    ),
+                    input = input,
+                    cancelled = cancelled,
+                    fallbackSpec = _settings.value.spec,
+                )
+                // Take responsibility for the file at the same moment the state starts referring
+                // to it, so the two cannot disagree. Read off the result rather than assigned
+                // inside the mapping: `Converted` is the only state that carries a staged file, so
+                // "the state and `pendingStaged` refer to the same file or to no file" is now the
+                // shape of the code rather than a rule two branches have to keep.
+                if (next is ConversionState.Converted) pendingStaged = next.staged
+                _state.value = next
             }
         }
     }
@@ -565,7 +628,7 @@ class ConversionViewModel @JvmOverloads constructor(
                 // than a fresh handle for the second failure's sake: a retry that fails again
                 // lands back here still carrying the file, not on a bare Failed that would take
                 // the offer away.
-                _state.value = ConversionState.Failed(e.message ?: "Could not save the file.", pending)
+                _state.value = ConversionState.Failed(e.message ?: SAVE_FAILED_MESSAGE, pending)
             }
         }
     }

app/src/main/java/org/libremediaconverter/convert/ConverterScreen.kt

@@ -94,24 +94,61 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
         state = state,
         settings = settings,
         validation = validation,
-        actions = ConverterActions(
+        actions = converterActions(
+            viewModel = viewModel,
             onPickInput = { pickInput.launch(arrayOf("*/*")) },
-            onPreset = viewModel::setPreset,
-            onContainer = viewModel::setContainer,
-            onVideoCodec = viewModel::setVideoCodec,
-            onAudioCodec = viewModel::setAudioCodec,
-            onSuggestion = viewModel::applySuggestion,
-            onQuality = viewModel::setQuality,
-            onEnginePreference = viewModel::setEnginePreference,
             onConvert = { requestNotifications.launch(Manifest.permission.POST_NOTIFICATIONS) },
-            onCancel = viewModel::cancel,
             onSave = { suggestedName -> chooseDestination.launch(suggestedName) },
-            onReset = viewModel::reset,
         ),
         modifier = modifier,
     )
 }
 
+/**
+ * Which of the ViewModel's methods each affordance on the screen calls.
+ *
+ * ## Why this is a function rather than an argument list
+ *
+ * It was an argument list, inside [ConverterScreen], which no test reached: `ConverterScreenContent`
+ * builds its own [ConverterActions], so every test in the suite drove the stateless inner and none
+ * of them ever saw the wiring.
+ *
+ * Most of the list is safe without a test, and saying so is more useful than pretending otherwise:
+ * `onContainer`, `onVideoCodec`, `onAudioCodec`, `onPreset`, `onSuggestion`, `onQuality` and
+ * `onEnginePreference` each take a distinct type, so binding one to another's setter does not
+ * compile. Verified rather than assumed — swapping `onVideoCodec` and `onAudioCodec` fails with
+ * *"Inapplicable candidate(s): fun setAudioCodec(codec: AudioCodec)"*.
+ *
+ * **[ConverterActions.onCancel] and [ConverterActions.onReset] are the exception.** Both are
+ * `() -> Unit`, so swapping them compiles silently — also verified — and ships a Cancel button that
+ * throws the conversion away and a Start-over button that leaves it on screen. That pair is what
+ * `ConverterWiringTest` exists for.
+ *
+ * The three launcher-backed actions stay parameters: they need an `ActivityResultLauncher`, which
+ * is the part that genuinely needs the composition, and keeping them out means the rest can be
+ * checked without one.
+ */
+@UnstableApi
+internal fun converterActions(
+    viewModel: ConversionViewModel,
+    onPickInput: () -> Unit,
+    onConvert: () -> Unit,
+    onSave: (suggestedName: String) -> Unit,
+): ConverterActions = ConverterActions(
+    onPickInput = onPickInput,
+    onPreset = viewModel::setPreset,
+    onContainer = viewModel::setContainer,
+    onVideoCodec = viewModel::setVideoCodec,
+    onAudioCodec = viewModel::setAudioCodec,
+    onSuggestion = viewModel::applySuggestion,
+    onQuality = viewModel::setQuality,
+    onEnginePreference = viewModel::setEnginePreference,
+    onConvert = onConvert,
+    onCancel = viewModel::cancel,
+    onSave = onSave,
+    onReset = viewModel::reset,
+)
+
 /**
  * Everything [ConverterScreenContent] can ask for, in one value.
  *

app/src/main/java/org/libremediaconverter/convert/MediaProbe.kt

@@ -50,19 +50,42 @@ object MediaProbe {
     )
 
     fun probe(context: Context, uri: Uri): InputProbe {
-        val extracted = probeWithExtractor(context, uri)
-        val info = probeWithFFprobe(context, uri)
+        val merged = merge(probeWithExtractor(context, uri), probeWithFFprobe(context, uri))
+        if (merged.kind == InputKind.UNPARSEABLE) {
+            // Not a failure: an unparseable input is a strong signal that this job belongs on
+            // FFmpeg. Reporting an unknown codec makes the router say so.
+            Log.i(TAG, "Neither MediaExtractor nor FFprobe could read $uri; routing to FFmpeg.")
+        }
+        return merged
+    }
 
+    /**
+     * What the two probes together say about one input.
+     *
+     * A pure function, and `internal` for the same reason [extractedFrom] is: the precedence rules
+     * below are the answer to "which probe wins", and until this was pulled out of [probe] the only
+     * way to ask was to have a real `MediaExtractor` and a real FFprobe **disagree**, which nothing
+     * on any source set can arrange. `RemuxTest` drives this on a device against committed
+     * fixtures, but only ever with one probe answering and the other agreeing or also failing --
+     * so every elvis here was taken in one direction and never the other.
+     *
+     * The rules, each of which is a decision rather than an accident:
+     *
+     * - **The extractor wins on codecs.** It is the platform's own view of what it can decode,
+     *   which is the thing the router is about to ask about. FFprobe's name for the same track can
+     *   differ, and the copy planner keys off these strings.
+     * - **FFprobe alone reports the container.** `MediaExtractor` cannot, which is why [InputProbe]
+     *   carries a nullable one and `CopyPlanner` treats null as "container unknown".
+     * - **Duration is the larger of the two**, not the first non-zero. Either probe can report zero
+     *   for a file the other times correctly, and a zero duration makes the FFmpeg progress
+     *   percentage undefined.
+     */
+    internal fun merge(extracted: Extracted?, info: FFprobeInfo?): InputProbe {
         val videoCodec = extracted?.videoCodec ?: info?.videoCodec
         val audioCodec = extracted?.audioCodec ?: info?.audioCodec
         val kind = classify(extracted, info)
 
-        if (kind == InputKind.UNPARSEABLE) {
-            // Not a failure: an unparseable input is a strong signal that this job belongs on
-            // FFmpeg. Reporting an unknown codec makes the router say so.
-            Log.i(TAG, "Neither MediaExtractor nor FFprobe could read $uri; routing to FFmpeg.")
-            return UNREADABLE
-        }
+        if (kind == InputKind.UNPARSEABLE) return UNREADABLE
 
         return InputProbe(
             videoCodec = videoCodec,
@@ -83,7 +106,7 @@ object MediaProbe {
      * audio file and a corrupt file indistinguishable. The source-info card cannot describe either
      * honestly until they are separate, and neither can the copy planner.
      */
-    private fun classify(extracted: Extracted?, info: FFprobeInfo?): InputKind = when {
+    internal fun classify(extracted: Extracted?, info: FFprobeInfo?): InputKind = when {
         info?.isImage == true -> InputKind.IMAGE
         extracted == null && info == null -> InputKind.UNPARSEABLE
         (extracted?.videoCodec ?: info?.videoCodec) != null -> InputKind.VIDEO
@@ -92,7 +115,12 @@ object MediaProbe {
         else -> InputKind.UNPARSEABLE
     }
 
-    private class Extracted(
+    /**
+     * `internal` rather than `private` so [extractedFrom] can be named from a test. The JVM test
+     * source set is a friend of `main`, so this stays invisible outside the module — the precedent
+     * is `MainActivity`'s `Destination`, and [containerFrom] beside it.
+     */
+    internal class Extracted(
         val videoCodec: String?,
         val audioCodec: String?,
         val durationMs: Long,
@@ -100,33 +128,55 @@ object MediaProbe {
         val height: Int,
     )
 
+    /**
+     * What a set of track formats says about a file.
+     *
+     * Split out of [probeWithExtractor] so the rules below can be tested against tracks a test
+     * *chooses*, rather than against whatever the committed fixtures happen to contain. The device
+     * tests exercise this through real files; none of them can construct a two-video-track input,
+     * a track that omits its duration, or an audio-before-video ordering on purpose.
+     *
+     * Three rules live here, and each is a decision rather than plumbing:
+     *
+     * - **First track of a type wins.** `video == null` is the whole guard. A file with two video
+     *   tracks must report the first, because that is the one an engine will transcode.
+     * - **Duration is the maximum across tracks**, not the first one found or the last. A file
+     *   whose audio outlasts its video is ordinary, and reporting the video's length would cut the
+     *   progress bar short.
+     * - **A track that omits `KEY_DURATION` contributes nothing** rather than zero. `MediaExtractor`
+     *   omits it for plenty of real tracks — see `MediaProbeTrackFieldsTest` — and `maxOf` against a
+     *   fabricated 0 would still be correct here, but reading a key that is absent is not.
+     */
+    internal fun extractedFrom(formats: List<MediaFormat>): Extracted {
+        var video: String? = null
+        var audio: String? = null
+        var durationUs = 0L
+        var width = 0
+        var height = 0
+
+        for (format in formats) {
+            val mime = format.getString(MediaFormat.KEY_MIME).orEmpty()
+            if (format.containsKey(MediaFormat.KEY_DURATION)) {
+                durationUs = maxOf(durationUs, format.getLong(MediaFormat.KEY_DURATION))
+            }
+            when {
+                mime.startsWith("video/") && video == null -> {
+                    video = shortName(mime)
+                    width = format.intOr(MediaFormat.KEY_WIDTH)
+                    height = format.intOr(MediaFormat.KEY_HEIGHT)
+                }
+
+                mime.startsWith("audio/") && audio == null -> audio = shortName(mime)
+            }
+        }
+        return Extracted(video, audio, durationUs / US_PER_MS, width, height)
+    }
+
     private fun probeWithExtractor(context: Context, uri: Uri): Extracted? {
         val extractor = MediaExtractor()
         return try {
             extractor.setDataSource(context, uri, null)
-            var video: String? = null
-            var audio: String? = null
-            var durationUs = 0L
-            var width = 0
-            var height = 0
-
-            for (i in 0 until extractor.trackCount) {
-                val format = extractor.getTrackFormat(i)
-                val mime = format.getString(MediaFormat.KEY_MIME).orEmpty()
-                if (format.containsKey(MediaFormat.KEY_DURATION)) {
-                    durationUs = maxOf(durationUs, format.getLong(MediaFormat.KEY_DURATION))
-                }
-                when {
-                    mime.startsWith("video/") && video == null -> {
-                        video = shortName(mime)
-                        width = format.intOr(MediaFormat.KEY_WIDTH)
-                        height = format.intOr(MediaFormat.KEY_HEIGHT)
-                    }
-
-                    mime.startsWith("audio/") && audio == null -> audio = shortName(mime)
-                }
-            }
-            Extracted(video, audio, durationUs / US_PER_MS, width, height)
+            extractedFrom(extractor.trackFormats())
         } catch (e: Exception) {
             Log.i(TAG, "Platform extractor could not read $uri.", e)
             null
@@ -135,7 +185,11 @@ object MediaProbe {
         }
     }
 
-    private class FFprobeInfo(
+    /**
+     * `internal` rather than `private` for the same reason [Extracted] is, and it should have been
+     * from the start: [merge] cannot be named from a test while half its signature is private.
+     */
+    internal class FFprobeInfo(
         val container: Container?,
         val videoCodec: String?,
         val audioCodec: String?,
@@ -269,25 +323,7 @@ object MediaProbe {
         val extractor = MediaExtractor()
         return try {
             extractor.setDataSource(context, uri, null)
-            var video: String? = null
-            var audio: String? = null
-            var width = 0
-            var height = 0
-            var fps = 0
-
-            for (i in 0 until extractor.trackCount) {
-                val format = extractor.getTrackFormat(i)
-                val mime = format.getString(MediaFormat.KEY_MIME).orEmpty()
-                if (mime.startsWith("video/") && video == null) {
-                    video = shortName(mime)
-                    width = format.intOr(MediaFormat.KEY_WIDTH)
-                    height = format.intOr(MediaFormat.KEY_HEIGHT)
-                    fps = format.intOr(MediaFormat.KEY_FRAME_RATE)
-                } else if (mime.startsWith("audio/") && audio == null) {
-                    audio = shortName(mime)
-                }
-            }
-            ConcatInput(video, audio, width, height, fps)
+            concatInputFrom(extractor.trackFormats())
         } catch (e: Exception) {
             Log.i(TAG, "Could not probe $uri for concat; will re-encode.", e)
             ConcatInput(null, null, 0, 0, 0)
@@ -296,6 +332,45 @@ object MediaProbe {
         }
     }
 
+    /**
+     * The join flow's read of the same track formats. See [extractedFrom] for why this is separate
+     * from the extractor.
+     *
+     * Deliberately **not** folded into [extractedFrom] despite the overlap. This one reads frame
+     * rate and does not read duration; that one reads duration and does not read frame rate. A
+     * merged version would have to compute both for every caller, and `ConcatPlanner` treats an
+     * unknown frame rate as "cannot prove a match" — so a field this flow does not need must not
+     * start arriving as a number.
+     */
+    internal fun concatInputFrom(formats: List<MediaFormat>): ConcatInput {
+        var video: String? = null
+        var audio: String? = null
+        var width = 0
+        var height = 0
+        var fps = 0
+
+        for (format in formats) {
+            val mime = format.getString(MediaFormat.KEY_MIME).orEmpty()
+            if (mime.startsWith("video/") && video == null) {
+                video = shortName(mime)
+                width = format.intOr(MediaFormat.KEY_WIDTH)
+                height = format.intOr(MediaFormat.KEY_HEIGHT)
+                fps = format.intOr(MediaFormat.KEY_FRAME_RATE)
+            } else if (mime.startsWith("audio/") && audio == null) {
+                audio = shortName(mime)
+            }
+        }
+        return ConcatInput(video, audio, width, height, fps)
+    }
+
+    /**
+     * Every track format this extractor holds, read once.
+     *
+     * The thin edge the two pure functions above leave behind: a `trackCount` and a
+     * `getTrackFormat` per index, which is the whole of what needs a real `MediaExtractor`.
+     */
+    private fun MediaExtractor.trackFormats(): List<MediaFormat> = (0 until trackCount).map(::getTrackFormat)
+
     /**
      * One track property as an Int, or [fallback] when the format has no Int to give.
      *

app/src/main/java/org/libremediaconverter/convert/OutputPublisher.kt

@@ -24,6 +24,20 @@ const val STAGED_FILE_GONE_MESSAGE: String =
     "The finished file is no longer in the cache, so there is nothing left to save. " +
         "Start over to make it again."
 
+/**
+ * What to tell the user when the copy into their chosen destination did not finish.
+ *
+ * A fallback, not the usual message: `publish` throws with a real reason most of the time — the
+ * volume filled, the provider revoked the grant — and that reason is better than this. This is for
+ * the exception that arrives with nothing to say, which would otherwise reach the screen as an
+ * empty failure.
+ *
+ * Kept next to [STAGED_FILE_GONE_MESSAGE] for exactly the reason that one names: **both ViewModels
+ * need it**, and saving is what it is about. It was written out twice before — `ConversionViewModel`
+ * and `JoinViewModel` each carried their own copy of the literal, agreeing by coincidence.
+ */
+const val SAVE_FAILED_MESSAGE: String = "Could not save the file."
+
 /**
  * A staged file that is still there to be saved, and everything the save dialog needs to offer it.
  *

app/src/main/java/org/libremediaconverter/convert/Transcoders.kt

@@ -4,6 +4,7 @@ import android.content.Context
 import android.net.Uri
 import androidx.media3.common.util.UnstableApi
 import org.libremediaconverter.codec.AndroidDeviceCodecs
+import org.libremediaconverter.ffmpeg.ConcatEngine
 import org.libremediaconverter.ffmpeg.FFmpegEngine
 import org.libremediaconverter.model.ConversionRequest
 import org.libremediaconverter.model.DeviceCodecs
@@ -40,6 +41,27 @@ interface SoftwareTranscoder {
     )
 }
 
+/**
+ * The join path. Implemented by [org.libremediaconverter.ffmpeg.ConcatEngine].
+ *
+ * Added last of the three, and the gap it closes was measured rather than guessed:
+ * `PerJobStagingTest`'s KDoc records that reverting `ConcatWorker` to a constant staging name left
+ * all 257 tests green, because nothing in the JVM suite can get past a `ConcatEngine` constructed
+ * in place. Everything after that line -- the failure mapping, the message fallback, the staged
+ * delete -- was untested on every source set.
+ *
+ * The result type stays nested in the implementation rather than being lifted here. Moving it would
+ * touch every call site to buy nothing: what a test needs is the ability to *not* run FFmpeg, and
+ * that is the method, not the type.
+ */
+interface ConcatJoiner {
+    suspend fun join(
+        inputs: List<Uri>,
+        output: File,
+        format: OutputFormat = OutputFormat.MP4_H264,
+    ): ConcatEngine.Result
+}
+
 /**
  * The seam that lets tests force failure paths.
  *
@@ -69,6 +91,9 @@ object ConversionDependencies {
     @Volatile
     var software: () -> SoftwareTranscoder = { FFmpegEngine() }
 
+    @Volatile
+    var concat: (Context) -> ConcatJoiner = { ConcatEngine(it) }
+
     @Volatile
     var publisher: (Context) -> OutputPublisher = { OutputPublisher(it) }
 
@@ -103,6 +128,7 @@ object ConversionDependencies {
     fun reset() {
         hardware = { Media3Engine(it) }
         software = { FFmpegEngine() }
+        concat = { ConcatEngine(it) }
         publisher = { OutputPublisher(it) }
         deviceCodecs = { AndroidDeviceCodecs.get() }
         probe = { context, uri -> MediaProbe.probe(context, uri) }

app/src/main/java/org/libremediaconverter/ffmpeg/ConcatEngine.kt

@@ -7,6 +7,7 @@ import com.arthenica.ffmpegkit.FFmpegKit
 import com.arthenica.ffmpegkit.FFmpegKitConfig
 import com.arthenica.ffmpegkit.ReturnCode
 import kotlinx.coroutines.suspendCancellableCoroutine
+import org.libremediaconverter.convert.ConcatJoiner
 import org.libremediaconverter.convert.MediaProbe
 import org.libremediaconverter.convert.StagingNames
 import org.libremediaconverter.model.ConcatPlanner
@@ -24,11 +25,11 @@ import kotlin.coroutines.resumeWithException
  * reliably fail when they differ — it can emit a file whose later segments are
  * garbled. See [ConcatPlanner].
  */
-class ConcatEngine(private val context: Context) {
+class ConcatEngine(private val context: Context) : ConcatJoiner {
 
     data class Result(val strategy: ConcatStrategy, val output: File)
 
-    suspend fun join(inputs: List<Uri>, output: File, format: OutputFormat = OutputFormat.MP4_H264): Result {
+    override suspend fun join(inputs: List<Uri>, output: File, format: OutputFormat): Result {
         require(inputs.size >= 2) { "Joining needs at least two files." }
 
         val paths = inputs.map { uri ->

app/src/main/java/org/libremediaconverter/join/JoinScreen.kt

@@ -56,17 +56,38 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
 
     JoinScreenContent(
         state = state,
-        actions = JoinActions(
+        actions = joinActions(
+            viewModel = viewModel,
             onPickInputs = { pickInputs.launch(arrayOf("video/*")) },
-            onJoin = viewModel::join,
-            onCancel = viewModel::cancel,
             onSave = { suggestedName -> chooseDestination.launch(suggestedName) },
-            onReset = viewModel::reset,
         ),
         modifier = modifier,
     )
 }
 
+/**
+ * Which of the ViewModel's methods each affordance on the join screen calls.
+ *
+ * The join-side twin of `converterActions`, and the transposition risk here is worse: **three**
+ * `() -> Unit` bindings rather than two. `onJoin`, `onCancel` and `onReset` are mutually
+ * interchangeable as far as the compiler is concerned, so a Join button that cancels, or a Cancel
+ * button that starts the job, is a swap nothing but a test would catch.
+ *
+ * See `converterActions` for why the launcher-backed actions stay parameters.
+ */
+@UnstableApi
+internal fun joinActions(
+    viewModel: JoinViewModel,
+    onPickInputs: () -> Unit,
+    onSave: (suggestedName: String) -> Unit,
+): JoinActions = JoinActions(
+    onPickInputs = onPickInputs,
+    onJoin = viewModel::join,
+    onCancel = viewModel::cancel,
+    onSave = onSave,
+    onReset = viewModel::reset,
+)
+
 /**
  * Everything [JoinScreenContent] can ask for, in one value.
  *

app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt

@@ -5,6 +5,7 @@ import android.net.Uri
 import androidx.lifecycle.AndroidViewModel
 import androidx.lifecycle.viewModelScope
 import androidx.media3.common.util.UnstableApi
+import androidx.work.Data
 import androidx.work.WorkInfo
 import androidx.work.WorkManager
 import kotlinx.coroutines.CoroutineDispatcher
@@ -19,6 +20,7 @@ import org.libremediaconverter.convert.ConversionDependencies
 import org.libremediaconverter.convert.InputFile
 import org.libremediaconverter.convert.InputQuery
 import org.libremediaconverter.convert.PendingSave
+import org.libremediaconverter.convert.SAVE_FAILED_MESSAGE
 import org.libremediaconverter.convert.STAGED_FILE_GONE_MESSAGE
 import org.libremediaconverter.convert.ScreenOwnership
 import org.libremediaconverter.model.ConcatStrategy
@@ -29,6 +31,108 @@ import org.libremediaconverter.work.jobSnapshots
 import java.io.File
 import java.util.UUID
 
+/**
+ * One update about a running join, as WorkManager last reported it.
+ *
+ * The join-side twin of `ConversionUpdate`, and deliberately the same shape: this pair of seams is
+ * one refactor done twice, and letting them diverge would make the two flows harder to compare than
+ * the duplication costs. There is no `progressPercent` here because `ConcatWorker` publishes none —
+ * a join is indeterminate.
+ */
+internal data class JoinUpdate(val state: WorkInfo.State, val runAttemptCount: Int, val outputData: Data)
+
+/**
+ * What the join screen should show, given what WorkManager last said about the job.
+ *
+ * The join-side twin of `conversionStateFrom`, extracted for the same reason and with the same two
+ * exclusions: the ownership check stays at the call site, and this takes no responsibility for the
+ * staged file. See that function's KDoc for the argument in full.
+ *
+ * Five arms had never been chosen by any test before this was cut out, because a real `ConcatWorker`
+ * only ever produces a terminal state with well-formed output.
+ *
+ * @param cancelled where a cancellation lands, which differs for a reattached job — see [observe].
+ */
+@UnstableApi
+internal fun joinStateFrom(update: JoinUpdate, inputs: List<InputFile>, cancelled: JoinState): JoinState =
+    when (update.state) {
+        // BLOCKED is a job waiting on a prerequisite, which the user has nothing to do about and
+        // nothing useful to be told about. It reads as "starting", like a fresh ENQUEUED.
+        WorkInfo.State.RUNNING, WorkInfo.State.BLOCKED -> JoinState.Joining(inputs)
+
+        // ENQUEUED after a run means a retry is pending -- the same rule, and the same reasoning, as
+        // the convert side. See `conversionStateFrom`.
+        WorkInfo.State.ENQUEUED ->
+            if (update.runAttemptCount > 0) {
+                JoinState.Waiting(inputs)
+            } else {
+                JoinState.Joining(inputs)
+            }
+
+        WorkInfo.State.SUCCEEDED -> joinedFrom(update.outputData)
+
+        // A worker that dies before it can report anything leaves no output data at all, and an
+        // exception's message can be an empty string. Both would read as a failure with nothing said,
+        // so blank falls back like missing does.
+        WorkInfo.State.FAILED -> JoinState.Failed(
+            update.outputData.getString(ConcatWorker.KEY_ERROR)
+                ?.takeIf { it.isNotBlank() }
+                ?: ConcatWorker.GENERIC_FAILURE_MESSAGE,
+        )
+
+        WorkInfo.State.CANCELLED -> cancelled
+    }
+
+/**
+ * The `SUCCEEDED` arm. A join that reported success and named no file has nothing to offer.
+ */
+@UnstableApi
+private fun joinedFrom(outputData: Data): JoinState {
+    val path = outputData.getString(ConcatWorker.KEY_OUTPUT_PATH)
+        ?: return JoinState.Failed(JOINED_WITHOUT_A_FILE_MESSAGE)
+    return JoinState.Joined(
+        staged = File(path),
+        strategy = strategyFrom(outputData.getString(ConcatWorker.KEY_STRATEGY)),
+        // A join enqueued before the worker reported these carries neither, and the fallback is
+        // the format such a job really used -- ConcatWorker.request has always defaulted to it,
+        // and the join screen has never offered a choice.
+        suggestedName = outputData.getString(ConcatWorker.KEY_SUGGESTED_NAME)
+            ?.takeIf { it.isNotBlank() }
+            ?: ConcatWorker.outputNameFor(ConcatWorker.DEFAULT_FORMAT),
+        mimeType = outputData.getString(ConcatWorker.KEY_MIME_TYPE)
+            ?.takeIf { it.isNotBlank() }
+            ?: ConcatWorker.DEFAULT_FORMAT.mimeType,
+    )
+}
+
+/**
+ * The strategy a finished join reported, or [ConcatStrategy.REENCODE] when it named none.
+ *
+ * **Looked up rather than `valueOf`, and that is a fix rather than a style choice.** `valueOf`
+ * throws `IllegalArgumentException` on a name this build does not define, and this runs inside a
+ * `viewModelScope` collect with no handler -- so the throw does not become a `Failed` state, it
+ * takes the process down.
+ *
+ * Reachable for the reason `WorkerEnumFallbackTest` and `JobTags` are both written on: WorkManager
+ * keeps finished work for about a week, so a downgrade or a rollback hands this build a job
+ * enqueued by another one. `ConcatWorker` writes `result.strategy.name` into the output `Data`, so
+ * a build that added a third strategy would leave this one crashing on its own completed joins.
+ *
+ * `ConcatWorker.kt` already made exactly this change for `KEY_FORMAT`, and says why in as many
+ * words: *"Looked up rather than `valueOf` … a format name this build does not define used to throw
+ * past the catch."* The same read on this side had not been changed with it.
+ *
+ * REENCODE is the safe default rather than an arbitrary one: it is the answer for inputs that do
+ * not match, so a job whose strategy cannot be read is described as the more conservative of the
+ * two rather than being claimed as a lossless stream copy.
+ */
+private fun strategyFrom(name: String?): ConcatStrategy =
+    ConcatStrategy.entries.firstOrNull { it.name == name } ?: ConcatStrategy.REENCODE
+
+/** A join that reported success and named no file. There is nothing to offer the user to save. */
+internal const val JOINED_WITHOUT_A_FILE_MESSAGE: String =
+    "Joining reported success but produced no file."
+
 sealed interface JoinState {
     data object Idle : JoinState
     data class Ready(val inputs: List<InputFile>) : JoinState
@@ -194,7 +298,7 @@ class JoinViewModel @JvmOverloads constructor(
     fun onInputsPicked(uris: List<Uri>) {
         val token = ownership.claim()
         if (uris.size < 2) {
-            _state.value = JoinState.Failed("Pick at least two files to join.")
+            _state.value = JoinState.Failed(ConcatWorker.TOO_FEW_INPUTS_MESSAGE)
             return
         }
         viewModelScope.launch {
@@ -250,56 +354,19 @@ class JoinViewModel @JvmOverloads constructor(
                 // takes ownership of the staged file, and a superseded observation must not do
                 // that either.
                 if (!ownership.stillHeldBy(token)) return@collect
-                _state.value = when (info.state) {
-                    WorkInfo.State.RUNNING, WorkInfo.State.BLOCKED -> JoinState.Joining(inputs)
-                    WorkInfo.State.ENQUEUED ->
-                        if (info.runAttemptCount > 0) {
-                            JoinState.Waiting(inputs)
-                        } else {
-                            JoinState.Joining(inputs)
-                        }
-
-                    WorkInfo.State.SUCCEEDED -> {
-                        val path = info.outputData.getString(ConcatWorker.KEY_OUTPUT_PATH)
-                        val strategy = info.outputData.getString(ConcatWorker.KEY_STRATEGY)
-                            ?.let(ConcatStrategy::valueOf) ?: ConcatStrategy.REENCODE
-                        if (path == null) {
-                            JoinState.Failed("Joining reported success but produced no file.")
-                        } else {
-                            val staged = File(path)
-                            // Take responsibility for the file at the same moment the state
-                            // starts referring to it, so the two cannot disagree.
-                            pendingStaged = staged
-                            JoinState.Joined(
-                                staged = staged,
-                                strategy = strategy,
-                                // A join enqueued before the worker reported these carries
-                                // neither, and the fallback is the format such a job really
-                                // used -- ConcatWorker.request has always defaulted to it, and
-                                // the join screen has never offered a choice.
-                                suggestedName = info.outputData
-                                    .getString(ConcatWorker.KEY_SUGGESTED_NAME)
-                                    ?.takeIf { it.isNotBlank() }
-                                    ?: ConcatWorker.outputNameFor(ConcatWorker.DEFAULT_FORMAT),
-                                mimeType = info.outputData
-                                    .getString(ConcatWorker.KEY_MIME_TYPE)
-                                    ?.takeIf { it.isNotBlank() }
-                                    ?: ConcatWorker.DEFAULT_FORMAT.mimeType,
-                            )
-                        }
-                    }
-
-                    // A worker that dies before it can report anything leaves no output data at
-                    // all, and an exception's message can be an empty string. Both would read as
-                    // a failure with nothing said, so blank falls back like missing does.
-                    WorkInfo.State.FAILED -> JoinState.Failed(
-                        info.outputData.getString(ConcatWorker.KEY_ERROR)
-                            ?.takeIf { it.isNotBlank() }
-                            ?: "Joining failed.",
-                    )
-
-                    WorkInfo.State.CANCELLED -> cancelled
-                }
+                val next = joinStateFrom(
+                    JoinUpdate(
+                        state = info.state,
+                        runAttemptCount = info.runAttemptCount,
+                        outputData = info.outputData,
+                    ),
+                    inputs = inputs,
+                    cancelled = cancelled,
+                )
+                // Read off the result rather than assigned inside the mapping -- see the same
+                // three lines in ConversionViewModel for why that is the better half of the swap.
+                if (next is JoinState.Joined) pendingStaged = next.staged
+                _state.value = next
             }
         }
     }
@@ -346,7 +413,7 @@ class JoinViewModel @JvmOverloads constructor(
                 // than leaving "Start over" -- which deletes it -- as the only thing on offer.
                 // Passing `pending` rather than rebuilding it is what keeps a retry that fails
                 // again on a carrying Failed instead of a bare one.
-                _state.value = JoinState.Failed(e.message ?: "Could not save the file.", pending)
+                _state.value = JoinState.Failed(e.message ?: SAVE_FAILED_MESSAGE, pending)
             }
         }
     }

app/src/main/java/org/libremediaconverter/ui/TestTags.kt

@@ -56,6 +56,20 @@ object TestTags {
      */
     const val RETRY_SAVE: String = "action.retrySave"
 
+    /**
+     * The adaptive shell around both screens -- `AppRoot`'s two layouts.
+     *
+     * Named because there is no other way to tell them apart from a test. Both render the same two
+     * destinations with the same labels and the same selection state, so every assertion that could
+     * be written without these tags is satisfied by either layout, and transposing the two bodies
+     * passed the whole suite. Exactly one of the two exists at a time, which is what makes
+     * `assertExists` / `assertDoesNotExist` on this pair a statement about the width class.
+     */
+    object Shell {
+        const val NAVIGATION_RAIL: String = "shell.navigationRail"
+        const val NAVIGATION_BAR: String = "shell.navigationBar"
+    }
+
     /** `ConverterScreen`. */
     object Converter {
         const val CHOOSE_FILE: String = "converter.chooseFile"

app/src/main/java/org/libremediaconverter/work/ConcatWorker.kt

@@ -15,7 +15,6 @@ import kotlinx.coroutines.CancellationException
 import org.libremediaconverter.convert.ConversionDependencies
 import org.libremediaconverter.convert.InputQuery
 import org.libremediaconverter.convert.StagingNames
-import org.libremediaconverter.ffmpeg.ConcatEngine
 import org.libremediaconverter.model.OutputFormat
 
 /**
@@ -37,9 +36,9 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
 
     override suspend fun doWork(): Result {
         val uris = inputData.getStringArray(KEY_INPUT_URIS)?.map(Uri::parse)
-            ?: return Result.failure(workDataOf(KEY_ERROR to "No input files."))
+            ?: return Result.failure(workDataOf(KEY_ERROR to NO_INPUTS_MESSAGE))
         if (uris.size < 2) {
-            return Result.failure(workDataOf(KEY_ERROR to "Pick at least two files to join."))
+            return Result.failure(workDataOf(KEY_ERROR to TOO_FEW_INPUTS_MESSAGE))
         }
         // Absent, not zero, when the picker could not size every input -- see the same read in
         // ConversionWorker and InputQuery for why the two are no longer one number.
@@ -77,7 +76,7 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
                 ),
             )
 
-            val result = ConcatEngine(applicationContext).join(uris, staged, format)
+            val result = ConversionDependencies.concat(applicationContext).join(uris, staged, format)
             Result.success(
                 workDataOf(
                     KEY_OUTPUT_PATH to staged.absolutePath,
@@ -107,7 +106,7 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
                 }
                 FailureOutcome.FAIL -> {
                     Log.e(TAG, "Joining failed.", e)
-                    Result.failure(workDataOf(KEY_ERROR to (e.message ?: "Joining failed.")))
+                    Result.failure(workDataOf(KEY_ERROR to (e.message ?: GENERIC_FAILURE_MESSAGE)))
                 }
             }
         }
@@ -137,6 +136,44 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
     )
 
     companion object {
+        /**
+         * What the user is told when a join arrives with fewer than two inputs.
+         *
+         * Shared with `JoinViewModel`, which refuses the same condition one layer up so the picker
+         * can answer without enqueueing anything. Two copies of this sentence existed before, and
+         * only the one here was pinned by a test (#139) — so the wording could drift on the screen
+         * without a single test noticing, for one message the user sees from one condition.
+         *
+         * Here rather than in the ViewModel because the rule is the worker's: `request(...)` takes
+         * a `List<Uri>` and checks nothing about its length, so this is the guard that always runs.
+         */
+        /**
+         * A job carrying no input array at all -- a downgrade, or a queue entry from a build that
+         * spelled the key differently.
+         *
+         * A constant rather than the literal it was, for the convention #158 established: a message
+         * the user can see is named once, so a test asserts the same string the worker writes
+         * rather than a copy of it that can drift.
+         */
+        const val NO_INPUTS_MESSAGE: String = "No input files."
+
+        const val TOO_FEW_INPUTS_MESSAGE: String = "Pick at least two files to join."
+
+        /**
+         * The last resort when a join fails and the exception says nothing.
+         *
+         * Shared with `JoinViewModel`, whose `FAILED` arm falls back to the same sentence when the
+         * output `Data` carries no error at all — a worker killed before it could write one. The two
+         * are a chain rather than a coincidence: this is what the worker puts *in* `KEY_ERROR`, and
+         * that is what the ViewModel says when `KEY_ERROR` never arrived. The user cannot tell the
+         * two apart and should not have to, so they are one sentence.
+         *
+         * The `Log.e` above deliberately keeps its own literal. A log line has a different audience
+         * and carries the exception with it; coupling it to the user-facing wording would mean
+         * rewording the screen to change a log.
+         */
+        const val GENERIC_FAILURE_MESSAGE: String = "Joining failed."
+
         const val KEY_INPUT_URIS = "input_uris"
         const val KEY_TOTAL_BYTES = "total_bytes"
         const val KEY_FORMAT = "format"

app/src/main/java/org/libremediaconverter/work/ConversionWorker.kt

@@ -313,7 +313,7 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
             }
             FailureOutcome.FAIL -> {
                 Log.e(TAG, "Conversion failed.", cause)
-                Result.failure(workDataOf(KEY_ERROR to (cause.message ?: "Conversion failed.")))
+                Result.failure(workDataOf(KEY_ERROR to (cause.message ?: GENERIC_FAILURE_MESSAGE)))
             }
         }
 
@@ -352,6 +352,20 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
     )
 
     companion object {
+        /**
+         * The last resort when a conversion fails and the exception says nothing.
+         *
+         * Shared with `ConversionViewModel`, whose `FAILED` arm falls back to the same sentence when
+         * the output `Data` carries no error — a worker killed before it could write one, which a
+         * refused foreground start after a process restart produces. The two are a chain rather than
+         * a coincidence: this is what goes *into* `KEY_ERROR`, and that is what is said when
+         * `KEY_ERROR` never arrived. The user cannot tell those apart and should not have to.
+         *
+         * See [ConcatWorker.GENERIC_FAILURE_MESSAGE] for the join-side twin, and the note there
+         * about why the neighbouring `Log.e` keeps its own literal.
+         */
+        const val GENERIC_FAILURE_MESSAGE: String = "Conversion failed."
+
         const val KEY_INPUT_URI = "input_uri"
         const val KEY_DISPLAY_NAME = "display_name"
         const val KEY_SIZE_BYTES = "size_bytes"

app/src/test/java/org/libremediaconverter/AdaptiveShellTest.kt

@@ -0,0 +1,129 @@
+package org.libremediaconverter
+
+import androidx.activity.ComponentActivity
+import androidx.compose.material3.windowsizeclass.WindowWidthSizeClass
+import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
+import androidx.compose.ui.test.onNodeWithTag
+import androidx.compose.ui.test.onNodeWithText
+import androidx.compose.ui.test.performClick
+import androidx.media3.common.util.UnstableApi
+import androidx.work.Data
+import org.junit.After
+import org.junit.Before
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.libremediaconverter.convert.ConversionDependencies
+import org.libremediaconverter.convert.installTestWorkManager
+import org.libremediaconverter.model.InputProbe
+import org.libremediaconverter.ui.TestTags
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+
+/**
+ * Which navigation affordance the shell actually renders, and which screen it actually shows.
+ *
+ * Assertion gaps rather than coverage gaps, both of them, and that is why they lasted.
+ * `AppRootRestorationTest` already drives `AppRoot` at `Compact` and `Expanded`, so JaCoCo is green
+ * on `useRail` -- but it asserts only that the *selected tab* survives recreation, through a stub
+ * `content` composable. Nothing anywhere queried for a rail or a bar, and nothing rendered the real
+ * screens. Two consequences, both measured before this file existed:
+ *
+ * - **Transposing the `NavigationRail` and `NavigationBar` bodies passed the entire suite.**
+ * - **Transposing `Content`'s two arms passed it too** -- a tablet showing the phone chrome, or the
+ *   Convert tab opening the Join screen, and 546 tests with nothing to say about either.
+ *
+ * `AppRoot`'s own KDoc is why this matters more than it looks: from targetSdk 37 the app is resized
+ * and rotated whether or not it is ready, so the width class is not a preference, it is whatever
+ * the system hands over.
+ *
+ * ## Two things this needed that the rest of the suite does not
+ *
+ * **`createAndroidComposeRule`, not `createComposeRule`.** Rendering `AppRoot` with its *default*
+ * content reaches `ConverterScreen`'s `viewModel = viewModel()`, which needs a
+ * `ViewModelStoreOwner`; the plain rule supplies none. It works because both ViewModels are
+ * `@JvmOverloads constructor(app: Application, …)`, so `AndroidViewModelFactory` can build them,
+ * and because `app/build.gradle.kts` already puts `ui-test-manifest`'s `ComponentActivity` in the
+ * merged manifest the unit tests build against -- which that file says in terms.
+ *
+ * **Tags on the two bars.** They are in `TestTags`, applied inside `main`, for the reason that
+ * file's KDoc gives: a tag the test hands down proves only that the test set it.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class AdaptiveShellTest {
+
+    @get:Rule
+    val composeRule = createAndroidComposeRule<ComponentActivity>()
+
+    @Before
+    fun setUp() {
+        val app = RuntimeEnvironment.getApplication()
+        installTestWorkManager(app, Data.EMPTY)
+        // The real screens are composed here, so their ViewModels are real too. Neither test is
+        // about probing or publishing; left alone they would reach the FFprobe loader and this
+        // machine's codec list, and decide things no assertion mentions.
+        ConversionDependencies.probe = { _, _ -> InputProbe() }
+    }
+
+    @After
+    fun tearDown() {
+        ConversionDependencies.reset()
+    }
+
+    @Test
+    fun `a phone gets the bottom bar and a tablet gets the rail`() {
+        setShell(WindowWidthSizeClass.Compact)
+
+        composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_BAR).assertExists()
+        composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_RAIL).assertDoesNotExist()
+    }
+
+    @Test
+    fun `an expanded window gets the rail`() {
+        setShell(WindowWidthSizeClass.Expanded)
+
+        composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_RAIL).assertExists()
+        composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_BAR).assertDoesNotExist()
+    }
+
+    /**
+     * The width class no test had ever passed.
+     *
+     * `useRail` is `!= Compact`, so Medium takes the rail with Expanded. Narrowing it to
+     * `== Expanded` is a one-character change that breaks every tablet and unfolded foldable and
+     * nothing else -- and until this test, nothing in either source set used `Medium` at all.
+     */
+    @Test
+    fun `a medium window is a rail window, not a phone`() {
+        setShell(WindowWidthSizeClass.Medium)
+
+        composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_RAIL).assertExists()
+        composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_BAR).assertDoesNotExist()
+    }
+
+    /**
+     * The mapping every other test stubs out: which screen each destination actually opens.
+     *
+     * Matched on each screen's own "choose a file" affordance rather than on a title, because those
+     * tags are applied by the screens themselves -- so this fails if the destinations are
+     * transposed, and it fails for the right reason.
+     */
+    @Test
+    fun `Convert opens the converter and Join opens the join screen`() {
+        setShell(WindowWidthSizeClass.Compact)
+
+        composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).assertExists()
+        composeRule.onNodeWithTag(TestTags.Join.CHOOSE_FILES).assertDoesNotExist()
+
+        composeRule.onNodeWithText(Destination.JOIN.label).performClick()
+
+        composeRule.onNodeWithTag(TestTags.Join.CHOOSE_FILES).assertExists()
+        composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).assertDoesNotExist()
+    }
+
+    /** [AppRoot] with its real content, which is the half nothing else composes. */
+    private fun setShell(width: WindowWidthSizeClass) {
+        composeRule.setContent { AppRoot(width) }
+    }
+}

app/src/test/java/org/libremediaconverter/codec/CodecVocabularyTest.kt

@@ -7,6 +7,7 @@ import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Test
 import org.libremediaconverter.model.CodecNames
+import org.libremediaconverter.model.InputProbe
 import org.libremediaconverter.model.VideoCodec
 
 /**
@@ -133,6 +134,38 @@ class CodecVocabularyTest {
      * landed, a device with no HEVC decoder answered true for `x265` and Media3 was handed a job it
      * could not do; now the router sends it to FFmpeg without spending the attempt.
      */
+    /**
+     * The sentinel is not just another unknown name, and the difference is the whole guard.
+     *
+     * `canDecode` ends `?: true` -- a name neither table knows keeps the permissive answer, because
+     * the app would rather try than refuse a file it might handle. `InputProbe.UNPARSEABLE` has to
+     * be the exception: the platform has *already* failed to parse the input, so there is nothing
+     * for a decoder to be permissive about, and waving it through spends a Media3 attempt on a job
+     * that cannot start.
+     *
+     * The `cinepak` line is what makes the sentinel line mean something. Without it, deleting the
+     * early return leaves this test green -- both names would fall through to the same `?: true`.
+     * The pair is the assertion.
+     *
+     * `DeviceCodecs.PERMISSIVE` carries the same rule and `ConversionRouterTest` pins its routing
+     * consequence. This is the implementation that runs on a device.
+     */
+    @Test
+    fun `the unparseable sentinel is refused even where an unknown name is waved through`() {
+        val everything = AndroidDeviceCodecs.forTesting(
+            encoders = emptySet(),
+            decoders = setOf("video/avc", "video/hevc"),
+        )
+        assertFalse(
+            "the platform could not parse this input, so there is nothing to decode with",
+            everything.canDecode(InputProbe.UNPARSEABLE),
+        )
+        assertTrue(
+            "a merely unknown name still keeps the permissive answer",
+            everything.canDecode("cinepak"),
+        )
+    }
+
     @Test
     fun `a device without the decoder now says so for the aliases it used to wave through`() {
         val hevcOnly = AndroidDeviceCodecs.forTesting(encoders = emptySet(), decoders = setOf("video/hevc"))

app/src/test/java/org/libremediaconverter/convert/ConversionStateMappingTest.kt

@@ -0,0 +1,238 @@
+package org.libremediaconverter.convert
+
+import android.net.Uri
+import androidx.media3.common.util.UnstableApi
+import androidx.work.Data
+import androidx.work.WorkInfo
+import androidx.work.workDataOf
+import org.junit.Assert.assertEquals
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.libremediaconverter.model.OutputFormat
+import org.libremediaconverter.work.ConversionWorker
+import org.robolectric.RobolectricTestRunner
+
+/**
+ * Every answer [conversionStateFrom] can give, chosen rather than stumbled into.
+ *
+ * ## What this revises
+ *
+ * The mapping is not cold code and never was: `ConversionViewModel$observe$1$1` reported 28 covered
+ * lines before this file existed, because every test that drives a real worker runs it. What no
+ * test did was **choose which arm it took**. A real worker reaches a terminal state with
+ * well-formed output, so `SUCCEEDED`-with-a-path and `FAILED`-with-a-message were the only arms any
+ * test had ever produced — the other six ran never.
+ *
+ * A `grep` for `WorkInfo.State.` across the JVM suite makes that look untrue: all six constants are
+ * there. They are in `ReattachmentTest`, driven into **`Reattachment.choose`** — a different
+ * function that encodes the same enqueued-means-retry rule. So that rule had a test in one of its
+ * two homes, and the copy the user's screen reads had none.
+ *
+ * ## Why the seam, and why these assertions
+ *
+ * `WorkManager.getInstance` is called in the ViewModel's constructor and `observe` is private, so
+ * nothing could hand this a chosen `WorkInfo`. Cutting the `when` out as a pure function over
+ * [ConversionUpdate] is the answer #141 took for `MediaProbe`, and `JobSnapshot` beside
+ * `Reattachment.choose` is the same shape again.
+ *
+ * The assertions are on the whole state, not on its type. `Converting(input, 40)` and
+ * `Converting(input, 0)` are both `Converting`, and a mapping that dropped the progress read would
+ * pass any test that only asked which class came back.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class ConversionStateMappingTest {
+
+    // --- running ------------------------------------------------------------
+
+    @Test
+    fun `a running job reports the progress it published`() {
+        // The percent is read from `progress`, not from `outputData`, and not from the settings.
+        // A mapping that returned Converting(input, 0) for every RUNNING would leave the bar
+        // pinned at zero for the whole conversion.
+        val state = map(WorkInfo.State.RUNNING, progress = 40)
+
+        assertEquals(ConversionState.Converting(INPUT, 40), state)
+    }
+
+    @Test
+    fun `a running job with no published progress reports zero rather than failing`() {
+        // getInt's default. A worker that has started but not yet called setProgress is ordinary,
+        // and must not read as an error.
+        val state = map(WorkInfo.State.RUNNING, progress = null)
+
+        assertEquals(ConversionState.Converting(INPUT, 0), state)
+    }
+
+    // --- enqueued: the rule that had a test only in its other home ----------
+
+    @Test
+    fun `an enqueued job that has already run is waiting to retry`() {
+        val state = map(WorkInfo.State.ENQUEUED, runAttemptCount = 1)
+
+        assertEquals(
+            "an ENQUEUED after a run is a pending retry, which the user is told about",
+            ConversionState.Waiting(INPUT),
+            state,
+        )
+    }
+
+    @Test
+    fun `an enqueued job that has never run is simply starting`() {
+        // The other side, and the reason the test above is not enough on its own: a mapping that
+        // ignored runAttemptCount and always answered Waiting would pass that one and fail this.
+        val state = map(WorkInfo.State.ENQUEUED, runAttemptCount = 0)
+
+        assertEquals(ConversionState.Converting(INPUT, 0), state)
+    }
+
+    // --- succeeded ----------------------------------------------------------
+
+    @Test
+    fun `a success that named no file is a failure, not an empty success`() {
+        // The job said it finished and named nothing. There is no file to offer, so `Converted`
+        // would put a Save button over a path that does not exist.
+        val state = map(WorkInfo.State.SUCCEEDED, data = Data.EMPTY)
+
+        assertEquals(ConversionState.Failed(SUCCEEDED_WITHOUT_A_FILE_MESSAGE), state)
+    }
+
+    @Test
+    fun `a success carries the worker's own name and type, not the current settings`() {
+        val state = map(
+            WorkInfo.State.SUCCEEDED,
+            data = workDataOf(
+                ConversionWorker.KEY_OUTPUT_PATH to "/cache/conversions/out.mkv",
+                ConversionWorker.KEY_SUGGESTED_NAME to "holiday.mkv",
+                ConversionWorker.KEY_MIME_TYPE to "video/x-matroska",
+                ConversionWorker.KEY_ENGINE_USED to "FFMPEG",
+                ConversionWorker.KEY_ROUTE_REASON to "container needs FFmpeg",
+            ),
+        )
+
+        val converted = state as ConversionState.Converted
+        assertEquals("holiday.mkv", converted.suggestedName)
+        assertEquals("video/x-matroska", converted.mimeType)
+        assertEquals("FFMPEG", converted.engineUsed)
+        assertEquals("container needs FFmpeg", converted.routeReason)
+    }
+
+    @Test
+    fun `a success from older work falls back to the current settings for name and type`() {
+        // WorkManager keeps finished work about a week, so a job enqueued before the worker
+        // reported these is ordinary for a few days rather than a corner case.
+        val state = map(
+            WorkInfo.State.SUCCEEDED,
+            data = workDataOf(ConversionWorker.KEY_OUTPUT_PATH to "/cache/conversions/out.mp4"),
+        )
+
+        val converted = state as ConversionState.Converted
+        assertEquals(FALLBACK_SPEC.mimeType, converted.mimeType)
+        assertEquals(
+            ConversionWorker.outputNameFor(INPUT.displayName, FALLBACK_SPEC),
+            converted.suggestedName,
+        )
+    }
+
+    @Test
+    fun `a blank name or type falls back the same way a missing one does`() {
+        // A blank string is not an answer. Without takeIf, the save dialog opens named "" and
+        // registered for a MIME type of "", which no provider will accept.
+        val state = map(
+            WorkInfo.State.SUCCEEDED,
+            data = workDataOf(
+                ConversionWorker.KEY_OUTPUT_PATH to "/cache/conversions/out.mp4",
+                ConversionWorker.KEY_SUGGESTED_NAME to "",
+                ConversionWorker.KEY_MIME_TYPE to "   ",
+            ),
+        )
+
+        val converted = state as ConversionState.Converted
+        assertEquals(FALLBACK_SPEC.mimeType, converted.mimeType)
+        assertEquals(
+            ConversionWorker.outputNameFor(INPUT.displayName, FALLBACK_SPEC),
+            converted.suggestedName,
+        )
+    }
+
+    // --- failed -------------------------------------------------------------
+
+    @Test
+    fun `a failure carries the reason the worker gave`() {
+        val state = map(
+            WorkInfo.State.FAILED,
+            data = workDataOf(ConversionWorker.KEY_ERROR to "Not enough free space to convert."),
+        )
+
+        assertEquals(ConversionState.Failed("Not enough free space to convert."), state)
+    }
+
+    @Test
+    fun `a failure with nothing said still says something`() {
+        // A worker killed before it could write output data leaves none at all -- a refused
+        // foreground start after a process restart is one way. Failed("") would render as a blank
+        // error card.
+        val state = map(WorkInfo.State.FAILED, data = Data.EMPTY)
+
+        assertEquals(ConversionState.Failed(ConversionWorker.GENERIC_FAILURE_MESSAGE), state)
+    }
+
+    @Test
+    fun `a failure whose message is blank falls back like a missing one`() {
+        val state = map(
+            WorkInfo.State.FAILED,
+            data = workDataOf(ConversionWorker.KEY_ERROR to "   "),
+        )
+
+        assertEquals(ConversionState.Failed(ConversionWorker.GENERIC_FAILURE_MESSAGE), state)
+    }
+
+    // --- cancelled and blocked ---------------------------------------------
+
+    @Test
+    fun `a cancellation lands wherever the caller said it should`() {
+        // Not a fixed state: a conversion started here goes back to Ready with the picked file,
+        // while one picked up by reattach goes to Idle, because the URI that job holds belongs to
+        // a process that no longer exists. `observe`'s KDoc is where that distinction is set.
+        val toReady = map(WorkInfo.State.CANCELLED, cancelled = ConversionState.Ready(INPUT))
+        val toIdle = map(WorkInfo.State.CANCELLED, cancelled = ConversionState.Idle)
+
+        assertEquals(ConversionState.Ready(INPUT), toReady)
+        assertEquals(ConversionState.Idle, toIdle)
+    }
+
+    @Test
+    fun `a blocked job looks like one that is starting`() {
+        // BLOCKED is a job waiting on a prerequisite. There is nothing useful to say about it that
+        // differs from "starting", and inventing a state for it would put a word on screen the
+        // user cannot act on.
+        val state = map(WorkInfo.State.BLOCKED)
+
+        assertEquals(ConversionState.Converting(INPUT, 0), state)
+    }
+
+    private fun map(
+        state: WorkInfo.State,
+        progress: Int? = null,
+        runAttemptCount: Int = 0,
+        data: Data = Data.EMPTY,
+        cancelled: ConversionState = ConversionState.Ready(INPUT),
+    ): ConversionState = conversionStateFrom(
+        ConversionUpdate(
+            state = state,
+            // Modelled on the call site, which reads `getInt(KEY_PROGRESS, 0)` -- so "no progress
+            // published" is the default reaching the mapping, not a null it has to handle.
+            progressPercent = progress ?: 0,
+            runAttemptCount = runAttemptCount,
+            outputData = data,
+        ),
+        input = INPUT,
+        cancelled = cancelled,
+        fallbackSpec = FALLBACK_SPEC,
+    )
+
+    private companion object {
+        val INPUT = InputFile(Uri.parse("content://test/holiday.mov"), "holiday.mov", 4096L)
+        val FALLBACK_SPEC = OutputFormat.MP4_H265.spec
+    }
+}

app/src/test/java/org/libremediaconverter/convert/Media3MuxerGuardTest.kt

@@ -0,0 +1,99 @@
+package org.libremediaconverter.convert
+
+import android.net.Uri
+import androidx.media3.common.util.UnstableApi
+import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withTimeout
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNotEquals
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.libremediaconverter.model.AudioCodec
+import org.libremediaconverter.model.AudioPlan
+import org.libremediaconverter.model.Container
+import org.libremediaconverter.model.ConversionRequest
+import org.libremediaconverter.model.CopyPlanner
+import org.libremediaconverter.model.InputProbe
+import org.libremediaconverter.model.OutputSpec
+import org.libremediaconverter.model.VideoCodec
+import org.libremediaconverter.model.VideoPlan
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import java.io.File
+import java.util.concurrent.CancellationException
+
+/**
+ * A job that reached Media3 with a container Media3 cannot mux.
+ *
+ * [Media3Muxers]' own KDoc names the defect this guards: *"the router claimed five containers while
+ * the engine silently wrote MP4 for all of them."* `factoryFor` answers null for fourteen of the
+ * app's containers, and `buildTransformer` turns that null into a failed job rather than letting
+ * `Transformer` fall back to its default muxer.
+ *
+ * The guard had never fired. `Media3Engine$buildTransformer$3` -- the `requireNotNull` message
+ * lambda -- was four lines and four branches at 0%, which is to say the entire repair for a defect
+ * the codebase went to the trouble of writing down was untested. Weakening it would restore that
+ * bug silently, because the wrong output is a *playable file with the wrong container*, not a crash.
+ *
+ * Same harness and same two disciplines as [Media3EngineEmptyCompositionTest]: assert the plan
+ * really is the one the test needs before driving the engine, and rule out
+ * `CancellationException` so an unresumed continuation cannot read as a pass.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class Media3MuxerGuardTest {
+
+    @Test
+    fun `a container Media3 cannot mux fails the job rather than silently writing MP4`() {
+        val context = RuntimeEnvironment.getApplication()
+        val engine = Media3Engine(context)
+        val request = ConversionRequest(
+            spec = OutputSpec(Container.WEBM, VideoCodec.VP9, AudioCodec.OPUS),
+            probe = InputProbe(videoCodec = "h264", audioCodec = "aac", container = Container.MP4),
+        )
+
+        // The premise, asserted rather than assumed -- three separate ways this test could pass
+        // over a path it never entered.
+        val plan = CopyPlanner.plan(request.spec, request.probe)
+        assertEquals("the plan has to still be WebM by the time the engine sees it", Container.WEBM, plan.container)
+        assertNull("...and Media3 really has no muxer for it", Media3Muxers.factoryFor(plan.container))
+        // Not the empty-composition refusal, which fires earlier and is a different test's subject.
+        assertNotEquals(VideoPlan.Drop, plan.video)
+        assertNotEquals(AudioPlan.Drop, plan.audio)
+
+        val failure = try {
+            runCatching {
+                runBlocking {
+                    withTimeout(TIMEOUT_MS) {
+                        engine.transcode(Uri.parse("file:///dev/null"), File(context.cacheDir, "guard.webm"), request) {
+                        }
+                    }
+                }
+            }.exceptionOrNull()
+        } finally {
+            engine.close()
+        }
+
+        assertFalse(
+            "the continuation was never resumed -- the refusal escaped instead of failing the job: $failure",
+            failure is CancellationException,
+        )
+        // Type *and* message, and the message half is the load-bearing one. Replacing the
+        // requireNotNull with a fallback factory does not make the export succeed here: it lets it
+        // run on and fail some other way, which a bare type assertion would happily accept.
+        assertTrue("expected the muxer guard to refuse the job, got $failure", failure is IllegalArgumentException)
+        assertTrue(
+            "the refusal has to name the container it could not mux, got: ${failure?.message}",
+            failure?.message.orEmpty().contains("cannot mux") &&
+                failure?.message.orEmpty().contains(Container.WEBM.name),
+        )
+    }
+
+    private companion object {
+        /** Nothing is decoded or muxed on this path -- the guard refuses before any of that. */
+        const val TIMEOUT_MS = 10_000L
+    }
+}

app/src/test/java/org/libremediaconverter/convert/MediaProbeMergeTest.kt

@@ -0,0 +1,166 @@
+package org.libremediaconverter.convert
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.libremediaconverter.model.Container
+import org.libremediaconverter.model.InputKind
+import org.libremediaconverter.model.InputProbe
+
+/**
+ * Which of the two probes wins, when they disagree.
+ *
+ * [MediaProbe.probe] runs `MediaExtractor` and FFprobe independently and then merges the two, and
+ * every rule in that merge is a decision. None of them had a test, for a reason that is structural
+ * rather than an oversight: `RemuxTest` drives the whole thing on a device against committed
+ * fixtures, but only ever with **one probe answering and the other agreeing or also failing**.
+ * Nothing on any source set can arrange for a real extractor and a real FFprobe to disagree, so
+ * every elvis in the merge was taken in one direction and never the other.
+ *
+ * Cutting `merge` out of `probe` is what makes the question askable. Both halves of its signature
+ * had to become `internal` for that -- `Extracted` already was, with a KDoc giving this exact
+ * reason; `FFprobeInfo` simply never got the same treatment.
+ */
+class MediaProbeMergeTest {
+
+    /**
+     * The rule with the loudest failure mode, and `isImageFormat`'s own KDoc names it: a false
+     * positive here "makes the source-info card describe a video as an image". So the image verdict
+     * has to beat a real video codec from the extractor, and the ordering that makes it do so is
+     * the first arm of `classify` rather than anything a reader would infer from the fields.
+     */
+    @Test
+    fun `an image verdict from FFprobe beats a video codec from the extractor`() {
+        val merged = MediaProbe.merge(
+            extracted = extracted(video = "h264"),
+            info = info(video = "mjpeg", isImage = true),
+        )
+
+        assertEquals(InputKind.IMAGE, merged.kind)
+    }
+
+    @Test
+    fun `the extractor wins on codecs, because it is the view the router will act on`() {
+        val merged = MediaProbe.merge(
+            extracted = extracted(video = "h264", audio = "aac"),
+            info = info(video = "hevc", audio = "mp3"),
+        )
+
+        assertEquals("h264", merged.videoCodec)
+        assertEquals("aac", merged.audioCodec)
+    }
+
+    @Test
+    fun `FFprobe answers for a file the extractor could not open`() {
+        val merged = MediaProbe.merge(extracted = null, info = info(video = "vp9", audio = "opus"))
+
+        assertEquals("vp9", merged.videoCodec)
+        assertEquals("opus", merged.audioCodec)
+        assertEquals(InputKind.VIDEO, merged.kind)
+    }
+
+    @Test
+    fun `the extractor answers for a file FFprobe could not read`() {
+        val merged = MediaProbe.merge(extracted = extracted(video = "h264", audio = "aac"), info = null)
+
+        assertEquals("h264", merged.videoCodec)
+        assertEquals("aac", merged.audioCodec)
+        assertNull("only FFprobe can name the container, so it stays unknown here", merged.container)
+    }
+
+    /**
+     * The larger of the two, not the first non-zero.
+     *
+     * Either probe can report zero for a file the other times correctly, and a zero duration makes
+     * the FFmpeg progress percentage undefined -- `FFmpegEngine` divides by it. Both orderings are
+     * asserted because "take the extractor's" and "take the larger" agree in one direction and not
+     * the other, and only one of them is the rule.
+     */
+    @Test
+    fun `duration is the longer of the two readings, whichever probe supplied it`() {
+        assertEquals(
+            5_000L,
+            MediaProbe.merge(extracted(duration = 0L), info(duration = 5_000L)).durationMs,
+        )
+        assertEquals(
+            5_000L,
+            MediaProbe.merge(extracted(duration = 5_000L), info(duration = 0L)).durationMs,
+        )
+    }
+
+    @Test
+    fun `dimensions come from the extractor, and from FFprobe only when it has none`() {
+        assertEquals(1920, MediaProbe.merge(extracted(width = 1920), info(width = 640)).width)
+        assertEquals(640, MediaProbe.merge(extracted = null, info = info(width = 640)).width)
+        assertEquals(0, MediaProbe.merge(extracted(width = 0), info(width = 0)).width)
+    }
+
+    @Test
+    fun `the container comes from FFprobe, which is the only probe that can name one`() {
+        val merged = MediaProbe.merge(extracted(video = "h264"), info(container = Container.MKV))
+
+        assertEquals(Container.MKV, merged.container)
+    }
+
+    @Test
+    fun `a file with audio and no video is audio-only, not unparseable`() {
+        val merged = MediaProbe.merge(extracted(video = null, audio = "mp3"), info = null)
+
+        assertEquals(InputKind.AUDIO_ONLY, merged.kind)
+        assertFalse(merged.hasVideo)
+    }
+
+    @Test
+    fun `a file neither probe could open is the one unreadable answer`() {
+        val merged = MediaProbe.merge(extracted = null, info = null)
+
+        assertEquals(MediaProbe.UNREADABLE, merged)
+        assertEquals(InputProbe.UNPARSEABLE, merged.videoCodec)
+    }
+
+    /**
+     * The arm the ticket was filed for: parsed, and carrying no stream either probe recognised.
+     *
+     * Distinct from "neither probe could open it" -- here the extractor opened the file happily and
+     * found nothing convertible, which is what a container holding only subtitles looks like. It
+     * has to reach the same [MediaProbe.UNREADABLE] answer, because the router keys off that and
+     * there is nothing here for Media3 to do either way.
+     *
+     * Its input was already being built elsewhere in the suite -- `MediaProbeTrackWalkTest` calls
+     * `extractedFrom(emptyList())` and gets exactly this -- and had simply never been handed to the
+     * merge.
+     */
+    @Test
+    fun `a file that parsed but carries no recognised stream is unreadable too`() {
+        val merged = MediaProbe.merge(extracted = MediaProbe.extractedFrom(emptyList()), info = null)
+
+        assertEquals(InputKind.UNPARSEABLE, merged.kind)
+        assertEquals(MediaProbe.UNREADABLE, merged)
+    }
+
+    @Test
+    fun `hasVideo follows the codec that survived the merge, not either probe alone`() {
+        assertTrue(MediaProbe.merge(extracted(video = null), info(video = "vp9")).hasVideo)
+        assertFalse(MediaProbe.merge(extracted(video = null, audio = "aac"), info(video = null)).hasVideo)
+    }
+
+    private fun extracted(
+        video: String? = "h264",
+        audio: String? = "aac",
+        duration: Long = 1_000L,
+        width: Int = 1280,
+        height: Int = 720,
+    ) = MediaProbe.Extracted(video, audio, duration, width, height)
+
+    private fun info(
+        container: Container? = null,
+        video: String? = "h264",
+        audio: String? = "aac",
+        duration: Long = 1_000L,
+        width: Int = 1280,
+        height: Int = 720,
+        isImage: Boolean = false,
+    ) = MediaProbe.FFprobeInfo(container, video, audio, duration, width, height, isImage)
+}

app/src/test/java/org/libremediaconverter/convert/MediaProbeTrackWalkTest.kt

@@ -0,0 +1,221 @@
+package org.libremediaconverter.convert
+
+import android.media.MediaFormat
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+
+/**
+ * The rules `MediaProbe` applies to a set of track formats.
+ *
+ * ## Why this exists, and what it revises
+ *
+ * Issue #84 classified `probeWithExtractor` and `probeForConcat` as device-bound and explicitly not
+ * a gap:
+ *
+ * > These are exercised by `RemuxTest`, `ConcatEngineTest` and `RealMediaBenchmark` in
+ * > `androidTest` … **Do not read their 0% as untested.**
+ *
+ * That was right about the measurement boundary and right about FFprobe. It was not right that
+ * these are only orchestration. The track walk is a **branch matrix**, and `androidTest` reaches it
+ * only through whatever the committed fixtures happen to contain — so none of the rules below is
+ * *chosen* by any test there. A fixture with two video tracks, a track that omits its duration, or
+ * an audio-before-video ordering is not something a device test would produce on purpose.
+ *
+ * The seam is the answer #133 preferred over driving `ShadowMediaExtractor`: the walk is a pure
+ * function over `List<MediaFormat>`, and what is left needing a device — `setDataSource`,
+ * `getTrackFormat`, `release` — is the thin edge `androidTest` should be covering. This is the
+ * `work/FailureOutcome.kt` pattern `CLAUDE.md` names.
+ *
+ * `MediaFormat` is a real one throughout, not a stub. `MediaProbeTrackFieldsTest` records why that
+ * matters: it is a heterogeneous map whose getters throw rather than coerce, and a hand-rolled
+ * double would not reproduce that.
+ */
+@RunWith(RobolectricTestRunner::class)
+class MediaProbeTrackWalkTest {
+
+    // --- extractedFrom: the conversion flow's read ---------------------------
+
+    @Test
+    fun `the first video track wins when a file carries two`() {
+        // `video == null` is the entire guard. A file with two video tracks must report the first,
+        // because that is the one an engine will transcode -- and the width and height must come
+        // from the same track, not be mixed across them.
+        val extracted = MediaProbe.extractedFrom(
+            listOf(
+                video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1920, height = 1080),
+                video(MediaFormat.MIMETYPE_VIDEO_HEVC, width = 640, height = 480),
+            ),
+        )
+
+        assertEquals("h264", extracted.videoCodec)
+        assertEquals(1920, extracted.width)
+        assertEquals(1080, extracted.height)
+    }
+
+    @Test
+    fun `the first audio track wins when a file carries two`() {
+        val extracted = MediaProbe.extractedFrom(
+            listOf(
+                audio(MediaFormat.MIMETYPE_AUDIO_AAC),
+                audio(MediaFormat.MIMETYPE_AUDIO_OPUS),
+            ),
+        )
+
+        assertEquals("aac", extracted.audioCodec)
+    }
+
+    @Test
+    fun `duration is the longest track, not the first or the last`() {
+        // A file whose audio outlasts its video is ordinary. Taking the video's length would cut
+        // the progress bar short; taking the last track's would be right only by accident of order.
+        val extracted = MediaProbe.extractedFrom(
+            listOf(
+                video(MediaFormat.MIMETYPE_VIDEO_AVC, durationUs = 10_000_000),
+                audio(MediaFormat.MIMETYPE_AUDIO_AAC, durationUs = 12_500_000),
+                audio(MediaFormat.MIMETYPE_AUDIO_OPUS, durationUs = 1_000_000),
+            ),
+        )
+
+        assertEquals(12_500L, extracted.durationMs)
+    }
+
+    @Test
+    fun `a track that does not declare its duration contributes nothing to it`() {
+        // MediaExtractor omits KEY_DURATION for plenty of real tracks -- MediaProbeTrackFieldsTest
+        // records the same for KEY_FRAME_RATE. Reading a key that is absent is what containsKey
+        // stands between us and.
+        val extracted = MediaProbe.extractedFrom(
+            listOf(
+                video(MediaFormat.MIMETYPE_VIDEO_AVC),
+                audio(MediaFormat.MIMETYPE_AUDIO_AAC, durationUs = 7_000_000),
+            ),
+        )
+
+        assertEquals(7_000L, extracted.durationMs)
+    }
+
+    @Test
+    fun `declaring audio before video changes nothing`() {
+        // Track order is a property of the container, not of the content. Both orderings have to
+        // reach the same answer or the same file remuxed twice would probe differently.
+        val videoFirst = MediaProbe.extractedFrom(
+            listOf(
+                video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1280, height = 720),
+                audio(MediaFormat.MIMETYPE_AUDIO_AAC),
+            ),
+        )
+        val audioFirst = MediaProbe.extractedFrom(
+            listOf(
+                audio(MediaFormat.MIMETYPE_AUDIO_AAC),
+                video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1280, height = 720),
+            ),
+        )
+
+        assertEquals(videoFirst.videoCodec, audioFirst.videoCodec)
+        assertEquals(videoFirst.audioCodec, audioFirst.audioCodec)
+        assertEquals(videoFirst.width, audioFirst.width)
+        assertEquals(videoFirst.height, audioFirst.height)
+    }
+
+    @Test
+    fun `a track that is neither audio nor video is ignored`() {
+        // Subtitle and timed-metadata tracks are common in MKV and MP4. Neither prefix matches, so
+        // neither slot is filled -- and, importantly, a subtitle track must not be mistaken for the
+        // absence of an audio track by some later `else`.
+        val extracted = MediaProbe.extractedFrom(
+            listOf(
+                MediaFormat().apply { setString(MediaFormat.KEY_MIME, "text/vtt") },
+                video(MediaFormat.MIMETYPE_VIDEO_AVC),
+            ),
+        )
+
+        assertEquals("h264", extracted.videoCodec)
+        assertNull(extracted.audioCodec)
+    }
+
+    @Test
+    fun `a file with no tracks reports nothing rather than zero-width video`() {
+        val extracted = MediaProbe.extractedFrom(emptyList())
+
+        assertNull(extracted.videoCodec)
+        assertNull(extracted.audioCodec)
+        assertEquals(0L, extracted.durationMs)
+        assertEquals(0, extracted.width)
+        assertEquals(0, extracted.height)
+    }
+
+    @Test
+    fun `an audio-only file reports no video codec at all`() {
+        // The distinction MediaProbe.classify turns into InputKind.AUDIO_ONLY, and the reason
+        // `hasVideo` exists: an audio file and a corrupt file must not look alike.
+        val extracted = MediaProbe.extractedFrom(listOf(audio(MediaFormat.MIMETYPE_AUDIO_AAC)))
+
+        assertNull(extracted.videoCodec)
+        assertEquals("aac", extracted.audioCodec)
+        assertEquals(0, extracted.width)
+    }
+
+    // --- concatInputFrom: the join flow's read -------------------------------
+
+    @Test
+    fun `the join read takes frame rate from the first video track`() {
+        val input = MediaProbe.concatInputFrom(
+            listOf(
+                video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1920, height = 1080, frameRate = 30),
+                video(MediaFormat.MIMETYPE_VIDEO_HEVC, width = 640, height = 480, frameRate = 60),
+                audio(MediaFormat.MIMETYPE_AUDIO_AAC),
+            ),
+        )
+
+        assertEquals("h264", input.videoCodec)
+        assertEquals("aac", input.audioCodec)
+        assertEquals(1920, input.width)
+        assertEquals(1080, input.height)
+        assertEquals(30, input.frameRate)
+    }
+
+    @Test
+    fun `a video track with no declared frame rate reports zero rather than guessing`() {
+        // ConcatPlanner treats 0 as "cannot prove a match" and re-encodes. A guessed 30 would read
+        // as agreement and produce a stream copy of clips that do not actually match -- the failure
+        // its KDoc says the whole flow is arranged to avoid.
+        val input = MediaProbe.concatInputFrom(listOf(video(MediaFormat.MIMETYPE_VIDEO_AVC)))
+
+        assertEquals(0, input.frameRate)
+    }
+
+    @Test
+    fun `a file with no tracks joins as entirely unknown`() {
+        val input = MediaProbe.concatInputFrom(emptyList())
+
+        assertNull(input.videoCodec)
+        assertNull(input.audioCodec)
+        assertEquals(0, input.width)
+        assertEquals(0, input.height)
+        assertEquals(0, input.frameRate)
+    }
+
+    private fun video(
+        mime: String,
+        width: Int = 1920,
+        height: Int = 1080,
+        durationUs: Long? = null,
+        frameRate: Int? = null,
+    ): MediaFormat = MediaFormat.createVideoFormat(mime, width, height).apply {
+        durationUs?.let { setLong(MediaFormat.KEY_DURATION, it) }
+        frameRate?.let { setInteger(MediaFormat.KEY_FRAME_RATE, it) }
+    }
+
+    private fun audio(mime: String, durationUs: Long? = null): MediaFormat =
+        MediaFormat.createAudioFormat(mime, SAMPLE_RATE, CHANNELS).apply {
+            durationUs?.let { setLong(MediaFormat.KEY_DURATION, it) }
+        }
+
+    private companion object {
+        const val SAMPLE_RATE = 48_000
+        const val CHANNELS = 2
+    }
+}

app/src/test/java/org/libremediaconverter/convert/OutputPublisherPublishTest.kt

@@ -232,6 +232,12 @@ class OutputPublisherPublishTest {
             RowShape.NO_SIZE_COLUMN to "a cursor with no SIZE column",
             RowShape.NULL_SIZE to "a cursor whose SIZE cell is null",
             RowShape.NO_ROWS to "a cursor holding no rows",
+            // The third case the KDoc names -- "a resolver call that throws" -- and the one the
+            // list was missing. It reaches `?: false` through `runCatching` rather than through a
+            // cursor answer, so it is the only one of the four that proves the catch is load
+            // bearing: a provider that revokes its grant between the picker and the write must not
+            // have its document deleted on the way out.
+            RowShape.QUERY_THROWS to "a provider that throws out of query",
         ).forEach { (shape, description) ->
             FakeSafProvider.deleteRequests.clear()
             FakeSafProvider.backingFile(documentUri).writeBytes(ByteArray(0))

app/src/test/java/org/libremediaconverter/convert/ScreenWiringTest.kt

@@ -0,0 +1,241 @@
+package org.libremediaconverter.convert
+
+import android.app.Application
+import android.net.Uri
+import androidx.media3.common.util.UnstableApi
+import androidx.work.Data
+import androidx.work.workDataOf
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotEquals
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.libremediaconverter.join.JoinState
+import org.libremediaconverter.join.JoinViewModel
+import org.libremediaconverter.join.joinActions
+import org.libremediaconverter.model.AudioCodec
+import org.libremediaconverter.model.Container
+import org.libremediaconverter.model.EnginePreference
+import org.libremediaconverter.model.OutputFormat
+import org.libremediaconverter.model.QualityTier
+import org.libremediaconverter.model.VideoCodec
+import org.libremediaconverter.work.ConcatWorker
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+
+/**
+ * That each affordance is wired to the ViewModel method it is named after.
+ *
+ * ## What this covers that no other test can
+ *
+ * `ConverterScreenContentTest`, `ConverterStateAffordancesTest` and `JoinScreenContentTest` all
+ * drive the **stateless** content composables, which build their own `ConverterActions`. So the
+ * wiring — the list of `viewModel::` references the stateful outer hands down — was seen by nothing
+ * in the suite.
+ *
+ * ## The hazard is narrower than "seventeen bindings", and this says so
+ *
+ * #156 was filed claiming a transposition of any two bindings would survive the suite. That is not
+ * true, and it was worth checking rather than testing on the assumption:
+ *
+ * | swap | result |
+ * |---|---|
+ * | `onVideoCodec` ↔ `onAudioCodec` | **rejected by the compiler** |
+ * | `onCancel` ↔ `onReset` | **compiles** |
+ *
+ * Every typed binding — container, both codecs, preset, suggestion, quality, engine preference —
+ * takes a distinct parameter type, so the compiler is already the test. Writing assertions for
+ * those would be theatre.
+ *
+ * **The `() -> Unit` bindings are the real gap**, because they are interchangeable to the compiler:
+ * two on the converter screen (`onCancel`, `onReset`) and three on the join screen (`onJoin`,
+ * `onCancel`, `onReset`). A Cancel that discards the finished file, or a Join that cancels, is a
+ * one-character mistake that ships.
+ *
+ * ## How they are told apart
+ *
+ * By effect, not by a recording double. `reset()` sets the state to `Idle`; `cancel()` with no
+ * active job leaves it alone (`ConversionViewModel.cancel` is `activeWorkId?.let(...)`, and
+ * `SettingsEditsTest` pins that). Driving each from a non-`Idle` state is therefore enough to say
+ * which one ran.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class ScreenWiringTest {
+
+    private lateinit var app: Application
+
+    @Before
+    fun setUp() {
+        app = RuntimeEnvironment.getApplication()
+        ConversionDependencies.publisher = { RecordingPublisher(app) }
+        ConversionDependencies.probe = { _, _ -> org.libremediaconverter.model.InputProbe() }
+    }
+
+    @After
+    fun tearDown() {
+        ConversionDependencies.reset()
+    }
+
+    // --- the converter screen ----------------------------------------------
+
+    @Test
+    fun `Start over resets, and Cancel does not`() {
+        // The transposition that compiles. If onReset were bound to cancel, this stays on Ready.
+        installTestWorkManager(app, Data.EMPTY)
+        val pick = ParkedPickDispatcher()
+        val viewModel = ConversionViewModel(app, pickDispatcher = pick)
+        val actions = converterActions(viewModel, onPickInput = {}, onConvert = {}, onSave = {})
+        viewModel.onInputPicked(INPUT_URI)
+        pick.runAll()
+        assertNotEquals(
+            "the fixture needs a non-Idle state or neither action is observable",
+            ConversionState.Idle,
+            viewModel.state.value,
+        )
+
+        actions.onReset()
+
+        assertEquals(ConversionState.Idle, viewModel.state.value)
+    }
+
+    @Test
+    fun `Cancel leaves the picked file on screen`() {
+        // The other half. Without it, a wiring with BOTH actions bound to reset passes the test
+        // above -- and that is exactly what a copy-paste of the wrong line produces.
+        installTestWorkManager(app, Data.EMPTY)
+        val pick = ParkedPickDispatcher()
+        val viewModel = ConversionViewModel(app, pickDispatcher = pick)
+        val actions = converterActions(viewModel, onPickInput = {}, onConvert = {}, onSave = {})
+        viewModel.onInputPicked(INPUT_URI)
+        pick.runAll()
+        val before = viewModel.state.value
+
+        actions.onCancel()
+
+        assertEquals(
+            "Cancel must not throw away the pick the way Start over does",
+            before,
+            viewModel.state.value,
+        )
+    }
+
+    @Test
+    fun `each settings affordance reaches the setting it is named after`() {
+        // The typed bindings. The compiler already rejects a transposition among these, so this is
+        // not that assertion -- it is the cheaper one that each is bound to *something*, and that a
+        // binding dropped to `{}` during an edit would be caught.
+        installTestWorkManager(app, Data.EMPTY)
+        val pick = ParkedPickDispatcher()
+        val viewModel = ConversionViewModel(app, pickDispatcher = pick)
+        val actions = converterActions(viewModel, onPickInput = {}, onConvert = {}, onSave = {})
+
+        actions.onPreset(OutputFormat.WEBM_VP9)
+        assertEquals(OutputFormat.WEBM_VP9.spec, viewModel.settings.value.spec)
+
+        actions.onContainer(Container.MKV)
+        assertEquals(Container.MKV, viewModel.settings.value.spec.container)
+
+        actions.onVideoCodec(VideoCodec.H264)
+        assertEquals(VideoCodec.H264, viewModel.settings.value.spec.videoCodec)
+
+        actions.onAudioCodec(AudioCodec.FLAC)
+        assertEquals(AudioCodec.FLAC, viewModel.settings.value.spec.audioCodec)
+
+        actions.onQuality(QualityTier.BEST)
+        assertEquals(QualityTier.BEST, viewModel.settings.value.quality)
+
+        actions.onEnginePreference(EnginePreference.FORCE_SOFTWARE)
+        assertEquals(EnginePreference.FORCE_SOFTWARE, viewModel.settings.value.enginePreference)
+
+        actions.onSuggestion(OutputFormat.MP4_H264.spec)
+        assertEquals(OutputFormat.MP4_H264.spec, viewModel.settings.value.spec)
+    }
+
+    @Test
+    fun `the launcher-backed actions are the ones the screen supplies`() {
+        // Not wired to the ViewModel at all, deliberately -- they need an ActivityResultLauncher.
+        // Asserted so that a later edit routing one of them at the ViewModel is noticed.
+        installTestWorkManager(app, Data.EMPTY)
+        val pick = ParkedPickDispatcher()
+        val viewModel = ConversionViewModel(app, pickDispatcher = pick)
+        val called = mutableListOf<String>()
+        val actions = converterActions(
+            viewModel,
+            onPickInput = { called += "pick" },
+            onConvert = { called += "convert" },
+            onSave = { called += "save:$it" },
+        )
+
+        actions.onPickInput()
+        actions.onConvert()
+        actions.onSave("holiday.mp4")
+
+        assertEquals(listOf("pick", "convert", "save:holiday.mp4"), called)
+    }
+
+    // --- the join screen, where three are interchangeable -------------------
+
+    @Test
+    fun `Start over resets the join, and Cancel does not`() {
+        installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
+        val pick = ParkedPickDispatcher()
+        val viewModel = JoinViewModel(app, pickDispatcher = pick)
+        val actions = joinActions(viewModel, onPickInputs = {}, onSave = {})
+        viewModel.onInputsPicked(TWO_INPUTS)
+        pick.runAll()
+        assertTrue(
+            "the fixture needs a non-Idle state: ${viewModel.state.value}",
+            viewModel.state.value !is JoinState.Idle,
+        )
+
+        actions.onReset()
+
+        assertEquals(JoinState.Idle, viewModel.state.value)
+    }
+
+    @Test
+    fun `Cancel leaves the picked files on screen`() {
+        installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
+        val pick = ParkedPickDispatcher()
+        val viewModel = JoinViewModel(app, pickDispatcher = pick)
+        val actions = joinActions(viewModel, onPickInputs = {}, onSave = {})
+        viewModel.onInputsPicked(TWO_INPUTS)
+        pick.runAll()
+        val before = viewModel.state.value
+
+        actions.onCancel()
+
+        assertEquals(before, viewModel.state.value)
+    }
+
+    @Test
+    fun `Join starts the job rather than cancelling or resetting it`() {
+        // The third of the join screen's interchangeable trio, and the one whose transposition is
+        // worst: a Join button bound to cancel does nothing at all, which reads as a dead button.
+        installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
+        val pick = ParkedPickDispatcher()
+        val viewModel = JoinViewModel(app, pickDispatcher = pick)
+        val actions = joinActions(viewModel, onPickInputs = {}, onSave = {})
+        viewModel.onInputsPicked(TWO_INPUTS)
+        pick.runAll()
+
+        actions.onJoin()
+
+        assertTrue(
+            "Join must leave Ready for a running state, not sit still and not go Idle: " +
+                "${viewModel.state.value}",
+            viewModel.state.value is JoinState.Joining || viewModel.state.value is JoinState.Joined,
+        )
+    }
+
+    private companion object {
+        val INPUT_URI: Uri = Uri.parse("content://test/holiday.mov")
+        val TWO_INPUTS = listOf(
+            Uri.parse("content://test/a.mp4"),
+            Uri.parse("content://test/b.mp4"),
+        )
+    }
+}

app/src/test/java/org/libremediaconverter/convert/SettingsEditsTest.kt

@@ -0,0 +1,197 @@
+package org.libremediaconverter.convert
+
+import android.app.Application
+import androidx.media3.common.util.UnstableApi
+import androidx.work.Data
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotEquals
+import org.junit.Assert.assertNull
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.libremediaconverter.model.AudioCodec
+import org.libremediaconverter.model.Container
+import org.libremediaconverter.model.EnginePreference
+import org.libremediaconverter.model.OutputFormat
+import org.libremediaconverter.model.QualityTier
+import org.libremediaconverter.model.VideoCodec
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+
+/**
+ * The seven one-line edits the settings sheet makes, and what each one leaves alone.
+ *
+ * ## Why these needed a file of their own
+ *
+ * `setPreset` was covered. The six beside it — `setContainer`, `setVideoCodec`, `setAudioCodec`,
+ * `applySuggestion`, `setQuality`, `setEnginePreference` — and `cancel()` had **no coverage at
+ * all**, which is the tell: they are reachable from the JVM suite by exactly the route `setPreset`
+ * already takes, and nothing had asked.
+ *
+ * ## What is actually being asserted
+ *
+ * Not "the setter sets something". Each of these copies into a nested `OutputSpec`, so the failure
+ * worth catching is **a setter that writes the right value into the wrong field, or that rebuilds
+ * the spec and silently discards the other two**. So every test here asserts the field it changed
+ * *and* that the rest of the spec survived — a `setContainer` implemented as
+ * `it.copy(spec = OutputFormat.MP4_H265.spec.copy(container = container))` would pass a test that
+ * only checked the container.
+ *
+ * `ConverterScreenContentTest` cannot cover this: it builds `ConverterActions` itself and never
+ * touches the ViewModel. That the *screen* calls these is #156's, and neither implies the other.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class SettingsEditsTest {
+
+    private lateinit var app: Application
+    private lateinit var viewModel: ConversionViewModel
+
+    @Before
+    fun setUp() {
+        app = RuntimeEnvironment.getApplication()
+        installTestWorkManager(app, Data.EMPTY)
+        viewModel = ConversionViewModel(app)
+    }
+
+    @After
+    fun tearDown() {
+        ConversionDependencies.reset()
+    }
+
+    @Test
+    fun `choosing a preset replaces the whole spec`() {
+        viewModel.setPreset(OutputFormat.WEBM_VP9)
+
+        assertEquals(OutputFormat.WEBM_VP9.spec, viewModel.settings.value.spec)
+    }
+
+    @Test
+    fun `changing the container leaves both codecs alone`() {
+        // Moved off the default spec first, and that is load-bearing rather than tidiness. The
+        // default IS `OutputFormat.MP4_H265.spec`, so a `setContainer` that rebuilt the spec from
+        // that preset instead of from the current one produced an identical answer and the
+        // mutation went green. Editing the codecs away from the default first is what makes
+        // "the other two survived" an assertion rather than a coincidence.
+        viewModel.setPreset(OutputFormat.WEBM_VP9)
+        val before = viewModel.settings.value.spec
+
+        viewModel.setContainer(Container.MKV)
+
+        val after = viewModel.settings.value.spec
+        assertEquals(Container.MKV, after.container)
+        assertEquals("the video codec is not the container's to change", before.videoCodec, after.videoCodec)
+        assertEquals("the audio codec is not the container's to change", before.audioCodec, after.audioCodec)
+    }
+
+    @Test
+    fun `changing the video codec leaves the container and the audio codec alone`() {
+        // The transposition this guards against is real: setVideoCodec and setAudioCodec take
+        // different enum types, but a copy(...) naming the wrong field compiles wherever the types
+        // happen to line up, and the picker would silently set the other one.
+        val before = viewModel.settings.value.spec
+
+        viewModel.setVideoCodec(VideoCodec.VP9)
+
+        val after = viewModel.settings.value.spec
+        assertEquals(VideoCodec.VP9, after.videoCodec)
+        assertEquals(before.container, after.container)
+        assertEquals(before.audioCodec, after.audioCodec)
+    }
+
+    @Test
+    fun `changing the audio codec leaves the container and the video codec alone`() {
+        val before = viewModel.settings.value.spec
+
+        viewModel.setAudioCodec(AudioCodec.OPUS)
+
+        val after = viewModel.settings.value.spec
+        assertEquals(AudioCodec.OPUS, after.audioCodec)
+        assertEquals(before.container, after.container)
+        assertEquals(before.videoCodec, after.videoCodec)
+    }
+
+    @Test
+    fun `applying a suggestion replaces the spec without disturbing quality or engine`() {
+        // A suggestion comes from ContainerCapabilities when the current spec is invalid, so it is
+        // a whole spec by construction. What it must not do is reset the two settings beside it.
+        viewModel.setQuality(QualityTier.BEST)
+        viewModel.setEnginePreference(EnginePreference.FORCE_SOFTWARE)
+
+        viewModel.applySuggestion(OutputFormat.MKV_H264.spec)
+
+        val settings = viewModel.settings.value
+        assertEquals(OutputFormat.MKV_H264.spec, settings.spec)
+        assertEquals(QualityTier.BEST, settings.quality)
+        assertEquals(EnginePreference.FORCE_SOFTWARE, settings.enginePreference)
+    }
+
+    @Test
+    fun `changing the quality leaves the spec and the engine preference alone`() {
+        // Both neighbours are moved off their defaults first. Asserting against AUTO -- which is
+        // what `ConversionSettings` starts with -- let a `setQuality` that also reset the engine
+        // preference to AUTO pass, because the reset and the survival looked identical.
+        viewModel.setPreset(OutputFormat.WEBM_VP9)
+        viewModel.setEnginePreference(EnginePreference.FORCE_SOFTWARE)
+        val before = viewModel.settings.value.spec
+
+        viewModel.setQuality(QualityTier.BEST)
+
+        val settings = viewModel.settings.value
+        assertEquals(QualityTier.BEST, settings.quality)
+        assertEquals(before, settings.spec)
+        assertEquals(
+            "quality is not the engine preference's to change",
+            EnginePreference.FORCE_SOFTWARE,
+            settings.enginePreference,
+        )
+    }
+
+    @Test
+    fun `changing the engine preference leaves the spec and the quality alone`() {
+        // Off the defaults for the same reason as the test above: QualityTier.FAST is the starting
+        // value, so asserting it here would have been satisfied by a reset as readily as by a
+        // survival.
+        viewModel.setPreset(OutputFormat.WEBM_VP9)
+        viewModel.setQuality(QualityTier.BEST)
+        val before = viewModel.settings.value.spec
+
+        viewModel.setEnginePreference(EnginePreference.FORCE_SOFTWARE)
+
+        val settings = viewModel.settings.value
+        assertEquals(EnginePreference.FORCE_SOFTWARE, settings.enginePreference)
+        assertEquals(before, settings.spec)
+        assertEquals(
+            "the engine preference is not the quality's to change",
+            QualityTier.BEST,
+            settings.quality,
+        )
+    }
+
+    @Test
+    fun `editing past every preset leaves no matching preset`() {
+        // `matchingPreset` is what the settings sheet reads to decide whether to show a preset as
+        // selected or to say "Custom". Editing one field of a preset must drop it out of the list
+        // rather than leaving the old one highlighted.
+        viewModel.setPreset(OutputFormat.MP4_H265)
+        assertEquals(OutputFormat.MP4_H265, viewModel.settings.value.matchingPreset)
+
+        viewModel.setAudioCodec(AudioCodec.FLAC)
+
+        assertNull(
+            "an edited spec is no longer any preset, and the sheet says Custom",
+            viewModel.settings.value.matchingPreset,
+        )
+        assertNotEquals(OutputFormat.MP4_H265.spec, viewModel.settings.value.spec)
+    }
+
+    @Test
+    fun `cancelling with no active job does nothing rather than throwing`() {
+        // `activeWorkId?.let(...)` -- the null side. A user can reach Cancel through a state that
+        // has already finished, and taking the app down for it would be worse than doing nothing.
+        viewModel.cancel()
+
+        assertEquals(ConversionState.Idle, viewModel.state.value)
+    }
+}

app/src/test/java/org/libremediaconverter/convert/UnreadableJoinInputTest.kt

@@ -0,0 +1,70 @@
+package org.libremediaconverter.convert
+
+import android.net.Uri
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.libremediaconverter.model.ConcatPlanner
+import org.libremediaconverter.model.ConcatStrategy
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+
+/**
+ * A clip in a join that nothing could read, from the probe all the way to the strategy.
+ *
+ * Both halves of this are covered already, and separately: `MediaProbeTrackWalkTest` pins what
+ * `concatInputFrom` makes of a track list, and `ConcatPlannerTest`'s
+ * `an unknown codec is not treated as a match` pins what the planner does with a hand-built
+ * `ConcatInput(video = null)`. **Nothing spanned the two**, and the span is the load-bearing part:
+ * the planner's safety rests on the probe really producing that shape, and the hand-built fixture
+ * would go on passing if it stopped.
+ *
+ * Measured rather than asserted: mutating `concatInputFrom`'s initial `video` to a non-null
+ * placeholder leaves `ConcatPlannerTest` green and turns this red.
+ *
+ * ## The asymmetry this protects
+ *
+ * `ConcatPlanner` guards its video check against a null codec (`ConcatStrategy.kt:51`) and its
+ * audio check not at all (`:54`). **That is correct, not an oversight.** `MediaProbe.shortName`
+ * returns a non-null `String`, so in `concatInputFrom` a null `audioCodec` means the track is
+ * *absent* — and two clips with no audio genuinely do match. A null `videoCodec` carries both
+ * meanings, absent or unreadable, which is why only that one is guarded.
+ *
+ * So the audio check is safe *because* the video guard fires first on a clip nothing could read.
+ * Nothing wrote that coupling down and nothing held it.
+ *
+ * ## What this deliberately does not cover
+ *
+ * `probeForConcat`'s `catch` arm (`MediaProbe.kt:300-302`). It is **not reachable on the JVM**:
+ * Robolectric's `MediaExtractor` never throws from `setDataSource`, measured across an
+ * unregistered `content://` authority, a missing `file://`, a file of garbage bytes and an `http://`
+ * URL — all four returned normally with `trackCount = 0`. So the failure arrives here as an empty
+ * track list rather than as an exception, which reaches the same `ConcatInput(null, null, 0, 0, 0)`
+ * by the other road. The catch stays device-only, and this file does not pretend otherwise.
+ */
+@RunWith(RobolectricTestRunner::class)
+class UnreadableJoinInputTest {
+
+    @Test
+    fun `a clip nothing could read probes as unknown, and an unknown clip is re-encoded`() {
+        val unreadable = MediaProbe.probeForConcat(RuntimeEnvironment.getApplication(), UNREADABLE)
+
+        assertNull("an unreadable clip proves nothing about its video codec", unreadable.videoCodec)
+        assertNull("nor about its audio codec", unreadable.audioCodec)
+        assertEquals("nor about its dimensions", 0, unreadable.width)
+        assertEquals(0, unreadable.height)
+        assertEquals(0, unreadable.frameRate)
+
+        assertEquals(
+            "a clip nothing could read is not evidence of a match with anything",
+            ConcatStrategy.REENCODE,
+            ConcatPlanner.plan(listOf(unreadable, unreadable)),
+        )
+    }
+
+    private companion object {
+        /** `content://` so the probe takes the SAF branch a real pick takes. Nothing answers it. */
+        val UNREADABLE: Uri = Uri.parse("content://test/vanished.mp4")
+    }
+}

app/src/test/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilderTest.kt

@@ -166,6 +166,30 @@ class FFmpegCommandBuilderTest {
         assertPair(cmd(OutputFormat.OPUS), "-c:a", "libopus")
     }
 
+    /**
+     * The arm most conversions actually take, and the only one in `audioArgs` with no test.
+     *
+     * `flac wav and opus select the right encoders` above covers the three named arms; MP3 has its
+     * own. AAC arrives through the `else`, so nothing named it and nothing pinned either half of
+     * what it emits -- neither `aac` nor `192k` appeared anywhere in this file. Both are shipped
+     * defaults: MP4 and M4A are the formats the picker offers first, so this is the audio
+     * every ordinary conversion gets.
+     *
+     * The bitrate is asserted as well as the encoder because it is the half a refactor is likelier
+     * to lose. An `-b:a` that quietly changed would not fail anything, would not look wrong in a
+     * command line, and would show up only as files that sound different from the ones the app
+     * produced last month.
+     */
+    @Test
+    fun `aac is the default encoder, at the bitrate the app ships`() {
+        assertPair(cmd(OutputFormat.MP4_H264), "-c:a", "aac")
+        assertPair(cmd(OutputFormat.MP4_H264), "-b:a", "192k")
+        // Through the `else` rather than through a named arm, so an AAC branch added above it later
+        // has to keep answering the same way.
+        assertPair(cmd(OutputFormat.M4A_AAC), "-c:a", "aac")
+        assertPair(cmd(OutputFormat.M4A_AAC), "-b:a", "192k")
+    }
+
     @Test
     fun `audio only formats never carry a video encoder`() {
         listOf(OutputFormat.MP3, OutputFormat.FLAC, OutputFormat.WAV, OutputFormat.OPUS)

app/src/test/java/org/libremediaconverter/join/JoinStateMappingTest.kt

@@ -0,0 +1,200 @@
+package org.libremediaconverter.join
+
+import android.net.Uri
+import androidx.media3.common.util.UnstableApi
+import androidx.work.Data
+import androidx.work.WorkInfo
+import androidx.work.workDataOf
+import org.junit.Assert.assertEquals
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.libremediaconverter.convert.InputFile
+import org.libremediaconverter.model.ConcatStrategy
+import org.libremediaconverter.work.ConcatWorker
+import org.robolectric.RobolectricTestRunner
+
+/**
+ * Every answer [joinStateFrom] can give, chosen rather than stumbled into.
+ *
+ * The join-side twin of `ConversionStateMappingTest`, and the argument is the same one: the mapping
+ * ran on every test that drove a real `ConcatWorker`, but a real worker only ever reaches a terminal
+ * state with well-formed output, so five arms had never been *chosen* by anything.
+ *
+ * ## The one that is not just coverage
+ *
+ * `an unknown strategy name is read as a re-encode rather than thrown` covers a real defect this
+ * seam exposed. The line it replaces was:
+ *
+ * ```kotlin
+ * .getString(ConcatWorker.KEY_STRATEGY)?.let(ConcatStrategy::valueOf) ?: ConcatStrategy.REENCODE
+ * ```
+ *
+ * `valueOf` throws on a name this build does not define, and this runs inside a `viewModelScope`
+ * collect with no handler — so it does not become a `Failed` state, it takes the process down.
+ * `ConcatWorker.kt` had already made this exact change for `KEY_FORMAT` and written down why; the
+ * matching read on this side had not been changed with it.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class JoinStateMappingTest {
+
+    @Test
+    fun `a running join is joining`() {
+        assertEquals(JoinState.Joining(INPUTS), map(WorkInfo.State.RUNNING))
+    }
+
+    @Test
+    fun `a blocked join looks like one that is starting`() {
+        // Folded into the RUNNING arm deliberately: a job waiting on a prerequisite is nothing the
+        // user can act on, and a separate word for it would be noise.
+        assertEquals(JoinState.Joining(INPUTS), map(WorkInfo.State.BLOCKED))
+    }
+
+    @Test
+    fun `an enqueued join that has already run is waiting to retry`() {
+        assertEquals(JoinState.Waiting(INPUTS), map(WorkInfo.State.ENQUEUED, runAttemptCount = 1))
+    }
+
+    @Test
+    fun `an enqueued join that has never run is simply starting`() {
+        // The other side. Without it, a mapping that ignored runAttemptCount passes the test above.
+        assertEquals(JoinState.Joining(INPUTS), map(WorkInfo.State.ENQUEUED, runAttemptCount = 0))
+    }
+
+    @Test
+    fun `a success that named no file is a failure, not an empty success`() {
+        assertEquals(
+            JoinState.Failed(JOINED_WITHOUT_A_FILE_MESSAGE),
+            map(WorkInfo.State.SUCCEEDED, data = Data.EMPTY),
+        )
+    }
+
+    @Test
+    fun `a success carries the strategy the worker actually used`() {
+        // Not cosmetic: the join screen tells the user whether their files were stream-copied or
+        // re-encoded, which is the difference between lossless and lossy.
+        val joined = map(
+            WorkInfo.State.SUCCEEDED,
+            data = workDataOf(
+                ConcatWorker.KEY_OUTPUT_PATH to "/cache/conversions/joined.mp4",
+                ConcatWorker.KEY_STRATEGY to ConcatStrategy.STREAM_COPY.name,
+            ),
+        ) as JoinState.Joined
+
+        assertEquals(ConcatStrategy.STREAM_COPY, joined.strategy)
+    }
+
+    @Test
+    fun `an unknown strategy name is read as a re-encode rather than thrown`() {
+        // The defect. A build that added a third strategy leaves finished joins in the queue naming
+        // it, and WorkManager keeps those about a week -- the premise WorkerEnumFallbackTest and
+        // JobTags are both written on. With `valueOf` this throws IllegalArgumentException inside a
+        // viewModelScope collect that has no handler, so it is not a Failed state, it is a crash.
+        //
+        // REENCODE rather than STREAM_COPY because it is the conservative answer: describing an
+        // unknown join as lossless would be a claim the app cannot support.
+        val joined = map(
+            WorkInfo.State.SUCCEEDED,
+            data = workDataOf(
+                ConcatWorker.KEY_OUTPUT_PATH to "/cache/conversions/joined.mp4",
+                ConcatWorker.KEY_STRATEGY to "SMART_CONCAT_V2",
+            ),
+        ) as JoinState.Joined
+
+        assertEquals(ConcatStrategy.REENCODE, joined.strategy)
+    }
+
+    @Test
+    fun `a success with no strategy at all falls back the same way`() {
+        val joined = map(
+            WorkInfo.State.SUCCEEDED,
+            data = workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/cache/conversions/joined.mp4"),
+        ) as JoinState.Joined
+
+        assertEquals(ConcatStrategy.REENCODE, joined.strategy)
+    }
+
+    @Test
+    fun `a success from older work falls back to the format such a job really used`() {
+        val joined = map(
+            WorkInfo.State.SUCCEEDED,
+            data = workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/cache/conversions/joined.mp4"),
+        ) as JoinState.Joined
+
+        assertEquals(ConcatWorker.outputNameFor(ConcatWorker.DEFAULT_FORMAT), joined.suggestedName)
+        assertEquals(ConcatWorker.DEFAULT_FORMAT.mimeType, joined.mimeType)
+    }
+
+    @Test
+    fun `a blank name or type falls back the same way a missing one does`() {
+        val joined = map(
+            WorkInfo.State.SUCCEEDED,
+            data = workDataOf(
+                ConcatWorker.KEY_OUTPUT_PATH to "/cache/conversions/joined.mp4",
+                ConcatWorker.KEY_SUGGESTED_NAME to "",
+                ConcatWorker.KEY_MIME_TYPE to "  ",
+            ),
+        ) as JoinState.Joined
+
+        assertEquals(ConcatWorker.outputNameFor(ConcatWorker.DEFAULT_FORMAT), joined.suggestedName)
+        assertEquals(ConcatWorker.DEFAULT_FORMAT.mimeType, joined.mimeType)
+    }
+
+    @Test
+    fun `a failure carries the reason the worker gave`() {
+        assertEquals(
+            JoinState.Failed("Not enough free space to join these files."),
+            map(
+                WorkInfo.State.FAILED,
+                data = workDataOf(
+                    ConcatWorker.KEY_ERROR to "Not enough free space to join these files.",
+                ),
+            ),
+        )
+    }
+
+    @Test
+    fun `a failure with nothing said still says something`() {
+        assertEquals(
+            JoinState.Failed(ConcatWorker.GENERIC_FAILURE_MESSAGE),
+            map(WorkInfo.State.FAILED, data = Data.EMPTY),
+        )
+    }
+
+    @Test
+    fun `a failure whose message is blank falls back like a missing one`() {
+        assertEquals(
+            JoinState.Failed(ConcatWorker.GENERIC_FAILURE_MESSAGE),
+            map(WorkInfo.State.FAILED, data = workDataOf(ConcatWorker.KEY_ERROR to " ")),
+        )
+    }
+
+    @Test
+    fun `a cancellation lands wherever the caller said it should`() {
+        // A join started here goes back to Ready with the picked files; one picked up by reattach
+        // goes to Idle, because those URIs belong to a process that no longer exists.
+        assertEquals(
+            JoinState.Ready(INPUTS),
+            map(WorkInfo.State.CANCELLED, cancelled = JoinState.Ready(INPUTS)),
+        )
+        assertEquals(JoinState.Idle, map(WorkInfo.State.CANCELLED, cancelled = JoinState.Idle))
+    }
+
+    private fun map(
+        state: WorkInfo.State,
+        runAttemptCount: Int = 0,
+        data: Data = Data.EMPTY,
+        cancelled: JoinState = JoinState.Ready(INPUTS),
+    ): JoinState = joinStateFrom(
+        JoinUpdate(state = state, runAttemptCount = runAttemptCount, outputData = data),
+        inputs = INPUTS,
+        cancelled = cancelled,
+    )
+
+    private companion object {
+        val INPUTS = listOf(
+            InputFile(Uri.parse("content://test/a.mp4"), "a.mp4", 1024L),
+            InputFile(Uri.parse("content://test/b.mp4"), "b.mp4", 2048L),
+        )
+    }
+}

app/src/test/java/org/libremediaconverter/join/SharedFailureMessagesTest.kt

@@ -0,0 +1,102 @@
+package org.libremediaconverter.join
+
+import android.app.Application
+import android.net.Uri
+import androidx.media3.common.util.UnstableApi
+import androidx.work.ListenableWorker
+import androidx.work.testing.TestListenableWorkerBuilder
+import androidx.work.workDataOf
+import kotlinx.coroutines.runBlocking
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.libremediaconverter.convert.ConversionDependencies
+import org.libremediaconverter.convert.RecordingPublisher
+import org.libremediaconverter.convert.installTestWorkManager
+import org.libremediaconverter.work.ConcatWorker
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+
+/**
+ * The two layers that refuse a short join, refusing it with one sentence.
+ *
+ * ## Why this is not "assert a constant equals itself"
+ *
+ * `ConcatWorker` and `JoinViewModel` both reject a join of fewer than two files, and before #158
+ * each carried **its own copy of the literal**. Only the worker's was pinned — by `RefusedJobTest`,
+ * added in #139 — so the wording on the screen could drift away from the wording in the job with no
+ * test saying anything, for one message the user sees from one condition.
+ *
+ * Sharing a constant makes them agree by construction. What it does *not* do is prove that both
+ * layers still reach it: a refactor that stops `JoinViewModel` refusing at all, or that gives it a
+ * different message, passes any test that only reads `TOO_FEW_INPUTS_MESSAGE`. So each layer is
+ * driven for real here — the ViewModel through `onInputsPicked`, the worker through `doWork` — and
+ * the assertion is that the two answers are **the same string**, taken from two running layers
+ * rather than from one declaration.
+ *
+ * That is the shape `CLAUDE.md` asks for: revert the sharing and this goes red, because the two
+ * sites drift the moment they are allowed to.
+ *
+ * ## Scope
+ *
+ * The arity guard's own behaviour on the ViewModel side — that it refuses one file, that it accepts
+ * two, that it claims ownership first — is #155's, and this deliberately does not duplicate it.
+ * This file is about the *agreement between layers*, which is what #158 changed.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class SharedFailureMessagesTest {
+
+    private lateinit var app: Application
+    private lateinit var viewModel: JoinViewModel
+
+    @Before
+    fun setUp() {
+        app = RuntimeEnvironment.getApplication()
+        ConversionDependencies.publisher = { RecordingPublisher(app) }
+        installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
+        viewModel = JoinViewModel(app)
+    }
+
+    @After
+    fun tearDown() {
+        ConversionDependencies.reset()
+    }
+
+    @Test
+    fun `both layers refuse a one-file join with the same sentence`() {
+        // The ViewModel, refusing before anything is enqueued.
+        viewModel.onInputsPicked(listOf(ONE_FILE))
+        val fromScreen = (viewModel.state.value as JoinState.Failed).message
+
+        // The worker, refusing a job that reached the queue anyway -- which it can, because
+        // ConcatWorker.request(...) takes a List<Uri> and checks nothing about its length.
+        val result = runBlocking { worker(ONE_FILE).doWork() }
+        val fromJob = (result as ListenableWorker.Result.Failure)
+            .outputData.getString(ConcatWorker.KEY_ERROR)
+
+        assertEquals(
+            "the screen and the job must say the same thing about the same refusal",
+            fromScreen,
+            fromJob,
+        )
+        // And that the shared sentence is the one either layer would have written on its own,
+        // rather than both having drifted together to something else.
+        assertEquals(ConcatWorker.TOO_FEW_INPUTS_MESSAGE, fromScreen)
+    }
+
+    private fun worker(vararg inputs: Uri): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
+        context = app,
+        inputData = workDataOf(
+            ConcatWorker.KEY_INPUT_URIS to inputs.map(Uri::toString).toTypedArray(),
+            ConcatWorker.KEY_TOTAL_BYTES to 1024L,
+        ),
+        runAttemptCount = 0,
+    ).build()
+
+    private companion object {
+        val ONE_FILE: Uri = Uri.parse("content://test/holiday.mp4")
+    }
+}

app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt

@@ -451,6 +451,99 @@ class ContainerCapabilitiesTest {
         }
     }
 
+    /**
+     * The video twin of `no audio track is accepted by every container in both modes`.
+     *
+     * Dead in production today, and deliberately so: every caller guards `NONE` before asking the
+     * matrix, so nothing reaches this arm through the app. **The asymmetry is the argument, not the
+     * reachability** -- its audio counterpart at the top of the same `when` has had a dedicated
+     * test since #136, and one of a matched pair being covered is how a later reader concludes the
+     * other was considered and exempted. It was not; it was simply missed.
+     *
+     * Not the same shape as the two `COPY -> error(...)` arms, which `docs/coverage-read-findings.md`
+     * records as a named exemption (F4). Those are guards that must not be provokable. This is a
+     * documented answer -- "no video track fits anywhere" -- and an answer is a thing to pin.
+     */
+    @Test
+    fun `no video track is accepted by every container in both modes`() {
+        Container.entries.forEach { container ->
+            listOf(CodecMode.COPY, CodecMode.ENCODE).forEach { mode ->
+                assertTrue(
+                    "$container should accept no video track ($mode)",
+                    ContainerCapabilities.accepts(container, VideoCodec.NONE, mode),
+                )
+            }
+        }
+    }
+
+    /**
+     * A suggestion that keeps the codec the user asked for, rather than falling back to the
+     * container's first encodable one.
+     *
+     * `repairVideo`'s third arm -- "the request is not a copy, and this container can encode it" --
+     * is the one that preserves intent, and it was the only arm of the four nothing reached. The
+     * property test above executes `repairVideo` on every case it walks and lands elsewhere each
+     * time: an explicit COPY that works, a source the container can carry untouched, or no video
+     * track at all.
+     *
+     * The route is indirect because it is the only one the app has. VP9 into WebM is a perfectly
+     * good video request; what makes it invalid is the *audio* -- WebM carries Opus and Vorbis, not
+     * AAC. So `validateAudio` refuses, `suggestions` looks for a container that can hold what was
+     * asked for, and MP4 can encode VP9. The suggestion has to come back carrying VP9: swapping to
+     * the container's first encodable codec would discard the choice the user made.
+     */
+    @Test
+    fun `a repaired suggestion keeps the video codec the user chose`() {
+        val invalid = ContainerCapabilities.validate(
+            OutputSpec(Container.WEBM, VideoCodec.VP9, AudioCodec.AAC),
+            h264Source,
+        )
+
+        assertTrue("WebM cannot hold AAC, so this spec is invalid", invalid is Validation.Invalid)
+        val suggestions = (invalid as Validation.Invalid).suggestions
+        assertTrue(
+            "expected a suggestion that still encodes VP9, got $suggestions",
+            suggestions.any { it.videoCodec == VideoCodec.VP9 },
+        )
+        assertEverySuggestionValid(invalid, h264Source)
+    }
+
+    /**
+     * The fallback in `firstContainerHolding`: when the input's own container cannot hold the
+     * codec the user asked for, any container that can will do.
+     *
+     * The preferred half -- "the container the input already uses" -- is what every other case
+     * reaches, because they all start from a file whose own container carries the codec in
+     * question. The elvis after it had never run.
+     *
+     * AVI is the input that makes it run: AVI predates H.265 and has no mapping for it, so asking
+     * an AVI for H.265 is refused, and the container the input already uses cannot be part of the
+     * answer. Without the fallback the only candidates left are AVI itself and the container
+     * holding the *source* codec -- also AVI -- so the refusal still offers something, but what it
+     * offers is H.264: the app quietly declines the codec the user asked for instead of moving them
+     * to a container that supports it.
+     *
+     * That is why this asserts the codec survives rather than that the list is non-empty. A
+     * non-empty assertion passes with the fallback deleted -- measured, not assumed.
+     */
+    @Test
+    fun `an input whose container cannot hold the requested codec is moved, not downgraded`() {
+        val aviSource = InputProbe(videoCodec = "h264", audioCodec = "aac", container = Container.AVI)
+
+        val invalid = ContainerCapabilities.validate(
+            OutputSpec(Container.AVI, VideoCodec.H265, AudioCodec.AAC),
+            aviSource,
+        )
+
+        assertTrue("AVI has no mapping for H.265", invalid is Validation.Invalid)
+        val suggestions = (invalid as Validation.Invalid).suggestions
+        assertTrue(
+            "expected a container that can actually hold H.265, got $suggestions",
+            suggestions.any { it.videoCodec == VideoCodec.H265 },
+        )
+        assertEverySuggestionValid(invalid, aviSource)
+    }
+
     @Test
     fun `resolving audio COPY before asking the matrix is required`() {
         // The audio twin of `resolving COPY before asking the matrix is required`, and the reason is

app/src/test/java/org/libremediaconverter/ui/TagTableUniquenessTest.kt

@@ -28,6 +28,7 @@ class TagTableUniquenessTest {
     fun `every tag constant has its own value`() {
         val tags = tagsIn(
             TestTags::class.java,
+            TestTags.Shell::class.java,
             TestTags.Converter::class.java,
             TestTags.Join::class.java,
         )

app/src/test/java/org/libremediaconverter/work/ConcatFailureTest.kt

@@ -0,0 +1,237 @@
+package org.libremediaconverter.work
+
+import android.app.Application
+import android.net.Uri
+import androidx.media3.common.util.UnstableApi
+import androidx.work.Data
+import androidx.work.ListenableWorker
+import androidx.work.testing.TestListenableWorkerBuilder
+import androidx.work.workDataOf
+import kotlinx.coroutines.runBlocking
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.libremediaconverter.convert.ConcatJoiner
+import org.libremediaconverter.convert.ConversionDependencies
+import org.libremediaconverter.convert.StagingNames
+import org.libremediaconverter.convert.installTestWorkManager
+import org.libremediaconverter.ffmpeg.ConcatEngine
+import org.libremediaconverter.model.ConcatStrategy
+import org.libremediaconverter.model.OutputFormat
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import java.io.File
+import java.util.UUID
+
+/**
+ * What a join does when the engine fails partway.
+ *
+ * Everything past `ConcatWorker`'s `setForeground` was untested on **every** source set, and the
+ * repo had already measured the cost: `PerJobStagingTest`'s KDoc records that reverting
+ * `ConcatWorker` to a constant staging name left all 257 tests green, because nothing in the JVM
+ * suite can get past a `ConcatEngine` constructed in place. `RefusedJobTest` says the same from the
+ * other side -- "the next thing past the count guard is `ConcatEngine`, which is native".
+ * `ConcatEngineTest` on a device tests the engine directly, bypassing the worker, and
+ * `ConcatWorkerTest` covers only the too-few-inputs guard and the happy path.
+ *
+ * `ConversionDependencies.concat` is the seam that closes it, added here to sit beside the
+ * `.hardware` and `.software` that `ConversionWorker` has had all along -- the asymmetry between the
+ * two workers was the whole reason one of them had a tested failure path and the other did not.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class ConcatFailureTest {
+
+    private lateinit var app: Application
+    private lateinit var publisher: AlwaysRoomPublisher
+
+    @Before
+    fun setUp() {
+        app = RuntimeEnvironment.getApplication()
+        publisher = AlwaysRoomPublisher(app)
+        ConversionDependencies.publisher = { publisher }
+        installTestWorkManager(app, Data.EMPTY)
+    }
+
+    @After
+    fun tearDown() {
+        ConversionDependencies.reset()
+    }
+
+    @Test
+    fun `a join whose engine fails reports the engine's own reason`() {
+        ConversionDependencies.concat = { FailingJoiner { error(DEMUXER_MESSAGE) } }
+
+        val result = runBlocking { joinWorker().doWork() }
+
+        assertEquals(
+            ListenableWorker.Result.failure(workDataOf(ConcatWorker.KEY_ERROR to DEMUXER_MESSAGE)),
+            result,
+        )
+    }
+
+    /**
+     * A failure carrying no message at all, which Kotlin and Java both allow and FFmpegKit's
+     * wrappers can produce.
+     *
+     * Without the fallback the user is shown an empty error, and `JoinViewModel` cannot tell that
+     * from a job that reported nothing -- the two would be one blank screen with different causes.
+     */
+    @Test
+    fun `a failure with no message of its own still says something`() {
+        ConversionDependencies.concat = { FailingJoiner { throw RuntimeException() } }
+
+        val result = runBlocking { joinWorker().doWork() }
+
+        assertEquals(
+            ListenableWorker.Result.failure(
+                workDataOf(ConcatWorker.KEY_ERROR to ConcatWorker.GENERIC_FAILURE_MESSAGE),
+            ),
+            result,
+        )
+    }
+
+    /**
+     * The staged file is deleted on the way out.
+     *
+     * The joiner writes before it fails, exactly as `PartialThenFailingTranscoder` does on the
+     * conversion side: a stub that only threw would let a missing `staged.delete()` pass. What it
+     * costs to lose is a full-size partial per failed join, sitting in cache until the sweep is old
+     * enough to be sure nobody is coming back for it.
+     *
+     * Asserted against the file the joiner was actually handed rather than by scanning the staging
+     * directory for a name. The first draft did scan, for a `"join-"` prefix that
+     * `StagingNames.forJob` does not produce -- it names files `<jobId>.<ext>` -- so the assertion
+     * was trivially true and the mutation walked straight through it.
+     */
+    @Test
+    fun `a failed join leaves nothing behind in staging`() {
+        val joiner = FailingJoiner { error(DEMUXER_MESSAGE) }
+        ConversionDependencies.concat = { joiner }
+
+        runBlocking { joinWorker().doWork() }
+
+        val staged = requireNotNull(joiner.lastOutput) { "the joiner never ran, so this proves nothing" }
+        assertEquals(
+            "the fixture has to write before it fails, or the delete is unobservable",
+            PARTIAL_BYTES,
+            joiner.bytesWritten,
+        )
+        assertFalse("a failed join must not leave its partial behind: $staged", staged.exists())
+    }
+
+    /**
+     * The success path, and the staging name #159's fixture and `PerJobStagingTest` both care about.
+     *
+     * Worth its place rather than a happy-path formality: `PerJobStagingTest`'s KDoc records that
+     * **reverting `ConcatWorker` to a constant staging name left all 257 tests green**, because
+     * nothing could reach the line that names the file. This is the test that was missing when that
+     * was written -- the join's output `Data` had never been read by anything on the JVM.
+     *
+     * The staged path is asserted to carry the job id, not a constant: two joins of the same format
+     * sharing one name is the defect, and `ConcatEngine`'s list file collided harder still.
+     */
+    @Test
+    fun `a join that works reports its own staged file, strategy and name`() {
+        val joiner = SucceedingJoiner()
+        ConversionDependencies.concat = { joiner }
+
+        val result = runBlocking { joinWorker().doWork() }
+
+        assertTrue("got $result", result is ListenableWorker.Result.Success)
+        val data = (result as ListenableWorker.Result.Success).outputData
+        assertEquals(
+            "the staged file has to be this job's, not a name every join shares",
+            File(publisherStagingDir(), StagingNames.forJob(JOB_ID, OutputFormat.MP4_H264.extension)).absolutePath,
+            data.getString(ConcatWorker.KEY_OUTPUT_PATH),
+        )
+        assertEquals(ConcatStrategy.STREAM_COPY.name, data.getString(ConcatWorker.KEY_STRATEGY))
+        assertEquals(OutputFormat.MP4_H264.mimeType, data.getString(ConcatWorker.KEY_MIME_TYPE))
+        assertTrue(
+            "the save dialog needs a name with the right extension, got ${data.getString(
+                ConcatWorker.KEY_SUGGESTED_NAME,
+            )}",
+            data.getString(ConcatWorker.KEY_SUGGESTED_NAME).orEmpty().endsWith(".${OutputFormat.MP4_H264.extension}"),
+        )
+    }
+
+    /**
+     * The arm beside the count guard: no URI array at all.
+     *
+     * Covered today only by `UnopenableUriTest` on a device, although it runs before staging and
+     * before any native code. It is the exact sibling of `RefusedJobTest`'s ConversionWorker twin,
+     * and it belongs on the JVM with it -- a device test for a branch that needs no device is a
+     * slower test that reports later.
+     */
+    @Test
+    fun `a join with no input array at all is refused with a message`() {
+        val result = runBlocking {
+            TestListenableWorkerBuilder<ConcatWorker>(
+                context = app,
+                inputData = workDataOf(ConcatWorker.KEY_FORMAT to OutputFormat.MP4_H264.name),
+                runAttemptCount = 0,
+            ).setId(JOB_ID).build().doWork()
+        }
+
+        assertEquals(
+            ListenableWorker.Result.failure(workDataOf(ConcatWorker.KEY_ERROR to ConcatWorker.NO_INPUTS_MESSAGE)),
+            result,
+        )
+    }
+
+    private fun publisherStagingDir(): File? = publisher.createStagingFile("probe").parentFile
+
+    private fun joinWorker(): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
+        context = app,
+        inputData = workDataOf(
+            ConcatWorker.KEY_INPUT_URIS to arrayOf(FIRST.toString(), SECOND.toString()),
+            ConcatWorker.KEY_TOTAL_BYTES to TOTAL_BYTES,
+            ConcatWorker.KEY_FORMAT to OutputFormat.MP4_H264.name,
+        ),
+        runAttemptCount = 0,
+    ).setId(JOB_ID).build()
+
+    private companion object {
+        val FIRST: Uri = Uri.parse("file:///tmp/one.mp4")
+        val SECOND: Uri = Uri.parse("file:///tmp/two.mp4")
+        const val TOTAL_BYTES = 2048L
+        const val DEMUXER_MESSAGE = "the demuxer rejected the input list"
+        const val PARTIAL_BYTES = 2048
+        val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-00000000000b")
+    }
+}
+
+/** A joiner that writes something and then fails, so a missing `staged.delete()` cannot pass. */
+private class FailingJoiner(private val failure: () -> Nothing) : ConcatJoiner {
+
+    /** The handle the worker created, kept so a test can ask whether it survived the failure. */
+    var lastOutput: File? = null
+    var bytesWritten = 0
+
+    override suspend fun join(inputs: List<Uri>, output: File, format: OutputFormat): ConcatEngine.Result {
+        lastOutput = output
+        output.writeBytes(ByteArray(PARTIAL_BYTES))
+        bytesWritten = PARTIAL_BYTES
+        failure()
+    }
+
+    private companion object {
+        const val PARTIAL_BYTES = 2048
+    }
+}
+
+/** The joiner that finishes, so the success path and the output `Data` can be read on the JVM. */
+private class SucceedingJoiner : ConcatJoiner {
+    override suspend fun join(inputs: List<Uri>, output: File, format: OutputFormat): ConcatEngine.Result {
+        output.writeBytes(ByteArray(OUTPUT_BYTES))
+        return ConcatEngine.Result(ConcatStrategy.STREAM_COPY, output)
+    }
+
+    private companion object {
+        const val OUTPUT_BYTES = 4096
+    }
+}

app/src/test/java/org/libremediaconverter/work/ForegroundTypeRegimeTest.kt

@@ -0,0 +1,88 @@
+package org.libremediaconverter.work
+
+import android.content.pm.ServiceInfo
+import org.junit.Assert.assertEquals
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+
+/**
+ * [ConversionForegroundType.current] answers differently on each of the three API regimes, and
+ * until this file only one of them was ever executed.
+ *
+ * `app/src/test/resources/robolectric.properties` pins the whole JVM suite to `sdk=36`, so every
+ * Robolectric test that reaches a `ForegroundInfo` takes the `mediaProcessing` arm and no other.
+ * The 33 and 34 arms were cold: 3 lines and 3 of 4 branches, measured on `main` at `d354f64`.
+ *
+ * **The instrumented test is not a substitute, and the reason is specific.**
+ * `ConversionWorkerTest.foregroundTypeMatchesTheRunningApiLevel` asserts against whichever API the
+ * leg happens to be — one arm per leg, never the other two — and the legs that would cover 33 and
+ * 34 are the ones issue #122 wedges. `docs/coverage-read-findings.md` records an API 33 run that
+ * reported `received: 60` and `failed: unknown`: the regime *was* exercised, and that leg could
+ * not have said so if it had broken. Four `@Config` classes here pin all three arms
+ * deterministically, in the same `./gradlew` invocation as everything else.
+ *
+ * `minSdk` is 33, so none of these is dead code — each is a device someone is running the app on.
+ *
+ * **SDK 35 is in the list for the boundary, not for the answer.** It shares its answer with 36,
+ * which would make it look redundant. It is not: relaxing `>= VANILLA_ICE_CREAM` to `>` is invisible
+ * at every level except exactly 35, so without this class that mutation survives the suite.
+ */
+@RunWith(RobolectricTestRunner::class)
+@Config(sdk = [33])
+class ForegroundTypeApi33Test {
+
+    /**
+     * Zero rather than a named constant because there is no constant to name: API 33 does not
+     * require a type, and `mediaProcessing` does not exist here to pass. `ForegroundInfo` reads 0
+     * as "no type at all", which is what this regime wants.
+     */
+    @Test
+    fun `api 33 asks for no foreground service type`() {
+        assertEquals(0, ConversionForegroundType.current())
+    }
+}
+
+/**
+ * API 34 makes a type mandatory and still has no `mediaProcessing`, so `dataSync` is the only
+ * sensible fit. See [ForegroundTypeApi33Test] for why this file exists.
+ */
+@RunWith(RobolectricTestRunner::class)
+@Config(sdk = [34])
+class ForegroundTypeApi34Test {
+
+    @Test
+    fun `api 34 falls back to dataSync, the only type that fits`() {
+        assertEquals(ServiceInfo.FOREGROUND_SERVICE_TYPE_DATA_SYNC, ConversionForegroundType.current())
+    }
+}
+
+/**
+ * The first level with `mediaProcessing`, and therefore the one that tells `>=` from `>`.
+ * See [ForegroundTypeApi33Test].
+ */
+@RunWith(RobolectricTestRunner::class)
+@Config(sdk = [35])
+class ForegroundTypeApi35Test {
+
+    @Test
+    fun `api 35 is the first level that takes mediaProcessing`() {
+        assertEquals(ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROCESSING, ConversionForegroundType.current())
+    }
+}
+
+/**
+ * The level the rest of the suite runs at, asserted here rather than assumed — it is the one arm
+ * that was already covered, and leaving it out would make this file look like it is about the old
+ * levels rather than about all three regimes. See [ForegroundTypeApi33Test].
+ */
+@RunWith(RobolectricTestRunner::class)
+@Config(sdk = [36])
+class ForegroundTypeApi36Test {
+
+    @Test
+    fun `api 36 keeps mediaProcessing`() {
+        assertEquals(ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROCESSING, ConversionForegroundType.current())
+    }
+}

app/src/test/java/org/libremediaconverter/work/HardwareFallbackTest.kt

@@ -0,0 +1,226 @@
+package org.libremediaconverter.work
+
+import android.app.Application
+import android.net.Uri
+import androidx.media3.common.util.UnstableApi
+import androidx.work.Data
+import androidx.work.ListenableWorker
+import androidx.work.testing.TestListenableWorkerBuilder
+import kotlinx.coroutines.CancellationException
+import kotlinx.coroutines.runBlocking
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertThrows
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.libremediaconverter.convert.ConversionDependencies
+import org.libremediaconverter.convert.HardwareTranscoder
+import org.libremediaconverter.convert.SoftwareTranscoder
+import org.libremediaconverter.convert.installTestWorkManager
+import org.libremediaconverter.model.Container
+import org.libremediaconverter.model.ConversionRequest
+import org.libremediaconverter.model.DeviceCodecs
+import org.libremediaconverter.model.EnginePreference
+import org.libremediaconverter.model.InputProbe
+import org.libremediaconverter.model.OutputFormat
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import java.io.File
+import java.util.UUID
+
+/**
+ * What happens when the hardware engine does not finish the job.
+ *
+ * `runMedia3OrFallBack` was eleven lines at 0% on the JVM and `isCancellation` had never been
+ * called by any unit test at all. Its own KDoc calls the fallback the protection against vendor
+ * hardware encoders that "cannot be tested for correctness", so it is the branch most likely to
+ * matter on a device nobody here owns — and it was reachable the whole time through
+ * `ConversionDependencies.hardware`, which no unit test had ever used.
+ *
+ * The sharp one is cancellation. `runMedia3OrFallBack` catches `Throwable`, so without the
+ * `isCancellation` re-throw a user cancelling a hardware transcode would have the app quietly
+ * start a *second* conversion in software — the one thing cancelling is supposed to prevent.
+ *
+ * `ForcedFailureTest` covers the failure half on a device. It does not cover the cancellation half,
+ * and this host cannot run it either way.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class HardwareFallbackTest {
+
+    private lateinit var app: Application
+    private lateinit var hardware: RecordingHardwareTranscoder
+    private lateinit var software: RecordingSoftwareTranscoder
+
+    @Before
+    fun setUp() {
+        app = RuntimeEnvironment.getApplication()
+        hardware = RecordingHardwareTranscoder()
+        software = RecordingSoftwareTranscoder()
+        ConversionDependencies.publisher = { AlwaysRoomPublisher(app) }
+        ConversionDependencies.hardware = { hardware }
+        ConversionDependencies.software = { software }
+        // A probe with real codecs, not the default: `InputProbe()` reports UNPARSEABLE, which
+        // PERMISSIVE.canDecode refuses, and the router would send every job here straight to
+        // FFmpeg without any of these tests mentioning why.
+        ConversionDependencies.probe = { _, _ -> H264_SOURCE }
+        ConversionDependencies.deviceCodecs = { DeviceCodecs.PERMISSIVE }
+        installTestWorkManager(app, Data.EMPTY)
+    }
+
+    @After
+    fun tearDown() {
+        ConversionDependencies.reset()
+    }
+
+    @Test
+    fun `a hardware failure runs the job again in software, on a clean staging file`() {
+        hardware.failWith = { error("the vendor encoder produced nothing usable") }
+
+        val result = runBlocking { worker().doWork() }
+
+        assertTrue("the job should still succeed, got $result", result is ListenableWorker.Result.Success)
+        assertEquals("the hardware engine gets exactly one attempt", 1, hardware.attempts)
+        assertEquals("and the job then goes to software", 1, software.attempts)
+        // The `staged.delete()` between the two, asserted where it is observable: FFmpeg must not
+        // find a half-written hardware output sitting at the path it is about to write.
+        assertFalse(
+            "the partial hardware output must be gone before FFmpeg starts",
+            software.outputExistedOnEntry,
+        )
+        assertEquals("the hardware engine is closed either way", 1, hardware.closes)
+    }
+
+    @Test
+    fun `a cancelled hardware transcode is not quietly retried in software`() {
+        hardware.failWith = { throw CancellationException("the user pressed Cancel") }
+
+        assertThrows(CancellationException::class.java) { runBlocking { worker().doWork() } }
+
+        assertEquals("the hardware engine ran", 1, hardware.attempts)
+        assertEquals(
+            "cancelling must not start a second conversion -- that is the whole point of cancelling",
+            0,
+            software.attempts,
+        )
+        assertEquals("and the engine is still closed on the way out", 1, hardware.closes)
+    }
+
+    @Test
+    fun `a hardware transcode that works never reaches the software engine`() {
+        val result = runBlocking { worker().doWork() }
+
+        assertTrue("got $result", result is ListenableWorker.Result.Success)
+        assertEquals(1, hardware.attempts)
+        assertEquals("the fallback is a fallback, not a second pass", 0, software.attempts)
+        assertEquals(1, hardware.closes)
+    }
+
+    /**
+     * #169: the display-name fallback, which reaches further than the notification title.
+     *
+     * `inputData.getString(KEY_DISPLAY_NAME) ?: "input"` had never taken its right-hand side. The
+     * value is not only the foreground notification's title: it feeds `outputNameFor`, so it is
+     * also the filename offered in the user's save dialog. A job enqueued by an older build, or
+     * built by hand, carries no such key.
+     */
+    @Test
+    fun `a job that names no input file still suggests an output name`() {
+        val result = runBlocking { worker(displayName = null).doWork() }
+
+        assertTrue("got $result", result is ListenableWorker.Result.Success)
+        val suggested = (result as ListenableWorker.Result.Success)
+            .outputData.getString(ConversionWorker.KEY_SUGGESTED_NAME)
+        assertTrue(
+            "expected a name built from the fallback, got $suggested",
+            suggested.orEmpty().startsWith("input"),
+        )
+    }
+
+    private fun worker(displayName: String? = DISPLAY_NAME): ConversionWorker {
+        val spec = OutputFormat.MP4_H265.spec
+        val entries = buildMap<String, Any> {
+            put(ConversionWorker.KEY_INPUT_URI, INPUT.toString())
+            displayName?.let { put(ConversionWorker.KEY_DISPLAY_NAME, it) }
+            put(ConversionWorker.KEY_SIZE_BYTES, INPUT_BYTES)
+            put(ConversionWorker.KEY_CONTAINER, spec.container.name)
+            put(ConversionWorker.KEY_VIDEO_CODEC, spec.videoCodec.name)
+            put(ConversionWorker.KEY_AUDIO_CODEC, spec.audioCodec.name)
+            // AUTO rather than FORCE_SOFTWARE, which is what every other worker test uses and is
+            // exactly why this path had no coverage: forcing software never enters the function.
+            put(ConversionWorker.KEY_ENGINE_PREFERENCE, EnginePreference.AUTO.name)
+        }
+        return TestListenableWorkerBuilder<ConversionWorker>(
+            context = app,
+            inputData = Data.Builder().putAll(entries).build(),
+            runAttemptCount = 0,
+        ).setId(JOB_ID).build()
+    }
+
+    private companion object {
+        val INPUT: Uri = Uri.parse("file:///tmp/holiday.mp4")
+        const val DISPLAY_NAME = "holiday.mp4"
+        const val INPUT_BYTES = 1024L
+        val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000009")
+        val H264_SOURCE = InputProbe(
+            videoCodec = "h264",
+            audioCodec = "aac",
+            container = Container.MP4,
+            durationMs = 1_000,
+        )
+    }
+}
+
+/**
+ * A hardware engine that writes something before it fails, and remembers being closed.
+ *
+ * Writing first is the point, exactly as it is for `PartialThenFailingTranscoder`: an engine that
+ * only threw would let a missing `staged.delete()` pass unnoticed.
+ */
+@UnstableApi
+private class RecordingHardwareTranscoder : HardwareTranscoder {
+
+    var attempts = 0
+    var closes = 0
+    var failWith: (() -> Unit)? = null
+
+    override suspend fun transcode(input: Uri, output: File, request: ConversionRequest, onProgress: (Int) -> Unit) {
+        attempts++
+        output.writeBytes(ByteArray(PARTIAL_BYTES))
+        failWith?.invoke()
+    }
+
+    override fun close() {
+        closes++
+    }
+
+    private companion object {
+        const val PARTIAL_BYTES = 2048
+    }
+}
+
+/** The software engine, recording whether the hardware attempt's leftovers were cleared first. */
+private class RecordingSoftwareTranscoder : SoftwareTranscoder {
+
+    var attempts = 0
+    var outputExistedOnEntry = false
+
+    override suspend fun run(
+        request: ConversionRequest,
+        inputPath: String,
+        output: File,
+        durationMs: Long,
+        onProgress: (Int) -> Unit,
+    ) {
+        attempts++
+        outputExistedOnEntry = output.exists()
+        output.writeBytes(ByteArray(OUTPUT_BYTES))
+    }
+
+    private companion object {
+        const val OUTPUT_BYTES = 512
+    }
+}

app/src/test/java/org/libremediaconverter/work/JobSnapshotsTest.kt

@@ -16,6 +16,7 @@ import androidx.work.testing.WorkManagerTestInitHelper
 import androidx.work.workDataOf
 import kotlinx.coroutines.runBlocking
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
 import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
@@ -125,6 +126,39 @@ class JobSnapshotsTest {
         assertEquals(newer.absolutePath, Reattachment.choose(snapshots)?.job?.outputPath)
     }
 
+    /**
+     * A job in the tag query that never recorded an output path at all.
+     *
+     * Distinct from the three cases above, which all *have* a path and differ in what it names. A
+     * job still running, or one that finished without writing its result key, carries no path at
+     * all -- and `getWorkInfosByTagFlow` returns it alongside the finished ones, because the tag is
+     * the worker class and every attempt ever enqueued carries it.
+     *
+     * The guard is the `?.` in `path?.let(::File)`. Without it the null goes straight into a `File`
+     * constructor. What this pins is the consequence rather than the null check: such a job must
+     * not be offered as a result, so `Reattachment.choose` has to walk past it to the job that
+     * really produced a file. Choosing it would put a Converted screen in front of the user with a
+     * Save button that has nothing to save.
+     */
+    @Test
+    fun `a job that recorded no output path is not offered as a result`() {
+        val real = stagedFile("real.mp4", bytes = 4096)
+        finishedWithOutput(real)
+        finishedWithNoOutput()
+
+        val snapshots = snapshots()
+
+        assertEquals("both jobs carry the tag, so both come back", 2, snapshots.size)
+        val silent = snapshots.single { it.outputPath == null }
+        assertFalse("no path means no output, not an empty one", silent.outputExists)
+        assertEquals("and no time either, for the same reason", 0L, silent.outputModifiedAt)
+        assertEquals(
+            "the reattachment has to walk past it to the job that really produced a file",
+            real.absolutePath,
+            Reattachment.choose(snapshots)?.job?.outputPath,
+        )
+    }
+
     private fun snapshots(): List<JobSnapshot> = runBlocking {
         workManager.jobSnapshots(
             tag = ConversionWorker::class.java.name,
@@ -152,6 +186,11 @@ class JobSnapshotsTest {
         ).result.get()
     }
 
+    /** A job that carries the tag and no result key -- still running, or finished without one. */
+    private fun finishedWithNoOutput() {
+        workManager.enqueue(OneTimeWorkRequestBuilder<ConversionWorker>().build()).result.get()
+    }
+
     private companion object {
         /** Two fixed moments a day apart, so the ordering is stated rather than raced for. */
         const val OLDER_MS = 1_700_000_000_000L

app/src/test/java/org/libremediaconverter/work/NotificationProgressTextTest.kt

@@ -0,0 +1,89 @@
+package org.libremediaconverter.work
+
+import android.app.Notification
+import androidx.media3.common.util.UnstableApi
+import androidx.work.Data
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotEquals
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.libremediaconverter.convert.installTestWorkManager
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import java.util.UUID
+
+/**
+ * The two things a progress notification can say, and that they are not the same thing.
+ *
+ * An assertion gap rather than a coverage one, and the distinction is the reason this file exists.
+ * JaCoCo is green on `build`'s `if (indeterminate)`, because `ProgressNotificationTest` drives it
+ * through a real worker -- but that test reads only the notification id and
+ * `Notification.EXTRA_PROGRESS`. **Nothing had ever read the text.** Swapping the two branches, or
+ * collapsing them into one string, passed the entire suite.
+ *
+ * What it costs to get wrong is small and constant: a conversion that has been running for four
+ * minutes still saying "Preparing", or one that has not started reporting yet claiming 0%. Neither
+ * is a crash, and neither would be found by anything else here -- which is exactly the kind of
+ * thing that survives for a long time.
+ *
+ * Nothing else in the suite constructs [ConversionNotifications] directly.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class NotificationProgressTextTest {
+
+    /**
+     * `build` reaches `WorkManager.getInstance` for the Cancel action's PendingIntent, so the
+     * notification cannot be built at all without one. That coupling is why nothing had ever
+     * constructed this class directly and read what it produced.
+     */
+    @Before
+    fun setUp() {
+        installTestWorkManager(RuntimeEnvironment.getApplication(), Data.EMPTY)
+    }
+
+    @Test
+    fun `an indeterminate notification says something different from a measured one`() {
+        val context = RuntimeEnvironment.getApplication()
+        val notifications = ConversionNotifications(context)
+
+        val preparing = notifications.build(JOB_ID, TITLE, percent = 0, indeterminate = true).text()
+        val measured = notifications.build(JOB_ID, TITLE, percent = 42, indeterminate = false).text()
+
+        assertNotEquals(
+            "the two states have to read differently, or the text says nothing at all",
+            preparing,
+            measured,
+        )
+        assertTrue(
+            "a measured notification has to carry its percentage, got \"$measured\"",
+            measured.contains("42"),
+        )
+        assertTrue(
+            "an indeterminate one must not invent one, got \"$preparing\"",
+            !preparing.contains("42") && !preparing.contains("0"),
+        )
+    }
+
+    /**
+     * The title is the caller's, not the builder's -- it is the file the user picked, and it is what
+     * tells two simultaneous conversions apart in the shade.
+     */
+    @Test
+    fun `the notification is titled with the file it is converting`() {
+        val context = RuntimeEnvironment.getApplication()
+
+        val built = ConversionNotifications(context).build(JOB_ID, TITLE, percent = 10)
+
+        assertEquals(TITLE, built.extras.getString(Notification.EXTRA_TITLE))
+    }
+
+    private fun Notification.text(): String = extras.getString(Notification.EXTRA_TEXT).orEmpty()
+
+    private companion object {
+        const val TITLE = "holiday.mp4"
+        val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-00000000000a")
+    }
+}

app/src/test/java/org/libremediaconverter/work/RefusedJobTest.kt

@@ -146,7 +146,7 @@ class RefusedJobTest {
 
         assertEquals(
             ListenableWorker.Result.failure(
-                workDataOf(ConcatWorker.KEY_ERROR to "Pick at least two files to join."),
+                workDataOf(ConcatWorker.KEY_ERROR to ConcatWorker.TOO_FEW_INPUTS_MESSAGE),
             ),
             result,
         )