CLAUDE.md's own rule is "re-measure before quoting", and the figure it carried
was measured on 2026-08-24 -- before the #52 children, the MediaProbe and codec
tests, and the guards from #100/#107 landed. Quoting it now would understate the
suite by twelve points, which is the same failure the bullet directly below it
was written to describe.
Measured on b53f326 with ./gradlew :app:jacocoTestReport:
LINE 1847/2268 81.4% (was 1519/2194, 69.2%)
BRANCH 837/1390 60.2% (was 53.2%)
against 417 JVM tests in 60 classes, all green.
The denominator moved too, 2194 -> 2268: the same push added production code of
its own, so this is not a pure numerator gain and the note now says so. The
Robolectric/isIncludeNoLocationClasses history is left exactly as it was -- it
explains why every pre-2026-08-24 figure was an artifact, and that is still the
most useful thing in the entry. "That date" is now spelled out, since the
headline date above it has moved and the phrase no longer points at itself.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The job summary is the deliverable #83 asked for -- "readable without opening a
log" -- and GitHub exposes no API that reads a job summary back: the check-run
output for the advisory job returns summary: null, so a write that silently did
not happen would be invisible to everything except a human on the run page. The
step log can be read, so it now carries one line saying which of the two
happened, including the case where GITHUB_STEP_SUMMARY is unset entirely, which
is what running the script by hand looks like.
"A comparison was asked for" and "a number was found to compare against" were one
variable, and collapsing them put the report one refactor away from being the
thing #83 filed. The sed that reads FAILS_ON_EMULATOR_API37_BASELINE is anchored
at the line start, so indenting the const into an object -- or renaming it, or
moving it -- empties it, and the old code then skipped the whole comparison while
the table kept printing exactly as before. Silent, and indistinguishable from a
run that matched.
Now an unreadable baseline is itself a deviation, with the notice naming the
const so the fix is obvious. Verified against the real captured log of run
32865281555 three ways: baseline file absent, const indented into an object, and
the committed file unchanged -- the first two announce, the third stays silent.
That job is continue-on-error and red on every PR by design, which CLAUDE.md
states plainly -- and that instruction is exactly why nobody reads it. Nothing in
a red X separates "the known three" from "the known three plus yours".
A bare failure count would not have fixed it, and this is measured rather than
assumed. The run is usually truncated: seven of eight advisory runs read on
2026-08-25 ended in `Test run failed to complete. Expected 3 tests, received 2.`
with INSTRUMENTATION_ABORTED, and one did not. A count taken from a truncated run
misleads in both directions -- a fourth marked test can still yield the same
number if the abort lands earlier, and the known set getting worse can lower it.
The test XML does not rescue it either, which was the thing worth checking before
building on it: it IS written for an aborted run, and it reports a tidy
tests="3" failures="3" for a run the runner had just described as truncated. So
the XML is the authority on how many results landed, the runner's own output is
the only authority on whether the run finished, and the report reads both and
says which number came from where.
The baseline is one number beside the marker, because the marker means "cannot
pass on this image": the count is both how many tests the advisory leg runs and
how many should fail. A smaller failure count is the interesting direction -- it
means one now passes, which is the documented trigger for deleting the
annotation.
Nothing about the job's status changes. It stays continue-on-error, stays red,
stays out of the required contexts; a deviation is a ::notice::, never an
::error::. The report is a separate script so it can be run against a real log
saved from a real CI run, which is how the comparison was shown to fire.
The gating legs get the shape without the comparison: they run the whole suite,
so comparing there would announce a deviation five times a run -- but a truncated
run reporting fewer results than it ran is what #108 looks like, and "completed
cleanly" is the field that would show it.
Closes#83
R29 found the discriminator claimed "exact across all seven" while r07 is recorded lower
down as "inconclusive rather than ruled out, because no evidence came back from it". A row
this page calls inconclusive cannot also be counted as evidence for the conclusion.
Checking it turned up a second instance of the same over-count, which R29 did not name. The
abort-cadence section said "Measured across the seven runs above" -- but the table records
r07's aborts as **not readable**, because adb wedged before a crash buffer could be taken.
Six runs contributed gaps, not seven.
Both now say six, and both say why. The discriminator paragraph also says what excluding r07
costs, which is nothing: it is a `host` row, so the discriminator predicts it would not boot,
and confirming a prediction with the one run whose evidence did not come back adds no
information in either direction. That is the point R29 made -- claiming six does not weaken
the conclusion -- and it is worth stating in the document rather than only in the ticket,
because the next reader will otherwise wonder whether a run was quietly dropped.
Deliberately left: "four of the seven runs show the directory creation itself is broken
during the loop". That is a count of how many runs showed something, not a claim that all
seven were readable for it, so it survives. Checked rather than assumed, and named here so
the next pass does not re-audit it.
R29's other half -- "state how r07's boot outcome was read" -- is not taken, because I do
not know and inventing a source would be worse than narrowing the claim. Narrowing is the
option R29 offered and the one that can be honest.
Closes#38.
build.yml's `release` job declares `contents: write`, and nothing checked it. Deleting those
lines leaves actionlint clean and CodeQL silent -- a narrower permission is not an alert --
and the job is `if: startsWith(github.ref, 'refs/tags/v')`, so no pull request and no merge
can exercise it. Measured with the declaration removed: every gating check still passed. The
first thing that would notice is a release failing to publish, at the moment someone is
trying to cut one.
The deletion also looks like tidying. #106 has just put a top-level `permissions: contents:
read` directly above it, so a reader could reasonably take the job-level block for a
duplicate. It is an override, and a comment saying so is not a check.
BackupExclusionsTest is the precedent: configuration rather than code, load bearing, and
unguarded because nothing compiles it.
The part worth reading twice is the second commit-worth of work in here. The test passed,
and then the mutation that is supposed to redden it did not:
BUILD SUCCESSFUL in 614ms
Gradle cannot infer that a test depends on a file outside the source set, so the task stayed
UP-TO-DATE and the test never ran. Under --rerun-tasks the same mutation failed it properly,
which is the tell: the assertion was right and the wiring was not. A guard that does not
re-run when its subject changes is not a guard -- it is a test that will be green on the day
it matters, which is worse than no test because it reads as cover.
Fixed by declaring the workflow as a task input. Verified the whole way round afterwards,
without --rerun-tasks: mutate the file and the task re-runs and fails; restore it and the
task re-runs and passes.
What this pins and what it does not: it asserts the declaration exists in the release job's
block. It cannot assert a release actually publishes -- that needs a tag push, which is the
thing no PR can do. A tripwire against silent removal, not proof the path works, and the
KDoc says so.
Closes#107.
CodeQL alert #1, the only open one on this repository:
actions/missing-workflow-permissions, warning / medium, build.yml:23
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN.
Alerts 2, 3 and 4 were the same rule against status_check.yml and are fixed -- that file
has a top-level block. build.yml declares permissions in exactly one place, the release
job's `contents: write`, and has no top-level default, so the `test` job inherits the
repository setting.
**Nothing is over-privileged today.** The repository default is already `read`
(default_workflow_permissions: read, can_approve_pull_request_reviews: false, read from the
API rather than assumed), so the test job holds a read token now. Saying so matters: this
is hygiene, and a commit that implied it was closing a live hole would be overstating it.
What it buys is that the default CANNOT widen these jobs later without someone editing this
file. That is not invented for the occasion -- it is the argument status_check.yml already
makes, which even names this file:
the token's reach should be readable here, and a default that widens later should not
silently widen these jobs with it. build.yml's release job makes the opposite
declaration for the same reason.
So the principle was decided, applied in two workflows and in one job of this one, and the
top level of build.yml was the gap.
Verified the thing that would actually break: the release job's `contents: write` still
wins. Top level is a default, not a ceiling -- parsed and printed both, test inherits
`contents: read`, release keeps `contents: write`.
Also ran the ticket's mutation, and it found something. Deleting the release job's
`contents: write` leaves actionlint green and CodeQL quiet -- a narrower permission is not
an alert -- so nothing would catch it until a tagged release failed to publish. That is a
separate gap and is filed rather than fixed here.
actionlint clean at the pinned digest. Comment and permissions only; no step, job or
trigger changes.
Closes#100.
R19 raised two things about this comment. One resolved itself: it used to explain why the
matrix had no API 37 row at all, and #56 added the gating row, so that half is gone.
The other survived, and this is it. The comment read
api-level must be "37.0". A bare 37 is not an SDK package and fails during setup
The second sentence is true and was measured -- it cost a run to find. The first overstates
it. What must be true is that the api-level is a POINT release; 37.0 is one of several.
api37-debug.yml's own input descriptions already say so:
API level, as the SDK spells it. 37.0, 37.1, 37.2-beta3, 36 ...
System image target. android-37.1 and 37.2-beta* ship ONLY as google_apis_ps16k
and docs/api-37-emulator-crash.md measures android-37.0 rev 6 and android-37.1 rev 8 side
by side, both aborting. So the repo already knows 37.1 exists and behaves the same; only
this comment implied otherwise.
That matters for the reader it is written for. Someone debugging this row and wondering
whether a newer image helps reads "must be 37.0" as a constraint and stops. The measured
answer is that it does not help, which is a better thing to learn than a rule that is not
one -- and the ps16k-only wrinkle above 37.0 is the detail that would actually bite them.
Comment only. No job, matrix, filter or gating behaviour changes. actionlint clean at the
pinned digest.
Closes#28.
RealMediaBenchmark's class KDoc said:
Populate with:
adb push <file>.mp4 /sdcard/Android/data/org.libremediaconverter/files/
Twelve lines below, the `samples` property KDoc -- on `get() = context.filesDir` -- says:
Internal storage, not the external files dir. Files placed in the external dir by
`adb push` or `adb shell cp` stay owned by the shell user, and the app then gets
EACCES trying to read them -- which presents as an unparseable input rather than a
permission problem.
Different directories, and the second exists specifically to explain why the first fails.
Anyone following the class KDoc stages files the benchmark cannot read, gets a skip, and
reads the skip as "not staged yet" -- the failure mode the property KDoc warns about, walked
into by the instruction in the same file.
The fix is not a corrected command. Restating the mechanism in a second place is what let
these drift, and a replacement command I have not executed would be the same defect with a
fresher date. The class KDoc now names [samples] as the single place that answers it.
Two things added that are checkable rather than remembered: the exact filenames the tests
look for, via [H264_SAMPLE] and [AV1_SAMPLE] -- the old text said `<file>.mp4`, so even the
right directory left you guessing -- and a note that the two skips every green E2E leg
reports are these.
Not claimed: that the benchmark misbehaves on CI. An earlier version of the ticket said so;
it was wrong, and measuring settled it -- both tests report SKIPPED on the gating legs, the
guards work, and "harmless in CI" is accurate. The failure that prompted the look is
Media3EngineTest, tracked as #102.
Closes#101.
ConversionViewModelProbeFailureTest's pickedProbe() helper held:
val ready = awaitState(viewModel.state, "Ready with a probe") {
it is ConversionState.Ready && it.input.probe != null
}
assertNull("nothing here should reach a terminal failure", (ready as? ConversionState.Failed))
The predicate requires `Ready`. `Ready` and `Failed` are sibling subtypes of one sealed
interface, so `ready as? Failed` is always null and the assertNull could never fire. R26
filed this PLAUSIBLE on types read; it is measured now.
Flipping the line to assertNotNull failed 3 of the 4 tests in the class -- three, because
pickedProbe() has three callers, which is also why a dead line here was worth removing
rather than shrugging at: it read as coverage in a helper the whole class depends on.
Deleted rather than replaced. There is nothing for a live assertion to add: a pick that
ended in Failed never satisfies the predicate, so awaitState fails on its timeout naming
what it was waiting for -- "Ready with a probe" -- which is a better failure message than
the assertion would have produced. The comment now says that, so the next reader does not
re-add the guard the predicate already is.
This is the ninth vacuous assertion this line of work has turned up, and the pattern is
consistent: they hide in helpers, they pass, and they look like care. The suite is green
before and after, which is exactly the point -- deleting a dead assertion cannot change a
result, and if it had, the line was not dead.
Closes#35.
SafPickerRoundTripTest began failing on gating legs at API 33, 34, 35 and 37
ninety minutes after it landed, on diffs that cannot cause it -- two KDoc
comments, a MIME lookup table, a README paragraph. Every failure named the
fixture root, so #93 was filed as a root-discovery race. It was not one, and
finding out what it was took making the test say something else first.
DocumentsUI was fine throughout: its own `ProvidersAccess: Matched roots` names
the fixture authority five times inside the sixty seconds the test spent failing.
What failed was reading any window at all -- 1095 `Retrieving node with selector`
against 1095 `Node not found` on that leg, against 7 and 2 on the green one. So
this now asks whether the app's OWN window is readable before it opens a picker,
and prints the accessibility window list when it is not.
That list named the culprit on the next occurrence:
What it could see: com.android.systemui[type=3], android[type=3]
No TYPE_APPLICATION window at all, on a device that had just logged `Displayed
org.libremediaconverter/.MainActivity`. `android[type=3]` is system_server, and
the same logcat says what it was holding, minutes before this class ran:
ANR in com.google.android.apps.nexuslauncher
Reason: Input dispatching timed out (Application does not have a focused window)
Window{4ed8414 u0 Application Not Responding: com.google.android.apps.nexuslauncher}
The launcher ANRs on a loaded runner emulator and the dialog it leaves behind
never goes away. It is opaque and fullscreen, so AccessibilityWindowManager drops
every application window beneath it -- which is how the app can be Displayed and
unreadable at once, the contradiction that made this look like a SAF bug for six
PRs. Present on both legs examined, API 33 and 34, at the failure timestamp.
So the dialog is dismissed, by resource id rather than by localised button text,
`aerr_wait` first so the app under it is left alone. Waking the device and
rebuilding the UiAutomation connection are kept behind it and are recorded as
measured non-causes rather than as fixes.
A second PickActivity is not a remedy for this either, and that was measured: the
failing leg opened one for the second test, in the same DocumentsUI process, and
read as little from it. The whole pick is still retried, but for a smaller and
separate claim -- a picker whose lists were built before their data arrived, which
#80's node-level re-find cannot reach because it re-acquires a handle inside the
one picker.
One API 37 run failed a step deeper, on the file rather than the root. That shape
has not been reproduced or diagnosed; the reopen covers it because a fresh pick
re-walks from Recent, and the KDoc says that rather than claiming more.
Two things the retry must not become. It must not tolerate an absent root, or
#64's MIME mutation goes vacuous -- so a missing node is reported rather than
retried away, and the mutation was re-run: both tests still fail, still with "the
system picker never showed BySelector [TEXT='\QLMC R38 fixtures\E']", in 126 s and
127 s against the 1200 s wrapper timeout. And it must not decide the picker has
closed by asking the same accessibility window list that is broken -- so the back
presses are counted against Activity.hasWindowFocus, which comes from the
framework.
Each new path was forced on and measured rather than trusted: the injected-failure
run showed the reopen recovering, with four OPEN_DOCUMENT starts for two tests;
the rebuild was forced unconditionally and the suite stayed green, ruling out a
connection that comes back without FLAG_RETRIEVE_INTERACTIVE_WINDOWS; the dialog
dismissal was forced with no dialog present, ruling out a blind click breaking a
healthy run. Dismissing a real ANR dialog has not been observed, because the fault
has never reproduced locally.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The shellcheck step added a few hours ago reads `git ls-files '*.sh'`. That is four files.
It does not read the inline `run:` blocks, and a good deal of this repo's bash lives there:
the release verification in build.yml, the emulator setup and teardown in status_check.yml
and api37-debug.yml. "shellcheck runs in CI" was true of the files and not of the blocks,
and CLAUDE.md said so rather than pretending otherwise.
actionlint closes that half. It parses each workflow and runs shellcheck over every `run:`,
on top of its own checks for expression syntax, `needs:` references, matrix keys and action
input names.
Pinned by digest, for the reason shellcheck is pinned -- a new rule making untouched files
fail is a red build whose diff cannot explain it -- and for a second reason of its own.
actionlint's documented install is
bash <(curl -s https://raw.githubusercontent.com/.../download-actionlint.bash)
off a moving branch. Running that in a repository that pins every action by SHA would
contradict its own supply-chain posture more than the linter is worth. That is why #70 was
filed instead of bolted onto the shellcheck commit.
It reported exactly one finding, and it is fixed here rather than suppressed: build.yml
parsed `ls` to pick the release APK (SC2012). The glob was already in the line, so a bash
array reads it without the pipe. Gradle's output names have no spaces today, which is the
kind of assumption that holds right up until it does not.
Proved it catches something, rather than trusting a green run: planting `if [ $UNQUOTED =
bad ]` into a build.yml `run:` block produces
shellcheck reported issue in this script: SC2086:info:4:6:
Removed again afterwards. A linter that cannot be shown to catch a plant is not wired in,
it is just running -- and SC2086 in a `run:` block is invisible to the .sh-file step, which
is the whole argument for this commit.
CLAUDE.md loses the "does not cover inline run: blocks" caveat, because it no longer does.
Both linters verified clean at their pinned digests.
Closes#70.
Two reasoned decisions were sitting in comments with nothing under them.
`AndroidDeviceCodecs.mimeFor`'s `COPY, NONE -> null` arm explains itself by
naming a consequence at another seam: returning null is what makes `canEncode`
answer true, because a copied or absent track places no demand on the hardware.
#90 pinned the null; nothing pinned the answer. Put a MIME in that arm and a
device with no matching encoder starts refusing stream copies — jobs that encode
nothing — and the router hands FFmpeg a re-mux Media3 could have done. Asserted
now against `forTesting(encoders = emptySet())`, with an H.264 refusal alongside
so a `canEncode` that simply said yes could not satisfy it.
The second is a whole table. `Media3Engine.videoMimeTypeFor` is `VideoCodec ->
MIME` on the same axis as `mimeFor`, and until #85 and #87 widened both to
`internal` no test could see them together. Each had per-arm coverage pinning its
own answers, which is exactly the shape that cannot notice the two tables
describing different codecs: change one arm and its own expectation together and
both suites stay green while the device is asked about H.265 and Transformer is
told to produce H.264.
They do not agree everywhere, and forcing them to would be a regression, so the
test sorts every codec into the three buckets that exist and asserts the fourth
is empty. H.264 and H.265 must match. VP8, VP9 and AV1 are named by the device
table and not by Transformer's, deliberately: `setVideoMimeType` rejects them so
the router never asks Media3, while the device may genuinely own a VP9 encoder
and `canEncode` has to answer about it truthfully. COPY and NONE are named by
neither. Sorting rather than filtering means a convergence fails too, so moving
the line requires saying so in the file.
Audio has no partner — `AndroidDeviceCodecs` enumerates video MIME types only,
so `audioMimeTypeFor` has nothing to cross-check against and a missing audio
encoder is still discovered by failing rather than up front. Named in the KDoc
as unfinished rather than left as an unexplained asymmetry.
Closes#86
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
run-e2e.sh installs a missing system image with
yes | sdkmanager --install "$pkg" > /dev/null 2>&1 || { echo " FAILED to install"; ... }
`yes` never ends. The moment sdkmanager exits and closes the pipe, `yes` dies of SIGPIPE
with 141, and this script runs under `pipefail`, which takes the rightmost non-zero status.
So a package that installed perfectly reported "FAILED to install $pkg" and returned 1.
R32 filed this PLAUSIBLE on shell semantics, unexecuted. It is demonstrated now:
set -o pipefail; yes | true -> 141 (three runs, three times)
set -o pipefail; yes | sh -c 'exit 3' -> 3
${PIPESTATUS[1]} for those two -> 0 and 3
The pipeline status genuinely cannot tell a clean install from a broken one; PIPESTATUS
can. That is the whole change -- no restructuring of the licence flow, so a fresh SDK still
gets its licences accepted exactly as before.
`echo no | avdmanager` eleven lines below is deliberately left alone, and the comment says
so. One line fits the pipe buffer, so echo has already exited before the close and there is
no signal to receive: `echo no | true` measured 0 on five consecutive runs against `yes |
true`'s 141 on three. Only an unbounded producer is exposed. Someone reading this fix later
would otherwise "fix" the echo too and change a line that was never wrong.
Why it went unnoticed: it only misfires when the image is ABSENT, and every existing
checkout already has the images. R32 noted the branch that made this the normal path. The
failure is also silent in the worst way -- the install succeeds, the script says it failed,
and the AVD is then created from a package that is really there.
shellcheck clean at the pinned digest (0.11.0, the version CI runs), bash -n clean.
Closes#41.
Both the dependency declaration and its catalog entry named
EscapedCoroutineErrors.kt as the sole reason kotlinx-coroutines-test is on the
test classpath. That file is gone, and nothing in the gate -- not ktlint, not
detekt, not lint -- fails on prose naming a deleted file, so this would have
survived as a reference a reader could only resolve through git history.
The dependency itself stays, and for a reason worth restating where it is
declared: `runTest` is what registers the collector callback, so the one test
that deliberately lets an error escape is the scope that receives it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
README promised, without qualification:
"Conversions run as durable background work, so they survive leaving the app and
are restored after a restart."
The first half is true and the reattachment work made it truer. The second half has one
exception the sentence does not admit, and it is the case a user is most likely to hit
without understanding it.
When Android refuses a foreground-service start, FailureOutcome retries -- ten attempts on
the default exponential backoff, 30 s doubling to a five-hour clamp, about eight and a half
hours in total -- and then returns FOREGROUND_DENIED on a FAILED job. Reattachment excludes
FAILED (Reattachment.kt:176). So the job is not restored, and neither is the message
explaining why: the user opens the app to an empty screen.
FailureOutcome's own KDoc already says this plainly -- "a user who was not watching when
the eleventh attempt ran will find an empty screen rather than the explanation". The code
was honest and the README was not, which is the wrong way round for the two documents.
The replacement says what actually happens and ends with the thing the user can act on:
reopening the app is what grants permission to run, so a conversion stalled this way should
be started again rather than waited on. That is the same reasoning FOREGROUND_DENIED_MESSAGE
is written on -- "open the app and start it again" is the fix, not filler.
Deliberately not claimed: that the app tells you. It does not, and #16 is the open ticket
for giving a present, willing user a way to make that retry happen now. Writing "you will
be told" here would be the same defect this commit is fixing, one release earlier.
Verified against the current code rather than the finding's date -- R35 was filed as
PLAUSIBLE on 2026-08-22 and both mechanisms it names are still in place.
Closes#44.
`drainEscapedCoroutineErrors()` cleared the collector at rule-construction time
with `runCatching { runTest {} }`, and discarding what it found was the whole
mechanism: it could not tell the one known deposit from an escaped error nobody
had asserted on. That traded a loud, misleading failure for a silent one, which
was acceptable only while exactly one depositor existed and the seam to remove it
did not.
The seam exists now, so the depositor is gone: the OOM is consumed by the test
that raises it. Every Compose class takes the v2 `createComposeRule()` directly,
and a future escaped error fails a test again instead of disappearing.
The two findings the drain's KDoc carried that outlive it: the v2 rule and the
non-v2 `StateRestorationTester` do interoperate -- the note now sits at the two
declarations that pair them -- and a drain could never have been a `@Before`
(the rule's `runTest` wraps it) or a `@BeforeClass` (Robolectric runs that
outside the sandbox classloader, where the collector is a different object).
Full JVM suite run twice in a row with the drain deleted: 373 tests, 0 failures
both times.
Closes#66
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JMR-devandClaude Opus 5