Close out the remaining audit findings - #46
Merged
Conversation
Six issues from the release-readiness audit, all of which had survived an adversarial challenge round. CI now installs rclone, and a skip in the cloud test module is a hard failure (#44). That module is the only place upload_save_cas, download_save_cas, prune_remote_versions and gc_blobs are exercised, and no runner had rclone, so the functions that can delete a save had less coverage than the ones that had already shipped four hotfixes. A new test file also pins every fix released in 0.6.1 through 0.6.3 at the call site rather than at the helper: reverting the 0.6.3 vbs fix used to leave 229 tests green. The watcher was inverted on Linux (#40). is_system_executable tested only for "/windows/", so nothing was excluded off Windows and /usr/libexec/xdg-desktop- portal matched the title "Portal" - a match that is then learned and written to games.yaml, leaving the game permanently "running" and its cloud restore silently disabled. It now excludes the OS directories by prefix, anchored so a game under /home, /opt or a mounted drive is untouched. Separately, title_matches_process never read its name argument, so under Proton, where the executable path is the wine loader, no game matched at all. The name is now weighed as one more segment under the same whole-token rules. Two silent failures (#41). A game whose saves have moved produced an empty Ludusavi scan, which was reported as "No changes detected since last backup" - indistinguishable from success, every session, forever. That is now a failure with a message that says what probably happened. And a callback raising inside the watcher was logged and swallowed while the tray still showed the green "Playing" state; it now escalates to the tray and fires a notification, which is what the tray exists for. Cross-machine remapping only matched Windows drive-letter paths (#42), so a Steam Deck restoring onto a desktop reported success and wrote the save under the other machine's home. POSIX homes are handled now. One existing test had asserted the old no-op as correct; that assertion encoded the bug and has been updated rather than worked around. A future-stamped version permanently stalled sync (#39). Version ids are wall clock and the policy orders them as strings, so one machine booting with a dead RTC pinned every other machine's "newest seen" to a date that never arrives. Cloud versions stamped more than a day ahead are now refused, with the reason logged. A day of tolerance absorbs ordinary clock disagreement between machines while excluding the years-ahead stamps a broken RTC produces. An existing test used a year-2099 id as a convenient "definitely newer" value; that fixture now uses a plausible one. CLOUD_FORMAT.md claimed a device writing the layout "gets restores on the desktop for free, because gsg cannot tell the difference" (#43). It can tell: every restore is gated on a Ludusavi mapping.yaml or the custom-game manifest, and a CAS-only upload carries neither. The claim is corrected in place rather than deleted, because it was published and repeated to an external contributor who may have built against it. Making a CAS manifest sufficient on its own stays open as #43. The README repeated both this and the Windows-only remap claim; both now match the code. Mutation-tested: reverting each of the six fixes turns the suite red, as does reverting any of the five fixes shipped in 0.6.1 through 0.6.3.
_install_startup writes a systemd user service off Windows, not a .vbs, so the test failed on the Linux legs - and worse, it wrote a real unit into the runner's ~/.config. A test has no business doing that. The structural assertion above it covers both platforms.
Merged
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.
Closes #39. Closes #40. Closes #41. Closes #42. Closes #43. Closes #44.
The six that survived the challenge round of the release-readiness audit, after #45 took the three data-loss ones.
CI can now catch a delete-path regression (#44)
rclone is installed on every matrix leg, and a skip in
test_cloud_cas.pyis now a hard failure — that module is the only placeupload_save_cas,download_save_cas,prune_remote_versionsandgc_blobsrun, and no runner had rclone. The code that can delete a save had less coverage than the code that had already shipped four hotfixes.The other half:
tests/test_shipped_fixes_stay_fixed.pypins every fix from 0.6.1–0.6.3 at the call site, not the helper. Reverting the 0.6.3.vbsfix used to leave 229 tests green.The watcher was inverted on Linux (#40)
is_system_executabletested only for"/windows/", so off Windows nothing was excluded and/usr/libexec/xdg-desktop-portalmatched the title "Portal". That match gets learned intogames.yaml, leaving the game permanently "running" and its cloud restore silently disabled. Now excluded by path prefix, anchored so a game under/home,/optor a mounted drive is untouched.Separately,
title_matches_processnever read itsnameargument. Under Proton the executable path is the wine loader, so the title appears nowhere in it and no Proton game matched at all — while native Linux games worked, making the failure look random. The name is now weighed as one more segment under the same whole-token rules.Two silent failures (#41)
A game whose saves have moved produces an empty Ludusavi scan, which was reported as
No changes detected since last backup— indistinguishable from success, every session, indefinitely. Now a failure that says what probably happened.A callback raising inside the watcher was logged and swallowed while the tray still showed the green "Playing" state. It now escalates to the tray and fires a notification.
Cross-machine remap was Windows-only (#42)
/home/deck/...never matched, so a Steam Deck restoring onto a desktop reported success and wrote the save under the other machine's home. POSIX homes handled now.One existing test asserted the old no-op as correct — that assertion encoded the bug, and is updated rather than worked around.
A future-stamped version stalled sync permanently (#39)
One machine booting with a dead RTC pinned every other machine's "newest seen" to a date that never arrives, so
gsg pullsaid "already up to date" forever with no recovery short of editing the bucket. Cloud versions stamped more than a day ahead are refused, with the reason logged — enough tolerance for ordinary clock disagreement, far short of a broken RTC.An existing test used a year-2099 id as a convenient "definitely newer" value; that fixture now uses a plausible one.
CLOUD_FORMAT.md promised something untrue (#43)
It said a device writing the layout "gets restores on the desktop for free, because
gsgcannot tell the difference". It can: every restore is gated on a Ludusavimapping.yamlor the custom-game manifest, and a CAS-only upload carries neither.Corrected in place rather than deleted, because it was published and repeated to an external contributor in #32 who may have built against it. What a device actually gets today — valid, deduplicated, GC-safe storage — is now stated plainly, and making a CAS manifest sufficient on its own stays tracked. The README repeated this and the Windows-only remap claim; both now match the code.
Verification
281 tests, ruff and mypy clean. Mutation-tested — reverting any of these turns the suite red:
plus the five already-shipped fixes from 0.6.1–0.6.3, all caught.
One thing worth flagging: my first version of the wiring tests used
tomllib, which is 3.11+ while this project supports 3.10 and CI runs it. Caught by mypy locally rather than by the runner.