release: v2.2.4 "Cartridge" — libretro core builds/installs for RetroArch#327
Merged
Conversation
Replaces RustyNES's bespoke `agy-review.sh` + workflow with the canonical
`Local_Only-Projects/antigravity-pr-review` template, so the three repos
that run this reviewer (RustyNES, RustySNES, RustyN64) stop diverging and a
future improvement to the template applies to all of them at once instead
of being re-invented per repo. After this change the two files are
byte-identical to the template.
WHY THIS WAS NEEDED. RustyNES's copy and the template had drifted into two
different solutions to the same problem. The template independently grew an
`AGY_DIFF_MODE=auto|inline|file` mechanism that hands a large diff to agy as
a single file under `.git/` (which git never lists in `status`, so it can't
show up as working-tree pollution) rather than truncating it. RustyNES,
without knowing the template had moved on, solved the same large-diff case
its own way -- splitting the diff into 150 KB parts under a gitignored work
dir. The template's approach is cleaner (one file, one read, no coverage
bookkeeping), so RustyNES adopts it.
THE ONE GENUINE GAP, NOW UPSTREAM. RustyNES's copy had exactly one thing the
template lacked and every installed repo needed: a fallback for GitHub's
20,000-line diff API limit. `gh pr diff` returns HTTP 406 for any diff over
that, and the template's `gh pr diff ... || exit 1` meant the review failed
outright on a large PR -- which is precisely the PR most worth reviewing (the
v2.2.3 mapper-rename PR was 67k lines). That fallback was ported into the
template and is part of this converged version: on a 406 (distinguished from
a genuine auth/network/bad-PR error, which stays fatal) it fetches the PR's
objects and the base branch into private `refs/agy/*`, takes `git merge-base`,
and computes the diff locally -- fetching objects but NEVER checking them out,
so the working tree stays on the default branch and a PR cannot rewrite its
own reviewer. The refs are removed on exit.
WHAT THE CONVERGED VERSION IS (union of the best of both):
* the template's `AGY_DIFF_MODE` file/inline diff handling;
* the 20,000-line API-limit local-git-diff fallback (from here);
* RustyNES's security hardening, folded UPSTREAM so all three inherit it:
the `isCrossRepository` fork gate in the script (a trusted commenter is
not a trusted diff, and the diff is what agy ingests under
--dangerously-skip-permissions), fail-closed metadata (a `gh pr view`
failure must never be indistinguishable from "same-repo"), the
`AGY_DRY_RUN` escape hatch for testing the acquisition + prompt-assembly
path without spending an agy run, the default-branch checkout (the
reviewer never runs PR-supplied code on the self-hosted runner),
`persist-credentials: false`, and `actions/checkout@v7`;
* the template's `synchronize` trigger (auto-re-review on every push),
which is safe under the default-branch checkout since the PR supplies
only the diff (data), never the reviewer code.
ONE RECONCILIATION TRAP FIXED IN THE STANDARD WORKFLOW. RustyNES's old
workflow set `MAX_DIFF_BYTES: "90000"` -- correct for the parts mechanism,
where that value was the inline budget. But the template script truncates
the diff to `MAX_DIFF_BYTES` BEFORE the inline/file decision, so carrying
that env onto the template script would have truncated every diff to 90 KB
and defeated file mode entirely. The standard workflow drops the override
(5 MB sanity-cap default) and documents why lowering it is wrong.
VERIFICATION. `bash -n` clean; `shellcheck -S warning` reports nothing beyond
the pre-existing SC1007/SC2218 in the template's config block; `actionlint`
clean against `.github/actionlint.yaml` (the `agy` self-hosted label is
declared there); the repo pre-commit hook passes on both files. The full
path -- fork gate -> 406 detection -> local `git diff` -> file-mode handoff --
was exercised end-to-end with `AGY_DRY_RUN=1` against this repo's own PR #325
(67,645 lines / 2.69 MB fetched via the fallback, handed off as a
`.git/agy-review-diff.*.patch` with a 2.6 KB prompt), and the `refs/agy/*`
and temp files were confirmed removed on exit.
No emulation-core, test, or build change -- reviewer tooling only.
…Arch
A libretro / RetroArch distribution cut. Its purpose is that the RustyNES
core builds and installs cleanly through the Libretro buildbot
(git.libretro.com/libretro/RustyNES) so RetroArch users can pull it from the
in-app Online Updater -> Core Downloader.
ZERO EMULATION-CORE CHANGES. The deterministic #![no_std] chip stack, the
save-state / TAS / netplay formats, and every golden vector are byte-identical
to v2.2.3, so AccuracyCoin holds 141/141 (100.00%) and nestest is 0-diff by
construction. This is a metadata + version + docs cut, not a code cut.
LIBRETRO AUDIT (the substantive work). crates/rustynes-libretro is a thin
wrapper over rustynes-core, so it inherits every recent change automatically
and needed no code change to carry them:
* the fast PPU dot path is the core default now, so the wrapper gets the
~11% rendering-heavy win for free;
* the PPU_SNAPSHOT_VERSION 8 / APU v4 save-state schema is transparent to
the wrapper, because get_serialize_size / on_serialize size and emit the
CURRENT snapshot via Nes::snapshot_core_into / VsDualSystem::snapshot, not
a hardcoded layout -- so RetroArch save states, run-ahead, and rollback
carry the v8 sprite-eval FSM + OAM data-bus state with no wrapper change
(and since libretro save states are session-local, the ADR-0028 epoch
never applies);
* the Mapper::mix_audio i32 widening, the Zapper model, and the mNNN_ mapper
rename are all below the crate's public dependency surface.
Both buildbot cross-ABIs the GitHub early-warning gate models build clean,
verified with the exact CI command:
cargo check --release -p rustynes-libretro --target x86_64-pc-windows-gnu
cargo check --release -p rustynes-libretro --target aarch64-linux-android
rustynes_libretro.info METADATA CORRECTED (the file RetroArch's core
downloader reads to learn the core's capabilities):
* disk_control "false" -> "true" -- THE REAL FIX. The FDS multi-side Disk
Control interface (enable_disk_control_interface() + the on_set_eject_state
/ on_get_image_index / on_get_num_images / on_replace_image_index callback
trampolines) has been wired since the buildbot recipe landed, but the .info
advertised it as absent, so RetroArch's Quick Menu -> Disk Control never
surfaced multi-disk FDS swapping. A multi-side FDS game is now swappable
from the RetroArch UI as intended.
* display_version "v1.0.0" -> "v2.2.4" (stale since the v1.0.0 era; the
runtime library_version the core reports has always tracked
CARGO_PKG_VERSION -- this is the static metadata string RetroArch shows).
* description mapper count 168 -> 172, plus a note that FDS multi-disk
swapping runs through the Disk Control interface.
Libretro core options (region / overscan / palette / accuracy toggles) remain
unexposed: core_options = "false" is accurate, not stale -- the CoreOptions
impl is deliberately empty. Documented as a future enhancement rather than a
v2.2.4 gap, since it is a new capability deserving its own focused work +
determinism/save-state testing rather than a rushed add in a distribution cut.
TOOLING. The Antigravity PR reviewer standardization onto the shared template
(scripts/agy-review.sh + .github/workflows/antigravity-review.yml, the prior
commit on this branch) rides along in this release -- the same canonical
version now installed across RustyNES / RustySNES / RustyN64.
VERSION + DOCS. Workspace 2.2.3 -> 2.2.4 (all 18 crates inherit); README badge
+ Current Release; CHANGELOG [2.2.4]; docs/STATUS.md current-release block;
AGENTS.md MC-PROJECT block + the never-claim-later guard (-> v2.2.4);
to-dos/ROADMAP.md re-synced from its stale v2.1.0 "current release" line to
v2.2.4 with a compact release-line bridge (full per-release detail stays in
CHANGELOG.md + docs/STATUS.md, the single source of truth). .gitignore needs
no change -- the libretro build artifacts (.so/.dll/.dylib/.a) are already
covered.
VERIFICATION. cargo fmt --all --check; clippy --workspace --all-targets
-D warnings (+ the retroachievements feature combo); no_std build
(thumbv7em-none-eabihf); markdownlint (pinned v0.39.0) on every touched
document + the release notes; the codename parses to "Cartridge" for the
release-auto title. AccuracyCoin 141/141 is inherited unchanged from v2.2.3.
Mobile versions stay frozen at their v2.0.x host-only release points; the
joint store launch remains v2.3.0.
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRustyNES v2.2.4 updates libretro metadata and release documentation without emulation-core changes. The Antigravity review workflow and script add synchronized PR triggers, bounded diff/prompt handling, fallback diff acquisition, safer execution, and improved failure diagnostics. Changesv2.2.4 release
Antigravity review execution
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 7 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…d on this PR
The Antigravity reviewer ran against this PR and flagged five security
regressions in `scripts/agy-review.sh` -- each a case where standardizing onto
the shared template quietly WEAKENED what RustyNES's prior (main) reviewer
already did. The reviewer catching a regression in its own standardization is
the system working as intended; all five are fixed, and the fixes are also
committed upstream in the canonical template so re-installs carry them.
1. Command injection in the `script(1)` PTY fallback (BLOCKING). The command
handed to `script -qfec` (run via `sh -c`) interpolated `${flags[*]}` raw;
the flag values come from env vars (AGY_MODEL / AGY_EFFORT /
AGY_PRINT_TIMEOUT), so a shell metacharacter in any would be evaluated by
the inner shell. Rebuilt with `printf '%q '` (main's proven form).
2. Arbitrary comment deletion (BLOCKING). The prior-comment cleanup selected
every comment CONTAINING the marker with no author filter, so any user
could put the marker (an HTML comment) in a comment and have the bot delete
arbitrary comments. Now scoped to
`.user.type == "Bot" and .user.login == "github-actions[bot]"`.
3. SQLite conversation-store data leak (BLOCKING). The empty-output fallback
read the most-recent `*.db` under `$CONV_DIR` by mtime with no session
scoping; on a shared runner that db can belong to an unrelated concurrent
local `agy` session, leaking its output into a public PR comment. Removed
entirely -- the PTY path + retry loop cover agy issue #76, and main never
had it. Dead `$CONV_DIR` dropped with it.
4. agy inherited the repo tokens (suggestion). agy runs under
--dangerously-skip-permissions and ingests an untrusted diff yet inherited
GH_TOKEN / GITHUB_TOKEN, which it never uses. Both invocations now run under
`env -u GH_TOKEN -u GITHUB_TOKEN`.
5. Missing issue_comment author re-check (suggestion). The workflow `if:`
gates `/agy-review` on OWNER/MEMBER/COLLABORATOR, but the script -- also
hand-runnable -- did not re-check it. The `author_association` gate is
restored in the script's `issue_comment` branch as defense in depth.
This is the same reconciliation error that produced the standardized template:
the earlier pass ported RustyNES's LARGE-DIFF features into the template but not
its SECURITY surface, and wrongly concluded RustyNES had nothing to add. main's
version was harder in these five places; the template (and the just-merged
RustySNES / RustyN64 standardizations) inherited the weaker forms. RustySNES and
RustyN64 need a follow-up re-sync from the now-hardened template.
Verified `bash -n` clean and `shellcheck -S warning` clean beyond the
pre-existing SC1007/SC2218 config-block findings; the CHANGELOG Tooling entry
records the hardening.
Owner
Author
Adjudication of the Antigravity review — all five fixed (
|
| Finding | Fix |
|---|---|
Blocking — script(1) command injection |
Rebuilt the fallback command with printf '%q ' so every token (including the env-derived flag values) is shell-escaped for the inner sh -c — main's proven form, replacing the raw ${flags[*]}. |
| Blocking — arbitrary comment deletion | Deletion is now author-scoped: .user.type == "Bot" and .user.login == "github-actions[bot]", so a user planting the marker in a comment can no longer trigger deletion of arbitrary comments. |
| Blocking — SQLite data leak | Removed the unscoped conversation-store fallback entirely. It read the newest *.db by mtime with no session scoping, so a concurrent local agy session's output could reach a public PR comment. The PTY path + retry loop cover agy issue #76 without it, and main never had it; the now-dead $CONV_DIR is dropped too. |
| Suggestion — token inheritance | Both agy invocations now run under env -u GH_TOKEN -u GITHUB_TOKEN, so agy (under --dangerously-skip-permissions, ingesting an untrusted diff) does not inherit the repo tokens it never uses. |
Suggestion — issue_comment author re-check |
Restored the author_association (OWNER/MEMBER/COLLABORATOR) gate in the script's issue_comment branch as defense in depth, so a hand-run or a lost workflow gate can't schedule agy for a stranger. |
The nitpick (workflow env comments aligned with script defaults) is noted; the workflow's optional-override block already documents the same defaults.
Verified bash -n clean, shellcheck -S warning clean beyond the pre-existing SC1007/SC2218. Follow-up: RustySNES and RustyN64 merged the pre-fix standardized reviewer and need a re-sync from the hardened template.
…f_err, UTF-8 fallback
The re-review of the hardened reviewer (from the first fix commit) flagged three
more items; all addressed, and mirrored upstream in the canonical template:
1. BLOCKING -- fixed /tmp log path. Standardizing dropped the run-id/PID suffix
(a 6th regression from main), so a fixed /tmp/agy-review.log collides
between concurrent jobs when RUNNER_TEMP is unset and is a predictable
symlink/tamper target. Restored LOG=...agy-review-${GITHUB_RUN_ID:-$$}.log
with the collision-avoidance comment (main's proven form).
2. diff_err is now allocated alongside diff_file / meta_file up front, so the
cleanup trap cannot reference it before it exists if the early metadata
fetch exits (it was pre-declared empty, so this was hardening, not a bug).
3. UTF-8 sanitize path gained a python3 fallback for hosts without iconv (a
template feature main never had, so not a regression -- agy's point is a
valid edge-case hardening): a no-iconv host could otherwise pass a
truncation-split multibyte sequence to agy, and Rust std::env::args()
panics on non-UTF-8. Both iconv and python3 absent is vanishingly rare on a
CI runner.
bash -n + shellcheck clean beyond the pre-existing SC1007/SC2218.
Owner
Author
Adjudication of the second Antigravity pass — all addressedThe re-review of the hardened script caught three more, all fixed (and mirrored in the canonical template):
The nitpicks are noted. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@CHANGELOG.md`:
- Line 17: Keep v2.2.4 marked as Unreleased rather than presenting it as the
current release until its tag exists, or update its date to the actual release
day. Apply this consistently in CHANGELOG.md at lines 17-17, docs/STATUS.md at
lines 3-4, and to-dos/ROADMAP.md at line 49.
In `@crates/rustynes-libretro/rustynes_libretro.info`:
- Line 38: Update the description value in rustynes_libretro.info to use an
accurate mapper count, replacing “over 172 mapper families” with either “172
mapper families” or “over 168 mapper families.”
In `@to-dos/ROADMAP.md`:
- Line 50: Reconcile the stale release-state guidance by updating or archiving
the later ROADMAP statements that identify v1.10.0 as the latest tag and v2.0.0
as pending, while preserving the authoritative v2.2.4 release narrative at
to-dos/ROADMAP.md:50-50. Remove the contradictory version-policy guidance at
AGENTS.md:188-188 so both Markdown files consistently reflect the current
release state.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f3435a4f-5e0b-4038-826f-d3dcb3393fb2
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!Cargo.lock
📒 Files selected for processing (10)
.github/release-notes/v2.2.4.md.github/workflows/antigravity-review.ymlAGENTS.mdCHANGELOG.mdCargo.tomlREADME.mdcrates/rustynes-libretro/rustynes_libretro.infodocs/STATUS.mdscripts/agy-review.shto-dos/ROADMAP.md
… count Addresses the CodeRabbit and agy review findings on PR #327. `rustynes_libretro.info`: the RetroArch core-downloader description said "over 172 mapper families", but 172 is the exact family count in the honesty-gated mapper matrix (`docs/STATUS.md`), not a lower bound. "over 172" reads as ">172", overstating coverage. Corrected to the precise "172 mapper families". `to-dos/ROADMAP.md`: four current-state narratives predated the v2.0.0 "Timebase" tag and still asserted v1.10.0 "Arcade" as the latest tagged release with v2.0.0 "code-complete on `main`, tag pending". v2.0.0 shipped 2026-07-03 and the release line has since advanced to v2.2.4 "Cartridge"; those statements now contradict the Status block at the top of the file and the release chain in AGENTS.md/CHANGELOG. Reconciled: - L40-41 blockquote: "code-complete on `main`, tag pending" -> "shipped as v2.0.0 on 2026-07-03", and the forward pointer relabelled to the "historical landing snapshot" section title. - L59 bullet: "Preceding additive/platform release" / "This is the latest in an unbroken ... chain" -> "Historical anchor - the last v1.x release" / "It closed an unbroken ... chain" (past tense; v1.10.0 is no longer latest). - L88 "Current state": v1.10.0-latest / v2.0.0-tag-pending -> "RustyNES v2.2.4 'Cartridge' is the latest tagged release ... v2.0.0 shipped 2026-07-03", with the retained v2.0.0 paragraph explicitly flagged as a historical snapshot rather than a current-state claim. - L90 section header: "code-complete, tag pending" -> "historical landing snapshot (shipped 2026-07-03; this section was written as it landed)". No behavioural change; documentation-only reconciliation so agents and readers receive one consistent version policy. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
….git/ Adopts the agy first-pass suggestion on this PR (scripts/agy-review.sh:888 in the diff): the on-disk large-diff handoff was written inside the hidden `.git/` directory (`.git/agy-review-diff.$$.patch`), which risks a file-read failure if the reviewer subagent or sandbox restricts tool access to `.git`. The `.git`-is-a- worktree-gitfile fallback wrote `.agy-review-diff.$$.patch` at the repo ROOT, where an untracked `.patch` pollutes `git status` if agy inspects working-tree state -- defeating the reason `.git/` was chosen in the first place. Write the diff into the dedicated, gitignored working-tree scratch dir instead, `.agy-review-work/agy-review-diff.$$.patch` -- which this repo's `.gitignore` has documented as the intended handoff location all along (lines 285-289), a comment the script never actually honoured until now. This: - removes the `.git/` tool-access dependency agy flagged; - collapses the two-branch `.git`-vs-root fork into one gitignored path, so the transient .patch is never working-tree pollution regardless of whether `.git` is a real dir or a worktree gitfile; - keeps per-run `$$` isolation, and the EXIT-trap cleanup now `rmdir`s the dir only when empty (concurrency-safe: a concurrent run's different-PID file is never clobbered; a SIGKILL leftover lands in a gitignored dir, not at root -- addressing the reviewer's SIGKILL nitpick). `.gitignore` already carries `/.agy-review-work/`, so no ignore change is needed here. This mirrors the canonical template (Local_Only-Projects/antigravity-pr-review) byte-for-byte in the diff-handoff block so the standardized reviewer does not diverge across projects. No change to diff content, prompt assembly, or the file-vs-inline decision. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
Adjudicating the
The canonical template ( |
Owner
Author
|
Adjudicating the latest The reviewer (the version on the default branch, which is what the workflow runs — it checks out
The Follow-up (tracked, not release-blocking): the on-disk handoff needs a fix validated against the live The actual content of this PR (the v2.2.4 release docs + libretro |
…d-dir The on-disk large-diff handoff (the `.agy-review-work/` file mode this PR introduced) never actually worked: for a diff over the inline budget the reviewer writes it to disk and tells agy to read it, but agy came back "the file does not exist on the filesystem" and produced an empty review — which is exactly what happened when this PR's own diff crossed the ~90 KB inline budget (below it the diff is inlined and the file path is never exercised, which is why every earlier head of #327 reviewed fine). Root cause, proven on the live `agy` runner (this host is the self-hosted runner) with a three-way probe under the exact review flags (`--sandbox --dangerously-skip-permissions`): 1) relative path `.agy-review-work/...`, no --add-dir -> FAIL ("does not exist") 2) same relative path + `--add-dir "$PWD"` -> PASS 3) absolute path, no --add-dir -> PASS agy's sandboxed file tool does NOT resolve a relative path against the shell's CWD — it resolves against agy's own workspace root, and under --sandbox the checkout is not in that workspace — so a CWD-relative handoff path is never found. Fix, both halves (each independently sufficient per the probe; a confirming run of the exact combined production-shaped invocation, an absolute path + `--add-dir "$PWD" --sandbox --dangerously-skip-permissions`, also passed reading a realistic ~400-line diff-shaped file with a token buried mid-file): - The prompt now hands agy the diff by ABSOLUTE path (`$agy_diff_file`) instead of the CWD-relative `$diff_name`. - `--add-dir "$PWD"` is appended to the flags, but ONLY in file-handoff mode (`use_file=1`). An inline review reads nothing from disk and keeps zero filesystem access, so the common path's prompt-injection surface is unchanged; only the large-diff path (which must read a file) gains workspace access to the checkout. Mirrors the canonical template (Local_Only-Projects/antigravity-pr-review), byte-identical in the handoff + flags regions, so the standardized reviewer stays in sync across all three repos. NOTE: the reviewer that runs on a PR is always the DEFAULT-BRANCH version (the workflow checks out `main`, never the PR head), so this fix does not change the review of #327 itself — it takes effect for every PR reviewed once #327 merges to main. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ooling Documents the reviewer handoff fix landed on this PR (absolute-path + --add-dir, proven on the live runner) in the CHANGELOG's [2.2.4] Tooling section, keeping the single source of truth for user-visible change in sync with the code (docs-as-spec). No behavioural change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
doublegate
added a commit
that referenced
this pull request
Jul 24, 2026
The Antigravity reviewer, run against #328, flagged real robustness gaps in the template it standardizes onto. Three fixed; the one "blocking" finding is a false positive (adjudicated on the PR). - **Negative/zero MAX_PROMPT_BYTES no longer bypasses the E2BIG guard.** The ceiling check was `[ "$MAX_PROMPT_BYTES" -le "$ARG_SIZE_CEILING" ]`, which a negative value satisfies — so the clamp was skipped and the later `head -c "$MAX_PROMPT_BYTES"` prompt cap ran as GNU `head -c -1` (print all-but-last-byte), leaving the prompt effectively uncapped and re-exposing the execve E2BIG failure the cap exists to prevent. Now requires a POSITIVE integer at or below the ceiling; negative, zero, and non-numeric all clamp to the ceiling. - **UTF-8 sanitize no longer risks wiping the prompt.** The iconv / python3 branches did `mv "$prompt_file.utf8" "$prompt_file"` on tool exit 0 without checking the result is non-empty; a sanitizer that exits 0 but emits zero bytes would blank the prompt and hand agy nothing. Both branches now gate the mv on `[ -s "$prompt_file.utf8" ]` and otherwise discard the temp and keep the original. - **Latent clamp-time crash fixed (found while hardening the clamp).** log() was defined AFTER the configuration block, but the MAX_PROMPT_BYTES clamp calls log() — so a clamp that fired died with `log: command not found` under set -e instead of warning and continuing. Moved log() / have_text() above the config block. This never bit in practice only because the default 125000 is valid and never triggers the clamp; the negative-value fix above makes the clamp reachable, so the ordering had to be correct. Not changed (adjudicated on #328): the "blocking" claim that $truncated is never surfaced — it is, at the comment-body assembly (`printf '%s' "$truncated"`); the unbuffer-vs-script prompt passing "inconsistency" — both ultimately pass the same `--print "$(cat "$prompt_file")"` argv (the script path via _agy_print.sh), so it is behaviourally identical; and the pre-truncated-handoff nitpick — MAX_DIFF_BYTES is the 5 MB pathological-diff sanity cap, documented as not-for-bounding-reviews. Mirrors the canonical template byte-identically. #327's copy is synced to main's reviewer when it rebases post-merge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Antigravity review (Gemini via Ultra)This PR bumps the workspace version to v2.2.4 ("Cartridge") and updates repository documentation, release notes, and libretro core metadata ( Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cuts v2.2.4 "Cartridge" — a libretro / RetroArch distribution release — and folds in the Antigravity reviewer standardization (2 commits).
Its purpose is that the RustyNES core builds and installs cleanly through the Libretro buildbot (git.libretro.com/libretro/RustyNES) so RetroArch users can pull it from the in-app Online Updater → Core Downloader.
Zero emulation-core changes
The deterministic
#![no_std]chip stack, save-state / TAS / netplay formats, and every golden vector are byte-identical to v2.2.3 — AccuracyCoin holds 141/141 (100.00%), nestest 0-diff, by construction. This is a metadata + version + docs cut.Libretro audit (the substantive work)
crates/rustynes-libretrowrapsrustynes-core, so it inherits every v2.2.3 change automatically and needed no code change:PPU_SNAPSHOT_VERSION8 / APU v4 schema is transparent (get_serialize_size/on_serializesize and emit the current snapshot viaNes::snapshot_core_into, not a fixed layout) — RetroArch save states / run-ahead / rollback carry the v8 state with no wrapper change;Mapper::mix_audioi32, the Zapper model, themNNN_rename are all below the public dependency surface.Both buildbot cross-ABIs the CI gate models build clean:
rustynes_libretro.infometadata correcteddisk_controlfalse→true— the real fix. The FDS multi-side Disk Control interface has been wired since the buildbot recipe landed, but the.infoadvertised it as absent, hiding multi-disk FDS swapping from RetroArch's Quick Menu.display_versionv1.0.0→v2.2.4; description mapper count168→172.core_options = "false"is accurate, not stale).Also in this PR
1a2e791a) onto the shared template — the canonical version now across RustyNES / RustySNES / RustyN64.Verification
fmt·clippy --workspace --all-targets -D warnings(+ retroachievements combo) ·no_std·markdownlint— all clean; AccuracyCoin 141/141 inherited unchanged.Release mechanics
On merge,
release-auto.ymltagsv2.2.4, publishes with.github/release-notes/v2.2.4.md, and builds the platform binaries. The buildbot then mirrors the tag and RustyNES appears in RetroArch's Core Downloader.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Compatibility