Files
Panama/user/agents/skills/ticket/COUNCIL.md
T
Gabriel Brown c6d299b675 Seat the ticket review council on Claude Opus 5.5 and Codex GPT-6.1 Sol
MiMo leaves the council. Codex is now installed and signed in through T3,
so the second model family comes from GPT-6.1 Sol instead.
2026-11-05 15:30:54 -05:00

138 lines
7.1 KiB
Markdown

# The review council
Several fresh agents from different model families review the same change, each knowing nothing
of how it was built. You judge what they return. This runs before `pre-mr-review`, because
that audit may not run anything and so cannot catch a claim that is plausible on the page and
false in reality. A reviewer that can run things can. It has caught a mitigation proven by
construction to be wrong, on a branch whose own audit had blessed it.
Order matters. `pre-mr-review`'s verdict gets pasted verbatim into `mr.md`, and anything that
lands after it invalidates that verdict. Council first, fix, commit, then the audit over the
final tree.
## Seats
| Seat | `providerInstanceId` | `model` |
|---|---|---|
| Claude | `claudeAgent` | `claude-opus-5-5` |
| Codex | `codex` | `gpt-6.1-sol` |
Klaude never takes a seat. It is the work account, and its only job is the `pre-mr-review` run
in Phase 2 step 7, which stays a separate required step after the council.
Adding a seat is adding a row. `orchestrator_capabilities` has the IDs, with `search` for a
model it does not list. Nothing else here changes with the seat count. T3 can leave out a model
the provider really has, as it does with `opencode-go/*`. List such a model under that provider's
`customModels` in `~/.t3/userdata/settings.json`, which T3 picks up without a restart.
## When it runs
Run it whenever the change adds or modifies a procedure, touches schema or migrations, touches
permissions or auth, involves concurrency, or has a risk table in its ticket. When unsure, run
it. A copy tweak, a styling fix, or a one line correction with no logic skips it.
## Convene
1. Confirm the suite is green, and note the base and head of the commit range. A seat should
never spend its review on code the suite already rejects.
2. In one turn, call `delegate_task` once per seat with the brief below: `role: "review"`,
`mode: "async"`, a title of `Council <seat> <KEY>`, and the seat's `target`. Leave `isolation`
unset, so each seat reads this checkout, and leave `runtimeMode` unset.
3. End your turn. T3 wakes you as each seat finishes. Leave the checkout alone until every seat
has returned, because they are reading it.
4. A seat refused with `model_unavailable`, or one that fails: record it with `ticket-page event`,
carry on with the rest, and name it at hand-off. Keep the seat empty rather than filling it with
another model. If no seat can run, ask Gib with AskUserQuestion. A timeout or silence is not
an approval.
Outside T3 Code (no `T3CODE_THREAD_ID`), run one fresh subagent from the harness with the same
brief. Never review the change in your own context: the value here is the reviewer not being
the author.
### The brief
A delegate gets none of this thread, so the brief is all it knows. Fill the placeholders.
```
Review only. Do not edit, commit, push, run anything destructive, or start subagents.
Repo: <absolute path> Range: <base>..<head> Ticket: <KEY>
Read in this order, or you will report conventions this project already settled:
1. .claude/docs/epics/<EPIC-KEY>/summary.md, if it exists
2. <scratch path of `ticket-page md <KEY>`>
3. <target-dir>/resources/
4. AGENTS.md
5. git diff <base>..<head>, then the code around it
Verify rather than suspect. You may run tests and read-only checks. Try to refute each finding
before you report it. A plausible worry is not a finding.
Re-derive every "proven by X" already written into the ticket's risk or test tables. A false
proof claim in Jira is worse than an unproven one, because it stops anyone else from checking.
Answer these against the ticket and plan, not the diff alone, quoting the AC ID or plan step:
- What did the ticket ask for that is missing or only partly done?
- What behavior is in the diff that nothing asked for?
- What looks implemented but is implemented wrongly?
Hold the change to its smallest correct form. Flag code, tests, or guards the ticket does not need.
blocker = the intended case is wrong. should = a real bug callers will hit. nit = everything else,
including rare paths, small performance costs, and style.
Reply with:
VERDICT: APPROVE or CHANGES
S1 [blocker|should|nit] [CONFIRMED|PLAUSIBLE] <file>:<line> <claim>. Failure: <concrete scenario>. Fix: <smallest fix>
```
## Judge the findings
Findings are not automatically true, and reviewers produce confident wrong answers. Check each
one against the code yourself, and regrade it: a rare path is a nit whatever label it came with.
Two seats reporting the same thing is one finding with more weight behind it. A finding only one
seat raised is still worth checking, since catching what the others miss is why the seats differ.
Put each on the page as you judge it, with its grade and its seats in the text:
`finding <KEY> add "[should] <text> (Claude, Codex)" --severity CONFIRMED --where <file:line>`, then
`fixed --commit <sha>`, or `rejected --reason "<why>"`.
- **Fix every blocker and should you confirm,** each meaningful fix as its own commit. These do
not defer to a follow-up ticket unless Gib decides that.
- **Take the smallest fix.** Prefer deleting or narrowing code over a new lock, cache, helper, or
option. Compare `git diff --shortstat <base>` before and after: if review grew the diff, cut
first. Review loops grow a change one cheap-looking fix at a time, and it is your job to hold
the scope.
- **Decline nits in one line** with `rejected --reason`, and list them at hand-off as follow-ups.
Agreeing just to end the review is how a change bloats.
- **When a fix changes a mechanism rather than a line,** write a test that fails against the old
behavior and passes against the new one, and confirm it fails by reverting the fix. A test that
passes either way proves nothing.
- **If a finding invalidates something already in Jira or a deliverable,** correct it there too.
Fixing the code does not fix the false claim.
## Stop
One round is the default. `pre-mr-review` reviews the whole diff, your fixes included, and serves
as the next pass. A second round runs only if a fix changed a mechanism: fresh `delegate_task`
calls, the same brief, the ledger of what you fixed and rejected with your evidence, and an
instruction to check only the open ids and the changed code. Two rounds is the limit. A finding
still disputed then goes to Gib with AskUserQuestion, both sides in two or three lines each.
## Checking a dev review
`review-ticket` borrows the seats for a draft instead of a diff. Same `delegate_task` settings, and
no code gets changed. Give each seat this:
```
Review only. Do not edit anything, write to Jira, or start subagents.
Repo: <absolute path> Draft: <dir>/review.md Story: <scratch path of the fetched story>
Check the draft against the code. Report each Current State claim that is false, each "does not
exist" that does, each named pattern that is a poor fit or has a better neighbour, each seam the
Relationships section gets wrong, and any story AC the implementation guidance would not satisfy.
Say whether the estimate misses work the code paths table leaves out.
Reply with:
VERDICT: APPROVE or CHANGES
S1 [blocker|should|nit] <section> <claim>. Evidence: <file:line>. Fix: <suggestion>
```