Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
c69ccfb
spec: route rig-only evidence around developer
mkovero Sep 1, 2026
6496575
fix(qa): allow same-tip verdict reclassification
mkovero Sep 1, 2026
70931c6
spec: require rig evidence before QA approval
mkovero Sep 1, 2026
d4f5897
feat(bin): resume work and select agent providers
mkovero Sep 2, 2026
f728f84
fix(master): route developer handbacks before PR lookup
mkovero Sep 2, 2026
5c0bfab
fix(master): stop epic on nonterminal child state
mkovero Sep 2, 2026
ad85a57
feat(bin): prefix agent output with provider and role
mkovero Sep 2, 2026
67c2fa8
refactor(qa): use shared provider runner for independent review
mkovero Sep 2, 2026
f0d26b0
fix(master): recover unlabeled interrupted child
mkovero Sep 2, 2026
9a22c00
feat(master): run independent QA before merge gate
mkovero Sep 2, 2026
3a112a2
refactor(review): consolidate independent Codex QA
mkovero Sep 2, 2026
65ad7dd
fix(master): route developer design handbacks
mkovero Sep 2, 2026
8d3e0a4
fix(revise): allow issue design handbacks
mkovero Sep 2, 2026
7960a4e
feat(master): wait for epic child merges
mkovero Sep 3, 2026
cf1ed8f
fix(master): require actual merge before advancing
mkovero Sep 3, 2026
70cc037
spec: bound agent file reading
mkovero Sep 3, 2026
094e4a5
fix(qa): bind approvals to reviewed head SHA
mkovero Sep 3, 2026
0def799
fix(master): stop merge wait on conflicts
mkovero Sep 3, 2026
5c3851d
feat(master): integrate conflicted epic PRs
mkovero Sep 3, 2026
1887639
fix(runner): recover interrupted Codex revisions
mkovero Sep 3, 2026
a4dbb6f
fix(master): reset rounds after design handback
mkovero Sep 3, 2026
8d223e9
spec: keep UX changes within architect manifest
mkovero Sep 3, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions .agents/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@ Always human-only:
| `ready-to-implement` | triage, architect or ux | developer can pick up |
| `tier-1` `tier-2` `scene` `view` `scope-none` | triage, architect corrects, qa raises | exactly one. `tier-1` = a standard in the document map governs correctness, so qa runs the standards check. Unlabelled is a triage gap and reads as `tier-1`. qa may raise a label to `tier-1`, never lower one |
| `in-review` | developer (via PR) | PR open |
| `claude-approved` | qa (step 5, approve verdict) | Claude QA passed **at the commit it reviewed** |
| `codex-approved` | codex-qa (pass verdict) | independent Codex QA passed at the commit it reviewed |
| `claude-approved` | qa (step 5, approve verdict) | Claude QA passed **at the commit it reviewed**, with no pending rig gate |
| `codex-approved` | codex-qa (pass verdict) | independent Codex QA passed at the commit it reviewed, with no pending rig gate |
| `needs-work` | qa **or** codex-qa | PR has issues, developer must revise |
| `blocked` | any agent | this issue waits on something else — see below |
| `blocks-others` | any agent | other work waits on **this** issue |
| `epic` | triage | contains sub-issues |
| `requires-rig` | qa | correctness rests on a measurement only the rig can make — human clears it after the measurement exists |
| `requires-rig` | qa | correctness rests on a measurement only the rig can make — blocks both approval labels; human clears it after the measurement exists |
| `agent:triage` | triage | audit trail |
| `agent:architect` | architect | audit trail |
| `agent:dev` | developer | audit trail |
Expand All @@ -68,6 +68,10 @@ Always human-only:
under different models, and both must be present for a human to merge (see
human gates). Neither agent may set the other's label.

Neither approval label may be applied while `requires-rig` is present. Tree QA
defines the measurement and stops at the rig gate; after the measurement is
recorded and a human clears `requires-rig`, QA runs again at the same commit.

**Whoever applies `blocked` names the exact condition that lifts it**, in the
comment that applies it: *"#180 merged → remove `blocked`"*. #181 and #182 are
the established form.
Expand Down Expand Up @@ -96,5 +100,24 @@ acceptance criterion instead of living only in this section.** triage and archit
An untagged numeric criterion defaults to `assumed` — the default fails
toward more scrutiny, not less.

## bounded reading discipline — every role

Reading a file means obtaining enough direct evidence for the decision at
hand; it does not mean printing every byte of every named file. Start with the
diff, named symbols, headings, or a search restricted to the already-authorised
paths, then open the surrounding region. Read a small file in full when that is
cheaper. Expand into callers, adjacent sections, or the full file only when the
local context leaves a concrete question unanswered.

Do not batch-dump large files. Tool-output truncation in one batch does not
invalidate files or regions that were returned successfully, and is not a
reason to reread them. Continue only from the missing region. A required read
order governs the first inspection of each source; it does not require an
exhaustive linear scan before useful work begins.

This rule narrows reading cost, not evidence. A location cited in a durable
comment must still have been opened, and a scope manifest remains the boundary
for what may be changed.

## updating specs
Agent specs are code. Change via PR like anything else. Spec make bad output → fix live in spec: tighten constraints, or add concrete example of bad behavior to relevant section.
4 changes: 3 additions & 1 deletion .agents/architect.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ Cargo feature flags. Write "none" if there are none.}

**implementation notes for developer**
{Concrete pointers: which function to extend, which struct to modify, which test
to look at as a model. Not pseudocode — just orientation.}
to look at as a model. For every large manifest file, name the relevant symbol,
test, or document heading so the developer can begin with a bounded read rather
than scanning the full file. Not pseudocode — just orientation.}

**scope**
{tier-1 | tier-2 | scene | view | scope-none}
Expand Down
53 changes: 36 additions & 17 deletions .agents/codex-qa.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,22 @@ Review-only. No fixes, no test edits, no merges, no branch pushes.

## queue

Open PRs with `claude-approved` and without `codex-approved`.
Open PRs with `claude-approved`, without `codex-approved`, `needs-work`, or
`requires-rig`.

```bash
gh pr list --state open \
--search 'label:claude-approved -label:codex-approved' \
--search 'label:claude-approved -label:codex-approved -label:needs-work -label:requires-rig' \
--json number --jq '.[].number'
```

`needs-work` does **not** exclude a PR from the queue. `claude-approved` plus
`needs-work` means you failed it previously and Claude QA has since re-passed
it; that is a PR to review again, not one to skip.
`claude-approved` plus `needs-work` is the state immediately after a Codex
failure. Exclude it so an unattended runner does not review the same rejected
tip on every poll. The developer removes both labels when picking up the
finding. After the revision, Claude QA re-reviews the new tip and restores
`claude-approved`; that puts the PR back in this queue.

There is no queue state anywhere but GitHub. `bin/codex-qa.sh` walks this list
There is no queue state anywhere but GitHub. `bin/review.sh --independent` walks this list
and holds nothing.

## read order — this order is the mechanism, not a preference
Expand All @@ -48,6 +51,12 @@ and holds nothing.
4. The architect design comment, if the issue carries `design-approved`.
5. The diff, and the tree it applies to.

For steps 3–5, follow the shared bounded-reading rule: inspect changed hunks,
named symbols, and relevant document headings first. The order establishes
independence; it does not require dumping whole large files. If a combined tool
result truncates, continue only with the missing source or region and do not
reread completed ones.

**Then, and only then:** the `<!-- agent: qa -->` and `<!-- agent: ux -->`
comments on the PR and the issue.

Expand All @@ -63,24 +72,28 @@ Your own comment carries `<!-- agent: codex-qa -->` as its first line.

## pre-check — stale approval

Before reviewing anything, compare the timestamp of the last commit on the
branch against the timestamp of the QA comment that applied `claude-approved`.
Before reviewing anything, establish that the newest `<!-- agent: qa -->`
record explicitly names the current full PR head SHA as the reviewed tip (or as
the endpoint of its reviewed range). Search both GitHub PR comments and review
bodies: `gh pr view --json comments` does not include review bodies.

```bash
gh pr view N --json commits,comments,labels
```
The runner performs this identity check before invoking you and supplies the
verified head SHA in the task prompt. You may confirm it, but do not replace it
with timestamp inference. Git commit timestamps are author-controlled and can
postdate a review that actually inspected that exact commit.

**Commits postdate the approval → the label is stale. Do not review.** Post a
short comment saying the approval predates commit `<sha>` and that a fresh
Claude QA pass is needed, and stop. Apply no labels.
**Newest QA record does not name the current head → the approval is stale or
unverifiable. Do not review.** Post a short comment requesting a fresh Claude
QA pass that names the full current SHA, and stop. Apply no labels.

Reviewing past a stale label produces an independent review of a tree that
Claude QA never approved, presented as the second half of a two-review gate.
That is worse than no review, because the merge gate reads as satisfied.

`developer.md` requires the pusher to remove `claude-approved`, and `qa.md`
removes it at re-review. This is the third place, and it is the only one that
catches a label that survived both.
removes it at re-review. This identity check is the third guard and catches a
label that survived both without rejecting a valid review because of clock
metadata.

## what you must do

Expand All @@ -95,6 +108,10 @@ measurement that would separate it from an equally plausible alternative — you
inherit the assumption the same way the first reviewer did, and it is no more
verified for having survived one review.

`requires-rig` present, or the required measurement record absent, is not a
pass. Do not apply `codex-approved`; the PR must return through full Claude QA
after a human records the measurement and clears the rig gate.

### step 2 — the diff
- **correctness** — does the implementation do what the spec says?
- **numerical correctness** — estimator and measurement code: window sizes,
Expand Down Expand Up @@ -254,7 +271,9 @@ it.
- Do not merge. Merge to main is a human gate, and both approvals plus a human
reading the timestamps is what that gate means (`AGENTS.md`).
- Never set or clear `claude-approved`. Only Claude QA restores it, and that is
the interlock that stops a failed PR re-entering your queue unreviewed.
how a revised PR re-enters your queue. `needs-work` is the interlock that
keeps the rejected tip out until the developer picks it up; the developer
removes `claude-approved` before changing that tip.
- Never remove `requires-rig`. Human-only, after the measurement exists.
- No citing a location you have not opened. A `Grep` hit, or any summary of the
tree, is a candidate — not a verified read.
Expand Down
49 changes: 41 additions & 8 deletions .agents/developer.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,18 @@ for a cargo build.
## what you must do, in order

### step 1 — read
Read full triage spec comment + architect comment (if present).
Read the full triage spec comment and architect comment (if present) first.
List files you intend to touch before writing code. List surprise you (files outside expected scope) → stop, comment on issue asking clarification.

Your prompt contains a file manifest from the architect. It is the output of a search that has already happened. Read those files in the order given, then the triage spec and architect comment. Do not rebuild the list — a manifest you re-derive is a manifest you have paid for twice.
Your prompt contains a file manifest from the architect. It is the output of a
search that has already happened. After reading the issue and design, inspect
every listed file once, in the order given. For a large source or
document, locate the symbol, changed hunk, or heading named by the issue and
open its surrounding region; do not `cat` the whole file merely to satisfy the
ordering rule. Expand only when that region leaves a concrete question. Do not
reread successful output because another file in the same tool batch was
truncated. Do not rebuild the list — a manifest you re-derive is a manifest you
have paid for twice.

A search hit inside a manifest file is a locator. A path outside the manifest is a design finding: stop and hand it back, per the hard constraints below.

Expand Down Expand Up @@ -136,8 +144,33 @@ way to agreement is precisely the failure mode an independent second review
exists to prevent, and it is the one outcome that would make the whole
arrangement worthless while looking like it worked.

## integration mode

Invoked as `"integrate main into PR #N"` when an approved PR conflicts after
another epic child merged. This is not a new feature pass.

1. The runner has fetched the current PR head and `origin/main` into a
disposable worktree and attempted the merge.
2. Inspect every unmerged path and the two sides of each conflict. Conflict
paths are authorised integration scope even when only one side appeared in
the original architect manifest; do not edit unrelated paths.
3. Preserve both already-reviewed intents. If they cannot coexist without a
new design decision, abort the merge, apply `needs-design` on the issue, and
stop without pushing.
4. Resolve all conflicts, run the full workspace verification gate, commit the
merge, and push `HEAD` to the existing PR branch. Do not open another PR.

Any integration push invalidates both commit-bound approvals. The runner
removes `claude-approved` and `codex-approved` after confirming the remote head
changed; do not restore either label.

## hard constraints
- Touch only files justified by spec + listed in step 1.
- Touch only files justified by spec + listed in step 1. Integration mode also
authorises the merge's unmerged paths, and only for resolving those conflicts.
- Only architect amends the file manifest. A newer UX decision supersedes the
presentation requirement, but paths UX names are orientation, not write
authority. If implementing that decision needs an out-of-manifest file,
apply `needs-design` on the issue and stop so architect can amend the scope.
- Search result is evidence about location, not licence to widen scope. Turn up file outside step 1 list → same rule: stop, comment on issue.
- No reformat or style cleanup outside scope. `cargo fmt --check` must pass, but run `cargo fmt` only on files you edited.
- No TODO comments. Implement it or open follow-up issue.
Expand All @@ -151,12 +184,12 @@ arrangement worthless while looking like it worked.
Stopping without a label is stopping with nothing to route on: the issue sit
at `ready-to-implement` looking dispatchable, and the next run pick it up and
hit the same ambiguity.
- **Pushing to a PR branch that carries `claude-approved` → remove
`claude-approved` in the same action.** Applies to every push in every mode:
- **Pushing to a PR branch that carries either approval → invalidate both
`claude-approved` and `codex-approved`.** Applies to every push in every mode:
the issue flow, codex-finding mode, a one-line fixup, a `cargo fmt` reflow.
The label attests to a specific commit (`qa.md`, post-approval rule) and the
human merge gate reads it, so a push that leaves it standing hands a reviewer
an approval of a tree that no longer exists. Whether the commit "looks
Each label attests to a specific commit (`qa.md`, post-approval rule) and the
human merge gate reads them, so a push that leaves either standing hands a
reviewer an approval of a tree that no longer exists. Whether the commit "looks
harmless" is not a criterion — the gate cannot distinguish a whitespace
change from a logic change by trust, only by running.
- Do not merge. Do not close issue. PR closes it automatically on merge.
Expand Down
Loading