Codex is right about this one. Cancellation in Kotlin is cooperative, and
everything from validateDownloadedApk() through saveMetadata() is plain
blocking code with no suspension point. Stopping during the signature
check was therefore not observed until after the temp file had been
renamed and its metadata written, so the worker committed the APK while
WorkManager reported the work cancelled.
That also raced onCancelDownload(): its checkStatus() could read the cache
before the commit and settle on NotDownloaded, after which the cancelled
work maps to Idle and the observer ignores it. The row then advertised
"Not ready" with a verified universal APK already in the cache, and
tapping it downloaded the same bytes again.
One checkpoint after validation, which is the slow step and so the most
likely moment to press Stop. The verified temp file is left in place, so
the next attempt resumes rather than starting over.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codex caught this and it is correct. WorkManager returns a retried
request to ENQUEUED for the duration of its backoff whether or not the
device is online, and the mapping sent every ENQUEUED record to
AwaitingConnectivity. With exponential backoff from 15s over three
attempts, a fully connected device claimed "Waiting for network…" in both
the row and the notification for roughly 45 seconds.
ENQUEUED covers two different waits and the state alone cannot separate
them; a non-zero runAttemptCount means the work already ran, so it is the
backoff. Adds a Retrying phase for that case, extracted as queuedPhase()
so the distinction is testable without a WorkInfo.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codex flagged this on the string-extraction change and it is right.
WorkManager keeps failed records in its own database across app updates,
and AAPT2 reassigns R.string ids on every build. A failure written by one
build and read by the next would resolve its persisted int against a
different resource table: wrong string, or NotFoundException, or
IllegalFormatException when the placeholder arity no longer matches. The
existing zero-check only caught an absent key, not a stale valid one.
ApkDownloadFailureReason now names each failure and owns its string, and
the boundary carries the enum name. This is the same treatment
DownloadPhase.fromKey already gives the phase across the same boundary,
including tolerating a name this build no longer has.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Progress moves out of the trailing slot and under the subtitle, so every
status now shows exactly one 48dp control there instead of a spinner and
a button competing for the same space. The trailing slot had three
different widths across states, which made the text column re-wrap on
every status change; it is now one width throughout.
"Get universal" and "Retry" become icon buttons. That removes the labels
they were leaning on, so both gain tooltips and real content
descriptions, and prepareRowTapAction() now drives the row's enabled flag
and its tap handler from one mapping. Previously the row rendered as
clickable in the ready state but onPrepareRowClicked ignored it, leaving
the icon as the only way to reach the universal download.
Resumable downloads get a progress bar for the first time, drawn flat via
amplitude 0 so a stalled download does not look like a live one.
Strings: every user-facing literal now lives in strings.xml. Download
failures were assembled as English sentences in the util layer, which has
no Context by design, so they crossed the WorkManager boundary already
formatted and could never be translated. ApkDownloadException now carries
a string resource and its arguments, and the ViewModel resolves them
against the device locale. util/ stays Context-free and its tests stay
plain JUnit.
Also drops translatable="false" from seven strings that were visible
prose, and stops showing raw exception text when the APK status cannot be
read.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The expressive wavy progress indicators are Compose-only in the 1.5.0
line, which has no stable release yet, so this overrides the BOM's 1.4.0
for that one artifact.
The override sits outside the BOM, so material3's own requirements win
and pull ui, runtime, foundation and animation from 1.11.4 to
1.12.0-beta01. That is the real cost of this change and the reason the
lock diff is 46 components rather than one.
Lock state and verification metadata regenerated for debug and both
release variants per docs/reproducible-builds.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses review on #812.
Each retry resampled the route but never rechecked the gate against it,
so a route change during a request or its backoff walked straight past a
cooldown. A Tor attempt failing with a 500, then the user switching to a
direct connection that is already rate limited, and the next attempt
contacts it regardless.
The check moves inside the loop, immediately after the route is sampled,
which makes it cover the first attempt too -- the separate post-wait
check it replaces was only ever that first iteration. The check before
the route wait stays: knowing the selected route is blocked is worth
avoiding a sixty-second Tor bootstrap for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses review on #812. Both findings are the same mistake: the route
was sampled at a moment that need not match the request it governs.
The gate was checked once, before a route wait that can last a minute.
Start with Tor selected, disable it during the wait, and the request
goes direct having consulted only the Tor deadline -- contacting a
direct IP whose own cooldown is still running. The gate is now
re-evaluated after the wait, when the route the request will take is
finally known.
The cooldown was likewise recorded against the mode selected when the
response arrived, not the one the call was made on. Changing the setting
mid-flight filed it in the wrong bucket, freeing the limited route and
suppressing the newly selected one. The route is now sampled
immediately before each attempt and reused for that attempt's response
and its retry decision.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses review on #812.
Scoping the gate to the route was right, but it was implemented as one
deadline that moved with the route, so a switch deleted the cooldown
rather than setting it aside. Rate-limited on a Tor exit, switch to
direct, switch back before the reset, and the app contacts that same
limited exit again with nothing left to stop it.
Tor and direct now carry their own deadlines. Switching route selects
the other one rather than forgetting this one, a success clears only the
route that succeeded, and when the route cannot be determined the
stricter of the two applies -- failing to identify a route must not
release a cooldown that is still running.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses review on #812.
isProxyEnabled() reports readiness, not route selection: it is false
while Tor is bootstrapping or restarting, even though requests will
still go through Tor. Using it as the route identity cleared a
Tor-earned gate mid-bootstrap and applied a direct-earned one to the
first Tor request -- the opposite of what scoping the gate was for. The
identity is now the selected mode from statusFlow.
The force flag turned out to be both unnecessary and harmful. Leaving
Downloading synchronously before the check already clears the entry
guard, so force only reached the completion guard -- which must stay
armed. A cancellation check can take a minute on the route timeout, and
WorkManager can surface Resumable meanwhile, so the user may start a new
download before it returns; force let the stale result overwrite work
that was running and strip the progress and stop controls. Removing it
restores that protection and needs no generation counter.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses review on #812.
Cancelling left the spinner up for as long as the status check took.
Forcing checkStatus() past its guard was not enough: resolveApkStatus()
calls checkForUpdate(), which reaches the network and can sit on the
60-second route timeout while Tor bootstraps. Nothing clears the state
in the meantime -- the cancelled job maps to Idle, which the observer
ignores -- so the stop button looked broken for the whole wait. The
state now leaves Downloading immediately and the check resolves it
afterwards.
The rate-limit gate was process-wide. GitHub counts unauthenticated
requests per IP, so a cooldown earned through a shared Tor exit was
being applied to a direct connection with an entirely different quota,
and vice versa -- potentially suppressing a usable route for an hour.
The gate now records which route earned it and is dropped when the
current route differs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses review on #812.
Pressing the new stop button before a partial file exists -- while
resolving the release, or waiting for Tor -- left the row disabled and
spinning for the lifetime of the ViewModel. Two guards conspired:
checkStatus() returns early while the state is Downloading, and the
downloader observer deliberately ignores the Idle that WorkManager
reports for a cancelled job. Both exist to stop a running job being
second-guessed from cache contents, and neither anticipated a job that
is no longer running.
checkStatus() takes a force flag, used only by cancellation, and clears
the stale progress along with the status.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
the rate limit that triggers the gate
Addresses review on #812.
The awaiting-route phase was never cleared. Reporting it before the wait
fixed the label on the wait itself, but nothing restored ResolvingRelease
afterwards, so the UI and notification claimed "Waiting for Tor" for the
whole metadata request and its retries -- and in direct mode, where the
wait returns immediately, for a wait that never happened. An
onResolvingRelease callback now fires once the route is ready.
A rate-limit rejection recorded the gate and returned the failure, while
the stale-cache fallback sat at the top of the function and was only
reached on a later call. The first About check therefore reported an
error the user could not act on, and an immediate retry succeeded from
metadata that was already present. The response that sets the gate now
serves the cached release straight away.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses review on #812.
Secondary rate limits were classed as permissions failures. GitHub
serves them as 403 with Retry-After while X-RateLimit-Remaining is
still nonzero, because the primary hourly quota is not what was hit --
so isRateLimited() returned false, blockedUntilMillis was never set, no
stale release was served, and every About sheet open kept contacting
GitHub through exactly the cooldown it had been asked to observe. The
predicate now also admits a 403 carrying a usable Retry-After; one that
cannot be parsed is still a permissions failure.
The phase reported during the Tor wait was the wrong one.
fetchLatestRelease() waits on the selected route itself, so the UI read
"Checking latest release..." for the whole bootstrap and only switched
to AwaitingNetworkRoute afterwards, when the second route check returns
immediately -- putting the wrong label on the one wait the phase exists
to explain. Reordering the two calls would have made cache hits wait on
Tor, since the cache returns before the route check, so the fetch now
reports from the inside via onAwaitingNetworkRoute and the caller keeps
its own check for the cache-hit path.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Preparing an APK is a five-stage operation rendered as a single 0-100
bar. Two of those stages run before the first byte -- a GitHub release
lookup, then awaitSelectedNetworkRoute, which blocks on Tor bootstrap --
and neither reported anything, so a download sat at 0% with no
explanation for as long as Tor took. The tail had the mirror problem: a
SHA-256 pass and a signature check over ~100MB, both sitting at 100%.
Carry a DownloadPhase on DownloadState.Downloading, reported from
UniversalApkManager through the worker's existing setProgressAsync and
mapWorkInfoToState. The About sheet names the phase instead of showing a
misleading percentage, and the spinner is indeterminate except while
bytes are actually moving. The notification does the same, and a phase
change forces a redraw so the every-5% threshold cannot suppress it.
The phase crosses a WorkManager Data boundary as a string, so fromKey
falls back to Transferring for an absent or unrecognised value -- work
enqueued by an older build must not crash a newer one.
A stop button is wired to the cancelDownload() that already existed on
the downloader interface but had no UI affordance.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts:
# app/src/main/java/com/bitchat/android/ui/AboutSheet.kt
Opening the About sheet runs a release check, and an exhausted quota fed
itself: only successes were cached, and a 403 reporting zero remaining
was classed retryable, so every sheet open spent three more requests
rediscovering the same limit. Unauthenticated GitHub allows 60 requests
an hour per IP, and over Tor that IP is an exit node shared with every
other user on it, so the ceiling arrives far sooner than per-user maths
suggests.
Three changes:
- Conditional requests. The client now stores the release ETag and
replays it as If-None-Match. GitHub does not charge a 304 against the
rate limit, so revalidating an expired cache is free where an
unconditional refetch costs one of the 60. This is why neither polling
nor long polling is the right answer here.
- A rate-limit gate. X-RateLimit-Reset and Retry-After were read only to
interpolate into an error string; they now set a deadline before which
no request is sent at all. While blocked, a stale cached release is
served in preference to an error the user cannot act on. Clamped to an
hour so a bad header cannot lock the feature out, and a reset time in
the past falls back to a fixed backoff rather than unblocking a skewed
clock immediately.
- Rate limits are no longer retried in-loop. The gate decides when it is
worth asking again. A plain 403 is a permissions failure and is no
longer retried either.
The gate's decision logic is pure and unit tested. The wiring around it
is not: that needs a MockWebServer, which is not currently a dependency.
Known gap: the cache and ETag are in memory only, so a process restart
still costs one request. Persisting them needs a Context threaded into
what is currently a context-free object; left as a follow-up.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
System/Light/Dark and the chat style picker now share the single Theme
card in About -> Settings: theme chips on the first row, chat style
chips on the second, ordered Bubbles then Matrix. Bubbles remains the
default for fresh installs.
The end side and the author-colour wash already attribute own bubbles,
so the @name heading on the first bubble of an own run was redundant.
Received bubbles keep their sender names; matrix mode is unchanged.
The no-break-space reservation narrowed the text's own wrap width, so
first lines wrapped early and carried dead slack. The cluster (timestamp
plus delivery checks) is now placed from the laid-out text: it rides
flush-right in the last line's slack when there is room and drops below
the text only when there is not. The body wraps at the full bubble
width either way, one-liners keep hugging their content, and matrix
mode is unchanged. AnnotatedClickableText gains an optional onTextLayout
callback to support the measurement.
The timestamp + checks cluster now overlays the bubble's bottom-end
corner while the body reserves an invisible no-break-space run (capped
by a zero-width word joiner so it is never trimmed) exactly where the
cluster lands. The reservation rides the last text line when there is
room and wraps only when there isn't, so the cluster sits flush-right
on the last line for every message length: no overlap, no forced new
line, no minimum bubble width. Matrix mode is unchanged.
Replaces the separate bottom meta row, which forced every bubble to at
least the meta row's width and added a line even to one-line messages.
The timestamp + checks cluster now trails the body inline: it rides
the last text line when there is room and wraps only when there isn't,
so bubbles hug their content again. Own private messages keep the
constant-width grey-to-green checks with the colour tween; matrix mode
is unchanged.
Bubbles now park a bottom meta row at their end edge, like classic
messengers: the timestamp sits right-aligned, followed at a fixed gap
by the delivery checks for own private messages, instead of trailing
the body text mid-line. Matrix mode keeps its inline trailing
timestamps.
Instead of trailing the timestamp mid-line, the delivery checks now
park at the bubble's bottom-end corner like classic messengers, with
the body text reserving a small end inset so the last line never
collides with them. The checks keep their constant-width grey-to-green
behaviour, colour tween, and scale pop (shared DeliveryStatusIcon).
Matrix mode is unchanged.
Delivery/read markers previously swapped glyphs as acknowledgements
arrived, reflowing the text around them. Both checks now render from
the start in a disabled grey and simply recolour as the state advances
(delivered lights the first check, read lights both), so nothing ever
pushes message text around. Read receipts use the app's primary green
instead of the blue accent, a quick colour tween lights the checks up,
and the standalone marker adds a snappy scale pop when the state
advances. Applies to both matrix and bubbles modes.
Bubbles mode now reads like a classic messenger thread:
- the sender's name heads the first bubble of each run instead of
floating above it; continuation bubbles skip it
- the delivery/read marker for own private messages trails the
timestamp inside the bubble (same glyph mapping as the standalone
marker); media rows keep the beneath-card marker since they have no
inline text
- display names and their #abcd disambiguation suffix are now
separated by a thin space (U+2009) in both matrix and bubbles modes
In bubbles mode, self-authored image, voice-note, and file rows now
align to the end side with the same tail-corner cue as text bubbles,
instead of sitting on the received side where they read as someone
else's content. Grouped self voice notes also keep the run on the
correct side. VoiceNotePlayer gains an optional modifier so the player
can hug the end side at a capped width. Received media and matrix mode
are unchanged.
When a slash command is sent without tapping the autocomplete list, the
command popup stays on screen with its old suggestions. The send path
clears the input field in code, and the popup is hidden only from the
field's text-change handler, which a code-driven clear does not run, so
the show flag is never turned off. The mention popup has the same cause.
CommandProcessor gains clearSuggestions(), which hides both popups and
empties their lists, the same resets the two select functions already do
inline. ChatScreen's send handler calls it where it already resets the
field.
The private chat sheet called updateMentionSuggestions on every keystroke
while rendering its own popups as hidden, so the only effect was on the
main composer behind it: typing @ in the sheet and then sending or
dismissing left a stale mention popup on the main screen. That call is
removed.
Tests in CommandProcessorTest open each popup and assert clearSuggestions
hides it. The unit tests do not drive Compose, so they cover the function
the send handler calls rather than the handler itself.
Full Android unit suite green; lint and the debug build pass.
Fixes#250.
Add a toggleable chat transcript style alongside the existing matrix
transcript. Bubbles mode renders text messages as classic messenger
bubbles: own messages on the right, peers on the left, each bubble
washed with the author's stable identity-derived peer colour so the
speaker stays identifiable without changing any theme, surface, or
background colours.
- ChatUiMode preference (Matrix / Bubbles, default Bubbles) persisted
via ChatUiModeManager, mirroring ThemePreferenceManager
- BubbleTextMessageLayout: rounded bubble with a subtle tail on the
speaker's side, width capped at 80% so long messages wrap; sender
labels, grouping, timestamps, mentions, links, long-press, and the
existing spring entry/placement animations are unchanged
- Delivery status for own private messages moves beneath the bubble in
bubbles mode so it never overlaps the tail
- Chat style picker in About -> Settings reusing the ThemeChip pattern
Use the About-button pixel speech bubble in app primary green with BIT inside, on a white circular plate so the watch icon matches in-app branding.
Co-authored-by: Cursor <cursoragent@cursor.com>
