JMR-dev/LibreMediaConverter
1
0
Fork 0
Code Issues 16 Pull Requests Actions Packages Projects Releases Wiki Activity

convert() can enqueue a second job over a live one, and currentInput() is over-general #202

New Issue
Closed
opened 2026-09-02 12:46:39 +00:00 by JMR-dev · 0 comments
JMR-dev commented 2026-09-02 12:46:39 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Wave 4, filed from a coverage read on main @ 54ca2dd, 2026-09-02. The shared filter note is on #194. This one changes production code, and the decision it rests on has been made — see below.

convert() can enqueue a second job over a live one

app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt:676-682

private fun currentInput(): InputFile? = when (val s = _state.value) {
    is ConversionState.Ready -> s.input
    is ConversionState.Converting -> s.input     // ci == 0
    is ConversionState.Waiting -> s.input        // ci == 0
    is ConversionState.Converted -> s.input      // ci == 0
    else -> null                                 // ci == 0
}

mi=22, ci=13, mb=7, cb=1 — one arm of eight branches is ever taken. Its only caller is convert() at :513.

The Convert button renders only in the Ready branch (ConverterScreen.kt:256 onward), so the three middle arms are not reachable by tapping it. They are reachable through the permission-result callback: ConverterScreen.kt:91 wires convert() to the POST_NOTIFICATIONS result, not to the button.

What they do when reached is the problem. convert() enqueues, then overwrites activeWorkId. Reaching it from Converting starts a second conversion while the first keeps running — with its foreground notification now orphaned, since nothing holds its id any more. That is a latent defect, not a branch waiting for a test, and writing a test first would freeze whichever reading the author happened to assume — the F1/F5 shape.

The decision, made

Guard on state. Narrow to what the join side already does:

private fun currentInput(): InputFile? = (_state.value as? ConversionState.Ready)?.input

JoinViewModel.kt:316 has been (_state.value as? JoinState.Ready)?.inputs ?: return all along. The two screens are the same shape and only one of them is over-general; this makes them agree.

Behaviour the tests assert

  • A permission answer arriving while the state is anything but Ready enqueues nothing and leaves the screen alone.
  • Specifically from Converting: the live job's id is still the one cancel() would reach, and no second WorkRequest exists.

Same shape, same ticket

Three more cold refusal arms of the same kind, all reachable through a launcher result that outlived the state it was started from — a result redelivered after process death arrives at a brand-new ViewModel sitting on Idle:

app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt:513   currentInput() ?: return
app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt:600   pendingSave() ?: return
app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt:316            (as? Ready)?.inputs ?: return
app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt:390            pendingSave() ?: return

Four sites, one idea: an answer that arrives after the screen has moved on does nothing, rather than crashing or acting on a stale assumption. Drive each with reset() (or a fresh ViewModel) followed by convert() / save(uri).

Acceptance: the mutations that must go red

  • Restore the four-arm when and assert from Converting that no second WorkRequest is enqueued.
  • currentInput()!! and pendingSave()!! at the four guard sites — each throws deterministically.

Restore, confirm green.

Note for the reviewer

This narrows a when to an as?. Check that no other caller of currentInput() appears in the diff — there is exactly one today (:513), and the guard only makes sense while that stays true.

_Wave 4, filed from a coverage read on `main` @ `54ca2dd`, 2026-09-02. The shared filter note is on #194. **This one changes production code**, and the decision it rests on has been made — see below._ ## `convert()` can enqueue a second job over a live one ``` app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt:676-682 ``` ```kotlin private fun currentInput(): InputFile? = when (val s = _state.value) { is ConversionState.Ready -> s.input is ConversionState.Converting -> s.input // ci == 0 is ConversionState.Waiting -> s.input // ci == 0 is ConversionState.Converted -> s.input // ci == 0 else -> null // ci == 0 } ``` `mi=22, ci=13, mb=7, cb=1` — one arm of eight branches is ever taken. Its only caller is `convert()` at `:513`. The Convert button renders **only** in the `Ready` branch (`ConverterScreen.kt:256` onward), so the three middle arms are not reachable by tapping it. They are reachable through the permission-result callback: `ConverterScreen.kt:91` wires `convert()` to the POST_NOTIFICATIONS **result**, not to the button. **What they do when reached is the problem.** `convert()` enqueues, then overwrites `activeWorkId`. Reaching it from `Converting` starts a second conversion while the first keeps running — with its foreground notification now orphaned, since nothing holds its id any more. That is a latent defect, not a branch waiting for a test, and writing a test first would freeze whichever reading the author happened to assume — the F1/F5 shape. ## The decision, made **Guard on state.** Narrow to what the join side already does: ```kotlin private fun currentInput(): InputFile? = (_state.value as? ConversionState.Ready)?.input ``` `JoinViewModel.kt:316` has been `(_state.value as? JoinState.Ready)?.inputs ?: return` all along. The two screens are the same shape and only one of them is over-general; this makes them agree. ## Behaviour the tests assert - A permission answer arriving while the state is anything but `Ready` enqueues nothing and leaves the screen alone. - Specifically from `Converting`: the live job's id is still the one `cancel()` would reach, and no second `WorkRequest` exists. ## Same shape, same ticket Three more cold refusal arms of the same kind, all reachable through a launcher result that outlived the state it was started from — a result redelivered after process death arrives at a brand-new ViewModel sitting on `Idle`: ``` app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt:513 currentInput() ?: return app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt:600 pendingSave() ?: return app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt:316 (as? Ready)?.inputs ?: return app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt:390 pendingSave() ?: return ``` Four sites, one idea: **an answer that arrives after the screen has moved on does nothing, rather than crashing or acting on a stale assumption.** Drive each with `reset()` (or a fresh ViewModel) followed by `convert()` / `save(uri)`. ## Acceptance: the mutations that must go red - Restore the four-arm `when` and assert from `Converting` that no second `WorkRequest` is enqueued. - `currentInput()!!` and `pendingSave()!!` at the four guard sites — each throws deterministically. Restore, confirm green. ## Note for the reviewer This narrows a `when` to an `as?`. Check that no *other* caller of `currentInput()` appears in the diff — there is exactly one today (`:513`), and the guard only makes sense while that stays true.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
above-cut
Worked autonomously overnight: local, JVM-verifiable, no product decision
 accessibility
Barrier affecting people with disabilities
 backlog
Held for manual review: product/UX call, CI/workflow, hardware, or unverifiable here
 bug
Something isn't working
 confirmed
Reviewer demonstrated the defect
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 plausible
Reviewer could not fully demonstrate it; treat as unproven
 question
Further information is requested
 sev:high
High severity
 sev:low
Low severity
 sev:medium
Medium severity
 wontfix
This will not be worked on
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
JMR-dev
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: JMR-dev/LibreMediaConverter#202