Repository navigation
feat(profiles): refuse to write a profile this version cannot safely save - #11
Merged
Merged
Conversation
…save A profile now records the newest Slidr that wrote it (`app_version`, raised on save and never lowered). Opening one from a newer build warns — and, more to the point, stops Slidr writing to it. The warning alone would not help. Loading a newer profile is harmless, because serde drops fields it does not know; the damage happens on the *next save*, which writes the profile back without them. So every write to the active profile now goes through `Shared::save_preset`, which refuses while the guard is armed — including the save on exit, and including renaming or importing such a profile (both rewrite the whole file). While looking for a way to test this, a worse variant turned up: a profile that fails to parse was silently replaced by a *blank* profile of the same name, which the exit save then wrote over the file. One unknown enum value — exactly what a newer version introduces — was enough to lose a profile. That path is now guarded the same way, with the parse error shown and logged instead of swallowed, and the file left untouched. Either way the user decides: "View only" keeps the guard (a banner on the settings page says changes are not being saved), or they take ownership, which stamps the profile with this version since the newer data is dropped by that save regardless. Upgrades are a non-event: an older stamp, or none at all, loads without comment. Verified against a throwaway config with three profiles — stamped 99.0.0, stamped 0.1.0, and one with an unknown curve preset. The two blocked cases left the file byte-identical after a graceful quit (the unknown field still in it); the old profile was rewritten with the stamp raised to this version. Co-Authored-By: Claude Opus 5 (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
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.
Opening a profile written by a newer Slidr with an older build usually destroys
it. This makes that impossible without an explicit decision.
The stamp
Preset.app_versionrecords the newest Slidr that has written the profile. Itis raised on every save and never lowered — otherwise an old build would
erase the evidence that a newer one had been there. Missing or unparseable
stamps count as "not newer", so profiles written before this existed keep
loading, and a corrupt stamp cannot lock anyone out.
Upgrades are deliberately a non-event: an older stamp loads without comment.
Why a warning alone would not be enough
Loading a newer profile is harmless — serde ignores fields it does not know.
The damage happens on the next save, which writes the profile back without
them. So the guard sits on writing, not reading:
Shared::save_preset, whichrefuses while the guard is armed (17 call sites in
glue.rs, plus the save onexit in
main.rs),under a new name, same hazard,
would hand back a quietly truncated profile. The source file is left alone.
The worse variant this turned up
While building a test fixture, an unknown enum value made
load_presetfail —and the old code replaced the profile with a blank one of the same name,
which the exit save then wrote over the file. One unknown
curve_presetvalue,exactly the kind of thing a newer version introduces, was enough to lose a
profile silently.
That path is now guarded identically: the parse error is logged and shown
instead of swallowed, and the file is not written.
What the user sees
A dialog (not dismissable by clicking the backdrop, since ignoring it costs you
the profile) naming both versions, or the parse error. Two choices:
says changes are not being saved, with a button to reopen the dialog.
stamps the profile with this version, because the newer data is dropped by
that save regardless. Warning about it every launch after that would be noise.
Verification
src/version.rshas unit tests for the comparison (partial versions,-rc/+buildsuffixes, empty and broken stamps,
higher()never lowering a stamp). 48 testspass on Windows.
End-to-end against a throwaway
SLIDR_CONFIG_DIRwith three profiles:curve_presetThe graceful quit matters: it is the exit-save path that used to do the damage.
🤖 Generated with Claude Code