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>
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>
Port the last bash dev-script (local_instrumented.sh) to a cross-platform,
stdlib-only Python 3 script (local_instrumented.py), matching api37_e2e.py's
style, and rewire the /preflight skill's local E2E off the GMD
apiXXDebugAndroidTest tasks (which fail locally under AEHD 2.2) onto it.
- local_instrumented.py preserves the .sh's behavior exactly: comma-separated
test-class CLI arg, pre-boot orphan-kill, manual cold-boot of the dev36 AVD
(no GMD, no snapshot), targeted connectedDebugAndroidTest, and the EXIT-trap
teardown (now try/finally + atexit + SIGINT/SIGTERM handlers, idempotent).
Exit codes 0/2/3/4 preserved.
- Cross-platform process kill abstracted per-OS: taskkill /F /IM on Windows,
pkill -f qemu-system on *nix; process listing via tasklist / ps ax.
- Teardown hardened vs the .sh: it now also reaps the emulator *launcher*
image, not just qemu -- the Windows -no-window emulator spawns a sibling
emulator.exe that briefly outlives the qemu VM, which a qemu-only sweep left
as an orphan on return (caught by the smoke run).
- SKILL.md + CLAUDE.md: replace the local api35/api36 GMD E2E steps with
local_instrumented.py; CI's own multi-API matrix is untouched. CLAUDE.md
documents the Python-first dev-script convention.
Validated: py_compile, argparse (--help / no-arg exit 2), and a guarded
emulator smoke run of org.libremail.data.local.DatabaseEncryptionTest -- boots,
passes, and tears down clean (no qemu/emulator orphan on return).
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>
A crash report captured throwable.stackTraceToString() verbatim, so mail/network
exceptions (Jakarta Mail, java.net) could embed server host:port tokens and account
emails/usernames in the report's stackTrace field — violating the PII-free-reports
constraint. Add StackTraceScrubber, applied in DiagnosticsCollector before the trace
enters toSubmissionPayload()/toStorageJson(): it keeps the non-PII value (exception
class names + every frame's class/method/file/line) and drops each header line's
free-text message (where hostnames/usernames live), then redacts any residual email
or host:port left on a wrapped continuation line. Frame lines are untouched, so a
frame's File.kt:42 is never mistaken for a host:port.
ReportStore did MutableStateFlow(scan()) in its constructor — a dir list + read +
JSON-parse of every stored report. As an eager @Singleton dep of CrashReporter, whose
install() runs on the MAIN thread in Application.onCreate(), this was main-thread disk
I/O that grows with the 30-day retention. Seed the flow empty and dispatch the initial
scan to an injectable scope (Dispatchers.IO by default); reactive consumers update when
it lands, and writes still re-scan synchronously so a crash-time save is never lost.
Tests: StackTraceScrubberTest (host/ip/port/email dropped from a ConnectException +
auth-failure trace while classes/frames survive; regex redaction of a continuation
line; null-message trace preserved verbatim); DiagnosticsCollector end-to-end scrub
test; ReportStore empty-seed + off-thread populate via a StandardTestDispatcher. Store
constructions in existing tests use an Unconfined scope to keep their synchronous
reopen semantics.
Closes#294Closes#296
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#309: AccountDao no longer uses @Insert(REPLACE). New insertIfAbsent (IGNORE)
+ @Update back a non-destructive upsert, and insertAtEnd updates an existing id
in place (preserving its sortOrder) instead of REPLACE. Re-adding an existing
account id (e.g. re-authing an Outlook account, whose id is the deterministic
outlook:<email>) therefore no longer cascade-deletes its account_settings +
signatures.
#305: normalizeEmailForAccountId always lowercases the domain (mail domains are
case-insensitive), and the whole address for the consumer providers (Gmail,
Yahoo, iCloud, AOL, Outlook). Applied at every id-derivation site
(MailProvider.createAccount, Account.outlook, ManualSetupViewModel) so
differently-cased addresses can't spawn duplicate accounts. The displayed email
keeps the user's casing.
#299: deleteAccount collects the account's message ids and draft attachment URIs
while the rows still exist, deletes the rows (now including the account's
drafts), then deleteRecursively()'s each message's on-disk attachment cache dir
and releases the drafts' now-unreferenced persistable URI grants.
Adds unit tests for id normalization + deleteAccount cleanup and DAO-level
instrumented tests for the non-destructive create/update.
Closes#309Closes#305Closes#299
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
IdleService runs continuously as a FOREGROUND_SERVICE_TYPE_DATA_SYNC
foreground service (push is on by default). With targetSdk 37, Android 14+'s
dataSync FGS runtime cap (~6h per rolling 24h) calls Service.onTimeout(...)
and then force-stops the service — throwing a system FGS-timeout exception —
if it doesn't stop itself. IdleService overrode onStartCommand/onDestroy/onBind
but not onTimeout, so after ~6 cumulative hours push silently died and the app
hit the exception; on API 35+ the budget is cumulative and a restart can't
recover it until the next 24h window.
Override both onTimeout(startId) (deprecated, API 34) and
onTimeout(startId, fgsType) (API 35+); both route to a clean shutdown that
re-asserts the already-scheduled 15-minute periodic sync, swaps the persistent
notification to a degraded "paused" text and DETACHes it so it survives, then
stopForeground(DETACH) + stopSelf so we never leave a dataSync FGS running past
its cap (the exact condition the platform kills on). This mirrors the existing
low-battery PushMode.POLLING fallback.
The push-status text choice is pulled into a pure PushStatusNotification.statusTextRes
seam and unit-tested on the JVM; the built notification's new timed-out text is
covered by PushStatusNotificationInstrumentedTest.
Closes#302
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The wrapper jar path was resolved from the script's own location, but
gradlew picks the *project* to build from the process's current
directory, not from its own script location. Invoking the helper from
a CWD outside its tree (e.g. another worktree) silently built the
wrong repo's :app, once observed as a ClassNotFoundException for a
test class that only existed in the intended worktree.
cd to the already-resolved repo/worktree root before invoking gradlew
so connectedDebugAndroidTest always targets the correct tree
regardless of the caller's CWD. Update the README's usage note to
match.
Closes#284
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
JMR-devandClaude Opus 4.8