Codex is right about this one. The downloader observer builds Downloading
out of whatever status it replaces, reading shareableFallback from an
existing Downloading or a current Ready. A ViewModel restored onto work
that is already active starts from Loading, so neither cast matches and
the fallback is null. WorkManager keeps a download running across process
death, so this is the ordinary case: background the app during a 42 MB
transfer over Tor, come back, and the row drops to "Prepare App for
Sharing" while Share via Hotspot and Share via Quick Share disappear
entirely. The installed APK never moved - it was on disk and shareable a
moment earlier - and it stays hidden until the download ends.
checkStatus() could not repair it because its guard conflated two things:
not letting a resolved status overwrite active work, which is right, and
not looking at local state at all during a download, which is not. Both
orderings lost. If the observer arrived first the guard returned early. If
checkStatus() arrived first it suspended on disk IO, the observer flipped
the state underneath it, and the re-check discarded the status it had just
resolved.
Resolves the local artifact either way and decides inside the same state
update, where the active download is visible: the download keeps the
status it owns, and adopts the artifact only when it is carrying none.
Metadata is still skipped while work is active, so this costs no extra API
budget.
Verified on a Pixel 9a by starting a download, force-stopping mid-transfer
and reopening: the row holds "App Ready for Offline Sharing" with both
sharing rows present, where it previously showed neither.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GitHub sends X-RateLimit-Reset on every REST response, an ordinary 403
included, and it always points at the current window. Feeding it through
retryAtMillis() therefore produced a non-null deadline for any 403, and
the classifier accepted that as proof of a limit. A permissions failure
with the quota untouched came back as RateLimited, so the caller persisted
a cooldown on that route and served stale metadata until a reset window
the failure had nothing to do with.
Classification now looks only at signals that actually mean this request
was the one refused: a spent quota, or an explicit Retry-After. Nothing
real is lost, because GitHub marks a primary limit with
X-RateLimit-Remaining: 0 and a secondary limit with Retry-After. The reset
header keeps its job of supplying the deadline once a limit is
established some other way.
ApkDownloadSourceTest already claimed this contract - its name is "403 is
only treated as a limit when response headers say so" - but its
permissions case passed no reset header at all, which is the one input
that hides the bug. Adds the case it was missing, which fails without this
change, and pins the secondary-limit path so tightening the reset header
cannot blind the client to a Retry-After.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The cooldown was tracked in two places. ApkRateLimitStore persisted it per
scope and per route, and the ViewModel kept a second copy in
downloadRetryAtMillis plus a retryAtMillis on Resumable and Error, frozen
into WorkManager output data on the way through. The copy was the weaker
of the two: it lived in memory, so a cold start lost it, and the deadline
it froze belonged to whichever route earned it, which is exactly the drift
the per-route store exists to prevent.
It also could not expire on its own. downloadRetryBlocked was computed
with System.currentTimeMillis() during composition, so nothing recomposed
when the deadline passed; scheduleRetryUnlock papered over that with a
viewModelScope delay that died with the process. Meanwhile the disabled
row and icon gave the user no countdown to read, so a tap simply did
nothing.
Drops the copy. The store is consulted where the request is actually made
and the UI stays enabled, which costs a worker that fails in well under a
tenth of a second without touching the network.
RateLimitedWithWait goes with it. Its "try again in %2$s min" was computed
at failure time and baked into static text that never ticked down, so it
was wrong within a minute; RateLimited says "try again later" and stays
true. Removing it leaves nothing pre-formatted, so Resumable and Error now
carry an ApkFailureMessage of string id plus arguments and the row
resolves it during composition. Failure text follows the device locale
rather than the locale the worker happened to run under.
Anchors the GitHub cooldown at the moment it is judged. now was sampled
before awaitRoute(), which can hold a request for the full 60-second route
timeout, so a relative Retry-After interpreted against it could land in
the past and let the very next check reach GitHub - the loop this branch
set out to close. Reads the clock again once the route is ready and once
the response arrives, and uses each where it applies.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
routedHttpClient() and webSocketClient() both did a plain check-then-set
on their AtomicReference: read, and if empty build a client and store it.
Two threads arriving together each saw an empty reference, each built a
full OkHttpClient, and the loser's client was dropped on the floor with
its connection pool and dispatcher threads already allocated. Nothing
closed it, so the leak lasted until the process died.
Moves construction inside a lock and re-checks the reference there, so
the second thread returns the first thread's client rather than building
its own. reset() takes the same lock, which is what makes the pairing
airtight: a build can no longer interleave with a reset and store a
client for the route that was just discarded. The fast path stays outside
the lock, so a warm client still costs a single volatile read.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The alpha bump was reverted in 2c19d00d, but its verification metadata
stayed behind: 45 components for compose 1.12.0-beta01 and material3
1.5.0-alpha25 that no lockfile resolves. Regenerated from upstream's file
so only artifacts this branch actually pulls are trusted.
Trusting artifacts nothing resolves is the opposite of what this file is
for, and it made the branch look like a Compose beta upgrade in review.
gradle/verification-metadata.xml: +322 lines -> +8, one component
(mockwebserver3, the dependency 2c19d00d genuinely added).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI failed at ':wear:compileDebugUnitTestKotlin' with okhttp, okio and
mockwebserver3 "not part of the dependency lock state". No test ran.
Adding okhttp-mockwebserver to the shared test bundle put okhttp and okio
on :wear's unit-test classpath as well, but only :app's lock state was
regenerated, so :wear/gradle.lockfile had no entry for any of them.
Regenerated lock state and verification metadata for debug and both
release variants per docs/reproducible-builds.md. No new components
needed trusting: the checksums already existed from the :app side, so
this is lockfile scope only.
Worth noting the local command that missed it was :app-scoped;
CI runs testDebugUnitTest at the root, which includes :wear.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Self and grouped voice-note bubbles had no long-press target: the sender
label is hidden and VoiceNotePlayer controls consume touches, so the
message action sheet was unreachable. combinedClickable on the shell
restores long-press for every media bubble.
