From 6d3f888784e38e321b3f0ff652c4d6a9bbdba463 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Tue, 18 Aug 2026 03:10:31 -0400 Subject: [PATCH] Protect live state during restore and reset --- .../quickshell/scripts/panama-settings-backup | 17 +++++++ config/dot/quickshell/services/Displays.qml | 5 ++ .../quickshell/services/SettingsBackup.qml | 25 ++++++++++ .../quickshell/services/SystemSettings.qml | 49 ++++++++++++++++++- config/dot/quickshell/services/Wallpaper.qml | 17 +++---- .../quickshell/settings-backup-harness.qml | 28 ++++++++++- .../quickshell/settings-system-harness.qml | 36 +++++++++++++- tests/quickshell/settings-backup-contract.sh | 8 +-- .../settings-backup-live-contract.sh | 15 ++++++ .../settings-commit-reset-contract.sh | 29 ++++++++++- 10 files changed, 212 insertions(+), 17 deletions(-) diff --git a/config/dot/quickshell/scripts/panama-settings-backup b/config/dot/quickshell/scripts/panama-settings-backup index db63077..7a57fd0 100755 --- a/config/dot/quickshell/scripts/panama-settings-backup +++ b/config/dot/quickshell/scripts/panama-settings-backup @@ -572,6 +572,23 @@ def command_restore(arguments: list[str]) -> None: home_present = False home_data = None + # Display geometry is never restored from a snapshot. Applying it requires + # the visible confirmation/recovery flow in Displays.qml; a settings-file + # restore followed by `hyprctl reload` must not bypass that safety boundary. + # Preserve the currently confirmed generation when it is readable, and + # otherwise remove the snapshot's geometry so startup uses shipped policy. + if desktop_present and desktop_data is not None: + desktop_data = dict(desktop_data) + try: + current_desktop = read_json(SETTINGS, "The current settings file") \ + if is_present(SETTINGS) else {} + except BackupError: + current_desktop = {} + if "displays" in current_desktop: + desktop_data["displays"] = current_desktop["displays"] + else: + desktop_data.pop("displays", None) + # Restoring remains undoable, but a corrupt current file must not prevent a # known-good snapshot from recovering the desktop. save_snapshot(require_any=False, validate=False) diff --git a/config/dot/quickshell/services/Displays.qml b/config/dot/quickshell/services/Displays.qml index 19c40b8..56e93f3 100644 --- a/config/dot/quickshell/services/Displays.qml +++ b/config/dot/quickshell/services/Displays.qml @@ -41,6 +41,7 @@ Singleton { property bool revertVerificationActive: false property int operationGeneration: 0 property int revertGeneration: -1 + property bool externalChangeBlocked: false property int secondsLeft: 0 readonly property bool awaitingConfirmation: root.pendingOutput !== "" @@ -271,6 +272,10 @@ Singleton { // Applies immediately and starts the countdown. Nothing is stored yet: the // settings file is only written by confirm(). function apply(output: string, mode: string, scale: real, transform: int): bool { + if (root.externalChangeBlocked) { + root.lastError = "Wait for Settings to finish restoring before changing a display."; + return false; + } if (root.busy) { root.lastError = "Wait for the current display operation to finish."; return false; diff --git a/config/dot/quickshell/services/SettingsBackup.qml b/config/dot/quickshell/services/SettingsBackup.qml index 3526706..b5ed82e 100644 --- a/config/dot/quickshell/services/SettingsBackup.qml +++ b/config/dot/quickshell/services/SettingsBackup.qml @@ -38,6 +38,10 @@ Singleton { property var initializeHome: function(ids) { HomePreferences.initialize(ids); } property var aliasHome: function(id, alias) { HomePreferences.setAlias(id, alias); } property var reloadDesktop: function() { DesktopPreferences.reload(); } + property var readDisplays: function() { return DesktopPreferences.get("displays"); } + property var protectDisplays: function(value) { return DesktopPreferences.set("displays", value); } + property var displayBusy: function() { return Displays.busy || Displays.awaitingConfirmation; } + property var setDisplayBlocked: function(blocked) { Displays.externalChangeBlocked = blocked; } property var applyCompositor: function() { SystemSettings.applyPersistedDisplayPolicy(); } property var reloadKeybinds: function() { Keybinds.applyReload(); } property var keybindsReloading: function() { return Keybinds.reloading; } @@ -47,6 +51,7 @@ Singleton { } property var applyWallpaper: function(path) { Wallpaper.set(path); } property var reloadShell: function() { Quickshell.reload(false); } + property var protectedDisplays: ({}) readonly property bool busy: listQuery.running || actionRun.running || applyRestoredState.running || settleReload.running @@ -81,6 +86,10 @@ Singleton { root.lastError = actionRun.restoring ? "That snapshot could not be restored." : "The settings could not be backed up."; + if (actionRun.restoring) { + root.setDisplayBlocked(false); + root.protectedDisplays = ({}); + } return; } root.lastAction = actionRun.restoring ? "restored" : "saved"; @@ -89,6 +98,10 @@ Singleton { root.lastError = homeReloaded ? "" : "Desktop settings were restored, but Home favourites could not be reloaded."; + if (!homeReloaded) { + root.setDisplayBlocked(false); + root.protectedDisplays = ({}); + } } else root.lastError = ""; root.refresh(); @@ -124,6 +137,8 @@ Singleton { // from leaving restored Home state stale indefinitely. if ((!root.keybindsReloading() && !root.systemBusy()) || attempts >= 30) { stop(); + root.setDisplayBlocked(false); + root.protectedDisplays = ({}); root.reloadShell(); } } @@ -162,6 +177,8 @@ Singleton { if (!root.reloadHomeState(text)) return false; root.reloadDesktop(); + if (!root.protectDisplays(root.protectedDisplays)) + return false; applyRestoredState.restart(); return true; } @@ -222,10 +239,18 @@ Singleton { function restore(name: string): bool { if (actionRun.running) return false; + if (root.displayBusy()) { + root.lastError = "Finish the current display change before restoring settings."; + return false; + } if (!root.snapshots.some(snapshot => snapshot.name === name)) { root.lastError = "That snapshot is not in the list."; return false; } + const currentDisplays = root.readDisplays(); + root.protectedDisplays = JSON.parse(JSON.stringify( + currentDisplays && typeof currentDisplays === "object" ? currentDisplays : {})); + root.setDisplayBlocked(true); actionRun.restoring = true; actionRun.exec([root.helperPath, "restore", name]); return true; diff --git a/config/dot/quickshell/services/SystemSettings.qml b/config/dot/quickshell/services/SystemSettings.qml index 2933ab3..29d4864 100644 --- a/config/dot/quickshell/services/SystemSettings.qml +++ b/config/dot/quickshell/services/SystemSettings.qml @@ -33,6 +33,16 @@ Singleton { property string quickshellVersion: "0.3.0" property string lastError: "" + // Explicit seams keep reset sequencing testable without changing the live + // keymap, wallpaper, or display from an isolated contract harness. + 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); } + property var setDisplayBlocked: function(blocked) { Displays.externalChangeBlocked = blocked; } + property var reloadKeybinds: function() { Keybinds.applyReload(); } + property var keybindsReloading: function() { return Keybinds.reloading; } + property var applyWallpaper: function(path) { Wallpaper.set(path); } + readonly property bool busy: monitorQuery.running || serviceQuery.running || versionQuery.running || configWrite.running || configVerify.running || bluebubblesQuery.running @@ -389,8 +399,22 @@ Singleton { // // Compositor-backed values are re-applied afterwards, since resetting the // stored value does not by itself tell Hyprland anything. - function restoreDefaults(): void { + function restoreDefaults(): bool { + if (root.displayBusy()) { + root.lastError = "Finish the current display change before restoring defaults."; + return false; + } + + const currentDisplays = root.readDisplays(); + const protectedDisplays = JSON.parse(JSON.stringify( + currentDisplays && typeof currentDisplays === "object" ? currentDisplays : {})); + root.setDisplayBlocked(true); DesktopPreferences.resetDesktopDefaults(); + if (!root.protectDisplays(protectedDisplays)) { + root.setDisplayBlocked(false); + root.lastError = "The current display setting could not be protected during reset."; + return false; + } // Home accessories keep their own store (panama-home.json), so a reset // that only cleared the schema store would silently leave a customised @@ -401,12 +425,33 @@ Singleton { HomePreferences.resetHomeDefaults(); resettleTimer.restart(); + return true; } Timer { id: resettleTimer interval: 60 - onTriggered: root.applyPersistedDisplayPolicy() + onTriggered: { + root.applyPersistedDisplayPolicy(); + root.reloadKeybinds(); + root.applyWallpaper(String(DesktopPreferences.get("wallpaperPath") ?? "")); + resetRelease.attempts = 0; + resetRelease.restart(); + } + } + + Timer { + id: resetRelease + property int attempts: 0 + interval: 100 + repeat: true + onTriggered: { + attempts++; + if ((!root.keybindsReloading() && !root.busy) || attempts >= 50) { + stop(); + root.setDisplayBlocked(false); + } + } } function setAutoHdr(enabled: bool): void { diff --git a/config/dot/quickshell/services/Wallpaper.qml b/config/dot/quickshell/services/Wallpaper.qml index d57688d..1e6d43e 100644 --- a/config/dot/quickshell/services/Wallpaper.qml +++ b/config/dot/quickshell/services/Wallpaper.qml @@ -33,6 +33,7 @@ Singleton { property bool scanning: false readonly property string configured: DesktopPreferences.get("wallpaperPath") + readonly property string shippedPath: `${Quickshell.env("HOME")}/Pictures/Wallpapers/faroe_islands.jpg` // Directories searched for wallpapers, in order. Screenshots are // deliberately excluded: a folder of 300 screenshots is not a wallpaper @@ -85,6 +86,7 @@ Singleton { id: apply property string requested: "" + property string storedValue: "" property var remaining: [] onExited: (exitCode, exitStatus) => { @@ -100,7 +102,7 @@ Singleton { return; } root.lastError = ""; - DesktopPreferences.set("wallpaperPath", apply.requested); + DesktopPreferences.set("wallpaperPath", apply.storedValue); root.refreshActive(); } } @@ -140,19 +142,16 @@ Singleton { // Applies to every connected output. Returns false when the path is not one // the schema will accept, so a caller can report the refusal. function set(path: string): bool { - if (PreferenceSchema.coerce("wallpaperPath", path) === undefined) { + const effectivePath = path === "" ? root.shippedPath : path; + if (PreferenceSchema.coerce("wallpaperPath", effectivePath) === undefined) { root.lastError = "That file path cannot be used as a wallpaper."; return false; } if (apply.running) return false; - apply.requested = path; - // "" clears the preference without touching what is on screen. - if (path === "") { - DesktopPreferences.set("wallpaperPath", ""); - return true; - } + apply.requested = effectivePath; + apply.storedValue = path; const outputs = Quickshell.screens.map(screen => screen.name).filter(name => !!name); if (outputs.length === 0) { @@ -161,7 +160,7 @@ Singleton { } apply.remaining = outputs.slice(1); - apply.exec(["hyprctl", "hyprpaper", "wallpaper", `${outputs[0]},${path}`]); + apply.exec(["hyprctl", "hyprpaper", "wallpaper", `${outputs[0]},${effectivePath}`]); return true; } diff --git a/config/dot/quickshell/settings-backup-harness.qml b/config/dot/quickshell/settings-backup-harness.qml index 6ae117a..37cb0de 100644 --- a/config/dot/quickshell/settings-backup-harness.qml +++ b/config/dot/quickshell/settings-backup-harness.qml @@ -13,6 +13,9 @@ ShellRoot { property var calls: [] property bool homeInitialized: false property var homeFavorites: [] + property bool displayOperationBusy: false + property bool displayBlocked: false + property var displayGeneration: ({ "DP-2": { mode: "4500x3000@60", scale: 1.5, transform: 0 } }) function record(name: string): void { const next = root.calls.slice(); @@ -43,6 +46,17 @@ ShellRoot { favorite.id === id ? { id: id, alias: alias } : favorite); }; SettingsBackup.reloadDesktop = function() { root.record("desktop.reload"); }; + SettingsBackup.readDisplays = function() { return root.displayGeneration; }; + SettingsBackup.protectDisplays = function(value) { + root.record("display.protect:" + JSON.stringify(value)); + root.displayGeneration = value; + return true; + }; + SettingsBackup.displayBusy = function() { return root.displayOperationBusy; }; + SettingsBackup.setDisplayBlocked = function(blocked) { + root.record("display.block:" + blocked); + root.displayBlocked = blocked; + }; SettingsBackup.applyCompositor = function() { root.record("system.apply"); }; SettingsBackup.reloadKeybinds = function() { root.record("keybinds.reload"); }; SettingsBackup.keybindsReloading = function() { return false; }; @@ -59,17 +73,29 @@ ShellRoot { root.calls = []; root.homeInitialized = false; root.homeFavorites = []; + root.displayOperationBusy = false; + root.displayBlocked = false; + SettingsBackup.protectedDisplays = root.displayGeneration; } function apply(output: string): bool { return SettingsBackup.handleRestoreOutput(output); } + function restoreWhileDisplayBusy(): bool { + root.displayOperationBusy = true; + SettingsBackup.snapshots = [{ name: "settings-20260818-010203004.json" }]; + return SettingsBackup.restore("settings-20260818-010203004.json"); + } + function status(): string { return JSON.stringify({ calls: root.calls, initialized: root.homeInitialized, - favorites: root.homeFavorites + favorites: root.homeFavorites, + displayBlocked: root.displayBlocked, + displays: root.displayGeneration, + lastError: SettingsBackup.lastError }); } } diff --git a/config/dot/quickshell/settings-system-harness.qml b/config/dot/quickshell/settings-system-harness.qml index b1d7065..4adfab9 100644 --- a/config/dot/quickshell/settings-system-harness.qml +++ b/config/dot/quickshell/settings-system-harness.qml @@ -6,6 +6,33 @@ import qs.config import qs.services ShellRoot { + id: root + + property var resetCalls: [] + property bool displayBlocked: false + + function recordReset(name: string): void { + const next = root.resetCalls.slice(); + next.push(name); + root.resetCalls = next; + } + + Component.onCompleted: { + SystemSettings.displayBusy = function() { return false; }; + SystemSettings.readDisplays = function() { return DesktopPreferences.get("displays"); }; + SystemSettings.protectDisplays = function(value) { + root.recordReset("display.protect"); + return DesktopPreferences.set("displays", value); + }; + SystemSettings.setDisplayBlocked = function(blocked) { + root.recordReset("display.block:" + blocked); + root.displayBlocked = blocked; + }; + SystemSettings.reloadKeybinds = function() { root.recordReset("keybinds.reload"); }; + SystemSettings.keybindsReloading = function() { return false; }; + SystemSettings.applyWallpaper = function(path) { root.recordReset("wallpaper.set:" + path); }; + } + IpcHandler { target: "settings-system-test" @@ -52,7 +79,14 @@ ShellRoot { }); } - function restoreDefaults(): void { SystemSettings.restoreDefaults(); } + function restoreDefaults(): bool { + root.resetCalls = []; + return SystemSettings.restoreDefaults(); + } + + function resetState(): string { + return JSON.stringify({ calls: root.resetCalls, displayBlocked: root.displayBlocked }); + } function panelAllowed(panel: string): bool { return SystemSettings.isGnomePanelAllowed(panel); diff --git a/tests/quickshell/settings-backup-contract.sh b/tests/quickshell/settings-backup-contract.sh index 08c1e78..6dbe65c 100755 --- a/tests/quickshell/settings-backup-contract.sh +++ b/tests/quickshell/settings-backup-contract.sh @@ -48,19 +48,21 @@ run save >/dev/null 2>&1 && fail 'backing up a missing settings file reported su [[ "$(run list)" == "[]" ]] || fail 'an empty backup directory did not list as empty' # ── A snapshot round-trips ─────────────────────────────────────────────────── -printf '{"gapsOut":24,"windowRounding":6}' >"$settings" +printf '{"gapsOut":24,"windowRounding":6,"displays":{"DP-2":{"mode":"3840x2160@60","scale":2,"transform":0}}}' >"$settings" mkdir -p "$(dirname "$home")" printf '{"initialized":true,"favorites":[{"id":"light.desk","alias":"Desk"}]}' >"$home" run save >/dev/null || fail 'save failed on a valid settings file' name="$(run list | jq -r '.[0].name')" [[ "$name" =~ ^settings-[0-9]{8}-[0-9]{9}\.json$ ]] || fail "unexpected snapshot name: $name" -[[ "$(run list | jq -r '.[0].keys')" == "2" ]] || fail 'snapshot key count is wrong' +[[ "$(run list | jq -r '.[0].keys')" == "3" ]] || fail 'snapshot key count is wrong' -printf '{"gapsOut":99}' >"$settings" +printf '{"gapsOut":99,"displays":{"DP-2":{"mode":"4500x3000@60","scale":1.5,"transform":0}}}' >"$settings" printf '{"initialized":false,"favorites":[]}' >"$home" restore_result="$(run restore "$name")" || fail 'restore failed' [[ "$(jq -r .gapsOut "$settings")" == "24" ]] || fail 'restore did not bring back the snapshot contents' [[ "$(jq -r .windowRounding "$settings")" == "6" ]] || fail 'restore lost a key' +[[ "$(jq -r '.displays["DP-2"].scale' "$settings")" == "1.5" ]] \ + || fail 'restore bypassed display confirmation by applying snapshot geometry' [[ "$(jq -r '.favorites[0].id' "$home")" == "light.desk" ]] || fail 'restore did not bring back Home favourites' [[ "$(jq -r '.favorites[0].alias' "$home")" == "Desk" ]] || fail 'restore lost a Home alias' jq -e '.home.present == true and .home.data.favorites[0].id == "light.desk"' <<<"$restore_result" >/dev/null \ diff --git a/tests/quickshell/settings-backup-live-contract.sh b/tests/quickshell/settings-backup-live-contract.sh index dfeca31..b6d2322 100755 --- a/tests/quickshell/settings-backup-live-contract.sh +++ b/tests/quickshell/settings-backup-live-contract.sh @@ -43,6 +43,8 @@ for mapping in \ 'HomePreferences.initialize(ids);' \ 'HomePreferences.setAlias(id, alias);' \ 'DesktopPreferences.reload();' \ + 'DesktopPreferences.set("displays", value);' \ + 'Displays.externalChangeBlocked = blocked;' \ 'SystemSettings.applyPersistedDisplayPolicy();' \ 'Keybinds.applyReload();' \ 'Wallpaper.set(path);' \ @@ -82,9 +84,11 @@ jq -e ' "home.alias:light.desk=Desk", "home.alias:light.office=Office", "desktop.reload", + "display.protect:{\"DP-2\":{\"mode\":\"4500x3000@60\",\"scale\":1.5,\"transform\":0}}", "system.apply", "keybinds.reload", "wallpaper.set:/tmp/restored-wallpaper.jpg", + "display.block:false", "shell.reload" ] and .initialized == true @@ -118,15 +122,26 @@ jq -e ' .calls == [ "home.reset", "desktop.reload", + "display.protect:{\"DP-2\":{\"mode\":\"4500x3000@60\",\"scale\":1.5,\"transform\":0}}", "system.apply", "keybinds.reload", "wallpaper.set:/tmp/restored-wallpaper.jpg", + "display.block:false", "shell.reload" ] and .initialized == false and .favorites == [] ' <<<"$status" >/dev/null || fail "absent Home handoff was wrong: $status" +# Restore refuses before launching the helper while a display apply/recovery is +# active, so no snapshot can race the confirmation boundary. +qs_test ipc call settings-backup-behavior reset >/dev/null +[[ "$(qs_test ipc call settings-backup-behavior restoreWhileDisplayBusy)" == "false" ]] \ + || fail 'snapshot restore started during an active display operation' +status="$(qs_test ipc call settings-backup-behavior status)" +jq -e '.calls == [] and (.lastError | contains("display change"))' <<<"$status" >/dev/null \ + || fail "display-busy restore refusal was not clean: $status" + trap - EXIT cleanup printf 'settings backup live contract: PASS\n' diff --git a/tests/quickshell/settings-commit-reset-contract.sh b/tests/quickshell/settings-commit-reset-contract.sh index 75d6c56..875e63b 100755 --- a/tests/quickshell/settings-commit-reset-contract.sh +++ b/tests/quickshell/settings-commit-reset-contract.sh @@ -20,6 +20,7 @@ set -euo pipefail repo_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" harness="$repo_dir/config/dot/quickshell/settings-system-harness.qml" system_settings="$repo_dir/config/dot/quickshell/services/SystemSettings.qml" +wallpaper_service="$repo_dir/config/dot/quickshell/services/Wallpaper.qml" # Preferences are committed to $XDG_CONFIG_HOME, and the Home store lives under # $XDG_STATE_HOME. Both are isolated so this contract cannot touch the real @@ -37,6 +38,16 @@ rg -Fq 'HomePreferences.resetHomeDefaults();' "$system_settings" \ if rg -q 'HomePreferences\.(favorites|initialized)\s*=' "$system_settings"; then fail 'restoreDefaults mutates Home aliases instead of using resetHomeDefaults' fi +rg -Fq 'root.protectDisplays(protectedDisplays)' "$system_settings" \ + || fail 'restoreDefaults can apply unconfirmed display geometry during reload' +rg -Fq 'Keybinds.applyReload();' "$system_settings" \ + || fail 'restoreDefaults does not replay shipped keybindings' +rg -Fq 'root.applyWallpaper(String(DesktopPreferences.get("wallpaperPath") ?? ""));' "$system_settings" \ + || fail 'restoreDefaults does not visibly reapply the shipped wallpaper' +rg -Fq 'const effectivePath = path === "" ? root.shippedPath : path;' "$wallpaper_service" \ + || fail 'clearing wallpaper preference leaves the old image visible' +rg -Fq 'property string storedValue:' "$wallpaper_service" \ + || fail 'the shipped wallpaper cannot remain represented by the default empty preference' qs_for_harness() { XDG_CONFIG_HOME="$config_home" XDG_STATE_HOME="$state_home" qs -p "$harness" "$@" @@ -92,6 +103,9 @@ before="$(qs_for_harness ipc call settings-system-test stored windowRounding)" # ── Reset spans every store, not just the schema one ───────────────────────── qs_for_harness ipc call settings-system-test seedHome >/dev/null qs_for_harness ipc call settings-system-test commit dockHideDelayMs 900 >/dev/null +display_fixture='{"DP-2":{"mode":"4500x3000@60","scale":1.5,"transform":0}}' +[[ "$(qs_for_harness ipc call settings-system-test commit displays "$display_fixture")" == "true" ]] \ + || fail 'the protected display fixture did not apply' sleep 0.4 home_before="$(qs_for_harness ipc call settings-system-test homeState)" @@ -100,11 +114,24 @@ jq -e '.count == 1 and .initialized == true' <<<"$home_before" >/dev/null \ [[ "$(qs_for_harness ipc call settings-system-test stored dockHideDelayMs)" == "900" ]] \ || fail 'the dock fixture did not apply' -qs_for_harness ipc call settings-system-test restoreDefaults >/dev/null +[[ "$(qs_for_harness ipc call settings-system-test restoreDefaults)" == "true" ]] \ + || fail 'restoreDefaults refused a safe reset' sleep 0.6 +reset_state="$(qs_for_harness ipc call settings-system-test resetState)" +jq -e '.calls == [ + "display.block:true", + "display.protect", + "keybinds.reload", + "wallpaper.set:", + "display.block:false" +] and .displayBlocked == false' <<<"$reset_state" >/dev/null \ + || fail "reset did not safely replay non-reactive state: $reset_state" + [[ "$(qs_for_harness ipc call settings-system-test stored dockHideDelayMs)" == "250" ]] \ || fail 'reset did not restore a schema default' +[[ "$(qs_for_harness ipc call settings-system-test stored displays | jq -cS .)" == "$(jq -cS . <<<"$display_fixture")" ]] \ + || fail 'reset replaced confirmed display geometry without confirmation' home_after="$(qs_for_harness ipc call settings-system-test homeState)" jq -e '.count == 0 and .initialized == false' <<<"$home_after" >/dev/null \