From 2fcaada7e8331b40dbe270a150f7b43c26c8b664 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Wed, 19 Aug 2026 08:37:43 -0400 Subject: [PATCH] Snapshot automatically before restoring defaults Restoring defaults clears every preference and the Home accessory store, and nothing in the app can undo it. Snapshots existed but were entirely manual, so the one irreversible action Panama offers was also the one with no safety net. It now snapshots first. Not fatal if that fails: someone who asked to reset gets their reset, and a snapshot that could not be written is reported rather than allowed to block what they asked for. The wiring is inverted deliberately. SettingsBackup already references SystemSettings, so referencing it back would make two singletons depend on each other, which is an initialisation-order problem waiting to happen. Instead SystemSettings exposes a seam defaulting to a no-op and SettingsBackup installs itself into it at startup -- the same shape as the seams the reset path already uses for test isolation. The contract asserts the snapshot is FIRST in the call sequence, not merely present. A snapshot taken after the stores were cleared would faithfully record the wiped state as the user's own, which is worse than no snapshot: it looks like a safety net and is a copy of the damage. Verified against both mutations -- removing the snapshot, and moving it after the wipe. One trap, hit for the third time today: QML allows only one Component.onCompleted per object, and SettingsBackup already had one. Adding a second does not fail locally -- it poisons the entire services module, so every singleton reports "Type X unavailable" and the real error is the last line of a forty-line cascade. Claude-Session: https://claude.ai/code/session_01BRvzt4H8XXLPVH5MyYdk9L --- .../quickshell/services/SettingsBackup.qml | 20 ++++++++++++++++++- .../quickshell/services/SystemSettings.qml | 18 +++++++++++++++++ .../quickshell/settings-system-harness.qml | 6 ++++++ .../settings-commit-reset-contract.sh | 7 +++++++ 4 files changed, 50 insertions(+), 1 deletion(-) diff --git a/config/dot/quickshell/services/SettingsBackup.qml b/config/dot/quickshell/services/SettingsBackup.qml index 985b4dc..6d04241 100644 --- a/config/dot/quickshell/services/SettingsBackup.qml +++ b/config/dot/quickshell/services/SettingsBackup.qml @@ -181,7 +181,24 @@ Singleton { } } - Component.onCompleted: root.refresh() + Component.onCompleted: { + root.refresh(); + + // Restoring defaults is the only irreversible action Panama offers, and + // it lives in SystemSettings -- which must not reference this singleton, + // since this one already references it. So the capability is PUSHED + // there rather than pulled from here. + // + // Returns false when a snapshot is already running rather than queueing: + // the caller is about to wipe the stores, and a snapshot landing after + // that would record the wiped state as if it were the user's. + SystemSettings.takeSafetySnapshot = function() { + if (actionRun.running) + return false; + root.save(); + return true; + }; + } function refresh(): void { if (!listQuery.running) @@ -195,6 +212,7 @@ Singleton { actionRun.exec([root.helperPath, "save", root.serializeHomeState()]); } + function serializeHomeState(): string { const current = root.readHomeState(); const favorites = []; diff --git a/config/dot/quickshell/services/SystemSettings.qml b/config/dot/quickshell/services/SystemSettings.qml index b511e3d..a778514 100644 --- a/config/dot/quickshell/services/SystemSettings.qml +++ b/config/dot/quickshell/services/SystemSettings.qml @@ -38,6 +38,13 @@ Singleton { property var displayBusy: function() { return Displays.busy || Displays.awaitingConfirmation; } property var readDisplays: function() { return DesktopPreferences.get("displays"); } property var protectDisplays: function(value) { return DesktopPreferences.set("displays", value); } + + // Installed by SettingsBackup at startup. A default no-op rather than a + // direct reference, because SettingsBackup already references this + // singleton and a mutual reference between two singletons is an + // initialisation-order problem waiting to happen. Tests override it the + // same way they override the seams above. + property var takeSafetySnapshot: function() { return false; } property var setDisplayBlocked: function(blocked) { Displays.externalChangeBlocked = blocked; } property var reloadKeybinds: function() { Keybinds.applyReload(); } property var keybindsReloading: function() { return Keybinds.reloading; } @@ -509,6 +516,17 @@ Singleton { return false; } + // Snapshot before wiping. Restoring defaults clears every preference + // and the Home accessory store, and there is no undo for it anywhere in + // the app -- so the one automatic snapshot Panama takes is the one taken + // immediately before the only irreversible action it offers. + // + // Deliberately not fatal if it fails: a user who asked to reset should + // get their reset, and a snapshot that could not be written is reported + // rather than allowed to block the thing they asked for. + if (!root.takeSafetySnapshot()) + console.warn("SystemSettings: could not snapshot before restoring defaults"); + root.setDisplayBlocked(true); DesktopPreferences.resetDesktopDefaults(); diff --git a/config/dot/quickshell/settings-system-harness.qml b/config/dot/quickshell/settings-system-harness.qml index a500fc6..c8579dd 100644 --- a/config/dot/quickshell/settings-system-harness.qml +++ b/config/dot/quickshell/settings-system-harness.qml @@ -43,6 +43,12 @@ ShellRoot { root.recordReset("display.block:" + blocked); root.displayBlocked = blocked; }; + // Recorded so the contract can prove the snapshot happens BEFORE the + // stores are cleared, not merely that it happens. + SystemSettings.takeSafetySnapshot = function() { + root.recordReset("snapshot"); + return true; + }; SystemSettings.reloadKeybinds = function() { root.recordReset("keybinds.reload"); }; SystemSettings.keybindsReloading = function() { return false; }; SystemSettings.applyWallpaper = function(path) { root.recordReset("wallpaper.set:" + path); }; diff --git a/tests/quickshell/settings-commit-reset-contract.sh b/tests/quickshell/settings-commit-reset-contract.sh index 67e8510..d3a6b41 100755 --- a/tests/quickshell/settings-commit-reset-contract.sh +++ b/tests/quickshell/settings-commit-reset-contract.sh @@ -136,7 +136,14 @@ jq -e '.count == 1 and .initialized == true' <<<"$home_before" >/dev/null \ sleep 0.6 reset_state="$(qs_for_harness ipc call settings-system-test resetState)" +# The snapshot must come FIRST. Restoring defaults is the only irreversible +# action Panama offers, and a snapshot taken after the stores were cleared would +# faithfully record the wiped state as if it were the user's. +jq -e '.calls[0] == "snapshot"' <<<"$reset_state" >/dev/null \ + || fail "reset did not snapshot before wiping the stores: $reset_state" + jq -e '.calls == [ + "snapshot", "display.block:true", "keybinds.reload", "wallpaper.set:",