This skill should be used when developing, debugging, refactoring, or building Genesis itself — tasks like "fix this in Genesis", "add a new MCP tool", "wire up the runtime", "Genesis won't start", "create a worktree", "debug the bridge", or "add a capability". Applies to any task modifying files under src/, .claude/, or tests/. Do NOT load for Genesis-as-tool work ("summarize this", "write a LinkedIn post", "research X") or general questions unrelated to Genesis internals.
Installs into .claude/skills of the current project.
Are you the author of Genesis Development?
Add the live security badge to your README. It updates with every re-scan.
[](https://www.skillsdirectory.com/skills/wingedguardian-genesis-development)
---
name: genesis-development
description: >
This skill should be used when developing, debugging, refactoring, or
building Genesis itself — tasks like "fix this in Genesis", "add a new
MCP tool", "wire up the runtime", "Genesis won't start", "create a
worktree", "debug the bridge", or "add a capability". Applies to any
task modifying files under src/, .claude/, or tests/. Do NOT load for
Genesis-as-tool work ("summarize this", "write a LinkedIn post",
"research X") or general questions unrelated to Genesis internals.
keywords: [genesis, worktree]
consumer: cc_foreground
phase: 10
skill_type: workflow
---
## Load Gate
Before reading any reference, confirm the task is Genesis-*development*,
not Genesis-*as-tool*. If uncertain, ask the user: "Are we modifying
Genesis itself, or using Genesis for something else?"
## On-Load Mindset
Internalize these immediately when this skill fires — they shape how to
work from the start, not just what to check before commit.
### Where your session ENDS — build sessions and closing sessions
**A session that writes code and a session that drives a PR to merge are
different SESSION TYPES, not two phases of one session's life** (user decision,
2026-09-02). Loading this skill makes you a **build session**.
- **A build session owns an item from Ready through opening its PR.** It runs
the review the change EARNS — `/deep-review` for a substantial one, because
that is what clears the commit gate's depth check; code-reviewer inline for a
small focused fix (the Adaptive Review Protocol below is authoritative, and
dispatching a full adversarial pass on a one-line change is not the bar) —
and then it is DONE with that item. Where its effect can be observed here, a
wrong run can be undone, and it is not on the never-run list, it is seen
working BEFORE the PR opens ("Verify BEFORE the PR" below). It does not wait
for Codex, and it
carries no review loop — the obligations under "When to DRIVE a Merge" below
pass to the closing session along with the PR; they do not lapse.
- **A closing session owns the open-PR queue**, whichever session built each
PR. Its unit of work is the queue, not the card. That is the
**`closing-session`** skill; load it instead when the job is "get the open PRs
merged".
The handoff between them is **the PR itself** — a durable artifact that survives
compaction and session death, so nothing has to be remembered across the
boundary. It also satisfies the `reviewer ≠ implementer, fresh context` contract
structurally rather than by discipline.
Why it matters here: fused, the first item's review loop consumes the whole
session — several compactions deep — while everything else the owner arrived
with goes untouched. **Finishing the PR is finishing the work.** Handing it to
the queue is not abandoning it.
Two consequences worth internalizing:
- **Throughput is a RATE, not a per-session virtue.** Closed/wk must exceed
opened/wk or the queue grows without bound (Little's Law). Build sessions do
NOT throttle themselves to protect it — that just relocates the queue upstream
onto the human deciding what not to start. Closing capacity is the control
variable. (`scripts/pr_flow_rate.py` measures the two rates; it lands with
PR #1613, so check that it exists before reaching for it.)
- **Compaction policy follows the same seam.** Reset context at
plan→implement. Implement→review is a session-TYPE boundary, not a compaction
decision — you hand off, you do not compact and continue. And never reset
mid-"fix the findings": that work needs the implementation context you would
be throwing away.
### PR readiness and mode
**Finish the work and clear blockers to entering review BEFORE opening a PR.**
Complete the implementation, required local review, and verification required
before publication. If unfinished work or an unresolved prerequisite prevents
normal review from proceeding, keep the change local, resolve the blocker, and
then open its PR. A draft PR is not a place to park incomplete or blocked work.
**A session opens every PR in regular, non-draft mode.** Never create a PR with
`--draft`. The exception is the runtime's own automated openers, which open
drafts by design: the executor's build lane (`autonomy/executor/pr_open.py`) and
the contribution opener (`contribution/pr_opener.py`).
**Moving an open PR to draft** is for a blocker to review that appeared AFTER it
opened (a prerequisite found missing, a design question raised in review); it
needs no explicit user request. It is never a way to hold work that was already
unfinished when the PR opened.
**A PR in draft returns to regular once the blocker that put it there is
resolved.** That holds for drafts opened before this rule. A draft from one of
the automated openers is different: it has no blocker to clear, neither opener
ever marks a PR ready, and the closing session counts and lists drafts without
closing them, so marking it ready is the owner's decision, which the closing
session's draft list is there to surface. Whoever resolves a blocker marks the
PR ready (`gh pr ready <N>`) and requests review (`@codex review`), because
marking a draft ready is not a step to rely on for triggering review. A draft
is never abandoned in that state.
CI and external review that run after opening, and verification explicitly
required after merge, remain their normal gates; waiting for those is not a
reason to use draft mode. Distinguish a blocker to entering review from a check
that review or merge will subsequently require. Open review findings, and a
dependency on another open PR named in the body, are of the second kind. A PR
sent BACK for rework or for an architecture decision is of the first kind,
because review stops until the rework lands (closing-session, "Sending a PR
back").
### Wiring Discipline
Every new component needs at least one call site in the actual runtime
path. Apply this 4-level verification taxonomy:
1. **Exists** — file/function present. Proves nothing.
2. **Substantive** — tests pass, handles happy + error. No runtime proof.
3. **Wired** — live call site, import chain unbroken. Minimum for "done."
4. **Data-Flow Verified** — real data flows end-to-end. Required for
critical paths.
Mark nothing "done" below Level 3.
### Measure, Do Not Choose
**If your reason for picking a command, a flag, a value or a procedure is a claim
about how something OUTSIDE this repo behaves — git, the shell, the harness, a
provider API — then "which of these is right?" is an experiment you have not run,
not a judgement you are entitled to make.** Run it first. Ship what the run says.
The tripwire is mechanical, so it cannot be reasoned around: you are about to write
an instruction someone will execute, and your justification is a sentence about
another program's semantics that you did not observe. Reading the manual feels like
evidence. It is not.
Three clauses, each bought by a defect:
1. **Enumerate the space; do not pick cases from it.** List the AXES that
independently change the behaviour, sweep the cross product, score every
candidate on every cell. The cases you would think of are the cases you already
believe in, which is exactly why they pass.
2. **Pre-register the predicate, the decision rule, and what you will do if
nothing passes.** That last clause is what stops the least-bad option being
rationalised into the right one.
3. **Control the instrument.** An ORACLE arm that must score 100% and a NO-OP arm
that must fail. If the oracle is not perfect, every number in the run is void —
including the flattering ones. Treat a surprising result as a suspicion about
the harness before it is a finding about the code.
MEASURED instance: a hook note telling a reader how to undo a destructive git
operation was wrong FOUR times, each version reasoned out and each refuted by a
state nobody had constructed. The fifth was swept rather than chosen — 320 states x
6 procedures with an oracle and a no-op. Of the 240 states where the reader's HEAD
had not moved, the winner scored 240/240; the best alternative managed 80, and the
procedure that had actually SHIPPED managed 56. The sweep also surfaced a boundary
no reasoning had: that same winner scores 0/80 once the reader has committed, where
a different command gets 72. Twenty minutes of compute against three review rounds
of guessing.
Full method, including how to split a space where the question stops being
well-posed: `references/high-stakes-verification.md` section 9.
**And where you cannot measure, READ THE AUTHORITATIVE SOURCE — never your own
recollection of it.** Measurement is the top of the ladder; this is the rung
below, and the one most often skipped because recall feels like knowledge. For
any claim about how something OUTSIDE this repo behaves — a flag's semantics, an
exit code's meaning, an API's contract, a shell builtin's edge case — the order
is MEASURE > the authoritative document (the vendor's own reference, the man
page, the spec) > everything else. Pretraining recall is not a source. It is a
hypothesis phrased confidently, and it is wrong most often exactly where a
flag's NAME supports the assumption everyone makes about it.
READ, from an external audit of a sibling toolkit (2026-09): its CI documented
`uv --frozen` as the check that fails when the lockfile has drifted from the
project file. It is not. Per the vendor's own reference — uv docs, "Locking and
syncing", consulted 2026-09-15 — `--frozen` uses the lockfile as the source of
truth *instead of* checking whether it is up to date, while `--locked` is the
flag that "requires that the lockfile is up-to-date" and errors when it is not.
(That citation is this passage obeying its own rule: an earlier draft asserted
the same thing from recall and cited nothing, which an adversarial review
caught.) The claim was plausible, the flag name invites the misreading, and
nothing in that pipeline could ever have contradicted it — the verification
everyone believed was running simply was not, and a green run said so every
time.
The habit that prevents it costs one lookup: when you write an instruction, a
CI step, or a code comment that names an external tool's flag, open that tool's
documentation in the same minute, and cite what you consulted so the next reader
re-checks rather than re-derives. "I am fairly sure that flag means X" is the
sentence to catch yourself in.
**A new SKILL has its own version of this, and it is easy to miss.** Dropping a
`SKILL.md` into `.claude/skills/` gets it INDEXED automatically (the catalog
generator scans the directory — no registry to update), which looks like done.
It is Level 1. The nudge that actually surfaces it scores **only** whole-word
skill-NAME tokens and explicit frontmatter `keywords:` — description prose is
deliberately not scored, and name words on the hook's `_NAME_TOKEN_STOPLIST`
("genesis", "user", "plan", …) never score either — so a skill whose name is a
concept nobody types is indexed and silent. MEASURED 2026-09-02: `closing-session` scored **0.0** on
every one of its own trigger phrases ("work the open PR queue", "review and fix
the open PRs") until `keywords:` was declared; with them, 4/4 trigger phrases
fire and 3/3 unrelated prompts stay silent. Note the extractor drops tokens
shorter than 3 characters and does no stemming, so `pr` can never match and
`merge` will not match "merging" — declare the surface forms. Verify a new
skill by scoring it against the phrasings a user would really type, in BOTH
directions.
### GROUNDWORK Code Is NOT Dead Code
Code tagged `# GROUNDWORK(feature-id): why` is intentional future
investment. Never delete or refactor it as dead code. Only remove when
the feature is fully active or the user explicitly cancels it.
### Due diligence before building (standing owner rule, 2026-10-04)
**Get it right the first time. On everything that could change the plan, the
bar is beyond reasonable doubt BEFORE building, not after review finds it.** A
PR that comes out half-built because half its premises were never checked is
the failure this rule exists to stop. The checklist, which every development
plan presented for approval answers (task-executor plans from `/task` keep their
own `TASK_INTAKE.md` section contract instead):
1. **Every free read is done.** A free read is anything this session can settle
read-only, or by a probe that changes nothing beyond scratch state, using
tools it already holds. It is owed whenever its answer could change a decision. That covers: the code and every caller the change touches,
its `CURRENT.md` entry, recent commits and PRs in the area, overlapping open
PRs and issues, peer sessions, code intelligence, an API or renderer probe,
and a corpus measurement. "I did not verify X, but it should be fine" is not
an answer for an X like that: verify it, or name it as a residual with the
reason no read or probe settles it.
2. **Premises measured, then red-teamed.** Try to disprove the root cause and the
change's consequences, and name the alternatives weighed, simplest first.
3. **The plan body states:** what, why and how; confidence per item with what
would disprove it; what was verified and what was NOT; alternatives; the
red-team of the root cause and consequences; regression markers; and the
test plan (targeted tests midway and at the end, CI for the full suite, a
functional check of the built thing, "Verify BEFORE the PR" below, and the
`E2E:` line for post-merge verification).
4. **A revised plan opens with a "Changed in this revision" section** right
after its title (YAML frontmatter stays on line 1, and the title stays the
first heading): what changed, and why.
5. **Ask the owner about intent, priorities and trade-offs. Decide measured
technical edge cases yourself** and state the call; the owner overrides at
approval. Design principles and standing axioms, approval and sovereignty
gates, irreversible or outward-facing actions, and anything user-visible
still go to the owner.
A question is material when its answer could change the plan; stakes set how
deep to go on each one, never whether it is asked. On the material ones, this
bar is higher than CLAUDE.md's investigate-below-90% floor. Detail, the
evidence behind the rule, and how it fits the rules it ties together:
`references/due-diligence.md`.
### Architecture Review
Every FINALIZED plan gets exactly ONE `genesis-architect` review — premise
check, scope drift, architecture — before it is presented for approval.
The agent's Step 0.5/0.6 take the plan file as input; hand it the path.
Revisions made in answer to that review do not re-trigger it. A plan too
small to write down needs none. `/plan-ceo-review` and `/office-hours` are
optional extras, if installed. Its findings are claims, handled under
CLAUDE.md's "Verify agent output": re-derive them, then fold the confirmed ones
into the plan BEFORE presenting it.
### Skill invocation points
Required steps at named moments. A skill marked "if installed" comes from an
optional plugin (gstack, superpowers); where it is absent, skip it — there is
no substitute checklist.
- **A test fails unexpectedly** → this skill's "Debugging Discipline"
section first; plus `superpowers:systematic-debugging` or `/investigate`,
if installed.
- **Before any done / fixed / passing claim** →
`superpowers:verification-before-completion`, if installed.
- **An external tool or repo surfaces as a candidate** → the `evaluate`
skill; on an ADOPT verdict, `integrate-module`.
- **A diff touches dashboard or other UI** → `/qa-only` or `/qa`, plus
`/design-review`, if installed.
### Plan documents carry a structured header
A plan doc that outlives one session (`~/.claude/plans/<name>.md`) opens with
YAML frontmatter naming what it commits to and what it was written against:
```yaml
---
plan: <slug> # matches the filename
status: active # active | stalled | superseded | done
updated: 2026-09-15 # last revision of the LIVE section
pinned:
main: <12-hex> # origin/main as of the `updated` date above
binds: "<one line — what adopting this plan commits us to>"
prevents: "<one line — what it forecloses, or nothing>"
---
```
Quote `binds` and `prevents` — they are prose, and unquoted, a `": "` makes the
whole header unparseable while a `" #"` **silently truncates** the value
(`binds: PR #2046 ships first` loads as `"PR"`).
Optional `decisions:` / `ledger:` / `issues:` lists name the trackers this plan
executes, in FULL ids, so a reader of either end can find the other. An absent
list means "none" — so only omit it once you have looked; write
`issues: unchecked` if you have not. In the body,
`## ═══ SUPERSEDED BELOW ═══` divides live content from archaeology; no divider
means the whole file is live. What the body must STATE is set by "Due diligence
before building" above, for every development plan presented for approval.
**`pinned.main` is the field that pays for itself**: it makes
`git fetch origin main --quiet && git log --oneline <pinned.main>..origin/main`
a one-command staleness read, on a document whose line numbers and PR heads
otherwise go quietly false. The fetch is load-bearing — against an unfetched
`origin/main` the command prints nothing and reads as "no drift". A moved main
means re-verify, never that the plan is wrong.
Nothing checks any of this yet — the header is written by hand, and a plan
missing it fails silently. Rationale field-by-field, what the divider does NOT
do for a grepper, and what was and was not carried across from the upstream
schema: `references/plan-docs.md`.
### Timeout Policy
The burden of proof is on you to justify why a timeout should exist.
Do not default to "add a timeout for safety." Instead:
1. **Identify the specific failure mode.** What hangs? Why? Is there
evidence this actually happens, or is it speculative?
2. **Justify the specific value.** Why this number and not another?
What legitimate work would be killed at a lower value?
3. **If you have no strong justification for a specific value, default
to 2 hours (7200s).** This is the project floor — generous enough to
never interfere with legitimate work while preventing permanent
resource lockout from truly hung processes.
4. **Surface the request to the user** with the value, the failure mode,
and the evidence. Never add a timeout as a "small improvement" or
"defense in depth."
Timeouts on reflections, CC calls, cognitive paths, and long-thinking
work fight Genesis instead of helping it — they cap legitimate long
thinking and add speculative defense against rare hangs. The exception
is raw subprocess calls with no external watchdog (e.g., deterministic
executor steps), where a hung process blocks shared resources (executor
semaphore) with no other recovery mechanism.
**The Bash TOOL's own timeout is separate — default 120000ms, HARD CEILING
600000ms (10 min).** An inner `timeout N …` INSIDE the command does NOT extend it:
the tool wrapper SIGTERMs the whole call at its own `timeout` param (default
120000ms → exit 143). To allow longer, set that `timeout` PARAMETER explicitly —
**but you cannot exceed 600000ms.** A larger value does not buy more time; the
call still dies at 10 minutes (MEASURED 2026-08-27: `timeout: 1600000` was killed
at exactly `10m 0s`). Treat "raise the timeout" as a fix that tops out, not one
that scales.
For long or unbounded work generally, use **`run_in_background: true`** —
harness-tracked, notifies on completion, and it does NOT inherit the 120s default
(MEASURED 2026-09-16: a 400s background task completed clean, exit 0). Prefer it
over a foreground timeout or `nohup … &` (detached but untracked → no completion
signal, so you end up hand-polling anyway).
**DEPLOYS ARE THE EXCEPTION, and this is the one to get right.**
`scripts/update.sh` must be **DETACHED FROM THE SESSION ENTIRELY** — not
foregrounded, and not backgrounded. (`bootstrap.sh` and `host-setup.sh` are long
too but are NOT in this group; see the warning below before reaching for either.)
- Foreground is impossible: MEASURED 2026-09-16, a bare `update.sh` ran **1022s**,
1.7× the 600000ms hard ceiling.
- `run_in_background` is NOT the fix either, despite having no 120s ceiling: it is
tied to the SESSION's lifetime, and deploys launched that way were killed mid-run
on 2026-07-22 (leaving genesis-server DOWN during bootstrap) and 2026-09-16
(during the pre-update backup). The precise kill mechanism is UNRESOLVED — three
candidates are eliminated in CC memory `deploy_detach_not_background`, and the
successor hypothesis there is hedged, not established. Detachment removes the
coupling to this session, which is what every remaining candidate runs through.
It is not immunity in general: an OOM kill reaches a systemd unit just as readily.
```bash
XDG_RUNTIME_DIR="${XDG_RUNTIME_DIR:-/run/user/$(id -u)}" \
DBUS_SESSION_BUS_ADDRESS="${DBUS_SESSION_BUS_ADDRESS:-unix:path=/run/user/$(id -u)/bus}" \
systemd-run --user --collect --unit genesis-deploy-manual \
--working-directory=$HOME/genesis \
--setenv=PATH="$PATH" --setenv=SSH_AUTH_SOCK --setenv=GENESIS_SYNC_PRIVATE_PATTERNS \
/bin/bash -c 'mkdir -p ~/tmp; exec ./scripts/update.sh > ~/tmp/deploy-$(date +%Y%m%d-%H%M).log 2>&1'
```
substituting the checkout root you actually ran from for `$HOME/genesis` if it
differs — `scripts/install.sh` sets `REPO_DIR` from its own location, so
noncanonical checkouts are supported, and `update.sh` derives its root the same way, so the canonical path is an example, not a
constant. (Keep the command itself pasteable — a literal `<placeholder>` in the
value is a shell syntax error.)
**Two preconditions, checked before detaching, not after:**
- **Linger.** A `--user` unit dies with the user manager when your last login
session ends unless linger is enabled. `loginctl show-user $(id -u) -p Linger`
must print `Linger=yes`; if it does not, run `loginctl enable-linger $(id -u)`
first (one-time, needs sudo). Without it the "detached" deploy survives the CC
session only as long as you stay logged in — which is the same coupling, one
level up.
- **Noninteractive sudo.** `update.sh` runs `bootstrap.sh` on every run that
applies a change (its "Run bootstrap" step), and bootstrap calls `sudo`. The no-sudo claim covers only
`update.sh`'s own text, not its transitive calls. If sudo prompts anywhere on
this install, the detached deploy aborts mid-bootstrap. Require NOPASSWD-style
sudo: sudo's per-terminal timestamps mean refreshing the credential in your
terminal does not authorize the no-TTY calls inside the unit, and a warm
cache expires mid-deploy anyway. Check it in the same no-TTY service
context the deploy will get, with the bus variables set:
`systemd-run --user --wait --pipe --quiet --collect -- sudo -n true` must
exit 0 (MEASURED 2026-10-01: `--scope --wait` is refused outright, "--wait
may not be combined with --scope"). If it fails, run the deploy in a
terminal instead.
**`mkdir -p ~/tmp` is part of the command, not tidiness.** `~/tmp` is not
guaranteed — `install.sh` and `bootstrap.sh` create it only when `/tmp` is small —
and the shell opens the redirect *before* exec'ing `update.sh`, so a missing
directory kills the unit instantly, before the deploy's own `mkdir -p "$HOME/tmp"`
near the top of `update.sh` can run. With `--collect` that leaves nothing behind to diagnose,
which is the same invisible failure the verify step below exists for.
**The bus variables come first, and they are not decoration.** A CC session often
has no `XDG_RUNTIME_DIR`, and without it the `systemd-run --user` CLIENT cannot
reach the user manager: MEASURED 2026-09-16, a scrubbed environment gives
`Failed to connect to bus: No medium found` and nothing starts at all. `--setenv`
configures the prospective *unit* and cannot help the client connect, so it is no
substitute. `update.sh` exports exactly these two near its top for the same reason, and
`cc/invoker.py` records the same failure as measured on a live install. This one is
easy to get wrong by testing in a session that happens to have the variables —
which is how it reached review here.
**A transient service inherits the user manager's environment, not your shell's.**
Only what you name with `--setenv` crosses over, so a deploy depending on an
exported variable silently changes behaviour: `SSH_AUTH_SOCK` (`update.sh`'s
`git fetch` of a private SSH remote fails without the agent) and
`GENESIS_SYNC_PRIVATE_PATTERNS` (`bootstrap.sh` branches on it) are forwarded above. Naming an unset variable is
harmless — measured, the unit still starts.
**Never apply this to `bootstrap.sh` or `host-setup.sh`.** Both need a channel a
detached service does not have. `bootstrap.sh` runs under `set -euo pipefail` and
calls bare `sudo` (the journald and sshd drop-in steps' `sudo mkdir`), so wherever
sudo prompts it aborts partway through configuring the machine — and "sudo is
passwordless here" is a fact about one box, not about the recipe. `host-setup.sh` is
worse: interactive, run on the bare host VM, and its "Delete and recreate? [Y/n]"
prompt treats EOF as the default `Y`, so detaching it stops and renames the existing
container. `update.sh` is the only script in this family with **no sudo calls of
its own** — it does invoke `bootstrap.sh` (its "Run bootstrap" step), which does call
sudo, hence the noninteractive-sudo precondition above — and it remains the only
one covered. Run the other two in a real terminal;
the advisory no longer fires on them rather than offer a recipe that breaks them.
Wrap **the command you actually ran** — `update.sh` and `bootstrap.sh` are not
interchangeable and take their own arguments. If you build that wrapper
programmatically, quote the
whole inner script as one unit (`shlex.quote`) rather than interpolating an
already-quoted command into `bash -c '…'`: MEASURED, the naive form turns
`--msg 'a b'` into `--msg a` and executes a `$(…)` the user quoted as data.
`--setenv=PATH` is load-bearing: a `--user` unit otherwise gets systemd's default
PATH, which omits `~/.local/bin`, so bootstrap takes its `uv not found — installing…`
branch rather than the branch an interactive run takes. The shipped code sidesteps
this by passing the whole environment (`updates.py::_apply_direct` uses
`env=os.environ.copy()`).
**Two things that look like detachment and are not.** `systemd-run --user --scope`
keeps the CALLER'S session id (MEASURED) — cgroup-isolated, still session-held;
`_apply_direct` gets away with it only because its parent is genesis-server rather
than a CC session. And **`setsid` is not detachment**: it runs the program in a new
session but does not background it or stop the caller waiting on it — MEASURED,
`timeout 1 setsid bash -c 'sleep 4; echo > f'` exits 124 having written no file.
What detaches is the default transient SERVICE mode (no `--scope`): MEASURED
2026-10-01 on systemd 255, a service child's parent is the user manager and it
runs in a new process session, while a `--scope` child's parent is the calling
shell. `--unit` does not detach anything — an auto-named service lands in the
same place — it only gives the unit a fixed name for the verify step below.
Then VERIFY it took — with the same two bus variables set (a scrubbed session
otherwise gets a bus error, which says nothing about the launch),
`systemctl --user is-active genesis-deploy-manual` must print `active`. `systemd-run` does NOT inherit the tool's cwd, so a bare relative
path resolves under `$HOME`, bash exits instantly, `--collect` reaps the unit, and
a failed launch is indistinguishable from a successful one. (It also reports
`inactive` once the deploy has FINISHED, so check promptly and read the log for the
outcome.) `genesis-server` going `inactive` mid-run is EXPECTED. When the server is
up, `POST /api/genesis/updates/apply` with `{"supervised": false}` does the same job
and passes the environment for you.
**A mid-run kill is never a no-op, but what it does depends on the signal and on
the phase.** Three windows, in order. From where `trap '_on_signal INT' INT` arms
the rollback trap — just BEFORE `_write_state "merging"`, not after the merge — an
interrupt runs `_on_signal` → `_do_rollback`. Between `trap '_on_signal_prestop …'`
and there, `_on_signal_prestop` restarts the services it stopped and explicitly does
not roll back. Earlier still — during the pre-update backup (the `backup.sh` call) —
**no INT/TERM trap is installed yet**, so a SIGTERM there runs no handler at all. That last point matters
for diagnosis: "no trap ran" does NOT identify a kill as SIGKILL, because the
untrapped window produces the same evidence. Note too that `_do_rollback` disarms
INT/TERM as it starts, so a second signal lands mid-rollback with default
disposition, and the rollback has failure exits that report `ROLLBACK INCOMPLETE`.
Either way: verify state (server active, no mid-rebase, pin, both CC versions)
before re-running; it is idempotent. (A non-blocking PreToolUse advisory hook,
`.claude/hooks/cc-deploy-timeout-guard`, nudges toward this on any deploy — it does
not check whether you already detached, which is tracked in issue #2088.)
**Why this paragraph is worded so defensively:** PR #1221 shipped both this policy
and that hook on 2026-07-22 prescribing `run_in_background`, from a CC memory
written two days earlier. An incident that same day refuted it. The correction
landed only in a second CC memory on 07-23, both public surfaces kept the original,
and a third deploy was killed by this advice 56 days later. A remedy is only as
good as the case it was actually executed against.
An earlier draft of this paragraph also claimed #1221 shipped a second, already-stale
assertion about signal handling. That was **false, and worth recording as the more
useful lesson**: it was inferred from the CC memory #1221 was written from, rather
than read out of the commit, which shipped the correct claim. A note is evidence
about what someone believed when they wrote it — never about what the code says
now, and never about what some other artifact contains. Open the commit.
### Verify Outcomes, Not Just Tests
`ruff check . && pytest -v` is the minimum bar, not the finish line.
After tests pass, verify the actual end-to-end outcome the change
delivers. Diff behavior between main and your changes when relevant.
For wiring changes: verify the init/bootstrap order passes the right
values at runtime, not just that parameters exist. For notification
changes: verify the notification actually arrives. Ask: "If the system
restarts right now, will this actually work?" If you can't answer yes
with evidence, you're not done.
**A check keyed on EXISTENCE can only confirm — it can never disconfirm.**
Before trusting a verification, ask what it would do if the thing were absent,
stale, or the wrong object. If the answer is "pass", it proved nothing. Three
shapes of this, all measured in one session 2026-09-08:
- **Waiting for a NEW artifact by testing that one exists.** A poll for "a
scheduled-review marker is present" passed instantly against a marker from six
days earlier. Key the wait on the artifact's IDENTITY — the head SHA it names —
not on its presence.
- **Matching the first row instead of the right one.** "Is main green?" matched
the first `push` run for that SHA, which was the CodeQL workflow (success),
while the CI workflow for the same SHA had failed. Filter by the thing you
actually mean (`--workflow ci.yml`), and prefer reading the JOB you care about
over an aggregate conclusion.
- **Counting from a listing sorted by the wrong key.** `gh pr list --state
merged --limit 40` sorts by CREATION, so PRs created earlier and merged today
fall outside the window and vanish; the count read low and looked plausible.
Use a query whose filter IS the property you are counting (`gh search prs
--merged-at`), per CLAUDE.md's truncated-listing rule.
The failure is silent and self-confirming in all three: an under-read is
indistinguishable from a clean result, so nothing prompts a second look. The
tell is that the check passed FASTER or more easily than the work should have
allowed — a poll that succeeds on the first attempt for something that takes
40 seconds to produce has not observed that thing.
**Verify in the REAL runtime context, not a shell proxy.** "Works when I
run it" is not "works where it runs." Same code + same uid ≠ same context:
a long-running systemd service (genesis-server, guardian) differs from your
interactive shell in mount namespace, seccomp, `NoNewPrivileges`, dropped
caps, and — the one that bites — **ptrace/`/proc` access**. Anything that
reads `/proc/<other-pid>/{environ,mem,stat}`, another process's env, sockets,
or namespaced/hardened resources MUST be verified by hitting the **live
endpoint** (`curl` the real server) or running inside the real service — not
`python -c` in your shell. (Origin, 2026-08: the CC-slot stale-code badge
read `/proc/<pid>/environ`, which succeeds in a shell but returns EACCES
under the server's `ProtectSystem=strict` sandbox — so `enumerate_cc_slots()`
returned 0 in-server and **three** features shipped green but inert since
July; the module's docstring claim "same-uid reads succeed" was a shell-tested
falsehood. The fix routed around the ptrace-gated read entirely.)
**And when the work is done, audit the CLAIMS as well as the outcome.**
Everything above distrusts a VERIFICATION; the companion failure is the
SENTENCES you wrote about the work — a number in a commit message, a docstring's
assertion, an in-thread reply saying a finding is fixed. A green suite says
nothing about any of them, and permanent record has no tone of voice, so the
next session reads a confident sentence as a measured one. Those surfaces are
ENUMERABLE, which makes the check mechanical rather than a mood.
`references/high-stakes-verification.md` §11 carries the canonical surface list
and the walk; do not keep a second copy of that list here.
### Verify BEFORE the PR, when this install can show it (standing user rule, 2026-09-24)
You build on a long-lived install, so **when the change's effect can
be observed here AND a wrong run can be undone, see it work before you open the
PR.** Review then looks at a change that works, not at its first attempt to run.
Put what you ran and what you saw under the PR's `## Testing` section. The `E2E:`
line keeps its meaning: the check to run AFTER the merge. For a hook that is still
needed, because a pre-PR run tests the worktree copy, not the merged copy the
harness fires.
**Which copy actually ran is the whole question. Check it; never assume it.**
- A hook guard run as `python3 <worktree>/scripts/hooks/<guard>.py`, with a real
payload on stdin, runs the branch's copy of that file. The harness, and the
worktree's own `.claude/hooks/genesis-hook`, run MAIN's copy by default (see
"The Gate Machinery" below). `GENESIS_HOOK_DEV_LOCAL=1` on that ONE launcher
call switches it, and stderr announces the switch. Never set it session-wide or
in `settings.json`: your own commits and pushes would then run under the gate
you are still editing.
- Which `genesis` package a Python file imports depends on the file. MEASURED
2026-09-25: from the worktree root, `import genesis` resolves to the MAIN tree
through the editable install. But more than twenty scripts and hooks put their
own tree's `src` on `sys.path`; run from a worktree they load the BRANCH's
package, whose database path then resolves inside the worktree, where no
database exists, while Qdrant is still the live one. Read the file's imports
before running it, and print `genesis.__file__` when in doubt.
- So runtime code (`src/genesis/**`) is verified by tests, or by the options
`references/worktrees.md` lists for runtime checks, never by a live run,
until there is a supported way to run a branch in the live runtime.
- Say in the PR which copy ran and what it touched.
**Only a run you can undo qualifies.** A script that deletes, prunes,
restores, deploys to the host, pushes, posts or sends a message, or writes to any
live store (the database, Qdrant, the graph engine) does NOT run before it
merges. A `--dry-run` flag is not an exception by itself: `restore.sh --dry-run`
still takes the live update and backup-restore locks, runs `git pull --rebase` on
the backup repository, and overwrites `~/.genesis/restore_status.json` on exit.
Run a dry-run only after reading that mode's code and finding that it writes
nothing. Do not improvise isolation: `GENESIS_HOME` moves neither
the database nor Qdrant, and many files hard-code `~/.genesis`. Tests are how such
a script is verified.
**Never run on the live install before it merges:** a branch's migration, privacy,
egress or credential handling, anything that loosens a gate other sessions rely
on, and anything whose verification needs days or a machine the owner does not
control. Test a migration in a pytest fixture, and patch EVERY store it touches on
the migration MODULE itself. The suite-wide fixture patches
`genesis.env.genesis_db_path`, but data migrations import that name directly
(`from genesis.env import genesis_db_path`), so do not rely on that fixture to
isolate a migration: patch the migration module's own names. A migration can
also reach live Qdrant and write under `~/.genesis`.
`d0008_reconcile_memory_cross_store`, for
example, deletes the Qdrant points it judges to be ghosts. Its test patches the
module's `genesis_db_path`, `get_client` and `_export_path`:
`tests/test_db/test_d0008_reconcile_memory_cross_store.py` is the pattern to
copy.
NEW behaviour on a risky surface (memory, graph, database) ships behind a
shadow flag, and the session never flips it: the owner does, after verifying.
Entity adjudication is the house example. `config/entity_adjudication.yaml` ships
it in `propose_only`, so a merge is recorded as a proposal. The owner reviews and
approves proposals and applies the approved ones (`entity_adjudication_approve`,
then `entity_adjudication_apply`) without changing the mode. Switching to `live`
is a separate decision: it applies only approved backlog rows, but from then on
it applies each new merge verdict with no approval step.
**Iterate with the owner.** When acceptance needs the owner's hands (a device, a
chat channel, another machine they use, or something only they can see), do not
open the PR on a guess. Tell them exactly what to try and what they should see,
ask through `AskUserQuestion`, fix what they report, and re-ask while it blocks.
Open the PR once they confirm it works. Waiting on the owner here is the design,
not a stall: no review round could have caught what they are about to see.
- **Do not dispatch work whose acceptance needs the owner's hands.** No
dispatched path is known to hold a change for the owner's test today.
- The autonomy executor pushes a completed code task's branch and opens a
draft PR for a build-lane task that passes its scope gate. It then deletes the
worktree and the local branch, so there is nothing left to test locally
(#2421).
- The executor's code steps have no MCP write except `observation_write`, so
`follow_up_create` is not available to them (#2422).
- A `direct_session_run` session's tools depend on its profile, and the
profiles differ in what they allow. Read the chosen profile's definition
(the built-ins in `src/genesis/cc/direct_session.py`, plus any install
overlay in `genesis.cc.profile_overlay`) before assuming the session can commit
code, keep a worktree, or record anything.
If a dispatched session finds an owner test is owed anyway, it states what to
try and what they should see in its final output. For an executor task that
output is read through `task_detail`, and it reaches no notification. A
follow-up row that a dispatched session creates (on a profile that allows it)
lands on the hidden tabled lane, so its `reason` must say it awaits the owner's
test in a foreground session.
- **The owner says they cannot test it this session, or the session ends
before they confirm:** a `follow_up_create` row naming the owner test still
owed and where the code is (branch, worktree path, head SHA). Never write the
change up as verified.
### Acceptance Bar + Measured Rate — the primary methodology
**Use this as often as it applies. It is the default way to build anything here,
not a special-occasion technique.** Unit tests prove the code does what you wrote.
This proves the thing actually WORKS — with numbers and a denominator.
Two artifacts, both produced BEFORE shipping:
**1. The acceptance bar — replay the real defect.** Take the actual failure that
motivated the work and run it through the new thing. If it does not catch/fix
the case it exists for, it does not ship, however elegant it is and however
green the suite is. Reconstruct the real case (from git history, from the
transcript, from live data) rather than a stylised approximation — a synthetic
case can pass while the real shape does not.
**2. The measured rate — run it against real data and produce a number.**
A claim like "low false-positive rate" or "it should be fine in practice" is not
evidence. Run the thing over a real corpus — recent commits, live rows, real
traffic, historical transcripts — and report `k/N (x.x%)`. **A number without a
denominator is not a measurement.** Then look at the individual hits and say
honestly which are real signal and which are noise; a "false-positive rate" that
turns out to be mostly true positives is a fire rate, and saying so is part of
the result.
**For a Bash GUARD's predicate there is a corpus of this install's own recorded
`(command, cwd)` pairs, and `scripts/replay_guard_corpus.py` builds it — but
REPLAY IS CURRENTLY UNAVAILABLE.** Every guard is refused; `--guard <name>` and
`--all` exit 2. So the `blocked k/N` measurement this bullet wants has to be
taken another way for now — by hand, which is how it was always taken: MEASURED
2026-09-16, `git log origin/main --pretty='%s%n%b' | grep -icE "real commands"`
returns 23 against 1,656 merged-PR commits, of which 121 are `fix(hooks)` or
`feat(hooks)`. Roughly one hook PR in five already does this sweep by hand.
The permission was withdrawn rather than lost. It used to rest on a declared
claim that a guard performs no writes, spawns or network calls, re-derived by
walking its imports — and that claim cannot be established by reading: the
spelling set is open, and four review rounds each surfaced the next round's
miss, including three fail-opens inside the checks themselves. Issue #2036
restores replay the other way round, by running it where the effects are
impossible, so nothing has to be proved about the guard at all.
What the tool still does is check that each declaration's prose still points at
code that exists: `--list` resolves every `Cite(module, symbol, fragment)`
against the symbol's source and refuses on a mismatch. Comment stripping is
complete for PYTHON, which is tokenized; a non-Python file loses only full-line
comments, so a fragment cited from a shell file can still match from inside a
trailing one — and a citation with no symbol matches the whole file, unscoped.
`--list` publishes both limits. That is worth running
after editing a guard — six citations in this table went stale within five days,
one onto a comment about an unrelated timeout. It tells you nothing about what
the guard DOES; `--list` prints its own limits.
Two limits will travel with the number when replay returns: it is stamped
UNCLASSIFIED because the corpus contains dangerous commands too, so it is one
side of the tradeoff and needs the positive control the bullet above demands;
and there is **no way to print the blocked command lines**, because those are
real commands containing secrets passed in argv (issue #2007 tracks what a
correct version owes). The count and the verdict are what go public.
The measurement is a GATE, not a footnote. Decide the acceptable threshold
BEFORE measuring, and if the number misses it, tighten and re-measure rather
than shipping with a caveat. When you tighten, re-run the acceptance bar in the
same breath — a filter that improves the rate by breaking the thing you built it
for has made it worse, and only running both together catches that.
Worked example (2026-08-27/28, an orphaned-literal detector). The tool itself
was still on an unmerged branch when this was written, so treat the first two
bullets as a method illustration; the figures in the paragraph after them are
derived from this repository's own history and can be re-derived.
- Acceptance: replayed a real review defect; the detector named the exact
sibling file. PASS.
- Measured, false-positive side: `1/151 real file-edits fired (0.7%)`. The one
hit was an identifier rather than prose, so the filter was tightened to
require interior whitespace, re-measured at `0/151 (0.0%)`, **and the
acceptance replay was re-run to confirm the tightening had not blinded it.**
**That example then became a lesson against itself, which is why it is kept.**
Everything above measures FALSE POSITIVES, and `0/151` reads as though the
tightening were free. It was not. Measuring the other direction needs
INDEPENDENT ground truth rather than the tool's own criterion — otherwise you
grade the tool on its own definition of success. Here that meant mining history
for a literal removed from one file and then removed AGAIN from a second file in
a later commit: the repo itself recording that the first fix left a sibling.
Over 1,584 commits that yields **95 verified cases (6.0% of commits)** — and the
interior-whitespace filter that scored so well on precision **excludes 48 of
those 95 (51%) by construction.** The number that actually decided the design
was recall against a budget cap: **28/47 in-scope cases caught (60%)** with a
cap of 6 literals per edit, versus **47/47 (100%)** with the cap lifted — every
one of the 19 misses was that single cap, and lifting it recovered all of them.
The rule that generalises: **a rate measured on one side of a tradeoff is half a
measurement.** A precision number with no recall number cannot distinguish a
good filter from a blind one, and the side you did not measure is the side that
will be wrong. Decide which direction matters for the thing you are building,
and measure that one first.
This also catches a specific self-deception. A first prototype of that same
detector used a regex and reported "2 findings" while **silently skipping the
entire class it was built for** (the pattern excluded backslashes; every prompt
string ends in `\n`). The acceptance replay is what exposed it. A matcher that
finds nothing is indistinguishable from a matcher that looks at nothing —
only replaying a known-positive tells them apart.
**A null result is a LEAD, never a clearance — and it travels with its blind
spot attached or it does not travel.** The paragraph above is about an
instrument that looked at nothing. This is the harder case: the instrument
worked, the reading was clean, and the reading was still wrong. Three null
results from one session were wrong the same way, and every one was overturned
by CONSTRUCTION rather than by more sampling:
- *"An expansion in verb position always emits ≥2 words, so it self-corrupts any
command it appears in."* Generalised from a handful of samples of ONE of the
construct's spellings. Another spelling admits a degenerate case that emits
exactly one word — so argv survives intact, and the parse resolves a verb bash
never runs. Stated abstractly on purpose; see the note under the
resource-defect bullet in Test-First Discipline.
- *"Zero over-block flips across 129,179 real commands."* The regression was
CONSTRUCTIBLE, and a later worker constructed it: a `BLOCK → ALLOW` flip on
an `rm -rf` of the production database's parent directory.
- *"Zero regressions, verified three ways."* All three ways were the same axis —
see the resource-defect bullet under Test-First Discipline.
A corpus measures what has been TYPED. It says nothing about what is TYPEABLE.
For a SAFETY property the question is therefore never "did I observe a failure"
but "can one be CONSTRUCTED", and only the second question has an answer that
clears anything. **So when a null result is handed to anyone — a subagent, a
peer session, a PR body, the user — state the DENOMINATOR and the METHOD'S BLIND
SPOT in the same breath as the number, and say in words that it is not proof.**
Then name the recipient's job: the person best placed to construct the
counterexample is whoever is about to rely on the null result, and they will not
go looking unless you tell them the search is still open. A null passed on bare
is read as a clearance by everyone downstream, which is how one unproven
sentence becomes the premise of three later decisions.
### Select, don't amputate — truncation is the absence of a decision
**Scope first, because bounding is often correct.** What makes something an
amputation is LOSS — the value cut here was the only copy. Nothing else. A
bounded PREVIEW of something stored intact elsewhere is a selection, and stays
one even if its handle is useless. Bounding against a hard external budget is
likewise correct: a hook's stdout cap, a context window, a column whose limit is
actually ENFORCED. That last qualifier is load-bearing here — this repo's SQLite
`TEXT` columns enforce no length at all, so "the database column" does not
excuse a cut; a self-imposed storage assumption is a decision to justify, not a
budget to obey. So is refusing an oversized value outright.
**And a SAFETY cap may be lossy — that is the one place cutting the only copy is
right.** Streaming is a TRANSPORT property and only-copy is a DURABILITY one;
check them separately, because a chunked read is often backed by a retained
source you could go back to. The case that earns the lossy cap is the source
that genuinely has no retained copy — a live subprocess pipe — where reading to
the end to avoid "truncating" is how a runaway command exhausts memory; this
repo bounds exactly that at a few MiB (`autonomy/executor/deterministic.py`
`_read_limited`, whose own comment names the `yes`-command threat; verified
2026-09-04). Losing the tail of a log beats losing the process. The obligation
there is not to keep the bytes, it is to be LOUD about the cut — say the output
was bounded and roughly by how much, so nobody reads a clipped log as a
complete one. That cited cap has since been fixed AT THE READ and is no longer
the silent-cut example it was: `_read_limited` drains the whole stream while
retaining only `limit` bytes and returns `(retained, total_size)`, so the caller
appends `... (truncated, N bytes total)` with N the TRUE drained total (PR
#1796; verified in `autonomy/executor/deterministic.py` 2026-09-08).
**It is still not a worked example of a loud cut, and the reason generalises:
a declaration has to survive the CONSUMERS, not just be emitted.** That marker
is appended at character 50,000 of the result field, and all six onward paths
head-slice it at 200-2000 characters — including the one that feeds the next
step's prompt — so the declaration is unreachable in every direction it
travels, and the slice that removes it declares nothing itself (MEASURED
2026-09-08). When you fix a silent cut, check the READERS of the field you just
made honest; otherwise you have moved the silence one layer out. A silent lossy
cap is still the defect; a declared one is a resource guard doing its job. This
section is about the remaining case.
**A handle that does not resolve is a separate defect, and do not conflate the
two** — that conflation is the mistake this section made about itself, twice.
Check the pointer, because a preview advertising a retrieval path that does not
exist teaches a lie; but when the full value survives somewhere, the fix is to
mend or drop the handle — removing the cap TO PREVENT DATA LOSS is fixing a
loss that never happened, and that misdiagnosis is how this rule causes the
damage it exists to stop. Whether the cap should exist at all is the separate
question the "what breaks if it is unbounded" test answers: a cap with no
external, safety, or measured compatibility justification may be removed once
that is established — for being unjustified, never for being an amputation.
**There, do not truncate.** Not strings, not lists, not context, not output.
Reaching for a character cap is a signal that a question was skipped, not
answered. Omitting is legitimate — it is a judgement about relevance. Truncating
is not: it is what happens when that judgement was never made, so the value gets
cut at a point that has nothing to do with meaning. A truncated value is
frequently worse than either alternative, because it still LOOKS complete, so
nobody checks it — at which point you may as well not have passed it at all.
Before bounding anything, answer: what is this value FOR, who reads it, why does
it need budgeting at all, and what actually breaks if it is unbounded? Solve
THAT. Usually the answer is "select less, whole" rather than "cut", and often the
bound turns out not to be load-bearing.
Three rules when a bound really is needed:
- **Bound by MEANING, not by one blanket number.** A closed set is validated
against that set — a value outside it is INVALID, not "too long". A timestamp
is a shape; half a timestamp is not a shorter timestamp. A blanket cap turns
100,000 characters of foreign data into 300 characters of foreign data and
calls it bounded.
**This governs HOW you bound, never WHETHER you may.** Rejecting a structured
value or a collection by size is correct and stays correct: an over-long id, an
over-large batch or an implausibly large file is simply not a value we accept,
and refusing it is a resource guard, not an amputation. What is forbidden is
silently CUTTING them to fit. Only free text gets a bound it is expected to sit
under; every other INBOUND value gets one it must not cross. Scope that to
inbound on purpose: a READ that pages a large collection — the first n WHOLE
elements plus a total and a truncation flag — is a selection with a
denominator, this section's own preferred shape, and the source collection is
valid precisely because it exceeds the page. Reject the oversized value you
are asked to ACCEPT; paginate the oversized collection you are asked to LIST.
- **Derive the number from the right thing, and record how.** A SAFETY bound —
memory exhaustion, an abuse ceiling, untrusted input — does not come from the
corpus at all: historical traffic says nothing about adversarial input,
concurrency, or the memory you actually have, and a cap chosen from observed
values will be exactly the wrong size when it matters. Derive those from the
protocol, the capacity and the threat model FIRST, then use `k/N` only to price
what the bound rejects. It is the COMPATIBILITY bounds — how long is this field
in practice, what does this cap cost real readers — that a corpus answers, and
for those: measure the real population and report `k/N` per the Acceptance Bar,
then name the corpus, the query and the date. Naming them is necessary and not
sufficient: the query is only re-derivable if the rows are still there when the
next reader runs it, so a table with a retention window yields an EPHEMERAL
observation. Say which one you have. And what the bound COSTS is a SECOND claim
needing its own denominator — "the cap discards the part worth keeping" is
exactly the sentence that sounds measured because it followed a measurement.
- **Omit explicitly, with a constant-bounded marker** (`<omitted: 104,823
chars>`) in preference to a mid-value cut. An honest gap beats a
plausible-looking fragment. The bound-plus-loud-flag half of this is already
the house pattern; the character-count marker is a proposal, so do not go
looking for a precedent that is not there.
**Declaration is a requirement on top of the loss rules, never a substitute
for them.** Stating the rule as "never cut" overshoots: this repo cuts
mid-value in several places on purpose and is right to — a resource guard on
an unbounded stream, a preview rendered next to the full record, a display
string trimmed before escaping so the cut cannot land mid-entity. Each of
those is a cut the loss rules PERMIT (a safety cap, a pointer-backed
selection, third-party display text) — which does not certify how each is
reported today: the safety cap above still cuts silently, and PERMITTED is
not DECLARED. The order matters: first the loss rules decide whether a cut may
happen at all — announcing a sliced KEY does not un-merge the two identities
it collapsed — and only then does declaration decide whether the permitted
cut is honest. A silent permitted cut is still a defect; a loud forbidden cut
is still forbidden.
**When the total is not already known, say that instead of computing it.**
Bounding a stream is the case — but first ask which KIND of bounded reader
you have, because the bound is on RETENTION, not necessarily on reading. A
drain-and-discard reader consumes to EOF anyway (a subprocess pipe must be
drained or the child blocks) and can count what it discards in constant
space, so it KNOWS the exact total and should say it. Only a stop-at-limit
reader, which truly stops consuming, cannot know. There, state the quantity
you KNOW — the cap — and mark the tail unknown:
`<kept first 40,000 chars; rest of stream omitted>`. Do not write
`<omitted: ≥40,000 chars>`: under this rule's own grammar that number
describes the OMITTED content, and one character over the cap makes it a
fabricated tail length — an invented number wearing the honest marker's
clothes, which is the exact thing the marker exists to prevent.
One trap deserves naming, because it is what produced this rule: **a cap can
manufacture a correctness bug in the very data it was added to protect.**
Truncating an identifier used as a KEY merges two distinct identities into one,
and downstream code then attributes one subject's state to another. That is not
hypothetical — it shipped here. Two roster peers whose names shared a prefix
collapsed onto a single key, so one peer's success cleared the other peer's
recorded failure. A short DISPLAY handle is a different thing and is fine; the
rule is about the stored key, not the rendered one.
**A real need to truncate is a CONVERSATION to have, not a magic number to pick
alone.** If you catch yourself choosing 300 or 200 or 1000, stop and raise it.
**When there is nobody to raise it with** — an unattended background session —
the rule is NOT "never pick a number". It is **never pick one silently.** Bound
if you must, and put the reasoning beside the number, where a reader of the value
will see it. That includes the question that comes BEFORE the number — whether
this value needed bounding at all — because "is this big enough to matter?" is
the same judgement drawing on the same missing information, and skipping it is
precisely how the number gets invented. State both. A number with its reasoning
next to it can be argued with and corrected, which is all anyone needed from you.
What made the original defect dangerous was never that 300 existed — it was that
300 arrived silent, looking deliberate, and was then defended.
That default is about SIZE, never about secrecy, and the two must not be
confused. If a value may carry a credential, token, or personal data, the
unbounded default does NOT apply — for UNAUTHORIZED egress and for storage you
cannot vouch for: there, fail closed, omit wholesale with a marker as the "omit
explicitly" rule says, and keep only non-sensitive metadata. Losing diagnostic
prose is recoverable; leaking a token is not. The scope qualifier is
load-bearing and an earlier revision dropped it while restructuring — read
unconditionally, "omit wholesale" would delete values existing features exist
to hold: the References store deliberately RETAINS credentials, and its own
dashboard tab masks the value behind an explicit reveal
(`dashboard/routes/references.py`; verified 2026-09-04). That is a statement
about ONE surface, not a security guarantee — other readers of the same store
return raw bodies (`reference_lookup`, the knowledge routes), which is worth
knowing precisely because a rule reader might otherwise lean on the reveal
gate as if it covered them. An authorized, gated store holding a secret is the
feature working, not a leak.
**Then ask WHO WROTE IT as well as where it is going — two independent axes,
and each governs a different decision.** Destination governs DISCLOSURE: what
may cross a trust boundary is decided by where it lands, whoever wrote it — a
user-authored secret bound for an external channel still gets scanned — and
quarantined when a SUPPORTED pattern matches. Say that precisely, because the
scanner is a high-confidence pattern gate by its own declaration, not a general
secret filter: content carrying a secret outside its pattern set passes as
safe, so nothing downstream may lean on that gate as proof of cleanliness
(`security/output_scanner.py`; verified 2026-09-05). Authorship is a RISK INPUT to sanitization, and the treatment
itself is chosen by the CONSUMING SINK: the same stranger-authored email
fields are HTML-escaped and truncated where the sink renders raw HTML
(`outreach/engagement.py` `_sanitize_ping_field`, whose docstring names both
the threat and the destination; verified 2026-09-04) and stored UNESCAPED
where the sink is a parameterised database column (`db/crud/email_threads.py`
`record_reply`; verified 2026-09-04). Unescaped is not unbounded, and an
earlier draft conflated them: what reaches that column is `body_preview`,
already cut to 500 characters upstream (`mail/reply_poller.py`,
`parsed.body[:500]`) — a silent mid-value cut of exactly the kind this section
exists to surface, sitting inside the example chosen to demonstrate the
opposite. The escaping point stands; the "canonical, unbounded" flourish did
not survive following its own pointer. Escaping at
ingestion would corrupt the stored copy; passing raw markup to a live
renderer hands a stranger the user's most trusted channel. Maximally
authorized destination, maximally defensive treatment AT THE RENDERER — any
rule that reads "it's going somewhere trusted, so pass it whole" deletes that
defence, and any rule that reads "a stranger wrote it, so escape it
everywhere" corrupts the stored original.
**And an authorized destination does not mean untreated.** The rule is narrow:
do not strip the USER'S OWN content on its way to the user. It is not a licence
to stop scrubbing on owner-facing surfaces, and this repo does not (each
verified 2026-09-04): it rewrites username-bearing paths out of what the owner's
own dashboard renders (`dashboard/routes/backup.py` `_scrub_reason`), gates
credential values behind an explicit reveal rather than showing them (the
References tab — that one surface, per the caveat above), applies the safe
mechanical rewrite to the draft the user reviews (`content/egress.py`
`should_gate`, `category == "content"` — non-fixable tells are FLAGGED, not
removed, so the copy they approve has been treated, not certified clean), and
in at least one subsystem deliberately keeps captured text out of its own
private store entirely (`attention/types.py`: "never stored";
`db/crud/attention.py`: "value-free … NO text"). "Into a private store" is
emphatically not a blanket exemption; some stores are built specifically to
never receive the value.
**One last thing, learned the hard way from this section itself.** Four review
rounds found defects in it, and every single one had the same shape: the cited
FACT was true, and the CONCLUSION drawn from it was false. A retrieval tool
really did lack an id lookup — and "therefore this is an amputation" was wrong,
because the value was stored whole elsewhere. A field really was written
unsliced — and "therefore it is a cheap place to record something" was wrong,
because reaching it aborted the whole run. Verifying that each cited fact is true
is the easy half, and it is not the half that fails. **State the inference as its
own claim, and check THAT.**
The sharpest instance is this section committing that error in the sentence next
to the one warning against it. A draft of the paragraph above cited a real file
that says, correctly, that a particular scrub applies to external audiences and
not to replies going to the user — and generalised it into "this repo leaves
owner-facing content untouched", which four other paths contradict. True fact,
false inference, two paragraphs from the rule forbidding exactly that. The
generalisation is the step to distrust, and it is seductive precisely when the
evidence under it is solid.
### A blocked compound command loses EVERYTHING in it
A PreToolUse block kills the **whole** Bash call, not the offending part — so a
guard firing on step 3 also silently discards steps 1 and 2, while the error
text talks only about step 3. This repo has many blocking guards
(`review_enforcement_commit`, `full_suite_guard`, `concurrent_test_guard`,
`git_push_guard`, the destructive/protected-path guards), so it is not rare.
**Never chain a state-changing step with a step that can be blocked.** Keep
`cd`, heredocs, file writes and restore-from-backup in their own invocation,
separate from test runs, commits, pushes, or anything a guard inspects.
**After any block, verify state before continuing.** Run `pwd`, and re-check the
file you believed you wrote. Do not assume the earlier half of the command ran.
Prefer `git -C <literal path>` over a persistent `cd`, so a lost `cd` cannot
silently redirect later commands. Do the same for scripts by spelling them
`$ROOT/scripts/…` (`ROOT="$(git rev-parse --show-toplevel)"`), which is what
`.claude/commands/deep-review.md` requires — a bare relative `scripts/…`
resolves against whatever the cwd drifted to.
**But the path is not what decides which worktree a script acts on. The PROCESS
CWD is.** Some scripts derive their target from the directory they run in —
`review_state.py`'s `evidence-path` and `mark` resolve the worktree via
`git rev-parse` with the inherited cwd, and never read `sys.argv[0]`. So run
those FROM the worktree they are about, in their own invocation, however you
spell the path. Get this wrong and review evidence is written under another
worktree's key, and the depth gate then blocks on a file you never wrote.
MEASURED 2026-08-28, with controls in both directions: from one cwd, the
relative, absolute and `$ROOT/`-prefixed spellings of `review_state.py` all
printed the SAME evidence path; the same absolute path run from three different
cwds printed three DIFFERENT ones. Path form: no effect. Cwd: decisive.
Measured cost in one session: four heredocs that never wrote, a
restore-from-backup that never ran (leaving a file deliberately regressed), and
a `cd` that never happened — so edits landed in the **wrong worktree** and had
to be reverted as cross-branch contamination. Guard false-positives make this
worse: `shell_parse` mis-parses backslash line-continuations and quoted heredoc
bodies, so legitimate commands get blocked too.
### Editing source in bypass/auto mode: reads via Bash, WRITES via Edit/Write
In bypass or auto permission mode the CC binary injects a meta message telling
the session to prefer Bash (`sed`, heredocs, scripts) over Read/Edit/Write —
and NOT only there: a third selector branch is a cohort assignment, so a
default-mode session can receive it too. It is an EFFICIENCY heuristic making no
correctness claim, which follows from how the message is phrased (a preference,
with its own escape hatch back to the dedicated tools) rather than from where it
is gated (provenance + full measurement:
`docs/reference/cc-compatibility.md` "Bypass/auto mode tells the agent to edit
via Bash"). For reads and search it is simply right — use `cat`/`sed -n`/`grep`/
`find` freely. For WRITES TO SOURCE it is not: a bare `sed -i` returns exit 0 on
an absent anchor (silent no-op), rewrites ALL matches of a non-unique one, and
silently hits the wrong line on a regex-metacharacter anchor where a literal was
meant — where Edit fails LOUDLY on each. A Bash write also fires none of the five
`Edit|Write` PostToolUse hooks (the repo's own post-edit verification plane) and
a heredoc sits in the blocked-compound blast radius above. MEASURED: 54.9% of
real edit anchors in this repo carry a character `sed` treats as special in BRE,
its default dialect (2,750 / 5,013; an ERE set scores 74.0% over the same
corpus, so quote the dialect or the number misleads) — the unsafe case is the
common one. Use Edit/Write for source; a script that does a literal replace AND
asserts its occurrence count is acceptable (it re-implements Edit's checks). This
is universal — the machinery is tracked, so every clone inherits it.
### Instance-Fix vs Class-Fix Gate
When a mechanism failed to write or propagate something (a memory, a
directive, a config row, a status flag), hand-writing the missing
artifact is a **data repair** — it mitigates ONE instance on ONE
install. It is never the fix. Before reporting anything as "fixed",
classify it:
- **Data repair** — you wrote the artifact the mechanism should have
written. Label it "data repair" explicitly, and in the same session
either fix the mechanism or get the user's explicit deferral (the mechanism
fix recorded as an ISSUE; a local row only if the deferral itself needs
tracking). Never report a data repair as "fixed".
- **Class fix** — you changed the mechanism so the artifact is written
correctly on every install, going forward, with a test proving it.
The test: "If a fresh install hits the same situation tomorrow, does my
change help them?" If the answer is no, you have repaired data, not
fixed anything. (Origin: 2026-07-17 — a stale-decision recurrence was
"fixed" with a hand-written memory + directive; the propagation
mechanism that failed to write them stayed broken.)
### Debugging Discipline (phase-gated)
Adapted from superpowers `systematic-debugging`. "Find root causes" is a value;
these are the GATES that make it enforceable:
- **Iron Law: no fix proposals until root-cause investigation completes.**
Investigation (read the full error, reproduce, check recent changes, gather
evidence) is a phase that FINISHES before any fix is proposed — never propose
fixes in the same breath as the symptom. "It's probably X, let me fix that"
= investigation skipped.
- **Fix-attempt cap: 3 failed fixes → STOP and question the architecture.**
The debugging twin of the review escalation cap, with the same mechanics
(count attempts visibly; the cap consumes standing approval). Each failed fix
revealing a new problem in a different place is not bad luck — it is the
signature of a wrong architecture or a wrong problem statement. Do not
attempt fix #4; bring the pattern to the user.
- **Date the code before classifying a red — a stale tree fabricates live
blockers.** MEASURED 2026-09-02: the main worktree sat at ONE commit from
09-01 13:40 to 09-02 19:06. A test run against it at ~18:05 failed
reproducibly and was reported as a live repo-wide blocker; it had been fixed
at 15:47 that day by a merged PR. The failure was real, reproducible, and
describing state that no longer existed. Nothing warns you — `git log --all`
and `git status` both work perfectly on a stale tree, and `git log --all`
even SHOWS the fix, because the fetch is fine and only the checkout is old.
Before calling any red live, FETCH AND COMPARE REFS —
`git fetch origin main --quiet` then
`git merge-base --is-ancestor origin/main HEAD`. Do NOT date the code
(`git log -1 --format=%ad -- <file>`) or read the reflog: neither compares
the checkout against current `origin/main`, and both mislead in BOTH
directions — a current tree holding an unchanged old file looks stale, and a
stale branch carrying one recent unrelated commit looks current. Dating it
can therefore reproduce the exact false blocker this bullet exists to
prevent.
**The same trap applies to your TOOLS, which is easier to miss**: a worktree
carries its own copy of `scripts/`, so a script run from an old branch is the
OLD script. MEASURED the same day — `git_push_guard.py --check-pr 1611` from
a days-old worktree reported `ci: pending` where the current copy reported
`ci: green`, same PR, same minute. Run repo tooling from a tree at
`origin/main`, not from whatever branch you happen to be on — and verify
that with EQUALITY, not the ancestry test above:
`[ "$(git rev-parse HEAD)" = "$(git rev-parse origin/main)" ]`. Ancestry is
satisfied by any branch that merely CONTAINS main, including a PR branch
that MODIFIES the tool — which is precisely the case where the verdict
differs and the one you must not run.
*"Verify against actual code" needs the companion "verify against actual
CURRENT code."*
- **Boundary instrumentation for multi-component failures.** When the path
crosses components (hook → server → engine; CI → build → deploy), don't
reason about where it breaks — LOG entry/exit at each boundary, run ONCE,
and let the evidence localize the failing component before investigating it.
(The "starved vs broken" check — inputs before regression-hunting — is the
special case of this.)
- **Read the reference implementation COMPLETELY before deriving from it.**
When a change must mirror what another subsystem does (what routing consumes,
what the runtime resolves, what a protocol expects), read that subsystem's
path END-TO-END first and derive from its own code/loader — never
incrementally guess-and-patch toward it. Incremental spec discovery is how a
review loop runs 7 rounds. (Origin: PR #1281 — the onboarding floor's key
list was wrong three times until it was derived from the router's own
`load_config` + call-site chains.)
- **Spec required-sets: enforce the WHOLE set at once, and lock it with ONE
test.** The spec-facing corollary of the rule above. When a change must make
code satisfy a canonical spec's *required-set* — a prompt's required JSON
fields, a validator's mandatory keys, an allow-list — read the spec's required
list in full and enforce ALL of it in one move; then write a single test
asserting the required block == the complete canonical set (not one assertion
per field). Adding only the field a reviewer just flagged leaves the next one
for the next round, and a per-field test goes green while the next missing
field ships. (Origin: PR #1333 — `_SALVAGE_PROMPT`'s Required block was
completed one field at a time across three Codex rounds — cognitive_state_update
→ confidence → observations — for what `REFLECTION_DEEP.md` declares as one
closed set `{observations, confidence, cognitive_state_update}`.)
- **Condition-based waiting.** When a fix or test must wait for a state change,
poll the CONDITION (with a bounded deadline), never sleep an arbitrary
duration — arbitrary sleeps are flaky under load and slow everywhere else.
This complements the Timeout Policy (which governs the values).
### Test-First Discipline
Adapted from superpowers `test-driven-development`, scoped to where it pays:
- **Bug fixes: failing reproduction test FIRST — always.** Before touching the
code, write the minimal test that reproduces the bug and WATCH IT FAIL for
the expected reason. Then fix; the same test proves the fix and pins the
regression. A repro test written after the fix proves nothing (it never
caught the bug).
- **Verify-RED, always and everywhere.** Any new test must be seen to FAIL
(correctly) at least once — via the bug, a reverted fix, or a deliberately
broken assertion — before its green is trusted. A test that has only ever
passed may be testing nothing; a whole suite passing every review round
while a reviewer keeps finding real spec bugs is the tell that the tests
encode the same wrong spec as the code.
- **A RED that comes back GREEN has AT LEAST six causes, and "the test is
vacuous" is the LAST one to reach for.** In rough order of how often they
actually occur:
1. **The run never EXECUTED** — a guard refused it, a lock held it, the tool
timed out — so there is no result at all. This is the CONFIDENT FALSE
NEGATIVE: a sweep reporting "all mutations survived" is far more often a
sweep that never ran. Make the runner ABORT when the test command emits no
result line, and treat a SKIPPED/deselected line FOR THE TEST UNDER
VERIFICATION the same way — it is a result line, and it still means
nothing ran. Scope that check to the target: a suite carrying legitimate
`skipif` tests emits SKIPPED lines on every healthy run, so a runner that
aborts on ANY of them refuses every run — and the agent then either sits
blocked or starts stripping skip markers to unblock itself.
2. **The MUTATION silently failed to apply** — the auto-formatter reflows
lines and a `str.replace()` anchor written from memory then matches nothing.
3. **The test ran against a DIFFERENT COPY of the code** — an installed
package shadowing the source tree, a stale `.pyc`, the wrong virtualenv, or
(measured, this session) a path relative to a process whose cwd had moved to
another worktree. The mutation applied, the run happened, the test is sound,
and none of the other causes fits.
**A test module can manufacture this for everyone else**, which is the
variant that hides longest: loading a script under a SHARED `sys.modules`
name and not restoring it. pytest imports every test module at COLLECTION,
so the last registration wins for the session — a module collected earlier
keeps the object it bound while production code doing a call-time
`from <name> import …` resolves the newer one, and
`monkeypatch.setattr(<module>, …)` then patches nobody. It passes in a
single-file run and fails only in the full suite, in collection order, so it
reads as a bug in whichever test depended on the patch. MEASURED on main:
four such modules, across `review_state` and `review_scope` — both imported
at call time by the commit gate. Use `tests.conftest.private_module`, which
registers, execs and restores; never hand-roll the sequence.
4. **The mutation was BEHAVIOURALLY NULL** — it applied and parses, so every
postcondition below passes, but it changed no behaviour: swapped operands
that commute, an edit inside a dead branch, a type annotation Python does
not enforce (also measured this session). The remedy is a different
MUTATION, not a different test.
5. **A SIBLING LAYER still enforces the invariant**, so the green is correct.
When two layers produce the same behaviour, mutate the WHOLE mechanism, not
one of its halves. (The duplicated-layer anecdote in the review-loop section
is the worked example.)
6. **A SIBLING TEST LEAKED STATE that masks the mutation** — an undone
`monkeypatch`, a stray `os.environ` entry, a mutated singleton or a
module-level cache — so the mutated path is never reached in THIS run.
It matches none of the five above: the run executed, the mutation applied
and is not behaviourally null, the code is the right copy, and no
production layer is enforcing anything. MEASURED in this repo: a leaked
disable lever made the very lock under test a no-op, and five real
failures read as a story about the mechanism instead. The remedy is test
ISOLATION (an autouse fixture that clears the lever) — NOT a different
mutation, and NOT a different test.
Only after all six: the test is vacuous. The list is ordered and still not
closed — if none of them fits, the vacuous conclusion is UNPROVEN rather than
established: look for the cause you have not modelled before rewriting a test
that may be sound, because rewriting a sound test is the expensive mistake
here.
The PRINCIPLE, which is what to remember:
**every injection must prove it applied, by its own postcondition, before any
result is read as RED.** Two corollaries follow, and both have bitten:
prove it against the value THAT injection was handed, never against the
pristine original — with two or more edits, the first edit keeps a whole-file
`mutated != original` true while a later one silently misses its anchor, so
the partial mutation reads as complete (assert the anchor matched the expected
number of times); and prove it with the mutated file's OWN parser —
`compile(src, path, "exec")` for Python (NOT `ast.parse`: that only builds a
tree, so it accepts context-invalid constructs like a `return` moved outside a
function or a `break` outside a loop, and the `SyntaxError` then surfaces at
COLLECTION, where a nonzero exit reads as a successful RED), `bash -n` for
shell — since an invalid mutation breaks collection and reads as a successful
RED, while the wrong language's parser
rejects a valid mutation and hides a real survivor. Restore from a file copy
taken beforehand, never `git checkout` — the work is uncommitted — and make
the restore ATTEMPT unconditional ON THE RUN'S OUTCOME (`trap restore EXIT`, a
`finally:`), never the tail of an `&&` chain. Unconditional means it always
RUNS, not that it always OVERWRITES: what it writes is still gated on the hash
check below. A `trap` that restores blindly is the very thing that destroys a
concurrent edit. The expected outcome here is a NONZERO exit, and under
`set -e` a trailing restore is exactly the statement that never runs (the
`out=$(cmd)` entry in Common Traps is the same mechanism), so the shape that
reads as careful leaves a deliberately-broken file in an uncommitted worktree
on the ordinary path — as well as on interruption or a tool timeout. Take
that copy ONCE for the whole sweep and refuse to start EACH cycle if the file
already differs from it — a copy re-taken immediately before each mutation
makes the check vacuous, since the file trivially matches a copy a moment
old; the baseline exists to catch a PREVIOUS cycle that failed to restore, or
a concurrent edit. Verify the restore by hash rather than assuming it. And
restore ONLY what you broke: compare against
the hash the MUTATION wrote before overwriting, because between the mutation
and the handler another session, agent or formatter may have edited that file,
and a blind snapshot restore silently destroys their uncommitted work — a
final hash check does not catch this, it only confirms the overwrite
succeeded. If the file no longer matches what the mutation wrote, PRESERVE it
and report the conflict instead. The cleanest way to avoid the window entirely
is to mutate inside an isolated worktree nobody else is editing.
- **A RED for the WRONG REASON is not a verify-RED — read WHICH assertion
fired, never just that one did.** The six causes above all ask why an
expected RED came back GREEN. This is the mirror, and the dangerous half of
it passes review: the run failed, so the checklist is satisfied and the test
is trusted, but it failed on something other than the defect. **The tell is
any failure line you have not actually read.** Two shapes, and they cost
differently:
1. **The PRIMARY assertion fired, but on a condition the FIXTURE created**
rather than the defect — a leftover competing candidate, an unaged file,
stale state from setup. This one goes green on the fix, because the fix's
side effects remove the condition, so it passes review and pins nothing.
This is the expensive one.
2. **A guard-the-guard / precondition assertion fired FIRST**, which proves
the fixture never built the hazard at all. This one does NOT go green on
the fix — the fixture is unchanged, so it fails again and says so. It
costs a round, not a false green, and the guard is doing exactly its job
(see the vacuous-shapes bullet below, which prescribes adding one).
Repair the fixture and re-run before recording a RED.
MEASURED twice in one session on the same PR: a fileless-session test whose
file was not yet aged past the 60-second window, and a selection test with a
competing candidate still in the tree — each reported a textbook RED while
exercising nothing. Name the assertion that fired and confirm its message
describes the defect. A RED you have not read is a GREEN you have not earned.
- **A fixture must create the shape its docstring claims — and the check is the
guard-the-guard assert, not a re-read.** This is the same obligation as the
vacuous-shapes bullet below; the addition here is WHERE the shape most often
goes wrong, which is a contract between two components. Two ways to break it,
both MEASURED in one session, and both PASS against the unfixed code — which
is the only way they are caught: a **hand-written intermediate** (the test fed
the consumer a value the real producer never emits, so the arity mismatch it
claimed to pin was never present) and an **inlined copy of the fix** (the test
ran the corrected logic as a snippet instead of invoking the real script, so
it graded its own copy and the shipped file never executed). If the defect
lives in the seam between a producer and a consumer, drive the REAL producer
into the REAL consumer; anything typed by hand in the middle is the bug's
hiding place, and anything inlined is a second copy of the code under test.
- **Two reviewers pushing OPPOSITE values for the same constant, one review
pass apart, is a signal about the DEPENDENCY, not about the value.** (Review
passes here, not the cross-model *rounds* the escalation cap counts — see
that section for the reserved sense.) The instinct is to adjudicate: measure
the box, pick the winner, move on. That answers the pass and leaves the
mechanism, because both reviewers are usually right about the environment
each has in mind — which means the code depends on something that is not
invariant. MEASURED: one reviewer moved a parse to field `$4`, the next moved
it to `$5`, and both were correct, for different releases of the tool being
parsed. The value was never the defect; depending on column POSITION was.
Remove the dependency — match the field by what it IS, derive the index, or
read a named output format — which is usually cheaper than a third pass of
picking a number. **Scope: this applies where the constant names a POSITION
or a shape in something external** (a field index, a column offset, an offset
into a format you do not control). It does NOT apply to a threshold or a
timeout: there the disagreement is about which failure mode each reviewer has
in mind, and the Timeout Policy above governs — name the failure mode, and
the value follows from it.
- **Vacuous-test shapes to check for by name** (a list of the common ones, not a
definition). The most frequent in practice is the one that never ran at all: a
test SKIPPED by a marker, or deselected by a `-k` filter or a wrong path, which
reports SKIPPED or "no tests ran" and never goes red — check the count, not just
the absence of failures. Beyond that, a test is vacuous when: its
assertion is ALSO true on the success path (`assert x.blocked is False` where
a successful call also returns False — assert the fact that DISTINGUISHES
them); its setup short-circuits the path it names (passing an explicit
argument the code prefers over the env var under test); or its fixture never
creates the shape it claims (a `bash -c 'sleep 30 # marker'` decoy
exec-replaces itself and loses the marker from its argv — add a
guard-the-guard assert that the fixture really has the property); or the code
under test HEALS the condition before the guarded step runs (an earlier
`git diff` refreshes the index as a side effect, so a test of an
index-refresh guard passed with the refresh deleted). To test a guard against
a transient condition, inject the condition AFTER the last step that could heal
it, and prove it with a mutation. Ask of every
new test: *would this still pass if the mechanism it names were deleted?*
When the DISTINGUISHING fact is produced by a lane the assertion cannot see,
asking every test to assert it is a convention, and conventions decay — the
merge gate's own suite carried that request in a fixture docstring and 3 tests
had already drifted from it. `tests/test_hooks/conftest.py`'s `offdiff_lock` is
the worked chokepoint: it fails any IN-PROCESS hook test whose finding was
silently discounted as outside the diff, so a forgotten allowlist entry can no
longer turn a not-block assertion green for the wrong reason. It fails OPEN,
so its own self-tests are the load-bearing part — a lock nobody checks is the
convention it replaced.
- **Contested/subtle specs: write the expectations first.** When what-should-
happen is itself under discussion (which keys count, which states clear an
alarm), enumerate the expectation table as failing tests BEFORE implementing
— it forces the spec question to surface at design time instead of review
round 4.
- **Corpus replay cannot find a false-POSITIVE class — generate the matrix.**
Replaying real recorded inputs proves only that shapes you have ALREADY run
still behave; it is structurally blind to a shape you have never typed. A
guard change measured "0 false positives across 18k real commands" and still
hard-blocked an ordinary command, because the corpus happened to contain no
instance of the one shape that mattered: a construct the parser mis-handles
that ALSO leaves the guard with no parsed segment. For any classifier
or guard, enumerate the CROSS PRODUCT of the axes that actually drive the
decision — {operation} × {the constructs your parser can mis-segment} ×
{evidence present, absent} × {interactive, unattended} — and assert the invariant per
cell, so an untested cell fails
loudly instead of silently. Keep the corpus replay as the realism check; the
generated matrix is the coverage check FOR THE MODEL YOU DECLARED, which is
the most it can be — a construct you never thought of has no cell to skip and
so passes in silence, which is the same false confidence one level up. Naming
it "the coverage check" without that qualifier is the trap. Discovering new
axes is a different instrument: differential or property testing against a
canonical parser, which fails on shapes nobody enumerated. Skip a cell only
with an explicit reason recorded in the skip, since a silent skip and a hole
look identical.
**Generate the cells from the GRAMMAR of what the rule flags, not from
history.** The axes above are the shape; this is where they come from. MEASURED
2026-09-09, after three corpus-based measurements had each missed a real
regression: enumerating {program} × {option form} × {construct} × {position} —
the axes the rule itself keys on — and running EVERY cell through real `bash`
via an argv-printing shim AND through the real guard subprocesses on BOTH trees
produced 746 cells and, more to the point, a control that MOVED: `main` 744
ALLOW / 2 BLOCK, PR head 339 / 407, after the fix 534 / 212. That surfaced 206
over-blocks which a 129,179-command corpus had shown none of. A second sweep
built 384 cells over the same four axes. Run the matrix against main as well as
against your branch, and compare it PER CELL, keyed by input — never by the
aggregate counts. Two cells can swap ALLOW and BLOCK while the totals sit
identical, and a resource-only fix legitimately preserves every verdict (the
next bullet is exactly that case), so a matching distribution proves nothing in
either direction. Establish liveness separately: use a deliberately mutated
control or another known outcome that the harness must distinguish. A correct
resource-only fix can preserve every production verdict, so it need not make
the two production trees differ.
**Instrument the probe before you trust 746 of anything.** These guards emit
ASK as JSON on stdout with EXIT CODE 0, so a probe reading only the return code
cannot tell ASK from ALLOW — and ASK is precisely the state a newly added flag
usually produces, so the sweep reports "no change" while the new flag fires on
every cell. Combine exit status with the JSON payload when status is zero:
BLOCK is the non-zero gate result, while ALLOW and ASK are distinguished by
the payload. Prove the harness can distinguish each outcome on known inputs
before believing any aggregate it prints.
- **A correctness test cannot see a RESOURCE defect — and on a path the harness
can SIGKILL, that defect is a bypass.** Verifying a change three ways is ONE
verification when all three ask the same question. MEASURED 2026-09-09 on an
in-flight fix that added a new scan to the shell parser the hook stack shares:
it was checked at the parser level, at the guard level, and against the
specific exploit it existed for — three passes, all CORRECTNESS, all green —
while the scan itself was quadratic in the length of a SINGLE token, the
ordinary accidental-O(n²) shape of a nested scan with no early exit. On the
linear form it held flat at 0.004s (4ms) at every length; on the pathological one
it ran to SECONDS, then minutes, at input sizes a caller can simply type. A
PreToolUse hook that overruns its wall-clock is SIGKILLed, and a killed hook
fails OPEN — the guard code says as much in its own comments — so a scan that
can be made slow enough disarms the gate rather than tripping it, and because
the parser is shared it does not stop at the one guard being edited. A FOURTH
correctness check would have passed too.
So, for any code the harness can kill: ask the three questions the correctness
suite never asks — what is its complexity, what input maximises it, and what
does the system do when it does not finish. Where "does not finish" means
PERMIT, a super-linear scan over attacker-influenced input IS the defect,
however correct its output. Measure its worst-case runtime in a controlled
performance probe, and keep a deterministic regression test for the bounded
algorithm in CI; a wall-clock assertion in an install-agnostic test is not
reliable. Record the measured margin against the smallest timeout on its path.
(The Generalizability
Gate's "retention machinery does not belong on a hook path" is the same
mechanism met from the other end: work that scales with the input, on a path
whose deadline is a security boundary.)
**This bullet is deliberately imprecise, and that is the second rule in it: a
lesson about a vulnerability does not need the vulnerability's parameters.**
The shape teaches — super-linear on attacker-chosen input, measured at seconds
where the linear form completes in milliseconds, on a path that fails open. The exact
input sizes, the timeout tier they defeat, the count of guards behind the
shared parser and the quoted fail-open line teach nothing further; they only
compose into a working recipe, and one that keeps working for every install
still on the pre-fix code. Nor does "the fix has landed" license the specifics
— check whether it landed on `main`, not whether a PR for it exists.
- **Anti-patterns (binding):** never assert on a mock's behavior when the real
code path can run; never add test-only methods/branches to production
classes; fakes implement the real contract (real method names, real return
types — import them). Test setup so complex it needs its own debugging =
the design is too coupled; fix the design.
### Code Intelligence — pick the right lane
**Serena (Python LSP) is always live** — it parses current files per query, so
it's the default for symbol/reference/impact questions ("who calls X", "what
breaks if I change Z") and never goes stale. CBM gives the architecture/graph
overview. GitNexus does what neither can — multi-hop blast radius, execution
flows, route/tool maps, coupling/community analysis — but it is **snapshot-
based**: its answers are only correct when the index matches the working tree,
and it drifts after you pull merged PRs (its reindex fires on local commit, not
on pull). So reach for GitNexus deliberately for its unique views, and run
**from the main checkout, run `scripts/lib/code_intel_index.sh "$PWD" gitnexus fast` first** when freshness matters. Linked worktrees deliberately skip indexing; for live "who calls this"
during active editing, prefer Serena. There is no "always run impact before
every edit" mandate — that just gates work behind a tool that's stale-by-design.
- **Blast radius / impact:** Serena `find_referencing_symbols` (live) for the
direct caller set; GitNexus `impact <symbol>` (reindex first) for multi-hop +
affected processes/risk. Use the full UID if ambiguous
(`Method:path/file.py:Class.method#N`).
- **Unfamiliar code:** `gitnexus context <symbol>` or browse
`gitnexus://repo/GENesis-AGI/processes` (when fresh).
- **Custom questions:** `gitnexus cypher` — LadybugDB uses `CodeRelation` with a
`type` property for edges, not Neo4j-style named edge labels.
Full syntax and Cypher examples: `.claude/docs/code-intelligence-guide.md`;
tool-selection decision matrix: `.claude/docs/code-intelligence.md`
### Advisory is the default; a BLOCK is what needs the argument
Before the fail-direction question below, there is a prior one, and it is the
one that gets skipped: **should this refuse at all?**
**Advisory is the default and the first step.** Escalating to a BLOCK needs a
specific, credible, MEASURED reason — the risk is that severe, or that frequent.
*"For safety"* and *"defense in depth"* are not reasons. Neither is having just
been bitten once: an n=1 incident earns a COUNTER, not a gate.
**Persisted state inside a guard needs its own justification, separate from
the decision to refuse.** Before a guard or gate grows a marker, cache or
counter that one invocation writes and a later one reads (a telemetry counter
no guard decision reads is observability, not guard state), show why the answer
cannot be derived at the moment it is needed from state that already exists,
and enumerate the lifecycle the state adds — write, read, retire, validate,
scope — because each stage is a defect site. Where a derivation exists, a second
should-fix-or-worse finding in machinery the change adds means delete it, not
harden it; where none exists, narrow the lifecycle. Approval records are out of
scope for deletion. MEASURED on the gate-menu feature at comparable size: the
persisted-marker layer drew roughly 15 should-fix-or-worse findings against
roughly 3 in the half it served (179 vs 155 non-comment lines, per #2027's PR
body); the marker designs were closed (#1863, #1999) and the counter-based one
merged (#2027). This prices the cost of STATE, not the case for gating; the
New-Store Gate below asks the neighbouring question (why a NEW store rather than
an existing one), and the `genesis-architect` agent's Step 0.7 asks this one at
review time.
**Fail-open is not an automatic defect — but say WHICH question you are
answering, because there are two and they get opposite defaults.** (1) The
VERDICT question: when a guard evaluates successfully, should its design be an
advisory or a block? Advisory by default, per the above. (2) The DEGRADE
question — what a guard does when it CANNOT evaluate — is decided per boundary
by the section below, and many of this repo's Bash hooks are REQUIRED to fail
closed there by the degrade-direction contract test
(`test_every_bash_hook_declares_its_degrade_direction`); nothing in this
section loosens that, and satisfying it with a false advisory claim in the
exemption list is the named failure, not compliance. The claim here is about
(1): a control that fights the operator daily against a harm that is usually
hypothetical is a cost, not a defense. Ask which direction THIS boundary should
fail in — open, closed, or open-and-loud — from consequence, never from which
one sounds safer.
⚠ **This governs GUARDS — advisory nudges and friction hooks — never APPROVAL
BOUNDARIES.** Anything that gates autonomy expansion, spending, destruction of
data, or publication to an external surface is a sovereignty boundary, not
friction to be tuned: never a downgrade candidate, and proposing to relax one
is itself the failure. The autonomous-CLI gate, ego proposals, per-transaction
financial approval and destructive-command confirmation are canonical members
of that class, not an exhaustive list — classify by what the boundary GATES,
never by whether it appears here.
Two consequences worth stating, because they are easy to get backwards:
- **Blocking stops foreground and background equally; advisory stops neither.
Both are fine.** What is NOT fine is an ask that reaches an UNATTENDED
session — an ask with no human present is a block nobody intended — or a
block that impedes background work nobody meant to impede. An ask leg is
legitimate exactly where the shipped ones sit: at an approval boundary in a
foreground flow, degrading to a DEFINED verdict when dispatched (the push
guard's ask converts to deny under `_is_dispatched()`) rather than hanging.
- **A guard is written for the agent, not the user.** Nine times in ten the user
need not know it fired.
This is a design axiom, not a preference: changing it is a redesign
conversation, not a session's call.
### Guard failure semantics — the third option is fail OPEN, LOUDLY
Every guard has TWO INDEPENDENT axes when it cannot evaluate, and collapsing
them into a single "fail-open or fail-closed" choice is what produces the bad
design:
- **DIRECTION** — what happens to the DECISION: open (the action proceeds) or
closed (it is refused). Decided per boundary, by consequence.
- **VISIBILITY** — what happens to the FAILURE: silent, or recorded where a
later reader will actually find it. **Silent is never correct, in either
direction.**
The third option is the one that gets forgotten: not open, not closed, but
**open and LOUD** — the action proceeds AND the guard's inability to evaluate
becomes a durable record plus an alert.
**THE TEST IS THE REPAIR PATH, and it is measurable — do not reach for a
judgement about severity.** Ask: with this failure present, can the session
still REPAIR the failure? Name the operations repair needs — read a file, edit
it, restore it, restart a unit — and check whether your refusal predicate
matches any of them. If the repair path survives, fail-closed is available. If
it does not, fail-closed is not "safer", it is a BRICK, and the answer is
open-loud however severe the thing you wanted to stop.
The test has a worked precedent, and it is worth reading for the SHAPE of the
argument rather than as a verdict. PR #2042's body justifies its fail-closed leg
by naming what survives — *"Read/Edit/Grep are ungated and `git checkout --
<file>` matches none of the matchers"* — which is exactly this question, asked
and answered with a measurement. (The quote is from the PR BODY; the function's
own docstring makes a related but weaker claim, so cite the PR, not the symbol.)
Two cautions travel with that precedent, both MEASURED, and they are why the
test is a question you re-ask rather than a conclusion you inherit. First, an
ungated repair verb is not automatically a SAFE one: in that same degraded state
`git_discard_guard` no longer takes its recovery snapshot, and its own
`also_lost` notice says so — *"a discard run now is not recoverable from it"* —
so `git checkout` being unmatched means it RUNS, not that it is safe to run
broadly. Second, the precedent covers only ONE of that helper's two failure
legs; the other is the worked example below of the test returning the opposite
answer.
**SYSTEMIC failure means no usable predicate survives**, so any refusal would be
TOTAL and the repair path closes with everything else — typically because the
machinery that runs guards is dead (the wrapper, the interpreter, the venv), or
because one missing dependency hits every guard at once. **There the default is
open-loud** (standing owner ruling, 2026-09-15). Genesis runs headless: the
interactive session IS the repair path, and the host Guardian is the
repair-of-the-repair. A guard layer that fails closed at that level does not
protect the machine, it bricks it, with nobody at a console to type the fix. A
messy action Genesis can clean up afterwards beats a system that can no longer
act at all.
Note what does NOT decide this: "how bad was the cause". A single missing
dependency produces EITHER regime, and the same helper contains one of each —
which is the clearest worked example in the repo, so apply the test to both:
- **`degraded_exit`'s predicate-gated leg** — a shared sibling (`shell_parse`)
is unimportable, so the guard refuses only what its degraded MATCHER names.
Read/Edit survive, `git checkout` is unmatched, the rates below are what it
costs. Repair path open ⇒ **PARTIAL**, and fail-closed is licensed.
- **`degraded_exit`'s unconditional leg** — `hook_input` ITSELF is unimportable
or version-skewed, so nothing the guard could import can recover it and each
guard `os._exit(2)`s with no payload read and no predicate consulted.
**The size of that refusal is no longer written down here, and that is the
point.** It is DERIVED on every test run by
`tests/test_hooks/test_import_time_degraded.py::test_every_bash_hook_declares_its_degrade_direction`,
which parses `.claude/settings.json` (never greps it), runs every hook whose
matcher can fire on Bash against a poisoned `hook_input`, and asserts the RULE
the number was only ever evidence for:
> A hook that can fire on Bash either carries a degraded handler — emits `GUARD
> DEGRADED`, exits 2 — or is NAMED: in `_ADVISORY_BY_DESIGN` with words verified
> verbatim against its own docstring, or in `_NOT_PYTHON_ON_BASH` because it
> never imports the module. There is no third bucket.
Polarity is ALLOWLIST, so a guard wired next year that forgets its handler fails
by construction instead of quietly ceasing to guard. That matters because the
omission is invisible in production: Claude Code treats a non-2 exit as a
non-blocking error **when the hook emits no `permissionDecision`**, so a guard
that dies on its import traceback exits 1, the command runs, and nothing
distinguishes it from a guard that looked and approved. (The qualifier is not
pedantry: an exit-0 hook carrying `permissionDecision: "ask"` does gate, and
this repo has one — so "non-2 means non-blocking" is true of the degraded leg,
where that channel is unreachable, and not of hooks in general.)
**THE POPULATION FILTER IS WHERE THIS KIND OF GATE FAILS, not the assertion**,
and the first version of this one proved it. It compared `matcher != "Bash"` —
an exact-string test against a field Claude Code treats as a REGEX — and
discarded any command its narrow pattern could not parse. An adversarial audit
broke it four ways against mutated copies of the real settings: a hook wired
`"Bash|Edit"` was invisible, `".*"` was invisible, and an EXISTING blocker
respelled as a bare `python3 …/guard.py` silently left the population with the
suite still green. `procedure_advisor.py` is wired on `".*"` in this repo, so
that was a live gap, not a future one. An enumerator that silently drops what it
cannot parse is a DENYLIST one level up. The fix is the shape
`test_hook_output_contract.py::_resolve` already uses: evaluate the matcher as a
regex, and make an unresolvable command a FAILING row rather than a skipped one.
Two limits remain, stated rather than assumed away: the enumeration is scoped to
the REPO settings, so a user-level `~/.claude/settings.json` can wire more on the
same matcher invisibly (MEASURED on one install: it wires a shell hook on
`Grep|Glob|Bash`); and shell hooks on the matcher never import `hook_input`, so
they are exempted BY NAME rather than left unseen.
**Why a count became a rule — four failures, and only three were mistakes.**
`grep degraded_exit(` finds six callers, and an early draft therefore said "six
guards"; but this leg is precisely the one where `degraded_exit` is UNREACHABLE,
so every refusing guard refuses from its own import handler and grep sees only
those that ALSO call it on the other leg. Count the condition, never the helper.
Correcting it to nine then left the denominator silently narrowed to the subset
actually executed — **a figure written as "N of M things wired on X" has to
re-enumerate X's full population programmatically or it ships one unstated
restriction copied verbatim into every surface that quotes it.** Naming what M
excludes is the fix; adjusting M is not.
And then the fourth: the corrected figure went stale within a day with nobody
being wrong at all, because another PR wired one more hook on the same matcher.
A denominator maintained by hand in four places is not a measurement, it is four
chances to be out of date — which is why the rule is asserted and the number is
computed. **Two limits stay stated rather than assumed away:** the enumeration is
scoped to the REPO settings, so a user-level `~/.claude/settings.json` can wire
more on the same matcher invisibly; and the matcher also carries shell hooks,
which never import `hook_input` and so cannot be affected — one of which is
itself a blocking guard.
**Bash alone is not the question the test asks** — what matters is whether ANY
route to repair survives, and Write/Edit is one.
**THIS TEST FOUND A REAL BRICK ON ITS FIRST APPLICATION, and the fix is the
worked example of what it is for.** `scripts/pretool_check.py` carried the same
unconditional block on `Write|Edit`, so the session could neither run a command
NOR edit a file: repair path CLOSED, SYSTEMIC, on a headless box. And that
refusal was a strict OVER-block rather than a conservative one — this guard
blocks CRITICAL-path writes in DISPATCHED sessions only, an interactive session
is allowed through by design, and its healthy session test
(`os.environ.get("GENESIS_CC_SESSION")`) needs nothing from `hook_input`. It was
refusing a category it was built never to refuse, and that category is the one
that performs repairs.
So the degraded block now asks the guard's own scope question with the one input
that cannot fail: a DISPATCHED session still refuses (nobody is present to
approve), an INTERACTIVE one is allowed with a loud notice. The security delta
is ZERO and is locked by a test rather than asserted — the healthy guard already
permits exactly that call, so restoring it surrenders no protection that existed.
Bash stays refused, which is the point: fail-closed keeps everything it was
protecting, and the session keeps the edit that repairs the tree.
**A DISPATCHED session remains SYSTEMIC by this same test** — Bash and Write/Edit
both refused, repair path closed — and that is the intended direction rather than
an oversight: an unattended session is not who should be self-repairing a broken
guard tree, and its repair route is a human interactive session, which is exactly
what this change makes usable again.
**The general rule, which is why this sits in the doctrine and not only in a
changelog:** when a fail-closed leg has no predicate, do not ask whether the
refusal is severe — ask which repair routes it closes, and leave one open on
purpose. Two other routes exist here and NEITHER is a design: an MCP editing
tool is matched by neither matcher, and the host Guardian can `REVERT_CODE` from
outside the container, but nothing currently WAKES it, because the container is
up and `genesis-server` never imports `hook_input`. A repair route that survives
by accident is not a repair route; name it and WIRE it.
**PARTIAL failure means a usable predicate survives**, and fail-CLOSED stays
correct there — `run_guard` is the other merged instance (an uncaught crash in
an irreversible-action guard exits 2 instead of the exit 1 CC treats as
non-blocking). What licenses them is MEASURED. Six guards are wired to
`degraded_exit`, and their predicate-gated matchers refuse between **1.90% and
21.47%** of 74,282 unique real commands individually; PR #2042 measures the
UNION of four of them at **28.80%**, which it headlines as "roughly 29% of
ordinary work is refused while the tree is broken".
Read that union as the shape of an operability budget, NOT as its value: the
other two wired guards are outside it, so the real six-guard figure is unmeasured
and strictly higher. A seventh guard does not get to add its rate to 28.80% and
call the result the cost — that arithmetic is the population error this file
keeps catching. Measure the union you actually have. What each protects earns its
share: irreversible for `worktree_cwd_guard`, merely heavy for
`full_suite_guard` — a distinction taken from PR #2042's own table rather than
assumed, and a reminder that "what it protects" is a mixed bag even inside one
wired set.
`background_pipe_guard` is the one that was deliberately NOT wired: its degraded
matcher would refuse **70.30%** of that same corpus, and a guard refusing the
majority of ordinary work does not make the broken state safe — it makes it
unrepairable, which is the brick again by another route.
**Ambiguity about WHICH REGIME you are in resolves toward OPEN-LOUD**, because
that direction's worst case is a recorded mess and the other's is a system
nobody can reach.
**This section decides the REGIME; it does not decide the direction within
one.** Once you have established PARTIAL, the per-boundary mandate under "Never
hand-roll `gh`/bash/CLI argv parsing" and its corollaries (a)/(b)/(c) govern
which way that guard fails — and corollary (c) settles the unattended-session
case in the direction of REFUSING, after three narrowing rounds on one
predicate. Nothing here loosens it. What this section adds is the prior
question those corollaries assume has already been answered: whether a usable
predicate exists at all.
Four rules follow, each cheap:
1. **"Could not evaluate" is its own state.** Never render it as allow, clean,
healthy, or no-finding. A check reports exactly one of four things:
CHECKED-CLEAN, FINDINGS, COULD-NOT-CHECK, OUT-OF-SCOPE. The merge gate
already works this way where it prints an unrecognised reviewer's comment
rather than scoring it silently; `InjectionHealth.errors` does it by emitting
a DEGRADED finding instead of resolving healthy.
2. **Write the degraded marker BEFORE the early return, with ZERO dependency on
the component that failed.** A refusal path that records its reason only
after the check it could not run has recorded nothing. READ, a sibling
toolkit's 2026-09 incident: its blocking hooks refused every command for 30
minutes while its own liveness canary reported OK — the dependency-missing
branch printed its refusal and exited BEFORE the line that writes the
heartbeat. The canary asked "did it receive a payload", never "could it
evaluate one", so a guard refusing everything left exactly the trace a guard
nobody called leaves.
3. **Degrade the RENDERER, never the VERDICT.** When the thing that failed is
how you SPEAK — a serializer, a formatter, a size budget — the decision
itself must still land. `print_json_bounded` is the in-repo instance: an
oversized advisory loses prose, never its `permissionDecision`. **It also
tells you when it could not keep that promise** — it returns False and emits
ANYWAY when the envelope alone exceeds the budget or no named text key was
trimmable, and CC discards an exit-0 hook's stderr, so that return value is
the only signal a caller gets. Check it: "something was trimmed" is not "the
decision landed". Another install's toolkit reaches the identical rule from
the other side, rendering verdicts through a fallback chain after a broken
serializer turned every refusal into empty stdout — which the harness reads
as ALLOW.
4. **The watcher must detect COULD-NOT-EVALUATE, not merely DID-NOT-RUN.** A
liveness check keyed on "is the timer active" or "did a heartbeat arrive"
cannot see a guard that ran, refused everything, and wrote nothing. Ask what
your watcher would report if the guard were evaluating nothing at all; if the
answer is "healthy", it is measuring the wrong thing.
### Common Traps
- **A `gh` listing is ALREADY capped before you pass a flag, and the caps are not
uniform.** MEASURED by reading `--help` on gh 2.98.0 (2026-08-20): `pr list` 30 ·
`issue list` 30 · `run list` **20** · `workflow list` **50** · `gist list` **10** ·
`release list` 30 · `repo list` 30 · `cache list` 30 · every `search` subcommand 30.
So `gh pr list --limit 30` is behaviourally identical to passing nothing, and a
session once reported its own cap back as the repo's open-PR count (said 30, the
real number was 78). The unflagged form is the dangerous one precisely because
nothing in the command hints a cap is in force. `scripts/hooks/capped_read_advisory.py`
now says so pre-flight, and a drift test re-reads `--help` so this table fails loudly
when gh moves a number rather than quietly naming a cap that no longer exists — treat
the numbers above as the reading at that version, not as durable facts. Raising
`--limit` gets you MORE ROWS, and `--paginate` is a `gh api` flag every one of these
subcommands rejects (MEASURED on gh 2.98.0: `unknown flag: --paginate`).
**A SHORT read is NOT proof of completeness, and an earlier version of this very
bullet said it was.** "Re-read until the result comes back short of your limit" is
the intuitive rule and it is false, because GitHub shortens a response on its own
for reasons the command cannot see. MEASURED, three independent ways: a FILTERED
`gh run list` (`--branch`/`--created`/`--event`/`--status`/`--user`) is served by the
workflow-runs endpoint, which returns at most 1,000 results for such a search;
`gh pr list --search` and `gh issue list --search` route through GitHub search and
stop at 1,000 (`gh pr list --help` advertises `-S, --search`); and any `gh search`
whose query TIMED OUT returns fewer rows than asked for with
`incomplete_results: true`. Each is short, and none is complete. `gh search` also
refuses a limit above 1,000 outright (`` `--limit` must be between 1 and 1000 ``),
so you cannot widen past it at all.
So: a SATURATED read supports "at least N" and never "N" — that much still holds.
For an exact count, use a source that reports a TOTAL rather than the length of a
list you asked for, and check that total is not itself partial: the Search API
returns `total_count` alongside `incomplete_results`, and the count is exact only
when that flag is `false`. Date-slicing a query is still the way to get under a
ceiling; just do not treat a short slice as self-certifying.
- **Fail-closed data access.** A data-access boundary must RAISE (or return a
clearly-typed "unknown/unavailable") on missing scope or an unavailable
dependency — it must NEVER silently return the wrong data, the singleton's
data, or an empty result that reads as "all clear". A monitoring/consistency
check whose dependency (Qdrant, FTS, a remote) is down reports `unknown`, never
`healthy`/`degraded` — a dependency outage that masquerades as data
corruption (or as cleanliness) is worse than a loud error. Prefer a helper
that raises over one that swallows (the `batch_retrieve_point_ids`-raises vs
`batch_retrieve_vectors`-swallows split exists for exactly this). Origin: the
home-anchored-DB reads that silently returned no data from an empty worktree
path, and the memory-integrity checker (2026-07).
- **Never replace a runtime the CURRENT session depends on — that is a HANDOFF,
not a repair.** Upgrading, downgrading, reinstalling or removing the
interpreter, Node, the CC binary, or the venv that the ACTIVE session (or its
hooks, MCP servers, or test runner) is running on kills the thing performing
the repair, mid-repair — and the second half of the procedure, the half that
puts the replacement in place, never runs. The safe sequence is side-by-side:
install the replacement ALONGSIDE, verify it works, switch the pointer, and
only then remove the old one; and run the whole procedure from OUTSIDE the
dependent session (a separate shell, a systemd unit, the host Guardian).
READ, a sibling install's 2026-09 incident: a rollback script uninstalled the
running Node before installing the intended version, the uninstall terminated
the agent session executing it, and the machine was left with neither. Before
touching any runtime, inventory what is running on it.
- **Ego sessions are ACTIVE.** `src/genesis/ego/` is live (v3.0a11).
Two egos: user ego (CEO, Opus) and Genesis ego (COO, Sonnet). Both
run on adaptive cadence via the awareness loop. Changes here are
production changes.
- **DB path confusion.** `genesis.db` is at `~/genesis/data/genesis.db`,
NOT `~/genesis/genesis.db`. Use `genesis.env.genesis_db_path()`.
- **Column names.** Use `db_schema` MCP before assuming column names.
The DB has 60+ tables.
- **Signal collectors.** Phase 1 built stubs; Phase 6 replaced some with
real implementations. Code that looks complete may not produce signals.
- **Capabilities manifest.** `~/.genesis/capabilities.json` is write-once
at bootstrap, not dynamic. New capabilities need registration in
`_CAPABILITY_DESCRIPTIONS` in `src/genesis/runtime/_capabilities.py`
AND a bootstrap init step.
- **APScheduler IntervalTrigger resets on restart.** `IntervalTrigger`
counts from server startup, not from last successful run. If the
server restarts more frequently than the interval, the job never
fires. Use `CronTrigger` for anything longer than a few hours.
Bit us with `user_model_evolution` (48h interval, daily restarts).
- **Silent skips are banned (provision-or-surface).** A setup/resilience
feature that gracefully skips on a missing prerequisite (a package, a
host knob) must either PROVISION the prerequisite (bootstrap.sh /
host-setup.sh / a guardian reconciler) or register an effective-fact in
`infra_profile` that the awareness posture check
(`awareness/loop.py::_check_infra_protection_posture`) reads — so an
unprotected box raises a standing alert instead of staying silent. A
graceful skip with neither = a box that runs unprotected with zero
signal (a sibling install ran weeks without swap/systemd-oomd until a
memory spike wedged it, 2026-07). Guardrail:
`tests/test_awareness/test_infra_protection_posture.py`.
- **Modules are NEVER subsystems.** A capability *module*
(`src/genesis/modules/**`, an external pluggable capability — "hands,
not brain", see `modules/base.py`) is not an internal Genesis
*subsystem* (memory, reflection, ego, triage, autonomy, sentinel).
Module memory writes must **never** set a `source_subsystem` value —
that tag means "internal decisional output, exclude from default
recall", which is wrong for module output. This is enforced
mechanically: any `.store()` under `modules/**` passing
`source_subsystem` is a hard CI failure in
`tests/test_memory/test_store_subsystem_coverage.py`, which also forces
every new memory-writer to either tag itself or be explicitly
classified as user-context. `_KNOWN_SUBSYSTEMS`
(`memory/retrieval.py`) is the authoritative subsystem list; adding a
module name to it is a category error.
- **Destructive data migrations must reconcile cross-store mirror fields.**
When a cleanup/backfill deletes data in one store (e.g. Qdrant vectors) but
another store mirrors that data's existence (e.g.
`memory_metadata.embedding_status`), the delete MUST also fix the mirror
field. A deleted vector left as `embedding_status='embedded'` is a field
that *lies*, and that lie is not cosmetic if any code path *reads* it —
`MemoryStore._mark_superseded` gates an `update_payload` on
`embedding_status != 'fts5_only'` and would fire a doomed write on the
now-deleted point. Before assuming a stale field is harmless, grep for its
*reads*, not just its writes. (Bit us in the source_subsystem purge, #918;
fixed by #921 Step 2c — reconcile tagged rows to `fts5_only`.)
- **`immutable=1` reads miss WAL-resident writes.** A read-only `sqlite3`
connection opened with `file:...?immutable=1` reads only the main db file
and ignores the `-wal`, so a change you JUST committed (still
un-checkpointed) is *invisible* — you get a false-negative "the write
didn't land." To verify a live write, use `?mode=ro` (WAL-aware) or query
through the server/CRUD path; reserve `immutable=1` for historical
read-only sampling where a little staleness is fine. (A reconcile UPDATE
read clean under `mode=ro` but appeared unchanged under `immutable=1`.)
- **Never hand-roll `gh`/bash/CLI argv parsing inside a hook or security gate.**
Re-implementing shell/`gh` command-line semantics by hand (regex, manual token
walking) creates an effectively UNBOUNDED adversarial-divergence tail: a good
reviewer (Codex especially) will keep surfacing real gaps from true semantics —
`--repo`/`-R`/`-Rvalue`, `GH_REPO`, `cd`, `&&`, `||`, `--body-file -`, duplicate
flags, URL-vs-branch targets, enterprise hosts, `--help`, nested `bash -c` — and
each named fix ships the next round's bug. This is the mechanism behind the
measured Codex review-loop (an internal analysis of recent review-looping PRs:
first-round findings were real catchable bugs, but later rounds were dominated by
fix-churn on the hand-rolled parser itself — no finite pre-push audit bounds that
tail). The cure is architectural,
not "review harder": bind atomically to the real tool (e.g. `gh pr merge
--match-head-commit`) and use a canonical parser (`shlex`/`bashlex`) — never
bespoke semantics. Choose the fail direction PER BOUNDARY by consequence: the
shared parser must DEGRADE gracefully (fail-open, never crash — that is
`shell_parse.py`'s stated contract), while each security-critical caller
(merge/push authorization) treats an unparseable command as a block (fail-closed
THERE). This bullet and its corollaries decide the DIRECTION for one guard's
degraded path, and they assume a usable predicate still exists. "Guard failure
semantics" above decides the prior question — whether one does — and when none
does (wrapper, interpreter or venv dead, or one dependency taking every guard
at once) its repair-path test governs instead of this bullet. A parser-wide absolute fail-closed is wrong — it would deny legitimate
uncommon commands without closing evasion paths. Same family as the
canonical-parser lesson (regex→yaml, #1393). Loci today:
`scripts/hooks/shell_parse.py` + `scripts/hooks/git_push_guard.py`.
**The boundary of the class — read this BEFORE you reason yourself out of it.**
The tar pit is NOT "who tokenizes the string." Delegating tokenization to
`shell_parse` and asking git for repo state does NOT exempt a guard: if the
guard's CORRECTNESS depends on modeling what a git command WILL DO — which
flags force, which operands are paths vs refs, which modes destroy, which
repo is targeted — it is argv→EFFECT mapping, and that mapping is the same
unbounded open-set surface as raw string parsing. This was reasoned around
once already (2026-08-23, PR #1432): the guard used the canonical tokenizer
and probed live git state, the author concluded "so it's not hand-rolling,"
and Codex returned **13 real findings (10 P1) — every one of them living in
the argv→effect layer**. The first architect finding of that shape (a
separated global value-flag bypass) was the CLASS signal and got
instance-patched; the next round found the rest of the class. n=1 IS the
signal: any reviewer finding that exposes a semantic-modeling gap in a guard
means STOP and re-architect — never patch the named instance.
**Decision test (verbatim, apply before shipping any guard):** could a git
flag you've never heard of change your guard's verdict? If yes, your claim
is open-set — redesign to closed-set token claims (exact-form whitelists /
literal token blocks) or to RECOVERABILITY (snapshot-then-allow, where a
miss degrades to the status quo instead of a broken guarantee). Do not ship
the open-set version and plan to harden it later; the review loop IS the
hardening loop, one bug per round, and it does not converge.
**An instrument's stated invariant is its blind spot — and so is the one it
does not state.** Before trusting a harness's clean result, read what it
promises to hold fixed, then ask what ELSE it holds fixed without saying so.
READ, and EXTERNAL to this repo — a sibling toolkit's own release notes,
2026-09, not a Genesis measurement: a fuzzer whose mutators were all fixed
one-character strings had a whole mutant shape unreachable by construction,
and that writeup attributes its own long-lived bypass to exactly that. The
remedy generalises even if the number does not: vary what the instrument
never varies, rather than running it more times.
Method for the whole class: `references/high-stakes-verification.md`.
**Three corollaries, each bought with a non-converging review loop.**
**(a) NEVER normalize the command text before a blind-spot probe.** A probe
whose whole job is "notice that this text is unparseable" must read the RAW
command. Preprocessing it can only ever DELETE the evidence the probe exists
to find. MEASURED: a normalization step added in good faith — to stop a class
of ordinary command from prompting — turned that shape into a silent
ALLOW on two independent guards. Ordinary, not adversarial: the shape was one
a developer writes without thinking, and the command really executed (verified
against a shimmed binary, so the proof was execution rather than parse). The
normalizer removed the very evidence the guard keyed on, and its own model of
the shell's comment syntax was narrower than the shell's, so it could delete
executed code as well. It then turned out not to be load-bearing at all: every
case cited to justify it was one where the parser already resolved the
operation, so its branch never ran. The fix was a DELETION, and it closed both
defects at once. When a guard loop will not converge, look for the component
that MODELS shell semantics and remove it — refining it is the loop.
Scope this to a probe. Normalizing before a *tokenizer whose tokens you are
about to use* is a different act with a different fail direction, and a
sibling guard does exactly that, deliberately. Before calling any such site an
instance of this rule, the question to answer is whether the normalization
CAN change the tokenizability verdict or hide a target — and that is a
possibility question, so a count cannot answer it. Demand a STRUCTURAL
argument. The model is already in the repo, at
`scripts/hooks/destructive_command_guard.py:101-120`: its replacements delete a
line continuation — which is what the shell does with it — or insert
whitespace and separators; none introduces a quote or an escape, so none can
corrupt the quote balance shlex decides on. (The one place the shell keeps a
backslash-newline literally, inside single quotes, is an over-block the guard
states rather than hides.) That is a `cannot` — but read the next paragraph
before reusing it, because the version of this argument that stood here for
weeks was WRONG in a way that shipped a live bypass.
**What that earlier version got wrong, and why it is the sharpest example on
this page.** It added: *"a backslash-newline is a continuation, not an escape
the shell keeps."* That is true only when the backslash is itself unescaped —
an ODD-length run. In an EVEN-length run every backslash is escaped by its
neighbour, so the last one is a literal character and the newline after it is
a REAL command separator. The guard folded it anyway, deleting the separator
and gluing the next command's first word onto the previous token, so no `rm`
token existed and a destructive command was ALLOWED — measured end-to-end
through the live hook, exit 0 where the plain-newline control gave exit 2. The
legacy regex net did not save it either: that fires only when tokenizing
FAILED, and this tokenized fine, just wrongly. Fixed 2026-09-02 by folding
only odd-length runs.
Note precisely what was and was not at fault, because the first attempt at
this correction got it wrong in an instructive way: it said the quote-balance
reasoning "was CORRECT and is not what broke", and exonerated it. Quote
corruption is indeed not the *mechanism* of the token-glue bypass — but the
quote-balance `cannot` **was not sound either, and it failed in the very same
cell**. Deleting one backslash from an even-length run leaves an ODD run whose
survivor escapes the next character; when that character is a quote, the fold
DOES introduce an escape and shlex's balance shifts. Measured: it turned
parseable commands UNPARSEABLE (11 of 20,000 random parseable commands under
the old fold, 0 under the fix), which dropped them into the legacy-regex net —
a net that matches neither `-Rf` nor `-r -f` nor `--recursive --force` nor a
quoted `'rm'`, so they failed OPEN. One quiet universal quantifier (*every*
backslash-newline is a continuation) falsified BOTH claims at once, and the
old code therefore had two bypass families rather than one. The fold now
always leaves an even-length run, which is what finally makes the `cannot`
true.
A structural `cannot` is only as good as the case-split it rests on, so state
the split explicitly and enumerate it — the same trap as the direction-claim
two paragraphs down, which was true of the operand and false of the option
token. That both the wrong premise AND the first correction of it were written
in a document teaching this exact discipline is the point: the danger is not
knowing the rule, it is believing you already applied it.
A corpus run only ever yields `did not, here`, and by the rule above it is
structurally blind to the shape nobody typed. Run the corpus as
corroboration, never as the proof, and pair it with a control that DOES flip
— an unflipped corpus and an inert measurement look identical.
State the direction too, not just a total — and then check the direction on
the token you did not think of. The same sibling's fold used to split a word
the shell joins, and the paragraph that stood here said the verdict could
therefore only move toward refusing. That was measured on path operands and
was false on the option token: the split could hide the flags, and a spelling
the shell runs as a recursive-force removal of a protected path was allowed.
A reviewer found it from the diff; the earlier audit had split every position
of a path and never the option, and its zero came from benign traffic with no
true-positive control. The fold now deletes the sequence, which is what the
shell does, and the guard is asserted to give the same answer for the
continued and the joined spelling. The general lesson is the one above: the
examples you enumerate are a sample, and a direction claim needs the cell that
would falsify it.
Deliberately stated without the triggering shapes. A guard's defeat
conditions are not a teaching aid, and this file is public.
**(b) When the parse is unreliable, ASK — do not BLOCK.** A hard block forces
a surgically precise trigger, and precision is exactly what an unreliable
parse cannot deliver. Measured over five review rounds: every narrowing
conjunct became a new way to STARVE the trigger (an over-strip that ate the
evidence; a decoy segment that stood the net down), while every widening
hard-blocked benign shapes (`git status # don't commit yet` was refused).
Emitting a PreToolUse `ask` inverts the cost of being wrong — a false
positive is one confirmation, a miss is the pre-existing status quo — which
is what lets the trigger stay broad instead of clever. Measured prompt rate
after widening: 0.43% of ~19k real commands.
**SUPERSEDED for the git-operation net by owner ruling 2026-09-08 (re-checked
against the code 2026-09-27).** That net no longer asks in any session type:
the commit gate and the push guard REFUSE an unreadable command that names a
gated operation, with a message giving the cause and a rewrite the session can
perform — both cite the ruling at their blind branch. The reasoning here still
prices the TRIGGER (a broad predicate remains right); what changed is the cost
of a false positive, now an agent rewrite rather than a human confirmation. So
wherever this passage and the paragraphs below say `ask` for that net, read
"refuse with an actionable message". A line continuation joined the same path
when the parser began REPORTING it as a blind spot (`_BLIND_CONTINUATION`)
instead of modelling the join: refused, never parsed around.
This does NOT loosen the fail-closed mandate above, and the two are easy to
read as contradicting each other. Rule (b) is scoped to the git-operation
blind-spot net — a guard whose trigger is deliberately broad and whose false
positive is one confirmation. It is NOT a template for every guard: where the
false negative is an irreplaceable path or a broad recursive removal and the
false positive is a rewrite, that asymmetry justifies refusing outright rather
than asking. Neither the protected-paths nor the destructive-command guard has
an `ask` branch at all — both return only 0 or 2 — so a person's presence
genuinely cannot change their verdict.
One clause of that used to read "hard-block an unreliable parse", and it is
FALSE. MEASURED 2026-09-06 through both live hook entries, controls flipping:
each guard refuses on the PARSED path and then falls back to a DIFFERENT
matcher, neither a subset nor a superset of it — it misses spellings the
parser catches, AND it drops the target tests the parser applies, so it also
refuses commands the parser allows. **A guard's fail direction is a property
of its DEGRADED path, and that path has to be read in BOTH directions.** The
two differ in what their fallback is for: `destructive_command_guard`'s is
fail-open by design and says so in its own docstring, so argue with the design
rather than patching it; `protected_paths_guard`'s calls itself "conservative:
over-blocks, never under", which is true relative to the old guard it
reinstates (`_legacy_substring_block`'s docstring) and false of the parsed
resolver it stands in for — one sentence, two readings, and that ambiguity is
the defect. Verify a docstring's stated direction IN CONTEXT; never quote it as
a fact. The shapes are deliberately not enumerated here, per the rule above.
That guard's MODULE docstring has since been split so it no longer states one
fail direction for both blind spots: a bounds-induced blind spot REFUSES and an
untokenizable one falls back. The sentence quoted above still sits on the
fallback helper, where both readings remain available, so the lesson is
unchanged — and note the citation here is now to a SYMBOL rather than to a line
range, because the range this paragraph originally named stopped containing the
quote the moment that docstring was edited. A line number is a claim with a
shelf life.
Within the git-operation blind-spot net, the two rules are scoped by who is
present: `ask` is the interactive form of the refusal, and where a session is
unattended fail-closed governs and (c) applies. The operation proceeds
unverified in neither case.
This is the shipped shape now, not an aspiration, and one distinction inside
it must stay visible. The shared parser still degrades to a naive split with
NO failure signal, so a caller that only asks "did I get a matching segment?"
allows. What closes the hole is a separate conjunction AT THE CALLER — no
matching segment, AND the raw text is un-tokenizable, AND it names a gated
operation — which yields `ask` interactively and a refusal when unattended.
The parser's contract did not change and must not: it degrades, the caller
chooses the fail direction, exactly as the mandate above requires.
Earlier revisions of this passage described that net in the present tense
while it was still unmerged. Both directions of that error are worth naming,
because fixing one produces the other: an unbuilt mechanism written as
shipped, and then — once it does ship — a hedge left standing that now
understates the tree. A status sentence in a durable document is a claim with
a date on it. Re-check it against the code whenever the surrounding work
lands, not only when it is first written.
Corollaries of the corollaries, each measured: a net that returns inline
PRE-EMPTS every gate below it (a hard block with no other backstop was
observed downgrading to a prompt) — set a reason and DEFER it to the tail
where the other decisions are resolved. And an invariant pair of the form
"never silently allowed" + "never hard blocked" is satisfied BY a block→ask
downgrade, so pin the verdict EXACTLY wherever a hard block is the contract.
Read "never hard blocked" here only as the shape of the trap — as an actual
invariant it is false unscoped, and (c) below replaces it.
**(c) The ask-cost argument does NOT survive the move to a refusal.** Rule (b)
buys its broad trigger with "a false positive costs one confirmation" — and
that is true only where someone can confirm. An unattended session has nobody
to answer, so the obvious completion of (b) is a deny leg for that path, and
that is where it goes wrong: the SAME broad predicate whose errors were cheap
now produces unappealable refusals of ordinary work. MEASURED: sharing one
predicate across both legs refused routine, entirely benign commands in
unattended sessions, in the one failure direction the design had been chosen
to avoid.
Narrowing the refusal predicate is the obvious repair and it does not
converge, for a reason worth stating precisely, because the imprecise version
of it is false. The claim is NOT that no raw-text rule could ever work — the
raw text does carry quote and comment syntax, and a complete canonical parser
could read it, which is what the canonical-parser rule above tells you to
reach for. The claim is bounded to a predicate built on the SAME degraded
parse that failed: at that point the guard cannot say whether an occurrence of
a gated verb is executed or merely quoted, commented, or documented, and that
inability is the premise of the net existing. A predicate with no more
information than the failure itself cannot both refuse the hidden operation
and permit the inert mention. Escaping that needs a different information
source — a fuller parser, or enforcement at the execution boundary, where
mentions are never classified at all — not a cleverer rule over the same text.
Three narrowing rounds on one predicate is the signature to STOP and make the
policy decision explicitly. The decision taken on the guard this was learned
from, and since shipped: the unattended path KEEPS refusing,
and the invariant gets scoped rather than deleted. "A benign shape is never
hard blocked" was simply false as written; what is true and testable is that
it is never hard blocked where a human can approve, and where no one can, the
refusal carries an ACTIONABLE stderr — the cause, plus a route that gives the
gate MORE information rather than less.
That last qualifier is load-bearing and the sloppy version of this sentence is
a bypass instruction. Telling an operator to find "a rephrasing that avoids
the predicate" invites mutating raw text until a degraded predicate stops
matching, while the same unverified operation still runs — the fail-closed
mandate defeated by its own error message. Only two routes are legitimate, and
neither is an evasion. If the text is PROSE that merely mentions a gated verb,
take it out of a shell command altogether — write the file with an editor tool
— because it was never an operation to gate and never should have been parsed
as one. If it IS the operation, express it so the parser can actually read it,
which does not dodge the gate but submits to it. A rewrite that suppresses the
trigger while still performing the operation is the one thing such a message
must never suggest, and a message is not "actionable" if that is what it
teaches. (Actionability here is a courtesy to the operator, NOT the
RECOVERABILITY of the Decision test above, which is about a MISS degrading to
the status quo. Same word, opposite failure direction; do not satisfy the
Decision test by printing a nicer error.) State the narrower invariant; do not
leave the false one standing, and do not delete the guarantee that still holds.
Two things keep this from reading as licence. The open-set imprecision is
tolerable here ONLY because the verdict is `ask` or `deny` and never `allow` —
an imprecise predicate that cannot authorize anything does not violate the
Decision test, while the same predicate wired to an allow would. And a broad
regex over raw text is admissible ONLY as a mention scan whose outcome is an
`ask` where a person is present and a refusal where none is — never an allow;
the moment it maps argv to an effect and authorizes on the result, it is the
hand-rolled-parser tar pit the mandate above forbids.
A measurement informed this rather than settling it, and it was afterwards
WITHDRAWN — which is the more useful half of the story. The claim was that
across a sample of unattended sessions the leg had never fired in either
direction. It does not survive: the transcripts it counted no longer existed
when someone went to re-derive it, and the sample was far too small to carry
a word as strong as "never" even while they did. Absence in a small sample is
the expected observation for a rare event, not evidence the event cannot
happen; the honest reading is that the leg's rate is LOW, which is a different
claim and a weaker one. What was actually being chosen was which promise the
suite should make, not which incident to prevent — and that conclusion never
rested on the number, which is why it outlived it.
A note on every number in these three rules, including the ones above. They
come from one operator's local session transcripts at one date, and no corpus
or harness is checked in, so a reader cannot reproduce or falsify them — the
differing denominators are different harvests, not one corpus quoted three
ways. That is not a hypothetical weakness: one of them was withdrawn the
first time anyone tried, for exactly that reason. Treat them as the scale at
which something was observed, never as a published result. A rule that only
holds at someone else's numbers is not a rule; each of these should stand on
its stated mechanism alone — and if one ever seems to DEPEND on a figure,
that dependency is the defect to fix, not the figure to defend.
- **`out=$(cmd)` under `set -e` swallows the failure path — and NO linter catches
it.** An assignment whose value is a command substitution INHERITS that
substitution's exit status, so under `set -euo pipefail` a bare
`out=$(cmd)` followed by `rc=$?` **never reaches the `rc=$?`** when `cmd`
fails: errexit fires first. Any error handling keyed on `rc` is dead code on
exactly the path it was written for. Two properties make this vicious: it is
invisible (the function just stops, printing nothing), and it hides behind
call sites — `f || echo …` / `if ! f` disable errexit INSIDE the function, so
the bug stays latent until someone writes the first bare call.
**shellcheck 0.9.0 does not flag it at any severity, including `-o all`**
(SC2155 is the *different* `local x=$(cmd)` declare-and-assign case; measured
2026-08-27 — no other linter was tested, and a future shellcheck could add it).
Always write `rc=0; out=$(cmd) || rc=$?` — but **declare `local` on its own
line first**: `local out=$(cmd) || rc=$?` NEVER fires, because `local` is a
command and the compound takes `local`'s status (0), not the substitution's.
MEASURED: the split form yields `rc=100`, the inline form yields `rc=0` and the
caller proceeds as if the command succeeded — strictly WORSE than the bug this
entry describes, since it converts a loud abort into a silent false success.
The same applies inside the error handler: with `set -o pipefail`, `x=$(… | grep -v … | tail -1)` aborts when
`grep` matches nothing, so a `${x:-fallback}` default written for that very
case never runs — guard it with `|| x=""`.
Origin: `scripts/lib/cc_version.sh` (caught only by adversarial review), then
three instances found in one `scripts/bootstrap.sh` function whose whole
diagnostic block was unreachable. Pinned by
`tests/test_scripts/test_bootstrap_guards.py::test_install_pkg_*`.
### Iterative-Refinement Discipline
AI refinement cycles degrade code they were asked to "improve" — validation
gets stripped, types relaxed, function scope widened. Published measurements
show vague improvement prompts degrade security fastest across iterations.
Three binding rules:
1. **Iterate with scoped, explicit prompts** ("fix the race in X by
serializing on Y"), never "improve/clean up/make robust".
2. **Be security-explicit when touching validation, auth, or boundaries** —
state what must not be weakened.
3. **Diff each refinement for what it REMOVED** (constraints, guards, type
enforcement), not just what it added.
Full failure-mode taxonomy + ordered audit passes: `references/ai-code-audit.md`.
### Anti-Rationalization
These are excuses sessions use to skip discipline. If you catch yourself
thinking any of these, STOP — you are rationalizing a shortcut.
| Rationalization | Why it's wrong |
|---|---|
| "This is just a simple fix, no tests needed" | Simple fixes break complex systems. The Qdrant regression was a "simple fix." Write the test. |
| "I already know what this function does" | You haven't read the implementation. Docstrings lie. Read the actual code. |
| "Tests pass, so we're done" | Tests verify what they cover, not the outcome. Verify actual end-to-end behavior. |
| "I'll clean this up in the next commit" | Next commit never comes in autonomous sessions. Do it now, or file it — Genesis-repo work is a GitHub issue, not a local row. |
| "This file is too large to read fully" | Read the relevant section. Partial reads lead to partial understanding and wrong fixes. |
| "The linter is happy, ship it" | Linters catch syntax, not logic. Clean lint with broken behavior is worse than a warning with correct behavior. |
| "This change is low-risk, no impact analysis needed" | Your confidence is based on what you know; checking callers reveals what you don't. Serena `find_referencing_symbols` is live — run it. For multi-hop blast radius from the main checkout, run `scripts/lib/code_intel_index.sh "$PWD" gitnexus fast`, then `impact`; in a linked worktree, use Serena for live branch truth. |
| "I can skip the worktree, I'll be quick" | Concurrent session safety exists because "quick" commits have destroyed work before. Always worktree. |
| "The error is transient, retry will fix it" | Diagnose first. Retrying a misdiagnosed error wastes tokens and masks root causes. |
| "I'll add the follow-up later" | Records not created in-session are lost. File it now while context is fresh — Genesis-repo work as a GitHub issue, user-owned work as a follow-up. |
| "I don't need a skill for this" | If a skill exists, use it. The using-superpowers Red Flags table exists for this exact rationalization. |
| "This review round is the same class, it doesn't really count" | For an EXTERNAL cross-model round, the counter decides, not you — a repeat-class external round still counts; update it every external cycle and STOP at the cap. (Internal same-model reviews are never rounds.) |
| "The user already said proceed, so I can keep looping" | The escalation/fix-attempt caps CONSUME standing approval. Round 4+ (or fix #4) on an old instruction is a violation, not obedience. |
| "I can read the summary instead of the source" | Summaries lose context. If you're about to change code, read the code, not the description of it. |
| "The missing data was the problem — I wrote it, so it's fixed" | The mechanism that failed to write it is the problem. Hand-written artifacts are data repair, not a fix (see Instance-Fix vs Class-Fix Gate). |
| "I'll just add the field the reviewer flagged" | A spec's required-set is closed — derive and enforce ALL of it at once, with one test locking the whole set, or the next round finds the next missing field (see Debugging Discipline: spec required-sets). |
### Code Discovery
Use the right tool for how you're exploring:
- **Architecture overview** — CBM `get_architecture(aspects=["overview"])`
- **Finding symbols** — CBM `search_graph(name_pattern="...")` or Serena `find_symbol`
- **Call tracing** — CBM `trace_path(function_name="...")` or Serena `find_referencing_symbols`
- **Impact / blast radius** — Serena `find_referencing_symbols` (live caller set); GitNexus `impact` (reindex first) for multi-hop + affected processes
- **Config/doc/non-code files** — Grep/Read directly
Full decision matrix: `.claude/docs/code-intelligence.md`
### Auditing Existing Capabilities — enumerate, don't spot-check
Before claiming Genesis "lacks X", "needs to add X", or is "weaker than
<external system> at X" — or before any competitive/architecture comparison —
verify by ENUMERATION, not a spot-check. **Auditing a symbol is not auditing the
stack**, and a negative from a positive search is not evidence of absence:
1. Enumerate the subsystem's full module inventory before concluding anything is absent.
2. Trace the call graph BOTH directions — mechanisms often live in the
wrapper/caller layer, not the first symbol (CRAG lives in the MCP recall
wrapper, not `retrieval.py`; the reranker is applied by the caller).
3. Grep by CONCEPT with several synonyms, not one symbol.
4. Verify built/enabled/disabled against RUNTIME state (env gates, server logs),
not code presence.
5. Multi-path systems → coverage matrix (N entry points × M mechanisms); hot
auto-fired paths often carry a thinner stack than the deep path — a gradient,
not an absence.
6. Confidence is capped by enumeration completeness.
A 2026-06-30 competitive audit wrongly claimed Genesis lacked CRAG,
scope-before-rank, and a live reranker — all three had already shipped. Full
protocol: procedure `codebase_audit` / CC memory `audit-enumerate-not-spotcheck`.
For "does Genesis already have X", consult the subsystem map
(`docs/architecture/CURRENT.md`, via the `subsystem-map` skill) FIRST;
`references/codebase-map.md` stays the package-level structural companion.
## Adaptive Review Protocol
**Review rides the commit, because the commit is the only mechanical
trigger.** "After each meaningful chunk" is unenforceable — nobody can gate a
vibe — but this repo commits continuously, so the commit IS the chunk, and the
commit gate already refuses unreviewed code commits in every session type
(hooks are session-agnostic; a dispatched builder hits the same gate an
interactive one does). What this paragraph adds is the named DEFAULT below the
substantial line: for an ordinary code commit, run the built-in `/code-review`
at **low** effort and let its findings feed the evidence you mark. Cheap,
catches drift while the fix is one edit instead of a review round, and it is
NOT a clearance — the marker flow is unchanged. (The gate auto-allows a
docs/config-only commit; any code change goes through the marker flow, and the
gate itself sets the depth — an ordinary edit takes a plain marker, a
substantial or prompt-surface one takes the adversarial pass.)
Every defect that
survives to the end-of-build review costs an adversarial cycle there, and
every one that survives THAT costs an external Codex round against the
escalation cap.
**For a SUBSTANTIAL change, the end-of-build adversarial review is canonical,
not the builder's choice: run `/deep-review`.** Judge substantial at end of
build over the WHOLE branch — the `merge-base(HEAD, origin/main)..working-tree`
range `/deep-review` and the CI check examine — not the last staged commit: the
commit gate classifies each commit's own staged diff, so continuous commits
leave the index empty and a branch split into individually-small commits would
otherwise slip the bar it clears in aggregate. The threshold VALUES are the
commit gate's — ≥50 reviewable lines, more than one code file, or any auth /
API / migration / prompt / agent / skill surface (the prompt-surface trigger
excludes the user-sovereign top-level CAPS docs, CLAUDE.md / SOUL.md / USER.md)
— applied here to the branch range rather than one staged commit. `/deep-review`
dispatches the adversarial pass in the required shape (genesis-architect, plus
genesis-security-reviewer when the diff touches a security surface) and writes
the evidence marker the gate reads; it is the end-of-build terminus, riding on
TOP of the during-build `/code-review` inline passes — which is exactly the
table's "Code-reviewer inline + `/deep-review`" for a substantial row, the two
playing different positions rather than a choice between them. Below
substantial, the table governs: a small focused fix ends with code-reviewer
inline. `/audit-changes` stays what it is — a light self-check that writes no
marker — and is a terminus only where the gate needs no marker at all: a
docs/config-only commit it auto-allows.
Choose the review level proportional to the change:
| Change type | Review level | Examples |
|---|---|---|
| Docs / text / comments | **None** | Markdown prose, inline comments |
| Simple mechanical | **None** | Variable rename, typo fix, import reorder |
| Small focused fix | **Code-reviewer agent inline** | Single-function bug fix, config tweak |
| Substantial change | **Code-reviewer inline + `/deep-review`** | Multi-file refactor, new MCP tool, wiring |
| Prompt / LLM behavior | **Both + extra scrutiny** | System prompts, skill instructions, routing |
Decision criteria when ambiguous: "If the change could break a runtime
path not covered by its own unit test, it needs `/deep-review`. If it only
touches things with clear, isolated test coverage, code-reviewer inline
is sufficient."
**`/review` is not in this repo — it comes from the optional `superpowers`
plugin.** Where that plugin is installed, `/review` and
`superpowers:code-reviewer` are the preferred path. Where it is not, those names
do not resolve, and older text saying "Run /review" sends you after nothing —
which is why the enforcement hooks now name the plugin as optional rather than
assuming it. **Check, don't assume, in either direction**: the plugin is offered
by the official marketplace, so its presence is per-install, not per-repo.
Always available, plugin or no: **`/deep-review`** (dispatches the adversarial
pass AND writes the evidence marker), the built-in `/code-review`, and the
by-hand `scripts/review_state.py evidence-path` → `mark` flow.
`/audit-changes` is a light self-check, not a substitute for any of them.
The enforcement hooks (`review_enforcement_prompt.py`,
`review_enforcement_commit.py`) still fire on every change — they are
safety nets, not the decision-maker. This protocol provides the
judgment framework.
### Review depth is machine-checked (the front-stop)
The "substantial → adversarial /review-level audit" decision above is no longer
left to judgment alone (that judgment is exactly what failed on PR #1353 — an
under-depth inline pass was self-certified as sufficient). The commit gate now
COMPUTES substantiality from the staged diff
(`review_scope.classify_change_substantiality`) and BLOCKS a substantial change
whose review marker is not an ADVERSARIAL audit (`review_enforcement_commit.py`
Rule 2.5). Substantiality is a surface-area × risk model — ≥50 reviewable lines OR
>1 code file OR an auth/api/migrations file OR an **executable prompt/agent/skill
surface** — and that last set is WIDER than it used to say here: read
`review_enforcement_commit._PROMPT_SURFACE_PREFIXES` + `_is_prompt_surface` rather
than a copied list. It covers `.claude/agents|commands|skills/*`,
`src/genesis/skills/*`, **`src/genesis/identity/`**, and **any**
`src/genesis/**/prompts/` path — so editing `src/genesis/identity/CODE_AUDITOR.md` or an
executor prompt forces `substantial` even at one line, which the old list denied.
So the "Prompt / LLM behavior → both + extra scrutiny" row above is machine-enforced (a
trivial edit to one is depth-audited). User-sovereign top-level CAPS docs
(`SOUL.md`/`USER.md`/`CLAUDE.md`) are exempt. Clearance binds the marker to the
reviewed diff's FULL content, so re-staging different content after the audit
re-blocks. A precision-filtered "no findings" inline pass is FALSE CONFIDENCE for a
substantial change — not clearance. Depth is override-exempt: a findings
`# review-override` does NOT waive it; only a loud, announced-on-stderr `# depth-ack`
does (announced, not RECORDED — nothing persists it, unlike the five PR-merge sigils
below, whose rows survive the session; the word "logged" now means a durable row) (the
audited escape for a genuine format mismatch). "Adversarial" is verified
STRUCTURALLY, and the recognised vocabulary is NOT what an earlier version of this
line said. `review_state._LADDER_LABEL_RE` accepts
**BLOCKER · SHOULD-FIX · CRITICAL · HIGH · MEDIUM · LOW · P1/P2/P3** — **`NOTE` is
not in it, and `MEDIUM` is** (the old text had that exactly backwards). A second
pattern, `_LADDER_PHRASE_RE`, also accepts a `"severity":` key or the phrase
`scope check`, which is why a `genesis-architect` audit clears the LADDER test even
when every finding is NOTE-level: it emits a `Scope Check:` block. That is ONE of
THREE tests, all required — `_evidence_is_adversarial` is a conjunction, so the
gate also demands `file:line` engagement (or the CODE_AUDITOR JSON's `"file"` +
`"line"` pair) and at least `_MIN_EVIDENCE_CHARS` of substance. A scope-check
header alone does NOT pass. The trap is a HAND-WRITTEN audit that is all
NOTE-level and carries neither marker — it fails on the ladder test before the
other two are reached. So run the real recall-tuned audit (`genesis-architect` /
`CODE_AUDITOR.md`); do not hand-write a soft prompt.
**Honest enforcement model — VERIFY IT, do not assume it.** This section used to
claim the enforcing teeth were "the independent cloud reviewer + a required human
approval, gated by **branch protection**", and that the local hook was merely
advisory friction. **Measured on a live deploy (2026-08-27), that was false in
both halves**, and the correction matters because it inverts which layer you can
actually rely on:
- `GET /repos/{owner}/{repo}/branches/main/protection` → **404, not protected**.
The protection the claim rested on did not exist. A *ruleset* did — a different
API, invisible to the branch-protection endpoint.
- That ruleset required **one status check** (`test`), not the other CI jobs that
blocked AT THE TIME. This whole bullet is a dated measurement, not current
state; see the CI paragraph below. Lint, leak-detector and the rest were never
server-side required.
- It carried `bypass_actors: [{actor_type: RepositoryRole, actor_id: 5 (admin),
bypass_mode: always}]`. **A bypass entry voids the rule for that actor.** The
sole author is the repo admin, and the merge command this skill mandates
carries `--admin`.
⇒ For the actor who merges, the server-side backstop was **void**, and the local
`git_push_guard.py` merge gate was the *only* real enforcement — precisely the
layer the old text told you to discount.
**⇒ THAT MEASUREMENT IS NOW HISTORY, and this is what the rules below are for.**
Since PR #1907 (applied 2026-09-10) the rules are SPLIT into two rulesets, and
the split is exactly the property the 2026-08-27 measurement found missing:
`.github/rulesets/checks.json` carries `"bypass_actors": []` and requires `test`,
`leak-detector` and `lint`, so **those three now bind an `--admin` merge
server-side** — GitHub reports `current_user_can_bypass: "never"` for the owner
on that ruleset. `approvals.json` keeps the admin bypass deliberately, because a
sole maintainer cannot approve their own pull request; it carries `pull_request`,
`update` and `creation` only. Rationale and the accepted residual:
`.github/rulesets/README.md`.
Kept rather than rewritten, because the reasoning is the lesson and the state is
the perishable part. Do not read the paragraph above as current — read it as what
you find when you finally query, and note that this correction itself has a date
on it.
**Two rules follow.**
1. **Never state where the teeth are without querying it.** Both endpoints, every
time: `branches/main/protection` AND `rulesets` (then the ruleset by id, since
the list view omits `bypass_actors` and `rules`). A rule with a matching bypass
actor is decoration for that actor. A "required" check list is meaningless
until you have compared it against the jobs that actually block.
2. **Do not weaken a local gate on the theory that something external catches it.**
That reasoning is only as good as the configuration you just verified, and
configuration drifts silently — nothing announces a ruleset edit.
The local gate's job is still to interrupt autopilot, and it is still editable by
the same author. But "editable in principle" is not "backed up in practice."
Where a real external backstop exists, name it and cite the query that proved it.
**Close every review with a verdict.** End each review with an explicit
`Ready to merge: Yes | No | With fixes` + a one-line reason, alongside the
DONE / DONE_WITH_CONCERNS completion status. And the DON'Ts that keep a review
honest: never "looks good" without reading the code; never a finding on code you
did not read; never vague ("improve error handling") — always `file:line` + why it
matters. (Deliberately NOT a "praise-first / acknowledge strengths" balance: an
adversarial audit's job is to assume bugs and enumerate the class, not to reassure.)
Three protocol steps apply to every review at "Code-reviewer inline" level or
above (full definitions in `.claude/agents/genesis-architect.md`):
- **Scope-drift check first**: compare stated intent (plan file / PR
description / commit messages) against `git diff --stat` vs the merge-base,
and open the review with the `Scope Check: CLEAN / DRIFT DETECTED /
REQUIREMENTS MISSING` + Intent/Delivered block. Informational, never
blocking.
- **Premise check second** (Step 0.6, method in `.claude/docs/premise-check.md`):
before reviewing the code, verdict each claim the change DEPENDS on
independently — with evidence, a confidence, and a falsifier — ask what the
caller does differently because of its output, and say whether a better shape
exists (an existing chokepoint it re-implements, a simpler mechanism, a place
the problem disappears). Emit the `Design-premise:` block. Also informational,
and its BROKEN verdict has a HIGH bar: it routes to the EXISTING
premise-wrong disposition (architecture conversation, or
`needs-architecture-session` + draft + a `ready` row) rather than to another round, so everything short of "the change cannot do what it
says" is SOUND-BUT-INFERIOR with the better shape named. Render a
better-shape finding on the severity ladder too (normally SHOULD-FIX), or it
is invisible to every surface that scores findings.
- **Completion status last**: every review (and every skill workflow that
concludes work) ends with exactly one of DONE / DONE_WITH_CONCERNS /
BLOCKED / NEEDS_CONTEXT — with concerns listed, or blocker + what was
tried, or exactly what context is missing. Findings use the
BLOCKER / SHOULD-FIX / NOTE severity ladder with per-finding confidence
and the pre-emit quote gate (a finding must quote its motivating
file:line or be confidence-capped).
### Review-loop discipline
- **A RELEASE review is scoped to what the release CHANGES; a whole-codebase
audit gates nothing.** Point a reviewer at "the repository" and it returns
findings that PRE-DATE the release — real, worth fixing, and not release
blockers. Treating them as blockers is how a release never ships. Scope the
gating pass to the diff between the previous RELEASE tag and HEAD (this repo
also carries non-release tags, so `git describe --tags` alone can land on the
wrong one); run the broader
audit separately, on its own clock, where its findings become ordinary work
rather than a stop. The distinction is not "which findings are real" but
"which findings this release is answerable for" — a defect that already
shipped in the last release is answerable by the next fix, not by this gate.
Release scope decides what BLOCKS, never whether an external round COUNTS: a
pre-dating finding is still a defect-bearing round on the escalation counter,
filed as ordinary work rather than held as a release blocker. "It pre-dates
the release" is otherwise just a new spelling of the rationalization that
counter exists to kill.
- **A pre-existing defect the change RECRUITS FOR is IN-arc — FOR A RELEASE
REVIEW.** The bullet above splits findings into "this release's" and
"pre-dating it", and there is a third bucket between them that neither
catches: a defect that already existed and which THIS change newly leads
people INTO — because it adds the documentation that sends them down that
path, flips the default that reaches it, or ships the feature whose obvious
next step lands on it. It is not in the diff, and the RELEASE is still
answerable for it, because the release is what made it reachable. Ask of every
pre-existing finding: *did we just build the road to it?* If yes, it gates
**the release**. (Adopted from a sibling toolkit's release-review protocol,
which carries a dated case of exactly this shape: a latent defect in an
install path that was harmless until a release's own docs began recommending
that path.)
**This does NOT override "Keep the PR the PR"** (standing user rule,
2026-09-09, further down this file). On an individual PR the routing question
is unchanged — *does the PR work without this fixed?* — and for a recruited
defect the answer is USUALLY yes, so it is FILED as an issue while the PR
merges. Run the question anyway rather than assuming the answer: a change that
flips a DEFAULT onto the latent path fails it, and a PR that breaks on first
use is not a merge. What this bullet changes is the RELEASE's answerability,
not whether a PR waits. Holding a PR for a recruited defect is the exact
friction the owner removed, so if you find yourself about to, you have
mis-scoped this bullet.
- **A review's findings are a SAMPLE, not a to-do list.** This is the single
highest-value habit in this section, and the one most often skipped. CLAUDE.md
already says to treat the *user's* examples as a sample and enumerate the
broader class — the same rule applies to REVIEWER output and is easy to miss
there, because N findings look exactly like an N-item work queue. Before
fixing any finding: name the CLASS it belongs to, enumerate the full
population of that class (programmatically — an AST walk or a grep that lists
every member, not a mental scan), and fix the population. Then the next round
has nothing of that class left to find.
Origin: 2026-08-27, three defect-bearing rounds on one PR where each round's
fix introduced the next round's defect — one of three renderers, then one of
two branches in the same function, then one direction of a two-directional
boundary. Every round I fixed exactly what was named. The enumeration that
finally closed it took one AST script and would have worked in round one.
- **Fixing the named instance is how a loop runs away.** If two consecutive
rounds each surface NEW defects, stop patching: that is the signature of
instance-fixing, and the commit gate hard-blocks TWICE: first at the SECOND
round (`review_enforcement_commit.py` mode-switch, cleared by `# audit-ack`),
then at the third (escalation cap, `# escalation-ack`). Both call `_deny`, so
the first stop arrives one round earlier than "the cap" suggests — see the
two-tier table below. Switch to enumeration BEFORE the gate has to say so.
- **Rising findings are a prompt to check PROVENANCE, not a verdict by count.**
When round N+1 returns more findings than round N, stop patching long enough to
determine whether the new findings land on additions made to address round N.
That provenance is the divergence signal; an unrelated increase alone is not.
If the added material is correct and separable, SPLIT it into one-concern PRs;
if it is wrong or unnecessary, REVERT to the smallest correct diff. Patching
again before that decision enlarges the surface the next round reviews.
MEASURED 2026-09-10 on PR #1831: insertions 731 → 827 → 1193 → 1372, findings
round 1 = 2 → round 2 = 4, and round 2's findings were entirely about round 1's
own additions. Every one was a genuine live bug — the fixes were correct AND
they were manufacturing the next round's findings, which is why the owner chose
SPLITTING there: a plain revert would have discarded correct work.
The discriminator is provenance, not growth or a raw count: the same session,
PR #1793 grew 85 → 139 → 209 insertions with findings 3 → 2 and converged.
The queue observation is descriptive only: the 196-PR sample included more
3+-round PRs among changes over 1,000 lines than among changes under 200; it
does not establish that size causes rounds. **The limit, honestly:** the
provenance trigger rests on one clear instance and one clear counter-instance.
Act on it as a signal; it is not a law.
- **Findings concentrating on safety code you built AROUND one external command:
read that command's manual for a native mode before writing the next guard.**
Adopt-before-build covers whole tools; this is the same question one level
down. When successive rounds land on hand-built protection around a single
command (snapshot, then refuse, then a destructive form of the command), the
tool often already ships the guarantee. Instance, #2722 (open as of 2026-10-01): moving the
rollback from hand-built snapshot-and-refuse around `reset --hard` to
`git reset --keep` retired most of a round's findings at once, and a later
premise check found that `git checkout --no-overwrite-ignore` refuses an ignored
file that `--keep` silently overwrites (MEASURED, git 2.43). The modes differ
in exactly these edges, so read every candidate's manual entry, not only the
first that fits. The trigger is the concentration of findings on one seam, not
their count.
- **Interrogate every MECHANISM you introduce along six axes BEFORE the first
review — original code and fixes alike.** The two bullets above enumerate the
class of a DEFECT, reactively, once a reviewer names one. This one is about the
extent of a MECHANISM — a deadline, a lock, a cache, a guard, a token — which is
correct on the path that motivated it and undefined everywhere else.
MEASURED 2026-09-09 on 31 inline review findings from two PRs, each attributed
by `git blame` at the SHA the reviewer was looking at, and classified as
"original" or "post-review" by whether the blamed commit predates that PR's
first review. Over all rounds, 21 of 31 (68%) blame to the original
implementation. But round 1 CANNOT blame to a fix commit — none exists yet — so
that figure is partly forced arithmetic. In the rounds that actually loop it is
close to even: **of the 21 findings from rounds 2+, 11 were already in the
original code and 10 were in code written to answer an earlier round.** Both
halves are worth the same attention; neither dominates.
Deduplicating re-posts and cross-reviewer duplicates and dropping one finding
later refuted by measurement leaves 25 distinct findings, and **24 of those 25
fall into six shapes.** These are the checklist:
1. **SIGNAL** (8, the largest) — does the thing you read change exactly when
the property you assert changes? A whole-database counter read as "did this
table change"; an mtime read as "did content change"; empty read as
"unbuilt"; a proxy's identity read as "same connection"; two autocommit
reads used as one snapshot.
2. **SCOPE and LIFETIME** (5) — a deadline per query when the operation is a
traversal; a lock per process when the race is cross-process; a lease that
expires mid-work; a hang moved from the operation to interpreter teardown.
3. **INPUT DOMAIN** (5) — the values the type permits, not the ones you
pictured: non-boolean, empty string, malformed timestamp, sub-second
precision, unescaped path.
4. **EQUIVALENCE** (3) — what does it claim to match, and has anything checked?
5. **FAILURE PATH** (2) — what is left behind when the work aborts.
6. **CALLER CONTRACT** (1) — does an explicit option the caller passed survive?
(The 25th was a test whose identity comparison was unsound in both directions.)
A finding is a POINT, so a fix that treats the finding as its spec inherits
that point as its only test — which is why the fix half of that near-even split
exists at all, and why the six axes are worth running over a fix and not only
over a feature.
METHOD NOTE, because getting this wrong inverted an earlier version of this
measurement: to attribute a finding to a commit, blame the line at the SHA the
reviewer was looking at. `original_commit_id` is the PR HEAD when the review was
posted, not the introducing commit — one commit here touched only changelog
files yet carries three findings on a Python file.
FALSIFIABLE, with a denominator: #1850 pushed four commits after its first
review round, and deduplicated they carried 5 findings between them — 1.25 per
post-review commit. If the next three PRs whose mechanisms are interrogated on
these six axes before pushing still average above one deduplicated finding per
post-review commit, the axes are not the right ones. Re-derive rather than
adding a seventh.
- **Run the pre-push adversarial pass with `/deep-review`** (`.claude/commands/deep-review.md`):
one command that dispatches a fresh-context `genesis-architect` (+ `genesis-security-reviewer`
on security surfaces) over the FULL branch diff with the right SHAPE — fail-open/state/TOCTOU/
hand-rolled-parsing hunting, not a lint/secrets scan — and writes the evidence marker. This is
the review that catches Round-1 bugs before Codex does; `/audit-changes` is only a light
self-check.
- **PR review-findings status = `python3 scripts/hooks/git_push_guard.py --check-pr <N>`
— ONLY.** This runs the SAME code path as the merge gate (strict fail-closed: a
failed scan is never reported clean). NEVER hand-roll a `gh api pulls/N/comments`
query to decide whether a PR is review-clean: a wrong filter's EMPTY result reads
exactly like "clean". Origin (2026-08-23, #1431/#1432): Codex authors BOTH its inline
findings AND its review-summary body as `chatgpt-codex-connector[bot]` — the REST
`user.login`, WITH the `[bot]` suffix. A hand-rolled filter keyed on a DIFFERENT login
(the GraphQL app login, which is not the REST login) matched nothing, and 13 real
findings (10 P1) were reported to the user as "review-clean" until the merge gate
blocked. An empty result from your own query is "my query found nothing", never "no
findings exist". Freshness is a SEPARATE gate, and it is NOT a blanket
reviewed-SHA-equals-HEAD rule: for a hook-surface or otherwise non-trivial delta a
current Codex review must COVER head (reviews-API `commit_id == head`, or a clean
Codex signal whose abbreviated id RESOLVES to the head, see the Pre-Merge Gate),
but a trivial NON-hook delta may still merge on a stale
review — `--check-pr` reports that as `codex-at-head : ok (STALE review of <sha>, delta
since is trivial)`, a pass, not a block.
**And the verdict is a function of the TREE YOU RAN IT FROM.** `--check-pr` is
(remote PR state × LOCAL gate code) — it executes the `git_push_guard.py` sitting
in your working copy, not the one on `main`. Run from a worktree carrying an
unmerged change to the gate, it reports what WOULD be true once that change lands:
a correct verification OF THE FIX, and a false statement about the PR. MEASURED
2026-09-09: a session reported a peer's PR as one blocker from green on exactly
that confusion. So say which tree produced any verdict you pass on, and for a
claim about a PR's CURRENT state, run it from a tree at `origin/main`. To bound
a verdict you already took, compare the CHECKER ITSELF and its effective policy
inputs across the two trees, not main's history: `git diff origin/main --
scripts/hooks/git_push_guard.py` (add whichever modules the checker imports),
then identify any local configuration that can alter required reviews or CI.
Hash the relevant blobs when the effective policy is identical. Reaching for
`git log origin/main -- <checker>` instead is the trap — it inspects only what
landed on main, so it comes back EMPTY in the exact case that bites you, a
worktree carrying an unmerged change to the gate. Empty history is not identical
logic. The same reasoning applies to any local checker whose answer is about a
remote object.
- **Hook-surface PRs merge only with a current GitHub Codex review — mechanical.**
A PR touching the enforcement-hook surface (the guard code itself) gets almost no
stale-review leniency: the merge gate (1) never classifies a post-review delta that
TOUCHES that surface as "review-trivial" — the test is on the DELTA's files, not the
PR's, so a docs-only follow-up commit on such a PR can still be trivial (see the
budgeting note further down, which measures exactly this) — and (2) refuses
`# stale-review-override` — regardless of Codex
head-freshness — unless recorded fallback-review evidence exists for the EXACT
base+head (`~/.genesis/override_review_evidence/<repo>__<pr>__<base12>__<sha>.txt`).
A current at-head Codex review does NOT substitute for that evidence: the same sigil
also waives `_check_base_is_default`, and the evidence identity binds the BASE tip,
which a head-only review cannot vouch for (a hook-surface PR retargeted to a
non-default base must be re-reviewed in that base's context). The surface is defined
authoritatively by `_HOOK_SURFACE_PREFIXES` + `_HOOK_SURFACE_FILES` in
`scripts/hooks/git_push_guard.py` (hook dirs, the global bash safety hook, the
review-scope/state modules, hook wiring in `.claude/settings.json`, and the tracked
configs the hooks read) — read those constants, kept exhaustive by the
`TestWiredHooksFenceGuardrail` test, rather than any hand-copied list. The override
procedure requires the user's explicit authorization, then a fallback adversarial
review (local `codex exec` when quota allows, else genesis-architect), evidence
recorded naming the head, then the merge re-run — the gate's block message walks
through it.
- **One reviewer at a time on a given diff — almost never two at once.** Run one
reviewer (e.g. Codex), apply/verify its findings, then run the next reviewer
(e.g. Claude) on the *fixed* code. The second reviewer should see the improved
code, not the same unfixed diff both would otherwise review; parallel also
doubles review spend per baseline. (Standing user directive.)
**Sequential means WAIT. It does not mean kill.** When a review is already
running and a different reviewer gets named — a tool switch, a stated
preference, a peer session's run — let the in-flight one FINISH. Terminating
it destroys the findings the next reviewer is supposed to build on, which is
the opposite of what this rule is for. MEASURED 2026-09-08: a running review
was killed four minutes in, mid-analysis on the load-bearing question, to
honour this rule's former "NEVER" — and nothing was gated on stopping it.
The "almost" is load-bearing. This is standard practice, not a prohibition:
* Two reviewers on DIFFERENT diffs is fine — no shared baseline, no
cross-bias. It is not licence to fan out across the queue: external
reviewers share one quota, and exhausting it strands the cross-model gate
every open PR depends on. Concurrency here is a tolerated exception, not a
throughput strategy.
* A peer session's reviewer is not yours to stop while that session is alive.
An ORPHAN — a process whose session is gone, or one wedged holding a
semaphore slot — is ordinary cleanup; establish it is orphaned first.
* The user may ask for both, in their own words naming the running one. A
preference for a different tool is NOT that ask; that misreading is the
incident above.
* ROUND 1 of a hook-surface PR runs two reviewers in parallel BY STANDING
RULE — see the gate-fix lane below. The rationale above does not bind
there: both reviewers see the same unfixed head, so neither could have
seen "the fixed code" whatever the order. This carve-out is scoped to
round 1 of that lane and to nothing else; rounds after it are sequential
like everywhere else.
**If you are about to terminate a running reviewer in order to satisfy this
rule, that is the tell that you are misreading it.** Wait, or ask.
This rule prohibits a STATE ("two running") rather than an ACTION, which is
why it is spelled out: a state prohibition can be satisfied by DESTROYING the
offending state, where an action prohibition — never push to main, never
hardcode the slug — can only be satisfied by not acting. The other instance of
the shape is `CLAUDE.md`'s genesis-bridge line ("must never run alongside the
server"), where the wrong discharge is worse: stopping the server rather than
declining to start the bridge. Prefer phrasing a new rule as an action; where
it must name a state, say which side gives way, because an absolute with an
unstated remedy is discharged in whatever direction the reader already leans.
- **A different model is the real correctness gate; Codex is the default.** A Claude
reviewer shares this model's blind spots, so it clears the LOCAL depth gate but is not
the cross-model gate. When the GitHub Codex reviewer is unavailable AND the install
has an approved alternative external reviewer — a DIFFERENT model, with explicit
per-use user approval every time — run it non-interactively over the diff with an
adversarial mandate. Which reviewer that is (if any) is install-local and belongs in
user-level config, not here.
**Unavailable is established by ASKING**: comment `@codex review`, wait, and read the
reply. An explicit usage-limits comment is the STRONGEST evidence available — but it
is not proof, and treating it as proof is how an alternate-reviewer approval gets
spent on a Codex that was never down. The two channels are INDEPENDENT: a
usage-limits ISSUE comment can sit there while a later trigger delivers real INLINE
findings (MEASURED on #1484 — see the inline-findings section below). So: re-trigger,
let time pass, and check the INLINE endpoint before concluding unavailable.
A second shape carries the same weight and the same caveat: an explicit
REVIEW-FAILURE reply that REPRODUCES. The bot answering `Codex Review: Something went
wrong` names its own cause, and a cause you can DISPROVE locally is the strongest
reading available — MEASURED 2026-09-09 on two PRs, twice each 43 minutes apart,
`Provided git ref <sha> does not exist` while `git ls-remote origin refs/heads/<branch>`
returned that exact sha, and two other PRs on the same account reviewed normally in the
same window. Reproduces + refuted locally + not account-wide = a defect on their side,
not a quota and not something a further retry fixes. One such reply is an incident, not
yet evidence: retry once, let time pass, and check the INLINE endpoint first, exactly
as the usage-limits case demands.
Everything weaker is not evidence at all — silence is not, and neither is
`--check-pr` reporting no review, which says the same thing whether the reviewer is
down or was simply never triggered at this head. Nor is a
`codex exec` quota error: that is a separate surface on separate quota.
Scope what you hand it exactly as `.claude/commands/deep-review.md` §1 specifies.
**Verify it saw a diff at all**: a clean verdict that does not demonstrate WHAT it
reviewed is void, and a false clean from the cross-model gate is worse than no review.
Do not merge on a same-model-only review.
- **Escalation cap — a HARD BLOCK at 3 CROSS-MODEL rounds that each find NEW defects.**
A *round* = one EXTERNAL cross-model review→fix→re-review iteration. INTERNAL
same-model reviews (genesis-architect / genesis-security / any subagent) are NOT
rounds — they never move the machine counter (see "THE COUNTER IS CROSS-MODEL ONLY"
below) and must not be counted in the visible tally either, or a session re-creates the
very false-stop this is meant to remove.
⚠ **CALL AN INTERNAL AUDIT A "PASS", NEVER A ROUND — including in your own
evidence file and your own prose.** The word is reserved for the counted thing,
and nothing enforces that, so it is on you. MEASURED 2026-09-25: a session ran
three internal architect audits, headed them `ROUND 1` / `ROUND 2` / `ROUND 3` in
its evidence file, and then told the owner its PR was "at round 4 with four
reviewed heads". `scripts/review_budget.py --repo <r> --pr <n>` said `count=1,
next_round=2`. It had been counting its own passes — the very thing this
definition excludes — and built a process question on the wrong number. **Never
state a round number you have not just read out of `review_budget.py`**; your own
tally is not a source, and `review_state.py mark` prints "internal review —
cross-model streak unchanged" precisely so you cannot mistake one for the other. A cloud-bot (Codex) re-review round counts; a
locally-run non-Anthropic reviewer (the install's configured secondary) counts. The
cap is enforced by three
mechanics, not by vibes:
1. **Visible streak counter.** From the first EXTERNAL round, the plan file (or task
list) carries `Cross-model streak: N (cap 3)`, updated every external cycle. It
counts consecutive defect-bearing external rounds for this cap — a STREAK
POSITION, never a round NUMBER. Round numbers come only from
`review_budget.py`'s round count, and the two differ (a locally run
secondary advances the streak without adding a reviewed head). The streak is a tracked artifact — "it's the same class, it doesn't really count" is exactly
the rationalization the counter exists to kill (for a repeat EXTERNAL round).
2. **The block point is BEFORE dispatching the next review.** The check is
"am I about to trigger a 4th consecutive defect-bearing external round
(streak at the cap)?" — evaluated at the mechanical moment
(the `@codex review` comment, the reviewer dispatch — NOT the push itself,
which triggers nothing TODAY — an owner-tunable
setting, so verify at the PR rather than trusting this clause), never after
reading the next batch of findings.
3. **The cap CONSUMES standing approval.** A prior "proceed", "merge when
clean", or "keep going until Codex is green" is VOID once the cap fires.
Continuing a round-4+ loop on an earlier instruction is a violation, not
obedience — STOP, post the round ledger (round → what it found → what it
cost), name the cap explicitly ("we've hit the 3-round escalation cap"),
and get a FRESH decision: HAND IT BACK through the established disposition
— architecture conversation, or `needs-architecture-session` + draft + a `ready` row
(three rounds
each finding something new, after a class-level audit, is the strongest
evidence available that the PREMISE and not the code is what is wrong —
and no further round can fix that), switch to a robust-by-construction
redesign, narrow scope, or shelve. The hand-back option is first because
it is the one nothing used to name, not because it is the likeliest —
decide it on evidence via `.claude/docs/premise-check.md`, and see the
two-path doctrine below.
**Tabulate findings by CLASS before fixing — but never let that change what
COUNTS.**
Tabulate the findings with a CLASS column before fixing ANY round's findings,
including round ONE. Deferring the tabulation to the second round is what
spends a round discovering a shared cause that was visible in the first: if the
opening review returns several instances of one generator, an instance-level
first pass fixes the ones named and ships the rest as the next round's
findings. The tabulation is cheap and the round it saves is not. Findings that look unrelated one at a time routinely share one
generator — five across three rounds once reduced to a single defect (two
layers that had to agree about every lever and could not), and patching
instances twice changed nothing while deleting the second layer removed all
five at once. If one class has ≥2 entries, look for the shared GENERATOR and fix
that; two findings can land in one superficial class without sharing a cause, so
the count is the prompt to look, not the verdict.
**Class grouping decides HOW you fix — never whether an external round counts.**
The counter is deliberately class-blind: `bump_review_round` increments on a
distinct staged diff and records no CLASS identity (it records only `source`, to
gate internal-vs-external), and the marking rule below is finding-based — ANY new
BLOCKER/SHOULD-FIX/P1/P2 from an EXTERNAL reviewer makes the round defect-bearing,
including the second instance of a class you have already named. On an external
round, passing `--clean` to keep a repeat-class round off the counter is a
falsification, and worse than miscounting: an external `--clean` RESETS the streak
to zero, so it disarms the cap outright rather than merely under-counting it.
(Internal reviews never count either way, so there is nothing to falsify there —
the temptation and the rule both live only on external marks.)
A corollary that costs a round if missed: when you delete a duplicated layer,
verify the class is closed by enumerating the registry for the BEHAVIOUR, not
for the deleted file's NAME. A name-scoped guard beneath a class-scoped claim
passes happily while a sibling oracle sits on the same event and matcher.
(And if the invariant turns out not to be statically checkable, say so and
narrow the test to what it can prove — a guard that cries wolf gets deleted by
whoever hits it next.)
These disciplines are backstopped by two complementary machine layers.
The LOCAL defect-bearing streak keeps its established interventions:
| Local streak | Gate | Required action |
|---|---|---|
| 2 | **MODE-SWITCH block** | Decide premise-vs-polish, run the fresh-context class audit, then use `# audit-ack` only if that audit happened. |
| 3 | **HARD STOP** | Hand back, redesign, narrow, or shelve. `# escalation-ack` records a fresh decision and resets only this local streak. |
Standing authorization is a separate GitHub-backed budget. It counts ROUNDS, not
local marks: a round is a distinct head that drew FINDINGS from any GitHub App
reviewer (owner ruling, 2026-10-01). Every reviewer on one head collapses to one
round; a dismissed review with findings still counts; a head a force-push removed
still counts. Only an explicit informational class opens nothing — Codex P3, Devin
🔍, CodeRabbit trivial/info and its nitpick section — while a comment a known parser
cannot read, and any top-level comment from a reviewer with no known format, DOES
count. Humans, workflow bots and CodeQL never open a round. A clean review, a Codex
clean comment and the configured identity template CONFIRM a head; they never add a
round. Evidence from before `ROUND_RULE_CUTOVER_ISO` (`scripts/review_budget.py`)
keeps the old rule (every head Codex reviewed, clean included), so no PR's count
moved at the cutover. The rule lives in `review_findings.is_finding` and
`review_budget.evaluate_evidence`; read those, not this paragraph, for an edge case.
Ordinary PRs carry standing authorization through four rounds. Starting with the
fifth review request or the fix commit after four rounds, every action needs its
own native user approval.
**ROUND 4 IS TERMINAL (owner ruling, 2026-09-25).** There is no ordinary round 5.
At four rounds the decision is not "another round" — it is MERGE, with the
outstanding issues accepted and FILED, or SEND IT BACK for rework. A fifth round
exists only where the owner explicitly authorizes one, and that authorization is
re-asked EVERY subsequent round, each of which is equally terminal and faces the
same decision. Earlier text here read "round 6 and later are strongly discouraged",
which invited a session to treat round 5 as ordinary; the discouragement was never
meant as a softer tier but as the mechanism that stops an authorized fifth round
coasting into a sixth unasked.
This approval is never a shell sigil and is never persisted. `# escalation-ack` still
serves the local round-3 intervention; `# final-round-accept` is legacy syntax and
authorizes no current gate. An earlier "okay" cannot authorize a later request or
commit. Foreground sessions receive the native approval dialog at the action;
autonomous/dispatched sessions are denied because nobody is present to answer it.
Unreadable or ambiguous GitHub evidence takes the same ask/deny direction and never
becomes zero rounds. Multiple gated actions must be split so one approval cannot cover
several commits or review requests.
The native approval enforcement in this section is implemented at Claude Code's
`PreToolUse` hook boundary. The shared budget evaluator is reusable, but external
coding agents do not yet receive an equivalent approval prompt from this change and
must not be described as mechanically protected by it. Within Claude Code, keep each
protected action in its own command: a HEAD-moving `git switch`/`git checkout` cannot
share a command with `git commit`, and a command containing multiple possible review
requests is rejected before any request is sent. Inline comment bodies containing
shell expansion are opaque at the hook boundary and therefore take the ask/deny path.
The round-2 block is not the round-3 cap arriving early — it is a different
instruction. It says the *approach* is wrong (you are fixing instances, not the
class), where the cap says *stop and re-decide*. Acking round 2 without actually
doing the fresh-context audit is how a session arrives at round 3 having learned
nothing. `# audit-ack` attests that the audit HAPPENED; it is not a "continue"
button. Note the fresh-context subagent this tier mandates is INTERNAL — mark it
plainly (`--source internal`, the default); it satisfies the depth gate and does
NOT advance the counter, so it can never be the round that hard-blocks you. Only a
repeat EXTERNAL (Codex, the configured secondary, …) non-convergence moves the streak
toward the cap.
(Origin, 2026-09-01: under the old model the audit this very tier demanded counted
as round 3 and tripped the HARD cap — the gate penalized the remedy it mandated.)
There is a separate depth tier that can fire at ANY round: a **substantial**
change (≥50 reviewable lines OR >1 code file OR auth/api/migrations OR any
prompt/agent/skill surface) whose marked review is not adversarial is blocked
with `# depth-ack`. A `mark` that lands with no `file:line` engagement prints a
WARNING and will be rejected by that gate — re-write the evidence with concrete
anchors rather than acking past it.
`review_state.py` still keeps the local `round` streak and legacy lifetime
fields for stale-worktree compatibility. Current authorization uses
`scripts/review_budget.py`; the commit gate reads the local streak from one snapshot
only for the round-2/round-3 interventions. The old lifetime APIs remain
import-compatible but current gates do not consult them. `FINAL_ROUND_CAP` is
**DELETED** (2026-09-25) — it was consulted by no gate, in zero conditionals, while
its value had been picked to avoid colliding with the four-head boundary, so a
retired tier was still shaping the live one and reading as though a seventh round
were real. The `final-round-accept` SIGIL is still parsed and loudly refused, so a
stale worktree is still told it is dead.
**THE COUNTER IS CROSS-MODEL ONLY.** The streak exists to catch *cross-model
non-convergence* — an EXTERNAL reviewer finding NEW defects round after round. It does
NOT exist to penalize the free, encouraged act of reviewing your own work. So `mark`
takes a `--source {internal,external}` (default `internal`) that records WHO PRODUCED THE
FINDINGS, and that is what decides whether the round counts.
**EXTERNAL is judged by the reviewing MODEL, not the gateway/provider — this is the "big
one".** External = a review by a non-ANTHROPIC model. Anthropic Claude via ANY route
counts as INTERNAL — including a Claude model reached through OpenRouter (the repo's
`openrouter-haiku/sonnet/opus` routes are Claude), so "it went through OpenRouter" NEVER
makes a review external. And Genesis's OWN cognitive/routing systems are never reviewers:
they are cognitive infrastructure, not a review service, so no internal Genesis model call
is ever `--source external`. The approved methods are **Codex** plus whatever the
install names as its SECONDARY reviewer — `merge_gate.secondary_reviewer` in local
`genesis.yaml` — one value, the model or CLI to invoke; **absent means there is no
standing secondary**,
and round 1 of the gate-fix lane below runs Codex-only. It is named in config rather
than here because which model an install can reach is install-local, and a version
frozen into this file goes stale the day the model does. **OpenRouter is NOT an
approved method** (a future option, not a current one).
Two mechanics for that key, because it is unlike its neighbours: it is read by
SESSIONS, not by the gate — no code consults it, so a typo degrades to "no
secondary" rather than failing a check. And add it under the EXISTING `merge_gate:`
block: a SECOND `merge_gate:` block trips the duplicate-key scan that
`_required_ci_workflows` / `_required_scheduled_review_kinds` / `_doc_findings_mode`
all share, and all three then discard your configured value and take their DEFAULT.
Say "default", not "fail closed" — the directions differ, which is the whole reason
it matters, and all three can now LOOSEN: `_doc_findings_mode` defaults to `skip`
(`_DEFAULT_DOC_FINDINGS_MODE`; cite the SYMBOL — the line this used to name now holds a sibling constant), scoring fewer findings and saying nothing about it;
`_required_ci_workflows` falls back to the shipped `("CI",)`, which is NARROWER than
any larger required set an install declared — it does print a NOTE, so that one is
loud rather than silent, but it is still a relaxation (Codex P2, #1903); and
`_required_scheduled_review_kinds` now defaults to the MINIMAL set `("leaks",)`
(⚠ it defaulted to the maximal `code-review` + `leaks` until the default was
narrowed, so a discard there TIGHTENED — it now narrows instead, and likewise
prints a NOTE when the key was visibly declared). The floor survives every
discard: `leaks` is irreducible. The rule below
keys on this:
- **`--source internal` (the default)** — a same-model self / genesis-architect /
genesis-security / any-subagent review. It is free and shares the author-model's
blind spots (rubber-stamp risk), so it **NEVER moves the streak** — not an
increment, not a reset — whatever it found. No outcome flag is needed (a bare
`python3 scripts/review_state.py mark --agent-output <path>` is a valid internal
review). This is the fix for the weeks of false blocks: your own audits, including
the one the round-2 mode-switch gate itself *mandates*, can never trip the cap.
- **`--source external`** — a non-Anthropic cross-model reviewer drove this round.
This is the ONLY kind that counts, so it REQUIRES exactly one outcome flag:
- a NEW **BLOCKER / SHOULD-FIX / P1 / P2** → `--defects`
(`… mark --agent-output <path> --source external --defects`) — the round counts.
- **no** new BLOCKER/SHOULD-FIX/P1/P2 → `--clean`
(`… --source external --clean`) — RESETS the streak (circuit-breaker
reset-on-success). Only an EXTERNAL clean round resets; an internal "looks
fine" can never reset a standing cross-model streak (that would be a
self-rubber-stamp reset).
`--source` describes the REVIEW THAT PRODUCED THE FINDINGS, not who typed the
evidence file: a mark recording "verified + fixed Codex's (or the secondary's) findings" is
`external`; a mark of your own architect/security audit is `internal`. A round is
CLEAN iff the external review found no BLOCKER/SHOULD-FIX/P1/P2 (and no security
CRITICAL/WARNING). NOTEs, nitpicks, and dispositioned optional-hardening do NOT make
a round defect-bearing. The ack (`# audit-ack` / `# escalation-ack`) is a conscious,
logged act (like `# review-override`); adding it — or falsely passing an external
`--clean` — WITHOUT the honest review result is the same violation as ignoring the
prose above. (Supersedes feea3f71/#1446: its unconditional required-outcome only
ever bit internal re-audits, which now can't inflate the streak at all; the outcome
requirement is kept where it is still load-bearing — on external marks.)
Caveats: **multiple findings in a single pass = one round** (not an
escalation); the same defect reappearing (an incomplete prior fix) is a
fix-it-properly issue, not an escalation trigger. This complements the
enumerate-class-then-lock convergence discipline — the cap is the escalation
trigger when the class won't lock within ≤3 rounds. (Origin: PR #1281 ran ~7
reviewer rounds because a standing "proceed once clean" silently carried
through rounds 4–6.)
**The Claude Code PR-side hook uses the same shared evaluator.** A review request reads the
rounds live from GitHub. Below four ordinary rounds it proceeds under
standing authorization. At four or more it emits a native user approval for exactly
one request; at five or more its message strongly recommends stopping. API, parse,
prefix-resolution, changed-file, or PR-identity uncertainty asks in foreground and
denies in autonomous sessions, and so do findings a review's body declares but whose
comments were deleted. A clean review comment's abbreviated SHA is resolved against the
PR's full commit list, and an ambiguous prefix makes the result unknown.
`--check-pr` prints the count as an advisory `rounds` row, with findings per round and
a RISING marker; it never moves the verdict.
- **GATE-FIX LANE — a hook-surface PR gets two standing discovery rounds
(standing user rule, 2026-09-09).** The shared evaluator now enforces the distinction
mechanically. After exactly two rounds, a current unreviewed head may receive
ONE confirmation request without another approval only when the request carries
`<!-- genesis-review-request head=<full-40-hex> kind=confirmation -->`. The marker is
dispatch evidence, never approval or review evidence. Once present, a repeated request
needs fresh approval. A review after that confirmation, or any further discovery
request, also needs fresh approval. Because a CLEAN confirmation adds no round, the
marker itself is the one-shot token: once a marker exists for any head other than the
current one, the confirmation is SPENT and every later request and fix commit asks. A
head that drew findings itself is a round, never a fix to confirm, so it gets no free
request. (Residual, by owner ruling: an approved UNMARKED request that comes back
clean leaves the marked one unspent.)
**Scope: any PR whose diff touches the enforcement-hook surface** —
`HOOK_SURFACE_PREFIXES` + `HOOK_SURFACE_FILES` in `scripts/review_budget.py`.
Read those constants; do not copy them here — a hand-copied list is wrong the moment
a hook is wired. `TestWiredHooksFenceGuardrail` keeps the WIRED-HOOK half
self-maintaining FOR THE SPELLINGS IT RECOGNISES, by parsing
`.claude/settings.json` (measured 2026-09-10: **54** wired command entries, **49**
of them matched, **5** unmatched, yielding **42** unique paths — an earlier draft
said "42 of 54", which divides a path count by an entry count and so implies twelve
misses where there are five. The five are `.claude/hooks/*.sh` and inline blobs
already covered by the prefixes, so no live gap — but a `python3 -u scripts/foo.py`
or `node …` wiring would slip past it); it cannot do the same for the tracked CONFIGS the
hooks read, because settings.json lists hooks and not the files they consume. Those
entries are maintained by hand, so adding a config a hook's behaviour depends on means
adding it to `_HOOK_SURFACE_FILES` deliberately (Codex P2, #1903). This lane is a STRICTER-REVIEW, FEWER-ROUNDS trade on that surface. It does
not apply anywhere else, and applying it to an ordinary PR doubles review spend for
nothing. The test is MECHANICAL and deliberately wider than the rationale — the
surface includes hook wiring and the tracked configs the hooks read, so a PR that only
incidentally edits one is still in the lane. That is the intended trade: a mechanical
test nobody has to adjudicate beats a judgement call about whether an edit "really"
changes the gate. Throughout this block "gate PR" means exactly "hook-surface PR".
1. **Round 1 is DUAL and immediate.** CONFIRM Codex has fired (it auto-reviews when
the PR opens). `--check-pr <N>` shows whether a review EXISTS at this head; it
CANNOT distinguish "never requested" from "still running", so if you did not
request one yourself, request one rather than reading a blank line as proof
nobody did. Then
run the secondary reviewer (`merge_gate.secondary_reviewer`) alongside it — two
sets of blind spots in one calendar round, which is the scarce resource here. No
secondary configured → Codex-only, and say so in the PR. Nothing validates that
key, so a TYPO looks exactly like "absent" — read the value before concluding
there is none, or the lane quietly runs at half strength while the PR truthfully
reports Codex-only.
**This lane carries STANDING approval for the secondary, and it is the ONE place
that overrides the per-use rule above.** The general rule — an alternate reviewer
needs explicit per-use approval, and only when Codex is UNAVAILABLE — still governs
everywhere else; here the secondary runs alongside a healthy Codex, at PR open,
without asking (owner decision, 2026-09-09). Do not generalise this to any other PR.
**One head = one round on every counter, however many reviewers saw it**; write the
local mark ONCE with combined evidence. That single mark is `--defects` if EITHER
reviewer raised a new BLOCKER/SHOULD-FIX/P1/P2 — a clean secondary does not launder
a defect-bearing Codex round.
**Two separate marks do NOT halve the budget; they can DISARM it, which is worse.**
An earlier draft said "halves", and the code says otherwise: `bump_review_round`
increments only on a DISTINCT staged hash, so a second defect mark on the same diff
is idempotent and costs nothing (`review_state.py:850-865`). But `--clean` resets
the streak to 0 **regardless of whether the staged diff changed** (`:780-784`), and
an idempotent same-hash `--defects` cannot restore the round it just erased. So on
one staged diff, in EITHER order, a mixed clean/defect pair records as CLEAN — the
defect-bearing round vanishes and the cap it was arming goes back to zero. That is
the failure the one-mark rule prevents (Codex P2, #1903).
2. **Triage before touching code — and here it OUTRANKS "fixing is usually free".**
That rule (below, and right for ordinary PRs) says a cheap finding costs nothing to
fix so just fix it. On this surface the cost is not the edit, it is the PUSH: it
invalidates the review, and a push whose DELTA touches the hook surface is never
classed review-trivial (`_classify_post_review_delta` tests the DELTA's files, not
the PR's), so a one-line courtesy fix to guard code buys a whole extra review
cycle. A push of ORDINARY PROSE can still ride the existing
review — that is the one cheap fix this rule does not charge you for, and note
how narrow it is: `_is_docs_or_config` DELIBERATELY excludes prompt surfaces
even when they are `.md` (`review_enforcement_commit.py:114-127`), so a skill,
command or agent file is SUBSTANTIAL and buys the full cycle
(`review_scope.classify_compare_substantiality`). Since a gate PR's docs are
usually exactly those files, assume you are paying (Codex P2, #1903). A cheap NON-floor finding gets a reply,
not a commit. Merge both reviewers' findings into ONE table,
then classify each: {live bug | latent trap | hardening | observation}. Only live
bugs and cheaper-now-than-later traps change code. The rest get a maintainer reply
carrying the evidence — IN-THREAD on the inline comment, from a maintainer
account: that is the ONLY shape the gate scores as engagement (it reads
`in_reply_to_id` on `pulls/N/comments`, so a top-level PR comment engages
nothing). Two findings therefore have no thread to reply into and must be
documented in the PR body instead: anything the SECONDARY reviewer raised, which
runs locally, and anything living in a review BODY rather than an inline comment.
Engagement costs ZERO rounds. Most findings never needed to become a diff.
3. **One batched fix push per round, self-audited first.** Not two, not three: every
push invalidates the review, so three pushes turn one round into three review
requests. (A compliant run has at most two CODE pushes total — the free
ordinary-prose push of rule 2 does not count against it — one answering round 1,
and rule 4's terminal push if the floor demands one.)
Before pushing, run the adversarial pass over your own FIX-CODE — reviewing your
own work needs no approval and is the specific gap that keeps biting. READ, three
instances (#1856, #1686, e7ae445a5): each a fail-open introduced BY fix-code
written under review pressure, and each caught only by a later round. The danger is the fixing, not the
reviewing.
4. **Round 2 is the discovery stop.** The two branches are: accept
and merge with every outstanding finding documented in the PR body, or abandon and
re-cut from a design that does not need a third round. **The always-fix floor is
never accepted** — a P1, a security defect, anything destructive or fail-open gets
FIXED in the terminal push. "Accept and document" covers everything that is NOT
floor-class, so a round 2 carrying floor findings has only two branches: fix them,
or abandon.
**A terminal push whose delta touches the hook surface needs a fresh review at that
head** — that delta is never review-trivial, by design (a docs-only terminal push
is the exception, and may ride the existing review — ORDINARY PROSE only, the
same narrowness rule 2 spells out: a skill, command or agent file is a PROMPT
SURFACE, classifies SUBSTANTIAL, and needs the fresh review even though it is
`.md`. That is the identical claim rule 2 already corrects, and it survived here
for a round because the fix was applied to the sentence that was flagged instead
of to every sentence making the claim). That confirming review
is a GATE REQUIREMENT, not a discovery round: if it is clean you merge. If it
surfaces something new, the SEVERITY routes it, exactly as it does anywhere
else in this rule — a non-floor finding is accepted and documented like any
other, and only a FLOOR-CLASS one (P1, security, destructive, fail-open) goes
to the owner under rule 5. An earlier draft sent everything to rule 5, which
contradicted the accept-and-document branch two sentences above and left one
confirmation result with two required actions (Codex P2, #1903). A round 2 with NO
floor findings needs no push at all, so there is nothing to re-review.
5. **A NEW P1 on the fix-code at round 2 is an owner decision, not round 3.** That is
the measured tripwire: a PR whose fix introduced a P1 is the kind that introduces
another. Stop and ask. The P1 itself is NOT optional (rule 4's floor) — what the
owner is deciding is fix-and-merge versus abandon-and-re-cut, never accept-and-ship.
**With no user to ask** (a dispatched session): do not merge and do not open round
3 — comment on the PR naming the tripwire and the P1, apply the
`needs-architecture-session` label, move it to draft (`gh pr ready <N> --undo`),
and open a `ready` follow-up (a dispatched session's follow-up lands in the `tabled` lane; the intake gap is tracked in #2857), the same
unattended route the architecture-session rule below uses.
6. **After round 1 the diff SHRINKS in SCOPE, never grows.** Line count is not the
measure — a floor fix under rule 4 may add lines and is mandatory. What may not
grow is what the PR is FOR. A revert is fine; new capability is a new PR. This is what actually kills whack-a-mole — the loop is sustained by
the diff expanding under review, not by the reviews themselves. (Composes with
"Keep the PR the PR" below: adjacent findings become issues.) Against
enumerate-the-CLASS above: fixing every instance of ONE defect inside the files
this PR already changes is the same fix, not growth — that is the shape the class
rule asks for. Reaching into files the PR does not touch, or adding capability to
make a class fixable, is growth: that is an issue or a follow-on PR. Ask "Keep the
PR the PR" FIRST — a PRE-EXISTING sibling instance the PR works fine without is an
issue even when it sits in a file you already touch; this rule governs the reach of
a fix you have already decided to make.
**The machine implements this lane.** Gate-surface classification comes from the
shared `review_budget` path predicate, including rename sources and destinations.
The second-round fix is permitted, and the exact-head marked confirmation is the one
exempt request. If that confirmation finds another defect and a further fix is needed,
the commit requires fresh native approval. The independent local streak may still
demand its round-2 class audit or round-3 stop; those are process interventions rather
than substitutes for the per-action user decision.
**Queue priority:** gate PRs get reviewed and driven before ordinary ones — read this
as a THIRD standing justification alongside the two in "When to DRIVE a Merge" below,
granted for the same reason those two are: a gate fix is what unblocks the PRs stacked
behind it, so leaving one to bake is the queue declining to repair itself.
**On the word "round" here:** one head that drew findings is one round however many
reviewers saw it. The shared evaluator deduplicates reviewers by commit identity, so
a dual round does not spend two rounds.
**A note for anyone reading only CLAUDE.md:** the shared evaluator now gates a third
discovery request mechanically. The local round-2 MODE-SWITCH still acts on commits
and remains a separate class-audit intervention.
- **Some PRs are not a review problem — hand them to an architecture session.**
When you ARE driving a PR toward green (per "When to DRIVE a Merge" below),
asking a few clarifying questions is not a substitute for the design
conversation. STOP and hand the PR to a foreground architecture conversation —
a session with the USER present, never a `genesis-architect` subagent, which is
review-session judgment by another name — when any of these holds: the conflict
is STRUCTURAL, main having superseded the mechanism the PR implements, so no
conflict resolution is correct on its own terms; it changes or contradicts the
spec at a significant level; it raises real questions about how Genesis
operates, its architecture, or its infrastructure; its consequences are
far-reaching or irreversible; or it is delicate enough that getting it wrong
damages a running Genesis. Treat the local round-2 mode switch, the round-3 hard
stop, or a native approval after the standing round budget as a trigger
to ask whether this is that kind of PR rather than merely another
round — taking this exit does NOT discharge that tier's own stop, which is
still owed; the handoff is the answer you bring to it, not a way around it.
If you ARE the session with the user present, hold the conversation now. With
no user to ask, the action is: comment on the PR naming WHICH trigger fired and
the evidence for it, apply the `needs-architecture-session` label (create it if
the repo lacks it — labels are per-repo and forks do not inherit them), move
the PR to draft (`gh pr ready <N> --undo`; see "PR readiness and mode"), open a
`ready` follow-up naming the PR and the decision it awaits — the label is a
GitHub annotation nothing drains, so the row is the intake (a dispatched session's follow-up lands in the `tabled` lane; the intake gap is tracked in #2857) — and move on to the
next PR. Expect it to be uncommon: the owner's estimate, explicitly unmeasured,
is on the order of 1 in 10 or fewer, so a session reaching for it often is
mis-triaging. Worked example, PR #1605: main's shared settings writer had
replaced the two inline heredocs the PR reimplements, and the literal base-wins
resolution measured 61 passed → 36 failed / 25 passed, because it deleted the
PR's own fix on two of its three declared paths.
- **External review feedback is a set of claims to VERIFY, not orders.** For
every bot/external finding: check it against the actual code (its stated
mechanism may be wrong even when the underlying concern is real — quote the
disproving file:line), check whether the "fix" breaks existing behavior or
violates YAGNI, and push back with technical reasoning when it's wrong for
this codebase. A finding that conflicts with the user's prior design
decisions (e.g. live network calls in a hot autonomy gate, weakening an
approval gate) is a STOP-and-discuss, never an auto-fix. Chasing a reviewer's
green checkmark with a change you believe is wrong is a discipline failure.
No performative agreement — state the verified fix, or the reasoned pushback.
- **Refuting a finding is itself a CLAIM, and it meets the same evidence bar you
just held the finding to.** The bullet above is about pushing back when the
reviewer is wrong; this is the cost of pushing back when YOU are. MEASURED
2026-09-09: a reviewer session refuted a cross-model reviewer's severity
assessment with a structural argument — "an expansion in verb position always
corrupts argv, so the exploit cannot exist" — which covered a few sampled
instances of one spelling rather than
the construct's grammar; a builder session then refuted the refutation with a
working construction, and the original severity was right all along. The tell
was sitting in the refutation's own text: it said *"I have NOT proven no clean
construction exists"* and stated the conclusion in the next sentence.
**An acknowledged gap in a proof is the reason NOT to state the conclusion —
never a hedge that licenses stating it.** So: a structural refutation must cover
the CONSTRUCT'S GRAMMAR — every form the syntax admits — not a sample of its
instances. Where it cannot, the finding STANDS and gets fixed or escalated on
its own merits; "I could not construct one" is a null result and clears nothing
(see the acceptance-bar section). Downgrading a severity is a refutation too,
and costs exactly the same evidence as dismissing the finding outright.
- **Waiving the review GATE is not waiving the FINDINGS.** When the user says
"skip the review" for a trivial change, that waives the blocking *ceremony*
(the gate and its `*-override` sigils — note `# review-override` is the one
that waives the findings scan) — it NEVER licenses ignoring a reviewer's
*substantive* findings. Read Codex's inline findings
(even non-blocking P2s) BEFORE merging even when the gate is waived, and
engage each on merits: verify it, then fix or consciously accept with a
stated reason. Merging past unread findings on a "skip review" is a trust
breach, not obedience. (Origin: #1439 merged past 3 correct Codex P2s.)
- **"Not blocking" describes the GATE, not the finding — and fixing is usually
FREE, so there is no tradeoff to weigh.** The subtler sibling of the rule
above: there, a waiver was granted; here the gate legitimately passes the
finding on its own rules, and the pass gets read as "handled".
MEASURED 2026-09-03 — two correct non-blocking findings merged UNFIXED because
nobody read them: #1620's P2 (an invalid `wing` raises `ValueError`, and
`tool_api.py:200-202` maps every exception to HTTP 500, so a caller input error
reads as a retryable server failure) and #1606's doc-path P1 above.
**Neither cost anything to fix.** A docs-only follow-up commit is
review-TRIVIAL: `classify_compare_substantiality` returns `inline` for doc
paths at ANY size, and `_classify_post_review_delta` tests the DELTA's files
rather than the PR's — so a CHANGELOG-only fix stays trivial even on a
hook-surface PR (verified: CHANGELOG-only → `inline`; a 1-line guard change →
`substantial`). So it costs zero CODEX rounds — but not zero blocks, and the
difference matters when you are budgeting the follow-up. On the canonical
public repo the SCHEDULED-review gate is head-pinned per kind, and `leaks` —
the only kind required by default — has ancestor relief
(`_MECHANICAL_RESCAN_BY_KIND`): the carried marker still satisfies the gate
once `leak-detector` is green at the new head. Budget the follow-up as: one
commit, no Codex round, no new scheduled review. An install that has ADDED
`code-review` back via `merge_gate.required_scheduled_reviews` budgets one
more: that kind has no ancestor relief, so the push invalidates its marker and
the gate blocks until a fresh scheduled review lands at the new head.
So: read every finding the report prints, fix the cheap ones, then merge.
The read is what the standing merge-when-green policy is buying — a gate
verdict of `ok` is not a report that there is nothing there.
And the inverse failure is as costly: do NOT escalate a cheap finding into a
mechanism change. The same session diagnosed its own inattention as a gate
defect and proposed weighting that would have taxed every PR — *"we'll never
get anything shipped if that's the standard"* (owner). Before proposing any
gate change, ask whether the mechanism is broken or you simply did not do the
work it assumed you would.
## The Gate Machinery — the sequence, and why it bites
Ten enforcement layers sit between a change and `main`. Learning them by hitting
them costs a session real time, every time. The canonical sequence:
```bash
python3 scripts/review_state.py evidence-path # -> ~/.genesis/review_evidence/<key>.txt
# ... write the adversarial audit to exactly that path ...
git add <files> # STAGE FIRST — mark hashes --cached
python3 scripts/review_state.py mark # INTERNAL genesis-architect audit — plain mark, never counts
git commit -F <msg-file> # bare, not piped (see below)
# (a non-Anthropic cross-model round would instead be: mark --source external --defects|--clean)
git push # approve the dialog on a branch's first push
gh pr create ...
gh pr comment <N> --body "@codex review" # after EVERY subsequent push
python3 scripts/hooks/git_push_guard.py --check-pr <N>
gh pr merge <N> --squash --admin --match-head-commit <head> # verbatim from --check-pr
```
**Ordering and lifetime rules that are not obvious:**
- **Stage → mark → commit.** `mark` hashes `git diff --cached`. Re-staging or
amending after marking invalidates it, and the check fails CLOSED.
- **Evidence expires in 30 minutes.** The audit file must be recent when you
`mark`, and the marker itself expires on the same clock. A long detour between
audit and commit means re-marking.
- **`evidence-path` and `mark` key off the PROCESS cwd**, not a flag. Run them
from the same worktree as the commit or the key diverges silently.
- **A successful commit WIPES the marker.** The next commit needs a fresh review;
this is deliberate, not a bug.
- **`--match-head-commit` is mechanically required**, not a nicety — the merge
arm blocks outright when a verified head exists and the flag is absent. Copy
the whole command from `--check-pr` output rather than reconstructing it.
**Traps with a real cost, each one measured:**
- **ANSWERING A FINDING TAKES AN IN-THREAD REPLY. A top-level PR comment
engages NOTHING.** The gate clears a finding from the score only when a
MAINTAINER account replied IN ITS THREAD: it reads `in_reply_to_id` from
`pulls/N/comments`, so a reply is matched to the inline comment it answers.
`gh pr comment N --body ...` creates an ISSUE comment, which has no
`in_reply_to_id` and answers nothing however thorough it is. Use:
gh api repos/<owner>/<repo>/pulls/<N>/comments \
-f body='verified and fixed at <sha>: …' -F in_reply_to=<comment_id>
(comment ids from `gh api repos/<owner>/<repo>/pulls/<N>/comments`.)
MEASURED 2026-09-09: switching to this form took two PRs from
`inline-findings: BLOCK` to `ok` with NO code change — 4 findings on #1847 and
5 on #1836 had been fixed for hours and still scored, because every reply was
top-level. The symptom is a score that will not fall while you are certain
you fixed everything; reach for this before you reach for an override.
TWO FINDINGS HAVE NO THREAD AT ALL and cannot be engaged this way: one
delivered in a review BODY rather than as an inline comment, and anything a
locally-run secondary reviewer raised. Document those in the PR body.
- **A BLOCKED Bash call runs NOTHING** — including earlier `&&` segments and
heredocs. If `mark && commit` is blocked, the `mark` did not happen either.
Run gate-adjacent steps as separate calls.
- **A secrets.env access that did NOT prompt may have been silenced, or may
have escaped the guard.** The guard matches only some access paths, so an
unprompted access proves nothing by itself. An install may set
`hooks.asks.secrets_env: off` in `~/.genesis/config/genesis.yaml`, which
removes the credentials guard's prompt: the hook then emits no decision, only
a context note naming the setting. That note is what proves a silenced
access; with no note, assume the access went past the guard unseen. It
approves nothing; other hooks and Claude Code's own permissions still decide
the command, and a dispatched session is still denied. The only other key is
`push_publish` (owner ruling 2026-10-01), same no-decision-plus-note shape.
It silences only the first push of the CURRENT branch, and only when the
whole command is exactly one plain `git push` (e.g. `git push -u origin
HEAD`) — a chained command still asks, so run the first push as its own
command. The remote git really pushes to must resolve (rewrites applied) to
exactly `https://github.com/<public repo>` — ssh/scp forms always ask — with
simple push config, no `http.*` config, no proxy/TLS/ssh/config env var in
the hook's environment, and a live probe confirming the branch is absent
there. Residue: shell-profile env/aliases/functions/PATH are invisible to the
hook, and a concurrent process can still change config or publish the
branch between check and push. `gh pr create` is not covered, since gh
without a TTY aborts rather than pushing. Force pushes, other destinations,
close-then-push, the no-open-PR block, round-cap asks and the dispatched deny
are untouched, and any doubt about the destination keeps the prompt.
- **Ack sigils bind per-guard, and mostly to the LAST pipeline segment.**
`git commit ... | tail # audit-ack` puts the ack on `tail`. Run the commit
bare. Some guards accept a sigil on any segment, others only on the offending
one — do not generalise from one guard's behaviour.
- **Sigils must lead the trailing comment.** The override is read from the
leading run of recognised tokens, so `# see audit-ack notes` overrides nothing.
- **`--no-verify` is blocked before any override is even considered.** There is
no way to skip the native hooks; fix the cause.
- **Chained commits are heavily restricted** — across worktrees, blocked
outright; within one, later commits must be pure `--amend` with no intervening
git command. `cd "$VAR" && git commit` fails closed with a *branch-verification*
message, which reads like a branch problem and is not: use a literal path.
- **Worktree removal is not yours to do.** `git worktree remove` is blocked;
`scripts/worktree_lifecycle.py` owns it: it archives stale worktrees on a
daily timer and never deletes them (archive retention is off, #2504). Leave a dead worktree alone.
- **Editing a tracked git hook blocks the commit** until its hash is re-recorded
(`scripts/update_hook_versions.sh`).
- **⚠ `scripts/hooks/*` is NOT synced — and a WORKTREE edit is still not live.**
`sync-hooks.sh` copies only the five GIT hooks (`commit-msg`, `post-commit`,
`pre-commit`, `prepare-commit-msg`, `pre-push`) plus one helper into
`.git/hooks/`. `.claude/hooks/genesis-hook` launches entry-point hook scripts
such as `git_push_guard.py`; that guard imports the shared `shell_parse.py`
module. `hook_output.py` is instead imported by other hook entry points. The
launcher resolves `HOOK_ROOT` to the **MAIN
worktree**, not the tree you are sitting in
(`HOOK_ROOT="$MAIN_ROOT"` unless `GENESIS_HOOK_DEV_LOCAL=1`). That is
deliberate — it stops per-branch hook drift, measured 2026-08 at 60 of 70
worktrees running a stale `full_suite_guard`. Three consequences:
(a) editing a guard in a worktree changes NOTHING until the branch reaches main;
(b) to exercise a guard change in place, set `GENESIS_HOOK_DEV_LOCAL=1` — it
runs the worktree copy and ANNOUNCES itself on stderr, so it is never a silent
downgrade; (c) **a main checkout behind `origin/main` runs a STALE gate in every
worktree** — MEASURED 2026-09-10 at 7,535 lines local vs 8,971 on `origin/main`,
a 1,436-line divergence that had been silently deciding every gate call. After
merging a PR, sync the main checkout, or the gate you are testing against is not
the gate that ships.
Two earlier versions of this bullet were wrong in opposite directions: one said
such an edit "changes nothing until `sync-hooks.sh` copies it" (wrong mechanism),
the replacement said it is "live on the very next Bash call" (wrong in every
worktree, which is where this repo mandates you work).
- **`.github/**` and prompt surfaces are never "docs"** for the doc/config
auto-allow (`_is_docs_or_config` returns False for both). They are NOT
equivalent beyond that: only a PROMPT surface forces `substantial` and thereby
the depth gate. A lone sub-50-line `.github/` workflow edit classifies as
`inline` (its `_scope_tag` is `config`, which is not domain-sensitive), so
Rule 2.5 demands nothing. An earlier version said both "always reach the depth
gate", which over-states the workflow half.
**CI is not one check.** `ci.yml` defines **15** jobs. `review-depth-check` and
`cc-pin-receipts` are **Advisory BY DESIGN** (they always exit 0); a red result
from another job blocks the local gate. `dependency-audit` has a narrower
exception: the audit command reports vulnerability findings without making the
job red, while setup or installation failures can still fail the job. (An
earlier version said "ten of eleven … only `review-depth-check`", wrong on both
halves; count the `jobs:` keys and inspect the steps rather than trusting any
prose figure, including this one.) Only THREE checks — `test`, `leak-detector`,
and `lint` — are server-side REQUIRED, so a red local-gate job outside that trio
does not block GitHub. Several are
reproducible locally BEFORE pushing, which is far cheaper than a red PR:
```bash
ruff check src/ tests/ scripts/
python scripts/check_external_io.py
python scripts/check_subsystem_map.py
python scripts/check_shared_artifact_consumers.py
python scripts/check_frozen_clock.py
```
Do NOT run the full pytest suite locally (it is banned, and the concurrent-test
guard blocks a second run anyway) — CI's `test` job is the blocking one.
**Detecting a running pytest: match argv STRUCTURE, never a substring.** Any
check that greps command lines for `pytest` matches ITS OWN command line —
`pgrep -f "python -m pytest"`, `ps | grep`, and a bash `case *pytest*` all
self-match, as does another session's wait-loop. Test `argv[0]`'s basename, or a
python interpreter with an adjacent `-m pytest`.
## Pre-Commit Gate
Verify before any commit:
- `git diff --cached --stat` — every file in the diff belongs to your work
- `git status --short` — check untracked files (should be staged or ignored)
- Review level applied matches the adaptive protocol above
- Staged files do not include secrets (`secrets.env`, `.env`, credentials)
- **New-Store Gate (anti-proliferation).** A new persistent store — a DB table, a
Qdrant collection, a file-plane under `~/.genesis/` — needs a written
justification for why an existing store cannot hold it, plus a note on how it
stays consistent with related stores (and its retention + backup path). The
memory subsystem already sprawls across ~9 logical systems / 3 physical planes
because this gate did not exist; a table that "felt cleaner" is how store #10
is born. Reuse an existing store or an existing convention (e.g. the
`~/.genesis/eval/golden/` install-local golden-set convention, #1143) unless
the justification is real. Prefer NOT reusing a store whose SEMANTICS differ
(don't shoehorn a store-health row into the model-eval `eval_runs` table just
because it is "a table that exists").
- **Private-data scan before every push (public repo).** Grep the ENTIRE diff
(`git diff origin/main...HEAD`) for private/identifying data — real names,
company/product names, emails, IPs, private career/project specifics, verbatim
user messages. Check ALL surfaces, not just prose: **source comments,
docstrings, and test fixtures/data** are the easy misses. Use a synthetic
stand-in in tests, never the real private artifact. (2026-07-01: a verbatim
private DM leaked via a test docstring + a code comment after the commit
message and PR body were already clean.)
- GROUNDWORK-tagged code not accidentally deleted
- New capabilities registered in `_capabilities.py` + bootstrap manifest
- **Conventional commit prefixes**: `feat:`, `fix:`, `refactor:`, `docs:`,
`test:`, `chore:`. Scope optional: `feat(ego): add cadence manager`.
Subject line under 72 characters. Dominant category wins if mixed.
- **NEVER push to main or merge into main without a PR and user approval.**
Enforced by PreToolUse hook.
- **Targeted tests during development.** Run ONLY the relevant test file(s)
for your changes. NEVER run the full test suite locally — CI handles that.
Check CI via `gh pr checks`. Bare `pytest` without a file path is banned.
- **Commit continuously**: after every logical unit of work. Uncommitted = lost.
- **PR closes a ledger item → cite `Ledger: <item-id>` in the PR body** (the
32-hex `session_ledger` row id, own line, e.g. `Ledger: 71337fab…`). The
repo-pulse worker auto-absorbs the row with PR evidence at the next session
boundary — deterministic, reversible via `session_ledger_update`. A bare id
mention WITHOUT the `Ledger:` marker is context, not completion (the pulse
only proposes it). Find ids via `session_charter` or the charter injection
block.
- **Anything a hook prints to CC's context has a hard, measured ceiling.**
The harness FILES a hook's stdout above **10,000 characters per hook entry**
(CC 2.1.246; undocumented, version-volatile — see
`docs/reference/cc-compatibility.md`) and shows the model a 2 KB preview;
the rest silently never arrives. The SessionStart injection is therefore
four `--part` entries with per-part budgets, CI ceilings on every tracked
identity file (`tests/test_scripts/test_context_injection_budget.py`), and a
watcher over the harness's own filings. Adding protocol text to an identity
file is a budget decision: put DETAIL in a reference doc and a pointer in the
injected file. If you add or grow a hook's stdout, measure it against the
cap; after a CC bump, re-run the probe (`GENESIS_CTX_PROBE_BYTES`).
**Emit through `scripts/hooks/hook_output.py`** — the single home of the
measured constant, whose `BoundedStdout` enforces the budget at the WRITER so
a block that forgets to check cannot overrun (2 of 12 blocks checked when the
budget was per-caller), and whose `print_json_bounded` trims named free-text
fields while never sacrificing the JSON envelope — an oversized advisory must
lose prose, never its `permissionDecision`. Only SessionStart,
UserPromptSubmit and UserPromptExpansion carry bare stdout to the model; a
Stop hook that `print`s on exit 0 is INERT.
**NEVER compute characters at the call site — say what the DEGRADE looks
like.** `out.emit_or_degrade(text, block=…, pointer=…, notice=…, reserve=…)`
settles every number (the divider, `print`'s newline, the closing-line
reserve, what is already emitted) and returns which branch it took. There is
no `fits()` for emitters to call any more, deliberately: five review findings
in one cycle were a caller re-deriving what the writer already knew — the
audit reserve counted twice, a `fits` that undercharged by one and approved
blocks the writer then destroyed, a 120-char reserve for a 206-char line, a
pointer reserved in a part that cannot emit one, a `keep` derived from the
budget constant instead of the room. The arithmetic was never hard; having it
in six places was. An AST test bans `.room`/`.fits` from the emitter so the
next "just one `fits` call" cannot reintroduce the class.
The general shape, which is the transferable part: when several call sites
must each remember to do something — bound their output, report a failed
read, escape untrusted text — that is a CONVENTION, and conventions are what
reviewers keep finding one instance of at a time. Move the obligation into a
chokepoint the callers cannot bypass, then LOCK the chokepoint with a test
that fails when someone routes around it. A chokepoint nobody is forced
through is a convention with better documentation.
## Never reinvent what GitHub already does natively
**Standing user rule, 2026-09-09.** Before writing repo-local code that decides
something about a pull request, branch or merge, check whether GitHub already
enforces it: rulesets and required status checks, CODEOWNERS, review-thread
resolution, stale-review dismissal on push, path-based labelling, issue and PR
templates. If it does, use that — and only depart from it for a specific,
concrete reason GitHub cannot meet, stated where the code lives.
**Why this is a rule and not a preference.** Enforcement written here is a
script that every install must have wired, correctly, forever; enforcement
written as GitHub config is one versioned thing that binds identically for
everyone and cannot drift between installs. MEASURED on this repo: hook WIRING
had silently diverged between two installs in a security hook, which is
invisible to CI because the scripts are tracked and the wiring is not.
`git_push_guard.py` is 8,971 lines (measured 2026-09-10; it was 8,222 on 09-09 — a figure in a durable doc is a claim with a date, re-measure rather than quote), and the fraction of it that only reads GitHub
state is the fraction that never needed to be local.
**How the layers actually split** — this is a division of labour, not a
migration, and the third layer is not a consolation prize:
- **GitHub config** takes what is a FACT ABOUT GITHUB STATE: is this check
green, is this branch behind, was this thread resolved, who owns this path.
- **A required check run** (issue #1670) takes composite judgements that still
read only GitHub state — CI rollup identity, review freshness, finding
scores. Port these one at a time, and DELETE each ported check from the local
guard in the same PR. Two enforcers of one rule is replica drift with extra
steps, and the copy nobody is watching is the one that goes wrong.
- **Local hooks** keep everything reading THIS BOX or shaping THIS SESSION:
review evidence and markers under `~/.genesis/`, override sigils, the
bash-safety and destructive-command guards, the round caps. These are not
merge conditions at all — a PreToolUse guard runs before a command executes,
which is a place GitHub has no presence. Round caps in particular: a research
pass across production OSS review tooling found no implementation of them, so
that machinery is ahead of the field rather than behind it.
**The check before you build:** name the GitHub feature you considered and why
it does not fit. "I did not think to look" is the failure this rule exists to
catch, and it is invisible afterwards — a hand-rolled implementation of a
platform feature looks exactly like a hand-rolled implementation of something
novel.
## Generalizability Gate — build for ANY install, not this one
Genesis is a public, cloneable system. Every change must work on ANY user's
install, not just the machine it was written on. Standing user directive.
**Hardware/scale adaptivity.** Other installs have different RAM, disk, CPU
count, and workload scale. Never hardcode absolute resource numbers or scale
assumptions:
- Memory/disk caps: percentage-of-available or config-derived, never fixed
GB (precedent: #1029 percentage-based memory caps). Concurrency: derive
from `os.cpu_count()`/config, never a literal core count.
- Hard minimums are allowed but must be EXPLICIT (documented in install
docs/config comments), not implicit assumptions that fail mysteriously.
- Workload scale varies (PR velocity, table sizes, transcript sizes):
enumerate with pagination/bounds and LOUD truncation markers, never
silent caps (precedent: repo-pulse `limit_hit`).
- Optional dependencies AND optional infrastructure (Ollama, GPU,
individual API keys, a host VM/guardian, Tailscale, voice/edge hardware)
must degrade gracefully behind detection/config — presence is never
assumed (precedent: Ollama-optional, `API_KEY_VOYAGE`-gated reranker,
guardian features no-op without `guardian_remote.yaml`).
**No install-specific values in code.** IPs, hostnames, usernames, absolute
`/home/<user>` paths, GitHub slugs, timezones: these belong in generated
local config (`~/.genesis/config/genesis.yaml`, written by
`setup-local-config.sh`) or config overlays — never in committed code,
defaults, or tests. Resolve repo paths via `genesis.env.repo_root()` /
`genesis_db_path()` (GENESIS_REPO_ROOT-aware); resolve GitHub slugs LIVE
(`gh repo view --json nameWithOwner`) — a configured slug can name a
real-but-wrong repo and return plausible stale data. Shipped config defaults
must work on a fresh install with ZERO overlay.
**Leak-detection patterns follow the same rule — never hardcode an install's
private literals into a tracked scanner.** A public repo's CI grep / gitleaks
rule / commit-msg hook / contribution sanitizer must ship only generic CLASS
patterns (all RFC1918, IPv6 ULA per RFC 4193, `/home/<user>` shapes — see
`scripts/check_portability.sh`). This install's SPECIFIC literals (its
hostnames, subnets, ULA prefixes, private repo name, timezone) live only in the
GENERATED `~/.genesis/release-fingerprints.txt` (built by
`genesis.contribution.fingerprints` at bootstrap; hand-edited section preserved,
backed up via `backup.sh`) and — opt-in — the public repo's
`GENESIS_PRIVATE_PATTERNS` Actions secret. A scanner that must exclude its own
definition files from scanning is self-allowlisting a leak. See procedure
`public_repo_leak_detection_design`.
**Tenant-neutral, not tenant-shaped.** Genesis is a single-user sovereign
system. Build clean single-user code; do NOT pre-genericise for multi-tenancy —
no `tenant_id` columns, ACL tables, or context objects that always resolve to
one identity — absent a committed multi-tenant requirement. Premature
genericisation taxes every change with abstraction for a customer that may never
exist, and the reliability work that WOULD precede multi-tenancy (a fail-closed
data boundary, consolidated stores, provenance) is worth doing on its own
single-user merits and makes the eventual retrofit easier as a side effect.
When that requirement lands, tenancy is a well-understood retrofit — not
insurance to carry now.
**Deploy-path answer required — "how does this reach other installs?"**
Every PR must have an answer for both an EXISTING install and a FRESH clone.
Merged-but-undeployable-elsewhere is a bug. The standard paths:
| Change type | Deploy path |
|---|---|
| Runtime code | `scripts/deploy_code_only.sh`, or `update.sh` for dependency, unit or host changes. Modes: `deploy` (the default: a locked pull + restart that queues behind update.sh and validation holds, refuses a dirty tree or a venv that does not match the pyproject, and stands the Guardian and watchdog down); `deploy` and `restart` also refuse while the server is running work a restart would cancel, naming each item: what the server itself reports at `GET /api/genesis/inflight` (internal bearer; every Claude invocation, plus a dispatched session's or CLI reflection's whole life, from before its Claude process starts until its result is delivered; NOT the short tail other subsystems run after their call returns, such as a chat turn delivering its reply: #2917), and any live Claude process below the server. They proceed only with `--allow-killing <item,…|all>` using the items the refusal prints (`<pid>@<start>` or a reported id); a server too old to answer (404) gets the process check alone, and any other failure to ask refuses unless `all` — a session the server launched must hand the restart off rather than override (launched detached, the script cannot tell it is one of them); `update.sh` does NOT refuse, and its restarts end those sessions too; `pull` (the same pull, no restart: it reports every change the running server has not loaded, and the next step); `restart`; `status`. Launch `deploy` and `restart` DETACHED as a transient unit (`systemd-run --user`, the command is in the script's header): a session's background job dies with the session (#2101). **Validators sample, they don't chase:** deploys need not be 1:1 with merges. Validate `main` at a cadence, and on a failure bisect the merged range; per-PR CI already covers per-merge granularity. Validating against the live server? Hold `flock -s -w 7200 "${GENESIS_HOME:-$HOME/.genesis}/locks/update.lock" <cmd>`, and deploy BEFORE taking the hold. The hold stops locked deploys, NOT every restarter (the watchdog, the dashboard's service routes, Guardian recovery) and not a bare `git pull`, so bracket the run and let the script judge it: keep the `bracket:` token `scripts/deploy_code_only.sh status` prints at the start (from a worktree it reports the main checkout), and run `scripts/deploy_code_only.sh status --verify <token>` at the end; exit 0 is a valid run. A token exists only when the server's boot commit is known, HEAD's runtime files (`src/`, `config/`, `pyproject.toml`, and the scripts it keeps imported, `_RUNTIME_RELOAD_SCRIPTS` in `scripts/lib/deploy_status.sh`) are the ones it booted from and were at every commit HEAD held since the boot (after a `pull` of code, even one a later commit undid: restart first — the server imports lazily, so the detour's modules stay loaded), and nothing there is edited outside git; otherwise it reads `unknown (<why>)`, which no token matches. It covers a restart (boot commit, MainPID, systemd invocation id) and an edit to an ignored override such as a `config/*.local.yaml` or to a user overlay in `~/.genesis/config`, and a change at HEAD to a script the server runs afresh (`_RUNTIME_FRESH_SCRIPTS`: voids the token, needs no restart); HEAD may move over docs, or hooks and scripts the server never runs. It is a tripwire, not a certificate: it cannot see, and reads valid through, a change to the venv's installed packages, the other files the server reads from `~/.genesis/config` (only the `*.local.yaml` overlays are fingerprinted), an edit undone without moving HEAD (a stash and its pop included), and a rewritten or backdated reflog. A daemon your command leaves behind keeps holding the lock, and overlapping holds can starve a waiting deploy (flock grants a late shared request ahead of a queued exclusive one) |
| DB schema | additive idempotent migration — applies at restart |
| One-off data fix / backfill | data-migration framework (post-boot, idempotent) — NEVER a hand-run script only this install executed |
| **Naming either migration** | **UTC timestamp id: `` `date -u +%Y%m%d%H%M%S` ``_description.py** (data migrations prefix a `d`). NEVER hand-pick the next number — the legacy 4-digit namespace is FROZEN and CI refuses a new one. An id you have to CHOOSE is an id two branches choose identically: measured 2026-09-03, one PR was renumbered twice in a day and four open PRs held live collisions, while a duplicate prefix aborts bootstrap on every install. Nobody allocates a timestamp. |
| **Changing an EXISTING migration** | **Don't — add a new one.** The legacy 4-digit set is frozen by ENUMERATED FILENAME, so renaming or deleting one fails CI. Installs that already applied `0050` will never run a renamed `0050_*`, while fresh installs will — two schemas diverging with nothing to notice. Discovery also REFUSES to run anything from a directory holding a file it cannot classify (a mistyped 13-digit id used to be skipped in silence, so the migration never ran at all). |
| Config default | repo config file (+ optional local overlay); works with no overlay |
| systemd unit / timer | registered in bootstrap.sh AND the update path — never hand-`systemctl enable`d only here |
| Hooks / MCP servers | land at next CC session start (note the mid-window in the PR) |
| Guardian / host VM | `update.sh` redeploy (Host-Deploy Gate below) |
**When a change CANNOT deploy through the standard paths** (one-time host
action: packages, sudoers, cgroup settings, firmware), it must ship one of:
(a) a gated self-heal that reconciles on a recurring tick (precedent: the
guardian's swap reconcile — checks every tick, repairs config + live state,
opt-out flag), or (b) an explicit, documented operator step in CHANGELOG +
install docs. Silent "works here because I hand-fixed it" divergence is the
failure mode this gate exists to kill — it bites hardest on guardian/host
changes.
**Empty-state correctness — a fresh install is state zero.** Every feature
must behave correctly with NO accumulated state: empty tables, no history,
no cursor files, first run ever. First runs bound their own work
(precedent: repo-pulse `lookback_days` — never "all history"); readers of
possibly-absent tables degrade explicitly (precedents: dashboard
`charters_available: false`; charter injection byte-identical when the
migration hasn't applied yet). Test the zero state, not just the populated
one — "works here" often means "works with two years of accumulated state."
**External-tool version drift.** Other installs run different versions of
`gh`, GitNexus, Node, and Claude Code — and upgrade on their own schedule.
Never key logic on one version's observed behavior without a fallback:
prefer first-class config over output-patching, and keep the patch as a
safety net when older versions ignore the config (precedent: `.gitnexusrc`
+ the strip job for rc-unaware versions); parse external-tool output
fail-closed against the LIVE stream, never assumed semantics; pin versions
only where the system owns the pin (`cc_version.sh` + cc-align).
**A settings lever for every autonomous behavior.** Anything that acts
without a user in the loop — detached workers, scheduled jobs, auto-writes
— ships its operator lever in the SAME PR: a settings domain
(`off | propose_only | live` or equivalent) plus an env kill switch, with
invalid values degrading toward LESS write authority (precedents:
`repo_pulse` domain + `GENESIS_REPO_PULSE_DISABLED`;
`session_ledger_shadow` live-coerced to shadow). Another operator must be
able to turn your feature off — or cap its authority — without editing
code. This is "the user decides tradeoffs" applied to every install.
**Retention for every unbounded store.** Any table, log, or directory that
grows without bound ships its prune path in the SAME PR, wired into
`disk_hygiene.sh` or an existing retention tick (precedents: repo-pulse
45d prune; ledger-shadow 45d prune; label-aware attention-snapshot GC;
hook audit stores 5 MB size trim). An unbounded store is a slow disk-leak on
someone else's smaller disk — retention is part of the feature, not a follow-up.
**But retention machinery does not belong on a hook path.** `disk_hygiene.sh` IS
the retention path; a guard computing a security verdict must not also be grooming
a store. Two measured reasons, both from the same feature: a helper whose work
scaled with the command exceeded a hook's registration timeout, and a hook killed
before its `exit 2` lets the refused command RUN; and an in-hook retention engine
whose failure mode is rewriting a file produced most of that feature's defects
across five review rounds. Prefer a store shape the daily timer can bound by
deleting whole old files — that is what makes the split possible at all.
**Install-agnostic tests.** Tests must pass on a fresh clone with no
Genesis services, no live DB, no network, no `gh` auth, no local config:
synthetic fixtures only (never real usernames/slugs/IPs — doubles as the
privacy gate), injectable runners for external commands, `tmp_path` over
real paths, no wall-clock dependence. CI on GitHub's runners IS the
reference "different install" — anything a test can't exercise there needs
an injectable seam, not a skip-on-my-machine guard.
## Host-Deploy Gate (merged ≠ deployed)
A merged PR that touches host-deployed paths is **NOT done at merge**. The
guardian and the host VM only pick up changes when `scripts/update.sh` runs —
merging and walking away leaves the host running stale code indefinitely
(observed live: a host guardian sat 3 PRs behind for a week because every
session assumed deploy "happens somehow").
**Trigger paths** (match = this gate applies): `src/genesis/guardian/`,
`scripts/guardian-gateway.sh`, `scripts/install_guardian.sh`,
`scripts/host-setup.sh`, `scripts/update.sh`, `scripts/lib/cc_version.sh`.
**After merging such a PR, in the same session:**
1. Run `scripts/update.sh` from `~/genesis`, DETACHED from the session with
the `systemd-run --user` recipe in the Timeout Policy above, never in the
foreground or via `run_in_background` (it redeploys the guardian when
guardian-relevant paths changed and heals host/container CC + Node pin
drift — including on a no-delta run).
2. Verify the deploy landed: gateway `version` op reports the expected
`deployed_commit` / CC version; guardian tick healthy in its journal.
3. State the deploy + verification result explicitly in the wrap-up. If the
deploy cannot happen this session, record it — never leave deploy as an
implicit assumption. A purely LOCAL blocker (host unreachable tonight) is a
`follow_up_create` row. A blocker exposing a REPO-level gap (a missing reconcile
mechanism, no self-heal path) needs BOTH: an issue for the mechanism fix, AND a
local row for the fact that THIS install is still undeployed — another
contributor can close the issue without ever touching this host, which is
exactly the stale-host failure above.
**The reverse direction is equally binding**: host VMs are deploy targets,
never edit-in-place dev environments. An emergency hand-edit on a host gets a
same-day PR that lands the same change at source — a host divergence that
outlives its incident is a bug.
## When to DRIVE a Merge (standing user rule, 2026-09-05)
A session does not shepherd its own PRs toward merge by default: open the
work and let the PR bake in the normal flow. DRIVING is the UNPROMPTED extra
cycle — polling a quiet PR's gates, soliciting re-review with nothing new
pushed, initiating a merge on a PR that still has un-green gates — and it is
justified ONLY when:
- tracked work is GATED on that PR's merge: the PR body carries a
`Ledger:`/`Follow-up:` completion marker, a hot follow-up's close depends
on it, a stacked branch needs its base, or a live hazard closes with it; or
- the user asks for that PR by name.
Three things are NEVER driving — they stay mandatory. They bind **whichever
session currently OWNS the PR**, which is not always the one that wrote it:
under "Where your session ENDS" above (owner ruling, 2026-09-14) a BUILD
session's ownership ends when the PR opens, and the closing/review session that
picks the queue up owns them from there. So "answered before you stop" means
answered by whoever holds the item when the finding arrives — it does not reach
back and re-attach a review loop to a build session that has already handed
off. A finding landing on a PR nobody holds is the drain's intake, not a
builder's debt.
- **Answering.** Findings received while you are present are answered before
you stop (the zero-drop rule), and the mandatory post-push
`@codex review` (Gate Machinery above) is part of answering. A fix round
you have begun is FINISHED, not "chased": answered, pushed, re-review
triggered — never abandoned half-answered.
- **Closing a green PR.** A PR already fully green at head is past baking:
merging it under the standing pre-approval (or asking, where none exists)
is closing, not driving — green DECAYS with base drift and head-freshness
rules, so a cleared gate is a now-or-re-earn asset.
- **The stopping handoff.** A session that stops while its own PR has any
gate un-green creates a `ready` follow-up naming the PR and the event it
awaits, BEFORE stopping. That row is the drain's intake — an un-driven
open PR without its row IS a stranded artifact under the zero-drop rule,
never "the normal flow". Until a dedicated closing station exists, the
queue drains through those rows under the project's ≈51/49 lean.
Scope against CLAUDE.md's ≈51/49 close-before-open lean: unchanged for
closing-shaped work (answering, merging green PRs, the handoff row). What
this rule removes is only the WATCHING — unprompted gate-polling and
review-soliciting between a push and the next external event. When you do
merge, the Pre-Merge Gate below governs unchanged.
### Which one first — prefer a `fix`, because it is usually SMALLER (standing user rule, 2026-09-10)
The section above says WHEN driving is justified. When several PRs qualify at
once, the owner's stated preference is the tiebreak: **a fix restores existing
functionality that is broken; new functionality can wait.** This is ONE axis in
a larger calculus — a gated PR, a live hazard, a user's named request all still
outrank it — not a hard ordering.
MEASURED across the 65 open `fix`/`feat` PRs on this repo, 2026-09-10:
| kind | n | mean added lines | ≥1000 lines | findings-blocked | mean findings score |
|---|---|---|---|---|---|
| `feat` | 30 | 2,024 | 67% | 77% | 3.00 |
| `fix` | 35 | 804 | 29% | 66% | 2.77 |
Chained to the round data: **86% of PRs over 1,000 lines reach 3+ Codex rounds,
against 6% under 200.** So the mechanism is `feat` → bigger → more rounds → more
of the maintainer's conscious decisions consumed, because past the escalation cap
every additional round costs a fresh human sign-off.
**The honest limit, which changes how to apply this.** The findings-score gap is
modest (3.00 vs 2.77; 77% vs 66% blocked), so *"feats attract more findings per
se"* is WEAKLY supported. *"Feats are bigger, and size drives rounds"* is
STRONGLY supported. The rule is therefore the SIZE axis wearing a `feat:` prefix
— which means **a small feat is not deprioritised, and a 2,000-line fix does not
get a pass.** Read the diffstat, not the prefix; a rule keyed on the prefix
rather than on the property would be wrong on exactly the cases where the
tiebreak matters.
### Never RETIRE a PR you are not the one reviving (standing user rule, 2026-09-08)
Terminology first, because the section above already uses "closing" to mean
merging: **RETIRING** is `gh pr close` — abandoning a PR unmerged. This rule is
about that, and only that. Merging a green PR is unaffected.
**A reviewer session retires nothing.** When a PR turns out to be wrong at the
premise — the mechanism cannot work, the design is contested, the fix belongs on
a different event — the disposition is `needs-architecture-session` **on the PR**,
carrying the evidence, and the PR stays OPEN. Retiring it belongs to the session
that actually takes up its revival, at the moment it takes it up.
Why the split, since "it's clearly dead, just close it" is the rationalization
this rule exists to stop:
- **A reviewer has read the diff, not the intent.** You can establish that a
mechanism does not work. You cannot establish that nothing in the branch is
worth reviving, that the author holds no context you lack, or that the right
successor design does not reuse most of it.
- **An open PR carrying an evidence-bearing comment is a live handoff; a closed
one is an archaeology task.** The next session finds an open PR by listing the
queue. It finds a closed one only if someone remembers it existed.
- **Retiring costs the review record its addressability.** Threads on a closed
PR stop being where the conversation happens, so the reasoning that just cost
a review round stops being read.
So: post the finding, flag it, leave it open, and let the reviving session
decide — including deciding to retire it in favour of a successor, which is that
session's call to make and to justify.
**A superseded PR is the one exception (standing owner rule, 2026-10-04).** The
closing session may retire it when all three hold:
1. **The owner authorized the supersession**: the decision to replace this
approach with another is the owner's, on record, not a reviewer's inference.
2. **The successor PR is open**, or already merged, so the work has a live home.
3. **It is superseded completely, in every part.** Walk the old PR's files one by
one against the successor and current `main`, and decide each as covered (name
the `file:line` that now does it) or moot (say why it no longer applies). One
part neither covered nor moot keeps the PR open, with a comment naming that
part.
The closing comment carries that per-file mapping and names the successor, so the
reasoning stays addressable after the close. Short of all three, name the
successor in a comment and leave the PR open.
If you believe any other PR should be retired and nobody is picking it up, that
is a question for the user, not a judgment call for the review station.
### Keep the PR the PR — adjacent findings become issues (standing user rule, 2026-09-09)
**A review session's job is to get THAT PR merged.** Reviews routinely surface
bugs that are real, pre-existing on `main`, and nothing to do with the diff in
front of you. Those do not belong in the PR and they must not hold it up.
Route every finding by ONE question — **does the PR work without this fixed?**
- **No, the PR is broken without it** → fix it in this PR. The builder session
should have caught it, and shipping a PR that cannot function is not a merge.
- **Yes, the PR works** → **file a GitHub issue and merge the PR.** Do not grow
the diff, do not open a discussion, do not park it in a reply and move on.
**No per-instance approval to file.** Do NOT ask — asking each time was the
friction this rule removes, and the owner relaxed the general rule on
2026-09-09 after reading what actually got filed. The gate on a public post is
now WHAT GOES IN IT, not permission: scrub anything personal or identifying —
names, hosts, IPs, paths embedding a username, anything about the user's
real-world life — so the issue carries technical detail only. Borderline, in
either direction? Ask. See CLAUDE.md, "Where deferred work goes".
**The exception that is NEVER waived: a security defect is not filed publicly
before it is fixed.** It goes to a private record under `~/.genesis/output/`
plus a `follow_up_create` row, and nothing about it reaches a public surface —
the relaxed filing rule above does not touch this one.
**The test is WHO GAINS, not the word "bypass"** (see CLAUDE.md, "Where
deferred work goes", for the full rule this mirrors). Security-class means
disclosure hands someone a capability they do not already have: a credential
exposure, an auth or privilege bypass, an injection path, anything reachable by
a party with LESS access than it grants. A fail-open in a LOCAL development
guard is usually not that — it runs only where an install wires it, whoever can
trigger it already has commit access to that checkout, and the worst case is
typically an under-reviewed change reaching a PR that still waits on maintainer
approval. Apply the test rather than the label; the catch-all phrasing this
replaces swept in most of what this repo builds. The exception the test catches
and the label does not: a guard whose job is stopping a SECRET or PRIVATE DATA
from reaching a public surface is security-class however local it is, because a
branch on a public repo is public the moment it is pushed, merged or not.
Write the issue while the context is in your head — the measurement, the
file:line, the falsifier, and why it was out of scope for the PR. An issue that
merely names a symptom costs the next session the whole investigation again, and
you already did it.
Why this is the rule: a review that widens into every adjacent defect stops
being a review and becomes an unbounded refactor, which is how a two-finding PR
turns into a six-round loop. The PR is the unit of work. The queue is the place
the rest of it goes.
### Before reviewing an OLD PR, check it is still work (standing user rule, 2026-09-15)
A review session drains a queue sorted by age, and the older the PR the more
likely it is no longer work — its content already merged under a different
number. Observed twice in one session on 2026-09-15 (unquantified: nobody
counted the old PRs that WERE still work, so treat this as a prompt to check,
not a base rate).
**Squash merges are why this is not obvious.** A squash rewrites the commits, so
the branch's own history never becomes an ancestor of `main`. MEASURED on a
two-commit branch whose work had landed as one squashed commit:
| test | result |
| --- | --- |
| `git merge-base --is-ancestor <branch> main` | rc 1 — reads as UNMERGED |
| `git cherry -v main <branch>` | `+` on BOTH commits — reads as unique |
| `git diff origin/main..HEAD -- <files>` | **empty — correctly detects it** |
`git cherry` compares patch-ids, so it DOES catch a squash of a single commit —
and is blind exactly when the squash combined several, which is the common case.
Do not rely on it.
```bash
git fetch origin main --quiet # a stale ref decides this test otherwise
git diff origin/main..HEAD -- <the files it claims to change> # empty ⇒ already there
```
TWO DOTS, not three. `origin/main...HEAD` is merge-base→HEAD: it ignores
everything that landed on `main` after the fork point, including the squash that
carries the work, so it is non-empty for a superseded PR every time. It is
exactly as blind as the ancestry tests above. (Caveat on the two-dot form: a
RENAME on `main` makes the old path read as a new file, so a renamed-and-merged
change still looks like work.)
Once you have a commit ON `main` that you suspect carries it, this NAMES the
successor — note it answers "which PRs contain this commit", so feeding it the
open PR's own head just returns that PR:
```bash
gh api repos/<owner>/<repo>/commits/<sha-on-main>/pulls --jq '.[] | "\(.number) \(.state)"'
```
**A second, separate question: is the tree you are standing in at the PR's
head?** A worktree created from a branch NAME sits wherever that ref pointed
when it was created. That check is already specified — with the fetch it needs,
the `main`-ancestry companion, and the reasons a ref comparison alone is not
enough — in the `closing-session` skill under *Establish freshness by comparing
REFS*, and in **Date the code before classifying a red** above. Use those; do
not re-derive a shorter version here. Note also that `rev-parse HEAD` is blind
to uncommitted edits, so check `status --porcelain` too.
MEASURED 2026-09-15, one worktree, the cost of skipping it: a tree four commits
behind its PR head produced a CRITICAL finding, a full adversarial audit, a
public issue, a commit and review evidence — all describing a defect the branch
had already fixed in a commit that tree could not see. The same audit declared
two functions absent from the codebase when both exist at the head, which turned
a reviewer's correct finding into a rejected one. Every measurement was true of
the tree it ran in and false about the PR.
**Treat a finding that is too good as a staleness signal first** — a defect that
would break the feature outright, a symbol a careful author somehow forgot.
Re-read the head before writing it down, and certainly before filing it
publicly. That check is cheap and unconditional; the judgement about whether a
finding is "too good" is not, which is why the action does not depend on it.
**A subagent inherits your staleness silently** — point an audit at a worktree
path and it will measure that tree with complete confidence. Put the head SHA in
the dispatch prompt and ask it to verify the match first.
**When a PR IS superseded: name the successor in a comment.** Leave it open
unless all three conditions of the superseded-PR exception in *Never RETIRE a PR
you are not the one reviving* above hold: owner-authorized supersession, the
successor open or merged, and a per-file check showing it superseded in every
part. Do not "rebase and revive" it either; that is how the same change lands
twice.
## Pre-Merge Gate
> **This is closing-session territory.** A build session's work ends when the PR
> is open (see "Where your session ENDS" above); driving it through this gate is
> the **`closing-session`** skill's job. The mechanics below stay here because
> they are the authority — that skill composes them rather than restating them.
> Read on when you are the one at the gate.
**Canonical pre-merge check:** run
`python3 scripts/hooks/git_push_guard.py --check-pr <N> [--repo OWNER/REPO]`
BEFORE proposing a merge. It runs the SAME check functions the enforcement gate uses, in this REPORT order
(mergeable → CI → base-invariant → pin-receipts → e2e-plan *(advisory)* →
Codex-freshness → scheduled-Claude-review → review-body → inline findings),
so the two use the same checks. The merge arm checks review-body and inline
findings before scheduled review, and additionally applies the
`--match-head-commit` binding, which has no report row —
this `--check-pr` read IS the mandatory pre-merge step: **always run it and read
the PR's automated-review comments (Codex, leak/CI, the scheduled Claude review)
before any merge** — never hand-roll a
gh/jq review check (a hand-rolled query once used the GraphQL bot login on the
REST endpoint, matched nothing, and reported "Codex clean" while P2s sat unread).
When all gates pass it prints the exact atomic merge command to copy
(`... --match-head-commit <verified-head>`); use that command verbatim.
`git_push_guard.py` enforces a **hard gate** at merge time. The gate reads only
the `gh pr merge` spelling, so the guard refuses a pull-request merge spelled
through the GitHub API instead: a non-GET to REST `pulls/N/merge` or
`merge-async`, a GraphQL `mergePullRequest` / `enablePullRequestAutoMerge` /
`enqueuePullRequest` mutation. It is a closed set: a `gh api` call that names a
merge passes only when it is a plain read (one literal endpoint, read-only flags),
and a GraphQL query or PUT endpoint it cannot read is refused too. The refusal
names the gated command. The recogniser is `scripts/hooks/gh_merge.py` (#2768);
it is a tripwire for ordinary spellings, not the boundary, which is server-side.
Only the bare `gh pr merge --help` skips the merge arm. Beyond the review
findings below, a gated `gh pr merge`:
- must carry `--admin` (explicit approval flag) and be bound to the reviewed head
via `--match-head-commit` (GitHub rejects it server-side if the head moved —
TOCTOU defense); the `--check-pr` command supplies this;
- requires Codex to have reviewed the **current head** — Codex does NOT auto-review
a later fix-commit, so comment `@codex review` and wait after any push.
**A clean Codex signal counts only when its id RESOLVES to the head.** Codex posts
NO review object when it finds nothing; its word on a clean head is a comment — a
clean re-review ("Codex Review: Didn't find any major issues. … **Reviewed
commit:** `<sha>`") or a `✅ Completed` Code Review row in its PR summary comment
(on PR open, often the only signal: #2418). Both name the commit by an ABBREVIATED
id, and a prefix that merely MATCHES the head binds nothing — the head is whatever
the branch's author pushed (#2487). So the gate resolves the id repo-wide via
`GET repos/{owner}/{repo}/commits/{short}` and accepts it only when GitHub resolves
it to exactly the PR head; a 422 (ambiguous or unknown) refuses it. Matching the
head implies the commit is the PR's. The comment must be the Codex Bot's, no Codex
review object (any state) may sit at the head, and no Codex issue comment on the PR
may carry findings (Codex usually files findings as a review object, but MEASURED
on 2 of 339 PRs it posted them as a `💡` issue comment with none). Any non-Codex edit
or deleted edit revision on any Codex comment permanently refuses clean signals;
the block names the editor when available. Edit-history completeness is required
only for Codex Bot comments; unrelated comments' edit data is ignored. History
veto covers head force-push, head-branch deletion or restoration, and base changes.
A base change stays a veto because the clean signal names the head, not the base
Codex reviewed against; retargeting changes the effective diff without moving the
head. A base force-push is retired: merging requires the default base, whose
ruleset forbids force-push and deletion, so a force-pushed non-default base can
reach a merge only through a base change, which vetoes. All 541 commits dropped
by 191 force-pushes still resolve repo-wide by 7-hex id, so dropped head commits
are not the binding risk.
A branch or tag exactly named after a short id can shadow GitHub's commit lookup
(which uses git name-guessing rules); measured `pull/2720/head` and `main` resolve.
Creating a shadow ref needs base-repo push rights, held only by the owner, and no
hex-named ref exists; fork authors cannot create one. A deleted comment leaves no
trace in the API. The block message says which clean signal it read and why it was
refused, and when no later clean signal can count it says a finding-free re-review
cannot help. `--check-pr` labels a pass from the gate's own record for its verified
head; a subsequent head move is stated without relabeling the original pass, while
merge-with stays bound to that verified head. When the signal does not resolve,
the routes are an owner-approved `# substitute-review` on another reviewer's review
at that exact head, or a conscious
`# stale-review-override` (which on the hook surface also needs fallback evidence).
**Smart-delta narrowing:** a STALE review passes anyway when the unreviewed
delta (`reviewed...head` via the compare API, classified by `review_scope`
substantiality) is provably review-trivial (docs-only / a small single-file
touch-up) — the merge is then still bound to the exact head that was
classified. A substantial or unclassifiable delta blocks; an ABSENT review
always blocks;
- requires **every scheduled Claude review at the current head**. Each scheduled
Claude review (a `/schedule` cloud routine) posts as the repo OWNER's account and
must carry a marker `<!-- genesis-scheduled-review: head=<full-40-hex-sha> kind=<name> -->`
naming the exact head it reviewed AND which routine it is (`kind`). The gate blocks
unless an owner-authored marker for EVERY effective required kind
(`_required_scheduled_review_kinds()` — DEFAULT `leaks` ALONE, which is also the
irreducible leak/secret scanner, so the default set and the floor coincide;
`code-review` is ADVISORY by default because no routine emits its marker — measured
2026-09-16, zero `code-review` markers across all 51 open non-draft PRs and the 40
most recently merged ones, against 47 and 31 `leaks` markers — and an install that
DOES run one re-arms it by naming a LARGER set in
`merge_gate.required_scheduled_reviews: [code-review, leaks]` in local `genesis.yaml`)
names the PR's current head — so if any required routine never ran, ran on a stale
commit, or was rate-limited, the merge blocks (naming the missing kinds). An ADVISORY
routine still posts its review on the PR to be read/addressed, but its absence does not
block. The block message is an **inventory**, not a diagnosis: under each missing
kind it lists EVERY marker block the scan found that names that kind, with its
status, and hides nothing. Run `python3 scripts/hooks/git_push_guard.py --check-pr <N>`
— it renders those
rows, not just the summary line (whose `present: none` clause reads like "nothing was
posted" in every case below, and is the exact wording an operator was once measured
acting wrongly on). Row statuses you will see:
* *accepted at a DIFFERENT head* — a routine ran, then a push moved the head. Routines
are generally not re-run on a push; re-review the current head and post the marker;
* *REFUSED* (at this head or another) — the body reads as carrying a blocking finding
and no clean-verdict line overrides it; see the clean-verdict rule below. The message
deliberately does NOT print the verdict string, because a gate that prints the line
that makes it pass is explaining how to get past itself;
* *could not be counted: <reason>* — a head that is not full 40-lowercase-hex, an
empty or refused field value (quoted back verbatim), an author who is not the repo
owner, a dismissed review, a stale unpublished draft;
* *unscoped* — blocks naming no REQUIRED kind, listed and credited to nothing:
guessing which review a block "meant" would steer you into attesting for one that
never ran. A value carrying a `/status` suffix (`kind=leaks/failed`) lands here and
is flagged as a run that reported its own failure.
The one conditional is a COUNT, keyed on a fact: a kind with no OWNER-authored block at
any head gets "a routine may still be in flight — waiting is the right move", because
that is the only state where patience can help. A stranger's comment is not evidence
about the owner's routine and never silences that note; an owner's block in ANY state
(accepted elsewhere, refused, dismissed, stale draft, malformed) is, and does.
Why an inventory and not a diagnosis: the previous shape picked one cause per kind and
hid the rest, and every one of nine review findings across six rounds was a hidden fact
— a refused `[P1]` on an older commit hidden behind a typo at the current one, the only
evidence a review had ever run hidden by a drive-by comment. Precedence is the right
shape for a VERDICT; for a REPORT, hiding a true fact is never correct.
Or append `# scheduled-review-override` to merge anyway (the conscious "merge without
the scheduled reviews" case). Head match is EXACT for the LLM marker — no delta
tolerance, unlike the Codex freshness gate, which grants relief on a provably trivial
delta. That asymmetry is deliberate: the Codex classifier judges code-review
substantiality by file type and size, and an inferential leak lands in exactly the small
doc edit it would wave through. The ONE relief the leaks kind gets is MECHANICAL, not a
delta tolerance: an ACCEPTED `leaks` marker on an ANCESTOR commit of head satisfies the
gate when the `leak-detector` job of the `CI` workflow is green at head (identity pinned
to that (name, workflow) pair; `_MECHANICAL_RESCAN_BY_KIND`), and NEVER when any
refused `leaks` marker — or any blocking finding the scan could not credit to a head
or kind (a tie, a malformed marker) — exists anywhere in the PR (the head axis is not
a time axis — a later acceptance at an older head must not outrank a refusal). `--check-pr` renders a
carried marker as `ok (leaks carried from <sha>, leak-detector green at head)`, never
as `ok (at head)`. Measured motive: 6 of 10 sampled multi-push PRs were blocked purely
because the routine does not re-stamp after a push, and the override had become
routine. The marker means "ran **clean**", not merely "ran": a
review whose body carries a blocking finding (`[P1]`/`HARD BLOCK`/`### ERROR`, unless a
clean verdict overrides) is rejected, and DISMISSED/PENDING(draft) reviews don't count.
**ALWAYS end a genuinely-clean scheduled review with an explicit verdict line**
(`VERDICT: PASS`, or `PII/Secrets/Wording: CLEAN`). The blocking patterns are plain
substrings with no negation awareness, so prose like "not a hard block" or "no hard
blocks found" TRIPS them — measured on two 2026-08-28 PRs whose markers both contained
that phrase in negated prose; the one that also carried a clean-verdict line was
accepted and the one without it was silently refused. Without the verdict line a clean
review can be rejected on wording alone;
Fail-closed: an unreadable comments/reviews fetch BLOCKS (never a false all-clear);
- requires the PR base to equal the repo's default branch (retarget guard);
- blocks unless mergeability is a definite `MERGEABLE` (a failed/unknown read
does not merge).
- **the CI gate** blocks red/pending checks; and — on the canonical public repo,
where CI always runs — an `absent` CI state (a readable EMPTY check set = CI
never ran, the tell of a conflicting branch or a dropped `pull_request` trigger)
also blocks, so an un-CI'd PR can't merge. Likewise `incomplete` (a NON-empty
rollup whose present checks are green but a REQUIRED workflow contributed no
verdict — e.g. a lone green CodeQL after a workflow-specific trigger drop, or a
fully-SKIPPED suite): the required identity is the rollup `workflowName`, config
driven via `merge_gate.required_ci_workflows: [<names>]` in local `genesis.yaml`
(default `CI`; fail-closed to the default on any malformed/empty config — there
is no disable value). An UNREADABLE CI read (`unknown`) fails OPEN, and off the
canonical repo `absent`/`incomplete` fail open too (another repo may
legitimately have no CI, or a differently-named suite). Waive with
`# ci-override` (never `--admin`).
- **Override sigils are split by boundary** — `# review-override` waives ONLY the
finding scans (review-body + inline P1s); `# scheduled-review-override` waives
ONLY the scheduled-Claude-review gate; CI is `# ci-override`. Append several in
one trailing comment when several waivers are genuinely intended.
**`# stale-review-override` is the EXCEPTION and is wider than its name.**
Besides Codex-at-head freshness and the base invariant, it also drops the
`--match-head-commit` TOCTOU binding — the guard's own module docstring says it
"waives that binding for ALL gates" — and un-pins the scheduled-review check
from head, making it point-in-time. Mechanically the merge arm only enforces
the binding `if verified_head:`, and the force path leaves that `None`. So
reaching for this sigil while believing the merge is still pinned to the
reviewed head is exactly the unsafe read: a concurrent push CAN swap in an
unreviewed head. It is a conscious "merge without current verification"
choice, and the right fix for a stale review is `@codex review`, not the sigil.
⚠ An earlier version of this bullet claimed the split meant one waiver "can't
silently disarm an unrelated gate", and listed this sigil as waiving "ONLY"
the review-context gates. Both were false of this one sigil.
- **`# substitute-review` is the NARROW Codex stand-in** (owner standing order,
2026-09-24). When Codex has not reviewed the head, a NON-dismissed,
NON-pending review by another GitHub App reviewer (any, such as Devin or
CodeRabbit, except the PR's own workflow bot and CodeQL) whose `commit_id` is EXACTLY the head
satisfies the Codex freshness check. The owner approves each use IN
CONVERSATION before you add the sigil — the sigil is the record of that yes, so
never add it without asking (owner ruling 2026-09-24, which replaced a native
permission prompt the first version raised). The gate prints a NOTE naming the
stand-in and marks its override-log row `codex-freshness:used`; a dispatched
session is refused, since nobody there can have approved it. Every other gate
still applies. Unlike `# stale-review-override` it keeps the base-branch check and
the `--match-head-commit` binding (labelled with the reviewer that verified the
head), so no fallback-evidence file is needed on this path. Only a review
record with a BODY counts: an empty-body record at head is the wrapper GitHub
makes when the bot replies inside a thread, not a review of the head. With
both sigils present, `# stale-review-override` wins and the stand-in is never
used — it is already the broader waiver. `--check-pr` says when a stand-in exists:
`codex-at-head : BLOCK — … — substitute available: <reviewer> reviewed this
head`. Codex remains the official reviewer; this exists for its outages.
The review-findings gate specifically:
1. After CI passes, the merge hook automatically checks PR comments
for automated review findings (ERROR, [P1], HARD BLOCK).
2. If review present with **blocking findings** → merge is **BLOCKED**
by the hook (exit code 2). Fix the findings first.
3. Inline findings are SCORED, and **SEVERITY FLOORS while the LANE governs
VOLUME.** Two independent rules, checked in that order:
**(a) The always-fix floor, every lane, before the score is consulted.** Any
unresolved Codex **P1**, any unresolved CodeRabbit **Critical/Major**, or any
unresolved Devin **severe bug / critical security** finding (🔴/🟥) blocks the
merge outright — whatever the change is. This is
`floor_hits = len(p1) + len(cr_block) + len(devin_block)` in
`_check_inline_review_findings`.
It is a RULE because it used to be an ACCIDENT: before the lanes existed the
single threshold was 1.0 and a P1 scores exactly 1.0, so the floor held by
arithmetic, unnamed and untested — and raising any threshold would have
deleted it in silence.
**(b) The per-lane score threshold, for everything below the floor.** Weights
— **Codex P1 = 1.0 · Codex P2 = 0.5 · CodeRabbit Critical OR Major = 1.0
each · Devin severe/critical (🔴/🟥) = 1.0 · Devin non-severe bug or security
warning (🟡/🟨) = 0.5** (`_CR_BLOCKING_SEVERITIES = {"critical", "major"}`,
`_CR_BLOCKING_WEIGHT = 1.0`, `review_findings.DEVIN_MARKERS`) — but what a change can AFFORD
depends on what it costs to be wrong (`_INLINE_SCORE_BLOCK_THRESHOLDS`).
**Devin scores like Codex (owner ruling, 2026-09-24).** Its comments open
with `<!-- devin-review-comment {json} -->` and a marker: red is Devin's severe
tier (bug / critical security), yellow its non-severe tier (bug / security
warning), and 🔍 is informational, surfaced and never scored. The marker agrees
with the metadata `kind` on every comment measured. A
marker outside that closed set is FORMAT DRIFT: surfaced, never scored. A
finding Devin posted twice counts once (dedup on its metadata `id`). It is
parsed only for the `devin-ai-integration[bot]` Bot account, and its branch
runs before the Codex badge match, so a Devin body QUOTING `![P1 Badge]` is
scored once, as Devin. CodeRabbit Minor and below stay SURFACE-ONLY by owner
ruling — only Devin gained scoring.
**Clearing a finding:** a MAINTAINER's in-thread reply, as always — or, for
a DEVIN finding only, Devin itself replying in that finding's thread with a
first line starting `✅ **Resolved**` (Devin may withdraw its own finding; no
bot can clear another's, free prose such as "Fixed in …" clears nothing, and
Codex and CodeRabbit findings still need a maintainer reply). Devin's reviewer
and its builder post under ONE login, so a self-withdrawal is reply text, not
an identity: it still clears, but the gate always LISTS it as
`[Devin self-withdrawn]` so you can check the fix landed. A thread
resolved in the GitHub UI does NOT clear a finding; reply instead. A finding
Devin posted more than once counts once: each copy's diff and documentation
scope is judged on its OWN anchor, the finding scores on its strongest copy
that is eligible to score, and every copy carrying an unrecognised marker is
reported as format drift, answered or not.
| lane | blocks at | what lands there |
|---|---|---|
| `critical` | **1.0** | enforcement-hook surface · `.github/**` AND the implementations behind its required checks (`scripts/ci/**`, `check_*.py`/`.sh`) · schema and data migrations · HTTP surfaces (`dashboard/routes/**`, `hosting/**`, `api.py`/`api_*`, `_blueprint.py`) · secrets and credentials |
| `standard` | **2.0** | ordinary runtime code |
| `light` | **3.0** | PROSE (including prompt surfaces) / tests / fixtures only, or vendored-only |
**`light` is PROSE, not `docs-config`.** A `.yaml`/`.toml`/`.ini`/`.cfg`
reaches `_category() == "docs-config"` through the shared classifier, but
config is not documentation — `config/desktop_takeover.yaml` arms desktop
takeover and `pyproject.toml` pins dependencies, so both are `standard`. The
light lane is `.md`/`.rst`/`.markdown`/`.adoc`, plus `.txt` and the
extensionless form ONLY on a known documentation stem (`CHANGELOG.txt` and a
bare `LICENSE` yes, `requirements.txt` no — the same split `_is_doc_path`
makes), plus tests and fixtures (`review_scope._is_lane_light`). Those
spellings are matched DIRECTLY rather than behind `_category`, because
`_category` calls `.adoc` and an extensionless `README` **code** — so an
earlier draft that listed them behind it advertised prose formats nothing
could reach.
**A PROMPT SURFACE is prose here.** `_category` calls `SKILL.md`,
`.claude/commands/*.md` and `src/genesis/skills/**/*.md` *code*; the lane
reads them as what they are. MEASURED: this is the single biggest effect of
the lane's own vocabulary — 14 of 40 recent PRs classify `light` where the
inherited tagger said `standard` — and it is mostly INERT, because 11 of those
14 touch nothing whose findings score at all (every path is an `_is_doc_path`
and `doc_findings` defaults to `skip`). Where it bites is a prose-plus-TESTS
PR, whose test findings then clear at 3.0.
So on a CRITICAL change two P2s still block exactly as before; on ordinary
code it now takes four. MEASURED over the 40 most recently merged PRs:
critical 35.0%, standard 20.0%, light 45.0% (a SLIDING window — see the
classifier docstring; re-running will not reproduce it, and a difference is
the merge queue moving, not a regression). The lane comes from
`review_scope.classify_lane`, which FAILS CLOSED to `critical` on an
unreadable file list — the lane relaxes a threshold, so the safe default is
the one that relaxes nothing.
⚠ **A stable distribution is NOT a coverage proof, and neither is a passing
example.** Building this lane produced the same miss twice, each time caught by
a method the previous one could not reach:
* A draft held critical at an unchanged 30.0% while having silently stopped
classifying `*route*`/`*controller*`/`*endpoint*` paths as critical — same
percentage, different membership, because none of the 40 sampled PRs touched
such a file. A **constructed test case** found it; the measurement could not.
* Four route-defining modules — `src/genesis/hosting/**` and
`dashboard/_blueprint.py`, one serving `/genesis/login` — were still outside
the lane after that fix. Every constructed case passed and the distribution
reproduced to the decimal. Only an **enumeration over every tracked module**
found them, which is why that class is now locked by a population check
rather than by more examples.
When you change what feeds a classifier: diff the per-item ASSIGNMENTS rather
than the totals, and lock a category by enumerating its population rather than
by naming the members you happened to think of.
**`auth` is deliberately NOT a critical input**, though it sits in
`_DOMAIN_SENSITIVE_TAGS` and drives the depth gate. Its glob is
`*auth* *session* …`, and MEASURED over 3,999 tracked files, 58 of the 59
`auth`-tagged files (98%) match on **"session"** — CC-session machinery, not
authentication. Exactly one is real (`dashboard/auth.py`). Re-inheriting that
set is the obvious "cleanup"; don't.
One thing the lane does not change: **a single CodeRabbit
Critical or Major blocks on its own, but ONLY from the INLINE endpoint.**
`_check_inline_review_findings` reads two channels and they are NOT symmetric:
`pulls/N/comments` (findings anchored inline) feeds the score, while the review
BODY — CodeRabbit's "Outside diff range comments", the findings it could not
anchor — is surfaced loudly and scores **0.0 at every severity**. The
`outside_critical` / `outside_major` accumulators appear in no term of
`score = len(p1) + 0.5*len(p2) + 1.0*len(cr_block)`, deliberately: an
undelivered finding has no comment thread, so there is no maintainer-reply
route to clear it. Read them anyway; nothing there will stop a merge. The
gate's own block message prints the whole formula; read it rather than
reciting this.
A finding is excluded from the score when a MAINTAINER reply engages it, or
when it is on a DOCUMENTATION path.
**`_is_doc_path` is ANY prose extension at ANY depth — not an allowlist of
directories.** OWNER DECISION 2026-09-04 (`815e0dbd2`, #1689) widened it:
`.md`/`.markdown`/`.rst`/`.adoc` anywhere, plus a known doc STEM
(CHANGELOG/README/LICENSE/…) with `.txt` or no extension. So `AGENTS.md`,
every `SKILL.md`, `CLAUDE.md` and all of `.claude/**/*.md` ARE doc paths — and
with the default `_DEFAULT_DOC_FINDINGS_MODE = "skip"` a finding on one NEVER
scores, at any severity. Findings are still SURFACED in every mode; the lever
decides only whether they COUNT.
⚠ This paragraph previously stated the exact opposite — that those files
"return False from `_is_doc_path`, so a P1 anchored on one contributes its
full 1.0" — describing the pre-#1689 world. It was itself a correction of an
earlier error (see the callout below) and went stale the same way, which is
the point: **verify a gate claim against the symbol before relying on it, and
distrust this file most where it sounds most certain.**
Pure WARNINGs/NOTEs (non-P1/P2) → merge allowed.
**This paragraph used to say "any P1" blocks, which was FALSE, and the
divergence cost a whole session.** A P1 anchored on `CHANGELOG.md` merged
(#1606, 2026-09-03) with no override; a session read this text, saw the merge,
concluded the gate had a hole, and built a reversal of the owner's directive
before checking whether one existed. Docs describe intent, code describes
reality — and when they disagree, suspect the doc. Verify against
`git_push_guard.py` before concluding the gate misbehaved.
**A doc-path P1 is still usually a CODE finding.** A changelog is where a PR
states its own blast radius, so a P1 there typically means *"your stated blast
radius is wrong"* — the anchor is prose, the defect is not. #1606's was
anchored on the sentence "the quiet path is a hand-edited overlay", which was
false: the settings writer persisted `default: glm-5.2` alone, so settings-UI
users hit the quiet path too. Correctly non-blocking; still needed fixing.
4. **An ABSENT review BLOCKS — it does not merge on CI alone.**
`_check_codex_reviewed_head` returns a block for `if not reviewed`, whatever
the reason for the absence (quota, never triggered, still running). The only
things that clear it are a review object at head, a clean Codex signal whose id
resolves uniquely to the head (see above), a provably review-trivial delta since a
stale review, a conscious
`# stale-review-override` — except on a hook-surface PR, where that sigil also
requires the exact base-and-head fallback-review evidence described above — or
an owner-approved `# substitute-review` resting on another reviewer's review
at that exact head.
⚠ This item previously read "no review comments at all (quota exhausted) →
merge allowed on CI alone", which is FALSE and contradicted the Pre-Merge Gate
section above ("an ABSENT review always blocks") four hundred lines earlier.
A quota message is evidence about ONE channel at ONE moment, never a licence
to merge unreviewed — re-trigger and check the INLINE endpoint.
5. **Override**: Append `# review-override` to the merge command to
bypass the gate (e.g., `gh pr merge 123 --squash --admin # review-override`).
The override is logged — one metadata row per sigil, written as one file per
merge under `~/.genesis/merge_overrides/` (sigil, PR, bound head, which gate it
waived, and the verdict THAT GUARD returned: `allowed` / `asked` / `blocked` /
`error`), so "was this escape reached for?" is a query rather than a survey:
`cat ~/.genesis/merge_overrides/*.jsonl`. One file per merge rather than one
shared log, so a multi-sigil merge is a single atomic record and the writer
never maintains a file on the merge path. The store is bounded to 5 MB by the
daily `disk_hygiene.sh` timer (`scripts/prune_hook_audit_logs.py`, oldest whole
records dropped) and backed up by `backup.sh` §6d.
`blocked` matters: a sigil can be appended to a command that some OTHER gate
inside the same guard then stops, which is a real signal but not an override
that took effect, and `asked` means the decision went to a human who may still
have denied it. Read `allowed` as "the sigil was accepted HERE", never as "the
merge happened": other PreToolUse matchers and the harness's own permission
prompt sit downstream of this guard and can still stop the command.
Never any command text — the row is metadata only, because a trailing comment
rides a Bash command that can carry a credential.
**Scope, stated so the log is not read as more complete than it is:** the five
PR-merge sigils are covered (`# review-override`, `# ci-override`,
`# stale-review-override`, `# scheduled-review-override`,
`# substitute-review`), from the point the
PR is resolved onward. A merge rejected BEFORE that point writes nothing —
no `--admin`, an unresolvable repo/PR, a compound carrying two publish/merge
operations, or a merge compounded with a local `git merge` into main are the
ones measured, and the list is illustrative rather than exhaustive: anything
that blocks before the PR resolves leaves no row. No OTHER sigil in
`shell_parse._KNOWN_SIGILS` is recorded either — `# depth-ack`, `# audit-ack`,
`# discard-override` and `# full-suite-ok` belong to gates outside this guard
and announce themselves on stderr only. The command-scoped acks
(`# merge-to-main-override`, `# escalation-ack`) are
NOT logged, for two different reasons. `# merge-to-main-override` waives a
gate that short-circuits BEFORE the branch is resolved, so a row could only
say the ack was typed, never that it waived anything. The local
`# escalation-ack` is outside this guard. `# final-round-accept` remains
parser-compatible for stale worktrees but authorizes no current gate. Native round
approvals are decisions rather than sigils and are deliberately not written as
reusable receipts.
6. **Read the PR's warning comments before merging — not just the hard gate.**
Beyond Codex, a structural-review bot posts under the repo-owner account
(review state COMMENTED) and emits **SOFT WARNINGs** (PII /
private-text / wording) that the hook does NOT block on and that a naive
`.comments` scan misses. Check BOTH `gh pr view N --json reviews,comments`
and `gh api repos/<owner>/<repo>/pulls/N/comments`, and address each soft
warning or consciously accept it. Never merge past an unread warning.
7. **Codex findings are INLINE review comments — invisible to `gh pr view`.**
Codex's review *body* is boilerplate ("Here are some automated review
suggestions"); its actual `[P1]`/`[P2]` findings live only at
`gh api repos/<slug>/pulls/N/comments`. Derive `<slug>` live —
`gh repo view --json nameWithOwner --jq .nameWithOwner` — NEVER hardcode
it (configs name several repos; the working repo is not the org default).
A **404 from that endpoint means WRONG SLUG or PR number, never "no
findings"** — a clean PR returns `[]`. The merge-gate hook blocks on a P1 or a
CodeRabbit Critical/Major OUTRIGHT (the always-fix floor), and otherwise on the
weighted inline SCORE against this change's LANE threshold — so two unresolved
P2s block a CRITICAL change, while ordinary code takes four (full table in the
Pre-Merge Gate section above). Unread P2s no longer slip through in pairs on the
surface where that mattered (2026-07-10: 8 real P2s on the entity-layer PRs
merged past the OLD P1-only gate, the exact gap this score closes). Note what it
does NOT close, and do not read it as more than it is: a LONE P2 still passes
unread, which is how #1620's HTTP-500 finding merged (2026-09-03), and on an
ordinary change three now do. The score bounds what the gate stops; only reading
the report stops the rest — which is exactly why the gate prints a NOTE naming
the lane and threshold whenever a non-zero score passes under one. And the two
channels are INDEPENDENT: Codex can post a quota/usage-limit message as an
ISSUE comment while a later `@codex review` trigger delivers real inline
findings anyway — a quota message is evidence about that channel at that
moment, never proof Codex "can't review". After time passes, re-trigger and
check the INLINE endpoint before concluding quota-limited (2026-08-26: #1484's
real P2 arrived inline while the issue-comment channel still showed only the
earlier quota message).
8. **A CONFLICTING PR silently suppresses the whole CI suite.** When a PR
has a merge conflict with main, GitHub cannot build the merge ref, so
`pull_request`-triggered workflows (the entire ci.yml suite) never run —
while CodeQL still passes on the head SHA, making the check list LOOK
green. A thin check list (only Analyze/CodeQL) means CHECK
`gh pr view N --json mergeable` — `CONFLICTING` needs the base branch merged
in before any CI verdict exists at all (2026-07-16: #1089 sat
conflict-suppressed through three pushes; main had moved under it via
concurrent sessions). Since #1484 the merge gate ENFORCES this class
mechanically on the canonical repo: a fully-empty rollup reads `ci: absent`
and blocks, and a thin/partial rollup missing the required CI workflow reads
`ci: incomplete` and blocks (see the CI-gate bullet above) — the trap text
stays because the DIAGNOSIS (check `mergeable` first) is still the fastest
route to the cause.
Three corollaries, each measured 2026-09-08 and each costing a wrong diagnosis:
- **`UNKNOWN` suppresses it too, not just `CONFLICTING`.** GitHub recomputes
mergeability lazily, and EVERY merge to main invalidates it repo-wide — so
right after a merge batch the whole queue reads `UNKNOWN` and its CI looks
missing. That is also why a merge gate refuses with "mergeable status is
'UNKNOWN'": wait and re-read rather than concluding anything.
- **A PR opened against a NON-DEFAULT base never ran CI at all**, because
`ci.yml` filters `on: pull_request: branches: [main]` — the filter is on the
BASE. Retargeting it to main afterwards fires nothing: the default
`pull_request` types are opened/synchronize/reopened, and a base change is
none of them. This one does NOT self-heal and nothing will ever fire for it
again on its own; it needs a push. Stacked PRs land here by construction.
- **`workflow_dispatch` is not a workaround.** `gh workflow run ci.yml --ref
<branch>` runs to completion and never appears in the PR's
`statusCheckRollup`, so it cannot satisfy a gate that reads the rollup.
9. **`gh run rerun` CANNOT clear a failure caused by a broken base.** It replays
the ORIGINAL merge commit rather than recomputing one against current main,
so re-running a PR whose base was broken faithfully reproduces the breakage.
MEASURED: a re-run started 15 minutes AFTER the fix merged still failed, and
`git merge-base --is-ancestor <fix-sha> refs/pull/<N>/merge` returned false —
the pull request's merge ref did not contain the fix. Only a push or
`gh pr update-branch <N>` reliably recomputes it, and both move the head.
Closing and reopening the PR is NOT a substitute. MEASURED 2026-10-01: a
reopen fired a new CI run that checked out a merge ref GitHub had built about
three and a half hours earlier, on a main several merges behind. Whatever you
try, read the job log's `HEAD is now at <merge sha>
Merge <head> into <base>` line and confirm the base is the main you expect.
"CI re-fired" is not evidence that it tested current main. Two consequences: do not "just
re-run" someone's stale red, and do not tell them it will clear — it will
not, and their next push clears it for free anyway.
10. **`gh run view --log-failed` truncates and shows only PASSED lines.** On a
large suite it cuts around 44%, so a FAILING job reads as a clean log with
no failure in it — the most misleading possible output, because it looks
like an answer. It reported nothing useful on three separate attempts in one
session. To find what actually failed, reproduce locally against a clean
`origin/main` checkout (which is what identified it, twice), or fetch the
raw job log rather than the CLI's rendering.
11. **Two individually-GREEN PRs can break main, and nothing warns you.** PR A
adds a repo-wide invariant test (one that ENUMERATES modules or consumers and
asserts a property); PR B adds a violation of it. Each is green against a
main that lacks the other, neither diff touches the other's files, so there
is no textual conflict, no reviewer signal, and both merge clean — then main
goes red, and because PR CI builds the merge commit it fails EVERY open PR on
a test unrelated to their changes. MEASURED 2026-09-08: a chokepoint test
requiring every consumer of the shared shell parser to use the checked entry
point, and a guard importing the unchecked one, merged an hour apart.
This is NOT the structural collision described under the architecture-session
rule — that one is main superseding a mechanism the PR uses, and it is
visible from the PR's own files. Here neither PR can see the other.
The detectable signature is a new ENUMERATING invariant test landing while
other PRs are in flight: when you merge one, re-run it against the other open
PRs' merge commits before merging them, not just against its own branch.
Sequencing follows from the same fact — batch merges rather than interleaving
them with active pushes, because each merge invalidates every other PR's
mergeability and manufactures the false "CI never ran" signal above.
## Reference Router
Read references ONLY when relevant to the specific task. Do NOT load all
references on every trigger.
| When you need... | Read... |
|---|---|
| Subsystem purpose/maturity/do-not-touch (judgment layer) | `docs/architecture/CURRENT.md` |
| Codebase structure, package map, gotchas, debugging | `references/codebase-map.md` |
| Package/module/symbol navigation (progressive drill) | `codebase_navigate` MCP tool (L0→L1→L2) |
| venv, DB paths, Qdrant, Ollama, network, commands | `references/environment.md` |
| Worktree rules, concurrent sessions, branch naming | `references/worktrees.md` |
| tracked_task, exc_info, os.killpg, logging patterns | `references/observability.md` |
| V3 state, build order, GROUNDWORK, architecture docs | `references/architecture.md` |
| Phase 6 contribution pipeline, sanitizer | `references/contribution.md` |
| Pending work, active incidents, subsystem status | `references/build-state.md` |
| Auditing/deep-reviewing AI-generated code (failure taxonomy, audit passes) | `references/ai-code-audit.md` |
| Writing or revising a multi-session plan document | `references/plan-docs.md` |
| What a plan owes before building: free reads, red-team, body fields, ask vs decide | `references/due-diligence.md` |
| Pre-release review, bug hunt, guard/gate change — verification method | `references/high-stakes-verification.md` |
| Choosing a command/value/procedure by reasoning about an external tool | same, section 9 |
| Auditing the CLAIMS a session wrote (wrap-up, or before any permanent record) | same, section 11 |
| Which code tool to use (CBM vs Serena vs GitNexus vs Grep) | `.claude/docs/code-intelligence.md` |
**Freshness rule:** On first read of `codebase-map.md` in a session,
verify structural claims against current code. If a package status or
gotcha has changed, flag to user before acting on stale assumptions.
`docs/architecture/CURRENT.md` carries per-entry `verified:` stamps
enforced by `scripts/check_subsystem_map.py` (CI `subsystem-map-check`) —
after changing a subsystem's capabilities, update its entry and stamp.
## Public Repo & Release Workflow
The public repo (`GENesis-AGI`) is the primary development repo.
Standard open-source workflow: PRs go directly to the public repo.
- **Squash merges only** — merge commits are disabled on the public repo.
Always `git pull --rebase origin main` **on main** after merging a PR before
committing locally, or push will be rejected (non-fast-forward).
- **Once a branch is pushed: merge main IN, never rebase it** — force-push is
banned, so a rebased pushed branch diverges from its remote with no way to
publish the rewrite. A merge commit on a *branch* is fine — the squash erases
it at PR merge; "squash only" above governs merging into main, not
reconciling a branch.
- **In a reconciliation merge, the base wins on policy.** The merge commit may
DELETE from your branch (a rule main superseded, a test that pinned your
now-dead behavior); it may never re-litigate main. Scope, precisely: base
wins where main *deliberately changed a thing your branch also changes*; on
your branch's own new work the merge integrates both sides — keep your
feature, adopt main's surrounding changes. "Base wins" is never
`--theirs` on a whole hunk. If you disagree with what main did, that is a
NEW PR with its own review (a sanctioned revert PR is exactly that — the ban
is on *unreviewed* reversals) — never a conflict resolution, which is the
one diff nobody re-reviews. And a clean auto-merge is not a clean
reconciliation: the superseded-rule case usually merges with NO textual
conflict, because your rule and main's live in different hunks — so after
merging main in, re-check every rule and test your branch carries against
what main changed in the interval. The lived failure (2026-09-04): main
widened a gate rule while a narrower rule for the same paths sat in review;
restoring the branch's rule "so its tests stay green" would have silently
reverted a deliberate policy change inside a merge commit. Delete your
superseded rule AND its tests — a test pinning behavior the base retired is
not coverage, it is a revert waiting to be committed — and name every such
deletion in the PR body; if you suspect main's change is a bug rather than
policy, file that before merging. A deleted test with no filed disagreement
is how coverage disappears.
- **The CI collected-test floor conflicts whenever main moved it while your
branch added tests.** `.github/workflows/ci.yml` carries one hand-maintained
`--min-collected` number, and both sides have raised it from a shared base.
Picking either side under-counts. The merged floor is MAIN's floor plus YOUR
branch's delta, and the delta is measured, never recalled. Run
`--collect-only` on main's version of each test file your branch changes and on
the merged version, after your LAST test-adding edit. A file new to the branch
counts in full. Keep main's comment block and add one paragraph showing your
per-file counts. The `--ceiling` on the same line merges the same way: main's
ceiling plus your branch's skip delta, counted on the CI runner from the junit
report, never locally (installed tools differ, so local skip counts are the
wrong denominator). Issue #2743 tracks replacing the hand-maintained number. A test path held in a shell variable reads to
`full_suite_guard` as a whole-suite run, so write the count as a script.
- **README is public-authoritative** — the public repo's `README.md` is
hand-crafted and must NEVER be overwritten.
- **Never edit `CHANGELOG.md` in an ordinary PR — add a `changelog.d/`
fragment.** (The one exception is the release-fold PR, where the
`CHANGELOG.md` diff is produced by `scripts/assemble_changelog.py` and the
section rename rather than written by hand — see the release procedure in
`docs/reference/recovery-and-portability-workflow.md`.)
One file per change, named `<YYYYMMDDHHMMSS>-<category>-<slug>.md`
(timestamp from `date -u +%Y%m%d%H%M%S`; category is one of `added`,
`changed`, `deprecated`, `removed`, `fixed`, `security`). The file holds
the entry exactly as it should appear — a Markdown bullet, spliced in
verbatim. `scripts/assemble_changelog.py` folds them into `[Unreleased]`
at RELEASE time, not per PR.
This is not bookkeeping preference: two branches editing one shared
changelog insert at the same position, which git calls a conflict.
Measured 2026-09-04 — of 49 open PRs, 21 could not merge and **18 of
those conflicted on `CHANGELOG.md` and nothing else**. Two branches
writing two different filenames have nothing to merge. A merge driver
cannot substitute: GitHub ignores repository `.gitattributes`
server-side, measured against its own merge engine.
- **CHANGELOG audience is users** — only include entries a user updating
their install would care about. No internal refactors, README changes,
CI tweaks, or process artifacts. Lead with the user-visible effect, not
the implementation technique. Same bar for a fragment: it becomes a
changelog entry verbatim.
- **No sensitive data in commits** — voice data, research profiles, IPs,
and secrets must never enter the repo. User data lives in overlays
outside the repo (e.g., `~/.claude/skills/*/`, `~/.genesis/`).
- **Individual campaigns are user data, not infrastructure** — a campaign's
name/prompt/targets/cadence live only in the `campaigns` DB table and the
private backups repo; never hardcode them into tracked source. Unlike modules
(which ship defaults under `config/modules/*.yaml`), campaigns ship ZERO
defaults (no `config/campaigns/`). Only campaign infrastructure ships. Express
reusable session types as generic roles (e.g. the `community-responder`
profile), not names coupled to a live campaign. See `src/genesis/campaigns/__init__.py`.
- **External egress is gated; owner-facing egress is not** — any autonomous send to the
outside world (Discord, Medium, Twitter/X, Slack, `DistributionManager.distribute`) MUST
route through the capability shadow-gate (`autonomy/shadow_gate`) before the enforce stage;
the `scripts/check_external_io.py` CI guard backstops new endpoints. Delivery TO the owner
(Telegram/voice/email-to-owner) is NEVER gated. Full contract in `autonomy/shadow_gate.py`.
