Choose which bots review which project

AGENTS.md

@@ -36,6 +36,10 @@ Dependency rule (Go-enforced, no cycles): `dialect ← engine ← crq`, `state 
   added survives being read and rewritten by an older one — which is what makes
   adding one safe without another dual-write, and without a schema bump (an
   unknown version auto-reinitialises and would erase the fleet's rounds).
+  `FireSlot.HoldUntil` is the compatibility exception for the binary immediately
+  before nested slot tolerance: its deadline is mirrored at the tolerant top
+  level and in the legacy pacing anchor so that writer both preserves and
+  honours it during a rolling deployment.
 - `internal/engine/` — PURE decision logic, `now` passed in, no ctx/gh:
   `DecideFire` (the single fire owner), `Progress` (fired/reviewing round
   transitions), `Completion` (the one "is the round done?"), `BlockingFindings`
@@ -72,9 +76,25 @@ r.Head == head → skip`. A completed round stays as the "this head was reviewed
 dedup marker. A rate-limited requeue parks the round in `awaiting_retry` (keeping
 its head/attempts/history), it does not delete a fired marker.
 
+The one exception to that skip is `Round.ReviewersChanged`: a reviewer change
+requeues the repository's completed rounds, but only for PRs that are open —
+marking the closed ones instead of handing Pump dead work. A marked round is
+reopened by whichever enqueue path next sees the PR alive, so reopening a PR
+picks up the requirements it missed while it was shut.
+
 The global `FireSlot` allows ≤1 concurrent fire fleet-wide (CAS). A bot ack
 releases the slot while the review keeps running (the round moves to
-`reviewing`); the round itself stays open until `Completion` is done.
+`reviewing`); the round itself stays open until `Completion` is done. The
+converse does not hold: convergence alone never releases the slot. A repository
+whose required set omits the primary converges as soon as its co-reviewers
+answer, and completing there would hand the slot to the next PR while the
+metered command is still unanswered — so a round that spent the quota stays
+`fired` until the primary acknowledges or its in-flight timeout expires. Staying
+`fired` holds the slot only while the round exists, and converging is what tells
+the agent to push, so the loop also stamps `FireSlot.HoldUntil`: the hold belongs
+to the command, not to the head it was posted at, and it outlives the supersede
+that the success it reported invites. Every fire gate asks `SlotHeld`, not
+"is there a round holding it".
 
 ## observe → decide → apply
 

README.md

@@ -386,6 +386,10 @@ crq feedback <repo> <pr>  # current normalized findings as JSON, WITHOUT trigger
 crq threads <repo> <pr>                                     # every unresolved thread, outdated included
 crq resolve <thread-id> [<thread-id>...]                    # resolve addressed review threads
 crq decline <thread-id> [...] --reason "<why>" [--resolve]  # record why a finding is declined
+crq reviewers <repo>      # which bots review this project, and what each costs
+crq reviewers set <repo> [--bots <a,b>] [--required <a,b>] # choose them (either flag alone)
+crq reviewers clear <repo>                                 # back to the fleet default
+
 crq dismiss <repo> <pr> <finding-id> [...] --reason "<why>"  # account for a finding with no thread
 crq autoreview            # ⭐ review ALL open PRs automatically, rate-coordinated
                           #    (--no-incremental = first review only; --once = single pass for cron)
@@ -514,7 +518,9 @@ the same fire step as the CodeRabbit one. Bugbot and Macroscope default to `self
 already auto-review every push: crq stays silent unless a bot it has seen working misses the current
 head for longer than `CRQ_COBOT_<NAME>_GRACE`. In every mode crq suppresses the trigger when the bot
 auto-reviews, has already reviewed the head, has a check run in flight, or has a live command on the
-PR — so no bot is ever double-asked.
+PR — so no bot is ever double-asked. A mode you set explicitly wins over requiredness, per-repo
+requiredness included: `crq reviewers set` gives a required bot the trigger its registry default
+would have had, but never overrides a `never` you configured yourself.
 
 A co-reviewer that joins a round on its own (an actionable comment, a review, a check run) gates that
 round dynamically: convergence waits for it even though it isn't required. An exhaustion notice — for

cmd/crq/main.go

@@ -211,6 +211,12 @@ func run(ctx context.Context, args []string) int {
 		}
 		printJSON(result)
 		return 0
+	case "reviewers":
+		if err := cfg.RequireState(); err != nil {
+			fatal(err)
+			return 1
+		}
+		return runReviewers(ctx, service, args[1:])
 	case "threads":
 		repo, pr, ok := repoPR(args[1:])
 		if !ok {
@@ -402,6 +408,10 @@ USAGE
   crq decline <thread-id> [...] --reason "<why>" [--keep-open]
                                    reply on a thread to record why a finding is declined
                                    (resolves it; --keep-open leaves it open)
+  crq reviewers <repo>             which bots review this project (and what each costs)
+  crq reviewers set <repo> [--bots <a,b>] [--required <a,b>]
+                                   choose this project's reviewers (either flag alone)
+  crq reviewers clear <repo>       go back to the fleet default
   crq threads <repo> <pr>          list every unresolved review thread, outdated ones included
   crq dismiss <repo> <pr> <finding-id> [...] --reason "<why>"
                                    account for a finding GitHub gives you no thread to close
@@ -558,6 +568,35 @@ replies contesting the decline, crq re-surfaces that reply as its own finding.
 
 Pass --keep-open to leave it unresolved anyway (an on-the-record disagreement you
 intend to keep working). Thread IDs come from .findings[].thread_id.
+`)
+	case "reviewers":
+		fmt.Print(`crq reviewers <repo>
+crq reviewers set <repo> [--bots <login,...>] [--required <login,...>]
+crq reviewers clear <repo>
+
+Which bots review one project, and what each of them costs.
+
+Without a subcommand it reports the reviewers that will actually run there — the
+fleet default, or this repository's own choice if it has one. Each entry carries
+its budget: "account" is serialized against the shared CodeRabbit allowance,
+"none" runs immediately, outside that queue. Budget is not requiredness: whether
+a round WAITS for a reviewer is --required, whatever the reviewer costs.
+
+  set     --bots chooses the co-reviewers; --required chooses which reviewers
+          gate convergence. Either flag alone updates only its own half. An
+          empty --bots means none here, which is a different answer from not
+          setting it at all; an empty --required is refused, because a round
+          that gates on nobody converges before any reviewer runs.
+  clear   drops the override so the repository follows the fleet again.
+
+The configuration lives in the shared state ref, not in a file the repository
+carries: the daemon has no checkout of the repos it reviews, and a daemon and an
+agent reading different configurations while writing one state ref is a class of
+bug worth not having.
+
+The primary reviewer is fleet-wide. Its markers and command are compiled into the
+classifiers when crq starts, so a per-repo primary would mean per-repo
+classifiers — a much larger change than choosing who else runs.
 `)
 	case "threads":
 		fmt.Print(`crq threads <repo> <pr>
@@ -778,6 +817,100 @@ func codeRabbitOrg(ctx context.Context, binary string) string {
 	return checkCodeRabbitAuth(ctx, tools).CurrentOrg
 }
 
+// runReviewers handles `crq reviewers [set|clear] <repo> [flags]`.
+func runReviewers(ctx context.Context, service *crq.Service, args []string) int {
+	action, rest := "show", args
+	if len(args) > 0 && (args[0] == "set" || args[0] == "clear") {
+		action, rest = args[0], args[1:]
+	}
+	var repo string
+	var bots, required *string
+	for i := 0; i < len(rest); i++ {
+		arg := rest[i]
+		switch {
+		case arg == "--bots", arg == "--required":
+			if i+1 >= len(rest) {
+				fatal(fmt.Errorf("%s needs a value", arg))
+				return 1
+			}
+			value := rest[i+1]
+			i++
+			if arg == "--bots" {
+				bots = &value
+			} else {
+				required = &value
+			}
+		case strings.HasPrefix(arg, "--bots="):
+			value := strings.TrimPrefix(arg, "--bots=")
+			bots = &value
+		case strings.HasPrefix(arg, "--required="):
+			value := strings.TrimPrefix(arg, "--required=")
+			required = &value
+		case strings.HasPrefix(arg, "-"):
+			fatal(fmt.Errorf("unknown flag %s (usage: crq reviewers set <repo> --bots <a,b>)", arg))
+			return 1
+		case repo == "":
+			repo = arg
+		default:
+			fatal(fmt.Errorf("unexpected argument %q", arg))
+			return 1
+		}
+	}
+	if repo == "" {
+		fatal(errors.New("usage: crq reviewers [set|clear] <repo> [--bots <a,b>] [--required <a,b>]"))
+		return 1
+	}
+
+	var view crq.ReviewerView
+	var err error
+	switch action {
+	case "clear":
+		// Ignoring a mutation flag here would turn a malformed automation call
+		// like `reviewers clear repo --bots codex` into a silent wipe.
+		if bots != nil || required != nil {
+			fatal(errors.New("clear takes no --bots/--required (it drops the whole override)"))
+			return 1
+		}
+		view, err = service.ClearReviewers(ctx, repo)
+	case "set":
+		if bots == nil && required == nil {
+			fatal(errors.New("set needs --bots or --required (crq reviewers clear <repo> drops the override)"))
+			return 1
+		}
+		view, err = service.SetReviewers(ctx, repo, splitList(bots), splitList(required))
+	default:
+		// `crq reviewers owner/repo --bots codex` is a set command missing its
+		// verb. Showing the configuration and exiting 0 tells automation the
+		// mutation worked when nothing changed.
+		if bots != nil || required != nil {
+			fatal(errors.New("did you mean `crq reviewers set`? --bots/--required only apply to set"))
+			return 1
+		}
+		view, err = service.Reviewers(ctx, repo)
+	}
+	if err != nil {
+		fatal(err)
+		return 1
+	}
+	printJSON(view)
+	return 0
+}
+
+// splitList turns an unset flag into nil and an empty one into an empty
+// non-nil slice: "not chosen" and "chosen to be none" are different answers.
+func splitList(value *string) []string {
+	if value == nil {
+		return nil
+	}
+	out := []string{}
+	for _, part := range strings.Split(*value, ",") {
+		if part = strings.TrimSpace(part); part != "" {
+			out = append(out, part)
+		}
+	}
+	return out
+}
+
 func repoPR(args []string) (string, int, bool) {
 	if len(args) != 2 {
 		return "", 0, false

internal/crq/auto.go

@@ -3,8 +3,6 @@ package crq
 import (
 	"context"
 	"errors"
-	"fmt"
-	"os"
 	"strings"
 	"time"
 
@@ -21,7 +19,10 @@ type AutoOptions struct {
 var errLostLeadership = errors.New("lost autoreview leadership mid-pass")
 
 func (s *Service) AutoReview(ctx context.Context, opts AutoOptions) error {
-	owner := fmt.Sprintf("host=%s pid=%d", s.cfg.Host, os.Getpid())
+	// The same identity capabilities are recorded under, so LaggingWriters can
+	// ask whether the process holding the lease understands what it is deciding
+	// from. Spelling it out here again would let the two drift apart.
+	owner := s.cfg.WriterID()
 	token := randomToken()
 	for {
 		held, err := s.acquireLeader(ctx, owner, token)
@@ -299,23 +300,51 @@ func (s *Service) needsReview(ctx context.Context, state State, repo string, pr
 		return false, "", err
 	}
 	if r := state.Round(repo, pr); r != nil && r.Head == head {
-		return false, head, nil
+		// Unless the round is a marker for reviewers that changed while this PR
+		// was closed: it answered for a set that no longer gates the head, and
+		// enqueueBatch reopens it. Deciding here rather than falling through to
+		// the live checks is also the only correct answer — those read Review
+		// objects, which a co-reviewer answering by comment never submits.
+		if !r.ReviewersChanged {
+			return false, head, nil
+		}
+		s.logEnqueue(repo, pr, head, "reviewer configuration changed while the pr was closed")
+		return true, head, nil
 	}
 	reviews, err := s.gh.ListReviews(ctx, repo, pr)
 	if err != nil {
 		return false, "", err
 	}
-	bot := dialect.NormalizeBotName(s.cfg.Bot)
+	cfg := s.cfgFor(state, repo)
+	bot := dialect.NormalizeBotName(cfg.Bot)
 	lastBotReview := ""
+	// Every reviewer this repository gates on, not just the primary. A repo that
+	// requires Codex or Bugbot and already has a CodeRabbit review at the head
+	// would otherwise never be enqueued, so the reviewer it chose is never asked.
+	reviewedHere := map[string]string{}
 	for _, review := range reviews {
-		if dialect.NormalizeBotName(review.User.Login) == bot && review.CommitID != "" {
+		login := dialect.NormalizeBotName(review.User.Login)
+		if review.CommitID == "" {
+			continue
+		}
+		if login == bot {
 			lastBotReview = dialect.ShortOID(review.CommitID)
 		}
+		reviewedHere[login] = dialect.ShortOID(review.CommitID)
 	}
 	if incremental {
 		need := lastBotReview != head
+		missing := cfg.Bot
+		if !need {
+			for _, login := range cfg.RequiredBots {
+				if reviewedHere[dialect.NormalizeBotName(login)] != head {
+					need, missing = true, login
+					break
+				}
+			}
+		}
 		if need {
-			s.logEnqueue(repo, pr, head, "no bot review at head")
+			s.logEnqueue(repo, pr, head, "no "+dialect.NormalizeBotName(missing)+" review at head")
 		}
 		return need, head, nil
 	}

internal/crq/codex_replay_test.go

@@ -513,9 +513,9 @@ func TestSelfHealCodexClaimPreventsDoublePost(t *testing.T) {
 	}
 	obs.eng.Events = events
 	before := f.codexPosted(repo, pr)
-	f.svc.selfHealCoReviewers(f.ctx, *round, obs.eng, f.clk.now())
+	f.svc.selfHealCoReviewers(f.ctx, f.svc.cfg, *round, obs.eng, f.clk.now())
 	// Second sweeper with the SAME stale observation (still CodexCommandID==0).
-	f.svc.selfHealCoReviewers(f.ctx, *round, obs.eng, f.clk.now())
+	f.svc.selfHealCoReviewers(f.ctx, f.svc.cfg, *round, obs.eng, f.clk.now())
 	if got := f.codexPosted(repo, pr) - before; got != 1 {
 		t.Fatalf("concurrent self-heals must post exactly once, got %d extra posts", got)
 	}