Skip to content

ci: sync Antigravity reviewer with template hardening fixes - #215

Closed
doublegate wants to merge 1 commit into
mainfrom
chore/agy-reviewer-sync
Closed

ci: sync Antigravity reviewer with template hardening fixes#215
doublegate wants to merge 1 commit into
mainfrom
chore/agy-reviewer-sync

Conversation

@doublegate

Copy link
Copy Markdown
Owner

Summary

Two reliability/robustness fixes to the Antigravity PR reviewer script, found by a CodeRabbit review on the sibling Rusty2600 project's own copy of this same shared template (~/Code/Local_Only-Projects/antigravity-pr-review/):

  • The singleton agy-serialization flock now fails closed (exits) on a missing flock, a failed fd redirect, or a lock timeout, instead of logging and proceeding unserialized — which left the exact concurrent-agy-process collision the lock exists to prevent still reachable.
  • The prior review comment is now deleted after the new one posts successfully, not before — a late failure (the OAuth guard tripping, a transient gh pr comment error) used to leave the PR with no review comment at all instead of the still-valid prior one.

Verified: bash -n clean, byte-identical to the shared template's own updated copy.

🤖 Generated with Claude Code

Two reliability/robustness fixes to the Antigravity PR reviewer script,
found by a CodeRabbit review on the sibling Rusty2600 project's own
copy of this same shared template:

- The singleton agy-serialization flock now fails CLOSED (exits) on a
  missing flock, a failed fd redirect, or a lock timeout, instead of
  logging and proceeding unserialized -- which left the exact
  concurrent-agy-process collision the lock exists to prevent still
  reachable.
- The prior review comment is now deleted AFTER the new one posts
  successfully, not before -- a late failure (an OAuth guard trip, a
  transient gh pr comment error) used to leave the PR with no review
  comment at all instead of the still-valid prior one.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@doublegate, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 42 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d3c1ad47-6c90-4458-93c5-14b1c64770d9

📥 Commits

Reviewing files that changed from the base of the PR and between 646a3e0 and 04979f9.

📒 Files selected for processing (1)
  • scripts/agy-review.sh

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

Antigravity review (Gemini via Ultra)

This PR updates scripts/agy-review.sh to fail closed when flock is missing or times out, reorders review comment publishing to occur before old comment deletion, and enforces an unconditional pre-post check for live OAuth URLs.

Blocking issues

  • Deletion of newly posted review when new_comment_id lookup fails (scripts/agy-review.sh:L547-L551):
    Silencing errors on the new_comment_id query (2>/dev/null) means any transient GitHub API error, rate limit, or empty response leaves new_comment_id unset. The fallback ${new_comment_id:-0} evaluates to 0, causing select(.id != 0) on line 550 to match all comments—including the newly posted review—and immediately delete it. If new_comment_id cannot be determined, comment cleanup must be skipped or safely aborted rather than falling back to 0.

Suggestions

  • Inconsistent and fragile jq string escaping for MARKER (scripts/agy-review.sh:L548 vs L550):
    Line 548 interpolates MARKER by breaking single quotes (contains("'"${MARKER}"'")), whereas line 550 uses double-quoted jq strings (contains(\"${MARKER}\")). If MARKER contains single quotes or special characters, line 548 will crash with a jq syntax error. Pass shell variables safely via --arg marker "$MARKER" in both gh api calls.
  • Redundant back-to-back gh api queries (scripts/agy-review.sh:L547-L550):
    The script performs two separate paginated API requests against repos/${REPO}/issues/${PR}/comments. Parse the comment ID directly from the URL output of gh pr comment, or fetch the comment list once to avoid unnecessary API consumption and potential race conditions between the two reads.

Nitpicks

  • Unchecked lock directory creation (scripts/agy-review.sh:L425): mkdir -p "$(dirname "$AGY_LOCK")" removed the || true fallback. If $AGY_LOCK is empty or points to a restricted path, dirname will target root or fail under set -e.

Automated first-pass review by agy on a self-hosted runner -- not a human review.

@doublegate

Copy link
Copy Markdown
Owner Author

Superseded by #259, which syncs the reviewer to the fixed version already on RustySNES main (its #270 sync + #273 comment-selection fix + agy-review-selftest.sh). This PR carried the earlier version with the self-deletion bug; #259 is byte-identical with RustySNES main and the template. Closing in favour of #259.

@doublegate doublegate closed this Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant