Replace the patchwork core with a round state machine, bot dialects, and a pure decision engine

AGENTS.md

@@ -0,0 +1,98 @@
+# crq architecture
+
+crq coordinates CodeRabbit/Codex PR reviews through one account-wide queue so
+the fleet never double-fires a review or races the shared rate limit. This is a
+map of how the code is laid out and the invariants that keep it correct. For the
+CLI contract read `README.md` and `llms.txt`; for usage read `crq help`.
+
+## Package layout
+
+Dependency rule (Go-enforced, no cycles): `dialect ← engine ← crq`, `state ← crq`,
+`gh ← {state, crq}`. The engine does no I/O by construction.
+
+- `internal/dialect/` — ALL bot-text knowledge, zero deps. CodeRabbit/Codex
+  completion, rate-limit, paused, in-progress, failed and clean-review
+  classifiers; finding parsers; the decline-reply verdict classifiers
+  (`IsReviewFindingWithdrawn`/`IsReviewFindingRetained`) that let crq read a
+  bot's rebuttal to a declined finding; SHA/severity vocabulary; the `Finding`
+  type (frozen JSON tags); the typed `BotEvent`/`Classifier`. The only place a
+  bot's literal wording may appear.
+- `internal/gh/` — GitHub REST/GraphQL transport. Owns the "GitHub REST quota"
+  concept under the name **Throttle** (`ThrottleWait`/`IsThrottled`). The only
+  package (besides dialect) allowed to say "rate limit".
+- `internal/state/` — persisted schema v3: one `Round` per PR, one global
+  `FireSlot`, the CodeRabbit `AccountQuota`, an `Archive` ring. Round transition
+  methods, the CAS store, and dashboard rendering.
+- `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`
+  /`FindingsOnHead`/`Converged`, `Policy`. Every rule is table-tested.
+- `internal/crq/` — orchestration only: `service.go` (Enqueue/Pump/Wait/Cancel),
+  `observe.go`, `auto.go`, `feedback.go` (Loop/Feedback assembly), `config.go`,
+  `calibration`/`preflight`/`init`. Holds `Service` and wires the packages.
+
+Vocabulary: two distinct concepts, never mixed. GitHub REST quota = **Throttle**
+(gh). CodeRabbit account quota = **AccountQuota** / "account blocked" (state,
+engine, crq). "rate limit" as literal text lives only in `gh` and `dialect`.
+
+## State: one Round per PR, never deleted
+
+```text
+queued → reserved → fired → reviewing → completed
+   ↑         │         │         │
+   └─────────┘         ├─────────┴→ awaiting_retry ─→ (fire-eligible once RetryAt passes)
+ (post failed)         └→ completed (review lands while slot held)
+ any phase → abandoned (PR closed, cancelled, or superseded by a new head)
+```
+
+Transitions are methods on `Round`; illegal edges error. A round is **never
+deleted** — only transitioned, or archived when a new head supersedes it. That
+is the invariant that makes the spam bug unrepresentable: "we already requested a
+review at this head" is a fact you'd have to destroy a record to forget, and no
+transition does that. `needsReview` collapses to `r, ok := Rounds[key]; ok &&
+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 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.
+
+## observe → decide → apply
+
+One flow drives both the daemon and the loop:
+
+1. **observe** (`crq/observe.go`) — the single place that asks GitHub "what
+   happened on this PR" and reduces it to an `engine.Observation` (head, open,
+   reviews, classified `BotEvent`s, adoptable commands, reactions). It also
+   carries the raw reviews/comments so `Feedback` parses findings from the same
+   fetch. Built once per decision.
+2. **decide** (`internal/engine`) — pure. `DecideFire` consolidates every fire
+   guard in order (open → head readable → head current → phase eligible → slot
+   free → account quota → min interval → not already reviewed → adopt/post);
+   nothing else may post the review command. `Progress` transitions a
+   fired/reviewing round. `Completion` answers convergence.
+3. **apply** (`crq/service.go`) — the only effects executor: CAS state writes +
+   `PostIssueComment`. `DryRun` short-circuits apply into "report, write nothing".
+
+Daemon `Pump` = Progress on the slot round + DecideFire on the next eligible.
+`crq loop` (Wait + Feedback) = the same DecideFire to fire, then `Completion` +
+findings filters to converge. The wait IS the round: a fired/reviewing round with
+a `WaitDeadline` is the in-flight wait. Loop exit codes are frozen: 0 converged/
+skipped, 10 findings, 2 timeout.
+
+## Adding a new bot-message format
+
+When a bot ships a new phrasing that crq must recognise, change three things and
+nothing else:
+
+1. the matching classifier/parser in `internal/dialect` (`coderabbit.go`,
+   `codex.go`, or `common.go`);
+2. one corpus file under `internal/dialect/testdata/{coderabbit,codex}/` holding
+   the real message;
+3. one row in `TestGoldenClassification` (`golden_test.go`) — the row IS the
+   spec for how that file classifies.
+
+Convergence/fire rules that consume those classifications live in
+`internal/engine` and are table-tested in `engine_test.go`; orchestration stays
+in `internal/crq`. Keep bot wording out of engine/state/crq.

CLAUDE.md

@@ -0,0 +1 @@
+AGENTS.md

README.md

@@ -95,7 +95,7 @@ Everything lives in one small **gate repo** (private is fine):
 
 | Piece | What it is |
 |-------|-----------|
-| 🔒 **State ref** | The typed queue state is JSON stored in a git ref (`CRQ_STATE_REF`, default `crq-state`), updated with optimistic **compare-and-swap** — a new commit is written only if the ref hasn't moved, so concurrent callers across machines never corrupt the queue. No database, service account, or always-on server. |
+| 🔒 **State ref** | The typed queue state is JSON stored in a git ref (`CRQ_STATE_REF`, default `crq-state-v3`), updated with optimistic **compare-and-swap** — a new commit is written only if the ref hasn't moved, so concurrent callers across machines never corrupt the queue. No database, service account, or always-on server. |
 | 📊 **Dashboard issue** | A tracking **issue** renders the live state below a hidden machine-readable block: status, the queue, in-flight review, recently requested review commands, and the current quota — every PR linked. The issue **title** is a one-glance status (`🐰 crq — 2 queued`). |
 | 🐰 **Calibration PR** | A throwaway draft PR where crq asks `@coderabbitai rate limit` to read your real quota *without spending a review*. crq prunes its own probe comments so the PR never hits GitHub's 2500-comment cap. |
 
@@ -154,7 +154,7 @@ export CRQ_REPO=YOURUSER/crq-state
 export CRQ_ISSUE=2
 export CRQ_CAL_PR=1
 export CRQ_SCOPE=YOURUSER
-export CRQ_STATE_REF=crq-state
+export CRQ_STATE_REF=crq-state-v3
 EOF
 ```
 
@@ -402,6 +402,12 @@ feedback. crq keys resolution off GitHub's own thread state, so a finding keeps
 surfaces still-open findings from earlier commits, so nothing is silently dropped between passes. A
 finding without `thread_id` came from a review body or comment GitHub can't expose as a resolvable
 thread; CodeRabbit clears those on its next review.
+
+`source: "review_reply"` is special: when you `crq decline --resolve` a finding and the bot replies
+**contesting** the decline ("I'm retaining the finding: …") rather than conceding ("I'm withdrawing
+this finding"), crq re-surfaces that rebuttal as a finding so the loop won't converge over a rebuttal
+you haven't answered. Fix it, or `crq decline` again with a stronger reason; a bot that then withdraws
+clears it. Ambiguous replies surface too — crq never buries a possible rebuttal on a false concession.
 </details>
 
 ## Configuration
@@ -414,18 +420,20 @@ Set these in `~/.config/crq/env` (sourced automatically) or as environment varia
 | `CRQ_ISSUE` | from `init` | dashboard issue number |
 | `CRQ_CAL_PR` | from `init` | calibration PR number |
 | `CRQ_SCOPE` | owner of `CRQ_REPO` | which owners/orgs share this quota (comma-separated) |
-| `CRQ_STATE_REF` | `crq-state` | git ref that stores the typed CAS state |
+| `CRQ_STATE_REF` | `crq-state-v3` | git ref that stores the typed CAS state |
 | `CRQ_REPOS` | _(all in scope)_ | `autoreview` allowlist — only these `owner/name` repos (comma-separated) |
 | `CRQ_EXCLUDE` | _(none)_ | `autoreview` denylist — never these `owner/name` repos (comma-separated) |
 | `CRQ_AUTOREVIEW_SKIP_AUTHORS` | `dependabot[bot]` | PR authors `autoreview` never enqueues (comma-separated; case and `[bot]` suffix don't matter) — set to empty to auto-review bot PRs too; manual `crq review` is unaffected |
 | `CRQ_AUTOREVIEW_SKIP_MARKER` | `<!-- crq:skip-autoreview -->` | exact PR-body marker that suppresses fleet auto-review; set empty to disable; manual `crq loop` is unaffected |
 | `CRQ_REQUIRED_BOTS` | `coderabbitai[bot]` | bots that must review the head for convergence (crq waits for all of them) |
+| `CRQ_CODEX_CMD` | `@codex review` | Codex trigger crq posts alongside the CodeRabbit command when Codex is in `CRQ_REQUIRED_BOTS` and does not auto-review the PR; empty disables Codex firing |
 | `CRQ_FEEDBACK_BOTS` | required bots + `chatgpt-codex-connector[bot]` | bots whose findings are surfaced — a superset of required bots, so Codex reviews show up without gating convergence on repos where Codex isn't installed |
 | `CRQ_TZ` | `UTC` | dashboard display timezone (IANA name, e.g. `Europe/Oslo`) |
 | `CRQ_MIN_INTERVAL` | `90s` | minimum time between fired reviews |
 | `CRQ_POLL` | `15s` | how often `crq loop` checks its place in line |
 | `CRQ_WAIT_TIMEOUT` | `0` | give up waiting for a slot after this long (`0` = never) |
 | `CRQ_FEEDBACK_WAIT_TIMEOUT` | `20m` | how long `crq loop` waits for feedback after firing |
+| `CRQ_SETTLE` | `90s` | after convergence the loop keeps polling this long before exiting 0, so a trailing review wave (e.g. a Codex auto-review of the pushed head) is caught by crq, not by a human re-checking the PR |
 | `CRQ_CALIBRATE_TTL` | `2m` | how long to trust a quota reading before re-asking CodeRabbit |
 | `CRQ_AUTOREVIEW_POLL` | `1m` | how often the `autoreview` daemon scans for PRs to enqueue |
 | `CRQ_INFLIGHT_TIMEOUT` | `15m` | backstop to release a stuck in-flight review |
@@ -442,6 +450,15 @@ them coming!`. crq recognizes that text as a successful, non-actionable review.
 before it satisfies the Codex gate; otherwise the clean summary is simply ignored rather than emitted
 as a false finding.
 
+**Codex firing and auto-detection:** when Codex is in `CRQ_REQUIRED_BOTS`, crq posts `CRQ_CODEX_CMD`
+in the same fire step as the CodeRabbit command — unless it detects Codex auto-review on the PR (any
+Codex review that no `@codex review` command preceded), in which case it never posts and simply waits
+for Codex's own review. Conversely, when Codex is *not* required but joins a round on its own (an
+actionable comment or review mid-round), the round gates on Codex dynamically: convergence waits for
+its review too. A Codex usage-limit notice releases that dynamic gate so an exhausted Codex can't
+stall rounds it volunteered for; an explicitly required Codex is still bounded only by the normal
+feedback deadline.
+
 **Multiple orgs:** CodeRabbit's quota is per-org, so PRs in different orgs draw from *different*
 buckets. Run a separate gate (its own `CRQ_REPO`) per org rather than mixing them — otherwise you'd
 serialize reviews that don't actually compete.

cmd/crq/main.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/kristofferR/coderabbit-queue/internal/crq"
+	ghapi "github.com/kristofferR/coderabbit-queue/internal/gh"
 )
 
 type stderrLogger struct{}
@@ -62,13 +63,13 @@ func run(ctx context.Context, args []string) int {
 		fatal(err)
 		return 1
 	}
-	gh, err := crq.NewGitHub(ctx)
+	gh, err := ghapi.NewGitHub(ctx)
 	if err != nil {
 		fatal(err)
 		return 1
 	}
 	gh.SetLogger(stderrLogger{})
-	store := crq.NewGitStateStore(cfg, gh)
+	store := crq.NewGitStateStore(cfg, gh, stderrLogger{})
 	service := crq.NewService(cfg, gh, store, stderrLogger{})
 
 	switch args[0] {
@@ -241,7 +242,7 @@ func debug(ctx context.Context, service *crq.Service, store crq.StateStore, cfg
 			fatal(err)
 			return 1
 		}
-		printJSON(state.Blocked)
+		printJSON(state.Account)
 		return 0
 	case "state":
 		state, _, err := store.Load(ctx)