Commit Graph
560 Commits
Author SHA1 Message Date
Jason Ross def89c4e2a Merge main into fix-reporting-pii-mainthread 2026-07-04 03:21:30 -05:00
Jason Ross 5125a6674c Merge pull request #320 from JMR-dev/fix-310-311-312-dao
perf/fix(data): batch sync updates, paging tiebreaker, migration-registration test
2026-07-04 03:20:59 -05:00
Jason Ross 42b8bcc91e Merge main into fix-310-311-312-dao 2026-07-04 03:01:22 -05:00
Jason Ross 61dc35e290 Merge main into fix-reporting-pii-mainthread 2026-07-04 03:01:17 -05:00
Jason Ross cbf284b5f5 Merge pull request #315 from JMR-dev/fix-account-lifecycle-integrity
fix(data): account-lifecycle data integrity (non-destructive upsert, id normalization, deleteAccount cleanup)
2026-07-04 03:00:44 -05:00
Jason Ross c645e0e1fe Merge main into fix-reporting-pii-mainthread 2026-07-04 02:45:04 -05:00
Jason Ross 61437e8670 Merge main into fix-account-lifecycle-integrity 2026-07-04 02:45:01 -05:00
Jason Ross 2b616df3e0 Merge main into fix-310-311-312-dao 2026-07-04 02:44:58 -05:00
Jason Ross 3062a3a3d9 Merge pull request #318 from JMR-dev/fix-295-targeted-batch-expunge
fix(mail): targeted + batch expunge (stop deleting unrelated \Deleted mail)
2026-07-04 02:44:21 -05:00
Jason Ross c98eac6c5b Merge main into fix-310-311-312-dao 2026-07-04 02:32:20 -05:00
JMR-devandClaude Opus 4.8 edbb1eb839 perf/fix(data): batch sync updates, paging tiebreaker, migration-registration test
#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>
2026-07-04 02:30:50 -05:00
Jason Ross 6810cc5390 Merge main into fix-account-lifecycle-integrity 2026-07-04 02:26:51 -05:00
Jason Ross 5e2d65d91a Merge main into fix-reporting-pii-mainthread 2026-07-04 02:26:50 -05:00
Jason Ross 6de885d53e Merge main into fix-295-targeted-batch-expunge 2026-07-04 02:26:48 -05:00
Jason Ross b95ae2e7eb Merge pull request #314 from JMR-dev/fix-302-idleservice-fgs-timeout
fix(push): handle Service.onTimeout to survive the dataSync FGS runtime cap
2026-07-04 02:26:16 -05:00
JMR-devandClaude Opus 4.8 f72c66d291 fix(mail): targeted + batch expunge (stop deleting unrelated \Deleted mail)
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>
2026-07-04 02:24:40 -05:00
Jason Ross 64f4c64936 Merge main into fix-reporting-pii-mainthread 2026-07-04 02:19:16 -05:00
JMR-devandClaude Opus 4.8 005f8a3aca fix(reporting): scrub PII from crash stack traces + move ReportStore scan off the main thread
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 #294
Closes #296

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-04 02:18:10 -05:00
Jason Ross 659f6e1d74 Merge main into fix-account-lifecycle-integrity 2026-07-04 02:14:30 -05:00
JMR-devandClaude Opus 4.8 cae11233f3 fix(data): account-lifecycle data integrity (non-destructive upsert, id normalization, deleteAccount cleanup)
#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 #309
Closes #305
Closes #299

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-04 02:13:21 -05:00
Jason Ross ce9e54f704 Merge main into fix-302-idleservice-fgs-timeout 2026-07-04 02:10:04 -05:00
JMR-devandClaude Opus 4.8 8904351a96 fix(push): handle Service.onTimeout to survive the dataSync FGS runtime cap
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>
2026-07-04 02:09:01 -05:00
Jason Ross b64c7f8dda Merge pull request #293 from JMR-dev/test-289-branch-coverage
test(coverage): high-value branch-coverage additions
2026-07-04 01:56:15 -05:00
Jason Ross 0e42358d27 Merge main into test-289-branch-coverage 2026-07-04 01:38:09 -05:00
JMR-devandClaude Opus 4.8 b165f72e3a test(coverage): high-value branch-coverage additions
Add focused JVM unit tests that close real (non-coroutine) branch gaps in
pure logic already >=95% line-covered (issue #289):

- richtext/RichTextEditing: removeLink, applyLink/styleAt/isStyled edges,
  quote/ordered marker detection+removal, remap* null branches.
- richtext/RichTextHtmlParser: new suite driving parseCssColor/parseFontSizePt/
  parseInlineStyles/parseBaseStyle/parseTextAlign/extractHref/unescape and the
  parser's malformed/stray/unclosed-tag edges.
- ui/compose/ComposeViewModel + ui/mailbox/MailboxViewModel: nav-arg blanks,
  signature-swap rebuild, autosave content detection, refresh/selection edges.
- data/repository/MailRepositoryImpl: non-selectable role folder, cancelOutboxMessage.
- data/repository/AccountRepositoryImpl: reorderAccounts.
- mail/HtmlToText, data/SignatureBlock, reporting/AppLog, reporting/DiagnosticsCollector.

Test-only; no production changes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-04 01:36:31 -05:00
Jason Ross 7966efde2f Merge pull request #291 from JMR-dev/test-288-coverage-gaps
test(coverage): close clean JVM-testable coverage gaps
2026-07-04 01:21:57 -05:00
Jason Ross 1af6ad3d52 Merge main into test-288-coverage-gaps 2026-07-04 01:05:26 -05:00
JMR-devandClaude Opus 4.8 a6c24f3cea test(coverage): close clean JVM-testable coverage gaps
Close the cleanly JVM-testable coverage gaps from the Phase-2 JaCoCo
audit (#288), bringing each targeted class to 100% line coverage:

- AccountSettingsRepository: observe Flow + the sibling setters
  (signature/notifications enabled, retention count/months incl. clamp).
- SignatureRepository: observeForAccount/get/getDefault/update + toDomain.
- CredentialStore (new): save/load/delete with a mocked KeystoreCrypto.
- EncryptedCacheGuard (new): the isCacheLocked() truth table.
- AttachmentUriGrants: releaseUnreferenced/referencedUris wiring.
- SyncScheduler: schedulePeriodicReportPurge.
- AppLockViewModel: nonce, onBackground cover branch, unwrap cancellation
  rethrow, awaitSyncEnqueue execution/interrupt branches.
- SmtpSender: send error paths (SSL/STARTTLS) + cc + inline+regular body.
- StartupReportViewModel: the @Inject real-clock constructor.

All test-only. Unit tests + jacocoTestReport + ktlintCheck + detekt green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-04 01:04:24 -05:00
Jason Ross a8399228f6 Merge pull request #287 from JMR-dev/test-infra-284-helper-cwd
test-infra: make local_instrumented.sh CWD-independent
2026-07-04 00:58:57 -05:00
Jason Ross 6ff43c1035 Merge main into test-infra-284-helper-cwd 2026-07-04 00:38:28 -05:00
Jason Ross fdb45131de Merge pull request #286 from JMR-dev/chore-237-localclipboard-migration
chore(ui): migrate LocalClipboardManager to LocalClipboard
2026-07-04 00:37:57 -05:00
JMR-devandClaude Opus 4.8 0b67cb952a fix(test-infra): make local_instrumented.sh CWD-independent
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>
2026-07-04 00:22:24 -05:00
Jason Ross b78921cd0b Merge main into chore-237-localclipboard-migration 2026-07-04 00:17:04 -05:00
Jason Ross 15f53633fa Merge pull request #283 from JMR-dev/test-257-reportupload-seams-e2e
test(reporting): ReportUpload testability seams + push IdleService E2E
2026-07-04 00:16:33 -05:00
JMR-devandClaude Opus 4.8 191e802eac chore(ui): migrate LocalClipboardManager to LocalClipboard
LocalClipboardManager/ClipboardManager are deprecated in Compose in favor
of LocalClipboard's suspend Clipboard API. Migrates the one call site,
ReportReviewScreen's "Copy report" action: LocalClipboardManager.current
becomes LocalClipboard.current, and the synchronous
clipboard.setText(AnnotatedString(...)) becomes a suspend
clipboard.setClipEntry(ClipEntry(ClipData.newPlainText(...))) run inside
the existing rememberCoroutineScope(). The clipboard interaction is
pulled into a small internal suspend function, copyReportPayloadToClipboard,
so it's unit-testable against a mocked Clipboard without an emulator.

Closes #237.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-04 00:16:32 -05:00
Jason Ross eda5103da2 Merge main into test-257-reportupload-seams-e2e 2026-07-03 23:58:56 -05:00
JMR-devandClaude Opus 4.8 0b44276909 test(reporting): ReportUpload testability seams + push IdleService E2E
Coverage lane 4 (#249) flagged reporting/push classes as unreachable by tests.
Add minimal, behaviour-preserving seams and the tests they unblock (issue #257):

- ReportUploadScheduler: inject Provider<WorkManager> (mirroring SyncScheduler)
  instead of calling the WorkManager.getInstance static that MockK can't stub on
  the abstract WorkManager (AbstractMethodError). New ReportUploadSchedulerTest
  pins the per-report unique-work name + REPLACE policy.
- ReportUploadWorker: take the ingest endpoint via a new @DebugReportEndpoint
  qualifier (provided from BuildConfig.DEBUG_REPORT_ENDPOINT in ReportingModule)
  rather than reading the BuildConfig static inline. New ReportUploadWorkerHttpTest
  drives the transmit path against an in-process JDK HttpServer on loopback and
  covers 2xx success + delete, 4xx failure, 5xx retry/attempt-cap, and network
  error. Production value is unchanged (empty by default).
- IdleService: extract the foreground-notification channel + push-mode-to-text
  logic into PushStatusNotification. New PushStatusNotificationInstrumentedTest
  asserts channel importance and the IDLE/POLLING notification text with a real
  application Context (never a mocked Context).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-03 23:58:00 -05:00
Jason Ross e587ba0d82 Merge pull request #282 from JMR-dev/test-221-cold-open-encrypted-cache
test(db): instrumented cold-open of a pre-encrypted cache
2026-07-03 23:47:44 -05:00
Jason Ross 24b9572179 Merge main into test-221-cold-open-encrypted-cache 2026-07-03 23:31:47 -05:00
JMR-devandClaude Opus 4.8 17d7135f59 test(db): instrumented cold-open of a pre-encrypted cache
Guards the SQLCipher cold-start crash fixed in 592a797 (bug #210): a cold
process opening an already-encrypted cache with nothing to convert reached
Room's keyed nativeOpen with the native .so unloaded and crash-looped with
UnsatisfiedLinkError. Every existing on-device test (DatabaseEncryptionTest,
DatabaseProvisionerInstrumentedTest, DatabaseModuleInstrumentedTest,
AccountDataMigratorTest) runs a conversion first, which loads the process-global
library in-process, masking the bug exactly as production did.

System.loadLibrary is process-global, so the instrumentation process can no
longer observe a cold open once it has minted the encrypted fixture. This adds
ColdOpenCacheProbe -- a debug-only ContentProvider declared with
android:process=":coldopen" -- to host the open in a separate, pristine app
process. The test mints the encrypted fixture in the instrumentation process
(a file created by a prior encrypted DB instance) and drives the cold open in
the :coldopen process via ContentResolver.call, mirroring DatabaseProvisioner's
encrypted branch + DatabaseModule's open lambda against the real DatabaseEncryption,
DeferredOpenHelperFactory and SupportOpenHelperFactory. A cold probe (a keyed open
with no preceding load, asserted to throw UnsatisfiedLinkError) makes the isolation
self-verifying: the test fails rather than passing hollow if the library was
already loaded in the harness process.

Verified locally on an API 36 emulator (connectedDebugAndroidTest): 1 test,
0 failures.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-03 23:30:49 -05:00
Jason Ross 165699d950 Merge pull request #280 from JMR-dev/test-infra-269-local-instrumented-helper
test-infra: reliable local instrumented-test helper (connectedDebugAndroidTest + emulator hygiene)
2026-07-03 23:29:06 -05:00
Jason Ross fc78d81886 Merge main into test-infra-269-local-instrumented-helper 2026-07-03 23:10:33 -05:00
Jason Ross 6151fea4d1 Merge pull request #279 from JMR-dev/test-275-lane5-followup-screens
test(coverage): lane 5 follow-up — UI tests for remaining screens
2026-07-03 23:10:03 -05:00
Jason Ross 633b3a94e2 Merge main into test-infra-269-local-instrumented-helper 2026-07-03 22:56:08 -05:00
JMR-devandClaude Opus 4.8 e006600424 test-infra: reliable local instrumented-test helper (connectedDebugAndroidTest + emulator hygiene)
Local Gradle Managed Device tasks (apiXXDebugAndroidTest) fail on this machine:
GMD's AVD snapshot step times out under AEHD 2.2
(AvdSnapshotHandler$EmulatorSnapshotCannotCreatedException), though the emulator
itself boots fine. CI is unaffected (it uses connectedDebugAndroidTest, not GMD).

Add .claude/skills/preflight/local_instrumented.sh, which cold-boots ONE emulator
by hand (-no-snapshot, no GMD) and runs :app:connectedDebugAndroidTest filtered to
a targeted set of test classes -- the same technique CI and api37_e2e.py already use.

The helper is deliberately targeted (the full ~114-test suite tends to wedge mid-run
on this box) and enforces emulator hygiene: it force-kills stray qemu/emulator
processes before booting, tears the emulator down afterward, and exits non-zero if an
orphaned qemu-system-x86_64-headless.exe survives -- accumulated orphans have frozen
this machine. Ships with a documented header and a short sibling README.

Closes #269

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-03 22:54:37 -05:00
Jason Ross 6bde35507b Merge main into test-275-lane5-followup-screens 2026-07-03 22:53:16 -05:00
Jason Ross e57ba70fb1 Merge pull request #278 from JMR-dev/test-220-native-lib-before-keyed-open
test(db): pin native-lib-before-keyed-open at the DatabaseModule factory site
2026-07-03 22:52:46 -05:00
Jason Ross 46c4110b19 Merge main into test-275-lane5-followup-screens 2026-07-03 22:39:38 -05:00
JMR-devandClaude Opus 4.8 65ff45ec31 test(coverage): lane 5 follow-up — UI tests for remaining screens
Adds instrumented Compose UI tests for the screens #250/#274 left uncovered,
so lane 6's ui-package coverage ratchet (#251, >=95%) can pass:

- ColorSwatchRow (compose/format): none entry + swatch rendering, selection
  callbacks, and selected-state semantics.
- LockScreen: locked title/body, optional error text, unlock callback.
- AddAnotherAccountScreen: confirmation + both onboarding choices.
- SignatureEditScreen: real ViewModel over an in-memory Room-backed
  SignatureRepository — new-vs-edit title, create/update round-trips.
- ReportReviewScreen: real ViewModel over a file-backed ReportStore (submitter
  stubbed disabled) — disclaimer/fields render, Submit gated on comment length
  + email validity, discard deletes and leaves.
- AppPasswordSetupScreen: real ViewModel over FakeAccountRepository — provider
  chrome + credential add, and the app-password help link asserted via
  Espresso-Intents (mirrors AccountPickerScreenTest) so no real browser opens.

All 23 tests pass locally on an API 36 emulator.

Closes #275

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-03 22:38:47 -05:00
Jason Ross e4a27944bd Merge main into test-220-native-lib-before-keyed-open 2026-07-03 22:37:29 -05:00