The sync engine (MailSyncer, MailBackfiller, MailPruner, and their WorkManager
workers) was completely silent, so a submitted debug report showed nothing
about whether sync ran, how much it fetched, or why it was skipped. Add
net-new AppLog breadcrumbs at each class's lifecycle points per the #324
strangler-migration plan: sync start/done/failed and per-folder fetch counts,
backfill slice start/done and per-folder page counts, prune's removed count,
and each worker's cache-locked deferral and success/retry outcome (the retry
path now also carries the scrubbed failure throwable via AppLog's #325
overloads).
Every breadcrumb is PII-safe by construction: accounts are identified only via
accountLogRef(account.id) (never the id or email directly), and a new
logSafeFolderLabel() helper logs a folder's name only when it matches a fixed
allowlist of known system folders (INBOX, Sent, Drafts, Trash, Spam/Junk,
Archive, and their common provider variants) — every other folder, however
nested or named, logs as a fixed placeholder.
Adding logging to these previously-silent classes meant every existing test
exercising them now hits android.util.Log (a throwing stub under plain JVM
unit tests), so each affected suite gains the same static Log mock already
established by AppLogTest/SendWorkerTest/ImapClientTest.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Migrate AppLockViewModel (9 sites) and AccountSetupViewModel (1 site) off
raw android.util.Log onto the AppLog seam (#325), so their diagnostic
lines land in the RingLogBuffer and reach a submitted DebugReport instead
of only Logcat. Adds three new breadcrumbs that were previously silent:
auth-seal unlock success, the clear-cache-and-restart recovery trigger
(with the disableAppLock flag), and every onForeground LockAction
decision. AccountSetupViewModel's success path also now logs "Outlook
account added" (no email). None of these call sites carry PII; where a
throwable is attached, AppLog's StackTraceScrubber redacts it before it
reaches the buffer.
Both ViewModel test suites now install a real RingLogBuffer and assert
against it instead of `verify { Log... }`, including dedicated no-PII
assertions (a known test email never appears in a recorded line). A
minimal `mockkStatic(Log::class)` stub stays in both test files' shared
setUp — AppLog still forwards to the real android.util.Log internally,
which throws "not mocked" in JVM unit tests when uninvoked; the not-yet
-landed guard-rule ticket (#331) will need to reconcile that with a
repo-wide "no raw Log outside AppLog.kt" rule.
Closes#326
Part of #324🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Migrate ImapClient/SendWorker/IdleService off raw android.util.Log to AppLog,
per the debug-logging strangler epic (#324), so their diagnostics reach the
RingLogBuffer (and a submitted DebugReport) instead of logcat-only:
- ImapClient: IDLE connect + IDLE push (message count) breadcrumbs.
- SendWorker: outbox-drain count on entry, per-message sent/failed result,
and the existing Graph->SMTP fallback warning.
- IdleService: IDLE watch start, cache-locked defer, and the existing
IDLE-dropped/retrying warning.
Also closes#297: SendWorker and IdleService logged the raw account.email via
Log.w on the Graph->SMTP fallback and IDLE-drop paths. Both now log
accountLogRef(account.id) instead -- a short, stable, non-reversible
per-account reference -- so the account's email never reaches Logcat or a
report.
Rewrites ImapClientTest/SendWorkerTest to install a real RingLogBuffer via
AppLog.install(...) and assert on its contents (migrated calls + new
breadcrumbs), instead of verifying a mocked Log; every assertion also checks
no line carries the test account's email, regression-covering #297. Adds a
SendWorkerTest case that drives a real SmtpSender against an in-process
GreenMail SMTP server end to end. android.util.Log is still stubbed (by
fully-qualified name, without importing it) where AppLog's Logcat passthrough
would otherwise crash the unmocked Android stub in a JVM test.
Closes#328Closes#297
Part of #324
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Migrates the DB/keystore area's raw android.util.Log calls to AppLog so
key-invalidation and DB-conversion breadcrumbs land in the process
RingLogBuffer (and thus a user-reviewed debug report) even in release
builds, where Log.d is otherwise stripped from Logcat only.
- DatabaseKeyCipher: 4 auth-bound-key decision points (encrypt retry,
isInvalidated's three branches) now log via AppLog.d(tag, msg, e).
- DatabaseEncryption.migrate: adds an AppLog.i "converting local cache
database (targetEncrypted=...)" breadcrumb at the start, alongside the
existing "converted" completion line now routed through AppLog.d.
- AccountDataMigrator: the "moved account tables into the account
database: $present" breadcrumb (table names only) now routed through
AppLog.d.
No PII or key material is logged; table-name sets and boolean flags only.
Adds instrumented tests (DatabaseKeyCipher is device-only and
behavior-preserving, so no new test there) asserting the breadcrumbs
land in a RingLogBuffer and never contain the seeded email, secret, or
passphrase.
Part of #324.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Migrate RestartActivity's one raw Log.w site to AppLog, clearing the
final raw android.util.Log site outside the auth/lock, DB/keystore,
connectivity/send, and sync-engine migration areas so the codebase is
ready for the detekt android.util.Log guard (#331).
RestartActivity runs in the separate :restart trampoline process,
where LibreMailApplication.onCreate returns early and never calls
AppLog.install, so this breadcrumb reaches Logcat only, never a
DebugReport. The migration is guard-compliance + Logcat-consistency
only; behavior is unchanged since AppLog forwards to Logcat.
RestartActivity is DEVICE-ONLY (multi-process kill/relaunch), so a
JVM buffer-capture test doesn't apply here. Added
RestartActivityLoggingTest, which instead pins the null-buffer shape
this call runs under in the trampoline process: it forwards to
Logcat and no-ops the buffer cleanly.
Closes#330
Part of #324🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add throwable-recording overloads to AppLog.d/w and make AppLog.e record the
throwable it is given: the throwable's stack trace is scrubbed via the existing
StackTraceScrubber (exception class names + frames kept; host/email-bearing
exception messages stripped) and appended to the buffered log line, so a
throwable can reach a user-reviewed DebugReport without leaking PII. The
existing no-throwable overloads are unchanged.
Add accountLogRef(accountId): a short, stable, non-reversible reference
(scheme prefix + truncated SHA-256 of the id) so downstream logging can
identify an account without logging the raw Account.id, which embeds the email.
Foundation for the #324 debug-logging strangler epic; consumed by #326–#330.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Scope :app:jacocoTestReport's denominator to the JVM-testable surface and
add a :app:jacocoTestCoverageVerification no-regression gate that shares the
same classDirectories/executionData/sourceDirectories, wired into both the
`check` lifecycle task and CI's unit-test job (part of the `CI passed` gate).
Excluded from the denominator (structurally unreachable from a JVM unit
test): Compose screen/component render code, Android framework entry points
(*Activity/*Service/Application/*BackupAgent), Hilt DI (**/di/**), and the
src/debug cold-open probe. Kept in scope: ViewModels, repositories, mappers,
DAOs, utils, richtext, mail, reporting logic, and the six WorkManager Workers.
Corrects PR #292, which excluded **/*Worker*: SyncWorker, BackfillWorker,
PruneWorker, SendWorker, ReportPurgeWorker and ReportUploadWorker are all
directly unit-tested, so they stay counted in both numerator and denominator
(only their Hilt wiring, WorkManagerModule, is excluded, via **/di/**).
Baseline: 80.21% line (4838/6032). Floor: 0.79 (~1.2% headroom) so ordinary
noise doesn't red-flag it while a real drop fails. Manual ratchet for now:
bump the floor up in the same PR when coverage rises materially.
Closes#251Closes#292
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#303: the reader's Reply now routes through MailRepository.buildReplyDraft
(quotes the original into a <blockquote>, bakes the signature, prefixes Re:/Fwd:
without double-prefixing) and opens compose on the built draft via
ReaderEvent.OpenCompose — the same high-fidelity path the mailbox uses — instead
of a bare compose prefill with an empty body. Adds Reply-All and Forward via an
app-bar overflow menu.
#304: SignatureEditViewModel.save, ReportReviewViewModel.submit,
AccountSettingsViewModel.removeAccount, and ProblemReportsViewModel.createManualReport
now flip a busy/saving flag synchronously before the first suspension and gate
their buttons, so a rapid double-tap can't create duplicate signatures/reports,
enqueue two uploads, or over-pop the back stack.
Closes#303Closes#304
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#310: add a single-transaction MessageDao.updateHeaderContents(List<MessageEntity>)
and route MailSyncer's per-message updateHeaderContent loop through it, so a folder's
recent-window refresh commits once instead of once per message (fsync/journal write
per message, amplified on the encrypted cache).
#311: append the `id` primary key as a tiebreaker to the four paged MessageDao
`ORDER BY timestampMillis DESC` queries for a total order, so rows sharing a second
(bulk mail) can't duplicate or skip across a LIMIT/OFFSET page boundary. Pure query-text
change: the exported Room schema (identityHash) is derived from table/index structure,
not @Query SQL, so no schema re-export or version bump; id is already in the projection,
so no new index.
#312: expose the registered migration list as DatabaseModule.ALL_MIGRATIONS (spread into
addMigrations) and assert in MigrationTest that it equals the reflectively-discovered set
of every Migration val, so a migration forgotten in addMigrations fails a test instead of
crash-looping all upgrading users at DB open (there is deliberately no destructive fallback).
Tests: new MessageDaoTest cases for the batch update and the id tiebreaker (all four
pagers), and the MigrationTest registration assertion; wired updateHeaderContents into
MailSyncConcurrencyTest's fake DAO. Instrumented MessageDaoTest + MigrationTest (31 tests)
green on a local emulator; unit tests + androidTest compile + ktlint + detekt green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ImapClient.deleteMessage and moveMessages flagged the target \Deleted then
called the untargeted Folder.expunge(), which permanently removes EVERY
\Deleted-flagged message in the folder — not just the intended UIDs. That is a
data-loss window whenever a second client, Gmail, or a partial earlier move has
left other messages flagged \Deleted. The repository's batch delete/expunge and
trash-fallback paths also looped single-UID deleteMessage, paying N logins + N
expunges for an N-message selection.
Add a batch deleteMessages(uids) that opens the folder once, flags the matched
messages \Deleted, and issues a single targeted UID EXPUNGE (RFC 4315) via
IMAPFolder.expunge(Message[]) through a shared expungeTargeted() helper. Route
moveMessages through the same helper, delegate single-UID deleteMessage to
deleteMessages, and route MailRepositoryImpl.expunge and the moveByRole trash
fallback through the batch method. moveToFolder already batches via moveMessages,
so it inherits the targeted expunge.
On a server without UIDPLUS, Angus raises "UID EXPUNGE not supported" rather than
silently falling back to the unrelated-mail-destroying untargeted expunge — a
loud failure is the safe outcome. Gmail, Outlook, and GreenMail all advertise
UIDPLUS.
Tests (GreenMail, no emulator): deleteMessages/moveMessages expunge only the
given UIDs and spare other \Deleted-flagged mail; a batch delete of three
messages opens exactly one connection and pays one LOGIN.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
exchangeToken's authorization-code exchange already requests
`openid email offline_access $OUTLOOK_SCOPE`, so the returned access token
is an outlook.office.com token usable for IMAP verification and the AuthState
already carries the refresh token and expiry. The immediate follow-up
refreshForScope(authState, OUTLOOK_SCOPE) was a second round-trip for the
same resource that only rotated the just-issued refresh token and added a
needless onboarding failure point (a transient network error there failed
sign-in after consent + code-exchange had already succeeded).
Build OAuthResult directly from the code-exchange tokenResponse
(accessToken + authState.jsonSerializeString()), dropping the extra refresh.
The durable AuthState is still serialized and persisted for later token
refresh; the Graph token remains a distinct resource minted on demand via
freshGraphToken.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Jason Ross
JMR-devandClaude Opus 4.8