chore(saves): URL encoding, ISO helper, dead code cleanup#264
Merged
danielcopper merged 9 commits intomainfrom May 5, 2026
Merged
chore(saves): URL encoding, ISO helper, dead code cleanup#264danielcopper merged 9 commits intomainfrom
danielcopper merged 9 commits intomainfrom
Conversation
User-typed slot values (e.g. "Mom & Dad", "draft=1") and server-issued device IDs were being interpolated raw into query strings, allowing reserved URL characters to corrupt the request. Wrap each value with urllib.parse.quote(safe="") in list_saves, upload_save, and get_save_summary. device_id is normally a UUID; encode it defensively.
domain.sync_action, domain.save_status, and services.saves all had near-duplicate ISO-8601 parsing — and three of them used lexical max() on raw timestamp strings, which silently breaks once timezones differ. Move parse_iso / parse_iso_to_epoch into lib.iso_time and switch all "newest server save" picks to epoch-keyed comparisons. Unparseable timestamps now sort to the bottom (0.0) instead of polluting the order. Allow domain → lib in importlinter: lib now hosts layer-agnostic helpers shared by domain and services.
The frontend never calls setGameSlot — only the export and the matching @migration_blocked wrapper existed. Slot changes go through switchSlot exclusively, which already calls SaveService.set_game_slot internally (and that service method stays).
The backend switch_slot only ever returns sync_disabled, not_installed, pending_uploads, or server_unreachable. The unresolved_conflicts branch in SavesTab and the union member in SwitchSlotResponse have always been dead.
domain.save_conflicts was orphaned by the 0.16 newest-wins rewrite (#253). Nothing in py_modules, main.py, or tests imports it apart from its own test file. The SaveConflict model class is unrelated and lives in models.saves — still in use across services.saves.
Mirrors the encoding applied in 04c1013 to list_saves, upload_save, and get_save_summary. download_save_content and update_device interpolated device_id raw — same UUID source, encode defensively for symmetry.
Aligns upload_save with the rest of the file — every other quote() call passes safe="". Existing test updated to assert the slash gets encoded to %2F too.
Adds tests for the two uncovered branches in domain/save_status.py: - Malformed timestamp -> _format_time_ago returns None -> 'Not synced' fallback - Naive timestamp (no tzinfo) -> treated as UTC, no crash The 'Just now' branch was already covered by test_synced_just_now. Branch coverage on save_status.py now 100%.
Replaces datetime.fromisoformat with parse_iso for symmetry with the rest of the codebase (introduced in df0cdec). Adds an explicit None guard for unparseable input — semantics preserved (still falls into the 'Failed to calculate session duration' path), and adds a regression test that didn't exist before.
|
4 tasks
danielcopper
added a commit
that referenced
this pull request
May 6, 2026
* chore(saves): remove conflict_mode setting The 0.16 newest-wins matrix sync rewrite removed the only meaningful consumer of conflict_mode (resolve_conflict_by_mode in domain/save_conflicts, deleted in #264). The remaining frontend gate kept the launch-time conflict check optional; that check should always run, so drop the setting end-to-end and let the interceptor block launches whenever a save conflict is pending. * chore(saves): remove clock_skew_tolerance_sec setting The only consumer (resolve_conflict_by_mode in domain/save_conflicts) was deleted with the 0.16 newest-wins matrix sync rewrite (#264). Drop the setting from the dataclass, defaults, sanitiser, and frontend type. * chore(saves): strip legacy settings keys on state load load_state merges persisted settings via dict.update, so old state files keep removed keys (conflict_mode, clock_skew_tolerance_sec) forever. Pop them in _migrate_loaded_state so the next save_state writes a clean file. * chore(saves): polish migration strip - guard, coverage, doc - Drop the throwaway empty-dict fallback from the legacy settings strip in `_migrate_loaded_state`; missing/non-dict settings now cleanly fall through the `isinstance` guard. - Pin the defensive guard with two tests: missing `settings` key and non-dict `settings` value. * refactor(saves): reduce _migrate_loaded_state cognitive complexity Extract the saves-block migration and the legacy-settings strip into private helpers (_migrate_saves_entries, _strip_legacy_settings). _migrate_loaded_state becomes a thin dispatcher. Cognitive complexity 22 -> below threshold (Sonar python:S3776).
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
1 participant
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
PR 1 of 2 for #258 (post-0.16 save sync polish).
list_saves,upload_save,get_save_summary,download_save_content,update_device). Previously interpolated raw — real bug for slot names containing&/=/ space; defensive on UUIDs.lib/iso_time(parse_iso,parse_iso_to_epoch) replaces four lexicalmax(updated_at)callsites with epoch-based comparison. Consolidates duplicate parsing acrossdomain/sync_action,services/saves,services/playtime,domain/save_status.domain → liballowed in.importlinter(lib stays independent of every other layer).set_game_slotDecky callable (no frontend caller; service method stays, still used internally byswitch_slot), theunresolved_conflictsswitchSlotreason (backend never returned it), and the entirely deaddomain/save_conflicts.pymodule (replaced by the matrix model in 0.16). Wiki references inBackend-Architecture.md/Development.mdupdated in a separate wiki commit._format_time_ago(parse failure + naive timestamp paths).I-1 from the parent issue was a docs-only fix and shipped earlier in the wiki. I-3 (
conflict_modesetting removal) is deferred to PR 2 because it cuts across model, persistence, settings UI, and launch interceptor logic.Closes I-4, I-5, I-6, I-7 from #258.
Test plan
unresolved_conflictsreason removal (no console errors)&/space round-trips through list/upload without server-side parse error