fix(cloud-sync): remap web-player states into RetroArch's numbered load slots - #3933
Merged
gantoine merged 9 commits intoJul 24, 2026
Conversation
…ad slots States have no `slot` column (unlike saves), so a state created through RomM's own web player -- named with a display label and timestamp, not a RetroArch slot number -- was surfaced in the cloud-sync manifest under its raw file name. Verified live: RetroArch's Load State menu only ever offers numbered slots 0-999, so a synced file named otherwise downloads successfully but is never reachable from that menu. Mirrors the reasoning already used for the retroarch-webdav-romm shim project's manifest builder: states are grouped by (rom, emulator, slot suffix) via the file name's trailing .state/.state<N>/.state.auto pattern, and the newest state in each bucket -- regardless of whether it came from RetroArch itself or the web player -- is advertised under the canonical name RetroArch's own upload for that slot would carry. GET/DELETE resolve that canonical name back to the same bucket via resolve_state_by_slot, so serving and deleting agree with what the manifest advertised even when the underlying row's real file name differs.
RetroArch uploads a PNG screenshot alongside every state it syncs, named
`<state file name>.png` (e.g. `test_rom.state.png`). Verified live: this
file name doesn't match RetroArch's own `.state[N]`/`.state.auto` slot
pattern once naively split on the last dot ("test_rom.state" is not any
ROM's name), so `_resolve_rom` failed and every screenshot upload 409'd.
game_name_from_file_name now strips a trailing `.png` before applying the
existing state-suffix stripping, so it resolves the owning ROM the same way
it would for the state itself. Screenshots have no ROM-name-derived slot of
their own -- they ride along with whichever state `resolve_state_by_slot`
already picked for that (rom, emulator, slot), via the new
`resolve_state_screenshot_by_slot`, and are stored as a Screenshot (RomM's
own state-thumbnail model), not a State.
build_manifest now also advertises a `<canonical state name>.png` entry
for any state that has one attached, so RetroArch's own upload of it
succeeds and diffs correctly on subsequent syncs instead of failing (and
retrying) forever.
…g path PUT wrote every state upload under the canonical slot name RetroArch requested and only patched file_size_bytes on the matched existing row -- but that row's own file_name (e.g. a web-player upload's timestamped name) was left untouched. The new bytes landed on disk under a different path than the DB row pointed at, so the row's real content and what's actually on disk silently diverged. Verified live: this surfaced as a state that could never stop being a 'Conflicting change' on every subsequent sync, no matter how many times it was re-uploaded -- each upload wrote to the canonical path again while the tracked row (and its real save data) still pointed at the old, now-stale file, so RomM's own view of "the current state" never actually caught up to what RetroArch kept sending. Now resolves the existing row first and writes to *its* real file name when one exists, only falling back to the canonical name for a genuinely new row. Same fix applied to the state-screenshot upload path, which had the identical bug (screenshot identity resolved by exact canonical-name match instead of the owning state's real name).
…h sync
PPSSPP doesn't save a single file per game like every other core -- it
mirrors a real PSP memory stick under saves/<core>/PSP/SAVEDATA/<slot>/
as several small files (PARAM.SFO, the actual save data, ICON0.PNG, ...)
that only make sense as a set, plus saves/<core>/PSP/SYSTEM/CACHE/ holds
pure engine caches with no save data at all. Neither fits the existing
one-WebDAV-path-per-asset model.
Ports the retroarch-webdav-romm shim's pspSave.ts approach: a save
folder's files are bundled into a single zip stored as one RomM Save
(slot=None), and unbundled again for GET/manifest purposes. The rom is
resolved from the folder's PARAM.SFO TITLE (normalized-matched against
roms on the psp platform) with PSP_SERIAL_MAP as an explicit override and
fallback for titles that don't normalize-match automatically. Files that
arrive before the folder resolves (RetroArch doesn't guarantee PARAM.SFO
lands first) are buffered on disk until it does, and merged in once
resolved.
Manifest lists each bundle member as its own {path, hash} entry, since
RetroArch diffs per-file, not per-bundle.
Also fixes a latent zipfile bug this surfaced: zipfile_inflate64 (pulled
in elsewhere for ROM archive reading) replaces zipfile._get_compressor
with a signature CPython 3.13's own ZipFile.writestr() can't call --
RomM already has a shim for this (utils/zip_cache.py's
_ensure_zipfile_writable), just needed calling here too.
gantoine
self-requested a review
July 24, 2026 14:59
Author
|
Verified the PSP bundling live against a real PPSSPP capture (Patapon (Europe), serial UCES00995) synced from an actual RetroArch 1.22.2 client to a deployed instance of this branch:
No code changes from this — just confirming the buffering/merge logic (previously only unit- and mock-tested) holds up against real out-of-order PPSSPP upload traffic, not just the synthetic ordering my tests assumed. |
…brary
RetroArch's own Cloud Sync client never issues PROPFIND (verified against
its source, already noted in this router's docstring), so this isn't on
RetroArch's actual sync path -- it's for a real WebDAV client (iOS Files'
"Connect to Server", Cyberduck, ...) to mount the same /api/cloud-sync
URL and browse/download the library as plain files, same as the
retroarch-webdav-romm shim's romBrowser.ts + webdavXml.ts.
Adds PROPFIND for roms/<platform>/<file> (RomM's own library, read-only --
no PUT/DELETE) and saves/states/ (the current cloud-sync manifest; unlike
the shim, only current entries are browsable here, not full history --
RomM's own web UI covers that). LOCK/UNLOCK are a fake always-succeeds
handshake some WebDAV clients require before they'll mount a server at
all, matching the shim.
GET/HEAD for a rom file redirects (307) to RomM's existing
/api/roms/{id}/content/{file_name} endpoint rather than reimplementing
Range support, multi-file zip caching and (in production) nginx
X-Accel-Redirect -- Basic Auth carries over on the redirect since that
endpoint already accepts it alongside OAuth.
Found and fixed along the way: Rom.has_multiple_files / .files depend on
columns/relationships get_roms_scalar doesn't eager-load, so accessing
them outside the query's own session raised DetachedInstanceError --
re-fetch the visibility-filtered ids via get_roms_by_ids (which does
eager-load them) instead of using the raw scalar results directly.
…ilter # Conflicts: # backend/endpoints/cloud_sync.py
2 tasks
…orever Verified live with a real WebDAV client (Cyberduck): every PropfindEntry href was relative to this router's own mount point (e.g. "roms/", "saves/Snes9x/"), not absolute from the server root the way WebDAV clients expect a <D:href> to be. The client couldn't match a "self" entry's href back to the path it had just requested, so it rendered that entry as an extra nested subfolder instead of recognizing it as the current directory -- and since every subfolder has the same mismatch, browsing into it produced another apparent copy of the whole tree, forever. Prefixing every href with this router's actual mount path (/api/cloud-sync) fixes the self-entry match and ends the loop. Also fixed along the way: href path segments were never percent-encoded, unlike the retroarch-webdav-romm shim's own hrefEscape (which used encodeURIComponent) that this was supposed to mirror -- a save/state/rom filename containing a space or other reserved character would have produced invalid hrefs.
sanitize_filename treated "+" as an invalid character alongside the actual Windows-reserved set (< > : " / \ | ? *), which broke cloud-sync's filename-based ROM matching for any title containing one -- RomM itself stores such ROMs (e.g. combo carts like "Super Mario All-Stars + Super Mario World") on disk with the "+" intact, so stripping it before resolving the ROM caused every save/state upload for that game to 409. Confirmed live against production (romm.vmyaman.com) via a real RetroArch client.
Keeping the test-file footprint down; the production fix (dropping "+" from sanitize_filename's invalid-char set) stands on its own and is covered live.
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.
Summary
Follow-up to #3931 (merged). Found while live-testing that PR's merged branch against a real RetroArch client: a state created through RomM's own web player (EmulatorJS) carries a display label + timestamp in its file name, not a RetroArch slot number. Since the
Statemodel has noslotcolumn (unlikeSave), such a state was surfaced in the cloud-sync manifest under its raw file name.Reproduced live: RetroArch's Load State menu only ever offers numbered slots 0-999. A web-player state synced this way downloads successfully to disk but is never reachable from that menu -- it just silently sits in the sync folder.
Fix
Mirrors the equivalent (already-shipped, already-live) fix in the
retroarch-webdav-rommshim project's manifest builder: states are grouped by(rom, emulator, slot suffix), where the slot suffix is derived from the file name's trailing.state/.state<N>/.state.autopattern. The newest state in each bucket -- regardless of whether it came from RetroArch itself or the web player -- is advertised in the manifest under the canonical name RetroArch's own upload for that slot would carry (<rom>.state,<rom>.state3,<rom>.state.auto, ...).GET/DELETE resolve that canonical name back to the real underlying row via a new
resolve_state_by_slot, so serving and deleting always agree with what the manifest advertised, even when the underlying row's actualfile_namediffers from the canonical one.Saves are untouched -- they already have a real
slotcolumn and are filtered correctly (slot is Noneexcludes web-player saves from the manifest).Test plan
TestCloudSyncStateSlotResolution,test_remaps_web_player_state_to_canonical_slot,test_newest_state_in_a_slot_wins_regardless_of_origin)ruff checkclean