The encryptCache setting is written immediately but the on-disk
conversion is deferred to the next cold start, so a transitional window
exists (setting off, DB still encrypted, SEALED_AUTH present) in which
three components answered from the setting and got it wrong (#479):
* AppLockViewModel fed the raw setting into KeyInvalidationPolicy, so
removing the device lock inside the window returned DISABLE_APP_LOCK:
app-lock silently off, no wipe, SEALED_AUTH orphaned - and the next
cold start hung forever in DatabaseKeyStore.resolvePassphrase
(session.await() nothing could complete), bricking the app behind the
CacheEncryptionGate until a data clear. onForeground now derives the
policy input as `encryptCache || hasAuthSealedPassphrase()` (the same
gate-on-the-seal fix SettingsViewModel.setAppLock already carries), so
the window routes to CLEAR_AND_DISABLE (wipe scheduled, restart) and
DISABLE_APP_LOCK is only reachable seal-free. The policy parameter is
renamed to `encryptedCacheProtected` to make the contract explicit.
* EncryptedCacheGuard.isCacheLocked() derived "locked" from the settings
pair, wrong in both transitional states: workers parked forever inside
provideDatabase when the setting was off but the DB still auth-sealed
(case A), and sync/push/send stalled needlessly while the seal was
still MASTER (case B). It now mirrors resolvePassphrase's blocking
branches - keyed off DatabaseKeyStore.sealState() plus, for the
AUTH-seal-with-setting-off window, a raw header read of the cache file
(still never touching Room).
* DatabaseProvisioner now releases the orphaned auth seal after the
decrypt-on-disable conversion (reseals under the master key,
best-effort), closing the window at its source instead of leaving
SEALED_AUTH to linger indefinitely.
All decision/fallback paths breadcrumb through AppLog (PII-free enums
and booleans only).
Closes#479
Resolves the highest-versioned AccountDatabase schema JSON under assets
instead of hardcoding a version filename, so a schema bump is validated
automatically instead of silently checked against a stale version. Fails
loudly if no schema asset is found.
Closes#477
Root cause: intermittently the launched activity window has has-window-focus=false
for the WHOLE instrumented run, so Espresso's RootViewPicker (onView().check(),
Intents.intended(), pressBack(), focus-dependent clipboard) times out after 10s and
fails EVERY focus-dependent test at once while the ~280 pure-Compose semantics tests
(which don't need window focus) pass. A failing E2E (35) leg's logcat (PR #470, run
28985259521) shows has-window-focus=true ZERO times across the whole session and both
the first attempt and the once-retry fail identically -- a persistent environmental
state, not a per-test transient. The prior mitigation, a single fire-and-forget
`adb shell input keyevent 82` (MENU) right after boot, is too weak: MENU no longer
dismisses the modern (API 30+) keyguard and, delivered before SystemUI/keyguard comes
up, is simply dropped -- so the insecure keyguard / non-interactive display persists
and no app window ever takes focus.
Fix: a single shared helper, .github/scripts/emulator_focus_gate.py, invoked
identically by BOTH E2E jobs (the e2e API 29-36 matrix AND e2e-preview API 37) and by
the local preflight runners (local_instrumented.py / api37_e2e.py), so it cannot drift.
It wakes the display (KEYCODE_WAKEUP), dismisses + disables the keyguard
(wm dismiss-keyguard, locksettings set-disabled true), keeps the screen on
(svc power stayon true + max screen_off_timeout), zeroes the animation scales, then
polls dumpsys power/window until the device is interactive AND a real window holds
input focus (mCurrentFocus non-null) -- re-nudging each iteration -- before the suite
runs. Applied uniformly, this also gives e2e-preview the animation-disable the matrix
already had. The gate is soft (bounded wait, then proceeds with a ::warning:: and the
final device state) and non-fatal (`|| true`), preserving #454's guarantee that the
unlock never aborts the boot; it leaves #454's manual boot, #460's path-filter and
#464's wedge-capture untouched.
The pure readiness parser is unit-tested by test_emulator_focus_gate.py (run by the
traffic-control-tests job). Determinism is validated by this PR's own matrix run.
Closes#468
Adds two iCloud-specific, provider-scoped policies that build on the
existing shared throttling framework (#360's AccountThrottleGate, #356's
BackfillPacer) without refactoring either:
- IcloudConnectionLimiter: a per-account permit gate capping an iCloud
account at 5 simultaneous connections (Apple documents 5-8; pinned to
the conservative low end). Wired into MailBackfiller around both
connection-opening call sites (the header-page fetch and the
body/attachment prefetch loop - the "1 + K + attachments" per-page
connection count issue #363 describes). A no-op passthrough for every
other provider, so it composes cleanly with #360's reactive backoff
(already consulted first, per account) and #356's slice pacing
(BackfillWorker composes BackfillPacer.runPaced around
MailBackfiller.runBackfill, so the cap sits one layer beneath the
pacer's cooldown/cap in the same call graph).
- IcloudSendLimits: enforces Apple's ~20 MB outgoing message-size cap
before SmtpSender ever opens a connection, estimating the actual
encoded wire size (base64 inflates binary attachment bytes by ~4/3)
rather than comparing raw file bytes, mirroring GraphSender's existing
pre-send attachment-size guard. An over-cap send throws
MessageTooLargeException, caught by SendWorker's existing runCatching
and turned into a clean, PII-free outbox error - no crash, no raw
provider rejection.
Both are new, self-contained files kept intentionally separate from a
shared cross-provider table, per the parallel-safety note on this ticket
(sibling issues #361/#362/#364 add their own provider's limits the same
way).
Touches two shared files: MailBackfiller.kt (new constructor dependency
+ two call sites wrapped in icloudConnectionLimiter.withPermit) and
SendWorker.kt (one guard call before smtpSender.send). Both are
minimal, additive edits — flagged for conflict-awareness with the
sibling provider tickets.
Also excludes MailBackfillerTest.kt from detekt's LargeClass rule
(config/detekt/detekt.yml), mirroring the existing MailRepositoryImplTest
exclusion: one cohesive single-SUT suite tipped over the LLOC boundary
by the new connection-cap wiring tests.
Closes#363
Add a throttle-aware Microsoft Graph HTTP layer (org.libremail.mail.graph) and route
the live me/sendMail path through it, composing with #360's AccountThrottleGate:
- GraphHttpClient: the single Graph HTTP transport seam; preserves the send path's
may-have-sent distinction (GraphTransportException) and parses Retry-After.
- GraphThrottle: caps Graph concurrency at 4, honors a 429/503 Retry-After via the
shared per-account backoff gate (retry after the honored wait, bounded), clears it
on a 2xx. Because the gate is shared+account-keyed, a Graph 429 also cools that
account's IMAP background work down.
- GraphBatch: multiplexes ops via $batch (<=20/call), collapsing N calls to
ceil(N/20); feeds per-op 429s back into the gate.
- GraphUploadSession: createUploadSession + chunked Content-Range PUTs for content
over Graph's ~4 MB one-shot ceiling (320 KiB-multiple chunks).
- GraphSender.send now honors a Graph 429 once (Retry-After) before falling back to
SMTP, instead of failing over on the first throttle.
Outlook mail is read over IMAP and the Graph token is Mail.Send-scoped, so $batch
reads and draft-based chunked attachment upload have no live call site yet (they need
the Mail.ReadWrite scope, a re-consent-forcing change kept out of this perf ticket);
both ship as fully-tested capabilities of the Graph layer.
Unit tests (MockK the HTTP client, coroutines-test virtual time, no real sleeps) cover
429+Retry-After backoff, $batch call-count reduction, and chunked upload; an
instrumented test exercises the toolkit under the Android runtime. PII-free AppLog
(accountLogRef) throughout; SPDX on every file.
Closes#364
Add Gmail's documented IMAP caps (15 max simultaneous connections, 2,500 MB/day
download, 500 MB/day upload, 10,000 messages/labels per limit) as provider-scoped
config/policy that feeds the existing #360/#356 pacing machinery instead of
reinventing it:
- GmailSyncLimits: pure constants + `appliesTo(account)` provider detection via the
existing MailProvider.forImapHost lookup (no changes to MailProvider itself).
- GmailBandwidthTracker: a new, per-account/per-day download-byte tracker (mirrors
AccountThrottleGate's shape: ConcurrentHashMap state, injectable clock, PII-free
once-per-crossing AppLog breadcrumb). Proactive and orthogonal to the #360
AccountThrottleGate (which only reacts to a provider-issued throttle) and #356's
BackfillPacer (which paces slice cadence, not bytes) - same relationship
InteractiveImapGate already documents having to AccountThrottleGate.
Wiring: MailRepositoryImpl.prefetchMessage (the single funnel both MailBackfiller
and MailSyncer's background prefetch already share) records bytes actually pulled
over the network for Gmail accounts; MailBackfiller/MailSyncer's prefetchIfEnabled
consult isOverDailyBudget once per account before starting a batch and defer
body/attachment prefetch for the rest of the day once Gmail's budget is reached -
header paging/sync is never gated, and interactive fetches (open, attachment tap,
inline images) are never gated either, matching the existing interactive-priority
principle (#355/#360).
The 15-connection cap is already satisfied by the existing architecture
(ImapConnectionCache keeps one reused connection per account plus one dedicated
IDLE connection - 2 total, well under the cap); GmailSyncLimitsTest pins that
invariant against the documented ceiling so a future change that grows per-account
concurrency trips a test before it could approach Gmail's real limit. The
10k-messages-per-label figure is captured as a documented constant only - it is
deliberately NOT wired into a backfill stop condition, since issue #12's full
history backfill is intentional and a large real mailbox can exceed 10k messages.
AccountThrottleGate, BackfillPacer, ThrottleClassifier/ThrottleSignal/ThrottleBackoff,
InteractiveImapGate, and MailProvider are all untouched.
Closes#361
The outbound-link E2E tests verified the opened page with Espresso-Intents
(intending(ACTION_VIEW).respondWith(...) + intended(...)). intended() runs an
onView(isRoot()).check(...) whose RootViewPicker waits up to 10s for a
window-focused root. On the CI matrix emulator the activity window
intermittently reports has-window-focus=false, so the assertion flakes with
RootViewPicker$RootViewWithoutFocusException, failing the whole E2E leg and
forcing a 9-min retry. The intent stubs were already present and do NOT fix
this: no external activity launches, the focus loss is environmental (the same
run failed 8 unrelated RootViewPicker-based tests at once).
Verify these ACTION_VIEW/browser-open link taps by injecting a recording
LocalUriHandler and asserting the exact URL the screen opens. That keeps the
tests entirely on Compose interactions, which do not depend on window focus
(280+ Compose-only tests passed in the same failing run), so they are
deterministic without weakening the assertion (still asserts the provider
page / host).
Converted (ACTION_VIEW / UriHandler "browser-open" shape):
- AppPasswordSetupScreenTest.tappingCreateAppPasswordPage_launchesBrowserIntentToHelpUrl
- AppPasswordSetupScreenTest.imapDisabledFailure_showsThePrompt_andHelpLinkOpensTheProviderPage
- OutlookImapNoticeScreenTest.tappingImapHelpLink_opensTheMicrosoftArticle
Real-intent tests (hasComponent/Settings action, no UriHandler seam) keep
Espresso-Intents and are out of scope here.
Mirror the e2e-preview job's proven wedge-capture onto the API 29-36 matrix
legs, now that #454 replaced reactivecircus/android-emulator-runner with the
same hand-provisioned manual boot e2e-preview uses.
Each leg now wraps its `./gradlew connectedDebugAndroidTest` in the #404
`timeout -k 30s $WEDGE_TIMEOUT` wrapper; on a hang (exit 124) capture_wedge
dumps the smoking gun (running test via TestRunner logcat, SIGQUIT thread
dumps of the app + instrumentation processes, service list / service check,
dumpsys activity+window, logcat tail) into a per-API-level
wedge-diagnostics-api<level> artifact (if-no-files-found: ignore so healthy
runs stay quiet).
Why it cannot re-hang the legs (the #406/90dfb18 revert reason): the wrapper
wraps ONLY the foreground gradle client, never the backgrounded emulator --
identical in shape to e2e-preview's run_shard. The reverted #404 wrapper wrapped
reactivecircus's emulator boot; #454 removed that. WEDGE_TIMEOUT reuses preview's
1200s: healthy matrix legs run ~8.3-12.0 min (whole job), well under 20 min, which
is itself well under this job's 50-min cap, so a genuine wedge is caught + captured
and a false trip on a healthy run is not possible.
Backfill chained bounded slices back-to-back with no gap, and on a large
mailbox `moreWork` never clears, so a single BackfillWorker run paged
flat-out for its whole session and kept the account's IMAP connection
saturated -- the background load that starves interactive message-opens
(#355) and worsens provider throttling (#360).
Add BackfillPacer, a small in-process primitive that bounds one run:
- inter-slice cooldown: a fixed 30s idle between chained slices;
- per-run slice cap: at most 4 slices per run, then defer to the 30-min
periodic cadence so one run cannot monopolise the account.
Composes with the two sibling mechanisms instead of duplicating them:
- #355 (InteractiveImapGate): the cooldown is SKIPPED while an interactive
fetch is active -- the next slice already parks at its per-page yield
point, so a fixed delay on top would only double the idle (no pathological
double-delay);
- #360 (AccountThrottleGate): a slice whose only outstanding work is a
throttled account returns moreWork=false, so the loop stops and no
cooldown is spent spinning on a backed-off provider.
The cooldown is a cancellable delay and the loop rechecks !isStopped before
each slice, so a WorkManager stop / teardown ends a run promptly. All
breadcrumbs are PII-free (durations/counts only).
Tests: BackfillPacerTest (JVM, virtual time) covers cooldown timing, cap,
cancellation, and the #355/#360 composition; BackfillWorkerTest asserts the
worker caps a flat-out run; BackfillPacerInstrumentedTest proves forward
progress across paced runs, the interactive skip, and prompt cancellation on
the real Android runtime.
Closes#356
The #399 E2E path-filter listed its safe paths as POSITIVE globs under
`predicate-quantifier: 'every'`. dorny/paths-filter's `every` makes the
per-file predicate "matches EVERY pattern", so `skippable` required a single
file to be under app/src/test AND docs AND scripts AND .claude AND be *.md at
once -- impossible. `skippable` was therefore always false and the full E2E
matrix ran for every PR, including the docs/scripts-only PRs it was meant to
skip (e.g. scripts-only #419).
Rewrite the filter the way dorny documents `every`: `non_skippable` lists the
same safe allow-globs, each NEGATED, so a file forces E2E iff it matches NONE
of them (i.e. it is outside the allow-list). e2e_needed = non_skippable, keeping
the fail-safe "run E2E unless every changed file is provably irrelevant" rule --
a mixed PR still runs the matrix.
Add an `e2e_harness` override so a change under .claude/skills/preflight/** (the
local instrumented-test harness) still forces E2E even though .claude/** is
otherwise skippable.
Skip: app/src/test/**, **/*.md, docs/**, scripts/**, .claude/** (minus preflight).
Run: everything else -- app/**, *.gradle*, gradle/**, gradle/libs.versions.toml,
app/schemas, app/proguard-rules.pro, .github/workflows/**, .github/scripts/**,
.claude/skills/preflight/**. This ci.yml change itself runs the matrix.
Verified with PyYAML (valid) + a dorny-semantics simulation over representative
changed-file sets (scripts-only/docs/unit-test/.claude skip; app/androidTest/
gradle/ci.yml/preflight/.github-scripts run).
Closes#420
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.
The prior fix matched merge_conditions to auto_merge_conditions, but Mergify's
ruleset-compatibility check kept flagging "Configuration not compatible with
required_status_checks ruleset rule". The actual in-place-checks requirement is
that queue_conditions == merge_conditions, and this config had no
queue_conditions block at all, which Mergify reads as a two-step-CI mismatch.
Add a queue_conditions block to queue_rules.default identical (same conditions,
same order) to merge_conditions:
- base = main
- -draft
- -conflict
- label != broken
- check-success = CI passed
Mergify runs three condition sets sequentially: auto_merge_conditions triggers
queueing, queue_conditions validates queue entry, merge_conditions validates the
merge. All three are now identical. With batch_size 1 and max_parallel_checks 1,
this makes Mergify validate PRs in place on the real branch, keeping the strict
require-up-to-date ruleset enabled (hard invariant). No other settings changed.
Mergify flagged the strict require_status_checks ruleset
(require-branches-up-to-date) as incompatible with speculative draft-PR
checks. Per Mergify, staying compatible requires in-place checks: this repo
already has merge_queue.max_parallel_checks: 1 and queue_rules batch_size: 1,
but queue_rules.default.merge_conditions was missing `base = main` and thus
did not match merge_protections_settings.auto_merge_conditions.
Add `base = main` to merge_conditions and order both lists identically so
Mergify validates PRs in place instead of via a speculative draft PR.
require-up-to-date stays enabled; batch_size, merge_method, and
max_parallel_checks are unchanged.
When the opt-in cache encryption (encryptCache) is ON, persist crash and
"Report a problem" reports encrypted at rest, decrypting them on read; when
OFF they stay plaintext exactly as before.
- ReportStore gains a ReportEncryption collaborator (default None = plaintext,
so existing call sites are unchanged). On write it seals the storage JSON with
AES-256-GCM and tags it with a marker prefix; on read it sniffs the prefix, so
pre-toggle plaintext and post-toggle sealed reports coexist. Writes FAIL
CLOSED: a sealing failure drops the report rather than leaving plaintext on
disk. Decrypt failures are logged (PII-free) and skipped.
- KeystoreReportEncryption reuses the vetted KeystoreCrypto (non-auth master
key, so a crash while the app is locked can still seal), and mirrors the
encryptCache setting into a crash-safe in-memory flag warmed at startup (no
DataStore read on the crashing thread).
- PII-free AppLog logging at the enable/disable transition and both fallback
paths; never logs report contents.
Tests: JVM unit tests for the ReportStore branching (seal-on-write, plaintext
when off, crash persistence, fail-closed, mixed files, decrypt-failure skip,
markSurfaced re-seal) and for KeystoreReportEncryption; an instrumented test
proves real Keystore ciphertext on disk + round-trip on device.
The `pull_request_rules` `queue` action no longer auto-queues PRs in current
Mergify: a green, matching PR just reported "Merge queue is ready - use
`@Mergifyio queue`" and sat there, never merging. Automatic queueing now lives
in `auto_merge_conditions` under `merge_protections_settings`; the old
`autoqueue`/queue-action auto path is deprecated and stops working 2026-07-16.
Replace the non-functional `pull_request_rules` block with
`merge_protections_settings.auto_merge_conditions`, mirroring the exact same
gating conditions the old queue action used (base = main, -draft, -conflict,
label != broken, check-success = CI passed).
This changes ONLY the trigger (manual -> automatic). It does not touch the
queue's merge semantics: queue_rules (batch_size 1, merge_method merge,
merge_conditions), merge_queue (max_parallel_checks 1), and priority_rules
(P0-P9) are all unchanged. require-up-to-date stays ON - only batching
(batch_size > 1) would force that GitHub checkbox off, and we keep batch_size 1.
When a merge queue is configured, a matched PR is auto-queued (not merged
directly), so it still goes through the serial queue, is updated onto latest
main, re-runs CI, and merges on the real green "CI passed".
Structure verified against the live Mergify docs (file-format, merge-queue
rules/priority/lifecycle/batches, merge-protections auto-merge). YAML parses.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The `timeout -k 30s` wrapper + `capture_wedge()` added in 2f32657 for the
matrix `e2e` job (API 29-36) reproducibly wedges every leg, while the
manually-provisioned API 37 preview shard running the identical capture
logic passes. Revert the two matrix "Run E2E tests" steps' `script:`
blocks to main's plain script (just the backgrounded logcat stream +
`./gradlew connectedDebugAndroidTest`) and drop the now-dead "Upload wedge
diagnostics" step from the matrix job.
Kept untouched: the job-level `timeout-minutes: 50` backstop added in
27ede55, and the entire `e2e-preview` job (its own capture_wedge/watchdog
and wedge-diagnostics-api37-preview-shard* upload are unaffected).
The matrix E2E job (api-level 29-36) had no timeout-minutes, so a wedge
hangs until GitHub's 6-hour default instead of being force-killed. The
sibling e2e-preview job already sets timeout-minutes: 35. A normal
matrix run is ~15-20 min and a retry-inclusive run ~40 min, so set
timeout-minutes: 50 to give headroom above the in-step wedge-capture
timeout (1200s) while still bounding worst-case runtime.
E2E legs intermittently WEDGE (hang) with no fast-fail until the job force-kill,
and GitHub's post-force-kill step behavior is unreliable, so #388's diagnostics
don't reliably capture the wedge — and don't capture wedge-specific state anyway.
Wrap the `connectedDebugAndroidTest` run (both the `e2e` matrix first-attempt +
retry, and each `e2e-preview` shard) in an explicit `timeout -k 30s 1200`
(20 min) — comfortably above a normal run (~13-15 min), well below the hard cap —
so a wedge trips the wrapper (exit 124), NOT the force-kill, GUARANTEEING the
capture runs while the emulator is still alive. On 124, capture_wedge grabs the
smoking gun into a `wedge-diagnostics-api<level>` artifact: the running/last test
(logcat TestRunner), SIGQUIT (kill -3) thread dumps of the app + instrumentation
processes (ART -> logcat + /data/anr), dumpsys activity/window, `service list` +
`service check input/window/activity` (the boot-race crux), sys.boot_completed +
init.svc.* state, the snapshot cache-hit note, and accel/kvm/mem/disk. Then it
exits with the real status so #388's diagnostics + the existing retry still fire;
a normal run finishes before the wrapper and is unaffected.
EVIDENCE ONLY — no boot-readiness guard/fix (maintainer: prove the cause first).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bakes the proven 2026-07-06 cold-vs-warm pause-hook flow into
scripts/device-testing/ as a first-class, reproducible `cold-fetch-ab`
scenario, upstreaming the scratchpad driver.
- fetchgate.py: FETCH_GATE pause/resume/query helpers through the guarded
adb wrapper, with ordered-broadcast read-back parsing (paused=[...]).
- scenarios.cold_fetch_ab: pre-arm halt -> detect sign-in (sync all
breadcrumb) -> confirm halt (prefetch skipped) -> wait for header sync ->
measure cold opens -> resume -> measure warm opens. ALWAYS resumes on exit
(finally), even on error -- never leaves fetch paused.
- report.render_cold_fetch_ab: gate summary, cold/warm tables, cold-vs-warm
delta, connect=0ms reuse proof, throttle signature.
- Portability (subsumes #392): file-based uiautomator dump (not /dev/tty),
UTF-8 adb decode + PYTHONUTF8/console I/O, openMessage-breadcrumb readiness,
row-selection hardening (skip non-message rows).
The pause hook is debug-build-only (#393/#395), so the scenario needs a debug
APK. Automated validation: mocked unittest coverage (adb/breadcrumbs/gate) for
the helpers and the A/B scenario incl. restore-on-error, plus a --dry-run path
exercised end-to-end through perf_harness.main. A full on-device run is a
follow-up. Dev-tooling only; no app/src changes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Final step of the Robolectric Compose epic (#373): now that batches
#376-384 have all landed and proven stable, measure the new whole-app
JVM line-coverage baseline and raise the no-regression floor to match.
Measured 87.89% line (7994/9095), up from 80.21% (4838/6032) when the
floor was last set. Floor moves 0.79 -> 0.84, a deliberately wider
~3.9% headroom (vs. the usual ~0.5-1%) for this first post-epic
measurement; the maintainer can tighten it further in a follow-up PR.
Docs (CLAUDE.md, preflight SKILL.md) updated to match.
Batch 9/9 (final) of the Robolectric Compose JVM-test epic (#373).
- AppLockGateHostJvmTest: drives the app-lock gate host on the JVM via the
v2 createComposeRule under Robolectric, covering the Unlocked / Checking /
Locked render branches, the "content stays composed after re-lock" latch,
and the no-FragmentActivity auth-error path. AppLockViewModel is mocked.
Drops **/AppLockGateHost* from jacocoNonJvmTestableSurface.
- LibreMailAppJvmTest: covers the JVM-tractable parts of LibreMailApp.kt —
LibreMailBottomBar, StartupCrashPrompt (+ its dialog buttons), and
LibreMailApp's cold-start "hold until known" guards.
- LibreMailApp itself KEPT excluded (the acceptable exception noted in #384):
its NavHost start destinations call hiltViewModel() and the graph needs
owners a plain JVM compose rule can't surface, so graph-level nav stays on
the instrumented OnboardingFlowTest. Documented in the jacoco list.
Instrumented LibreMailBottomBarTest / StartupCrashPromptTest stay as the
on-device E2E. JaCoCo floor unchanged (0.79); scoped line coverage 0.8426.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Convert the Paging 3 mailbox list + folder drawer to Robolectric JVM Compose
tests (batch 8/9 of umbrella #373) and drop them from jacocoNonJvmTestableSurface.
- MailboxScreenJvmTest drives the real MailboxScreen + MailboxViewModel over
mocked repositories, feeding Paging via static PagingData.from flows (no real
Room/Paging source, mirroring MailboxViewModelTest). Covers the no-accounts
welcome fallback, populated list (sender/subject/snippet, offline badge,
unified per-account labels + filter chips, drafts/outbox entries), the
empty/loading/no-results states, search open/close, and the multi-select
contextual action bar (overflow, archive/spam/delete confirms, move picker,
archive-hidden-in-archive, disambiguated app-bar title).
- FolderDrawerJvmTest drives the callback-driven FolderDrawer: friendly role
names, duplicate-name provider disambiguation + account-switch gap, folder
taps, the multi-account switcher/dropdown, and the unread badge (incl. 99+ cap).
- Remove **/MailboxScreen* and **/FolderDrawer* from jacocoNonJvmTestableSurface;
overall JVM line coverage 84.69% (floor unchanged at 0.79).
The instrumented MailboxScreenTest/FolderDrawerTest stay as the on-device E2E.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Convert the settings screens/components to Robolectric JVM Compose tests
(umbrella #373, batch 5/9) and drop their globs from
`jacocoNonJvmTestableSurface`, so they count toward JaCoCo's JVM-testable
surface without an emulator.
New `src/test` Robolectric Compose tests (v2 createComposeRule, @Config sdk=36,
NATIVE graphics), mocking each ViewModel where needed:
- SettingsComponentsJvmTest (SectionHeader/SwitchRow/ClickRow/RadioRow/RetentionSection)
- SettingsScreenJvmTest (+ stateless ContactAutocompleteRow)
- AccountSettingsScreenJvmTest
- SignaturesScreenJvmTest
- SignatureEditScreenJvmTest
Line coverage of the newly-included files: SettingsComponents 100%,
SignatureEditScreen 100%, SignaturesScreen 97%, SettingsScreen 95%,
AccountSettingsScreen 84%. Overall scoped line coverage 86.2%. The instrumented
androidTest E2E stay; the JaCoCo floor is unchanged (re-ratchet is #386).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Port the instrumented ComposeScreenTest to a Robolectric JVM Compose
test (batch 7/9 of umbrella #373) so ComposeScreen's render + interaction
code counts toward JaCoCo's JVM-testable surface, and drop
`**/ComposeScreen*` from `jacocoNonJvmTestableSurface`.
ComposeViewModel is large (7 collaborators, several Context/Room-backed),
so it is mocked — mirroring AccountPickerScreenJvmTest / ManualSetup
ScreenJvmTest — with its state/accounts/finished flows stubbed so every
render/state branch is injectable. A RESUMED lifecycle owner (also the
back-press dispatcher owner) and a no-op ActivityResultRegistryOwner let
`collectAsStateWithLifecycle`, the BackHandler, and the attachment/inline
-image launchers compose on the JVM. The embedded RichTextBodyField renders
live; its toolbar accessibility labels and body-change plumbing are covered.
The instrumented ComposeScreenTest stays as the on-device E2E. JaCoCo floor
unchanged (0.79); overall line coverage 0.86.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Port the reader screen's chrome to a Robolectric JVM Compose test (umbrella
ReaderScreenJvmTest drives the real ReaderViewModel over a mocked
MailRepository/SettingsRepository via the v2 createComposeRule() — no emulator —
covering the top bar, star/delete/reply/reply-all/forward actions, the
attachment accordion + downloaded indicator, the attachment download-failure
snackbar, and the loading/plain-text/empty/error/remote-images-banner branches.
WebView caveat: the HTML body renders through HtmlBody, a hardened WebView that
Robolectric can only present as a non-rendering shadow, so the banner branch is
driven via an HTML message with a blank body (no HtmlBody call) and no
WebView-rendered HTML is asserted. HtmlBody.kt stays in scope, covered by its
existing HtmlBodyTest/InlineImageResolverTest. The instrumented ReaderScreenTest
stays as the on-device E2E. ReaderScreenKt lands at 94.3% line coverage; the
bundle rises to 82.7%, above the unchanged 0.79 floor.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Implements issue #409: a serial Mergify merge queue that supersedes the
hand-rolled poor-man's queue (autoupdate.yml + ci-trigger.yml +
traffic-control.yml, all already disabled_manually).
- queue_rules "default": batch_size 1 (serial, no batching), merge_method
merge (merge commits, never squash/rebase), merge_conditions gate on
check-success = "CI passed" + -draft + -conflict + label != broken.
- merge_queue.max_parallel_checks 1 (true serial; unambiguously
require-up-to-date-compatible).
- priority_rules map P0..P9 labels (P0 = 10000 highest .. P9 = 1000).
- pull_request_rules queue action triggers auto-queueing (queue_conditions
alone do NOT auto-queue per Mergify lifecycle docs).
require-up-to-date STAYS ON (Phase 1 is the only trilemma combo that keeps
the checkbox literally enabled AND preserves merge commits). No batching
(that is Phase 2 / #410). Single required gate stays "CI passed".
Validated: YAML parses and conforms to Mergify's published JSON schema
(negative-control confirmed). Merging this activates Mergify, so NO
auto-merge — must be reviewed first.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Investigate Mergify (free-for-OSS merge queue + batching + speculative checks)
as the right-way replacement for the hand-rolled traffic-controller
(autoupdate.yml + ci-trigger.yml + mothballed traffic-control.yml) and the
manual serial-bump grind, now that GitHub's native merge queue is org-only and
unavailable to a user account.
Proposal only — NO live .mergify.yml, nothing activates:
- docs/ci/mergify-integration-spec.md: how the queue coexists with the single
`CI passed` gate; the require-up-to-date x merge-commits x batching trilemma
and its resolution (Phase 1 serial keeps the rule literally; Phase 2 merge-batch
moves the up-to-date GUARANTEE into the queue); P0-P9 -> priority_rules mapping;
what it replaces; interaction with path-filter/sharding/wedge-diag; risks;
phased adopt recommendation.
- docs/ci/mergify.yml.proposed: annotated, NOT-active proposed config.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Convert the VM-driven mail list screens (DraftsScreen, OutboxScreen,
ProblemReportsScreen) to Robolectric JVM Compose tests in the `test`
source set, driving each real ViewModel over a mocked MailRepository /
ReportStore + DiagnosticsCollector via the v2 createComposeRule() — no
emulator. Each test covers the empty/populated render states, item
rendering (subject/recipient/body, queued-vs-failed status, crash/manual
kind labels), and the interactions (open, delete, cancel, retry, create).
Drop the three now-JVM-covered globs from jacocoNonJvmTestableSurface so
the screens count toward the JaCoCo denominator; measured coverage is
DraftsScreen 100%, OutboxScreen 100%, ProblemReportsScreen 97%, and the
bundle line ratio rises to ~82.7% (floor 0.79 unchanged). The instrumented
androidTest E2Es (DraftsScreenTest / OutboxScreenTest /
ProblemReportsScreenTest) stay as the on-device tests.
Part of the Robolectric Compose umbrella (#373); mirrors the #375/#376
pattern (AddAnotherAccountScreenJvmTest, format-control JVM tests).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a cheap `changes` job (dorny/paths-filter v4, pinned SHA) that sets
e2e_needed=false only when EVERY changed file is in a safe allow-list
(app/src/test/**, **/*.md, docs/**, scripts/**, .claude/**); anything
else -- or any non-pull_request event -- defaults to true (conservative,
"err toward running E2E").
Gate `e2e` and `e2e-preview` on needs.changes.outputs.e2e_needed so the
whole matrix runs or skips together, and rewrite the `ci-passed` gate:
it now BLOCKS on changes!=success, any of traffic-control-tests /
static-analysis / debug-build / unit-tests !=success, or e2e/e2e-preview
==failure|cancelled -- while TOLERATING an intentional e2e/e2e-preview
'skipped'. So test-only/docs PRs go green on the fast gate, a real E2E
failure/cancel still blocks, and a broken filter (changes!=success)
still blocks. Branch protection ("CI passed") context is unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add Robolectric JVM Compose tests (umbrella #373, batch 3/9) for the
account-setup screens and drop them from `jacocoNonJvmTestableSurface` so
their render/interaction code counts toward the JVM-testable coverage surface:
- AccountPickerScreen (98.9% line)
- AppPasswordSetupScreen (98.7% line)
- ManualSetupScreen (98.5% line)
Each test drives the real screen via the v2 `createComposeRule()` under
RobolectricTestRunner with a mocked ViewModel (their own logic stays covered by
the ViewModel unit tests), a RESUMED LifecycleOwner for
`collectAsStateWithLifecycle`, a no-op ActivityResultRegistry for the Outlook
launcher, and a recording UriHandler for the app-password help links — covering
render, per-provider chrome, field/submit wiring, and the enabled/busy/error/
done branches. The instrumented androidTest E2Es stay as the on-device coverage.
The JaCoCo floor (0.79) is unchanged — the re-ratchet is the final #373 step.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add Robolectric JVM Compose tests (umbrella #373, batch 2/9) for the
stateless onboarding + lock screens, and drop each from
jacocoNonJvmTestableSurface so its render/interaction code now counts as
JVM-testable surface:
- LockScreen: locked title/body, optional error, unlock callback.
- WelcomeContent + OnboardingWelcomeScreen: render + add-account; the
wrapper's NotificationPermissionEffect launcher is wired to a no-op
ActivityResultRegistry so no system dialog is surfaced on the JVM.
- LicenseScreen: real bundled GPL text renders, Agree gated on
scroll-to-end, Decline.
- ContactsAccessContent (skip/grant/request/rationale) plus the
ContactsAccessScreen wrapper, driven by a mocked OnboardingViewModel.
- BatteryOptimizationScreen: offered vs. done states; Take me there marks
the prompt handled and resolves the settings intent; Not now finishes.
Contacts/Battery use a tall @Config qualifier so their centered,
non-scrolling columns fit without the lower controls clipping. The
instrumented androidTest tests are kept (and remain the coverage for the
system back press, which the JVM compose rule cannot drive). JaCoCo floor
unchanged at 0.79 (the re-ratchet is the final #373 step).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The on-device perf harness cannot force a genuinely uncached body fetch: proactive
backfill (#12) and post-sync body prefetch (#88/#89) warm the cache before a test can
open a message. Add a debug-only, adb-reachable hook to pause proactive fetch so a real
uncached open can be measured.
Components:
- DebugFetchGate (src/main): thread-safe in-memory holder of paused FetchScopes
(BACKFILL, PREFETCH; `all` alias). Defaults to not-paused; HEADER_SYNC and on-demand
OPEN are never gateable.
- FetchGateReceiver (src/debug only): BroadcastReceiver registered in the debug manifest,
driven by `adb shell am broadcast -a org.libremail.debug.FETCH_GATE -n .../FetchGateReceiver
--es action <pause|resume|query> --es scope <backfill,prefetch|all>`. Returns the state as
ordered-broadcast result data (paused=[...]) for a synchronous read-back.
Enforcement (each read guarded by BuildConfig.DEBUG so R8 strips it from release):
- BackfillWorker.doWork() entry -> skip-and-reschedule when BACKFILL is paused, mirroring
the existing cache-lock deferral (covers periodic + backfillNow()).
- MailSyncer/MailBackfiller.prefetchIfEnabled -> early-return when PREFETCH is paused.
openMessage / fetchBodyMarkingSeen / fetchAttachment are deliberately NOT gated.
Debug-only: receiver + <receiver> live wholly in src/debug; every gate read in main is
behind BuildConfig.DEBUG. Verified on assembleRelease that R8 strips DebugFetchGate /
FetchScope / FetchGateReceiver and the log strings from the release APK, and the merged
release manifest has no FETCH_GATE receiver.
PII-free AppLog breadcrumbs on pause/resume/query and on each gate-triggered defer/skip
(scope names only).
Tests: DebugFetchGateTest, BackfillWorkerTest / MailSyncerTest / MailBackfillerTest
enforcement cases, and FetchGateReceiverInstrumentedTest (ordered-broadcast -> gate ->
read-back; gated worker defers while an un-gated path runs).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Port the instrumented ColorSwatchRow / FontPicker / FontSizePicker /
ParagraphAlignmentControl tests to Robolectric JVM Compose tests (v2
createComposeRule, @GraphicsMode NATIVE, @Config sdk=36) in the `test`
source set, and drop their four globs from `jacocoNonJvmTestableSurface`
so they count toward the JVM coverage metric. The instrumented tests stay.
Also fix a latent gap in the #375 infra: the JaCoCo agent skips classes
with no code-source location, which is exactly how Robolectric loads the
classes-under-test through its sandbox classloader — so Robolectric-only
Compose coverage recorded as zero (the PoC AddAnotherAccountScreen
included). `isIncludeNoLocationClasses = true` on the Test tasks makes
that coverage register; scoped bundle line coverage rises ~0.80 -> ~0.82.
Floor left at 0.79 (#386 re-ratchets).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The dominant merge-blocking flake was the "Set up Android SDK" step
(android-actions/setup-android v4.0.1) dying BEFORE the emulator starts:
Wrong version in preinstalled sdkmanager
Warning: ... preparing SDK package Android Emulator: Error reading Zip
content from a SeekableByteChannel.
Error: The process '.../sdkmanager' failed with exit code 1
Root cause: the action's default cmdline-tools version (20.0) rarely matches the
runner image's preinstalled one, so it logs "Wrong version in preinstalled
sdkmanager" and re-fetches cmdline-tools with NO checksum; it then runs its
default `sdkmanager tools platform-tools` install. Any of those downloads can be
a corrupt/truncated zip, which sdkmanager turns into an un-retried exit 1. v4.0.1
is the latest release, so this is fixed by configuration + hardening, not a bump.
Harden with verify -> reject -> retry, never trusting sdkmanager's exit code
alone, via a new stdlib-only helper .github/scripts/setup_android_sdk.py:
- bootstrap: download the pinned cmdline-tools zip, verify size + SHA-256
(authoritative pin, cross-checked against Google's published SHA-1), and
install it to $ANDROID_SDK_ROOT/cmdline-tools/20.0 -- the exact path
setup-android probes first, so the action reuses the verified tree and never
does its own unverified "Wrong version" re-download. A mismatch (corrupt OR
wrong version) deletes the bad zip + any half-extracted dir and re-downloads.
- install: sdkmanager --install with retry + backoff; on a corrupt package zip it
purges the partial/corrupt package dir (and sdkmanager's temp dirs) before
retrying, forcing a fresh download instead of a re-read.
- setup-android now runs with packages: "" (no flaky tools/platform-tools
install) and cmdline-tools-version: "14742923" (reuse the verified bootstrap).
- actions/cache restore + success-gated save so only a verified SDK is ever
cached (integrity gates the cache); shrinks the re-download/corruption surface.
Applied to every SDK-setup job (debug-build, unit-tests, static-analysis, e2e
matrix, e2e-preview). Emulator BOOT logic, #372 API-37 sharding, and #388
diagnostics are untouched. Pure-logic helpers are unit-tested
(test_setup_android_sdk.py, run by the traffic-control-tests job).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The API 29-36 `e2e` matrix uploaded only its test report, so an emulator
flake or a red leg (e.g. `E2E (31)` dying on a bare `sdkmanager` exit 1)
left nothing to diagnose. Bring the #334 API-37 diagnostics to the matrix,
inline (no changes to `e2e-preview`, which PR #372 is restructuring):
- Stream `adb logcat -v time` to `$RUNNER_TEMP/logcat-api<level>.txt` at the
top of both the "Run E2E tests" and retry reactivecircus steps (emulator is
booted there); backgrounded so gradle stays the exit-status-bearing command.
- New `if: failure()` step dumps device + runner state (adb devices, logcat
tail, emulator -accel-check, /dev/kvm, free -h, df -h) to the step log and a
diagnostics file; every probe guarded with `|| true`.
- New `if: always()` upload-artifact (same pinned v7 SHA) `e2e-diagnostics-api<level>`
carries the logcat + diagnostics files, `if-no-files-found: warn`.
- Make "Install SDK platform and build-tools" diagnosable: bounded 3x retry with
backoff for a transient sdkmanager failure, and print `--list_installed` on a
hard failure instead of a bare exit 1.
Keeps reactivecircus/android-emulator-runner and the existing boot-race retry.
Additive/diagnostic only; no boot-affecting flags change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Robolectric resolved its android-all-instrumented runtime jar lazily at test
time via its own MavenDependencyResolver/MavenArtifactFetcher, and that
download is unreliable on CI runners: AddAnotherAccountScreenJvmTest failed
with `AssertionError at MavenArtifactFetcher ... IOException` ("Failed to
fetch maven artifact"), though it passed locally where ~/.m2 was warm.
Resolve the jar through Gradle instead (reliable, cached, persisted by the CI
Gradle cache) and hand it to Robolectric in offline mode so it never hits the
network at test time:
- Pin org.robolectric:android-all-instrumented:16-robolectric-13921718-i7
(exactly what Robolectric 4.16.1 DefaultSdkProvider maps @Config(sdk=36) to)
in the version catalog.
- Add it to a dedicated resolvable configuration (NOT testImplementation/
testRuntimeOnly, which would flatten the ~200MB instrumented framework onto
the JVM test classpath and collide with the stub android.jar).
- syncRobolectricAndroidAll stages the jar under its Maven filename, and
robolectric.offline + robolectric.dependency.dir point Robolectric's
LocalDependencyResolver at it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Enables unit-testing Jetpack Compose UI on the JVM via Robolectric, so
render-only screens can leave the jacocoNonJvmTestableSurface exclusion
list and be counted by JaCoCo without an emulator.
- add Robolectric 4.16.1 (test scope) + Compose ui-test-junit4/-manifest
- testOptions.unitTests.isIncludeAndroidResources = true so resources
(strings, Material3 theme) resolve on the JVM
- src/test/resources/robolectric.properties pins sdk=36 (targetSdk 37 is
a preview level Robolectric 4.16 has no sandbox for)
- PoC: AddAnotherAccountScreenJvmTest drives the screen with the v2
createComposeRule under RobolectricTestRunner (3 tests, green on the JVM)
- drop AddAnotherAccountScreen from jacocoNonJvmTestableSurface (now
JVM-covered); floor stays 0.79 — re-ratchet deferred to end of #373
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The spike commits already implemented the N=2 shard matrix (numShards/
shardIndex via -Pandroid.testInstrumentationRunnerArguments.*), per-shard
test-retry parity, adb start-server before the boot loop, and shard-suffixed
artifact names. This drops the SPIKE / DRAFT "do not merge as-is" framing from
the ci.yml comments and reframes docs/perf/api37-e2e-sharding-spike.md from a
feasibility spike into the adopted design, so the change is mergeable as-is.
Also fixes the doc's section 3a example, which showed 1-based shardIndex values
[1, 2]; shardIndex is 0-based (0..numShards-1) and the implementation correctly
uses matrix.shard: [0, 1] -- [1, 2] would run an empty bucket and silently drop
half the suite.
Fan-in unchanged and verified: ci-passed still lists e2e-preview once; GHA
matrix aggregation makes its result `failure` if either shard fails, so both
shards must pass for the gate to go green. Branch protection requires the
"CI passed" context (not the per-leg "E2E (API 37 preview) (N)" check names),
so no branch-protection change is needed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SignaturesScreenTest built a real SignaturesViewModel by hand but tore down
with a bare `db.close()` that never cancelled viewModelScope. The ViewModel's
`signatures` StateFlow is a Room InvalidationTracker Flow kept alive by
stateIn(WhileSubscribed(5_000)), so the collector could stay live up to 5s
after the UI detached — a re-query then landed on the just-closed in-memory DB
and threw SQLITE_MISUSE ("connection is closed"). Timing-dependent, hence the
intermittent API-37 CI failure in tappingRadioOnNonDefault_makesItTheDefault.
Fix: hold the ViewModel in an androidx.lifecycle.ViewModelStore and, in @After,
call store.clear() (→ ViewModel.onCleared() → cancels viewModelScope) BEFORE
db.close(), so the collector is gone before the DB closes. Behaviour and
assertions are unchanged; the fix removes the race by construction.
Audited the androidTest tree for the same hazard and fixed two siblings the
same way:
- AccountSettingsScreenTest: had the same live-Room-Flow-vs-close race,
previously worked around by never closing the in-memory DB at all; now
clears the ViewModel then closes the DB.
- ComposeScreenTest: ComposeViewModel's init launches a viewModelScope
coroutine that reads the real accountSettings/signature Room repos; clear
the store before db.close() to avoid the same in-flight-read-vs-close race.
Verified locally: connectedDebugAndroidTest green for all three classes
(12/12) on a cold-booted emulator, plus the JVM fast gate (assembleDebug,
testDebugUnitTest, jacocoTestCoverageVerification, compileDebugAndroidTestKotlin,
lintDebug, ktlintCheck, detekt).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Folds the #370 root-cause finding into the spike. The stable `e2e` matrix
retries its test run once; `e2e-preview` runs connectedDebugAndroidTest exactly
once, so a flaky test self-heals on API 29-36 but wedges the required gate on
API 37 (e.g. #370's SignaturesScreenTest teardown race).
Doc: adds risk item 9 (retry-parity gap + its sharding interaction — per-test
flake is NOT amplified by sharding unlike boot flake, and a per-shard retry
costs only B + T/N; framed mitigation-not-fix) and two §6 recommendations
(retry parity, mirrored into api37_e2e.py; adb start-server before the boot
loop).
PoC (ci.yml): per-shard single test retry (::warning:: on retried-but-passed)
+ adb start-server before the boot loop. The api37_e2e.py retry mirror stays a
documented recommendation (local path needs a real-emulator validation this
spike did not boot). Still DRAFT, not auto-merged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Feasibility spike for sharding the e2e-preview job (the hand-provisioned
API 37 / google_apis_ps16k 16 KB-page emulator), CI's longest leg
(~16.4-17.6 min). docs/perf/api37-e2e-sharding-spike.md breaks the leg into
fixed overhead B ~8.3 min (setup + boot + Gradle daemon/config/compile/install)
vs parallelizable test execution T ~8.8 min, models B + T/N for N=2/3/4, and
recommends N=2 (~17.1 -> ~12.7 min, ~28% off the critical path) capped by the
API 30 matrix wall (~12.0 min) beyond N=3.
DRAFT PoC (do NOT merge as-is): converts e2e-preview to a strategy.matrix.shard
[0, 1] fan-out passing AndroidJUnitRunner numShards/shardIndex through the
existing -Pandroid.testInstrumentationRunnerArguments.* channel (no GMD, no
orchestrator, no Gradle change). Artifact names gain a shard suffix;
ci-passed still lists e2e-preview once (matrix fan-in keeps the single gate).
Local preflight stays single-emulator. Relates to #258.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CacheEncryptionGate.kt (the gate composable, blank cover, error screen, and ephemeral
report-review screen added for #359) is pure Compose render code, structurally
unreachable from a JVM unit test the same way every other Screen file in
jacocoNonJvmTestableSurface is. Left in scope, it dragged the whole-app line ratio to
0.78, just under the 0.79 no-regression floor.
Excluded it via "**/CacheEncryptionGateKt*" rather than the usual bare
"**/CacheEncryptionGate*" pattern this list otherwise uses, because
CacheEncryptionGateViewModel is named with "CacheEncryptionGate" as a literal
prefix - the bare wildcard would also have swallowed the already JVM-tested,
94%-covered ViewModel and its sealed CacheEncryptionGateState. CacheEncryptionGateViewModel
and CacheEncryptionUnavailableException stay in scope unchanged.
Verified locally: testDebugUnitTest + jacocoTestCoverageVerification now pass, with
the line ratio recovered to about 0.807 (5,044 covered / 6,249 total lines) - the
same 5,044 covered lines as before, just a smaller, honestly-JVM-testable denominator.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reworks #367. When SQLCipher native library loading fails while the opt-in
encrypted cache is enabled, the app previously degraded to a plaintext cache
(a silent fail-open that defeats the feature). Now it FAILS CLOSED.
DatabaseProvisioner raises a distinct CacheEncryptionUnavailableException
instead of degrading: it does NOT open plaintext, NOT wipe the on-disk
ciphertext, and NOT write the encryptCache setting. The throw is not
memoized, so a later launch re-attempts and recovers automatically if the
library loads.
A new CacheEncryptionGate wraps the app inside AppLockGateHost (so the
passphrase is already unlocked), probes prepareCache() before any DB-backed
screen composes, and on failure shows CacheEncryptionErrorScreen with the
exact message "Error - decryption could not proceed. Native decryption
library load failure." plus a "Report a problem" action. That action
generates an EPHEMERAL PII-free report via the existing DiagnosticsCollector
(never written to ReportStore, since encryption is unavailable in that
moment) for on-screen review and explicit Copy/Save; the copy says so.
The plaintext AccountDatabase tolerates the exception so accounts stay
readable for the error gate and the report. The encryptCache setting is now
written by exactly one caller: the user Settings toggle.
Tests: fail-closed raises the signal with no plaintext open / no wipe / no
setting write / not memoized; the gate VM resolves Ready vs Unavailable and
builds the ephemeral report; an instrumented error-screen UI test and an
AccountDatabase-resilience instrumented test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bind the non-exportable AES-256-GCM keys that seal the SQLCipher cache
passphrase to the hardware StrongBox secure element when the device has
one. Applied in the single shared place, AesGcmKeystoreCipher, so it
covers both the master (KeystoreCrypto) and auth-bound (DatabaseKeyCipher)
keys.
Devices without StrongBox throw StrongBoxUnavailableException at
KeyGenerator.generateKey(); a new generate-with-fallback path catches it
and regenerates a TEE-backed key so key creation still succeeds
everywhere. Guarded on API 28+ (minSdk is 29). Framing, seal/unseal, and
the missing-key policies are unchanged; the passphrase is still never
plaintext at rest and never logged.
Adds a JVM regression test for the StrongBox->TEE fallback via the
existing test seams (existingKey / a new generateKey seam).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a cross-platform, standard-library-only Python package under
scripts/device-testing/ that replicates LibreMail's on-device performance-test
scenarios and logging capture, codifying the methodology run by hand on
2026-07-05 (Pixel 10 Pro XL).
Modules:
- breadcrumbs.py: a pure, unit-tested parser for the ImapPerf / MailReader /
Reader / MailBackfiller breadcrumbs, plus open-correlation that reproduces the
manual timing-tables.md figures exactly.
- adb.py: a safety-guarded adb wrapper -- an allow-list of adb subcommands and a
deny-list + assertions on shell commands. The only sanctioned app-state
mutation is clearing LibreMail's own cache/ (exact-match); no pm clear /
uninstall / data wipe, and no touching databases/ files/ shared_prefs/
datastore/ can be constructed.
- uidump.py: uiautomator XML parser + screen recognition (mailbox rows with the
cached "Available offline" flag, reader, and the keyguard / foreign-app guards).
- scenarios.py: cold-open, message-open (uncached), back-nav, prefetch A/B
(fetch-policy toggle) and cross-provider, each keyguard-guarded and driven
through the guarded wrapper.
- report.py + perf_harness.py: aggregates, a timing-tables.md renderer mirroring
the manual write-up, and the CLI (timestamped run dir with the raw logcat, a
filtered breadcrumb extract, and the tables). --dry-run prints the exact
command plan without touching device state.
Tests (stdlib unittest, 68 cases) validate the parser, guardrails, UI
recognition and report against the manual run's real captures (fixtures include
a verbatim perf-extract slice and the reader/lockscreen/alarm dumps). Track the
*.log fixture past the gitignore *.log rule via a scoped negation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Restructure isConnectionDrop as leading guard clauses (definite-drop
types, then a not-MessagingException early return) instead of a when
expression, per maintainer review feedback on PR #368. Behavior is
unchanged; verified by the existing ImapConnectionCacheTest suite
(all 8 cases still pass), including the FolderClosedException /
StoreClosedException cases that depend on the check running before
the MessagingException .cause guard.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolve the IdleService.kt conflict as a union of both intents:
- #354 (already on main): foreground-service lifecycle rework —
onStartCommand delegates to the IdleForegroundStarter seam
(START_NOT_STICKY), cap-window skip/degrade.
- #357 Part 2 / #368: reused-connection idle-eviction sweep and
low-battery teardown of reused connections.
In startWatchingIfNeeded(), reconcileWatchers() stays inside the
cache-lock-guarded launch and evictIdleReuseConnectionsLoop() launches as
a sibling coroutine that runs while the service lives (its original #368
placement, independent of the cache-lock guard). No behavior change to
either side.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
An on-device drilldown proved Gmail server-side throttles LibreMail's
connect-per-operation IMAP: every op was a fresh CONNECT+TLS+LOGIN, and
full-history backfill's body+attachment prefetch generated ~601 connections in
~22 min, tripping (and sustaining) Gmail's per-account rate/bandwidth clamp
(body download collapsed to ~4 KB/s). The `live` gauge peaked at only 5 (Gmail
allows ~15), so it is connection *volume*, not count. Outlook IMAP on the same
device opened in 2-3 s. Reusing one warm socket per account (~601 -> ~1) removes
the throttle's trigger. This wires the reuse path the #125 spike built and left
OFF (issue #357 Part 2 — connection reuse only; prefetch is a separate PR).
How it is enabled (with a safety switch):
- New `BuildConfig.IMAP_CONNECTION_REUSE` (default true) drives the production
`ImapClient` no-arg `@Inject` constructor. To disable if a server misbehaves,
flip it to "false" in app/build.gradle.kts — a build-config change, no Kotlin
edit. The internal `ImapClient(reuseConnections, reuseIdleTimeoutMillis)`
constructor stays the test/harness seam.
- Universal: applies to all providers (incl. Outlook). No per-provider caps or
throttling here — that is a separate effort (#356/#360-#364).
Hardening `ImapConnectionCache` for production (was a spike):
- Transparent stale recovery: broadened drop detection to Angus's own
`iap.ConnectionException` (and a MessagingException caused by one) — the real
signal `folder.open()` throws on a server-dropped idle socket, which the
IOException-only check missed, so the reconnect now actually fires. A dropped
reused socket is rebuilt once and the op retried, so callers see no spurious
error; a genuine app error (e.g. message-not-found) is never retried.
- Idle eviction: `evictIdle()` closes a connection unused past the reuse idle
timeout (default 5 min), swept every 2 min by `IdleService`; skips any
in-use connection.
- Teardown: `IdleService` also tears down reused connections on the low-battery
push-teardown path (#88/#89/#90), mirroring the IDLE connection teardown.
- Concurrency: one connection per account behind a per-account mutex; the
eviction sweep takes the lock non-blockingly so it never stalls or interrupts
an in-flight op. Coexists with IMAP IDLE (its own separate connection).
- PII-free AppLog on the lifecycle (open / reuse-hit / reconnect-stale / evict /
teardown) keyed by an opaque per-cache ordinal, plus the #358 ImapPerf
breadcrumb (connect~=0ms on a reuse hit).
Tests (all via the fast gate, no emulator):
- ImapConnectionCacheTest: reuse, retry-once stale recovery, narrow drop
detection, deterministic idle eviction (injected clock), teardown.
- ImapFolderOpenLatencyTest (GreenMail + counting proxy): N ops share one
connection/LOGIN; a force-dropped socket is transparently reconnected; an app
error does not reconnect; idle eviction LOGS-OUT and the next op reconnects.
- Correctness suites (ImapClientTest/ImapClientBackfillTest/MailBackfillerTest)
pinned to reuse-off to keep their connect-per-op assertions unchanged.
Fast gate green: assembleDebug, testDebugUnitTest, compileDebugAndroidTestKotlin,
lintDebug, ktlintCheck, detekt.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On Android 15+ / SDK 37 devices with 16 KB memory pages (e.g. Pixel 10 Pro XL,
and the API-37 `google_apis_ps16k` emulator image), a native `.so` not aligned
for 16 KB pages fails to load with `UnsatisfiedLinkError` at
`SQLiteConnection.nativeOpen`. With the opt-in SQLCipher encrypted cache on, this
crashed the app on every cold start (issue #359, x4 on-device) instead of
degrading, and encryption silently never applied.
Fix: DatabaseProvisioner's encryption gate now catches `LinkageError`
(UnsatisfiedLinkError and related native-link failures) when opening/converting
the encrypted cache and degrades cleanly instead of propagating the crash — it
turns `encryptCache` off (so the next start does not re-attempt and re-wipe),
clears any on-disk ciphertext the plaintext framework opener cannot parse
(resetting its now-useless seals), and opens the cache unencrypted. The cache is
a re-syncable copy of server mail, so clearing it loses nothing that cannot be
re-fetched. PII-free AppLog.w breadcrumb on the degrade path.
Dependency: no bump needed or available. The repo already pins the newest
SQLCipher it references, `net.zetetic:sqlcipher-android:4.16.0`, which
docs/play-compliance.md certifies (ELF p_align = 0x4000) as 16 KB-aligned on
every ABI; SQLCipher has shipped 16 KB-aligned binaries since well before it, and
the other two bundled `.so` files (Compose graphics-path, DataStore
shared-counter) are already 16 KB-aligned per that doc. The graceful-degrade
catch is therefore the actionable fix.
Tests:
- Unit (DatabaseProvisionerTest): a simulated native-load failure degrades to a
plaintext open without crashing, turns encryptCache off, and wipes + reseals an
already-encrypted cache.
- Instrumented (DatabaseProvisionerInstrumentedTest): a fresh encrypt-on start
loads the real SQLCipher native library and opens the keyed cache — CI's API-37
`google_apis_ps16k` 16 KB job exercises the actual `.so` load, catching any
future 16 KB-alignment regression.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Root cause: after #302's runtime-cap fallBackToPeriodicSync() stops the
dataSync foreground service, IdleService was restarted (START_STICKY
null-intent redelivery + explicit startForegroundService) and onStartCommand
unconditionally called startForeground(DATA_SYNC) while the rolling-24h budget
was still exhausted. The platform rejected the start with
ForegroundServiceStartNotAllowedException; it was uncaught, the process
crashed, and START_STICKY restarted straight back into the same rejection -- a
crash loop until the 24h window freed budget (#354).
Fix (IdleService.kt):
- onStartCommand now returns START_NOT_STICKY. Push is app-managed
(LibreMailApplication.ensurePushStarted deterministically restarts it), so the
sticky null-intent auto-restart was redundant and fired exactly when a dataSync
FGS start is illegal.
- Guard the foreground start via a new JVM-testable IdleForegroundStarter seam:
a ForegroundServiceStartNotAllowedException (caught via its IllegalStateException
supertype, so no minSdk-29 class load) degrades like the cap handler --
schedulePeriodicSync(), keep the degraded POLLING notification, stopSelf()
promptly (avoids the "did not call startForeground in time" ANR) -- instead of
propagating.
- Record the cap event (elapsedRealtime); while still inside the cap window,
onStartCommand skips the now-guaranteed-illegal foreground start entirely.
- onTimeout stop path kept fast so ForegroundServiceDidNotStopInTimeException
stays mitigated.
PII-free AppLog.w/i on the degrade paths.
Tests:
- Unit (IdleForegroundStarterTest): onStartCommand returns START_NOT_STICKY; a
rejected start is caught and routed to degrade without propagating; the cap
window skips the attempt; a non-ISE propagates.
- Instrumented (IdleServiceForegroundStartInstrumentedTest): the degrade path on
a real Context -- rejection caught, periodic-sync fallback scheduled, degraded
"instant delivery paused" notification built, watching skipped.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds AppLog breadcrumbs to the message-open path so a debug report can show
where the reader's spinner time goes:
- ImapClient.withStore: per-op connect vs. work timing plus a live
connect-per-op connection gauge (issue #125's provider-ceiling context).
- fetchBodyMarkingSeen: select/body/flag phase timings plus PII-free size
counts (RFC822 size, body chars, attachment count).
- MailRepositoryImpl.openMessage: end-to-end open latency plus the
cached-vs-fetched branch, keyed by accountLogRef and logSafeFolderLabel.
- ReaderViewModel: spinner-to-ready latency, split success vs. failure.
All breadcrumbs are PII-free: accounts are logged via the existing
accountLogRef hash, folders via the existing logSafeFolderLabel allowlist,
and everything else is sizes/durations/booleans only.
Fixes the 4 unit-test classes that exercise this code without mocking
android.util.Log (a throwing stub under plain JVM tests): mockkStatic(Log)
is now installed in MailRepositoryImplCoverageTest, ImapClientBackfillTest,
ImapFolderOpenLatencyTest, and ReaderViewModelActionsTest, following the
existing MailBackfillerTest/ImapClientTest conventions. detekt.yml gains two
more ForbiddenImport excludes for the newly Log-importing test files.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the traffic-control (runner-priority orchestration) job verbatim out of
.github/workflows/ci.yml into a new standalone workflow,
.github/workflows/traffic-control.yml, so the heavy CI jobs no longer depend
on it. The job's YAML (name, runs-on, timeout-minutes, permissions, env,
steps) and its documentation comment move unchanged; the decision core
.github/scripts/traffic_control.py is untouched and still unit-tested by the
traffic-control-tests job in ci.yml.
In ci.yml: removed the traffic-control job, dropped needs: traffic-control
from the five heavy jobs (static-analysis, debug-build, unit-tests, e2e,
e2e-preview) and from traffic-control-tests (its only needs, which would
otherwise dangle at a now-deleted job), and updated the now-stale header and
ci-passed comments to point at the extracted workflow.
The new workflow will be disabled pending a rebuild as a published GitHub
Action.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#350 made ci-trigger.yml dispatch ci.yml with the built-in GITHUB_TOKEN, on the
claim that a workflow_dispatch is anti-recursion-exempt so no PAT is needed. In
practice a GITHUB_TOKEN-triggered run is held in `action_required` awaiting manual
approval and never runs un-attended, so auto-updated PRs' CI never ran (stalled
#285). The original #349 design was right: dispatch with a PAT so the run executes
as the authorized owner with no approval gate.
- ci-trigger.yml: the trigger step's GH_TOKEN is now
`${{ secrets.AUTOUPDATE_TOKEN || github.token }}` (was `${{ github.token }}`).
AUTOUPDATE_TOKEN (the PAT) is REQUIRED for the scheduler; the `|| github.token`
fallback stays fail-open but only starts CI if repo settings don't gate
GITHUB_TOKEN-triggered runs.
- autoupdate.yml: branch update stays on GITHUB_TOKEN (must NOT retrigger CI --
that would re-introduce the cascade). Clarified that AUTOUPDATE_TOKEN is still
required by the repo (by ci-trigger.yml) so the secret isn't deleted.
- Corrected the now-wrong "no PAT needed / workflow_dispatch anti-recursion-exempt"
comments in ci-trigger.yml and the traffic_control.py docstrings.
updates = GITHUB_TOKEN, triggering = PAT.
Validation: all three workflow YAMLs parse clean; traffic-control unit tests still
pass (59 tests) -- the change is workflow-env only, script logic unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolve DatabaseModule conflict from #320: main replaced the explicit .addMigrations(...) chain with .addMigrations(*ALL_MIGRATIONS) plus an introspectable ALL_MIGRATIONS list guarded by databaseModuleRegistersEveryDeclaredMigration (registered == declared). Add MIGRATION_19_20 to ALL_MIGRATIONS so the unified-inbox covering-index migration (cache schema v19->v20) is both registered on the Room builder and satisfies that safety-net test. Schema 20.json, the v20 @Database version, and DatabaseEncryptionTest's schema-version assertion (20) are unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
End the merge cascade and give the traffic-controller ownership of CI *triggering*.
- autoupdate.yml updates PR branches with the built-in GITHUB_TOKEN instead of a PAT,
so an update push no longer auto-retriggers CI (GitHub's anti-recursion rule) — the
cascade (every merge re-runs every PR, cancel-in-progress thrashing them) is gone.
- New scheduler ci-trigger.yml -> traffic_control.py --mode trigger (re-)triggers CI
for the highest-priority PR(s) whose head SHA has absent/stale checks, a few at a
time (inflight cap), in the existing P0-P9 / broken-draft priority order — a
poor-man's merge queue reusing the priority core. It runs after autoupdate finishes
(workflow_run, race-free) plus a cron backstop plus manual dispatch.
- Triggering uses workflow_dispatch, which is EXEMPT from anti-recursion, so the
built-in GITHUB_TOKEN (actions: write) starts the run — NO PAT / secret change needed.
- ci.yml gains a workflow_dispatch trigger (pr/head_sha/reason inputs) and a per-PR
concurrency group unifying pull_request and dispatch runs; its on: pull_request path
is kept so brand-new PRs, human pushes, and fork PRs always get CI (fail-open).
Pure select_triggers / classify_sha_runs decision core added to traffic_control.py with
24 new unit tests (priority order, oldest-first fairness, inflight cap, fork skip, P0
bypass+preempt, head-SHA needy classification, and a liveness/anti-starvation simulation).
Closes#349
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a detekt style>ForbiddenImport rule that forbids `import android.util.Log`
so all logging flows through org.libremail.reporting.AppLog, which mirrors each
line into the debug-report RingLogBuffer. A raw android.util.Log import writes to
Logcat only and never reaches a user-reviewed DebugReport (epic #324, strangler
final step).
Excludes the AppLog facade itself (the one sanctioned wrapper) and the unit tests
that mockkStatic(Log) to verify forwarding — AppLog forwards to Log, a throwing
stub under plain JVM unit tests, so those tests must mock it; they do not bypass
the facade.
Closes#331
Part of #324
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds a fast traffic-control-tests job (ubuntu, actions/checkout +
actions/setup-python, no emulator/Gradle) that runs the 37 pure-stdlib
unit tests for .github/scripts/traffic_control.py on every PR, and
wires it into ci-passed's needs so a regression blocks merge instead
of only being caught locally.
Closes#346
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Restore (and extend to drafts) the old bash's broken-reclaim behaviour that the
initial Python refactor had dropped. runs_to_cancel now cancels an OTHER PR's
active/queued runs when EITHER:
(a) THIS PR is P0 and that PR is strictly-lower (reclaim every lower runner); OR
(b) that PR is broken/draft (effective priority 10) and THIS PR is strictly-higher
(effective priority < 10) — a wasted run any ready PR may reclaim.
P1-P9 still never bump a *normal* (non-broken/draft) lower run; a broken/draft PR
(P10) preempts nothing (nothing is strictly-lower than the bottom, and the
equal-or-higher invariant means a P10 never cancels another P10). Self / main-push /
equal-or-higher invariants unchanged.
Updates the module docstring + ci.yml comments (the "only P0 preempts" wording
becomes: P0 preempts everything strictly-lower; additionally, any strictly-higher PR
preempts a broken/draft run) and the job step/permission/needs comments. Adds unit
tests: P3 reclaims a broken P10 run and a draft P10 run; P3 does not bump a normal P5
run; a P10 self preempts nothing; plus an end-to-end P5-reclaims-draft-then-waits
scenario. 37 unit tests pass; ci.yml parses clean; --dry-run shows a P3 cancelling a
draft (and broken) run while still yielding to a higher P1.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The priority-based runner orchestration ("traffic-control") lived as a large
inline-bash step in ci.yml — a two-pass preemption + hold-back script that was
effectively untestable in YAML. Move it into .github/scripts/traffic_control.py,
structured as a pure decision CORE + a thin gh-I/O SHELL:
* Pure functions (no network/clock/subprocess), unit-testable in isolation:
- effective_priority(pr): lowest-numbered P0-P9, default P5; broken OR draft => 10.
- runs_to_cancel(this_pr, all_prs, self_run_id): PASS 1 — run ids to cancel,
empty unless THIS PR is P0; only strictly-lower running/queued runs; never self
(by number or run id), never equal-or-higher.
- wait_blockers(this_pr, all_prs): PASS 2 — yield to any strictly-higher PR with
an active/queued run, and to same-level peers ordered ahead (running-first,
then oldest createdAt). Empty => proceed.
* Shell (run_live): gathers the snapshot via gh, applies cancels, runs the bounded
hold-back poll loop; always exits 0. --dry-run feeds the core a snapshot JSON and
prints decisions with zero network.
* No jq/bash dependency (cross-platform, per the repo's Python-stdlib convention).
ci.yml's traffic-control job now checks out the repo and runs the module. Job
permissions gain `contents: read` (for checkout) alongside the existing
`actions: write` / `pull-requests: read`; env and downstream `needs:` wiring
unchanged; step stays `continue-on-error`.
33 stdlib unittest cases cover priority resolution, P0-only preemption, the
self/main/equal-or-higher invariants, and the same-level running-first/oldest
ordering.
Behaviour is preserved except the ticket's refinements: (1) drafts now count as
P10 (bottom); (2) an explicit same-level running-first-then-oldest tiebreaker; and
(3) per the ticket's order-of-operations, ONLY P0 preempts — the old bash also let
any higher-priority PR cancel a `broken` target's run, which no longer happens
(a broken/draft run is only cancelled by a P0, via the same strictly-lower rule).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Explains the traffic-control job in ci.yml (priority labels, preemption
vs. bounded hold-back, safety invariants, and the known FIFO-runner
limitation) for developers new to the repo. Notes that #342 will refactor
this logic into a tested Python module, at which point this doc gets
updated.
Closes#343
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The sync engine (MailSyncer, MailBackfiller, MailPruner, and their WorkManager
workers) was completely silent, so a submitted debug report showed nothing
about whether sync ran, how much it fetched, or why it was skipped. Add
net-new AppLog breadcrumbs at each class's lifecycle points per the #324
strangler-migration plan: sync start/done/failed and per-folder fetch counts,
backfill slice start/done and per-folder page counts, prune's removed count,
and each worker's cache-locked deferral and success/retry outcome (the retry
path now also carries the scrubbed failure throwable via AppLog's #325
overloads).
Every breadcrumb is PII-safe by construction: accounts are identified only via
accountLogRef(account.id) (never the id or email directly), and a new
logSafeFolderLabel() helper logs a folder's name only when it matches a fixed
allowlist of known system folders (INBOX, Sent, Drafts, Trash, Spam/Junk,
Archive, and their common provider variants) — every other folder, however
nested or named, logs as a fixed placeholder.
Adding logging to these previously-silent classes meant every existing test
exercising them now hits android.util.Log (a throwing stub under plain JVM
unit tests), so each affected suite gains the same static Log mock already
established by AppLogTest/SendWorkerTest/ImapClientTest.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codify the maintainer rule that app source-code changes must add
appropriate, PII-free logging via the AppLog facade at key points
(lifecycle transitions, error/fallback paths, state changes) so
behaviour is diagnosable from a user's debug report.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Migrate AppLockViewModel (9 sites) and AccountSetupViewModel (1 site) off
raw android.util.Log onto the AppLog seam (#325), so their diagnostic
lines land in the RingLogBuffer and reach a submitted DebugReport instead
of only Logcat. Adds three new breadcrumbs that were previously silent:
auth-seal unlock success, the clear-cache-and-restart recovery trigger
(with the disableAppLock flag), and every onForeground LockAction
decision. AccountSetupViewModel's success path also now logs "Outlook
account added" (no email). None of these call sites carry PII; where a
throwable is attached, AppLog's StackTraceScrubber redacts it before it
reaches the buffer.
Both ViewModel test suites now install a real RingLogBuffer and assert
against it instead of `verify { Log... }`, including dedicated no-PII
assertions (a known test email never appears in a recorded line). A
minimal `mockkStatic(Log::class)` stub stays in both test files' shared
setUp — AppLog still forwards to the real android.util.Log internally,
which throws "not mocked" in JVM unit tests when uninvoked; the not-yet
-landed guard-rule ticket (#331) will need to reconcile that with a
repo-wide "no raw Log outside AppLog.kt" rule.
Closes#326
Part of #324🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Migrate ImapClient/SendWorker/IdleService off raw android.util.Log to AppLog,
per the debug-logging strangler epic (#324), so their diagnostics reach the
RingLogBuffer (and a submitted DebugReport) instead of logcat-only:
- ImapClient: IDLE connect + IDLE push (message count) breadcrumbs.
- SendWorker: outbox-drain count on entry, per-message sent/failed result,
and the existing Graph->SMTP fallback warning.
- IdleService: IDLE watch start, cache-locked defer, and the existing
IDLE-dropped/retrying warning.
Also closes#297: SendWorker and IdleService logged the raw account.email via
Log.w on the Graph->SMTP fallback and IDLE-drop paths. Both now log
accountLogRef(account.id) instead -- a short, stable, non-reversible
per-account reference -- so the account's email never reaches Logcat or a
report.
Rewrites ImapClientTest/SendWorkerTest to install a real RingLogBuffer via
AppLog.install(...) and assert on its contents (migrated calls + new
breadcrumbs), instead of verifying a mocked Log; every assertion also checks
no line carries the test account's email, regression-covering #297. Adds a
SendWorkerTest case that drives a real SmtpSender against an in-process
GreenMail SMTP server end to end. android.util.Log is still stubbed (by
fully-qualified name, without importing it) where AppLog's Logcat passthrough
would otherwise crash the unmocked Android stub in a JVM test.
Closes#328Closes#297
Part of #324
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Migrates the DB/keystore area's raw android.util.Log calls to AppLog so
key-invalidation and DB-conversion breadcrumbs land in the process
RingLogBuffer (and thus a user-reviewed debug report) even in release
builds, where Log.d is otherwise stripped from Logcat only.
- DatabaseKeyCipher: 4 auth-bound-key decision points (encrypt retry,
isInvalidated's three branches) now log via AppLog.d(tag, msg, e).
- DatabaseEncryption.migrate: adds an AppLog.i "converting local cache
database (targetEncrypted=...)" breadcrumb at the start, alongside the
existing "converted" completion line now routed through AppLog.d.
- AccountDataMigrator: the "moved account tables into the account
database: $present" breadcrumb (table names only) now routed through
AppLog.d.
No PII or key material is logged; table-name sets and boolean flags only.
Adds instrumented tests (DatabaseKeyCipher is device-only and
behavior-preserving, so no new test there) asserting the breadcrumbs
land in a RingLogBuffer and never contain the seeded email, secret, or
passphrase.
Part of #324.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The API 37 preview E2E boot has flaked twice (#285, #333) with only
"did not boot within 300s" and no root-cause signal. Add rich boot
diagnostics by default, kept in parity between CI and the local
hand-provisioning script (api37_e2e.py):
- Launch the emulator with `-verbose -debug init,avd_config,kernel`
(diagnostics only; no boot-affecting flag changed), still redirecting
to $EMU_LOG.
- Stream `adb logcat -v time` to a file from the moment the device
registers (via `adb wait-for-device logcat`, backgrounded).
- On a boot timeout, dump accel-check, /dev/kvm presence, GPU mode,
free mem/disk, the AVD config.ini and the emulator.log tail; CI writes
these to a boot-diagnostics file, the local script prints them.
- CI uploads emulator.log + logcat.txt + boot-diagnostics.txt as an
artifact with `if: always()` so they survive a timeout/cancel, and
prints a concise summary (accel/KVM status + last 50 lines of
emulator.log) to the step log.
The existing 2-attempt boot retry + boot-completed wait loop are
unchanged; the diagnostics are additive.
Closes#334
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Migrate RestartActivity's one raw Log.w site to AppLog, clearing the
final raw android.util.Log site outside the auth/lock, DB/keystore,
connectivity/send, and sync-engine migration areas so the codebase is
ready for the detekt android.util.Log guard (#331).
RestartActivity runs in the separate :restart trampoline process,
where LibreMailApplication.onCreate returns early and never calls
AppLog.install, so this breadcrumb reaches Logcat only, never a
DebugReport. The migration is guard-compliance + Logcat-consistency
only; behavior is unchanged since AppLog forwards to Logcat.
RestartActivity is DEVICE-ONLY (multi-process kill/relaunch), so a
JVM buffer-capture test doesn't apply here. Added
RestartActivityLoggingTest, which instead pins the null-buffer shape
this call runs under in the trampoline process: it forwards to
Logcat and no-ops the buffer cleanly.
Closes#330
Part of #324🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add throwable-recording overloads to AppLog.d/w and make AppLog.e record the
throwable it is given: the throwable's stack trace is scrubbed via the existing
StackTraceScrubber (exception class names + frames kept; host/email-bearing
exception messages stripped) and appended to the buffered log line, so a
throwable can reach a user-reviewed DebugReport without leaking PII. The
existing no-throwable overloads are unchanged.
Add accountLogRef(accountId): a short, stable, non-reversible reference
(scheme prefix + truncated SHA-256 of the id) so downstream logging can
identify an account without logging the raw Account.id, which embeds the email.
Foundation for the #324 debug-logging strangler epic; consumed by #326–#330.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Port the last bash dev-script (local_instrumented.sh) to a cross-platform,
stdlib-only Python 3 script (local_instrumented.py), matching api37_e2e.py's
style, and rewire the /preflight skill's local E2E off the GMD
apiXXDebugAndroidTest tasks (which fail locally under AEHD 2.2) onto it.
- local_instrumented.py preserves the .sh's behavior exactly: comma-separated
test-class CLI arg, pre-boot orphan-kill, manual cold-boot of the dev36 AVD
(no GMD, no snapshot), targeted connectedDebugAndroidTest, and the EXIT-trap
teardown (now try/finally + atexit + SIGINT/SIGTERM handlers, idempotent).
Exit codes 0/2/3/4 preserved.
- Cross-platform process kill abstracted per-OS: taskkill /F /IM on Windows,
pkill -f qemu-system on *nix; process listing via tasklist / ps ax.
- Teardown hardened vs the .sh: it now also reaps the emulator *launcher*
image, not just qemu -- the Windows -no-window emulator spawns a sibling
emulator.exe that briefly outlives the qemu VM, which a qemu-only sweep left
as an orphan on return (caught by the smoke run).
- SKILL.md + CLAUDE.md: replace the local api35/api36 GMD E2E steps with
local_instrumented.py; CI's own multi-API matrix is untouched. CLAUDE.md
documents the Python-first dev-script convention.
Validated: py_compile, argparse (--help / no-arg exit 2), and a guarded
emulator smoke run of org.libremail.data.local.DatabaseEncryptionTest -- boots,
passes, and tears down clean (no qemu/emulator orphan on return).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Scope :app:jacocoTestReport's denominator to the JVM-testable surface and
add a :app:jacocoTestCoverageVerification no-regression gate that shares the
same classDirectories/executionData/sourceDirectories, wired into both the
`check` lifecycle task and CI's unit-test job (part of the `CI passed` gate).
Excluded from the denominator (structurally unreachable from a JVM unit
test): Compose screen/component render code, Android framework entry points
(*Activity/*Service/Application/*BackupAgent), Hilt DI (**/di/**), and the
src/debug cold-open probe. Kept in scope: ViewModels, repositories, mappers,
DAOs, utils, richtext, mail, reporting logic, and the six WorkManager Workers.
Corrects PR #292, which excluded **/*Worker*: SyncWorker, BackfillWorker,
PruneWorker, SendWorker, ReportPurgeWorker and ReportUploadWorker are all
directly unit-tested, so they stay counted in both numerator and denominator
(only their Hilt wiring, WorkManagerModule, is excluded, via **/di/**).
Baseline: 80.21% line (4838/6032). Floor: 0.79 (~1.2% headroom) so ordinary
noise doesn't red-flag it while a real drop fails. Manual ratchet for now:
bump the floor up in the same PR when coverage rises materially.
Closes#251Closes#292
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
MIGRATION_19_20 (issue #187) bumped the Room cache schema to version 20,
but DatabaseEncryptionTest.schemaVersionIsCarriedOntoTheEncryptedFile still
asserted the plaintext -> encrypted conversion carried version 19, so it
failed across all E2E levels after the rebase onto main.
DatabaseEncryption.migrate() carries PRAGMA user_version dynamically
(userVersion = source.version -> target.version = userVersion), and a fresh
Room open now stamps 20, so v20 genuinely survives the conversion. Update the
expected constant to 20; the assertion's intent (the version survives the
round-trip) is unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#303: the reader's Reply now routes through MailRepository.buildReplyDraft
(quotes the original into a <blockquote>, bakes the signature, prefixes Re:/Fwd:
without double-prefixing) and opens compose on the built draft via
ReaderEvent.OpenCompose — the same high-fidelity path the mailbox uses — instead
of a bare compose prefill with an empty body. Adds Reply-All and Forward via an
app-bar overflow menu.
#304: SignatureEditViewModel.save, ReportReviewViewModel.submit,
AccountSettingsViewModel.removeAccount, and ProblemReportsViewModel.createManualReport
now flip a busy/saving flag synchronously before the first suspension and gate
their buttons, so a rapid double-tap can't create duplicate signatures/reports,
enqueue two uploads, or over-pop the back stack.
Closes#303Closes#304
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#310: add a single-transaction MessageDao.updateHeaderContents(List<MessageEntity>)
and route MailSyncer's per-message updateHeaderContent loop through it, so a folder's
recent-window refresh commits once instead of once per message (fsync/journal write
per message, amplified on the encrypted cache).
#311: append the `id` primary key as a tiebreaker to the four paged MessageDao
`ORDER BY timestampMillis DESC` queries for a total order, so rows sharing a second
(bulk mail) can't duplicate or skip across a LIMIT/OFFSET page boundary. Pure query-text
change: the exported Room schema (identityHash) is derived from table/index structure,
not @Query SQL, so no schema re-export or version bump; id is already in the projection,
so no new index.
#312: expose the registered migration list as DatabaseModule.ALL_MIGRATIONS (spread into
addMigrations) and assert in MigrationTest that it equals the reflectively-discovered set
of every Migration val, so a migration forgotten in addMigrations fails a test instead of
crash-looping all upgrading users at DB open (there is deliberately no destructive fallback).
Tests: new MessageDaoTest cases for the batch update and the id tiebreaker (all four
pagers), and the MigrationTest registration assertion; wired updateHeaderContents into
MailSyncConcurrencyTest's fake DAO. Instrumented MessageDaoTest + MigrationTest (31 tests)
green on a local emulator; unit tests + androidTest compile + ktlint + detekt green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ImapClient.deleteMessage and moveMessages flagged the target \Deleted then
called the untargeted Folder.expunge(), which permanently removes EVERY
\Deleted-flagged message in the folder — not just the intended UIDs. That is a
data-loss window whenever a second client, Gmail, or a partial earlier move has
left other messages flagged \Deleted. The repository's batch delete/expunge and
trash-fallback paths also looped single-UID deleteMessage, paying N logins + N
expunges for an N-message selection.
Add a batch deleteMessages(uids) that opens the folder once, flags the matched
messages \Deleted, and issues a single targeted UID EXPUNGE (RFC 4315) via
IMAPFolder.expunge(Message[]) through a shared expungeTargeted() helper. Route
moveMessages through the same helper, delegate single-UID deleteMessage to
deleteMessages, and route MailRepositoryImpl.expunge and the moveByRole trash
fallback through the batch method. moveToFolder already batches via moveMessages,
so it inherits the targeted expunge.
On a server without UIDPLUS, Angus raises "UID EXPUNGE not supported" rather than
silently falling back to the unrelated-mail-destroying untargeted expunge — a
loud failure is the safe outcome. Gmail, Outlook, and GreenMail all advertise
UIDPLUS.
Tests (GreenMail, no emulator): deleteMessages/moveMessages expunge only the
given UIDs and spare other \Deleted-flagged mail; a batch delete of three
messages opens exactly one connection and pays one LOGIN.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
exchangeToken's authorization-code exchange already requests
`openid email offline_access $OUTLOOK_SCOPE`, so the returned access token
is an outlook.office.com token usable for IMAP verification and the AuthState
already carries the refresh token and expiry. The immediate follow-up
refreshForScope(authState, OUTLOOK_SCOPE) was a second round-trip for the
same resource that only rotated the just-issued refresh token and added a
needless onboarding failure point (a transient network error there failed
sign-in after consent + code-exchange had already succeeded).
Build OAuthResult directly from the code-exchange tokenResponse
(accessToken + authState.jsonSerializeString()), dropping the extra refresh.
The durable AuthState is still serialized and persisted for later token
refresh; the Graph token remains a distinct resource minted on demand via
freshGraphToken.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A crash report captured throwable.stackTraceToString() verbatim, so mail/network
exceptions (Jakarta Mail, java.net) could embed server host:port tokens and account
emails/usernames in the report's stackTrace field — violating the PII-free-reports
constraint. Add StackTraceScrubber, applied in DiagnosticsCollector before the trace
enters toSubmissionPayload()/toStorageJson(): it keeps the non-PII value (exception
class names + every frame's class/method/file/line) and drops each header line's
free-text message (where hostnames/usernames live), then redacts any residual email
or host:port left on a wrapped continuation line. Frame lines are untouched, so a
frame's File.kt:42 is never mistaken for a host:port.
ReportStore did MutableStateFlow(scan()) in its constructor — a dir list + read +
JSON-parse of every stored report. As an eager @Singleton dep of CrashReporter, whose
install() runs on the MAIN thread in Application.onCreate(), this was main-thread disk
I/O that grows with the 30-day retention. Seed the flow empty and dispatch the initial
scan to an injectable scope (Dispatchers.IO by default); reactive consumers update when
it lands, and writes still re-scan synchronously so a crash-time save is never lost.
Tests: StackTraceScrubberTest (host/ip/port/email dropped from a ConnectException +
auth-failure trace while classes/frames survive; regex redaction of a continuation
line; null-message trace preserved verbatim); DiagnosticsCollector end-to-end scrub
test; ReportStore empty-seed + off-thread populate via a StandardTestDispatcher. Store
constructions in existing tests use an Unconfined scope to keep their synchronous
reopen semantics.
Closes#294Closes#296
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#309: AccountDao no longer uses @Insert(REPLACE). New insertIfAbsent (IGNORE)
+ @Update back a non-destructive upsert, and insertAtEnd updates an existing id
in place (preserving its sortOrder) instead of REPLACE. Re-adding an existing
account id (e.g. re-authing an Outlook account, whose id is the deterministic
outlook:<email>) therefore no longer cascade-deletes its account_settings +
signatures.
#305: normalizeEmailForAccountId always lowercases the domain (mail domains are
case-insensitive), and the whole address for the consumer providers (Gmail,
Yahoo, iCloud, AOL, Outlook). Applied at every id-derivation site
(MailProvider.createAccount, Account.outlook, ManualSetupViewModel) so
differently-cased addresses can't spawn duplicate accounts. The displayed email
keeps the user's casing.
#299: deleteAccount collects the account's message ids and draft attachment URIs
while the rows still exist, deletes the rows (now including the account's
drafts), then deleteRecursively()'s each message's on-disk attachment cache dir
and releases the drafts' now-unreferenced persistable URI grants.
Adds unit tests for id normalization + deleteAccount cleanup and DAO-level
instrumented tests for the non-destructive create/update.
Closes#309Closes#305Closes#299
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
IdleService runs continuously as a FOREGROUND_SERVICE_TYPE_DATA_SYNC
foreground service (push is on by default). With targetSdk 37, Android 14+'s
dataSync FGS runtime cap (~6h per rolling 24h) calls Service.onTimeout(...)
and then force-stops the service — throwing a system FGS-timeout exception —
if it doesn't stop itself. IdleService overrode onStartCommand/onDestroy/onBind
but not onTimeout, so after ~6 cumulative hours push silently died and the app
hit the exception; on API 35+ the budget is cumulative and a restart can't
recover it until the next 24h window.
Override both onTimeout(startId) (deprecated, API 34) and
onTimeout(startId, fgsType) (API 35+); both route to a clean shutdown that
re-asserts the already-scheduled 15-minute periodic sync, swaps the persistent
notification to a degraded "paused" text and DETACHes it so it survives, then
stopForeground(DETACH) + stopSelf so we never leave a dataSync FGS running past
its cap (the exact condition the platform kills on). This mirrors the existing
low-battery PushMode.POLLING fallback.
The push-status text choice is pulled into a pure PushStatusNotification.statusTextRes
seam and unit-tested on the JVM; the built notification's new timed-out text is
covered by PushStatusNotificationInstrumentedTest.
Closes#302
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
JMR-dev
