Skip to content

v3 roadmap: one instruction, one reviewer list, one owner of the wait #42

Description

@kristofferR

Tracking issue for the v3 work: turning the agent-facing review protocol from prose into commands, and scoping the queue to the reviewer that actually meters it.

Why

crq's protocol lived in documentation. llms.txt, SKILL.md and crq help loop each described the same rules — drain findings before starting a round, hold the head while a reviewer is pending, pick a delay — and drifted apart. Nine of fifteen merged PRs before this changed that prose rather than any decision.

Agents then improvised the parts crq did not own, and the improvisations were uniform. Across 16 real driving sessions, every single one wrapped the tool the same way:

set +e; crq loop OWNER/REPO N > /tmp/out.json; echo "CRQ_EXIT:$?"

An exit code smuggled out through an echoed sentinel into a temp file. Several runs were SIGTERM'd mid-wait or orphaned at a session teardown, and restarting was wrong both ways: it either re-fired a review (spending account quota) or hit the dedupe and reported a converged round whose findings were never collected.

Guiding invariant: anything in the docs that tells an agent when to wait, hold, push, or resolve is a bug in crq next.

The sizing rule

#39 took four review rounds and 52 findings, and every round after the first found regressions introduced by the previous round's fixes. The reviewers were right each time. The cause was size — 24 files, a new command, a reordered decision path and changed decline semantics all interacting, so each correction moved something else.

So, for everything below:

  1. One thesis per PR. If the description needs "and also", split it.
  2. Converge in ≤2 review rounds. A third round that finds a regression from the second means land what is verified and move the rest out.
  3. Prefer the visible failure. Replace the agent review protocol with one instruction and a killable wait #39's worst moment was suppressing a finding to break a deadlock. A repeated instruction is annoying; a swallowed finding is harmful.
  4. Verify before writing. Three of the first seven planned items turned out to be already fixed. Re-read scope against main before starting anything here.

Done

In review

  • Every finding has a way to stop being returnedLet an agent account for a finding GitHub cannot close #48. crq dismiss for a finding GitHub gives no thread to close. Only intrinsically threadless sources, only from a round that has progressed, and the reason is stored.
  • The threads a push leaves behindList the threads a push leaves behind #50. After a push every previous-head thread is outdated and drops out of findings, so crq was forcing agents to the raw GitHub API — the one thing the skill forbids. Title parsing lives in dialect and is tested against the corpus.
  • Repo-local gitGive crq its own place on disk for a repository #53. A bare mirror per repository and a detached worktree per head, so the daemon can work in a repository it was not run inside. Credentials come from the same resolution the API client uses.
  • Act on a transition instead of logging itAct on a transition instead of logging it #54. crq watch [--dispatch] drives every open PR through the same next oracle and starts a fix session for the ones that need one. Sessions run concurrently, off the decision loop; the decisions stay serial, which is what keeps the metered review in one queue.
  • crq holdHold a PR in one write instead of two commands and a race #55. One write instead of a skip marker plus crq cancel and a race between them, rechecked in the CAS that reserves the round.
  • Tidy spent trigger commentsDelete the trigger comments nothing needs any more #56. Deletes crq's own review requests once the round has progressed and the bot has answered. Never the bots' comments: an auto-reply can be a rate-limit or skipped-review notice crq reads as evidence.
  • Per-repo reviewer configChoose which bots review which project #57. crq reviewers [set|clear] <repo>, stored in the state ref both the daemon and agents already read. The primary stays fleet-wide, because its markers are compiled into the classifiers.

Still open

Two things worth remembering about the work itself

Both cost several review rounds before being spotted:

  • When a claim keeps being wrong, consider deleting the claim. The queue's ordering was refined across four rounds — simulate it, fall back to Seq, drop positions while the slot is held — before it became clear that order past the front is not knowable at all, because slot release comes from acknowledgement or the in-flight timeout rather than pacing. Removing the prediction was less code than any of the refinements.
  • An exemption applied per-site will keep missing a site. "A co-only round is not governed by the queue" was fixed three times in three rounds — the account window, the ordering, the follower pass — before being made structural by partitioning those rounds out before any gate applies.

Ordering

The seven PRs in review are independent except for two stacks that have since
been flattened: #57 was rebased onto main after #51 merged, and #54 carries
#53's workspace. Merge order is otherwise free.

What is left after them has no coupling at all — the behaviour lab, per-bot
SeenActiveAt, declined re-review recovery, and the two small items split out
of #55 and #57 rather than widening either.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions