The other half of the hole the previous commit closed. `git ls-files '*.sh'` does not match workflow `run:` blocks, and a good deal of this repo's bash lives there -- so a workflow edit was still the case where the gate passed and CI's Static analysis leg went red. Pinned by digest, read out of status_check.yml rather than copied, for the reason the shellcheck section gives and for actionlint's own: its documented install is `curl | bash` off a moving branch, which does not belong in a repo that pins every action by SHA. The container runtime detection and the SELinux `:z` mount option are hoisted out of the shellcheck branch so both checks share one answer rather than deciding it twice and drifting. Verified that it bites rather than assumed: status_check.yml was given a `needs: [a-job-that-does-not-exist]`, and the real pre-commit hook blocked with actionlint's own message -- `job "static-analysis" needs job "a-job-that-does-not-exist" which does not exist in this workflow [job-needs]`. Workflow restored; nothing but the hook is in this diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
42 KiB
CLAUDE.md
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
LibreMediaConverter is an Android media converter (Kotlin, Jetpack Compose, Material 3) with two
conversion engines: Media3 Transformer for the hardware path and FFmpeg for everything the platform
cannot do. See @README.md for the architecture and @LICENSES/README.md for the split license —
this file covers only what is not obvious from the code.
Build, test, lint
Everything is on Java 25 — daemon, CI, IDE, and the app's own bytecode. Four places say so and they must not drift apart:
| Where | What sets it |
|---|---|
| Gradle daemon | gradle/gradle-daemon-jvm.properties → toolchainVersion=25 |
| CI | java-version: '25' in both workflows |
| IDE | .idea/misc.xml |
| App bytecode | compileOptions in app/build.gradle.kts |
Do not pick a JDK for the daemon — the repo does. gradle-daemon-jvm.properties carries foojay
download URLs per platform, so Gradle provisions and runs the daemon on Java 25 regardless of what
JAVA_HOME says (that only sets the launcher — ./gradlew --version prints both). Change it with
./gradlew updateDaemonJvm --jvm-version=NN, never by hand.
Reaching 25 in the bytecode row took a deliberate build change. AGP 9's built-in Kotlin compiles
with the KGP it bundles — 2.2.10 for AGP 9.3.1 — and that caps jvmTarget at 24. The root
build.gradle.kts puts KGP (and the lockstep Compose compiler plugin) on the buildscript classpath
so AGP picks up 2.4.10 instead, which supports up to 26. That is why the module applies
com.android.application and the Compose plugin by id() rather than from the catalog. Verified end
to end, not assumed: compiled classes report major version 69, D8 dexes them, and R8 minifies them.
Consequences worth knowing before touching any of it:
- Raising
kotlinrequires a matchingcompose-compiler-gradle-plugin; they are one version. - Still do not apply
org.jetbrains.kotlin.android— incompatible with AGP 9's DSL. - Java 24 is not an option even though Kotlin allows it: Adoptium dropped the EOL non-LTS, so there is no installable temurin-24. 25 is LTS and in the repo.
The Gradle wrapper does not float and cannot: distributionUrl names one archive and
distributionSha256Sum is that file's checksum. Bump it with ./gradlew wrapper --gradle-version X --gradle-distribution-sha256-sum <sha> so the two stay consistent.
./gradlew :app:assembleDebug # build debug APK
./gradlew :app:testDebugUnitTest # JVM unit tests
./gradlew :app:ktlintCheck # formatting
./gradlew :app:ktlintFormat # fix formatting in place
./gradlew :app:detekt # static analysis
./gradlew :app:lintDebug # Android lint
./gradlew :app:jacocoTestReport # coverage (XML+HTML under app/build/reports/jacoco/)
# single unit test:
./gradlew :app:testDebugUnitTest --tests "org.libremediaconverter.model.ConversionRouterTest"
CI's "Static analysis" gate is exactly ./gradlew :app:ktlintCheck :app:detekt :app:lintDebug --continue. Run it with --continue locally too: one round trip gives you all three lists instead
of the first one that fails.
Before treating a change as done, run: assembleDebug + testDebugUnitTest +
compileDebugAndroidTestKotlin + ktlintCheck + detekt + lintDebug.
compileDebugAndroidTestKotlin matters more here than it looks — the instrumented suite cannot run
on this machine (below), so without it an androidTest compile error is not discovered until CI.
ktlint and detekt also cover the test/androidTest source sets that lintDebug skips.
Instrumented tests: where they actually run
This section said the opposite until 2026-08-24, and both of its claims had been false for two days. Read it as the current answer, and see the git history if you need the old one.
-
Local emulators work, for API 33-36.
tools/local-emulator/run-e2e.shruns them on this host. The segfault that made this look impossible was not a broken machine: SwiftShader's Reactor JIT writes generated shader code onto the heap and executes it, Fedora's SELinux policy deniesexecheap, and qemu dies. Choosing a different renderer avoids it entirely —-gpu host,angle_indirectandswangle_indirectall boot, whileauto,off,guestandswiftshader_indirectdo not.docs/local-emulator.mdhas the evidence and the per-API renderer table. -
CI runs API 37, and it gates. The matrix is 33/34/35/36/37. Seven of the 71 instrumented tests cannot be run on that image, for three measured reasons and two inherited: three Media3 tests fail inside the emulator's own
c2.goldfish.h264.decoder, one SAF test takes the framework down when it rotates the display, and its sibling — the SAF picker round trip — abortssystem_serverfrom the task-snapshot path whether it passes or not. The sixth, that class's two saves through the picker (#226 and #250), carry the marker because they open the same picker and a second DocumentsUI dialog on top of it — not because either has ever been observed here. It cannot be: the rotation test runs first and takes the framework down, so all five advisory runs at the previous baseline reportedexpected: 6, received: 4, failed: 4, and the four were the three Media3 tests plus the rotation — runs 34041156680, 34041593697, 34042397320, 34043502322 and 34045105857. No picker test has ever reported on the advisory leg, which is a correction to what the marker's own KDoc used to say. All seven carry@FailsOnEmulatorApi37and run in a separatecontinue-on-errorjob; the gating leg runs the other 64 — the same 64 for the third time running, which is exactly how this paragraph goes stale unnoticed: 69−5, 70−6 and 71−7 are all 64.These two numbers move with the suite and are derived, not remembered.
grep -cE '^\s*@Test'overapp/src/androidTestis the first; the second is that minus the marker count.github/scripts/e2e-report-shape.shgreps. Cross-check against any run's shape rather than trusting the sentence: a leg below 37 reports the first asexpected, and the API 37 gating leg reports the second.That third reason is why "cannot pass" became "cannot be run" on 2026-09-05. Four gating runs were read logcat-first — 34006456986, 34001744574, 34001377499 and the green 34002313300 — and each carries exactly two
hasReadColorBufferDmaaborts before the suite (surfaceflinger, during boot and the SystemUI disable) and exactly one during it:system_server, threadTaskSnapshotPer, always inside the picker test's window, and nothing else in the gating set reached the mapper at all. Whether the leg went red was luck — one run passed the test and lost the leg anyway withfailed: 0, another passed it 0.6 s after the abort and went green. That is #108, it cost roughly a third of the gating legs over the wave-4 landings (#190), and a marker is what it needed.docs/api-37-emulator-crash.mdhas the timings.A second thing came out of those logcats, and it withdraws a caveat rather than adding one. The API 37 row was documented as the one leg running "with SystemUI disabled and the framework restarted under it", which nothing else does. Neither half was ever happening:
adb shell stopandstartare root-only and answeredMust be rooton every leg ever run, andpm disable-userdoes not stop SystemUI starting on this image anyway — measured on CI and locally, with and without a real restart. So this row's device configuration is the same as the other four's, and a green here means what a green at 33–36 means.E2E_DISABLE_SYSTEM_UIis kept under its now-stale name because what it really buys is a 45-second window with no new gralloc aborts before the suite starts, which is load-bearing;.github/scripts/e2e-run.sh's header is where that is written down.That job is still called
E2E API 37 Media3 hardware transcode (advisory), which no longer describes everything in it. The name is kept deliberately — it is not a required context and people have learned to look for it — so read the marker, not the name, for what it holds. It is red on every PR, by design: do not read it as your change breaking something, and do not read a green run as evidence those seven tests pass.docs/api-37-emulator-crash.mdhas the measurements.That instruction is also why nobody looks, so the job now reports its own shape — expected, received, failed, and whether the run completed — to the job summary, and compares it against
FAILS_ON_EMULATOR_API37_BASELINE, committed beside the marker. A deviation is a::notice::; the job stays advisory and its conclusion is untouched. Add or remove a@FailsOnEmulatorApi37and that number changes in the same diff, or the next run says so. A bare failure count would not have worked: the run is usually truncated by anINSTRUMENTATION_ABORTED, and the test XML is written anyway and says nothing about it —.github/scripts/e2e-report-shape.shis 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 cleanlyonly ever meant "instrumentation was not aborted", which stays true of a leg theWEDGE_TIMEOUTkilled 22 minutes in — so without that row the table readreceived: 59, completed cleanly: yesfor a leg that had just died. The wedge is passed to the report asE2E_WEDGED_AFTERbye2e-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 seven tests are the one thing CI cannot answer for.
On a device or emulator, build only the ABI it can execute:
./gradlew :app:connectedDebugAndroidTest -PabiFilters=x86_64
FFmpeg's native libraries dominate the APK, so shipping arm64 to an x86_64 emulator doubles the install for code that can never run — and on API 37 the full APK does not fit at all.
Conventions
-
ktlint owns formatting, detekt owns static analysis. detekt's formatting ruleset is off, so the two can never disagree about the same line. Never hand-fix a formatting complaint — run
ktlintFormat. Style isintellij_ideaat 120 columns, set in.editorconfig. -
detekt config is
config/detekt/detekt.yml, merged onto detekt's defaults (buildUponDefaultConfig = true), so it carries only the rules this codebase legitimately breaks — each with the reason written next to it. Relax a rule that way or fix the code; never a bare@Suppress. Do not invent config keys: unknown ones are rejected. -
The
modelpackage is excluded fromReturnCountandCyclomaticComplexMethodonly. It is the decision layer, where one branch is one documented user-visible outcome and the metric counts answers rather than complexity. Every other rule still applies there. -
Coverage is reported, not gated — 94.2% of lines (2234/2372), 87.5% of branches (1171/1338), measured 2026-09-05 with
./gradlew :app:jacocoTestReport, against 628 JVM tests in 96 classes.Every figure this file carried before 2026-08-24 was an artifact, roughly half the real one. Robolectric loads classes through its own sandbox classloader with no source location, JaCoCo skips no-location classes by default, and nothing told it otherwise — so not one Robolectric test counted, and Robolectric is what exercises the framework edge here. The
isIncludeNoLocationClassesblock inapp/build.gradle.ktsis what fixes it; do not delete it as stray config, and re-run the numbers if you ever touch it. Same commit, same 335 tests: 29.7% -> 69.2% with that block alone.The old entry also explained the wrong thing. It said coverage fell as the suite grew from 11 test files to 43 because "the denominator outran the numerator" on framework-edge code "the JVM cannot reach". The JVM reaches that code fine. What actually happened is that the new tests were disproportionately Robolectric, so each one added denominator and no numerator — the measurement was punishing exactly the tests that were hardest to write.
Two things still hold. A floor needs a baseline that has settled, and this one has not. It moved 39 points in a single build change on 2026-08-24; then another 16 as the #52 test push and the fixes it turned up landed — 69.2% -> 84.9% line, 53.2% -> 63.8% branch — while the denominator grew 2194 -> 2321, because that work added production code of its own; then again on 2026-08-27 as #132 and #133's ten children landed — 84.9% -> 87.1% line, 63.8% -> 69.1% branch, 454 -> 502 tests. Branch moved four times as far as line that last time, and that is the shape to expect from this kind of work rather than a surprise: those children targeted decision code — enum fallbacks, refusal arms, cursor shapes, a
whenover container rules — where one test chooses a branch the suite had never taken. Line coverage barely notices; branch coverage is the whole point.Then #153's five children on 2026-08-29 — 87.1% -> 88.9% line, 69.1% -> 75.4% branch, 502 -> 546 tests.
That last branch figure moved for two reasons, and only one of them is new tests. The numerator rose 974 -> 1011; the denominator fell 1410 -> 1340. Both are the seam work. Pulling a
whenout of a lambda inside acollectdeletes the coroutine state machine's synthesized branches around it, and what is left is a plain function whose branches a test can choose:ConversionViewModel$observe$1$1went from carrying the whole mapping to 6 branches, while the extractedConversionViewModelKtcovers 41 of 42 andJoinViewModelKt38 of 39. So a seam is worth more than the tests it enables — it also stops the measurement counting scaffolding.Be careful quoting a branch move on its own for that reason. A percentage that rises because the denominator shrank is not the same claim as one that rises because more branches are tested, and this entry has a history of explaining its own numbers wrongly.
Then wave 3 (#167-#178) on 2026-09-02 — 88.9% -> 92.8% line, 75.4% -> 81.3% branch, 546 -> 584 tests, in ten PRs from #179 to #188.
Its shape is different from the two before it, and the difference is the thing to carry forward. Waves 1 and 2 were finding uncovered code. By wave 3 there was not much of that left, so the gaps were sorted into two kinds before any test was written:
- coverage gaps — the line never executes. Filtered to sites where JaCoCo reports
mi > 0, a concrete instruction no test runs, which is what separates a real gap from a partial branch on a compound condition. That filter cut the candidate list roughly in half and was right to. Wave 4 found it wrong in both directions, though — use the two filters below instead. - assertion gaps — JaCoCo is green and nothing checks the answer.
MainActivity's rail and bottom bar were both executed byAppRootRestorationTestand transposing them passed the entire suite; so did swapping the two progress-notification strings, and swappingContent's two destinations. No coverage number would ever have found any of the three.
Wave 4 (2026-09-02) corrected that first filter, and the correction is the reusable part.
mi > 0fails in both directions. It over-reports on Compose:JoinScreen.kt:222readsmi=10and alsoci=38, andJoinStateAffordancesTestalready clicks that Save button and assertssave:joined.mp4— the missed instructions are the synthesized$changed/$dirtyrecomposition-skip path, the same codegen this file already warns about for branch counts, showing up in the instruction count too. And it under-reports on warm methods with cold arms:ConversionViewModel.cancel()misses no line, yetactiveWorkId?.let(...)had only ever been entered on the null side in 584 tests. Use two filters together instead:ci == 0— the line never executed. This is JaCoCo's own missed-line definition, so it totals exactly the reported missed-line count and needs no judgement.ci > 0 && mb > 0at method level — a covered method with an arm nothing takes. This is the only one that finds thecancel()shape.
Of wave 4's 251 missed branches, just 18 sat on lines that do execute, so the branch gap and the line gap are largely the same gap; the second filter is about which of them are reachable.
So every ticket named the mutation that had to go red, and that was its acceptance criterion rather than a coverage delta. It caught two vacuous tests written in the same session, before either shipped:
- a
firstContainerHoldingtest asserting a refusal still offered something. True, and useless: the source container is a candidate in its own right, so the list stays non-empty whatever the fallback does. What it actually buys is the codec the user asked for. - a staged-delete test scanning for a
"join-"prefixStagingNames.forJobdoes not produce — it names files<jobId>.<ext>, so the assertion was true of everything.
It also corrected a third test that was not vacuous:
probeForConcat's KDoc claimed to drive thecatcharm, and rethrowing from that catch left it green. That is how the arm turned out to be unreachable — see the next paragraph. A passing test with a wrong explanation is its own failure mode.A green mutation is only evidence when the mutation is a real change, which is the mirror trap: one
classifymutation stayed green because reordering two arms was semantically equivalent for every reachable input. A bad mutation and a weak test look identical in the output.Three things came back not as the ticket described them, which is a result rather than a shortfall:
ContainerCapabilities:282'sexcludefilter cannot drop anything.repairalways changes a codec on the shared container — a codec it left alone is onevalidatewould not have refused — and the one non-defaultexcludecarriesCOPYwhile every candidate carriesNONE. F4-shaped.probeForConcat's catch arm is unreachable on this runtime. Robolectric'sMediaExtractornever throws fromsetDataSource, measured across an unregisteredcontent://authority, a missingfile://, a file of garbage bytes and anhttp://URL — all four returned withtrackCount = 0. It stays device-only.JobSnapshots:31's missed arm was not the!isFileone the ticket named — that is already covered by thereclaimedfixture. It waspath == null: a job carrying no output path at all. Read the report, not the ticket, when the two disagree.
One item was included against the F4 rule rather than exempted by it, and the distinction is worth having written down since both live in the same function:
ContainerCapabilities:94(accepts(container, VideoCodec.NONE, mode)) is dead in production today — every caller guardsNONEfirst — and was tested anyway, because its audio twin at:101has had a test since #136 and the asymmetry was the argument. TheCOPY -> error(...)arms beside it stay exempt, because a second line of defence that can be provoked is not one.Denominators moved here too, in both directions and for two different reasons: 1340 -> 1342 branches from
MediaProbe.merge, 2348 -> 2352 lines from theConcatJoinerinterface. Neither is new untested code.And re-measure before quoting: this entry was once written quoting 81.4%, measured four hours earlier, and was already three points stale by the time it was ready to merge.
Wave 4's read (2026-09-02) moved no number at all, and that is its result. It was a triage rather than a test push: twelve tickets (#192-#203), four deferred candidates (#204), and five findings (F6-F10 in
docs/coverage-read-findings.md). What it establishes is the shape of what is left, which is different again from wave 3's:- Of 169 never-executed lines, 81 are native or device edges and stay that way —
FFmpegEngine33,Media3Engine24,ConcatEngine14,MainActivity.onCreate10 — their zeroes being thetestDebugUnitTest-only measurement boundary that #84, #85, #86 and #88 each recorded before. A further 34 are device-bound only until a seam moves them:AndroidDeviceCodecs20 (#194) and the 14 ofMediaProbe's 26 that arereadMediaInformation(#195). Do not read that second group as exempt — the two tickets exist because it is not. - Most of the rest is already closed with a reason on record, or compiler-generated: default-arg
bridges, DI factory lambdas, synthetic
NoWhenBranchMatchedExceptionarms, coroutine completion. - Six of the ten findings in that document are now "no action" or "not a test gap". By this point the report's remaining red is mostly arms nothing can reach, members nothing calls, and arms a test can reach but cannot pin — and a coverage number tells none of them apart.
The biggest single gap it found was not a missed line.
ConversionViewModel.cancel()andJoinViewModel.cancel()report every line covered; only the null arm ofactiveWorkId?.let(workManager::cancelWorkById)had ever been entered, so nothing in 584 tests connected the Cancel button to WorkManager (#192). That is what the second filter above is for.It also re-opened a mechanism, not a close: #86 and #133 ruled
AndroidDeviceCodecs.probe()out throughShadowMediaCodecList, on the grounds that the builder cannot setisAliasorcanonicalName. A pure seam does not have that constraint, and #133 did not evaluate one. Read #194 before re-arguing either way — and note the reason it is worth cutting is not coverage but that therunCatchingfallback logs "assuming permissive" while returning empty sets, which makescanEncodeandcanDecodeanswer no for everything.Wave 4's tests then landed on 2026-09-05, as #206-#217 for the twelve tickets plus #218 (#159) and #219 (#122): 92.8% -> 94.2% line, 81.3% -> 87.5% branch, 584 -> 628 tests in 87 -> 96 classes. Missed lines 169 -> 138, missed branches 251 -> 167.
Its branch move is a different animal from the 2026-08-29 seam work's, and the difference is the point. That one gained 6.3 branch points with the numerator up 37 (974 -> 1011) while the denominator fell 70 (1410 -> 1340) — much of the rise was scaffolding leaving the measurement rather than arms being covered. Here the numerator is up 80 (1091 -> 1171) and the denominator moved -4 (1342 -> 1338). So this one is almost entirely tests choosing arms nothing had chosen, which is what the entry above warns to check before quoting a branch figure. The line denominator rose the other way, 2352 -> 2372, and that is new production code rather than untested code: the seams the wave cut —
capabilitiesFrom,ffprobeInfoFrom,sessionOutcome, andsweepScope/startupSweep.The two-filter method above is what found the work, and its second filter earned its place: the largest single gap of the wave (#192, the Cancel button never shown to reach WorkManager) sits on lines that were already green and no line-level filter could see it.
One result worth carrying forward about evidence rather than coverage. #218 fixed a flake whose reproduction is statistical, and running the whole suite six times per arm caught nothing either way — at the observed rate a clean six-run arm is roughly a coin flip, so the comparison was underpowered and proved nothing. What settled it was a deterministic mutation, and then the merge train confirmed it by accident: the race reproduced on #217's Unit tests leg, which sits below #218 and carries the unfixed scope. Prefer a mutation that must go red to a repetition count when a fix is for something intermittent.
Every number above is
testDebugUnitTestonly, and on 2026-09-05 the instrumented suite got its first read for that reason —docs/e2e-read-findings.md, entries E1-E7, tickets #223-#230. Four waves had been steered by a figure that cannot seeapp/src/androidTestat all, so nothing had ever asked what those 60 device tests pin, only that they were green.It found one test that passes while testing nothing, and it is the one that matters most.
HardwareFallbackTestis the only automated check of the hardware→software fallback against a real codec failure, and on run34004304566the API 33, 34, 35 and 37 legs each logRouting sample_h264_444.mp4 -> ... via FFMPEG (NO_HARDWARE_ENCODER)(API 36's logcat artifact on that run is truncated, so it is unread rather than different): emulators expose no hardware encoder, so the job never reaches Media3 and thecatchit exists to prove is never entered. Its two assertions — succeeded, output non-empty — are true anyway, and it finishes in 448 ms. Deleting thatcatchreddens nothing on any leg (#223).Two things generalise from it. A test can assert and still not reach, which no coverage number and no "does it assert something" review would catch — the filter that works is does this test's premise hold on the machine that runs it?. And the codebase already knew: the sibling
ForcedFailureTestpinsDeviceCodecs.PERMISSIVEagainst exactly this hazard and writes out why, as doesConversionWorkerTest. The difference is that their assertions are about the path, so without the pin they would fail loudly;HardwareFallbackTest's are about the output, so it passes quietly. Prefer asserting the path over asserting the artefact where the two differ.The read was a triage, not a test push, and six of its seven findings are prose rather than code — the suite itself is in good shape. What had drifted is its self-description.
Working the tickets then found the thing the read could not: one production defect. #238 — joining files picked through the system picker failed outright on the stream-copy path. The concat demuxer whitelists protocols separately from
-safe 0, andffkitsafwas not on the list; onlySTREAM_COPYfeeds it a list file, and every existing join test passedUri.fromFile, so the one broken combination was the only one a user could reach. Not a missed line and not an unasserted value — two covered things no test put together, which is the gap shape a coverage number is worst at.E7 is the other reusable result, because it re-scoped its own ticket. A real
DocumentsProvidercannot be reached without the picker: an unprotected one is refused at install, instrumentation runs in the app's uid so the test APK's identity is no help, and shell identity is denied too — each denial namingACTION_OPEN_DOCUMENT. So #226 has no cheap headless half. But the input bridge needs no documents provider at all, which is what kept #225 headless and is how #238 surfaced.The 2026-09-06 re-check found that the read's own last PR had re-introduced the drift the read was about, and that is the entry worth carrying forward. #226 moved the suite 69 -> 70 and the markers 5 -> 6 and changed neither the count in this file, the marker's KDoc, nor the two comments in
status_check.yml. The gating figure is what hid it: 69 - 5 and 70 - 6 are both 64, so the one number a reader checks against a run had not moved — which is precisely why the paragraph above says to derive these rather than remember them. Worse, two KDoc claims in the new test described a draft rather than the code: it says MP3 was chosen so the setup could not depend on the device's codecs, while the code converts at the defaultMP4_H265/FASTand therefore routes oncanEncode(H265)— the negation of the stated reason. That is E1 and E3's failure mode, committed by the wave that found it. All of it is fixed; the standing item is #250, because #226 proved D4's premise and never drove its delete arm. - coverage gaps — the line never executes. Filtered to sites where JaCoCo reports
-
Nothing is committed or pushed until the local gate is green, at every supported API level. Source work (
app/src/main) needs the unit tests and the instrumented tests passing on every level; test work (app/src/test,app/src/androidTest) needs the whole suite passing on every level.tools/git-hooks/local-gate.shenforces it aspre-commitandpre-push; wire it up once withgit config core.hooksPath tools/git-hooks.33-36 run the whole suite on emulators. API 37 cannot be run on an emulator on this host at all — not "is red", cannot run: measured 2026-09-06, the image logs
3 new surfaceflinger aborts in 45 s (want 0)and then the APK install itself fails withCan't find service: package, because the framework is gone before Gradle installs anything.Starting 0 tests. So the hook runs API 37 on the attached Pixel 10 Pro XL when it is there, and says plainly that the level is uncovered when it is not — CI's gating leg being what answers for it then. It never claims five levels having run four.It runs shellcheck and actionlint too, at CI's exact pins — shellcheck over
git ls-files '*.sh', actionlint over the workflows, the same digests and the same file sets that leg uses. actionlint is not an afterthought to shellcheck but the other half of the same hole: much of this repo's bash lives in workflowrun:blocks, which'*.sh'does not match at all. That gap was found the hard way: the gate checked ktlint, detekt and Android lint, so a new.shfile was precisely the case where it passed and CI still went red, and the first file it could not check was itself. The digest is read out ofstatus_check.ymlrather than copied — two copies drift, and the symptom of that drift is the gate passing while CI fails, which is the one thing this check exists to prevent.The sweep is cached under the hash of the
app/srcsubtree, not the whole repo tree. Keying it on the whole tree was the first cut and it was wrong: editing a comment inCLAUDE.mdthrew away a sweep of byte-identical application code and re-ran forty minutes of emulators to prove nothing, which is how a gate teaches people to resent it. Any change underapp/srcstill invalidates it, and the JVM gate runs unconditionally. There is deliberately no skip variable, and--no-verifyneeds the repo owner's say-so each time rather than being reached for when the gate is inconvenient.Why it is worth tens of minutes a commit: the alternative was measured on 2026-09-06, when one PR spent several gating legs learning one leg at a time what a sweep answers in one pass — and the failing leg moved between runs (API 35 red then green, API 34 green then red). One leg at a time that reads as someone else's flake; as a sweep it is one signal.
-
Testable code is not done until it is tested. If a piece is unit testable, it gets unit tests before it counts as done. If it is e2e testable, it gets e2e tests. Both clauses apply — a change that is both needs both.
Three things make that a real bar rather than a slogan here:
- Unit-testable is broader than it looks. The pure-seam pattern —
work/FailureOutcome.ktdocuments the reasoning — turns "needs a device" into "a pure function plus a thin edge". Robolectric is in the JVM source set,compose-ui-test-junit4with it, so Compose screens are unit testable too. Reach for the seam before concluding something cannot be unit tested. - E2E is runnable locally, API 33-36, via
tools/local-emulator/run-e2e.sh— see "Instrumented tests: where they actually run" above. That was believed impossible until the SELinux/renderer cause was found, and it is what makes the e2e half of this norm enforceable. - A test has to bite. Revert the line it covers, confirm it goes red, restore. A review of this codebase ran 46 mutations against a 257-test suite and 9 were vacuous — five of them passing the whole suite over a completely unguarded code path. Green is not evidence.
Name what you did not cover and why. Genuine exemptions exist; implied coverage is the problem.
- Unit-testable is broader than it looks. The pure-seam pattern —
-
kotlin.code.style=official. Gradle stays Kotlin DSL. -
A stacked PR does not merge with
gh pr merge, andMERGEDis not proof it reachedmain. Two separate traps, both measured on 2026-08-27 while landing #144-#151.gh pr mergeuses the GraphQL mutation, which refuses a stacked PR outright: "This pull request is part of a stack and must be merged using the asynchronous merge REST API." So doesPUT .../pulls/{n}/merge. The one that works isgh api -X PUT repos/OWNER/REPO/pulls/N/merge-async -f merge_method=merge, which returns a uuid to poll at.../merge-async/{uuid}untilstatusismergedorfailed.The second trap is worse, because nothing looks wrong. GitHub retargets a stacked PR's base to
mainwhen the PR below it merges, but asynchronously. Merge a stack faster than that settles — five PRs about thirty seconds apart, in the case that found this — and each one merges into its own base branch, which has itself already been merged and left behind. Every call returnsstatus: mergedand every one is true.gh pr list --state opencomes back empty, every PR showsMERGED, and none of the content is onmain.What caught it was a coverage re-measure reading two points lower than the same tree had measured an hour earlier; a fresh
git pullchanged nothing, which is what turned it into a question.git merge-base --is-ancestor <merge-sha> origin/mainanswers it in one line. Do that after merging a stack, or merge one at a time and re-readbaseRefNamebetween. #160 is what the recovery cost.The auto-retarget belongs to the stacking feature specifically. A PR opened with a plain
gh pr create --base some-branchdoes not retarget when that branch merges — it is left pointing at a dead base and has to be moved by hand. -
File one-off issues with
tools/github/file-issue.sh, notgh issue create.gh issue createdoes not touch the project board, so the issue exists, carries its labels, and is invisible in the Kanban — indistinguishable from never having been filed. Measured 2026-08-24: eight issues filed as a scripted batch all reached the board; one filed as a one-off minutes later did not. A batch carries the board step in its loop; one-offs are where it slips, which is what the script is for. It resolves the project and Status ids by name rather than caching them, and it reads the item back — a mutation returning 200 is not evidence the board shows what was asked for. Exit 3 means the issue was created but did not reach the board, and prints the number so it cannot be lost quietly.above-cutandbacklogare labels from the 2026-08-22 triage pass — "worked autonomously overnight" and "held for manual review". They are not board columns. Status carries board state; do not put a cut label on a newly filed ticket. -
shellcheck runs in CI, inside the Static analysis job, over
git ls-files '*.sh'so a new script is covered without editing the workflow. It runs at full severity,infoincluded: the two findings that raises today are answered with targeteddisabledirectives carrying their reason, exactly asconfig/detekt/detekt.ymlcarries only the rules this codebase legitimately breaks. Do not silence it with--severity=warning— that hides the next real finding too. It is pinned by image digest, and joins ktlint/detekt/JaCoCo in the "Dependency versions" rule above — for exactly the reason stated there, demonstrated the day it was added. The first cut used the runner's ambient shellcheck. That is 0.9.0, while the container used to check locally was 0.11.0, and the two disagree about how to report a trap handler: 0.11.0 saysSC2329once on the declaration, 0.9.0 saysSC2317on each of seven lines in the body. Same script, same directive, one green and one red. Directives that must survive both name both codes.Locally, use the same pin rather than whatever is installed:
podman run --rm -v "$PWD:/mnt:z" docker.io/koalaman/shellcheck@sha256:61862eba... <files>(the digest is instatus_check.yml; there is no shellcheck system package on this host).actionlintcovers the half shellcheck cannot see — the inlinerun:blocks, where a good deal of this repo's bash lives. It runs shellcheck over eachrun:plus its own checks on expression syntax,needs:references, matrix keys and action inputs. It sits in the same job, pinned by digest for the reason above and one of its own: its documented installer is acurl | bashoff a moving branch, which does not belong in a repo that pins every action by SHA. Locally:podman run --rm -v "$PWD:/repo:z" -w /repo docker.io/rhysd/actionlint@sha256:9d360886... -color.
Dependency versions
Libraries float on minor + patch (coreKtx = "1.+"). Three groups deliberately do not:
agp,kotlin,kspare version-locked to each other. AGP 9.3.1's POM declareskotlin-gradle-plugin2.2.10, and that is what AGP's built-in Kotlin compiles with. Android lint will suggest Kotlin 2.4.10; taking it breaks the Compose compiler unless KGP is also forced onto the root buildscript classpath. Move all three together, by hand, or none.- ktlint, detekt and JaCoCo are pinned. A new rule in a linter makes files nobody touched stop passing, so CI goes red on a PR whose diff cannot explain it. Upgrading them is its own commit: run the tool, read the new findings, fix or relax them.
- The FFmpeg AAR is a committed file, not a coordinate.
+ does not mean "newest stable" on its own — Gradle will happily resolve it to an alpha, and
androidx routinely publishes alphas numbered above the current stable (at last check: lifecycle,
navigation, work, datastore and annotation all did). The componentSelection block in
app/build.gradle.kts rejects prereleases, which is the only reason 2.+ means 2.11.0 rather than
2.12.0-alpha01. Do not remove it. To try a prerelease, name the exact version in the catalog —
that pins it, which is the right way round.
Because versions float, a build can change without a commit. ./gradlew :app:dependencies --configuration debugRuntimeClasspath shows what actually resolved.
Traps
-
Do not apply
org.jetbrains.kotlin.android. AGP 9 has built-in Kotlin; applying the legacy plugin fails the build. This is whylibs.versions.tomlpinskotlinto AGP's bundled KGP version rather than the newest Kotlin release — the Compose compiler plugin must match it. -
The FFmpeg AAR is committed under
bin/, deliberately. It is not on any Maven repo (ffmpeg-kit was archived and delisted). Rebuilding per CI run made red builds ambiguous: broken code, or a cross-compile that hiccuped?bin/README.mdhas provenance and how to regenerate it. -
Anything touching Media3 carries
@UnstableApirather than swallowing the marker with@OptIn. Android lint'sUnsafeOptInUsageErrorcatches a missed one. -
Release builds ship both ABIs.
-PabiFiltersis a test-run override only;build.ymlverifies the released APK carries every ABI and that all native libraries are 16 KB aligned. -
A JUnit
Timeout— rule or@Test(timeout=)— cannot be used in the JVM suite. Both run the test body on a separate thread, and every Compose test here goes through Robolectric's paused main looper:UnsupportedOperationException: main looper can only be controlled from main thread, fromShadowPausedLooperunderRobolectricIdlingStrategy.runUntilIdle. The identical tests pass with the timeout removed, so it is the mechanism, not the test. What bounds a hung run instead istimeouton theTesttasks plus the jstack watchdog beside it inapp/build.gradle.kts, neither of which moves a thread.HangBoundTestguards both numbers, and a timed-out run writes no XML for the class that hung — the dump is its only attribution, so do not delete the watchdog as stray config. It has since been exercised in anger: on 2026-09-05 it caught #125's Room/WorkManager deadlock on CI, failing in 10m57s with the hung test named, where that ticket had predicted a 60-minute cap and no cause. #125 is closed as bounded on the strength of it — the inversion itself is internal to the two libraries and still live atwork-runtime2.11.2 /room2.7.0. -
The JVM suite does not run
LibreMediaConverterApp.app/src/test/resources/robolectric.propertiesnamesTestLibreMediaConverterAppfor every test, and it differs from the real class in exactly one thing:sweepScopeisDispatchers.Unconfined, so the startup staging sweep finishes beforeonCreate()returns instead of running onDispatchers.IO.That line is load-bearing — do not delete it as stray config. Robolectric builds an
Applicationper test class that asks for one, and eachonCreatelaunched a sweep over the shared<cacheDir>/conversions/that nothing joined. So a test asserting about a staged file was racing every sweep the classes before it had left in flight (#159). It was CI-only until wave 4 added ten Robolectric classes, at which pointOutputPublisherStagingTestfailed on roughly one local run in six. Per-test opt-in was measured and rejected: 27 of the 58 Robolectric classes touch that directory. TheSupervisorJobis kept in the test scope so a throwing sweep is swallowed there exactly as in production — the dispatcher is the only intended difference.It cost one assertion, knowingly.
AppStartSweepTestused to open by asserting that the manifest'sandroid:nameis what Robolectric instantiated, so the sweep is code that actually runs. Anapplication=override replaces the manifest rather than being checked against it, andapplicationInfo.classNamereports the override too — measured — so that claim is not merely unasserted on the JVM now, it is unobservable, and a rewritten version would assert the override against itself. The manifest link is device-only. What remains is theas LibreMediaConverterAppcast in that class'ssetUp, which catches only the test app ceasing to extend the real one.