fix(steamdeck): collapse nested if-let in resolve_slot_name_for_sync (clippy CI-green)#13
Closed
terafin wants to merge 15 commits into
Closed
fix(steamdeck): collapse nested if-let in resolve_slot_name_for_sync (clippy CI-green)#13terafin wants to merge 15 commits into
terafin wants to merge 15 commits into
Conversation
…C/MPK/CPK + TG-16) Four related libretro-vs-standalone scanner gaps in one batch — all fix real data-loss / silent-skip paths reported by SGM-Helper users on Steam Deck (RetroDECK) and SS1 (libretro cores), all touch helpers/*/scanner.rs (plus helpers/*/syncer.rs for the RTC dedup-key fix), all ship with happy-path + regression-guard tests. ## Part 1 — PS1 memcard trailing-frame relax (was PR #4, fixes #3) Real PS1 hardware writes "MC" magic at memcard frame 63 (the "write test sector" — bytes 8064-8191). SwanStation (libretro Duckstation port) and Beetle PSX leave frame 63 zero-filled OR write actual game-state continuation bytes through it. Validator was rejecting every libretro PS1 save with "outside allowed console families." Drops the over-strict trailing-frame check. Frame 0 magic + frames 1-15 directory frame checksums are sufficient — these are real structural validators a corrupt memcard would fail. Tests added: zero-filled trailing-frame accepted, non-MC bytes accepted, frame 0 magic still required, directory checksum still required, strict-format memcards still accepted (regression guard). ## Part 2 — Sega RetroDECK path-hints (was PR #6, fixes #5) RetroDECK's flatpak (net.retrodeck.retrodeck) writes saves under single-word lowercase directory names. The Sega classifier had matched some (megadrive, megacd, sega32x) but was missing several: gamegear, mastersystem, megacdjp, sega32xjp, saturnjp, sega32xna, megadrivejp. Adds the missing variants. Test enumerates all 11 cases (5 fixes + 6 regression guards proving the previously-working paths still classify correctly). ## Part 3 — Preserve .rtc / .mpk / .cpk through dedup (was PR #8, fixes #7) Helper's save_selection_key collapsed Pokemon Crystal.rtc and Pokemon Crystal.srm to the same key (stem-only), so the .rtc was silently dropped — losing the in-game real-time clock state. Same class of bug exists for N64 controller-pak data (.mpk / .cpk) which sits alongside cart .srm. Suffixes the dedup key with the extension when ext is rtc/mpk/cpk: "crystal:rtc" vs "crystal:srm" no longer collide. Also adds classifier size-check arm for gameboy .rtc (1..=64 bytes) so the small clock-state payload doesn't get filtered as "implausible save." Test: classify_accepts_tiny_rtc_files_for_gameboy with 8 / 13 / 32 / 48 / 64-byte .rtc payloads, regression guard that an 8-byte .srm is still rejected as bogus (proves the relaxation is .rtc-scoped). ## Part 4 — TurboGrafx-16 / PC Engine classifier (was PR #10, fixes #9) The classify_supported_save function had branches for every other Sega/Nintendo/Sony system but no TG-16 / PCEngine / SuperGrafx block. Saves from RetroArch's Beetle PCE Fast / Beetle SuperGrafx core were returning None and getting skipped with "outside allowed console families" — but they're well-formed BRAM/SRAM saves. Adds contains_any block matching pcengine|tgfx16|turbografx|supergrafx, an infer_nec_slug returning "tgfx16", and an is_plausible_save_for_system arm for tgfx16 covering .sav 2KB / 8KB and .brm 2KB BRAM sizes. Test: classify_recognizes_pcengine_tgfx16_paths enumerates RetroDECK, RetroArch, and Beetle subdir variants. ## Scope - All 4 fixes are additive: missing branches added, over-strict checks relaxed in a single direction. No previously-accepted save shape is now rejected. - All test coverage is parallel across helpers/mister, helpers/steamdeck, helpers/windows — the triplet structure stays in sync. Consolidates and supersedes PR #4 + PR #6 + PR #8 + PR #10. All four share helpers/*/scanner.rs and the same libretro-vs-standalone lens. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ction loop The scanner_libretro_format_batch commit accidentally truncated the slice literal in scanner.rs:2949 across all 3 helpers (mister, steamdeck, windows), saving mid-edit. The `for (subdir, ...) in &[ ... ]` loop's slice body was cut, leaving the file with mismatched-delimiter syntax errors (cargo fmt failed across the workspace). Restored Sega RetroDECK-single-word test loop body from canonical commit 76d7726 (the original "add missing RetroDECK single-word Sega dir hints" PR). The pcengine test's assertion ended up using `*expected_slug` from the dropped Sega destructuring; restored the hardcoded "tgfx16" literal from canonical commit 36cc90d. Also pulls in pre-existing cargo-fmt drift in helpers/*/syncer.rs that was blocking CI fmt-check independently. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The save_selection_key helper had a `if let Some(ext) = ... { if matches!(...) { ... } }`
shape that clippy's collapsible_if lint rejected. Collapsed into a single let-chain
condition matching the surrounding style in this file (which already uses `if let
Some(...) = ... && <bool>` for the Wii data.bin branch right above and for the
preferred_path dedup loop).
Fixes CI clippy -D warnings failure on feat/scanner-libretro-format-batch.
The two CI-failing scanner tests added in 6e66567 had two RCA-distinct issues that surfaced together once the let-chain syntax errors from the slice-literal truncation were restored: 1. Test payload was 0x42 ('B') — printable ASCII. classify_supported_save's first guard calls looks_plain_text(), which returns true for any file whose first 4 KiB is >= 95% printable. A flat 0x42 buffer hits 100% printable and short-circuits classification to None before any path-hint dispatch runs. Switched the test payloads to 0xA5 (alternating bits, non-printable). Note this only affects the new tests; the existing non_ps1_saves_use_identity_adapter test calls normalize_save_for_sync, not classify_supported_save, and so isn't gated by looks_plain_text. 2. infer_sega_slug() didn't carry the no-space RetroDECK variants ("mastersystem", "gamegear") in its inner Sega-system disambiguator. The OUTER Sega path-hint contains_any() block already accepts those single-word forms, so /gamegear/Test Game.srm correctly entered the Sega branch — but infer_sega_slug then fell through every match and returned the default "genesis" slug. Added the no-space variants to the master-system and game-gear branches in all three helpers. Also drops "saturnjp" from the test list with a comment — Saturn classification runs the full passes_binary_validation() Bk-header check inside classify_if_valid, which a flat 0xA5 payload cannot satisfy. That path-hint ->slug routing is already covered by the saturnjp entry in the outer Sega contains_any list; what's NOT yet covered is a synthetic Bk-header fixture that would let us assert the full classify_if_valid pipeline. Worth its own test and out of scope here. Result: workspace test suite goes from 113 passed / 2 failed → 115 passed / 0 failed across all three helpers.
…C/MPK/CPK + TG-16) Four related libretro-vs-standalone scanner gaps in one batch — all fix real data-loss / silent-skip paths reported by SGM-Helper users on Steam Deck (RetroDECK) and SS1 (libretro cores), all touch helpers/*/scanner.rs (plus helpers/*/syncer.rs for the RTC dedup-key fix), all ship with happy-path + regression-guard tests. ## Part 1 — PS1 memcard trailing-frame relax (was PR #4, fixes #3) Real PS1 hardware writes "MC" magic at memcard frame 63 (the "write test sector" — bytes 8064-8191). SwanStation (libretro Duckstation port) and Beetle PSX leave frame 63 zero-filled OR write actual game-state continuation bytes through it. Validator was rejecting every libretro PS1 save with "outside allowed console families." Drops the over-strict trailing-frame check. Frame 0 magic + frames 1-15 directory frame checksums are sufficient — these are real structural validators a corrupt memcard would fail. Tests added: zero-filled trailing-frame accepted, non-MC bytes accepted, frame 0 magic still required, directory checksum still required, strict-format memcards still accepted (regression guard). ## Part 2 — Sega RetroDECK path-hints (was PR #6, fixes #5) RetroDECK's flatpak (net.retrodeck.retrodeck) writes saves under single-word lowercase directory names. The Sega classifier had matched some (megadrive, megacd, sega32x) but was missing several: gamegear, mastersystem, megacdjp, sega32xjp, saturnjp, sega32xna, megadrivejp. Adds the missing variants. Test enumerates all 11 cases (5 fixes + 6 regression guards proving the previously-working paths still classify correctly). ## Part 3 — Preserve .rtc / .mpk / .cpk through dedup (was PR #8, fixes #7) Helper's save_selection_key collapsed Pokemon Crystal.rtc and Pokemon Crystal.srm to the same key (stem-only), so the .rtc was silently dropped — losing the in-game real-time clock state. Same class of bug exists for N64 controller-pak data (.mpk / .cpk) which sits alongside cart .srm. Suffixes the dedup key with the extension when ext is rtc/mpk/cpk: "crystal:rtc" vs "crystal:srm" no longer collide. Also adds classifier size-check arm for gameboy .rtc (1..=64 bytes) so the small clock-state payload doesn't get filtered as "implausible save." Test: classify_accepts_tiny_rtc_files_for_gameboy with 8 / 13 / 32 / 48 / 64-byte .rtc payloads, regression guard that an 8-byte .srm is still rejected as bogus (proves the relaxation is .rtc-scoped). ## Part 4 — TurboGrafx-16 / PC Engine classifier (was PR #10, fixes #9) The classify_supported_save function had branches for every other Sega/Nintendo/Sony system but no TG-16 / PCEngine / SuperGrafx block. Saves from RetroArch's Beetle PCE Fast / Beetle SuperGrafx core were returning None and getting skipped with "outside allowed console families" — but they're well-formed BRAM/SRAM saves. Adds contains_any block matching pcengine|tgfx16|turbografx|supergrafx, an infer_nec_slug returning "tgfx16", and an is_plausible_save_for_system arm for tgfx16 covering .sav 2KB / 8KB and .brm 2KB BRAM sizes. Test: classify_recognizes_pcengine_tgfx16_paths enumerates RetroDECK, RetroArch, and Beetle subdir variants. ## Scope - All 4 fixes are additive: missing branches added, over-strict checks relaxed in a single direction. No previously-accepted save shape is now rejected. - All test coverage is parallel across helpers/mister, helpers/steamdeck, helpers/windows — the triplet structure stays in sync. Consolidates and supersedes PR #4 + PR #6 + PR #8 + PR #10. All four share helpers/*/scanner.rs and the same libretro-vs-standalone lens. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ction loop The scanner_libretro_format_batch commit accidentally truncated the slice literal in scanner.rs:2949 across all 3 helpers (mister, steamdeck, windows), saving mid-edit. The `for (subdir, ...) in &[ ... ]` loop's slice body was cut, leaving the file with mismatched-delimiter syntax errors (cargo fmt failed across the workspace). Restored Sega RetroDECK-single-word test loop body from canonical commit 76d7726 (the original "add missing RetroDECK single-word Sega dir hints" PR). The pcengine test's assertion ended up using `*expected_slug` from the dropped Sega destructuring; restored the hardcoded "tgfx16" literal from canonical commit 36cc90d. Also pulls in pre-existing cargo-fmt drift in helpers/*/syncer.rs that was blocking CI fmt-check independently. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The save_selection_key helper had a `if let Some(ext) = ... { if matches!(...) { ... } }`
shape that clippy's collapsible_if lint rejected. Collapsed into a single let-chain
condition matching the surrounding style in this file (which already uses `if let
Some(...) = ... && <bool>` for the Wii data.bin branch right above and for the
preferred_path dedup loop).
Fixes CI clippy -D warnings failure on feat/scanner-libretro-format-batch.
The two CI-failing scanner tests added in 6e66567 had two RCA-distinct issues that surfaced together once the let-chain syntax errors from the slice-literal truncation were restored: 1. Test payload was 0x42 ('B') — printable ASCII. classify_supported_save's first guard calls looks_plain_text(), which returns true for any file whose first 4 KiB is >= 95% printable. A flat 0x42 buffer hits 100% printable and short-circuits classification to None before any path-hint dispatch runs. Switched the test payloads to 0xA5 (alternating bits, non-printable). Note this only affects the new tests; the existing non_ps1_saves_use_identity_adapter test calls normalize_save_for_sync, not classify_supported_save, and so isn't gated by looks_plain_text. 2. infer_sega_slug() didn't carry the no-space RetroDECK variants ("mastersystem", "gamegear") in its inner Sega-system disambiguator. The OUTER Sega path-hint contains_any() block already accepts those single-word forms, so /gamegear/Test Game.srm correctly entered the Sega branch — but infer_sega_slug then fell through every match and returned the default "genesis" slug. Added the no-space variants to the master-system and game-gear branches in all three helpers. Also drops "saturnjp" from the test list with a comment — Saturn classification runs the full passes_binary_validation() Bk-header check inside classify_if_valid, which a flat 0xA5 payload cannot satisfy. That path-hint ->slug routing is already covered by the saturnjp entry in the outer Sega contains_any list; what's NOT yet covered is a synthetic Bk-header fixture that would let us assert the full classify_if_valid pipeline. Worth its own test and out of scope here. Result: workspace test suite goes from 113 passed / 2 failed → 115 passed / 0 failed across all three helpers.
…car saves Cloud-side sibling of the local dedup fix in feat-preserve-rtc. A GBC RTC game stores .srm (32KB SRAM) + .rtc (8B clock sidecar); both uploaded under the same slotName="default" collided as competing versions of one record, so /latest returned the newest (.rtc) and the download wrote the 8B .rtc OVER the 32KB .srm (then removed the old variant). Data loss. Two layers (save-format spec authored by Ullr; RSM model by Idunn): - ROOT (resolve_slot_name_for_sync): sidecar exts (.rtc/.mpk/.cpk) now resolve to a DISTINCT cloud slot, RSM-native form "<primary> (<LABEL>)" -- e.g. "default (RTC)", "default (Controller Pak)", "default (Cartridge Pak)"; parenthetical, never the conflict-key delimiter, mirroring the PSX "Memory Card 1/2" precedent. Primaries (incl. container-equivalent srm/sav, mcr/vmp/gme) keep the base slot. So .srm and .rtc become separate (rom,slot) records and never compete as versions. - GUARD (allow_write): at both download/write sites, never write a canonical whose format crosses the sidecar/primary boundary onto a mismatched target (an 8B .rtc can never land on a .srm). Sidecars must match exactly; primary to primary (PSX mcr to vmp etc.) is always allowed (no regression). Uses RSM's new /latest format field (Option; None on older RSM -> prior behavior). Pure seams exposed for verification: is_sidecar, allow_write, sidecar_label, resolve_slot_name_for_sync. Unit tests cover the logic (guard matrix, slot labels, no-regression primary-to-primary); the DAT-format/real-save gate is Ullr's. 109 lib tests pass. OUT OF SCOPE: N64 battery-format conversion (.srm-combined to .sra/.eep/.fla). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the black-box verification surface Ullr's fixture matrix drives, so the
sidecar slot/guard fix is provable over real save bytes with no network/sync:
- scanner::classify(path, &bytes) -> {system, format, is_sidecar}: split the
existing classify_supported_save into a bytes-feedable core (size + plain-text
passed in) so classification is table-driveable from fixtures, not just live
files. format = bare lowercased ext (matches RSM /latest + the guard).
- `explain-sync --fixtures-dir D --cloud-state S [--manifest M]`: offline (no
config/auth) subcommand. Reads fixtures-manifest.tsv (<relpath>\t<lower
identity>) + a cloud-state JSON keyed "<identity>::<slotName>", then per
fixture prints classify | upload-slot (resolve_slot_name_for_sync) | cloud
format | write-guard verdict (allow_write). Identity is used AS-IS (bare SHA1
for GB, projection-key for N64-pak/PSX).
Verified end-to-end against the Crystal case: legacy collision (.rtc latest
under "default") -> .srm guard=SKIP (clobber prevented); fixed (distinct slots)
-> both ALLOW (each round-trips). classify unit test + 110 lib tests pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per the save-format spec (Ullr): the gameboy non-.rtc SRAM arm becomes the authoritative enum 512 | 2048 | 8192 | 32768 | 65536 | 131072 across all three helpers. Drops the non-real power-of-two values (1024/4096/16384) and ADDS 131072 — the previous 65536 ceiling silently REJECTED legitimate 128K MBC5 saves (a real classify bug). The .rtc => (1..=64) sidecar arm is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two operational reliability fixes that surface during real-world use of the watch loop, especially after reboots/crashes or under transient sync errors. Both touch helpers/*/syncer.rs (+ helpers/*/watcher.rs for part 2), parallel across mister/steamdeck/windows. After a reboot or crash, sync.lock can be left on disk holding a PID that no longer corresponds to a running process. The watch loop then bails on every cycle with "another sync in progress (lock held by PID N)" and the user has to manually delete the lockfile. Adds a SyncLock helper that, on acquire, checks whether the recorded PID is still running: - If running: lock is held — bail with the existing error message. - If NOT running OR can't be read: treat the lockfile as stale, log the recovery, overwrite with our PID and proceed. The check is intentionally conservative: only the "PID not running" case triggers recovery; any other error (unreadable file, malformed content, permission denied) bails as before — we'd rather wait for human intervention than risk concurrent sync. The watch loop's outer driver was treating ANY error from a single sync cycle as fatal — including transient errors (lock-contention from a CLI sync racing the daemon, network blips, 5xx from RSM). The service would exit and systemd would respawn it… which is fine until respawn-rate-limiter kicks in and the watcher just stops. Wraps each cycle's sync call in error handling that logs the error with full context but continues to the next cycle. The watch interval is preserved — no tight-loop on persistent failure. Tested via helpers/*/tests/e2e_smoke.rs covering: transient error continues, persistent error still logs each cycle, normal success path unchanged. - Both fixes are additive operational hardening. No previously-successful sync is now skipped; no previously-failed sync now silently succeeds. - Includes cargo fmt + clippy passes from the same branch (collapsible_if, derive(Debug) on SyncLock) for clean build. Consolidates and supersedes PR #1 + PR #2 (both touch helpers/*/syncer.rs in the same reliability lens). Squashed to single commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a `secure` opt-in (env/ini: SECURE, default false) that flips base_url to https://. Wires it through ConfigOverrides + AppConfig across the mister, steamdeck, and windows helpers (struct fields, the global_overrides initializers, and all AppConfig test fixtures).
…(clippy) clippy::collapsible_if (-D warnings) at syncer.rs:2818, in the sidecar-slot code (carried from #11). It was masked until #13's secure E0063 was fixed and clippy could compile+run. Collapse the two nested `if let` into a let-chain (`if let Some(ext) = ... && let Some(label) = sidecar_label(&ext)`), matching the codebase's existing let-chain style. Behavior identical. cargo clippy --all-targets -- -D warnings now green on all 3 helpers. (steamdeck-only — mister/windows were already clippy-clean.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
intarweb-sync-bot
Bot
force-pushed
the
intarweb-dev
branch
from
959c079 to
27cad92
Compare
Collaborator
Author
|
Superseded: the collapsible_if collapse was folded UPSTREAM into the durable home joeblack2k#12 (feat/syncer-reliability, commit 8a41b09 — where resolve_base_slot_for_sync/sidecar_label actually live; #11 was the classifier batch, not the slot-resolution code). Sync carried it → intarweb-dev CI verified GREEN (sha 27cad92, 'success test'). This internal intarweb-dev PR was the transient immediate-green; the upstream fold makes it permanent (survives the sync regen). Closing. — Idunn (carry-coordinator) |
terafin
added a commit
that referenced
this pull request
Jun 30, 2026
carry) The #13 secure-flag carry to intarweb-dev covered the 3 lib.rs ConfigOverrides initializers but NOT the test-code AppConfig{} initializers, so the lib-test crate didn't compile (E0063 missing field secure) — which is why #14's cloud_restore unit tests couldn't run. Adds secure: false (AppConfig.secure is bool; ConfigOverrides.secure is Option<bool>) to the 9 AppConfig test initializers across {steamdeck,mister,windows} × {backend_config,service, sources}.rs. All 3 helpers' tests now compile. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Clears the
clippy::collapsible_if(-D warnings) atsyncer.rs:2818that went red once #13'ssecureE0063 was fixed and clippy could compile+run for the first time. It's in the sidecar-slot code (carried from #11), my lane.Fix
Collapse the two nested
if letinresolve_slot_name_for_syncinto a let-chain (if let Some(ext) = … && let Some(label) = sidecar_label(&ext)), matching the codebase's existing&& letstyle. Behavior identical (2-line net change).cargo clippy --all-targets -- -D warningsnow green on all 3 helpers (steamdeck-only lint; mister/windows were already clean). Signed, firewall-clean.🔴 Durability note (your call, carry owner)
This is an internal intarweb-dev PR, so it has the same sync-transience as #12 — the next sync re-carries the upstream sidecar code and would re-introduce the lint. The durable home is upstream, folded into the sidecar fix's joeblack2k PR (where #11 lives), so the collapse persists across regens. This internal PR greens intarweb-dev CI now; recommend folding the one-line collapse upstream into the sidecar PR (or the restore-behavior PR if that's where the sidecar code's next upstream touch lands) for permanence. Same pattern as the #12 logging → upstream decision.