Source: whole-repo multi-agent review, 2026-07-09 (run wf_b41de68c-e85). The run was cut short by usage limits before its verification pass, so every finding below is an unverified finder candidate — validate each against the current code before implementing. Findings are listed medium first, then low. Critical/high candidates from the same run were verified separately and have their own issues.
Medium (11)
app/src/main/kotlin/org/libremail/ui/LibreMailApp.kt:78 — A mailto:/ACTION_SEND compose intent bypasses the GPL license-acceptance gate and the whole onboarding flow: the pendingCompose LaunchedEffect navigates to the compose route with no start-destination guard, unlike the pendingOpenMessageId effect right below it which checks start != Routes.ONBOARDING.
severity: medium · category: security · angle: security
Fresh install, license not yet accepted: LibreMailApp starts on Routes.ONBOARDING showing LicenseScreen. Any app fires an ACTION_VIEW mailto: or ACTION_SEND intent; MainActivity.kt:124 unconditionally parses it into pendingCompose, and LibreMailApp.kt:76-88 immediately navigates to Routes.compose(...) on top of the license screen — the user reaches (and uses) the compose UI without ever accepting the license, in a zero-account state onboarding was supposed to prevent.
app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:43 — Inbox identity is a copy-pasted magic string (const INBOX = "INBOX" here and again in MailSyncer.kt:210) compared case-SENSITIVELY in the UI (MailboxScreen.kt:268/276/280, FolderDrawer.kt:79, refresh() line 440), while the protocol/domain layers deliberately match it case-INSENSITIVELY (ImapClient.kt:161, withInbox line 471, Folder.kt:67) — the mechanism should be FolderRole.INBOX or one normalized identity at the sync boundary.
severity: medium · category: altitude · angle: altitude
Against a server whose LIST returns the inbox as "Inbox" (IMAP spec makes the INBOX atom case-insensitive, so servers may report any casing): withInbox/roleFor recognize it (ignoreCase) so the drawer shows one Inbox row with fullName "Inbox", but tapping it sets selectedFolder="Inbox" while background/push sync (MailSyncer's own "INBOX" constant) stores message rows under folder="INBOX". The per-account inbox pager queries folder="Inbox" and misses every background/push-synced row (rows also get duplicated under two folder keys), and all inbox-gated UI — the account filter chips, Drafts/Outbox entries, and the unified-inbox highlight — silently disappears because selectedFolder == "INBOX" is false. Each new consumer (next provider, notifications, widgets) must re-learn which casing to compare.
app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:222 — currentFolderRole resolves the selected folder's role against the DRAWER account's folder list (folders <- drawerAccount), which can be a different account than the one whose mail is shown, so Delete's permanent-vs-trash decision and the CAB's Archive/Spam visibility use the wrong account's folder tree.
severity: medium · category: correctness · angle: logic · flagged by 2 finder(s)
Two accounts A (Gmail, trash='[Gmail]/Trash') and B. User selects A's Trash (selectFolder), reopens the drawer, switches the drawer account dropdown to B (setDrawerAccount only changes explicitDrawerAccountId, not the shown mail), closes the drawer, long-presses messages and taps Delete. currentFolderRole looks up '[Gmail]/Trash' in B's folder list -> null -> requestDelete builds a non-permanent PendingAction ('move to trash' dialog) and confirmPending calls trash() instead of expunge(): moveByRole resolves A's trash as the destination == source folder and (unlike moveToFolder) does NOT skip same-folder moves, so it copy+deletes the messages onto themselves — the user's 'delete from trash' silently does nothing permanent. The inverse collision (fullName matching a TRASH/SPAM-role folder in the drawer account while the shown folder is normal) mislabels a permanent expunge as a move. The app-bar title (MailboxScreen.kt:210) breaks the same way.
app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:344 — The keep-alive PagingDataPresenter collects the cached pager for the ViewModel's entire lifetime, so every Room messages-table invalidation reloads a full paged generation even while the app is backgrounded or the user is on another tab — unlike every other flow in this class, which is gated by WhileSubscribed(5s).
severity: medium · category: efficiency · angle: efficiency
User opens the mailbox once (creating the ViewModel), then backgrounds the app while a large initial backfill syncs thousands of messages in batches: each insert batch invalidates the Room PagingSource, the keep-alive presenter drives the new generation's initial load, and the device re-runs the paged folder query (multi-page window) dozens/hundreds of times with the screen off — pure wasted DB I/O and CPU competing with the sync itself. The documented goal (issue #219: keep the pager warm across a reader visit) only needs the collector alive while the mailbox/reader UI is around; gating the keep-alive on a subscription signal with a timeout (WhileSubscribed-style, tied to the screen's collector) or cancelling it in a lifecycle-aware way gets the same no-flash return without ViewModel-lifetime requerying.
app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:365 — The server-search trigger applies filter{length>=2} BEFORE distinctUntilChanged and keys distinctness on the query text only, so closing and reopening search with the same query — or changing the account/folder filter mid-search — never re-fires the server search, silently omitting server-only results.
severity: medium · category: correctness · angle: invariants · flagged by 5 finder(s)
User searches "invoice" (server hits fetched into cache), taps close (closeSearch sets query to "" and clearSearchResults deletes the transient hits), reopens search and types "invoice" again: the intermediate "" is dropped by the length filter, so distinctUntilChanged sees consecutive equal "invoice" values and suppresses the emission — searchServer never runs and the previously-cleared server-only matches are permanently missing from the results. Same suppression when the user switches the account chip during an active search (account/folder are read at fire time but are not part of the distinct key), so the new account's server hits are never fetched.
app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:378 — selectFolder feeds the server-listed folder fullName straight into sync and the pager while the rest of the app keys the inbox on the literal 'INBOX' (syncAll/syncAccount/IDLE, the default _selectedFolder, the unified pager, and MailboxScreen's selectedFolder == INBOX gates), so a server that LISTs the inbox with different casing (e.g. Exchange's 'Inbox') stores the same messages under two folder keys.
severity: medium · category: correctness · angle: crossfile
Account on a server whose LIST returns 'Inbox' (ImapClient.listFolders preserves the reported case; withInbox only compares case-insensitively). Background syncAll writes rows with id 'acct:INBOX:uid' / folder='INBOX'; tapping the drawer's inbox calls selectFolder(acct, 'Inbox'), which syncs and pages folder='Inbox', creating duplicate rows 'acct:Inbox:uid'. The unified All-Inboxes view (folder='INBOX') then diverges from the per-account inbox, reading a message in one view leaves its twin unread in the other (phantom unread badges via observeUnreadCounts grouping on folder), and the account-filter/drafts/outbox rows gated on selectedFolder == INBOX disappear when browsing the account's actual inbox.
app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:382 — selectFolder stores and queries the server-reported inbox fullName verbatim (e.g. "Inbox"), while sync, unified queries, and unread counts use the canonical literal "INBOX" (MailSyncer.INBOX), splitting one inbox into two case-distinct folder keys on servers whose LIST reports a non-uppercase INBOX.
severity: medium · category: correctness · angle: logic
A server reports the inbox as "Inbox" in LIST (ImapClient.listFolders keeps folder.fullName verbatim, line 153; the code itself hedges with equals(INBOX, ignoreCase = true) at MailboxViewModel.kt:471 acknowledging case varies). Background sync writes message rows with folder="INBOX", but tapping the drawer's inbox row calls selectFolder(acct, "Inbox"): the SQLite BINARY-collated folder='Inbox' query shows none of the already-synced INBOX rows (empty folder / spinner), then syncFolder("Inbox") re-fetches the same messages under a second folder key — duplicate rows and doubled unread counts — and folderUnreadCounts["Inbox"] misses the counts recorded under "INBOX", so the drawer badge is wrong.
app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:388 — selectFolder discards the Result returned by mailSyncer.syncFolder, so a failed initial sync of a never-synced folder is silently swallowed — unlike refresh(), which routes the same call's failure into the error snackbar.
severity: medium · category: correctness · angle: logic · flagged by 4 finder(s)
Device offline (or server error/throttle): user opens a folder they have not synced before. syncFolder returns Result.failure, the finally block clears isSyncingFolder, and MailboxScreen's empty-state logic (loadState settled, isSyncingFolder false) renders 'No messages' with no error indication — defeating issue #149's stated goal of only showing the empty state once the fetch 'confirms the folder really is empty'. The user has no way to distinguish an empty folder from a failed fetch; a pull-to-refresh in the same conditions would show 'Sync failed'.
app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:392 — The stale-sync guard compares latestFolderSelection to the sync's key by Pair value-equality, so an older still-running sync for the SAME folder passes the guard and clears _isSyncingFolder while a newer sync for that folder is still in flight; selectUnifiedInbox/selectAccount also never reset the flag.
severity: medium · category: concurrency · angle: concurrency
User taps an uncached folder (sync1 starts, slow/failing network — e.g. Gmail throttling at 30s+), navigates away and re-taps the same folder (sync2 starts; key2 == key1 by value). sync1's timeout fires first, its finally sees latestFolderSelection == key and sets _isSyncingFolder=false while sync2 is still fetching, so MailboxScreen shows the "No messages" empty state on a folder whose initial fetch is still running — the exact issue-#149 regression the guard was added to prevent. Conversely, selecting a folder then tapping "All inboxes" leaves _isSyncingFolder=true, holding a bogus spinner over an empty unified inbox until the stale sync ends.
app/src/main/kotlin/org/libremail/ui/reader/HtmlBody.kt:84 — The reader WebView never disables cookies, so once remote images are enabled (per-message tap or the global loadRemoteImages setting) tracking domains can set persistent cookies in the app-global WebView cookie jar and correlate the user across messages, senders, and time — undermining the tracking-pixel protection this file documents.
severity: medium · category: security · angle: security
User has 'load remote images by default' on (or taps Show images on one marketing email). An response sets a Set-Cookie identity cookie; CookieManager (never configured anywhere in app/src/main — no setAcceptCookie(false), no clearing) accepts and persists it. Every later HTML email referencing tracker.example sends the cookie back, letting the tracker link message opens to one stable identity across senders and sessions. LOAD_NO_CACHE and blockNetworkLoads do not mitigate this.
app/src/main/kotlin/org/libremail/ui/reader/HtmlBody.kt:109 — HtmlBody's hardened-WebView behaviors — the shouldOverrideUrlLoading scheme allowlist and hasGesture gate (anti intent://-auto-launch), blockNetworkLoads tracking-pixel blocking, the cid: shouldInterceptRequest wiring, and the lastLoaded scroll-preserving reload guard — have no test at any level: ReaderScreenJvmTest deliberately avoids instantiating HtmlBody (blank HTML body, documented WebView caveat) and defers to 'the instrumented ReaderScreenTest' as the on-device E2E, but every message in ReaderScreenTest is isHtml = false, so no test ever renders an HTML body.
severity: medium · category: test-gap · angle: tests
A refactor drops or reorders if (!request.hasGesture()) return true or widens the scheme check in shouldOverrideUrlLoading (only the pure helpers cidKey/resolveInlineImage/wrapHtml are unit-tested, not the WebViewClient using them); every unit, JVM-Compose, and instrumented test stays green, and a malicious email with a meta-refresh to an intent:// or market: URI auto-launches arbitrary activities the moment the user opens it — likewise a regression flipping blockNetworkLoads would silently re-enable tracking pixels.
Low (13)
app/src/main/kotlin/org/libremail/contacts/ContactsRepository.kt:23 — The contacts LIKE pattern interpolates the raw user query without escaping SQL wildcard characters % and _, so queries containing them match unintended contacts.
In recipient autocomplete the user types "a_b" (or pastes something containing %): '_' matches any single character and '%' matches any run, so "a_b" suggests every contact containing "axb", "a1b", etc., and a query like "100%" degenerates to matching every contact containing "100" followed by anything — wrong suggestions one keystroke away from picking the wrong recipient. The pattern needs LIKE ... ESCAPE with % / _ escaped in the query.
app/src/main/kotlin/org/libremail/contacts/ContactsRepository.kt:26 — ContactsRepository.search wraps the entire ContactsContract query in runCatching and silently discards the exception with no AppLog call, violating CLAUDE.md's DoD requirement of logging on error/fallback paths.
READ_CONTACTS is revoked mid-session (SecurityException) or the contacts provider throws (getColumnIndexOrThrow / provider crash): search() silently returns an empty list, recipient autocomplete just stops suggesting contacts, and a user's debug report contains no record that contact lookups are failing - indistinguishable from 'no matching contacts'. A PII-free AppLog.w on the caught throwable (auto-scrubbed per CLAUDE.md) is required here.
app/src/main/kotlin/org/libremail/ui/AppViewModel.kt:32 — AppViewModel's documented 'only the first determination is used' contract (take(1) on startDestination, and the same on licenseAccepted at line 45) is untested: all four AppViewModelTest cases feed single-emission flowOf(), so deleting take(1) passes the whole suite.
severity: low · category: test-gap · angle: tests
take(1) is removed as apparently redundant; tests stay green, but observeAccounts re-emits when the user adds their first account mid-onboarding, startDestination flips from ONBOARDING to MAILBOX, and the NavHost tears down the in-progress onboarding flow (contacts/battery steps and the add-another screen are skipped) — exactly the regression the comment on lines 22-24 warns about.
app/src/main/kotlin/org/libremail/ui/AppViewModel.kt:43 — licenseAccepted only selects the onboarding graph's start destination, so a user who already has accounts (startDestination = MAILBOX) but has never accepted the GPL-3.0 license (upgrade from a pre-#172 build) bypasses the mandatory license gate entirely.
Existing install with accounts upgrades to a build containing the #172 license gate: settingsRepository.settings.licenseAccepted is false, but AppViewModel resolves startDestination = Routes.MAILBOX because accounts is non-empty, and LibreMailApp only consults licenseAlreadyAccepted inside onboardingGraph's startDestination — which is never navigated to. The user is never shown the license screen the gate declares mandatory ('the user must scroll through the GPL-3.0 text and tap Agree before reaching anything else', Routes.kt:36-39), and the accepted flag stays false forever.
app/src/main/kotlin/org/libremail/ui/mailbox/MailboxScreen.kt:429 — OutboxEntry is a line-for-line copy of DraftsEntry (MailboxScreen.kt:410-426) differing only in the icon vector and string resource; they should be one parameterized row composable.
severity: low · category: reuse · angle: reuse
The clickable-row layout (padding, icon tint, 16dp spacer, titleMedium primary-colored text) is duplicated. A change to one entry — e.g. adding a trailing chevron, changing padding, or adding a contentDescription for accessibility — must be manually mirrored in the other; if it isn't, the Drafts and Outbox rows stacked directly on top of each other in the inbox drift visibly apart. Replace both with a single MailboxShortcutEntry(icon, text, onClick).
app/src/main/kotlin/org/libremail/ui/mailbox/MailboxScreen.kt:524 — MessageRow memoizes the relative timestamp with remember(message.timestampMillis), whose key never changes, so the displayed age ("2 minutes ago") freezes for as long as the row stays in composition.
User leaves the inbox open (or the mailbox stays in the back stack while reading mail): a message that arrived "1 minute ago" still says "1 minute ago" an hour later, because DateUtils.getRelativeTimeSpanString is only re-run when timestampMillis changes — which it never does. Even pull-to-refresh doesn't fix rows whose Message content is unchanged, since Paging emits equal items and the remember key is identical. Time is an input here and cannot be a remember key; the value needs periodic invalidation (e.g. a minute ticker) or at least recomputation on refresh.
app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:287 — requestDelete() derives the permanent-vs-trash decision from currentFolderRole, which resolves selectedFolder against the DRAWER account's folder list (folders <- drawerAccount), not the account whose mail is shown — after setDrawerAccount() points the drawer elsewhere, the role lookup is done against the wrong account and the delete confirmation/action can be the wrong operation.
User browses account A's Spam folder (fullName "Spam", role SPAM -> delete should permanently expunge), opens the drawer and switches the drawer to account B (setDrawerAccount, no folder tap), closes the drawer, long-press-selects messages and taps Delete: currentFolderRole now looks up "Spam" in B's folder list (miss or NORMAL role) -> permanent=false -> the dialog says 'move to trash' and trash() runs instead of expunge(). With provider folder-name collisions in the opposite direction (drawer account maps the name to TRASH/SPAM, shown account doesn't) the same mismatch selects the permanent expunge path for messages in a non-trash folder.
app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:426 — closeSearch()'s clearSearchResults() races an in-flight searchServer() round-trip: hits inserted after the clear survive as orphaned transient rows, defeating the stated invariant that transient hits never outlive the search session.
User types a query (>=2 chars); after the 400 ms debounce searchServer starts its multi-second IMAP SEARCH; the user closes search while it is in flight. clearSearchResults() deletes existing transient rows, then the still-running searchServer completes and inserts its hits — transient rows now persist in the cache with no active search, surfacing as stale/incorrectly-cached entries in the next search that matches them (and accumulating until some later closeSearch happens to run after all fetches settle).
app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:440 — refresh() has a dead first branch: accountId == null && folder == INBOX and the final else both call mailSyncer.syncAll(), so the three-way if/else-if/else collapses to if (accountId != null) syncFolder(accountId, folder) else syncAll().
The redundant branch implies a behavioral distinction between 'unified inbox' and 'unified non-inbox folder' refresh that does not exist. A maintainer later changing the unified-folder refresh path (e.g. to sync only that folder across accounts) will plausibly edit only one of the two textually separate syncAll() branches, introducing a real divergence the current structure invites; the accompanying comment ('a specific folder refreshes just that folder') already mis-describes the accountId==null non-INBOX case. Collapse to the two-way if.
app/src/main/kotlin/org/libremail/ui/reader/HtmlBody.kt:117 — shouldOverrideUrlLoading swallows startActivity failures via runCatching with no AppLog, and the security-relevant blocked-navigation branches (non-http/https/mailto scheme at line 115, no-gesture at line 116) also return silently with no logging, violating CLAUDE.md's DoD error/fallback-path logging rule.
On a managed/minimal device with no browser (or a disabled handler), tapping an http link in an email throws ActivityNotFoundException, which runCatching discards: the tap does nothing, and the debug report shows no trace. Likewise a malicious email attempting an intent:/market: auto-launch is (correctly) blocked, but the block leaves no PII-free log line (e.g. scheme only), so the defensive path is invisible when diagnosing 'links do nothing' reports.
app/src/main/kotlin/org/libremail/ui/reader/ReaderScreen.kt:424 — ReaderScreen's Header re-implements MailboxScreen's private Avatar composable inline (same 40dp CircleShape primaryContainer box and identical initial-derivation expression sender.trim().firstOrNull()?.uppercase() ?: "?") instead of sharing one Avatar composable.
severity: low · category: reuse · angle: reuse
The avatar rendering and initial-derivation logic exist twice (MailboxScreen.kt:558-573 Avatar vs ReaderScreen.kt:416-428). A fix to one copy — e.g. handling senders whose display name starts with a quote character or emoji surrogate pair (where firstOrNull() yields half a code point), or a size/color restyle — silently leaves the other screen rendering a different avatar for the same sender. Extract MailboxScreen's Avatar into a shared composable and call it from Header.
app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt:121 — Every emission of the Room attachments flow re-runs downloadedAttachmentParts — a second DAO query plus a File.exists()/length() disk stat per attachment — even though Room invalidates the whole attachments table, not just this message's rows.
Reader is open on a message while background aggressive sync (prefetchMessage) writes attachment rows for other messages: each table write re-emits attachmentDao.observeForMessage(messageId), and the collect body re-executes attachmentDao.getForMessage plus per-file disk stats for a message whose attachment state hasn't changed — repeated redundant DB+filesystem I/O for the whole time the reader is open during a sync. Cheaper: compute the downloaded set once per distinct attachments list (e.g. distinctUntilChanged() on the flow, or fold downloadedAttachmentParts into the same DAO mapping) — incremental updates on actual downloads are already handled in downloadAttachment.
app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt:154 — toggleStar's failure rollback restores the negation of ITS OWN captured 'starred' rather than reconciling with the latest attempted state, so two rapid toggles whose persists both fail can leave the shown star opposite to the store.
Message unstarred. Tap star (shown true, persist(true) in flight), tap again (shown false, persist(false) in flight). persist(true) fails first -> rollback sets isStarred=false (ok); then persist(false) fails -> rollback sets isStarred=!false=true. UI now shows starred while the store never accepted any change (still unstarred); the mismatch persists until the message is reopened.
Source: whole-repo multi-agent review, 2026-07-09 (run `wf_b41de68c-e85`). The run was cut short by usage limits before its verification pass, so every finding below is an **unverified finder candidate** — validate each against the current code before implementing. Findings are listed medium first, then low. Critical/high candidates from the same run were verified separately and have their own issues.
## Medium (11)
### `app/src/main/kotlin/org/libremail/ui/LibreMailApp.kt:78` — A mailto:/ACTION_SEND compose intent bypasses the GPL license-acceptance gate and the whole onboarding flow: the pendingCompose LaunchedEffect navigates to the compose route with no start-destination guard, unlike the pendingOpenMessageId effect right below it which checks start != Routes.ONBOARDING.
- severity: **medium** · category: `security` · angle: `security`
> Fresh install, license not yet accepted: LibreMailApp starts on Routes.ONBOARDING showing LicenseScreen. Any app fires an ACTION_VIEW mailto: or ACTION_SEND intent; MainActivity.kt:124 unconditionally parses it into pendingCompose, and LibreMailApp.kt:76-88 immediately navigates to Routes.compose(...) on top of the license screen — the user reaches (and uses) the compose UI without ever accepting the license, in a zero-account state onboarding was supposed to prevent.
### `app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:43` — Inbox identity is a copy-pasted magic string (const INBOX = "INBOX" here and again in MailSyncer.kt:210) compared case-SENSITIVELY in the UI (MailboxScreen.kt:268/276/280, FolderDrawer.kt:79, refresh() line 440), while the protocol/domain layers deliberately match it case-INSENSITIVELY (ImapClient.kt:161, withInbox line 471, Folder.kt:67) — the mechanism should be FolderRole.INBOX or one normalized identity at the sync boundary.
- severity: **medium** · category: `altitude` · angle: `altitude`
> Against a server whose LIST returns the inbox as "Inbox" (IMAP spec makes the INBOX atom case-insensitive, so servers may report any casing): withInbox/roleFor recognize it (ignoreCase) so the drawer shows one Inbox row with fullName "Inbox", but tapping it sets selectedFolder="Inbox" while background/push sync (MailSyncer's own "INBOX" constant) stores message rows under folder="INBOX". The per-account inbox pager queries folder="Inbox" and misses every background/push-synced row (rows also get duplicated under two folder keys), and all inbox-gated UI — the account filter chips, Drafts/Outbox entries, and the unified-inbox highlight — silently disappears because selectedFolder == "INBOX" is false. Each new consumer (next provider, notifications, widgets) must re-learn which casing to compare.
### `app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:222` — currentFolderRole resolves the selected folder's role against the DRAWER account's folder list (folders <- drawerAccount), which can be a different account than the one whose mail is shown, so Delete's permanent-vs-trash decision and the CAB's Archive/Spam visibility use the wrong account's folder tree.
- severity: **medium** · category: `correctness` · angle: `logic` · flagged by 2 finder(s)
> Two accounts A (Gmail, trash='[Gmail]/Trash') and B. User selects A's Trash (selectFolder), reopens the drawer, switches the drawer account dropdown to B (setDrawerAccount only changes explicitDrawerAccountId, not the shown mail), closes the drawer, long-presses messages and taps Delete. currentFolderRole looks up '[Gmail]/Trash' in B's folder list -> null -> requestDelete builds a non-permanent PendingAction ('move to trash' dialog) and confirmPending calls trash() instead of expunge(): moveByRole resolves A's trash as the destination == source folder and (unlike moveToFolder) does NOT skip same-folder moves, so it copy+deletes the messages onto themselves — the user's 'delete from trash' silently does nothing permanent. The inverse collision (fullName matching a TRASH/SPAM-role folder in the drawer account while the shown folder is normal) mislabels a permanent expunge as a move. The app-bar title (MailboxScreen.kt:210) breaks the same way.
### `app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:344` — The keep-alive PagingDataPresenter collects the cached pager for the ViewModel's entire lifetime, so every Room messages-table invalidation reloads a full paged generation even while the app is backgrounded or the user is on another tab — unlike every other flow in this class, which is gated by WhileSubscribed(5s).
- severity: **medium** · category: `efficiency` · angle: `efficiency`
> User opens the mailbox once (creating the ViewModel), then backgrounds the app while a large initial backfill syncs thousands of messages in batches: each insert batch invalidates the Room PagingSource, the keep-alive presenter drives the new generation's initial load, and the device re-runs the paged folder query (multi-page window) dozens/hundreds of times with the screen off — pure wasted DB I/O and CPU competing with the sync itself. The documented goal (issue #219: keep the pager warm across a reader visit) only needs the collector alive while the mailbox/reader UI is around; gating the keep-alive on a subscription signal with a timeout (WhileSubscribed-style, tied to the screen's collector) or cancelling it in a lifecycle-aware way gets the same no-flash return without ViewModel-lifetime requerying.
### `app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:365` — The server-search trigger applies filter{length>=2} BEFORE distinctUntilChanged and keys distinctness on the query text only, so closing and reopening search with the same query — or changing the account/folder filter mid-search — never re-fires the server search, silently omitting server-only results.
- severity: **medium** · category: `correctness` · angle: `invariants` · flagged by 5 finder(s)
> User searches "invoice" (server hits fetched into cache), taps close (closeSearch sets query to "" and clearSearchResults deletes the transient hits), reopens search and types "invoice" again: the intermediate "" is dropped by the length filter, so distinctUntilChanged sees consecutive equal "invoice" values and suppresses the emission — searchServer never runs and the previously-cleared server-only matches are permanently missing from the results. Same suppression when the user switches the account chip during an active search (account/folder are read at fire time but are not part of the distinct key), so the new account's server hits are never fetched.
### `app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:378` — selectFolder feeds the server-listed folder fullName straight into sync and the pager while the rest of the app keys the inbox on the literal 'INBOX' (syncAll/syncAccount/IDLE, the default _selectedFolder, the unified pager, and MailboxScreen's selectedFolder == INBOX gates), so a server that LISTs the inbox with different casing (e.g. Exchange's 'Inbox') stores the same messages under two folder keys.
- severity: **medium** · category: `correctness` · angle: `crossfile`
> Account on a server whose LIST returns 'Inbox' (ImapClient.listFolders preserves the reported case; withInbox only compares case-insensitively). Background syncAll writes rows with id 'acct:INBOX:uid' / folder='INBOX'; tapping the drawer's inbox calls selectFolder(acct, 'Inbox'), which syncs and pages folder='Inbox', creating duplicate rows 'acct:Inbox:uid'. The unified All-Inboxes view (folder='INBOX') then diverges from the per-account inbox, reading a message in one view leaves its twin unread in the other (phantom unread badges via observeUnreadCounts grouping on folder), and the account-filter/drafts/outbox rows gated on selectedFolder == INBOX disappear when browsing the account's actual inbox.
### `app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:382` — selectFolder stores and queries the server-reported inbox fullName verbatim (e.g. "Inbox"), while sync, unified queries, and unread counts use the canonical literal "INBOX" (MailSyncer.INBOX), splitting one inbox into two case-distinct folder keys on servers whose LIST reports a non-uppercase INBOX.
- severity: **medium** · category: `correctness` · angle: `logic`
> A server reports the inbox as "Inbox" in LIST (ImapClient.listFolders keeps folder.fullName verbatim, line 153; the code itself hedges with equals(INBOX, ignoreCase = true) at MailboxViewModel.kt:471 acknowledging case varies). Background sync writes message rows with folder="INBOX", but tapping the drawer's inbox row calls selectFolder(acct, "Inbox"): the SQLite BINARY-collated folder='Inbox' query shows none of the already-synced INBOX rows (empty folder / spinner), then syncFolder("Inbox") re-fetches the same messages under a second folder key — duplicate rows and doubled unread counts — and folderUnreadCounts["Inbox"] misses the counts recorded under "INBOX", so the drawer badge is wrong.
### `app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:388` — selectFolder discards the Result<Int> returned by mailSyncer.syncFolder, so a failed initial sync of a never-synced folder is silently swallowed — unlike refresh(), which routes the same call's failure into the error snackbar.
- severity: **medium** · category: `correctness` · angle: `logic` · flagged by 4 finder(s)
> Device offline (or server error/throttle): user opens a folder they have not synced before. syncFolder returns Result.failure, the finally block clears isSyncingFolder, and MailboxScreen's empty-state logic (loadState settled, isSyncingFolder false) renders 'No messages' with no error indication — defeating issue #149's stated goal of only showing the empty state once the fetch 'confirms the folder really is empty'. The user has no way to distinguish an empty folder from a failed fetch; a pull-to-refresh in the same conditions would show 'Sync failed'.
### `app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:392` — The stale-sync guard compares latestFolderSelection to the sync's key by Pair value-equality, so an older still-running sync for the SAME folder passes the guard and clears _isSyncingFolder while a newer sync for that folder is still in flight; selectUnifiedInbox/selectAccount also never reset the flag.
- severity: **medium** · category: `concurrency` · angle: `concurrency`
> User taps an uncached folder (sync1 starts, slow/failing network — e.g. Gmail throttling at 30s+), navigates away and re-taps the same folder (sync2 starts; key2 == key1 by value). sync1's timeout fires first, its finally sees latestFolderSelection == key and sets _isSyncingFolder=false while sync2 is still fetching, so MailboxScreen shows the "No messages" empty state on a folder whose initial fetch is still running — the exact issue-#149 regression the guard was added to prevent. Conversely, selecting a folder then tapping "All inboxes" leaves _isSyncingFolder=true, holding a bogus spinner over an empty unified inbox until the stale sync ends.
### `app/src/main/kotlin/org/libremail/ui/reader/HtmlBody.kt:84` — The reader WebView never disables cookies, so once remote images are enabled (per-message tap or the global loadRemoteImages setting) tracking domains can set persistent cookies in the app-global WebView cookie jar and correlate the user across messages, senders, and time — undermining the tracking-pixel protection this file documents.
- severity: **medium** · category: `security` · angle: `security`
> User has 'load remote images by default' on (or taps Show images on one marketing email). An <img src="https://tracker.example/p.gif"> response sets a Set-Cookie identity cookie; CookieManager (never configured anywhere in app/src/main — no setAcceptCookie(false), no clearing) accepts and persists it. Every later HTML email referencing tracker.example sends the cookie back, letting the tracker link message opens to one stable identity across senders and sessions. LOAD_NO_CACHE and blockNetworkLoads do not mitigate this.
### `app/src/main/kotlin/org/libremail/ui/reader/HtmlBody.kt:109` — HtmlBody's hardened-WebView behaviors — the shouldOverrideUrlLoading scheme allowlist and hasGesture gate (anti intent://-auto-launch), blockNetworkLoads tracking-pixel blocking, the cid: shouldInterceptRequest wiring, and the lastLoaded scroll-preserving reload guard — have no test at any level: ReaderScreenJvmTest deliberately avoids instantiating HtmlBody (blank HTML body, documented WebView caveat) and defers to 'the instrumented ReaderScreenTest' as the on-device E2E, but every message in ReaderScreenTest is isHtml = false, so no test ever renders an HTML body.
- severity: **medium** · category: `test-gap` · angle: `tests`
> A refactor drops or reorders `if (!request.hasGesture()) return true` or widens the scheme check in shouldOverrideUrlLoading (only the pure helpers cidKey/resolveInlineImage/wrapHtml are unit-tested, not the WebViewClient using them); every unit, JVM-Compose, and instrumented test stays green, and a malicious email with a meta-refresh to an intent:// or market: URI auto-launches arbitrary activities the moment the user opens it — likewise a regression flipping blockNetworkLoads would silently re-enable tracking pixels.
## Low (13)
### `app/src/main/kotlin/org/libremail/contacts/ContactsRepository.kt:23` — The contacts LIKE pattern interpolates the raw user query without escaping SQL wildcard characters % and _, so queries containing them match unintended contacts.
- severity: **low** · category: `correctness` · angle: `pitfalls`
> In recipient autocomplete the user types "a_b" (or pastes something containing %): '_' matches any single character and '%' matches any run, so "a_b" suggests every contact containing "axb", "a1b", etc., and a query like "100%" degenerates to matching every contact containing "100" followed by anything — wrong suggestions one keystroke away from picking the wrong recipient. The pattern needs LIKE ... ESCAPE with % / _ escaped in the query.
### `app/src/main/kotlin/org/libremail/contacts/ContactsRepository.kt:26` — ContactsRepository.search wraps the entire ContactsContract query in runCatching and silently discards the exception with no AppLog call, violating CLAUDE.md's DoD requirement of logging on error/fallback paths.
- severity: **low** · category: `conventions` · angle: `conventions`
> READ_CONTACTS is revoked mid-session (SecurityException) or the contacts provider throws (getColumnIndexOrThrow / provider crash): search() silently returns an empty list, recipient autocomplete just stops suggesting contacts, and a user's debug report contains no record that contact lookups are failing - indistinguishable from 'no matching contacts'. A PII-free AppLog.w on the caught throwable (auto-scrubbed per CLAUDE.md) is required here.
### `app/src/main/kotlin/org/libremail/ui/AppViewModel.kt:32` — AppViewModel's documented 'only the first determination is used' contract (`take(1)` on startDestination, and the same on licenseAccepted at line 45) is untested: all four AppViewModelTest cases feed single-emission flowOf(), so deleting take(1) passes the whole suite.
- severity: **low** · category: `test-gap` · angle: `tests`
> take(1) is removed as apparently redundant; tests stay green, but observeAccounts re-emits when the user adds their first account mid-onboarding, startDestination flips from ONBOARDING to MAILBOX, and the NavHost tears down the in-progress onboarding flow (contacts/battery steps and the add-another screen are skipped) — exactly the regression the comment on lines 22-24 warns about.
### `app/src/main/kotlin/org/libremail/ui/AppViewModel.kt:43` — licenseAccepted only selects the onboarding graph's start destination, so a user who already has accounts (startDestination = MAILBOX) but has never accepted the GPL-3.0 license (upgrade from a pre-#172 build) bypasses the mandatory license gate entirely.
- severity: **low** · category: `correctness` · angle: `crossfile`
> Existing install with accounts upgrades to a build containing the #172 license gate: settingsRepository.settings.licenseAccepted is false, but AppViewModel resolves startDestination = Routes.MAILBOX because accounts is non-empty, and LibreMailApp only consults licenseAlreadyAccepted inside onboardingGraph's startDestination — which is never navigated to. The user is never shown the license screen the gate declares mandatory ('the user must scroll through the GPL-3.0 text and tap Agree before reaching anything else', Routes.kt:36-39), and the accepted flag stays false forever.
### `app/src/main/kotlin/org/libremail/ui/mailbox/MailboxScreen.kt:429` — OutboxEntry is a line-for-line copy of DraftsEntry (MailboxScreen.kt:410-426) differing only in the icon vector and string resource; they should be one parameterized row composable.
- severity: **low** · category: `reuse` · angle: `reuse`
> The clickable-row layout (padding, icon tint, 16dp spacer, titleMedium primary-colored text) is duplicated. A change to one entry — e.g. adding a trailing chevron, changing padding, or adding a contentDescription for accessibility — must be manually mirrored in the other; if it isn't, the Drafts and Outbox rows stacked directly on top of each other in the inbox drift visibly apart. Replace both with a single `MailboxShortcutEntry(icon, text, onClick)`.
### `app/src/main/kotlin/org/libremail/ui/mailbox/MailboxScreen.kt:524` — MessageRow memoizes the relative timestamp with remember(message.timestampMillis), whose key never changes, so the displayed age ("2 minutes ago") freezes for as long as the row stays in composition.
- severity: **low** · category: `correctness` · angle: `logic` · flagged by 2 finder(s)
> User leaves the inbox open (or the mailbox stays in the back stack while reading mail): a message that arrived "1 minute ago" still says "1 minute ago" an hour later, because DateUtils.getRelativeTimeSpanString is only re-run when timestampMillis changes — which it never does. Even pull-to-refresh doesn't fix rows whose Message content is unchanged, since Paging emits equal items and the remember key is identical. Time is an input here and cannot be a remember key; the value needs periodic invalidation (e.g. a minute ticker) or at least recomputation on refresh.
### `app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:287` — requestDelete() derives the permanent-vs-trash decision from currentFolderRole, which resolves selectedFolder against the DRAWER account's folder list (folders <- drawerAccount), not the account whose mail is shown — after setDrawerAccount() points the drawer elsewhere, the role lookup is done against the wrong account and the delete confirmation/action can be the wrong operation.
- severity: **low** · category: `correctness` · angle: `invariants`
> User browses account A's Spam folder (fullName "Spam", role SPAM -> delete should permanently expunge), opens the drawer and switches the drawer to account B (setDrawerAccount, no folder tap), closes the drawer, long-press-selects messages and taps Delete: currentFolderRole now looks up "Spam" in B's folder list (miss or NORMAL role) -> permanent=false -> the dialog says 'move to trash' and trash() runs instead of expunge(). With provider folder-name collisions in the opposite direction (drawer account maps the name to TRASH/SPAM, shown account doesn't) the same mismatch selects the permanent expunge path for messages in a non-trash folder.
### `app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:426` — closeSearch()'s clearSearchResults() races an in-flight searchServer() round-trip: hits inserted after the clear survive as orphaned transient rows, defeating the stated invariant that transient hits never outlive the search session.
- severity: **low** · category: `concurrency` · angle: `invariants` · flagged by 2 finder(s)
> User types a query (>=2 chars); after the 400 ms debounce searchServer starts its multi-second IMAP SEARCH; the user closes search while it is in flight. clearSearchResults() deletes existing transient rows, then the still-running searchServer completes and inserts its hits — transient rows now persist in the cache with no active search, surfacing as stale/incorrectly-cached entries in the next search that matches them (and accumulating until some later closeSearch happens to run after all fetches settle).
### `app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:440` — refresh() has a dead first branch: `accountId == null && folder == INBOX` and the final `else` both call mailSyncer.syncAll(), so the three-way if/else-if/else collapses to `if (accountId != null) syncFolder(accountId, folder) else syncAll()`.
- severity: **low** · category: `simplification` · angle: `reuse`
> The redundant branch implies a behavioral distinction between 'unified inbox' and 'unified non-inbox folder' refresh that does not exist. A maintainer later changing the unified-folder refresh path (e.g. to sync only that folder across accounts) will plausibly edit only one of the two textually separate syncAll() branches, introducing a real divergence the current structure invites; the accompanying comment ('a specific folder refreshes just that folder') already mis-describes the accountId==null non-INBOX case. Collapse to the two-way if.
### `app/src/main/kotlin/org/libremail/ui/reader/HtmlBody.kt:117` — shouldOverrideUrlLoading swallows startActivity failures via runCatching with no AppLog, and the security-relevant blocked-navigation branches (non-http/https/mailto scheme at line 115, no-gesture at line 116) also return silently with no logging, violating CLAUDE.md's DoD error/fallback-path logging rule.
- severity: **low** · category: `conventions` · angle: `conventions`
> On a managed/minimal device with no browser (or a disabled handler), tapping an http link in an email throws ActivityNotFoundException, which runCatching discards: the tap does nothing, and the debug report shows no trace. Likewise a malicious email attempting an intent:/market: auto-launch is (correctly) blocked, but the block leaves no PII-free log line (e.g. scheme only), so the defensive path is invisible when diagnosing 'links do nothing' reports.
### `app/src/main/kotlin/org/libremail/ui/reader/ReaderScreen.kt:424` — ReaderScreen's Header re-implements MailboxScreen's private Avatar composable inline (same 40dp CircleShape primaryContainer box and identical initial-derivation expression `sender.trim().firstOrNull()?.uppercase() ?: "?"`) instead of sharing one Avatar composable.
- severity: **low** · category: `reuse` · angle: `reuse`
> The avatar rendering and initial-derivation logic exist twice (MailboxScreen.kt:558-573 Avatar vs ReaderScreen.kt:416-428). A fix to one copy — e.g. handling senders whose display name starts with a quote character or emoji surrogate pair (where firstOrNull() yields half a code point), or a size/color restyle — silently leaves the other screen rendering a different avatar for the same sender. Extract MailboxScreen's Avatar into a shared composable and call it from Header.
### `app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt:121` — Every emission of the Room attachments flow re-runs downloadedAttachmentParts — a second DAO query plus a File.exists()/length() disk stat per attachment — even though Room invalidates the whole attachments table, not just this message's rows.
- severity: **low** · category: `efficiency` · angle: `efficiency`
> Reader is open on a message while background aggressive sync (prefetchMessage) writes attachment rows for other messages: each table write re-emits attachmentDao.observeForMessage(messageId), and the collect body re-executes attachmentDao.getForMessage plus per-file disk stats for a message whose attachment state hasn't changed — repeated redundant DB+filesystem I/O for the whole time the reader is open during a sync. Cheaper: compute the downloaded set once per distinct attachments list (e.g. distinctUntilChanged() on the flow, or fold downloadedAttachmentParts into the same DAO mapping) — incremental updates on actual downloads are already handled in downloadAttachment.
### `app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt:154` — toggleStar's failure rollback restores the negation of ITS OWN captured 'starred' rather than reconciling with the latest attempted state, so two rapid toggles whose persists both fail can leave the shown star opposite to the store.
- severity: **low** · category: `concurrency` · angle: `invariants`
> Message unstarred. Tap star (shown true, persist(true) in flight), tap again (shown false, persist(false) in flight). persist(true) fails first -> rollback sets isStarred=false (ok); then persist(false) fails -> rollback sets isStarred=!false=true. UI now shows starred while the store never accepted any change (still unstarred); the mismatch persists until the message is reopened.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Source: whole-repo multi-agent review, 2026-07-09 (run
wf_b41de68c-e85). The run was cut short by usage limits before its verification pass, so every finding below is an unverified finder candidate — validate each against the current code before implementing. Findings are listed medium first, then low. Critical/high candidates from the same run were verified separately and have their own issues.Medium (11)
app/src/main/kotlin/org/libremail/ui/LibreMailApp.kt:78— A mailto:/ACTION_SEND compose intent bypasses the GPL license-acceptance gate and the whole onboarding flow: the pendingCompose LaunchedEffect navigates to the compose route with no start-destination guard, unlike the pendingOpenMessageId effect right below it which checks start != Routes.ONBOARDING.security· angle:securityapp/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:43— Inbox identity is a copy-pasted magic string (const INBOX = "INBOX" here and again in MailSyncer.kt:210) compared case-SENSITIVELY in the UI (MailboxScreen.kt:268/276/280, FolderDrawer.kt:79, refresh() line 440), while the protocol/domain layers deliberately match it case-INSENSITIVELY (ImapClient.kt:161, withInbox line 471, Folder.kt:67) — the mechanism should be FolderRole.INBOX or one normalized identity at the sync boundary.altitude· angle:altitudeapp/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:222— currentFolderRole resolves the selected folder's role against the DRAWER account's folder list (folders <- drawerAccount), which can be a different account than the one whose mail is shown, so Delete's permanent-vs-trash decision and the CAB's Archive/Spam visibility use the wrong account's folder tree.correctness· angle:logic· flagged by 2 finder(s)app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:344— The keep-alive PagingDataPresenter collects the cached pager for the ViewModel's entire lifetime, so every Room messages-table invalidation reloads a full paged generation even while the app is backgrounded or the user is on another tab — unlike every other flow in this class, which is gated by WhileSubscribed(5s).efficiency· angle:efficiencyapp/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:365— The server-search trigger applies filter{length>=2} BEFORE distinctUntilChanged and keys distinctness on the query text only, so closing and reopening search with the same query — or changing the account/folder filter mid-search — never re-fires the server search, silently omitting server-only results.correctness· angle:invariants· flagged by 5 finder(s)app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:378— selectFolder feeds the server-listed folder fullName straight into sync and the pager while the rest of the app keys the inbox on the literal 'INBOX' (syncAll/syncAccount/IDLE, the default _selectedFolder, the unified pager, and MailboxScreen's selectedFolder == INBOX gates), so a server that LISTs the inbox with different casing (e.g. Exchange's 'Inbox') stores the same messages under two folder keys.correctness· angle:crossfileapp/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:382— selectFolder stores and queries the server-reported inbox fullName verbatim (e.g. "Inbox"), while sync, unified queries, and unread counts use the canonical literal "INBOX" (MailSyncer.INBOX), splitting one inbox into two case-distinct folder keys on servers whose LIST reports a non-uppercase INBOX.correctness· angle:logicapp/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:388— selectFolder discards the Result returned by mailSyncer.syncFolder, so a failed initial sync of a never-synced folder is silently swallowed — unlike refresh(), which routes the same call's failure into the error snackbar.correctness· angle:logic· flagged by 4 finder(s)app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:392— The stale-sync guard compares latestFolderSelection to the sync's key by Pair value-equality, so an older still-running sync for the SAME folder passes the guard and clears _isSyncingFolder while a newer sync for that folder is still in flight; selectUnifiedInbox/selectAccount also never reset the flag.concurrency· angle:concurrencyapp/src/main/kotlin/org/libremail/ui/reader/HtmlBody.kt:84— The reader WebView never disables cookies, so once remote images are enabled (per-message tap or the global loadRemoteImages setting) tracking domains can set persistent cookies in the app-global WebView cookie jar and correlate the user across messages, senders, and time — undermining the tracking-pixel protection this file documents.security· angle:securityapp/src/main/kotlin/org/libremail/ui/reader/HtmlBody.kt:109— HtmlBody's hardened-WebView behaviors — the shouldOverrideUrlLoading scheme allowlist and hasGesture gate (anti intent://-auto-launch), blockNetworkLoads tracking-pixel blocking, the cid: shouldInterceptRequest wiring, and the lastLoaded scroll-preserving reload guard — have no test at any level: ReaderScreenJvmTest deliberately avoids instantiating HtmlBody (blank HTML body, documented WebView caveat) and defers to 'the instrumented ReaderScreenTest' as the on-device E2E, but every message in ReaderScreenTest is isHtml = false, so no test ever renders an HTML body.test-gap· angle:testsLow (13)
app/src/main/kotlin/org/libremail/contacts/ContactsRepository.kt:23— The contacts LIKE pattern interpolates the raw user query without escaping SQL wildcard characters % and _, so queries containing them match unintended contacts.correctness· angle:pitfallsapp/src/main/kotlin/org/libremail/contacts/ContactsRepository.kt:26— ContactsRepository.search wraps the entire ContactsContract query in runCatching and silently discards the exception with no AppLog call, violating CLAUDE.md's DoD requirement of logging on error/fallback paths.conventions· angle:conventionsapp/src/main/kotlin/org/libremail/ui/AppViewModel.kt:32— AppViewModel's documented 'only the first determination is used' contract (take(1)on startDestination, and the same on licenseAccepted at line 45) is untested: all four AppViewModelTest cases feed single-emission flowOf(), so deleting take(1) passes the whole suite.test-gap· angle:testsapp/src/main/kotlin/org/libremail/ui/AppViewModel.kt:43— licenseAccepted only selects the onboarding graph's start destination, so a user who already has accounts (startDestination = MAILBOX) but has never accepted the GPL-3.0 license (upgrade from a pre-#172 build) bypasses the mandatory license gate entirely.correctness· angle:crossfileapp/src/main/kotlin/org/libremail/ui/mailbox/MailboxScreen.kt:429— OutboxEntry is a line-for-line copy of DraftsEntry (MailboxScreen.kt:410-426) differing only in the icon vector and string resource; they should be one parameterized row composable.reuse· angle:reuseapp/src/main/kotlin/org/libremail/ui/mailbox/MailboxScreen.kt:524— MessageRow memoizes the relative timestamp with remember(message.timestampMillis), whose key never changes, so the displayed age ("2 minutes ago") freezes for as long as the row stays in composition.correctness· angle:logic· flagged by 2 finder(s)app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:287— requestDelete() derives the permanent-vs-trash decision from currentFolderRole, which resolves selectedFolder against the DRAWER account's folder list (folders <- drawerAccount), not the account whose mail is shown — after setDrawerAccount() points the drawer elsewhere, the role lookup is done against the wrong account and the delete confirmation/action can be the wrong operation.correctness· angle:invariantsapp/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:426— closeSearch()'s clearSearchResults() races an in-flight searchServer() round-trip: hits inserted after the clear survive as orphaned transient rows, defeating the stated invariant that transient hits never outlive the search session.concurrency· angle:invariants· flagged by 2 finder(s)app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt:440— refresh() has a dead first branch:accountId == null && folder == INBOXand the finalelseboth call mailSyncer.syncAll(), so the three-way if/else-if/else collapses toif (accountId != null) syncFolder(accountId, folder) else syncAll().simplification· angle:reuseapp/src/main/kotlin/org/libremail/ui/reader/HtmlBody.kt:117— shouldOverrideUrlLoading swallows startActivity failures via runCatching with no AppLog, and the security-relevant blocked-navigation branches (non-http/https/mailto scheme at line 115, no-gesture at line 116) also return silently with no logging, violating CLAUDE.md's DoD error/fallback-path logging rule.conventions· angle:conventionsapp/src/main/kotlin/org/libremail/ui/reader/ReaderScreen.kt:424— ReaderScreen's Header re-implements MailboxScreen's private Avatar composable inline (same 40dp CircleShape primaryContainer box and identical initial-derivation expressionsender.trim().firstOrNull()?.uppercase() ?: "?") instead of sharing one Avatar composable.reuse· angle:reuseapp/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt:121— Every emission of the Room attachments flow re-runs downloadedAttachmentParts — a second DAO query plus a File.exists()/length() disk stat per attachment — even though Room invalidates the whole attachments table, not just this message's rows.efficiency· angle:efficiencyapp/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt:154— toggleStar's failure rollback restores the negation of ITS OWN captured 'starred' rather than reconciling with the latest attempted state, so two rapid toggles whose persists both fail can leave the shown star opposite to the store.concurrency· angle:invariants