1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
|
# 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.
## Build & test verification — mandatory
**Never conclude a build or test result from grepped console output**, and never
verify a launch with `adb shell monkey`. Use the shared wrappers, which report
from exit codes, parsed JUnit XML, and a logcat capture started before launch:
```
/workspace/devtools/bin/gradle-test /workspace/feedreader -- --rerun-tasks
/workspace/devtools/bin/android-launch-check org.terst.feedreader \
--apk app/build/outputs/apk/debug/app-debug.apk --settle 15
```
These exist because each of the following happened in this repo and was reported
as fact: Gradle running against a different directory after a silent `cd`
failure, `monkey` reporting a healthy app as crashed and a crashed app as
healthy, and `adb install` refusing a filename into `/dev/null` so the previous
build stayed installed and got tested instead.
`android-launch-check` is also the fastest way to check the schema-downgrade
crash described in the published-APK notes — it prints the fatal stack directly.
See `/workspace/devtools/README.md`.
|
