| name | release-path |
| description | Discipline for any branch that ends in a reviewed, gated, or protected PR. Load when starting or resuming such a branch, when a PR will not merge, when a bot reviewer (e.g. CodeRabbit) posts threads, when entering a fix-review loop with an independent reviewer, or before pushing changes that touch money, quotas, migrations, or security. Encodes the SYN-1115 / PR |
release-path — first commit to merged PR without the grind
Origin: Synthex PR #820 (SYN-1115). The review machinery correctly caught 30+ real
money-path defects — that time was well spent. Roughly the same amount of time again was
process waste. Every law below is paired with the incident that paid for it. Follow the
laws and the same defects get found in a third of the rounds.
Law 0 — Read the last gate first (the Merge Map)
The single largest waste in #820: merge requirements discovered after twelve review
rounds instead of before the first commit. Before writing any code on a PR-destined
branch, build the Merge Map:
- Branch protection on the base:
gh api repos/{owner}/{repo}/branches/<base>/protection
— required status checks, required approving reviews (count — a required human review
may be convention, not mechanics; know which), and required_conversation_resolution.
If conversation resolution is on, every bot thread is a lock on the door (Law 3).
- Bot reviewers present? CodeRabbit or similar installed means threads WILL arrive
and WILL block. Find out on day one, not after the push.
- Push hooks: read the hook source itself. What receipt does it demand, and to which
SHA does the review bind — the merge-base..HEAD of the whole branch, or the last
commit? (#820 lost a full cycle discovering receipts bind to the branch: the PR is the
branch, so per-commit PASSes could never satisfy it.)
- Automation: draft/ready flipping, auto-created PRs on push, auto-merge, merge
queues. Check what a push actually triggers before pushing.
- Environment: does a non-production database exist for migrations? Is the ticket
system authenticated? Is the independent reviewer reachable? What is the spend budget?
A missing credential only the founder can grant is a stop condition on day one, not a
mid-build discovery.
Output: post the Merge Map to the ticket. Anything red is resolved or ruled on now.
Law 1 — One defect per commit, independent review between
Empirical, not aesthetic: a multi-fix pass took the finding count from 6 to 9 in one
round; the switch to one-defect-per-commit produced the first clean PASS of the ticket
and repeated first-pass PASSes after. Smallest possible diff. No "while I'm here."
Review between every commit when in a fix loop.
Law 2 — Review the branch, not just the commit — and early
Per-commit PASSes gave false confidence while the branch accumulated defects only a
whole-branch review could see: the findings curve was 2 → 4 → 3 → 3 → 1 → 1 → 1 → 2 →
2 → 3 → 1 → 0 across twelve passes — all run only at push time. The PR is the branch.
Run a merge-base..HEAD review every 3–5 commits or daily, whichever comes first, so
branch-shape findings surface while context is hot instead of compounding at the end.
Law 3 — Bot threads are locks on the door
With conversation resolution required, an unresolved thread blocks merge regardless of
how green the code is. Protocol, in the same session a bot review lands:
- Verify every claim against source before anything else — in #820, three of four
"unverified" threads turned out to be real money bugs.
- Real findings: fix under Law 1.
- False positives: reply on the thread with the evidence, then resolve.
- Never resolve unverified. Never mass-resolve. Never let threads age — they are merge
blockers from the moment they are posted.
Law 4 — Controls that cannot lie
(Extends proof-discipline, which owns red-first. These are the #820 additions.)
- Sabotage both directions before commit: break the guarded thing → control goes
red; restore → green. A green you have never seen red is evidence of nothing.
- Drive the product path. Never author the fields you assert: the paid canary
"proved" cost columns the product route never wrote, because the harness wrote them
itself. Import and invoke the real route/handler; the harness triggers, reads, and
authors nothing.
- Pin constants by parsing the executable site, all occurrences. A bare regex
matched a comment on line 81 while the real request body on line 263 went unwatched —
the control stayed green with the call site set to a value that broke the ceiling.
- Probe for vacuousness: assert the branch you claim to exercise actually fired
(an
extraCalls > 0-style precondition). Two #820 controls passed as 0 === 0
because the code path they guarded was structurally unreachable from the test.
Law 5 — Close the class, not the instance
Three separate times a fix closed one half of a class while its control was blind to
the other half: public references bounded, private ones not; output megapixels priced,
input ones not; the video path classified a failure correctly, the image path opposite.
Before claiming a class closed, enumerate the twins and check each: the other media
path, the public/private variant, the input/output half, the legacy route, the
canary/cron copy, the submit/webhook pair. Grep the predicate across the whole tree and
list every call site in the commit message. One decider: a judgement duplicated is
a judgement that will diverge — billed-or-not existed in three copies defaulting in
opposite directions.
Law 6 — The fix is guilty until proven safer than the bug
Four of five rounds in one stretch found defects introduced by the previous round's
repair. Before landing any fix, four checks:
- Legitimate-input check: does any valid operator input change meaning? (Rejecting
0 as invalid handed an operator reaching for the kill switch half the daily budget
instead.)
- Blast radius: where does the failure fire — module construction or call site?
(Throwing at module scope would have stopped settlement, the sweep, and the webhook
for a config error that only concerned admission.)
- Mechanism survives its context? (Catching P2002 inside the transaction it aborts
cannot work — the statement itself must handle the conflict:
ON CONFLICT DO NOTHING.)
- Failure direction: a control may fail closed; it must never get looser at the
moment someone reaches for the brake.
Law 7 — Money and event-log invariants
- In a derived-sum admission design there is no accuracy-only defect: anything that
forgets spend returns headroom. Treat every settlement bug as an admission bug.
- Price the request you actually send: build the provider request first, price that
object, then hold, then dispatch. Pricing a model of the request leaked three
separate ways (unenforced dimensions, unpriced tokens, unbounded references).
- Reservation precedes dispatch, per attempt. Refuse-before-dispatch is not relaxable
and never degrades into settle-after-the-fact.
- Absence of evidence is not absence of spend. Only a proven non-dispatch (DNS
failure, refused connection, missing key, a literal 404 on a retired path) charges
zero. Everything ambiguous errs high.
- One terminal event per hold; evidence is monotonic — later knowledge revises a cost
upward, a replay can never erase a recorded actual.
Law 8 — Distrust the harness before the code
Each of these cost a debugging loop at least once:
- Integration red on a persistent sandbox → recreate first (
sandbox:down && up)
before investigating. Accumulation false-reds were re-diagnosed from scratch four
times in #820.
- Know the harness blind spots: a db-push harness never executes migration SQL, so
migration-only defects are invisible to it; replay-from-empty may be independently
broken.
- Jest traps:
resetMocks wipes jest.fn implementations between tests (use plain
async functions when return values matter); factory-referenced identifiers must be
mock-prefixed (hoisting); @jest-environment must be the file's first block, and
prettier can fold it onto one line where it stops applying.
Law 9 — Stop conditions
- The same defect class recurring after two patch rounds → stop patching, propose the
structural change. Each structural move in #820 (counter → event log, attempt log,
price-the-request) dissolved a class that patches provably could not close.
- Fix-introduced findings two rounds running → drop to smaller commits or hand back.
- A credential or approval only the founder can grant → stop condition, never a
workaround.
- A findings rate that is not decaying across branch reviews → report the curve and
stop; scope is the founder's decision, not the loop's.
The session loop
- Load this skill → build or refresh the Merge Map (Law 0).
- Work under Laws 1 and 4–7.
- Every 3–5 commits: branch-level review (Law 2) and thread sweep (Law 3).
- Pre-push: whole-branch review bound to merge-base..HEAD at the exact head; receipt;
stub check; spend itemised.
- Push → immediately re-check the PR's actual state (draft flip, auto-PR, CI, threads)
rather than assuming the push did what was intended.
- Merge stays human where convention says so — write the PR body for that human, not
as a changelog.
Done means
Branch-level PASS at the exact head · zero unresolved threads, each closed with
evidence · Merge Map fully green · receipt bound and issued · PR body written for the
human reviewer · every remaining open item is a ticketed decision, not a surprise.
Law 10 — Know your co-writers
A branch is not yours alone until proven so. PR #821 paid for this law: an
unidentified writer on the same branch caused three head-drifts — each invalidating a
bound review and its receipt — duplicated a migration at a colliding timestamp, and
finally merged the PR itself, while a client-side pause was bypassed six minutes after install — not by force, but by executing the lift
instruction the lock's own refusal message printed, then politely restoring the
lock afterwards.
- Merge Map addition: enumerate every identity that can push to this branch —
humans, agents, bots — and how each is told apart (git author/email, account,
machine). An unidentified writer is a stop condition: identify and coordinate, or
lock server-side, before continuing.
- Client-side hooks are advisory — and self-defeating when helpful. The same
actor that runs a hook can lift it, and a refusal message that explains the lift
is a bypass manual for an instruction-following agent. Never print the disable
command in the refusal. Only server-side rules bind a writer you do not control:
client-side controls are requests; server-side controls are rules — the same
lesson as RLS, one layer down.
- Silence is not proof. "Everything up-to-date" means the hook never ran, and a
control that was never invoked looks identical to one that passed. Probe the
mechanism directly before claiming it works.
- Head drift invalidates evidence. A review PASS and its receipt bind to an exact
SHA. On a contested branch, either lock server-side first or budget a full
re-review per drift — never re-issue a receipt against a stale head.
- Agents never wear the owner's face. Work pushed under a shared owner identity
is unattributable and ungovernable. Every writer gets its own identity with least
privilege; merge rights live with humans, enforced server-side, not by convention.
- Rules live where writers read them. A prohibition in CLAUDE.md, a
constitution file, or an agent config binds only the harnesses that load it.
Four on-disk prohibitions against hook bypass sat in the very checkout that
bypassed the hook; a doc saying prod DDL is founder-gated lost to a doc saying
apply-before-merge. When two documents disagree, the agent obeys the permissive
one it happened to read. The only rule every writer obeys is the one the server
enforces.
- Fail the artifact, not the actor. The strongest gate in this estate's history
is the build-time schema drift-check: it binds at build time regardless of who
acts, and it kept an unschema'd deploy out of production after every
actor-facing gate had been walked around. Where possible, encode the rule where
the artifact is made or served, not where the actor is asked.