Merge remote-tracking branch 'origin/main' into m-129-tmp

Bound the JVM suite's hangs so a deadlock ends in minutes with a stack

.github/scripts/e2e-report-shape-test.sh

@@ -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

.github/scripts/e2e-report-shape.sh

@@ -208,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
 

.github/scripts/testdata/marker-shapes/MarkerShapes.kt

@@ -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
+    }
+}

.gitignore

@@ -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.

CLAUDE.md

@@ -130,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
@@ -147,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.
@@ -243,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.

app/build.gradle.kts

@@ -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.*")

app/src/main/java/org/libremediaconverter/model/ContainerCapabilities.kt

@@ -178,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)

app/src/test/java/org/libremediaconverter/ci/HangBoundTest.kt

@@ -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
+    }
+}

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

@@ -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 {
 
@@ -35,6 +40,29 @@ class ContainerCapabilitiesTest {
         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 ----------------------------
 
     /**
@@ -97,7 +125,17 @@ 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 cases = listOf(
@@ -109,15 +147,31 @@ class ContainerCapabilitiesTest {
             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,
         )
 
         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", invalid.suggestions.isNotEmpty())
+            assertTrue("no alternatives offered for $spec on $probe", invalid.suggestions.isNotEmpty())
             invalid.suggestions.forEach { suggestion ->
                 assertTrue(
-                    "suggested $suggestion for $spec is itself invalid",
+                    "suggested $suggestion for $spec on $probe is itself invalid",
                     ContainerCapabilities.validate(suggestion, probe).isValid,
                 )
             }