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 (14)
app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt:93 — runBackfill silently skips an entire account when credential resolution fails — runCatching { connectionFactory.imapParamsFor(account) }.getOrNull() ?: continue swallows the exception with no AppLog breadcrumb, violating CLAUDE.md's Definition of done ('appropriate logging ... error/fallback paths ... diagnosable from a user's debug report'); contrast with the throttle skip two lines above (line 90), which does log.
severity: medium · category: conventions · angle: security · flagged by 2 finder(s)
An Outlook account's refresh token expires (or CredentialStore returns null → MissingCredentialsException). Every backfill slice hits this line and skips the account forever: full-history backfill for that account is permanently stuck, yet the debug report shows only 'backfill slice: maxBatches=20' followed by 'backfill slice done: moreWork=false' — indistinguishable from a healthy, fully-backfilled account. A throttling error thrown during token refresh also bypasses ThrottleClassifier here, so it is never recorded against the account either.
app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt:140 — ThrottleClassifier.classify is applied to failures from the whole page pipeline including local Room writes (persistBatch, backfillProgressDao.upsert), and its LOCKOUT pattern Regex("lock(ed|out)") matches SQLite/SQLCipher's "database is locked", so a transient local DB-busy error records a 1-4 hour provider LOCKOUT backoff.
severity: medium · category: correctness · angle: invariants · flagged by 4 finder(s)
During a backfill page, persistBatch throws SQLiteDatabaseLockedException("database is locked") (SQLCipher/Room busy contention). pageFolder's catch classifies the message: "locked" matches the LOCKOUT pattern, so throttleGate.onThrottle(account, LOCKOUT) sets a 1h backoff (4h cap on repeats) and runBackfill skips the account for the rest of every slice inside that window — hours of stalled backfill from a purely local error the provider never sent. MailSyncer.syncFolderHeaders's onFailure classify (which also wraps the NonCancellable Room persist block) has the same exposure and feeds the same gate.
app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt:228 — markComplete can persist complete=true computed under a stale retention policy AFTER AccountRepositoryImpl.resetBackfillProgress deleted the progress row, permanently stopping backfill for a folder the user just loosened retention on.
severity: medium · category: concurrency · angle: crossfile
A periodic backfill slice is mid-backfillFolder, having read the old policy (e.g. count=100) at MailBackfiller.kt:94. The user loosens retention to unlimited: SettingsViewModel calls resetBackfillProgress -> backfillProgressDao.deleteAll() + backfillNow() (ExistingWorkPolicy.KEEP, so the running work is not restarted, and no MailMaintenanceGate is taken). The running slice then hits the OLD count floor (reachedCountFloor) and markComplete upserts BackfillProgressEntity(complete=true), resurrecting the just-deleted row. Every future run early-returns on progress?.complete == true, so older history is never fetched until the user changes retention again.
app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:62 — syncAll syncs independent accounts sequentially under one global mutex, and per-account IDLE pushes (syncAccount) queue on the same global mutex, so one slow server delays every other account's new-mail detection.
severity: medium · category: efficiency · angle: efficiency
Two accounts configured; account A's IMAP server is unresponsive (connect/read timeouts of 30-60s+). Every periodic syncAll serially waits out A's timeout before touching account B, and an IDLE push for B (syncAccount) blocks on the global syncMutex for the whole duration — delaying B's new-mail notification by A's timeout on every cycle. The races the mutex documents (double-notify, stale deleteSyncedInWindowNotIn snapshot) are all per-(account, folder), so cross-account concurrency is safe. Cheaper alternative: a per-account Mutex map plus coroutineScope { accounts.map { async { syncFolderHeaders(...) } } }.
app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:68 — Per-account sync failures in syncAll are swallowed with no AppLog line: syncFolderHeaders' onFailure (lines 161-166) only feeds ThrottleClassifier and never logs, and syncAll's fold only stashes firstError, which is then discarded entirely when any other account succeeded (line 71) — violating CLAUDE.md's Definition of done requirement that error/fallback paths carry AppLog logging so behaviour is diagnosable from a user's debug report.
severity: medium · category: conventions · angle: conventions
User has two accounts; account B's IMAP auth breaks (changed app password). Every 15-minute sync, account A succeeds so syncAll returns success; the debug report shows only 'sync all: 2 accounts' and 'sync all done: fetched=N' with zero trace of B's failure. User reports 'my second account gets no new mail' and the DebugReport contains no breadcrumb of the error, its type, or which account failed — SyncWorker's 'sync worker: retry' log fires only when ALL accounts fail.
app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:75 — syncAll runs prefetchIfEnabled for every account whenever at least one account synced — including an account whose sync just failed with a classified throttle — and the prefetch path never consults AccountThrottleGate, violating the gate's documented contract that background work checks isThrottled/remainingBackoffMillis before touching an account.
severity: medium · category: correctness · angle: crossfile
Two accounts, fetch policy ALWAYS. Gmail rate-limits account B: syncFolderHeaders(B) fails, ThrottleClassifier records onThrottle(B). Account A succeeds, so result.isSuccess is true and accounts.forEach { prefetchIfEnabled(it, INBOX) } runs for B too, opening connect-per-op IMAP body/attachment fetches for every unfetched message against the server that just throttled us — the hammering behavior #360's gate was built to prevent, escalating the clamp/lockout (the proven docs/perf/issue-125 failure mode).
app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:96 — syncFolderHeaders wraps the whole sync in runCatching, which swallows CancellationException — cancellation is converted into Result.failure (or Result.success in syncAll) instead of propagating, unlike MailBackfiller.pageFolder and IdleService.watchAccount which explicitly rethrow it.
severity: medium · category: correctness · angle: logic · flagged by 2 finder(s)
SyncWorker is stopped (constraint loss) or an IDLE renewal's withTimeoutOrNull cancels mid-syncAccount while imapClient.fetchRecent is in flight: runCatching catches the CancellationException, records it as that account's failure, and syncAll's loop keeps iterating the remaining accounts in a cancelled coroutine (each caught again at its first suspension). If earlier accounts succeeded, anySuccess=true makes a cancelled run report Result.success and log 'sync all done', then enter the prefetch loop before cancellation finally escapes; otherwise the cancellation is logged as 'sync all failed' and, via MailboxViewModel.refresh()'s onFailure, can be surfaced as a user-facing sync-error message for what was merely a cancel.
app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:115 — Every sync loads ALL synced message ids of the folder into a List + HashSet just to classify at most 50 fetched rows, a cost that grows linearly with backfilled mailbox size on the hottest sync path.
severity: medium · category: efficiency · angle: efficiency · flagged by 2 finder(s)
With full-history backfill (#12) a folder can hold 100k+ rows; getSyncedIds(account.id, folder).toHashSet() then materializes 100k strings on every 15-minute periodic sync, every IDLE push, every pull-to-refresh and folder open — inside the NonCancellable block — only to test isEmpty() and membership of <=50 ids. Cheaper alternative already in the DAO: countSynced(accountId, folder) == 0 for the first-sync check plus existingIds(entities.map { it.id }) (a <=50-parameter IN query, the exact pattern MailBackfiller.persistBatch uses).
app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:123 — The empty-server-folder wipe branch (if (fetched.isEmpty()) deleteSyncedByAccountFolder) has no test in either direction: no test asserts the wipe fires on an empty fetch, and none asserts it is keyed on the RAW fetch rather than the age-filtered entities list (the subtlety the comment at lines 123-128 exists to protect).
severity: medium · category: test-gap · angle: tests
A plausible refactor changes the guard to entities.isEmpty() (the variable actually persisted). Every test in MailSyncerTest/MailSyncConcurrencyTest still passes (they never verify deleteSyncedByAccountFolder and never combine an age cutoff with an all-old recent window). Shipped: for an account with age-based retention whose newest 50 messages are all older than the cutoff (low-traffic mailbox), every sync wipes the entire cached folder including all backfilled history — offline-first cache destroyed and re-downloaded in a loop. Conversely, deleting the branch outright (stale rows lingering after a server-side folder purge) is equally invisible to the suite.
app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:128 — MailSyncer's two row-deletion paths (deleteSyncedByAccountFolder at line 128 and deleteSyncedInWindowNotIn at line 146) delete Room rows (attachment metadata cascades via FK) but never delete the per-message on-disk attachment cache directory, orphaning downloaded attachment bytes that only MailPruner and account deletion ever clean up.
severity: medium · category: resource-leak · angle: invariants
User (from another client) deletes a message with a 20 MB attachment that LibreMail had prefetched; the next sync's deleteSyncedInWindowNotIn removes the row, but attachmentCacheDir(cacheDir, messageId) survives — only MailPruner.deleteCacheFiles (MailPruner.kt:83) and AccountRepositoryImpl (account removal) delete these dirs, and the pruner computes victims from still-existing DB rows so it can never find orphans. On an unlimited-retention account the pruner never runs at all, so orphaned attachment bytes of server-deleted mail accumulate indefinitely (disk leak, and deleted-elsewhere mail content lingers on device until OS cache pressure).
app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:144 — The positive-UID guard on the windowed reconcile bound (entity.uid.takeIf { it > 0L } feeding deleteSyncedInWindowNotIn) is untested in MailSyncer: no test fetches a message with an unresolvable UID (-1/0), even though the identical #95 hazard in MailBackfiller has two dedicated regression tests (MailBackfillerTest lines 342-385).
severity: medium · category: test-gap · angle: tests
A simplification to entities.minOf { it.uid } (or dropping the takeIf) passes the whole suite — MailSyncerTest and MailSyncConcurrencyTest only ever use positive UIDs. Shipped: the first time a server returns one row whose UID fails to resolve (UIDFolder.getUID == -1, a known IMAP occurrence per #95), minWindowUid collapses to -1 and deleteSyncedInWindowNotIn becomes a whole-folder delete that silently wipes all backfilled history below the 50-message window on every sync.
app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:176 — MailSyncer.prefetchIfEnabled is a near line-for-line copy of MailBackfiller.prefetchIfEnabled (same DebugFetchGate check, same SyncResourcePolicy.shouldPrefetchContent call, same Gmail daily-budget check with identical log strings), and the copies have already diverged: the backfill copy wraps mailRepository.prefetchMessage(id) in icloudConnectionLimiter.withPermit(account) (MailBackfiller.kt:334) while the syncer copy calls it bare (MailSyncer.kt:204), so post-sync prefetch bypasses the issue-#363 iCloud connection cap. Extract one shared prefetch-gate helper (e.g. a small PrefetchGate class or a function taking account + ids) that both call.
severity: medium · category: reuse · angle: reuse
iCloud account with FetchPolicy.ALWAYS: a foreground sync's prefetch loop opens connect-per-operation IMAP connections outside the per-account Semaphore while a concurrent backfill slice holds all 5 permits — total concurrent connections exceed the 5 that IcloudConnectionLimiter pins production to (Apple's documented ceiling is 5-8), exactly the overlap (#363) the limiter exists to prevent. Maintenance cost going forward: any change to the ~25-line precondition chain (a new provider budget, a changed battery gate, a new FetchScope) must be hand-mirrored into both copies or the foreground and backfill prefetch paths silently disagree — the divergence has already happened once.
app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:198 — Gmail daily download budget is checked once before an unbounded whole-folder prefetch loop, so a single pass can overshoot the 2,500 MB/day budget arbitrarily with no re-check per message.
severity: medium · category: correctness · angle: concurrency · flagged by 3 finder(s)
Gmail account with FetchPolicy.ALWAYS and a large backfilled history: MessageDao.getUnfetchedIds returns every unfetched id in the folder (no LIMIT), so after isOverDailyBudget passes once at e.g. 2,400 MB, the loop keeps calling prefetchMessage (bodies + all attachments, each feeding recordDownload) for thousands of messages, downloading gigabytes past the budget. From MailboxViewModel.refresh()/selectFolder this runs in viewModelScope with no WorkManager time bound, so it can run for hours — tripping exactly the Gmail server-side clamp (the proven 31-40s message-open throttling) that issue #361's tracker exists to prevent. MailBackfiller's counterpart (line 328) re-checks only per 50-message page, but MailSyncer's batch is the whole folder.
app/src/main/kotlin/org/libremail/data/sync/ThrottleClassifier.kt:78 — LOCKOUT_PATTERNS use unanchored substring regexes — Regex("lock(ed|out)") matches 'locked' inside unrelated words and messages ('blocked', SQLite's 'database is locked'), so a local, non-provider error is classified as a provider LOCKOUT and triggers a 1-4 hour per-account backoff.
severity: medium · category: correctness · angle: logic · flagged by 2 finder(s)
MailSyncer.syncFolderHeaders and MailBackfiller.pageFolder wrap Room DAO writes inside the same runCatching/try whose failure is fed to ThrottleClassifier.classify (MailSyncer.kt:165, MailBackfiller.kt:138). If a Room write throws SQLiteDatabaseLockedException (message 'database is locked (code 5)') or any transport error whose text contains 'blocked' (e.g. '...client host blocked...'), 'lock(ed|out)' matches the substring 'locked', classify returns ThrottleKind.LOCKOUT, and AccountThrottleGate.onThrottle imposes LOCKOUT_BASE_MS = 1h (escalating to 4h) — so backfill skips that healthy account for hours over a transient local DB error, and the debug report logs a false 'throttled kind=LOCKOUT' breadcrumb. The KDoc's own stated invariant ('a false positive would silently stall an account, so the patterns anchor on explicit throttle/lock wording') is violated: the regex has no word boundary. Regex("suspend(ed)?") has the same unanchored problem. Fix: anchor with \b (e.g. Regex("\block(ed|out)\b")) and/or exclude 'database is locked' by classifying only errors from the network fetch, not the whole persist block.
Low (11)
app/src/main/kotlin/org/libremail/data/sync/AccountThrottleGate.kt:37 — Backoff windows are measured with the wall clock (System.currentTimeMillis) instead of SystemClock.elapsedRealtime, so a device clock adjustment stretches or collapses a throttle window.
throttledUntilMillis is computed as now + backoff from System.currentTimeMillis (line 54) and compared against the same clock in remainingBackoffMillis (line 81). If the user (or an NTP correction) sets the clock back 3 hours while a Yahoo LOCKOUT backoff (1h base) is active, remainingBackoffMillis reports ~4h and backfill skips the account for 3 extra hours; a forward jump ends a lockout window immediately, resuming traffic against a provider that is still inside its documented ~1-hour auth lock and risking the lock being re-armed (the exact behaviour issue #360 exists to prevent). Durations should be measured on the monotonic elapsedRealtime clock; wall-clock is only correct for calendar boundaries like GmailBandwidthTracker's day epoch.
app/src/main/kotlin/org/libremail/data/sync/GmailBandwidthTracker.kt:79 — The 'is this account subject to a bandwidth budget' provider dispatch (GmailSyncLimits.appliesTo(account) && bandwidthTracker.isOverDailyBudget(account.id)) is copy-pasted at three call sites (MailSyncer:198, MailBackfiller:328, MailRepositoryImpl:361) instead of the tracker owning it via Account-taking recordDownload/isOverDailyBudget or a provider-limits policy object.
When the sibling Yahoo (#362), iCloud (#363), and Outlook (#364) bandwidth budgets land, every one of the three sites must grow a parallel 'appliesTo && overBudget' branch per provider, and a missed site silently exempts that path from the cap — the same divergence-by-duplication that already let the syncer's prefetch copy drop the iCloud limiter and the throttle-gate skip. Concrete cost is N-providers x M-call-sites branch maintenance plus silent enforcement gaps, versus one account-aware check inside the tracker/policy type.
app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt:109 — The throttle-recovery wiring (if (result.batches > 0) throttleGate.onSuccess(account.id)) is untested: MailBackfillerTest never asserts a successful page clears a previously-throttled account's attempt count (grep confirms no onSuccess reference in the test), and AccountThrottleGateTest only covers the gate method in isolation, not the backfiller calling it.
severity: low · category: test-gap · angle: tests
If this line is dropped or the batches > 0 condition inverted during a refactor, all tests pass. Shipped: an account throttled once keeps its attempt count forever (nothing else in the backfill path calls onSuccess), so each subsequent transient throttle — even days apart with successful backfill in between — escalates the backoff toward the 15-minute rate-limit / 4-hour lockout ceilings, silently stalling that account's history backfill far longer than designed.
app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt:217 — The lowest-positive-UID guard expression entities.mapNotNull { entity -> entity.uid.takeIf { it > 0L } }.minOrNull() — the issue-#95 defence against unresolved-UID placeholders (UIDFolder.getUID == -1) collapsing a bound to <= 0 — is duplicated verbatim in MailBackfiller.kt:217 and MailSyncer.kt:144; it should be one named helper (e.g. List<MessageEntity>.lowestResolvedUid()).
severity: low · category: reuse · angle: reuse
The 'uid <= 0 means unresolved placeholder' sentinel convention is encoded in two hot paths with different blast radii: in MailSyncer it bounds deleteSyncedInWindowNotIn (getting it wrong turns a windowed reconcile into a whole-folder delete of backfilled history) and in MailBackfiller it sets the persisted paging boundary (getting it wrong falsely marks a folder fully backfilled). If the sentinel or guard ever changes (e.g. a new placeholder value or a >= 1 tightening), whoever edits one copy has no signpost to the other, regressing exactly one of the two #95 bug classes.
app/src/main/kotlin/org/libremail/data/sync/MailConnectionFactory.kt:34 — The process-wide OAuth access-token cache (tokenCache/refreshMutexes) has no eviction path, so a removed account's still-valid bearer token survives account deletion and is silently reused if the account is re-added.
User removes an Outlook account (AccountRepositoryImpl.deleteAccount calls credentialStore.delete(id) but nothing touches MailConnectionFactory); the singleton tokenCache keeps the account's unexpired Graph/Outlook access token (up to ~1h) in memory despite the user's expectation that removal wiped its credentials. If the same account id is re-added within that window, cachedAccessToken() returns the pre-removal session's token at line 69 without ever consulting the freshly-stored AuthState, so the new session transparently runs on the old session's bearer credential; the maps also grow unboundedly across add/remove cycles since keys are never removed.
app/src/main/kotlin/org/libremail/data/sync/MailConnectionFactory.kt:74 — OAuth access-token refresh — redeeming the (rotating) refresh token and persisting the updated AuthState (lines 74-76) — is a significant state change with no AppLog logging anywhere in the chain (this file has zero AppLog calls, and the org.libremail.auth package it delegates to has none either), contrary to CLAUDE.md's Definition of done ('significant state changes' must be logged so behaviour is diagnosable from a debug report).
An Outlook account starts refresh-looping (e.g. cached token repeatedly judged expired due to a missing expiry, or the persisted rotated AuthState write races) or refreshes fail intermittently. The debug report contains no 'token refreshed / refresh failed / AuthState persisted' breadcrumbs at all — the only downstream signal is a generic worker 'retry' with a scrubbed throwable, making token-rotation bugs (historically this repo's Outlook sign-in breakers) undiagnosable from user reports.
app/src/main/kotlin/org/libremail/data/sync/PruneWorker.kt:37 — PruneWorker's runCatching swallows CancellationException — unlike BackfillWorker, which explicitly rethrows it (BackfillWorker.kt:69) — so a normal WorkManager stop is logged as a warning-level 'prune worker: retry' failure and post-cancellation code keeps running.
WorkManager stops the 12-hour periodic prune mid-run (battery drops below not-low, or the ~10-min execution window expires). MailPruner.prune()'s ensureActive/suspending DAO calls throw CancellationException, runCatching captures it, and the onFailure branch runs inside an already-cancelled coroutine: AppLog.w(TAG, 'prune worker: retry', error) writes a misleading error breadcrumb (with the cancellation stack) into the RingLogBuffer/DebugReport for every routine stop, polluting user debug reports with phantom prune failures. SyncWorker has the same gap one level down (MailSyncer.syncFolderHeaders' runCatching converts cancellation into Result.failure, producing the same false 'sync worker: retry' log). The sibling BackfillWorker shows the intended pattern: if (error is CancellationException) throw error before treating the failure as a retry.
app/src/main/kotlin/org/libremail/data/sync/SendWorker.kt:102 — sendQueued's runCatching swallows CancellationException and treats a cancelled send as a genuine failure (setError + failed=true), inconsistent with the explicit rethrow in sendOutlook a few lines below.
SendScheduler.sendNow uses ExistingWorkPolicy.REPLACE, so queuing message B while message A is mid-send cancels the running drain: the in-flight smtpSender/graphSender send throws CancellationException, the fold's onFailure runs and attempts outboxDao.setError(entity.id, e.message) — writing an internal cancellation message ('...was cancelled') as the row's user-visible error when the write lands (e.g. a TimeoutCancellationException from an inner withTimeout while the job is alive), and marking the run failed. A stop-driven cancellation only escapes because Room's suspend setError happens to rethrow on the cancelled job — accidental, not designed, correctness.
app/src/main/kotlin/org/libremail/data/sync/SyncScheduler.kt:35 — SyncSchedulerTest pins only unique-name + existing-work policy for each job; none of the WorkRequest constraints (CONNECTED for sync/backfill, battery-not-low for backfill/prune, charging-only for report purge) or the periodic intervals are asserted, though they are inspectable via request.workSpec.
severity: low · category: test-gap · angle: tests
Deleting .setRequiresBatteryNotLow(true) from backfillConstraint (or swapping backfillConstraint for networkConstraint in schedulePeriodicBackfill) leaves the entire suite green. Shipped: the bulk full-history backfill pages a large mailbox on a nearly-dead battery, or the report purge runs off-charger — exactly the resource-drain behaviors these constraints were added to prevent, with no test able to catch the regression.
app/src/main/kotlin/org/libremail/data/sync/ThrottleClassifier.kt:61 — ThrottleClassifier.causeChain is a verbatim 9-line copy of ImapAuthError.causeChain (app/src/main/kotlin/org/libremail/mail/ImapAuthError.kt:47) — the KDoc even acknowledges it ('Mirrors the same-shaped walk in org.libremail.mail.ImapAuthError'); both should call one shared Throwable.causeChain() extension (isCacheEncryptionUnavailable at data/local/CacheEncryptionUnavailableException.kt:39 is a third, cycle-unguarded variant via generateSequence(this) { it.cause } that could use it too).
severity: low · category: reuse · angle: reuse
The cycle-guard walk is security/robustness-relevant (a self-referential cause chain from a hostile or buggy server path must not hang classification). A fix or improvement to the walk — e.g. capping chain depth, or switching the O(n^2) identity scan to an IdentityHashMap — lands in one copy and not the other, so throttle classification and IMAP-disabled detection silently diverge in how they traverse the same exceptions; the third generateSequence variant already lacks the cycle guard the other two carry.
app/src/main/kotlin/org/libremail/data/sync/ThrottleClassifier.kt:99 — The rate-limit pattern Regex("service (not|un)available") is missing a space after 'not', so it matches 'service unavailable'/'service notavailable' but not the canonical SMTP/IMAP 421 wording 'Service not available'.
A provider sheds load with the RFC-standard '421 Service not available' response: the message contains 'service not available' which the regex cannot match (it requires 'notavailable' with no space), so classify() returns null, no backoff is recorded, and the client keeps hammering a server that asked it to back off — the exact behaviour the classifier exists to prevent (false negative only, so it degrades to transient-error handling).
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 (14)
### `app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt:93` — runBackfill silently skips an entire account when credential resolution fails — `runCatching { connectionFactory.imapParamsFor(account) }.getOrNull() ?: continue` swallows the exception with no AppLog breadcrumb, violating CLAUDE.md's Definition of done ('appropriate logging ... error/fallback paths ... diagnosable from a user's debug report'); contrast with the throttle skip two lines above (line 90), which does log.
- severity: **medium** · category: `conventions` · angle: `security` · flagged by 2 finder(s)
> An Outlook account's refresh token expires (or CredentialStore returns null → MissingCredentialsException). Every backfill slice hits this line and skips the account forever: full-history backfill for that account is permanently stuck, yet the debug report shows only 'backfill slice: maxBatches=20' followed by 'backfill slice done: moreWork=false' — indistinguishable from a healthy, fully-backfilled account. A throttling error thrown during token refresh also bypasses ThrottleClassifier here, so it is never recorded against the account either.
### `app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt:140` — ThrottleClassifier.classify is applied to failures from the whole page pipeline including local Room writes (persistBatch, backfillProgressDao.upsert), and its LOCKOUT pattern Regex("lock(ed|out)") matches SQLite/SQLCipher's "database is locked", so a transient local DB-busy error records a 1-4 hour provider LOCKOUT backoff.
- severity: **medium** · category: `correctness` · angle: `invariants` · flagged by 4 finder(s)
> During a backfill page, persistBatch throws SQLiteDatabaseLockedException("database is locked") (SQLCipher/Room busy contention). pageFolder's catch classifies the message: "locked" matches the LOCKOUT pattern, so throttleGate.onThrottle(account, LOCKOUT) sets a 1h backoff (4h cap on repeats) and runBackfill skips the account for the rest of every slice inside that window — hours of stalled backfill from a purely local error the provider never sent. MailSyncer.syncFolderHeaders's onFailure classify (which also wraps the NonCancellable Room persist block) has the same exposure and feeds the same gate.
### `app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt:228` — markComplete can persist complete=true computed under a stale retention policy AFTER AccountRepositoryImpl.resetBackfillProgress deleted the progress row, permanently stopping backfill for a folder the user just loosened retention on.
- severity: **medium** · category: `concurrency` · angle: `crossfile`
> A periodic backfill slice is mid-backfillFolder, having read the old policy (e.g. count=100) at MailBackfiller.kt:94. The user loosens retention to unlimited: SettingsViewModel calls resetBackfillProgress -> backfillProgressDao.deleteAll() + backfillNow() (ExistingWorkPolicy.KEEP, so the running work is not restarted, and no MailMaintenanceGate is taken). The running slice then hits the OLD count floor (reachedCountFloor) and markComplete upserts BackfillProgressEntity(complete=true), resurrecting the just-deleted row. Every future run early-returns on progress?.complete == true, so older history is never fetched until the user changes retention again.
### `app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:62` — syncAll syncs independent accounts sequentially under one global mutex, and per-account IDLE pushes (syncAccount) queue on the same global mutex, so one slow server delays every other account's new-mail detection.
- severity: **medium** · category: `efficiency` · angle: `efficiency`
> Two accounts configured; account A's IMAP server is unresponsive (connect/read timeouts of 30-60s+). Every periodic syncAll serially waits out A's timeout before touching account B, and an IDLE push for B (syncAccount) blocks on the global syncMutex for the whole duration — delaying B's new-mail notification by A's timeout on every cycle. The races the mutex documents (double-notify, stale deleteSyncedInWindowNotIn snapshot) are all per-(account, folder), so cross-account concurrency is safe. Cheaper alternative: a per-account Mutex map plus coroutineScope { accounts.map { async { syncFolderHeaders(...) } } }.
### `app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:68` — Per-account sync failures in syncAll are swallowed with no AppLog line: syncFolderHeaders' onFailure (lines 161-166) only feeds ThrottleClassifier and never logs, and syncAll's fold only stashes firstError, which is then discarded entirely when any other account succeeded (line 71) — violating CLAUDE.md's Definition of done requirement that error/fallback paths carry AppLog logging so behaviour is diagnosable from a user's debug report.
- severity: **medium** · category: `conventions` · angle: `conventions`
> User has two accounts; account B's IMAP auth breaks (changed app password). Every 15-minute sync, account A succeeds so syncAll returns success; the debug report shows only 'sync all: 2 accounts' and 'sync all done: fetched=N' with zero trace of B's failure. User reports 'my second account gets no new mail' and the DebugReport contains no breadcrumb of the error, its type, or which account failed — SyncWorker's 'sync worker: retry' log fires only when ALL accounts fail.
### `app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:75` — syncAll runs prefetchIfEnabled for every account whenever at least one account synced — including an account whose sync just failed with a classified throttle — and the prefetch path never consults AccountThrottleGate, violating the gate's documented contract that background work checks isThrottled/remainingBackoffMillis before touching an account.
- severity: **medium** · category: `correctness` · angle: `crossfile`
> Two accounts, fetch policy ALWAYS. Gmail rate-limits account B: syncFolderHeaders(B) fails, ThrottleClassifier records onThrottle(B). Account A succeeds, so result.isSuccess is true and accounts.forEach { prefetchIfEnabled(it, INBOX) } runs for B too, opening connect-per-op IMAP body/attachment fetches for every unfetched message against the server that just throttled us — the hammering behavior #360's gate was built to prevent, escalating the clamp/lockout (the proven docs/perf/issue-125 failure mode).
### `app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:96` — syncFolderHeaders wraps the whole sync in runCatching, which swallows CancellationException — cancellation is converted into Result.failure (or Result.success in syncAll) instead of propagating, unlike MailBackfiller.pageFolder and IdleService.watchAccount which explicitly rethrow it.
- severity: **medium** · category: `correctness` · angle: `logic` · flagged by 2 finder(s)
> SyncWorker is stopped (constraint loss) or an IDLE renewal's withTimeoutOrNull cancels mid-syncAccount while imapClient.fetchRecent is in flight: runCatching catches the CancellationException, records it as that account's failure, and syncAll's loop keeps iterating the remaining accounts in a cancelled coroutine (each caught again at its first suspension). If earlier accounts succeeded, anySuccess=true makes a cancelled run report Result.success and log 'sync all done', then enter the prefetch loop before cancellation finally escapes; otherwise the cancellation is logged as 'sync all failed' and, via MailboxViewModel.refresh()'s onFailure, can be surfaced as a user-facing sync-error message for what was merely a cancel.
### `app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:115` — Every sync loads ALL synced message ids of the folder into a List + HashSet just to classify at most 50 fetched rows, a cost that grows linearly with backfilled mailbox size on the hottest sync path.
- severity: **medium** · category: `efficiency` · angle: `efficiency` · flagged by 2 finder(s)
> With full-history backfill (#12) a folder can hold 100k+ rows; getSyncedIds(account.id, folder).toHashSet() then materializes 100k strings on every 15-minute periodic sync, every IDLE push, every pull-to-refresh and folder open — inside the NonCancellable block — only to test isEmpty() and membership of <=50 ids. Cheaper alternative already in the DAO: countSynced(accountId, folder) == 0 for the first-sync check plus existingIds(entities.map { it.id }) (a <=50-parameter IN query, the exact pattern MailBackfiller.persistBatch uses).
### `app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:123` — The empty-server-folder wipe branch (`if (fetched.isEmpty()) deleteSyncedByAccountFolder`) has no test in either direction: no test asserts the wipe fires on an empty fetch, and none asserts it is keyed on the RAW fetch rather than the age-filtered `entities` list (the subtlety the comment at lines 123-128 exists to protect).
- severity: **medium** · category: `test-gap` · angle: `tests`
> A plausible refactor changes the guard to `entities.isEmpty()` (the variable actually persisted). Every test in MailSyncerTest/MailSyncConcurrencyTest still passes (they never verify deleteSyncedByAccountFolder and never combine an age cutoff with an all-old recent window). Shipped: for an account with age-based retention whose newest 50 messages are all older than the cutoff (low-traffic mailbox), every sync wipes the entire cached folder including all backfilled history — offline-first cache destroyed and re-downloaded in a loop. Conversely, deleting the branch outright (stale rows lingering after a server-side folder purge) is equally invisible to the suite.
### `app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:128` — MailSyncer's two row-deletion paths (deleteSyncedByAccountFolder at line 128 and deleteSyncedInWindowNotIn at line 146) delete Room rows (attachment metadata cascades via FK) but never delete the per-message on-disk attachment cache directory, orphaning downloaded attachment bytes that only MailPruner and account deletion ever clean up.
- severity: **medium** · category: `resource-leak` · angle: `invariants`
> User (from another client) deletes a message with a 20 MB attachment that LibreMail had prefetched; the next sync's deleteSyncedInWindowNotIn removes the row, but attachmentCacheDir(cacheDir, messageId) survives — only MailPruner.deleteCacheFiles (MailPruner.kt:83) and AccountRepositoryImpl (account removal) delete these dirs, and the pruner computes victims from still-existing DB rows so it can never find orphans. On an unlimited-retention account the pruner never runs at all, so orphaned attachment bytes of server-deleted mail accumulate indefinitely (disk leak, and deleted-elsewhere mail content lingers on device until OS cache pressure).
### `app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:144` — The positive-UID guard on the windowed reconcile bound (`entity.uid.takeIf { it > 0L }` feeding deleteSyncedInWindowNotIn) is untested in MailSyncer: no test fetches a message with an unresolvable UID (-1/0), even though the identical #95 hazard in MailBackfiller has two dedicated regression tests (MailBackfillerTest lines 342-385).
- severity: **medium** · category: `test-gap` · angle: `tests`
> A simplification to `entities.minOf { it.uid }` (or dropping the takeIf) passes the whole suite — MailSyncerTest and MailSyncConcurrencyTest only ever use positive UIDs. Shipped: the first time a server returns one row whose UID fails to resolve (UIDFolder.getUID == -1, a known IMAP occurrence per #95), minWindowUid collapses to -1 and deleteSyncedInWindowNotIn becomes a whole-folder delete that silently wipes all backfilled history below the 50-message window on every sync.
### `app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:176` — MailSyncer.prefetchIfEnabled is a near line-for-line copy of MailBackfiller.prefetchIfEnabled (same DebugFetchGate check, same SyncResourcePolicy.shouldPrefetchContent call, same Gmail daily-budget check with identical log strings), and the copies have already diverged: the backfill copy wraps mailRepository.prefetchMessage(id) in icloudConnectionLimiter.withPermit(account) (MailBackfiller.kt:334) while the syncer copy calls it bare (MailSyncer.kt:204), so post-sync prefetch bypasses the issue-#363 iCloud connection cap. Extract one shared prefetch-gate helper (e.g. a small PrefetchGate class or a function taking account + ids) that both call.
- severity: **medium** · category: `reuse` · angle: `reuse`
> iCloud account with FetchPolicy.ALWAYS: a foreground sync's prefetch loop opens connect-per-operation IMAP connections outside the per-account Semaphore while a concurrent backfill slice holds all 5 permits — total concurrent connections exceed the 5 that IcloudConnectionLimiter pins production to (Apple's documented ceiling is 5-8), exactly the overlap (#363) the limiter exists to prevent. Maintenance cost going forward: any change to the ~25-line precondition chain (a new provider budget, a changed battery gate, a new FetchScope) must be hand-mirrored into both copies or the foreground and backfill prefetch paths silently disagree — the divergence has already happened once.
### `app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:198` — Gmail daily download budget is checked once before an unbounded whole-folder prefetch loop, so a single pass can overshoot the 2,500 MB/day budget arbitrarily with no re-check per message.
- severity: **medium** · category: `correctness` · angle: `concurrency` · flagged by 3 finder(s)
> Gmail account with FetchPolicy.ALWAYS and a large backfilled history: MessageDao.getUnfetchedIds returns every unfetched id in the folder (no LIMIT), so after isOverDailyBudget passes once at e.g. 2,400 MB, the loop keeps calling prefetchMessage (bodies + all attachments, each feeding recordDownload) for thousands of messages, downloading gigabytes past the budget. From MailboxViewModel.refresh()/selectFolder this runs in viewModelScope with no WorkManager time bound, so it can run for hours — tripping exactly the Gmail server-side clamp (the proven 31-40s message-open throttling) that issue #361's tracker exists to prevent. MailBackfiller's counterpart (line 328) re-checks only per 50-message page, but MailSyncer's batch is the whole folder.
### `app/src/main/kotlin/org/libremail/data/sync/ThrottleClassifier.kt:78` — LOCKOUT_PATTERNS use unanchored substring regexes — Regex("lock(ed|out)") matches 'locked' inside unrelated words and messages ('blocked', SQLite's 'database is locked'), so a local, non-provider error is classified as a provider LOCKOUT and triggers a 1-4 hour per-account backoff.
- severity: **medium** · category: `correctness` · angle: `logic` · flagged by 2 finder(s)
> MailSyncer.syncFolderHeaders and MailBackfiller.pageFolder wrap Room DAO writes inside the same runCatching/try whose failure is fed to ThrottleClassifier.classify (MailSyncer.kt:165, MailBackfiller.kt:138). If a Room write throws SQLiteDatabaseLockedException (message 'database is locked (code 5)') or any transport error whose text contains 'blocked' (e.g. '...client host blocked...'), 'lock(ed|out)' matches the substring 'locked', classify returns ThrottleKind.LOCKOUT, and AccountThrottleGate.onThrottle imposes LOCKOUT_BASE_MS = 1h (escalating to 4h) — so backfill skips that healthy account for hours over a transient local DB error, and the debug report logs a false 'throttled kind=LOCKOUT' breadcrumb. The KDoc's own stated invariant ('a false positive would silently stall an account, so the patterns anchor on explicit throttle/lock wording') is violated: the regex has no word boundary. Regex("suspend(ed)?") has the same unanchored problem. Fix: anchor with \b (e.g. Regex("\\block(ed|out)\\b")) and/or exclude 'database is locked' by classifying only errors from the network fetch, not the whole persist block.
## Low (11)
### `app/src/main/kotlin/org/libremail/data/sync/AccountThrottleGate.kt:37` — Backoff windows are measured with the wall clock (System.currentTimeMillis) instead of SystemClock.elapsedRealtime, so a device clock adjustment stretches or collapses a throttle window.
- severity: **low** · category: `correctness` · angle: `pitfalls`
> throttledUntilMillis is computed as now + backoff from System.currentTimeMillis (line 54) and compared against the same clock in remainingBackoffMillis (line 81). If the user (or an NTP correction) sets the clock back 3 hours while a Yahoo LOCKOUT backoff (1h base) is active, remainingBackoffMillis reports ~4h and backfill skips the account for 3 extra hours; a forward jump ends a lockout window immediately, resuming traffic against a provider that is still inside its documented ~1-hour auth lock and risking the lock being re-armed (the exact behaviour issue #360 exists to prevent). Durations should be measured on the monotonic elapsedRealtime clock; wall-clock is only correct for calendar boundaries like GmailBandwidthTracker's day epoch.
### `app/src/main/kotlin/org/libremail/data/sync/GmailBandwidthTracker.kt:79` — The 'is this account subject to a bandwidth budget' provider dispatch (GmailSyncLimits.appliesTo(account) && bandwidthTracker.isOverDailyBudget(account.id)) is copy-pasted at three call sites (MailSyncer:198, MailBackfiller:328, MailRepositoryImpl:361) instead of the tracker owning it via Account-taking recordDownload/isOverDailyBudget or a provider-limits policy object.
- severity: **low** · category: `altitude` · angle: `altitude`
> When the sibling Yahoo (#362), iCloud (#363), and Outlook (#364) bandwidth budgets land, every one of the three sites must grow a parallel 'appliesTo && overBudget' branch per provider, and a missed site silently exempts that path from the cap — the same divergence-by-duplication that already let the syncer's prefetch copy drop the iCloud limiter and the throttle-gate skip. Concrete cost is N-providers x M-call-sites branch maintenance plus silent enforcement gaps, versus one account-aware check inside the tracker/policy type.
### `app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt:109` — The throttle-recovery wiring (`if (result.batches > 0) throttleGate.onSuccess(account.id)`) is untested: MailBackfillerTest never asserts a successful page clears a previously-throttled account's attempt count (grep confirms no onSuccess reference in the test), and AccountThrottleGateTest only covers the gate method in isolation, not the backfiller calling it.
- severity: **low** · category: `test-gap` · angle: `tests`
> If this line is dropped or the `batches > 0` condition inverted during a refactor, all tests pass. Shipped: an account throttled once keeps its attempt count forever (nothing else in the backfill path calls onSuccess), so each subsequent transient throttle — even days apart with successful backfill in between — escalates the backoff toward the 15-minute rate-limit / 4-hour lockout ceilings, silently stalling that account's history backfill far longer than designed.
### `app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt:217` — The lowest-positive-UID guard expression `entities.mapNotNull { entity -> entity.uid.takeIf { it > 0L } }.minOrNull()` — the issue-#95 defence against unresolved-UID placeholders (UIDFolder.getUID == -1) collapsing a bound to <= 0 — is duplicated verbatim in MailBackfiller.kt:217 and MailSyncer.kt:144; it should be one named helper (e.g. `List<MessageEntity>.lowestResolvedUid()`).
- severity: **low** · category: `reuse` · angle: `reuse`
> The 'uid <= 0 means unresolved placeholder' sentinel convention is encoded in two hot paths with different blast radii: in MailSyncer it bounds deleteSyncedInWindowNotIn (getting it wrong turns a windowed reconcile into a whole-folder delete of backfilled history) and in MailBackfiller it sets the persisted paging boundary (getting it wrong falsely marks a folder fully backfilled). If the sentinel or guard ever changes (e.g. a new placeholder value or a >= 1 tightening), whoever edits one copy has no signpost to the other, regressing exactly one of the two #95 bug classes.
### `app/src/main/kotlin/org/libremail/data/sync/MailConnectionFactory.kt:34` — The process-wide OAuth access-token cache (tokenCache/refreshMutexes) has no eviction path, so a removed account's still-valid bearer token survives account deletion and is silently reused if the account is re-added.
- severity: **low** · category: `security` · angle: `security`
> User removes an Outlook account (AccountRepositoryImpl.deleteAccount calls credentialStore.delete(id) but nothing touches MailConnectionFactory); the singleton tokenCache keeps the account's unexpired Graph/Outlook access token (up to ~1h) in memory despite the user's expectation that removal wiped its credentials. If the same account id is re-added within that window, cachedAccessToken() returns the pre-removal session's token at line 69 without ever consulting the freshly-stored AuthState, so the new session transparently runs on the old session's bearer credential; the maps also grow unboundedly across add/remove cycles since keys are never removed.
### `app/src/main/kotlin/org/libremail/data/sync/MailConnectionFactory.kt:74` — OAuth access-token refresh — redeeming the (rotating) refresh token and persisting the updated AuthState (lines 74-76) — is a significant state change with no AppLog logging anywhere in the chain (this file has zero AppLog calls, and the org.libremail.auth package it delegates to has none either), contrary to CLAUDE.md's Definition of done ('significant state changes' must be logged so behaviour is diagnosable from a debug report).
- severity: **low** · category: `conventions` · angle: `conventions`
> An Outlook account starts refresh-looping (e.g. cached token repeatedly judged expired due to a missing expiry, or the persisted rotated AuthState write races) or refreshes fail intermittently. The debug report contains no 'token refreshed / refresh failed / AuthState persisted' breadcrumbs at all — the only downstream signal is a generic worker 'retry' with a scrubbed throwable, making token-rotation bugs (historically this repo's Outlook sign-in breakers) undiagnosable from user reports.
### `app/src/main/kotlin/org/libremail/data/sync/PruneWorker.kt:37` — PruneWorker's runCatching swallows CancellationException — unlike BackfillWorker, which explicitly rethrows it (BackfillWorker.kt:69) — so a normal WorkManager stop is logged as a warning-level 'prune worker: retry' failure and post-cancellation code keeps running.
- severity: **low** · category: `concurrency` · angle: `pitfalls` · flagged by 2 finder(s)
> WorkManager stops the 12-hour periodic prune mid-run (battery drops below not-low, or the ~10-min execution window expires). MailPruner.prune()'s ensureActive/suspending DAO calls throw CancellationException, runCatching captures it, and the onFailure branch runs inside an already-cancelled coroutine: AppLog.w(TAG, 'prune worker: retry', error) writes a misleading error breadcrumb (with the cancellation stack) into the RingLogBuffer/DebugReport for every routine stop, polluting user debug reports with phantom prune failures. SyncWorker has the same gap one level down (MailSyncer.syncFolderHeaders' runCatching converts cancellation into Result.failure, producing the same false 'sync worker: retry' log). The sibling BackfillWorker shows the intended pattern: `if (error is CancellationException) throw error` before treating the failure as a retry.
### `app/src/main/kotlin/org/libremail/data/sync/SendWorker.kt:102` — sendQueued's runCatching swallows CancellationException and treats a cancelled send as a genuine failure (setError + failed=true), inconsistent with the explicit rethrow in sendOutlook a few lines below.
- severity: **low** · category: `concurrency` · angle: `concurrency`
> SendScheduler.sendNow uses ExistingWorkPolicy.REPLACE, so queuing message B while message A is mid-send cancels the running drain: the in-flight smtpSender/graphSender send throws CancellationException, the fold's onFailure runs and attempts outboxDao.setError(entity.id, e.message) — writing an internal cancellation message ('...was cancelled') as the row's user-visible error when the write lands (e.g. a TimeoutCancellationException from an inner withTimeout while the job is alive), and marking the run failed. A stop-driven cancellation only escapes because Room's suspend setError happens to rethrow on the cancelled job — accidental, not designed, correctness.
### `app/src/main/kotlin/org/libremail/data/sync/SyncScheduler.kt:35` — SyncSchedulerTest pins only unique-name + existing-work policy for each job; none of the WorkRequest constraints (CONNECTED for sync/backfill, battery-not-low for backfill/prune, charging-only for report purge) or the periodic intervals are asserted, though they are inspectable via request.workSpec.
- severity: **low** · category: `test-gap` · angle: `tests`
> Deleting `.setRequiresBatteryNotLow(true)` from backfillConstraint (or swapping backfillConstraint for networkConstraint in schedulePeriodicBackfill) leaves the entire suite green. Shipped: the bulk full-history backfill pages a large mailbox on a nearly-dead battery, or the report purge runs off-charger — exactly the resource-drain behaviors these constraints were added to prevent, with no test able to catch the regression.
### `app/src/main/kotlin/org/libremail/data/sync/ThrottleClassifier.kt:61` — ThrottleClassifier.causeChain is a verbatim 9-line copy of ImapAuthError.causeChain (app/src/main/kotlin/org/libremail/mail/ImapAuthError.kt:47) — the KDoc even acknowledges it ('Mirrors the same-shaped walk in org.libremail.mail.ImapAuthError'); both should call one shared Throwable.causeChain() extension (isCacheEncryptionUnavailable at data/local/CacheEncryptionUnavailableException.kt:39 is a third, cycle-unguarded variant via generateSequence(this) { it.cause } that could use it too).
- severity: **low** · category: `reuse` · angle: `reuse`
> The cycle-guard walk is security/robustness-relevant (a self-referential cause chain from a hostile or buggy server path must not hang classification). A fix or improvement to the walk — e.g. capping chain depth, or switching the O(n^2) identity scan to an IdentityHashMap — lands in one copy and not the other, so throttle classification and IMAP-disabled detection silently diverge in how they traverse the same exceptions; the third generateSequence variant already lacks the cycle guard the other two carry.
### `app/src/main/kotlin/org/libremail/data/sync/ThrottleClassifier.kt:99` — The rate-limit pattern Regex("service (not|un)available") is missing a space after 'not', so it matches 'service unavailable'/'service notavailable' but not the canonical SMTP/IMAP 421 wording 'Service not available'.
- severity: **low** · category: `correctness` · angle: `logic`
> A provider sheds load with the RFC-standard '421 <domain> Service not available' response: the message contains 'service not available' which the regex cannot match (it requires 'notavailable' with no space), so classify() returns null, no backoff is recorded, and the client keeps hammering a server that asked it to back off — the exact behaviour the classifier exists to prevent (false negative only, so it degrades to transient-error handling).
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 (14)
app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt:93— runBackfill silently skips an entire account when credential resolution fails —runCatching { connectionFactory.imapParamsFor(account) }.getOrNull() ?: continueswallows the exception with no AppLog breadcrumb, violating CLAUDE.md's Definition of done ('appropriate logging ... error/fallback paths ... diagnosable from a user's debug report'); contrast with the throttle skip two lines above (line 90), which does log.conventions· angle:security· flagged by 2 finder(s)app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt:140— ThrottleClassifier.classify is applied to failures from the whole page pipeline including local Room writes (persistBatch, backfillProgressDao.upsert), and its LOCKOUT pattern Regex("lock(ed|out)") matches SQLite/SQLCipher's "database is locked", so a transient local DB-busy error records a 1-4 hour provider LOCKOUT backoff.correctness· angle:invariants· flagged by 4 finder(s)app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt:228— markComplete can persist complete=true computed under a stale retention policy AFTER AccountRepositoryImpl.resetBackfillProgress deleted the progress row, permanently stopping backfill for a folder the user just loosened retention on.concurrency· angle:crossfileapp/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:62— syncAll syncs independent accounts sequentially under one global mutex, and per-account IDLE pushes (syncAccount) queue on the same global mutex, so one slow server delays every other account's new-mail detection.efficiency· angle:efficiencyapp/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:68— Per-account sync failures in syncAll are swallowed with no AppLog line: syncFolderHeaders' onFailure (lines 161-166) only feeds ThrottleClassifier and never logs, and syncAll's fold only stashes firstError, which is then discarded entirely when any other account succeeded (line 71) — violating CLAUDE.md's Definition of done requirement that error/fallback paths carry AppLog logging so behaviour is diagnosable from a user's debug report.conventions· angle:conventionsapp/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:75— syncAll runs prefetchIfEnabled for every account whenever at least one account synced — including an account whose sync just failed with a classified throttle — and the prefetch path never consults AccountThrottleGate, violating the gate's documented contract that background work checks isThrottled/remainingBackoffMillis before touching an account.correctness· angle:crossfileapp/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:96— syncFolderHeaders wraps the whole sync in runCatching, which swallows CancellationException — cancellation is converted into Result.failure (or Result.success in syncAll) instead of propagating, unlike MailBackfiller.pageFolder and IdleService.watchAccount which explicitly rethrow it.correctness· angle:logic· flagged by 2 finder(s)app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:115— Every sync loads ALL synced message ids of the folder into a List + HashSet just to classify at most 50 fetched rows, a cost that grows linearly with backfilled mailbox size on the hottest sync path.efficiency· angle:efficiency· flagged by 2 finder(s)app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:123— The empty-server-folder wipe branch (if (fetched.isEmpty()) deleteSyncedByAccountFolder) has no test in either direction: no test asserts the wipe fires on an empty fetch, and none asserts it is keyed on the RAW fetch rather than the age-filteredentitieslist (the subtlety the comment at lines 123-128 exists to protect).test-gap· angle:testsapp/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:128— MailSyncer's two row-deletion paths (deleteSyncedByAccountFolder at line 128 and deleteSyncedInWindowNotIn at line 146) delete Room rows (attachment metadata cascades via FK) but never delete the per-message on-disk attachment cache directory, orphaning downloaded attachment bytes that only MailPruner and account deletion ever clean up.resource-leak· angle:invariantsapp/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:144— The positive-UID guard on the windowed reconcile bound (entity.uid.takeIf { it > 0L }feeding deleteSyncedInWindowNotIn) is untested in MailSyncer: no test fetches a message with an unresolvable UID (-1/0), even though the identical #95 hazard in MailBackfiller has two dedicated regression tests (MailBackfillerTest lines 342-385).test-gap· angle:testsapp/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:176— MailSyncer.prefetchIfEnabled is a near line-for-line copy of MailBackfiller.prefetchIfEnabled (same DebugFetchGate check, same SyncResourcePolicy.shouldPrefetchContent call, same Gmail daily-budget check with identical log strings), and the copies have already diverged: the backfill copy wraps mailRepository.prefetchMessage(id) in icloudConnectionLimiter.withPermit(account) (MailBackfiller.kt:334) while the syncer copy calls it bare (MailSyncer.kt:204), so post-sync prefetch bypasses the issue-#363 iCloud connection cap. Extract one shared prefetch-gate helper (e.g. a small PrefetchGate class or a function taking account + ids) that both call.reuse· angle:reuseapp/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt:198— Gmail daily download budget is checked once before an unbounded whole-folder prefetch loop, so a single pass can overshoot the 2,500 MB/day budget arbitrarily with no re-check per message.correctness· angle:concurrency· flagged by 3 finder(s)app/src/main/kotlin/org/libremail/data/sync/ThrottleClassifier.kt:78— LOCKOUT_PATTERNS use unanchored substring regexes — Regex("lock(ed|out)") matches 'locked' inside unrelated words and messages ('blocked', SQLite's 'database is locked'), so a local, non-provider error is classified as a provider LOCKOUT and triggers a 1-4 hour per-account backoff.correctness· angle:logic· flagged by 2 finder(s)Low (11)
app/src/main/kotlin/org/libremail/data/sync/AccountThrottleGate.kt:37— Backoff windows are measured with the wall clock (System.currentTimeMillis) instead of SystemClock.elapsedRealtime, so a device clock adjustment stretches or collapses a throttle window.correctness· angle:pitfallsapp/src/main/kotlin/org/libremail/data/sync/GmailBandwidthTracker.kt:79— The 'is this account subject to a bandwidth budget' provider dispatch (GmailSyncLimits.appliesTo(account) && bandwidthTracker.isOverDailyBudget(account.id)) is copy-pasted at three call sites (MailSyncer:198, MailBackfiller:328, MailRepositoryImpl:361) instead of the tracker owning it via Account-taking recordDownload/isOverDailyBudget or a provider-limits policy object.altitude· angle:altitudeapp/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt:109— The throttle-recovery wiring (if (result.batches > 0) throttleGate.onSuccess(account.id)) is untested: MailBackfillerTest never asserts a successful page clears a previously-throttled account's attempt count (grep confirms no onSuccess reference in the test), and AccountThrottleGateTest only covers the gate method in isolation, not the backfiller calling it.test-gap· angle:testsapp/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt:217— The lowest-positive-UID guard expressionentities.mapNotNull { entity -> entity.uid.takeIf { it > 0L } }.minOrNull()— the issue-#95 defence against unresolved-UID placeholders (UIDFolder.getUID == -1) collapsing a bound to <= 0 — is duplicated verbatim in MailBackfiller.kt:217 and MailSyncer.kt:144; it should be one named helper (e.g.List<MessageEntity>.lowestResolvedUid()).reuse· angle:reuseapp/src/main/kotlin/org/libremail/data/sync/MailConnectionFactory.kt:34— The process-wide OAuth access-token cache (tokenCache/refreshMutexes) has no eviction path, so a removed account's still-valid bearer token survives account deletion and is silently reused if the account is re-added.security· angle:securityapp/src/main/kotlin/org/libremail/data/sync/MailConnectionFactory.kt:74— OAuth access-token refresh — redeeming the (rotating) refresh token and persisting the updated AuthState (lines 74-76) — is a significant state change with no AppLog logging anywhere in the chain (this file has zero AppLog calls, and the org.libremail.auth package it delegates to has none either), contrary to CLAUDE.md's Definition of done ('significant state changes' must be logged so behaviour is diagnosable from a debug report).conventions· angle:conventionsapp/src/main/kotlin/org/libremail/data/sync/PruneWorker.kt:37— PruneWorker's runCatching swallows CancellationException — unlike BackfillWorker, which explicitly rethrows it (BackfillWorker.kt:69) — so a normal WorkManager stop is logged as a warning-level 'prune worker: retry' failure and post-cancellation code keeps running.concurrency· angle:pitfalls· flagged by 2 finder(s)app/src/main/kotlin/org/libremail/data/sync/SendWorker.kt:102— sendQueued's runCatching swallows CancellationException and treats a cancelled send as a genuine failure (setError + failed=true), inconsistent with the explicit rethrow in sendOutlook a few lines below.concurrency· angle:concurrencyapp/src/main/kotlin/org/libremail/data/sync/SyncScheduler.kt:35— SyncSchedulerTest pins only unique-name + existing-work policy for each job; none of the WorkRequest constraints (CONNECTED for sync/backfill, battery-not-low for backfill/prune, charging-only for report purge) or the periodic intervals are asserted, though they are inspectable via request.workSpec.test-gap· angle:testsapp/src/main/kotlin/org/libremail/data/sync/ThrottleClassifier.kt:61— ThrottleClassifier.causeChain is a verbatim 9-line copy of ImapAuthError.causeChain (app/src/main/kotlin/org/libremail/mail/ImapAuthError.kt:47) — the KDoc even acknowledges it ('Mirrors the same-shaped walk in org.libremail.mail.ImapAuthError'); both should call one shared Throwable.causeChain() extension (isCacheEncryptionUnavailable at data/local/CacheEncryptionUnavailableException.kt:39 is a third, cycle-unguarded variant via generateSequence(this) { it.cause } that could use it too).reuse· angle:reuseapp/src/main/kotlin/org/libremail/data/sync/ThrottleClassifier.kt:99— The rate-limit pattern Regex("service (not|un)available") is missing a space after 'not', so it matches 'service unavailable'/'service notavailable' but not the canonical SMTP/IMAP 421 wording 'Service not available'.correctness· angle:logic