crq full code review (post-shipping quality pass)

[kristofferR]

Fresh CodeRabbit + Codex review of the current shipped crq code (main @ 076810b).

The tool is already shipped & deployed; this PR re-reviews the consolidated code end-to-end to catch anything the incremental rounds missed. Findings will be fixed on main.

Known/intentional (please don't re-litigate): the deferred large-scope items are tracked in #2–#5, and the README <details> collapsibles are intentional (markdownlint MD033 declined). Everything else: fix away.

Summary by CodeRabbit

• New Features
  • Added crq autoreview daemon with --once and --no-incremental, repo allow/deny filtering, and improved per-host coordination
  • Added crq version and crq help
• Bug Fixes
  • Hardened distributed lock and staleness handling; improved completion/in-flight detection with head-SHA deduping and stronger error propagation
  • Improved queue/pump/wait/cancel correctness and reduced default lock backoff
• Documentation
  • Major README refresh: updated setup/loop guidance, configuration, troubleshooting, and safer review triggering
• Examples
  • Added examples/monitor.sh and examples/review-loop.sh; removed examples/agent-loop.sh

[kristofferR]

@codex review

[kristofferR]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f6cd52de40

ℹ️ 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".

[chatgpt-codex-connector]

Refresh leadership during long autoreview scans

When multiple hosts run crq autoreview, the leader heartbeat is only refreshed at the start of each pass, but this per-PR scan can take longer than the default CRQ_LEADER_TTL=180 when scanning hundreds of PRs (CRQ_AUTOREVIEW_MAX_SCAN=400) and doing head/review lookups plus enqueue writes. In that case a standby can decide the ref is stale and steal leadership while the original leader is still scanning, so the advertised singleton turns into two daemons doing the same expensive scan/enqueue work and increasing secondary-limit pressure; heartbeat inside the long scan or make the TTL cover the worst-case pass duration.

Useful? React with 👍 / 👎.

[coderabbitai]

♻️ Duplicate comments (1)

examples/review-loop.sh (1)

43-45: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Use inclusive timestamp checks to avoid boundary misses.

since is second-granularity; strict > can miss feedback that lands in the same second and force a false timeout path. Switch these filters to >=.

Suggested patch

-        | jq --arg bot "$BOT" --arg rl "$RL" --arg since "$since" 'add | map(select(.user.login==$bot and ((.updated_at // .created_at) > $since) and ((.body//"")|ascii_downcase|contains($rl|ascii_downcase)|not))) | length' 2>/dev/null)
+        | jq --arg bot "$BOT" --arg rl "$RL" --arg since "$since" 'add | map(select(.user.login==$bot and ((.updated_at // .created_at) >= $since) and ((.body//"")|ascii_downcase|contains($rl|ascii_downcase)|not))) | length' 2>/dev/null)
@@
-        | jq --arg bot "$BOT" --arg since "$since" 'add | map(select(.user.login==$bot and .submitted_at > $since)) | length' 2>/dev/null)
+        | jq --arg bot "$BOT" --arg since "$since" 'add | map(select(.user.login==$bot and .submitted_at >= $since)) | length' 2>/dev/null)

🤖 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/review-loop.sh` around lines 43 - 45, The timestamp comparisons in
both jq filter expressions use strict greater-than `>` operators with the
`since` variable, which can miss feedback that lands in the same second due to
second-level granularity of the timestamp. Change the comparison operators from
`>` to `>=` in both places: update the filter that checks `(.updated_at //
.created_at) > $since` to use `>=` instead, and update the filter that checks
`.submitted_at > $since` to use `>=` instead. This ensures inclusive boundary
checks that won't miss feedback entries created in the same second as the since
timestamp.

🤖 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/review-loop.sh`:
- Around line 43-45: The timestamp comparisons in both jq filter expressions use
strict greater-than `>` operators with the `since` variable, which can miss
feedback that lands in the same second due to second-level granularity of the
timestamp. Change the comparison operators from `>` to `>=` in both places:
update the filter that checks `(.updated_at // .created_at) > $since` to use
`>=` instead, and update the filter that checks `.submitted_at > $since` to use
`>=` instead. This ensures inclusive boundary checks that won't miss feedback
entries created in the same second as the since timestamp.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e3d76004-c63e-4dce-bdc6-fcb62d9f4096

📥 Commits

Reviewing files that changed from the base of the PR and between 9e1b0fd and f6cd52d.

📒 Files selected for processing (2)

• crq
• examples/review-loop.sh

📜 Review details

🔇 Additional comments (5)

examples/review-loop.sh (1)

57-61: LGTM!

crq (4)

229-238: LGTM!

---

547-548: LGTM!

Also applies to: 559-560

---

977-979: LGTM!

---

1059-1059: 📐 Maintainability & Code Quality

Version claim for gh api --slurp is accurate.

The --slurp flag was indeed added in GitHub CLI 2.48.0, so the upgrade guidance in the error message is correct.

[kristofferR]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: b77d3a055e

ℹ️ 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".

[kristofferR]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

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)

README.md (1)

346-347: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Document crq wait exit code 3 in the command reference.

Line 346-Line 347 omits the dedup/already-reviewed return code (3), which is part of cmd_wait behavior. This can mislead loop implementations that rely on this section.

Suggested patch

-`<repo>` is `owner/name`; `<pr>` is the number. **Exit codes:** `crq wait` returns `0` once your
-review is fired (or `2` if `CRQ_WAIT_TIMEOUT` is hit); other commands return `0` on success.
+`<repo>` is `owner/name`; `<pr>` is the number. **Exit codes:** `crq wait` returns `0` when a new
+review is fired, `3` when the current head was already reviewed (deduped), and `2` if
+`CRQ_WAIT_TIMEOUT` is hit; other commands return `0` on success.

🤖 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 346 - 347, The documentation for `crq wait` command
exit codes in the README.md is incomplete and missing the exit code 3 behavior.
Update the exit codes section (around lines 346-347) to include documentation
for exit code 3, which represents the dedup/already-reviewed case. Add this exit
code to the existing exit code documentation list that currently only mentions
exit codes 0 and 2 to ensure users implementing loops have complete information
about all possible return values from the `crq wait` command.

crq (1)

678-687: 🎯 Functional Correctness | 🟠 Major

Validate full SHA before truncating to short form.

The current code truncates the GitHub API response to 9 characters before validation, allowing any non-empty all-hex string (regardless of origin) to pass as a valid commit SHA. Validate the complete response is exactly 40 lowercase hex characters, then derive the 9-character prefix. This ensures we catch truncated or corrupted API responses that happen to produce valid-looking hex strings.

🛡️ Proposed fix

-  local nonce json n bu bu_epoch lf lf_epoch qn cfresh infp head repo pr seq cid hsha key lastsha crrev_p
+  local nonce json n bu bu_epoch lf lf_epoch qn cfresh infp head repo pr seq cid hsha hsha_full key lastsha crrev_p
...
-    hsha="$(gh api "repos/$repo/pulls/$pr" --jq '.head.sha // empty' 2>/dev/null | cut -c1-9)"
+    hsha_full="$(gh api "repos/$repo/pulls/$pr" --jq '.head.sha // empty' 2>/dev/null)"
     key="$repo#$pr"
     lastsha="$(jq -r --arg k "$key" '.fired[$k] // empty' <<<"$json")"
@@
-    if [ -z "$hsha" ] || [ -n "${hsha//[0-9a-f]/}" ]; then
+    if [ "${`#hsha_full`}" -ne 40 ] || [ -n "${hsha_full//[0-9a-f]/}" ]; then
       # Couldn't read a valid head SHA (transient/secondary-limit) — DON'T fire with a bogus SHA;
       # leave the PR queued so the next pump retries once the API recovers.
       log "skip: could not read a valid $key head SHA (transient) — leaving it queued"
+    else
+      hsha="${hsha_full:0:9}"

-  local repo pr head crrev crrev_raw reviewed reviewed_raw json present key firedsha rlc scanned idx deferred scan_cursor=0 searched
+  local repo pr head head_full crrev crrev_raw reviewed reviewed_raw json present key firedsha rlc scanned idx deferred scan_cursor=0 searched
...
-        head="$(gh api "repos/$repo/pulls/$pr" --jq '.head.sha // empty' 2>/dev/null | cut -c1-9)"
+        head_full="$(gh api "repos/$repo/pulls/$pr" --jq '.head.sha // empty' 2>/dev/null)"
         # Require a real short SHA — a failed lookup leaves a non-empty NON-hex error body (gh dumps
         # it to stdout, --jq skipped), which must not be treated as a head and drive an enqueue (crq:975).
-        [ -n "$head" ] && [ -z "${head//[0-9a-f]/}" ] || continue
+        [ "${`#head_full`}" -eq 40 ] && [ -z "${head_full//[0-9a-f]/}" ] || continue
+        head="${head_full:0:9}"

🤖 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 `@crq` around lines 678 - 687, The hsha variable assignment currently truncates
the GitHub API response to 9 characters using cut before any validation occurs,
which allows corrupted or truncated responses that happen to be valid hex to
pass validation. Refactor the assignment to first capture the full response from
the gh api call without truncating, then validate that the complete response is
exactly 40 characters and contains only lowercase hex digits, and only after
successful validation should the 9-character prefix be extracted and assigned to
hsha. Update the validation logic if needed to check the full 40-character
response before the truncation happens.

🤖 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 692-699: The current elif statement combines the gh api call and
jq parsing in a way that fails silently when either command errors, leaving
crrev_p empty and causing the condition to evaluate false, which allows
execution to fall through to the duplicate post branch. Separate the gh api
command from the jq parsing by first executing the gh api call and explicitly
checking its exit status, then pipe only successful output to jq and check that
result separately. Add explicit error handling or exit logic to prevent falling
through to the live post branch when either the gh api call or jq parsing fails
transiently, ensuring that duplicate reviews are not posted due to temporary API
or parsing errors.

---

Outside diff comments:
In `@crq`:
- Around line 678-687: The hsha variable assignment currently truncates the
GitHub API response to 9 characters using cut before any validation occurs,
which allows corrupted or truncated responses that happen to be valid hex to
pass validation. Refactor the assignment to first capture the full response from
the gh api call without truncating, then validate that the complete response is
exactly 40 characters and contains only lowercase hex digits, and only after
successful validation should the 9-character prefix be extracted and assigned to
hsha. Update the validation logic if needed to check the full 40-character
response before the truncation happens.

In `@README.md`:
- Around line 346-347: The documentation for `crq wait` command exit codes in
the README.md is incomplete and missing the exit code 3 behavior. Update the
exit codes section (around lines 346-347) to include documentation for exit code
3, which represents the dedup/already-reviewed case. Add this exit code to the
existing exit code documentation list that currently only mentions exit codes 0
and 2 to ensure users implementing loops have complete information about all
possible return values from the `crq wait` command.

🪄 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: d6338af8-d6f7-4f4e-b0aa-c7fe2df1c7d0

📥 Commits

Reviewing files that changed from the base of the PR and between 9e1b0fd and b77d3a0.

📒 Files selected for processing (4)

• README.md
• crq
• examples/monitor.sh
• examples/review-loop.sh

📜 Review details

🔇 Additional comments (4)

examples/monitor.sh (1)

60-62: LGTM!

examples/review-loop.sh (1)

43-45: LGTM!

Also applies to: 57-61

README.md (1)

178-345: LGTM!

Also applies to: 349-420

crq (1)

228-247: LGTM!

Also applies to: 547-548, 559-560, 623-677, 688-691, 700-726, 922-983, 988-1028, 1058-1084

[kristofferR]

@codex review

[kristofferR]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[kristofferR]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2a27efa3d4

ℹ️ 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".

[chatgpt-codex-connector]

Avoid force-updating a stolen leader ref

If this leader is paused after the ownership check above and another daemon steals the stale ref after CRQ_LEADER_TTL, the resumed process still patches the ref with force=true, which overwrites the new leader unconditionally; GitHub's refs API describes force=false as the mode that avoids overwriting work (https://docs.github.com/rest/git/refs#update-a-reference). In that pause/steal window, both autoreview daemons can believe they are leader and run scans concurrently, so the heartbeat should be a conditional fast-forward/CAS-style update rather than a forced ref write.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Refresh leadership during long scans

This heartbeat is only performed before the autoreview pass; with the default cap of 400 scanned PRs, the head/reviews/comment lookups plus enqueue calls can easily take longer than CRQ_LEADER_TTL=180s in a large scope. When that happens, a standby treats this still-running daemon as stale and starts its own scan, so the singleton guarantee is lost until the original loop reaches the next iteration; heartbeat periodically inside the scan loop or size the TTL above the worst-case pass duration.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Persist the autoreview cursor for --once

When crq autoreview --once is used from cron or a timer and there are more than CRQ_AUTOREVIEW_MAX_SCAN scan-eligible PRs, this break exits right after deferring the tail and the in-memory scan_cursor is lost. The next cron invocation starts at offset 0 again, so the same first 400 PRs are scanned every time and PRs beyond the cap may never be enqueued; either persist the cursor across --once runs or keep scanning until the deferred tail is covered before exiting.

Useful? React with 👍 / 👎.

[kristofferR]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c05444d555

ℹ️ 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".

[chatgpt-codex-connector]

Track edited bot feedback in the monitor

When CodeRabbit updates an existing walkthrough/summary comment for a new review instead of adding a new comment, the issue/review counts returned by bot_count stay identical, so this comparison never exits with NEW_FEEDBACK and an unattended loop can sit until IDLE_TIMEOUT despite fresh feedback. Track the newest updated_at/submitted_at timestamp (while still excluding rate-limit warnings) rather than only comparing counts.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Poll edited CodeRabbit comments in the README loop

Users copying this recommended loop can still miss a completed review when CodeRabbit refreshes an existing summary/walkthrough comment: the poll only counts issue comments whose created_at is newer than since, so an edited bot comment with a newer updated_at is ignored and the loop times out without processing the feedback. Match the standalone example by comparing updated_at // created_at for issue comments.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Sort warning probes by recency before applying the cap

In an owner with more than CRQ_WARNING_SCAN_LIMIT open PRs and calibration unavailable, this only inspects the first default search results before looking for warning comments. The gh manual documents --limit as the maximum results fetched and the default --sort as best-match (with updated available), so a fresh CodeRabbit rate-limit warning outside those first 40 PRs is invisible; crq can then clear blocked_until and post another review while the account is still limited.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Check PR summaries outside issue comments

In --no-incremental mode, a PR with no formal CodeRabbit review relies on this issue-comment scan to decide whether a review already happened, but CodeRabbit's default PR summary is written into the PR description unless users move it to the walkthrough (see https://docs.coderabbit.ai/pr-reviews/summaries). For summary-only completions using that default placement, the marker is never present in issues/$pr/comments, so autoreview --no-incremental can enqueue and spend another review on a PR that was already reviewed once.

Useful? React with 👍 / 👎.