Crafter.Review/AGENTS.md

# Crafter.Review — developer guide

This file is for changing crafter-review itself. `README.md` says what it is and how a repository uses it; `process/prepush.md` is the procedure the `/prepush` skill drives, and `process/lenses/` holds the lens texts.

## What it is

`crafter-review freeze` exports `base..HEAD` with `git format-patch` into `.review-gates/push/<slug>/` and hashes the export. It refuses on identity, trailer, dirty-tree, merge and fast-forward failures. `crafter-review run` partitions the export mechanically (`:Inventory`: every non-blank line in exactly one unit, fixed slots for what is absent), then runs the gates the config enables, in order.
- Each gate is a lens; Gate 3 has two, the bug-prover and the security lens; Gate 4 is the commit-truth lens, the messages and the documentation of the range (2026-10-31). Every lens is off until the consumer's config turns it on.
- Each lens is one `claude --print` process per unit (block passes). Where the lens has one, a whole pass gates the blocks and hands them its summary.
- A pass returns structured output: a verdict, findings, notes, and one coverage entry per unit. `:Normalize` derives the verdict from the findings and fails closed on a missing or blocking entry.
- Clean results are stored per unit under a content key and reused across hashes (`:Plan`). The static lenses need three clean reads of a unit before it is reused; the functional lens needs one.
- Processes run on the seats of the seats file (`SeatPool::ResolveSeatsFile`: `--env-file`, `$SEATS_ENV_FILE`, the consumer's `.env`, else the machine's own) and of the process environment, the file's lines over the environment's; with no seat in either, on this machine's login. A lens's processes get the environment the config names: `none`; `browser`, a Playwright MCP server per pass (`:Mcp`, `:Pool`); `supabase`, the local stack's database through the `sql` helper (`:PgWire`); or `browser+supabase`.
- A browser pass holds a stack of its own (`Pool::Stacks`), and a pass launches only when it fits in free memory at its environment's measured cost (`Pool::Admission`, `:Memory`).
- Drivers on one machine share one board, `~/.cache/review-gates/seat-board.json` (`SeatPool::Board`). `State::RunLock` allows one full run per hash.
- The kernel modes (2026-10-31): `--mode series` and `--mode reply` run the pre-send pipeline of a kernel contributor's workbench with the same machinery. Their own pieces: the inventory of a cover letter, a mail draft and the camera review's shape (`:Inventory` PartitionReply, PartitionMail), the ten kernel lenses with their gates (`Plan::Lenses` rows with `kernel`, `Plan::Gates(kind)`), text units keyed on what they cite and the anchor (`Plan::TextDeps`) and a series' code units on the code like the push gates, the persona gate - one `maintainer-persona` instance per `--reviewers` entry, its whole pass pruning the not-mine units - with the persona vocabulary in `:Schema`, `:Normalize` and the matrix, the references prefetched through `utilities/lore.py` and the persona corpora (`:Refs`), the kernel texts in `:Prompt`, the `GATES CLEAN` verdict, `personaPass` in unit-state, and the subsystem's journal in the run layer and the dispositions block. Their prompts, keys and plans were checked byte for byte against the pipeline they replace, on its own artifacts.

## The rule it lives by

**Anything that reaches a prompt, a reuse key, a shared file or an exit code is pinned byte for byte by the goldens (`tests/Goldens`) and the checks.** A change to any of it is deliberate: it edits the goldens in the same commit and, where it reaches a prompt, comes with the dry-run proof below. Stored results outlive a release and every driver on the machine shares the files, so a byte that moves by accident is a bug.

## Layout

```
project.cpp            crafter.review-lib, crafter-review, the tests, Crafter.Network (pinned; --local
                       takes ../Crafter.Network), Crafter.LintRules (pinned pragma), scripts flatpak/install
lib/json.hpp           nlohmann/json 3.12.0, vendored, never edited
interfaces/            Crafter.Review.cppm (the umbrella) and one partition per concern
implementations/       the partitions; main.cpp hands argv to Crafter::Review::Main
process/               prepush.md, lenses/*.md, templates/{config.json,manifest.md}, skill/prepush/SKILL.md
tests/<Name>/main.cpp  one test each; tests/Common/FakePg.hpp is the Postgres stand-in two of them share
packaging/flatpak/     the manifest, the prune of the base, metainfo, flatpakref
.forgejo/workflows/    ci.yaml
```

The partitions, in import order:

| Partition | What it holds | Notes |
|---|---|---|
| `:Unicode`, `:Text`, `:Json` | Unicode 16.0.0's character tables; the text, number, URL and path semantics the rest relies on (`Dyn::`); JSON | the tables are checked-in data; `Json` is `nlohmann::ordered_json`, so key order and unknown fields survive a rewrite |
| `:Crypto`, `:Platform`, `:Process`, `:Http` | hashes, HMAC, PBKDF2 and the encodings; files, locks and signals; child processes; HTTP | spawn, reaper, supervisor, signal and stop threads; flock and locked read-modify-write; the sandbox |
| `:Core` | the version, `PromptVersion`, the home, the invocation | |
| `:Schema`, `:Tools`, `:Plan`, `:Artifact`, `:Inventory`, `:Dispositions`, `:State`, `:Normalize` | the structured-output schema, the tool lists, planning and the reuse keys, the frozen artifact, the inventory, the disposition register, run state, normalisation and merge | `:Tools` is a leaf, so `:Config` and `:Pool` both read it without importing each other |
| `:PgWire`, `:Totp` | the Postgres client, the `sql` helper and its broker, the `totp` helper | the client, the splitter, the refusals, the broker |
| `:Stream`, `:Prompt`, `:Config`, `:Memory`, `:Mcp` | claude's stream, the prompts, the consumer config, memory admission, the Playwright MCP config | a prompt literal changes only with a dry-run proof |
| `:Seats`, `:SeatPool`, `:OpenCode` | seat usage and its report, the seat pool and the board, the OpenCode harness | `:Seats` is the library; the report's glue is in `:Driver` |
| `:Pool`, `:Freeze`, `:Run`, `:Driver` | the pass pool, freeze, a run's gates, the commands | `:Run` holds `Run`, `slots` and `app_ready`; `:Driver` the command line |

Every seam a test needs is a member that starts as the real one: `Run` has `appReady`, `probe`, `query`, `bypassesRls`, `migrationsAtHead`, `migrationsDrift`, `treeDrift`, `promptFor`, `runPass`, `resumable` and `instrumentPreflight`. `SlotsPlan` takes a `Machine`. A regex on unbounded input is a hand-written scanner, never `std::regex`.

## Rules for a change

- **`crafter-build --local test` passes** before and after; add a check for what you changed. It needs no network and no seat. Run `test` before `lint`: the four AST rules of Crafter.LintRules read the PCMs a test build leaves. `format` and `lint` are clean.
- **A prompt change is proven with a `--dry-run`** on a frozen range before and after (`tests/Goldens/fixture/commits`, or a scratch repository frozen with `freeze`). The prompts under `<scratch>/<hash8>-run-<n>-dry/prompts/` and the system files must differ exactly where intended, and the goldens change with them. A text change a lens reads bumps `PromptVersion` (`:Core`) only when earlier clean results must NOT carry over (2026-09-27: a layout change that left the text a lens reads the same did not).
- **The lens texts and `_common-repo.md` carry no header.** A charter's sha256 - with the repository's own section appended, when `lenses.<name>.section` gives the lens one - is part of every reuse key, and the charter and its mode's common text (`_common-repo.md` for a commit range, `_common.md` in the kernel modes) are part of every prompt; `--version` prints the shipped texts' sha256s. Editing one is a prompt change.
- **A reuse-key change ships with an attack on its STORE side**: "two units, one key, different verdicts" and "a FIX FIRST round, then a text-only re-freeze" (2026-10-06; the StoreRules and CodeKeys tests).
- **A gate change ships with a firing proof**: a scratch repository with one planted defect per gate, each lens run with `--only <lens> --units <unit>` (PARTIAL by design) on the consumer's `.env` seats. It must give FIX FIRST on the planted range and SEND-READY on the corrected one. Record it in the lens's calibration section with the date, the units, the turns and the cost. It costs money: only with the owner's go.
- **The driver never runs a consumer's build, tests, stack or package manager**; the skill does that, by hand, from the config's commands. The functional lens observes a running application it did not start.
- **Secrets never reach a log, a dump or a prompt.**
  - Seat tokens go into a seat's own config dir, and API keys into that process's environment alone (`:SeatPool`).
  - The database URL stays in the driver: a db pass gets the address of the driver's SQL broker and its stack's token (`PgWire::Broker`, `Runner::ProcessEnv`). The URL is masked in the dump, the log and `config`.
  - INPUTS prints the login fixture's path, never its content.
- **A shared file keeps its path, shape and locking.** These are the board, the ledger, the memory record, `unit-state.json`, the run lock, the scratch markers, `progress.json`, `log.txt`, `resume-<h8>.json` and the label-to-filename encoding in `out/`. Every driver on the machine reads and writes them, so a field another driver wrote survives a rewrite.
- **Every child process goes through `Process::Start`**; `popen` and `system` are not used. A claude pass and a seat probe run under `crafter-review __supervise`, which ends the pass's whole tree.

## Tests

One directory per test under `tests/`, each an ordinary `main()` linking the library: exit 0 passes, nonzero fails, 77 skips.
- the pipeline's pure parts: Stream, Normalize, Merge, Dispositions, Config, CommitsMode, CodeKeys, StoreRules, PgWire, StacksMemory, ReviewFixes and Freeze;
- the pool and the seats: Scheduler, SeatPool, Runner, Drivers, OpenCode and Seats;
- the foundations: Crypto, Text, Platform and Process;
- Goldens: the pure pipeline against the recorded answers in `tests/Goldens/fixture`;
- Runs: full runs end to end on a claude stand-in;
- Commands: init, doc, stop, seats, browser;
- LensTools: every allowlisted program on PATH, strict inside the Flatpak build.

Conventions:
- **Git, temp dirs and timeouts.** Every git call runs with `GIT_CONFIG_GLOBAL=/dev/null` and `GIT_CONFIG_SYSTEM=/dev/null`; the owner's global config signs commits. Tests work under their own temp dirs, with `REVIEW_GATES_BOARD` and `--scratch` pointing there, and each stays under crafter-build's 60 s.
- **Subprocesses.** A test that needs one re-executes its own binary. The test binaries of Runs and Commands are crafter-review itself when given arguments, in native mode whatever builds them.
- **Stand-ins.** The claude stand-in is a `/bin/sh` script, and the Postgres stand-in an in-process thread (`tests/Common/FakePg.hpp`).
- **Goldens.** `tests/Goldens/fixture/` holds the recorded answers the suite compares against: the inventory, the plans, the keys, the prompts, the normalised results, the stream, the config, the SQL helper's output. A deliberate change edits them in the same commit, by hand; so do `:Unicode`'s tables (Unicode 16.0.0) and `tests/Text/fixture/vectors.json`. The entries that follow from the lens table and the config defaults - the config cases through `config`, the commit-truth lens's plans, the not-reviewed lists, the estimates and the gates - are re-derived by `python3 -I tests/Goldens/derive.py <checkout> <crafter-review binary>`, which asserts that the recorded table still reproduces the recorded entries before it writes; review its diff.
- **The suite runs inside the Flatpak build too** (`crafter-build test` is a build command of the manifest), where `/.flatpak-info` exists: a test pins `Platform::SetMode(Native)`.

## Behaviour the checks hold

**Pinned byte for byte**, by the goldens or a check:
- the inventory: its ids, lines, excerpts and `inventory-<h8>.json`;
- freeze's checks, messages, `freeze.json` and the worktree layout. The artifact hash depends on the git version, because format-patch ends each patch with it; the Sdk's git is 2.56.0 today, the host's too;
- the schema bytes;
- the block and whole reuse keys, which drop the git signature, so a re-freeze under another git keeps every key;
- the system files and user prompts, apart from the helper command strings, which are in no key;
- the SQL and TOTP helpers' output, refusals and exit codes, and the broker protocol;
- the stream classification, normalize and merge, the dump's keys and their order, the stdout summary, and the exit codes of `run` and `freeze`;
- every shared file (Rules, above). The opencode meta line is one line with `", "` and `": "` separators (`DumpSpaced`), because salvage matches on that prefix.

**Worth knowing**, each held by a test:

- **The helpers are subcommands.** Prompts and the `Bash(…:*)` allowlist name `<exe> sql` and `<exe> totp <fixture>`. `<exe>` is the running binary's absolute path, `/app/bin/crafter-review` in the Flatpak. These strings are in no reuse key. The stream's SQL-call detection matches the helper with any path before it (Stream, Runner).
- **Printed command lines name the invocation**: `flatpak run net.catcrafts.CrafterReview` in the Flatpak, the binary's absolute path natively (Freeze, StacksMemory, Goldens).
- **Messages that point at a pid.** In the sandbox, the run-lock message says "wait for it or stop it (`<invocation> stop <its scratch dir>`)", and `seats` adds a line naming `stop`; natively it says "wait for it or kill it". The pid inside a sandbox ignores SIGTERM.
- **The dump's `driver` field and the first log line** read `crafter-review <version> (prompt v1)`. Nothing reads them.
- **`freeze.json` records `gitVersion`.** When the hashes differ, `freeze --verify`'s MISMATCH names a git version change as the likely cause (Freeze).
- **A repository's own lens sections** (`lenses.<name>.section` in the config, `Config::LensSectionsOf`, `SectionProblems`): `Prompt::Charter` appends the file the config names for the lens to the shipped charter after one blank line, so `charterSha` and every reuse key cover it; the path is relative and inside the repository (the check `slots.dir` shares: an absolute path or a `..` part is a config problem, and `LensSectionsOf` resolves no other), a file missing or one the run could not use (`Prompt::ReadSection`, the reader `Charter` uses too: a dangling link, no regular file, unreadable, not UTF-8) is a config problem (the run refuses), a name that is no lens is Merge's unknown key, and the run logs which charters got one. Nothing lists a directory (2026-11-01, Jorijn: a path per lens, where a listed directory had to tolerate or refuse every foreign entry) (Config, CommitsMode, KernelModes).
- **A reply's gate-state is beside the draft**, a directory artifact's inside it; the run lock and the dumps are named by the kind (`reply-<h8>-run-N.json`), the resume file by the hash alone (`resume-<h8>.json`); a mode of another name shares them all, and the dump's `mode` says which ran. A kernel mode requires `--subsystem`; commits refuses `--reviewers`; `--reads` is 1 for a reply. The kernel modes check no tree (the lenses read several), excuse no unit from the holes, and their `lensesOff` and `environments` in the dump list the kind's lenses alone (`Plan::KindLenses`; `lensesOff` those of them the mode does not list) (KernelModes).
- **The repository under review is the freeze record's**, not the working directory's: `Run::reviewed` is named in the prompt and keys the calibration, and `--trees` defaults to the record's worktree, else its repository when that is not the working directory, else `.` (`Freeze::ReviewedBy`, `TreeOf`). The working directory stays `Run::repo`: the config, `.env`, the slots, the lens processes' cwd and every logged path. That is how a workbench reviews other repositories with `freeze --repo` (Freeze, Runs).
- **`config` reports an absolute `functional.mcp.command` that is no program here**, such as `/usr/bin/npx` inside the Flatpak, whose `/usr` is the Sdk's. `config` then exits 1, and a run stops at preflight (Config).
- **JSON edge cases.**
  - `NaN` and `Infinity` are refused when read: a claude stream, opencode or seat-probe line that holds one is skipped. They are written as `null`.
  - A lone surrogate is replaced.
  - A float from 1e15 up to 1e16 prints with an exponent.
  - An integer beyond 64 bits becomes a float.

  No file the drivers write holds these in practice, and a file that fails to parse is treated as a damaged one.
- **HTTP.** `ClientHTTP1::Fetch` ignores `*_proxy` variables. The OpenRouter requests send `User-Agent: crafter-review`; `app_ready` sends `review-gates`. Reason phrases are the standard ones, not the server's.
- **Seat placement** (SeatPool, Seats, Drivers): The settings beside the seats count too. `run` (Gate 0) and `seats` each resolve the seats file from their own `--env-file` with `SeatPool::ResolveSeatsFile` (`--env-file`, `$SEATS_ENV_FILE`, `<cwd>/.env` when it is a file holding a seat or a placement setting, each with a value (a blank line is neither), or one that cannot be read as UTF-8 text (its error is then the command's) - a readable application `.env`, or one that is no file, is passed over, recorded as `passedOver` and logged by the run - the machine's own), reading each file once (`SeatsFile::Text()`: a pipe or a FIFO `--env-file` gives its settings and its seats alike, and a failed read raises its own error where the tokens are laid over the environment, `SeatPool::WithFileTokens` - the one place, so `LoadSeats` takes the merged environment and reads no file). `SeatPool::SettingsOf` lays the file's placement settings (`Seats::SettingsFromEnvFile`, one dotenv grammar with the tokens', `DotenvLines`; `SeatPool::PlacementSettings`, the fifteen names the readers read, also deciding what makes `<cwd>/.env` a seats file - a test pins that each moves a reader) under the process environment into a map handed to the placement readers alone - `ApiConfig`, `PoolConfig`, `Seats::ConfigFromEnv` - names its other `REVIEW_GATES_*` lines (the run logs them, `seats` prints them on stderr), and the process environment is not touched, so no other reader sees a line of the file: a path's or a binary's line reaches nothing, and two drivers reading one `.env` keep one board and one ledger (the Python driver's `config_env` of 2026-10-17; the first self-review's rounds 3 to 10, 2026-11-01, reworked it as an application to the process environment until round 11 found the map the readers already took) (SeatPool).
  - An account whose 5-hour limit Anthropic has removed reports the weekly window alone. Its burst window (`Seats::BurstWindow`) is the weekly one: it is ranked and admitted by the weekly window with the passes in flight projected on it (0.04 points per pass at xhigh, 0.125 otherwise), under the 99 % cutoff.
  - A pass's projected share of the windows follows the run's effort: 0.5 / 0.125 points of the 5-hour window and 0.125 / 0.04 of the weekly one at max / xhigh (`SeatPool::LensCostPct`, `LensCost7dPct`), the pooled means of 5438 passes on 141 seat-runs (2026-10-12..15) rounded up. The weekly cutoff is 99 %: the running passes are projected against it, so the last percent is spent a pass at a time.
  - Every OAuth account and the local login takes at most 128 passes at once, every driver's counted.
  - There is no machine-wide width (2026-10-31, Jorijn: "there is no need to do a global cap anymore, the cap should be 128 per available account"). `--concurrency` caps only when given, and the dump's `args.concurrency` is then null. The board's driver settings read `128 per account`, which every driver's `seats` shows as that driver's width (Scheduler, Runs).
  - A 'concurrents' rejection pauses the seat for 20 s or its retry-after, not until a window's reset.
  - Board entries and the folded `foreign` record carry `cost7d`, and so does the dump's seat summary.
  - The `seats` report and the log table say `no limit` in such an account's 5-hour column.
- **The ledger's cycles and credit-outs** (SeatPool, Seats, Drivers): an API seat's cycle starts on its grant day, `ANTHROPIC_API_KEY<n>_GRANT_DAY` (`Seats::ApiGrantDays`; the seat token reader takes the line), else `REVIEW_GATES_API_CYCLE_DAY`, the day every OpenRouter key's starts on; `LoadSeats` puts an API seat's day in the seat and the ledger (`Ledger::SetDay`). A new cycle starts the spend over (`Ledger::Entry`) and carries a credit-out, with the time the key was last seen answering again. `Probe` re-probes a paid key the ledger holds out on credit (`ledgerOut` `credit`; one out on its budget it skips) - an API key with a request, an OpenRouter key through `ProbeOpenRouter` - and `seats` every API key; `Ledger::Reconcile` has the ledger follow an ok or credit-error reading in one locked update on the file's entry: an ok on a key out since before the probe started lifts the mark - an API key's is new credits (`Ledger::GrantEntry`: a cycle from the grant day when that is within the week, else from today), an OpenRouter key keeps its cycle's spend - a credit error on a key the file has as fine, with no lift seen since the probe started, is recorded, and anything else writes nothing. A mark is local time with its UTC offset (`2026-10-25T02:00:05+01:00`) and marks are ordered by instant, across the hour that repeats when DST ends and between drivers in different zones; a stamp without an offset, as older drivers wrote, is read as local time. `seats` stamps what it writes after its probe, not at the start its probes share. An ok reading over a credit error recorded since the probe started leaves the seat out, and a key that answers with its cycle's budget spent is out on that budget. `seats` shows the probe's reading beside the driver's verdict (`LedgerOut`, the one `LoadSeats` holds a seat out by), the grant day and the next credits (`NextGrant`).
- **Board identity and liveness, process lifetime, stop:** see "Inside the Flatpak".
- **Edge cases:**
  - Usage errors of `run`, `freeze`, `slots`, `config` and the other commands exit 3, the preflight's code, and so does an INVARIANT failure of `run` and `slots`. The helpers exit 64.
  - `--only` and `--units` skip empty items, and one that names nothing is a usage error rather than no filter (Runs).
  - Salvage tries a hash's earlier scratch dirs newest first by run number.
  - A dry run clears the prompts a dry run before it left in the directory they share (Runs).
  - A run never takes the number of a dump on record, so it cannot write its dump over another run's (Runs).
  - A stop or a SIGTERM between gates, or while a gate's pool is set up, stays in force: the next gate does not run (Runs).
  - The stop file `<scratch>/stop`, checked every 2 s and handled as SIGTERM; a claimed scratch dir drops a stale one (Runs).
  - A patch that is not UTF-8 stops `run` at preflight, exit 3 (Runs). An exception no command catches ends it with its message and exit 1, never an abort (Commands).
  - The SQL helper's connect retry does not sleep after its last attempt, and TOTP reads the clock once per code.
  - A broker call whose token is not a string gets NO DATABASE and the exit mark (PgWire).
  - An API seat's `.claude.json` is created whole or not at all, never over another writer's.
  - claude's stream-json is split on newlines only, after universal newlines: `\v`, `\f`, `\x1c`–`\x1e`, `\x85`, U+2028 and U+2029 do not end a line inside one (Stream).
  - `progress.json` is written atomically, through a temporary file and a rename.
  - The `seats` API report says the ledger counts what "the review drivers" spent: every driver writes it.
  - In the Flatpak, a pass's `SHELL` is `/bin/bash` (Platform).
- **Commands beyond the gates**: `init`, `doc`, `browser install`, `stop`, and `--version`, which prints the home and the charters' sha256s (Commands).
- **Order, not value:** `estimate.cacheWrite`'s last digit depends on the order the passes ran, because it is a float sum.
- **A mode is an artifact kind plus the lenses it runs and its effort** (2026-11-01: `modes.<name>` in the config, `Config::ModeOf`, `ModeNames`, `LensesOff(config, mode)`). The three kinds (`Plan::Kinds`: reply, series, commits) are modes of their own name; a mode of another name states its `kind`. The kind decides the partition and hash, the common text and run layer, the schemas, the disposition and reuse-key rules and what the CLI requires (`Run::Arguments::kind`, `Plan::IsKernelKind`); the mode decides which of the kind's lenses run (`Plan::KindLenses`, in the kind's gate order, `Plan::Gates`) and the effort (`modes.<name>.effort`, else the kind's default). Every lens is off until a mode lists it (2026-10-31, Jorijn: "all lenses off by default and opt in only"): `Config::Defaults()` runs no lens, while the template lists every lens of each kind at the kind's effort, to trim (Jorijn, 2026-11-01, on the populated template as the example: "looks good"; `init` says to trim); `lenses.<name>.run` is retired with a warning that says so; a lens of another kind, a name that is no lens, a lens listed twice and an unknown effort are config problems; `--mode` names any configured mode, the dump records `mode` and `kind`, and a reuse key carries no mode name, so a clean read bought in one mode counts in another of the same kind. The run lock, the run numbers and the dump names are the kind's (`<kind>-<h8>-run-N.json`): one full run per hash across the modes of a kind, their rounds counting on together; the dump's `mode` says which ran (Config, CommitsMode, KernelModes).
- **The commit-truth lens** (`repo-facts.md`, 2026-10-31): Gate 4, after the proofs, so the messages and documents are read once the code they describe has settled. Its block kinds are text, doc and slot: the message units keyed on their text, the base and the code up to their commit (`MessageDeps`); a documentation unit on its file, its normalised text, the base and the code of the whole range (`DocKeyParts`); a diffstat on its text alone; `whole:order` and `whole:absent` in slot passes of its own (a lens with no whole pass asks the slots named in its row; never stored). A documentation file's diff header and a part the commit writes nothing in are mechanical n/a. Budgets 60/40 for a text or doc block, 50/35 for a slot; one read. The owner's gate is 5. The other lenses' prompts, keys and plans are untouched (CommitsMode, CodeKeys, Runs, the Goldens' series and gates).

**Known quirks, kept:**
- the label-to-filename collision in `out/`;
- `code_norm` strips the patch tail only when the signature is present;
- the two different rules for "has a body";
- resume ignores contests (this fails closed);
- a round is marked complete even on FIX FIRST;
- the SQL helper's "statements 4-4" wording and its edge case at exactly 40000 characters.

## Inside the Flatpak

`Platform::Init` decides once whether the binary runs sandboxed: `/.flatpak-info` exists. Tests pin the mode, the identity's `/proc` root and the paths.

- **Environment.** Flatpak points `XDG_{CONFIG,CACHE,DATA,STATE}_HOME` at `~/.var/app/<id>`. The binary restores the host's (`HOST_XDG_*`, else the defaults under `$HOME`) for itself and every child, and a child's `SHELL` becomes `/bin/bash` where flatpak set `/bin/sh` (`RestoreShell`). `CRAFTER_REVIEW_HOME` is honoured natively only, so a host setting cannot swap the Flatpak's lens texts for a checkout's.
- **Board identity.** A full run publishes `<bwrapinfo child-pid>:<starttime of /proc/1>`: the host pid of the sandbox's init and its host start time. That is what `flatpak ps` shows as `child-pid`, so a driver outside that checks the pid sees the sandbox alive exactly as long as it runs. Without `bwrapinfo.json` (as in `flatpak build`) a full run refuses to start. Dry runs and every other command never join the board.
- **Lifetime lock.** Every full run holds `LOCK_EX` on `<board dir>/drivers/<pid>-<start>.lock`. It is created under a temporary name, locked, then renamed into place, so a prober can never lock a fresh file. Probers try `LOCK_SH|LOCK_NB`, and whoever finds it free unlinks it under the board lock; `Board::View` never does.
- **Liveness.**
  - A lock file exists: the lock decides.
  - No lock file, natively: the pid and its start time decide.
  - No lock file, in the sandbox: always alive. The sandbox's `/proc` is never consulted for other keys: its pids count up from 1 and can equal a live host pid.
- **Stop.** A SIGTERM to the `flatpak run` pid kills only the outer bwrap, and the published pid ignores SIGTERM. So a watcher thread checks `<scratch>/stop` every 2 s, and `crafter-review stop` writes it.
  - A stop is handled exactly like SIGTERM, and it stays in force across gates.
  - Claiming a scratch dir drops a stale `stop`.
  - A process-group SIGTERM to the `setsid` launch reaches the driver; `flatpak kill` is the hard stop.
- **gpg.** Only `xdg-run/gnupg/S.keyboxd` is exposed, so `%G?` verifies against the public keys and the agent stays out of reach.

**Process lifetime.**
- **Spawning.** Every child is spawned from one long-lived spawner thread, because PR_SET_PDEATHSIG fires when the forking *thread* ends. The child gets PDEATHSIG, a `getppid()` check, fds 3 and above closed, the signal mask cleared, and SIGPIPE and SIGXFSZ back at their defaults.
- **The driver.** It ignores SIGPIPE, is a child subreaper, has one reaper thread owning `waitpid(-1)`, and handles signals on a thread of their own.
- **The supervisor.** `crafter-review __supervise` is single-threaded, a subreaper, and holds PDEATHSIG SIGTERM. It forks claude with PDEATHSIG SIGKILL, waits for it, and then kills and reaps everything left in its subtree: the MCP server and Chromium go with it, even though Playwright starts Chromium detached. On SIGTERM it SIGSTOPs its subtree, rescans until the set is stable, SIGKILLs it and exits 128+9, so a timed-out pass reports -9, a SIGKILLed child's code. That SIGTERM comes from a timeout, an abort, or the driver's death, SIGKILL included.
- **At exit** `run` and `freeze` kill their own remaining descendants, such as a gpg daemon, and never a process they did not start.

## Environment variables

What the binary reads at run time, and how. A variable the driver only sets or strips for its children (`CLAUDE_CONFIG_DIR`, the bare `ANTHROPIC_API_KEY`, `CLAUDE_CODE_ENTRYPOINT`, `MAX_THINKING_TOKENS`, the opencode settings) is not read and not listed.

| Name | Read by | Default | Parsing rule | What it does |
|---|---|---|---|---|
| `CLAUDE_CODE_OAUTH_TOKEN1`..`6` (+ legacy `CLAUDE_CODE_OAUTH_TOKEN`) | `:Seats` SeatTokensFromEnv, via `:SeatPool` LoadSeats (run) and `seats` | none | The fixed names 1..6 only; `TOKEN7` does nothing. Stripped; an empty value is no seat. The bare name is seat 1 when `TOKEN1` is empty. The env file's lines override the process environment. Stripped from every child's environment. | OAuth seats `1`..`6`; a token reaches claude only as its seat's `CLAUDE_CONFIG_DIR/.credentials.json` |
| `ANTHROPIC_API_KEY1`..`6` | `:Seats` ApiKeysFromEnv (run, seats) | none | The fixed names 1..6, stripped; empty is no seat. The bare `ANTHROPIC_API_KEY` is never a seat (the env-file parser wants the digit too) and is stripped from every child. | API-credit seats `api1`..`api6`, each key in its own process's `ANTHROPIC_API_KEY` only |
| `ANTHROPIC_API_KEY1_GRANT_DAY`..`6` | `:Seats` ApiGrantDays, via LoadSeats (run) and `seats` | `REVIEW_GATES_API_CYCLE_DAY` | Stripped decimal digits of any script, 1..31; anything else is ignored. The env file's lines override the process environment. | The day of the month seat n's ledger cycle starts on |
| `OPENROUTER_KEY`, `OPENROUTER_KEY1`..`6` | `:Seats` OpenRouterKeysFromEnv, via LoadSeats (run; `seats` ignores them) | none | As the OAuth tokens; the bare name is `or1` when `KEY1` is empty. Never in `auto`: `--seats openrouter` or `orN` ids; mixed with other seats it is a preflight error (exit 3). | OpenRouter seats `or1`..`or6` |
| `SEATS_ENV_FILE` | `:SeatPool` ResolveSeatsFile (run's Gate 0, `seats`) | `<cwd>/.env` if it is a file holding a seat or a placement setting, each with a value (or no UTF-8 text: its error is the command's), else the machine's `$XDG_CONFIG_HOME/crafter-review/env` (`~/.config/crafter-review/env`) | `--env-file` wins (in run and `seats` even `--env-file ""`, meaning no file); empty counts as unset. Seat lines are taken (`[export ]NAME=value`, one pair of matching quotes stripped), overriding the process environment; the file's placement settings (`SeatPool::PlacementSettings`) go, under the process environment, into the map the placement readers take (`SeatPool::SettingsOf`, from the one read the tokens come from too), so every placement knob below may come from the file, the process environment winning; no other reader sees a line of it, and its other `REVIEW_GATES_*` lines are named (the run's log, `seats` on stderr). A file that cannot be read, or is not UTF-8: run stops at preflight (exit 3), `seats` exits 1 with the read error. | The dotenv file the seat secrets come from |
| `XDG_CONFIG_HOME` | `:SeatPool` GlobalEnvFile | `~/.config` | The directory whose `crafter-review/env` is the machine's own seats file, the last place a run or `seats` looks (2026-11-01). Runs, Commands and KernelModes, the tests that run `run` and `seats`, point it (and `HOST_XDG_CONFIG_HOME`) at a path nothing creates, so a global file on the machine stays out of them. |
| `REVIEW_GATES_SEATS` | `:Driver` RunCommand → LoadSeats | `auto` | Only without `--seats`; the process environment's alone, never from the seats file. Stripped, case-sensitive; unset or empty is `auto` (every api and oauth seat, else `local`); `local`, `api`, `oauth`, `openrouter` select that kind; anything else is a comma list of ids (`apiN`, `N`, `orN`, `local`; blank items skipped). An id not configured, or OpenRouter mixed with others, is a preflight error (exit 3). | The seats a run places lens processes on |
| `REVIEW_GATES_SEAT_MAX_UTIL` | `:Seats` ConfigFromEnv (`seats`; run through `:SeatPool` PoolConfig) | `85` | A finite number >= 0, in decimal or exponent notation; surrounding whitespace, `_` between digits and Unicode digits are taken. Empty, garbage, negative, `inf`, `nan` keep the default; a whole value is stored as an int. The process environment, else the seats file (`SeatPool::SettingsOf`). | Ceiling (%) of an OAuth seat's projected 5-hour utilisation |
| `REVIEW_GATES_LENS_COST_PCT` | `:Seats` ConfigFromEnv; `:SeatPool` PoolConfig | run: `0.5` at max and any other effort, `0.125` at xhigh; `seats`: `10` | As MAX_UTIL. PoolConfig puts in the effort's figure only when it is unset or blank, so a set but invalid value gives 10, in run too. | Percentage points of the 5-hour window a running process is projected to add (its cost on the board too) |
| `REVIEW_GATES_SEAT_POLICY` | `:Seats` ConfigFromEnv | `spread` | Exactly `pack` (case-sensitive, untrimmed) is pack; anything else spread. | pack: the first eligible OAuth seat by id; spread: the lowest projected one |
| `REVIEW_GATES_SEAT_WAIT_MIN` | `:Seats` ConfigFromEnv | `15` | As MAX_UTIL | Shown by `seats --json` as `waitMaxMin`; nothing else reads it |
| `REVIEW_GATES_SEAT_HANDOVER_PCT` | `:Seats` ConfigFromEnv | `90` | As MAX_UTIL | Shown in the `seats` report and `--json` only |
| `REVIEW_GATES_OAUTH_CONCURRENCY` | `:SeatPool` PoolConfig | `128` | As MAX_UTIL, truncated toward zero. | Passes at once on one OAuth account or the local login, every driver's counted |
| `REVIEW_GATES_LENS_COST_7D_PCT` | `:SeatPool` PoolConfig | `0.04` at xhigh, `0.125` otherwise | As MAX_UTIL. | Percentage points of the weekly window a running pass is projected to add: the placement of an account without a 5-hour limit, and `cost7d` on the board |
| `REVIEW_GATES_SEAT_ORDER` | `:SeatPool` ApiConfig | `api-first` | Exactly `oauth-first` is oauth-first; anything else api-first. | Whether the paid seats (API, OpenRouter) or the OAuth/local ones rank first |
| `REVIEW_GATES_API_BUDGET_USD` | `:SeatPool` ApiConfig (run, seats) | `200.0` | MAX_UTIL's syntax, kept if >= 0 (`inf` means no cap); empty, garbage, negative, `nan` give the default. `--api-budget` overrides it in run. | USD one API key may spend per cycle; at it the key is out |
| `REVIEW_GATES_API_LENS_COST_USD` | `:SeatPool` ApiConfig | `1.0` | As API_BUDGET_USD | USD reserved per running process: a pass fits while spent + (running+1) × this + other drivers' cost ≤ budget |
| `REVIEW_GATES_API_CONCURRENCY` | `:SeatPool` ApiConfig | `8` | As API_BUDGET_USD, truncated toward zero; `0` keeps every pass off the API seats; `inf` exits 1 with an OverflowError. | Processes at once on one API seat, every driver's counted |
| `REVIEW_GATES_API_CYCLE_DAY` | `:SeatPool` ApiConfig | `1` | As API_CONCURRENCY, then clamped to 1..31; a day past a month's end is its last day. | The grant day of an API seat without its own `_GRANT_DAY`, and the day the driver's spend on every OpenRouter key starts over |
| `REVIEW_GATES_OPENROUTER_BUDGET_USD` | `:SeatPool` ApiConfig | `100.0` | As API_BUDGET_USD | USD per cycle per OpenRouter key |
| `REVIEW_GATES_OPENROUTER_LENS_COST_USD` | `:SeatPool` ApiConfig | `0.1` | As API_BUDGET_USD | Reserve per process on an OpenRouter seat |
| `REVIEW_GATES_OPENROUTER_CONCURRENCY` | `:SeatPool` ApiConfig | `16` | As API_CONCURRENCY | Processes at once on one OpenRouter seat |
| `REVIEW_GATES_SCRATCH` | `:Run` ScratchRoot (run, slots); `:SeatPool` LedgerPath (run, seats) | `~/.cache/review-gates` | `--scratch` wins; empty counts as unset; used verbatim; never from the seats file. | Where the `<hash8>-run-<n>` dirs, `memory.json` and `seat-ledger.json` live; not the board |
| `REVIEW_GATES_BOARD` | `:SeatPool` BoardPath (run, seats, stop) | `~/.cache/review-gates/seat-board.json` | Non-empty: the board file, verbatim; its directory also holds `drivers/`, the lifetime locks; never from the seats file. | The one board per machine every driver shares; set it for tests and hand-made boards |
| `REVIEW_GATES_MEM_RESERVE_MB` | `:Memory` Reserve (run's admission, slots) | `max(2048, MemTotal / 10)` MB | The process environment's alone, never from the seats file. Unset or empty: the default. Otherwise a number in MAX_UTIL's syntax, truncated toward zero; a negative value enlarges the budget. Garbage and `nan` exit 1 with a ValueError, `inf` with an OverflowError; a value past int64 saturates. | MB of available memory never given to passes or slots |
| `CRAFTER_REVIEW_HOME` | `:Core` Home | `<exe>/../share/crafter-review` (the Flatpak's `/app/share/crafter-review`), else the checkout when the exe runs from `<checkout>/bin/crafter.review-exe-*/` | Natively only; ignored in the Flatpak. Empty counts as unset. A set value without `process/lenses` is an error, with no fallback. | Where `process/` (the lenses, the templates, the skill) is read from |
| `REVIEW_GATES_CLAUDE_BIN` | `:Pool` Runner (lens processes, run's probes); `:Seats` FindClaudeBin (second) | FindClaudeBin's search | Non-empty: used verbatim, unchecked; never from the seats file. In run: `--claude-bin`, then this, then FindClaudeBin. | The claude binary |
| `CLAUDE_BIN` | `:Seats` FindClaudeBin (`seats`, run's last fallback) | after it: `claude` on PATH, the editor-extension bundles (`~/.vscode-oss`, `.vscode`, `.vscode-insiders`, `.cursor`; newest first), `~/.claude/local/claude`, `~/.local/bin/claude`, then the bare `claude` | Non-empty: verbatim. FindClaudeBin checks it before `REVIEW_GATES_CLAUDE_BIN`, so it wins in `seats` and loses in run. | The claude binary |
| `REVIEW_GATES_OPENCODE_BIN` | `:OpenCode` DefaultBin (`--harness opencode`) | `opencode` on PATH | `--opencode-bin` wins; empty counts as unset; never from the seats file. | The opencode binary |
| `REVIEW_GATES_DB` | `:PgWire` SqlMain → CallBroker (`crafter-review sql`, in a pass) | none: NO DATABASE | `<host>:<port>#<token>`, split at the last `#`, then at the last `:`. Unset, empty or no `#`: NO DATABASE, exit 2; a port that is no integer: NO DATABASE with the parse error, exit 2; a port outside 0..65535: exit 1 with an OverflowError. The driver sets it per pass (`""` without a database) and strips an inherited one. | The driver's SQL broker and this pass's database token |
| `REVIEW_GATES_STRUCTURED_OUT` | the opencode tool `:OpenCode` ToolSource generates (reads `process.env`) | none | A file path; unset or empty: the tool writes nothing and still reports success. Set per process by `:OpenCode` ChildEnv. | Where an opencode pass's structured result goes |
| `REVIEW_GATES_SIMULATE_FAIL` | `:Pool` Runner (`--dry-run` only) | none | The process environment's alone, never from the seats file. Split on commas, items untrimmed, empty ones dropped: a unit id fails that unit's block pass, `whole` every whole pass. Ignored without `--dry-run`. | A test hook: "simulated failure (REVIEW_GATES_SIMULATE_FAIL)" |
| `PATH` | `:Process` spawn and the supervisor; `:Seats` FindClaudeBin, ProbeSeat | unset: `/usr/local/bin:/usr/bin:/bin` (spawn), `/usr/bin:/bin` (supervisor); the Flatpak sets `/app/bin:/usr/bin` | Colon-separated; an empty entry is the cwd; a name with `/` is used as is. | Where child programs (claude, git, npx …) are found |
| `HOME` | `:Platform` Home (every `~` path); `:Seats` ProbeEnv | unset or empty: the password database's, else `/` | Verbatim. | `~/.cache/review-gates`, the XDG defaults, claude's install locations, `~/.cache/ms-playwright`, init's home-directory check |
| `TZ` | `:Seats` LocalZone (the `seats` report); the C library behind `:Platform` LocalTime | the zone `/etc/localtime` points to, else the C library's `%Z` | A leading `:` stripped. `UTC`, or a name with a file under `/usr/share/zoneinfo`, is the zone (the C library converts with TZ set to it: libc++ has no tzdb); any other name only labels the report. | The zone of the `seats` report and of every local timestamp (the default `--date`, the ledger's cycle) |
| `TMPDIR` | libc++'s `temp_directory_path`, from `:Seats` ProbeSeat and `:Freeze` | `/tmp` | The library's lookup: TMPDIR, TMP, TEMP, TEMPDIR. | Where the probes' config dirs (`rg-seat-*`, removed after) and freeze's temp dirs go |
| `HOST_XDG_{CONFIG,CACHE,DATA,STATE}_HOME` | `:Platform` RestoreHostXdg (Init, the sandbox only) | `$HOME/.config`, `.cache`, `.local/share`, `.local/state` | Non-empty: copied verbatim into `XDG_<KIND>_HOME`, which is always overwritten. Never read natively. | The host's XDG dirs for git, claude, npx and Playwright, and for the machine's own seats file (`$XDG_CONFIG_HOME/crafter-review/env`, `SeatPool::GlobalEnvFile`), instead of `~/.var/app/…` |
| `SHELL` | `:Platform` RestoreShell (Init, the sandbox only) | — | Kept when it ends in `/bash` or `/zsh` and is executable; else `/bin/bash` when that is executable. Never read natively. | The shell claude's Bash tool runs commands with |
| `XDG_RUNTIME_DIR` | `:Platform` Paths (at Init, the sandbox only) | `/run/user/<uid>` | Empty counts as unset; verbatim. | Where `.flatpak/<instance>/bwrapinfo.json` gives the driver its host-pid identity |
| `XDG_CACHE_HOME` | `:Driver` PlaywrightBrowsers (`browser install`) | `~/.cache` | Empty counts as unset; in the Flatpak it already holds the host's. | The parent of `ms-playwright` when PLAYWRIGHT_BROWSERS_PATH is unset |
| `PLAYWRIGHT_BROWSERS_PATH` | `:Driver` PlaywrightBrowsers (`browser install`) | `$XDG_CACHE_HOME/ms-playwright`, else `~/.cache/ms-playwright` | Non-empty and not `0`: verbatim; `0` or empty: the default. | Where `browser install` checks that `chromium-<rev>` landed |

At build time:

| Name | Read by | Default | Parsing rule | What it does |
|---|---|---|---|---|
| `CRAFTER_REVIEW_VERSION` | `project.cpp` Version(), when crafter-build configures | `git describe --tags --always --dirty --exclude=latest`, else `0.0.0-dev` | Non-empty: verbatim. It becomes the library's `CRAFTER_REVIEW_VERSION` define, so each value is a library variant of its own. | The version `--version` and the driver line print; the manifest sets `0.0.0-dev`, CI the tag's |
| `CRAFTER_BUILD_MARCH`, `CRAFTER_BUILD_MTUNE` | Crafter.Build's ApplyStandardArgs | `native` | Non-empty: `-march` / `-mtune`; a `--march=` / `--mtune=` argument wins. | The manifest sets `x86-64-v2` and `generic`, so the shipped binary runs on any x86_64 of the last decade |

In the tests only:

| Name | Read by | Default | Parsing rule | What it does |
|---|---|---|---|---|
| `PATH` | LensTools | `""` | Colon-separated | Every program a lens's allowlist names, bash, node and npx must be on it: a skip natively, a FAIL inside the Flatpak |
| `FAKE_*` | the claude stand-ins of Runs and Scheduler | set by the test | — | Plumbing between a test and its stand-in |

Runs and Commands drop every `REVIEW_GATES_*`, `CRAFTER_REVIEW_*`, `SEATS_ENV_FILE` and seat secret (Runs also `FAKE_*`, Commands also `CLAUDE_BIN`) from the environment they run the binary with, then set their own `CRAFTER_REVIEW_HOME` and `REVIEW_GATES_BOARD` (Commands also `REVIEW_GATES_SCRATCH` and `TZ=UTC`), and Runs, Commands and KernelModes pin `XDG_CONFIG_HOME` and `HOST_XDG_CONFIG_HOME` (the Flatpak's `Platform::Init` copies the latter over the former) to `no-config` in their own temporary directory, a path nothing creates, so a developer's own `~/.config/crafter-review/env` stays out in both modes: a developer's own settings never reach them.

## CI and publishing

`.forgejo/workflows/ci.yaml` is Crafter.Build's workflow for this app.
- **Builds.** A pull request builds the Flatpak, runs the suite inside it and smoke-tests the app as it ships: the version and home, every library resolved and no RUNPATH outside `/app`, the bundled tools, and a freeze and a dry run on a scratch repository.
- **Publishing.** Every push to master is a release (2026-11-02, Jorijn: no rolling `beta` or `latest` builds): CI publishes `stable`, tags the pushed commit with the version of the metainfo's newest `<release>` and creates the release with its notes. A push to master therefore adds a new `<release>`; one whose version is released already fails the job before anything is built or published.
- **The runner.** It runs on the Catcrafts organisation's privileged `flatpak` runner, in two jobs by trust, as imsd-messages' workflow does: `check` (a pull request, a run by hand on another branch) builds cold with no host volume; `publish` (master) mounts the crafter-build installation the publish jobs share (`crafter-flatpak-org-system`), the published repository and its signing key. flatpak-builder's state stays in the workspace, so every run builds cold. The workflow is unverified on the forge until its first run; its smoke test passes when run by hand against a local build.
- **The shared repo.** Both apps publish into one repo. The publish step holds `/srv/flatpak/.publish.lock`, which protects anything only once Crafter.Build's workflow takes the same lock. This one writes its own flatpakref and never the shared `catcrafts.flatpakrepo`.

Pins, moved together:
- the Flatpak's base, `net.catcrafts.CrafterBuild` `stable` (Crafter.Build 3.0.0 today), whose crafter-build and LLVM compile the project, with the manifest's `runtime-version`;
- the Crafter.LintRules pragma in `project.cpp`, a commit that builds with the base's crafter-build;
- the Crafter.Network commit in `project.cpp`. While Crafter.Network commits are not on forgejo, `crafter-build --local run flatpak --deps=local` builds against the checkouts next to this one.

`run flatpak` and `run install` mirror Crafter.Build's scripts. `--base=<branch>` and `--deps=local` write a gitignored copy of the manifest, and flatpak-builder keys its cache on the manifest's path. The cleanup is a keep-list prune (`packaging/flatpak/prune-base.sh`), because top-level `cleanup` does not reach the base's files.

## Licence

crafter-review is AGPL-3.0-only, Copyright (C) 2026 Catcrafts®. nlohmann/json is MIT, vendored unmodified.