Add JVM-only unit tests (74 across 4 new, purely-additive files) covering
the data/repository, data-mapper, and pure domain packages. No production
code is changed.
- AccountRepositoryImplTest: first tests for AccountRepositoryImpl — add/
test/delete/observe + reset-backfill, success and rejected-LIST failure
paths (class now 100% instruction & line).
- MailRepositoryImplCoverageTest: the MailRepositoryImpl methods/edges the
existing suite skipped — observe-* flows, getMessage/getDraft, setStarred,
deleteMessage, sendMessage + copyAttachments (incl. unreadable-URI skip),
searchServer (all-accounts vs. filtered), and the account/row-gone
fall-throughs.
- MappersTest: entity<->domain mappers not otherwise pinned, incl. the
unknown-enum fallbacks and FetchedMessage id/uid rules.
- DomainModelCoverageTest: AccountSettings.signatureBlock branches,
Signature.plainText, default-arg constructors, and display-name fallbacks
(domain/model now 100% instruction & line).
Closes#246
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
In age-based retention, MailSyncer fetched the newest-N headers but only capped that window by the
retention COUNT, not the age cutoff. On a low-traffic mailbox whose newest-N span older than the
cutoff, each sync re-inserted messages the age pruner had just deleted, and the next prune deleted
them again — a churn loop of wasted DB writes + prune deletes (issue #193).
Sync now drops fetched messages older than policy.ageCutoffMillis before persisting (the same cutoff
the pruner uses), so sync and prune keep exactly the same set in both retention modes. Count/unlimited
modes have a null cutoff and are unchanged. The empty-folder wipe is keyed on the raw fetch (server
truth), so a folder holding only past-cutoff mail is left to the pruner rather than wiped.
MailPruner's KDoc now documents the sync alignment for both modes.
Closes#193
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Make the latest-API-level emulator E2E (api36DebugAndroidTest, the
highest level in the E2E matrix and its Gradle Managed Device task) an
actually-run, required step:
- CLAUDE.md: preflight now runs api36DebugAndroidTest, and a change is
not done until that E2E runs and passes locally (not merely compiles).
The full multi-API matrix and the API 37 preview job stay CI's job.
- preflight skill: add the api36 E2E as the final step, note the
emulator/managed-device precondition, and replace the old
"don't run E2E locally" guidance so the two files agree.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
State plainly that a change isn't complete without passing unit tests
and E2E/instrumented tests covering it, with no softening about
running the emulator matrix locally being optional.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codify that a task/PR isn't complete without both passing unit tests
and E2E/instrumented tests covering the change. Writing and committing
the E2E/instrumented test is required; only running it against a
booted emulator locally stays optional, since CI's E2E matrix covers
that.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Completes the crash-interrupted #192 WIP (app/build.gradle.kts already had a
jacocoTestReport task and toolVersion pin recovered onto build-192-jacoco):
- Move the JaCoCo tool version into gradle/libs.versions.toml instead of a
hardcoded string in app/build.gradle.kts, matching how every other plugin
version in this repo is sourced.
- Fix the generated-code exclusion list against the real compileDebugKotlin
output (verified by inspecting the compiled class tree): Room's
KSP-generated `_Impl` DAOs/database and the Compose compiler's per-file
ComposableSingletons holders are the only generated code that actually
lands in classDirectories, since Hilt/Dagger's generated Java and AGP's
BuildConfig/R/Manifest are compiled by a separate javac task this report
never reads. Drop the blanket `**/*$$*` exclude the WIP had — it was
silently discarding ~200 real classes' worth of coverage on Kotlin's own
`$$inlined$` synthetic classes (e.g. Flow.map { ... } transforms in the
repositories), which is hand-written logic, not generated boilerplate.
- Add Hilt_*/Dagger* prefix patterns so the (currently inert,
belt-and-suspenders) Hilt exclusions are actually correct if the
classDirectories scope ever changes.
- Add a minimal CI step to the existing unit-tests job that runs
jacocoTestReport and uploads the XML+HTML report as a build artifact.
No coverage threshold gate yet (a jacocoTestCoverageVerification rule is
a natural follow-up once there's a baseline).
- Document the new :app:jacocoTestReport task in CLAUDE.md.
Verified on JDK 21: fast gate (assembleDebug, testDebugUnitTest,
compileDebugAndroidTestKotlin, lintDebug, ktlintCheck, detekt) plus
jacocoTestReport all pass, from both a warm and a `clean` build. The
report shows real signal (30% instruction / 38% line coverage) with no
generated classes leaking in.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Clears the "hiltViewModel is deprecated; moved to package
androidx.hilt.lifecycle.viewmodel.compose" compile warnings across the 16 ui/**
files. Pure import swap: the old androidx.hilt.navigation.compose.hiltViewModel
inline-delegates to the new symbol, which is already transitively on the compile
classpath via the pinned hilt-navigation-compose:1.3.0 -- no dependency change,
identical signatures, behaviour byte-for-byte identical.
Closes#236
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Locks in the "pre-auth DB entry point defers while the encrypted cache is locked"
invariant that had zero coverage (which is how the PruneWorker/BackfillWorker gap
in #224 slipped in). Adds SyncWorkerTest and SendWorkerTest (locked -> retry with
the Lazy DB deps never resolved; unlocked -> runs), and a DatabaseProvisionerTest
case proving prepareCache() suspends on an auth-bound resolvePassphrase until it
resolves.
IdleService shares the same guard but is an Android Service (its start path needs
startForeground/Context), so its gate is covered by the instrumented test (#226)
rather than a JVM unit test.
Closes#225
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds ndk.abiFilters = ["arm64-v8a", "armeabi-v7a"] to the release build
type only, so the release APK/AAB ship the two ARM ABIs (64-bit + 32-bit)
instead of a universal build. x86 and x86_64 are intentionally dropped.
defaultConfig and the debug type are left untouched: CI's E2E matrix runs
the debug build on x86_64 emulators and needs the x86_64 native libs
(incl. libsqlcipher.so).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PruneWorker and BackfillWorker were the only two pre-auth background DB entry
points that opened the database without first checking
EncryptedCacheGuard.isCacheLocked(). With encryptCache + appLock both on and a
headless cold start where the user hasn't authenticated (WorkManager after
reboot, or the periodic backfill/prune window while locked), the first DAO call
runs prepareCache() -> resolvePassphrase() -> session.await(), parking the
worker thread until unlock and serializing other DB openers behind the held
prepareCache mutex. Self-heals on unlock, but wastes wakelock/battery and makes
zero progress while locked.
Switch both workers' MailPruner/MailBackfiller injection to dagger.Lazy and add
the isCacheLocked() guard before resolving it, mirroring SyncWorker/SendWorker.
Add JVM regression tests (locked -> retry with the Lazy dep never resolved;
unlocked -> runs; failure -> retry).
Closes#224
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Issue #124 paged only the unified "All inboxes" browse list. The
per-account folder view and every search path still loaded the whole
folder / entire unified inbox into memory and re-materialized it on each
cache write. Extend Paging 3 to them, mirroring the unified pager.
- MessageDao: add Room PagingSources for the per-account browse list
(`pagingFolderSummaries`, `inInbox = 1`) and for paged search
(`pagingUnifiedFolderSearchSummaries` / `pagingFolderSearchSummaries`).
The search queries LIKE-match the same columns the old in-memory
`matchesSearch` filter scanned (sender, sender address, subject,
snippet) and leave `inInbox` unfiltered so transient server-search hits
still surface. Read-only @Query methods — no schema change, no migration.
- MailRepository: add `pagedFolderMessages` and the two paged-search
flows via a shared `mailboxPager` whose PagingConfig adds
`maxSize = MAILBOX_PAGE_SIZE * 5` so a long scroll drops far-offscreen
pages. A `likePattern` helper escapes the LIKE metacharacters (\ % _)
so a query containing them still matches literally. The unified pager
(`pagedUnifiedFolderMessages`) is left untouched for its sibling PR.
- MailboxViewModel: collapse the old `messages` list flow and the
unified-only paged flow into one `pagedMessages` that dispatches each
(account, folder, query) to the matching pager; `cachedIn` is kept so
"select all" reads the loaded snapshot. Removes the now-dead
observe*FolderMessages / matchesSearch paths.
- MailboxScreen: render every mode from the single paged list. Gate the
empty state on `itemCount == 0 && refresh is NotLoading &&
append.endOfPaginationReached` so it no longer flashes on an
empty→loaded transition, preserving the search "no results", the
syncing-folder spinner (#149), and browse "no messages" states.
Behaviour is preserved: search filtering columns/scope, multi-select and
"select all", unread counts, and the browse-vs-search state machine
(server search + debounce + open/close) are unchanged — only the local
list materialization moved from Kotlin into paged SQL.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The matrix E2E (29) job intermittently fails (~2%, API-29 only) in
reactivecircus/android-emulator-runner's un-guarded, fatal post-boot
`adb shell input keyevent 82`: on snapshot resume sys.boot_completed=1 is
restored before system_server republishes the `input` binder service, so
the job aborts before Gradle runs with "No service published for: input"
(fast-fail ~1m43s). Proven on run 28667366203.
Make the "Run E2E tests" step non-fatal (id + continue-on-error) and add a
guarded second attempt (if steps.e2e.outcome == 'failure'). Two independent
boots drop the race to ~0.04%; a genuine failure on both attempts still
fails the job (outcome, not conclusion). Definitive manual-boot fix: #218.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two fixes from the Paging 3 perf audit of the mailbox list (#212, #213):
- Bound the unified-inbox paging window (#212): the only PagingConfig left
maxSize at Int.MAX_VALUE, so pages were never evicted and a deep scroll
accumulated the whole inbox in memory. Set maxSize = MAILBOX_PAGE_SIZE * 5
(200), satisfying maxSize >= pageSize + 2*prefetchDistance (120). Safe with
enablePlaceholders = false (the UI null-guards evicted positions).
- Memoize the per-row relative timestamp (#213): MessageRow formatted it via
DateUtils.getRelativeTimeSpanString un-remembered, allocating a String on
every recomposition. Wrapped in remember(timestampMillis).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
assembleDebug, testDebugUnitTest, and lintDebug never compile the
androidTest source set, so a change that breaks it (e.g. an
instrumented test calling a UI API that just changed signature) passed
preflight locally yet only failed once CI ran. Add
:app:compileDebugAndroidTestKotlin to the fast gate to catch that
class of breakage before pushing, and update CLAUDE.md's summary of
the gate to match.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The opt-in encrypted cache crash-looped on launch (UnsatisfiedLinkError:
No implementation found for SQLiteConnection.nativeOpen) on any cold start
after encryption was enabled — reported after an app upgrade.
System.loadLibrary("sqlcipher") was only invoked as a side effect of an
actual plaintext<->encrypted conversion (DatabaseEncryption.migrate) or the
one-time #111 account migration. On a steady-state start the cache is
already encrypted and the account migration is already done, so both no-op
and nothing loads the native library before Room opens the keyed database
via SupportOpenHelperFactory -> nativeOpen. The previous process survived
only because an earlier conversion had loaded the .so in-memory; the next
cold start (e.g. an upgrade) crashes.
Load the library explicitly in DatabaseProvisioner whenever it commits to an
encrypted open (idempotent; no-ops when already loaded). Add regression
assertions: the encrypted path must load it, the plaintext path must not.
Verified on a Pixel 10 Pro XL — an in-place update preserving the already-
encrypted cache now launches to the mailbox instead of crash-looping.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SmtpSender.inlinePart built the MIME header as `<${attachment.contentId}>`
and GraphSender put contentId straight into the Graph JSON — neither
stripped CR/LF or other ISO control characters. Not exploitable today
(contentId is always an app-generated `img-<uuid>@libremail`), but if an
external value ever reached contentId the SMTP path would be a MIME
header-injection vector.
New shared sanitizeContentId() strips ISO control chars (incl. CR/LF),
applied at both sinks: the SMTP Content-ID header and the Graph JSON
field. No behavior change for the app-generated ids in use today.
Tests: SmtpSenderTest sends an inline image whose contentId contains
`\r\nX-Injected: evil` and asserts (via GreenMail) no injected header
appears on any MIME part and the Content-ID stays a single line;
GraphSenderTest asserts the control chars are stripped from the payload.
Closes#204
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
RichTextHtml.inlineCss and baseCss interpolated the font-family CSS value into
the emitted style attribute raw. The whole attribute is escaped (escapeAttr), so
a value can't break out of style="…" or inject a tag, and it isn't reachable
today (the picker only offers the fixed FontRegistry stacks; reply/forward
flattens sender HTML to plaintext first) — but a non-registry value would let
`;`/`:` inject a sibling CSS declaration inside the attribute.
Constrain the emitted font-family to a safe charset (the characters a real font
stack uses — letters, digits, spaces, commas, quotes, hyphens, periods,
underscores), dropping anything else instead of emitting it raw. All seven
bundled FontRegistry stacks are within this set, so the built-in fonts are
unaffected. RichTextHtml stays a pure module (no dependency on the UI-layer
FontRegistry), so the charset restriction is the layer-clean form of the
whitelist.
Test: a RichStyle.FontFamily("Arial; color:red") no longer appears raw in the
emitted HTML (inline and base-style), while a registry stack with quotes/commas
still emits.
Closes#205
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The compose attachment picker and inline-image picker call
takePersistableUriPermission on every pick, but nothing ever released
those grants (releasePersistableUriPermission was absent repo-wide). The
app accumulated indefinite read access to every file/photo ever attached
and could hit the per-app persisted-grant cap — after which the take
(swallowed by runCatching) silently fails and a later draft's image
won't reload. (Post-batch security review, Low.)
The picked bytes are copied into the app cache at enqueue
(copyAttachments), so a grant is only truly needed to reload an image
when a *draft* is reopened. New AttachmentUriGrants releases a URI's
grant once no remaining draft or outbox row references it; callers invoke
it after the referencing row is gone:
- MailRepositoryImpl.deleteDraft (a deleted draft can't reopen)
- MailRepositoryImpl.cancelOutboxMessage
- SendWorker after a send succeeds, and when a queued message is dropped
because its account was removed
A URI still referenced by another live draft/outbox row is kept; a
release of a grant not actually held throws and is swallowed.
Also hardens attachment filenames: sanitizeAttachmentName strips path
separators and ISO control chars (incl. CR/LF) from picked/received
display names before they become on-disk or MIME filenames, so a crafted
name can't traverse directories or inject header lines. Applied in the
compose picker (queryFileName) and outbox/incoming staging.
Tests: pure release-decision (unreferencedUris), the filename sanitizer,
and the repository wiring (deleteDraft/cancelOutboxMessage call the
releaser with the removed row's URIs, after deleting the row).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wire inline images through the whole send pipeline.
Compose UI: an image-picker (image/*, persistable URI grant, mirroring the
attachment picker) behind a new toolbar button appended at the END of the
toolbar — after the block/link buttons and the font/size/align controls — so it
never shifts the bullet button the compose E2E taps without scrolling. Picking
an image adds an inline OutgoingAttachment and hands the editor a
PendingInlineImage, which RichTextEditing.insertImage drops as a [image: name]
token + RichImage(contentId) at the caret. Deleting the token drops the image:
onBodyChange reconciles inline attachments against the body's surviving cid:
references. Inline images are tracked in ComposeUiState alongside regular
attachments but kept out of the attachment-chip row.
Domain/persistence: OutgoingAttachment gains contentId/isInline; the shared
draft/outbox attachment JSON carries them (drafts need no migration — an older
draft reads back as a plain attachment). The outbox stages files by index as
before but now also stores per-file {contentId,isInline} metadata in a new
OutboxEntity.attachments column (Room 17 -> 18, MIGRATION_17_18, DEFAULT '' per
the bccAddresses precedent so fresh-install == migrated; 18.json committed). The
send worker pairs each staged file with its metadata by index (with a positional
fallback for messages queued before the column existed).
SMTP (SmtpSender): inline images wrap the body in a multipart/related, each with
a Content-ID matching the HTML's cid: and inline disposition; regular
attachments keep today's multipart/mixed shape.
Graph (GraphSender): inline fileAttachments carry isInline + contentId.
Tests: GreenMail asserts multipart/related with a Content-ID matching the cid;
GraphSenderTest asserts the inline payload; a mapper test proves an inline
image's cid<->file pairing round-trips a draft save/reopen; RichTextEditing
tests cover insertImage (token/RichImage placement + offset shift + html
round-trip); MigrationTest gains migrate17To18. Reader-side cid: rendering stays
out of scope (follow-up).
Closes#77
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Jason Ross
JMR-devandClaude Opus 4.8