# FeeDreader — Agent Instructions

Personal Android RSS/Atom reader, `org.terst.feedreader`. Built for Peter's own use, greenfield,
hands-on-testing-rounds workflow (sideload a debug build, use it, batch feedback). See `BACKLOG.md`
for open work.

## Test-driven development is mandatory

Every change to production code (`app/src/main/`) follows red-green-refactor, in that literal order:

1. **Write the test first**, for the specific behavior you're about to add or change. Not a test
   for the whole feature at the end — the test that would fail without the change you're about to
   make.
2. **Run it. Confirm it fails, and fails for the right reason** — an assertion mismatch that
   demonstrates the missing behavior, not a compile error, not a typo, not a test that was
   accidentally already passing.
3. Only now write the minimal production code to make it pass.
4. Run again. Confirm green.
5. Refactor if needed, keeping it green.
6. Run the full quality gate (below) before committing.

**If it's discovered that production code was written before, or without, a preceding failing
test** — mid-session, in review, by the pre-commit hook, however it surfaces — **the fix is to
discard the whole diff and redo it properly**, test first. Do not backfill tests onto code that
already exists to make the rule look satisfied after the fact: a test written against working code
tends to just confirm what the code already does, not drive the design or catch what the code
missed. `git diff` to see what's uncommitted, stash or note anything worth keeping conceptually,
then `git checkout -- <files>` (or `git reset` for staged-but-uncommitted work) and start over from
a failing test. This applies to every agent working in this repo, not just whoever wrote the
original rule.

**Scoping exception:** pure resource/config/manifest changes with no meaningful unit-testable
behavior (an XML theme parent, a manifest permission, a Gradle version bump) don't get a red step —
there's nothing to fail. These are instead covered by the static-analysis gate (lint/detekt) acting
as the regression check. If you're unsure whether something falls in this bucket, default to
writing the test.

## Quality gate — every commit

Enforced by `.githooks/pre-commit` (see below; run `git config core.hooksPath .githooks` once per
clone to activate it — this is a manual one-time step per checkout, not automatic, since hooks
aren't cloned by default):

1. **Build** — `./gradlew compileDebugKotlin` must succeed.
2. **Tests** — `./gradlew testDebugUnitTest` must pass in full.
3. **Static analysis / code-quality audit** — `./gradlew detekt lintDebug` must both come back
   clean: zero lint errors (warnings are fine to accumulate but should trend down, not up — see
   "periodic checks"), zero new detekt findings beyond the tracked baseline.
4. **Coverage must not decrease** — Jacoco line coverage over `app/src/main/java` excluding
   `ui/**` (Compose screens are impractical to unit-test for coverage and would make the metric
   meaningless — see `app/build.gradle.kts` Jacoco exclusions) is compared against
   `.coverage-baseline`. A commit that drops it fails.
   - **Justifiable exceptions exist** — e.g. deleting dead code that happened to be covered,
     where the deletion is the point (percentage can dip when the denominator shrinks even though
     real risk went down). These aren't silently allowed: set
     `COVERAGE_EXCEPTION="reason for the drop"` when committing, which the hook logs and lets
     through instead of blocking. Don't reach for this to avoid writing a test you were supposed to
     write — it's for cases where the drop itself is the correct outcome.
   - A commit that *raises* coverage automatically updates `.coverage-baseline` as part of the
     hook.
5. **WebView JavaScript justification** — any new or changed `javaScriptEnabled = true` /
   `setJavaScriptEnabled(true)` site must carry a `@SuppressLint("SetJavaScriptEnabled")` plus an
   inline comment explaining why (matches the existing pattern in `ReaderExtractor.kt`,
   `LinkReaderScreen.kt`, `BrowserScreen.kt`). This app's whole security posture around untrusted
   article content rests on JS being off almost everywhere — a new JS-enabled surface is exactly
   the kind of thing that should never land silently.
6. **Manifest permission diff** — if `AndroidManifest.xml`'s `<uses-permission>` list changed, the
   hook prints a warning requiring the commit message to say why. Doesn't block (permissions
   sometimes need to land before the feature using them), but makes it impossible to miss in review.
7. **Secret scan** — a lightweight grep for API-key-shaped strings and common credential patterns
   across the staged diff. Blocks on a hit; override only if you're certain it's a false positive
   (e.g. a test fixture with an obviously-fake key).
8. **Schema-migration test requirement** — if `AppDatabase.kt`'s `version = N` changed, the diff
   must also touch a migration test file (Room's `MigrationTestHelper`, run against a real
   pre-migration DB). No mechanism currently exists for this (see Task: Room migration tests below)
   — until it does, this check is a documented rule, not yet hook-enforced.

Never bypass this hook with `--no-verify` without asking first, same as the general git-safety
rules — if a check is wrong or too strict for a specific case, fix the check or use the documented
exception mechanism, don't route around it silently.

## Periodic checks (not per-commit — too heavy, or need a human/agent read, not a mechanical gate)

- **Dependency freshness** — `./gradlew lintDebug` already reports outdated dependency versions via
  its `GradleDependency` check (no extra plugin needed — the `com.github.ben-manes.versions` /
  `io.github.ben-manes.versions` plugin was tried during setup and reproducibly broke
  `kspDebugKotlin` in this environment, so it's deliberately not used here). Batch-upgrade
  periodically from those findings rather than drip-updating one dependency per unrelated commit.
- **Dead-code / reachability sweep** — the kind of audit that found the orphaned `/tabs/meals`-style
  routes on doot and the dead DAO methods here: re-run after a significant feature batch, not every
  commit. Trace every nav route, DAO method, and setting to a real caller.
- **Room migration correctness** — beyond the per-commit "a test exists" check above, periodically
  verify the full migration chain (v1→latest) runs clean against a realistic seeded DB, not just the
  latest single-step migration in isolation.
- **Detekt/lint baseline review** — baselines exist so new code doesn't inherit old debt as a blocker,
  but a stale baseline just hides debt forever. Periodically review what's in it and pay some down.

## Why these specific checks

Every check above traces back to something that actually happened in this codebase, not a generic
checklist:
- The lint zero-errors gate exists because `themes.xml`'s theme parent silently required API 29
  against a minSdk of 26 — a real crash risk on real devices that sat unnoticed until an audit found
  it.
- The WebView-JS-justification check exists because this app's core security property (JS off for
  untrusted article content) depends entirely on that staying true at every callsite, and one
  existing site (`ReaderWebView.kt`) already didn't carry the same justification pattern as the
  other three.
- The dead-code sweep exists because three DAO methods (`ArticleDao.delete`,
  `FilterRuleDao.getAll`, `ArticleDao.markAllReadForFeed`) were superseded by newer methods and left
  behind, undetected until an audit.
- The migration-test requirement exists because `AppDatabase` is on its 11th schema version with
  zero migration tests, several of which are one-off hardcoded data repairs — exactly the kind of
  change most likely to corrupt a real, non-reproducible production DB if wrong.