Skip to content

Commit ee40268

Browse files
authored
fix(version-switch): refuse vanished targets (#1576)
## What this is PR 4 of #1570. A version can disappear after the picker loads but before the user selects it. The backend now performs a fresh exact-id availability check immediately before moving a shortcut binding onto an already-local target. A definitive target 404 refuses the switch. Temporary uncertainty still fails open so the offline **Switch anyway** path remains usable and bounded by one short attempt. ## What changed - Local targets receive exactly one executor-backed `get_rom_once(target_rom_id)` after all non-writing guards pass and immediately before the binding-write path. - Only `RommNotFoundError` returns: `{success: false, reason: "version_vanished", message: "This version is no longer available on RomM."}` - Timeout, transport, authentication, 5xx, malformed, empty, and wrong-id probe responses log uncertainty and fail open. - `allow_stranded` bypasses only the save-stranding guard; both **Sync now & switch** and **Switch anyway** retries still receive the final liveness check. - Server-only targets keep their mandatory full-detail fetch. Its typed 404 maps to the same `version_vanished` refusal; other failures remain classified normally. - A refusal leaves the binding, appId, launch options, artwork, cache, collections, playtime, rows, installs, saves, sync state, and completion stamps untouched. - All three frontend switch paths use the same failure handling. Only explicit `server_unreachable` updates global connection state. - Switch success no longer claims that RomM is connected when the optional probe may have failed open. - A vanished-target refusal reloads the picker directly without emitting `version_switched`. - All list loads share latest-request-wins publication, so a late refusal reload cannot overwrite a newer successful switch, another app's state, or a newer connection verdict. The active-target no-op is not probed, and no network request is held inside a SQLite unit of work. ## Safety The check narrows the stale-list window but does not pretend RomM and SQLite share a transaction. RomM can still delete an id after a successful response; retained local identity and the PR-3 recovery path keep that state recoverable. No library sync or fetch-stamp invalidation is introduced. ## Docs Updated backend architecture, Steam shortcut architecture, and the managing-games user guide. Frozen ADR history remains unchanged. ## Verification The full `mise run gate` passed on the implementation state: - 6395 backend tests - 2267 frontend tests - Ruff and basedpyright clean - TypeScript, ESLint, Prettier, Deno, build, size limit, import contracts, and repository invariant checks passed Review fixes then passed 100 focused backend/contract tests, 57 frontend tests, changed-file static/type/format checks, and an independent review with no remaining findings. ## On-device verification Passed in Game Mode with a safely staged local binding: a retained id that RomM answers with 404 was temporarily bound to the existing Steam appId, remained visible and disabled, and returned an honest `not_found` response on Download. Switching back to its live local sibling succeeded through the new pre-write target probe; the appId stayed stable, RomM was not reported offline, the picker guard released, and the database binding returned to the live id. No RomM data was deleted for the test. Closes part of #1570.
1 parent 4e54167 commit ee40268

9 files changed

Lines changed: 923 additions & 81 deletions

File tree

docs/architecture/backend-architecture.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,26 @@ excluded from default ranking. A bound-id 404 is a successful entity verdict (`b
142142
`server_query_failed: false`), not a global reachability signal. Artwork and save reads do not contribute availability
143143
evidence; `fetch_cover_base64` remains a nullable data callable.
144144

145+
#### Version-switch target liveness
146+
147+
`VersionSwitchService.switch_version` obtains a fresh exact-id verdict immediately before every actual binding move onto
148+
an already-local target. The save-stranding guard runs first, so its initial soft block performs no target request; both
149+
follow-up choices (`Sync now` retry and `Switch anyway`) re-enter the service and receive the same liveness guard.
150+
`allow_stranded` bypasses only save stranding. The active-target no-op, invalid local context, active download,
151+
non-member target, and target bound to another shortcut all stop before the request because none can enter the binding
152+
write.
153+
154+
The local-target request uses `RommRomReader.get_rom_once` on the worker executor: one attempt, three-second timeout, no
155+
retry-progress event, and no open UoW. Only `RommNotFoundError` returns the canonical `version_vanished` refusal.
156+
Timeout, transport/DNS/SSL, authentication, server errors, and malformed or empty successful payloads are logged and
157+
fail open, preserving the fast offline switch path. The existing short write UoW still rechecks membership and
158+
bound-elsewhere after the request; the request narrows the race but cannot form a transaction across RomM and SQLite.
159+
160+
A server-only target still needs its full detail before it can be validated and persisted. That mandatory fetch keeps
161+
the normal retry/classification policy and is not preceded by a redundant exact-id probe; only its typed 404 is peeled
162+
into `version_vanished`. A refusal performs no binding, row/install/applied-launch-options write, launch-command
163+
resolution, event, cache invalidation, sync, or completion-stamp update.
164+
145165
#### LibraryService decomposition (`services/library/`)
146166

147167
The library sync subsystem is a façade over three sub-services that coordinate through a shared `LibrarySyncStateBox`:

docs/architecture/steam-non-steam-shortcuts.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,22 @@ a shortcut still bound to a vanished id show the retained context while the user
151151
tab likewise skips positively vanished inactive installs before checking local drift, then continues through later live
152152
candidates.
153153

154+
The list verdict is advisory UI state, not authority for a later write. Immediately before `switch_version` moves the
155+
binding onto an already-local target, it checks that exact target id again through the same three-second, single-attempt
156+
`get_rom_once` path. The request runs on the worker executor outside the write UoW and after the save-stranding guard
157+
permits the attempt. Consequently an initial unsynced-save warning makes no target request, while both `Sync now` and
158+
`Switch anyway` retries are protected; `allow_stranded` never bypasses liveness. A typed target 404 returns
159+
`version_vanished` without changing the binding or any recorded launch state. Every other optional-probe outcome fails
160+
open, so a local switch remains fast when RomM is uncertain or offline. The active-target no-op does not probe because
161+
it moves no binding.
162+
163+
A server-only target already requires its full RomM detail for membership validation and row construction. That fetch
164+
keeps its normal retry policy, doubles as the liveness verdict, and receives no second probe. Its typed 404 produces the
165+
same `version_vanished` refusal; other failures retain their ordinary classified reason. Network I/O remains outside the
166+
short write UoW, whose fresh membership and bound-elsewhere checks still decide SQLite races. This leaves an unavoidable
167+
cross-system interval after a successful response: the liveness check reduces stale-list risk but is not a transaction
168+
with RomM.
169+
154170
## Sync-start reconcile of Steam-UI-deleted shortcuts
155171

156172
A user can delete a RomM shortcut through **Steam's own UI** (remove from library), which the plugin never observes. The

docs/user-guide/managing-games.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,14 @@ title and Region/Languages rows update to reflect it, and its cover refreshes to
6969
keeps its name, its place in your collections, and its playtime — all tied to the shortcut, which never changes. A
7070
single-version game shows no Version control.
7171

72+
The plugin checks the selected version again immediately before moving the shortcut. If RomM now answers that the exact
73+
version no longer exists, the switch is refused with **Could not switch version**, nothing about the shortcut changes,
74+
and the picker refreshes directly so the row becomes unavailable without starting a library sync. This protection also
75+
applies after **Sync now & switch** and **Switch anyway**; the latter bypasses only the unsynced-save warning. If RomM
76+
cannot provide a definitive answer because of a timeout, connection/sign-in/server error, or malformed response, the
77+
local switch is allowed to continue. In particular, the offline **Switch anyway** path does not wait through the normal
78+
retry sequence. A target-specific refusal does not by itself change the plugin's global online/offline status.
79+
7280
Switching **never deletes anything.** ROM files already on disk stay put, and save files are never moved or deleted by a
7381
switch. What happens depends on whether the version you're leaving is downloaded and whether its saves are synced:
7482

py_modules/services/version_switch.py

Lines changed: 64 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
markers the picker paints. Retained local rows are checked for ephemeral liveness
88
on each read; only an exact-ROM 404 marks one vanished. ``switch_version`` moves
99
the group's Steam-shortcut binding to a chosen sibling (the active version),
10+
rechecking a local target's exact-id liveness immediately before the write and
1011
persisting a server-only target first when needed.
1112
1213
A version switch is a pure binding move: the shortcut's name and appId stay
@@ -21,7 +22,8 @@
2122
resolved by ``sibling_group_key`` over the migration-010 index; the server view
2223
comes from ``get_rom(bound).sibling_roms``. Server I/O and the relaunch resolver
2324
run outside the Unit of Work (ADR-0006) — read/close, then fetch/resolve, then a
24-
short write UoW.
25+
short write UoW. The liveness request narrows the race but cannot transact with
26+
RomM and SQLite as one unit.
2527
"""
2628

2729
from __future__ import annotations
@@ -540,8 +542,10 @@ async def switch_version(self, app_id: int, target_rom_id: int, allow_stranded:
540542
first — the refusal carries ``server_reachable`` (from the reachability
541543
probe), ``unsynced_rom_id``, and ``unsynced_version_name`` so the frontend
542544
modal can offer "Sync now & switch" / "Switch anyway" / "Cancel".
543-
``allow_stranded`` is the "Switch anyway" override — honoured even offline
544-
(the switch is purely local). Saves are never deleted or transferred.
545+
``allow_stranded`` is the "Switch anyway" override for the save-stranding
546+
gate only. The local target still receives a short, single-attempt exact-id
547+
liveness probe; only a typed 404 refuses it, while uncertainty fails open so
548+
an offline switch stays fast. Saves are never deleted or transferred.
545549
546550
On success returns
547551
``{success: True, rom_id, target_installed, launch_options, app_id}``:
@@ -553,10 +557,10 @@ async def switch_version(self, app_id: int, target_rom_id: int, allow_stranded:
553557
target outside the group → ``not_in_group``; target bound to a *different*
554558
shortcut (grandfathered duplicate, ADR-0021 §5) → ``bound_elsewhere``; a
555559
server-only target whose detail the aggregate rejects → ``invalid_target``;
556-
a failed server-only target fetchthe :func:`classify_error` slug for the
557-
exception, so a server that answered 404 for a dropped id reads
558-
``not_found`` and only a genuine transport failure reads
559-
``server_unreachable`` (#1570).
560+
a definitive 404 for either target shape``version_vanished``; any other
561+
failed mandatory server-only target fetch → the :func:`classify_error` slug
562+
for the exception. The optional local probe instead fails open on every
563+
non-404 outcome (#1570).
560564
"""
561565
app_id = int(app_id)
562566
target_rom_id = int(target_rom_id)
@@ -588,21 +592,7 @@ async def switch_version(self, app_id: int, target_rom_id: int, allow_stranded:
588592
return self._switch_success(ctx.bound_rom_id, ctx.bound_installed, launch_options, app_id)
589593

590594
if ctx.target_is_local:
591-
# Fast reject an invalid local target before the drift/reachability
592-
# probes; the write UoW re-checks these same facts (TOCTOU).
593-
if not target_in_sibling_group(
594-
bound_group_key=ctx.group_key,
595-
target_group_key=ctx.target_group_key,
596-
target_is_local=True,
597-
target_is_server_sibling=False,
598-
):
599-
return self._not_in_group()
600-
if ctx.target_app_id is not None and ctx.target_app_id != app_id:
601-
return self._bound_elsewhere(target_rom_id)
602-
block = await self._save_stranding_block(ctx, allow_stranded)
603-
if block is not None:
604-
return block
605-
return await self._switch_local(app_id, target_rom_id, ctx.group_key)
595+
return await self._switch_to_local_target(app_id, target_rom_id, allow_stranded, ctx)
606596

607597
# Target not persisted locally — the save-stranding gate on the bound
608598
# version applies first (allow_stranded skips it), then fetch the detail,
@@ -612,7 +602,10 @@ async def switch_version(self, app_id: int, target_rom_id: int, allow_stranded:
612602
return block
613603
try:
614604
target_dict = await self._loop.run_in_executor(None, self._romm_api.get_rom, target_rom_id)
615-
except Exception as e: # classified: a 404 on the target is not an offline server
605+
except RommNotFoundError:
606+
self._logger.warning(f"Version switch: target rom {target_rom_id} is gone from the server")
607+
return self._version_vanished()
608+
except Exception as e: # mandatory fetch: preserve every non-404 classified failure
616609
self._logger.warning(f"Version switch: target fetch failed for rom {target_rom_id}: {e}")
617610
reason, message = classify_error(e)
618611
return {
@@ -640,6 +633,46 @@ async def switch_version(self, app_id: int, target_rom_id: int, allow_stranded:
640633
None, self._persist_and_bind, target_dict, app_id, ctx.platform_slug, ctx.group_key
641634
)
642635

636+
async def _switch_to_local_target(
637+
self, app_id: int, target_rom_id: int, allow_stranded: bool, ctx: _SwitchContext
638+
) -> dict[str, Any]:
639+
"""Validate a local target, guard its liveness, then enter the binding write."""
640+
# Fast reject invalid local state before drift/reachability probes; the
641+
# write UoW re-checks these same membership/collision facts (TOCTOU).
642+
if not target_in_sibling_group(
643+
bound_group_key=ctx.group_key,
644+
target_group_key=ctx.target_group_key,
645+
target_is_local=True,
646+
target_is_server_sibling=False,
647+
):
648+
return self._not_in_group()
649+
if ctx.target_app_id is not None and ctx.target_app_id != app_id:
650+
return self._bound_elsewhere(target_rom_id)
651+
block = await self._save_stranding_block(ctx, allow_stranded)
652+
if block is not None:
653+
return block
654+
target_vanished = await self._loop.run_in_executor(None, self._probe_switch_target_vanished, target_rom_id)
655+
if target_vanished:
656+
return self._version_vanished()
657+
return await self._switch_local(app_id, target_rom_id, ctx.group_key)
658+
659+
def _probe_switch_target_vanished(self, rom_id: int) -> bool:
660+
"""Probe one local switch target once; only a typed 404 refuses it."""
661+
try:
662+
response: Any = self._romm_api.get_rom_once(rom_id)
663+
except RommNotFoundError:
664+
return True
665+
except Exception as e:
666+
self._logger.warning(f"Version switch: target liveness probe failed open for rom {rom_id}: {e}")
667+
return False
668+
payload_id = response.get("id") if isinstance(response, dict) else None
669+
if type(payload_id) is not int or payload_id != rom_id:
670+
self._logger.warning(
671+
f"Version switch: target liveness probe returned an untrustworthy payload for rom {rom_id}; "
672+
"failing open"
673+
)
674+
return False
675+
643676
async def _save_stranding_block(self, ctx: _SwitchContext, allow_stranded: bool) -> dict[str, Any] | None:
644677
"""Soft-block the switch when the bound version has un-uploaded save drift.
645678
@@ -874,3 +907,11 @@ def _bound_elsewhere(target_rom_id: int) -> dict[str, Any]:
874907
"reason": "bound_elsewhere",
875908
"message": f"Version {target_rom_id} is already used by another shortcut.",
876909
}
910+
911+
@staticmethod
912+
def _version_vanished() -> dict[str, Any]:
913+
return {
914+
"success": False,
915+
"reason": "version_vanished",
916+
"message": "This version is no longer available on RomM.",
917+
}

src/api/backend.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -505,14 +505,21 @@ export interface SwitchVersionUnsyncedSaves {
505505
* `bound_elsewhere` (a grandfathered duplicate),
506506
* `invalid_target` (a server-only target the aggregate rejects),
507507
* `download_in_progress` (a group download is running — cancel it first), or
508+
* `version_vanished` (RomM definitively no longer has the target), or
508509
* `server_unreachable`. All surface via the picker's toast fallback
509510
* (`result.message`). The literal reason union excludes `unsynced_saves` so the
510511
* soft-block variant narrows cleanly.
511512
*/
512513
export interface SwitchVersionFailure {
513514
success: false;
514515
reason:
515-
"not_found" | "not_in_group" | "bound_elsewhere" | "invalid_target" | "download_in_progress" | "server_unreachable";
516+
| "not_found"
517+
| "not_in_group"
518+
| "bound_elsewhere"
519+
| "invalid_target"
520+
| "download_in_progress"
521+
| "version_vanished"
522+
| "server_unreachable";
516523
message: string;
517524
}
518525

0 commit comments

Comments
 (0)