From b3f63c1f38930cd3d7e606ed8d2552ecd7cc6210 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Fri, 4 Sep 2026 09:38:05 -0600 Subject: [PATCH 1/3] Require an adversarial review by a subagent before committing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codifies the process gap this session exposed. Reviews run before commit caught, 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 passed the whole suite; a fix covering 3 of 7 call sites while claiming cross-platform parity; a comment and a decision record asserting a race was closed when the code only narrowed it; and generated source telling every user a feature does nothing, in the release that made it work. The one substantial change that skipped review was the largest in the batch, which is the argument for not letting size be the reason to skip. The reviewer must be a separate agent. What is usually wrong is the author's mental model of the change, and a self-review carries that model into the second pass. The rule also says what to hand the reviewer, including the explicit diff base — a diverged local branch otherwise sweeps the whole tree into the diff, which happened three separate times this session. Scoped: skipped for mechanical changes (formatting, typos, version bumps, changelog edits, file moves), reached for when a change has behaviour, touches native code, or spans a platform boundary. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 7b93c3c..94b1b5c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -162,6 +162,47 @@ 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. +### 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, 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 From 2a63ace80a99924756b244d1d0e6b484144532b7 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Fri, 4 Sep 2026 13:50:41 -0600 Subject: [PATCH 2/3] Add a decision-log step to the pre-commit checklist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The checklist covered tests, formatting and linting but never prompted the question "does this decision belong in decisions/". More importantly it never prompted the reverse question, which is the one that actually bit. Both halves 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 asserted it was fine. A record making a claim the code does not support is worse than no record, because a maintainer will act on it. - 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. So the step asks both: does this need a new record, and does it invalidate one that already exists. Grep decisions/ for the mechanism you are changing. Also says to correct a wrong record in place with a note rather than deleting the claim. The wrong version is the part a future reader needs in order to recognise the mistake; append-only applies to superseding whole decisions, not to silently editing away an error inside one. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 94b1b5c..382a1ad 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -162,6 +162,37 @@ 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 From 1712fa5c80e8b62dc6c927a589400eb32f8cb543 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Sat, 5 Sep 2026 19:44:51 -0600 Subject: [PATCH 3/3] Say where the review examples come from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The anecdotes are from mob: MobLazyList and the iOS release-build linking failure are not code in this repo, so a reader looking for them locally finds nothing. They stay because this repo builds and deploys that native code, and a release-only linking failure is precisely the kind of thing that surfaces here first — but the provenance should be stated rather than inferred. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 382a1ad..8d7ba93 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -216,8 +216,10 @@ hard. 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, each of which -would otherwise have shipped: +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