Opening an uncached message stalled ~35-74s (avg 48s) behind the reader
spinner because the on-demand IMAP body fetch has no priority over the
continuous full-history backfill (#12) and loses the race for the
account's IMAP throughput (connect-per-op client, no shared lock).
Introduce InteractiveImapGate (@Singleton), a process-wide priority
signal mirroring MailMaintenanceGate/AccountThrottleGate (#360):
- MailRepositoryImpl wraps the user-facing IMAP paths (openMessage,
inlineImages, downloadAttachment, buildReplyDraft) in withInteractive
{}, which raises an in-flight counter for the duration and always
lowers it in a finally, so a failed fetch can never strand it.
- MailBackfiller parks (awaitInteractiveIdle) at its per-page yield
point while the counter is non-zero, resuming the instant it clears.
This is also the slice's first yield point, so a slice never begins a
page while a user is waiting on a body.
- Backfill's own content prefetch bypasses the gate (calls
ensureAttachmentFile directly) so it never yields to itself.
A counter, not a mutex, is used so overlapping interactive fetches run
concurrently and backfill waits for all to clear. PII-free AppLog
park/resume breadcrumbs (accountLogRef) at the backfill yield points.
Builds on #360's throttle framework (orthogonal: that backs off after a
provider rejects background work; this yields to a foreground fetch).
Tests: InteractiveImapGateTest (counter/park/resume/error-release/
concurrency + Turbine), MailBackfillerTest park+resume+no-deadlock,
MailRepositoryImplTest gate-held-during-open, and
InteractiveImapGateInstrumentedTest (on-device pause/resume across the
CI API matrix).
Closes#355
Compose BOM 2026.06.00 deprecates the v1 test-rule factories in
androidx.compose.ui.test.junit4 in favour of the ...junit4.v2 package
(v2 composes on StandardTestDispatcher instead of UnconfinedTestDispatcher).
Swap the import in all 30 androidTest classes from
androidx.compose.ui.test.junit4.createAndroidComposeRule to
androidx.compose.ui.test.junit4.v2.createAndroidComposeRule.
v2's createAndroidComposeRule<A>() returns the same
AndroidComposeTestRule type, so the call sites are unchanged. The tests
already wait on async state via waitUntil/waitForIdle rather than
assuming eager effect execution, so no test semantics needed adjusting
for the StandardTestDispatcher change. The JVM (test) source set already
used the v2 createComposeRule from PR #375.
Closes#385
The matrix `e2e` job (API 29-36) relied on reactivecircus/android-emulator-
runner default boot, whose un-guarded, fatal `adb shell input keyevent 82`
races system_server binder republish on snapshot resume ("No service published
for: input") -- an intermittent boot race that flaked the merge queue and hit
BOTH the run and its retry once #446 let runs finally reach boot on API 33.
Replace the android-emulator-runner boot (snapshot-generate + run + retry steps
plus the AVD snapshot cache) with the e2e-preview job proven hand-provisioned
manual boot:
- avdmanager creates the AVD (google_apis/x86_64, pixel_2 -- kept in lockstep
with testOptions.managedDevices in app/build.gradle.kts);
- a 2-attempt COLD boot loop (-no-snapshot), each with ONE bounded
`timeout 300 adb wait-for-device shell wait-for-sys.boot_completed` so a stuck
emulator fails fast instead of hanging to the 50-min job cap;
- a NON-FATAL `adb shell input keyevent 82 || true` unlock (kills the race) plus
a boot_completed readiness gate before connectedDebugAndroidTest;
- emulator flags mirror e2e-preview; gradle retries once on a test failure.
Cold boot drops the AVD snapshot cache (snapshot resume is the documented root
cause of the race); the shared android-sdk-v1 cache and #446 hardened
pre-install step are untouched. No #404 wedge-capture wrapper (it was reverted
from this matrix job in 90dfb18 for hanging all 8 legs). Streamed logcat +
emulator.log + boot-diagnostics are uploaded for parity diagnosability.
Closes#448
The Gradle wrapper had no distributionSha256Sum, so the Gradle 9.6.0
distribution was the one unpinned download in CI (validateDistributionUrl
only checks the URL shape, not content). Pin it to Gradle's published
SHA-256 so the wrapper fails closed on any corrupt/tampered distribution,
matching the SHA-256 integrity pin cmdline-tools already gets (#389).
The E2E (33) matrix leg deadlocked the merge queue for ~2h when
android-emulator-runner's un-guarded "Create AVD and generate snapshot" step
died with "Error on ZipFile unknown archive" installing a corrupt Android
Emulator SDK zip. #389 hardened the platform/build-tools install (SHA-verify ->
reject-corrupt -> purge -> re-download) but left the emulator + system-image
install to the action, un-guarded.
Pre-install "emulator" + "system-images;android-<api>;google_apis;x86_64"
through setup_android_sdk.py before the emulator-runner steps, so a corrupt zip
is self-healed here and the action then finds both packages already installed
and skips its fragile fetch. Runs on both AVD-cache hit and miss (the emulator
binary + image live under the SDK root, not the ~/.android AVD-snapshot cache,
so they must be present for even a cached AVD to boot). Not added to the
android-sdk-v1 cache (kept small); re-install is a fast sdkmanager no-op when
already present.
The e2e-preview (API 37) job already routes emulator + system image through the
hardened installer, so no change there. setup_android_sdk.py already handles
these package ids generically; add a unit assertion pinning the matrix's
google_apis/x86_64 id to the correct purge path.
Closes#443
Show a lightweight, dependency-free looping illustration of the
"tap Battery -> choose Unrestricted" path on BatteryOptimizationScreen,
before the user is sent to system settings (complements the #150 deep
link, which cannot guarantee the exact per-OEM screen).
- BatteryGuideAnimation: a stylized Compose illustration driven by
rememberInfiniteTransition (no Lottie / AnimatedVectorDrawable, no new
dependency). A highlight moves from a "Battery" row to an
"Unrestricted" option while a tap dot pulses.
- Reduced motion: rememberReducedMotion() reads ANIMATOR_DURATION_SCALE;
when animations are off it renders the same card at rest (no motion).
- TalkBack: the illustration exposes a single contentDescription
mirroring the retained onboarding_battery_guidance text (additive, not
a replacement). Screen made scrollable so the actions stay reachable.
- PII-free AppLog breadcrumbs: shown / opening-settings / skipped.
- Robolectric JVM tests (animated + static + reduced-motion decision)
and the instrumented step test exercise the guide; the step test
disables device animations so the static path renders on-device.
Addresses the below-cut data-core review nits from #313:
- SignatureRepository.delete: wrap delete + default-promotion in one SignatureDao
@Transaction (deletePromotingDefault) so a crash between them can't leave an
account with signatures but no default; log the promotion (PII-free).
- SignatureRepository.create: move the count-then-default check-then-act into a
SignatureDao @Transaction (insertMakingFirstDefault) so two concurrent
first-creates can't both become default.
- AccountSettingsRepository.update: route the read-modify-write through an
AccountSettingsDao @Transaction (readModifyWrite) so concurrent per-field
setters can't clobber each other.
- MailRepositoryImpl expunge/move/move-by-role: chunk the unbounded
getRoutingByIds/deleteByIds IN(:ids) queries (500/chunk) like MailPruner,
removing the latent SQLITE_MAX_VARIABLE_NUMBER crash.
- MessageDao.observeSummaries: remove the dead whole-table projection (superseded
by Paging #124/#214); migrate test/debug-probe callers to getById or the paged
query (which now guards the #51 CursorWindow regression).
- AccountDataMigrator: fix stale KDoc (schema is v2 with sortOrder, not v1).
- DatabaseEncryption.migrate: also sweep the stale -journal sidecar (journal_mode
= DELETE), matching AccountDataMigrator's sweep.
Unit tests updated for the repository delegations; instrumented DAO tests cover
the new @Transaction behaviour; MailRepositoryImplTest covers the chunk split;
DatabaseEncryptionTest covers the -journal sweep.
Closes#313
Six of the seven LOW findings collected in #308; the seventh is
deliberately skipped (see below).
- SettingsViewModel: run the app-lock disable-path Keystore/DataStore
reseal off the main dispatcher (withContext(Dispatchers.Default)),
matching AppLockViewModel's threading policy - no Keystore crypto on
Main.
- AppLockGateHost: clear the covered app content out of the semantics
tree while locked so TalkBack can't traverse the occluded mailbox/
compose nodes behind the opaque cover; content stays composed so its
state still survives a re-lock.
- ReaderViewModel.toggleStar: reconcile the optimistic star on a failed
persist - roll it back and surface a one-shot StarFailed event instead
of leaving the star stuck in a state the store rejected.
- OnboardingViewModel: persist firstAddedAccountId in SavedStateHandle so
a process kill mid-onboarding still finishes onto the first account's
inbox rather than the unfiltered mailbox (preserves #30).
- RichTextEditor: memoize the formatting toolbar's parse + selection
scans with remember(value) so the per-keystroke hot path isn't
re-derived on every recomposition.
- AccountSetupViewModel.onOutlookResult: treat a normal OAuth cancel
(null result) as a no-op instead of surfacing an error snackbar.
Skipped: MailboxViewModel per-keystroke search re-paging - the finding
is documented-intentional and only a "could". The local pager narrows
cached results instantly as you type while the expensive server search
is already debounced (400ms); debouncing the local pager would add lag
for no clear win, and correct scoping (query only, not account/folder)
adds risk to a hot, well-tested path.
Each behavioral change ships a JVM/Robolectric test; the toolbar
memoization (a pure refactor) adds a toolbarStateOf test. PII-free
AppLog breadcrumbs added on the new fallback/state-change paths.
Closes#308
Adds the reactive throttle layer for #360: when a provider rate-limits or
locks us (IMAP `[THROTTLED]`/"too many connections" NO, HTTP 429, auth
lockout), the app now backs off exponentially and pauses the offending
activity instead of hammering the server (which the perf drilldown proved
makes throttling worse — `docs/perf/issue-125-*`).
- ThrottleClassifier: message-text (IMAP/SMTP) + HTTP-status classification of
throttle vs lockout, distinct from ordinary transient errors; conservative
so a wrong password or the #390 "IMAP disabled" state is never misread.
- ThrottleBackoff: pure exponential schedule with equal jitter, a bounded cap,
and Retry-After honoring; a longer window for a lockout (sized to Yahoo's ~1h).
- AccountThrottleGate: per-account backoff state (@Singleton), so one throttled
account never stalls the others; PII-free AppLog breadcrumbs on throttle/clear.
- MailBackfiller: skips an account inside its backoff window and stops paging one
that throttles mid-slice (a degradation, not a moreWork spin) — resumes on a
later slice once the window elapses. Reset on the next successful page.
- MailSyncer: foreground sync feeds the gate but is never blocked by it, so
interactive work keeps priority over background backfill.
Integrates with the existing WorkManager retry (#403) and connection reuse
(#357) rather than duplicating them. Unit tests cover classification (positive
+ negative), the backoff schedule, and the degrade-not-tight-loop behaviour
(virtual time for the timing).
Closes#360
Address the Phase-3 review nits collected in #298:
- perf(HtmlToText): hoist the 4 per-call Regex literals in convert() to
private vals so each compiles once, not once per fetched HTML body.
- fix(ReportStore): write reports via temp-file + atomic rename so a crash
mid-write can't truncate a .json that scan() then silently drops. Temp uses
a non-.json suffix so it is never scanned.
- fix(RichTextEditing): applyLink now splits partially-overlapping links
(keeping the non-overlapping remainder) instead of un-linking it whole,
mirroring subtractRange.
- perf(GraphSender): guard attachment size before readBytes() so an oversized
file can't OOM or blow Graph sendMail's ~4 MB request cap; fails
mayHaveSent=false so the outbox falls back to SMTP (which streams).
- perf(RichText): mergeSameValueSpans is O(n) via a last-run-per-style map
instead of O(n^2) indexOfLast; output is identical.
- fix(DiagnosticsCollector): bucket provider labels by DNS-label boundary, not
raw substring, so mail.notgmail.example no longer reads as Gmail.
Adds/updates unit tests for each behavioural change; pure-perf nits keep their
existing green coverage plus a direct mergeSameValueSpans equivalence test.
MainActivity is exported (launcher / mailto: / share), so although the
per-message ACTION_OPEN_MESSAGE intent is explicit and carries no manifest
intent-filter, any app could still craft an explicit intent at the exported
component and drive the reader to an arbitrary cached message id (#307,
domain/platform review nit 1).
Trust only this app's own notification taps: openMessage now attaches an
unforgeable sender-token PendingIntent (its creator package is stamped by the
system and cannot be forged), and messageId yields the id only when that token
was minted by us. A foreign caller carries no token, or one attributed to its
own package, so its intent is ignored and logged PII-free via AppLog.
NotificationIntentsTest gains a case proving a token-less ACTION_OPEN_MESSAGE
intent is rejected while the genuine one still resolves; existing cases move to
the new messageId(context, intent) signature.
When account setup obtains a valid credential but the IMAP AUTHENTICATE step is
rejected because IMAP access is switched off for the mailbox, surface an
actionable "turn on IMAP" dialog (with the provider's enable-IMAP help link)
instead of the opaque generic auth error (#390).
- ImapAuthError.isImapDisabled classifies the failure on two signals: explicit
provider "IMAP is disabled/not enabled" server text (e.g. Gmail's "not enabled
for IMAP use"), and -- for the Outlook XOAUTH2 path -- a valid-token
AUTHENTICATE rejection, which outlook.office365.com reports only as a generic
"AUTHENTICATE failed". Ordinary wrong-password / expired-token / network errors
are deliberately not matched, so they keep the generic error.
- imapDisabledPromptFor resolves a provider-aware prompt (brand via
MailProvider.brandFor; Outlook + Gmail enable-IMAP help URLs, generic
otherwise).
- Shared ImapDisabledDialog reused by the Outlook picker, the app-password form,
and manual setup -- the three points where the auth failure surfaces. The
reactive complement to the pre-auth Outlook notice (#411/#426).
- PII-free AppLog breadcrumbs at the classification/prompt points (accountLogRef
only; never the email/host/token).
Tests: ImapAuthErrorTest (provider text + OAuth inference + a real GreenMail
wrong-password negative), ImapDisabledPromptTest (brand/URL resolution),
ImapDisabledDialogJvmTest (Robolectric), per-view-model + per-screen wiring
tests, and an instrumented AppPasswordSetupScreenTest case driving the failure
end to end.
Closes#390
persistBatch refreshed each pre-existing backfilled header with a per-row
updateHeaderContent in its own implicit transaction — the same N-commits-per-page
anti-pattern #310 fixed in MailSyncer. Route the whole pre-existing subset through
the batched MessageDao.updateHeaderContents(List) @Transaction so a page costs one
commit instead of one fsync per message (amplified on the encrypted cache).
Semantics are unchanged: updateHeaderContents applies updateHeaderContent to each
row in list order, so the same rows get the same values (and the same casefold
columns); the isNotEmpty guard still skips an empty refresh batch; brand-new rows
stay insert-only. No schema change.
Adds a PII-free, counts-only AppLog breadcrumb at the persist point.
Tests:
- MailBackfillerTest: the refresh routes through the batched update and never the
per-row one; a partial page refreshes only its pre-existing subset in one batched
call; an all-new page skips the batch entirely (empty boundary); breadcrumb counts.
- MessageDaoTest (real Room): the batch writes byte-for-byte the same row as the
per-row path; an empty batch is a no-op.
Closes#322
The targeted UID EXPUNGE (IMAPFolder.expunge(Message[])) from #295/#318 throws
"UID EXPUNGE not supported" on a server without the UIDPLUS extension, which
broke delete/move entirely on rare self-hosted/legacy IMAP servers (#319).
expungeTargeted now probes UIDPLUS from the folder's own already-open protocol
(via IMAPFolder.doCommand, so it never opens a second connection + LOGIN and
keeps the one-connection-per-batch invariant of #125/#295). With UIDPLUS it
still uses the targeted UID EXPUNGE. Without it, it falls back to a plain,
untargeted EXPUNGE only when provably safe: the messages we just flagged are the
only \Deleted ones in the folder. When unrelated \Deleted mail is present a plain
EXPUNGE would destroy it, and there is no UIDPLUS-free way to expunge a single
UID, so we refuse and fail loud, preserving the #295 "never touch unrelated
\Deleted mail" invariant.
PII-free AppLog breadcrumbs record the fallback decision. Covered by GreenMail
unit tests for both branches (with UIDPLUS via the default probe; without via an
injected capability seam, since GreenMail always advertises UIDPLUS).
Closes#319
At account-add both LibreMailApplication's push collector and
IdleService.reconcileWatchers react to the accounts table. The account row was
inserted before its credential was saved, so a watcher could observe the new
account and call MailConnectionFactory.resolveSecret before the secret existed,
logging "No stored credentials" on the first IDLE attempt (it self-healed on
retry, but fired a failed IDLE + log noise on every add).
Primary fix: reorder the writes so the credential is committed before the account
row (the credentials table has no FK to accounts; account_settings does, so its
ensureDefaults still follows the insert). Any reactive observer of the account row
is then guaranteed to see the credential.
Defense-in-depth: resolveSecret now throws a typed MissingCredentialsException and
the IDLE watcher treats it as a transient miss, deferring quietly (short flat
re-check, PII-free info log) instead of the warn + exponential backoff a real
connection drop gets. Genuinely-absent credentials keep deferring without noise.
Tests: AccountRepositoryImplTest pins the credential-before-row order
(coVerifyOrder); MailConnectionFactoryTest asserts the typed exception; a new
instrumented test drives the real repository add path against a real
AccountDatabase + Keystore-backed CredentialStore and proves the secret is
resolvable the instant the account row becomes observable.
New personal outlook.com accounts ship with IMAP OFF by default, so
Microsoft OAuth succeeds while the later IMAP AUTHENTICATE step fails — a
confusing dead-end (#390 handles this reactively). This adds a proactive
interstitial shown when the user taps Outlook during onboarding, before
the OAuth browser launches.
The screen asks whether IMAP is enabled (with a short why-we-need-it
explanation), links Microsoft's canonical "POP, IMAP, and SMTP settings
for Outlook.com" help article and the Outlook.com IMAP settings page
(opened via UriHandler/Custom Tab), and puts a "Sign in" button at the
bottom that continues the existing Outlook OAuth flow unchanged.
- New OutlookImapNoticeScreen (onboarding package) reusing the picker's
exact AppAuth launch wiring via AccountSetupViewModel.
- AccountPickerScreen gains an optional onPickOutlook callback: onboarding
routes the Outlook tap to the notice; the standalone "Add account" entry
still launches auth inline (unchanged).
- New ONBOARDING_OUTLOOK_IMAP route; onboarding setup-form destinations
extracted into onboardingSetupDestinations() for readability.
- PII-free AppLog breadcrumbs: notice shown, help/settings link tapped,
sign-in continued.
- Robolectric JVM Compose test (render, both help links, sign-in
continuation, done/error/busy states) + instrumented E2E (Espresso-
Intents for the help ACTION_VIEW and the AppAuth sign-in launch) +
OnboardingFlowTest coverage of picker -> notice navigation.
Closes#411
Preserve the in-flight #359 crash-loop fix recovered from an orphaned agent
worktree (host crashed before it committed). Widens the fail-closed
LinkageError handling to the keyed opens that previously escaped it
(AccountDataMigrator, deferred Room open, headless workers/IdleService via a
new EncryptedCacheWorkerGuard), plus unit + instrumented regression tests.
Not yet validated end-to-end; gate + E2E run to follow.
mergify[bot]
JMR-dev