Add a detekt style>ForbiddenImport rule that forbids `import android.util.Log`
so all logging flows through org.libremail.reporting.AppLog, which mirrors each
line into the debug-report RingLogBuffer. A raw android.util.Log import writes to
Logcat only and never reaches a user-reviewed DebugReport (epic #324, strangler
final step).
Excludes the AppLog facade itself (the one sanctioned wrapper) and the unit tests
that mockkStatic(Log) to verify forwarding — AppLog forwards to Log, a throwing
stub under plain JVM unit tests, so those tests must mock it; they do not bypass
the facade.
Closes#331
Part of #324
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds a fast traffic-control-tests job (ubuntu, actions/checkout +
actions/setup-python, no emulator/Gradle) that runs the 37 pure-stdlib
unit tests for .github/scripts/traffic_control.py on every PR, and
wires it into ci-passed's needs so a regression blocks merge instead
of only being caught locally.
Closes#346
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Restore (and extend to drafts) the old bash's broken-reclaim behaviour that the
initial Python refactor had dropped. runs_to_cancel now cancels an OTHER PR's
active/queued runs when EITHER:
(a) THIS PR is P0 and that PR is strictly-lower (reclaim every lower runner); OR
(b) that PR is broken/draft (effective priority 10) and THIS PR is strictly-higher
(effective priority < 10) — a wasted run any ready PR may reclaim.
P1-P9 still never bump a *normal* (non-broken/draft) lower run; a broken/draft PR
(P10) preempts nothing (nothing is strictly-lower than the bottom, and the
equal-or-higher invariant means a P10 never cancels another P10). Self / main-push /
equal-or-higher invariants unchanged.
Updates the module docstring + ci.yml comments (the "only P0 preempts" wording
becomes: P0 preempts everything strictly-lower; additionally, any strictly-higher PR
preempts a broken/draft run) and the job step/permission/needs comments. Adds unit
tests: P3 reclaims a broken P10 run and a draft P10 run; P3 does not bump a normal P5
run; a P10 self preempts nothing; plus an end-to-end P5-reclaims-draft-then-waits
scenario. 37 unit tests pass; ci.yml parses clean; --dry-run shows a P3 cancelling a
draft (and broken) run while still yielding to a higher P1.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The priority-based runner orchestration ("traffic-control") lived as a large
inline-bash step in ci.yml — a two-pass preemption + hold-back script that was
effectively untestable in YAML. Move it into .github/scripts/traffic_control.py,
structured as a pure decision CORE + a thin gh-I/O SHELL:
* Pure functions (no network/clock/subprocess), unit-testable in isolation:
- effective_priority(pr): lowest-numbered P0-P9, default P5; broken OR draft => 10.
- runs_to_cancel(this_pr, all_prs, self_run_id): PASS 1 — run ids to cancel,
empty unless THIS PR is P0; only strictly-lower running/queued runs; never self
(by number or run id), never equal-or-higher.
- wait_blockers(this_pr, all_prs): PASS 2 — yield to any strictly-higher PR with
an active/queued run, and to same-level peers ordered ahead (running-first,
then oldest createdAt). Empty => proceed.
* Shell (run_live): gathers the snapshot via gh, applies cancels, runs the bounded
hold-back poll loop; always exits 0. --dry-run feeds the core a snapshot JSON and
prints decisions with zero network.
* No jq/bash dependency (cross-platform, per the repo's Python-stdlib convention).
ci.yml's traffic-control job now checks out the repo and runs the module. Job
permissions gain `contents: read` (for checkout) alongside the existing
`actions: write` / `pull-requests: read`; env and downstream `needs:` wiring
unchanged; step stays `continue-on-error`.
33 stdlib unittest cases cover priority resolution, P0-only preemption, the
self/main/equal-or-higher invariants, and the same-level running-first/oldest
ordering.
Behaviour is preserved except the ticket's refinements: (1) drafts now count as
P10 (bottom); (2) an explicit same-level running-first-then-oldest tiebreaker; and
(3) per the ticket's order-of-operations, ONLY P0 preempts — the old bash also let
any higher-priority PR cancel a `broken` target's run, which no longer happens
(a broken/draft run is only cancelled by a P0, via the same strictly-lower rule).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Explains the traffic-control job in ci.yml (priority labels, preemption
vs. bounded hold-back, safety invariants, and the known FIFO-runner
limitation) for developers new to the repo. Notes that #342 will refactor
this logic into a tested Python module, at which point this doc gets
updated.
Closes#343
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
The API 37 preview E2E boot has flaked twice (#285, #333) with only
"did not boot within 300s" and no root-cause signal. Add rich boot
diagnostics by default, kept in parity between CI and the local
hand-provisioning script (api37_e2e.py):
- Launch the emulator with `-verbose -debug init,avd_config,kernel`
(diagnostics only; no boot-affecting flag changed), still redirecting
to $EMU_LOG.
- Stream `adb logcat -v time` to a file from the moment the device
registers (via `adb wait-for-device logcat`, backgrounded).
- On a boot timeout, dump accel-check, /dev/kvm presence, GPU mode,
free mem/disk, the AVD config.ini and the emulator.log tail; CI writes
these to a boot-diagnostics file, the local script prints them.
- CI uploads emulator.log + logcat.txt + boot-diagnostics.txt as an
artifact with `if: always()` so they survive a timeout/cancel, and
prints a concise summary (accel/KVM status + last 50 lines of
emulator.log) to the step log.
The existing 2-attempt boot retry + boot-completed wait loop are
unchanged; the diagnostics are additive.
Closes#334
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>
Jason Ross
JMR-devandClaude Opus 4.8