feat(mobile): add the App Store 4.7 self-audit, a release build path, and a UniFFI smoke test - #332
Conversation
…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>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📜 Recent review details⏰ Context from checks skipped due to timeout. (6)
🧰 Additional context used📓 Path-based instructions (2)**/*📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
android/**/*.kt⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (4)
WalkthroughThe PR adds Android release alignment checks, x86_64 emulator instrumentation tests for ChangesAndroid mobile readiness
Salvage manifest records
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
Possibly related PRs
🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
Comment |
There was a problem hiding this comment.
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.mdand mark previously “deferred” readiness items as DONE indocs/mobile-readiness.md. - Add an Android instrumented test (
connectedDebugAndroidTest) that validates UniFFI library load + basic FFI calls. - Extend
.github/workflows/android.ymlwith an unsignedassembleReleasestep 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. |
| 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" |
| ```text | ||
| grep -rn 'BillingClient|StoreKit|purchase|SKProduct' android/app/src/main/kotlin ios/RustySNES/Sources | ||
| -> nothing | ||
| ``` |
There was a problem hiding this comment.
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
📒 Files selected for processing (6)
.github/workflows/android.ymlCHANGELOG.mdandroid/app/build.gradle.ktsandroid/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.ktdocs/app-store-4-7-self-audit.mddocs/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 generatedsnesdev_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 relevantto-dos/sprint file.
**/*: Preserve the one-directional crate graph: chip crates must not depend on one another;rustysnes-coreties them together.
Never commit commercial ROMs; only commit derived screenshots and hashes.
Keepdocs/STATUS.mdas the authoritative per-subsystem status and update project documentation in the same PR as code changes.
Do not treat RustyNESv2.0orengine-lineageanchors as project releases.
Files:
android/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.ktdocs/app-store-4-7-self-audit.mdandroid/app/build.gradle.ktsCHANGELOG.mddocs/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.mddocs/mobile-readiness.md
**/*.{rs,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Chip-behavior changes must update both the chip implementation and the corresponding
docs/<subsystem>.mddocumentation.A chip change must update both the chip implementation and its corresponding
docs/<chip>.mddocumentation in the same change.
Files:
docs/app-store-4-7-self-audit.mdCHANGELOG.mddocs/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.mddocs/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 generateddocs/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.mddocs/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.mdCHANGELOG.mddocs/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-privilegepermissions:block, and secrets that could reach the log. The
ci-successjob is the required check and must depend on every gate it claims to
aggregate — a new job that is not in itsneeds: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.mdwhen 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 CorrectnessKeep the existing Kotlin source-set DSL.
AGP 8.7.3 with
org.jetbrains.kotlin.android2.0.20 supportsAndroidSourceSet.kotlin.srcDirs(...). Thekotlin.directoriessyntax 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 QualityKeep the changelog entry under
[Unreleased]. Thev1.30.0entry is already correctly placed; no change is needed.> Likely an incorrect or invalid review comment.
| smoke: | ||
| runs-on: ubuntu-latest | ||
| env: | ||
| CARGO_NET_RETRY: "10" | ||
| CARGO_TERM_COLOR: always | ||
| steps: |
There was a problem hiding this comment.
📐 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)))
PYRepository: 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
| # 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 |
There was a problem hiding this comment.
📐 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 ^3Then 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.
| 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. |
There was a problem hiding this comment.
🔒 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
doneRepository: 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:
- 1: https://developer.apple.com/support/downloads/terms/app-review-guidelines/App-Review-Guidelines-English-UK.pdf
- 2: https://developer.apple.com/app-store/review/guidelines/
- 3: https://archive.ph/0Mw6J
- 4: https://developer.apple.com/news/?id=ey6d8onl
🏁 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/ / /g; s/&/\&/g; s/"/"/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.mdRepository: 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
| ## 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. |
There was a problem hiding this comment.
🔒 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" .)
doneRepository: 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:
- 1: https://developer.android.com/guide/topics/providers/document-provider
- 2: https://developer.android.com/training/data-storage/shared/documents-files
- 3: https://developer.android.com/reference/android/provider/DocumentsProvider
- 4: https://android.googlesource.com/platform/development/+/master/samples/ApiDemos/src/com/example/android/apis/content/DocumentsSample.java
- 5: https://stackoverflow.com/questions/58323881/crash-on-contentresolver-openinputstreamuri-getting-security-permission-denied
🌐 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:
- 1: https://developer.apple.com/documentation/uikit/uidocumentpickerdelegate/documentpicker(_:didpickdocumentsat:)
- 2: https://developer.apple.com/documentation/uikit/providing-access-to-directories
- 3: https://developer.apple.com/documentation/uikit/uidocumentpickerviewcontroller
- 4: https://apple-docs.everest.mt/docs/uikit/uidocumentpickerviewcontroller/
- 5: https://developer.apple.com/library/archive/documentation/FileManagement/Conceptual/DocumentPickerProgrammingGuide/CreatinganOutstandingUserExperience/CreatinganOutstandingUserExperience.html
- 6: https://developer.apple.com/library/archive/documentation/FileManagement/Conceptual/DocumentPickerProgrammingGuide/AccessingDocuments/AccessingDocuments.html
- 7: https://useyourloaf.com/blog/accessing-security-scoped-files/
- 8: https://developer.apple.com/documentation/foundation/nsurl
- 9: https://developer.apple.com/documentation/uikit/providing-access-to-directories?language=obj_7
🏁 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)))
PYRepository: 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)
PYRepository: 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
| > **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. | ||
|
|
There was a problem hiding this comment.
📐 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
| - **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. |
There was a problem hiding this comment.
🎯 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 currentassembleReleasecoverage 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>
There was a problem hiding this comment.
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
📒 Files selected for processing (2)
.github/workflows/android.ymldocs/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>.mddocumentation.A chip change must update both the chip implementation and its corresponding
docs/<chip>.mddocumentation in the same change.
Files:
docs/SALVAGE_MANIFEST.md
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Do not commit or vendor the generatedsnesdev_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 relevantto-dos/sprint file.
**/*: Preserve the one-directional crate graph: chip crates must not depend on one another;rustysnes-coreties them together.
Never commit commercial ROMs; only commit derived screenshots and hashes.
Keepdocs/STATUS.mdas the authoritative per-subsystem status and update project documentation in the same PR as code changes.
Do not treat RustyNESv2.0orengine-lineageanchors 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 generateddocs/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-privilegepermissions:block, and secrets that could reach the log. The
ci-successjob is the required check and must depend on every gate it claims to
aggregate — a new job that is not in itsneeds: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
| - `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` |
There was a problem hiding this comment.
🔒 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>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
docs/app-store-4-7-self-audit.md (4)
3-4: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPin 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 with1b6a69eac33624ff42ac0a125786c32cd6480040and 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 winNarrow Criterion 3 to direct app networking.
After a user selects a cloud-backed document,
OpenDocument/openInputStreamand iOS.fileImporter/Data(contentsOf:)can retrieve content through the provider. NoINTERNETpermission 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 liftMake Criterion 1 evidence match its PASS claim.
The
findcommands check only.sfc,.smc, and anassetspath. They do not inspectfirmware.bin, extensionless files, other resources, or built APK and iOS bundles. A bundled file outside those patterns producesnothingwhile 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 liftCover 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
📒 Files selected for processing (3)
.github/workflows/android.ymlandroid/app/src/androidTest/kotlin/com/doublegate/rustysnes/MobileCoreSmokeTest.ktdocs/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>.mddocumentation.A chip change must update both the chip implementation and its corresponding
docs/<chip>.mddocumentation in the same change.
Files:
docs/app-store-4-7-self-audit.md
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Do not commit or vendor the generatedsnesdev_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 relevantto-dos/sprint file.
**/*: Preserve the one-directional crate graph: chip crates must not depend on one another;rustysnes-coreties them together.
Never commit commercial ROMs; only commit derived screenshots and hashes.
Keepdocs/STATUS.mdas the authoritative per-subsystem status and update project documentation in the same PR as code changes.
Do not treat RustyNESv2.0orengine-lineageanchors as project releases.
Files:
docs/app-store-4-7-self-audit.mdandroid/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 generateddocs/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-privilegepermissions:block, and secrets that could reach the log. The
ci-successjob is the required check and must depend on every gate it claims to
aggregate — a new job that is not in itsneeds: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
| # 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 |
There was a problem hiding this comment.
🎯 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/workflowsRepository: 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/workflowsRepository: 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:
- 1: https://developer.android.com/guide/practices/page-sizes
- 2: https://android16kbchecker.web.app/
- 3: https://xckevin.com/en/blog/android-16kb-page-size-elf-ndk/
- 4: https://github.com/hotbrainstech/verify-16k-page-align
- 5: https://developer.android.com/tools/zipalign
- 6: https://stackoverflow.com/questions/79754247/how-to-fix-warning-apk-does-not-support-16-kb-devices
🏁 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.")
PYRepository: 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>
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
Suggestions
Nitpicks
Automated first-pass review by |
The
v1.30.0rung — 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.ymlexists and gates 16 KB alignment twice (on the.sos and again inside the APK),android/gradlewand its wrapper are committed, andios.ymlalready 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:
<uses-permission>element, so not evenINTERNET. An app withoutINTERNETcannot download a ROM by any route.httpstring in the bundle is theDOCTYPEURL inInfo.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 fornintendo,famicom,snes,super scope,multitap,game boy,mario,zeldareturns nothing outside this project's own identifiers.Two re-audit triggers are recorded rather than left implicit:
rustysnes-monetization, if activated — it is inert today (noBillingClient, noStoreKit, 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
assembleReleasepathSigning 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).
isMinifyEnabledisfalsetoday, 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
assembleDebugalready proves the bindings compile —MainActivitycallsMobileCoredirectly, so drifted bindgen output fails the Kotlin compile. What no build can prove is thatSystem.loadLibraryfinds the.sofor 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;drainAudiois 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
buildwould 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.ymlparses; jobs arebuild(17 steps) andsmoke(8).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 currentv2.38.0.buildjob's proven NDK discovery verbatim (sdkmanageris not onPATHonubuntu-latesteven though$ANDROID_HOMEis 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:
assembleReleaseproducing 64-bit libraries with valid 16 KB alignment.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.