Keep the fleet's policy in one place instead of on every machine - #58
Conversation
The reviewers were a property of the process: one CRQ_COBOTS for every repository the fleet touches. A repo that only wants Codex, or one where Bugbot should gate, had no way to say so — and the answer to "which bots do you want here" was to edit a global env file and restart a daemon. crq reviewers <repo> now reports who runs there and what each one costs; crq reviewers set chooses them; crq reviewers clear goes back to the fleet default. Each reviewer reports its budget, which is the only property the queue cares about: account is serialized against the shared CodeRabbit allowance, none runs immediately and waits for nobody. The configuration lives in the shared state ref rather than a .crq.yaml the repository carries. The daemon has no checkout of the repositories it reviews, so an in-repo file would be invisible to it or cost a REST fetch per PR — and a daemon and an agent reading different configurations while writing one state ref is a class of bug worth not having. The override reaches the derived views, not just the stored list: after applying it, Reviewers, RequiredBots and FeedbackBots are rebuilt exactly as LoadConfig builds them, so crq cannot gate on or surface findings from a bot the repository excluded. "Chosen to be none" is kept distinct from "not chosen". The primary stays fleet-wide. Its markers and command are compiled into the dialect classifiers when the Service is built, so a per-repo primary means per-repo classifiers.
Eight review findings, two of them P1. Choices resolved against the fleet's enabled list, so the feature could only ever subtract: with CRQ_COBOTS=codex, asking a project for bugbot was rejected as unknown though it is a registered reviewer. They resolve against the registry now, and a bot the fleet does not enable is built from its registry entry. Naming a bot in --required enables it too, the rule fleet parsing already follows — one that gates but is never triggered waits forever. A primary that is itself a registry bot keeps its silenced entry through an override. That entry carries its wording and check-run hooks, so dropping it costs the PRIMARY its evidence and the round waits out a clean result crq can no longer read. Same defect as the fleet path had, one layer up. A second call naming one half no longer wipes the other: the existing override is loaded and merged, which is what the nil-versus-empty contract promised. An empty --required is refused, because gating on nobody makes Feedback converge before any chosen reviewer runs, and clear takes no mutation flags — ignoring them turned a malformed call into a silent wipe. The Codex-only degrade is gated on Codex being a reviewer here, or an unsolicited auto-review would release the head during an account block on behalf of a bot the project excluded. And sharing a ref only stops an old binary ERASING the field; it does not make it act on one. State now records which hosts write it and what they understand, and `crq reviewers` names any host driving the queue that predates overrides, instead of implying agreement.
Nine review findings, four of them P1, and they converge on one mistake: the override changed who has to answer without changing who gets asked. `--required bugbot` with no `--bots` set the required half only, so nothing enabled Bugbot and it became an unknown reviewer with no command. Required now implies enabled whichever half was named, the rule fleet parsing already follows. A retained co-reviewer that becomes required is promoted out of a never trigger for the same reason — Codex's fleet entry is never while it is optional, so keeping that entry and flipping only the Required bit left the engine waiting for evidence no command was posted for. Only a never trigger is promoted, so a deliberate selfheal survives. Autoreview decided from the primary alone, so a repo requiring Codex whose head CodeRabbit had already reviewed was never enqueued and Codex was never asked. It evaluates the repository's own required set now. And changing the requirements on a head with a completed round stranded the PR outright: convergence reported the new reviewer pending while enqueue kept skipping the head, because a completed round IS the dedup marker. Those rounds are reopened when the required set actually changes. Three more. The two halves are merged inside the CAS closure, or two hosts setting different halves both derive from one snapshot and the later write drops the other's. An explicit CRQ_FEEDBACK_BOTS survives an override, since it is the one list an operator may widen beyond who reviews. And the lagging-host check now matches the leader's "host=x pid=n" form against the host key capabilities are recorded under — otherwise every current-version daemon read as needing an upgrade. The docs no longer call the Codex example Codex-only: an override chooses the co-reviewers, so the primary is still triggered and still spends quota. Leaving it out of --required means the round does not wait for it.
Three of four findings; the fourth is answered rather than fixed. `crq reviewers owner/repo --bots codex` is a set command missing its verb. It showed the configuration and exited 0, which tells automation the mutation worked when nothing changed. It is refused now, naming the verb. `crq reviewers clear owner-repo` cleared a key nothing uses and exited 0, so a typo left the real override in force while the caller believed it had restored the fleet default. Clear validates the shape set already validates. On the fourth: reopening a completed round can re-ask a primary that finished with a completion reply and no Review object, because DecideFire's already-reviewed gate reads submitted reviews. Marking the round co-only does not help — that flag records what a fire DID, it does not gate one — and the alternatives all need an observation the CAS closure cannot make. So the trade is stated in the code: one duplicate review in an uncommon case, against a PR stranded silently and permanently. The loud failure is the better one, and the gate is where a real fix belongs.
Four P1s, and the first retires a trade-off I had argued for instead of fixing. Reopening a completed round could re-ask a primary that answered with a completion reply and no Review object, because the already-reviewed gate read only obs.Reviews. The reviewer's point was that the gate is where the fix belongs, and they were right: it now also counts a completion reply paired to that round's own command. Reopening no longer risks a duplicate review, so the round can be requeued when the required set changes without the cost I had written into a comment. A repository enabling a bot the fleet disabled was built from registry defaults, silently discarding the operator's CRQ_COBOT_<NAME>_CMD, TRIGGER and GRACE — a different bot than the one configured. Every registry co-reviewer is now resolved with the environment applied, enabled or not, and an override picks from that. Capabilities are keyed per PROCESS rather than per host. A new CLI and an old daemon on one machine is the ordinary upgrade, and a per-host key let the CLI's write vouch for the daemon that had not been upgraded — hiding exactly the case the signal exists to catch. And applyFire revalidates the override before acting. Deciding and writing are two steps, so an operator removing a co-reviewer between them would otherwise have crq post that bot's trigger on the authority of a configuration that no longer exists.
Ten findings across both reviewers; two are answered rather than fixed.
Changing a repository's required set requeued every completed round it
had. Rounds are never deleted, so a repository's merged and closed PRs
stay behind as completed dedup markers: on a busy repo that hands Pump
hundreds of dead rounds to observe and drop, one per tick, ahead of every
real one. A stranded PR is by definition an open one, so the requeue asks
GitHub which PRs are open first and touches only those.
Reopening also stamped LastAttemptAt, which is the adoption floor for a
FAILED attempt. Raising it discarded a newly required co-reviewer's own
unanswered trigger as too old to adopt, so crq posted that bot a second
request for the very round the reopen exists to let it answer.
The reopened round's already-reviewed gate read the ADOPTION helper,
which deliberately omits Completion's prior-review requirement and its
failed-summary guard — it answers "was this command spoken for", a
weaker question. A completion reply nothing backs would have written a
completed marker that every later same-head check skips, forever.
Capabilities are keyed per process, but a round recorded its bare
hostname, which no writer entry can match. Every `crq reviewers` call
during a fire therefore named the current process in lagging_hosts and
told the operator to upgrade a binary that already understands
overrides. The round now records the writer id; the dashboard still
prints the machine name.
applyFire re-read the state on EVERY verdict to revalidate the override,
including the FireNo no-op that is most of what a pump does — an extra
four API calls per tick against the git-backed store. Only the verdicts
that post a configuration-chosen command pay for it now.
Setting reviewers with neither half named wrote a fresh UpdatedAt
anyway, and that timestamp IS the override's identity, so a no-op call
discarded every in-flight fire decision for the repository. Reading a
repository now applies the shape check set and clear apply, so
`crq reviewers show` fails instead of reporting the fleet default for a
repository nobody has.
The help conflated budget with requiredness ("none ... waits for
nobody"), required --bots for an update the handler accepts with
--required alone, and offered an empty value for a flag that refuses
one.
Not fixed: feedback.go reading isConfiguredBot from the fleet config
rather than the repo's is identical by construction — the primary is not
overridable, so ForRepo never touches Config.Bot. And SetReviewers'
store.Update is an operator's explicit mutation, not the apply phase of
a review decision: Enqueue and Cancel write outside it too, and DryRun
means "request no reviews", not "silently ignore a configuration
command".
A reviewer change requeues a repository's completed rounds, but three paths let one slip through and leave the PR waiting on a bot crq never asks. A closed PR's round was skipped, and closed is not final: reopened at the same head, its completed round is the dedup marker that hides the requirement added while it was shut. Mark those rounds instead of requeueing them — requeueing dead work would put it ahead of every live round — and let whichever enqueue path next finds the PR alive reopen it. FireDedupe was exempt from the apply-time revalidation because it posts nothing. But its completed round asserts that everyone gating the head has answered, and an override landing after the decision finds the round still queued, so nothing requeues it. Revalidate dedupe too, and report the refusal as the lost race it is rather than as a dedupe. `crq reviewers <repo>` accepted "owner/", "/name" and "owner/name/extra". The read path never contacts GitHub, so a typo printed the fleet default and exited 0, reading as a report about a project crq follows.
Two P1/P2s from Codex, both about a decision outliving the state it was made from. Convergence and slot release were the same question. A repository whose required set omits the primary — required Codex only — converges the moment Codex answers, so Progress completed the round and released the fire slot while the `@coderabbitai review` it had posted was still unacknowledged. The next PR could then spend the account allowance alongside it, which is the one thing the queue exists to prevent, and a late account-block reply landed on a completed round nothing progresses. A round that spent the quota now stays fired until the primary reacts or replies, or until its in-flight timeout gives up on it; at that timeout a converged round completes rather than buying a second metered review no configured reviewer asked for. A co-only round posted no command of its own and still completes at once. The override revalidation ran in applyFire, one read before the effect's own write. SetReviewers committing in that window left the effect applying a decision the new configuration would not have made — for FireDedupe permanently, since dedupe posts nothing, the round is still queued when the override lands (requeuing a queued round is a no-op), and the marker it writes is what stops the newly required reviewer from ever being asked. The check now runs inside each effect's own CAS mutation: the dedupe write, the slot reservation, and the co-reviewer trigger claims. That is the commit point in every case — nothing is posted before it — so the window is gone, and the separate Load with it.
Progress now keeps the slot until the primary acknowledges the command a round fired, but the loop releases that slot too: completeWaitRound ends the wait by completing the round, and the two convergence callers reach it on exactly the case the engine rule was written for — a required set that omits the primary, converged on a co-reviewer alone while the `@coderabbitai review` this round posted is still unanswered. Ask the engine the same question there (PrimaryAckPending, exported for it) and leave such a round fired. Its head is already the dedup marker, so nothing re-fires it, and any Pump finishes it on the acknowledgement or on the in-flight timeout. The timed-out and never-fired waits still end unconditionally: neither is a claim that the round is done.
Four gaps review found in the reviewer override, each one a decision made from a configuration that no longer holds by the time it is written. Completion is the one that cannot be undone: reopenForChangedReviewers leaves an in-flight round alone because it is already going to answer, so a required reviewer added between Progress deciding and applyTransition writing is caught by neither — the completed marker dedupes the head under the set that no longer gates it. Revalidate the override inside that write, as the fire paths already do. The self-heal sweep needs the same: its claim is what authorizes the post, and it was the one trigger path that could still post for a bot `crq reviewers set` had removed. A repository may also name the primary when the fleet configuration carried no entry for it — not enabled, not fleet-required — and skipping it as "not a co-reviewer here" costs the PRIMARY the wording and check-run hooks its own evidence is read through, so a check-only clean result is never fetched. Add the silenced entry the fleet parse would have had. And promoteTrigger can now tell a `never` that fell out of the registry defaults from one CRQ_COBOT_<NAME>_TRIGGER named: the fleet parse lets the operator's value win over the required trigger, and a per-repo requirement must not switch a command back on behind them. Finally, writer identity: host and pid do not name a process over time. A container daemon restarts as pid 1 and pids are reused, so an old binary could inherit a capable predecessor's recorded capabilities and vouch for itself while ignoring every override. Stamp a per-run id, and let the autoreview leader use WriterID rather than spelling the format twice.
Progress and the loop both keep a converged round `fired` when the primary has not acknowledged the command it posted, so the slot is not handed to the next pull request while the account allowance is still in flight. But staying `fired` holds the slot only for as long as the round exists, and converging is exactly what tells the agent to push: `Enqueue` supersedes the round, `Normalize` drops a slot no round holds, and the next `Pump` buys a second metered review while the first is unanswered. The obligation belongs to the command, not to the head it was posted at. The loop now stamps `FireSlot.HoldUntil` in the same write that declines to complete the round, bounded by that command's in-flight window — the deadline crq would have given up at anyway, so an unanswered command cannot wedge the queue for longer than waiting for it would have. The fire gates ask `SlotHeld` rather than "is a round holding it", and `Normalize` keeps a slot the hold still covers.
Per-host configuration files diverge the moment somebody edits one, and nothing says so: each host is behaving correctly according to what it can see. Today one repository was excluded here and still reviewed by the Mac, because "which repositories crq covers" was an environment variable on two machines rather than a decision the fleet had made. This repository already argues the point, on the per-repository reviewer map: a daemon and an agent reading different configurations while writing one shared state ref is a new class of divergence, and both already read that ref. The same reasoning applies one level up, to the settings that say what the fleet does at all. So scope, repos, exclude, required-bots, min-interval, rate-limit-fallback, calibrate-ttl, settle and skip-marker move into the state ref, reachable through crq config. Three kinds of setting deliberately stay local: where the state lives — a host cannot read fleet policy until it knows where to look — credentials, and what a machine can physically do, which is genuinely not the same answer everywhere. Recorded wins over the environment, so adoption is one machine at a time: until something is recorded every host keeps its own, and crq config seed writes this host's answers for whatever the fleet has not decided. Seeding never overwrites a recorded value, or the last machine to run it would win — the divergence this is meant to end. A value this binary cannot read is refused where it is set, and one arriving from a newer crq leaves the host on its own value rather than acting on half a policy; crq doctor reports that, and any variable still set locally that the fleet overrides. Stored as a flat map so an older binary round-trips settings it does not know rather than dropping them.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughFleet policy is persisted in schema v5, applied to queue and reviewer workflows, reconciled against existing rounds, rendered in dashboards, and exposed through configuration and reviewer CLI commands. Drain installation, quota handling, watch passes, and documentation now use fleet-aware settings. ChangesFleet policy rollout
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@codex review |
# Conflicts: # AGENTS.md # README.md # cmd/crq/main.go # internal/crq/auto.go # internal/crq/config.go # internal/crq/feedback.go # internal/crq/reviewers.go # internal/crq/service.go # internal/crq/state.go # internal/state/dashboard.go # internal/state/state.go # llms.txt # skills/coderabbit-queue/SKILL.md
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1b7da34d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e5f21e7ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/crq/main.go`:
- Around line 80-105: Rework the dry-run flow around parseDrainArgs, LoadConfig,
NewGitHub, and GitStateStore.Load so the normal authenticated path proceeds
through the common install logic first without performing a discarded probe.
Only when that path cannot load GitHub fleet state should it construct and print
the host-only DrainPlan with PolicySource "host" and the existing warning; avoid
repeating argument parsing, configuration loading, GitHub creation, or state
reads.
- Around line 1139-1142: Update the fleet settings enumeration in the crq config
help text to include feedback-bots alongside the other fleet-owned settings.
Keep the existing ordering and wording unchanged for all other entries, and use
the registered feedbackBotsKey name.
- Around line 709-712: Remove one of the duplicate three-line `crq reviewers`
command blocks from `usage()` in `cmd/crq/main.go`, leaving a single complete
reviewers/help section and preserving the surrounding usage text.
In `@internal/crq/fleetconfig.go`:
- Around line 62-63: Memoize the static registry returned by fleetSettings using
a read-only cache such as sync.OnceValue, ensuring the map is initialized once
from the dialect registry and subsequently shared safely. Update callers
including FleetKeys, fleetCfg, applyFleet, ValidateFleetSetting, and FleetConfig
to avoid rebuilding or mutating the cached map while preserving existing
behavior.
In `@README.md`:
- Around line 516-519: Update README.md lines 516-519 to describe the primary
reviewer as fleet-wide rather than a per-host setting, while preserving its
association with CRQ_BOT, CRQ_REVIEW_CMD, and the related markers. Make no
changes to llms.txt lines 222-228 or skills/coderabbit-queue/SKILL.md lines
200-206.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 17bcd4a7-0553-4a31-bd3a-66ee06fa83f2
📒 Files selected for processing (40)
AGENTS.mdREADME.mdcmd/crq/main.gointernal/crq/auto.gointernal/crq/cliquota.gointernal/crq/cliquota_test.gointernal/crq/config.gointernal/crq/drain.gointernal/crq/drain_test.gointernal/crq/drainswitch.gointernal/crq/drainswitch_test.gointernal/crq/feedback.gointernal/crq/feedback_test.gointernal/crq/fleetcmd.gointernal/crq/fleetconfig.gointernal/crq/fleetconfig_test.gointernal/crq/init.gointernal/crq/init_test.gointernal/crq/next.gointernal/crq/replay_test.gointernal/crq/repoconfig.gointernal/crq/repoconfig_test.gointernal/crq/reviewers.gointernal/crq/service.gointernal/crq/service_test.gointernal/crq/state.gointernal/crq/watch.gointernal/crq/watch_test.gointernal/state/dashboard.gointernal/state/dashboard_sync_test.gointernal/state/dashboard_test.gointernal/state/fleet.gointernal/state/state.gointernal/state/statusline.gointernal/state/store.gointernal/state/store_version_test.gointernal/state/tolerant.gointernal/state/writers_test.gollms.txtskills/coderabbit-queue/SKILL.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
internal/state/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
internal/state/**/*.go: Persist oneRoundper PR and never delete rounds; use transitions or archival when superseded, preserving reviewed-head deduplication records.
Preserve legacy Codex fields through dual writes and normalization, and round-trip unknown JSON members for forward compatibility; schema v5 remains an intentional compatibility fence.
Files:
internal/state/tolerant.gointernal/state/statusline.gointernal/state/dashboard_test.gointernal/state/dashboard.gointernal/state/fleet.gointernal/state/writers_test.gointernal/state/store.gointernal/state/dashboard_sync_test.gointernal/state/store_version_test.gointernal/state/state.go
internal/crq/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
internal/crq/**/*.go: Keepinternal/crqorchestration-only: observe, decide, and apply effects through the service; it must not contain decision rules or bot-specific wording.
service.gois the only effects executor for CAS state writes andPostIssueComment;DryRunmust report without writing.
Files:
internal/crq/init_test.gointernal/crq/init.gointernal/crq/replay_test.gointernal/crq/next.gointernal/crq/cliquota_test.gointernal/crq/reviewers.gointernal/crq/drainswitch_test.gointernal/crq/config.gointernal/crq/feedback.gointernal/crq/drainswitch.gointernal/crq/repoconfig.gointernal/crq/feedback_test.gointernal/crq/auto.gointernal/crq/fleetconfig.gointernal/crq/fleetcmd.gointernal/crq/cliquota.gointernal/crq/state.gointernal/crq/watch_test.gointernal/crq/fleetconfig_test.gointernal/crq/drain_test.gointernal/crq/repoconfig_test.gointernal/crq/watch.gointernal/crq/service_test.gointernal/crq/service.gointernal/crq/drain.go
internal/crq/config.go
📄 CodeRabbit inference engine (AGENTS.md)
A new co-reviewer that should ship enabled by default must be added to
parseCoBots.
Files:
internal/crq/config.go
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: kristofferR/coderabbit-queue
Timestamp: 2026-07-28T00:12:23.246Z
Learning: Maintain the dependency direction `dialect ← engine ← crq`, `state ← crq`, `gh ← {state, crq}`, and `workspace ← crq`, with no dependency cycles.
Learnt from: CR
Repo: kristofferR/coderabbit-queue
Timestamp: 2026-07-28T00:12:23.246Z
Learning: Use the distinct vocabulary consistently: GitHub REST quota is `Throttle`; CodeRabbit account quota is `AccountQuota` or `account blocked`; literal `rate limit` belongs only in `gh` and `dialect`.
Learnt from: CR
Repo: kristofferR/coderabbit-queue
Timestamp: 2026-07-28T00:12:23.246Z
Learning: The loop exit-code contract is frozen: 0 for converged/skipped/held, 10 for findings, and 2 for timeout; a hold is terminal and must not be reported as timeout.
📚 Learning: 2026-07-27T01:11:18.244Z
Learnt from: kristofferR
Repo: kristofferR/coderabbit-queue PR: 57
File: internal/crq/feedback.go:91-92
Timestamp: 2026-07-27T01:11:18.244Z
Learning: When reviewing code in `internal/crq` that deals with bot configuration, keep the separation of concerns intact: `Config.isConfiguredBot(login)` should only be evaluated against the fleet-wide primary `Config.Bot`. Do not override or reinterpret `Config.Bot` based on repo-specific reviewer/co-reviewer data. If repo-added co-reviewers are involved, they must be handled via repo-derived bot sets (e.g., `Config.evidenceBots()`), and not via `isConfiguredBot`. `Config.ForRepo(...)` should adjust co-reviewer-derived fields like `CoBots`, `RequiredBots`, `Reviewers`, and `FeedbackBots`, but intentionally not change the meaning of `Config.Bot` used by `isConfiguredBot`.
Applied to files:
internal/crq/init_test.gointernal/crq/init.gointernal/crq/replay_test.gointernal/crq/next.gointernal/crq/cliquota_test.gointernal/crq/reviewers.gointernal/crq/drainswitch_test.gointernal/crq/config.gointernal/crq/feedback.gointernal/crq/drainswitch.gointernal/crq/repoconfig.gointernal/crq/feedback_test.gointernal/crq/auto.gointernal/crq/fleetconfig.gointernal/crq/fleetcmd.gointernal/crq/cliquota.gointernal/crq/state.gointernal/crq/watch_test.gointernal/crq/fleetconfig_test.gointernal/crq/drain_test.gointernal/crq/repoconfig_test.gointernal/crq/watch.gointernal/crq/service_test.gointernal/crq/service.gointernal/crq/drain.go
📚 Learning: 2026-07-27T01:11:20.071Z
Learnt from: kristofferR
Repo: kristofferR/coderabbit-queue PR: 57
File: internal/crq/repoconfig.go:163-183
Timestamp: 2026-07-27T01:11:20.071Z
Learning: In internal/crq, treat the “observe → decide → apply” effects restriction as applying only to effects produced by review decisions. Explicit operator mutation commands (e.g., Service.SetReviewers, Service.ClearReviewers, enqueue/cancel flows) are allowed to perform state mutations outside the standard apply path, so review logic should not incorrectly require them to follow observe/decide/apply.
For CRQ_DRY_RUN: it should suppress review requests and fire-record writes, but it must not make explicit configuration/mutation commands (e.g., `crq reviewers set`) silently succeed without persisting the requested changes.
Ensure Service.applyFire revalidates repository reviewer overrides just before posting, to protect against stale/changed overrides since earlier reads.
Applied to files:
internal/crq/init_test.gointernal/crq/init.gointernal/crq/replay_test.gointernal/crq/next.gointernal/crq/cliquota_test.gointernal/crq/reviewers.gointernal/crq/drainswitch_test.gointernal/crq/config.gointernal/crq/feedback.gointernal/crq/drainswitch.gointernal/crq/repoconfig.gointernal/crq/feedback_test.gointernal/crq/auto.gointernal/crq/fleetconfig.gointernal/crq/fleetcmd.gointernal/crq/cliquota.gointernal/crq/state.gointernal/crq/watch_test.gointernal/crq/fleetconfig_test.gointernal/crq/drain_test.gointernal/crq/repoconfig_test.gointernal/crq/watch.gointernal/crq/service_test.gointernal/crq/service.gointernal/crq/drain.go
🪛 LanguageTool
skills/coderabbit-queue/SKILL.md
[style] ~200-~200: To strengthen your wording, consider replacing the phrasal verb “leave out”.
Context: ... override chooses the co-reviewers. Leaving the primary out of --required means the round does not w...
(OMIT_EXCLUDE)
🔇 Additional comments (45)
internal/state/state.go (1)
419-430: LGTM!Also applies to: 484-497, 535-579
internal/state/fleet.go (1)
24-76: LGTM!internal/crq/init.go (1)
41-52: LGTM!internal/crq/init_test.go (1)
14-43: LGTM!internal/crq/fleetconfig.go (1)
550-613: LGTM!internal/crq/fleetcmd.go (2)
43-82: LGTM!Also applies to: 87-158, 168-240
457-482: 🩺 Stability & AvailabilityNo change needed.
EndRoundonly archives the matching round, deletes itsst.Roundsentry, and never inserts a new key during iteration.internal/crq/fleetconfig_test.go (1)
18-1181: LGTM!internal/crq/reviewers.go (1)
324-378: LGTM!Also applies to: 389-392
cmd/crq/main.go (2)
539-600: LGTM!
1907-1922: 🩺 Stability & AvailabilityNo change needed.
GitStateStore.logfnil-checks before callings.log.Printf, so passing a nil logger viaNewGitStateStore(..., nil)is safe here.internal/crq/repoconfig.go (1)
166-184: LGTM!Also applies to: 208-213
internal/state/store.go (2)
84-85: LGTM!Also applies to: 140-203, 254-256, 383-383
101-133: 🎯 Functional CorrectnessNo change needed for
SyncDashboardrendering config.internal/state/dashboard.go (1)
259-260: LGTM!Also applies to: 414-415
internal/state/statusline.go (1)
18-19: LGTM!internal/state/dashboard_sync_test.go (1)
92-248: LGTM!internal/state/dashboard_test.go (1)
936-948: LGTM!internal/state/store_version_test.go (1)
79-129: LGTM!internal/crq/next.go (1)
177-183: LGTM!Also applies to: 253-264
internal/crq/replay_test.go (1)
238-256: LGTM!internal/crq/drain.go (1)
27-45: LGTM!Also applies to: 55-84, 176-265, 377-536
internal/crq/drain_test.go (1)
15-15: LGTM!Also applies to: 73-148, 254-338
internal/state/tolerant.go (1)
21-22: LGTM!internal/state/writers_test.go (1)
8-13: LGTM!Also applies to: 102-126
internal/crq/state.go (1)
33-34: LGTM!Also applies to: 53-56, 102-113
AGENTS.md (1)
33-33: LGTM!Also applies to: 42-44
internal/crq/config.go (1)
86-96: LGTM!Also applies to: 274-281
README.md (1)
400-402: LGTM!Also applies to: 491-515, 520-528
llms.txt (1)
214-220: LGTM!skills/coderabbit-queue/SKILL.md (1)
185-199: LGTM!internal/crq/drainswitch.go (1)
84-91: LGTM!Also applies to: 104-130
internal/crq/drainswitch_test.go (1)
58-78: LGTM!internal/crq/auto.go (1)
6-6: LGTM!Also applies to: 224-253, 254-277, 317-317
internal/crq/watch.go (2)
180-253: LGTM!Also applies to: 267-269, 809-823
299-337: 🚀 Performance & ScalabilityNo change needed. GitHub REST GET calls are already retried with cached ETags, so unchanged ref/commit/tree/blob URLs are revalidated without consuming quota.
> Likely an incorrect or invalid review comment.internal/crq/service.go (2)
51-86: LGTM!Also applies to: 228-291, 305-352, 593-625, 743-789, 1049-1073, 1529-1612, 1841-1886, 1909-2013, 2052-2118, 2252-2259
857-878: 🗄️ Data Integrity & IntegrationNo change needed.
overrideChangedalready rejects committedOutRetrytransitions whenst.FleetRevisiondiffers by comparing the full effectiveFleetRevision, so fleet timing/rate-limit changes are covered.internal/crq/cliquota.go (1)
65-75: LGTM!Also applies to: 84-84, 94-98, 110-130
internal/crq/feedback.go (1)
705-705: LGTM!Also applies to: 867-875, 910-915
internal/crq/repoconfig_test.go (1)
70-101: LGTM!Also applies to: 697-697, 868-918, 1000-1030, 1036-1069
internal/crq/service_test.go (1)
76-76: LGTM!Also applies to: 268-273, 425-425, 461-461, 630-710, 721-756, 758-872, 2504-2598
internal/crq/watch_test.go (1)
692-695: LGTM!Also applies to: 811-839, 1098-1111, 1127-1275
internal/crq/cliquota_test.go (1)
6-6: LGTM!Also applies to: 102-148
internal/crq/feedback_test.go (1)
103-141: LGTM!Also applies to: 2404-2442
An authenticated `crq drain install --dry-run` probed the state ref only to decide it should fall through, then paid for the same read again inside InstallDrain. Move the host-only fallback to where the state is actually read, so the preview costs one read: main now answers only the host that cannot get that far — no credentials, or no CRQ_REPO to look in. Also drop the duplicate `crq reviewers` block from `crq help`, and name feedback-bots among the fleet settings `crq config` enumerates.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca3f36ad17
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Six places still decided from this host's startup configuration while the fleet's was in hand: - an observed account block was discarded whenever ANY fleet setting had moved since the observation. Update reports ErrNoChange as success, so the caller was told the window had been recorded while the account stayed open, and the next fire went out inside it. Only the scope says whose account a notice is about; the window is now recomputed under the current policy, inside the write. - `crq threads` classified authors with this host's reviewer set, so a login only the fleet names came back as a human — disagreeing with `crq feedback` over the same thread. - `Tidy` compared trigger comments against commands this host knows, so a co-reviewer command the fleet configured read as edited by hand and its spent comments stayed on the PR. - `crq init` applied the fleet's scope to its own copy and then printed the host's, installing a fallback that scans another account. - the installed service froze a DERIVED feedback-bots list as an explicit one, and dropped the per-bot settings of every co-reviewer this host has switched off — the fallbacks a repository override or a fleet `cobots` change picks that bot up with. And `scope` now validates GitHub owner logins rather than repository-name characters: `-team` could be recorded fleet-wide, where it fails every autoreview pass on every host.
|
@codex review |
|
bugbot run |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9a2f23932
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Four places drew a conclusion from a policy or a slot and then wrote it somewhere it no longer belonged: - releasing the fire slot went by key alone, so an orphaned hold — the slot kept for a metered command whose round was superseded by the push its success invited — was dropped by whatever retired the QUEUED replacement at that key: a fleet exclusion, a cancel, a closed PR. The hold belongs to the command, so the slot is now released only by the round that took it, matched on its token. - a CLI-reported account block was refused because ANY fleet setting had moved between preflight's read and the write. Only the scope says whose account the block is about, and cliOrgMatches already asks that against the current policy; an edit to settle or repos left the shared quota open and the caller told to run preflight again. - a calibration reply adopted under the TTL the read started with was committed with CheckedAt=now even when the fleet had shortened calibrate-ttl under it, presenting a reading the new window calls stale as fresh for a whole TTL. readQuota now returns when the reply it adopted was written, and the write revalidates that age. - applyFleet applied the recorded settings alphabetically, so `cobots` resolved through ForRepo against THIS HOST's required list: a fleet recording `required-bots=coderabbitai[bot]` with `cobots=""` had the host's locally required co-reviewer put straight back, and the rebuild preserved it. required-bots is now applied first, giving both keys one baseline.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74860c60a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
CRQ_COBOTS defaulted to every co-reviewer in the registry, so a fresh install asked Bugbot and Macroscope on every round of every repository — posting a trigger comment, waiting out a grace period, and getting nothing, because nobody had an account for either. A co-reviewer is opt-in now. Anything named in CRQ_REQUIRED_BOTS is still enabled, since requiring a reviewer crq never asks would hang the round forever. The repository page marks a bot crq has never seen work there, and sorts the proven ones first. It still offers it: a bot cannot produce evidence until it is asked, so hiding the unproven ones would make the first one impossible to turn on. But "enabled" and "works" are different claims and the page now says which it means. Two bugs fell out of this. Narrowing a reviewer set reopened finished rounds. Removing two co-reviewers from a fleet default proposed re-reviewing SEVENTEEN completed rounds — that many metered reviews, against a shared allowance, to reach the answer already on record. Only ADDING a reviewer can invalidate a round: one that converged did so with more evidence than the new configuration asks for. Narrowing is free now, and the impact preview asks the same question so it stops warning about work that will not happen. And crq recorded a co-reviewer's answer only on the reviewing sweep, so a round that never fires — because the account is blocked, or the primary does not run there — was observed on the fire path and nowhere else. A co-reviewer could review it every time and crq would never notice. That is exactly what made a working Codex read as "never answered": it reviewed #58 twice today while the Bots page called it unverified.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/crq/init.go (1)
36-47: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winApply fleet policy before creating the calibration PR.
ensureCalibrationPRruns before the shared state is read andapplyFleetis called. If the host leavesCalibrationPRunset while fleet state supplies one,Initcan create a host-local calibration PR before replacingcfgwith the fleet value. Move the state read/application before this external call and add a regression test for fleet calibration precedence. The fleet-policy contract requires recorded fleet values to override host values.Suggested ordering
+ state, err := store.Update(ctx, func(st *State) error { return nil }) + if err != nil { + return InitResult{}, err + } + cfg = applyFleet(cfg, state.FleetConfig, nil) + if cfg.CalibrationPR <= 0 { pr, err := ensureCalibrationPR(ctx, cfg, gh) ... } - state, err := store.Update(ctx, func(st *State) error { return nil }) - if err != nil { - return InitResult{}, err - } - cfg = applyFleet(cfg, state.FleetConfig, nil)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/crq/init.go` around lines 36 - 47, In Init, read shared state and apply applyFleet before calling ensureCalibrationPR, so fleet-provided CalibrationPR values take precedence over host configuration and prevent unnecessary PR creation. Preserve the existing state-update error handling, and add a regression test covering an unset host CalibrationPR with a recorded fleet CalibrationPR.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/crq/drainswitch.go`:
- Around line 109-129: Update validOwnerLogin to reject GitHub-invalid owner
logins by enforcing the 39-character maximum and rejecting any login containing
consecutive hyphens, while preserving the existing character and
leading/trailing hyphen checks.
In `@internal/crq/service.go`:
- Around line 2113-2116: The calibration probe and pruning logic in readQuota
should use the fleet configuration passed through cfg rather than the host-level
s.cfg.RateLimitCommand. Update the relevant command references in readQuota so
both writes follow cfg’s adopted calibration TTL and policy while preserving the
existing quota calculation behavior.
---
Outside diff comments:
In `@internal/crq/init.go`:
- Around line 36-47: In Init, read shared state and apply applyFleet before
calling ensureCalibrationPR, so fleet-provided CalibrationPR values take
precedence over host configuration and prevent unnecessary PR creation. Preserve
the existing state-update error handling, and add a regression test covering an
unset host CalibrationPR with a recorded fleet CalibrationPR.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ea7b2393-21f6-4a6c-90c9-6d8e18970083
📒 Files selected for processing (17)
cmd/crq/main.gointernal/crq/cliquota_test.gointernal/crq/drain.gointernal/crq/drain_test.gointernal/crq/drainswitch.gointernal/crq/feedback.gointernal/crq/fleetcmd.gointernal/crq/fleetconfig.gointernal/crq/fleetconfig_test.gointernal/crq/init.gointernal/crq/init_test.gointernal/crq/service.gointernal/crq/service_test.gointernal/crq/threads.gointernal/crq/threads_test.gointernal/crq/tidy.gointernal/crq/tidy_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
internal/crq/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
internal/crq/**/*.go: Keepinternal/crqorchestration-only; it wires packages and executes effects, while observation is built once, decisions come frominternal/engine, and state writes plus comment posting occur only in the apply layer.
Keep bot wording out of orchestration code; consume classified events and registry data instead of adding per-bot event kinds or verdicts.
Files:
internal/crq/tidy.gointernal/crq/threads_test.gointernal/crq/init.gointernal/crq/init_test.gointernal/crq/feedback.gointernal/crq/cliquota_test.gointernal/crq/drainswitch.gointernal/crq/threads.gointernal/crq/tidy_test.gointernal/crq/drain_test.gointernal/crq/service_test.gointernal/crq/fleetconfig.gointernal/crq/fleetconfig_test.gointernal/crq/fleetcmd.gointernal/crq/service.gointernal/crq/drain.go
internal/{engine,state,crq}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use
AccountQuotaor “account blocked” for CodeRabbit account quota; reserveThrottlefor GitHub REST quota and do not mix the concepts.
Files:
internal/crq/tidy.gointernal/crq/threads_test.gointernal/crq/init.gointernal/crq/init_test.gointernal/crq/feedback.gointernal/crq/cliquota_test.gointernal/crq/drainswitch.gointernal/crq/threads.gointernal/crq/tidy_test.gointernal/crq/drain_test.gointernal/crq/service_test.gointernal/crq/fleetconfig.gointernal/crq/fleetconfig_test.gointernal/crq/fleetcmd.gointernal/crq/service.gointernal/crq/drain.go
internal/crq/service.go
📄 CodeRabbit inference engine (AGENTS.md)
Make
service.gothe only effects executor for CAS state writes andPostIssueComment;DryRunmust report without writing.
Files:
internal/crq/service.go
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: kristofferR/coderabbit-queue
Timestamp: 2026-07-28T05:52:07.035Z
Learning: The dependency direction must remain acyclic: `dialect ← engine ← crq`, `state ← crq`, `gh ← {state, crq}`, and `workspace ← crq`.
Learnt from: CR
Repo: kristofferR/coderabbit-queue
Timestamp: 2026-07-28T05:52:07.035Z
Learning: The global `FireSlot` permits at most one concurrent primary fire fleet-wide; use CAS coordination and release the slot only according to the defined acknowledgment and hold semantics.
Learnt from: CR
Repo: kristofferR/coderabbit-queue
Timestamp: 2026-07-28T05:52:07.035Z
Learning: `DryRun` must short-circuit application into reporting with no writes, and loop exit codes remain frozen: 0 for converged/skipped/held, 10 for findings, and 2 for timeout.
📚 Learning: 2026-07-27T01:11:18.244Z
Learnt from: kristofferR
Repo: kristofferR/coderabbit-queue PR: 57
File: internal/crq/feedback.go:91-92
Timestamp: 2026-07-27T01:11:18.244Z
Learning: When reviewing code in `internal/crq` that deals with bot configuration, keep the separation of concerns intact: `Config.isConfiguredBot(login)` should only be evaluated against the fleet-wide primary `Config.Bot`. Do not override or reinterpret `Config.Bot` based on repo-specific reviewer/co-reviewer data. If repo-added co-reviewers are involved, they must be handled via repo-derived bot sets (e.g., `Config.evidenceBots()`), and not via `isConfiguredBot`. `Config.ForRepo(...)` should adjust co-reviewer-derived fields like `CoBots`, `RequiredBots`, `Reviewers`, and `FeedbackBots`, but intentionally not change the meaning of `Config.Bot` used by `isConfiguredBot`.
Applied to files:
internal/crq/tidy.gointernal/crq/threads_test.gointernal/crq/init.gointernal/crq/init_test.gointernal/crq/feedback.gointernal/crq/cliquota_test.gointernal/crq/drainswitch.gointernal/crq/threads.gointernal/crq/tidy_test.gointernal/crq/drain_test.gointernal/crq/service_test.gointernal/crq/fleetconfig.gointernal/crq/fleetconfig_test.gointernal/crq/fleetcmd.gointernal/crq/service.gointernal/crq/drain.go
📚 Learning: 2026-07-27T01:11:20.071Z
Learnt from: kristofferR
Repo: kristofferR/coderabbit-queue PR: 57
File: internal/crq/repoconfig.go:163-183
Timestamp: 2026-07-27T01:11:20.071Z
Learning: In internal/crq, treat the “observe → decide → apply” effects restriction as applying only to effects produced by review decisions. Explicit operator mutation commands (e.g., Service.SetReviewers, Service.ClearReviewers, enqueue/cancel flows) are allowed to perform state mutations outside the standard apply path, so review logic should not incorrectly require them to follow observe/decide/apply.
For CRQ_DRY_RUN: it should suppress review requests and fire-record writes, but it must not make explicit configuration/mutation commands (e.g., `crq reviewers set`) silently succeed without persisting the requested changes.
Ensure Service.applyFire revalidates repository reviewer overrides just before posting, to protect against stale/changed overrides since earlier reads.
Applied to files:
internal/crq/tidy.gointernal/crq/threads_test.gointernal/crq/init.gointernal/crq/init_test.gointernal/crq/feedback.gointernal/crq/cliquota_test.gointernal/crq/drainswitch.gointernal/crq/threads.gointernal/crq/tidy_test.gointernal/crq/drain_test.gointernal/crq/service_test.gointernal/crq/fleetconfig.gointernal/crq/fleetconfig_test.gointernal/crq/fleetcmd.gointernal/crq/service.gointernal/crq/drain.go
🔇 Additional comments (20)
internal/crq/init.go (1)
17-22: LGTM!Also applies to: 58-66
internal/crq/init_test.go (1)
1-43: LGTM!Also applies to: 45-77
internal/crq/fleetconfig.go (1)
94-94: LGTM!Also applies to: 196-196, 442-449, 464-466, 538-546, 568-608, 610-632
internal/crq/fleetcmd.go (1)
459-487: LGTM!cmd/crq/main.go (1)
58-73: LGTM!Also applies to: 84-104, 126-126, 334-346, 537-598, 728-729, 879-907, 1029-1029, 1117-1155, 1926-1938
internal/crq/feedback.go (1)
910-915: LGTM!Also applies to: 927-927
internal/crq/threads.go (1)
39-49: LGTM!Also applies to: 65-65
internal/crq/drain.go (1)
25-29: LGTM!Also applies to: 60-87, 400-496
internal/crq/drain_test.go (2)
150-172: LGTM!Also applies to: 338-420
174-180: 🎯 Functional CorrectnessNo duplicate
unreadableStoredeclaration present.The file declares
unreadableStoreexactly once, so no compile-time redeclaration issue remains.internal/crq/fleetconfig_test.go (1)
94-104: LGTM!Also applies to: 251-284, 486-519
internal/crq/service.go (4)
599-643: LGTM!Also applies to: 778-798
888-932: LGTM!Also applies to: 1146-1146, 1338-1338, 1969-2012, 2155-2178
933-957: 🩺 Stability & AvailabilityNo slot-release issue remains.
ProgressassignsKeepWaitingdirectly only while the round is still waiting; the other outcomes handled here are written to the slot-holding round, andOutAbandonalready releases the slot before returning.
1937-1943: 🩺 Stability & AvailabilityNo change needed.
crq cancelis documented as abandoning the current round, and the auto-clear timeout is the stated recovery for an “in flight” PR when cancellation does not apply; no tests imply cancel must unstick an already-gone round.> Likely an incorrect or invalid review comment.internal/crq/threads_test.go (1)
92-129: LGTM!internal/crq/tidy.go (1)
69-75: LGTM!internal/crq/service_test.go (1)
2577-2618: LGTM!Also applies to: 3392-3442
internal/crq/cliquota_test.go (1)
149-178: LGTM!internal/crq/tidy_test.go (1)
396-406: 🎯 Functional CorrectnessVerify the fleet-derived bugbot trigger under the bugbot key.
cursor[bot]andbugbotshare the same registry aliases (bugbot run/cursor review), so this does not currently show a missing fleet lookup. Check that the test uses a command that is not shared withcursor review, and assert the command is materialized inresult.Deletedrather than only checking the absence of the edited message.
validOwnerLogin already rejects the characters and the leading or trailing hyphen no GitHub login may hold, but let through two logins GitHub also cannot issue: one longer than 39 characters, and one with consecutive hyphens. Recorded fleet-wide, either fails every scan on every host at once — the reach that makes catching it at `crq config set` worth more than the per-host check.
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Per-host configuration files diverge the moment somebody edits one, and nothing says so: each host is behaving correctly according to what it can see.
That happened today. A repository was excluded on this host and still reviewed by the Mac, because "which repositories crq covers" was an environment variable on two machines rather than a decision the fleet had made.
The rule this follows
internal/state/state.goalready argues it, on the per-repository reviewer map:This applies the same reasoning one level up, to the settings that say what the fleet does at all.
The split
Fleet — in the state ref, via
crq config:scope,repos,exclude,required-bots,min-interval,rate-limit-fallback,calibrate-ttl,settle,skip-marker.Per host — stays in the environment, three kinds and no more:
CRQ_REPO,CRQ_ISSUE,CRQ_STATE_REF) — a host cannot read fleet policy until it knows where to look;CRQ_DISPATCH_CMD,CRQ_WORKSPACE,CRQ_DISPATCH_CONCURRENCY) — genuinely not the same answer everywhere.Adoption
Recorded wins over the environment, so this goes in one machine at a time: until something is recorded, every host keeps using its own.
crq config seedwrites this host's current answers for whatever the fleet has not decided yet.Seeding never overwrites a recorded value — otherwise the last machine to run it wins, which is the divergence this is meant to end.
Failure modes
crq config set. The ref is shared, so a value only some hosts can parse breaks the fleet from the inside.crq doctorreports both that and any variable still set locally that the fleet overrides — naming the variable, since the remedy is to delete it.Ref #42
Summary by CodeRabbit
crq configwithshow,set,unset, andseedto manage fleet policy in shared state.crq reviewerscommands for selecting bots and required reviewers.CRQ_STATE_REFschema version and fencing notes.