Skip to content

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

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

ci: sync Antigravity reviewer with template hardening fixes#335
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.

Rebased onto the current main (including #330-#334, notably #333's own OAuth-leak-guard hardening) — the two changes here are additive on top of that work, not a regression of it. Verified: bash -n clean, shellcheck shows no new warnings, byte-identical to the shared template's own updated copy.

🤖 Generated with Claude Code

Two reliability/robustness fixes from the shared template
(~/Code/Local_Only-Projects/antigravity-pr-review/), found by an agy/
CodeRabbit review on the sibling Rusty2600 project's own copy:

- The singleton agy-serialization flock now fails CLOSED (exits) on a
  missing flock, a failed fd redirect, or a 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 (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.

Also picks up the template's own already-more-refined OAuth-leak
guard (oauth_url_present/OAUTH_URL_RE), which this repo's copy had
drifted behind.

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: 44 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: 8406152a-0deb-4642-bc55-3761d6ce8cde

📥 Commits

Reviewing files that changed from the base of the PR and between 71d48ef and 23a1d13.

📒 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 require flock for job serialization and reorders comment management to post new review comments before deleting prior ones.

Blocking issues

  • Self-deletion of newly posted review comments due to invalid JQ syntax (scripts/agy-review.sh:L548-L551):
    In lines 548–549, contains("'"${MARKER}"'") expands in Bash to single-quoted JQ strings (contains('MARKER')), which is invalid syntax in JQ (JQ string literals require double quotes). Because stderr is suppressed with 2>/dev/null, JQ parse failure silently causes new_comment_id to evaluate to an empty string. In line 551, ${new_comment_id:-0} falls back to 0, causing the subsequent cleanup loop to match and immediately delete the review comment that was just posted.

  • Missing test coverage for behavior changes:
    Per the project style guide ("Tests accompany behavior changes"), changes to critical execution paths—specifically strict locking failure modes and publish-before-delete reordering—have been added without unit or integration tests.

Suggestions

  • Broken pagination handling in JQ query (scripts/agy-review.sh:L548-L549):
    gh api --paginate executes the --jq expression on each page payload independently. Using [.[] | ...] | last | .id outputs the last matching comment ID per page rather than across all pages, yielding multi-line output if comments span multiple API pages.

  • Fragile lock directory creation (scripts/agy-review.sh:L425-L426):
    Removing 2>/dev/null || true from mkdir -p "$(dirname "$AGY_LOCK")" means an unset or malformed $AGY_LOCK variable will immediately abort execution at mkdir or exec 9>> without emitting a clean log error message.

Nitpicks

  • Unsafe JQ parameter interpolation (scripts/agy-review.sh:L551):
    Line 551 interpolates ${new_comment_id:-0} directly into the --jq double-quoted string. Pass variables via --argjson instead of inline string expansion.

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

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