If SQLite clearAll fails mid-panic, in-memory state was already cleared
but a process restart could reload encrypted history from disk (#699).
Fall back to deleting the database files before reporting failure.
A relay that fails is retried on an exponential backoff, then abandoned
for the lifetime of the process. Nothing brings it back: the relay layer
registers no connectivity callback, the periodic subscription validator
only repairs subscriptions on sockets that are already open (and returns
immediately when connectedRelayCount is 0, which is exactly the state
after an outage), and connect() runs once from NostrClient.initialize().
The remaining paths that reset reconnectAttempts are a manual retry, a
Tor state change, and a successful open.
Two ways a relay died permanently:
- Any error whose message mentioned DNS returned before scheduling
anything at all. "Unable to resolve host" is what this device reports
when it simply has no network, so a moment in a tunnel killed every
relay at once, with no retry ever.
- Otherwise the schedule stopped at MAX_RECONNECT_ATTEMPTS. With
INITIAL=1s and MULTIPLIER=2 that is nine waits totalling about eight
and a half minutes, after which the relay was dead. MAX_BACKOFF_INTERVAL
was unreachable: attempt 9 asks for 256s and attempt 10 gave up, so the
five-minute ceiling the constant defines never applied to anything.
Let the backoff saturate at MAX_BACKOFF_INTERVAL and keep retrying there.
A name-resolution failure now backs off like any other error. Steady state
costs one connection attempt per relay per five minutes; the previous
behaviour cost the user every internet DM, delivery receipt and geohash
channel until they noticed and restarted the app.
The schedule moves into RelayReconnectPolicy so it is unit-testable
without OkHttp or a Context.
The collision case fails on main: two packets sharing a 64-byte prefix and
a timestamp, where the second was silently dropped.
The other two pass before and after on purpose. Replay of an identical
packet must still be caught, and the same packet arriving from two
different peers must still be tracked separately — strengthening the
identity must not quietly weaken either.
Replay and duplicate detection keyed on a 32-bit contentHashCode over at
most the first 64 bytes of the payload. Two packets from the same peer in
the same millisecond that agreed on that prefix were the same packet as
far as this cache was concerned, and a collision here is a dropped
message: the second is discarded and nothing reports it.
PacketIdUtil is the identity the rest of the stack already uses for this
question — gossip sync membership, and the message IDs MessageHandler
assigns — and iOS derives it identically: first 16 bytes of SHA-256 over
type, senderID, timestamp and the whole payload. The security path now
agrees with the sync path instead of carrying a weaker private notion of
"same packet", and the FRAGMENT special case disappears because the full
payload is covered either way.
Peer scoping is deliberately kept. PacketIdUtil covers the packet's own
senderID, which is not the peer it arrived from once relayed.
isDuplicate() incremented totalChecks outside the lruLock that guards
every other mutation on this class, so concurrent callers raced on the
read-modify-write and lost increments. duplicateCount and evictionCount
were already incremented under the lock; totalChecks was the one
counter left outside it.
getStats().totalChecks feeds hitRate and is the only denominator for
duplicateCount, so a systematic undercount skews both. Moved the
increment inside the existing synchronized block.
Added a concurrency test that reproduces the race: run against the
prior code it failed reliably (3/3 runs); against the fix it passes.
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>
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.
