Rewrite crq as a Go CLI#10
Conversation
Replace the bash implementation with a Go CLI (module github.com/kristofferR/coderabbit-queue, stdlib only). - State moves from the dashboard issue body to a compare-and-swap git ref (CRQ_STATE_REF); the issue becomes a rendered mirror with the emoji dashboard, TZ-localized timestamps, and a hidden machine-state block. - Commands: loop, feedback, resolve, autoreview, doctor, status, cancel, init, preflight, plus debug (enqueue/pump/refresh/state) for diagnosis. - feedback emits normalized JSON (inline, review-thread, outside-diff review body, prompt blocks, Codex issue comments) and surfaces unresolved, non-outdated review threads regardless of HEAD, so findings dropped between passes are not lost. - Account-wide FIFO queue with calibration-based rate-limit awareness and a leader lease for the autoreview daemon; gate repo is never reviewed. - Top-level ./crq becomes a `go run` dev launcher; install.sh builds and installs the binary. - Docs (README, llms.txt, coderabbit-queue skill) rewritten for the new CLI. Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR moves crq from a bash implementation to a Go CLI and service stack. It adds typed config, persisted Git-backed state, GitHub API and feedback handling, auto-review and preflight flows, plus updated wrappers, installer logic, docs, and tests. ChangesGo-based crq rewrite
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Every GitHub call funneled through request/requestPaged/GraphQL with a single Do that surfaced a 403 as an opaque APIError, so a rate limit produced no backoff. The autoreview daemon then spun on 403s every poll, perpetuating the shared account-wide REST limit (5,000/hr per user). - Add a rate-limit-aware send(): classify 403/429 as primary (X-RateLimit-Remaining 0 / reset header) or secondary (Retry-After / "secondary rate limit" / abuse text), then back off — honoring the reset header, else exponential — and retry, bounded by CRQ_GITHUB_RETRIES and CRQ_GITHUB_MAX_WAIT (default 6 / 120s). Past the cap it returns a typed RateLimitError carrying the reset time, so humans and agents get an actionable message instead of a raw 403. - Log each backoff through the client logger (visible in the daemon log and CLI stderr). - autoreview daemon now sleeps until the reset on a rate-limited leader acquire instead of returning, which made launchd restart it and re-hammer. - Detect GraphQL RATE_LIMITED errors too. Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
…ap wedges it crq posts "@coderabbitai rate limit" probes to the calibration PR to read the shared quota but never deleted them, so the thread grew ~2 comments per probe (crq's probe + CodeRabbit's reply) until it hit GitHub's hard 2500-comment cap. Past the cap GitHub rejects every new comment, which wedged calibration — and therefore review firing — for all repos, with no built-in remedy. - Prune spent calibration comments (crq's probes + CodeRabbit replies) older than the calibration TTL window after each read, fetching only the oldest page so cost stays bounded (new ListIssueCommentsPage + DeleteIssueComment). - On a comment-cap post error, prune the oldest probes to drop back under the cap and retry the probe once, so a wedged PR self-heals with no manual rotation, config edit, or daemon restart. Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
|
@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: 21
🤖 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 137-146: The resolve command is accepting thread IDs without
preserving the advertised repository/PR scope, so the target context is lost.
Update `parseResolveArgs` and `ResolveThreads` handling in `cmd/crq/main.go` so
`<repo>` and `<pr>` are either validated and passed through to the resolver or
removed from the command contract entirely. Ensure `crq resolve` cannot operate
on raw `--thread` values alone and that each thread is checked against the
intended PR before mutation.
- Around line 562-587: The doctor readiness check is too strict because it
requires gh to be installed and authenticated even when GITHUB_TOKEN or GH_TOKEN
can already authenticate GitHub access. Update the logic around checkGitHubAuth
and report.Ready so env-based authentication counts as authenticated, and adjust
the recommendations to avoid telling users to install gh when a token is
present. Keep the readiness state aligned with how NewGitHub authenticates in
headless environments.
In `@examples/monitor.sh`:
- Around line 5-7: The repo auto-detection in monitor.sh can fail before the
intended usage message is shown because the gh repo view call is evaluated
inside the REPO default expansion. Update the script around the PR/REPO
initialization so the gh repo view result is captured in a way that does not
trigger set -e exit, then keep the explicit : "${REPO:?usage: monitor.sh <PR>
[owner/repo]}" check as the user-facing fallback. Use the existing PR and REPO
setup to locate the change.
In `@examples/review-loop.sh`:
- Around line 9-28: The wrapper script is swallowing non-success signals from
crq loop, so automation cannot tell actionable findings or timeouts apart from
convergence. Update the case handling around crq loop in the review-loop script
so the 10 and 2 paths preserve their original exit statuses instead of falling
through as success, while keeping the existing messages and symbol references to
crq loop, rc, and the case statement intact.
In `@install.sh`:
- Around line 38-48: The install flow in install.sh only falls back when
download fails, but a corrupt archive or missing extracted binary can still
abort because tar and install run under set -e. Update the release-asset path
around the download/tar/install sequence so failures from extracting or
installing the asset are handled and trigger the existing source-build fallback
instead of exiting. Keep the change localized to the CRQ_INSTALL_REF check and
the release asset block, preserving the success path and the final source build
fallback message.
- Around line 55-60: The fallback source extraction in the install script is too
specific because it only recognizes coderabbit-queue-* directories, so
configurable repositories set via CRQ_INSTALL_REPO can fail. Update the archive
directory discovery logic around the src_dir assignment to derive the expected
top-level directory name from CRQ_INSTALL_REPO (or its repository basename)
instead of hardcoding coderabbit-queue-*. Keep the existing tar extraction flow
intact, but make the find pattern match the archive’s actual extracted directory
name for forks and renamed mirrors.
In `@internal/crq/auto.go`:
- Around line 110-117: The allow-repo path in auto.go is reusing the backing
array from s.cfg.Scope via targets := s.cfg.Scope and targets = targets[:0],
which can mutate shared config state. Update the target selection logic in the
scope/AllowRepos branch to build targets from a new slice instead of re-slicing
s.cfg.Scope, while keeping the byRepo behavior unchanged in the same code path.
- Around line 92-95: In auto.go, the branch in the lease update logic that
checks st.Leader.ExpiresAt and compares st.Leader.Token to token is treating a
no-op as success. Change this path to return ErrNoChange instead of nil when
another leader still holds the lease, so StateStore.Update does not persist a
new revision or timestamp. Keep the behavior tied to the existing held flag and
the update flow in the autoreview/state handling code.
- Around line 17-19: The empty !opts.Incremental branch in autoReview has no
effect and should be removed. Keep the existing opts.Incremental handling in
enqueueIfNeedsReview, and delete the no-op conditional block so the flow in
auto.go is simpler and easier to follow.
In `@internal/crq/feedback.go`:
- Line 5: The finding ID generation in feedback.go currently uses SHA-1, which
triggers security scanners and should be replaced. Update the finding ID hashing
logic in the relevant ID generation code to use SHA-256 instead of sha1, keeping
the rest of the identifier construction unchanged so the behavior remains a
drop-in replacement.
- Around line 184-185: The early return in feedback convergence handling is
letting dedupe shortcut a still-pending review state. In the `feedback.go` flow
around the `report.Converged` / `waitCode == 3` check, only return success from
the convergence path when the report is actually fully converged, including all
required entries in `report.ReviewedBy`; otherwise keep waiting and preserve the
non-success status. Update the logic in the `report`/`waitCode` branch so dedupe
cannot bypass the normal convergence check.
In `@internal/crq/github.go`:
- Around line 143-183: GraphQL rate-limit errors are being returned directly
from GitHub.GraphQL, which skips the retry/backoff path used by send. Update
GraphQL so envelope errors with type RATE_LIMITED or a rate-limit message are
surfaced in a way that reuses the same retry-aware handling as HTTP 403/429
responses, rather than immediately returning a RateLimitError; keep the existing
GitHub.GraphQL, send, and RateLimitError symbols as the main touchpoints.
- Around line 698-700: Repo existence checks are swallowing lookup failures by
returning only a bool from GitHub.RepoExists, which makes auth, rate-limit, and
network errors look like a missing repo. Change RepoExists to surface the
request error from g.request instead of collapsing it to false, then update the
Init path that calls RepoExists so it can distinguish a real 404 from other
failures and report the correct remediation. Use the RepoExists and Init symbols
to locate the call chain and preserve the real error context in the gate-repo
check.
In `@internal/crq/init.go`:
- Around line 96-99: The fallback path in CreateRef handling is returning the
wrong error when both operations fail, which hides the real failure. In the init
flow around gh.CreateRef and gh.UpdateRef, make the nested failure return
updateErr instead of the original err so the caller sees the actual failed
operation. Keep the successful fallback behavior unchanged, but ensure the
branch creation path reports the forced update error when UpdateRef is what
failed.
In `@internal/crq/preflight.go`:
- Around line 112-126: The preflight runner in preflight.go can deadlock when
parsePreflightStream exits on malformed JSON before the child process finishes
writing, because cmd.Wait() may block on a still-full stdout pipe. Update the
run flow around parsePreflightStream, cmd.Wait, and the stderr/duration/report
handling so that a parse failure does not stop draining the child output; either
keep consuming stdout until the process exits or explicitly terminate the child
on parseErr before waiting. Make sure the error path still sets report.Status,
report.Error, and report.ExitCode consistently in the preflight execution
function.
- Around line 83-85: Redact passthrough secrets before storing the command in
the preflight report: the `report.Command` assignment in `preflight.go`
currently appends `coderabbitArgs(opts)` verbatim, which can leak values like
`--api-key` when `PreflightReport` is later serialized. Update the
`coderabbitArgs`/`report.Command` path to sanitize sensitive flags before
assignment, preserving the command shape but replacing secret values with a
redacted placeholder for both `--api-key value` and `--api-key=value` forms.
In `@internal/crq/service.go`:
- Around line 155-158: Handle failures from headShort in the queue processing
path instead of converting them into a skipped PumpResult; in the logic around
headShort and isShortSHA, return the error when GitHub read fails so callers can
back off and the queue item is not silently reported as successful. Apply the
same change in the other affected branch noted by the review so both paths in
service.go propagate read errors rather than masking them with Reason: "could
not read head".
- Around line 186-217: The dry-run path in the pump logic is happening too late
in the reservation flow, so the item is removed from Queue and marked InFlight
before returning. Move the s.cfg.DryRun check in the Pump path before the
s.store.Update reservation block in internal/crq/service.go, and only
reserve/update InFlight when not in dry-run mode; use the Pump, s.store.Update,
and InFlight/ReservedAt logic as the anchors when making the change.
In `@internal/crq/state.go`:
- Around line 292-295: Remove the unused stateHash helper from the state.go
code, since it is not referenced anywhere in the provided diff and is causing
static-analysis warnings. Delete the entire stateHash function rather than
keeping or modifying it, and make sure any remaining code paths in State-related
logic do not depend on it.
In `@internal/crq/store.go`:
- Around line 96-110: The returned state from Update is missing the computed
DashboardSHA because compareAndSwap only mutates its local copy before
persisting. Move the DashboardSHA computation into Update before calling
compareAndSwap, or otherwise ensure Update returns the same state shape that is
written to state.json so SyncDashboard publishes the persisted dashboard hash.
Use the Update and compareAndSwap symbols to keep the returned State aligned
with the stored one.
In `@tests/test_cli.sh`:
- Around line 20-28: The `doctor` CLI test is assuming a zero exit code, but
`main` in `crq` intentionally exits non-zero when `doctor().Ready` is false, so
the `jq` shape check never runs on clean hosts. Update the assertion around the
`crq doctor` invocation to tolerate the documented non-ready path while still
validating the JSON output, using the `doctor` command behavior from
`cmd/crq/main.go` and the `doctor()` readiness result as the locator. Ensure the
pipeline captures the output for `jq` even when the command exits 1, so the test
can verify fields like `.status`, `.ready`, and `.tools.gh.found` regardless of
readiness.
🪄 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
Run ID: 0d4082a2-3b6a-4744-bb3f-453a2dd971fd
📒 Files selected for processing (23)
README.mdcmd/crq/main.gocrqexamples/monitor.shexamples/review-loop.shgo.modinstall.shinternal/crq/auto.gointernal/crq/config.gointernal/crq/feedback.gointernal/crq/feedback_test.gointernal/crq/github.gointernal/crq/github_test.gointernal/crq/init.gointernal/crq/preflight.gointernal/crq/preflight_test.gointernal/crq/service.gointernal/crq/service_test.gointernal/crq/state.gointernal/crq/store.gollms.txtskills/coderabbit-queue/SKILL.mdtests/test_cli.sh
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.0)
examples/review-loop.sh
[warning] 8-8: set +e (or set +o errexit) disables the shell's errexit option, so the script keeps running after a command fails. This masks failures of security-critical operations (downloads, signature/checksum verification, permission changes, cleanup of secrets), letting the script proceed with a bad or insecure state. Leave errexit enabled (set -e / set -euo pipefail), or handle failures explicitly with if/|| and an explicit exit instead of globally turning off failure detection.
Context: set +e
Note: [CWE-754] Improper Check for Unusual or Exceptional Conditions.
(set-plus-e-error-masking-bash)
internal/crq/state.go
[warning] 297-297: SHA-1 is a cryptographically broken hash function vulnerable to collision attacks and is unsuitable for security purposes such as signatures, integrity checks, or password hashing. Use a SHA-2 family function (e.g. sha256.New() / sha256.Sum256()) or SHA-3 instead.
Context: sha1.Sum
Note: [CWE-327] Use of a Broken or Risky Cryptographic Algorithm.
(weak-hash-sha1-go)
internal/crq/feedback.go
[warning] 503-503: SHA-1 is a cryptographically broken hash function vulnerable to collision attacks and is unsuitable for security purposes such as signatures, integrity checks, or password hashing. Use a SHA-2 family function (e.g. sha256.New() / sha256.Sum256()) or SHA-3 instead.
Context: sha1.Sum
Note: [CWE-327] Use of a Broken or Risky Cryptographic Algorithm.
(weak-hash-sha1-go)
internal/crq/preflight.go
[warning] 244-244: SHA-1 is a cryptographically broken hash function vulnerable to collision attacks and is unsuitable for security purposes such as signatures, integrity checks, or password hashing. Use a SHA-2 family function (e.g. sha256.New() / sha256.Sum256()) or SHA-3 instead.
Context: sha1.Sum
Note: [CWE-327] Use of a Broken or Risky Cryptographic Algorithm.
(weak-hash-sha1-go)
🪛 golangci-lint (2.12.2)
internal/crq/github_test.go
[error] 19-19: response body must be closed
(bodyclose)
[error] 22-22: response body must be closed
(bodyclose)
[error] 27-27: response body must be closed
(bodyclose)
internal/crq/config.go
[error] 130-130: Error return value of f.Close is not checked
(errcheck)
[medium] 126-126: G304: Potential file inclusion via variable
(gosec)
internal/crq/state.go
[error] 293-293: Error return value of encoding/json.Marshal is not checked: unsafe type time.Time found
(errchkjson)
[medium] 4-4: G505: Blocklisted import crypto/sha1: weak cryptographic primitive
(gosec)
[medium] 298-298: G401: Use of weak cryptographic primitive
(gosec)
[error] 292-292: func stateHash is unused
(unused)
internal/crq/feedback.go
[medium] 5-5: G505: Blocklisted import crypto/sha1: weak cryptographic primitive
(gosec)
[medium] 504-504: G401: Use of weak cryptographic primitive
(gosec)
[error] 323-323: S1007: should use raw string (...) with regexp.MustCompile to avoid having to escape twice
(staticcheck)
internal/crq/store.go
[error] 205-205: Error return value of encoding/json.Marshal is not checked: unsafe type time.Time found
(errchkjson)
internal/crq/auto.go
[error] 17-17: SA9003: empty branch
(staticcheck)
internal/crq/preflight.go
[medium] 7-7: G505: Blocklisted import crypto/sha1: weak cryptographic primitive
(gosec)
[medium] 94-94: G204: Subprocess launched with variable
(gosec)
[medium] 245-245: G401: Use of weak cryptographic primitive
(gosec)
[error] 305-305: param max has same name as predeclared identifier
(predeclared)
internal/crq/github.go
[error] 94-94: Error return value of resp.Body.Close is not checked
(errcheck)
[error] 96-96: Error return value of io.Copy is not checked
(errcheck)
[error] 104-104: Error return value of io.Copy is not checked
(errcheck)
[error] 122-122: Error return value of resp.Body.Close is not checked
(errcheck)
[error] 127-127: Error return value of resp.Body.Close is not checked
(errcheck)
[error] 373-373: func addQuery is unused
(unused)
[error] 327-327: assigned to wait, but reassigned without using the value
(wastedassign)
cmd/crq/main.go
[medium] 664-664: G204: Subprocess launched with variable
(gosec)
internal/crq/service.go
[error] 157-157: error is not nil (line 156) but it returns nil
(nilerr)
[error] 183-183: error is not nil (line 171) but it returns nil
(nilerr)
[error] 424-424: param max has same name as predeclared identifier
(predeclared)
[error] 626-626: QF1001: could apply De Morgan's law
(staticcheck)
[error] 684-684: QF1001: could apply De Morgan's law
(staticcheck)
🪛 LanguageTool
llms.txt
[style] ~39-~39: Consider using a different verb for a more formal wording.
Context: ...e findings. - 10: read .findings[], fix valid issues, validate, commit, push, r...
(FIX_RESOLVE)
skills/coderabbit-queue/SKILL.md
[grammar] ~6-~6: Ensure spelling is correct
Context: ...automatically. --- # coderabbit-queue (crq) CodeRabbit's PR-review limit is account-...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
README.md
[style] ~10-~10: ‘On top of that’ might be wordy. Consider a shorter alternative.
Context: ...es not burn the same shared rate limit. On top of that queue, it provides a PR review-loop dri...
(EN_WORDINESS_PREMIUM_ON_TOP_OF_THAT)
[style] ~71-~71: This phrasing can be overused. Try elevating your writing with a more formal alternative.
Context: ...g lives in one small gate repo, private if you want. There is no database, service account,...
(IF_YOU_WANT)
🪛 OpenGrep (1.23.0)
internal/crq/state.go
[WARNING] 298-298: MD5 and SHA1 are cryptographically broken and should not be used for security purposes. Use SHA-256 or stronger.
(coderabbit.crypto.go-weak-hash)
internal/crq/feedback.go
[WARNING] 504-504: MD5 and SHA1 are cryptographically broken and should not be used for security purposes. Use SHA-256 or stronger.
(coderabbit.crypto.go-weak-hash)
internal/crq/preflight.go
[WARNING] 245-245: MD5 and SHA1 are cryptographically broken and should not be used for security purposes. Use SHA-256 or stronger.
(coderabbit.crypto.go-weak-hash)
🪛 Shellcheck (0.11.0)
tests/test_cli.sh
[info] 12-12: This ! is not on a condition and skips errexit. Use && exit 1 instead, or make sure $? is checked.
(SC2251)
[info] 13-13: This ! is not on a condition and skips errexit. Use && exit 1 instead, or make sure $? is checked.
(SC2251)
[info] 14-14: This ! is not on a condition and skips errexit. Use && exit 1 instead, or make sure $? is checked.
(SC2251)
🔇 Additional comments (9)
go.mod (1)
1-3: LGTM!internal/crq/config.go (1)
46-220: LGTM!internal/crq/github_test.go (1)
10-61: LGTM!crq (1)
2-4: LGTM!examples/monitor.sh (1)
2-3: LGTM!Also applies to: 9-9
examples/review-loop.sh (1)
2-7: LGTM!install.sh (1)
2-37: LGTM!Also applies to: 62-70
internal/crq/service_test.go (1)
12-258: LGTM!internal/crq/feedback_test.go (1)
8-151: LGTM!
- autoreview: drop the dead !Incremental branch; return ErrNoChange when another host holds the leader lease (avoids no-op state writes); stop aliasing cfg.Scope's backing array when building the allow-repo target list. - pump: surface head/review-read errors (incl. rate limits) instead of silently skipping; check DryRun before reserving so a dry run no longer mutates the queue. - loop: don't treat a deduped HEAD as converged until CodeRabbit has actually reviewed it (reviewed_by gate), so a still-pending review isn't reported as done. - preflight: avoid a stdout-pipe deadlock when JSON parsing bails early; redact --api-key/--token values from the reported command. - github: RepoExists returns an error so auth/rate-limit failures aren't reported as "repo missing"; init surfaces it. - init: return the real error when both CreateRef and the UpdateRef fallback fail. - misc: hash finding IDs with SHA-256; drop the unused stateHash; doctor treats GITHUB_TOKEN/GH_TOKEN as GitHub-ready; persist DashboardSHA on the returned state. Declined: resolve repo/PR scope (review-thread IDs are global GraphQL node IDs). Deferred: GraphQL inline rate-limit retry (typed error already handled by the daemon) and the example/installer shell-script nits. Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
…iew round) - install.sh: fall back to the source build when a release archive downloads but is corrupt or missing the binary (previously aborted under set -e), and match the extracted source dir without hardcoding the repo name so CRQ_INSTALL_REPO forks work. - examples/review-loop.sh: exit with crq's own status code so automation can tell findings (10) and timeout (2) apart from convergence (0). Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
…ning findings
Core bug: GitHub's GraphQL API reports a review-thread author's login without the
"[bot]" suffix ("coderabbitai"), but crq matched against "coderabbitai[bot]" (the
REST form), so every CodeRabbit review thread was dropped from feedback. No
thread_id was ever surfaced, so addressed findings could never be resolved on
GitHub via `crq resolve` and piled up unresolved.
- inBots(): match bot authors tolerant of the "[bot]" suffix, applied to the
review, review-thread, review-comment, and issue-comment paths.
- dedupe: normalize the bot name in keys so the resolvable review_thread finding
wins over its prompt-block duplicate.
- GraphQL: retry RATE_LIMITED envelope errors with the same backoff as send
(shared backoffWait/sleepCtx helpers).
- examples/monitor.sh: don't run the gh fallback inside a ${2:-...} default; under
set -e a gh failure aborted before the usage message could print.
- tests/test_cli.sh: capture doctor's JSON so pipefail doesn't fail the test on
doctor's documented non-ready exit.
Tests added for inBots and GraphQL-login thread matching.
Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
Declined findings were left silently unresolved, so reviewers couldn't tell a deliberate decline from an ignored comment. `crq decline <repo> <pr> --thread <id> --reason "<why>"` posts the reason as a reply on the review thread (GraphQL addPullRequestReviewThreadReply) and leaves it unresolved by default; --resolve also closes it as won't-fix. Documented in README, llms.txt, and the skill. Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
Hand-polling `gh api .../pulls/N/reviews|comments` in a loop to wait for a CodeRabbit review or its outcome bypasses crq, drains the shared account-wide GitHub REST quota (also spent by the autoreview daemon and every other agent), and competes with crq's own polling. Point agents at `crq loop` / `crq feedback` / `crq status` instead — documented in both llms.txt and the coderabbit-queue skill. Claude-Session: https://claude.ai/code/session_01EusbXoXtCWegJN33mAmawX
… agent cheat sheet) The rewrite flattened the README. Bring back the original's structure and style — the plain-English problem and what-it-does with emoji bullets, the⚠️ turn-off- auto-review callout, how-it-works diagram + table, persistent-service setup, the ⭐ recommended review loop, the full command list with the feedback JSON shape, the config table, the 🤖 agent cheat sheet, troubleshooting, and a how-it-works note — all updated for the Go CLI: crq loop/feedback/resolve/decline, git-ref CAS state, Go install, and no GitHub API hand-polling. Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
A network blip (the review loop died mid-outage on "context deadline exceeded") or a transient 5xx propagated as a fatal error, killing `crq loop` and the autoreview daemon. send() now retries transient transport errors (timeouts, refused/reset connections, DNS hiccups, TLS handshake failures, short EOFs) and 5xx responses (500/502/503/504) with exponential backoff — clamped to CRQ_GITHUB_MAX_WAIT, bounded by CRQ_GITHUB_RETRIES — while still surfacing real caller cancellation (ctx done) immediately. GraphQL inherits this via send. Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
Per-request retries gave up after ~2 min, so a multi-minute outage (the review loop just died on "context deadline exceeded") surfaced a confusing error mid-run. send() now gives transient network errors a separate, time-based budget: it keeps retrying the request — which doubles as the connectivity probe — on a backoff that plateaus at 30s, for up to CRQ_NETWORK_MAX_WAIT of *continuous* downtime (default 30m), then fails with a clear "github unreachable for <duration>" message. 5xx and rate-limit retries keep their bounded budgets, and real caller cancellation (ctx done) still returns immediately. Tests for the transient-error and plateau logic. Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
- Network retries now have no time cap by default (CRQ_NETWORK_MAX_WAIT=0): crq keeps probing every ~30s until connectivity returns instead of giving up, so a long internet drop never kills a review loop or the autoreview daemon. Set the env var to bound it. Only real caller cancellation (ctx) stops the wait. - Surface progress to stderr so a long wait reads as working, not hung: the GitHub client logs "github unreachable … offline <dur>" on each retry and "reachable again after <dur>" on recovery; crq loop logs queue/rate-limit waits (reason + elapsed) and feedback waits (per-bot reviewed status + elapsed/timeout), throttled to ~30s. README config/troubleshooting/agent notes updated. Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
…kticked headers) When inline comments fail to post, CodeRabbit drops the findings into a "Comments failed to post" review-body section whose line header is un-backticked (561-573:), unlike the backticked `561-573:` form used in "Outside diff range comments". crq only matched the backticked form, so these surfaced only via the lower-fidelity prompt block (severity "unknown", weak title). Make the backticks optional so the rich version (real title + severity) is parsed and wins dedup over the prompt copy. Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
A spurious GitHub 401, or a gh OAuth token that rotated mid-run, killed the review loop with "Bad credentials". send() now retries 401s within the bounded backoff budget, re-resolving the token (GITHUB_TOKEN/GH_TOKEN or `gh auth token`) before each retry so a rotated token is picked up; a genuinely bad token still surfaces after the budget. Token access is mutex-guarded. Adds an httptest covering a 503 + 401 -> 200 retry sequence. Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
…id-pass Ref #2: a large autoreview pass enqueued each PR with its own CAS write + dashboard sync, risking GitHub secondary write limits before the queue was even populated. The pass now collects every PR that needs review and applies them in a single batched CAS write (enqueueBatch) plus one sync. Ref #4: leadership was refreshed only once per pass, so a pass longer than CRQ_LEADER_TTL could let a standby steal the lease mid-scan (brief double leadership). The pass now renews the lease via compare-and-swap partway through and aborts if it has been stolen. The lease already uses CAS, which closed the force-PATCH window the bash version had. Adds tests for batched enqueue (in-batch dedup + distinct seqs) and the lease. Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
…ited A rate-limited fire was requeued with both a Blocked window AND a sticky Warn="rate limited", and the Warn was only cleared on the next successful fire. Once the window passed, the dashboard showed "not currently limited" in the Rate-limit row while the rate-limited warning still lingered below it. Rate-limit state now lives only in Blocked: requeueInflight sets the Blocked window (a short re-calibrate window when no reset is parseable) and no longer sets Warn for rate limits; Warn is reserved for other transient problems (post failure, in-flight timeout). RefreshQuota also clears a stale rate-limited Warn once the account reads as available, which self-heals existing state. Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
Address Codex's review of f9f7512 on PR #10 (and fallout from the prior closed-PR change): - service.go: return a terminal "pr closed" result from Wait when the target PR was dropped from the queue, instead of polling forever (regression from the closed-PR drop); base in-flight completion detection on the trigger comment's GitHub timestamp rather than a local clock that can run ahead and miss a same-second completion; treat a benign queue-head race as a lost race (ErrNoChange -> lost_race) instead of failing the loop with a CAS error; surface reaction-list failures so an acknowledged review isn't refired; dedupe already-reviewed heads with suffix-tolerant bot matching. - service.go: re-check a still-pending calibration probe for a late reply instead of honoring the full-TTL freshness shortcut, so a late "rate-limited" reply can't be ignored while Pump fires into the limit (without re-posting a probe every cycle). - auto.go: propagate rate-limit errors out of needsReview so the autoreview pass backs off instead of scanning the rest of the candidates under the throttle. - state.go: protect recently-fired markers (those still in History) from the FiredMax trim, so the marker written just before Normalize can't be evicted and cause a duplicate review. Declined: requiring the in-flight head to match before returning fired — that needs re-enqueue logic that risks the common Wait path, is rare in loop usage (the loop doesn't push during an in-flight review), and the autoreview daemon self-heals via re-enqueue on its next pass. Ref #10 Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
internal/crq/feedback.go (1)
220-221: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRequire full convergence after dedupe.
This still lets a deduped CodeRabbit review return success when
report.ReviewedBy[s.cfg.Bot]is true but other configuredRequiredBotsremain false. Onlyreport.Convergedshould end the feedback wait successfully.Proposed fix
- if report.Converged || (waitCode == 3 && report.ReviewedBy[s.cfg.Bot]) { + if report.Converged { return report, 0, 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/feedback.go` around lines 220 - 221, The success condition in feedback waiting is too permissive after deduping, because the `waitFeedback` path can return early when `report.ReviewedBy[s.cfg.Bot]` is true even if other required bots have not converged. Update the guard in the `feedback.go` logic around `report.Converged` so that only full convergence (`report.Converged`) ends the wait successfully, and remove the `waitCode == 3 && report.ReviewedBy[...]` shortcut.
🤖 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/auto.go`:
- Around line 241-244: Normalize bot logins inside needsReview so comparisons
against s.cfg.Bot treat suffix-less and [bot]-suffixed values as equivalent.
Update the queued/fired review checks in needsReview to use the same bot-login
normalization behavior as botReviewedHead, so autoreview does not re-enqueue PRs
already reviewed by coderabbitai[bot] when the configured bot is coderabbitai.
Keep the change scoped to the login comparison logic in needsReview and reuse
the existing bot identity handling rather than introducing a separate comparison
path.
In `@internal/crq/feedback.go`:
- Around line 184-190: The wait result handling in feedback.go is collapsing
every code 2 into a timeout, which loses the closed/merged PR skip case. Update
the logic around Service.Wait / waitToFire and the waitCode == 2 branch to
preserve and inspect the PumpResult so closed PRs return a skipped terminal
FeedbackReport instead of Status: "timeout"; use the existing FeedbackReport,
waitToFire, and Service.Wait flow to distinguish the skip from a real wait
timeout.
In `@internal/crq/github.go`:
- Around line 879-893: The searchOwnerQualifier logic is masking /users/{login}
lookup failures by returning a default user qualifier, which can send org
queries to the wrong scope and hide transient errors. Update
GitHub.searchOwnerQualifier to propagate the request failure instead of
collapsing it to user:, and let the caller path in EachOpenPR/backoff handle the
error so owner-type resolution failures are surfaced rather than cached or
retried as the wrong qualifier.
In `@internal/crq/state.go`:
- Around line 121-149: `normalize` in `state.go` is letting `s.Fired` exceed
`cfg.FiredMax` whenever too many keys are marked recent by `s.History`. Update
the protection logic so only a bounded subset of recent entries is retained
before rebuilding `s.Fired`—prefer the most-recent matching history/current head
entries rather than every key in `recent`. Keep the existing trim of
non-protected `items`, but ensure the final `protected` set itself is capped so
normalization always returns a `Fired` map within `FiredMax`.
---
Duplicate comments:
In `@internal/crq/feedback.go`:
- Around line 220-221: The success condition in feedback waiting is too
permissive after deduping, because the `waitFeedback` path can return early when
`report.ReviewedBy[s.cfg.Bot]` is true even if other required bots have not
converged. Update the guard in the `feedback.go` logic around `report.Converged`
so that only full convergence (`report.Converged`) ends the wait successfully,
and remove the `waitCode == 3 && report.ReviewedBy[...]` shortcut.
🪄 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
Run ID: 77dfb251-9ab1-4dcc-b3f1-11b40cc3817b
📒 Files selected for processing (17)
README.mdinstall.shinternal/crq/auto.gointernal/crq/config.gointernal/crq/feedback.gointernal/crq/feedback_test.gointernal/crq/github.gointernal/crq/github_test.gointernal/crq/init.gointernal/crq/preflight.gointernal/crq/preflight_test.gointernal/crq/service.gointernal/crq/service_test.gointernal/crq/state.gointernal/crq/state_test.gollms.txttests/test_cli.sh
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.0)
tests/test_cli.sh
[warning] 22-22: set +e (or set +o errexit) disables the shell's errexit option, so the script keeps running after a command fails. This masks failures of security-critical operations (downloads, signature/checksum verification, permission changes, cleanup of secrets), letting the script proceed with a bad or insecure state. Leave errexit enabled (set -e / set -euo pipefail), or handle failures explicitly with if/|| and an explicit exit instead of globally turning off failure detection.
Context: set +e
Note: [CWE-754] Improper Check for Unusual or Exceptional Conditions.
(set-plus-e-error-masking-bash)
🪛 golangci-lint (2.12.2)
internal/crq/preflight_test.go
[medium] 55-55: G306: Expect WriteFile permissions to be 0600 or less
(gosec)
internal/crq/feedback.go
[error] 509-509: S1007: should use raw string (...) with regexp.MustCompile to avoid having to escape twice
(staticcheck)
🔇 Additional comments (15)
internal/crq/state_test.go (1)
8-28: LGTM!tests/test_cli.sh (1)
20-27: LGTM!internal/crq/config.go (1)
81-81: LGTM!llms.txt (1)
88-92: LGTM!internal/crq/feedback_test.go (1)
170-195: LGTM!internal/crq/preflight_test.go (1)
46-65: LGTM!internal/crq/preflight.go (1)
121-137: LGTM!install.sh (1)
35-49: LGTM!Also applies to: 64-69
internal/crq/service.go (1)
19-30: LGTM!Also applies to: 195-244, 248-253, 295-302, 435-483, 633-643, 705-732
internal/crq/github.go (1)
47-48: LGTM!Also applies to: 218-227, 486-557, 811-872
internal/crq/feedback.go (1)
77-94: LGTM!Also applies to: 125-154, 733-746
internal/crq/init.go (1)
102-102: LGTM!internal/crq/github_test.go (1)
9-9: LGTM!Also applies to: 151-175, 177-246
internal/crq/service_test.go (1)
106-108: LGTM!Also applies to: 134-137, 249-249, 289-350, 352-357
README.md (1)
293-296: LGTM!Also applies to: 337-337, 376-376
![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: aa6bb1bc39
ℹ️ 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".
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/crq/service.go (1)
318-346: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not carry
recordedacross CAS retry attempts.
StateStore.Updatecan rerun the callback after a CAS conflict. If the first mutation setsrecorded = truebut loses the CAS, a laterErrNoChangecan be reported as success even though posted metadata was not persisted.Proposed fix
key := QueueKey(item.Repo, item.PR) - recorded := false state, err := s.store.Update(ctx, func(st *State) error { if st.InFlight == nil || st.InFlight.Token != token { return ErrNoChange } - recorded = true st.InFlight.Phase = "posted" st.InFlight.FiredAt = &firedAt st.InFlight.FiredCommentID = commentID @@ if err != nil { return State{}, err } - if !recorded { + if state.InFlight == nil || state.InFlight.Token != token || state.InFlight.Phase != "posted" || state.Fired[key] != head { return state, ErrNoChange }🤖 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/service.go` around lines 318 - 346, The posted-metadata update in the CRQ service is leaking the mutable recorded flag across StateStore.Update retry attempts, which can turn a later ErrNoChange into an incorrect success. Move the success detection inside the Update callback in the method that updates st.InFlight and st.History, or derive it from the returned state instead of an outer variable, so each CAS retry starts clean and only a truly persisted write is treated as recorded.internal/crq/auto.go (1)
56-82: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPass the caller context into one-shot cleanup.
internal/crq/auto.go:57,70,82,92-95— makefinishAutoReviewOncetakectxand wrap it withcontext.WithoutCancel(ctx)before the 30s timeout so leader release keeps request values without being cut short by cancellation.🤖 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/auto.go` around lines 56 - 82, The one-shot cleanup path in auto review is losing the caller context and can be cancelled too early. Update the finishAutoReviewOnce flow in auto.go to accept ctx from the caller, then use context.WithoutCancel(ctx) before applying the 30s timeout so leader release retains request-scoped values while ignoring cancellation. Make sure every opts.Once return path that currently calls finishAutoReviewOnce passes the active ctx through this helper.Source: Linters/SAST tools
🤖 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/config.go`:
- Around line 79-81: The config initialization currently hard-codes RequiredBots
to coderabbitai[bot] even when Bot is overridden, which leaves the resolved
settings inconsistent. Update the config setup in the CRQ config loader so
RequiredBots is derived from the already-resolved Bot value when
CRQ_REQUIRED_BOTS is unset, and keep the existing environment override behavior
intact. Use the Bot and RequiredBots fields in the config struct and the
stringEnv/listEnv logic to ensure a custom CRQ_BOT automatically flows into the
default required bots list.
---
Outside diff comments:
In `@internal/crq/auto.go`:
- Around line 56-82: The one-shot cleanup path in auto review is losing the
caller context and can be cancelled too early. Update the finishAutoReviewOnce
flow in auto.go to accept ctx from the caller, then use
context.WithoutCancel(ctx) before applying the 30s timeout so leader release
retains request-scoped values while ignoring cancellation. Make sure every
opts.Once return path that currently calls finishAutoReviewOnce passes the
active ctx through this helper.
In `@internal/crq/service.go`:
- Around line 318-346: The posted-metadata update in the CRQ service is leaking
the mutable recorded flag across StateStore.Update retry attempts, which can
turn a later ErrNoChange into an incorrect success. Move the success detection
inside the Update callback in the method that updates st.InFlight and
st.History, or derive it from the returned state instead of an outer variable,
so each CAS retry starts clean and only a truly persisted write is treated as
recorded.
🪄 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
Run ID: 7066f06c-df24-477b-b23e-b3b3b8f25b18
📒 Files selected for processing (12)
cmd/crq/main.gointernal/crq/auto.gointernal/crq/config.gointernal/crq/config_test.gointernal/crq/feedback.gointernal/crq/feedback_test.gointernal/crq/github.gointernal/crq/github_test.gointernal/crq/service.gointernal/crq/service_test.gointernal/crq/state.gointernal/crq/state_test.go
📜 Review details
🧰 Additional context used
🪛 golangci-lint (2.12.2)
internal/crq/auto.go
[error] 57-57: Function finishAutoReviewOnce should pass the context parameter
(contextcheck)
[error] 70-70: Function finishAutoReviewOnce should pass the context parameter
(contextcheck)
[error] 82-82: Function finishAutoReviewOnce should pass the context parameter
(contextcheck)
internal/crq/service.go
[error] 307-307: Non-inherited new context, use function like context.WithXXX instead
(contextcheck)
🔇 Additional comments (12)
cmd/crq/main.go (1)
293-293: LGTM!Also applies to: 310-310, 332-332
internal/crq/state.go (1)
130-177: LGTM!Also applies to: 236-310, 327-333
internal/crq/state_test.go (1)
30-57: LGTM!internal/crq/config_test.go (1)
8-19: LGTM!internal/crq/github.go (1)
314-350: LGTM!Also applies to: 360-503, 821-906
internal/crq/github_test.go (1)
15-51: LGTM!Also applies to: 177-236, 290-341
internal/crq/service.go (2)
394-395: LGTM!Also applies to: 589-616, 641-672
305-307: 🩺 Stability & AvailabilityNo change needed for the retry context.
context.Background()is intentional here so the follow-up write can finish even if the original request is canceled.> Likely an incorrect or invalid review comment.internal/crq/auto.go (1)
152-169: LGTM!Also applies to: 293-311
internal/crq/feedback.go (1)
38-38: LGTM!Also applies to: 143-144, 181-197, 228-229, 270-289, 870-871
internal/crq/service_test.go (1)
17-43: LGTM!Also applies to: 75-79, 175-191, 255-322, 395-438, 440-484, 504-536
internal/crq/feedback_test.go (1)
4-4: LGTM!Also applies to: 198-227, 358-426
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7b9aabb04
ℹ️ 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".
|
@coderabbitai review |
✅ Action performedReview finished.
|
…autoreview Address Codex's review of d7b9aab on PR #10: - feedback.go: bound top-level issue-comment findings to the current head's commit time so a stale finding from a previous head can't trap crq loop on non-empty findings (lazily fetched, and notices are filtered first); suppress a CodeRabbit "Prompt for AI agents" duplicate when its inline thread is already resolved/outdated, so an addressed finding doesn't reappear thread-less and block convergence; recognize dotless root files (Dockerfile, Makefile, LICENSE) in detail summaries so their findings aren't dropped. - service.go: accept same-second bot completions in inflightStatus — GitHub timestamps are second-granular, so a strict After missed a completion landing in the trigger's second and refired a duplicate review. - auto.go: give each autoreview scope its own scan budget so a large first scope can't starve later owners/orgs in a multi-entry CRQ_SCOPE; surface a real (non-rate-limit) scan/pump failure from `autoreview --once` so cron/CI doesn't report success when nothing was scanned. Ref #10 Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
- autoreview: drop the dead !Incremental branch; return ErrNoChange when another host holds the leader lease (avoids no-op state writes); stop aliasing cfg.Scope's backing array when building the allow-repo target list. - pump: surface head/review-read errors (incl. rate limits) instead of silently skipping; check DryRun before reserving so a dry run no longer mutates the queue. - loop: don't treat a deduped HEAD as converged until CodeRabbit has actually reviewed it (reviewed_by gate), so a still-pending review isn't reported as done. - preflight: avoid a stdout-pipe deadlock when JSON parsing bails early; redact --api-key/--token values from the reported command. - github: RepoExists returns an error so auth/rate-limit failures aren't reported as "repo missing"; init surfaces it. - init: return the real error when both CreateRef and the UpdateRef fallback fail. - misc: hash finding IDs with SHA-256; drop the unused stateHash; doctor treats GITHUB_TOKEN/GH_TOKEN as GitHub-ready; persist DashboardSHA on the returned state. Declined: resolve repo/PR scope (review-thread IDs are global GraphQL node IDs). Deferred: GraphQL inline rate-limit retry (typed error already handled by the daemon) and the example/installer shell-script nits. Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
… codes Address CodeRabbit's review of ed9968a on PR #10: - auto.go: route pass/pump rate-limit errors through the shared leader backoff (and skip Pump when the pass was throttled) instead of immediately re-scanning and hammering the quota; cap each SearchOpenPRs to the remaining scan budget; load the queue snapshot once per pass and reuse it across candidates instead of reloading git-backed state per PR. - github.go: treat an already-expired rate-limit reset hint as hint-less backoff so it consumes the retry budget instead of hot-looping with attempt frozen; fixes both the REST and GraphQL send paths. - preflight.go: report a malformed-JSON parse failure as exit 1, not 2 — our own cancel() of the child no longer masquerades as a timeout. - feedback.go: mark the configured required-bot key (suffix-tolerant) rather than inserting the raw API login, so a coderabbitai vs coderabbitai[bot] spelling mismatch can't wedge convergence. - init.go: wrap both calibration-ref errors with %w. - install.sh: isolate the release-asset and source-build temp dirs. - tests/test_cli.sh: assert crq doctor exits 0 or 1 instead of "|| true". - llms.txt: document path/line/url/thread_id as optional finding fields. - README.md: relabel examples/review-loop.sh as a one-shot wrapper. Ref #10 Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
…safety Address Codex's review of 5b58cba on PR #10. Root cause first: Codex's GitHub login is chatgpt-codex-connector[bot], but the required-bots default was chatgpt-codex, so inBots never matched it — crq surfaced CodeRabbit's "Skipped: comment is from another GitHub bot" replies instead of Codex's actual findings, and never tracked Codex as a reviewer. - config.go: default CRQ_REQUIRED_BOTS to chatgpt-codex-connector[bot] so Codex's review comments and findings are recognized. - github.go/auto.go: stream open PRs (EachOpenPR) and stop at the post-filter scan budget instead of a fixed pre-filter search cap, so excluded/gate-repo results can't crowd out in-scope PRs (restores full autoreview coverage) while still not over-fetching pages; detect org vs user scopes and use the org:/user: search qualifier accordingly (cached per login). - service.go: drop a PR from the queue if it was closed or merged after queueing, instead of firing a review at a dead PR that can never converge. - feedback.go: stop treating commit-blind issue comments as current-head reviews (a stale completion summary no longer marks a bot reviewed); propagate issue-comment list errors and GraphQL rate-limit failures instead of silently reporting clean/converged from incomplete data; return the slot-wait timeout (Wait code 2) immediately instead of entering the feedback poll on stale findings. Ref #10 Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
Address Codex's review of f9f7512 on PR #10 (and fallout from the prior closed-PR change): - service.go: return a terminal "pr closed" result from Wait when the target PR was dropped from the queue, instead of polling forever (regression from the closed-PR drop); base in-flight completion detection on the trigger comment's GitHub timestamp rather than a local clock that can run ahead and miss a same-second completion; treat a benign queue-head race as a lost race (ErrNoChange -> lost_race) instead of failing the loop with a CAS error; surface reaction-list failures so an acknowledged review isn't refired; dedupe already-reviewed heads with suffix-tolerant bot matching. - service.go: re-check a still-pending calibration probe for a late reply instead of honoring the full-TTL freshness shortcut, so a late "rate-limited" reply can't be ignored while Pump fires into the limit (without re-posting a probe every cycle). - auto.go: propagate rate-limit errors out of needsReview so the autoreview pass backs off instead of scanning the rest of the candidates under the throttle. - state.go: protect recently-fired markers (those still in History) from the FiredMax trim, so the marker written just before Normalize can't be evicted and cause a duplicate review. Declined: requiring the in-flight head to match before returning fired — that needs re-enqueue logic that risks the common Wait path, is rare in loop usage (the loop doesn't push during an in-flight review), and the autoreview daemon self-heals via re-enqueue on its next pass. Ref #10 Claude-Session: https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
1 participant
Summary
Rewrites
crqfrom the bash script into a Go CLI (modulegithub.com/kristofferR/coderabbit-queue, standard library only). Same job — one account-wide queue in front of CodeRabbit so parallel PRs/agents don't burn the shared review rate limit — with a typed state model, richer feedback extraction, and a fleet auto-review daemon.What changed
State backend. State moves from the dashboard issue body to a compare-and-swap git ref (
CRQ_STATE_REF, defaultcrq-state). The issue is now a rendered mirror — emoji dashboard, timezone-localized timestamps, and a hidden machine-state block.Commands.
crq loop <repo> <pr>— the review-round primitive: enqueue + rate-coordinated trigger + wait for both required bots on the current head + emit normalized JSON (exit0converged /10findings /2timeout).crq feedback— normalized findings without triggering (inline, review-thread, collapsed outside-diff review body, prompt blocks, Codex issue comments).crq resolve --thread— resolve addressed GitHub review threads.crq autoreview [--once] [--no-incremental]— fleet daemon that keeps every open PR inCRQ_SCOPEreviewed, FIFO, behind a leader lease. Replaces the old hand-rolled monitor loop.crq doctor/crq status/crq cancel/crq init/crq preflight, pluscrq debug <enqueue|pump|refresh|state>for diagnosis.Feedback quality.
feedback/loopsurface unresolved, non-outdated review threads regardless of commit, so a real finding CodeRabbit dropped between passes is not silently lost. Resolution is keyed off GitHub thread state: resolve an addressed thread on GitHub (crq resolve) and it stops being reported.Packaging. Top-level
./crqbecomes ago rundev launcher;install.shbuilds and installs the binary. Docs (README.md,llms.txt, thecoderabbit-queueskill) rewritten for the new CLI.Migration
The new git-ref state schema is JSON-compatible with the old issue-body state (queue /
firedmap / history all carry over), so an existing deployment migrates without losing queued PRs or re-firing reviews already done.Validation
go build ./...,go vet ./..., andgo test ./...all pass (queue dedup, typed state, rate-limit parsing, outside-diff + prompt-block extraction, and cross-commit thread surfacing are covered).https://claude.ai/code/session_01Cd81RkkVcgUw6f3uewriBs
Summary by CodeRabbit
crqCLI with queue/state operations,feedback(structured JSON), and threadresolve/decline.crq loopfor autonomous review rounds with documented exit codes, pluscrq preflightandcrq doctorreadiness checks.install.shand refreshed legacy example scripts to delegate tocrq loop.