Add F5: areEnabled() is never called, so it is not a test gap

Found while decomposing #132 into children. It was item 6 there, and it looked like the cheapest item on the list: three cold lines, a KDoc with real user-visible stakes, and a permission Robolectric can flip in one line. grep -rn 'areEnabled' app/src returns the declaration and nothing else. Both workers construct ConversionNotifications and only ever call build(). So the behaviour the KDoc describes -- warning when progress will be invisible -- does not happen, and a test would assert that a function nobody calls returns what the platform told it. Green, vacuous, and worse than nothing, because it would imply the disabled-notification case is handled. Recorded rather than tested, and the summary now names what F1 and F5 have in common: a comment describing behaviour the code lacks, where the tempting fix freezes the wrong answer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merge branch 'main' into docs/coverage-read-findings
2026-08-26 22:03:11 -05:00 · 2026-08-26 22:01:55 -05:00 · 2026-08-26 21:47:17 -05:00 · 2026-08-26 21:21:34 -05:00 · 2026-08-26 21:18:30 -05:00 · 2026-08-26 00:41:44 -05:00
35 changed files with 3113 additions and 131 deletions
@@ -0,0 +1,193 @@
+#!/usr/bin/env bash
+#
+# Exercises e2e-report-shape.sh's baseline counter against fixture source, with no emulator and
+# no CI run. Run it directly:
+#
+#   .github/scripts/e2e-report-shape-test.sh
+#
+# WHY THIS CAN EXIST AT ALL: the counter is a pure function of the working tree. It greps
+# `app/src/androidTest` for `@FailsOnEmulatorApi37` and compares the total against the number
+# committed in FailsOnEmulatorApi37.kt. Nothing about that needs a device, which is the whole
+# reason #120 could be measured rather than argued about.
+#
+# WHY A THROWAWAY REPO ROOT rather than a knob on the script. The report finds its own root from
+# `BASH_SOURCE`, so a copy of it placed at `<root>/.github/scripts/` reads `<root>/app/src/...`.
+# Building that root is three mkdirs and costs the shipped script nothing:
+#
+#   - the REAL script is what runs, byte for byte, so reverting the matcher reddens this test
+#     rather than a testing-only code path beside it;
+#   - no environment variable exists that could point the LIVE count somewhere else, which is
+#     the failure mode #83 built the baseline check to prevent in the first place;
+#   - XML_DIR resolves inside the throwaway root, so a stale app/build/outputs left by a real
+#     run on a developer machine cannot leak into the numbers here.
+#
+# WHAT IT GUARDS (#120). The old matcher looked for the string anywhere on any line, so a KDoc
+# saying `Deliberately not @FailsOnEmulatorApi37` counted as a marked test and every PR got a
+# deviation notice that was wrong. The obvious repair -- count only lines that are nothing but
+# the annotation -- silently stops counting `@FailsOnEmulatorApi37 @Test`, which is legal Kotlin,
+# and undercounting is the direction that hides a genuine new marker. The fixture carries every
+# shape at once -- three that count and three that must not, enumerated in its own header -- so
+# both mistakes fail here instead of on a PR: against testdata/marker-shapes the old matcher says
+# 5, own-line-only says 2, and the shipped one 3.
+#
+# NOT WIRED INTO CI, deliberately and as a known gap. Adding a step to the Static analysis job
+# would add a new way for a gating job to go red, and #120 was explicit that nothing about it may
+# change any job's status or the pass/fail rules. shellcheck still covers this file, since that
+# step reads `git ls-files '*.sh'` rather than a fixed list.
+set -uo pipefail
+
+SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
+REPORT="$SCRIPT_DIR/e2e-report-shape.sh"
+FIXTURE_DIR="$SCRIPT_DIR/testdata/marker-shapes"
+FIXTURE="$FIXTURE_DIR/MarkerShapes.kt"
+
+TMP="$(mktemp -d)"
+# Single quotes: the path is expanded when the trap fires, not when it is set.
+trap 'rm -rf -- "$TMP"' EXIT
+
+failures=0
+
+pass() { printf 'ok    %s\n' "$1"; }
+
+fail() {
+  failures=$((failures + 1))
+  printf 'FAIL  %s\n' "$1"
+  shift
+  printf '        %s\n' "$@"
+}
+
+# assert_contains <name> <haystack> <needle>
+# `case` rather than grep: the strings being matched carry backticks and an em dash, and this
+# way neither the shell nor a regex engine gets an opinion about them.
+assert_contains() {
+  case "$2" in
+    *"$3"*) pass "$1" ;;
+    *) fail "$1" "wanted to find: $3" "in:" "$2" ;;
+  esac
+}
+
+assert_absent() {
+  case "$2" in
+    *"$3"*) fail "$1" "did NOT want to find: $3" "in:" "$2" ;;
+    *) pass "$1" ;;
+  esac
+}
+
+# make_root <marked-tree-dir> <baseline-number>
+# Assembles a throwaway repo root around the given tree and prints its path.
+make_root() {
+  local tree="$1" baseline="$2" root testdir
+  root="$(mktemp -d "$TMP/root.XXXXXX")"
+  testdir="$root/app/src/androidTest/java/org/libremediaconverter"
+  mkdir -p "$root/.github/scripts" "$testdir"
+  cp -- "$REPORT" "$root/.github/scripts/"
+  cp -- "$tree"/*.kt "$testdir/"
+
+  # The synthetic stand-in for the committed baseline. Its KDoc names the marker the way the real
+  # file does -- in brackets, never with an `@` -- because the real file lives inside the tree
+  # being counted, so an `@` spelling here would add a phantom to every number below.
+  cat > "$testdir/FailsOnEmulatorApi37.kt" <<EOF
+package org.libremediaconverter
+
+/** Stand-in for the real marker file. Only [FAILS_ON_EMULATOR_API37_BASELINE] is read. */
+const val FAILS_ON_EMULATOR_API37_BASELINE = $baseline
+EOF
+
+  # A clean, untruncated run of exactly <baseline> tests, all failing -- which is what the
+  # advisory leg looks like when nothing has drifted. It leaves the marked count as the only
+  # field that can deviate, so every assertion below is about the thing under test.
+  cat > "$root/gradle.log" <<EOF
+> Task :app:connectedDebugAndroidTest
+Starting $baseline tests on test(AVD) - 16
+There was $baseline failure(s).
+EOF
+
+  printf '%s\n' "$root"
+}
+
+# run_report <root>  -- stdout of the real script; its summary lands in <root>/summary.md.
+run_report() {
+  E2E_WEDGED_AFTER='' GITHUB_STEP_SUMMARY="$1/summary.md" \
+    bash "$1/.github/scripts/e2e-report-shape.sh" 37 "$1/gradle.log" \
+    "$1/app/src/androidTest/java/org/libremediaconverter/FailsOnEmulatorApi37.kt"
+}
+
+# ---------------------------------------------------------------------------
+# The fixture still carries every shape.
+#
+# Three of the checks below are covered twice over -- deleting a real annotation moves the count
+# and fails a case further down. The two decoys are not: drop the KDoc mention and the count
+# stays 3, so the precision this whole ticket is about would stop being tested and nothing would
+# say so. That asymmetry is why the shapes are asserted by name rather than only by their effect
+# on the total.
+# ---------------------------------------------------------------------------
+fixture_text="$(cat -- "$FIXTURE")"
+assert_contains "fixture: the import"           "$fixture_text" 'import org.libremediaconverter.FailsOnEmulatorApi37'
+assert_contains "fixture: annotation own line"  "$fixture_text" '
+    @FailsOnEmulatorApi37
+    @Test'
+assert_contains "fixture: annotation with @Test on one line" "$fixture_text" '@FailsOnEmulatorApi37 @Test'
+assert_contains "fixture: annotation nested and indented" "$fixture_text" '
+        @FailsOnEmulatorApi37'
+assert_contains "fixture: KDoc mention (this is #120)" "$fixture_text" "* Deliberately not \`@FailsOnEmulatorApi37\`"
+assert_contains "fixture: commented-out annotation" "$fixture_text" '// @FailsOnEmulatorApi37'
+
+# ---------------------------------------------------------------------------
+# 1. The fixture's three real annotations against a baseline of 3: no deviation.
+#
+# This one case fails under both wrong matchers -- the old one counts 5, own-line-only counts 2 --
+# which is why it is first.
+# ---------------------------------------------------------------------------
+root="$(make_root "$FIXTURE_DIR" 3)"
+out="$(run_report "$root")"
+assert_contains "3 real markers, baseline 3: reports a match" "$out" '  baseline: matches (3 expected, 3 failed)'
+assert_absent   "3 real markers, baseline 3: says nothing about the tree" "$out" 'the tree carries'
+assert_contains "3 real markers, baseline 3: summary agrees" \
+  "$(cat -- "$root/summary.md")" '**Matches the committed baseline of 3**'
+
+# ---------------------------------------------------------------------------
+# 2. A fourth REAL annotation. The count has to move and the deviation has to fire.
+#
+# The important half of #120: precision was the bug, but a matcher that stopped noticing a new
+# marker would have been a worse one, silently.
+# ---------------------------------------------------------------------------
+plus_one="$(mktemp -d "$TMP/plusone.XXXXXX")"
+cp -- "$FIXTURE" "$plus_one/"
+cat > "$plus_one/FourthMarker.kt" <<'EOF'
+package org.libremediaconverter.fixture
+
+class FourthMarker {
+    @FailsOnEmulatorApi37
+    @Test
+    fun addedToday() = Unit
+}
+EOF
+root="$(make_root "$plus_one" 3)"
+out="$(run_report "$root")"
+assert_contains "a 4th real marker: the deviation fires, and counts 4" "$out" \
+  "  baseline DEVIATION: the tree carries 4 tests marked \`@FailsOnEmulatorApi37\` but the baseline says 3 — update FAILS_ON_EMULATOR_API37_BASELINE"
+assert_contains "a 4th real marker: the summary carries it too" "$(cat -- "$root/summary.md")" \
+  "- the tree carries 4 tests marked \`@FailsOnEmulatorApi37\` but the baseline says 3"
+
+# ---------------------------------------------------------------------------
+# 3. Delete the same-line annotation and the count must drop to 2.
+#
+# This is the trap, pinned down. `@FailsOnEmulatorApi37 @Test` on one line is what separates the
+# shipped matcher from `^[[:space:]]*@NAME[[:space:]]*$`, and without this case the fixture entry
+# guarding it could be deleted as decoration -- case 1 would then pass under the wrong matcher.
+# Here the same-line entry is worth exactly one, and it is asserted to be.
+# ---------------------------------------------------------------------------
+minus_same_line="$(mktemp -d "$TMP/minus.XXXXXX")"
+sed -e '/@FailsOnEmulatorApi37 @Test/d' -- "$FIXTURE" > "$minus_same_line/MarkerShapes.kt"
+root="$(make_root "$minus_same_line" 3)"
+out="$(run_report "$root")"
+assert_contains "same-line annotation removed: counts 2, so it was worth 1" "$out" \
+  "  baseline DEVIATION: the tree carries 2 tests marked \`@FailsOnEmulatorApi37\` but the baseline says 3 — update FAILS_ON_EMULATOR_API37_BASELINE"
+
+echo
+if [ "$failures" -eq 0 ]; then
+  echo "e2e-report-shape-test.sh: all checks passed"
+  exit 0
+fi
+echo "e2e-report-shape-test.sh: $failures check(s) failed"
+exit 1
@@ -30,16 +30,31 @@
 #
 # Usage:
 #   e2e-report-shape.sh <label> <gradle-log> [<baseline-file>]
+#   E2E_WEDGED_AFTER=<seconds>   the wrapper timeout killed gradle after that many seconds
 #
 # With a third argument the run is compared against the baseline in that file (advisory mode)
 # and a `::notice::` is emitted per deviation. NEVER `::error::`: the advisory job is
 # `continue-on-error: true` and stays that way, and an error annotation would be a new way for
 # a diagnostic to change a conclusion.
+#
+# WHY THE WEDGE ARRIVES AS AN ENV VAR (#118) rather than being read out of the log like every
+# other field: there is nothing in the log to read. A wedge is gradle never returning, so gradle
+# never printed a verdict, never printed a truncation line, and never aborted instrumentation --
+# the log of a wedged leg is the log of a run that simply stops. Measured on job 98035980326:
+# `expected: 59`, `received: 59`, `completed cleanly: yes`, six seconds before the wedge warning,
+# for a leg that the timeout had killed 22 minutes in. Only e2e-run.sh knows, because only it
+# saw `timeout` exit 124, so it says so. Guessing it from a log that ends abruptly would call
+# every cancelled run a wedge.
+#
+# It is read as a STRING and only ever interpolated into one. `[ -n ... ]`, never `-gt`: it
+# crosses a process boundary from a shell that deliberately sets it EMPTY on every non-wedge
+# path, and an arithmetic test on an empty string is the header's rule four paragraphs up.
 set -uo pipefail

 LABEL="${1:-unknown}"
 LOG="${2:-}"
 BASELINE_FILE="${3:-}"
+WEDGED_AFTER="${E2E_WEDGED_AFTER:-}"

 SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
 REPO_ROOT="$(cd -- "$SCRIPT_DIR/../.." && pwd)"
@@ -115,6 +130,13 @@ elif [ -n "$abort_received" ]; then
 elif [ -n "$expected" ] && [ -z "$abort_line" ]; then
  received="$expected"
  received_src="the run was not truncated, so every expected test reported"
+  # ... unless it was killed, in which case "not truncated" is only "gradle never got as far as
+  # saying so". This is the branch the wedged leg in #118 took -- with no XML written yet, the
+  # number is what the runner was TOLD to run, and the source line said the opposite in the same
+  # table that called the leg clean. The number is deliberately left alone: it is still the best
+  # available answer, and only the claim about where it came from was wrong.
+  [ -n "$WEDGED_AFTER" ] \
+    && received_src="no test XML was written and gradle never printed a truncation line — but the leg was killed mid-run, so this is what it was told to run, not what reported"
 fi

 failed="unknown"
@@ -152,6 +174,12 @@ elif [ "$no_run" = "nothing" ]; then
  # "cleanly" would be a lie about a run that left no evidence it happened.
  completed="unknown"
  completed_src="no runner output to read"
+elif [ -n "$WEDGED_AFTER" ]; then
+  # The wedge is checked LAST of the four, so it only ever overrides the `yes`. The two "no"s
+  # above are already right and name the abort, which the wedge row does not; `unknown` is
+  # already right too. A wedge on top of an abort is both facts, and both get printed.
+  completed="**no**"
+  completed_src="the wrapper timeout killed gradle after ${WEDGED_AFTER}s — instrumentation itself was never aborted, which is why nothing in the log says so"
 else
  completed="yes"
  completed_src="no truncation line and no \`INSTRUMENTATION_ABORTED\`"
@@ -180,11 +208,34 @@ if [ -n "$BASELINE_FILE" ]; then
  advisory="yes"
  [ -f "$BASELINE_FILE" ] \
    && baseline="$(sed -nE 's/^const val FAILS_ON_EMULATOR_API37_BASELINE = ([0-9]+).*/\1/p' "$BASELINE_FILE" | head -1)"
-  # The #81 check, verbatim: what the tree actually carries. Reported next to the baseline so a
-  # stale baseline shows up here rather than only once the emulator disagrees with it.
+  # What the tree actually carries. Reported next to the baseline so a stale baseline shows up
+  # here rather than only once the emulator disagrees with it.
+  #
+  # ANCHORED AT LINE START, AND WHITESPACE-OR-END-OF-LINE AFTER THE NAME (#120). The #81 check
+  # this replaces matched the string anywhere on any line, so #113's KDoc reading `Deliberately
+  # not @FailsOnEmulatorApi37` counted as a fourth marked test and the report announced a
+  # deviation on every PR. That is worse than a wrong number: #83 built this so a new failure
+  # could not be invisible, and a notice that is wrong every time teaches everyone to skim past
+  # deviation notices.
+  #
+  # THE OBVIOUS REPAIR IS A TRAP, and the reason for the second half of the pattern.
+  # `^[[:space:]]*@NAME[[:space:]]*$` -- "the annotation on a line of its own" -- also stops
+  # counting `@FailsOnEmulatorApi37 @Test`, which is legal Kotlin, and UNDERcounting is the
+  # dangerous direction: it hides a genuine new marker, which is the one thing this exists to
+  # catch. Measured against `testdata/marker-shapes`, a fixture carrying every shape at once:
+  # the old matcher says 5, own-line-only says 2, this one says 3. On the real tree, 4 / 3 / 3.
+  # e2e-report-shape-test.sh runs that fixture through this whole script.
+  #
+  # `^[[:space:]]*@` cannot match an `import` line, so the old `grep -v import` goes with it
+  # rather than staying to imply a filter is still doing work.
+  #
+  # This is a regex over source text and not a parser. An annotation inside a multi-line string,
+  # or inside a `/* */` block that opened mid-line, would still be counted. Neither exists here;
+  # if one ever does, this check wants a different tool rather than a longer regex.
  if [ -d "$REPO_ROOT/app/src/androidTest" ]; then
-    marked="$(grep -rn "@FailsOnEmulatorApi37" "$REPO_ROOT/app/src/androidTest" --include='*.kt' \
-      | grep -v import | grep -c FailsOn || true)"
+    marked="$(grep -rhcE '^[[:space:]]*@FailsOnEmulatorApi37([[:space:]]|$)' \
+      "$REPO_ROOT/app/src/androidTest" --include='*.kt' \
+      | awk '{ total += $1 } END { print total + 0 }' || true)"
  fi
 fi

@@ -217,6 +268,11 @@ echo "----- RUN SHAPE (api${LABEL}) -----"
 echo "  expected:          $expected"
 echo "  received:          $received"
 echo "  failed:            $failed"
+# Above `completed cleanly`, because it is the line that says what happened to the leg and the
+# other one only qualifies it. A reader who stops after three rows still sees it.
+if [ -n "$WEDGED_AFTER" ]; then
+  echo "  wedged:            yes -- gradle was killed after ${WEDGED_AFTER}s and never returned"
+fi
 echo "  completed cleanly: ${completed//\*/}"
 if [ -n "$abort_received" ]; then
  echo "  received before the abort: $abort_received"
@@ -249,6 +305,9 @@ if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then
    echo "| expected | $expected | $expected_src |"
    echo "| received | $received | $received_src |"
    echo "| failed | $failed | $failed_src |"
+    if [ -n "$WEDGED_AFTER" ]; then
+      echo "| wedged | **yes** | \`timeout\` fired after ${WEDGED_AFTER}s and killed gradle (exit 124), which is what e2e-run.sh then captured the wedge diagnostics for |"
+    fi
    echo "| completed cleanly | $completed | $completed_src |"
    if [ -n "$abort_received" ]; then
      echo "| received before the abort | $abort_received | the same line — the XML above counts the truncated test as a failure, this number does not |"
@@ -239,20 +239,34 @@ timeout -k 30s "$WEDGE_TIMEOUT" \
  ${E2E_EXTRA_GRADLE_ARGS:-} 2>&1 | tee "$GRADLE_LOG" || status=$?
 echo "::endgroup::"

+# Whether the wrapper timeout fired, decided ONCE. 124 is `timeout` saying it killed the
+# command, and two places downstream need that fact: capture_wedge below, and the report, which
+# otherwise calls a killed leg `completed cleanly: yes` (#118). Deriving it twice is how those
+# two would drift apart -- the report would keep printing after someone changed what a wedge
+# means here. It stays a string: empty on every other path, so those legs pass an empty
+# E2E_WEDGED_AFTER and the report behaves exactly as before.
+wedged=""
+[ "$status" -eq 124 ] && wedged="$WEDGE_TIMEOUT"
+
 # The run-shape report: expected/received/failed and whether the run finished, every time,
 # green or red. It never changes `status` -- it is a diagnostic, and the header's rule about
 # diagnostics applies to it as much as to every probe below.
 #
+# E2E_WEDGED_AFTER is the wedge, told to the report rather than left for it to infer. It cannot
+# be inferred: a wedge is gradle never returning, so gradle printed no verdict at all, and the
+# log the report reads looks like a run that simply stopped. Only this script knows the
+# difference, because only this script saw the exit status.
+#
 # The baseline argument, and only it, turns on the comparison, and only the advisory API 37 job
 # passes E2E_ADVISORY=1. Comparing on the gating legs would announce a deviation on all five of
 # them every run, since they run the whole suite rather than the marked three. They still get
 # the report: a truncated run reporting fewer results than it ran is what #108 looks like, and
 # `completed cleanly` is the field that shows it.
 if [ "${E2E_ADVISORY:-}" = "1" ]; then
-  bash "$SCRIPT_DIR/e2e-report-shape.sh" "$LABEL" "$GRADLE_LOG" \
+  E2E_WEDGED_AFTER="$wedged" bash "$SCRIPT_DIR/e2e-report-shape.sh" "$LABEL" "$GRADLE_LOG" \
    "$REPO_ROOT/app/src/androidTest/java/org/libremediaconverter/FailsOnEmulatorApi37.kt" || true
 else
-  bash "$SCRIPT_DIR/e2e-report-shape.sh" "$LABEL" "$GRADLE_LOG" || true
+  E2E_WEDGED_AFTER="$wedged" bash "$SCRIPT_DIR/e2e-report-shape.sh" "$LABEL" "$GRADLE_LOG" || true
 fi

 if [ "$status" -eq 0 ]; then
@@ -260,7 +274,7 @@ if [ "$status" -eq 0 ]; then
  exit 0
 fi

-if [ "$status" -eq 124 ]; then
+if [ -n "$wedged" ]; then
  capture_wedge "api${LABEL}"
 else
  echo "::error::E2E api${LABEL} failed (exit $status)"
@@ -0,0 +1,54 @@
+// NOT A TEST, AND NEVER COMPILED. This is fixture data for e2e-report-shape-test.sh, which
+// copies it into a throwaway repo root and runs the real report script against that. It lives
+// under .github/ deliberately: Gradle only compiles app/src/**, ktlint and detekt are applied to
+// :app only, and the report's own count reads app/src/androidTest -- so nothing here can reach
+// the build, the linters, or the number the advisory job compares against. Verified by running
+// the report against the real repo root with this file committed: still 3.
+//
+// It carries every shape the counter has to tell apart, in one file, because the bug in #120 was
+// exactly that two of them look alike to a substring match. Three count and three must not:
+//
+//   COUNTS       the annotation on its own line
+//   COUNTS       the annotation sharing a line with @Test -- legal Kotlin, and the case the
+//                obvious "own line only" repair silently drops
+//   COUNTS       the annotation indented inside a nested class
+//   must NOT     a KDoc mentioning it -- this is #120 itself, copied from Media3EngineTest
+//   must NOT     a commented-out annotation
+//   must NOT     the import
+//
+// Three count. That is what the synthetic baseline in the test is set to, so the fixture and the
+// baseline agree exactly the way the real tree and FAILS_ON_EMULATOR_API37_BASELINE do.
+//
+// The `@Test` here is spelled the way a real test spells it so the fixture reads like source
+// rather than like a regex exercise. Nothing runs it.
+
+package org.libremediaconverter.fixture
+
+import org.junit.Test
+import org.libremediaconverter.FailsOnEmulatorApi37
+
+class MarkerShapes {
+    @FailsOnEmulatorApi37
+    @Test
+    fun ownLine() = Unit
+
+    @FailsOnEmulatorApi37 @Test
+    fun sameLineAsTest() = Unit
+
+    /**
+     * Deliberately not `@FailsOnEmulatorApi37`: nothing here decodes or encodes, so no emulator
+     * codec is involved and the API 37 image has no quarrel with it.
+     */
+    @Test
+    fun mentionedInKdoc() = Unit
+
+    // @FailsOnEmulatorApi37 -- taken off on 2026-01-01, kept as a note rather than deleted
+    @Test
+    fun commentedOut() = Unit
+
+    class Nested {
+        @FailsOnEmulatorApi37
+        @Test
+        fun indentedDeeper() = Unit
+    }
+}
@@ -13,6 +13,7 @@
 .externalNativeBuild
 .cxx
 local.properties
+.kotlin

 # The FFmpeg AAR is committed under bin/ so test runs do not depend on a rebuild.
 # Build outputs from tools/ffmpeg are not.
@@ -76,11 +76,11 @@ days. Read it as the current answer, and see the git history if you need the old
  `angle_indirect` and `swangle_indirect` all boot, while `auto`, `off`, `guest` and
  `swiftshader_indirect` do not. `docs/local-emulator.md` has the evidence and the per-API renderer
  table.
- **CI runs API 37, and it gates.** The matrix is 33/34/35/36/37. **Three** of the 59 instrumented
+- **CI runs API 37, and it gates.** The matrix is 33/34/35/36/37. **Three** of the 60 instrumented
  tests cannot pass on that image, for two unrelated reasons: two Media3 hardware transcodes fail
  inside the emulator's own `c2.goldfish.h264.decoder`, and one SAF test takes the framework down
  when it rotates the display. All three carry `@FailsOnEmulatorApi37` and run in a separate
-  `continue-on-error` job; the gating leg runs the other 56.
+  `continue-on-error` job; the gating leg runs the other 57.

  That job is still called `E2E API 37 Media3 hardware transcode (advisory)`, which no longer
  describes everything in it. The name is kept deliberately — it is not a required context and
@@ -98,6 +98,13 @@ days. Read it as the current answer, and see the git history if you need the old
  is written anyway and says nothing about it — `.github/scripts/e2e-report-shape.sh` is where that
  is measured and explained.

+  Every leg prints that table, advisory or not, and **on the wedge path it also carries a `wedged:`
+  row** (#118). `completed cleanly` only ever meant "instrumentation was not aborted", which stays
+  true of a leg the `WEDGE_TIMEOUT` killed 22 minutes in — so without that row the table read
+  `received: 59, completed cleanly: yes` for a leg that had just died. The wedge is passed to the
+  report as `E2E_WEDGED_AFTER` by `e2e-run.sh`, which is the only thing that can know it: a wedge
+  is gradle never returning, so the log it left says nothing about it.
+
 Still true, and the reason the advisory job is not simply deleted: **API 37 needs a manual check on
 the Pixel 10 Pro XL before each release.** Those three tests are the one thing CI cannot answer
 for.
@@ -123,10 +130,10 @@ 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** — **69.2% of lines (1519/2194), 53.2% of branches**,
-  measured 2026-08-24 with `./gradlew :app:jacocoTestReport`.
+- **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.

-  **Every figure this file carried before that date was an artifact, roughly half the real one.**
+  **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
  skips no-location classes by default, and nothing told it otherwise — so **not one Robolectric
  test counted**, and Robolectric is what exercises the framework edge here. The
@@ -140,9 +147,12 @@ 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 now moved by
-  39 points in a single build change, so it has not. And **re-measure before quoting** — that
-  instruction is the only reason this was caught.
+  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.
 - **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.
@@ -236,3 +246,12 @@ Because versions float, a build can change without a commit. `./gradlew :app:dep
  `@OptIn`. Android lint's `UnsafeOptInUsageError` catches a missed one.
 - **Release builds ship both ABIs.** `-PabiFilters` is a test-run override only; `build.yml`
  verifies the released APK carries every ABI and that all native libraries are 16 KB aligned.
+- **A JUnit `Timeout` — rule or `@Test(timeout=)` — cannot be used in the JVM suite.** Both run the
+  test body on a separate thread, and every Compose test here goes through Robolectric's paused
+  main looper: `UnsupportedOperationException: main looper can only be controlled from main
+  thread`, from `ShadowPausedLooper` under `RobolectricIdlingStrategy.runUntilIdle`. The identical
+  tests pass with the timeout removed, so it is the mechanism, not the test. What bounds a hung
+  run instead is `timeout` on the `Test` tasks plus the jstack watchdog beside it in
+  `app/build.gradle.kts`, neither of which moves a thread. `HangBoundTest` guards both numbers,
+  and **a timed-out run writes no XML for the class that hung** — the dump is its only
+  attribution, so do not delete the watchdog as stray config.
@@ -1,5 +1,7 @@
 import org.gradle.api.tasks.PathSensitivity
 import org.gradle.testing.jacoco.tasks.JacocoReport
+import java.io.File
+import java.time.Duration

 plugins {
    // Applied by id: these two come from the root buildscript classpath, which is what
@@ -217,6 +219,115 @@ tasks.withType<Test>().configureEach {
        .withPropertyName("releaseWorkflow")
        .withPathSensitivity(PathSensitivity.RELATIVE)

+    // Same reasoning, same trap: HangBoundTest reads the two numbers below out of this file, and
+    // they are the one part of the change that does not compile. Without this the task stays
+    // UP-TO-DATE when the build script changes, so the guard would go stale on exactly the edit
+    // it exists to catch.
+    inputs.file(project.file("build.gradle.kts"))
+        .withPropertyName("moduleBuildScript")
+        .withPathSensitivity(PathSensitivity.RELATIVE)
+
+    // --- Bounding a hung run (#125) -----------------------------------------------------------
+    //
+    // This suite had no timeout of any kind, so a hang ran until something outside it gave up.
+    // #125 is a real Java-level deadlock -- a lock-order inversion between Room's
+    // TransactionExecutor and WorkManager's SerialExecutorImpl, reached through the WorkInfo flow
+    // -- and one local run sat in it for 47 minutes. On CI it would burn the Unit tests job's
+    // 30-minute cap and report as a job timeout with no cause at all.
+    //
+    // WHY NOT A JUnit `Timeout` RULE, which is the obvious answer: it runs the test body on a
+    // separate thread, and this suite is thread-affine. Measured here, `@Rule Timeout` and
+    // `@Test(timeout = ...)` against a `createComposeRule()` Robolectric test both give:
+    //
+    //     java.lang.UnsupportedOperationException: main looper can only be controlled from main
+    //       at org.robolectric.shadows.ShadowPausedLooper.executeOnLooper
+    //       at androidx.compose.ui.test.RobolectricIdlingStrategy.runUntilIdle
+    //
+    // The same two tests with the timeout removed pass, so that is the mechanism and not the
+    // probe. Nothing that moves a test off its own thread can be used here.
+    //
+    // `Task.timeout` moves nothing -- it stops the forked test JVM from outside. Its weakness is
+    // that it kills without a thread dump, and the jstack is the only reason #125 could be named
+    // at all; the watchdog below is what answers that, and it only dumps.
+    //
+    // THE NUMBER, against the slowest observed *pass* rather than the typical one. Eight CI runs
+    // sampled 2026-08-26, whole `./gradlew :app:testDebugUnitTest` invocation with compilation in
+    // it and this task a subset: 62, 76, 77, 79, 81, 84, 86 and 90 seconds. Locally the task
+    // itself is ~11 s over 454 tests. Ten minutes is ~6.7x the slowest of those and a third of
+    // the job's 30-minute cap, so a fired timeout still has room to be reported and uploaded. It
+    // is deliberately nowhere near the observed duration: a timeout that fires on a healthy slow
+    // runner turns a real signal into noise and teaches people to re-run reflexively.
+    timeout.set(Duration.ofMinutes(10))
+
+    // The dump, two minutes before the kill. jstack is what turned #125 from "CI timed out" into
+    // a named lock-order inversion, and `Task.timeout` on its own would have thrown it away.
+    //
+    // It is deliberately incapable of failing a build: it reads a live process and writes a file.
+    // Nothing here kills, interrupts or signals anything, so the worst a misfire can do is leave a
+    // stack trace nobody needed. It has one, and it is the ordinary CI shape rather than an exotic
+    // case: the worker is found by scanning this daemon's descendants for GradleWorkerMain, which
+    // cannot tell one invocation's worker from the next, and the Unit tests job runs
+    // testDebugUnitTest and jacocoTestReport back to back against the same daemon. If this task's
+    // own worker lived and died inside a single poll, the watchdog can adopt the following one.
+    //
+    // Everything it needs is read here, at configuration time, and captured by value. Reaching
+    // back through the task or the project from inside the action would not survive the
+    // configuration cache, which `gradle.properties` turns on for every build.
+    val threadDump = layout.buildDirectory.file("reports/hang/$name-threads.txt").get().asFile
+    val taskPath = path
+    val dumpAfterNanos = Duration.ofMinutes(8).toNanos()
+    val captureWindowNanos = Duration.ofMinutes(1).toNanos()
+    val pollMillis = 1_000L
+    doFirst {
+        val watchdog = Thread {
+            val startedAt = System.nanoTime()
+            var worker: ProcessHandle? = null
+            while (true) {
+                Thread.sleep(pollMillis)
+                val elapsed = System.nanoTime() - startedAt
+                val watched = worker
+                if (watched == null) {
+                    // Gradle forks the worker moments after this task starts. If none has shown
+                    // up by the end of the capture window there is nothing to watch, and going on
+                    // polling would only risk adopting some other build's.
+                    if (elapsed > captureWindowNanos) return@Thread
+                    worker = ProcessHandle.current().descendants()
+                        .filter { it.info().commandLine().orElse("").contains("GradleWorkerMain") }
+                        .findFirst().orElse(null)
+                } else if (!watched.isAlive) {
+                    return@Thread // the run finished; this is the healthy exit
+                } else if (elapsed >= dumpAfterNanos) {
+                    val jstack = File(File(System.getProperty("java.home"), "bin"), "jstack")
+                    threadDump.parentFile.mkdirs()
+                    if (jstack.canExecute()) {
+                        ProcessBuilder(jstack.absolutePath, "-l", watched.pid().toString())
+                            .redirectErrorStream(true)
+                            .redirectOutput(threadDump)
+                            .start()
+                            .waitFor()
+                    } else {
+                        threadDump.writeText("no jstack at ${jstack.absolutePath}\n")
+                    }
+                    // To stdout as well as to the file, and that is the half that matters on CI:
+                    // the Unit tests job uploads app/build/reports/tests/ and nothing else, so a
+                    // dump that only ever existed under reports/hang/ would be unreachable from a
+                    // red run -- which is the "timed out with no cause" this exists to end. The
+                    // step log always survives, and needs no workflow edit to say so.
+                    println(
+                        "$taskPath is still running after ${Duration.ofNanos(elapsed).toMinutes()} " +
+                            "minutes and is about to be timed out. Thread dump of pid " +
+                            "${watched.pid()}, also written to $threadDump -- look for 'Found one " +
+                            "Java-level deadlock' (that is #125).\n" + threadDump.readText(),
+                    )
+                    return@Thread
+                }
+            }
+        }
+        watchdog.isDaemon = true
+        watchdog.name = "hang-watchdog"
+        watchdog.start()
+    }
+
    extensions.configure<JacocoTaskExtension> {
        isIncludeNoLocationClasses = true
        excludes = listOf("jdk.internal.*")
@@ -11,15 +11,26 @@ import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.withTimeout
 import org.junit.After
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.libremediaconverter.FailsOnEmulatorApi37
+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.InputKind
+import org.libremediaconverter.model.InputProbe
 import org.libremediaconverter.model.OutputFormat
+import org.libremediaconverter.model.OutputSpec
+import org.libremediaconverter.model.VideoCodec
+import org.libremediaconverter.model.VideoPlan
 import java.io.File
+import java.util.concurrent.CancellationException
 import java.util.concurrent.Executors
 import java.util.concurrent.TimeUnit

@@ -170,6 +181,63 @@ class Media3EngineTest {
        )
    }

+    /**
+     * The builders that used to throw where nothing could catch them.
+     *
+     * `EditedMediaItem.Builder` rejects a composition with both tracks removed —
+     * checkState("Audio and video cannot both be removed") — and the engine builds it on its own
+     * HandlerThread. That build sat *between* two narrow `runCatching` blocks, one around
+     * `buildTransformer` and one around `start`, so the exception reached the thread's uncaught
+     * handler and took the process with it while the continuation was never resumed.
+     *
+     * `ContainerCapabilities.validate` now refuses the spec that gets here from the picker; this
+     * is the other half — the engine surviving a request that arrives without being validated.
+     * Deliberately not `@FailsOnEmulatorApi37`: nothing here decodes or encodes, so no emulator
+     * codec is involved. The builder refuses the input before any media is touched.
+     */
+    @Test
+    fun aPlanThatRemovesBothTracksFailsInsteadOfKillingTheProcess() {
+        val request = ConversionRequest(
+            spec = OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE),
+            probe = InputProbe(
+                videoCodec = null,
+                audioCodec = "mp3",
+                hasVideo = false,
+                container = Container.MP3,
+                kind = InputKind.AUDIO_ONLY,
+            ),
+        )
+        // Asserted rather than assumed: ConversionRequest's default probe says hasVideo = true,
+        // and with it this same spec plans to (Encode, Drop) and nothing throws at all — which
+        // would make the whole test vacuous without a word of warning.
+        val plan = CopyPlanner.plan(request.spec, request.probe)
+        assertEquals(VideoPlan.Drop, plan.video)
+        assertEquals(AudioPlan.Drop, plan.audio)
+
+        val failure = runCatching {
+            runBlocking {
+                withTimeout(BUILDER_TIMEOUT_MS) {
+                    engine.transcode(Uri.fromFile(input), output, request) {}
+                }
+            }
+        }.exceptionOrNull()
+
+        // Two assertions, and the second is not pedantry. withTimeout raises
+        // TimeoutCancellationException, and `java.util.concurrent.CancellationException` *extends*
+        // IllegalStateException — so testing only the type below would call an unresumed
+        // continuation a pass. A hang is the other half of this defect and every bit as bad as the
+        // crash: the worker would sit holding a foreground service forever.
+        assertFalse(
+            "the continuation was never resumed — the failure escaped instead of being reported: " +
+                "$failure",
+            failure is CancellationException,
+        )
+        assertTrue(
+            "the builder's refusal must surface as a failed job, not a dead process; got $failure",
+            failure is IllegalStateException,
+        )
+    }
+
    private fun durationMsOf(file: File): Long {
        val extractor = MediaExtractor()
        return try {
@@ -211,5 +279,12 @@ class Media3EngineTest {

    private companion object {
        const val TIMEOUT_SECONDS = 120L
+
+        /**
+         * Short on purpose. Nothing is decoded or encoded on this path — the builder refuses the
+         * input outright — so anything approaching this is a hang, which is what the test is
+         * looking for.
+         */
+        const val BUILDER_TIMEOUT_MS = 30_000L
    }
 }
@@ -101,7 +101,42 @@ sealed interface ConversionState {
        val mimeType: String = "",
    ) : ConversionState
    data class Saved(val displayName: String) : ConversionState
-    data class Failed(val message: String) : ConversionState
+
+    /**
+     * The job, or the save that followed it, could not be finished.
+     *
+     * [retry] is non-null for exactly one cause: a [ConversionViewModel.save] whose copy to the
+     * user's destination threw. That save deliberately keeps the staged file -- it can be the only
+     * copy of an hour of transcoding -- and this is what lets the screen offer it again. Every
+     * other failure leaves it null, because there is nothing staged to offer: a transcode that
+     * died produced no output, and a save that found the file gone has nothing left to save.
+     *
+     * Nullable rather than a `SaveFailed` state of its own. What the screen does with the message
+     * is identical either way, so a second variant would make every exhaustive `when` grow an arm
+     * that duplicates this one.
+     *
+     * A view of the file, not a second owner of it -- see [PendingSave].
+     */
+    data class Failed(val message: String, val retry: PendingSave? = null) : ConversionState
+}
+
+/**
+ * The staged output a save would target from this state, or null when there is nothing to save.
+ *
+ * One function for two callers that have to agree. [ConversionViewModel.save] picks the file to
+ * copy with it, and `ConverterScreen` registers its `CreateDocument` contract with the MIME type
+ * it returns; when those two read the state separately, a retry offered after a failed save opened
+ * the dialog with the *picker's* current type instead of the finished job's -- wrong for any job
+ * whose spec has been edited since, and for every reattached job, whose spec was never in these
+ * settings at all.
+ *
+ * Top-level and `internal` rather than a member of the ViewModel, so the screen can call it
+ * without one -- which is also what makes the derivation testable on the JVM.
+ */
+internal fun ConversionState.pendingSave(): PendingSave? = when (this) {
+    is ConversionState.Converted -> PendingSave(staged, suggestedName, mimeType)
+    is ConversionState.Failed -> retry
+    else -> null
 }

@UnstableApi
@@ -158,13 +193,34 @@ class ConversionViewModel @JvmOverloads constructor(
    private var observer: Job? = null
    private var activeWorkId: UUID? = null

+    /**
+     * Who is allowed to write to this screen — see [ScreenOwnership] for the rule and why
+     * cancelling the superseded coroutine is not one.
+     *
+     * Every write below that lands after a suspension point is guarded by it: the two in
+     * [onInputPicked] and the one in [observe].
+     *
+     * [save] is the one left out, deliberately — and not because it is safe in both directions.
+     * Nothing can overwrite what it writes: it is reachable only from [ConversionState.Converted]
+     * or a [ConversionState.Failed] carrying its file, so the only observation that could belongs
+     * to a job already in a terminal state, which will not emit again. What it can still do is
+     * land on top of a [reset] taken while its copy was in flight, putting `Saved` on a screen the
+     * user has just cleared. Guarding it would drop that write instead, reporting nothing for a
+     * file that may genuinely have reached the user's destination. Which of those two is right is
+     * a question about what the screen should offer during a save, not about this race, so it is
+     * filed as issue #123 rather than decided here in passing.
+     */
+    private val ownership = ScreenOwnership()
+
    /**
     * The staged output this ViewModel is responsible for deleting.
     *
-     * A field rather than something read back out of [_state], because the state machine
-     * cannot answer the question on the path that needs it most: a failed [save] lands on
-     * [ConversionState.Failed], which carries a message and no file at all. By then the
-     * only remaining reference would have been lost.
+     * A field rather than something read back out of [_state], and still one now that
+     * [ConversionState.Failed] carries a [PendingSave] after a failed [save]. That handle is a
+     * view for the screen to offer a retry through; this one is the single reference [reset]
+     * deletes through, and keeping the two apart is what stops a second owner appearing. Reading
+     * the file back out of the state machine instead would mean trusting every state that has no
+     * file -- `Idle`, `Saved`, a transcode failure -- to say so.
     */
    private var pendingStaged: File? = null

@@ -207,6 +263,10 @@ class ConversionViewModel @JvmOverloads constructor(
     * `Data` — see [ConversionState.Converted].
     */
    private fun reattach() {
+        // Read before the launch, and before the query it is about to suspend in. This is the
+        // claim the answer will belong to: anything the user does from here on supersedes it, and
+        // reading it on the far side of the query would read whatever superseded it instead.
+        val token = ownership.current
        viewModelScope.launch {
            val reattachment = Reattachment.choose(
                workManager.jobSnapshots(
@@ -217,8 +277,17 @@ class ConversionViewModel @JvmOverloads constructor(

            // The query suspends, so by now the user may have picked a file or started a
            // conversion of their own. Either owns the screen; reattaching over it would throw
-            // away what they just did. Both this check and the assignment below run on the main
-            // dispatcher with no suspension point between them, so nothing can interleave.
+            // away what they just did.
+            //
+            // This catches a pick that has already *landed*, and only that. It used to claim that
+            // "both this check and the assignment below run on the main dispatcher with no
+            // suspension point between them, so nothing can interleave" — which was the exact
+            // opposite of what happens. There is no assignment below. There is observe(), which
+            // launches a *separate* coroutine that must suspend on `collect` before it can write
+            // anything, so the check happens at one moment and the write lands at another with a
+            // whole pick able to fit in between. That was issue #49, and believing this comment is
+            // why it read as flaky CI for two days. What actually holds the line is the token
+            // observe() carries: see [ScreenOwnership].
            if (_state.value !is ConversionState.Idle || activeWorkId != null) return@launch

            // Only a job that is the sole explanation for its staged file gets to name the input.
@@ -244,7 +313,7 @@ class ConversionViewModel @JvmOverloads constructor(
            activeWorkId = reattachment.job.id
            // No initial state of our own: the flow's first emission carries the job's real
            // state, so observe() maps it exactly as it would for a conversion started here.
-            observe(reattachment.job.id, input, cancelled = ConversionState.Idle)
+            observe(reattachment.job.id, input, cancelled = ConversionState.Idle, token = token)
        }
    }

@@ -260,25 +329,34 @@ class ConversionViewModel @JvmOverloads constructor(
    fun setQuality(quality: QualityTier) = _settings.update { it.copy(quality = quality) }
    fun setEnginePreference(preference: EnginePreference) = _settings.update { it.copy(enginePreference = preference) }

+    /**
+     * The tap is the claim, which is why [ScreenOwnership.claim] is called here and not inside the
+     * `launch`. A claim made in the coroutine would only be immediate for as long as
+     * `Dispatchers.Main.immediate` happened to run it inline, and a deferred claim leaves the same
+     * gap this closes: it is the difference between the user owning the screen from the moment
+     * they tapped and owning it from whenever their coroutine got around to running.
+     */
    fun onInputPicked(uri: Uri) {
+        val token = ownership.claim()
        viewModelScope.launch {
            // Both the metadata query and the probe touch disk, and the probe spawns FFprobe.
            // Neither belongs on the main thread.
            val file = withContext(pickDispatcher) { InputQuery.describe(getApplication(), uri) }
+            // Every write below the hop above is guarded, this one included: two picks in quick
+            // succession suspend here together, and without this the slower one would land last
+            // and put the file the user did not choose on screen.
+            if (!ownership.stillHeldBy(token)) return@launch
            // Show the file as soon as its name and size are known. Probing now runs FFprobe on
            // every pick, which is a native process spawn, and making the whole screen wait on it
            // would read as the app having ignored the tap.
            _state.value = ConversionState.Ready(file)

            val probe = withContext(pickDispatcher) { probeOrUnreadable(uri) }
-            // Only fill in the probe if the user has not moved on in the meantime.
-            _state.update { current ->
-                if (current is ConversionState.Ready && current.input.uri == uri) {
-                    ConversionState.Ready(file.copy(probe = probe))
-                } else {
-                    current
-                }
-            }
+            // Only fill in the probe if the user has not moved on in the meantime. The claim is
+            // what says whether they have -- it covers a second pick of the same URI, which a
+            // comparison of URIs cannot, and every state a later claim could have written.
+            if (!ownership.stillHeldBy(token)) return@launch
+            _state.value = ConversionState.Ready(file.copy(probe = probe))
        }
    }

@@ -329,10 +407,13 @@ class ConversionViewModel @JvmOverloads constructor(
            quality = settings.quality,
            enginePreference = settings.enginePreference,
        )
+        // Tapping Convert claims the screen for this job, which is what supersedes the pick's
+        // still-in-flight probe and any reattachment that has not finished asking.
+        val token = ownership.claim()
        activeWorkId = request.id
        workManager.enqueue(request)
        _state.value = ConversionState.Converting(input, 0)
-        observe(request.id, input)
+        observe(request.id, input, token = token)
    }

    /**
@@ -340,12 +421,27 @@ class ConversionViewModel @JvmOverloads constructor(
     *   picked file, ready to convert again. For one picked up by [reattach] there is no picked
     *   file — the URI that job holds belongs to a process that no longer exists — so it lands
     *   on Idle instead, rather than offering a Convert button over a file nothing can open.
+     * @param token the claim this observation belongs to. Nothing here can write until `collect`
+     *   has resumed with a `WorkInfo`, which is some time after the caller decided to observe, so
+     *   the claim is checked again at the last possible moment rather than trusted from then. This
+     *   is issue #49's fix and the only thing standing between a superseded observation and the
+     *   user's screen — see [ScreenOwnership].
     */
-    private fun observe(id: UUID, input: InputFile, cancelled: ConversionState = ConversionState.Ready(input)) {
+    private fun observe(
+        id: UUID,
+        input: InputFile,
+        cancelled: ConversionState = ConversionState.Ready(input),
+        token: Long,
+    ) {
        observer?.cancel()
        observer = viewModelScope.launch {
            workManager.getWorkInfoByIdFlow(id).collect { info ->
                if (info == null) return@collect
+                // Ahead of the `when`, not merely ahead of the assignment: the SUCCEEDED branch
+                // takes ownership of the staged file, and a superseded observation must not do
+                // 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,
@@ -426,35 +522,50 @@ class ConversionViewModel @JvmOverloads constructor(
    /**
     * Copies the staged result out to the destination the user picked.
     *
+     * Reached from [ConversionState.Converted] and again from a [ConversionState.Failed] that an
+     * earlier save left carrying its file. [pendingSave] is what makes those one call rather than
+     * two, so a retry cannot drift from the first attempt in what it copies or what it calls it.
+     *
     * The existence check is not redundant with the one reattachment already made. That one ran
     * inside a tag query which, for a result offered on launch, can be hours older than the tap —
     * and `cacheDir` is exactly the directory the OS empties when it wants space, which is also
     * what the sweep does to anything a day old. Without it the file's absence arrived as
-     * `staged.inputStream()` throwing, and `e.message` put a raw ENOENT path on screen.
+     * `staged.inputStream()` throwing, and `e.message` put a raw ENOENT path on screen. A retry
+     * meets that same check a second time, which is the point of reusing it here.
     */
    fun save(destination: Uri) {
-        val converted = _state.value as? ConversionState.Converted ?: return
-        if (!converted.staged.isFile) {
+        val pending = _state.value.pendingSave() ?: return
+        if (!pending.staged.isFile) {
+            // No retry handle: the file such a state would offer again is exactly the one that
+            // has gone, so carrying it would put a button on screen that cannot do anything.
            _state.value = ConversionState.Failed(STAGED_FILE_GONE_MESSAGE)
            return
        }
        viewModelScope.launch {
            runCatching {
                withContext(Dispatchers.IO) {
-                    publisher.publish(converted.staged, destination)
-                    converted.staged.delete()
+                    publisher.publish(pending.staged, destination)
+                    pending.staged.delete()
                }
            }.onSuccess {
                // publish() already deleted it; nothing left to clean up.
                pendingStaged = null
-                _state.value = ConversionState.Saved(converted.suggestedName)
+                _state.value = ConversionState.Saved(pending.suggestedName)
            }.onFailure { e ->
                // Deliberately NOT cleared. A failed save may mean the staged file is the
                // only copy of an hour of transcoding, and the user's destination did not
                // receive it -- deleting here would destroy the work to tidy up a cache
                // directory. It stays collectable: by a later reset(), or by the sweep once
                // it is old enough to be certain nobody is coming back for it.
-                _state.value = ConversionState.Failed(e.message ?: "Could not save the file.")
+                //
+                // `pending` rides on the state so the screen can offer that file again. It used
+                // to live only in `pendingStaged`, where nothing on screen could reach it -- so
+                // the single button this branch rendered was "Start over", which deletes the very
+                // file the paragraph above goes out of its way to keep. It is `pending` rather
+                // 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)
            }
        }
    }
@@ -468,8 +579,18 @@ class ConversionViewModel @JvmOverloads constructor(
     * cancelled with [viewModelScope] if the Activity finishes first, so it is a best
     * effort rather than a guarantee. `OutputPublisher.sweepStaging` is the backstop for
     * the times it does not run.
+     *
+     * **It still deletes from a [ConversionState.Failed] carrying a [PendingSave], and that is a
+     * decision rather than something inherited.** Deletion is acceptable there only because the
+     * alternative was offered first: the screen puts "Try saving again" directly above this
+     * button, so reaching it is the user saying the work is not worth keeping. Until that button
+     * existed, this delete was the only thing a failed save could lead to — which was the defect.
     */
    fun reset() {
+        // Start over is a claim like any other. The cancel below is a request honoured at the next
+        // suspension point, so a collector already on its way to a write has nothing left to
+        // honour it at; the claim is what actually stops that write landing on top of Idle.
+        ownership.claim()
        observer?.cancel()
        observer = null
        activeWorkId = null
@@ -73,8 +73,11 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
    // stands: some providers rewrite a document's extension to match it, so an MP3 offered as
    // video/webm can arrive with the wrong one. Read straight off the collected state, so this
    // recomposes because it depends on that rather than because an unrelated line happens to.
+    // Through pendingSave() rather than a cast to Converted, so a retry offered after a failed
+    // save opens the dialog with the type its first attempt used -- the cast answered null for a
+    // Failed, and the fallback below is the current picker, which a reattached job never set.
    // Remembered against the type so the launcher re-registers only when it actually changes.
-    val destinationMime = (state as? ConversionState.Converted)?.mimeType ?: settings.spec.mimeType
+    val destinationMime = state.pendingSave()?.mimeType ?: settings.spec.mimeType
    val chooseDestination = rememberLauncherForActivityResult(
        remember(destinationMime) { ActivityResultContracts.CreateDocument(destinationMime) },
    ) { uri -> uri?.let(viewModel::save) }
@@ -325,13 +328,36 @@ internal fun ConverterScreenContent(
                            color = MaterialTheme.colorScheme.error,
                            style = MaterialTheme.typography.bodyMedium,
                        )
-                        Button(
-                            onClick = actions.onReset,
-                            modifier = Modifier
-                                .fillMaxWidth()
-                                .height(PrimaryButtonHeight)
-                                .testTag(TestTags.START_OVER),
-                        ) { Text("Start over") }
+                        val retry = s.retry
+                        if (retry == null) {
+                            // Nothing was staged, so "Start over" is the whole of what is on
+                            // offer and stays the primary button.
+                            Button(
+                                onClick = actions.onReset,
+                                modifier = Modifier
+                                    .fillMaxWidth()
+                                    .height(PrimaryButtonHeight)
+                                    .testTag(TestTags.START_OVER),
+                            ) { Text("Start over") }
+                        } else {
+                            Button(
+                                onClick = { actions.onSave(retry.suggestedName) },
+                                modifier = Modifier
+                                    .fillMaxWidth()
+                                    .height(PrimaryButtonHeight)
+                                    .testTag(TestTags.RETRY_SAVE),
+                            ) { Text("Try saving again") }
+                            // Start over still deletes the file this state is carrying, and that
+                            // is deliberate: `reset()` is what stops a full-size output sitting in
+                            // cache until the sweep. What makes the delete acceptable is the
+                            // button above it. Deletion is the user's choice only once the
+                            // alternative has been offered -- and until that button existed, this
+                            // one was the only thing a failed save could lead to.
+                            OutlinedButton(
+                                onClick = actions.onReset,
+                                modifier = Modifier.fillMaxWidth().testTag(TestTags.START_OVER),
+                            ) { Text("Start over") }
+                        }
                    }
                }
            }
@@ -68,42 +68,64 @@ class Media3Engine(private val context: Context) : HardwareTranscoder {
    ): Unit = suspendCancellableCoroutine { cont ->
        val plan = CopyPlanner.plan(request.spec, request.probe)
        handler.post {
-            val transformer = runCatching { buildTransformer(plan, cont) }
-                .getOrElse {
-                    cont.resumeWithException(it)
-                    return@post
-                }
-
-            // Dropping the tracks the target does not have is what stops an audio-only export
-            // from carrying a re-encoded video track. Without setRemoveVideo, asking for M4A
-            // produced an HEVC stream in a file named .m4a.
-            val item = EditedMediaItem.Builder(MediaItem.fromUri(input))
-                .setRemoveVideo(plan.video == VideoPlan.Drop)
-                .setRemoveAudio(plan.audio == AudioPlan.Drop)
-                .build()
-
-            // A Composition is the only way to ask for transmuxing; the plain
-            // start(EditedMediaItem, path) overload always re-encodes. This is the remux path.
-            val composition = Composition.Builder(EditedMediaItemSequence.Builder(item).build())
-                .setTransmuxVideo(plan.video == VideoPlan.Copy)
-                .setTransmuxAudio(plan.audio == AudioPlan.Copy)
-                .build()
-
-            cont.invokeOnCancellation {
-                // cancel() has the same single-thread requirement as start().
-                handler.post { runCatching { transformer.cancel() } }
-            }
-
-            runCatching { transformer.start(composition, output.absolutePath) }
-                .onFailure {
-                    cont.resumeWithException(it)
-                    return@post
-                }
-
-            pollProgress(transformer, cont, onProgress)
+            // One guard around the whole body, deliberately.
+            //
+            // This used to be two narrow ones — around `buildTransformer` and around
+            // `transformer.start` — with the two Media3 builders sitting unguarded between them.
+            // On this thread that is not a small gap: nothing here has a caller to throw back to,
+            // so an escaping exception reaches the HandlerThread's uncaught handler and takes the
+            // process down, while [cont] is never resumed either way. `EditedMediaItem.Builder`
+            // does exactly that for a plan that drops both tracks
+            // ("Audio and video cannot both be removed"), which a queued job can still carry.
+            // Widening the guard costs nothing on success and turns every such refusal into a
+            // failed job with a reason.
+            runCatching { startExport(input, output, plan, cont, onProgress) }
+                .onFailure { if (cont.isActive) cont.resumeWithException(it) }
        }
    }

+    /**
+     * Builds the export and hands it to Transformer. Runs on the HandlerThread; may throw.
+     *
+     * Everything Transformer's single-thread contract covers lives here, so that the caller has
+     * exactly one place to catch. Returning normally means the export is running and [cont] belongs
+     * to the listener; throwing means it never started and the caller owns resuming.
+     */
+    private fun startExport(
+        input: Uri,
+        output: File,
+        plan: ConversionPlan,
+        cont: CancellableContinuation<Unit>,
+        onProgress: (Int) -> Unit,
+    ) {
+        val transformer = buildTransformer(plan, cont)
+
+        // Dropping the tracks the target does not have is what stops an audio-only export
+        // from carrying a re-encoded video track. Without setRemoveVideo, asking for M4A
+        // produced an HEVC stream in a file named .m4a.
+        val item = EditedMediaItem.Builder(MediaItem.fromUri(input))
+            .setRemoveVideo(plan.video == VideoPlan.Drop)
+            .setRemoveAudio(plan.audio == AudioPlan.Drop)
+            .build()
+
+        // A Composition is the only way to ask for transmuxing; the plain
+        // start(EditedMediaItem, path) overload always re-encodes. This is the remux path.
+        val composition = Composition.Builder(EditedMediaItemSequence.Builder(item).build())
+            .setTransmuxVideo(plan.video == VideoPlan.Copy)
+            .setTransmuxAudio(plan.audio == AudioPlan.Copy)
+            .build()
+
+        // Registered before start(), so a cancellation racing the export always finds a
+        // transformer to cancel.
+        cont.invokeOnCancellation {
+            // cancel() has the same single-thread requirement as start().
+            handler.post { runCatching { transformer.cancel() } }
+        }
+
+        transformer.start(composition, output.absolutePath)
+        pollProgress(transformer, cont, onProgress)
+    }
+
    /**
     * @throws IllegalArgumentException if [plan] names a container Media3 cannot mux. That is a
     *   routing bug rather than a runtime condition — [org.libremediaconverter.model.ConversionRouter]
@@ -23,6 +23,24 @@ 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."

+/**
+ * A staged file that is still there to be saved, and everything the save dialog needs to offer it.
+ *
+ * The three travel together because a save cannot be repeated without all of them: the file to
+ * copy, the name to suggest, and the MIME type `CreateDocument` has to be registered with. None of
+ * them can be rederived from the pickers once the job is over -- they come from the job's own
+ * output `Data`, and a reattached job's spec was never in the current settings at all.
+ *
+ * Kept next to [STAGED_FILE_GONE_MESSAGE] for the same reason it is: both ViewModels need it and
+ * staging is what it is about.
+ *
+ * **A view of the staged file, never an owner of it.** The delete still runs through each
+ * ViewModel's own `pendingStaged` field, so a state carrying one of these can be dropped without
+ * losing the only reference -- which is what keeps "a `Failed` that carries a file" from being a
+ * new way to leak one.
+ */
+data class PendingSave(val staged: File, val suggestedName: String, val mimeType: String)
+
 /**
 * Staging and publication of conversion output.
 *
@@ -0,0 +1,57 @@
+package org.libremediaconverter.convert
+
+/**
+ * Which of the things writing to a screen is still allowed to.
+ *
+ * Both ViewModels are a state machine written to from several coroutines that each suspend before
+ * they write: a pick hops to a dispatcher for the metadata query, a reattachment hops for the tag
+ * query, and an observation of a WorkManager job cannot write at all until its `collect` has
+ * resumed with a `WorkInfo`. Whoever resumes last wins, which is how issue #49 let a finished job
+ * from an earlier session take a screen the user had already picked a file on.
+ *
+ * The rule this makes enforceable is one line: **every write that lands after a suspension point
+ * checks the claim it was made under, and drops itself if that claim has been superseded.** The
+ * claim is taken synchronously, when the user acts; the check happens immediately before the
+ * write. Superseded work is *dropped*, not reordered — a dropped write cannot come back later.
+ *
+ * Cancelling the superseded coroutine is not a substitute and was never going to be. `Job.cancel`
+ * is a request, honoured at the next suspension point; a collector that has already resumed and is
+ * on its way to `_state.value = …` has no suspension point left to honour it at, so the write
+ * lands anyway. Cancellation also cannot help at all in the case #49 actually reported, where
+ * nothing supersedes the observation until after it has been launched. Both ViewModels still
+ * cancel their old observer, because leaving a collector running is a leak — but the guarantee
+ * does not rest on it.
+ *
+ * **Confined to the main dispatcher, and that confinement is the atomicity argument.** Every
+ * claim and every check runs there, with no suspension point between a check and the write it
+ * guards, so a claim can never land between the two. Nothing here is synchronized and nothing is
+ * `@Volatile`: making the field visible across threads would invite exactly the off-main use this
+ * cannot support, and would replace an argument that holds with one that only looks like it does.
+ */
+internal class ScreenOwnership {
+
+    private var claims = 0L
+
+    /**
+     * The claim in force now.
+     *
+     * Read by work that is about to suspend and will want to know, when it comes back, whether
+     * the screen it was reading is still the screen it is writing to. Read it *before* the
+     * suspension, not after — reading it afterwards would return whatever claim superseded it,
+     * which is the bug rather than the check for it.
+     */
+    val current: Long get() = claims
+
+    /**
+     * Takes the screen, invalidating every write still in flight under an older claim.
+     *
+     * Called synchronously from the user's action rather than from inside the coroutine it
+     * starts. A claim made inside a `launch` is only immediate while the dispatcher happens to
+     * run it inline, and a deferred claim is no claim at all: it would leave the same gap this
+     * exists to close.
+     */
+    fun claim(): Long = ++claims
+
+    /** Whether [token] is still the claim in force, and may therefore write. */
+    fun stillHeldBy(token: Long): Boolean = token == claims
+}
@@ -46,9 +46,10 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod

    // The contract's MIME type comes from the finished job rather than from a literal: some
    // providers rewrite a document's extension to match it, so naming MP4 for a join that is not
-    // one can hand the user a file the extension lies about. Remembered against that type so the
-    // launcher re-registers only when it actually changes.
-    val destinationMime = (state as? JoinState.Joined)?.mimeType ?: ConcatWorker.DEFAULT_FORMAT.mimeType
+    // one can hand the user a file the extension lies about. Through pendingSave() rather than a
+    // cast to Joined, so a retry after a failed save opens with the type its first attempt used.
+    // Remembered against that type so the launcher re-registers only when it actually changes.
+    val destinationMime = state.pendingSave()?.mimeType ?: ConcatWorker.DEFAULT_FORMAT.mimeType
    val chooseDestination = rememberLauncherForActivityResult(
        remember(destinationMime) { ActivityResultContracts.CreateDocument(destinationMime) },
    ) { uri -> uri?.let(viewModel::save) }
@@ -226,13 +227,32 @@ internal fun JoinScreenContent(state: JoinState, actions: JoinActions, modifier:
                            color = MaterialTheme.colorScheme.error,
                            style = MaterialTheme.typography.bodyMedium,
                        )
-                        Button(
-                            onClick = actions.onReset,
-                            modifier = Modifier
-                                .fillMaxWidth()
-                                .height(PrimaryButtonHeight)
-                                .testTag(TestTags.START_OVER),
-                        ) { Text("Start over") }
+                        val retry = s.retry
+                        if (retry == null) {
+                            // Nothing staged, so "Start over" is all there is and stays primary.
+                            Button(
+                                onClick = actions.onReset,
+                                modifier = Modifier
+                                    .fillMaxWidth()
+                                    .height(PrimaryButtonHeight)
+                                    .testTag(TestTags.START_OVER),
+                            ) { Text("Start over") }
+                        } else {
+                            Button(
+                                onClick = { actions.onSave(retry.suggestedName) },
+                                modifier = Modifier
+                                    .fillMaxWidth()
+                                    .height(PrimaryButtonHeight)
+                                    .testTag(TestTags.RETRY_SAVE),
+                            ) { Text("Try saving again") }
+                            // Start over still deletes the carried file, for the reason the
+                            // converter screen writes out next to the same pair of buttons:
+                            // the delete is a choice only once the alternative is on screen.
+                            OutlinedButton(
+                                onClick = actions.onReset,
+                                modifier = Modifier.fillMaxWidth().testTag(TestTags.START_OVER),
+                            ) { Text("Start over") }
+                        }
                    }
                }
            }
@@ -18,7 +18,9 @@ import kotlinx.coroutines.withContext
 import org.libremediaconverter.convert.ConversionDependencies
 import org.libremediaconverter.convert.InputFile
 import org.libremediaconverter.convert.InputQuery
+import org.libremediaconverter.convert.PendingSave
 import org.libremediaconverter.convert.STAGED_FILE_GONE_MESSAGE
+import org.libremediaconverter.convert.ScreenOwnership
 import org.libremediaconverter.model.ConcatStrategy
 import org.libremediaconverter.work.ConcatWorker
 import org.libremediaconverter.work.JobTags
@@ -44,7 +46,30 @@ sealed interface JoinState {
        val mimeType: String,
    ) : JoinState
    data class Saved(val displayName: String) : JoinState
-    data class Failed(val message: String) : JoinState
+
+    /**
+     * The join, or the save that followed it, could not be finished.
+     *
+     * [retry] is non-null for exactly one cause, and for the same reason as on
+     * `ConversionState.Failed`: a [JoinViewModel.save] whose copy to the user's destination threw
+     * keeps the staged file, and this is what lets the screen offer it again. Every other failure
+     * leaves it null — a join that died produced no output, and a save that found the file gone
+     * has nothing left to save.
+     */
+    data class Failed(val message: String, val retry: PendingSave? = null) : JoinState
+}
+
+/**
+ * The staged output a save would target from this state, or null when there is nothing to save.
+ *
+ * The join tab's half of `ConversionState.pendingSave`, and it exists for the same reason: `save`
+ * and `JoinScreen`'s `CreateDocument` registration both have to answer this question, and answering
+ * it twice is how a retry ends up opening the dialog with a type the finished job never chose.
+ */
+internal fun JoinState.pendingSave(): PendingSave? = when (this) {
+    is JoinState.Joined -> PendingSave(staged, suggestedName, mimeType)
+    is JoinState.Failed -> retry
+    else -> null
 }

@UnstableApi
@@ -52,6 +77,15 @@ class JoinViewModel @JvmOverloads constructor(
    app: Application,
    /** Where [reset] runs its delete. See the same parameter on `ConversionViewModel`. */
    private val cleanupDispatcher: CoroutineDispatcher = Dispatchers.IO,
+    /**
+     * Where the metadata query behind a pick runs. See the same parameter on `ConversionViewModel`.
+     *
+     * The join side had no such seam, and the gap was not cosmetic: the one write `onInputsPicked`
+     * makes lands *after* this hop, so a test that wants to ask what happens while a pick is still
+     * in flight had no way to hold one there. Issue #49's race is exactly that question, and it
+     * went unasked on this side for as long as the dispatcher was a literal.
+     */
+    private val pickDispatcher: CoroutineDispatcher = Dispatchers.IO,
 ) : AndroidViewModel(app) {

    private val workManager = WorkManager.getInstance(app)
@@ -66,13 +100,28 @@ class JoinViewModel @JvmOverloads constructor(
    private var observer: Job? = null
    private var activeWorkId: UUID? = null

+    /**
+     * Who is allowed to write to this screen -- see [ScreenOwnership], which carries the rule and
+     * the reason cancelling the superseded coroutine is not one.
+     *
+     * The convert side had issue #49 reported against it four times in two days; this side has the
+     * identical shape and was never reported, because nothing was watching. Every write below that
+     * lands after a suspension point is guarded: the one in [onInputsPicked] and the one in
+     * [observe]. [save] is the one left out, deliberately and with the same limit its counterpart
+     * in `ConversionViewModel` spells out: nothing can overwrite what it writes, but it can still
+     * land on top of a [reset] taken while its copy was in flight. Which way that should go is a
+     * question about the save screen rather than about this race -- issue #123.
+     */
+    private val ownership = ScreenOwnership()
+
    /**
     * The staged output this ViewModel is responsible for deleting.
     *
     * Held here rather than read back out of [_state] for the same reason as in
-     * `ConversionViewModel`: a failed [save] lands on [JoinState.Failed], which carries a
-     * message and no file, so the state machine cannot answer this on the one path that
-     * most needs it.
+     * `ConversionViewModel`, and still held here now that [JoinState.Failed] carries a
+     * [PendingSave] after a failed [save]: that handle is a view for the screen to offer a retry
+     * through, this one is the single reference [reset] deletes through, and keeping the two
+     * apart is what stops a second owner of the file appearing.
     */
    private var pendingStaged: File? = null

@@ -91,6 +140,10 @@ class JoinViewModel @JvmOverloads constructor(
     * the rules about which job and why.
     */
    private fun reattach() {
+        // Read before the launch, and before the query it is about to suspend in: this is the
+        // claim the answer belongs to. Reading it on the far side of the query would read whatever
+        // superseded it, which is the bug rather than the check for it.
+        val token = ownership.current
        viewModelScope.launch {
            val reattachment = Reattachment.choose(
                workManager.jobSnapshots(
@@ -100,8 +153,15 @@ class JoinViewModel @JvmOverloads constructor(
            ) ?: return@launch

            // The query suspends, so the user may have picked files or started a join in the
-            // meantime. Theirs wins. No suspension point between this check and the assignment
-            // below, and both run on the main dispatcher, so nothing can interleave.
+            // meantime. Theirs wins.
+            //
+            // This catches a pick that has already *landed*, and only that. It used to claim there
+            // was "no suspension point between this check and the assignment below", which was the
+            // opposite of what happens: there is no assignment below, only observe(), which
+            // launches a separate coroutine that cannot write until its `collect` resumes. The
+            // check happens at one moment and the write lands at another, with a whole pick able
+            // to fit in between -- issue #49. The token observe() carries is what holds that line;
+            // see [ScreenOwnership].
            if (_state.value !is JoinState.Idle || activeWorkId != null) return@launch

            // Joins used to stage under one constant name, so two finished joins always reported
@@ -121,19 +181,29 @@ class JoinViewModel @JvmOverloads constructor(
                InputFile(Uri.EMPTY, "", sizeBytes = null)
            }
            activeWorkId = reattachment.job.id
-            observe(reattachment.job.id, inputs, cancelled = JoinState.Idle)
+            observe(reattachment.job.id, inputs, cancelled = JoinState.Idle, token = token)
        }
    }

+    /**
+     * The tap is the claim, which is why it is taken here rather than inside the `launch` -- and
+     * above the early return, so the refusal below is covered by it too. A claim made in the
+     * coroutine is only immediate while `Dispatchers.Main.immediate` happens to run it inline, and
+     * a deferred claim leaves exactly the gap this closes.
+     */
    fun onInputsPicked(uris: List<Uri>) {
+        val token = ownership.claim()
        if (uris.size < 2) {
            _state.value = JoinState.Failed("Pick at least two files to join.")
            return
        }
        viewModelScope.launch {
-            val files = withContext(Dispatchers.IO) {
+            val files = withContext(pickDispatcher) {
                uris.map { InputQuery.describe(getApplication(), it) }
            }
+            // Guarded like every other write that lands after a hop: two picks in quick succession
+            // suspend here together, and the slower one would otherwise land last.
+            if (!ownership.stillHeldBy(token)) return@launch
            _state.value = JoinState.Ready(files)
        }
    }
@@ -147,10 +217,13 @@ class JoinViewModel @JvmOverloads constructor(
            // did answer would hand the space check a lower bound it would read as a total.
            totalBytes = InputQuery.total(inputs.map { it.sizeBytes }),
        )
+        // Tapping Join claims the screen for this job, superseding any reattachment that has not
+        // finished asking.
+        val token = ownership.claim()
        activeWorkId = request.id
        workManager.enqueue(request)
        _state.value = JoinState.Joining(inputs)
-        observe(request.id, inputs)
+        observe(request.id, inputs, token = token)
    }

    /**
@@ -158,12 +231,25 @@ class JoinViewModel @JvmOverloads constructor(
     *   files, ready to join again. For one picked up by [reattach] there are no picked files —
     *   what that job holds are URIs granted to a process that no longer exists — so it lands on
     *   Idle rather than offering to re-join files nothing can open.
+     * @param token the claim this observation belongs to. Nothing here can write until `collect`
+     *   has resumed with a `WorkInfo`, which is some time after the caller decided to observe, so
+     *   the claim is checked again at the last possible moment rather than trusted from then. See
+     *   [ScreenOwnership], and issue #49.
     */
-    private fun observe(id: UUID, inputs: List<InputFile>, cancelled: JoinState = JoinState.Ready(inputs)) {
+    private fun observe(
+        id: UUID,
+        inputs: List<InputFile>,
+        cancelled: JoinState = JoinState.Ready(inputs),
+        token: Long,
+    ) {
        observer?.cancel()
        observer = viewModelScope.launch {
            workManager.getWorkInfoByIdFlow(id).collect { info ->
                if (info == null) return@collect
+                // Ahead of the `when`, not merely ahead of the assignment: the SUCCEEDED branch
+                // 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 ->
@@ -229,29 +315,38 @@ class JoinViewModel @JvmOverloads constructor(
     * join offered by reattachment was last seen during a tag query that may be hours old, and
     * `cacheDir` is reclaimed by the OS and swept by this app. Without it the file's absence
     * reached the screen as a raw ENOENT path.
+     *
+     * Reached from [JoinState.Joined] and again from a [JoinState.Failed] an earlier save left
+     * carrying its file; [pendingSave] is what makes those the same call.
     */
    fun save(destination: Uri) {
-        val joined = _state.value as? JoinState.Joined ?: return
-        if (!joined.staged.isFile) {
+        val pending = _state.value.pendingSave() ?: return
+        if (!pending.staged.isFile) {
+            // No retry handle -- the file it would offer again is the one that has gone.
            _state.value = JoinState.Failed(STAGED_FILE_GONE_MESSAGE)
            return
        }
        viewModelScope.launch {
            runCatching {
                withContext(Dispatchers.IO) {
-                    publisher.publish(joined.staged, destination)
-                    joined.staged.delete()
+                    publisher.publish(pending.staged, destination)
+                    pending.staged.delete()
                }
            }.onSuccess {
                // publish() already deleted it; nothing left to clean up.
                pendingStaged = null
-                _state.value = JoinState.Saved(joined.suggestedName)
+                _state.value = JoinState.Saved(pending.suggestedName)
            }.onFailure { e ->
                // Deliberately NOT cleared -- see the same branch in ConversionViewModel.
                // A failed save can leave the staged file as the only copy of the work, so
                // it is left for a later reset() or for the sweep to collect once its age
                // makes it certain nobody is coming back for it.
-                _state.value = JoinState.Failed(e.message ?: "Could not save the file.")
+                //
+                // `pending` travels on the state so the screen can offer the file again rather
+                // 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)
            }
        }
    }
@@ -261,8 +356,16 @@ class JoinViewModel @JvmOverloads constructor(
     *
     * Best effort, not a guarantee: the delete is cancelled with [viewModelScope] if the
     * Activity finishes first. `OutputPublisher.sweepStaging` is the backstop.
+     *
+     * It deletes from a [JoinState.Failed] carrying a [PendingSave] too, deliberately and for the
+     * reason `ConversionViewModel.reset` writes out: the screen offers "Try saving again" above
+     * this button, so deletion is what the user chose rather than all this state could do.
     */
    fun reset() {
+        // Start over is a claim like any other. The cancel below is a request honoured at the next
+        // suspension point, so a collector already on its way to a write has nothing left to
+        // honour it at; the claim is what stops that write landing on top of Idle.
+        ownership.claim()
        observer?.cancel()
        observer = null
        activeWorkId = null
@@ -129,9 +129,25 @@ object ContainerCapabilities {
            }
        }

-        if (spec.videoCodec == VideoCodec.NONE && spec.audioCodec == AudioCodec.NONE) {
+        // Two faces of one rule: the output would carry no tracks at all.
+        //
+        // The first is visible in the spec alone — NONE on both axes. The second only emerges once
+        // the spec meets the probe, because [CopyPlanner] drops a video track the *input* does not
+        // have no matter which codec was named for it, so "H.265 + no audio" on an MP3 plans to
+        // (Drop, Drop) exactly as "None + None" does. Asking the spec alone answered the first and
+        // missed the second, and the miss was not cosmetic: `EditedMediaItem.Builder` refuses that
+        // composition with IllegalStateException("Audio and video cannot both be removed"), on
+        // Transformer's own thread, where the user would have seen a dead app rather than a reason.
+        if (spec.audioCodec == AudioCodec.NONE && (spec.videoCodec == VideoCodec.NONE || !probe.hasVideo)) {
            return Validation.Invalid(
-                "This would produce an empty file — keep at least one track.",
+                if (spec.videoCodec == VideoCodec.NONE) {
+                    "This would produce an empty file — keep at least one track."
+                } else {
+                    // Names both halves. "No video track" alone reads as though the video setting
+                    // were the only thing wrong, and the user would fix that and still be stuck.
+                    "This file has no video track, so turning the audio off too would produce an " +
+                        "empty file."
+                },
                suggestions(
                    // Ask for both tracks back, then let repair settle what this container and
                    // this input can actually give.
@@ -162,7 +178,12 @@ object ContainerCapabilities {
            if (!probe.hasVideo) {
                return Validation.Invalid(
                    "This file has no video track to copy.",
-                    listOf(spec.copy(videoCodec = VideoCodec.NONE)),
+                    // Dropping the video is the right shape of answer, but it is only half of one:
+                    // `spec.copy(videoCodec = NONE)` is valid exactly when the audio axis already
+                    // happened to be fine, and refused otherwise — a Vorbis or PCM source into MP4,
+                    // an MP3 into WebM. Handing it to the shared path repairs both axes and drops
+                    // anything that still fails, so the chip cannot lead to a second error.
+                    suggestions(spec.copy(videoCodec = VideoCodec.NONE), probe, exclude = spec),
                )
            }
            val source = CodecNames.videoFromName(probe.videoCodec)
@@ -284,8 +305,12 @@ object ContainerCapabilities {
    private fun repairVideo(spec: OutputSpec, probe: InputProbe): VideoCodec {
        val container = spec.container
        if (spec.videoCodec == VideoCodec.NONE || !container.canHoldVideo) return VideoCodec.NONE
+        // There is no video track to make one out of, so naming a codec would be a suggestion
+        // [CopyPlanner] drops on the floor. It also read as a non-sequitur: before this line, the
+        // repair offered for an MP3 was "H.264", the first codec MP4 happens to encode.
+        if (!probe.hasVideo) return VideoCodec.NONE

-        val source = CodecNames.videoFromName(probe.videoCodec).takeIf { probe.hasVideo }
+        val source = CodecNames.videoFromName(probe.videoCodec)
        val copyable = source != null && accepts(container, source, CodecMode.COPY)

        return when {
@@ -45,6 +45,17 @@ object TestTags {

    const val SAVE_FILE: String = "action.saveFile"

+    /**
+     * The retry a `Failed` offers after a save that threw, on both screens.
+     *
+     * Its own tag rather than [SAVE_FILE], because the two are different claims about the screen.
+     * [SAVE_FILE] is the first attempt from a finished job; this one may appear only where a staged
+     * file survived a failed save. Sharing a tag would collapse "a transcode failure offers nothing
+     * to save" and "a failed save offers the file again" into one query, and that first assertion
+     * is the one stopping a Save button from appearing where there is nothing to save.
+     */
+    const val RETRY_SAVE: String = "action.retrySave"
+
    /** `ConverterScreen`. */
    object Converter {
        const val CHOOSE_FILE: String = "converter.chooseFile"
@@ -25,8 +25,18 @@ private val LightColorScheme = lightColorScheme(
 * Material 3 theme.
 *
 * Dynamic color (Material You) needs API 31+; minSdk is 33, so it is available
- * unconditionally and no version guard is required. It stays switchable so users can
- * opt back to the brand palette.
+ * unconditionally and no version guard is required.
+ *
+ * [dynamicColor] has no caller. `MainActivity` is the single call site and takes the
+ * default, so the parameter is always `true`, the two dynamic branches always win, and
+ * [DarkColorScheme] and [LightColorScheme] are dead: nothing in the app can opt back to the
+ * brand palette. `ThemeColorSchemeTest` reaches those two branches only by passing
+ * [dynamicColor] explicitly -- a test doing it, not a feature.
+ *
+ * That is known rather than an oversight. #68 holds the choice between adding a switch,
+ * deleting the dead branches together with the template palette, and replacing that palette
+ * first; it is undecided, so nothing here should be read as a promise that any of them
+ * happens.
 */
@Composable
 fun LibreMediaConverterTheme(
@@ -0,0 +1,98 @@
+package org.libremediaconverter.ci
+
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import java.io.File
+
+/**
+ * That a hung unit-test run still ends by itself, and still says why.
+ *
+ * `:app:testDebugUnitTest` had no timeout of any kind until #125 was filed. That ticket is a real
+ * Java-level deadlock between Room's `TransactionExecutor` and WorkManager's `SerialExecutorImpl`,
+ * reached through the WorkInfo flow the ViewModel collects, and one local run sat in it for 47
+ * minutes. Nothing inside the suite could break it: the deadlock is monitor contention, which is
+ * not interruptible, so it runs until something outside the JVM gives up.
+ *
+ * Two numbers in `app/build.gradle.kts` are what bound it now, and neither compiles, so nothing
+ * else would notice their removal:
+ *
+ *  - `timeout.set(...)` on every `Test` task, which stops the forked test JVM.
+ *  - the watchdog's `dumpAfterNanos`, which jstacks that JVM *before* the timeout kills it.
+ *
+ * The second is the one worth guarding hardest, and the one that most looks like stray config.
+ * Gradle's timeout kills without a thread dump, and the jstack -- with its "Found one Java-level
+ * deadlock" section naming both monitors -- is the only reason #125 could be described at all.
+ * The ordering between the two numbers is what makes it work: dump first, kill second. Reverse
+ * them, or delete the watchdog, and the suite still stops hanging but every hang from then on
+ * reports as a bare "Timeout has been exceeded" with nothing to read. Measured against a probe
+ * that hung one test: no test XML was written for the class that hung, so the hanging test itself
+ * gets no attribution from the report at all.
+ *
+ * The range on the timeout is not decoration either, and it is the half a future edit is most
+ * likely to get wrong. Below it, a healthy-but-slow runner trips the bound and a real signal
+ * becomes noise people learn to re-run through; above it, CI's 30-minute job cap fires first and
+ * the bound never gets to say anything.
+ *
+ * `ReleasePermissionTest` is the precedent and its caveat applies here too. This asserts the two
+ * numbers are present, sanely sized and correctly ordered. It cannot assert that the timeout
+ * fires -- that needs a hang, which is what the whole change exists to prevent. Refs #125.
+ */
+class HangBoundTest {
+
+    @Test
+    fun `every Test task is bounded, and bounded between the slow runner and the job cap`() {
+        assertTrue(
+            "app/build.gradle.kts sets its Test task timeout to ${timeoutMinutes}m, which is " +
+                "outside $SANE_MINUTES. Under that range a slow CI runner trips a bound meant for " +
+                "deadlocks -- the slowest observed passing run of the whole invocation was 90s. " +
+                "Over it, the Unit tests job's own 30-minute cap kills the job first and the " +
+                "timeout never reports. `null` means the line is gone or the block was rewritten, " +
+                "and without it #125's deadlock has nothing to stop it: monitor contention breaks " +
+                "no interrupt, so it runs until CI gives up and reports a timeout with no cause.",
+            timeoutMinutes in SANE_MINUTES,
+        )
+    }
+
+    @Test
+    fun `the thread dump is taken before the timeout kills the JVM it would dump`() {
+        assertTrue(
+            "app/build.gradle.kts takes its hang thread dump after ${dumpAfterMinutes}m but times " +
+                "the task out at ${timeoutMinutes}m, so the JVM is already dead when jstack runs " +
+                "and every future hang reports as a bare `Timeout has been exceeded`. The dump " +
+                "has to come first -- it is the only attribution a hanging test gets, since the " +
+                "test XML never names it.",
+            (dumpAfterMinutes ?: 0) < (timeoutMinutes ?: 0),
+        )
+    }
+
+    /** Minutes given to a whole `Test` task before Gradle stops the forked JVM. */
+    private val timeoutMinutes: Int?
+        get() = minutesIn("""timeout\.set\(Duration\.ofMinutes\((\d+)\)\)""")
+
+    /** Minutes the watchdog waits before jstacking the forked JVM. */
+    private val dumpAfterMinutes: Int?
+        get() = minutesIn("""val dumpAfterNanos = Duration\.ofMinutes\((\d+)\)""")
+
+    /**
+     * Read out of the build script rather than from a model: the numbers live in a Kotlin DSL block
+     * that no unit test can instantiate, and a scan that reports `null` when the shape changes is a
+     * better trade than not checking them at all.
+     */
+    private fun minutesIn(pattern: String): Int? =
+        Regex(pattern).find(buildScript.readText())?.groupValues?.get(1)?.toInt()
+
+    /**
+     * Found by walking up rather than by a fixed relative path: Gradle's working directory for the
+     * unit tests is the module, but that is a default rather than a promise.
+     */
+    private val buildScript: File
+        get() = generateSequence(File(".").absoluteFile) { it.parentFile }
+            .map { File(it, "app/build.gradle.kts") }
+            .firstOrNull { it.isFile }
+            ?: error("could not find app/build.gradle.kts above ${File(".").absolutePath}")
+
+    private companion object {
+        /** Above the slowest observed passing run, below the Unit tests job's `timeout-minutes`. */
+        val SANE_MINUTES = 3..29
+    }
+}
@@ -93,8 +93,12 @@ class ConversionViewModelCleanupTest {
        assertTrue("a failed save must not destroy the only copy", staged.exists())
        assertEquals(emptyList<File>(), publisher.discarded)

-        // Failed carries no file reference at all, so this only works because the handle is
-        // a ViewModel field rather than something read back out of the state machine.
+        // The handle is a ViewModel field rather than something read back out of the state
+        // machine, and stays one now that a save-failed `Failed` also carries a `PendingSave`:
+        // that is a view for the screen to offer a retry through, never a second owner of the
+        // file. This delete goes through the field, which is what keeps a state that is dropped
+        // rather than read from taking the only reference with it. What the state carries, and
+        // what the screen then does with it, are `FailedSaveRetryTest`'s.
        viewModel.reset()

        assertEquals(listOf(staged), publisher.discarded)
@@ -53,10 +53,11 @@ import java.io.File
 *   it -- so it is unobservable from a JVM test, the same limit `FileCardTest` records for
 *   `HorizontalDivider`. The message text itself is asserted; the colour would need a screenshot.
 * - **The three `assertDoesNotExist` checks on [TestTags.Converter.FILE_CARD] are compile-guarded,
- *   not guarded by this file.** `Idle` is a `data object`, and `Saved` and `Failed` carry only a
- *   `displayName` and a `message`; none of the three has an `input`, so `FileCard(s.input)` does not
- *   compile in those arms. The lines stay because they state the intent cheaply, but they are not
- *   what stops a `FileCard` appearing there and this file does not claim they are.
+ *   not guarded by this file.** `Idle` is a `data object`, `Saved` carries a `displayName`, and
+ *   `Failed` carries a message and -- after a failed save only -- the staged file it left behind;
+ *   none of the three has an `input`, so `FileCard(s.input)` does not compile in those arms. The
+ *   lines stay because they state the intent cheaply, but they are not what stops a `FileCard`
+ *   appearing there and this file does not claim they are.
 * - **Which constant each chip hands back** belongs to `ConverterPickerSelectionTest`, and **what
 *   the file card says about an unknown size** to `FileCardTest`. This file asserts that `Ready`
 *   puts those leaves on screen at all, not what they then do.
@@ -326,6 +327,22 @@ class ConverterStateAffordancesTest {
        composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD).assertDoesNotExist()
    }

+    /**
+     * **The assertion that bounds #30's whole change**, and the one worth breaking things to keep.
+     *
+     * A transcode that died staged nothing, so its `Failed` carries no [PendingSave] and there is
+     * nothing for a save dialog to be handed. Making the retry unconditional -- or making the
+     * `WorkInfo.State.FAILED` arm of `ConversionViewModel.observe` carry a handle it has no file
+     * for -- puts a button on screen that can only fail, and this is what notices.
+     */
+    @Test
+    fun `a transcode failure offers no way to save`() {
+        setContent(ConversionState.Failed(message = "Ran out of space while writing the output."))
+
+        composeRule.onNodeWithTag(TestTags.RETRY_SAVE).assertDoesNotExist()
+        composeRule.onNodeWithTag(TestTags.SAVE_FILE).assertDoesNotExist()
+    }
+
    @Test
    fun `tapping start over after a failure resets and does nothing else`() {
        setContent(ConversionState.Failed(message = "Ran out of space while writing the output."))
@@ -335,6 +352,50 @@ class ConverterStateAffordancesTest {
        assertEquals(listOf("reset"), fired)
    }

+    // ----------------------------------------------------- Failed, carrying a file
+
+    /**
+     * The defect in #30, stated as what the branch must render.
+     *
+     * `save()` keeps the staged file on a failure deliberately -- it can be the only copy of an
+     * hour of transcoding -- and before this the only control here was "Start over", wired to
+     * `reset()`, which deletes exactly that file. Both buttons, not one: the restart has to stay
+     * reachable, because leaving a full-size file in cache is the outcome it exists to avoid.
+     */
+    @Test
+    fun `a failed save offers the file again as well as a restart`() {
+        setContent(failedSave())
+
+        composeRule.onNodeWithTag(TestTags.RETRY_SAVE).assertExists()
+        composeRule.onNodeWithTag(TestTags.START_OVER).assertExists()
+    }
+
+    /**
+     * The name, not just that something fired: it comes from the finished job, and a retry wired to
+     * a literal or to the picker's current guess would hand the dialog a name the job never chose.
+     */
+    @Test
+    fun `tapping try saving again hands back the name the job chose`() {
+        setContent(failedSave())
+
+        composeRule.onNodeWithTag(TestTags.RETRY_SAVE).performScrollTo().performClick()
+
+        assertEquals(listOf("save:holiday.mp4"), fired)
+    }
+
+    /**
+     * Start over from here still resets, and resetting still deletes -- see `reset()`'s KDoc for
+     * why that is acceptable now and was not before. What it must not do is save on the way past.
+     */
+    @Test
+    fun `tapping start over after a failed save resets and does not save`() {
+        setContent(failedSave())
+
+        composeRule.onNodeWithTag(TestTags.START_OVER).performScrollTo().performClick()
+
+        assertEquals(listOf("reset"), fired)
+    }
+
    // ------------------------------------------------------------------ Harness

    private fun input() = InputFile(
@@ -348,6 +409,21 @@ class ConverterStateAffordancesTest {
     * missing file rather than throwing, so the size line reads `0 B` and no temporary folder is
     * needed to render the arm.
     */
+    /**
+     * A `Failed` an earlier save left carrying its file, which is the only way [retry] is non-null.
+     *
+     * The same missing `staged` path as [converted], and for the same reason: this arm renders no
+     * size line at all, so nothing here ever touches the filesystem.
+     */
+    private fun failedSave() = ConversionState.Failed(
+        message = "There was not enough room on the destination.",
+        retry = PendingSave(
+            staged = File("no-such-staged-output.mp4"),
+            suggestedName = "holiday.mp4",
+            mimeType = "video/mp4",
+        ),
+    )
+
    private fun converted(routeReason: String = "") = ConversionState.Converted(
        input = input(),
        staged = File("no-such-staged-output.mp4"),
@@ -0,0 +1,370 @@
+package org.libremediaconverter.convert
+
+import android.app.Application
+import android.net.Uri
+import androidx.media3.common.util.UnstableApi
+import androidx.work.workDataOf
+import kotlinx.coroutines.Dispatchers
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertNull
+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.pendingSave
+import org.libremediaconverter.model.InputProbe
+import org.libremediaconverter.model.OutputFormat
+import org.libremediaconverter.work.ConcatWorker
+import org.libremediaconverter.work.ConversionWorker
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import java.io.File
+
+/**
+ * A failed save has to leave the file *offerable*, not merely undeleted.
+ *
+ * The defect is #30, and both halves of it were already written down in `main`. `save()`'s
+ * `onFailure` kept the staged file on purpose -- "deleting here would destroy the work to tidy up
+ * a cache directory" -- and then handed the screen a `Failed` carrying a message and nothing else,
+ * so the single control that branch rendered was "Start over", wired to `reset()`, which deletes
+ * exactly that file. The intent and the affordance disagreed, and the affordance won.
+ *
+ * `ConversionViewModelCleanupTest` already pins the *keeping*: after a failed save the file is
+ * still on disk and nothing has been discarded. It stays green with the state carrying nothing,
+ * because it reads the filesystem rather than the state. This file asserts the other half -- that
+ * the handle reaches the state a screen can read -- and the negative that bounds it: a failure
+ * with nothing staged behind it must not sprout a save button.
+ *
+ * Both ViewModels in one class, following `MissingStagedFileTest`. They are separate state
+ * machines that can each hold a staged file at once, but this defect and its fix are the same
+ * shape in both, and splitting them would put the two halves of one invariant in two files.
+ *
+ * ### Not asserted here, so each is a decision rather than an omission
+ *
+ * - **That the destination received the bytes.** [RecordingPublisher.publish] is a stub, which is
+ *   the only way to make a save fail deterministically -- and making it fail is what every case
+ *   here needs. `OutputPublisherPublishTest` owns what a real publish writes.
+ * - **The screen's two buttons.** `ConverterStateAffordancesTest` and `JoinStateAffordancesTest`
+ *   own what each state renders; this file owns what each state carries.
+ * - **`ConverterScreen`'s `destinationMime` line itself.** It lives in the entry point, above the
+ *   `ScreenContent` seam, and reaching it needs a real ViewModel inside a composition. What it
+ *   reads -- `pendingSave()?.mimeType` -- is asserted directly instead, which is why that
+ *   derivation was moved out of the entry point in the first place.
+ * - **Picking a new input while a `Failed` carries a file.** `onInputPicked` overwrites the state
+ *   without discarding, from `Converted` exactly as much as from a carrying `Failed`, and neither
+ *   branch renders a picker. It is a pre-existing path this change neither opens nor widens: the
+ *   carried handle is a view of `pendingStaged`, never a second owner of the file.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class FailedSaveRetryTest {
+
+    private lateinit var app: Application
+    private lateinit var publisher: RecordingPublisher
+    private lateinit var staged: File
+
+    @Before
+    fun setUp() {
+        app = RuntimeEnvironment.getApplication()
+        publisher = RecordingPublisher(app)
+        ConversionDependencies.publisher = { publisher }
+        // MediaProbe spawns FFprobe, whose loader throws with no native library present.
+        ConversionDependencies.probe = { _, _ -> InputProbe() }
+
+        staged = publisher.createStagingFile("holiday.mp4").apply { writeBytes(ByteArray(4096)) }
+    }
+
+    @After
+    fun tearDown() {
+        ConversionDependencies.reset()
+    }
+
+    // ------------------------------------------------------------------ Convert
+
+    /**
+     * The bite named in #30's fix. Emitting a plain `Failed` from `save()`'s `onFailure` -- which
+     * is what `main` did -- reddens this case on the null handle, and nothing else in the suite.
+     */
+    @Test
+    fun `a failed save leaves the staged file offerable, not merely undeleted`() {
+        val viewModel = failedSaveViewModel()
+
+        val failed = viewModel.state.value as ConversionState.Failed
+        val retry = failed.retry
+        assertNotNull("a failed save must leave the staged file offerable, not just on disk", retry)
+        assertEquals("the retry must name the file the conversion actually produced", staged, retry?.staged)
+        // Both from the job's own output Data rather than from the pickers, so a retry opens the
+        // same dialog the first attempt did.
+        assertEquals(SUGGESTED_NAME, retry?.suggestedName)
+        assertEquals(JOB_MIME_TYPE, retry?.mimeType)
+        assertTrue("a failed save must not destroy the only copy", staged.exists())
+    }
+
+    /**
+     * The whole point of carrying the handle: the second attempt is a real save, not a new job.
+     *
+     * `publishFailure` is cleared between the two calls, so one `RecordingPublisher` plays both a
+     * full destination and an empty one -- which is exactly the user's situation.
+     */
+    @Test
+    fun `retrying a failed save publishes the file and leaves nothing staged`() {
+        val viewModel = failedSaveViewModel()
+
+        publisher.publishFailure = null
+        viewModel.save(DESTINATION)
+
+        val saved = awaitState(viewModel.state, "Saved") { it is ConversionState.Saved }
+        assertEquals(SUGGESTED_NAME, (saved as ConversionState.Saved).displayName)
+        assertFalse("a successful retry should have removed the staged file", staged.exists())
+        // Nothing to collect afterwards: the retry published it, so reset() has no work left.
+        viewModel.reset()
+        assertEquals(emptyList<File>(), publisher.discarded)
+    }
+
+    /**
+     * The second failure must not eat the file the first one kept.
+     *
+     * A `Failed` built fresh from `e.message` alone would drop the handle here while every other
+     * assertion in this file stayed green -- the file is still on disk, and the first failure
+     * already proved the state can carry it.
+     */
+    @Test
+    fun `a retry that fails again still carries the file rather than dropping it`() {
+        val viewModel = failedSaveViewModel()
+
+        publisher.publishFailure = IllegalStateException("destination volume still full")
+        viewModel.save(DESTINATION)
+
+        // Waited for by the *second* message rather than by `is Failed`: the state was already
+        // Failed when the retry started, so the type alone would be satisfied before it ran.
+        val failed = awaitState(viewModel.state, "the second failure") {
+            it is ConversionState.Failed && it.message == "destination volume still full"
+        } as ConversionState.Failed
+        assertEquals("the second failure must offer the same file the first one did", staged, failed.retry?.staged)
+        assertTrue(staged.exists())
+        assertEquals(emptyList<File>(), publisher.discarded)
+    }
+
+    /**
+     * "Start over" still deletes, and that is the decision `reset()`'s KDoc records: acceptable
+     * only because "Try saving again" is on screen beside it. Exactly once, through the publisher.
+     */
+    @Test
+    fun `start over from a failed save discards the carried file exactly once`() {
+        val viewModel = failedSaveViewModel()
+
+        viewModel.reset()
+
+        assertEquals(ConversionState.Idle, viewModel.state.value)
+        assertEquals(listOf(staged), publisher.discarded)
+        assertFalse(staged.exists())
+    }
+
+    /**
+     * A retry meets the same existence check the first attempt did, so a file collected by the
+     * sweep or by the OS in between is reported as a sentence rather than as a raw ENOENT path.
+     * And the state that reports it carries nothing: there is no file left to offer.
+     */
+    @Test
+    fun `a retry whose staged file has gone says so and offers nothing further`() {
+        val viewModel = failedSaveViewModel()
+        assertTrue("the fixture must start with a real staged file", staged.delete())
+
+        publisher.publishFailure = null
+        viewModel.save(DESTINATION)
+
+        val failed = viewModel.state.value as ConversionState.Failed
+        assertEquals(STAGED_FILE_GONE_MESSAGE, failed.message)
+        assertNull("a file that has gone cannot be offered again", failed.retry)
+    }
+
+    /**
+     * The negative that bounds the whole change, and the reason `retry` is nullable.
+     *
+     * A transcode that died staged nothing, so there is no file to hand back -- and a `Failed`
+     * that carried one anyway would put a save button on a screen with nothing to save. Driven
+     * through a worker that really fails rather than by constructing the state, because the line
+     * under test is the `WorkInfo.State.FAILED` arm of `observe`.
+     */
+    @Test
+    fun `a transcode failure carries nothing to save`() {
+        installFailingTestWorkManager(app, workDataOf(ConversionWorker.KEY_ERROR to "The encoder gave up."))
+        val viewModel = ConversionViewModel(app, Dispatchers.Unconfined)
+        viewModel.onInputPicked(Uri.parse("content://test/holiday.mp4"))
+        awaitState(viewModel.state, "Ready") { it is ConversionState.Ready }
+
+        viewModel.convert()
+
+        val failed = awaitState(viewModel.state, "Failed") { it is ConversionState.Failed } as ConversionState.Failed
+        assertEquals("The encoder gave up.", failed.message)
+        assertNull("a transcode failure has nothing staged, so it must offer no save", failed.retry)
+        assertNull("and nothing for the save dialog to open with either", failed.pendingSave())
+    }
+
+    /**
+     * What the save dialog reopens with, which is the entry point's only reader of this state.
+     *
+     * The pickers are moved *after* the job finishes, which is what makes this bite: a retry that
+     * asked the current settings would offer `audio/mpeg` for a file the job wrote as MP4. The
+     * same gap is permanent for a reattached job, whose spec was never in these settings at all.
+     */
+    @Test
+    fun `a retry offers the type the job chose, not the one the pickers now show`() {
+        val viewModel = failedSaveViewModel()
+
+        viewModel.setPreset(OutputFormat.MP3)
+
+        assertEquals(
+            "the fixture needs the pickers to disagree with the job",
+            "audio/mpeg",
+            viewModel.settings.value.spec.mimeType,
+        )
+        assertEquals(JOB_MIME_TYPE, viewModel.state.value.pendingSave()?.mimeType)
+    }
+
+    // --------------------------------------------------------------------- Join
+
+    @Test
+    fun `a failed join save leaves the staged file offerable, not merely undeleted`() {
+        val viewModel = failedJoinSaveViewModel()
+
+        val failed = viewModel.state.value as JoinState.Failed
+        val retry = failed.retry
+        assertNotNull("a failed save must leave the staged file offerable, not just on disk", retry)
+        assertEquals(staged, retry?.staged)
+        assertEquals(SUGGESTED_NAME, retry?.suggestedName)
+        assertEquals(JOB_MIME_TYPE, retry?.mimeType)
+        assertTrue(staged.exists())
+    }
+
+    @Test
+    fun `retrying a failed join save publishes the file and leaves nothing staged`() {
+        val viewModel = failedJoinSaveViewModel()
+
+        publisher.publishFailure = null
+        viewModel.save(DESTINATION)
+
+        val saved = awaitState(viewModel.state, "Saved") { it is JoinState.Saved }
+        assertEquals(SUGGESTED_NAME, (saved as JoinState.Saved).displayName)
+        assertFalse(staged.exists())
+        viewModel.reset()
+        assertEquals(emptyList<File>(), publisher.discarded)
+    }
+
+    @Test
+    fun `a join retry that fails again still carries the file rather than dropping it`() {
+        val viewModel = failedJoinSaveViewModel()
+
+        publisher.publishFailure = IllegalStateException("destination volume still full")
+        viewModel.save(DESTINATION)
+
+        // By the second message, not by `is Failed` -- see the converter case above.
+        val failed = awaitState(viewModel.state, "the second failure") {
+            it is JoinState.Failed && it.message == "destination volume still full"
+        } as JoinState.Failed
+        assertEquals(staged, failed.retry?.staged)
+        assertTrue(staged.exists())
+        assertEquals(emptyList<File>(), publisher.discarded)
+    }
+
+    @Test
+    fun `start over from a failed join save discards the carried file exactly once`() {
+        val viewModel = failedJoinSaveViewModel()
+
+        viewModel.reset()
+
+        assertEquals(JoinState.Idle, viewModel.state.value)
+        assertEquals(listOf(staged), publisher.discarded)
+        assertFalse(staged.exists())
+    }
+
+    @Test
+    fun `a join failure carries nothing to save`() {
+        installFailingTestWorkManager(app, workDataOf(ConcatWorker.KEY_ERROR to "The files could not be joined."))
+        val viewModel = JoinViewModel(app, Dispatchers.Unconfined)
+        viewModel.onInputsPicked(listOf(Uri.parse("content://test/a.mp4"), Uri.parse("content://test/b.mp4")))
+        awaitState(viewModel.state, "Ready") { it is JoinState.Ready }
+
+        viewModel.join()
+
+        val failed = awaitState(viewModel.state, "Failed") { it is JoinState.Failed } as JoinState.Failed
+        assertEquals("The files could not be joined.", failed.message)
+        assertNull("a join failure has nothing staged, so it must offer no save", failed.retry)
+        assertNull(failed.pendingSave())
+    }
+
+    // ------------------------------------------------------------------ Harness
+
+    /**
+     * A ViewModel driven to `Converted` and then through a save that threw.
+     *
+     * The WorkManager is installed here rather than in `@Before`, because two cases in this class
+     * need one whose workers fail instead.
+     */
+    private fun failedSaveViewModel(): ConversionViewModel {
+        installTestWorkManager(app, conversionOutput())
+        // Unconfined so reset()'s delete runs inline instead of on a real IO thread.
+        val viewModel = ConversionViewModel(app, Dispatchers.Unconfined)
+        viewModel.onInputPicked(Uri.parse("content://test/holiday.mkv"))
+        awaitState(viewModel.state, "Ready") { it is ConversionState.Ready }
+        viewModel.convert()
+        awaitState(viewModel.state, "Converted") { it is ConversionState.Converted }
+
+        publisher.publishFailure = IllegalStateException("destination volume full")
+        viewModel.save(DESTINATION)
+        awaitState(viewModel.state, "Failed") { it is ConversionState.Failed }
+        return viewModel
+    }
+
+    /** The join tab's equivalent, driven to `Joined` and then through a save that threw. */
+    private fun failedJoinSaveViewModel(): JoinViewModel {
+        installTestWorkManager(app, joinOutput())
+        val viewModel = JoinViewModel(app, Dispatchers.Unconfined)
+        viewModel.onInputsPicked(listOf(Uri.parse("content://test/a.mp4"), Uri.parse("content://test/b.mp4")))
+        awaitState(viewModel.state, "Ready") { it is JoinState.Ready }
+        viewModel.join()
+        awaitState(viewModel.state, "Joined") { it is JoinState.Joined }
+
+        publisher.publishFailure = IllegalStateException("destination volume full")
+        viewModel.save(DESTINATION)
+        awaitState(viewModel.state, "Failed") { it is JoinState.Failed }
+        return viewModel
+    }
+
+    /**
+     * The output `Data` a finished conversion reports.
+     *
+     * The name and type are set rather than left out, so the assertions above are about what the
+     * *job* chose. Both ViewModels fall back to a derivation when they are missing, and a fixture
+     * that omitted them would be asserting the fallback while looking like it asserted the job.
+     *
+     * Spelled out per worker rather than shared with [joinOutput], even though the two constants
+     * hold the same strings today. A test that leaned on that would be asserting a coincidence.
+     */
+    private fun conversionOutput() = workDataOf(
+        ConversionWorker.KEY_OUTPUT_PATH to staged.absolutePath,
+        ConversionWorker.KEY_SUGGESTED_NAME to SUGGESTED_NAME,
+        ConversionWorker.KEY_MIME_TYPE to JOB_MIME_TYPE,
+    )
+
+    /** The output `Data` a finished join reports. See [conversionOutput]. */
+    private fun joinOutput() = workDataOf(
+        ConcatWorker.KEY_OUTPUT_PATH to staged.absolutePath,
+        ConcatWorker.KEY_SUGGESTED_NAME to SUGGESTED_NAME,
+        ConcatWorker.KEY_MIME_TYPE to JOB_MIME_TYPE,
+    )
+
+    private companion object {
+        val DESTINATION: Uri = Uri.parse("content://test/destination.mp4")
+
+        const val SUGGESTED_NAME = "holiday.mp4"
+
+        /** What the job wrote. [OutputFormat.MP3]'s `audio/mpeg` is what the pickers move to. */
+        const val JOB_MIME_TYPE = "video/mp4"
+    }
+}
@@ -0,0 +1,105 @@
+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.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.InputKind
+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
+
+/**
+ * What happens when Media3 refuses the export before it starts.
+ *
+ * `EditedMediaItem.Builder` rejects a composition with both tracks removed —
+ * checkState("Audio and video cannot both be removed") — and [Media3Engine] builds it on its own
+ * HandlerThread. That build used to sit *between* two narrow `runCatching` blocks, one around
+ * `buildTransformer` and one around `start`, so the exception escaped `handler.post`'s body: it
+ * reached the thread's uncaught handler, which on Android takes the process down, and the
+ * continuation was left unresumed either way.
+ *
+ * Robolectric runs the real [android.os.HandlerThread] and the real Media3 builders, so the whole
+ * sequence happens here — the engine really posts, really builds, and really throws. What it cannot
+ * reproduce is the *consequence* of an escaped throw: a JVM background thread dying is not process
+ * death. So the assertion is on the half that is observable everywhere and is the half that
+ * matters to the user — the suspension is resolved, with the reason, rather than left hanging.
+ * `Media3EngineTest.aPlanThatRemovesBothTracksFailsInsteadOfKillingTheProcess` is the same case on
+ * a device.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class Media3EngineEmptyCompositionTest {
+
+    @Test
+    fun `a plan that removes both tracks fails the job instead of escaping the handler thread`() {
+        val context = RuntimeEnvironment.getApplication()
+        val engine = Media3Engine(context)
+        val request = ConversionRequest(
+            spec = OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE),
+            probe = InputProbe(
+                videoCodec = null,
+                audioCodec = "mp3",
+                hasVideo = false,
+                container = Container.MP3,
+                kind = InputKind.AUDIO_ONLY,
+            ),
+        )
+
+        // Asserted rather than assumed: ConversionRequest's default probe says hasVideo = true, and
+        // with it this same spec plans to (Encode, Drop), nothing throws, and the test would pass
+        // over a code path it never entered.
+        val plan = CopyPlanner.plan(request.spec, request.probe)
+        assertEquals(VideoPlan.Drop, plan.video)
+        assertEquals(AudioPlan.Drop, plan.audio)
+
+        val failure = try {
+            runCatching {
+                runBlocking {
+                    withTimeout(TIMEOUT_MS) {
+                        engine.transcode(Uri.parse("file:///dev/null"), File(context.cacheDir, "empty.mp4"), request) {}
+                    }
+                }
+            }.exceptionOrNull()
+        } finally {
+            engine.close()
+        }
+
+        // Both halves are load-bearing, and the second is not pedantry: withTimeout raises
+        // TimeoutCancellationException, and `java.util.concurrent.CancellationException` *extends*
+        // IllegalStateException — so testing only the first would call an unresumed continuation a
+        // pass. This assertion was written that way, and the mutation is what found it.
+        assertFalse(
+            "the continuation was never resumed — the failure escaped instead of being reported: $failure",
+            failure is CancellationException,
+        )
+        assertTrue(
+            "the builder's refusal must surface as a failed job; got $failure",
+            failure is IllegalStateException,
+        )
+    }
+
+    private companion object {
+        /**
+         * Short on purpose. Nothing is decoded, encoded or muxed on this path — the builder refuses
+         * the input outright — so anything approaching this is a hang, which is the failure mode
+         * this test is looking for.
+         */
+        const val TIMEOUT_MS = 10_000L
+    }
+}
@@ -0,0 +1,63 @@
+package org.libremediaconverter.convert
+
+import kotlinx.coroutines.CoroutineDispatcher
+import java.util.concurrent.ConcurrentLinkedQueue
+import kotlin.coroutines.CoroutineContext
+
+/**
+ * A dispatcher that holds a pick in flight until the test lets it finish.
+ *
+ * Issue #49 is about what a ViewModel does *while* a pick is between the tap and the write it
+ * eventually makes. Both ViewModels put a blocking hop there — the metadata query, and on the
+ * convert side the probe as well — and both hops go through an injectable dispatcher. Handing
+ * them this one turns "the pick has been made but has not landed yet" from a window a test has
+ * to race into a state it can simply sit in.
+ *
+ * Nothing here is a fake pick. The real `InputQuery.describe` still runs, on this thread,
+ * whenever [runAll] is called; the only thing under the test's control is *when*.
+ *
+ * Confined to the thread that drives the test. Both ViewModels reach `withContext(pickDispatcher)`
+ * from a coroutine on the main dispatcher, so [dispatch] is only ever called from there — the
+ * queue is concurrent anyway, because a dispatcher that quietly dropped a block from another
+ * thread would fail as a hang rather than as an assertion.
+ */
+class ParkedPickDispatcher : CoroutineDispatcher() {
+
+    private val parked = ConcurrentLinkedQueue<Runnable>()
+
+    /**
+     * How many blocks are waiting.
+     *
+     * Asserted on before the interesting part of a test, because "the pick was in flight" is a
+     * premise rather than a detail: a zero here means the pick had already landed and whatever
+     * the test went on to prove was proved about a different situation.
+     */
+    val parkedCount: Int get() = parked.size
+
+    override fun dispatch(context: CoroutineContext, block: Runnable) {
+        parked += block
+    }
+
+    /**
+     * Removes everything parked, oldest first, and hands it to the caller to run.
+     *
+     * What [runAll] cannot express: two picks are two hops through this dispatcher, and the defect
+     * they can produce is the *first* one finishing last. Running them in the order they arrived
+     * is the one order in which nothing goes wrong, so a test has to be able to choose.
+     */
+    fun takeParked(): List<Runnable> = generateSequence { parked.poll() }.toList()
+
+    /**
+     * Runs everything parked, and everything that parks as a result.
+     *
+     * The loop is not defensive: `ConversionViewModel.onInputPicked` makes two hops through this
+     * dispatcher — the metadata query, then the probe — and the second is only enqueued once the
+     * first has run. Draining once would leave the probe parked for the rest of the process.
+     */
+    fun runAll() {
+        while (true) {
+            val next = parked.poll() ?: return
+            next.run()
+        }
+    }
+}
@@ -0,0 +1,125 @@
+package org.libremediaconverter.convert
+
+import android.app.Application
+import android.net.Uri
+import androidx.media3.common.util.UnstableApi
+import androidx.work.workDataOf
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertNull
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.libremediaconverter.model.InputProbe
+import org.libremediaconverter.work.ConversionWorker
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import java.io.File
+
+/**
+ * The half of issue #49 that is not about reattachment at all.
+ *
+ * `onInputPicked` makes two writes and both of them land after a hop off the main thread, so both
+ * belong to whichever pick was in flight rather than to whichever pick the user last made. Nothing
+ * was enforcing that. Two taps in quick succession — an easy thing to do while a `content://`
+ * metadata query is slow — put the loser's file on screen if its query happened to come back
+ * second, which is the same defect the ticket reported against reattachment with a different
+ * coroutine on the losing side.
+ *
+ * Both cases below were measured rather than assumed: deleting either guard turns the matching
+ * test red, and deleting the probe one turns nine other tests red with it. Neither was ever
+ * reported, because a pick that loses to another pick still shows *a* file the user chose -- which
+ * is what made it worth closing alongside #49 rather than leaving as a second thing to find.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class PickOwnershipTest {
+
+    private lateinit var app: Application
+    private lateinit var parkedPick: ParkedPickDispatcher
+    private lateinit var viewModel: ConversionViewModel
+
+    @Before
+    fun setUp() {
+        app = RuntimeEnvironment.getApplication()
+        ConversionDependencies.publisher = { RecordingPublisher(app) }
+        ConversionDependencies.probe = { _, _ -> PROBE }
+        installTestWorkManager(app, workDataOf(ConversionWorker.KEY_OUTPUT_PATH to "/dev/null"))
+
+        parkedPick = ParkedPickDispatcher()
+        viewModel = ConversionViewModel(app, pickDispatcher = parkedPick)
+    }
+
+    @After
+    fun tearDown() {
+        ConversionDependencies.reset()
+    }
+
+    /**
+     * Two taps, and the first one's metadata query is the slow one.
+     *
+     * The order is chosen rather than raced: both queries are parked, and this runs the second
+     * before the first. Without an ownership check the straggler writes last and the screen ends
+     * up showing a file the user moved off two taps ago.
+     */
+    @Test
+    fun `the slower of two picks does not land on top of the faster one`() {
+        viewModel.onInputPicked(FIRST)
+        viewModel.onInputPicked(SECOND)
+
+        val queries = parkedPick.takeParked()
+        assertEquals("both picks should be in flight", 2, queries.size)
+        // The second pick's query comes back first; the first pick's is the straggler.
+        queries[1].run()
+        queries[0].run()
+
+        val current = viewModel.state.value
+        assertEquals(
+            "a pick the user has already replaced took the screen: $current",
+            SECOND,
+            (current as ConversionState.Ready).input.uri,
+        )
+    }
+
+    /**
+     * The second of `onInputPicked`'s two writes, which lands a whole probe later.
+     *
+     * The probe hop is a native process spawn, so it is the longest gap in a pick and the easiest
+     * one to pick again during. This used to be guarded by comparing URIs against the state, which
+     * answers a narrower question than the one that matters — it cannot tell a second pick of the
+     * same file from the first, and it reads a state that a later claim may not have written yet,
+     * which is exactly this case: the newer pick has claimed the screen but its own query has not
+     * come back, so the state still names the older file and the comparison waves it through.
+     */
+    @Test
+    fun `a probe from a pick the user has moved off does not fill the card in`() {
+        viewModel.onInputPicked(FIRST)
+        parkedPick.takeParked().single().run()
+        assertEquals(FIRST, (viewModel.state.value as ConversionState.Ready).input.uri)
+
+        // The user picks again while the first pick is still probing.
+        viewModel.onInputPicked(SECOND)
+        val pending = parkedPick.takeParked()
+        assertEquals("the first probe and the second query should both be waiting", 2, pending.size)
+        pending[0].run()
+
+        assertNull(
+            "a probe belonging to a pick the user replaced must not reach the card",
+            (viewModel.state.value as ConversionState.Ready).input.probe,
+        )
+
+        // And the pick that did win still fills its own card in, probe included.
+        pending[1].run()
+        parkedPick.runAll()
+        val settled = viewModel.state.value as ConversionState.Ready
+        assertEquals(SECOND, settled.input.uri)
+        assertNotNull("the winning pick's own probe still has to land", settled.input.probe)
+    }
+
+    private companion object {
+        val FIRST: Uri = Uri.fromFile(File("/tmp/first.mp4"))
+        val SECOND: Uri = Uri.fromFile(File("/tmp/second.mp4"))
+        val PROBE = InputProbe(videoCodec = "h264")
+    }
+}
@@ -0,0 +1,156 @@
+package org.libremediaconverter.convert
+
+import android.app.Application
+import android.net.Uri
+import androidx.media3.common.util.UnstableApi
+import androidx.work.WorkManager
+import androidx.work.workDataOf
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.libremediaconverter.model.InputProbe
+import org.libremediaconverter.work.ConversionWorker
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import java.io.File
+
+/**
+ * Issue #49, on the JVM and without the race.
+ *
+ * `ReattachOnLaunchTest.doesNotOverwriteAPickTheUserHasAlreadyMade` has been catching this on
+ * devices since 2026-08-24 — four occurrences, spread across API 33, 35 and 36, which is what
+ * ruled out an emulator-image quirk. Every one was on attempt 1 and every one passed on re-run,
+ * which is why it was read as flaky infrastructure for two days. It is not. The assertion it fails
+ * on is `expected null, but was:<Converted>`: a finished job from an earlier session taking a
+ * screen the user had already picked a file on.
+ *
+ * The defect is a check-then-act whose act is deferred into another coroutine. `reattach()` reads
+ * `_state.value` and then calls `observe()`, which *launches* a collector that has to suspend on
+ * `getWorkInfoByIdFlow(...).collect` before it can write anything. So the check happens at one
+ * moment and the write lands at another:
+ *
+ * 1. `init` starts the tag query and suspends in it.
+ * 2. The user picks a file; `onInputPicked` suspends in its metadata query.
+ * 3. The query comes back. `_state.value` is still `Idle` — step 2 has not written yet — so the
+ *    guard passes and an observation of the old job is launched.
+ * 4. The pick lands. `Ready(picked)`. The user owns the screen.
+ * 5. The observation's first `WorkInfo` arrives and writes `Converted(yesterday)` over it.
+ *
+ * The comment above that guard claimed "no suspension point between this check and the assignment
+ * below, so nothing can interleave". There is no assignment below, and the check and the write
+ * are in different coroutines.
+ *
+ * [ReattachGuardsTest] covers the case where the pick has already *landed*, which the plain guard
+ * does catch. This covers the one where it is still in flight, which it does not.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class ReattachmentOwnershipTest {
+
+    private lateinit var app: Application
+    private lateinit var publisher: RecordingPublisher
+    private lateinit var workManager: WorkManager
+    private lateinit var staged: File
+
+    @Before
+    fun setUp() {
+        app = RuntimeEnvironment.getApplication()
+        publisher = RecordingPublisher(app)
+        ConversionDependencies.publisher = { publisher }
+        ConversionDependencies.probe = { _, _ -> InputProbe() }
+
+        staged = publisher.createStagingFile("holiday_converted.mp4").apply { writeBytes(ByteArray(4096)) }
+        installTestWorkManager(app, workDataOf(ConversionWorker.KEY_OUTPUT_PATH to staged.absolutePath))
+        workManager = WorkManager.getInstance(app)
+        // The situation reattachment exists for: a conversion that finished in a process that is
+        // gone, with its output still in the cache and nothing in the UI holding its id.
+        workManager.enqueue(
+            ConversionWorker.request(
+                inputUri = Uri.parse("content://test/holiday.mp4"),
+                displayName = "holiday.mp4",
+                sizeBytes = 4_096L,
+            ),
+        ).result.get()
+    }
+
+    @After
+    fun tearDown() {
+        ConversionDependencies.reset()
+    }
+
+    /**
+     * The race, made into a state the test can sit in rather than one it has to catch.
+     *
+     * The pick is parked on a dispatcher this test owns, so it stays in flight — issued, not yet
+     * written — for as long as the assertions need it to be. Everything else is production: a
+     * real `WorkManager` holding a real finished job, the real `reattach`, the real `observe`.
+     *
+     * Determinism comes from where Robolectric leaves the main looper. `reattach`'s tag query hops
+     * to a real [kotlinx.coroutines.Dispatchers.IO] thread, so its continuation can only come back
+     * as a message posted to the main looper — and that looper is paused, so it cannot run until
+     * something pumps it. `onInputPicked` is an ordinary synchronous call from this thread. The
+     * pick is therefore *always* issued before the guard runs; none of it is left to timing, which
+     * is the whole point of writing it here rather than relying on the rare device sighting.
+     */
+    @Test
+    fun `a conversion found while the user was picking never reaches the screen`() {
+        val picked = Uri.fromFile(File(app.cacheDir, "beach.mp4").apply { writeBytes(ByteArray(2048)) })
+        val parkedPick = ParkedPickDispatcher()
+        val viewModel = ConversionViewModel(app, pickDispatcher = parkedPick)
+        viewModel.onInputPicked(picked)
+
+        assertEquals(
+            "the pick must still be in flight, or this proves something about a different situation",
+            1,
+            parkedPick.parkedCount,
+        )
+
+        // The control, and the reason this test does not rest on a settle window being long
+        // enough. A second ViewModel with nothing to supersede it reattaches to the same job
+        // through the same code; when it has arrived, the whole query-guard-observe-write path has
+        // demonstrably run to completion. `viewModel` started its own reattachment first, so it
+        // has had at least as long. Waiting on this rather than on a sleep is what makes the
+        // assertion below "it did not happen" rather than "it had not happened yet".
+        reattachmentHasRunToCompletion()
+
+        val current = viewModel.state.value
+        assertTrue("reattachment took the screen from the user: $current", current is ConversionState.Idle)
+
+        // And the pick, when it lands, is what stays there.
+        parkedPick.runAll()
+        val ready = awaitState(viewModel.state, "Ready") { it is ConversionState.Ready }
+        assertEquals(picked, (ready as ConversionState.Ready).input.uri)
+        reattachmentHasRunToCompletion()
+        assertEquals("the user's pick must survive a late reattachment", ready, viewModel.state.value)
+    }
+
+    /**
+     * The other half of the contract: a reattachment nobody has superseded still takes the screen.
+     *
+     * Without this, dropping every reattachment on the floor would pass the test above. Same job,
+     * same WorkManager, same production path — only the pick is missing.
+     */
+    @Test
+    fun `a conversion nobody has superseded still reaches the screen`() {
+        val converted = awaitState(ConversionViewModel(app).state, "Converted") {
+            it is ConversionState.Converted
+        }
+
+        assertEquals(staged.absolutePath, (converted as ConversionState.Converted).staged.absolutePath)
+        assertEquals("holiday.mp4", converted.input.displayName)
+    }
+
+    /**
+     * Drives a throwaway ViewModel through a whole reattachment, and returns once it has landed.
+     *
+     * [awaitState] pumps the main looper, which is what runs every reattachment continuation
+     * waiting on it — this one's, and the one belonging to the ViewModel under test, which was
+     * posted earlier and therefore runs first.
+     */
+    private fun reattachmentHasRunToCompletion() {
+        awaitState(ConversionViewModel(app).state, "Converted") { it is ConversionState.Converted }
+    }
+}
@@ -82,6 +82,28 @@ class SucceedingWorkerFactory(private val outputData: Data) : WorkerFactory() {
    }
 }

+/**
+ * Stands in for a worker that died, reporting [outputData] on the way out.
+ *
+ * The counterpart to [SucceedingWorkerFactory], and needed for the same reason: the real workers
+ * cannot run on the JVM, so the only way to ask what a ViewModel does with a `FAILED` `WorkInfo` is
+ * to produce one. A `Result.failure` carrying `KEY_ERROR` is exactly what both real workers report
+ * when their engine gives up, and it is the one path where nothing has ever been staged.
+ *
+ * `runAttemptCount` is irrelevant here: `Result.failure` is terminal, so WorkManager does not retry
+ * it and the state goes straight to `Failed` rather than through `Waiting`.
+ */
+class FailingWorkerFactory(private val outputData: Data) : WorkerFactory() {
+
+    override fun createWorker(
+        appContext: Context,
+        workerClassName: String,
+        workerParameters: WorkerParameters,
+    ): ListenableWorker = object : Worker(appContext, workerParameters) {
+        override fun doWork(): Result = Result.failure(outputData)
+    }
+}
+
 /**
 * Installs a synchronous test WorkManager whose workers succeed with [outputData].
 *
@@ -89,6 +111,16 @@ class SucceedingWorkerFactory(private val outputData: Data) : WorkerFactory() {
 */
 fun installTestWorkManager(context: Context, outputData: Data): SucceedingWorkerFactory {
    val factory = SucceedingWorkerFactory(outputData)
+    installWorkManager(context, factory)
+    return factory
+}
+
+/** Installs a synchronous test WorkManager whose workers fail, reporting [outputData]. */
+fun installFailingTestWorkManager(context: Context, outputData: Data) {
+    installWorkManager(context, FailingWorkerFactory(outputData))
+}
+
+private fun installWorkManager(context: Context, factory: WorkerFactory) {
    WorkManagerTestInitHelper.initializeTestWorkManager(
        context,
        Configuration.Builder()
@@ -98,7 +130,6 @@ fun installTestWorkManager(context: Context, outputData: Data): SucceedingWorker
            .setWorkerFactory(factory)
            .build(),
    )
-    return factory
 }

 /**
@@ -0,0 +1,87 @@
+package org.libremediaconverter.join
+
+import android.app.Application
+import android.net.Uri
+import androidx.media3.common.util.UnstableApi
+import androidx.work.workDataOf
+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.ParkedPickDispatcher
+import org.libremediaconverter.convert.RecordingPublisher
+import org.libremediaconverter.convert.installTestWorkManager
+import org.libremediaconverter.work.ConcatWorker
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+
+/**
+ * `PickOwnershipTest`'s case on the join side.
+ *
+ * `onInputsPicked` makes one write and it lands after a hop off the main thread, so it belongs to
+ * whichever pick was in flight rather than to whichever set of files the user last chose. Two
+ * selections in quick succession — likelier here than on the convert side, since a join picks
+ * several files at a time and the metadata query is per file — put the loser's files on screen if
+ * its query came back second.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class JoinPickOwnershipTest {
+
+    private lateinit var app: Application
+    private lateinit var parkedPick: ParkedPickDispatcher
+    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"))
+
+        parkedPick = ParkedPickDispatcher()
+        viewModel = JoinViewModel(app, pickDispatcher = parkedPick)
+    }
+
+    @After
+    fun tearDown() {
+        ConversionDependencies.reset()
+    }
+
+    /**
+     * Two selections, with the first one's metadata query the slow one.
+     *
+     * The order is chosen rather than raced: both queries are parked, and this runs the second
+     * before the first.
+     */
+    @Test
+    fun `the slower of two selections does not land on top of the faster one`() {
+        viewModel.onInputsPicked(FIRST)
+        viewModel.onInputsPicked(SECOND)
+
+        val queries = parkedPick.takeParked()
+        assertEquals("both selections should be in flight", 2, queries.size)
+        // The second selection's query comes back first; the first one's is the straggler.
+        queries[1].run()
+        queries[0].run()
+
+        val current = viewModel.state.value
+        assertEquals(
+            "a selection the user has already replaced took the screen: $current",
+            SECOND,
+            (current as JoinState.Ready).inputs.map { it.uri },
+        )
+    }
+
+    private companion object {
+        val FIRST = listOf(
+            Uri.parse("content://test/first-a.mp4"),
+            Uri.parse("content://test/first-b.mp4"),
+        )
+        val SECOND = listOf(
+            Uri.parse("content://test/second-a.mp4"),
+            Uri.parse("content://test/second-b.mp4"),
+        )
+    }
+}
@@ -0,0 +1,166 @@
+package org.libremediaconverter.join
+
+import android.app.Application
+import android.net.Uri
+import androidx.media3.common.util.UnstableApi
+import androidx.work.WorkManager
+import androidx.work.workDataOf
+import org.junit.After
+import org.junit.Assert.assertEquals
+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.ParkedPickDispatcher
+import org.libremediaconverter.convert.RecordingPublisher
+import org.libremediaconverter.convert.awaitState
+import org.libremediaconverter.convert.installTestWorkManager
+import org.libremediaconverter.model.ConcatStrategy
+import org.libremediaconverter.work.ConcatWorker
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import java.io.File
+
+/**
+ * Issue #49 on the join side, where nothing was watching for it.
+ *
+ * `reattach()` checks that the screen is still free, and then hands the answer to `observe()`,
+ * which writes from a *different* coroutine that has to suspend on `collect` before it can write
+ * anything at all. So the check happens at one moment and the write lands at another, with a
+ * whole pick able to fit in between:
+ *
+ * 1. `init` starts the tag query and suspends in it.
+ * 2. The user picks files; `onInputsPicked` suspends in its metadata query.
+ * 3. The query comes back. The screen is still `Idle` — step 2 has not written yet — so the
+ *    guard passes and an observation of the old job is launched.
+ * 4. The pick lands. `Ready(picked)`. The user owns the screen.
+ * 5. The observation's first `WorkInfo` arrives and writes `Joined(yesterday's file)` over it.
+ *
+ * The comment above that guard used to say "no suspension point between this check and the
+ * assignment below, so nothing can interleave". There is no assignment below, and the two lines
+ * are in different coroutines.
+ *
+ * The convert side has been failing this on CI for two days — four occurrences across three API
+ * levels, each read as flaky infrastructure. `JoinViewModel` has the identical shape and no test
+ * at all, which is why this one was written before the fix rather than after it.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class JoinReattachmentOwnershipTest {
+
+    private lateinit var app: Application
+    private lateinit var publisher: RecordingPublisher
+    private lateinit var workManager: WorkManager
+    private lateinit var staged: File
+
+    @Before
+    fun setUp() {
+        app = RuntimeEnvironment.getApplication()
+        publisher = RecordingPublisher(app)
+        ConversionDependencies.publisher = { publisher }
+
+        staged = publisher.createStagingFile("joined-yesterday.mp4").apply { writeBytes(ByteArray(4096)) }
+        installTestWorkManager(
+            app,
+            workDataOf(
+                ConcatWorker.KEY_OUTPUT_PATH to staged.absolutePath,
+                ConcatWorker.KEY_STRATEGY to ConcatStrategy.STREAM_COPY.name,
+            ),
+        )
+        workManager = WorkManager.getInstance(app)
+        // The situation reattachment exists for: a join that finished in a process that is gone,
+        // with its output still in the cache and nothing in the UI holding its id.
+        workManager.enqueue(
+            ConcatWorker.request(
+                inputs = listOf(
+                    Uri.parse("content://test/yesterday-a.mp4"),
+                    Uri.parse("content://test/yesterday-b.mp4"),
+                ),
+                totalBytes = 8_192L,
+            ),
+        ).result.get()
+    }
+
+    @After
+    fun tearDown() {
+        ConversionDependencies.reset()
+    }
+
+    /**
+     * The race, made into a state the test can sit in rather than one it has to catch.
+     *
+     * The pick is parked on a dispatcher this test owns, so it is in flight — issued, not yet
+     * written — for as long as the assertions need it to be. Everything else is real: a real
+     * `WorkManager` holding a real finished job, the production `reattach`, the production
+     * `observe`.
+     *
+     * Determinism comes from where Robolectric leaves the main looper. `reattach`'s tag query
+     * hops to a real [kotlinx.coroutines.Dispatchers.IO] thread, so its continuation can only
+     * come back as a message posted to the main looper — and that looper is paused, so it cannot
+     * run until something pumps it. `onInputsPicked` is an ordinary synchronous call from this
+     * thread. The pick is therefore always issued before the guard runs, with nothing left to
+     * timing.
+     */
+    @Test
+    fun `a join found while the user was picking never reaches the screen`() {
+        val parkedPick = ParkedPickDispatcher()
+        val viewModel = JoinViewModel(app, pickDispatcher = parkedPick)
+        viewModel.onInputsPicked(PICKED)
+
+        assertEquals(
+            "the pick must still be in flight, or this proves something about a different situation",
+            1,
+            parkedPick.parkedCount,
+        )
+
+        // The control, and the reason this test does not rest on a settle window being long
+        // enough. A second ViewModel with nothing to supersede it reattaches to the same job
+        // through the same code; when it has arrived, the whole query-guard-observe-write path
+        // has demonstrably run to completion. `viewModel` started its own reattachment first, so
+        // it has had at least as long. Waiting on this rather than on a sleep is what makes the
+        // assertion below "it did not happen" instead of "it had not happened yet".
+        reattachmentHasRunToCompletion()
+
+        val current = viewModel.state.value
+        assertTrue("reattachment took the screen from the user: $current", current is JoinState.Idle)
+
+        // And the pick, when it lands, is what stays there.
+        parkedPick.runAll()
+        val ready = awaitState(viewModel.state, "Ready") { it is JoinState.Ready }
+        assertEquals(PICKED, (ready as JoinState.Ready).inputs.map { it.uri })
+        reattachmentHasRunToCompletion()
+        assertEquals("the user's pick must survive a late reattachment", ready, viewModel.state.value)
+    }
+
+    /**
+     * The other half of the contract: a reattachment nobody has superseded still takes the screen.
+     *
+     * Without this, dropping every reattachment on the floor would pass the test above. It is the
+     * same job, the same WorkManager and the same production path — only the pick is missing.
+     */
+    @Test
+    fun `a join nobody has superseded still reaches the screen`() {
+        val joined = awaitState(JoinViewModel(app).state, "Joined") { it is JoinState.Joined }
+
+        assertEquals(staged.absolutePath, (joined as JoinState.Joined).staged.absolutePath)
+    }
+
+    /**
+     * Drives a throwaway ViewModel through a whole reattachment, and returns once it has landed.
+     *
+     * [awaitState] pumps the main looper, which is what runs every reattachment continuation
+     * waiting on it — this one's and the one belonging to the ViewModel under test, which was
+     * posted earlier and therefore runs first.
+     */
+    private fun reattachmentHasRunToCompletion() {
+        awaitState(JoinViewModel(app).state, "Joined") { it is JoinState.Joined }
+    }
+
+    private companion object {
+        val PICKED = listOf(
+            Uri.parse("content://test/clip-one.mp4"),
+            Uri.parse("content://test/clip-two.mp4"),
+        )
+    }
+}
@@ -18,6 +18,7 @@ import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.libremediaconverter.convert.InputFile
+import org.libremediaconverter.convert.PendingSave
 import org.libremediaconverter.model.ConcatStrategy
 import org.libremediaconverter.ui.TestTags
 import org.robolectric.RobolectricTestRunner
@@ -219,6 +220,51 @@ class JoinStateAffordancesTest {
        assertEquals(listOf("reset"), events)
    }

+    /**
+     * The negative that bounds #30 on this screen. A join that died staged nothing, so its `Failed`
+     * carries no [PendingSave] and there is nothing a save dialog could be handed. A retry button
+     * rendered unconditionally here could only fail, and this is what notices.
+     */
+    @Test
+    fun `a failed join offers no way to save`() {
+        setContent(JoinState.Failed(message = "The second file has no audio track, so joining stopped."))
+
+        composeRule.onNodeWithTag(TestTags.RETRY_SAVE).assertDoesNotExist()
+        composeRule.onNodeWithTag(TestTags.SAVE_FILE).assertDoesNotExist()
+    }
+
+    /**
+     * #30 on this screen: `save()` keeps the staged file when the copy out throws, and until this
+     * branch grew a second button the only control it rendered was "Start over" -- `reset()`, which
+     * deletes exactly that file. Both, not one: the restart still has to be reachable.
+     */
+    @Test
+    fun `a failed join save offers the file again as well as a restart`() {
+        setContent(failedSave())
+
+        composeRule.onNodeWithTag(TestTags.RETRY_SAVE).assertExists()
+        composeRule.onNodeWithTag(TestTags.START_OVER).assertExists()
+    }
+
+    /** The name comes from the job, so a retry wired to a literal would hand back the wrong one. */
+    @Test
+    fun `tapping try saving again hands back the name the join chose`() {
+        setContent(failedSave())
+
+        composeRule.onNodeWithTag(TestTags.RETRY_SAVE).performScrollTo().performClick()
+
+        assertEquals(listOf("save:joined.mp4"), events)
+    }
+
+    @Test
+    fun `tapping start over after a failed join save resets and does not save`() {
+        setContent(failedSave())
+
+        composeRule.onNodeWithTag(TestTags.START_OVER).performScrollTo().performClick()
+
+        assertEquals(listOf("reset"), events)
+    }
+
    /** Anything `FileRow` tagged, whichever file it is showing. The prefix comes from the table. */
    private val isFileRow = SemanticsMatcher("is a join file row") { node ->
        node.config.getOrNull(SemanticsProperties.TestTag)?.startsWith(TestTags.Join.fileRow("")) == true
@@ -230,6 +276,19 @@ class JoinStateAffordancesTest {
        sizeBytes = 4_000_000L,
    )

+    /**
+     * A `Failed` an earlier save left carrying its file, which is the only way `retry` is non-null.
+     * `staged` names a missing file for the same reason [joined] does -- this arm reads no length.
+     */
+    private fun failedSave() = JoinState.Failed(
+        message = "There was not enough room on the destination.",
+        retry = PendingSave(
+            staged = File("no-such-staged-output.mp4"),
+            suggestedName = "joined.mp4",
+            mimeType = "video/mp4",
+        ),
+    )
+
    /** `staged` names a missing file deliberately -- see the same helper in `JoinScreenContentTest`. */
    private fun joined(strategy: ConcatStrategy) = JoinState.Joined(
        staged = File("no-such-staged-output.mp4"),
@@ -11,6 +11,11 @@ import org.junit.Test
 * `OutputFormat` used to be twelve hand-picked triples, and its KDoc defended that on the grounds
 * that a closed set was what made routing decidable. Opening it up moves that burden here, so this
 * is where decidability now has to be proven.
+ *
+ * That includes what a refusal offers instead. `Validation.Invalid` promises every suggestion is
+ * itself valid and names this class as the proof, so a branch that assembles its own suggestion
+ * list rather than going through `suggestions()` is only checked here if some row happens to reach
+ * it — which is how a dead-end chip survived two widenings of that table.
 */
 class ContainerCapabilitiesTest {

@@ -20,6 +25,44 @@ class ContainerCapabilitiesTest {
        container = Container.MP4,
    )

+    /**
+     * An MP3, and the reason several rules below need a second probe.
+     *
+     * `hasVideo = false` is the load-bearing field. Every rule that reads only the spec answers the
+     * same for this input as for a video file, which is exactly how a spec naming a video codec was
+     * called valid for a file with no video track to put in it.
+     */
+    private val mp3Source = InputProbe(
+        videoCodec = null,
+        audioCodec = "mp3",
+        hasVideo = false,
+        kind = InputKind.AUDIO_ONLY,
+        container = Container.MP3,
+    )
+
+    /**
+     * An audio-only input carrying a codec MP4 has no place for at all.
+     *
+     * Vorbis lives in Ogg and Matroska; MP4 carries AAC, MP3, Opus and FLAC. That gap is what turns
+     * a suggestion which merely drops the video track into a second refusal.
+     */
+    private val vorbisSource = InputProbe(
+        videoCodec = null,
+        audioCodec = "vorbis",
+        hasVideo = false,
+        kind = InputKind.AUDIO_ONLY,
+        container = Container.OGG,
+    )
+
+    /** The same shape, for the other codec MP4 refuses. One case is a coincidence; two is the rule. */
+    private val pcmSource = InputProbe(
+        videoCodec = null,
+        audioCodec = "pcm_s16le",
+        hasVideo = false,
+        kind = InputKind.AUDIO_ONLY,
+        container = Container.WAV,
+    )
+
    // --- copy and encode are different questions ----------------------------

    /**
@@ -82,20 +125,56 @@ class ContainerCapabilitiesTest {
        }
    }

-    /** A suggestion that is itself invalid is worse than no suggestion. */
+    /**
+     * A suggestion that is itself invalid is worse than no suggestion.
+     *
+     * Only a branch that assembles its own suggestion list can break that promise: [suggestions]
+     * ends by filtering on `validate(...).isValid`, so everything routed through it is valid by
+     * construction. Those branches are what this table has to cover — the image output, and copy
+     * the video from a file that has none, which built its list by hand and came back refused for
+     * a Vorbis or PCM source into MP4 and an MP3 into WebM. The Advanced picker showed a one-tap
+     * fix that led straight to a second error, through two widenings of this table that never
+     * reached the branch.
+     */
    @Test
    fun `every suggestion is itself valid`() {
-        val broken = OutputSpec(Container.WEBM, VideoCodec.H264, AudioCodec.AAC)
-        val result = ContainerCapabilities.validate(broken, h264Source)
+        val cases = listOf(
+            OutputSpec(Container.WEBM, VideoCodec.H264, AudioCodec.AAC) to h264Source,
+            // The audio-only input. Every rejection it can reach used to hand back `None + None`
+            // — a spec validation refuses in the next breath — because these branches built their
+            // suggestion by hand instead of going through the repair-and-filter path.
+            OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE) to mp3Source,
+            OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.NONE) to mp3Source,
+            OutputSpec(Container.MP4, VideoCodec.NONE, AudioCodec.NONE) to mp3Source,
+            OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.AAC) to mp3Source,
+            // Copy-the-video-from-a-file-with-no-video, the last branch that built its offer by
+            // hand. It escaped the five rows above because `spec.copy(videoCodec = NONE)` is valid
+            // exactly when the audio axis happens to be fine — true for the AAC and MP3 sources
+            // used there, false for any audio the target container cannot carry.
+            OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.COPY) to vorbisSource,
+            OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.COPY) to pcmSource,
+            OutputSpec(Container.WEBM, VideoCodec.COPY, AudioCodec.COPY) to mp3Source,
+            // The same branch with audio the container *can* hold, which is the half that already
+            // worked and must keep working: the repair here is a copy, so the offer is the very
+            // spec the caller handed to `suggestions`. It survives only because the exclusion is
+            // against what the user asked for rather than against the repair.
+            OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.COPY) to mp3Source,
+            // The one branch that still builds its list by hand, so that it is asserted rather
+            // than merely reasoned about: an image container takes `None + None` and nothing else,
+            // which makes its single offer valid by construction.
+            OutputSpec(Container.GIF, VideoCodec.H264, AudioCodec.AAC) to h264Source,
+        )

-        val invalid = result as? Validation.Invalid
-            ?: throw AssertionError("expected H.264 in WebM to be rejected")
-        assertTrue("no alternatives offered", invalid.suggestions.isNotEmpty())
-        invalid.suggestions.forEach { suggestion ->
-            assertTrue(
-                "suggested $suggestion is itself invalid",
-                ContainerCapabilities.validate(suggestion, h264Source).isValid,
-            )
+        cases.forEach { (spec, probe) ->
+            val invalid = ContainerCapabilities.validate(spec, probe) as? Validation.Invalid
+                ?: throw AssertionError("expected $spec to be rejected")
+            assertTrue("no alternatives offered for $spec on $probe", invalid.suggestions.isNotEmpty())
+            invalid.suggestions.forEach { suggestion ->
+                assertTrue(
+                    "suggested $suggestion for $spec on $probe is itself invalid",
+                    ContainerCapabilities.validate(suggestion, probe).isValid,
+                )
+            }
        }
    }

@@ -127,6 +206,117 @@ class ContainerCapabilitiesTest {
        assertTrue((result as Validation.Invalid).suggestions.isNotEmpty())
    }

+    /**
+     * The same rule, seen only against the probe.
+     *
+     * A video codec named for a file with no video track is dropped, not encoded — so
+     * MP4/H.265/None on an MP3 empties the output exactly as None/None does. Reading the spec
+     * alone answered "valid" because the spec names a video codec, and the job went to Media3,
+     * where `EditedMediaItem.Builder` refuses a composition with both tracks removed by throwing
+     * on Transformer's own HandlerThread.
+     */
+    @Test
+    fun `a video codec named for a file with no video track and no audio is refused`() {
+        ContainerCapabilities.encodableVideo(Container.MP4).forEach { codec ->
+            val spec = OutputSpec(Container.MP4, codec, AudioCodec.NONE)
+            val result = ContainerCapabilities.validate(spec, mp3Source)
+
+            assertFalse(
+                "MP4/${codec.label}/None on an audio-only input plans to (Drop, Drop) and would " +
+                    "produce an empty file; it must be refused. Got $result",
+                result.isValid,
+            )
+        }
+    }
+
+    /**
+     * The refusal is only worth having if it leads somewhere.
+     *
+     * The COPY form of this was already refused, but its one hand-built suggestion was
+     * `None + None` — which validation refuses in the next breath, so the Advanced picker offered
+     * a one-tap fix that fixed nothing. Every face of the rule now goes through the shared
+     * suggestion path, so the offer keeps the one track the input actually has.
+     */
+    @Test
+    fun `refusing an empty output still offers a way to keep the audio`() {
+        listOf(VideoCodec.H265, VideoCodec.H264, VideoCodec.COPY, VideoCodec.NONE).forEach { codec ->
+            val spec = OutputSpec(Container.MP4, codec, AudioCodec.NONE)
+            val invalid = ContainerCapabilities.validate(spec, mp3Source) as? Validation.Invalid
+                ?: throw AssertionError("expected MP4/${codec.label}/None to be rejected")
+
+            assertTrue(
+                "a refusal with no way out is a dead end in the Advanced picker",
+                invalid.suggestions.isNotEmpty(),
+            )
+            assertTrue(
+                "every suggestion must keep a track, got ${invalid.suggestions}",
+                invalid.suggestions.all { it.audioCodec != AudioCodec.NONE },
+            )
+        }
+    }
+
+    /**
+     * A repair must not name a track the input does not have.
+     *
+     * `repairVideo` used to fall through to "the first codec this container can encode" whenever
+     * nothing else fitted, and for an MP3 that produced the non-sequitur `MP4 · H.264 · Copy`.
+     * It validated, so nothing caught it — but [CopyPlanner] drops that video track anyway, which
+     * makes the codec in the offer a fiction.
+     */
+    @Test
+    fun `a repair for a file with no video track never names a video codec`() {
+        listOf(
+            OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE),
+            OutputSpec(Container.MP4, VideoCodec.NONE, AudioCodec.NONE),
+            OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.NONE),
+        ).forEach { spec ->
+            val invalid = ContainerCapabilities.validate(spec, mp3Source) as Validation.Invalid
+            invalid.suggestions.forEach {
+                assertEquals(
+                    "offering ${it.videoCodec.label} for a file with no video track is a fiction; " +
+                        "CopyPlanner drops it. Suggested $it for $spec",
+                    VideoCodec.NONE,
+                    it.videoCodec,
+                )
+            }
+        }
+    }
+
+    /**
+     * The rule stated as the property it is, over the whole matrix.
+     *
+     * A plan of (Drop, Drop) is precisely the composition `EditedMediaItem.Builder` refuses to
+     * build, so no non-image spec that reaches it may be called valid. Sweeping every container ×
+     * codec × codec against both probes is what stops the next container or codec from
+     * reintroducing the gap on an axis nobody thought to write a case for.
+     *
+     * Image outputs are exempt and deliberately so: GIF and PNG frames carry no codecs at all, and
+     * `None + None` is the only spec they accept — but they never reach Media3, because the router
+     * sends every image output to FFmpeg.
+     */
+    @Test
+    fun `no valid non-image spec plans to remove both tracks`() {
+        val specs = Container.entries
+            .filterNot { it == Container.GIF || it == Container.IMAGE_SEQUENCE }
+            .flatMap { container -> VideoCodec.entries.map { container to it } }
+            .flatMap { (container, video) -> AudioCodec.entries.map { OutputSpec(container, video, it) } }
+        val cases = specs.flatMap { spec -> listOf(h264Source, mp3Source).map { spec to it } }
+
+        val empties = cases.filter { (spec, probe) ->
+            val plan = CopyPlanner.plan(spec, probe)
+            plan.video == VideoPlan.Drop && plan.audio == AudioPlan.Drop
+        }
+
+        assertTrue("the sweep found nothing to check — the filter has gone wrong", empties.isNotEmpty())
+        empties.forEach { (spec, probe) ->
+            assertFalse(
+                "$spec on $probe plans to (Drop, Drop) — an empty file, and the composition " +
+                    "Media3 cannot build — so it must not validate",
+                ContainerCapabilities.validate(spec, probe).isValid,
+            )
+        }
+    }
+
    @Test
    fun `copying is offered as the fix when the codec is right but unencodable`() {
        val av1Source = InputProbe(videoCodec = "av1", audioCodec = "aac", container = Container.MKV)
@@ -350,6 +350,34 @@ class ConversionRouterTest {
        }
    }

+    /**
+     * Why `Media3Engine` still needs a guard of its own.
+     *
+     * `ContainerCapabilities.validate` now refuses "a video codec with the audio off" for an input
+     * with no video track, so neither the picker nor `ConversionWorker` will start one. Routing is
+     * a separate question and still answers MEDIA3 — nothing about a dropped track makes the job
+     * un-hardware-able — so a request that skips validation, from a direct
+     * `ConversionWorker.request(...)` or a job queued before the settings changed, arrives at the
+     * engine with a plan Media3 cannot build. That has to fail the job, not the process.
+     */
+    @Test
+    fun `a plan that drops both tracks still routes to media3`() {
+        val audioOnly = InputProbe(
+            videoCodec = null,
+            audioCodec = "mp3",
+            hasVideo = false,
+            container = Container.MP3,
+            kind = InputKind.AUDIO_ONLY,
+        )
+        val spec = OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE)
+
+        val plan = CopyPlanner.plan(spec, audioOnly)
+        assertEquals(VideoPlan.Drop, plan.video)
+        assertEquals(AudioPlan.Drop, plan.audio)
+
+        assertEquals(Engine.MEDIA3, route(spec, probe = audioOnly).engine)
+    }
+
    @Test
    fun `audio-only formats are flagged as such`() {
        assertEquals(true, OutputFormat.MP3.isAudioOnly)
@@ -146,6 +146,33 @@ class CopyPlannerTest {
        assertTrue("copying the only track is still a remux", plan.isPureRemux)
    }

+    /**
+     * The one plan `Media3Engine` cannot be handed.
+     *
+     * `EditedMediaItem.Builder` refuses a composition with both tracks removed —
+     * checkState("Audio and video cannot both be removed") — and this is how an ordinary-looking
+     * spec reaches it: a video codec named for a file that has no video, with the audio switched
+     * off. Neither half is unusual on its own, which is why validation could read the spec, see a
+     * video codec, and call it fine.
+     */
+    @Test
+    fun `an audio-only source with the audio dropped removes both tracks`() {
+        val audioOnly = InputProbe(
+            videoCodec = null,
+            audioCodec = "mp3",
+            hasVideo = false,
+            container = Container.MP3,
+            kind = InputKind.AUDIO_ONLY,
+        )
+        val plan = CopyPlanner.plan(
+            OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE),
+            audioOnly,
+        )
+        assertEquals(VideoPlan.Drop, plan.video)
+        assertEquals(AudioPlan.Drop, plan.audio)
+        assertTrue("an empty plan is not a remux", !plan.isPureRemux)
+    }
+
    @Test
    fun `copying one track and encoding the other is not a pure remux`() {
        val plan = CopyPlanner.plan(
@@ -0,0 +1,131 @@
+package org.libremediaconverter.ui.theme
+
+import androidx.compose.material3.ColorScheme
+import androidx.compose.material3.MaterialTheme
+import androidx.compose.ui.graphics.luminance
+import androidx.compose.ui.test.junit4.v2.createComposeRule
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotEquals
+import org.junit.Assert.assertTrue
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+
+/**
+ * The theme has to resolve the scheme its arguments name, and `ThemeKt` had no test at all --
+ * 23 lines, none of them covered, which is how #68 was found.
+ *
+ * Only two of the four branches in [LibreMediaConverterTheme]'s `when` are reachable from the
+ * app. `MainActivity` is the single call site and passes no arguments, so `dynamicColor` is
+ * always `true` and the live choice is between the dynamic dark and dynamic light schemes.
+ * Those two are what ships, and asserting on them survives whichever way #68 is decided.
+ *
+ * **The other two branches have no caller.** `dynamicColor = false` is passed below by this
+ * test and by nothing else in `app/src`, so the coverage it produces is not evidence that a
+ * switch exists -- misreading it that way is the whole reason #68 was filed. #68 is the open
+ * decision about whether one ever will exist.
+ *
+ * What the assertions distinguish the branches on was measured under Robolectric `sdk=36`
+ * rather than assumed. The dynamic palette resolves to the platform's own default there --
+ * dark background `#121318` against light `#FAF8FF`, dark primary `#B0C6FF` -- and that is a
+ * different hue from the brand palette's [Purple80] / [Purple40]. A dynamic scheme reads the
+ * device, so those exact values belong to the Robolectric stub and to no particular phone,
+ * which is why the live-branch tests compare the two resolved schemes against each other
+ * instead of hard-coding either one.
+ */
+@RunWith(RobolectricTestRunner::class)
+class ThemeColorSchemeTest {
+
+    // The **v2** rule (`androidx.compose.ui.test.junit4.v2`), as everywhere else in this
+    // source set.
+    @get:Rule
+    val composeRule = createComposeRule()
+
+    /**
+     * Every scheme the `when` can produce, read out of [MaterialTheme] inside the content
+     * lambda -- the only place that shows what the theme actually chose, rather than what the
+     * caller hoped for.
+     *
+     * All four are resolved in one composition because `setContent` may be called once per
+     * test, and a comparison needs at least two of them.
+     */
+    private fun resolveAll(): Schemes {
+        lateinit var dynamicDark: ColorScheme
+        lateinit var dynamicLight: ColorScheme
+        lateinit var brandDark: ColorScheme
+        lateinit var brandLight: ColorScheme
+        composeRule.setContent {
+            LibreMediaConverterTheme(darkTheme = true) { dynamicDark = MaterialTheme.colorScheme }
+            LibreMediaConverterTheme(darkTheme = false) { dynamicLight = MaterialTheme.colorScheme }
+            LibreMediaConverterTheme(darkTheme = true, dynamicColor = false) {
+                brandDark = MaterialTheme.colorScheme
+            }
+            LibreMediaConverterTheme(darkTheme = false, dynamicColor = false) {
+                brandLight = MaterialTheme.colorScheme
+            }
+        }
+        composeRule.waitForIdle()
+        return Schemes(dynamicDark, dynamicLight, brandDark, brandLight)
+    }
+
+    private class Schemes(
+        val dynamicDark: ColorScheme,
+        val dynamicLight: ColorScheme,
+        val brandDark: ColorScheme,
+        val brandLight: ColorScheme,
+    )
+
+    /**
+     * The live branches, and the one assertion that catches them being swapped: both dynamic
+     * schemes come from the same device palette, so they are similar enough that identity or a
+     * bare inequality would prove nothing. Background luminance is not similar -- it is the
+     * thing dark mode is for.
+     */
+    @Test
+    fun `dark mode resolves a darker scheme than light mode`() {
+        val schemes = resolveAll()
+
+        val dark = schemes.dynamicDark.background.luminance()
+        val light = schemes.dynamicLight.background.luminance()
+        assertTrue(
+            "darkTheme = true should resolve the dynamic dark scheme, whose background " +
+                "luminance ($dark) is below the light scheme's ($light)",
+            dark < light,
+        )
+    }
+
+    /**
+     * Which of the two dark branches ran, and which of the two light ones: the scheme a live
+     * call resolves is the dynamic one, not the brand palette sitting next to it.
+     */
+    @Test
+    fun `the live branches take the dynamic palette rather than the brand one`() {
+        val schemes = resolveAll()
+
+        assertNotEquals(
+            "the default dynamicColor = true should not resolve the brand dark palette",
+            Purple80,
+            schemes.dynamicDark.primary,
+        )
+        assertNotEquals(
+            "the default dynamicColor = true should not resolve the brand light palette",
+            Purple40,
+            schemes.dynamicLight.primary,
+        )
+    }
+
+    /**
+     * The two dead branches. Nothing in `app/src` passes `dynamicColor = false`; this test
+     * does it directly, because the parameter is public, and that is the only way either
+     * branch runs. Covered so a later decision on #68 starts from a tested `when` -- not
+     * because the brand palette is reachable in the app.
+     */
+    @Test
+    fun `the brand palette branches run only when dynamicColor is passed explicitly`() {
+        val schemes = resolveAll()
+
+        assertEquals(Purple80, schemes.brandDark.primary)
+        assertEquals(Purple40, schemes.brandLight.primary)
+    }
+}
@@ -0,0 +1,327 @@
+# Coverage-read findings
+
+**Status:** five findings, none fixed, none urgent. F5 was added on 2026-08-27, found while decomposing #132 into children — it had been listed there as a test gap, and is not one. Every entry here is a *code* observation —
+something a test would document rather than repair. The test gaps found in the same read are
+tickets #132 and #133, not entries here; see [Not covered here](#not-covered-here).
+**Scope:** what a JaCoCo read on 2026-08-26 turned up that writing a test would not fix. This is
+a survey, not a work order. Acting on any entry is a separate decision and would be its own commit.
+**Last verified:** `main` at `dc8b7c3`, 2026-08-26. Coverage re-measured that day with
+`./gradlew :app:jacocoTestReport`: **84.9% line (1971/2321), 63.8% branch (900/1410)**, against
+**456 JVM tests in 68 classes**. `CLAUDE.md` quotes 454 in 67 from four hours earlier; the
+percentages are unchanged, so no figure there is stale.
+
+## Why this document is separate from `defect-audit.md`
+
+`defect-audit.md` is the record of the 2026-08-22 defect sweep: sixteen entries, each a thing that
+is *wrong at runtime*. Nothing here is wrong at runtime today. These are arms that cannot be
+reached, accessors nobody calls, and one KDoc that contradicts the code beside it — the category
+`defect-audit.md` calls **latent**, plus one that is not a defect at all and is recorded so the
+next coverage read does not re-file it.
+
+They are here rather than in that document because folding them in would inflate a sixteen-entry
+audit whose status metadata has already gone stale once, and because they share a provenance:
+every one fell out of reading a coverage report, and every one is the kind of thing a coverage
+report is *good* at surfacing and a test is bad at fixing. F5 is the clearest case — it was filed
+as a test gap first, and only stopped being one when someone went looking for its callers.
+
+Entry ids are `F1`–`F5` so they cannot be confused with `defect-audit.md`'s `D1`–`D16`.
+
+## How to read the confidence labels
+
+Same vocabulary as `defect-audit.md`, deliberately, so the two read alike:
+
+- **Confirmed by inspection** — the control flow is fully readable and the finding follows from it.
+- **Latent** — not reachable through today's UI, but wrong, and one change away from being live.
+- **No action** — recorded because it looks like a finding and is not.
+
+Nothing below was observed on a device, and nothing below needs to be: every entry is a claim about
+what the code says, checkable by reading it.
+
+---
+
+## F1 — `FFmpegCommandBuilder` emits a Vorbis encoder that `ContainerCapabilities` says does not exist
+
+**Severity: low · Latent · the more interesting reading is a missing feature, not dead code**
+
+```
+app/src/main/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilder.kt:188
+app/src/main/java/org/libremediaconverter/model/ContainerCapabilities.kt:84-91
+```
+
+`FFmpegCommandBuilder.audioArgs` carries a live Vorbis arm:
+
+```kotlin
+AudioCodec.VORBIS -> listOf("-c:a", "libvorbis", "-q:a", "5")
+```
+
+`ContainerCapabilities` states, immediately above the set that governs it, that no such thing
+exists:
+
+> `/** Vorbis is absent for the same reason: nothing here emits a Vorbis encoder. */`
+> `private val ENCODABLE_AUDIO = setOf(AAC, OPUS, MP3, FLAC, PCM)`
+
+One of those two is wrong. The comment is the one that is wrong as written — something here does
+emit a Vorbis encoder, twelve lines of `FFmpegCommandBuilder`.
+
+### Why the arm is unreachable today
+
+Traced, not assumed:
+
+| step | where | effect |
+|---|---|---|
+| `validate` runs before routing | `ConversionWorker.kt:123` | a spec is checked on every job, however it was enqueued |
+| `validateAudio` refuses non-encodable | `ContainerCapabilities.kt:246-251` | `VORBIS !in ENCODABLE_AUDIO` → `Invalid("This app cannot encode Vorbis audio.")` |
+| the only spec→plan encode path | `CopyPlanner.kt:104` | `AudioPlan.Encode(requested)` — but `requested` cannot be Vorbis by the row above |
+| the fallback encode path | `CopyPlanner.kt:112-115` | draws from `encodableAudio(container)`, itself filtered by `ENCODABLE_AUDIO` |
+
+So `AudioPlan.Encode(VORBIS)` is not constructible through the app, and line 188 is dead.
+
+### The reading that matters more
+
+`CARRIES_AUDIO` lists Vorbis for WebM (`ContainerCapabilities.kt:62`) and OGG (`:67`). Because
+`encodableAudio` filters through `ENCODABLE_AUDIO`, the picker offers **Opus and nothing else** for
+WebM, and Opus/FLAC for OGG. FFmpeg on this device can encode Vorbis — the command is written and
+correct — and the app declines to offer it.
+
+So the honest framing is not "delete a dead arm". It is: **is `ENCODABLE_AUDIO`'s omission of
+Vorbis a deliberate product call, or an accident that has been costing WebM/OGG users a format the
+app already supports?** Nothing in the repo records that decision.
+
+### The precedent for whichever way it goes
+
+`Media3Engine.audioMimeTypeFor` has the *same* Vorbis arm, and handles it exactly right
+(`Media3Engine.kt:221-233`): the KDoc names it dead, says why the arm stays anyway ("deleting a
+right answer out of unreachable code buys nothing"), and points at `Media3EngineMimeTypesTest`,
+which asserts which three of six codecs actually arrive — so the set moving fails a test rather
+than surprising someone.
+
+`FFmpegCommandBuilder`'s arm has none of that. Whatever is decided, the fix is to make the two
+files agree and to say so in one place.
+
+### What a fix has to decide
+
+1. Whether Vorbis belongs in `ENCODABLE_AUDIO`. If yes, this is a feature and needs an e2e test
+   that produces a playable Vorbis file; if no, go to 2.
+2. Correct the `ContainerCapabilities.kt:84` comment, which is false as written, and give the
+   `FFmpegCommandBuilder` arm the treatment `Media3Engine.kt:221-233` already models.
+
+---
+
+## F2 — `ConversionRequest.hardwareEncodeAvailable` is written, read by nothing, and its KDoc describes behaviour that was removed
+
+**Severity: low · Confirmed by inspection**
+
+```
+app/src/main/java/org/libremediaconverter/model/OutputFormat.kt:211-219
+app/src/main/java/org/libremediaconverter/work/ConversionWorker.kt:117
+```
+
+The property is set on every request:
+
+```kotlin
+hardwareEncodeAvailable = devices.canEncode(spec.videoCodec),
+```
+
+`grep -rn 'hardwareEncodeAvailable' app/src/main` returns **that line and nothing else**. No
+production code reads it. Its getter is one of three uncovered methods in `OutputFormat.kt`, which
+is what surfaced it.
+
+Its KDoc (`OutputFormat.kt:211-218`) explains at length what it is for:
+
+> Knowing this lets the Fast tier choose a genuinely fast software preset instead of a mislabelled
+> slow one.
+
+`FFmpegCommandBuilder` no longer does that, and its own test says so —
+`FFmpegCommandBuilderTest.kt:132`, `the encoder choice no longer depends on hardware availability`:
+
+> Once FFmpeg stopped selecting MediaCodec encoders, this flag only affects whether the router sends
+> the job to Media3 at all — not what FFmpeg does.
+
+That second clause is also not true. `ConversionRouter` decides hardware encodability by calling
+`device.canEncode(videoEncode)` itself (`ConversionRouter.kt:153`); it never reads
+`request.hardwareEncodeAvailable`. The flag is computed from the same source the router
+independently consults, carried through the request, and dropped.
+
+This is the shape of open issue **#68** — a KDoc promising a switch that does not exist.
+
+**Not harmful.** It costs one `canEncode` call per job and a field on a data class. It is recorded
+because the KDoc actively misleads: a reader changing the Fast-tier preset logic would look here
+first, and this is not where that decision lives.
+
+### What a fix has to decide
+
+Whether to delete the property (and the constructor parameter, and the four
+`FFmpegCommandBuilderTest` call sites that pass it) or to keep it and rewrite the KDoc to say it is
+vestigial. Deleting is cleaner; the test at `:132` is worth keeping either way, since it pins the
+"FFmpeg does not select MediaCodec encoders" rule that the deletion would otherwise erase.
+
+---
+
+## F3 — `ConversionRequest.videoCodec` and `.audioCodec` have no callers anywhere
+
+**Severity: low · Confirmed by inspection**
+
+```
+app/src/main/java/org/libremediaconverter/model/OutputFormat.kt:222-223
+```
+
+```kotlin
+val container: Container get() = spec.container      // used: FFmpegConcatCommand.kt:42, :80
+val videoCodec: VideoCodec get() = spec.videoCodec   // no callers
+val audioCodec: AudioCodec get() = spec.audioCodec   // no callers
+```
+
+Three delegating accessors on `ConversionRequest`; the first is used twice, the other two are used
+nowhere in `main`, `test` or `androidTest`. Everything that wants those values reads
+`request.spec.videoCodec` or takes the `OutputSpec` directly.
+
+**This is not a test gap and must not be filed as one.** A test asserting
+`request.videoCodec == request.spec.videoCodec` is vacuous by construction — it restates the
+implementation and would pass against any delegation, right or wrong. That is precisely the failure
+mode `CLAUDE.md` records from the mutation review (9 of 46 mutations vacuous, five over completely
+unguarded paths).
+
+The two accessors are either convenience worth keeping for symmetry with `container`, or two lines
+to delete. Deleting them costs nothing and removes two uncovered methods that will otherwise be
+re-found by every future coverage read.
+
+---
+
+## F4 — Two guards are reachable only by direct call, and that is correct
+
+**Severity: n/a · No action**
+
+```
+app/src/main/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilder.kt:167-168
+app/src/main/java/org/libremediaconverter/model/ConversionRouter.kt:175-176
+```
+
+```kotlin
+VideoCodec.COPY, VideoCodec.NONE -> error("encodeVideo called for $codec, which is not an encode")
+```
+
+```kotlin
+if (plan.video == VideoPlan.Copy && video == null) return false
+if (plan.audio == AudioPlan.Copy && audio == null) return false
+```
+
+Both sit in private functions (`encodeVideo`, `media3CanMux`), and both are unreachable because a
+caller upstream already excluded the case — which each says in its own comment. `ConversionRouter`'s
+is labelled "the second line of defence"; `CopyPlanner` is the first.
+
+**Recorded so the next coverage read does not treat them as gaps.** A second line of defence that
+can be provoked is not a second line of defence. Making these reachable from a test would mean
+widening the functions to `internal`, which buys a test that asserts an `error()` fires when called
+in a way production cannot call it. This is the same judgement issue **#88** reached about
+`getForegroundInfo` and closed on: naming the exemption rather than covering it.
+
+Neither should change unless the upstream guard does. If `CopyPlanner` ever stops resolving `COPY`
+before the builder sees it, `FFmpegCommandBuilder.kt:167` becomes live and wants a test that day.
+
+---
+
+## F5 — `ConversionNotifications.areEnabled()` is never called
+
+**Severity: low · Confirmed by inspection · found while decomposing the test-gap ticket**
+
+```
+app/src/main/java/org/libremediaconverter/work/ConversionNotifications.kt:60-62
+```
+
+```kotlin
+fun areEnabled(): Boolean = context.getSystemService(NotificationManager::class.java)
+    .areNotificationsEnabled()
+    .also { if (!it) Log.i(TAG, "Notifications disabled; progress will not be visible.") }
+```
+
+`grep -rn 'areEnabled' app/src` returns **that declaration and nothing else**. `ConversionNotifications`
+is constructed in both workers (`ConversionWorker.kt:55`, `ConcatWorker.kt:35`) and only `build()` is
+ever called on it.
+
+**This entry exists because it was very nearly filed as a test gap.** Its three lines are cold on the
+JVM, it has a KDoc explaining real user-visible stakes — a foreground service without
+`POST_NOTIFICATIONS` shows only in the Task Manager, so progress silently vanishes — and Robolectric
+can flip that permission in one line. Everything about it reads like a cheap, worthwhile test.
+
+It is not, because **the behaviour the KDoc describes does not happen**. Nothing consults
+`areEnabled()`, so nothing warns, degrades, or logs when notifications are off. A test would assert
+that a function nobody calls returns what the platform told it — green, vacuous, and actively
+misleading, since it would imply the app handles the disabled-notification case. That is the failure
+mode `CLAUDE.md` records from the mutation review, reached from the opposite direction: not a test
+that fails to bite, but a test with nothing to bite.
+
+### What a fix has to decide
+
+Whether the app should act on this at all. The KDoc argues it should — a conversion whose progress is
+invisible is a real complaint, and `ConversionViewModel` or the worker's foreground start is where a
+check would go. If yes, that is a **feature** with a test; if no, delete the method and the KDoc's
+claim with it. What must not happen is a test that makes the current state look handled.
+
+Related: **#16** is open on an adjacent gap — a user who *can* unblock a foreground-denied retry has
+no way to make it happen now.
+
+---
+
+## Summary
+
+| ID | Finding | Severity | Evidence | Action |
+|---|---|---|---|---|
+| F1 | `FFmpegCommandBuilder` emits a Vorbis encoder `ContainerCapabilities` says does not exist | low | confirmed by inspection; unreachability traced through four call sites | **decide**: feature or dead arm — the comment is false either way |
+| F2 | `hardwareEncodeAvailable` written, never read; KDoc describes removed behaviour | low | confirmed by inspection; `FFmpegCommandBuilderTest:132` corroborates | **decide**: delete or mark vestigial |
+| F3 | `ConversionRequest.videoCodec` / `.audioCodec` have no callers | low | confirmed by inspection | delete, or keep for symmetry — **not** a test gap |
+| F4 | Two private guards reachable only by direct call | n/a | confirmed by inspection | **no action** — named exemption, per #88 |
+| F5 | `ConversionNotifications.areEnabled()` is never called | low | confirmed by inspection; grep returns the declaration only | **decide**: act on it or delete it — **not** a test gap |
+
+Order, if these are acted on: **F1 and F5 first, separately.** They are the two with a possible
+user-visible answer — a format the app can produce and does not offer, and a warning the app
+documents and does not give — and either answer changes what the tidying should look like. F2 and F3
+are tidying and belong in one commit with each other, not with F1 or F5. F4 is finished by being
+written down.
+
+**F1 and F5 share a shape worth naming:** both are places where a comment describes behaviour the
+code does not have, and in both the tempting fix (delete the dead arm, test the dead method) would
+freeze the wrong answer in place. The decision comes first.
+
+## Not covered here
+
+**The test gaps from the same read.** Seven JVM-side gaps (**#132**) and three seam questions
+(**#133**) came out of this coverage read and are tracked there, because they are work rather than
+observations. This document holds only what a test would not fix. #133 also records why
+`AndroidDeviceCodecs.probe()` was considered and left out, so that spike is not run a third time.
+
+**`ConversionForegroundType.current()`**, which looked like the sharpest gap in the read and is not.
+Its API 33 and 34 arms are cold on the JVM, but issue **#88** already established that the class is
+covered by `ConversionWorkerTest.foregroundTypeMatchesTheRunningApiLevel` across the CI matrix, and
+that its 0% is the `testDebugUnitTest`-only measurement boundary.
+
+The premise worth re-checking was whether the 33/34 legs still complete, given #122's wedge.
+**They mostly do, and #122 is not resolved** — this entry said "they do" on first writing, from a
+single green run, and the PR carrying this very document proved that wrong:
+
+| run | API 33 leg | shape |
+|---|---|---|
+| `32933262839` (#127) | success, 7m16s | `expected 60, received 60, failed 0, completed cleanly: yes` |
+| `33033036857` (PR #131, docs-only) | **failure, 23m08s** | `expected 60, received 60, failed unknown, wedged: yes — gradle killed after 1200s` |
+
+Five of the last six completed API 33 legs passed in about seven minutes, so the wedge is
+intermittent rather than systematic. **What it costs is the verdict, not the execution**: `received:
+60` on the wedged run means all sixty tests still reported, so the API 33 regime *was* exercised —
+but `failed:` reads `unknown`, so that leg could not have told anyone if it had broken.
+
+That is why this stays a note and not a ticket, and also why it is not simply deleted: #88's
+reasoning holds, but the leg it rests on cannot be relied on to report a failure. A
+`@Config(sdk = 33)` / `@Config(sdk = 34)` JVM test would pin all three arms deterministically in one
+run for about three lines. Small, and worth doing the next time this file is opened — but it is
+insurance against a flaky leg, not the uncovered behaviour it first looked like.
+
+**The Compose screens' branch coverage.** `ConverterScreenKt` reports 110 of 200 branches missed and
+`JoinScreenKt` 60 of 82, which looks alarming and is not a signal: the Compose compiler synthesises
+`$changed`/`$dirty` recomposition-skip tests that JaCoCo counts as branches. The line figures are
+the real ones — **34 of 383** and **20 of 143** missed — and the screens are among the
+better-covered files in the repo, which is what #52, #57 and #61 were for. **Do not chase the
+branch number here.** If a future read wants a screen metric, use lines.
+
+**Anything requiring a device.** `MediaProbe`'s FFprobe half (`MediaProbe.kt:151, 156-158, 173-188`)
+and `FFmpegEngine` in full report 0% on the JVM and are covered by `androidTest`. JaCoCo measures
+`testDebugUnitTest` only; their zeroes are a boundary, as #84, #85, #86 and #88 each recorded
+before this.