feat(code-review)!: two rosters in the default, and a manifest a repo can start from

[pedromvgomes]

Follows #84. Five things, one of them breaking.

The default reviews pull requests on codex

The change most likely to surprise, because it applies to every repo that has not written a manifest — which is most of them, since working without one is what makes adoption cheap.

The default now carries a roster per provider with the same panels and rules for each: the local pass is Claude (quick), the pull request is codex (standard-codex). A change is read by two models trained differently before anyone else sees it, rather than twice by the model that already read it. The codex panels judge and validate on codex too, so a PR is reconciled without the model that reviewed it locally. Every model is named — a reviewer on the CLI's own default is a model nobody chose.

run --pr therefore needs codex on PATH. A machine without it does not get a silent half-review: the codex runs report they could not start, and if nothing answered at all the run refuses rather than reporting an empty finding set.

The escalation criteria changed with it:

criterion	panel	why
auth, crypto, concurrency, sensitive-data, fix-revert	deep	exploitable, or lands on somebody not in the room, or indistinguishable from correct until production
referencing_files >= 20	deep	unchanged
migrations, ci-cd, iac	standard	irreversible or wide, but the risk is not in the diff
changed_files >= 20	standard	unchanged

auth and migrations escalated on path globs, which duplicated — badly — signals the binary already detects and tests. Demonstrated: request-gating middleware under internal/session/ carries the auth signal, matched no glob, and got the shallowest panel. They are signals now. migrations drops to standard because its risk is the table it locks, not anything legible in the diff.

context, so a rule can say where it applies

A rule names one panel, so a two-roster manifest could not say "this criterion, but on the other roster for a pull request". Writing it twice does not work either — equal cost is not a raise, so the first-written rule wins in both contexts, decided by an order the design says carries no meaning.

  - to: deep-codex
    all:
      - signals: {in: [auth]}
      - context: {in: [pr]}

in and not_in over worktree/pr; unknown names refused at parse. The one condition that tests the run rather than the change, so it can never be unavailable.

It exposed a bug: an all: rule stopped at its first unreadable condition, so a rule guarded to the other context looked unreadable rather than inapplicable. A repo whose language has no symbol extractor saw every PR-only rule reported as skipped on every local review, and a repo-written manifest refused the review outright.

agtk code-review init, and initialize → register

A manifest was never required — a repo without one uses the default in the binary, which is what makes adoption work. What was missing was a way to start from it. init writes it to .agents/code-review/manifest.yaml, refuses to overwrite without --force, and --dry-run reports the path.

That forced a rename: initialize stores a GitHub App private key, an unrelated job, and init beside initialize is two near-identical words where the shorter is a strict prefix — a typo runs the other. It is register now, which its own summary always called it. initialize still resolves as an unlisted alias and says so on stderr.

Skill: panel and commit targets

/panel-code-review quick and /panel-code-review the last commit both work now. Neither was documented; the commit target was a regression from deep-code-review, which took ranges.

A named panel silences the rules — --panel wins outright, so an escalation that fired does not raise past it. The skill now reports that rather than printing the panel as though the rules chose it. And it always passes --head with a commit, since that flag defaults to the working tree: --base HEAD~1 alone reviews the last commit plus anything uncommitted.

Reviewed, and what it found

/code-review on this branch returned seven findings. All seven were verified against running code and fixed. Two mattered:

• init broke reviewing outright. It promised "the default verbatim, so nothing changes", but Builtin is provenance: the default's referencing_files rules are skipped only while they ship in the binary, and refused once a repo owns them. Same bytes, panel: quick before and agtk: escalate[2]: … after. The refusal is correct — a repo with a manifest can act on the advice — so init now names the rule it just made a refusal, and the remedy. The original test only asserted the bytes parse; parsing is not selection.
• The escalation fix went too wide. Suppressing the refusal on any readable no meant a repo's broken rule passed for a change outside its globs and refused for one inside — failing by mood. Only a context guard settles a rule now.

Plus: --dry-run refused exactly when the path question was worth asking, gofmt (which would have failed CI), a comment claiming two rules "cannot fire" when they fire and merely cannot raise, and release notes that never mentioned the codex requirement.

Verification

gofmt -s, go build, go vet, full suite — clean. New tests cover the context condition and its guards, the narrowed refusal, init (verbatim write, overwrite refusal, dry-run with and without an existing manifest, and that it names an unrunnable rule), and the register/init split. Each was mutation-tested — nine mutations across these commits, each caught by its intended test, code restored.

Behaviour verified end to end against the built binary, not only in tests: both rosters and their context guards, init before/after on a repo whose language has no symbol extractor, commit and range targets, and --panel overriding a fired rule.

Open, and yours to call

• The codex model picks are inferred. astra on security and judge, sol elsewhere — version number is my only evidence; the driver refuses to map families across vendors, so there is no opus-equivalent to lean on.
• The two standard-codex rules cannot raise while that is also the PR default. Kept so both rosters read identically, and so lowering the default does not silently drop a criterion.
• --panel quick on a PR runs the Claude roster, crossing the local/PR split. Correct for an explicit override, but both quick and quick-codex are reachable from either target.
• feature-flags, message-consumers, public-api, shared-kernel still do not escalate — too loose for a shipped default (shared-kernel fires on all of pkg/, public-api on any Go struct tag).

[github-actions]

bulwark

• ✅ scan — no findings

• ✅ coverage — go: 81.3% (baseline 81.2%); go patch: 87.9% (131/149 new lines; baseline 81.2%)

📦 Full bulwark output — complete scan and coverage logs.