crq — full implementation (review)#1
Conversation
…docs Hardening found during live testing: - _lock_ref_sha: gh dumps 404 bodies to stdout and skips --jq on errors, so a free lock looked 'held' and acquire never created it — validate the SHA now. - lock age via an epoch embedded in the lock commit message (one fetch, our format) instead of a fragile committer.date lookup; exponential backoff with jitter so a thundering herd can't trip GitHub secondary rate limits and starve everyone. - rate-limit signal: calibration (@coderabbitai rate limit) is authoritative; a stale WARNING never overrides a fresh 'available now'. WARNING scan is fallback only. - pump: lockless idle fast-path; single post-gc in-flight check; gc closed-PR scan gated by CRQ_GC_TTL and only drops on explicit 'closed' (not on API error bodies). - CRQ_DRY_RUN for safe testing. Docs: friendlier README (clear problem statement, how it works, recommended PR review loop, troubleshooting, LLM cheat-sheet); examples/review-loop.sh + monitor.sh; Claude skill. Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
crq only controls review timing if CodeRabbit auto-review is OFF; otherwise CodeRabbit reviews every push automatically, bypassing the queue and burning the shared rate limit. Add a required-prerequisite callout to README + SKILL. Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
…review With native CodeRabbit auto-review off (so crq controls the rate), 'crq autoreview' reproduces it, coordinated: enqueues every open PR in scope whose HEAD CodeRabbit hasn't reviewed (new PR = auto-review; new commits = incremental) and fires them FIFO when unblocked. Flags mirror CodeRabbit's toggles: default = Automatic + Incremental review --no-incremental = Automatic review only (first review, no re-review on push) --once = single pass (cron/monitor) The gate repo (calibration PR) is excluded. README/SKILL updated; the caveat now explains you don't lose auto-review — crq re-creates it rate-aware. Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
crq autoreview is an account-wide feature for all PRs, not a PR-loop add-on — give it its own top-level section ahead of the agent-loop section, framed around reviewing every open PR. Drop the migration/'getting it back' framing. Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
…omplement Restore the previous section wording; reword so it's clear every open PR gets auto-reviewed (the queue only orders to respect the rate limit, it's not a cap). Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
…al install + launchd
- Dedup: record the commit each review was requested for ('.fired' map). pump drops a
queued PR without re-posting if a review was already requested for that exact HEAD, and
autoreview skips it too — so autoreview + an agent's 'crq wait' never double-review a commit.
- 'crq version' / --version / -v; help and version work without gh installed.
- Preflight: clear errors if gh or jq is missing, or if gh isn't logged in (gh auth token).
- autoreview never touches the gate repo (calibration PR).
- README: collapsed manual-install section; collapsed launchd/systemd persistent-run setup;
note that autoreview + crq wait never double-post.
Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
…reviewed history - Bump to 1.0.0. - Dashboard issue redesigned: status line, info table, queue and a new 'Recently reviewed' history (last 20), every PR rendered as a GitHub link, timestamps humanized. - State gains a capped 'history' array; pump appends each fired review. Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
Previously, after calibration read 'available now', crq trusted it for CRQ_CALIBRATE_TTL (~120s) even if a fired review then got rate-limited — so it could keep firing into WARNINGs. Now: when an in-flight review's PR shows a fresh rate-limit WARNING, set blocked_until from that WARNING's countdown immediately and back off; a fresh 'available now' calibration no longer overrides a fresh WARNING-derived block. Dashboard: show 'available now' / '✅ not currently limited' when there's capacity (was an empty '?' / '—'), and '⛔ resets <time> (~Nm)' with remaining 0 when rate-limited. Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
…r autoreview By default autoreview covers all open PRs in CRQ_SCOPE; set CRQ_REPOS to restrict to specific owner/name repos, or CRQ_EXCLUDE to skip some. Comma-separated, spaces tolerated. Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
Dashboard times render via strflocaltime under CRQ_TZ (IANA, e.g. Europe/Oslo); defaults to UTC. Internal state stays ISO8601 UTC. Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
… machine state) The human dashboard is now rendered to the gate repo's README.md so the repo's home page IS the live status. To avoid commit spam it's only committed when user-visible content changes (queue / in-flight / rate-limit / history) — gated by a content signature, not every internal tick. The tracking issue keeps the machine-readable state JSON; its title still shows one-glance status. Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
…ial README dashboard crq init now opens https://github.com/<gate> (its README is the live dashboard) in the user's default browser (open/xdg-open/wslview/powershell; CRQ_NO_OPEN=1 to skip), and publishes the initial README dashboard on first creation. Browser-open and the initial-state publish are guarded so re-running init never wipes live state. Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
- review-loop.sh: wait_for_review returns 1 on timeout (was returning 0 because the final sleep succeeded), so callers can tell 'review landed' from 'timed out'. - monitor.sh: fail fast if REPO is empty (not in a git repo / no default) instead of building malformed repos//pulls/... API URLs. Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
|
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:
📝 WalkthroughWalkthrough
Changescrq script functional changes
Docs, examples, and SKILL.md
Sequence Diagram(s)sequenceDiagram
participant Agent as AI Agent / CI
participant crq_autoreview as crq autoreview
participant GitHub as GitHub API
participant GateRepo as Gate Repo (lock + dashboard)
participant CodeRabbit as coderabbitai[bot]
rect rgba(70, 130, 180, 0.5)
Note over crq_autoreview,GitHub: Scan phase
crq_autoreview->>GitHub: list open PRs for CRQ_SCOPE owners
GitHub-->>crq_autoreview: PR list
crq_autoreview->>crq_autoreview: filter CRQ_REPOS/CRQ_EXCLUDE, skip gate repo
crq_autoreview->>GitHub: get HEAD SHA + last coderabbitai[bot] reviewed commit per PR
crq_autoreview->>crq_autoreview: enqueue PRs where HEAD != last reviewed commit
end
rect rgba(60, 179, 113, 0.5)
Note over crq_autoreview,GateRepo: Pump phase
crq_autoreview->>GateRepo: acquire git-ref lock (epoch=now in commit message)
GateRepo-->>crq_autoreview: lock acquired
crq_autoreview->>GitHub: calibrate rate-limit quota
crq_autoreview->>GitHub: post `@coderabbitai` review (validate numeric comment id)
GitHub-->>crq_autoreview: comment id
crq_autoreview->>GateRepo: update dashboard (readme_sig gates README publish), release lock
end
rect rgba(220, 120, 50, 0.5)
Note over GitHub,CodeRabbit: Review + inflight GC
CodeRabbit-->>GitHub: post review OR rate-limit WARNING
crq_autoreview->>GitHub: inflight_status check
alt WARNING detected after fired comment
crq_autoreview->>crq_autoreview: requeue PR, set blocked_until, clear fired marker
else review completed
crq_autoreview->>crq_autoreview: clear in_flight, record history
end
end
Agent->>crq_autoreview: crq wait REPO PR (blocks until fired)
crq_autoreview-->>Agent: exit 0 (review fired) or timeout exit code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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: 6c2bf815f2
ℹ️ 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".
- acquire: nonce fallback (urandom/PID) when uuidgen is absent + never proceed on empty nonce; release refuses an empty nonce (P1). - acquire: steal pre-epoch/foreign locks via the commit's committer.date when epoch= is absent (avoids deadlock-until-manual-unlock) (P2). - pump: require a zero exit AND numeric comment id before marking fired/in_flight, else leave the PR queued (gh prints error bodies to stdout) (P1). - gc: a rate-limited fire is requeued and its fired-marker cleared, so dedup/autoreview don't skip that commit forever (P1). - state_write: propagate issue-write failure; mutating callers no longer report success on a failed persist (P2). - autoreview: --paginate the reviews lookup so the latest review wins (P2); --limit 1000 on PR search so >30 open PRs are covered (P2); run enqueue/pump in subshells so a transient die can't kill the watcher (P2). Ref #1 Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 036a3a0a45
ℹ️ 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: 036a3a0a45
ℹ️ 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".
- cmd_wait: enqueue inside the loop and check its status; don't treat 'absent from queue' as 'fired' when the enqueue write failed. Subshells isolate a transient die (P2). - gc timeout path: requeue the timed-out PR and clear its fired-marker (same fix as the rate-limited path) so the commit isn't skipped forever (P2). - monitor.sh + global loop monitor: --paginate the latest-review lookup so HEAD-vs-reviewed uses the newest review, not page 1 (P2). Ref #1 Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
|
@codex review |
- inflight_status / calib reply / warning scan: '[...]|last|{...}' emitted {u:null,b:null}
on an EMPTY match, so an in-flight review with no warning was misread as warning:0 and
immediately requeued (risking duplicate reviews), and empty calib/warning matches parsed
as garbage. Use 'last // empty | {...}' so an empty match emits nothing (P1).
- autoreview + warning scan: --archived=false on gh search prs, so an archived (read-only)
repo's open PR can't be enqueued and then jam the FIFO head (P1).
- state_write: publish the README dashboard BEFORE storing readme_sig; on publish failure
keep the old signature so the next change retries (no permanently-stale dashboard) (P2).
- (crq:444 timeout fired-marker was already fixed in df337c6.)
Ref #1
Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: adfd7deac5
ℹ️ 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".
- cmd_wait: check in-flight/queue completion BEFORE (re)enqueuing, so another runner firing our PR doesn't recreate a duplicate queue entry; only treat 'absent' as fired once we've actually enqueued (enqueued flag) (P2). - cmd_cancel: clear the .fired marker when cancelling, so a stuck review can be retried for the same commit instead of being dedup-dropped (P2). - autoreview: case-insensitive gate-repo exclusion (GitHub owner/repo names fold case), so a differently-cased CRQ_REPO can't let the calibration PR get auto-reviewed (P2). Ref #1 Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47d93f8856
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47d93f8856
ℹ️ 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/coderabbit-queue/SKILL.md (1)
42-42: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winPrefer a non-piped install path in skill guidance.
Line 42 currently promotes remote script execution as the direct fallback. For agent-facing security posture, make the reviewed/manual install path primary and keep
curl | bashas an explicit opt-in shortcut.🤖 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 `@skills/coderabbit-queue/SKILL.md` at line 42, The installation instructions in the SKILL.md file currently promote the piped curl command (curl -fsSL https://raw.githubusercontent.com/kristofferR/coderabbit-queue/main/install.sh | bash) as the primary installation method, which raises security concerns. Restructure the installation guidance so that the manual or reviewed installation path is presented first as the recommended approach, and demote the piped curl | bash method to a secondary position as an optional shortcut for users who explicitly opt-in to that approach. This ensures the skill guidance follows security best practices by defaulting to a safer, more transparent installation method.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 `@crq`:
- Around line 756-760: Add validation for the CRQ_SCOPE variable in the
cmd_autoreview function. Similar to how CRQ_REPO and CRQ_ISSUE are validated at
the beginning of the function with the need statements, add a need CRQ_SCOPE
validation in that same section. This ensures that if CRQ_SCOPE is not set, the
function will fail with a clear error message instead of silently executing zero
iterations of the for loop that depends on it.
- Around line 564-567: In the dry-run mode block (the elif branch checking
CRQ_DRY_RUN), the state_write call is executed but its return status is not
checked before returning 0. This is inconsistent with other code paths (lines
500, 587, 635, 646, 657) which properly propagate state_write failures. Modify
the dry-run block to capture the return value of state_write and conditionally
return based on success or failure, ensuring state changes are not silently lost
if the write operation fails.
In `@examples/monitor.sh`:
- Around line 38-41: The current logic uses the local git HEAD from git
rev-parse to make enqueue decisions, but this can be unrelated to the actual PR
being reviewed. Replace the HEAD variable assignment to instead query the remote
PR information using repos/$REPO/pulls/$PR and extract the .head.sha value
(shortened to 9 characters to match the current format). This ensures the
comparison between CRREV and HEAD is against the actual target PR's head commit,
not the local checkout state.
In `@examples/review-loop.sh`:
- Around line 31-33: The wait_for_review function only checks issue comments via
the gh api call on lines 31-32 but does not include pull request reviews, which
are retrieved through a different API endpoint. Additionally, if the API call on
line 42 times out or fails, line 43 continues to execute with potentially stale
data. Modify the function to query both issue comments and pull request reviews
(the reviews endpoint returns a different structure), combine the results to
detect if there is any new feedback from coderabbitai[bot], and ensure that the
timeout/error handling on line 42 prevents line 43 from executing when the API
call fails.
In `@README.md`:
- Around line 64-68: Fix the markdown linting violations in the README.md file
by adding blank lines around fenced code blocks and declaring missing fence
languages. Add a blank line after the closing fence marker of the yaml
configuration example block (after line 68), and declare the fence language as
"text" for the ASCII diagram code block that begins with "agent A" (around line
75) by changing the opening fence from three backticks to three backticks
followed by "text". Apply the same pattern to any other fenced code blocks
mentioned (lines 112-125) by ensuring blank lines surround them and any
undeclared fences have a language specified.
---
Outside diff comments:
In `@skills/coderabbit-queue/SKILL.md`:
- Line 42: The installation instructions in the SKILL.md file currently promote
the piped curl command (curl -fsSL
https://raw.githubusercontent.com/kristofferR/coderabbit-queue/main/install.sh |
bash) as the primary installation method, which raises security concerns.
Restructure the installation guidance so that the manual or reviewed
installation path is presented first as the recommended approach, and demote the
piped curl | bash method to a secondary position as an optional shortcut for
users who explicitly opt-in to that approach. This ensures the skill guidance
follows security best practices by defaulting to a safer, more transparent
installation method.
🪄 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: 821a5e41-ab72-4876-877c-471836097c8a
📒 Files selected for processing (6)
README.mdcrqexamples/agent-loop.shexamples/monitor.shexamples/review-loop.shskills/coderabbit-queue/SKILL.md
💤 Files with no reviewable changes (1)
- examples/agent-loop.sh
📜 Review details
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~390-~390: To make your writing flow more naturally, try moving the adverb ‘never’ closer to the verb ‘lost’.
Context: ...ppens under this lock, so the dashboard never has lost updates, and FIFO order uses a monotoni...
(PERF_TENS_ADV_PLACEMENT)
🪛 markdownlint-cli2 (0.22.1)
README.md
[warning] 64-64: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 75-75: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 125-125: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🪛 SkillSpector (2.1.1)
skills/coderabbit-queue/SKILL.md
[info] 42: [SC2] External Script Fetching: Remote code is downloaded and executed. This bypasses code review and could introduce malicious code.
Remediation: Avoid downloading and executing remote scripts. Use trusted packages from PyPI/npm. If remote fetch is required, verify checksums and use HTTPS.
(Supply Chain (SC2))
[info] 42: [SC2] External Script Fetching: Remote code is downloaded and executed. This bypasses code review and could introduce malicious code.
Remediation: Avoid downloading and executing remote scripts. Use trusted packages from PyPI/npm. If remote fetch is required, verify checksums and use HTTPS.
(Supply Chain (SC2))
🔇 Additional comments (16)
crq (16)
15-16: LGTM!Also applies to: 27-29, 38-46
105-130: LGTM!
140-148: LGTM!
163-163: LGTM!
176-176: LGTM!
187-280: LGTM!
296-296: LGTM!Also applies to: 316-328
382-388: LGTM!
398-460: LGTM!
469-475: LGTM!
500-500: LGTM!Also applies to: 509-509
570-584: LGTM!
633-635: LGTM!Also applies to: 646-646, 657-674
676-740: LGTM!
761-796: LGTM!
806-848: LGTM!
| > ```yaml | ||
| > reviews: | ||
| > auto_review: | ||
| > enabled: false | ||
| > ``` |
There was a problem hiding this comment.
Fix fenced-block markdownlint violations (MD031/MD040).
Line 64 and Line 125 need blank lines around fenced blocks, and Line 75 should declare a fence language (for example text) to satisfy the current lint profile.
Suggested patch
> - **Or per repository:** commit a `.coderabbit.yaml` with:
+>
> ```yaml
> reviews:
> auto_review:
> enabled: false
> ```
+>
-```
+```text
agent A ─┐
agent B ─┼─► crq wait <repo> <pr>
agent C ─┘ │
▼
┌──────────────────────────────┐ asks "any capacity?"
│ global lock (a git ref) │ ───────────────────────► CodeRabbit
│ + FIFO queue (in an issue) │ ◄─────────────────────── "available now" / "in 3h"
└──────────────────────────────┘
│ when unblocked, fire the FIFO head — exactly one
▼
posts "`@coderabbitai` review" on the next PR in linecrq help # verify (needs gh + jq installed)
Also applies to: 75-87, 112-125
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 64-64: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 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 `@README.md` around lines 64 - 68, Fix the markdown linting violations in the
README.md file by adding blank lines around fenced code blocks and declaring
missing fence languages. Add a blank line after the closing fence marker of the
yaml configuration example block (after line 68), and declare the fence language
as "text" for the ASCII diagram code block that begins with "agent A" (around
line 75) by changing the opening fence from three backticks to three backticks
followed by "text". Apply the same pattern to any other fenced code blocks
mentioned (lines 112-125) by ensuring blank lines surround them and any
undeclared fences have a language specified.
Source: Linters/SAST tools
|
@codex review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
examples/monitor.sh (1)
29-33:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize and apply warning filtering consistently across all bot counters.
bot_count()excludes rate-limit warnings only for issue comments (i) and uses a case-sensitive body match. This can still spuriously emitNEW_FEEDBACKfrom warning-only bot activity.Suggested patch
bot_count() { # --slurp + a standalone jq with `add`: combine all pages before counting. Plain --paginate --jq # runs the filter per page, so a new (non-bot) page could shift the string and false-wake the loop. - c=$(gh api "repos/$REPO/pulls/$PR/comments" --paginate --slurp 2>/dev/null | jq "add|map(select(.user.login|test(\"$BOTS\")))|length" 2>/dev/null) - r=$(gh api "repos/$REPO/pulls/$PR/reviews" --paginate --slurp 2>/dev/null | jq "add|map(select(.user.login|test(\"$BOTS\")))|length" 2>/dev/null) + c=$(gh api "repos/$REPO/pulls/$PR/comments" --paginate --slurp 2>/dev/null | jq --arg bots "$BOTS" --arg rl "rate limited by coderabbit.ai" 'add|map(select((.user.login|test($bots)) and (((.body // "")|ascii_downcase|contains($rl)|not))))|length' 2>/dev/null) + r=$(gh api "repos/$REPO/pulls/$PR/reviews" --paginate --slurp 2>/dev/null | jq --arg bots "$BOTS" --arg rl "rate limited by coderabbit.ai" 'add|map(select((.user.login|test($bots)) and (((.body // "")|ascii_downcase|contains($rl)|not))))|length' 2>/dev/null) # conversation comments too, but EXCLUDE rate-limit WARNING posts — they're not review feedback # and would otherwise wake the loop as NEW_FEEDBACK when no actual review arrived. - i=$(gh api "repos/$REPO/issues/$PR/comments" --paginate --slurp 2>/dev/null | jq "add|map(select((.user.login|test(\"$BOTS\")) and (.body|contains(\"rate limited by coderabbit.ai\")|not)))|length" 2>/dev/null) + i=$(gh api "repos/$REPO/issues/$PR/comments" --paginate --slurp 2>/dev/null | jq --arg bots "$BOTS" --arg rl "rate limited by coderabbit.ai" 'add|map(select((.user.login|test($bots)) and (((.body // "")|ascii_downcase|contains($rl)|not))))|length' 2>/dev/null) echo "${c:-0}:${r:-0}:${i:-0}" }🤖 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 `@examples/monitor.sh` around lines 29 - 33, The bot feedback counting in the three variables (c for pull request comments, r for reviews, and i for issue comments) is inconsistent. Currently, only the i variable filters out rate-limit warning messages from coderabbit.ai, while c and r variables count all bot activity including these warnings. This causes false NEW_FEEDBACK detections. Apply the same rate-limit warning filter (excluding messages containing "rate limited by coderabbit.ai") consistently across all three counter assignments for c, r, and i variables to ensure that only actual review feedback is counted, not spurious warnings.
🤖 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.
Duplicate comments:
In `@examples/monitor.sh`:
- Around line 29-33: The bot feedback counting in the three variables (c for
pull request comments, r for reviews, and i for issue comments) is inconsistent.
Currently, only the i variable filters out rate-limit warning messages from
coderabbit.ai, while c and r variables count all bot activity including these
warnings. This causes false NEW_FEEDBACK detections. Apply the same rate-limit
warning filter (excluding messages containing "rate limited by coderabbit.ai")
consistently across all three counter assignments for c, r, and i variables to
ensure that only actual review feedback is counted, not spurious warnings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 72ed1083-27af-4c81-9c0d-57de2421db4d
📒 Files selected for processing (2)
crqexamples/monitor.sh
📜 Review details
🔇 Additional comments (9)
crq (8)
668-669: Past review feedback addressed.The dry-run path now properly propagates
state_writefailures, consistent with the live firing paths.
879-892: Past review feedback addressed and signal handling looks correct.The
need CRQ_SCOPEvalidation prevents the silent no-op. The signal traps properly chain: INT/TERM →exit 0→ EXIT trap → cleanup functions.
216-234: LGTM!
487-490: LGTM!
571-573: LGTM!
700-704: LGTM!
904-907: LGTM!
946-948: LGTM!examples/monitor.sh (1)
21-25: LGTM!
…nsitively (CodeRabbit) Round-9 excluded rate-limit WARNING posts only from the issue-comment counter and matched the marker case-sensitively. Apply the exclusion uniformly across all three bot_count counters (comments/reviews/issue-comments) via a shared case-insensitive filter, so warning-only bot activity never spuriously emits NEW_FEEDBACK regardless of which endpoint or marker casing. Ref #1 Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
|
@codex review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9dc6b4c498
ℹ️ 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".
| if [ "$incremental" = "0" ]; then | ||
| [ -z "$crrev" ] && { ( cmd_enqueue "$repo" "$pr" ) || true; } # auto-review only: never reviewed | ||
| else | ||
| [ "$crrev" != "$head" ] && { ( cmd_enqueue "$repo" "$pr" ) || true; } # + incremental: HEAD not reviewed |
There was a problem hiding this comment.
On a first crq autoreview pass for a scope with many unreviewed PRs, this calls cmd_enqueue once per PR; each call takes the global lock and writes both the issue state and README dashboard. With the default scan cap of 400, a single pass can therefore create hundreds of issue edits/README commits and hit GitHub secondary write limits before the queue is populated. Accumulate the PRs and append them in one locked state update instead of shelling out per PR.
Useful? React with 👍 / 👎.
- inflight_status: a rate-limit warning now ALWAYS requeues (future reset -> back off; expired/ unparseable -> retry now) and the 'newer bot comment = done' check excludes warning comments, so an expired-reset warning is no longer miscounted as a completed review (crq:504) - _leader_beat: re-confirm we still hold the leader ref immediately before the force-PATCH, so a heartbeat delayed past CRQ_LEADER_TTL can't clobber a host that legitimately stole it (crq:218) - gc: cap retained fired-markers at CRQ_FIRED_MAX (500) so markers for long-closed PRs can't grow the dashboard JSON without bound (crq:681) - _warning_latest: bound the under-lock WARNING scan to CRQ_WARNING_SCAN_LIMIT (40) PRs so it can't exceed CRQ_LOCK_TTL and let another process steal the lock mid-write (crq:459) Deferred (large-scope only; you run a 3-repo allowlist): batched autoreview enqueues (crq:942) and true >1000-PR search partitioning (crq:945) — the scan cap + rotating cursor + 1000-cap log already bound these; full batching/partitioning is a separate refactor not worth the complexity here. Ref #1 Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16625e6b5c
ℹ️ 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".
| msg="$(_lock_commit_msg "$cur")" | ||
| case "$msg" in *"$_LEADER_ID epoch="*) _leader_beat; return $? ;; esac # already ours -> heartbeat (fail -> re-contend) | ||
| cdate="$(gh api "repos/$CRQ_REPO/git/commits/$cur" --jq '.committer.date // empty' 2>/dev/null)" | ||
| age=999999; [ -n "$cdate" ] && age=$(( $(now) - $(iso2epoch "$cdate") )) |
There was a problem hiding this comment.
When the commit lookup for a foreign leader is rate-limited or otherwise fails, cdate stays empty and age remains 999999, so this path treats a fresh leader as stale and deletes its ref. That lets a standby steal leadership during transient GitHub/API failures and can leave multiple crq autoreview processes scanning/enqueueing in the same account; only steal after successfully parsing a leader commit timestamp older than CRQ_LEADER_TTL.
Useful? React with 👍 / 👎.
| gh api "repos/$REPO/pulls/$PR/reviews" --paginate \ | ||
| --jq '.[]|select(.user.login=="coderabbitai[bot]" and .commit_id!=null)|.commit_id' 2>/dev/null | tail -1 | cut -c1-9 |
There was a problem hiding this comment.
Since this helper only looks at formal pulls/$PR/reviews, in the comment-only completion path that the README loop supports, CRREV never advances to the current HEAD. A re-armed monitor will then keep enqueueing and pumping that same head every 60 seconds; dedup usually prevents another review command, but it still churns the shared state/README and can duplicate once the fired marker is cleared or pruned. Include the same issue/inline-comment signals in the reviewed-head test.
Useful? React with 👍 / 👎.
| cdate="$(gh api "repos/$CRQ_REPO/git/commits/$cur" --jq '.committer.date // empty' 2>/dev/null)" | ||
| age=999999; [ -n "$cdate" ] && age=$(( $(now) - $(iso2epoch "$cdate") )) | ||
| [ "$age" -lt "$CRQ_LEADER_TTL" ] && return 1 # foreign + fresh -> stand by | ||
| gh api -X DELETE "repos/$CRQ_REPO/git/refs/heads/$CRQ_LEADER_REF" >/dev/null 2>&1 # foreign + stale -> steal |
There was a problem hiding this comment.
When two standby crq autoreview daemons observe the same stale leader, this unconditional delete can run after another standby has already deleted the stale ref and created/confirmed its own fresh leader. Because the delete is by ref name rather than tied to the cur SHA read above, it can remove that new leader while it is already scanning, then this process can create a second leader for a split-brain pass; re-read that the ref still points at cur immediately before deleting or use a compare-and-swap strategy.
Useful? React with 👍 / 👎.
| # (don't let a failed README update leave the dashboard stale forever). | ||
| sig="$prev_sig" | ||
| if [ "$new_sig" != "$prev_sig" ]; then | ||
| if update_readme "$json"; then sig="$new_sig"; else log "WARN: README dashboard publish failed — will retry next change"; fi |
There was a problem hiding this comment.
If the README Contents API update succeeds but the subsequent issue edit fails (for example a transient issue-write/secondary-limit failure), the new readme_sig is never persisted, so every retry republishes the same dashboard commit before failing the machine-state write again. That can create a burst of duplicate README commits and a dashboard that shows state the queue never actually saved; avoid publishing until the issue update can be committed, or make the README write idempotent across failed state writes.
Useful? React with 👍 / 👎.
| sleep "$CRQ_AUTOREVIEW_POLL" | ||
| done | ||
| log "crq autoreview leader for $CRQ_REPO ($_LEADER_ID)" | ||
| local owner repo pr head crrev json present key firedsha rlc scanned idx deferred scan_cursor=0 searched |
There was a problem hiding this comment.
When users run the documented crq autoreview --once mode from cron or a timer and the scope has more than CRQ_AUTOREVIEW_MAX_SCAN eligible PRs, this cursor is recreated as 0 for every invocation, so each run starts from the same front segment and the rotation is discarded when the process exits. The long-running daemon eventually reaches the tail, but repeated --once runs can leave later PRs uninspected indefinitely; persist the cursor between runs or avoid applying the per-process scan cap in --once mode.
Useful? React with 👍 / 👎.
- _leader_try: if the foreign leader's commit timestamp can't be read (API blip), assume it's ALIVE and stand by instead of treating age as infinite and stealing — prevents stealing a fresh leader during transient failures (crq:234) - _leader_try: before deleting a stale leader ref, re-confirm it still points at the same commit we measured, so we don't delete a fresh leader another standby just created (crq:236) Accepted as fundamental/low-impact (GitHub refs have no compare-and-swap, dual-writes aren't atomic; the state LOCK prevents corruption and dedup catches duplicate reviews, so these are brief/self-correcting): residual force-heartbeat window (crq:218, window already shrunk), and the README-before-state write order (crq:345) which is the lesser evil vs the stale-dashboard problem crq:273 fixed. Deferred: per-run cursor persistence for --once (crq:905; the daemon rotates in-process). Ref #1 Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7dc14de2ec
ℹ️ 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".
| since="$(date -u +%Y-%m-%dT%H:%M:%SZ)" | ||
| if ! crq wait "$REPO" "$PR"; then # coordinated, FIFO, never fires while rate-limited |
There was a problem hiding this comment.
In rounds where a previous or delayed CodeRabbit response lands while this loop is still blocked in crq wait (for example behind the FIFO queue or rate limit), that response is newer than this since value. After crq finally fires the next request, wait_for_review "$since" can immediately succeed on the stale response and run process_review_and_push before feedback for the current request/commit has landed; capture the lower bound after the wait returns, or have crq wait expose the fired timestamp/comment id.
Useful? React with 👍 / 👎.
| crrev="$(gh api "repos/$repo/pulls/$pr/reviews" --paginate \ | ||
| --jq ".[]|select(.user.login==\"$CRQ_BOT\" and .commit_id!=null)|.commit_id" 2>/dev/null | tail -1 | cut -c1-9)" |
There was a problem hiding this comment.
When this reviews lookup is temporarily rate-limited or otherwise fails, stderr is discarded and the empty crrev value is later treated the same as “never reviewed.” For PRs whose HEAD was already reviewed but do not have a matching .fired marker in crq state (for example reviews that predate crq or pruned/migrated state), a transient API failure can enqueue and eventually spend quota on a duplicate review request; make the enqueue decision only after the reviews request succeeds.
Useful? React with 👍 / 👎.
| c=$(gh api "repos/$REPO/pulls/$PR/comments" --paginate --slurp 2>/dev/null | BOTS="$BOTS" jq "$f" 2>/dev/null) | ||
| r=$(gh api "repos/$REPO/pulls/$PR/reviews" --paginate --slurp 2>/dev/null | BOTS="$BOTS" jq "$f" 2>/dev/null) | ||
| i=$(gh api "repos/$REPO/issues/$PR/comments" --paginate --slurp 2>/dev/null | BOTS="$BOTS" jq "$f" 2>/dev/null) | ||
| echo "${c:-0}:${r:-0}:${i:-0}" |
There was a problem hiding this comment.
When any of these three gh api calls is rate-limited or transiently fails, stderr is discarded and the empty result is rendered as 0 here. A healthy baseline such as 2:1:4 then becomes 0:0:0, so the loop exits NEW_FEEDBACK and wakes an unattended agent even though no bot feedback arrived; only compare after all counters were fetched successfully, or retry/keep BASE on failure.
Useful? React with 👍 / 👎.
| if ! _leader_try; then | ||
| log "lost crq autoreview leadership — standing by" | ||
| while ! _leader_try; do [ "$once" = "1" ] && return 0; sleep "$CRQ_AUTOREVIEW_POLL"; done |
There was a problem hiding this comment.
In large scopes, this is the last leader heartbeat before the code scans and may enqueue up to CRQ_AUTOREVIEW_MAX_SCAN PRs, and each enqueue takes the global lock and writes GitHub state. If that pass runs longer than CRQ_LEADER_TTL (180s by default), a standby can legitimately steal the stale leader ref while this process is still scanning, so two autoreview daemons can enqueue/pump simultaneously; heartbeat inside the scan/enqueue loop or set the TTL from the worst-case pass duration.
Useful? React with 👍 / 👎.
| [ -f "${CRQ_CONFIG:-$HOME/.config/crq/env}" ] && . "${CRQ_CONFIG:-$HOME/.config/crq/env}" | ||
| : "${CRQ_REPO:?run 'crq init' once and configure ~/.config/crq/env (see the README)}" | ||
|
|
||
| still_open() { [ "$(gh pr view "$PR" --repo "$REPO" --json state -q .state 2>/dev/null)" = "OPEN" ]; } |
There was a problem hiding this comment.
If this gh pr view call is rate-limited or hits a transient GitHub/API error while the PR is still open, stderr is discarded and the comparison evaluates false, causing the autonomous loop to exit and print the same success/closed message as a real closure. Distinguish CLOSED/MERGED from an unknown state, or retry the state lookup before terminating the review loop.
Useful? React with 👍 / 👎.
…x round 11 tail) Distinguish 'the API call failed' from 'the thing is absent' across the loop/monitor/autoreview: - examples/review-loop.sh: still_open() only exits on explicit CLOSED/MERGED (an empty state from a transient gh failure no longer looks like a closure); start the feedback window AFTER crq fires so a delayed earlier response can't satisfy wait_for_review for a review that hasn't run yet - examples/monitor.sh: bot_count returns an error (caller skips the tick) instead of fabricating 0:0:0 from a failed gh call, which would look like feedback vanished and false-wake the loop - autoreview: skip a PR when its reviews lookup FAILS rather than treating an unreadable history as 'never reviewed' and enqueueing a redundant review (crq:954) Remaining open are fundamental/large-scope/accepted (crq:218 force-PATCH window, crq:345 dual-write order, crq:909 --once cursor, crq:915 long-pass heartbeat, crq:942/945 >cap scaling) — the state lock prevents corruption in all of them. Ref #1 Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52c4aefdb6
ℹ️ 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".
| still_open() { [ "$(gh pr view "$PR" --repo "$REPO" --json state -q .state)" = "OPEN" ]; } | ||
|
|
||
| while still_open; do | ||
| since="$(date -u +%Y-%m-%dT%H:%M:%SZ)" |
There was a problem hiding this comment.
In this copy/paste loop, any delayed CodeRabbit response from a previous round that lands while crq wait is blocked is newer than this timestamp. After crq wait finally fires the current request, the polling below can immediately set got=1 on that stale response and run process_review_and_push before feedback for the just-fired review has arrived; capture the lower bound after a successful crq wait like the standalone example does.
Useful? React with 👍 / 👎.
| set -uo pipefail | ||
| REPO="${REPO:?set REPO=owner/name}"; PR="${PR:?set PR=<number>}" | ||
|
|
||
| still_open() { [ "$(gh pr view "$PR" --repo "$REPO" --json state -q .state)" = "OPEN" ]; } |
There was a problem hiding this comment.
If gh pr view is rate-limited or hits a transient API/auth failure while the PR is still open, the command substitution is empty and this condition evaluates false, so the recommended unattended loop exits and prints the same converged/closed message as a real closure. Treat only explicit CLOSED/MERGED as terminal or retry an unknown state, as the standalone script does.
Useful? React with 👍 / 👎.
| if ! crrev_raw="$(gh api "repos/$repo/pulls/$pr/reviews" --paginate \ | ||
| --jq ".[]|select(.user.login==\"$CRQ_BOT\" and .commit_id!=null)|.commit_id" 2>/dev/null)"; then continue; fi |
There was a problem hiding this comment.
When CodeRabbit completes the first run with a conversation comment/reaction instead of a formal PR review (a completion path inflight_status and the README loop both support), this lookup leaves crrev empty. After the PR gets any new commit, the fired marker no longer matches the head, so crq autoreview --no-incremental treats the PR as never reviewed and enqueues another review despite promising to review each PR only once; include the same comment/reaction completion signal in the “ever reviewed” test.
Useful? React with 👍 / 👎.
| elif [ "$enqueued" = "1" ]; then | ||
| log "fired $repo#$pr"; return 0 # was queued, now gone -> fired & cleared |
There was a problem hiding this comment.
When the current head already has a fired marker, for example because crq autoreview or another runner already requested/reviewed that commit, cmd_pump dedups by removing this waiter's queue entry without posting a new review. This branch then returns success as if a fresh request was fired, so loops that start their feedback window after crq wait will wait for feedback that will never be newer instead of processing the existing review; distinguish the dedup/already-requested case from a newly fired review.
Useful? React with 👍 / 👎.
| gh api "repos/$REPO/pulls/$PR/reviews" --paginate \ | ||
| --jq '.[]|select(.user.login=="coderabbitai[bot]" and .commit_id!=null)|.commit_id' 2>/dev/null | tail -1 | cut -c1-9 |
There was a problem hiding this comment.
When the pulls/$PR/reviews request is rate-limited or transiently fails, this helper returns an empty CRREV, which line 53 treats the same as “HEAD has not been reviewed.” For PRs whose current head was already reviewed but lack a live crq fired marker (for example reviews predating crq or pruned state), the monitor can enqueue and pump a duplicate review just because the reviews endpoint was unreadable; make this helper signal failure and skip the tick instead.
Useful? React with 👍 / 👎.
…tion edges) - README example loop: still_open() only exits on explicit CLOSED/MERGED (transient gh failure no longer reads as a closure); start the feedback window AFTER crq fires so a delayed prior-round response can't falsely satisfy the poll. Matches examples/review-loop.sh (README crq:274/277). - examples/monitor.sh: cr_last_review signals failure; the enqueue decision is skipped when the reviews lookup fails (don't treat unreadable history as 'HEAD unreviewed' and enqueue) (monitor:43). - cmd_wait: when a wait leaves the queue via DEDUP (commit already reviewed) report 'already reviewed' instead of claiming 'fired' (crq:734). - autoreview --no-incremental: honor comment-only CodeRabbit completions (walkthrough/summary comment), not just formal reviews, so an already-reviewed PR isn't re-enqueued (crq:956). Ref #1 Claude-Session: https://claude.ai/code/session_01QVnKpFmFMYvRTyeoYozXst
Three findings, all from the previous two commits, all the same root error: the table said "#1 is what fires next" in situations where that is not determined. **With nothing eligible, the earliest window is not the next fire.** If no pump runs between two retry times — a polling gap, or the daemon down — both rounds are eligible at the next pass and NextEligible takes the lower Seq, not the earlier window. A position is now claimed only when some round can fire right now, in which case it IS the lowest-Seq ready one. Otherwise the soonest opening is still shown, without saying whose it is. **An empty ready time meant two different things.** "Nothing is holding this round" and "something is holding it whose end is unknowable" both rendered as an empty time and sorted alike, so a slot-blocked round could take the front. They rank apart now. **A cooling co-only round was jumping the queue.** Partitioning it out of the gates was right; concatenating it ahead of everything was not, because its own cooldown still applies. Both groups merge into one list ordered by readiness. Also: a reserved round on a RETRY still carried the previous attempt's FiredAt, since retries preserve that history deliberately — so the in-flight table ordered it by a fire hours old and printed that time for a command this attempt had not posted. One correction to the last commit: dropping a follower's time is right, but it was dropping the REASON too. "Slot busy" and "account blocked" are true and worth saying; only the start time is unknowable. A round with nothing of its own holding it is the one that is purely behind.
Two findings, and the second corrects an assumption in the exemption itself. **Exempting co-only rounds from the slot was wrong**, though exempting them from the account window was right. They do not need the slot — but Pump returns as soon as it sees a slot holder, so the quota-free path that would advance them is never reached while one is held. The dashboard was promoting such a round to "#1, ready now" for work the daemon cannot take until the holder is acknowledged. The account window genuinely does not apply to them; the slot, in practice, does. (An agent's own `crq next` can still resolve such a round directly through the quota-free bypass, which is why this describes what the queue will do rather than forbidding anything.) **The in-flight table showed a stale fire time for a reservation.** A retry keeps the previous attempt's FiredAt as history and Reserve does not clear it, so a reserved round printed an earlier attempt's timestamp as though the current command had gone out — and if that post hangs, the misleading value is what stays on the dashboard next to a stranded reservation. Ordering was fixed last round; the rendered cell was not. Unrelated observation while verifying: TestLoopSettleWindowCatchesTrailingWave is flaky. It sleeps 50ms and depends on real 1ms polls, so it loses under parallel load — it failed once and then passed five consecutive full-package runs plus a run against the unmodified tree. Left alone here; noted on the roadmap.
* Show parked rounds on the dashboard
A round in awaiting_retry whose RetryAt has not passed is correctly not
fire-eligible, so QueuedRounds excluded it — and then it vanished from the
dashboard entirely. Two PRs parked until 00:07Z rendered byte-identically to
an empty queue ("_Nothing queued._", title "idle"), so a reader concluded
their enqueue had been dropped.
Add parkedRounds: the active rounds that no other part of the dashboard
accounts for — not fire-eligible, and not named in the "In flight"/"Feedback
wait" rows. That also catches two neighbouring blind spots: every reviewing
round past the first (only reviewing[0] gets a row) and a reserved round whose
fire slot was cleared. Every active round now appears in exactly one of "In
flight", "Feedback wait", "Queue" or "Parked".
The queue heading gains a "· N parked" suffix and the empty-state text is
suppressed whenever work is parked; RenderTitle reports parked counts so a
parked-only state is never "idle". FireEligible/QueuedRounds and firing
behaviour are untouched — this is rendering only.
Adds internal/state/dashboard_test.go, the first test file in the package.
* Render one ordered review queue on the dashboard
The dashboard had two ways to be waiting for a fire slot. "Queue" listed
fire-eligible rounds; a round in awaiting_retry whose RetryAt had not passed
fell out of QueuedRounds and needed a separate "Parked" table to be visible at
all. Rendering live fleet state showed why that is the wrong cut: two rounds
sat parked with near-identical retry times and nothing said which went first,
or how they related to the rounds listed above them.
They were never racing. NextEligible takes the lowest Seq among eligible
rounds, Seq survives AwaitRetry untouched, and the FireSlot admits one fire
fleet-wide — a cooling-down round is already queued, in order. What was wrong
is the model: a round's position in line (Seq) and its permission to run
(RetryAt) are independent properties, and crq rendered the first as a queue and
the second as a lifecycle phase.
State.Queue is the single view: every round waiting for a fire, ordered by
(ReadyAt, Seq) — the order they actually reach the slot. ReadyAt folds in the
account-wide quota block, which fixes a real bug: the parked table printed the
round's own RetryAt while DecideFire also gates on Account.BlockedUntil, so a
window CodeRabbit later extended left the dashboard promising a retry time the
fire gate would refuse. FireEligible, QueuedRounds and NextEligible are
untouched; this is a view, not a schema or a rule.
The single "In flight"/"Feedback wait" summary rows are replaced by an In
flight section listing every reserved/fired/reviewing round. Those rows showed
one round each, which hid every reviewing round past the first and any reserved
round whose FireSlot Normalize had cleared. In flight and Queue now partition
Active() by phase, so "every active round is on the dashboard" holds by
construction rather than by a catch-all — pinned by a test.
* Make the ready column a promise firing will keep
Rebased onto the next/wait work and addressed the review round.
The queue table advertises when each round will fire. Three cases printed a time
the fire gate would have refused:
- **Pacing was left out.** DecideFire rejects every round until
LastFired + CRQ_MIN_INTERVAL, so a round was shown "ready now" that firing
would refuse for up to another 90 seconds — or far longer, since the interval
is configurable. It was excluded as sub-minute churn, which was wrong on both
counts: it is an absolute boundary, so it does not churn between renders.
- **Gates did not compose.** The first matching branch won, so a one-minute
cooldown inside a two-hour account block advertised the cooldown. The ready
time is now the latest of every gate, and the reason is whichever one binds.
- **A slot held by another PR was shown as "ready now".** Its release time is
unknowable, so the column stays empty and the reason carries the truth.
Also: a `reserved` round with no FireSlot behind it was reported as "Awaiting
feedback". It has not posted its command, so no feedback can be coming, and Pump
cannot advance it without the slot — the reader was sent looking for a review
nobody requested. It is named as a stranded reservation now.
MinInterval reaches the renderer through StoreConfig, which is exactly what that
struct is for: the fields the store and dashboard need from crq's Config.
* Project pacing along the queue, and let a held slot outrank a timestamp
Three findings, all the ready column still overstating readiness.
**Pacing is serial and was applied in parallel.** The current
LastFired + MinInterval boundary was applied to every entry independently, so with
a free slot and three ready rounds all three read "ready now" — but firing the
front pushes the next out by a whole interval. It is now projected along the
queue: each place in line adds an interval, so only the front claims now.
**A held fire slot now outranks every projected time.** It was recorded only when
no timed gate had produced a ReadyAt, so a cooldown or pacing boundary was shown
as a promise while the round could not fire at all. Its release time is
unknowable, so no timestamp there is honest.
**A stranded reservation is found wherever it sits.** In flight is ordered by fire
time, so an older reviewing round hid a later slot-less reserved round and the
header reported a plain feedback wait — and a reviewing round alongside a stranded
one is the normal shape, not an edge case.
* Show only times crq can know, and order the queue the way firing does
Round two, and all three findings are consequences of round one's pacing fix.
**Projected times were anchored to render time.** Deriving "front plus one
interval" from `now` gave an unchanged state a different body — and therefore a
different DashboardSHA — every minute, which would have turned every render back
into a dashboard write. The original code's rationale for omitting pacing was half
right after all: an absolute LastFired + MinInterval boundary is stable and worth
showing, a projection from render time is not. Rounds behind the front now say so
instead of naming a time crq cannot know until the round ahead of them fires.
**The order disagreed with what firing does.** NextEligible takes the lowest Seq
among rounds eligible at that moment, so a round still cooling down when the front
fires can overtake a ready round with a higher Seq. With seq 1 ready, seq 2 cooling
for 60s and seq 3 ready on a 90s interval, the queue really runs 1, 2, 3 while
sorting by ready time rendered 1, 3, 2. The order is simulated now — advance to
when something can run, take the lowest Seq that can, add an interval, repeat —
which is the same rule, so the two cannot drift.
**The title contradicted the body.** The body names a stranded reservation; the
title still reported a feedback wait for a round that never posted a command.
The pacing test from round one asserted the churning behaviour, so it now states
the corrected rule and additionally pins stability: the same state rendered a
minute later must produce the same queue.
* Start the queue simulation where firing can resume
Round three, one finding. The simulation began at render time (or the pacing
boundary), ignoring the account block — so with a lower-Seq round cooling for an
hour, a ready higher-Seq round, and the account blocked for two hours, the queue
rendered the ready one first. Both are eligible the moment the block clears, and
NextEligible then takes the lowest Seq, so the real order is the other way round.
The clock now starts at the later of now, the pacing boundary and the account
block: the point where firing can actually resume.
* Stop the renderer undoing the model, and stop ordering what cannot be ordered
Round four. The model had stopped promising times it could not keep; the view and
two edge cases had not caught up.
**The renderer turned "unknown" back into "now".** Queue clears the ready time for
a round blocked behind another PR's slot, and the table printed every zero as
"now" — so it read "ready: now | slot busy", contradicting itself across two
columns. A gate whose end is unknowable now renders as unknown.
**Followers repeated the front's boundary.** Two rounds sharing an account block
both showed that same timestamp, but after the front fires pacing pushes the next
out by another interval, so the second was advertising a moment firing will
refuse. A follower may only show a time strictly later than the round ahead of it
could finish, and the floor is built from absolute gates alone so nothing is
anchored to render time.
**With the slot held, the simulated ORDER was a guess too.** Not just the times:
which round is picked when the holder releases depends on which cooldowns have
elapsed by then, and that moment is unknowable. It falls back to Seq — the only
ordering that is true whenever several rounds become eligible together.
**A stranded reservation was hidden behind states that clear by themselves.** An
account block or another PR's review resolves on its own; a reserved round with no
slot behind it cannot be advanced by Pump at all. It is reported first now, in
both the heading and the title.
* Only call a reservation stranded when no slot backs it
Round five, one finding, and a regression from round four: making a stranded
reservation outrank every other state meant the happy path started reporting it.
Every normal fire passes through PhaseReserved WITH a valid FireSlot while the
command is being posted. Testing the phase alone therefore announced "no fire slot
backs it" during ordinary operation — loudly, since that state now wins the
heading and the title.
Holding the slot is the whole distinction, so it is checked: a reserved round that
IS the slot holder is mid-fire, and only one whose slot has gone is stranded. The
test pins both directions, because the diagnostic is worth nothing if it fires on
the path it is meant to distinguish from.
* Stop claiming an order while the slot decides it
Round six, three findings.
**A reservation sorted by when it was enqueued.** A reserved round has no FiredAt
— the command is not posted yet — so the in-flight table fell through to
EnqueuedAt and put a PR reserved seconds ago ahead of reviews fired an hour
earlier, contradicting its own ordering. It sorts by ReservedAt now.
**A co-only round was told to wait for the account window.** It spends no quota,
so DecideFire resolves it before that gate ever applies; labelling it "account
blocked until T" described a wait the next observation can end immediately. The
window no longer gates a round already degraded to its co-reviewers.
**The queue stopped claiming positions while the slot is held**, rather than
claiming better ones. Two rounds ago that ordering became a simulation, then a Seq
fallback; both were still guesses, because which round fires next depends on whose
cooldown has elapsed at the moment the slot releases — and that moment is
unknown. Refining the guess was the wrong move: the honest answer is that the
order is not knowable yet, so the position column says so and the rows remain
informative without pretending to a sequence.
* Stop predicting a queue order that cannot be predicted
Four review rounds went into refining this prediction; the fourth explained why
there was never one to make. Simulating the queue assumed a slot frees one pacing
interval after a fire, but release comes from the bot acknowledging the command or
from CRQ_INFLIGHT_TIMEOUT — neither of which pacing knows. With seq 1 ready, seq 2
cooling five minutes and seq 3 ready, an unacknowledged seq 1 leaves both
followers eligible together and NextEligible then takes seq 2, the opposite of
what any simulation rendered.
So the simulation is gone. The queue reports what is known: rows ordered by the
gate that binds them and then by Seq, a position for the front alone, and every
follower marked as behind an earlier round rather than given a time. A follower
cannot start before its own gate, but neither can it start then — that depends on
when the round ahead of it finishes — so printing that gate in a "ready" column
stated a lower bound as if it were the answer. The header and the front row still
carry the block or cooldown that explains the wait.
Also fixed: exempting a co-only round from the account block reached its label but
not the ordering, because the shared simulation clock still advanced to the block.
Such a round spends no quota and DecideFire resolves it before that gate, so
immediately actionable work was being listed behind a wait that does not apply to
it.
The net effect is less code and fewer claims: three of the tests here asserted
behaviour of the prediction, and now state the two things that are actually true —
which round is next, and that the rest are not ranked.
* Take co-only rounds out of the queue instead of exempting them from it
Third round running, third place the exemption was missing: the account window,
then the ordering clock, now the follower pass — which had already rewritten an
eligible co-only round as "behind an earlier round" before the slot check spared
it. Exempting gate-by-gate was always going to keep missing one.
A co-only round waits for nothing the queue serializes. It spends no account
quota, takes no fire slot, and DecideFire resolves it before either gate, so its
position behind other rounds is meaningless too. It is now partitioned out before
any queue-wide gate is applied, and listed first — actionable work should not sit
under rounds waiting for a slot it never needs.
Only its own cooldown still binds, because that belongs to the round rather than
to the queue.
The test asserts the whole exemption at once, with an account block, a pacing
boundary and a held slot all active, rather than one gate at a time — which is how
the previous two fixes each looked complete.
* Claim a next round only when one can actually fire
Three findings, all from the previous two commits, all the same root error: the
table said "#1 is what fires next" in situations where that is not determined.
**With nothing eligible, the earliest window is not the next fire.** If no pump
runs between two retry times — a polling gap, or the daemon down — both rounds are
eligible at the next pass and NextEligible takes the lower Seq, not the earlier
window. A position is now claimed only when some round can fire right now, in
which case it IS the lowest-Seq ready one. Otherwise the soonest opening is still
shown, without saying whose it is.
**An empty ready time meant two different things.** "Nothing is holding this
round" and "something is holding it whose end is unknowable" both rendered as an
empty time and sorted alike, so a slot-blocked round could take the front. They
rank apart now.
**A cooling co-only round was jumping the queue.** Partitioning it out of the
gates was right; concatenating it ahead of everything was not, because its own
cooldown still applies. Both groups merge into one list ordered by readiness.
Also: a reserved round on a RETRY still carried the previous attempt's FiredAt,
since retries preserve that history deliberately — so the in-flight table ordered
it by a fire hours old and printed that time for a command this attempt had not
posted.
One correction to the last commit: dropping a follower's time is right, but it was
dropping the REASON too. "Slot busy" and "account blocked" are true and worth
saying; only the start time is unknowable. A round with nothing of its own holding
it is the one that is purely behind.
* A held slot stops everything, free-running rounds included
Two findings, and the second corrects an assumption in the exemption itself.
**Exempting co-only rounds from the slot was wrong**, though exempting them from
the account window was right. They do not need the slot — but Pump returns as soon
as it sees a slot holder, so the quota-free path that would advance them is never
reached while one is held. The dashboard was promoting such a round to "#1, ready
now" for work the daemon cannot take until the holder is acknowledged. The account
window genuinely does not apply to them; the slot, in practice, does.
(An agent's own `crq next` can still resolve such a round directly through the
quota-free bypass, which is why this describes what the queue will do rather than
forbidding anything.)
**The in-flight table showed a stale fire time for a reservation.** A retry keeps
the previous attempt's FiredAt as history and Reserve does not clear it, so a
reserved round printed an earlier attempt's timestamp as though the current
command had gone out — and if that post hangs, the misleading value is what stays
on the dashboard next to a stranded reservation. Ordering was fixed last round;
the rendered cell was not.
Unrelated observation while verifying: TestLoopSettleWindowCatchesTrailingWave is
flaky. It sleeps 50ms and depends on real 1ms polls, so it loses under parallel
load — it failed once and then passed five consecutive full-package runs plus a
run against the unmodified tree. Left alone here; noted on the roadmap.
---------
Co-authored-by: kristofferR <kristofferR@users.noreply.github.com>
1 participant
Opens the entire
crqimplementation as a PR so CodeRabbit and Codex can review it.Base is the repo's initial commit, so the diff is effectively the whole tool: the
crqCLI, README, examples (monitor.sh,review-loop.sh), and the Claude skill.This is a review-only PR (the code already lives on
main); not intended to merge.Already validated locally: shellcheck-clean, and a CodeRabbit CLI review/fix loop converged to 0 findings.
Summary by CodeRabbit
crq autoreviewto run queue-coordinated, background review watching across eligible open PRs (--once, incremental control).