Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ Dependency rule (Go-enforced, no cycles): `dialect ← engine ← crq`, `state
them to a bot is dialect's `ClassifyCheckRun`, never gh's.
- `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. `Round.CoBots` holds per-
methods, durable tombstones for tidied trigger comments, the CAS store, and
dashboard rendering. `Round.CoBots` holds per-
co-reviewer trigger bookkeeping; Codex's entry is **dual-written** to the
legacy `Codex*` round fields because the fleet shares one state ref across
binary versions (`Normalize` folds them back on load). `Round` and `State` also
Expand Down
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,16 @@ reviewed. The two flags mirror CodeRabbit's own toggles: default = *Automatic +
commit it requested a review for, so the same commit is never reviewed twice. One process is the
leader at a time (a lease in the shared state), so running the daemon on several machines is safe.

With `CRQ_TIDY=1`, the daemon also **deletes crq's own spent trigger comments** as each round
progresses — the
`@coderabbitai review` / `@codex review` one-liners it posted, which otherwise bury the conversation a
human came to read. It removes a comment only when crq wrote it (never one it adopted from a person,
never a bot's own comment, and never one edited into something else), only from a round that has moved
on, only after the bot answered it, and only once it is too old to adopt again (older than the head
commit, or than a later force-push). Automatic tidying is opt-in so older binaries sharing the state
ref never mis-pair a delayed reply after a newer daemon deletes its command. You can also run a pass
by hand with `crq tidy <repo> <pr>` (`--dry-run` reports what it would remove).

<details>
<summary>Run it persistently (macOS launchd / Linux systemd)</summary>

Expand Down Expand Up @@ -385,6 +395,7 @@ crq loop <repo> <pr> # blocking one-shot round: fire + wait + emit JSON fin
crq feedback <repo> <pr> # current normalized findings as JSON, WITHOUT triggering a review
crq resolve <thread-id> [<thread-id>...] # resolve addressed review threads
crq decline <thread-id> [...] --reason "<why>" [--resolve] # record why a finding is declined
crq tidy <repo> <pr> # delete crq's own spent review-trigger comments (--dry-run previews)
crq autoreview # ⭐ review ALL open PRs automatically, rate-coordinated
# (--no-incremental = first review only; --once = single pass for cron)
crq status # show the dashboard: queue, in-flight, quota, next slot
Expand Down Expand Up @@ -460,6 +471,7 @@ Set these in `~/.config/crq/env` (sourced automatically) or as environment varia
| `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_TIDY` | `0` | set to `1` to delete crq's own spent review-trigger comments as rounds progress (`crq tidy` by hand is unaffected) |
| `CRQ_REQUIRED_BOTS` | `coderabbitai[bot]` | bots that must review the head for convergence (crq waits for all of them) |
| `CRQ_COBOTS` | `codex,bugbot,macroscope` | co-reviewers crq surfaces and (optionally) triggers; set empty to disable all |
| `CRQ_COBOT_<NAME>_REQUIRED` | `0` | make that co-reviewer gate convergence (folds it into `CRQ_REQUIRED_BOTS`); `<NAME>` ∈ `CODEX`, `BUGBOT`, `MACROSCOPE` |
Expand Down
52 changes: 52 additions & 0 deletions cmd/crq/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,28 @@ func run(ctx context.Context, args []string) int {
}
printJSON(result)
return 0
case "tidy":
dryRun := hasFlag(args[1:], "--dry-run")
Comment thread
kristofferR marked this conversation as resolved.
repo, pr, ok := repoPR(positional(args[1:]))
if !ok {
fatal(errors.New("usage: crq tidy <repo> <pr> [--dry-run]"))
return 1
}
if bad, found := unknownFlag(args[1:], "--dry-run"); found {
fatal(fmt.Errorf("unknown flag %s (usage: crq tidy <repo> <pr> [--dry-run])", bad))
return 1
}
if err := cfg.RequireState(); err != nil {
fatal(err)
return 1
}
result, terr := service.Tidy(ctx, repo, pr, dryRun)
if terr != nil {
fatal(terr)
return 1
}
printJSON(result)
return 0
case "autoreview", "auto":
fs := flag.NewFlagSet("autoreview", flag.ContinueOnError)
fs.SetOutput(os.Stderr)
Expand Down Expand Up @@ -356,6 +378,7 @@ 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 tidy <repo> <pr> [--dry-run] remove crq's own spent review-trigger comments
crq autoreview [--once] [--no-incremental]
keep open PRs reviewed, rate-coordinated
crq preflight [--type all|committed|uncommitted] [--base <branch>]
Expand Down Expand Up @@ -509,6 +532,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 "tidy":
fmt.Print(`crq tidy <repo> <pr> [--dry-run]

Delete the review-trigger comments crq posted that nothing needs any more. A PR
driven through a dozen rounds collects a dozen "@coderabbitai review" comments,
which buries the conversation a human came to read.

A comment is removed only when all three hold:

* crq WROTE it, and it still READS as that one-line command. The candidates
are the comments each round recorded posting, not anything matching the text
and not one the round adopted: a person's "@coderabbitai review" is their
decision to ask and not crq's to erase. crq posts under your own account, so
a recorded comment someone has since edited is their words, and it stays.
* the round that asked has PROGRESSED. A live round keeps its command, because
that is the comment crq adopts instead of posting another one.
* the bot answered it, and it predates the current head. Adoption only ever
considers commands newer than the head commit, so deleting one of those
would make the next pump post a duplicate and buy a second review; a head
crq cannot read leaves the check unevaluable, and everything stays. Only a
request crq's own retry replaced is spent whatever its timestamp says.

It never deletes the bots' own comments. An auto-generated reply can be a
rate-limit or skipped-review notice, which crq reads as evidence and surfaces as
a finding — deleting those would destroy feedback nobody had read yet.

Set CRQ_TIDY=1 to run it automatically as rounds progress under crq autoreview.
--dry-run reports what it would remove.
`)
case "autoreview", "auto":
fmt.Print(`crq autoreview [--once] [--no-incremental]
Expand Down
9 changes: 8 additions & 1 deletion internal/crq/auto.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,14 @@ func (s *Service) AutoReview(ctx context.Context, opts AutoOptions) error {
s.log.Printf("warning: autoreview pass failed: %v", passErr)
}
passFailure = passErr
if _, err := s.Pump(ctx); err != nil {
pumped, err := s.Pump(ctx)
// A round that just progressed is the moment its trigger comments
// stop being needed, so tidying here costs one observation on a PR
// crq was already looking at rather than a sweep of the fleet.
if tidyErr := s.tidyAfterPump(ctx, pumped); err == nil {
err = tidyErr
}
if err != nil {
if _, ok := ghapi.ThrottleWait(err); ok {
if cont, serr := s.sleepThrottle(ctx, opts, "pump", err); serr != nil || !cont {
if opts.Once {
Expand Down
39 changes: 37 additions & 2 deletions internal/crq/codex_replay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ func TestObserveScopesShellFilterToCodeRabbit(t *testing.T) {
f.gh.reviews[key] = []ghapi.Review{crShell, codexReview}
f.gh.mu.Unlock()

obs, err := f.svc.observe(f.ctx, f.svc.cfg, repo, pr, nil, f.clk.now())
obs, err := f.svc.observe(f.ctx, f.svc.cfg, repo, pr, nil, nil, f.clk.now())
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -446,6 +446,41 @@ func TestFireCoOnlyPostFailureParks(t *testing.T) {
}
}

// A co-only round's CommandID is the co-reviewer's trigger comment — one
// comment under two names. Recording it as the primary's own would let
// unrelated CodeRabbit activity pass for the answer that trigger is still
// waiting on, and would put the same id on the cleanup list twice.
func TestFireCoOnlyRecordsItsAnchorAsTheCoReviewersCommand(t *testing.T) {
base := time.Date(2026, 7, 17, 12, 0, 0, 0, time.UTC)
f := newCodexReplayFixture(t, base, func(cfg *Config) {
cfg.RequiredBots = []string{cfg.Bot, codexLogin}
})
repo, pr, head := "o/r", 13, "aaaabbbbccccdddd"
f.openPull(repo, pr, head)
f.setCommitDate(head, base.Add(-time.Hour))
// CodeRabbit already reviewed the head → DecideFire returns FireCoOnly.
f.botReview(repo, pr, 500, head, base.Add(-time.Minute))

f.enqueue(repo, pr)
if res := f.pump(); res.Action != "fired" {
t.Fatalf("expected a co-only fire, got %+v", res)
}
r := f.round(repo, pr)
if r == nil || !r.CoOnly {
t.Fatalf("expected a co-only round, got %+v", r)
}
if len(r.PostedCommands) != 1 {
t.Fatalf("posted = %v, want the one comment crq wrote", r.PostedCommands)
}
posted := r.PostedCommands[0]
if posted.ID != r.CommandID {
t.Errorf("posted id = %d, want the round's anchor %d", posted.ID, r.CommandID)
}
if dialect.NormalizeBotName(posted.Bot) != dialect.NormalizeBotName(codexLogin) {
t.Errorf("posted bot = %q, want the co-reviewer it was addressed to", posted.Bot)
}
}

// TestSelfHealCodexClaimPreventsDoublePost pins claim-before-post: two sweepers
// observing the same round with CodexCommandID==0 must produce exactly one
// `@codex review` — the second claim fails under CAS.
Expand Down Expand Up @@ -495,7 +530,7 @@ func TestSelfHealCodexClaimPreventsDoublePost(t *testing.T) {
}
st, _, _ := f.store.Load(f.ctx)
round := st.Round(repo, pr)
obs, err := f.svc.observe(f.ctx, f.svc.cfg, repo, pr, round, f.clk.now())
obs, err := f.svc.observe(f.ctx, f.svc.cfg, repo, pr, round, nil, f.clk.now())
if err != nil {
t.Fatal(err)
}
Expand Down
32 changes: 19 additions & 13 deletions internal/crq/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,24 @@ type Config struct {
// CompletionMarker identifies the bot's reply to a processed review command
// (CodeRabbit: "Review finished."). Feedback uses it to count a command
// round that produced no review object toward convergence.
CompletionMarker string
Host string
Timezone string
MinInterval time.Duration
InflightTimeout time.Duration
PollInterval time.Duration
WaitTimeout time.Duration
CalibrationTTL time.Duration
RateLimitFallback time.Duration
AutoReviewPoll time.Duration
AutoReviewMaxScan int
LeaderTTL time.Duration
FiredMax int
CompletionMarker string
Host string
Timezone string
MinInterval time.Duration
InflightTimeout time.Duration
PollInterval time.Duration
WaitTimeout time.Duration
CalibrationTTL time.Duration
RateLimitFallback time.Duration
AutoReviewPoll time.Duration
AutoReviewMaxScan int
LeaderTTL time.Duration
FiredMax int
// Tidy removes crq's own spent review-trigger comments as rounds progress.
// It is opt-in while older fleet binaries share the state ref: those
// binaries preserve tombstones as unknown fields but cannot use them when
// pairing a delayed reply. CRQ_TIDY=1 turns it on.
Tidy bool
NoOpen bool
DryRun bool
FeedbackWaitTimeout time.Duration
Expand Down Expand Up @@ -172,6 +177,7 @@ func LoadConfig() (Config, error) {
AutoReviewMaxScan: intEnv(env, "CRQ_AUTOREVIEW_MAX_SCAN", 400),
LeaderTTL: durationEnv(env, "CRQ_LEADER_TTL", 3*time.Minute),
FiredMax: intEnv(env, "CRQ_FIRED_MAX", 500),
Tidy: stringEnv(env, "CRQ_TIDY", "0") == "1",
NoOpen: env["CRQ_NO_OPEN"] != "",
DryRun: env["CRQ_DRY_RUN"] == "1",
FeedbackWaitTimeout: durationEnv(env, "CRQ_FEEDBACK_WAIT_TIMEOUT", 20*time.Minute),
Expand Down
27 changes: 27 additions & 0 deletions internal/crq/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,33 @@ func TestLoadConfigEmptyAutoReviewSkipMarkerDisablesOptOut(t *testing.T) {
}
}

func TestLoadConfigTidyIsOptIn(t *testing.T) {
t.Setenv("CRQ_CONFIG", filepath.Join(t.TempDir(), "missing-env"))
t.Setenv("CRQ_TIDY", "restore-after-test")
os.Unsetenv("CRQ_TIDY")

cfg, err := LoadConfig()
if err != nil {
t.Fatal(err)
}
if cfg.Tidy {
t.Fatal("automatic tidying must stay off until every fleet binary understands tombstones")
}
}

func TestLoadConfigTidyCanBeEnabled(t *testing.T) {
t.Setenv("CRQ_CONFIG", filepath.Join(t.TempDir(), "missing-env"))
t.Setenv("CRQ_TIDY", "1")

cfg, err := LoadConfig()
if err != nil {
t.Fatal(err)
}
if !cfg.Tidy {
t.Fatal("CRQ_TIDY=1 did not enable automatic tidying")
}
}

func TestAuthorSetNormalizesCaseAndBotSuffix(t *testing.T) {
set := authorSet("Dependabot[bot], renovate ,")
if len(set) != 2 || !set["dependabot"] || !set["renovate"] {
Expand Down
2 changes: 1 addition & 1 deletion internal/crq/feedback.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func (s *Service) Feedback(ctx context.Context, repo string, pr int) (FeedbackRe
// findings from the raw reviews/comments and derives convergence from
// engine.Completion over the same snapshot — no second fetch path, and the
// "is head reviewed?" rules live only in the engine.
obs, err := s.observe(ctx, s.cfg, repo, pr, round, now)
obs, err := s.observe(ctx, s.cfg, repo, pr, round, collectPosted(st, repo, pr).commands, now)
if err != nil {
return FeedbackReport{}, err
}
Expand Down
96 changes: 95 additions & 1 deletion internal/crq/feedback_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,93 @@ func TestFeedbackRejectsCompletionReplyFromEarlierRound(t *testing.T) {
}
}

func TestFeedbackPairsRepliesWithTidiedCommandHistory(t *testing.T) {
cfg := firingConfig()
sha := "abcdef1234567890"
head := sha[:9]
firedAt := time.Now().UTC().Add(-10 * time.Minute)
oldCommandAt := firedAt.Add(-5 * time.Minute)
completion := "<!-- This is an auto-generated reply by CodeRabbit -->\n✅ Action performed\n\nReview finished."
mk := func(id int64, login, body string, at time.Time) ghapi.IssueComment {
ic := ghapi.IssueComment{ID: id, Body: body, CreatedAt: at, UpdatedAt: at}
ic.User.Login = login
return ic
}

gh := newFakeGitHub()
pull := ghapi.Pull{State: "open"}
pull.Head.SHA = sha
gh.pulls[fakeKey("o/repo", 3)] = pull
// Tidy removed command 1. Its delayed completion lands after command 2,
// and must still pair with the remembered command 1.
gh.comments[fakeKey("o/repo", 3)] = []ghapi.IssueComment{
mk(2, "kristofferR", cfg.ReviewCommand, firedAt),
mk(3, cfg.Bot, completion, firedAt.Add(30*time.Second)),
}
prior := ghapi.Review{
ID: 9,
CommitID: "0123456fedcba",
State: "COMMENTED",
SubmittedAt: oldCommandAt.Add(-time.Hour),
Body: "**Actionable comments posted: 2**",
}
prior.User.Login = cfg.Bot
gh.reviews[fakeKey("o/repo", 3)] = []ghapi.Review{prior}

store := NewMemoryStore(cfg)
if _, err := store.Update(context.Background(), func(st *State) error {
old, err := st.NewRound("o/repo", 3, "0123456fe", oldCommandAt)
if err != nil {
return err
}
if err := old.Reserve("old", "host", oldCommandAt); err != nil {
return err
}
if err := old.Fire(1, oldCommandAt); err != nil {
return err
}
old.RecordPosted(cfg.Bot, 1, oldCommandAt)
st.PutRound(*old)

current, err := st.Supersede("o/repo", 3, head, firedAt)
if err != nil {
return err
}
if err := current.Reserve("current", "host", firedAt); err != nil {
return err
}
if err := current.Fire(2, firedAt); err != nil {
return err
}
current.RecordPosted(cfg.Bot, 2, firedAt)
if err := current.Acknowledge(); err != nil {
return err
}
st.PutRound(*current)
// Tidy persisted the old command outside Archive before removing it.
// Enough unrelated rounds then finish to evict this PR's old round from
// the fleet-wide ring before its delayed completion arrives.
st.RecordTidied("o/repo", 3, PostedCommand{ID: 1, Bot: cfg.Bot, At: oldCommandAt})
for i := 0; i < ArchiveMax; i++ {
st.Archive = append(st.Archive, Round{
Repo: "o/other", PR: 100 + i, Head: "999999999",
Phase: PhaseCompleted, EnqueuedAt: oldCommandAt,
})
}
return nil
}); err != nil {
t.Fatal(err)
}

report, err := NewService(cfg, gh, store, nil).Feedback(context.Background(), "o/repo", 3)
if err != nil {
t.Fatal(err)
}
if report.ReviewedBy[cfg.Bot] || report.Converged {
t.Fatalf("the old command's delayed completion converged the current round: %#v", report)
}
}

func TestFeedbackSkipsReviewAnsweredCommandsWhenPairingCompletionReplies(t *testing.T) {
cfg := Config{
Bot: "coderabbitai[bot]",
Expand Down Expand Up @@ -1022,14 +1109,21 @@ func TestFeedbackUsesNoActionCompletionAfterCodexThumbsUp(t *testing.T) {
pull.State = "open"
pull.Head.SHA = "abcdef1234567890"
gh.pulls[fakeKey("o/repo", 1)] = pull
command := ghapi.IssueComment{
ID: 99,
Body: "@coderabbitai review",
CreatedAt: started,
UpdatedAt: started,
}
command.User.Login = "kristofferR"
comment := ghapi.IssueComment{
ID: 1,
Body: "No actionable comments were generated in the recent review. 🎉",
CreatedAt: started.Add(time.Minute),
UpdatedAt: started.Add(time.Minute),
}
comment.User.Login = "coderabbitai[bot]"
gh.comments[fakeKey("o/repo", 1)] = []ghapi.IssueComment{comment}
gh.comments[fakeKey("o/repo", 1)] = []ghapi.IssueComment{command, comment}
thumb := ghapi.Reaction{Content: "+1", CreatedAt: started.Add(2 * time.Minute)}
thumb.User.Login = "chatgpt-codex-connector[bot]"
gh.reactions[99] = []ghapi.Reaction{thumb}
Expand Down
Loading