From 7b8270fdd6238d79475b4c51cd666cac348958be Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Tue, 18 Aug 2026 03:19:45 -0400 Subject: [PATCH] Keep Settings recovery isolated and display-safe --- .../quickshell/scripts/panama-settings-backup | 25 ++++++++------ .../quickshell/services/SystemSettings.qml | 8 +++++ .../quickshell/settings-system-harness.qml | 23 ++++++++++++- tests/quickshell/settings-backup-contract.sh | 12 ++++--- .../settings-commit-reset-contract.sh | 33 +++++++------------ 5 files changed, 64 insertions(+), 37 deletions(-) diff --git a/config/dot/quickshell/scripts/panama-settings-backup b/config/dot/quickshell/scripts/panama-settings-backup index 7a57fd0..39717c7 100755 --- a/config/dot/quickshell/scripts/panama-settings-backup +++ b/config/dot/quickshell/scripts/panama-settings-backup @@ -577,17 +577,22 @@ def command_restore(arguments: list[str]) -> None: # 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: + try: + current_desktop = read_json(SETTINGS, "The current settings file") \ + if is_present(SETTINGS) else {} + except BackupError: + current_desktop = {} + + if isinstance(current_desktop, dict) and "displays" in current_desktop: + # Even a Home-only snapshot must retain the confirmed monitor layout. + # In that case the restored desktop file contains only the protected + # geometry; every ordinary desktop preference remains absent/default. + desktop_present = True + desktop_data = dict(desktop_data) if desktop_data is not None else {} + desktop_data["displays"] = current_desktop["displays"] + elif 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) + desktop_data.pop("displays", None) # Restoring remains undoable, but a corrupt current file must not prevent a # known-good snapshot from recovering the desktop. diff --git a/config/dot/quickshell/services/SystemSettings.qml b/config/dot/quickshell/services/SystemSettings.qml index 29d4864..67261fd 100644 --- a/config/dot/quickshell/services/SystemSettings.qml +++ b/config/dot/quickshell/services/SystemSettings.qml @@ -206,6 +206,11 @@ Singleton { } // ── Applying options ──────────────────────────────────────────────────── + // Test harnesses may replace the external compositor boundary while still + // exercising validation, commit routing, persistence, and reset replay. + // Production leaves this unset and always uses the verified Hyprland path. + property var compositorApplyOverride: null + // `values` maps schema keys to values, e.g. { vrrPolicy: 3, gapsOut: 12 }. // The whole batch is validated before anything is sent, so one bad value // rejects the batch rather than half-applying it. @@ -227,6 +232,9 @@ Singleton { if (Object.keys(requested).length === 0) return false; + if (root.compositorApplyOverride !== null) + return root.compositorApplyOverride(requested); + // A write in flight is queued rather than refused. Options are applied // and verified one batch at a time, but the callers are a settings UI // and a startup replay of every compositor-backed preference -- they diff --git a/config/dot/quickshell/settings-system-harness.qml b/config/dot/quickshell/settings-system-harness.qml index 4adfab9..7c59615 100644 --- a/config/dot/quickshell/settings-system-harness.qml +++ b/config/dot/quickshell/settings-system-harness.qml @@ -9,6 +9,7 @@ ShellRoot { id: root property var resetCalls: [] + property var appliedBatches: [] property bool displayBlocked: false function recordReset(name: string): void { @@ -18,6 +19,18 @@ ShellRoot { } Component.onCompleted: { + // Keep compositor verification entirely inside the isolated harness. + // Production applyOptions is covered separately by the Hyprland write + // contract; this seam proves commit/reset routing without changing the + // desktop that is running the test. + SystemSettings.compositorApplyOverride = function(requested) { + const batches = root.appliedBatches.slice(); + batches.push(requested); + root.appliedBatches = batches; + for (const key in requested) + DesktopPreferences.set(key, requested[key]); + return true; + }; SystemSettings.displayBusy = function() { return false; }; SystemSettings.readDisplays = function() { return DesktopPreferences.get("displays"); }; SystemSettings.protectDisplays = function(value) { @@ -85,7 +98,15 @@ ShellRoot { } function resetState(): string { - return JSON.stringify({ calls: root.resetCalls, displayBlocked: root.displayBlocked }); + return JSON.stringify({ + calls: root.resetCalls, + displayBlocked: root.displayBlocked, + appliedBatches: root.appliedBatches + }); + } + + function applyState(): string { + return JSON.stringify(root.appliedBatches); } function panelAllowed(panel: string): bool { diff --git a/tests/quickshell/settings-backup-contract.sh b/tests/quickshell/settings-backup-contract.sh index 6dbe65c..f119952 100755 --- a/tests/quickshell/settings-backup-contract.sh +++ b/tests/quickshell/settings-backup-contract.sh @@ -79,16 +79,20 @@ absent_result="$(run restore "$absent_name")" || fail 'restore failed for a snap jq -e '.home.present == false and (.home | has("data") | not)' <<<"$absent_result" >/dev/null \ || fail 'restore did not return absent Home state for the live service to reload' -# Desktop absence is symmetric: a Home-only snapshot removes a desktop file -# created later and restores the Home store. +# Desktop absence is symmetric for ordinary preferences, but confirmed display +# geometry is protected state: it must survive even a Home-only snapshot. rm -f "$settings" printf '{"initialized":true,"favorites":[{"id":"light.porch","alias":"Porch"}]}' >"$home" run save >/dev/null || fail 'save failed when desktop settings were absent' desktop_absent_name="$(run list | jq -r '.[0].name')" -printf '{"gapsOut":47}' >"$settings" +printf '{"gapsOut":47,"windowRounding":9,"displays":{"DP-2":{"mode":"4500x3000@60","scale":1.5,"transform":0}}}' >"$settings" printf '{"initialized":false,"favorites":[]}' >"$home" run restore "$desktop_absent_name" >/dev/null || fail 'Home-only snapshot restore failed' -[[ ! -e "$settings" ]] || fail 'restore did not preserve the snapshot’s absent desktop state' +[[ -e "$settings" ]] || fail 'Home-only restore discarded confirmed display geometry' +[[ "$(jq -r '.displays["DP-2"].scale' "$settings")" == "1.5" ]] \ + || fail 'Home-only restore changed confirmed display geometry' +[[ "$(jq 'keys == ["displays"]' "$settings")" == "true" ]] \ + || fail 'Home-only restore retained ordinary desktop preferences' [[ "$(jq -r '.favorites[0].id' "$home")" == "light.porch" ]] \ || fail 'Home-only snapshot did not restore Home state' assert_transaction_clean diff --git a/tests/quickshell/settings-commit-reset-contract.sh b/tests/quickshell/settings-commit-reset-contract.sh index 875e63b..41fdbb8 100755 --- a/tests/quickshell/settings-commit-reset-contract.sh +++ b/tests/quickshell/settings-commit-reset-contract.sh @@ -24,7 +24,8 @@ 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 -# desktop's settings; the compositor is the live one and is restored below. +# desktop's settings. The harness replaces the compositor write seam as well, +# so interruption cannot leave the daily desktop modified. config_home="$(mktemp -d /tmp/panama-commit-config.XXXXXX)" state_home="$(mktemp -d /tmp/panama-commit-state.XXXXXX)" @@ -53,11 +54,7 @@ qs_for_harness() { XDG_CONFIG_HOME="$config_home" XDG_STATE_HOME="$state_home" qs -p "$harness" "$@" } -original_rounding="$(hyprctl -j getoption decoration:rounding | jq -r .int)" -original_gaps="$(hyprctl -j getoption general:gaps_out | jq -r .css | awk '{print $1}')" - restore() { - hyprctl eval "hl.config({ decoration = { rounding = $original_rounding }, general = { gaps_out = $original_gaps } })" >/dev/null 2>&1 || true qs_for_harness kill >/dev/null 2>&1 || true rm -rf "$config_home" "$state_home" } @@ -76,17 +73,13 @@ qs_for_harness ipc show 2>/dev/null | rg -q '^target settings-system-test$' || f [[ "$(qs_for_harness ipc call settings-system-test stored showSeconds)" == "false" ]] \ || fail 'a local key was not stored' -# ── A compositor key reaches Hyprland, then is stored ──────────────────────── -target_rounding=$(( original_rounding == 11 ? 13 : 11 )) +# ── A compositor key reaches the verified apply boundary, then is stored ──── +target_rounding=11 [[ "$(qs_for_harness ipc call settings-system-test commit windowRounding "$target_rounding")" == "true" ]] \ || fail 'commitPreference refused a compositor key' - -for _ in $(seq 1 40); do - [[ "$(hyprctl -j getoption decoration:rounding | jq -r .int)" == "$target_rounding" ]] && break - sleep 0.1 -done -[[ "$(hyprctl -j getoption decoration:rounding | jq -r .int)" == "$target_rounding" ]] \ - || fail "a compositor-backed commit did not reach Hyprland (rounding=$(hyprctl -j getoption decoration:rounding | jq -r .int))" +apply_state="$(qs_for_harness ipc call settings-system-test applyState)" +jq -e '.[-1].windowRounding == 11' <<<"$apply_state" >/dev/null \ + || fail "a compositor-backed commit did not reach the apply boundary: $apply_state" [[ "$(qs_for_harness ipc call settings-system-test stored windowRounding)" == "$target_rounding" ]] \ || fail 'a verified compositor commit was not stored' @@ -137,14 +130,10 @@ home_after="$(qs_for_harness ipc call settings-system-test homeState)" jq -e '.count == 0 and .initialized == false' <<<"$home_after" >/dev/null \ || fail "reset left the Home accessory store customised: $home_after" -# Resetting a stored value does not by itself tell Hyprland anything, so the -# reset must re-apply compositor-backed defaults too. -for _ in $(seq 1 40); do - [[ "$(hyprctl -j getoption decoration:rounding | jq -r .int)" == "18" ]] && break - sleep 0.1 -done -[[ "$(hyprctl -j getoption decoration:rounding | jq -r .int)" == "18" ]] \ - || fail "reset did not re-apply the compositor default (rounding=$(hyprctl -j getoption decoration:rounding | jq -r .int))" +# Resetting a stored value does not itself apply compositor policy, so the last +# isolated batch must contain the shipped default. +jq -e '.appliedBatches[-1].windowRounding == 18' <<<"$reset_state" >/dev/null \ + || fail "reset did not re-apply the compositor default: $reset_state" trap - EXIT restore