ci(review): standardize the Antigravity reviewer + fix the large-diff handoff

[doublegate]

What

Standardizes the Antigravity PR reviewer onto the shared cross-repo template and fixes the large-diff handoff, landed on main on its own — split out of the v2.2.4 release branch (#327) so the reviewer fix reaches the default branch (where the workflow always runs it) independently of the release.

Two files change; agy-review.sh is a near-total rewrite because it converges onto the template.

Why split it out

The agy workflow always runs the reviewer already on the default branch (it checks out main, never the PR head), so a fix inside a PR can't review its own PR. Landing the reviewer here first means:

• main gets the fixed reviewer now, so it can be validated live before the release ships;
• the v2.2.4 release PR (release: v2.2.4 "Cartridge" — libretro core builds/installs for RetroArch #327) shrinks to pure release content and rebases cleanly onto this (the reviewer files are byte-identical).

Contents

• Template convergence — large-diff file handoff, 20,000-line gh pr diff API-limit local-git diff fallback, isCrossRepository fork gate, fail-closed metadata, default-branch checkout + persist-credentials: false, synchronize auto-re-review.
• Five security hardenings the reviewer flagged against itself — printf '%q '-escaped script(1) fallback (command injection), author-scoped comment deletion, dropped unscoped SQLite fallback (data leak), env -u GH_TOKEN GITHUB_TOKEN, restored issue_comment author re-check.
• Large-diff handoff made readable (headline) — agy's sandboxed file tool resolves relative paths against its own workspace root, not the shell CWD, so the on-disk handoff read back as "does not exist" (latent until a diff crossed ~90 KB). Proven on the live runner; fixed with an absolute-path prompt + --add-dir "$PWD" scoped to file-handoff mode only.

Validation

Root cause + fix proven on the live agy runner (three-way probe under the exact --sandbox --dangerously-skip-permissions flags: relative-no-add-dir FAILS, relative+add-dir PASSES, absolute PASSES; plus a combined production-shaped run reading a realistic diff-shaped file PASSES). This PR's own diff (~42 KB) is inlined, so it also exercises the inline path live.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Reviews now run when pull requests are updated with new commits.
  • Large changes can be handled through improved diff retrieval and file-based review input.

• Bug Fixes

  • Improved safeguards prevent oversized prompts and malformed text from disrupting reviews.
  • Review failures now provide clearer diagnostic output.
  • Temporary review artifacts are cleaned up more reliably.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 22 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: 9cc1a8a5-15c8-4fea-bdf8-0b2ccdcad30c

📥 Commits

Reviewing files that changed from the base of the PR and between 60c1451 and 409e7d9.

📒 Files selected for processing (1)

• scripts/agy-review.sh

📝 Walkthrough

Walkthrough

The workflow now reviews synchronized pull requests and uses script defaults for diff sizing. The script adds fail-closed diff fallback, bounded prompt assembly, inline or file delivery, UTF-8 sanitization, PTY retries, environment isolation, and bounded failure output.

Changes

Antigravity review automation

Layer / File(s)	Summary
Workflow triggers and runtime configuration .github/workflows/antigravity-review.yml	Adds the synchronize trigger, removes the active 90KB diff override, and updates workflow comments.
Diff acquisition and prompt delivery scripts/agy-review.sh	Adds configuration limits, fail-closed diff fallback, byte truncation, inline or scratch-file delivery, prompt-size enforcement, and UTF-8 sanitization.
Review execution and failure reporting scripts/agy-review.sh	Updates temporary-file cleanup, locking, PTY-compatible retries, credential isolation, failure logging, and comment-author filtering documentation.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Workflow
  participant agy-review.sh
  participant GitHub
  participant agy
  Workflow->>agy-review.sh: start review on pull request event
  agy-review.sh->>GitHub: request pull request diff
  GitHub-->>agy-review.sh: diff or max-lines failure
  agy-review.sh->>agy-review.sh: fetch refs and build bounded prompt
  agy-review.sh->>agy: run review with isolated credentials
  agy-->>agy-review.sh: review output or stderr
  agy-review.sh-->>Workflow: publish result or bounded failure details

Loading

Possibly related PRs

• doublegate/RustyNES#324: Introduced the workflow and script that this PR further hardens.
• doublegate/RustyNES#327: Also updates Antigravity diff fallback, mode selection, and prompt byte limits.

🚥 Pre-merge checks | ✅ 9

✅ Passed checks (9 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the workflow reviewer standardization and the large-diff handoff fix.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Docs-As-Spec Sync	✅ Passed	Branch diff only touches .github/workflows/antigravity-review.yml and scripts/agy-review.sh; no rustynes-* chip crates or docs files changed, so docs sync is not applicable.
Changelog Entry For User-Visible Changes	✅ Passed	Root CHANGELOG.md has an empty [Unreleased] section, and the only touched file is scripts/agy-review.sh (tooling-only reviewer logic), so no user-visible changelog entry is required.
No Unwrap/Expect/Panic On Untrusted Input	✅ Passed	PR diff contains no new .unwrap(), .expect(), or panic!; the changed workflow and shell script add no Rust-style untrusted-input unwrapping.
Safety Comment On New Unsafe Blocks	✅ Passed	The PR only changes scripts/agy-review.sh, and the patch adds no Rust unsafe {} blocks or unsafe fns.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ci/agy-reviewer-standardize-handoff-fix

---

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

[doublegate]

Adjudicating the agy review of this PR. Three findings fixed in 409e7d97, three adjudicated as no-change.

Fixed:

• Suggestion — negative MAX_PROMPT_BYTES bypasses the E2BIG guard. Correct and fixed. [ "$MAX_PROMPT_BYTES" -le "$ARG_SIZE_CEILING" ] accepts a negative value, so the clamp was skipped and head -c "$MAX_PROMPT_BYTES" ran as GNU head -c -1 (all-but-last-byte), leaving the prompt effectively uncapped. Now requires a positive integer at or below the ceiling; negative, zero, and non-numeric all clamp.
• Suggestion — UTF-8 sanitize mv without a non-empty check. Fixed. Both the iconv and python3 branches now gate the mv on [ -s "$prompt_file.utf8" ], so a sanitizer that exits 0 but emits zero bytes can no longer blank the prompt.
• (Bonus, found while fixing the above) latent clamp-time crash. log() was defined after the config block but the clamp calls it, so a firing clamp died with log: command not found under set -e instead of warning. Moved log() / have_text() above the config block. The negative-value fix makes the clamp actually reachable, so this ordering had to be correct.

Adjudicated — no change:

• Blocking — "$truncated is never referenced/appended." False positive. It is surfaced: the comment-body assembly does printf '%s' "$truncated" (between the review body and the footer, at the body_file heredoc). Truncation notices reach the posted PR comment.
• Suggestion — standardize the unbuffer vs script prompt passing. Behaviourally already identical: the unbuffer path passes --print "$(cat "$prompt_file")" directly, and the script/PTY path calls _agy_print.sh, which does the exact same --print "$(cat "$prompt_file")". Both load the prompt into an argv value the same way and strip trailing newlines identically — the difference is structural (inline vs delegated), not behavioural, so there's no correctness gap to close.
• Nitpick — diff_file truncated before the file-mode copy. By design. MAX_DIFF_BYTES is the 5 MB pathological-diff sanity cap, and the workflow explicitly documents "Do NOT lower this to bound the review." At its default it never truncates a real diff before the handoff; lowering it is a documented misconfiguration, not a code path to guard.

All fixes ride the canonical template (byte-identical) so the three consuming repos stay in sync.

[github-actions]

Antigravity review (Gemini via Ultra)

This PR updates the GitHub Actions workflow and review script to support PR synchronization triggers, raise default diff limits, and hand off large diffs via an on-disk file reference instead of splitting diffs into multi-part prompt chunks.

Blocking issues

• scripts/agy-review.sh:L203-L207 and scripts/agy-review.sh:L304-L310: Silent truncation notice drop. The truncated string variable is assigned truncation warnings when diff size exceeds MAX_DIFF_BYTES or when the assembled prompt is capped at MAX_PROMPT_BYTES. However, $truncated is never appended to $prompt_file or $body_file, nor referenced anywhere else in the script. Truncated diffs or prompts will result in incomplete reviews without informing either agy or the PR author, violating the style guide mandate against silent failure paths.

Suggestions

• scripts/agy-review.sh:L394: In the unbuffer execution path, --print "$(cat "$prompt_file")" expands the entire prompt content into a command-line argument. This strips trailing newlines and subjects up to 125 KB of prompt text to Tcl argument parsing inside unbuffer. Consider passing $prompt_file as a file reference (similar to how script invokes _agy_print.sh) rather than expanding its raw content on argv.
• scripts/agy-review.sh:L48-L54: Validation of MAX_PROMPT_BYTES relies on [ "$MAX_PROMPT_BYTES" -gt 0 ] with 2>/dev/null to catch non-integer values. Prefer explicit integer pattern matching before numeric comparisons to avoid relying on suppressed shell test errors.

Nitpicks

• scripts/agy-review.sh:L321: The Python UTF-8 sanitizer line leaves the file handle open without an explicit context manager or close call.
• .github/workflows/antigravity-review.yml:L88: Commented default example for MAX_DIFF_BYTES duplicates the script default of 5000000; ensure inline comments match script fallback values.

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

[doublegate]

Adjudicating the re-review (on the hardened 409e7d97). The three real issues from the first pass are fixed; everything here is a re-raise of an adjudicated item or a nitpick — no further change.

• Blocking — "$truncated never appended to $body_file." This remains a false positive, now verifiable on the exact head under review: $truncated is consumed in the comment-body assembly —

  445  body_file="$(mktemp)"
  446  {
  447    printf '%s\n' "$MARKER"
  448    printf '## Antigravity review (Gemini via Ultra)\n\n'
  449    cat "$out_file"
  450    printf '%s' "$truncated"      # <-- appended to the posted comment
  451    printf '\n\n<sub>...</sub>\n'
  452  } > "$body_file"
  

  The assignment sites (~L203, ~L304) and the consumption (L450) are ~250 lines apart, which appears to be why the static pass doesn't connect them. Truncation notices do reach the posted PR comment, so there is no silent-failure path.

• Suggestion — pass $prompt_file to the unbuffer path instead of --print "$(cat …)". Can't be avoided: agy has no stdin/--prompt-file input for the prompt (--print takes the prompt as its VALUE), so the prompt is always an argv string. The script/PTY path's _agy_print.sh does the identical --print "$(cat "$prompt_file")" internally — routing the unbuffer path through it would relocate the $(cat), not remove it. Behaviourally identical; no correctness gap.

• Suggestion — explicit integer pattern-match for MAX_PROMPT_BYTES instead of -gt 0 + 2>/dev/null. Functionally correct and verified: a non-integer (abc) makes [ "$X" -gt 0 ] error, the && short-circuits, and the ! clamps it — 2>/dev/null only hides the "integer expression expected" stderr. Negative/zero/non-numeric/over-ceiling all clamp (tested). This is a stylistic preference over correct, tested behaviour.

• Nitpick — Python sanitizer leaves the file handle open. Harmless: it's a one-shot python3 -c subprocess that exits immediately, so the OS reclaims the descriptor at once; no leak accumulates.

• Nitpick — the workflow's commented MAX_DIFF_BYTES: "5000000" example duplicates the script default. Intentional — the commented overrides in the workflow document the script's actual defaults so a maintainer sees them without opening the script. They match by design.

CI is green and there are no unresolved review threads; proceeding.