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

Say in the run-shape table when the wedge timeout was what killed the leg

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

@@ -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\`"
@@ -217,6 +245,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 +282,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 |"

.github/scripts/e2e-run.sh

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

CLAUDE.md

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

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

@@ -193,6 +193,25 @@ 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.
      *
@@ -244,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(
@@ -254,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.
@@ -281,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)
         }
     }
 
@@ -297,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))
         }
     }
 
@@ -366,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)
     }
 
     /**
@@ -377,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,
@@ -528,6 +587,10 @@ class ConversionViewModel @JvmOverloads constructor(
      * 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

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

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

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

@@ -20,6 +20,7 @@ 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
@@ -76,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)
@@ -90,6 +100,20 @@ 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.
      *
@@ -116,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(
@@ -125,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
@@ -146,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)
         }
     }
@@ -172,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)
     }
 
     /**
@@ -183,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 ->
@@ -301,6 +362,10 @@ class JoinViewModel @JvmOverloads constructor(
      * 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

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/main/java/org/libremediaconverter/ui/theme/Theme.kt

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

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

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

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

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

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

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

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

@@ -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"),
+        )
+    }
+}

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

@@ -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"),
+        )
+    }
+}

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,
                 )
             }

app/src/test/java/org/libremediaconverter/ui/theme/ThemeColorSchemeTest.kt

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