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 (12)
app/src/main/kotlin/org/libremail/mail/GraphSender.kt:53 — The Graph sendMail size guard is per-attachment only, so multiple attachments that together exceed the ~4 MB request cap are fully read into memory, base64-encoded, and transmitted in a request guaranteed to be rejected.
severity: medium · category: correctness · angle: logic · flagged by 5 finder(s)
An Outlook user attaches two 2.5 MB photos: each passes the 3 MB per-file MAX_ATTACHMENT_BYTES check, so send() reads both files fully (readBytes), base64-encodes them into a ~6.7 MB JSON payload (with additional transient UTF-16 String and toByteArray copies, ~20 MB+ heap spike on a low-RAM device), uploads the whole doomed request over mobile data, and Graph rejects it (413/request too large) before the outbox falls back to SMTP — the exact wasted read + encode + round trip the guard's own comment says it exists to prevent. Cheaper alternative: guard on the summed estimated encoded size before reading any bytes — attachments.sumOf { it.file.length() } scaled by 4/3 plus the body length, the same pre-read estimation IcloudSendLimits.estimatedEncodedBytes already implements — and fail with mayHaveSent=false to take the SMTP fallback immediately.
app/src/main/kotlin/org/libremail/mail/ImapClient.kt:741 — evictIdleReusedConnections/closeReusedConnections are driven only by IdleService (sweep loop in its scope; closeAll only on the low-battery PushMode change), so whenever IdleService is not running — push disabled in settings, or the dataSync FGS-cap degrade path that calls stopSelf without closeReusedConnections — warm authenticated IMAP sockets from the reuse cache are never evicted for the life of the process.
severity: medium · category: resource-leak · angle: crossfile
User turns the push-mail preference off (IdleService never starts) while IMAP_CONNECTION_REUSE stays ON. Every 15-minute periodic sync borrows/creates a per-account kept-alive socket via ImapConnectionCache; with no eviction sweep running, each socket sits open past DEFAULT_REUSE_IDLE_TIMEOUT_MS until the server unilaterally drops it, holding an authenticated session per account indefinitely — the exact battery/socket lingering the eviction contract ('driven by a periodic sweep in IdleService') was added to prevent. Same after the FGS runtime-cap degrade stops the service mid-day.
app/src/main/kotlin/org/libremail/mail/ImapConnectionCache.kt:107 — The transparent stale-socket retry re-runs the caller's whole block, making the copy-then-expunge move non-idempotent: a drop after the server executed COPY but before the response was read causes the retried block to COPY again, duplicating the message in the destination folder
severity: medium · category: concurrency · angle: invariants · flagged by 3 finder(s)
With reuse ON (production default), moveMessages runs copyMessages + setFlags(DELETED) + expungeTargeted inside one withStore block. The server performs the UID COPY but the connection drops before the tagged OK is read (NAT rebind mid-response); the IOException classifies as a connection drop, reconnectAndRetry reconnects and re-runs the entire block: getMessageByUID still resolves the source UIDs, copyMessages runs a second time, then delete+expunge proceed — the destination folder ends with two copies of each moved message. Noted in the class KDoc as deferred to a 'mutation-idempotency review', so it ships as-is; flagging so the debt is tracked as a real duplicate-mail bug rather than only a code comment.
app/src/main/kotlin/org/libremail/mail/ImapConnectionCache.kt:179 — closeAll() has a TOCTOU race with concurrent withStore(): an operation can reconnect an entry after closeAll closed it (or insert a new one mid-iteration), and the trailing entries.clear() then orphans a live authenticated socket that no sweep can ever reach
severity: medium · category: resource-leak · angle: pitfalls · flagged by 2 finder(s)
Low battery triggers IdleService.onPushModeChanged -> imapClient.closeReusedConnections() while a sync/prefetch is in flight. closeAll acquires entry E's mutex after the op releases it, closes the store, sets store=null, and moves on; before entries.clear() runs, a second op calls withStore, computeIfAbsent returns the still-mapped E, reconnects (store != null), and starts its network work; clear() then removes E from the map. The op completes and leaves a connected TLS socket held by an Entry unreachable from entries — evictIdle and future closeAll iterate only entries, so the socket stays open until server timeout/process death, draining battery on exactly the low-battery path the teardown exists for, and counting against the provider's per-account connection ceiling. Fix: remove entries individually (e.g. entries.remove(key, entry)) under the entry lock instead of a bulk clear() after the loop.
app/src/main/kotlin/org/libremail/mail/graph/GraphBatch.kt:66 — GraphBatch.execute (and GraphUploadSession.upload's session-create POST at GraphUploadSession.kt:49-58) send top-level Graph requests with only a Content-Type header and expose no parameter to supply the bearer token, so every $batch and createUploadSession call is guaranteed a 401.
severity: medium · category: correctness · angle: invariants
Graph's $batch and createUploadSession endpoints require an Authorization: Bearer header (contrast GraphSender, which passes it). Neither class accepts an access token, and GraphSubRequest per-op headers cannot substitute for the envelope-level auth. Both classes are currently exercised only by tests using fake clients; the first production caller that wires them (the issue-#364 backfill/large-attachment paths they were built for) will get HTTP 401 on every call — batched sync reads and chunked uploads fail 100% of the time, and the 401 is not even a throttle so it surfaces as a plain failed response.
app/src/main/kotlin/org/libremail/mail/graph/GraphThrottle.kt:38 — Graph's 4-concurrent-requests-per-mailbox cap is implemented as one process-global Semaphore shared by all accounts, not keyed per account like the rest of the throttle policy in the same class.
severity: medium · category: altitude · angle: altitude
With two Outlook accounts signed in, account B's user-initiated me/sendMail queues behind account A's four in-flight $batch backfill reads or upload-session chunks even though each mailbox is entitled to its own 4-request budget — sends and syncs slow down for no provider reason. The class already keys backoff by accountId (throttleGate.onThrottle(accountId, ...)) and IcloudConnectionLimiter shows the correct per-account-semaphore shape (permits.computeIfAbsent(account.id)), so when multi-account Outlook use grows this will be re-fixed by re-implementing keying the layer already has.
app/src/main/kotlin/org/libremail/notifications/MailNotifier.kt:35 — MailNotifier — the component that posts all new-mail notifications — has no test at all (no JVM unit test, no instrumented test), unlike every other file in the mail/push/notifications area.
severity: medium · category: conventions · angle: conventions · flagged by 2 finder(s)
A regression in any of its untested branches ships silently: breaking the notificationId collision branch (lines 129-132, hash == summaryId -> +1) makes a message notification silently replace the account's group summary; removing the hasPermission() gate crashes notifyNewMail with SecurityException on API 33+ when POST_NOTIFICATIONS is denied; a change to channelId()/ensureAccountChannel breaks the per-account channel deep-link from settings and the legacy 'new_mail' channel retirement — all core user-visible alerting with zero CI protection. The repo's own pattern for this exact shape exists (PushStatusNotificationTest for the pure decision + PushStatusNotificationInstrumentedTest for the real Notification build, and NotificationIntentsTest next door), so the gap is fixable without new infrastructure.
app/src/main/kotlin/org/libremail/push/IdleService.kt:192 — IdleService's watcher-reconciliation and reconnect logic (reconcileWatchers/watchAccount) — which encodes the fixes for shipped bugs #90 and #403 — has no test; only the extracted IdleForegroundStarter start/degrade seam and the notification text are covered.
severity: medium · category: test-gap · angle: tests
Untested branches: PushMode.POLLING must cancel every IDLE watcher and call closeReusedConnections() (the #90 low-battery teardown — regressing it silently burns battery on kept-alive sockets while the notification claims polling); removing an account must cancel exactly its watcher (regression leaks an IDLE connection authenticated against a deleted account); watchAccount's exponential backoff must double, cap at MAX_BACKOFF_MS, and reset after a successful IDLE_RENEWAL_MS cycle (regression = reconnect storm or permanently slow retry -> missed push mail); the MissingCredentialsException flat 1s defer (#403) must not escalate into warn+backoff. All of this is coroutine logic driven by injected seams (accountDao, batteryStatusProvider, imapClient, mailSyncer) that could be extracted into a JVM-testable reconciler like IdleForegroundStarter was for #354, but today any regression here reaches users as silently missed or stuck push mail.
app/src/main/kotlin/org/libremail/push/IdleService.kt:205 — reconcileWatchers keys watchers only by account.id and never restarts one when the account row changes, so a running IDLE watcher keeps the stale Account snapshot (host/port/security/username) forever.
severity: medium · category: correctness · angle: invariants
AccountDao.update refreshes an account's IMAP server settings (e.g. user corrects a wrong host or port). observeAll emits the updated entity, but since account.id is already in watchers, the launched watchAccount coroutine keeps calling imapParamsFor(staleAccount) — which builds params from the captured object's old host/port — on every reconnect. IDLE push retries the dead/old server with exponential backoff indefinitely (battery burn, no push mail) until the service process is killed and recreated, while the user believes the fix took effect.
app/src/main/kotlin/org/libremail/push/IdleService.kt:214 — onPushModeChanged() calls startAsForeground() from an unguarded background coroutine, so a push-mode flip landing after the dataSync FGS runtime-cap timeout throws ForegroundServiceStartNotAllowedException in a SupervisorJob child with no handler and crashes the process — the exact #354 crash class the IdleForegroundStarter seam guards only on the onStartCommand path.
severity: medium · category: concurrency · angle: pitfalls
Android 14+ device: the dataSync runtime cap fires onTimeout -> degradeToPeriodicSync() (stopForeground + stopSelf), but scope is only cancelled in onDestroy. In that window the reconcileWatchers combine collector (Dispatchers.IO) emits a battery-driven PushMode change (mode != shownMode), onPushModeChanged() runs startAsForeground(mode); post-timeout the system rejects the startForeground for the capped dataSync type with ForegroundServiceStartNotAllowedException (an IllegalStateException), which nothing catches; the child of the service's SupervisorJob routes it to the default handler and the app crashes.
app/src/main/kotlin/org/libremail/push/IdleService.kt:312 — IDLE renewal tears down the whole TLS connection and triggers a full account sync every 9 minutes per account even when no mail arrived, instead of re-issuing IDLE on the live connection.
severity: medium · category: efficiency · angle: efficiency
Phone idle overnight with 2 push-enabled IMAP accounts and zero new mail: every 9 minutes withTimeoutOrNull(IDLE_RENEWAL_MS) cancels ImapClient.idle(), which closes the socket; the loop then pays a fresh CONNECT + TLS + LOGIN, and idle()'s unconditional on-connect pushes.trySend(Unit) (ImapClient.kt:580) fires a full syncAccount — fetchRecent of the whole recent-window headers plus body prefetch — on every renewal. That is ~160 reconnects and ~160 full syncs per account per day with nothing to sync: continuous radio wakeups/battery drain, and exactly the connection-churn traffic the repo's own issue-125 investigation proved Gmail throttles. Cheaper alternative: renew IDLE in place — abort it from a watchdog with a NOOP via folder.doCommand()/getMessageCount() so the existing while (isActive) inbox.idle() loop re-issues IDLE on the same authenticated connection (no teardown gap, so the gap-covering sync becomes unnecessary) — or at minimum suppress the on-connect sync signal for timeout-driven renewals, keeping it only for the initial connect and error reconnects.
app/src/main/kotlin/org/libremail/push/IdleService.kt:338 — The reuse cache's idle eviction is driven solely by IdleService's sweep loop, and onDestroy only cancels the scope without closing reused connections — so whenever push is off, warm IMAP sockets are never evicted despite the cache's documented 5-minute idle timeout
severity: medium · category: resource-leak · angle: concurrency
User disables push mail (IdlePushManager.stop), or IdleService degrades and self-stops via the dataSync FGS cap/background-start path. onDestroy() runs scope.cancel() only; closeReusedConnections()/evictIdleReusedConnections() are called nowhere else in the app (verified by grep). The @Singleton ImapClient's ImapConnectionCache keeps its authenticated TLS socket(s) open, and the 15-minute periodic WorkManager sync re-warms them each cycle, so with push disabled the app permanently holds one live socket per account with no sweeper — the exact lingering-socket battery drain evictIdle was designed to prevent (its KDoc assumes the IdleService sweep is always running). Fix: close reused connections in onDestroy and/or drive eviction from a component that lives whenever syncs do (e.g. the sync worker).
Low (13)
app/src/main/kotlin/org/libremail/mail/HtmlToText.kt:20 — BLOCK_BREAK handles tr/table but not td/th, so adjacent table cells concatenate with no separator in the plain-text conversion.
An HTML receipt contains <tr><td>Total</td><td>$42.00</td></tr>. td tags are removed by the generic TAG regex without inserting any whitespace, producing the line 'Total$42.00' (and e.g. 'Item3Qty2' for adjacent cells) in the text/plain alternative part and in quoted replies to HTML mail — garbling table-heavy messages (receipts, order confirmations, alerts) that the converter's stated goal is to keep legible. Adding td/th to BLOCK_BREAK (or emitting a space/tab per cell) fixes it.
app/src/main/kotlin/org/libremail/mail/IcloudSendLimits.kt:51 — Outgoing message-size policy is a provider-specific object with its own inline provider check and dedicated SendWorker call site — and the SMTP-send guard keys provider identity off the IMAP host — instead of a maxOutgoingBytes capability on MailProvider checked once in the send path.
Two costs: (1) issues #361/#362/#364 (Gmail/Yahoo-AOL/Outlook outgoing caps) each need another copy-pasted XSendLimits object, another requireWithinLimit call inserted into SendWorker, and a duplicated base64 wire-size estimator — forget one call site and that provider's oversized sends go back to burning a connection on a guaranteed server rejection with a generic error; (2) MailProvider.forImapHost(account.imap.host) gates an SMTP-path policy on receive-path config, so a manually configured account using smtp.mail.me.com with a non-iCloud IMAP host silently escapes the guard today. The KDoc documents the per-ticket split as deliberate, so this is flagged as the consolidation debt it creates, not an oversight.
app/src/main/kotlin/org/libremail/mail/ImapClient.kt:173 — ImapClient repeats the open-folder session boilerplate ('store.getFolder(folder); mailbox.open(mode); try { … } finally { runCatching { mailbox.close(false) } }') verbatim in 10 operations, the identical 3-item header FetchProfile (ENVELOPE+FLAGS+UID) 3 times (fetchRecent/fetchOlderThan/search), and the '(mailbox as UIDFolder).getMessageByUID(uid.toLong()) ?: error("Message $uid not found")' lookup 4 times, instead of a single withOpenFolder(store, name, mode) { … } helper plus a headerFetchProfile()/requireMessageByUid() pair.
severity: low · category: reuse · angle: reuse
Any change to the session contract — e.g. logging a failed close instead of swallowing it, switching close(false) to close(true) on some path, or adding RFC822.SIZE to the header profile so list rows can show sizes — must be hand-applied at 10 (or 3) sites; missing one leaves that operation silently on the old behaviour (fetchOlderThan already diverged once by needing its own copy of the fetchRecent profile). The duplication is exactly what a shared helper in this same class would eliminate.
app/src/main/kotlin/org/libremail/mail/ImapClient.kt:544 — ImapClient.idle() re-implements the existing openConnectedStore() helper inline: lines 543-545 duplicate its protocol selection ('imaps' vs 'imap'), Session.getInstance(buildProps(...)).getStore(protocol), and store.connect(host, port, username, secret) — the only difference is not passing the reuse flag to buildProps.
severity: low · category: reuse · angle: reuse
A future change to connection establishment made in openConnectedStore (e.g. an extra auth property, proxy support, or a different connect overload for a provider quirk) silently does not apply to the long-lived IDLE connection, so push connects with different TLS/auth behaviour than every other IMAP operation; calling openConnectedStore(params) (with the reuse flag parameterised) keeps the two paths provably identical.
app/src/main/kotlin/org/libremail/mail/ImapClient.kt:711 — The ImapPerf breadcrumb machinery is implemented twice: ImapClient.withStore's no-reuse branch (its own liveConnectionCount AtomicInteger, NANOS_PER_MS constant, and the '"$op connect=Xms work=Yms live=Z"' format string) duplicates ImapConnectionCache.runReusing/ensureConnected/liveCount()/elapsedMs (ImapConnectionCache.kt lines 98-125, 190-192, 225), which emit the same PERF_TAG="ImapPerf" line with independent code.
severity: low · category: reuse · angle: reuse
The breadcrumb format is a debug-report attribution contract (per the #125/#357 perf docs); if one copy changes — say adding a bytes= field or renaming live= — reuse-ON and reuse-OFF builds emit inconsistent ImapPerf lines and any tooling or triage that parses them misattributes latency for one path. The two live-connection counters also count different things under the same 'live=' label (per-op sockets vs cached stores), which a shared breadcrumb helper would have forced into one definition.
app/src/main/kotlin/org/libremail/mail/SmtpSender.kt:146 — The 15-second network timeout policy is independently copy-pasted in three transports: SmtpSender.TIMEOUT_MS="15000", ImapClient.TIMEOUT_MS="15000" (ImapClient.kt:774), and GraphHttpClient.TIMEOUT_MS=15_000 (GraphHttp.kt:114).
A timeout tuning — e.g. raising connect timeout for slow cellular links, or making it user-configurable — must be re-fixed in three files with two different types (String vs Int); missing one leaves transports silently diverging (IMAP fetches tolerate 30 s while SMTP sends still abort at 15 s), a class of drift a single shared transport-timeout constant/policy would prevent.
app/src/main/kotlin/org/libremail/mail/graph/GraphBatch.kt:47 — GraphBatch and GraphUploadSession (and, transitively, GraphThrottle.recordSubResponseThrottle) are dead production code: no caller outside their own unit/instrumented tests exists anywhere in app/src/main — the only non-test reference is a GraphSender comment explaining why the upload-session path is deliberately NOT used (the send-only token lacks Mail.ReadWrite).
~250 lines of Graph-protocol infrastructure (20-op batch cap, 320 KiB chunk-multiple rule, per-sub-response Retry-After parsing) plus GraphBatchTest/GraphUploadSessionTest must be maintained and kept in sync with Microsoft Graph documentation while nothing exercises them at runtime; a reader of GraphThrottle reasonably assumes batched reads and chunked uploads are live traffic and sizes throttle policy (MAX_CONCURRENT_REQUESTS, retry budgets) around paths that never run. Either wire them in for the Outlook read path or delete them (Git preserves them for when #364's read multiplexing lands).
app/src/main/kotlin/org/libremail/mail/graph/GraphBatch.kt:62 — GraphBatch.execute awaits each 20-op $batch chunk strictly sequentially even though GraphThrottle permits 4 concurrent Graph requests, multiplying multiplexed-read latency by the chunk count.
A backfill page of 100 message reads chunks into 5 batch calls; the for (chunk in chunks) loop suspends on throttle.execute for each in turn, so the page takes 5 full HTTP round trips end-to-end (~1.5 s at 300 ms RTT) where the existing MAX_CONCURRENT_REQUESTS=4 semaphore would allow them in ~2 round-trip waves (~600 ms). Cheaper alternative: launch the chunks concurrently (coroutineScope { chunks.map { async { … } } }.awaitAll(), flattened in order) — the Semaphore in GraphThrottle already enforces the 4-concurrent mailbox cap, and per-chunk results are independent, so ordering of the returned list is preserved by awaiting in list order. Low because GraphBatch currently has no production caller.
app/src/main/kotlin/org/libremail/mail/graph/GraphUploadSession.kt:43 — upload() requires the entire content as one in-memory ByteArray and additionally allocates a copyOfRange per chunk, although the class exists precisely for large (>4 MB, up to ~150 MB) payloads.
When this session-upload path gains a caller (its stated purpose: attachments over the 4 MB one-shot cap), uploading e.g. a 60 MB attachment means the caller must first materialize all 60 MB on the heap, and then line 79's content.copyOfRange(offset, end) allocates a further ~4.7 MB copy per PUT — sustained heap pressure that can hit Android's per-app heap limit and OOM-kill the process mid-send, or trigger GC churn during the transfer. Cheaper alternative: accept a File (or InputStream + length) and read each chunk into one reusable chunkSize buffer via RandomAccessFile/FileInputStream, so peak extra heap is one chunk (~4.7 MB) regardless of attachment size. No production caller exists yet (GraphSender deliberately falls back to SMTP), so the cost is latent — hence low.
app/src/main/kotlin/org/libremail/notifications/MailNotifier.kt:130 — Per-message notification ids are messageId.hashCode(), guarded against colliding only with the same account's summary id — a 32-bit hash collision between two message ids, or with another account's summary id, silently replaces a still-unacknowledged notification.
Two distinct message ids whose String.hashCode() collide (or a message id hashing to another account's "summary:".hashCode(), or to summaryId+1 via the +1 dodge) cause manager.notify to reuse an existing notification id: the earlier unread-mail notification is overwritten by an unrelated message's content, so the user never sees one of the new-mail alerts — violating the stated invariant that "a later batch never overwrites an earlier, still-unacknowledged one".
app/src/main/kotlin/org/libremail/push/IdlePushManager.kt:18 — IdlePushManager.start() (and stop()) swallow startForegroundService/stopService failures via bare runCatching with no AppLog call, violating CLAUDE.md's Definition-of-done rule that error/fallback paths and lifecycle transitions must be logged so behaviour is diagnosable from a debug report.
start() is invoked while the process is backgrounded; startForegroundService throws ForegroundServiceStartNotAllowedException and is silently swallowed (the comment at lines 15-17 documents the swallow but nothing is logged). The user reports 'instant push never works'; the DebugReport's RingLogBuffer contains no trace that a push start was attempted and rejected, so the failure is undiagnosable — the exact class of gap CLAUDE.md's 'appropriate logging ... error/fallback paths ... diagnosable from a user's debug report' rule forbids.
app/src/main/kotlin/org/libremail/push/IdleService.kt:294 — The POST_NOTIFICATIONS grant check is copy-pasted three times: IdleService.hasNotificationPermission() (line 294), MailNotifier.hasPermission() (MailNotifier.kt line 134), and inline in OnboardingWelcomeScreen.kt line 117 — all the identical ContextCompat.checkSelfPermission(context, POST_NOTIFICATIONS) == PERMISSION_GRANTED expression, with no shared helper.
severity: low · category: reuse · angle: reuse
If the gate ever needs refinement — e.g. also consulting NotificationManagerCompat.areNotificationsEnabled() (a user can disable notifications app-wide without revoking the runtime permission, in which case notify() posts nothing) — the fix must be found and applied in three files; updating only MailNotifier leaves IdleService's degraded-mode status notification path (degradeToPeriodicSync) using the stale rule. A single top-level hasPostNotificationsPermission(context) helper next to MailNotifier removes the divergence risk.
app/src/main/kotlin/org/libremail/push/IdleService.kt:329 — The IDLE reconnect loop implements its own private backoff knob (INITIAL_BACKOFF_MS/MAX_BACKOFF_MS) and neither classifies failures via ThrottleClassifier nor records/honors the shared per-account AccountThrottleGate that sync, backfill, and Graph all feed (#360).
When a provider throttles or connection-locks an account at LOGIN (e.g. Gmail 'too many simultaneous connections' or the rate clamp documented in the #125 perf drilldown), MailSyncer/MailBackfiller back the account off exponentially through the gate — but the IDLE watcher keeps re-authenticating every <=5 min and fires an on-connect sync each time, re-tripping the very limit the gate exists to respect. The #360 design goal ('paths never fight the same provider limit from two directions') is defeated by this third, ungated direction, and any per-provider connection-lockout fix (#360-#364 family) must be re-applied separately inside this loop.
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 (12)
### `app/src/main/kotlin/org/libremail/mail/GraphSender.kt:53` — The Graph sendMail size guard is per-attachment only, so multiple attachments that together exceed the ~4 MB request cap are fully read into memory, base64-encoded, and transmitted in a request guaranteed to be rejected.
- severity: **medium** · category: `correctness` · angle: `logic` · flagged by 5 finder(s)
> An Outlook user attaches two 2.5 MB photos: each passes the 3 MB per-file MAX_ATTACHMENT_BYTES check, so send() reads both files fully (readBytes), base64-encodes them into a ~6.7 MB JSON payload (with additional transient UTF-16 String and toByteArray copies, ~20 MB+ heap spike on a low-RAM device), uploads the whole doomed request over mobile data, and Graph rejects it (413/request too large) before the outbox falls back to SMTP — the exact wasted read + encode + round trip the guard's own comment says it exists to prevent. Cheaper alternative: guard on the summed estimated encoded size before reading any bytes — attachments.sumOf { it.file.length() } scaled by 4/3 plus the body length, the same pre-read estimation IcloudSendLimits.estimatedEncodedBytes already implements — and fail with mayHaveSent=false to take the SMTP fallback immediately.
### `app/src/main/kotlin/org/libremail/mail/ImapClient.kt:741` — evictIdleReusedConnections/closeReusedConnections are driven only by IdleService (sweep loop in its scope; closeAll only on the low-battery PushMode change), so whenever IdleService is not running — push disabled in settings, or the dataSync FGS-cap degrade path that calls stopSelf without closeReusedConnections — warm authenticated IMAP sockets from the reuse cache are never evicted for the life of the process.
- severity: **medium** · category: `resource-leak` · angle: `crossfile`
> User turns the push-mail preference off (IdleService never starts) while IMAP_CONNECTION_REUSE stays ON. Every 15-minute periodic sync borrows/creates a per-account kept-alive socket via ImapConnectionCache; with no eviction sweep running, each socket sits open past DEFAULT_REUSE_IDLE_TIMEOUT_MS until the server unilaterally drops it, holding an authenticated session per account indefinitely — the exact battery/socket lingering the eviction contract ('driven by a periodic sweep in IdleService') was added to prevent. Same after the FGS runtime-cap degrade stops the service mid-day.
### `app/src/main/kotlin/org/libremail/mail/ImapConnectionCache.kt:107` — The transparent stale-socket retry re-runs the caller's whole block, making the copy-then-expunge move non-idempotent: a drop after the server executed COPY but before the response was read causes the retried block to COPY again, duplicating the message in the destination folder
- severity: **medium** · category: `concurrency` · angle: `invariants` · flagged by 3 finder(s)
> With reuse ON (production default), moveMessages runs copyMessages + setFlags(DELETED) + expungeTargeted inside one withStore block. The server performs the UID COPY but the connection drops before the tagged OK is read (NAT rebind mid-response); the IOException classifies as a connection drop, reconnectAndRetry reconnects and re-runs the entire block: getMessageByUID still resolves the source UIDs, copyMessages runs a second time, then delete+expunge proceed — the destination folder ends with two copies of each moved message. Noted in the class KDoc as deferred to a 'mutation-idempotency review', so it ships as-is; flagging so the debt is tracked as a real duplicate-mail bug rather than only a code comment.
### `app/src/main/kotlin/org/libremail/mail/ImapConnectionCache.kt:179` — closeAll() has a TOCTOU race with concurrent withStore(): an operation can reconnect an entry after closeAll closed it (or insert a new one mid-iteration), and the trailing entries.clear() then orphans a live authenticated socket that no sweep can ever reach
- severity: **medium** · category: `resource-leak` · angle: `pitfalls` · flagged by 2 finder(s)
> Low battery triggers IdleService.onPushModeChanged -> imapClient.closeReusedConnections() while a sync/prefetch is in flight. closeAll acquires entry E's mutex after the op releases it, closes the store, sets store=null, and moves on; before entries.clear() runs, a second op calls withStore, computeIfAbsent returns the still-mapped E, reconnects (store != null), and starts its network work; clear() then removes E from the map. The op completes and leaves a connected TLS socket held by an Entry unreachable from `entries` — evictIdle and future closeAll iterate only `entries`, so the socket stays open until server timeout/process death, draining battery on exactly the low-battery path the teardown exists for, and counting against the provider's per-account connection ceiling. Fix: remove entries individually (e.g. entries.remove(key, entry)) under the entry lock instead of a bulk clear() after the loop.
### `app/src/main/kotlin/org/libremail/mail/graph/GraphBatch.kt:66` — GraphBatch.execute (and GraphUploadSession.upload's session-create POST at GraphUploadSession.kt:49-58) send top-level Graph requests with only a Content-Type header and expose no parameter to supply the bearer token, so every $batch and createUploadSession call is guaranteed a 401.
- severity: **medium** · category: `correctness` · angle: `invariants`
> Graph's $batch and createUploadSession endpoints require an Authorization: Bearer header (contrast GraphSender, which passes it). Neither class accepts an access token, and GraphSubRequest per-op headers cannot substitute for the envelope-level auth. Both classes are currently exercised only by tests using fake clients; the first production caller that wires them (the issue-#364 backfill/large-attachment paths they were built for) will get HTTP 401 on every call — batched sync reads and chunked uploads fail 100% of the time, and the 401 is not even a throttle so it surfaces as a plain failed response.
### `app/src/main/kotlin/org/libremail/mail/graph/GraphThrottle.kt:38` — Graph's 4-concurrent-requests-per-mailbox cap is implemented as one process-global Semaphore shared by all accounts, not keyed per account like the rest of the throttle policy in the same class.
- severity: **medium** · category: `altitude` · angle: `altitude`
> With two Outlook accounts signed in, account B's user-initiated me/sendMail queues behind account A's four in-flight $batch backfill reads or upload-session chunks even though each mailbox is entitled to its own 4-request budget — sends and syncs slow down for no provider reason. The class already keys backoff by accountId (throttleGate.onThrottle(accountId, ...)) and IcloudConnectionLimiter shows the correct per-account-semaphore shape (permits.computeIfAbsent(account.id)), so when multi-account Outlook use grows this will be re-fixed by re-implementing keying the layer already has.
### `app/src/main/kotlin/org/libremail/notifications/MailNotifier.kt:35` — MailNotifier — the component that posts all new-mail notifications — has no test at all (no JVM unit test, no instrumented test), unlike every other file in the mail/push/notifications area.
- severity: **medium** · category: `conventions` · angle: `conventions` · flagged by 2 finder(s)
> A regression in any of its untested branches ships silently: breaking the notificationId collision branch (lines 129-132, hash == summaryId -> +1) makes a message notification silently replace the account's group summary; removing the hasPermission() gate crashes notifyNewMail with SecurityException on API 33+ when POST_NOTIFICATIONS is denied; a change to channelId()/ensureAccountChannel breaks the per-account channel deep-link from settings and the legacy 'new_mail' channel retirement — all core user-visible alerting with zero CI protection. The repo's own pattern for this exact shape exists (PushStatusNotificationTest for the pure decision + PushStatusNotificationInstrumentedTest for the real Notification build, and NotificationIntentsTest next door), so the gap is fixable without new infrastructure.
### `app/src/main/kotlin/org/libremail/push/IdleService.kt:192` — IdleService's watcher-reconciliation and reconnect logic (reconcileWatchers/watchAccount) — which encodes the fixes for shipped bugs #90 and #403 — has no test; only the extracted IdleForegroundStarter start/degrade seam and the notification text are covered.
- severity: **medium** · category: `test-gap` · angle: `tests`
> Untested branches: PushMode.POLLING must cancel every IDLE watcher and call closeReusedConnections() (the #90 low-battery teardown — regressing it silently burns battery on kept-alive sockets while the notification claims polling); removing an account must cancel exactly its watcher (regression leaks an IDLE connection authenticated against a deleted account); watchAccount's exponential backoff must double, cap at MAX_BACKOFF_MS, and reset after a successful IDLE_RENEWAL_MS cycle (regression = reconnect storm or permanently slow retry -> missed push mail); the MissingCredentialsException flat 1s defer (#403) must not escalate into warn+backoff. All of this is coroutine logic driven by injected seams (accountDao, batteryStatusProvider, imapClient, mailSyncer) that could be extracted into a JVM-testable reconciler like IdleForegroundStarter was for #354, but today any regression here reaches users as silently missed or stuck push mail.
### `app/src/main/kotlin/org/libremail/push/IdleService.kt:205` — reconcileWatchers keys watchers only by account.id and never restarts one when the account row changes, so a running IDLE watcher keeps the stale Account snapshot (host/port/security/username) forever.
- severity: **medium** · category: `correctness` · angle: `invariants`
> AccountDao.update refreshes an account's IMAP server settings (e.g. user corrects a wrong host or port). observeAll emits the updated entity, but since account.id is already in watchers, the launched watchAccount coroutine keeps calling imapParamsFor(staleAccount) — which builds params from the captured object's old host/port — on every reconnect. IDLE push retries the dead/old server with exponential backoff indefinitely (battery burn, no push mail) until the service process is killed and recreated, while the user believes the fix took effect.
### `app/src/main/kotlin/org/libremail/push/IdleService.kt:214` — onPushModeChanged() calls startAsForeground() from an unguarded background coroutine, so a push-mode flip landing after the dataSync FGS runtime-cap timeout throws ForegroundServiceStartNotAllowedException in a SupervisorJob child with no handler and crashes the process — the exact #354 crash class the IdleForegroundStarter seam guards only on the onStartCommand path.
- severity: **medium** · category: `concurrency` · angle: `pitfalls`
> Android 14+ device: the dataSync runtime cap fires onTimeout -> degradeToPeriodicSync() (stopForeground + stopSelf), but scope is only cancelled in onDestroy. In that window the reconcileWatchers combine collector (Dispatchers.IO) emits a battery-driven PushMode change (mode != shownMode), onPushModeChanged() runs startAsForeground(mode); post-timeout the system rejects the startForeground for the capped dataSync type with ForegroundServiceStartNotAllowedException (an IllegalStateException), which nothing catches; the child of the service's SupervisorJob routes it to the default handler and the app crashes.
### `app/src/main/kotlin/org/libremail/push/IdleService.kt:312` — IDLE renewal tears down the whole TLS connection and triggers a full account sync every 9 minutes per account even when no mail arrived, instead of re-issuing IDLE on the live connection.
- severity: **medium** · category: `efficiency` · angle: `efficiency`
> Phone idle overnight with 2 push-enabled IMAP accounts and zero new mail: every 9 minutes withTimeoutOrNull(IDLE_RENEWAL_MS) cancels ImapClient.idle(), which closes the socket; the loop then pays a fresh CONNECT + TLS + LOGIN, and idle()'s unconditional on-connect pushes.trySend(Unit) (ImapClient.kt:580) fires a full syncAccount — fetchRecent of the whole recent-window headers plus body prefetch — on every renewal. That is ~160 reconnects and ~160 full syncs per account per day with nothing to sync: continuous radio wakeups/battery drain, and exactly the connection-churn traffic the repo's own issue-125 investigation proved Gmail throttles. Cheaper alternative: renew IDLE in place — abort it from a watchdog with a NOOP via folder.doCommand()/getMessageCount() so the existing `while (isActive) inbox.idle()` loop re-issues IDLE on the same authenticated connection (no teardown gap, so the gap-covering sync becomes unnecessary) — or at minimum suppress the on-connect sync signal for timeout-driven renewals, keeping it only for the initial connect and error reconnects.
### `app/src/main/kotlin/org/libremail/push/IdleService.kt:338` — The reuse cache's idle eviction is driven solely by IdleService's sweep loop, and onDestroy only cancels the scope without closing reused connections — so whenever push is off, warm IMAP sockets are never evicted despite the cache's documented 5-minute idle timeout
- severity: **medium** · category: `resource-leak` · angle: `concurrency`
> User disables push mail (IdlePushManager.stop), or IdleService degrades and self-stops via the dataSync FGS cap/background-start path. onDestroy() runs scope.cancel() only; closeReusedConnections()/evictIdleReusedConnections() are called nowhere else in the app (verified by grep). The @Singleton ImapClient's ImapConnectionCache keeps its authenticated TLS socket(s) open, and the 15-minute periodic WorkManager sync re-warms them each cycle, so with push disabled the app permanently holds one live socket per account with no sweeper — the exact lingering-socket battery drain evictIdle was designed to prevent (its KDoc assumes the IdleService sweep is always running). Fix: close reused connections in onDestroy and/or drive eviction from a component that lives whenever syncs do (e.g. the sync worker).
## Low (13)
### `app/src/main/kotlin/org/libremail/mail/HtmlToText.kt:20` — BLOCK_BREAK handles tr/table but not td/th, so adjacent table cells concatenate with no separator in the plain-text conversion.
- severity: **low** · category: `correctness` · angle: `logic`
> An HTML receipt contains `<tr><td>Total</td><td>$42.00</td></tr>`. td tags are removed by the generic TAG regex without inserting any whitespace, producing the line 'Total$42.00' (and e.g. 'Item3Qty2' for adjacent cells) in the text/plain alternative part and in quoted replies to HTML mail — garbling table-heavy messages (receipts, order confirmations, alerts) that the converter's stated goal is to keep legible. Adding td/th to BLOCK_BREAK (or emitting a space/tab per cell) fixes it.
### `app/src/main/kotlin/org/libremail/mail/IcloudSendLimits.kt:51` — Outgoing message-size policy is a provider-specific object with its own inline provider check and dedicated SendWorker call site — and the SMTP-send guard keys provider identity off the IMAP host — instead of a maxOutgoingBytes capability on MailProvider checked once in the send path.
- severity: **low** · category: `altitude` · angle: `altitude`
> Two costs: (1) issues #361/#362/#364 (Gmail/Yahoo-AOL/Outlook outgoing caps) each need another copy-pasted XSendLimits object, another requireWithinLimit call inserted into SendWorker, and a duplicated base64 wire-size estimator — forget one call site and that provider's oversized sends go back to burning a connection on a guaranteed server rejection with a generic error; (2) MailProvider.forImapHost(account.imap.host) gates an SMTP-path policy on receive-path config, so a manually configured account using smtp.mail.me.com with a non-iCloud IMAP host silently escapes the guard today. The KDoc documents the per-ticket split as deliberate, so this is flagged as the consolidation debt it creates, not an oversight.
### `app/src/main/kotlin/org/libremail/mail/ImapClient.kt:173` — ImapClient repeats the open-folder session boilerplate ('store.getFolder(folder); mailbox.open(mode); try { … } finally { runCatching { mailbox.close(false) } }') verbatim in 10 operations, the identical 3-item header FetchProfile (ENVELOPE+FLAGS+UID) 3 times (fetchRecent/fetchOlderThan/search), and the '(mailbox as UIDFolder).getMessageByUID(uid.toLong()) ?: error("Message $uid not found")' lookup 4 times, instead of a single withOpenFolder(store, name, mode) { … } helper plus a headerFetchProfile()/requireMessageByUid() pair.
- severity: **low** · category: `reuse` · angle: `reuse`
> Any change to the session contract — e.g. logging a failed close instead of swallowing it, switching close(false) to close(true) on some path, or adding RFC822.SIZE to the header profile so list rows can show sizes — must be hand-applied at 10 (or 3) sites; missing one leaves that operation silently on the old behaviour (fetchOlderThan already diverged once by needing its own copy of the fetchRecent profile). The duplication is exactly what a shared helper in this same class would eliminate.
### `app/src/main/kotlin/org/libremail/mail/ImapClient.kt:544` — ImapClient.idle() re-implements the existing openConnectedStore() helper inline: lines 543-545 duplicate its protocol selection ('imaps' vs 'imap'), Session.getInstance(buildProps(...)).getStore(protocol), and store.connect(host, port, username, secret) — the only difference is not passing the reuse flag to buildProps.
- severity: **low** · category: `reuse` · angle: `reuse`
> A future change to connection establishment made in openConnectedStore (e.g. an extra auth property, proxy support, or a different connect overload for a provider quirk) silently does not apply to the long-lived IDLE connection, so push connects with different TLS/auth behaviour than every other IMAP operation; calling openConnectedStore(params) (with the reuse flag parameterised) keeps the two paths provably identical.
### `app/src/main/kotlin/org/libremail/mail/ImapClient.kt:711` — The ImapPerf breadcrumb machinery is implemented twice: ImapClient.withStore's no-reuse branch (its own liveConnectionCount AtomicInteger, NANOS_PER_MS constant, and the '"$op connect=Xms work=Yms live=Z"' format string) duplicates ImapConnectionCache.runReusing/ensureConnected/liveCount()/elapsedMs (ImapConnectionCache.kt lines 98-125, 190-192, 225), which emit the same PERF_TAG="ImapPerf" line with independent code.
- severity: **low** · category: `reuse` · angle: `reuse`
> The breadcrumb format is a debug-report attribution contract (per the #125/#357 perf docs); if one copy changes — say adding a bytes= field or renaming live= — reuse-ON and reuse-OFF builds emit inconsistent ImapPerf lines and any tooling or triage that parses them misattributes latency for one path. The two live-connection counters also count different things under the same 'live=' label (per-op sockets vs cached stores), which a shared breadcrumb helper would have forced into one definition.
### `app/src/main/kotlin/org/libremail/mail/SmtpSender.kt:146` — The 15-second network timeout policy is independently copy-pasted in three transports: SmtpSender.TIMEOUT_MS="15000", ImapClient.TIMEOUT_MS="15000" (ImapClient.kt:774), and GraphHttpClient.TIMEOUT_MS=15_000 (GraphHttp.kt:114).
- severity: **low** · category: `altitude` · angle: `altitude`
> A timeout tuning — e.g. raising connect timeout for slow cellular links, or making it user-configurable — must be re-fixed in three files with two different types (String vs Int); missing one leaves transports silently diverging (IMAP fetches tolerate 30 s while SMTP sends still abort at 15 s), a class of drift a single shared transport-timeout constant/policy would prevent.
### `app/src/main/kotlin/org/libremail/mail/graph/GraphBatch.kt:47` — GraphBatch and GraphUploadSession (and, transitively, GraphThrottle.recordSubResponseThrottle) are dead production code: no caller outside their own unit/instrumented tests exists anywhere in app/src/main — the only non-test reference is a GraphSender comment explaining why the upload-session path is deliberately NOT used (the send-only token lacks Mail.ReadWrite).
- severity: **low** · category: `dead-code` · angle: `reuse`
> ~250 lines of Graph-protocol infrastructure (20-op batch cap, 320 KiB chunk-multiple rule, per-sub-response Retry-After parsing) plus GraphBatchTest/GraphUploadSessionTest must be maintained and kept in sync with Microsoft Graph documentation while nothing exercises them at runtime; a reader of GraphThrottle reasonably assumes batched reads and chunked uploads are live traffic and sizes throttle policy (MAX_CONCURRENT_REQUESTS, retry budgets) around paths that never run. Either wire them in for the Outlook read path or delete them (Git preserves them for when #364's read multiplexing lands).
### `app/src/main/kotlin/org/libremail/mail/graph/GraphBatch.kt:62` — GraphBatch.execute awaits each 20-op $batch chunk strictly sequentially even though GraphThrottle permits 4 concurrent Graph requests, multiplying multiplexed-read latency by the chunk count.
- severity: **low** · category: `efficiency` · angle: `efficiency`
> A backfill page of 100 message reads chunks into 5 batch calls; the `for (chunk in chunks)` loop suspends on throttle.execute for each in turn, so the page takes 5 full HTTP round trips end-to-end (~1.5 s at 300 ms RTT) where the existing MAX_CONCURRENT_REQUESTS=4 semaphore would allow them in ~2 round-trip waves (~600 ms). Cheaper alternative: launch the chunks concurrently (coroutineScope { chunks.map { async { … } } }.awaitAll(), flattened in order) — the Semaphore in GraphThrottle already enforces the 4-concurrent mailbox cap, and per-chunk results are independent, so ordering of the returned list is preserved by awaiting in list order. Low because GraphBatch currently has no production caller.
### `app/src/main/kotlin/org/libremail/mail/graph/GraphUploadSession.kt:43` — upload() requires the entire content as one in-memory ByteArray and additionally allocates a copyOfRange per chunk, although the class exists precisely for large (>4 MB, up to ~150 MB) payloads.
- severity: **low** · category: `efficiency` · angle: `efficiency`
> When this session-upload path gains a caller (its stated purpose: attachments over the 4 MB one-shot cap), uploading e.g. a 60 MB attachment means the caller must first materialize all 60 MB on the heap, and then line 79's content.copyOfRange(offset, end) allocates a further ~4.7 MB copy per PUT — sustained heap pressure that can hit Android's per-app heap limit and OOM-kill the process mid-send, or trigger GC churn during the transfer. Cheaper alternative: accept a File (or InputStream + length) and read each chunk into one reusable chunkSize buffer via RandomAccessFile/FileInputStream, so peak extra heap is one chunk (~4.7 MB) regardless of attachment size. No production caller exists yet (GraphSender deliberately falls back to SMTP), so the cost is latent — hence low.
### `app/src/main/kotlin/org/libremail/notifications/MailNotifier.kt:130` — Per-message notification ids are messageId.hashCode(), guarded against colliding only with the same account's summary id — a 32-bit hash collision between two message ids, or with another account's summary id, silently replaces a still-unacknowledged notification.
- severity: **low** · category: `correctness` · angle: `invariants` · flagged by 2 finder(s)
> Two distinct message ids whose String.hashCode() collide (or a message id hashing to another account's "summary:<id>".hashCode(), or to summaryId+1 via the +1 dodge) cause manager.notify to reuse an existing notification id: the earlier unread-mail notification is overwritten by an unrelated message's content, so the user never sees one of the new-mail alerts — violating the stated invariant that "a later batch never overwrites an earlier, still-unacknowledged one".
### `app/src/main/kotlin/org/libremail/push/IdlePushManager.kt:18` — IdlePushManager.start() (and stop()) swallow startForegroundService/stopService failures via bare runCatching with no AppLog call, violating CLAUDE.md's Definition-of-done rule that error/fallback paths and lifecycle transitions must be logged so behaviour is diagnosable from a debug report.
- severity: **low** · category: `conventions` · angle: `conventions`
> start() is invoked while the process is backgrounded; startForegroundService throws ForegroundServiceStartNotAllowedException and is silently swallowed (the comment at lines 15-17 documents the swallow but nothing is logged). The user reports 'instant push never works'; the DebugReport's RingLogBuffer contains no trace that a push start was attempted and rejected, so the failure is undiagnosable — the exact class of gap CLAUDE.md's 'appropriate logging ... error/fallback paths ... diagnosable from a user's debug report' rule forbids.
### `app/src/main/kotlin/org/libremail/push/IdleService.kt:294` — The POST_NOTIFICATIONS grant check is copy-pasted three times: IdleService.hasNotificationPermission() (line 294), MailNotifier.hasPermission() (MailNotifier.kt line 134), and inline in OnboardingWelcomeScreen.kt line 117 — all the identical ContextCompat.checkSelfPermission(context, POST_NOTIFICATIONS) == PERMISSION_GRANTED expression, with no shared helper.
- severity: **low** · category: `reuse` · angle: `reuse`
> If the gate ever needs refinement — e.g. also consulting NotificationManagerCompat.areNotificationsEnabled() (a user can disable notifications app-wide without revoking the runtime permission, in which case notify() posts nothing) — the fix must be found and applied in three files; updating only MailNotifier leaves IdleService's degraded-mode status notification path (degradeToPeriodicSync) using the stale rule. A single top-level hasPostNotificationsPermission(context) helper next to MailNotifier removes the divergence risk.
### `app/src/main/kotlin/org/libremail/push/IdleService.kt:329` — The IDLE reconnect loop implements its own private backoff knob (INITIAL_BACKOFF_MS/MAX_BACKOFF_MS) and neither classifies failures via ThrottleClassifier nor records/honors the shared per-account AccountThrottleGate that sync, backfill, and Graph all feed (#360).
- severity: **low** · category: `altitude` · angle: `altitude`
> When a provider throttles or connection-locks an account at LOGIN (e.g. Gmail 'too many simultaneous connections' or the rate clamp documented in the #125 perf drilldown), MailSyncer/MailBackfiller back the account off exponentially through the gate — but the IDLE watcher keeps re-authenticating every <=5 min and fires an on-connect sync each time, re-tripping the very limit the gate exists to respect. The #360 design goal ('paths never fight the same provider limit from two directions') is defeated by this third, ungated direction, and any per-provider connection-lockout fix (#360-#364 family) must be re-applied separately inside this loop.
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 (12)
app/src/main/kotlin/org/libremail/mail/GraphSender.kt:53— The Graph sendMail size guard is per-attachment only, so multiple attachments that together exceed the ~4 MB request cap are fully read into memory, base64-encoded, and transmitted in a request guaranteed to be rejected.correctness· angle:logic· flagged by 5 finder(s)app/src/main/kotlin/org/libremail/mail/ImapClient.kt:741— evictIdleReusedConnections/closeReusedConnections are driven only by IdleService (sweep loop in its scope; closeAll only on the low-battery PushMode change), so whenever IdleService is not running — push disabled in settings, or the dataSync FGS-cap degrade path that calls stopSelf without closeReusedConnections — warm authenticated IMAP sockets from the reuse cache are never evicted for the life of the process.resource-leak· angle:crossfileapp/src/main/kotlin/org/libremail/mail/ImapConnectionCache.kt:107— The transparent stale-socket retry re-runs the caller's whole block, making the copy-then-expunge move non-idempotent: a drop after the server executed COPY but before the response was read causes the retried block to COPY again, duplicating the message in the destination folderconcurrency· angle:invariants· flagged by 3 finder(s)app/src/main/kotlin/org/libremail/mail/ImapConnectionCache.kt:179— closeAll() has a TOCTOU race with concurrent withStore(): an operation can reconnect an entry after closeAll closed it (or insert a new one mid-iteration), and the trailing entries.clear() then orphans a live authenticated socket that no sweep can ever reachresource-leak· angle:pitfalls· flagged by 2 finder(s)app/src/main/kotlin/org/libremail/mail/graph/GraphBatch.kt:66— GraphBatch.execute (and GraphUploadSession.upload's session-create POST at GraphUploadSession.kt:49-58) send top-level Graph requests with only a Content-Type header and expose no parameter to supply the bearer token, so every $batch and createUploadSession call is guaranteed a 401.correctness· angle:invariantsapp/src/main/kotlin/org/libremail/mail/graph/GraphThrottle.kt:38— Graph's 4-concurrent-requests-per-mailbox cap is implemented as one process-global Semaphore shared by all accounts, not keyed per account like the rest of the throttle policy in the same class.altitude· angle:altitudeapp/src/main/kotlin/org/libremail/notifications/MailNotifier.kt:35— MailNotifier — the component that posts all new-mail notifications — has no test at all (no JVM unit test, no instrumented test), unlike every other file in the mail/push/notifications area.conventions· angle:conventions· flagged by 2 finder(s)app/src/main/kotlin/org/libremail/push/IdleService.kt:192— IdleService's watcher-reconciliation and reconnect logic (reconcileWatchers/watchAccount) — which encodes the fixes for shipped bugs #90 and #403 — has no test; only the extracted IdleForegroundStarter start/degrade seam and the notification text are covered.test-gap· angle:testsapp/src/main/kotlin/org/libremail/push/IdleService.kt:205— reconcileWatchers keys watchers only by account.id and never restarts one when the account row changes, so a running IDLE watcher keeps the stale Account snapshot (host/port/security/username) forever.correctness· angle:invariantsapp/src/main/kotlin/org/libremail/push/IdleService.kt:214— onPushModeChanged() calls startAsForeground() from an unguarded background coroutine, so a push-mode flip landing after the dataSync FGS runtime-cap timeout throws ForegroundServiceStartNotAllowedException in a SupervisorJob child with no handler and crashes the process — the exact #354 crash class the IdleForegroundStarter seam guards only on the onStartCommand path.concurrency· angle:pitfallsapp/src/main/kotlin/org/libremail/push/IdleService.kt:312— IDLE renewal tears down the whole TLS connection and triggers a full account sync every 9 minutes per account even when no mail arrived, instead of re-issuing IDLE on the live connection.efficiency· angle:efficiencyapp/src/main/kotlin/org/libremail/push/IdleService.kt:338— The reuse cache's idle eviction is driven solely by IdleService's sweep loop, and onDestroy only cancels the scope without closing reused connections — so whenever push is off, warm IMAP sockets are never evicted despite the cache's documented 5-minute idle timeoutresource-leak· angle:concurrencyLow (13)
app/src/main/kotlin/org/libremail/mail/HtmlToText.kt:20— BLOCK_BREAK handles tr/table but not td/th, so adjacent table cells concatenate with no separator in the plain-text conversion.correctness· angle:logicapp/src/main/kotlin/org/libremail/mail/IcloudSendLimits.kt:51— Outgoing message-size policy is a provider-specific object with its own inline provider check and dedicated SendWorker call site — and the SMTP-send guard keys provider identity off the IMAP host — instead of a maxOutgoingBytes capability on MailProvider checked once in the send path.altitude· angle:altitudeapp/src/main/kotlin/org/libremail/mail/ImapClient.kt:173— ImapClient repeats the open-folder session boilerplate ('store.getFolder(folder); mailbox.open(mode); try { … } finally { runCatching { mailbox.close(false) } }') verbatim in 10 operations, the identical 3-item header FetchProfile (ENVELOPE+FLAGS+UID) 3 times (fetchRecent/fetchOlderThan/search), and the '(mailbox as UIDFolder).getMessageByUID(uid.toLong()) ?: error("Message $uid not found")' lookup 4 times, instead of a single withOpenFolder(store, name, mode) { … } helper plus a headerFetchProfile()/requireMessageByUid() pair.reuse· angle:reuseapp/src/main/kotlin/org/libremail/mail/ImapClient.kt:544— ImapClient.idle() re-implements the existing openConnectedStore() helper inline: lines 543-545 duplicate its protocol selection ('imaps' vs 'imap'), Session.getInstance(buildProps(...)).getStore(protocol), and store.connect(host, port, username, secret) — the only difference is not passing the reuse flag to buildProps.reuse· angle:reuseapp/src/main/kotlin/org/libremail/mail/ImapClient.kt:711— The ImapPerf breadcrumb machinery is implemented twice: ImapClient.withStore's no-reuse branch (its own liveConnectionCount AtomicInteger, NANOS_PER_MS constant, and the '"$op connect=Xms work=Yms live=Z"' format string) duplicates ImapConnectionCache.runReusing/ensureConnected/liveCount()/elapsedMs (ImapConnectionCache.kt lines 98-125, 190-192, 225), which emit the same PERF_TAG="ImapPerf" line with independent code.reuse· angle:reuseapp/src/main/kotlin/org/libremail/mail/SmtpSender.kt:146— The 15-second network timeout policy is independently copy-pasted in three transports: SmtpSender.TIMEOUT_MS="15000", ImapClient.TIMEOUT_MS="15000" (ImapClient.kt:774), and GraphHttpClient.TIMEOUT_MS=15_000 (GraphHttp.kt:114).altitude· angle:altitudeapp/src/main/kotlin/org/libremail/mail/graph/GraphBatch.kt:47— GraphBatch and GraphUploadSession (and, transitively, GraphThrottle.recordSubResponseThrottle) are dead production code: no caller outside their own unit/instrumented tests exists anywhere in app/src/main — the only non-test reference is a GraphSender comment explaining why the upload-session path is deliberately NOT used (the send-only token lacks Mail.ReadWrite).dead-code· angle:reuseapp/src/main/kotlin/org/libremail/mail/graph/GraphBatch.kt:62— GraphBatch.execute awaits each 20-op $batch chunk strictly sequentially even though GraphThrottle permits 4 concurrent Graph requests, multiplying multiplexed-read latency by the chunk count.efficiency· angle:efficiencyapp/src/main/kotlin/org/libremail/mail/graph/GraphUploadSession.kt:43— upload() requires the entire content as one in-memory ByteArray and additionally allocates a copyOfRange per chunk, although the class exists precisely for large (>4 MB, up to ~150 MB) payloads.efficiency· angle:efficiencyapp/src/main/kotlin/org/libremail/notifications/MailNotifier.kt:130— Per-message notification ids are messageId.hashCode(), guarded against colliding only with the same account's summary id — a 32-bit hash collision between two message ids, or with another account's summary id, silently replaces a still-unacknowledged notification.correctness· angle:invariants· flagged by 2 finder(s)app/src/main/kotlin/org/libremail/push/IdlePushManager.kt:18— IdlePushManager.start() (and stop()) swallow startForegroundService/stopService failures via bare runCatching with no AppLog call, violating CLAUDE.md's Definition-of-done rule that error/fallback paths and lifecycle transitions must be logged so behaviour is diagnosable from a debug report.conventions· angle:conventionsapp/src/main/kotlin/org/libremail/push/IdleService.kt:294— The POST_NOTIFICATIONS grant check is copy-pasted three times: IdleService.hasNotificationPermission() (line 294), MailNotifier.hasPermission() (MailNotifier.kt line 134), and inline in OnboardingWelcomeScreen.kt line 117 — all the identical ContextCompat.checkSelfPermission(context, POST_NOTIFICATIONS) == PERMISSION_GRANTED expression, with no shared helper.reuse· angle:reuseapp/src/main/kotlin/org/libremail/push/IdleService.kt:329— The IDLE reconnect loop implements its own private backoff knob (INITIAL_BACKOFF_MS/MAX_BACKOFF_MS) and neither classifies failures via ThrottleClassifier nor records/honors the shared per-account AccountThrottleGate that sync, backfill, and Graph all feed (#360).altitude· angle:altitude