From c415c4f1766366ad9f3c295e46c93df65eb1917b Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Tue, 18 Aug 2026 05:51:32 -0400 Subject: [PATCH] fix: harden notification application rule integration --- config/dot/quickshell/services/Notifs.qml | 35 ++++-- .../NotificationAppRulesHarness.qml | 113 +++++++++++++++++ .../notification-app-rules-contract.sh | 119 +++++++++++++++++- 3 files changed, 252 insertions(+), 15 deletions(-) create mode 100644 tests/quickshell/NotificationAppRulesHarness.qml diff --git a/config/dot/quickshell/services/Notifs.qml b/config/dot/quickshell/services/Notifs.qml index e383384..a4cf51f 100644 --- a/config/dot/quickshell/services/Notifs.qml +++ b/config/dot/quickshell/services/Notifs.qml @@ -52,13 +52,21 @@ Singleton { return stored && typeof stored === "object" && !Array.isArray(stored) ? stored : {}; } + // The schema change is the persistence boundary. This branch keeps a + // session fallback only so it remains usable while that companion change + // is being integrated; it intentionally makes no restart guarantee then. + readonly property bool appRulesSchemaAvailable: PreferenceSchema.has("notificationAppRules") + readonly property var appRules: Object.assign({}, root.persistedAppRules, root.fallbackAppRules) readonly property var applications: { + // byId()/heuristicLookup() do not make a binding by themselves. This + // read updates persisted app labels once DesktopEntries finishes scan. + const entries = DesktopEntries.applications.values; const remembered = root.rememberedApplications; return Object.keys(root.appRules).map(appId => ({ id: appId, - name: remembered[appId]?.name || appId + name: root.applicationLabel(appId, entries, remembered) })).sort((a, b) => a.name.localeCompare(b.name)); } @@ -74,11 +82,16 @@ Singleton { readonly property bool hasNotifications: root.history.length > 0 - function notificationAppId(notification: Notification): string { + function notificationAppId(notification: var): string { const desktopEntry = String(notification.desktopEntry ?? "").trim(); return desktopEntry || String(notification.appName ?? "").trim() || "Notifications"; } + function applicationLabel(appId: string, entries: var, remembered: var): string { + const entry = DesktopEntries.byId(appId) || DesktopEntries.heuristicLookup(appId); + return entry?.name || remembered[appId]?.name || appId; + } + function normalizedAppRule(rule: var): var { const source = rule && typeof rule === "object" && !Array.isArray(rule) ? rule : {}; return { @@ -106,14 +119,14 @@ Singleton { showContentOnLockScreen: patch.showContentOnLockScreen === undefined ? current.showContentOnLockScreen : patch.showContentOnLockScreen === true }; - if (DesktopPreferences.set("notificationAppRules", next)) + if (root.appRulesSchemaAvailable && DesktopPreferences.set("notificationAppRules", next)) root.fallbackAppRules = {}; else root.fallbackAppRules = next; return true; } - function rememberApplication(notification: Notification): string { + function rememberApplication(notification: var): string { const appId = root.notificationAppId(notification); const next = Object.assign({}, root.rememberedApplications); next[appId] = { @@ -128,12 +141,12 @@ Singleton { // These policy getters deliberately accept Notification objects, so a lock // screen can use the same source of truth without duplicating app matching. - function shouldShowOnLockScreen(notification: Notification): bool { + function shouldShowOnLockScreen(notification: var): bool { const rule = root.appRule(root.notificationAppId(notification)); return rule.enabled && rule.showOnLockScreen; } - function shouldShowContentOnLockScreen(notification: Notification): bool { + function shouldShowContentOnLockScreen(notification: var): bool { const rule = root.appRule(root.notificationAppId(notification)); return rule.enabled && rule.showOnLockScreen && rule.showContentOnLockScreen; } @@ -174,7 +187,10 @@ Singleton { actionIconsSupported: true inlineReplySupported: true - onNotification: notification => { + onNotification: notification => root.handleNotification(notification) + } + + function handleNotification(notification: var): void { // Replayed from before a shell reload. Letting these through would // re-toast and re-list everything on every edit, so they are left // untracked and allowed to die. @@ -201,12 +217,11 @@ Singleton { if (!root.doNotDisturb) root.popups = [notification].concat(root.popups); - } } // ── Mutation ──────────────────────────────────────────────────────────── - function pushHistory(n: Notification): void { + function pushHistory(n: var): void { const next = [n].concat(root.history); // Anything past the cap is released, otherwise it stays tracked @@ -259,7 +274,7 @@ Singleton { // Called from the `closed` signal — the object is on its way out, so this // only ever removes references, never touches the notification. - function forget(n: Notification): void { + function forget(n: var): void { delete root.arrivals[n.id]; if (root.history.indexOf(n) !== -1) root.history = root.history.filter(x => x !== n); diff --git a/tests/quickshell/NotificationAppRulesHarness.qml b/tests/quickshell/NotificationAppRulesHarness.qml new file mode 100644 index 0000000..f4bdf8f --- /dev/null +++ b/tests/quickshell/NotificationAppRulesHarness.qml @@ -0,0 +1,113 @@ +import Quickshell +import Quickshell.Io +import QtQuick + +import qs.config +import qs.services + +ShellRoot { + id: root + + function notification(idValue: int, desktopEntryValue: string, appNameValue: string): var { + const closeHandlers = []; + return { + id: idValue, + desktopEntry: desktopEntryValue, + appName: appNameValue, + appIcon: "", + transient: false, + lastGeneration: false, + tracked: false, + dismissed: false, + closed: { + connect: callback => closeHandlers.push(callback) + }, + dismiss: function() { + this.dismissed = true; + for (const callback of closeHandlers) + callback(); + } + }; + } + + function resetNotifications(): void { + Notifs.history = []; + Notifs.popups = []; + Notifs.unreadCount = 0; + Notifs.doNotDisturb = false; + } + + function reset(): void { + root.resetNotifications(); + Notifs.fallbackAppRules = {}; + Notifs.rememberedApplications = {}; + DesktopPreferences.set("notificationAppRules", {}); + } + + IpcHandler { + target: "notification-app-rules-test" + + function exercise(): string { + root.reset(); + + const signal = root.notification(1, "org.signal.Signal.desktop", "Signal"); + Notifs.handleNotification(signal); + const appId = Notifs.notificationAppId(signal); + const initialRules = DesktopPreferences.get("notificationAppRules"); + + root.resetNotifications(); + Notifs.setAppRule(appId, { enabled: false }); + const muted = root.notification(2, "org.signal.Signal.desktop", "Signal"); + Notifs.handleNotification(muted); + const mutedResult = { + tracked: muted.tracked, + history: Notifs.history.length, + popups: Notifs.popups.length, + unread: Notifs.unreadCount + }; + + root.reset(); + Notifs.doNotDisturb = true; + const dnd = root.notification(3, "org.signal.Signal.desktop", "Signal"); + Notifs.handleNotification(dnd); + + Notifs.setAppRule("org.privacy.App.desktop", { + showOnLockScreen: false, + showContentOnLockScreen: false + }); + const privateNotification = root.notification(4, "org.privacy.App.desktop", "Private"); + + return JSON.stringify({ + appId: appId, + initialRules: initialRules, + muted: mutedResult, + dnd: { + tracked: dnd.tracked, + history: Notifs.history.length, + popups: Notifs.popups.length, + unread: Notifs.unreadCount + }, + privacy: { + visible: Notifs.shouldShowOnLockScreen(privateNotification), + content: Notifs.shouldShowContentOnLockScreen(privateNotification) + } + }); + } + + function persist(): string { + root.reset(); + const notification = root.notification(5, "org.persist.App.desktop", "Persist"); + Notifs.handleNotification(notification); + Notifs.setAppRule("org.persist.App.desktop", { + enabled: false, + showOnLockScreen: true, + showContentOnLockScreen: false + }); + return JSON.stringify(DesktopPreferences.get("notificationAppRules")); + } + + function restored(): string { + return JSON.stringify(DesktopPreferences.get("notificationAppRules")); + } + } +} diff --git a/tests/quickshell/notification-app-rules-contract.sh b/tests/quickshell/notification-app-rules-contract.sh index 93ef26d..440aced 100755 --- a/tests/quickshell/notification-app-rules-contract.sh +++ b/tests/quickshell/notification-app-rules-contract.sh @@ -5,6 +5,7 @@ set -euo pipefail repo_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" service="$repo_dir/config/dot/quickshell/services/Notifs.qml" page="$repo_dir/config/dot/quickshell/modules/settings/NotificationsPage.qml" +harness_fixture="$repo_dir/tests/quickshell/NotificationAppRulesHarness.qml" fail() { printf 'notification application rules contract: %s\n' "$1" >&2 @@ -13,6 +14,7 @@ fail() { [[ -f "$service" ]] || fail 'notification service is missing' [[ -f "$page" ]] || fail 'notification settings page is missing' +[[ -f "$harness_fixture" ]] || fail 'runtime harness fixture is missing' SERVICE_PATH="$service" PAGE_PATH="$page" bun -e ' const source = await Bun.file(process.env.SERVICE_PATH).text(); @@ -59,15 +61,19 @@ const explicitRule = normalizedAppRule({ enabled: false, showOnLockScreen: false if (JSON.stringify(explicitRule) !== JSON.stringify({ enabled: false, showOnLockScreen: false, showContentOnLockScreen: false })) fail(`explicit rule was not preserved: ${JSON.stringify(explicitRule)}`); -for (const required of ["rememberApplication", "appRule", "setAppRule", "shouldShowOnLockScreen", "shouldShowContentOnLockScreen"]) { +for (const required of ["rememberApplication", "appRule", "setAppRule", "handleNotification", "shouldShowOnLockScreen", "shouldShowContentOnLockScreen"]) { functionBody(required); } -const arrival = source.indexOf("onNotification: notification =>"); -const tracked = source.indexOf("notification.tracked = true", arrival); -const muted = source.indexOf("!root.appRule(appId).enabled", arrival); -if (arrival === -1 || tracked === -1 || muted === -1 || muted > tracked) +const handler = source.indexOf("function handleNotification(notification: var)"); +const tracked = source.indexOf("notification.tracked = true", handler); +const muted = source.indexOf("!root.appRule(appId).enabled", handler); +if (handler === -1 || tracked === -1 || muted === -1 || muted > tracked) fail("muted applications are not rejected before tracking/history/unread/toast work"); +if (!source.includes("root.handleNotification(notification)")) + fail("NotificationServer does not delegate delivery to the callable handler"); +if (!source.includes("PreferenceSchema.has(\"notificationAppRules\")")) + fail("the schema dependency is not explicit"); if (!source.includes("DesktopPreferences.get(\"notificationAppRules\")")) fail("rules are not read through DesktopPreferences"); if (!source.includes("DesktopPreferences.set(\"notificationAppRules\", next)")) @@ -78,6 +84,14 @@ if (!source.includes("root.fallbackAppRules = next")) fail("missing-schema preference writes do not retain an in-memory fallback"); if (!source.includes("if (!root.doNotDisturb)")) fail("global DND popup override was removed"); +for (const required of [ + "DesktopEntries.applications.values", + "DesktopEntries.byId(appId)", + "DesktopEntries.heuristicLookup(appId)" +]) { + if (!source.includes(required)) + fail(`persisted desktop entry ids are not reactively resolved through ${required}`); +} for (const required of [ "Notifs.applications", @@ -92,3 +106,98 @@ for (const required of [ console.log("notification application rules contract: PASS"); ' + +state_home="$(mktemp -d /tmp/panama-notification-rules-state.XXXXXX)" +config_home="$(mktemp -d /tmp/panama-notification-rules-config.XXXXXX)" +config_path="$state_home/quickshell" +harness="$config_path/notification-app-rules-harness.qml" +shell_log="$state_home/notification-app-rules.log" + +cleanup() { + if [[ -n "${bus_pid:-}" ]]; then + kill "$bus_pid" >/dev/null 2>&1 || true + fi + rm -rf "$state_home" "$config_home" +} +trap cleanup EXIT + +cp -a "$repo_dir/config/dot/quickshell" "$config_path" +cp "$harness_fixture" "$harness" + +# This is intentionally a copy-local integration dependency. The production +# schema is Claude's change; the runtime contract proves persistence only once +# that key exists and never stages a schema edit from this branch. +perl -0pi -e 's@(\n // ── Capture)@\n {\n key: "notificationAppRules", type: "json", def: {}, group: "notifications", internal: true\n },$1@' \ + "$config_path/config/PreferenceSchema.qml" +rg -q 'key: "notificationAppRules", type: "json"' "$config_path/config/PreferenceSchema.qml" \ + || fail 'temporary schema integration key was not installed' + +mapfile -t dbus_info < <(dbus-daemon --session --fork --print-address=1 --print-pid=1) +bus_address="${dbus_info[0]:-}" +bus_pid="${dbus_info[1]:-}" +[[ -n "$bus_address" && "$bus_pid" =~ ^[0-9]+$ ]] || fail 'private D-Bus session did not start' + +qs_for_test() { + DBUS_SESSION_BUS_ADDRESS="$bus_address" \ + XDG_STATE_HOME="$state_home" XDG_CONFIG_HOME="$config_home" \ + qs -p "$harness" "$@" +} + +stop_harness() { + qs_for_test kill >/dev/null 2>&1 || true + for _ in $(seq 1 40); do + ! qs_for_test ipc show >/dev/null 2>&1 && return + sleep 0.1 + done + fail 'isolated notification harness did not stop cleanly' +} + +start_harness() { + qs_for_test --daemonize >"$shell_log" 2>&1 + for _ in $(seq 1 40); do + qs_for_test ipc show 2>/dev/null | rg -q '^target notification-app-rules-test$' && return + sleep 0.1 + done + sed -n '1,240p' "$shell_log" >&2 + fail 'isolated notification harness did not start' +} + +start_harness +exercise="$(qs_for_test ipc call notification-app-rules-test exercise)" +jq -e ' + .appId == "org.signal.Signal.desktop" and + .initialRules == { + "org.signal.Signal.desktop": { + enabled: true, + showOnLockScreen: true, + showContentOnLockScreen: true + } + } and + .muted == { tracked: false, history: 0, popups: 0, unread: 0 } and + .dnd == { tracked: true, history: 1, popups: 0, unread: 1 } and + .privacy == { visible: false, content: false } +' <<<"$exercise" >/dev/null || fail "runtime notification policy fixture failed: $exercise" + +persisted="$(qs_for_test ipc call notification-app-rules-test persist)" +jq -e '. == { + "org.persist.App.desktop": { + enabled: false, + showOnLockScreen: true, + showContentOnLockScreen: false + } +}' <<<"$persisted" >/dev/null || fail "runtime persistence fixture wrote the wrong shape: $persisted" + +settings_file="$config_home/panama/settings.json" +for _ in $(seq 1 40); do + [[ -f "$settings_file" ]] && jq -e '.notificationAppRules["org.persist.App.desktop"].enabled == false' "$settings_file" >/dev/null && break + sleep 0.1 +done +[[ -f "$settings_file" ]] || fail 'runtime persistence fixture did not write settings.json' + +stop_harness +start_harness +restored="$(qs_for_test ipc call notification-app-rules-test restored)" +[[ "$restored" == "$persisted" ]] || fail "notification rules did not survive isolated restart: $restored" +stop_harness + +printf 'notification application rules runtime contract: PASS\n'