diff --git a/CLAUDE.md b/CLAUDE.md index 7b93c3c..8d7ba93 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -162,6 +162,80 @@ analysis. See [`README.md`](README.md#security-scan-mix-mobsecurity_scan) for the full layer list and the one-time `brew install` of external scanners. +### Decision log — check both directions + +Before committing, ask two questions, not one. + +**Does this need a new record?** Anything non-obvious: a tradeoff, a workaround, +a convention, a "why X and not Y". The test is whether a reader six months from +now would ask why it is like this. If the commit message is explaining a +decision, that decision belongs in `decisions/` where it is findable, not only +in `git log`. Record it in the same commit, not as a follow-up. + +**Does this INVALIDATE an existing record?** This is the half that gets missed, +and it is the more dangerous one. A record asserting a property the code no +longer has is worse than no record: it is a claim a maintainer will act on. +Grep `decisions/` for the mechanism you are changing before you commit. + +Both failed in one session, on the same change: + +* A decision record claimed "the frame-registry generation is untouched because + the parked slot stops re-registering once it stops laying out." It reasoned + about the outgoing direction only. The returning direction was broken — + silently, for exactly the screens the change optimised for — and the record + said it was fine. +* Source comments elsewhere stated invariants the same change inverted: + `MobLazyList`'s latch reasoned that "only navigation changes the container's + identity", which had just stopped being true. + +When you correct a record, correct it **in place** with a note saying what was +wrong, rather than quietly deleting the claim. The wrong version is the part a +future reader needs to recognise, and `decisions/` is append-only for +superseding whole decisions, not for silently editing away a mistake inside one. + +### Adversarial review — before the commit, by a subagent + +**Non-trivial work gets an adversarial review before it is committed.** Spawn a +subagent, point it at the actual diff, and tell it to find defects rather than +to approve. Act on what it finds, then commit. + +It must be a **separate agent**, not a re-read of your own work. The thing that +is wrong is usually the author's mental model of the change, and that model is +exactly what a self-review carries into the second pass. + +Give the reviewer: the diff to read (`git diff ..HEAD`, and the base +explicitly, since a diverged local branch will otherwise sweep in the whole +tree), what the change claims to do, and the specific things you are least sure +about. Tell it to cite `file:line` for every finding, to rank them +blocking / should-fix / nitpick, and to separate what it verified in source from +what it is reasoning about platform semantics. Ask it to say plainly if the +change is sound rather than inventing problems — but only after it has looked +hard. + +**Skip it for** mechanical or trivial changes: formatting, a typo, a version +bump, a changelog edit, moving a file. Reach for it when the change has +behaviour, touches native code, or spans a platform boundary. + +This is not ceremony. In one session, pre-commit reviews caught the following +in `mob` — the examples are from there because that is where the session ran, +and this repo builds and deploys exactly that native code — each of which would +otherwise have shipped: + +* a helper defined inside `#if !MOB_RELEASE` but called unconditionally from + Swift, which linked in debug and would have failed **every iOS release + build**; +* a cache whose tests asserted the write path and nothing about the read, so + deleting the lookup, or reading under a constant key, passed the whole suite; +* a fix that covered 3 of 7 call sites on one platform while claiming parity + with the other; +* a comment and a decision record asserting a race was closed when the code + only narrowed it; +* generated source telling every user that a feature does nothing, in the + release that made it work. + +The one substantial change that skipped review that session was the largest one +in the batch. Do not let size be the reason to skip. + ## Release flow Canonical process lives in