Skip to content

feat(mobile): add the App Store 4.7 self-audit, a release build path, and a UniFFI smoke test - #332

Merged
doublegate merged 4 commits into
mainfrom
feat/mobile-store-readiness
Aug 2, 2026
Merged

feat(mobile): add the App Store 4.7 self-audit, a release build path, and a UniFFI smoke test#332
doublegate merged 4 commits into
mainfrom
feat/mobile-store-readiness

Conversation

@doublegate

@doublegate doublegate commented Aug 2, 2026

Copy link
Copy Markdown
Owner

The v1.30.0 rung — mobile store-readiness engineering. The store submission itself is explicitly out of scope, and Mobile Phase 6 stays NOT GREENLIT.

The audit came first, and it found stale claims

Four entries in docs/mobile-readiness.md's "Not yet verified / explicitly deferred" list were no longer true: android.yml exists and gates 16 KB alignment twice (on the .sos and again inside the APK), android/gradlew and its wrapper are committed, and ios.yml already boots a simulator, installs the app, launches it and requires it to still be alive.

They are marked DONE in place rather than deleted — a readiness document that silently drops items cannot be audited backwards.

1. App Store §4.7 self-audit — the one store-facing item that isn't maintainer-blocked

docs/app-store-4-7-self-audit.md. PASS on all five criteria, with every finding citing the file and line it was read from so it can be re-run rather than trusted.

The strongest evidence is capability, not intent:

  • Android declares no permissions at all — no <uses-permission> element, so not even INTERNET. An app without INTERNET cannot download a ROM by any route.
  • iOS has no networking code and no ATS config. The only http string in the bundle is the DOCTYPE URL in Info.plist's XML preamble.

Every user-visible string in both shells was enumerated. The complete set is RustySNES, Open ROM, Save State, Load State. A search for nintendo, famicom, snes, super scope, multitap, game boy, mario, zelda returns nothing outside this project's own identifiers.

Two re-audit triggers are recorded rather than left implicit:

  1. The peripheral UI, when it lands — Super Scope / Mouse / Multitap will need user-visible names, and that is a fresh trademark decision this audit does not pre-approve. They are absent today only because the picker UI doesn't exist yet.
  2. rustysnes-monetization, if activated — it is inert today (no BillingClient, no StoreKit, no purchase call reaches either shell), so §3.1 and the ad-disclosure rules do not engage. Activating it changes that immediately.

2. An unsigned assembleRelease path

Signing material is the maintainer's to provision, so a signed release build stays out of reach. An unsigned one is not — and it still runs R8, resource shrinking and the release manifest merge, which is where release-only breakage lives (a missing keep rule stripping a class the UniFFI bindings reach reflectively; the debug build never notices because it doesn't minify).

isMinifyEnabled is false today, so at this revision it proves the release variant assembles. It is wired now because the moment minification is enabled, this step is what catches the fallout. The release APK gets its own 16 KB gate — not redundant, because the release variant has its own packaging and shrinking, so alignment has to be proven on the artifact that would actually ship.

3. An instrumented UniFFI smoke test

assembleDebug already proves the bindings compileMainActivity calls MobileCore directly, so drifted bindgen output fails the Kotlin compile. What no build can prove is that System.loadLibrary finds the .so for the device's ABI, that JNA's mapping matches its symbols, and that a call marshals across and returns. This project has already shipped one native Android crash a build could not have caught.

Four assertions, all about the bridge rather than emulation: the library loads and a core constructs; a frame runs and returns a framebuffer of exactly width * height * 4; drainAudio is non-destructive within a frame (pinning the documented contract the shell relies on); reset and power-cycle cross safely.

Deliberately ROM-free — the app takes ROMs only from the user's document picker, which is criterion 2 of the audit above.

It is a separate CI job. An emulator is the flakiest thing in that workflow, and a flaky step inside build would put the 16 KB alignment gates — which are not flaky, and which gate a real Play requirement — behind an AVD boot.

Verification

  • .github/workflows/android.yml parses; jobs are build (17 steps) and smoke (8).
  • Every pinned action SHA was resolved against the GitHub API, not assumed. Worth stating: I checked reactivecircus/android-emulator-runner's tag ref, got the annotated tag object's SHA, and briefly used that — which is not a commit and would have failed the job. Dereferenced to the real commit, then moved the pin to the current v2.38.0.
  • The smoke job reuses the build job's proven NDK discovery verbatim (sdkmanager is not on PATH on ubuntu-latest even though $ANDROID_HOME is set — that job found it the hard way). Two jobs building the same libraries with different NDKs would also make a divergence between them unattributable.

The Android CI additions cannot be verified locally — this environment has no Android SDK — so their first real run is on this PR, which is how android.yml's own 16 KB gate was developed (it failed on its first run, and the 4 KB libraries were real).

🤖 Generated with Claude Code

Summary

This change claims that mobile v1.30.0 is ready for store-readiness review, but not greenlit for Mobile Phase 6.

The claim depends on:

  • The App Store §4.7 audit remaining accurate.
  • Unsigned assembleRelease producing 64-bit libraries with valid 16 KB alignment.
  • The UniFFI smoke test loading AccuracySNES HiROM, producing frames and nonempty interleaved stereo audio, and passing reset and power-cycle checks.

The claim is false if an audit assertion is outdated, the release APK fails the alignment gate, or the emulator test fails for native loading, frame output, audio output, reset, or power-cycle behavior. Distribution signing, TestFlight, Play Data Safety, and store submission remain unverified.

The change adds no AccuracySNES dossier assertions and does not move the coverage denominator. It adds no emulator-core behavior change. The observable change is CI validation of existing mobile-core behavior with a real AccuracySNES test ROM instead of ROM-free audio assertions.

…oke test

The v1.30.0 rung. An audit first found that four entries in
docs/mobile-readiness.md's "not yet verified" list had gone stale -- android.yml,
the ./gradlew wrapper, the iOS simulator RUN, and much of the CI the plan asked
for all exist. They are now marked DONE in place rather than deleted: a
readiness document that silently drops items cannot be audited backwards.

Three real gaps closed.

The App Store 4.7 self-audit (docs/app-store-4-7-self-audit.md) is the one
store-facing item that is not maintainer-blocked, and it had never been done. It
passes on all five criteria, and the strongest evidence is capability rather
than intent: Android declares NO permissions at all, not even INTERNET, and iOS
has no networking code, so neither shell CAN obtain game software. Every
user-visible string in both shells was enumerated -- the complete set is
RustySNES, Open ROM, Save State, Load State. Two re-audit triggers are recorded:
the peripheral UI when it lands, since Super Scope / Mouse / Multitap names are
a fresh trademark decision the audit does not pre-approve, and
rustysnes-monetization if it is ever activated.

An unsigned assembleRelease path with its own 16 KB gate on the release APK.
Signing material is the maintainer's to provision, so a signed release build
stays out of reach; an unsigned one still runs R8, resource shrinking and the
release manifest merge, which is where release-only breakage lives.

An instrumented UniFFI smoke test, in its own CI job. assembleDebug already
proves the bindings COMPILE, because MainActivity calls MobileCore directly.
What no build can prove is that System.loadLibrary finds the .so for the
device's ABI, that JNA's mapping matches its symbols, and that a call marshals
across and returns -- this project has already shipped one native Android crash
a build could not have caught. Separate job, because an emulator is the flakiest
thing in that workflow and a flaky step inside `build` would put the 16 KB
gates behind an AVD boot.

Mobile Phase 6 stays NOT GREENLIT. Passing the audit removes a prerequisite from
that gate's checklist; it does not move the gate. Distribution signing,
TestFlight and Play's Data Safety form remain maintainer-blocked.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 2, 2026 17:11
@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4d04e9cc-753d-4902-9371-5f176cff088d

📥 Commits

Reviewing files that changed from the base of the PR and between ec27cbf and f05e357.

📒 Files selected for processing (3)
  • .gitignore
  • android/app/build.gradle.kts
  • android/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.kt
📜 Recent review details
⏰ Context from checks skipped due to timeout. (6)
  • GitHub Check: test-light
  • GitHub Check: lint
  • GitHub Check: accuracysnes
  • GitHub Check: build
  • GitHub Check: smoke
  • GitHub Check: build demo + docs
🧰 Additional context used
📓 Path-based instructions (2)
**/*

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*: Do not commit or vendor the generated snesdev_wiki/ mirror; it is gitignored and intended only as a local reference.
Keep commits focused and use Conventional Commits: <type>(<scope>): <subject>, with an imperative subject of at most 72 characters.
Do not use emojis in code, comments, or commit messages.
Before opening a PR, ensure formatting, Clippy, workspace tests, the core embedded build, rustdoc with warnings denied, documentation coverage, and changelog requirements pass.
Ticket completion must be reflected in the relevant to-dos/ sprint file.

**/*: Preserve the one-directional crate graph: chip crates must not depend on one another; rustysnes-core ties them together.
Never commit commercial ROMs; only commit derived screenshots and hashes.
Keep docs/STATUS.md as the authoritative per-subsystem status and update project documentation in the same PR as code changes.
Do not treat RustyNES v2.0 or engine-lineage anchors as project releases.

Files:

  • android/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.kt
  • android/app/build.gradle.kts
android/**/*.kt

⚙️ CodeRabbit configuration file

android/**/*.kt: Kotlin/Compose front end over the UniFFI bridge. Flag recomposition footguns (unstable
lambdas or parameters causing needless recomposition) and any direct file or preferences
access that bypasses the bridge.

Files:

  • android/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.kt
🔇 Additional comments (4)
android/app/build.gradle.kts (2)

21-21: LGTM!

Also applies to: 54-58


147-155: 🩺 Stability & Availability

Keep the current dependency. tests/roms/AccuracySNES/build/accuracysnes-hirom.sfc is tracked and not ignored, so a clean checkout provides the input.

			> Likely an incorrect or invalid review comment.
.gitignore (1)

278-280: LGTM!

android/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.kt (1)

4-106: LGTM!


Walkthrough

The PR adds Android release alignment checks, x86_64 emulator instrumentation tests for MobileCore, mobile readiness audit records, and salvage manifest entries. It configures the Android test runner, test assets, dependencies, and CI smoke-test environment.

Changes

Android mobile readiness

Layer / File(s) Summary
Release APK alignment validation
.github/workflows/android.yml
The workflow assembles an unsigned release APK and validates 64-bit native libraries for 16 KB-or-larger ELF alignment.
Instrumented MobileCore smoke testing
android/app/build.gradle.kts, android/app/src/androidTest/kotlin/..., .github/workflows/android.yml, .gitignore
The Android project configures instrumentation support, packages the AccuracySNES HiROM test ROM, and tests native construction, framebuffer sizing, audio output, reset, and power-cycle behavior on an x86_64 emulator.
Mobile audit and readiness records
docs/app-store-4-7-self-audit.md, docs/mobile-readiness.md, CHANGELOG.md
The documents record the App Store §4.7 audit, completed readiness checks, outstanding distribution work, and the v1.30.0 changelog entry.

Salvage manifest records

Layer / File(s) Summary
Recovered file manifest entries
docs/SALVAGE_MANIFEST.md
The manifest records 40 recovered scripts and five recovered documentation files, including exclusions and source mappings.

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

Sequence Diagram(s)

sequenceDiagram
  participant smoke_job
  participant Android_x86_64_emulator
  participant Gradle_connected_debug_tests
  participant MobileCoreSmokeTest
  participant MobileCore
  smoke_job->>Android_x86_64_emulator: Start API 34 emulator with KVM
  smoke_job->>Gradle_connected_debug_tests: Run connected UniFFI debug tests
  Gradle_connected_debug_tests->>MobileCoreSmokeTest: Execute instrumentation tests
  MobileCoreSmokeTest->>MobileCore: Construct and exercise native core
  MobileCore-->>MobileCoreSmokeTest: Return frame, audio, reset, and power-cycle results
Loading

Possibly related PRs

  • doublegate/RustySNES#289: Both PRs modify Android workflow artifact builds and enforce 16 KB ELF alignment for native libraries.
🚥 Pre-merge checks | ✅ 10
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title uses valid Conventional Commit syntax, an allowed type and scope, imperative mood, and accurately describes the mobile readiness changes.
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.
Changelog Entry ✅ Passed The full origin/main...HEAD diff contains a CHANGELOG.md hunk adding the v1.30.0 mobile store-readiness entry.
Docs-As-Spec ✅ Passed Against origin/main, the PR changes only Android, workflow, changelog, and docs paths; git diff shows no changes under crates/, so no crate behavior or matching spec update is required.
Accuracysnes Bookkeeping ✅ Passed Against base 368cc9f, the PR changes eight paths and none is under tests/roms/AccuracySNES/gen/src/, so the bookkeeping conditions are not triggered.
No Panic On Untrusted Input ✅ Passed The complete PR adds no .unwrap(), .expect(), or panic!(); its packaged ROM reaches loadRom, which returns typed MobileError for invalid input.
Safety Comment On New Unsafe ✅ Passed The PR changes no Rust files, and the complete base-to-HEAD diff adds no line containing unsafe; therefore no new unsafe block or unsafe fn lacks a SAFETY comment.

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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR is part of the v1.30.0 “mobile store-readiness engineering” rung. It adds an App Store §4.7 self-audit document, extends Android CI to exercise an unsigned assembleRelease build path (plus a release-APK alignment gate), and introduces an instrumented UniFFI runtime smoke test to validate the Kotlin↔native bridge on an actual Android runtime.

Changes:

  • Add docs/app-store-4-7-self-audit.md and mark previously “deferred” readiness items as DONE in docs/mobile-readiness.md.
  • Add an Android instrumented test (connectedDebugAndroidTest) that validates UniFFI library load + basic FFI calls.
  • Extend .github/workflows/android.yml with an unsigned assembleRelease step and a release-APK 16KB alignment gate.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
docs/mobile-readiness.md Updates the “Not yet verified / explicitly deferred” list to reflect newly completed mobile readiness checks and documents remaining maintainer-blocked items.
docs/app-store-4-7-self-audit.md Adds a documented, file/line-cited App Store §4.7 self-audit for the current mobile shells.
CHANGELOG.md Records the v1.30.0 mobile readiness additions and the readiness-doc corrections.
android/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.kt Adds an instrumented UniFFI runtime smoke test covering library load and basic core lifecycle/IO calls.
android/app/build.gradle.kts Wires up the Android instrumentation runner and adds AndroidX test dependencies/source set for androidTest.
.github/workflows/android.yml Adds an unsigned assembleRelease build path, a release-APK alignment check, and a separate emulator-based smoke-test job.

Comment on lines +279 to +294
run: |
set -euo pipefail
apk=$(find android/app/build/outputs/apk/release -name '*.apk' | head -1)
test -n "$apk" || { echo "no release APK was produced"; exit 1; }
work=$(mktemp -d)
unzip -q "$apk" 'lib/*' -d "$work"
fail=0
for so in "$work"/lib/arm64-v8a/*.so "$work"/lib/x86_64/*.so; do
[ -e "$so" ] || continue
align=$(readelf -lW "$so" | awk '$1 == "LOAD" { print $NF; exit }')
case "$align" in
0x4000|0x10000) echo "OK $(basename "$so") $align" ;;
*) echo "FAIL $(basename "$so") $align (need 16 KB or larger)"; fail=1 ;;
esac
done
exit "$fail"
Comment on lines +99 to +102
```text
grep -rn 'BillingClient|StoreKit|purchase|SKProduct' android/app/src/main/kotlin ios/RustySNES/Sources
-> nothing
```

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🤖 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 @.github/workflows/android.yml:
- Around line 369-379: Set an explicit timeout-minutes value on the
android-emulator-runner step named “Run the instrumented UniFFI smoke test” so a
stalled AVD boot or emulator test fails within minutes instead of using the
default runner limit.
- Around line 278-294: Update the release APK alignment gate to reuse the
existing aligned16k helper and presence-check pattern from the debug/jniLibs
gates. In the release assertion block, validate every LOAD segment rather than
only the first, accept alignments meeting the 16 KB divisibility requirement
instead of exact string matches, and fail when no native libraries are checked,
while preserving the existing APK discovery and failure reporting.
- Around line 333-351: Extract the duplicated NDK and cargo-ndk installation
steps from both Android workflow jobs into a new composite action at
.github/actions/android-ndk-setup/action.yml, keeping the pinned NDK version and
setup behavior centralized there. Replace both inline copies in android.yml with
uses of this composite action, preserving the existing job behavior and ensuring
both jobs share one version source.
- Around line 318-323: Update the smoke job in the Android workflow to include a
descriptive name, then add smoke to the ci-success job’s needs list so smoke
failures block the aggregate required check.

In
`@android/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.kt`:
- Around line 59-71: Update drain_audio_is_non_destructive_within_a_frame to
initialize a deterministic non-zero audio fixture before calling drainAudio,
then assert the intended behavior based on the native sink.append operation: the
first call returns samples and consumes the buffer, so the second call returns
zero. Keep the test focused on validating drainAudio with actual buffered audio
rather than an empty no-ROM frame.

In `@docs/app-store-4-7-self-audit.md`:
- Around line 3-4: Update the audit metadata to reference commit
1b6a69eac33624ff42ac0a125786c32cd6480040 instead of main at the A6.15 merge
(`#331`), and explicitly identify whether that pinned commit is the PR head or
base.
- Around line 16-24: Expand the §4.7 audit beyond the five local checks by
recording applicability and supporting evidence for requirements §4.7.1–§4.7.5,
including privacy/content controls, native API restrictions, per-software
consent, software indexing with universal links, and age
identification/restriction. If those checks remain out of scope, rename the
audit as limited and explicitly list the exclusions; then align the
corresponding scope statements in CHANGELOG.md and docs/mobile-readiness.md.
- Around line 39-64: Narrow Criterion 3 to state that neither shell initiates
direct network requests, rather than claiming it cannot fetch software by any
route. Update the surrounding Android and iOS evidence to describe the absence
of app-declared networking capabilities and APIs without asserting that picker
providers cannot retrieve cloud-backed content.
- Around line 26-37: Update the Criterion 1 section to make its evidence
exhaustive: add inventories covering source-tree files and packaged Android/iOS
bundles, including firmware.bin, extensionless files, and other resources, while
acknowledging that no bundles are available in this checkout; alternatively,
narrow the PASS claim to only the explicitly checked paths. Keep the claim
aligned with the evidence presented.

In `@docs/mobile-readiness.md`:
- Around line 217-221: Update the later Phase 6 checklist to remove the
completed android.yml and ./gradlew work items, and revise the §4.7 audit entry
to remove the obsolete “third has findings” wording. Keep the document’s
remaining outstanding items accurate and consistent with the completed statuses
described above.
- Around line 241-246: Update the release-build coverage wording in
docs/mobile-readiness.md lines 241-246 and CHANGELOG.md lines 27-32 to describe
assembleRelease as covering release assembly and packaging only; remove claims
that it currently exercises R8 or resource shrinking, and state that those
checks remain future coverage while release.isMinifyEnabled and resource
shrinking are disabled.
🪄 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: fe5fa7d2-93e8-4744-96d9-ce93c3796109

📥 Commits

Reviewing files that changed from the base of the PR and between 368cc9f and 1b6a69e.

📒 Files selected for processing (6)
  • .github/workflows/android.yml
  • CHANGELOG.md
  • android/app/build.gradle.kts
  • android/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.kt
  • docs/app-store-4-7-self-audit.md
  • docs/mobile-readiness.md
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
  • GitHub Check: accuracysnes
  • GitHub Check: lint
  • GitHub Check: test-light
  • GitHub Check: copilot-pull-request-reviewer
  • GitHub Check: build demo + docs
  • GitHub Check: build
  • GitHub Check: smoke
  • GitHub Check: review
🧰 Additional context used
📓 Path-based instructions (9)
**/*

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*: Do not commit or vendor the generated snesdev_wiki/ mirror; it is gitignored and intended only as a local reference.
Keep commits focused and use Conventional Commits: <type>(<scope>): <subject>, with an imperative subject of at most 72 characters.
Do not use emojis in code, comments, or commit messages.
Before opening a PR, ensure formatting, Clippy, workspace tests, the core embedded build, rustdoc with warnings denied, documentation coverage, and changelog requirements pass.
Ticket completion must be reflected in the relevant to-dos/ sprint file.

**/*: Preserve the one-directional crate graph: chip crates must not depend on one another; rustysnes-core ties them together.
Never commit commercial ROMs; only commit derived screenshots and hashes.
Keep docs/STATUS.md as the authoritative per-subsystem status and update project documentation in the same PR as code changes.
Do not treat RustyNES v2.0 or engine-lineage anchors as project releases.

Files:

  • android/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.kt
  • docs/app-store-4-7-self-audit.md
  • android/app/build.gradle.kts
  • CHANGELOG.md
  • docs/mobile-readiness.md
android/**/*.kt

⚙️ CodeRabbit configuration file

android/**/*.kt: Kotlin/Compose front end over the UniFFI bridge. Flag recomposition footguns (unstable
lambdas or parameters causing needless recomposition) and any direct file or preferences
access that bypasses the bridge.

Files:

  • android/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.kt
docs/**/*.md

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Before changing a subsystem, consult docs/architecture.md, docs/STATUS.md, CONTRIBUTING.md, the relevant subsystem documentation, and applicable ADRs.

New subsystems must add documentation under docs/.

Files:

  • docs/app-store-4-7-self-audit.md
  • docs/mobile-readiness.md
**/*.{rs,md}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Chip-behavior changes must update both the chip implementation and the corresponding docs/<subsystem>.md documentation.

A chip change must update both the chip implementation and its corresponding docs/<chip>.md documentation in the same change.

Files:

  • docs/app-store-4-7-self-audit.md
  • CHANGELOG.md
  • docs/mobile-readiness.md
docs/**/*

📄 CodeRabbit inference engine (docs/testing-strategy.md)

Chip crates should exceed 90% unit-test coverage, and each chip should be fuzzable in isolation.

Files:

  • docs/app-store-4-7-self-audit.md
  • docs/mobile-readiness.md
docs/**

⚙️ CodeRabbit configuration file

docs/**: Docs are the spec, not a history log. Flag claims that contradict the code, counts that
contradict the generated docs/accuracysnes-coverage.md, and any statement of coverage that
is broader than what the corresponding test actually asserts.

Files:

  • docs/app-store-4-7-self-audit.md
  • docs/mobile-readiness.md
**/*.md

⚙️ CodeRabbit configuration file

**/*.md: Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes
rather than style nits. The markdownlint gate is pinned to v0.39.0 via pre-commit —
do not report rules that version does not have (MD060 in particular).

Files:

  • docs/app-store-4-7-self-audit.md
  • CHANGELOG.md
  • docs/mobile-readiness.md
.github/workflows/**

⚙️ CodeRabbit configuration file

.github/workflows/**: Flag third-party actions pinned to a branch rather than a tag or commit SHA, jobs without
a least-privilege permissions: block, and secrets that could reach the log. The
ci-success job is the required check and must depend on every gate it claims to
aggregate — a new job that is not in its needs: list is a gate nothing enforces.

Files:

  • .github/workflows/android.yml
CHANGELOG.md

📄 CodeRabbit inference engine (CONTRIBUTING.md)

User-visible changes must be recorded under the [Unreleased] section.

For the full pull request diff against its base branch, modify CHANGELOG.md when user-visible behavior changes, including emulator output, frontend features, CLI flags, public APIs, or AccuracySNES cartridge contents. Do not require it for purely internal changes, tests, comments, or CI configuration.

Files:

  • CHANGELOG.md
🪛 LanguageTool
docs/mobile-readiness.md

[style] ~242-~242: ‘out of reach’ might be wordy. Consider a shorter alternative.
Context: ...n, so a signed release build is still out of reach; an unsigned one is not, and it runs ...

(EN_WORDINESS_PREMIUM_OUT_OF_REACH)


[uncategorized] ~247-~247: The official name of this software platform is spelled with a capital “H”.
Context: ...a simulator, not just a build — DONE.** .github/workflows/ios.yml picks an available...

(GITHUB)


[locale-violation] ~249-~249: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ... it and requires it to still be alive afterwards. That closes the "no ROM has ever actua...

(AFTERWARDS_US)

🪛 zizmor (1.28.0)
.github/workflows/android.yml

[info] 318-318: workflow or action definition without a name (anonymous-definition): this job

(anonymous-definition)

🔇 Additional comments (6)
.github/workflows/android.yml (1)

258-273: LGTM!

Also applies to: 296-302

android/app/build.gradle.kts (2)

21-21: LGTM!

Also applies to: 140-145


51-56: 🎯 Functional Correctness

Keep the existing Kotlin source-set DSL.

AGP 8.7.3 with org.jetbrains.kotlin.android 2.0.20 supports AndroidSourceSet.kotlin.srcDirs(...). The kotlin.directories syntax applies to AGP 9's built-in Kotlin model.

			> Likely an incorrect or invalid review comment.
android/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.kt (1)

1-57: LGTM!

Also applies to: 74-82

CHANGELOG.md (2)

34-49: LGTM!


14-15: 📐 Maintainability & Code Quality

Keep the changelog entry under [Unreleased]. The v1.30.0 entry is already correctly placed; no change is needed.

			> Likely an incorrect or invalid review comment.

Comment thread .github/workflows/android.yml
Comment on lines +318 to +323
smoke:
runs-on: ubuntu-latest
env:
CARGO_NET_RETRY: "10"
CARGO_TERM_COLOR: always
steps:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check ci-success aggregation and permissions blocks for the android workflow.
set -euo pipefail

echo "=== android.yml: top-of-file permissions and job list ==="
sed -n '1,40p' .github/workflows/android.yml

echo
echo "=== every job defined in android.yml ==="
rg -nP '^  [a-zA-Z0-9_-]+:\s*$' .github/workflows/android.yml

echo
echo "=== any ci-success aggregator across all workflows ==="
fd -e yml -e yaml . .github/workflows --exec rg -n -A 20 'ci-success' {} \;

echo
echo "=== permissions blocks per workflow ==="
fd -e yml -e yaml . .github/workflows --exec sh -c 'echo "--- $1"; rg -n -A 3 "permissions:" "$1" || echo "(none)"' _ {} \;

Repository: doublegate/RustySNES

Length of output: 2084


🏁 Script executed:

#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re

path = Path(".github/workflows/android.yml")
text = path.read_text()

top_permissions = re.search(
    r"^permissions:\s*\n(?P<body>(?:^[ \t]+.*\n?)*)",
    text,
    re.MULTILINE,
)
print("workflow_permissions:")
print(top_permissions.group("body").rstrip() if top_permissions else "(none)")

jobs = {}
job_matches = list(re.finditer(r"^  ([A-Za-z0-9_-]+):\s*$", text, re.MULTILINE))
for i, match in enumerate(job_matches):
    name = match.group(1)
    start = match.end()
    end = job_matches[i + 1].start() if i + 1 < len(job_matches) else len(text)
    block = text[start:end]
    if name in {"smoke", "ci-success"}:
        needs = re.search(r"^    needs:\s*(.+)$", block, re.MULTILINE)
        job_name = re.search(r"^    name:\s*(.+)$", block, re.MULTILINE)
        job_permissions = re.search(r"^    permissions:\s*$([\s\S]*?)(?=^    [A-Za-z_-]+:|\Z)", block, re.MULTILINE)
        jobs[name] = {
            "needs": needs.group(1).strip() if needs else "(none)",
            "name": job_name.group(1).strip() if job_name else "(none)",
            "permissions": job_permissions.group(0).strip() if job_permissions else "(none)",
        }

print("selected_jobs:")
for name, data in jobs.items():
    print(name, data)

ci = jobs.get("ci-success", {})
needs_text = ci.get("needs", "")
print("smoke_in_ci_success_needs:", bool(re.search(r"\bsmoke\b", needs_text)))
PY

Repository: doublegate/RustySNES

Length of output: 314


Add smoke to ci-success.needs. The workflow grants contents: read, but ci-success omits smoke; a failing smoke test can merge without failing the required check. Add a name to smoke for readable check output.

🧰 Tools
🪛 zizmor (1.28.0)

[info] 318-318: workflow or action definition without a name (anonymous-definition): this job

(anonymous-definition)

🤖 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 @.github/workflows/android.yml around lines 318 - 323, Update the smoke job
in the Android workflow to include a descriptive name, then add smoke to the
ci-success job’s needs list so smoke failures block the aggregate required
check.

Sources: Path instructions, Linters/SAST tools

Comment on lines +333 to +351
# The same NDK and the same discovery as the `build` job, deliberately: `sdkmanager` is NOT
# on PATH on `ubuntu-latest` even though `$ANDROID_HOME` is set, which that job found the
# hard way on its first run. Two jobs building the same libraries with different NDKs would
# also make a divergence between them impossible to attribute.
- name: Install the NDK from the runner's Android SDK
run: |
set -euo pipefail
: "${ANDROID_HOME:?the runner image is expected to provide an Android SDK}"
sdk="$ANDROID_HOME/cmdline-tools/latest/bin/sdkmanager"
if [ ! -x "$sdk" ]; then
echo "::error::no sdkmanager at $sdk"
ls -la "$ANDROID_HOME/cmdline-tools" || true
exit 1
fi
"$sdk" --install "ndk;27.2.12479018"
echo "ANDROID_NDK_HOME=$ANDROID_HOME/ndk/27.2.12479018" >> "$GITHUB_ENV"

- name: Install cargo-ndk
run: cargo install cargo-ndk --locked --version ^3

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the NDK setup into a composite action to enforce the invariant the comment states.

Lines 337-351 duplicate lines 70-86 verbatim, including the NDK version 27.2.12479018. The comment at lines 333-336 states that the two jobs must build with the same NDK and that a divergence would be impossible to attribute. A copied literal is the mechanism most likely to produce exactly that divergence: a version bump applied to one job and not the other passes review and CI.

The repository already uses this pattern with ./.github/actions/rust-setup. Move the NDK install and the cargo-ndk install into a single composite action and call it from both jobs. The version then exists in one place.

Proposed structure

New file .github/actions/android-ndk-setup/action.yml:

name: Android NDK setup
description: Installs the pinned NDK from the runner's Android SDK and cargo-ndk.
runs:
  using: composite
  steps:
    - name: Install the NDK from the runner's Android SDK
      shell: bash
      run: |
        set -euo pipefail
        # `sdkmanager` is NOT on PATH on `ubuntu-latest` even though `$ANDROID_HOME` is set.
        : "${ANDROID_HOME:?the runner image is expected to provide an Android SDK}"
        sdk="$ANDROID_HOME/cmdline-tools/latest/bin/sdkmanager"
        if [ ! -x "$sdk" ]; then
          echo "::error::no sdkmanager at $sdk"
          ls -la "$ANDROID_HOME/cmdline-tools" || true
          exit 1
        fi
        "$sdk" --install "ndk;27.2.12479018"
        echo "ANDROID_NDK_HOME=$ANDROID_HOME/ndk/27.2.12479018" >> "$GITHUB_ENV"

    - name: Install cargo-ndk
      shell: bash
      run: cargo install cargo-ndk --locked --version ^3

Then in .github/workflows/android.yml, replace both copies:

-      # The same NDK and the same discovery as the `build` job, deliberately: `sdkmanager` is NOT
-      # on PATH on `ubuntu-latest` even though `$ANDROID_HOME` is set, which that job found the
-      # hard way on its first run. Two jobs building the same libraries with different NDKs would
-      # also make a divergence between them impossible to attribute.
-      - name: Install the NDK from the runner's Android SDK
-        run: |
-          set -euo pipefail
-          : "${ANDROID_HOME:?the runner image is expected to provide an Android SDK}"
-          sdk="$ANDROID_HOME/cmdline-tools/latest/bin/sdkmanager"
-          if [ ! -x "$sdk" ]; then
-            echo "::error::no sdkmanager at $sdk"
-            ls -la "$ANDROID_HOME/cmdline-tools" || true
-            exit 1
-          fi
-          "$sdk" --install "ndk;27.2.12479018"
-          echo "ANDROID_NDK_HOME=$ANDROID_HOME/ndk/27.2.12479018" >> "$GITHUB_ENV"
-
-      - name: Install cargo-ndk
-        run: cargo install cargo-ndk --locked --version ^3
+      # Shared with the `build` job so both jobs cannot drift onto different NDKs.
+      - uses: ./.github/actions/android-ndk-setup
🤖 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 @.github/workflows/android.yml around lines 333 - 351, Extract the duplicated
NDK and cargo-ndk installation steps from both Android workflow jobs into a new
composite action at .github/actions/android-ndk-setup/action.yml, keeping the
pinned NDK version and setup behavior centralized there. Replace both inline
copies in android.yml with uses of this composite action, preserving the
existing job behavior and ensuring both jobs share one version source.

Comment thread .github/workflows/android.yml
Comment on lines +16 to +24
App Store Review Guideline **§4.7 (Mini apps, mini games, streaming games, chatbots, plug-ins and
game emulators)** permits retro game console emulator apps, subject to the software offered inside
them being lawful and, in practice, user-provided. Google Play's equivalent position is narrower in
form but the same in substance for this app: the emulator itself is not the problem, the content
supply is.

Separately from §4.7, **trademark exposure** is its own App Review and legal concern — an emulator
that names or depicts a console manufacturer's marks invites a rejection that has nothing to do with
§4.7. Both are audited here because both are decided by the same shipped strings.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

printf '%s\n' '--- candidate files ---'
git ls-files 'docs/app-store-4-7-self-audit.md' 'docs/architecture.md' 'docs/STATUS.md' 'CONTRIBUTING.md' 'CHANGELOG.md' 'docs/**/*.md' | sed -n '1,120p'
printf '%s\n' '--- audit outline and contents ---'
if command -v ast-grep >/dev/null 2>&1; then ast-grep outline docs/app-store-4-7-self-audit.md; fi
wc -l docs/app-store-4-7-self-audit.md
cat -n docs/app-store-4-7-self-audit.md
printf '%s\n' '--- required project context ---'
for f in docs/architecture.md docs/STATUS.md CONTRIBUTING.md CHANGELOG.md; do
  if test -f "$f"; then
    echo "### $f"
    sed -n '1,240p' "$f"
  fi
done

Repository: doublegate/RustySNES

Length of output: 50378


🌐 Web query:

Apple App Store Review Guidelines section 4.7 and subsections 4.7.1 4.7.2 4.7.3 4.7.4 4.7.5 current text emulator apps

💡 Result:

Apple App Store Review Guideline 4.7 permits apps to offer software not embedded in the binary, including HTML5 and JavaScript mini apps and mini games, streaming games, chatbots, plug-ins, and specifically retro game console and PC emulator apps [1][2]. Developers are responsible for ensuring all software offered within their app complies with these guidelines and all applicable laws [1][2]. Apps operating under this guideline must adhere to the following requirements: 4.7.1: All software offered must comply with privacy guidelines (including Guideline 5.1), provide a method for filtering objectionable material, include a mechanism to report content and handle timely responses, provide the ability to block abusive users, and follow Guideline 3.1 regarding digital goods or services [1][3]. 4.7.2: Apps may not extend or expose native platform APIs or technologies to the hosted software without prior permission from Apple [1][2][4]. 4.7.3: Data or privacy permissions may not be shared with any individual software offered in the app without explicit user consent for each instance [1][2]. 4.7.4: Developers must provide an index of all software and metadata available within the app, which must include universal links leading to all such software [1][2]. 4.7.5: The app must provide a way for users to identify software that exceeds the app’s age rating and utilize an age restriction mechanism based on verified or declared age to limit access by underage users [1][2][4].

Citations:


🏁 Script executed:

printf '%s\n' '--- mobile-readiness references ---'
fd -i 'mobile-readiness.md' docs
f=$(fd -i -t f 'mobile-readiness.md' docs | head -n 1)
if test -n "$f"; then
  wc -l "$f"
  rg -n -C 5 '4\.7|self-audit|store|ROM|network|privacy|index|age|universal|monetiz' "$f"
fi
printf '%s\n' '--- all App Store 4.7 references ---'
rg -n -C 3 '4\.7|App Store|emulator app|self-audit' --glob '*.md' .
printf '%s\n' '--- focused changelog entry ---'
rg -n -C 20 'App Store §4\.7|§4\.7 self-audit' CHANGELOG.md
printf '%s\n' '--- cited shell source files ---'
for n in MainActivity.kt ContentView.swift EmulatorViewModel.swift AndroidManifest.xml Info.plist; do
  p=$(fd -i -t f "$n" . | head -n 1)
  if test -n "$p"; then
    echo "### $p"
    rg -n -C 3 'OpenDocument|fileImporter|openInputStream|startAccessing|http|uses-permission|INTERNET|RustySNES|Save State|Load State|Open ROM|network|URLSession|URLRequest|BillingClient|StoreKit|purchase|SKProduct' "$p"
  fi
done
printf '%s\n' '--- mobile source inventory ---'
git ls-files | rg '(^|/)(android|ios)/|mobile-readiness'

Repository: doublegate/RustySNES

Length of output: 50378


🏁 Script executed:

printf '%s\n' '--- official Apple guideline section ---'
url='https://developer.apple.com/app-store/review/guidelines/'
if command -v curl >/dev/null 2>&1; then
  curl -L --max-time 20 -sS "$url" |
    tr '\r' '\n' |
    sed 's/<[^>]*>/ /g' |
    sed 's/&nbsp;/ /g; s/&amp;/\&/g; s/&quot;/"/g' |
    tr -s ' ' |
    grep -o -E '.{0,180}4\.7(\.1|\.2|\.3|\.4|\.5)?.{0,700}' |
    sed -n '1,30p'
fi
printf '%s\n' '--- mobile-platform ADR references ---'
if test -f docs/adr/0012-mobile-platform-target.md; then
  rg -n -C 5 'App Store|4\.7|privacy|ROM|network|store|user|software|emulator' docs/adr/0012-mobile-platform-target.md
fi
printf '%s\n' '--- audit and readiness scope statements ---'
sed -n '1,28p' docs/app-store-4-7-self-audit.md
sed -n '331,384p' docs/mobile-readiness.md

Repository: doublegate/RustySNES

Length of output: 7879


Assess App Store §4.7.1–§4.7.5 before recording PASS.

This formal §4.7 audit checks only five local criteria. Apple also requires privacy and content controls, no unauthorized native API exposure, per-software permission consent, a software index with universal links, and age identification and restriction. A local ROM picker does not establish that these rules are inapplicable. Add an applicability result and evidence for each subrule, or rename this to a limited audit and list the exclusions. Align CHANGELOG.md and docs/mobile-readiness.md with that scope.

🤖 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 `@docs/app-store-4-7-self-audit.md` around lines 16 - 24, Expand the §4.7 audit
beyond the five local checks by recording applicability and supporting evidence
for requirements §4.7.1–§4.7.5, including privacy/content controls, native API
restrictions, per-software consent, software indexing with universal links, and
age identification/restriction. If those checks remain out of scope, rename the
audit as limited and explicitly list the exclusions; then align the
corresponding scope statements in CHANGELOG.md and docs/mobile-readiness.md.

Source: Path instructions

Comment thread docs/app-store-4-7-self-audit.md
Comment on lines +39 to +64
## Criterion 2 — every ROM is user-supplied, through the system picker · **PASS**

There is exactly one way a ROM enters either app, and it is the platform's own document picker. The
user chooses a file they already possess; the app never names, suggests, or reaches for a source.

| shell | mechanism | file:line |
|---|---|---|
| Android | `ActivityResultContracts.OpenDocument()`, read via `contentResolver.openInputStream` | `MainActivity.kt:73`, `:173` |
| iOS | SwiftUI `.fileImporter`, read under `startAccessingSecurityScopedResource()` | `ContentView.swift:52`, `EmulatorViewModel.swift:26` |

The iOS path going through a security-scoped resource is the correct sandbox behaviour for a
user-selected file and is worth noting as evidence the picker is genuine rather than decorative.

## Criterion 3 — no capability to obtain game software · **PASS**

Neither shell can fetch anything.

- **Android declares no permissions at all** — `AndroidManifest.xml` contains no
`<uses-permission>` element, so not even `INTERNET`. An app without `INTERNET` cannot download a
ROM by any route.
- **iOS has no networking code and no ATS configuration.** The only `http` string anywhere in the
bundle is the `DOCTYPE` URL in `Info.plist`'s XML preamble (`Info.plist:2`) — a schema identifier,
not a request.

This is the strongest single fact in the audit: the "user-provided software" requirement is
enforced by the app's *capabilities*, not merely by its UI.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- candidate files ---'
fd -i -t f 'app-store-4-7-self-audit\.md|architecture\.md|STATUS\.md|CONTRIBUTING\.md|AndroidManifest\.xml|MainActivity\.kt|ContentView\.swift|EmulatorViewModel\.swift' .

printf '%s\n' '--- audit document ---'
audit=$(fd -i -t f '^app-store-4-7-self-audit\.md$' . | head -n 1)
[ -n "$audit" ] && sed -n '1,100p' "$audit"

printf '%s\n' '--- required project documentation ---'
for name in architecture.md STATUS.md CONTRIBUTING.md; do
  f=$(fd -i -t f "^${name}$" . | head -n 1)
  if [ -n "$f" ]; then
    printf '\n--- %s ---\n' "$f"
    sed -n '1,180p' "$f"
  else
    printf '\n--- %s: not found ---\n' "$name"
  fi
done

printf '%s\n' '--- cited implementation locations ---'
for pattern in 'AndroidManifest.xml' 'MainActivity.kt' 'ContentView.swift' 'EmulatorViewModel.swift'; do
  while IFS= read -r f; do
    printf '\n--- %s ---\n' "$f"
    wc -l "$f"
    case "$f" in
      *AndroidManifest.xml) cat -n "$f" ;;
      *MainActivity.kt) sed -n '50,195p' "$f" | cat -n ;;
      *ContentView.swift) sed -n '35,75p' "$f" | cat -n ;;
      *EmulatorViewModel.swift) sed -n '1,55p' "$f" | cat -n ;;
    esac
  done < <(fd -i -t f "$pattern" .)
done

Repository: doublegate/RustySNES

Length of output: 35041


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- Android manifest ---'
cat -n android/app/src/main/AndroidManifest.xml

printf '%s\n' '--- Android ROM picker and read path ---'
sed -n '55,90p;155,185p;320,365p' android/app/src/main/kotlin/com/doublegate/rustysnes/MainActivity.kt | cat -n

printf '%s\n' '--- iOS importer and read path ---'
sed -n '20,65p' ios/RustySNES/Sources/ContentView.swift | cat -n
sed -n '1,55p' ios/RustySNES/Sources/EmulatorViewModel.swift | cat -n

printf '%s\n' '--- mobile readiness references ---'
f=$(fd -i -t f '^mobile-readiness\.md$' . | head -n 1)
if [ -n "$f" ]; then
  sed -n '1,220p' "$f"
else
  printf '%s\n' 'docs/mobile-readiness.md not found'
fi

printf '%s\n' '--- Android/iOS networking and document-provider surface ---'
rg -n -i --glob '!snesdev_wiki/**' \
  'OpenDocument|ACTION_OPEN_DOCUMENT|openInputStream|fileImporter|URLSession|URLRequest|NWConnection|\.data\(from:|HttpURLConnection|OkHttp|ktor|uses-permission|INTERNET|http://|https://' \
  android ios docs/app-store-4-7-self-audit.md

printf '%s\n' '--- Android dependency declarations ---'
while IFS= read -r f; do
  printf '\n--- %s ---\n' "$f"
  sed -n '1,180p' "$f"
done < <(fd -i -t f 'build\.gradle(\.kts)?$|settings\.gradle(\.kts)?$' android)

Repository: doublegate/RustySNES

Length of output: 29861


🌐 Web query:

Android official documentation ACTION_OPEN_DOCUMENT cloud storage provider downloads content while serving openInputStream INTERNET permission

💡 Result:

When using ACTION_OPEN_DOCUMENT to access files from a cloud storage provider in Android, your application does not need the INTERNET permission to call openInputStream(uri) on the ContentResolver [1][2]. The Storage Access Framework (SAF) manages permissions and data access securely [1][3]. When a user selects a file through the system picker UI initiated by ACTION_OPEN_DOCUMENT, the system grants your application a narrow, temporary URI permission to access that specific document [3]. This mechanism bypasses the need for your app to have broad storage or network permissions to retrieve the file data [1][3]. Key considerations include: 1. Permission Model: Access is granted by the system based on the user's explicit selection [3]. You do not require explicit INTERNET permission for the ContentResolver to interact with the provider [2]. The cloud provider itself handles the necessary network communication to fetch the file content, abstracting this complexity away from the client application [1][3]. 2. Implementation: You invoke openInputStream(uri) on the ContentResolver using the URI returned in the activity result [2][4]. The ContentResolver handles the communication with the document provider, which ensures the file data is delivered to your application's input stream [3][2]. 3. Security: If you attempt to access a document URI without having received it through an approved SAF intent flow, the system will throw a SecurityException [3][5]. In summary, the INTERNET permission is not a requirement for the client app to consume an input stream from a document provider via SAF, as the system-level URI grant handles the access authorization [3][2].

Citations:


🌐 Web query:

Apple official documentation UIDocumentPickerViewController security scoped URL file provider downloads document on demand

💡 Result:

When using UIDocumentPickerViewController to select documents or directories from a file provider, the system provides security-scoped URLs that allow your app to access files outside its sandbox [1][2][3]. For on-demand file access and downloading, the following mechanisms are essential: 1. File Coordination: When you work with external files, you must use NSFileCoordinator [3][4]. Coordinated reads are critical because when your app performs a coordinated read on a file placeholder, the system automatically triggers the document provider extension to download or make the file accessible [5]. 2. Security-Scoped Access: You must explicitly call startAccessingSecurityScopedResource on the security-scoped URL before accessing the file [6][7][8]. Once access is complete, you must balance this by calling stopAccessingSecurityScopedResource [8][4]. The defer block is the recommended pattern for ensuring the stop method is called even if an error occurs during processing [7][9]. 3. UIDocument Advantage: If you use a subclass of UIDocument to manage your files, the framework automatically handles security-scoped resource management, including calling start and stop methods, as well as file coordination [6][3][8]. This is the recommended approach for accessing files outside your app's sandbox [6]. 4. Persistence: You should not store the raw URL provided by the picker, as it may change [6][3]. Instead, save a security-scoped bookmark using bookmarkData(options:includingResourceValuesForKeys:relativeTo:) with the.withSecurityScope option [3][4]. This allows your app to regain access to the file or directory in future sessions [2][8].

Citations:


🏁 Script executed:

#!/bin/bash
set -eu

python3 - <<'PY'
from pathlib import Path
import re
import xml.etree.ElementTree as ET

manifest = Path("android/app/src/main/AndroidManifest.xml").read_text()
android = Path("android/app/src/main/kotlin/com/doublegate/rustysnes/MainActivity.kt").read_text()
content = Path("ios/RustySNES/Sources/ContentView.swift").read_text()
model = Path("ios/RustySNES/Sources/EmulatorViewModel.swift").read_text()

root = ET.fromstring(manifest)
permissions = [
    node.attrib.get("{http://schemas.android.com/apk/res/android}name")
    for node in root.findall("uses-permission")
]

print("android_permissions:", permissions)
print("android_picker:", bool(re.search(r"ActivityResultContracts\.OpenDocument\(\)", android)))
print("android_reads_picker_uri:", bool(re.search(
    r"contentResolver\.openInputStream\(uri\)", android)))
print("ios_picker:", bool(re.search(r"\.fileImporter\(", content)))
print("ios_reads_selected_url:", bool(re.search(r"Data\(contentsOf:\s*url\)", model)))
print("android_direct_network_api_tokens:", bool(re.search(
    r"URL\(|HttpURLConnection|OkHttp|Retrofit|ktor|WebSocket|Socket", android,
    re.IGNORECASE)))
print("ios_direct_network_api_tokens:", bool(re.search(
    r"URLSession|URLRequest|NWConnection|URLProtocol|CFStreamCreatePairWithSocket",
    content + model, re.IGNORECASE)))
print("android_local_only_enforcement:", bool(re.search(
    r"scheme|ContentResolver\.SCHEME_CONTENT|DocumentsContract|isLocal",
    android, re.IGNORECASE)))
print("ios_local_only_enforcement:", bool(re.search(
    r"scheme|isFileURL|FileManager|isLocal",
    content + model, re.IGNORECASE)))
PY

Repository: doublegate/RustySNES

Length of output: 428


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- alleged local-only matches ---'
rg -n -i 'scheme|ContentResolver\.SCHEME_CONTENT|DocumentsContract|isLocal|isFileURL|FileManager' \
  android/app/src/main/kotlin/com/doublegate/rustysnes/MainActivity.kt \
  ios/RustySNES/Sources/ContentView.swift \
  ios/RustySNES/Sources/EmulatorViewModel.swift || true

printf '%s\n' '--- exact picker/read call lines ---'
rg -n 'OpenDocument|openInputStream|fileImporter|Data\(contentsOf|startAccessingSecurityScopedResource|stopAccessingSecurityScopedResource' \
  android/app/src/main/kotlin/com/doublegate/rustysnes/MainActivity.kt \
  ios/RustySNES/Sources/ContentView.swift \
  ios/RustySNES/Sources/EmulatorViewModel.swift

python3 - <<'PY'
from pathlib import Path
import re

android = Path("android/app/src/main/kotlin/com/doublegate/rustysnes/MainActivity.kt").read_text()
ios = Path("ios/RustySNES/Sources/ContentView.swift").read_text() + \
      Path("ios/RustySNES/Sources/EmulatorViewModel.swift").read_text()

local_checks = [
    r'uri\.scheme\s*==',
    r'uri\.scheme\s*!=',
    r'Uri\.SCHEME_FILE',
    r'URL\(.*isFileURL',
    r'\.isFileURL',
    r'ContentResolver\.SCHEME_CONTENT',
    r'DocumentsContract',
]
for label, text in [("android", android), ("ios", ios)]:
    matches = [pattern for pattern in local_checks if re.search(pattern, text, re.I | re.S)]
    print(f"{label}_explicit_local_scheme_checks:", matches)
PY

Repository: doublegate/RustySNES

Length of output: 1469


Narrow Criterion 3 to direct app networking. ACTION_OPEN_DOCUMENT and iOS .fileImporter can return cloud-backed documents whose providers download content during openInputStream or Data(contentsOf:). No INTERNET permission or networking API proves only that the shells do not initiate direct network requests. Replace “Neither shell can fetch anything” and “by any route” with this narrower claim, or add local-only URI checks and tests.

🤖 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 `@docs/app-store-4-7-self-audit.md` around lines 39 - 64, Narrow Criterion 3 to
state that neither shell initiates direct network requests, rather than claiming
it cannot fetch software by any route. Update the surrounding Android and iOS
evidence to describe the absence of app-declared networking capabilities and
APIs without asserting that picker providers cannot retrieve cloud-backed
content.

Source: Path instructions

Comment thread docs/mobile-readiness.md
Comment on lines +217 to +221
> **Audited 2026-08-02 (`v1.30.0`).** Four entries in this list had gone stale — `android.yml`, the
> `./gradlew` wrapper, the iOS simulator *run*, and the §4.7 self-audit all exist now. They are kept
> below, marked **DONE**, rather than deleted: a readiness document that silently drops items is one
> nobody can audit backwards. What remains genuinely outstanding is stated as such.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Remove completed checks from the outstanding Phase 6 list.

This change marks .github/workflows/android.yml, ./gradlew, and the §4.7 audit as completed. The later Phase 6 checklist still presents the workflow and wrapper as work to complete in Lines 287-290, and the struck audit entry retains obsolete “third has findings” wording in Lines 306-308. The readiness document now reports conflicting statuses. Update the later checklist and stale audit description.

As per path instructions: Docs are the spec, not a history log. Flag prose that has drifted from the code it describes.

🤖 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 `@docs/mobile-readiness.md` around lines 217 - 221, Update the later Phase 6
checklist to remove the completed android.yml and ./gradlew work items, and
revise the §4.7 audit entry to remove the obsolete “third has findings” wording.
Keep the document’s remaining outstanding items accurate and consistent with the
completed statuses described above.

Source: Path instructions

Comment thread docs/mobile-readiness.md
Comment on lines +241 to +246
- **An unsigned `assembleRelease` path exists — DONE (`v1.30.0`).** Signing material is the project
owner's to provision, so a *signed* release build is still out of reach; an unsigned one is not,
and it runs R8, resource shrinking and the release manifest merge — where release-only breakage
actually lives. `isMinifyEnabled` is `false` today, so at this revision it proves the release
variant assembles; it is wired now because the moment minification is enabled this step is what
catches the fallout. The release APK gets its own 16 KB gate, on the artifact that would ship.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Correct the current release-build coverage claim.

android/app/build.gradle.kts, Lines 24-27 set release.isMinifyEnabled = false. The current gate does not exercise R8 or resource shrinking. Android guidance enables isMinifyEnabled and isShrinkResources for those checks. (developer.android.com)

  • docs/mobile-readiness.md#L241-L246: describe current assembleRelease coverage as release assembly and packaging; retain R8/resource shrinking as future coverage.
  • CHANGELOG.md#L27-L32: apply the same wording to the release note.

As per path instructions: Docs are the spec, not a history log. Flag prose that has drifted from the code it describes.

🧰 Tools
🪛 LanguageTool

[style] ~242-~242: ‘out of reach’ might be wordy. Consider a shorter alternative.
Context: ...n, so a signed release build is still out of reach; an unsigned one is not, and it runs ...

(EN_WORDINESS_PREMIUM_OUT_OF_REACH)

📍 Affects 2 files
  • docs/mobile-readiness.md#L241-L246 (this comment)
  • CHANGELOG.md#L27-L32
🤖 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 `@docs/mobile-readiness.md` around lines 241 - 246, Update the release-build
coverage wording in docs/mobile-readiness.md lines 241-246 and CHANGELOG.md
lines 27-32 to describe assembleRelease as covering release assembly and
packaging only; remove claims that it currently exercises R8 or resource
shrinking, and state that those checks remain future coverage while
release.isMinifyEnabled and resource shrinking are disabled.

Source: Path instructions

The job failed on its first run with `can't find crate for core: the
aarch64-linux-android target may not be installed`.

The reasoning in the comment was wrong, not just the target list. "x86_64 only,
the emulator runs the host ABI" confuses what the emulator RUNS with what the
build COMPILES: Gradle's cargoNdkBuild builds every ABI in build.gradle.kts's
`cargoAbis` map -- arm64-v8a AND x86_64 -- before the instrumented test can
install anything, so both Rust targets have to be present regardless of which
one the device is.

Also salvages this project's agent scratch out of /tmp ahead of a reboot: 40
one-off probe scripts and 5 documents into `salvaged/`, which is gitignored, so
the only tracked change is docs/SALVAGE_MANIFEST.md. Dropped ROM-derived
framebuffer captures, regenerable AccuracySNES scene captures, ~1.4 MB of
CHANGELOG working copies, release notes already in CHANGELOG.md, and ~45 PR
bodies and bot-review adjudications for merged PRs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@docs/SALVAGE_MANIFEST.md`:
- Around line 30-69: Replace every machine-specific absolute source value in the
tracked SALVAGE_MANIFEST entries with a redacted source-relative path, removing
usernames, temporary-directory prefixes, repository paths, and agent UUIDs.
Preserve each corresponding salvaged filename and provenance mapping, and apply
the same cleanup to the additional entries referenced in the comment.
🪄 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: 775cceaa-fee8-4507-be11-dfa89b022748

📥 Commits

Reviewing files that changed from the base of the PR and between 1b6a69e and b05b714.

📒 Files selected for processing (2)
  • .github/workflows/android.yml
  • docs/SALVAGE_MANIFEST.md
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
  • GitHub Check: lint
  • GitHub Check: accuracysnes
  • GitHub Check: test-light
  • GitHub Check: smoke
  • GitHub Check: build demo + docs
  • GitHub Check: build
🧰 Additional context used
📓 Path-based instructions (7)
docs/**/*.md

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Before changing a subsystem, consult docs/architecture.md, docs/STATUS.md, CONTRIBUTING.md, the relevant subsystem documentation, and applicable ADRs.

New subsystems must add documentation under docs/.

Files:

  • docs/SALVAGE_MANIFEST.md
**/*.{rs,md}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Chip-behavior changes must update both the chip implementation and the corresponding docs/<subsystem>.md documentation.

A chip change must update both the chip implementation and its corresponding docs/<chip>.md documentation in the same change.

Files:

  • docs/SALVAGE_MANIFEST.md
**/*

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*: Do not commit or vendor the generated snesdev_wiki/ mirror; it is gitignored and intended only as a local reference.
Keep commits focused and use Conventional Commits: <type>(<scope>): <subject>, with an imperative subject of at most 72 characters.
Do not use emojis in code, comments, or commit messages.
Before opening a PR, ensure formatting, Clippy, workspace tests, the core embedded build, rustdoc with warnings denied, documentation coverage, and changelog requirements pass.
Ticket completion must be reflected in the relevant to-dos/ sprint file.

**/*: Preserve the one-directional crate graph: chip crates must not depend on one another; rustysnes-core ties them together.
Never commit commercial ROMs; only commit derived screenshots and hashes.
Keep docs/STATUS.md as the authoritative per-subsystem status and update project documentation in the same PR as code changes.
Do not treat RustyNES v2.0 or engine-lineage anchors as project releases.

Files:

  • docs/SALVAGE_MANIFEST.md
docs/**/*

📄 CodeRabbit inference engine (docs/testing-strategy.md)

Chip crates should exceed 90% unit-test coverage, and each chip should be fuzzable in isolation.

Files:

  • docs/SALVAGE_MANIFEST.md
docs/**

⚙️ CodeRabbit configuration file

docs/**: Docs are the spec, not a history log. Flag claims that contradict the code, counts that
contradict the generated docs/accuracysnes-coverage.md, and any statement of coverage that
is broader than what the corresponding test actually asserts.

Files:

  • docs/SALVAGE_MANIFEST.md
**/*.md

⚙️ CodeRabbit configuration file

**/*.md: Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes
rather than style nits. The markdownlint gate is pinned to v0.39.0 via pre-commit —
do not report rules that version does not have (MD060 in particular).

Files:

  • docs/SALVAGE_MANIFEST.md
.github/workflows/**

⚙️ CodeRabbit configuration file

.github/workflows/**: Flag third-party actions pinned to a branch rather than a tag or commit SHA, jobs without
a least-privilege permissions: block, and secrets that could reach the log. The
ci-success job is the required check and must depend on every gate it claims to
aggregate — a new job that is not in its needs: list is a gate nothing enforces.

Files:

  • .github/workflows/android.yml
🔇 Additional comments (1)
.github/workflows/android.yml (1)

316-321: LGTM!

Also applies to: 334-335

Comment thread docs/SALVAGE_MANIFEST.md
Comment on lines +30 to +69
- `salvaged/scripts/a5_18-parked.patch` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/a5_18-parked.patch`
- `salvaged/scripts/a5_19-wip.patch` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/a5_19-wip.patch`
- `salvaged/scripts/atdone.lua` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/atdone.lua`
- `salvaged/scripts/base-agy.sh` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/base-agy.sh`
- `salvaged/scripts/capture_with_input.lua` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/capture_with_input.lua`
- `salvaged/scripts/doctor_scan.py` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/951bcba8-82dc-4ed4-8363-b59d0791cee6/scratchpad/doctor_scan.py`
- `salvaged/scripts/e506.rs` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/e506.rs`
- `salvaged/scripts/e801.lua` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/e801.lua`
- `salvaged/scripts/e801.rs` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/e801.rs`
- `salvaged/scripts/e806.rs` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/e806.rs`
- `salvaged/scripts/e902.rs` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/e902.rs`
- `salvaged/scripts/edge_probe.lua` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/edge_probe.lua`
- `salvaged/scripts/fails.lua` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/fails.lua`
- `salvaged/scripts/final.lua` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/final.lua`
- `salvaged/scripts/fixed_crossval.lua` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/fixed_crossval.lua`
- `salvaged/scripts/fixed_scenes.lua` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/fixed_scenes.lua`
- `salvaged/scripts/gh.py` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/951bcba8-82dc-4ed4-8363-b59d0791cee6/scratchpad/gh.py`
- `salvaged/scripts/held_1based.lua` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/held_1based.lua`
- `salvaged/scripts/held_probe.lua` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/held_probe.lua`
- `salvaged/scripts/idx_probe.lua` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/idx_probe.lua`
- `salvaged/scripts/inspect_rom.py` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/inspect_rom.py`
- `salvaged/scripts/liveness.fixed.rs` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/liveness.fixed.rs`
- `salvaged/scripts/lrcv.c` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/lrcv.c`
- `salvaged/scripts/m2list.lua` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/m2list.lua`
- `salvaged/scripts/meas.lua` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/meas.lua`
- `salvaged/scripts/new_rtt_tests.rs` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/new_rtt_tests.rs`
- `salvaged/scripts/old_rtt_tests.rs` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/old_rtt_tests.rs`
- `salvaged/scripts/one.lua` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/one.lua`
- `salvaged/scripts/pad_probe.lua` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/pad_probe.lua`
- `salvaged/scripts/pc.lua` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/pc.lua`
- `salvaged/scripts/pinexact_hash_baseline.rs` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/pinexact_hash_baseline.rs`
- `salvaged/scripts/pr260-fixes.patch` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/pr260-fixes.patch`
- `salvaged/scripts/probe.rs` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/probe.rs`
- `salvaged/scripts/resolve.py` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/resolve.py`
- `salvaged/scripts/run-guardtest.sh` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/run-guardtest.sh`
- `salvaged/scripts/run-nightlytest.sh` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/run-nightlytest.sh`
- `salvaged/scripts/shift.rs` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/shift.rs`
- `salvaged/scripts/spc_probe.rs` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/spc_probe.rs`
- `salvaged/scripts/wire_i18n.py` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/wire_i18n.py`
- `salvaged/scripts/wram_probe.lua` <- `/tmp/claude-1000/-home-parobek-Code-OSS-Public-Projects-RustySNES/fc11bae0-ecd6-457a-ac1b-be930be88017/scratchpad/wram_probe.lua`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Remove machine-specific absolute paths from the tracked manifest.

These entries expose the local username, /tmp/claude-1000, repository paths, and agent UUIDs. The paths are not portable provenance data, especially because salvaged/ is gitignored. Replace each source value with a redacted source-relative path, or keep the full paths outside the repository.

Also applies to: 82-86

🤖 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 `@docs/SALVAGE_MANIFEST.md` around lines 30 - 69, Replace every
machine-specific absolute source value in the tracked SALVAGE_MANIFEST entries
with a redacted source-relative path, removing usernames, temporary-directory
prefixes, repository paths, and agent UUIDs. Preserve each corresponding
salvaged filename and provenance mapping, and apply the same cleanup to the
additional entries referenced in the comment.

…ssertion

Four findings, all real.

CRITICAL -- the release-APK alignment gate could pass on an APK that would fail
on Play. I re-implemented the check instead of reusing the logic the debug gate
already settled, and got all three of its properties wrong:

  1. exact string equality (`0x4000|0x10000`) rejects other valid alignments --
     the requirement is "at least 16 KB", so it is DIVISIBILITY. The debug
     gate's own comment records this exact mistake: an equality test wrongly
     flagged JNA's 64 KB-aligned libjnidispatch.so.
  2. `awk ... exit` read only the FIRST LOAD segment.
  3. no `checked == 0` guard, so a missing ABI directory makes the glob match
     nothing and the gate PASSES on an APK with no libraries in it.

The gate now uses the same aligned16k() helper and the same guard as the debug
one. Repeating a bug the file already documents having fixed is the argument for
reusing settled logic rather than writing it again.

VACUOUS TEST -- drain_audio_is_non_destructive_within_a_frame asserted that two
successive drainAudio() calls return the same count, claiming to pin the
documented non-destructive contract. Measured on the host: a no-ROM frame
produces NO audio, `first=0 second=0`, so `0 == 0` passed and proved nothing --
it would have gone on passing had the contract inverted. Replaced with an
assertion the bridge can actually support without a ROM (interleaved stereo, so
an even length); the real contract stays covered host-side by
drain_audio_returns_interleaved_stereo_samples, which loads a ROM first.

Also: the §4.7 audit's monetization grep lacked -E, so its `|` were literal and
it would have reported "nothing" for a reason unrelated to the code -- an audit
whose command cannot be re-run is an assertion, not an audit. And the emulator
step is now bounded by timeout-minutes, since splitting the job contained the
blast radius of a flaky AVD without bounding its runtime.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (4)
docs/app-store-4-7-self-audit.md (4)

3-4: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Pin the audited revision to an immutable commit.

main at the A6.15 merge (#331) is mutable and does not identify whether the audit targets the PR head or base. Replace it with 1b6a69eac33624ff42ac0a125786c32cd6480040 and state whether that commit is the PR head or base.

As per path instructions, “Docs are the spec, not a changelog”; the audit record must identify the revision it describes.

🤖 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 `@docs/app-store-4-7-self-audit.md` around lines 3 - 4, Update the audit
header’s audited revision to the immutable commit
1b6a69eac33624ff42ac0a125786c32cd6480040 and explicitly identify whether it is
the PR head or base; remove the mutable “main at the A6.15 merge (`#331`)”
reference.

Source: Path instructions


52-64: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Narrow Criterion 3 to direct app networking.

After a user selects a cloud-backed document, OpenDocument/openInputStream and iOS .fileImporter/Data(contentsOf:) can retrieve content through the provider. No INTERNET permission proves only that the app does not initiate direct network requests. It does not prove “Neither shell can fetch anything,” “by any route,” or that “no data leaves the device” in the later Data Safety statement. Narrow the wording or add explicit local-only URI checks and tests.

As per path instructions, “Docs are the spec, not a changelog”; networking claims must match the picker behavior they describe.

🤖 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 `@docs/app-store-4-7-self-audit.md` around lines 52 - 64, Revise Criterion 3 to
claim only that the app performs no direct network requests, rather than
asserting that neither shell can fetch content by any route. Update the Android
permission and iOS networking evidence to acknowledge that provider-backed
document access through OpenDocument/openInputStream and
fileImporter/Data(contentsOf:) may retrieve cloud content, and avoid implying
that this proves no data leaves the device.

Source: Path instructions


31-37: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Make Criterion 1 evidence match its PASS claim.

The find commands check only .sfc, .smc, and an assets path. They do not inspect firmware.bin, extensionless files, other resources, or built APK and iOS bundles. A bundled file outside those patterns produces nothing while Line 28 still claims that no ROM, BIOS, or firmware is shipped. Narrow the claim or add source-tree and built-bundle inventories.

As per path instructions, “Docs are the spec, not a changelog”; the PASS claim must match the evidence presented.

🤖 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 `@docs/app-store-4-7-self-audit.md` around lines 31 - 37, Update Criterion 1 in
the self-audit so its PASS claim matches the evidence: either narrow the claim
to the specific .sfc, .smc, and assets-path checks, or expand the documented
inventory to cover source-tree files, extensionless firmware/resources, built
Android APK contents, and iOS bundles. Ensure the evidence and conclusion no
longer assert that no ROM, BIOS, or firmware is shipped beyond what was actually
inspected.

Source: Path instructions


16-24: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Cover or explicitly exclude §4.7.1–§4.7.5.

The document calls itself a formal §4.7 self-audit, but the listed criteria do not record applicability or evidence for the privacy/content controls, native API restrictions, per-software consent, software indexing and universal links, or age restrictions. This becomes a blocking gap when the document is used as store-readiness evidence. Add the missing checks, or rename this to a limited audit and list the exclusions in the readiness records.

As per path instructions, “Docs are the spec, not a changelog”; the audit scope must match the requirements it claims to assess.

🤖 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 `@docs/app-store-4-7-self-audit.md` around lines 16 - 24, Expand the §4.7
self-audit document to explicitly assess §4.7.1–§4.7.5, including
privacy/content controls, native API restrictions, per-software consent,
software indexing and universal links, and age restrictions, with applicability
and evidence recorded for each. If any requirement is intentionally excluded,
rename the audit as limited and document those exclusions in the readiness
records.

Source: Path instructions

🤖 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 @.github/workflows/android.yml:
- Around line 285-318: Add a ZIP-alignment validation before the release APK
extraction and ELF checks, invoking zipalign with page size 16 and checking the
APK in place. Fail the workflow when zipalign reports misaligned entries, while
preserving the existing aligned16k and library-presence checks.

In
`@android/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.kt`:
- Around line 59-83: Update drain_audio_returns_a_well_formed_interleaved_buffer
to load a deterministic ROM or audio fixture before calling runFrame, ensuring
drainAudio returns a non-zero buffer. Assert the resulting interleaved stereo
buffer’s expected shape and sample content rather than only checking even
length, while using the existing MobileCore setup and fixture-loading APIs.

---

Duplicate comments:
In `@docs/app-store-4-7-self-audit.md`:
- Around line 3-4: Update the audit header’s audited revision to the immutable
commit 1b6a69eac33624ff42ac0a125786c32cd6480040 and explicitly identify whether
it is the PR head or base; remove the mutable “main at the A6.15 merge (`#331`)”
reference.
- Around line 52-64: Revise Criterion 3 to claim only that the app performs no
direct network requests, rather than asserting that neither shell can fetch
content by any route. Update the Android permission and iOS networking evidence
to acknowledge that provider-backed document access through
OpenDocument/openInputStream and fileImporter/Data(contentsOf:) may retrieve
cloud content, and avoid implying that this proves no data leaves the device.
- Around line 31-37: Update Criterion 1 in the self-audit so its PASS claim
matches the evidence: either narrow the claim to the specific .sfc, .smc, and
assets-path checks, or expand the documented inventory to cover source-tree
files, extensionless firmware/resources, built Android APK contents, and iOS
bundles. Ensure the evidence and conclusion no longer assert that no ROM, BIOS,
or firmware is shipped beyond what was actually inspected.
- Around line 16-24: Expand the §4.7 self-audit document to explicitly assess
§4.7.1–§4.7.5, including privacy/content controls, native API restrictions,
per-software consent, software indexing and universal links, and age
restrictions, with applicability and evidence recorded for each. If any
requirement is intentionally excluded, rename the audit as limited and document
those exclusions in the readiness records.
🪄 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: a3f86578-fb65-46f3-84ba-4cc9935c4564

📥 Commits

Reviewing files that changed from the base of the PR and between b05b714 and ec27cbf.

📒 Files selected for processing (3)
  • .github/workflows/android.yml
  • android/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.kt
  • docs/app-store-4-7-self-audit.md
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
  • GitHub Check: test-light
  • GitHub Check: accuracysnes
  • GitHub Check: lint
  • GitHub Check: smoke
  • GitHub Check: build
  • GitHub Check: review
  • GitHub Check: build demo + docs
🧰 Additional context used
📓 Path-based instructions (8)
docs/**/*.md

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Before changing a subsystem, consult docs/architecture.md, docs/STATUS.md, CONTRIBUTING.md, the relevant subsystem documentation, and applicable ADRs.

New subsystems must add documentation under docs/.

Files:

  • docs/app-store-4-7-self-audit.md
**/*.{rs,md}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Chip-behavior changes must update both the chip implementation and the corresponding docs/<subsystem>.md documentation.

A chip change must update both the chip implementation and its corresponding docs/<chip>.md documentation in the same change.

Files:

  • docs/app-store-4-7-self-audit.md
**/*

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*: Do not commit or vendor the generated snesdev_wiki/ mirror; it is gitignored and intended only as a local reference.
Keep commits focused and use Conventional Commits: <type>(<scope>): <subject>, with an imperative subject of at most 72 characters.
Do not use emojis in code, comments, or commit messages.
Before opening a PR, ensure formatting, Clippy, workspace tests, the core embedded build, rustdoc with warnings denied, documentation coverage, and changelog requirements pass.
Ticket completion must be reflected in the relevant to-dos/ sprint file.

**/*: Preserve the one-directional crate graph: chip crates must not depend on one another; rustysnes-core ties them together.
Never commit commercial ROMs; only commit derived screenshots and hashes.
Keep docs/STATUS.md as the authoritative per-subsystem status and update project documentation in the same PR as code changes.
Do not treat RustyNES v2.0 or engine-lineage anchors as project releases.

Files:

  • docs/app-store-4-7-self-audit.md
  • android/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.kt
docs/**/*

📄 CodeRabbit inference engine (docs/testing-strategy.md)

Chip crates should exceed 90% unit-test coverage, and each chip should be fuzzable in isolation.

Files:

  • docs/app-store-4-7-self-audit.md
docs/**

⚙️ CodeRabbit configuration file

docs/**: Docs are the spec, not a history log. Flag claims that contradict the code, counts that
contradict the generated docs/accuracysnes-coverage.md, and any statement of coverage that
is broader than what the corresponding test actually asserts.

Files:

  • docs/app-store-4-7-self-audit.md
**/*.md

⚙️ CodeRabbit configuration file

**/*.md: Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes
rather than style nits. The markdownlint gate is pinned to v0.39.0 via pre-commit —
do not report rules that version does not have (MD060 in particular).

Files:

  • docs/app-store-4-7-self-audit.md
android/**/*.kt

⚙️ CodeRabbit configuration file

android/**/*.kt: Kotlin/Compose front end over the UniFFI bridge. Flag recomposition footguns (unstable
lambdas or parameters causing needless recomposition) and any direct file or preferences
access that bypasses the bridge.

Files:

  • android/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.kt
.github/workflows/**

⚙️ CodeRabbit configuration file

.github/workflows/**: Flag third-party actions pinned to a branch rather than a tag or commit SHA, jobs without
a least-privilege permissions: block, and secrets that could reach the log. The
ci-success job is the required check and must depend on every gate it claims to
aggregate — a new job that is not in its needs: list is a gate nothing enforces.

Files:

  • .github/workflows/android.yml
🪛 LanguageTool
docs/app-store-4-7-self-audit.md

[style] ~104-~104: To elevate your writing, try using an alternative expression here.
Context: ...ources -> nothing ``` -E, and that matters for reproducibility: without it `grep` ...

(MATTERS_RELEVANT)

🔇 Additional comments (3)
.github/workflows/android.yml (1)

398-402: LGTM!

android/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.kt (1)

28-57: LGTM!

Also applies to: 85-92

docs/app-store-4-7-self-audit.md (1)

66-93: LGTM!

Also applies to: 94-112

Comment on lines +285 to +318
# The SAME three properties the debug gate above settled on, and the first version of
# this step got all three wrong by re-implementing instead of reusing:
# 1. DIVISIBILITY, not string equality. `0x8000` and `0x20000` are valid -- the
# requirement is "at least 16 KB". The debug gate's own comment records that an
# equality test wrongly flagged JNA's 64 KB-aligned libjnidispatch.so.
# 2. EVERY LOAD segment, not just the first (`awk ... exit` read one and stopped).
# 3. A `checked == 0` guard. Without it a missing ABI directory makes the glob match
# nothing and the gate PASSES on an APK with no libraries in it at all.
aligned16k() {
local a
for a in $(readelf -lW "$1" | awk '$1 == "LOAD" { print $NF }'); do
[ $(( a % 16384 )) -eq 0 ] || return 1
done
return 0
}
fail=0
checked=0
for abi in arm64-v8a x86_64; do
for so in "$work"/lib/"$abi"/*.so; do
[ -e "$so" ] || { echo "::error::the release APK carries no .so for $abi"; exit 1; }
checked=$((checked + 1))
if aligned16k "$so"; then
echo "ok: $abi/$(basename "$so")"
else
echo "::error::$so has LOAD segment(s) not a multiple of 16 KB"
readelf -lW "$so" | awk '$1 == "LOAD"'
fail=1
fi
done
done
if [ "$checked" -eq 0 ]; then
echo "::error::the release alignment gate checked nothing"
exit 1
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- workflow files ---'
git ls-files '.github/workflows/**'
printf '%s\n' '--- android workflow structure and relevant sections ---'
wc -l .github/workflows/android.yml
sed -n '1,80p' .github/workflows/android.yml
sed -n '250,335p' .github/workflows/android.yml
sed -n '370,420p' .github/workflows/android.yml
printf '%s\n' '--- zipalign and aggregation references ---'
rg -n -C 3 'zipalign|ci-success|^permissions:|^[[:space:]]+permissions:|^[[:space:]]+needs:' .github/workflows

Repository: doublegate/RustySNES

Length of output: 19756


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- Android build configuration ---'
fd -i -t f 'build.gradle|build.gradle.kts|gradle.properties|libs.versions.toml|settings.gradle|settings.gradle.kts' android
for f in $(fd -i -t f 'build.gradle|build.gradle.kts|gradle.properties|libs.versions.toml|settings.gradle|settings.gradle.kts' android); do
  printf '\n--- %s ---\n' "$f"
  rg -n -C 4 'com.android|agp|androidGradlePlugin|compileSdk|minSdk|targetSdk|packaging|jniLibs|useLegacyPackaging|zipalign|build-tools|sdkmanager|assembleRelease|assembleDebug' "$f" || true
done
printf '%s\n' '--- Android workflow build/setup sections ---'
sed -n '80,255p' .github/workflows/android.yml
sed -n '320,412p' .github/workflows/android.yml
printf '%s\n' '--- release APK packaging references ---'
rg -n -C 4 'packaging|jniLibs|useLegacyPackaging|doNotStrip|zipAlign|zipalign|page size|16 KB|16KB' android .github/workflows

Repository: doublegate/RustySNES

Length of output: 37135


🌐 Web query:

Android 16 KB page sizes APK ZIP alignment zipalign -P 16 official documentation

💡 Result:

To ensure your application is compatible with 16 KB page size requirements, you must perform two distinct types of alignment: aligning the ELF segments of your native libraries (.so files) and ZIP-aligning your APK [1][2]. Starting November 1, 2025, all new apps and updates submitted to Google Play that target Android 15 and higher must support 16 KB page sizes [1]. 1. ELF Alignment Native libraries must be built with 16 KB segment alignment. This is achieved at compile-time by passing linker flags to the NDK [3]. When building your libraries, use the following linker flags: -Wl,-z,max-page-size=16384 You can verify the alignment of your.so files using tools like readelf or objdump to ensure the LOAD segments are aligned to 16 KB (0x4000) [3][4]. 2. APK ZIP Alignment Once your libraries are correctly built, you must use the zipalign tool to align the uncompressed native libraries within the APK to 16 KB boundaries [1][5]. This ensures the files can be memory-mapped (mmap) efficiently by the Android system [5]. Use the following zipalign command: zipalign -v -P 16 -f 4 APK_NAME.apk OUTPUT_NAME.apk In this command: -P 16: Specifies that uncompressed.so files should be aligned to a 16 KiB page boundary [5]. -f: Overwrites the existing output file if necessary [5]. -v: Provides verbose output to confirm the alignment process [5]. -c: (Optional) Can be used to check the alignment of an existing APK without modifying it [1][5]. The official Android documentation confirms that using -P 16 is the recommended approach to ensure compatibility with both 16 KB and 4 KB page size devices [5]. If you do not perform these steps, your app may fail compatibility checks or trigger warnings in the Google Play Console [6][2].

Citations:


🏁 Script executed:

#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re

workflow = Path(".github/workflows/android.yml").read_text()
gradle = Path("android/build.gradle.kts").read_text()

start = workflow.index("- name: Assert 16 KB page alignment inside the RELEASE APK")
end = workflow.index("\n      - name: Upload the APK", start)
step = workflow[start:end]

assert "unzip -q \"$apk\" 'lib/*' -d \"$work\"" in step
assert "zipalign" not in step
assert "readelf -lW" in step
assert re.search(r'for abi in arm64-v8a x86_64;', step)
assert 'id("com.android.application") version "8.7.3"' in gradle

print("release step extracts lib/* and checks ELF LOAD segments")
print("release step contains no zipalign invocation")
print("AGP version: 8.7.3")
print("Therefore ZIP entry offsets are not an explicit property checked by this gate.")
PY

Repository: doublegate/RustySNES

Length of output: 358


🌐 Web query:

site:developer.android.com/guide/practices/page-sizes AGP 8.5.1 16 KB zip alignment

💡 Result:

Android Gradle Plugin (AGP) version 8.5.1 and higher is required to properly support 16 KB page size alignment for apps that use uncompressed shared libraries [1]. Key technical details regarding this requirement include: 1. Requirement for 16 KB Alignment: Devices using 16 KB page sizes require that uncompressed shared libraries (.so files) shipped within an app are aligned on a 16 KB zip-aligned boundary to function correctly [1]. 2. Why AGP 8.5.1 is Necessary: While AGP versions 8.3 through 8.5 align apps to 16 KB by default, they do not always ensure that bundletool zip-aligns the resulting APKs correctly when built from an Android App Bundle (AAB) [1]. Upgrading to AGP 8.5.1 or higher ensures this alignment is handled correctly during the build process, preventing potential installation failures on 16 KB devices [1]. 3. Alternative for Older AGP Versions: If upgrading to AGP 8.5.1+ is not possible, developers must configure their Gradle build to use compressed shared libraries [1]. However, this is discouraged because compressed libraries must be extracted to disk during installation, which increases the app's installation footprint and can lead to more frequent installation failures due to insufficient disk space [1]. 4. Compatibility Recommendation: To ensure 16 KB compatibility, the official recommendation is to update tools to the latest versions, specifically AGP 8.5.1 or higher and NDK version r28 or higher, and ensure all prebuilt dependencies are also 16 KB compatible [1].

Citations:


Add a 16 KB ZIP-alignment check for the release APK.

This step checks ELF LOAD alignment only. An APK with aligned ELF segments but a 4 KB-aligned uncompressed .so ZIP entry passes this step and fails 16 KB compatibility checks. Run zipalign -c -P 16 -v 4 "$apk" before extraction.

🤖 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 @.github/workflows/android.yml around lines 285 - 318, Add a ZIP-alignment
validation before the release APK extraction and ELF checks, invoking zipalign
with page size 16 and checking the APK in place. Fail the workflow when zipalign
reports misaligned entries, while preserving the existing aligned16k and
library-presence checks.

…n fail

The audio assertion took two wrong turns of the same kind, and the reviewer was
right both times. It first asserted two successive drainAudio() calls return
equal counts (claiming the non-destructive contract), then that the buffer
length is even (claiming interleaved stereo). A no-ROM frame produces NO audio,
measured on the host as first=0 second=0 -- so the first passed on `0 == 0` and
the second on `0 % 2 == 0`. Neither could fail for the reason it named.

The fix is not a better assertion, it is a cart. AccuracySNES's HiROM image is
64 KB, tracked, and dual-licensed with the repo, so unlike any commercial ROM it
can be packaged into a test APK. Gradle's `copyTestRom` places it as an
androidTest asset (gitignored -- build output, matching how jniLibs is handled)
and the test loads it, asserts it loaded, runs frames, and asserts real audio
crossed the bridge.

That also closes something docs/mobile-readiness.md records as never having
happened: no ROM had ever actually booted on a device or simulator. It has now.

Verified on the host before wiring it, because an assertion nobody has watched
pass is the same gamble as one that cannot fail: a booted cart emits 1066
interleaved samples from the FIRST frame. The eight frames are margin, and the
comment says so rather than repeating my initial guess that the APU needs the
IPL handshake to complete first -- which the measurement disproves.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown

Antigravity review (Gemini via Ultra)

This PR adds an App Store §4.7 self-audit document, configures an unsigned Android release build with 16 KB ELF alignment verification in CI, adds an instrumented UniFFI runtime smoke test for Android, and updates mobile readiness status documentation.

Blocking issues

  • Missing test ROM build step in CI causes smoke test failure (android/app/build.gradle.kts:137-142, .github/workflows/android.yml:348-372)
    copyTestRom copies accuracysnes-hirom.sfc from tests/roms/AccuracySNES/build into src/androidTest/assets. As noted in .gitignore, this binary is generated build output, not checked-in source. The CI smoke job checks out the repo and immediately invokes ./gradlew connectedDebugAndroidTest without building the AccuracySNES ROM first. Gradle's Copy task completes silently when source files are missing, resulting in MobileCoreSmokeTest.kt:61-63 throwing java.io.FileNotFoundException at runtime when trying to open the missing asset.

Suggestions

  • Validate test ROM existence during Gradle sync/build (android/app/build.gradle.kts:137-142)
    copyTestRom should assert that accuracysnes-hirom.sfc exists (or declare an explicit task dependency on the ROM build step) so missing test assets trigger a clear build failure instead of a runtime test exception.
  • Ensure temporary directory cleanup in alignment gate (.github/workflows/android.yml:283)
    work=$(mktemp -d) is created without a trap 'rm -rf "$work"' EXIT handler, leaving orphaned files in /tmp if downstream assertions fail early under set -e.

Nitpicks

  • Unbound glob evaluation risk in APK alignment loop (.github/workflows/android.yml:302)
    If unzip extracts no libraries for an ABI, "$work"/lib/"$abi"/*.so expands to a literal string; while [ -e "$so" ] handles this correctly, using shopt -s nullglob makes the expansion explicit.

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

@doublegate
doublegate merged commit 3d6eeb7 into main Aug 2, 2026
18 checks passed
@doublegate
doublegate deleted the feat/mobile-store-readiness branch August 2, 2026 20:45
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.

2 participants