selectFolder() kicked off its background syncFolder() fetch fire-and-forget
with no loading flag, so opening a per-account folder with no cached
messages yet flashed "No messages to display" for the whole IMAP fetch
instead of a spinner. Adds isSyncingFolder, a StateFlow set for the
duration of that sync (mirroring isRefreshing) and cleared via try/finally
regardless of outcome. A private latestFolderSelection token guards the
clear so a stale sync from a folder no longer selected can't hide the
spinner for whichever folder is actually selected now.
MailboxScreen's non-paged empty branch now holds NoMessagesState back
while isSyncingFolder is true, showing a CircularProgressIndicator
instead — mirroring how the unified-inbox paged branch already gates on
loadState.refresh.
Closes#149
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Reorders SettingsScreen's top-level (non-Advanced) sections per #158:
Accounts, Message downloading, Contacts, Appearance, Settings Backup,
Notifications, Storage on this device, then a header-less trailing
Report a Problem row (mirrors AccountSettingsScreen's headerless
"Remove account" row now that Diagnostics is down to one item).
- Move "Message downloading" up to directly follow Accounts.
- Add a two-line descriptive subtext under the Appearance header
("Match device theme" / "Material You theming (Android 12+)"); no
new toggle, since LibreMailTheme already always follows the system
light/dark setting via isSystemInDarkTheme().
- Rename settings_backup "Backup" -> "Settings Backup" and
settings_new_mail "New-mail notifications" -> "New mail
notifications" (drop hyphen).
- Drop the now-single-item settings_diagnostics header string; keep
Contacts positioned directly before Appearance (its prior relative
spot), per the ticket's suggested safest default for the
unresolved placement question.
Advanced's internal order is untouched (out of scope; tracked by
#162).
Closes#158
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Replace the small inline "Report sent. Thank you!" text with an AlertDialog
carrying the fuller thank-you/no-guarantee message, and gate the screen's
auto-navigate-on-delete LaunchedEffect so it no longer fires while a submit
is in flight or has just succeeded — the dialog's acknowledgement is what
calls onDone() for that path instead. This closes the race where
ReportUploadWorker deletes the report row (and thus flips state.exists to
false) moments after SubmitUiState.SUCCEEDED, which could previously
navigate the user away before the confirmation was ever visible. Discard
and the other non-success paths (FAILED/UNAVAILABLE) are unaffected and
still auto-navigate immediately.
Closes#161
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The TextButton trigger and its DropdownMenu in AccountSwitcher were
siblings in the drawer's outer Column, so the dropdown's Popup anchored
to the whole Column instead of the button. Wrap both in a shared Box,
the standard Compose pattern, so the menu opens directly below the
account switcher regardless of drawer scroll position or folder count.
Closes#147
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Extract the AES-256-GCM Android Keystore plumbing that `KeystoreCrypto` and
`DatabaseKeyCipher` copy-pasted (~60 lines) into a shared alias-parameterized
base, `AesGcmKeystoreCipher`: the encrypt/decrypt bodies, existing-key lookup /
get-or-create under a lock, key deletion, and the 5 identical GCM constants now
live in ONE place. Each cipher keeps only its delta — the `KeyGenParameterSpec`
(via `keySpecBuilder()`) and, for the auth-bound key, the invalidation handling.
Preserve — deliberately — the two ciphers' different missing-key-on-decrypt
behavior via a `generateKeyOnDecrypt` policy parameter, documented on the base:
- master key (`KeystoreCrypto`, true): auto-generates on a missing alias, correct
for a first-run key with nothing sealed yet.
- auth-bound cache key (`DatabaseKeyCipher`, false): fails fast, because a missing
auth-bound key means it was INVALIDATED and silently regenerating it would
re-arm the lock against a cache that can no longer be decrypted.
Also map the opaque `AEADBadTagException` (thrown when the master path generates a
fresh key then can't decrypt old data) to a clear `GeneralSecurityException`,
while leaving `KeyPermanentlyInvalidatedException` to propagate unwrapped.
Unify the accepted-authenticator policy behind one source of truth,
`AuthenticatorPolicy.ACCEPTED`, mapped into each API's vocabulary
(`AppLockManager.AUTHENTICATORS` for BiometricManager / BiometricPrompt,
`DatabaseKeyCipher.keySpec` for KeyProperties / KeyGenParameterSpec) so the two
can no longer drift — a drift that yields a prompt that succeeds but a key that
throws `UserNotAuthenticatedException` at use.
Add JVM tests for the shared base (both `generateKeyOnDecrypt` modes + the AES-GCM
error mapping) and for the authenticator mapping. #100's seal-exchange and
policy-table safety net stays green.
Closes#102
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Close the test-coverage gap on the app-lock security core (#100): the
branching that decides when to WIPE user data or drop the lock, which
shipped largely untested.
- AppLockViewModelTest: pin the onAuthenticated unlock/arm classification
(OK / UNRECOVERABLE / RETRY) and the onForeground LockAction dispatch --
DISABLE_APP_LOCK persists the setting, CLEAR_* set the pending flag and
drop the gate BEFORE the awaited re-sync enqueue and process restart,
and CLEAR_AND_REQUIRE_AUTH clears + restarts but keeps app-lock on.
- KeyInvalidationPolicyTest: make the exhaustive 16-row decision table a
test, with a completeness guard so no row can be dropped. The common
(appLock on, encrypt off, secure, valid) -> REQUIRE_AUTH row is now
pinned, so a mutation to PROCEED (a silent lock bypass) fails.
- DatabaseKeyStoreTest: new JVM tests for the dual-seal exchange
(sealWithAuth dropping SEALED_MASTER, sealWithMaster, resetSealedPassphrase,
unlockWithAuth, clear-pending) pinning the "never both seals at once" and
"not recoverable without auth" invariants.
- SettingsViewModelTest: setAppLock reject / reseal / disable branches.
To make the device-only DatabaseKeyStore crypto JVM-testable, add a
minimal @VisibleForTesting DataStore seam (mirroring AppLockViewModel's
injectable dispatcher); production still uses the real per-app DataStore.
No crypto plumbing is refactored.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Inline images in rich HTML emails (embedded via Content-ID and
<img src="cid:...">, e.g. USPS Informed Delivery digests) were listed
under Attachments with a download button and never rendered in the body.
Two bugs combined; both are fixed here.
1. Misclassification: ImapClient classified any part with a filename as
an attachment, sweeping inline images (which carry a filename AND a
Content-ID under Content-Disposition: inline) into the list. A part is
now a downloadable attachment only when its disposition is attachment,
or it has a filename but no Content-ID; an inline image is collected
separately and excluded from the displayed list (AttachmentDao filters
contentId IS NULL). The Content-ID is read via MimePart.getContentID()
so it resolves from IMAP BODYSTRUCTURE rather than a per-part header
fetch that Angus leaves unpopulated.
2. No rendering path: HtmlBody's WebViewClient now overrides
shouldInterceptRequest to resolve cid:<id> to the matching part's
bytes (backing the CSP's existing cid: allowance). Content-ID is
threaded end-to-end through AttachmentPart, Attachment,
AttachmentEntity, and MailRepository.inlineImages(); ReaderViewModel
surfaces the cid->bytes map to the WebView.
Schema: adds attachments.contentId (v16 -> v17, MIGRATION_16_17).
Tests: MIME-part classification (inline+cid excluded, real/disposition/
filename-only kept), a GreenMail multipart/related round-trip, the
cid->bytes resolver, repository inlineImages(), the DAO display filter,
and the v16->v17 migration.
Closes#133
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The key-invalidation recovery restart was unreliable in two ways, both in
AppLockViewModel:
1. Same-process self-restart race: restartProcess() did
context.startActivity(...) immediately followed by Runtime.exit(0) in the
same process, so ActivityManager could schedule the relaunch into the
process being killed and drop it — the app just closed, recovering only on
the next manual launch. Fixed with a ProcessPhoenix-style separate-process
trampoline (RestartActivity in a distinct ":restart" process, driven by
ProcessRestarter): it kills the original process by PID and only then
relaunches, so the relaunch is issued from a process that survives the kill.
No new dependency; LibreMailApplication early-returns in the ":restart"
process so it runs no normal startup work.
2. Lost syncNow() enqueue: clearCacheAndRestart() enqueued the post-wipe
re-sync fire-and-forget, but WorkManager persists the WorkSpec
asynchronously on its serial task executor, so exiting raced that insert and
could drop the re-sync (now user-visible after #118: an empty mailbox until
the next periodic sync). syncNow() now returns its enqueue Operation, and
clearCacheAndRestart awaits it (bounded by a 5s timeout) before restarting,
so the WorkSpec is durably persisted first.
CLEAR_PENDING recovery-flag semantics are preserved; the cache wipe still
happens at cold start in DatabaseModule (unchanged).
Tests: JVM unit tests assert the enqueue Operation is awaited before the
restart is triggered (order) and that a timed-out enqueue still restarts;
SyncSchedulerTest pins syncNow() returning the enqueue Operation. The
separate-process kill/relaunch is device-only and noted for on-device
wipe+resync verification.
Closes#99
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DatabaseModule.provideDatabase ran the whole startup sequence with
runBlocking while Hilt constructed the singleton database — a DataStore
read, a Keystore op, a possible SQLCipher re-key conversion, and (since
#111) the cross-database AccountDataMigrator — synchronously on whichever
thread first injected it, which can be the main thread (jank / ANR).
Move that work behind DatabaseProvisioner.prepareCache(): a memoized,
mutex-guarded suspend that runs the same sequence, in the same order, on
the IO dispatcher. Both databases' Room builders now open through a
DeferredOpenHelperFactory whose delegate — and therefore the gate — is
materialised only when Room first OPENS the database, on its background
query executor, never at inject time. AccountDatabase's open gates on the
same prepareCache(), preserving the #111 migrate-before-open ordering that
the old construction-time dependency on LibreMailDatabase enforced.
Behaviour, ordering, and crash-safety are unchanged — only where and when
the work runs moved off the (possibly main) inject thread.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A message with several attachments used to render every AttachmentRow
stacked vertically, pushing the message body arbitrarily far down. Now
only the first attachment shows by default; when there is more than one,
the extras collapse behind a "See x more attachments" control that
expands and collapses with an animated, rotating chevron. A single
attachment renders exactly as before (no accordion).
The count uses a plurals resource (quantity one/other) so it reads
"See 1 more attachment" / "See 2 more attachments" correctly. The toggle
is one clickable Role.Button whose label and chevron contentDescription
expose the expanded state to screen readers. Download/open behavior of
each row is unchanged.
Refs #134
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Make BackupPolicy.EXCLUDED_DATABASE_PATHS the true single source of truth
by deriving it from DatabaseFiles.NAME and DatabaseFiles.ACCOUNTS_NAME plus
their SQLite sidecars via a new DatabaseFiles.fileNames() helper, instead of
a hand-maintained list. This adds libremail-accounts.db (accounts + encrypted
credentials, split into their own DB by #118/#111) to the never-back-up set,
matching the field's stated intent, so a newly added database can never
silently fall out of the exclusions again.
Also fix DatabaseFiles.clear to wipe the cache DB via
context.deleteDatabase(NAME), which additionally removes the -mj*
master-journal temp files the hand-rolled suffix list missed. It still wipes
ONLY the cache DB (NAME) and never the accounts DB (ACCOUNTS_NAME), preserving
the sign-in-survives-cache-wipe separation from #111.
Update the backup XML comments (data_extraction_rules.xml, backup_rules.xml)
to note libremail-accounts.db is also kept off-device by the strict include-
allowlist, and extend the tests to assert the accounts DB is covered by the
exclusion SoT and that the derivation stays in lockstep with the XML resources.
There is no active backup leak today: the XML is a strict include-allowlist,
so the accounts DB was already excluded by omission. This closes the SoT drift
#118 introduced and the -mj* gap, so the security posture no longer depends on
the allowlist staying strict by luck.
Closes#103
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Recipient autocomplete's READ_CONTACTS permission was requested lazily on
every compose-screen open (a LaunchedEffect(Unit)), re-prompting users who
had declined. Move the request to a dedicated, skippable onboarding step and
add a Settings entry to turn it on later, each with an in-context rationale.
- #127: new skippable ONBOARDING_CONTACTS step (mirrors the battery step),
requested once. ComposeScreen no longer prompts; it only reads the current
grant on resume, so a grant made later (e.g. from Settings) still takes
effect the next time compose opens.
- #128: the onboarding step and the Settings request show a short rationale
(contacts are used only for on-device autocomplete, never uploaded) and
handle shouldShowRequestPermissionRationale so a re-request explains itself.
docs/play-permissions.md updated to match.
- #129: Settings -> Contacts -> Recipient autocomplete reflects on / off /
blocked-in-settings; requests in-app when grantable, deep-links to the app's
system settings when permanently denied.
Graceful degradation is preserved: ContactsRepository.search still runCatch-es,
ComposeViewModel.searchContacts() still guards on contactsAllowed, and the
suggestion list still renders only when non-empty.
Adds a pure ContactPermissionDecision (JVM unit-tested), extends the onboarding
view-model tests, and adds Compose UI tests for the onboarding step
(skip / grant / deny / rationale) and the Settings row states.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Prototype the per-account connection reuse the #125 investigation recommended
and deferred, behind an OFF-by-default flag so it cannot destabilize `main`.
- ImapConnectionCache: keeps one authenticated Store alive per account, guarded
by a per-account mutex, keyed by connection identity (not the rotating
secret), with lazy catch-and-retry-once stale handling. No eviction policy
yet beyond an explicit closeReusedConnections() hook.
- ImapClient gains a `reuseConnections` flag (default false via the @Inject
no-arg constructor). With it off, withStore is byte-for-byte the previous
connect + LOGOUT-per-call; with it on, calls borrow the kept-alive Store.
- ImapFolderOpenLatencyTest flips the flag on: the same real-IMAP operations
that cost N connections / N LOGINs collapse to 1 connection / 1 LOGIN, with
the necessary per-open EXAMINE unchanged (proven via CountingImapProxy +
GreenMail; localhost is ~0 RTT so this proves structure, not wall-clock).
- docs/perf/issue-125-connection-reuse-spike.md: prototype design, the
flag-off-vs-on proof, per-decision trade-offs, and the refined real-device
validation plan. References #125; does not close it (needs device validation).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The unified inbox query (WHERE folder = ?, no accountId) has no folder-leading
index, so it scans in timestamp order and materializes the whole unified inbox
(~4k rows at a 20k cache) into memory on every emission. Apply Paging 3 to the
unified browse path so query, mapping, and recomposition cost scale with the
visible window, not the total cache.
- MessageDao.pagingUnifiedFolderSummaries: a PagingSource over the folder's
synced rows (inInbox = 1); unified search keeps the whole-folder query so it
can still surface transient server-search hits.
- MailRepository.pagedUnifiedFolderMessages: a Pager (pageSize 40, initialLoad
120, no placeholders) mapping summaries to domain.
- MailboxViewModel.pagedMessages: paged while browsing the unified inbox, else
empty; the messages list flow stays empty in that state so the whole cache is
never materialized. Selection captures each row's accountId at tap time, so
"Move" still resolves the selection's account without an in-memory list.
- MailboxScreen renders the unified browse list via collectAsLazyPagingItems;
per-account and search views render the flat list unchanged (issue #86 stays
flat).
Profiling (docs/perf/issue-124-unified-inbox-paging.md) on an api29 emulator:
current whole-inbox first-emit ~24.6 ms at a 20k cache vs. the paged first page
~6.8 ms and flat regardless of cache size (~3.6x). EXPLAIN QUERY PLAN shows the
paged query still stops early on the existing timestamp index, so no
(folder, ...) index and no schema migration are added.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Device upgrade testing surfaced a crash: on a cache last written before
v13, account_settings has 4 columns (accountId, signature,
signatureEnabled, notificationsEnabled) but the destination table has 6
(retentionCount/retentionMonths were added at v13). The migrator ran
`INSERT OR IGNORE INTO account_settings SELECT * FROM cache...`, which
supplied 4 values for 6 columns and threw SQLiteException — and because
the done-flag is only set after a successful copy, every launch re-ran
and re-crashed (crash loop).
AccountDataMigrator now copies each table by the column names present in
BOTH the freshly-created destination and the (possibly older) source, so
columns the source lacks take the destination's defaults instead of
overflowing the value list. Verified on-device: the upgrade migrates a
pre-v13 install cleanly and the account stays signed in (sync/backfill
workers run). Regression test seeds a v12 cache and asserts the copy.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions[bot]
JMR-devandClaude Opus 4.8