From 48d77b573890a01465c618080d6a673598c4b741 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 7 Aug 2026 15:14:56 +0200 Subject: [PATCH] Persist on-screen notification popups across shell restarts (#6600) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Persist on-screen notification popups across shell restarts Mirror every popup to its own file under ~/.local/state/omarchy/notifications/ for exactly as long as it is on screen: written when the toast appears, deleted when it expires, is dismissed, is acted upon, or is replaced via freedesktop replaces_id. On startup the directory is read back and still-valid popups re-shown, so toasts survive the restart omarchy-update performs — critical alerts, which never expire, always make it across. Restored popups keep ids from the previous server generation, so the replaces_id cleanup tracks them separately instead of mistaking a fresh notification's reused id for a replacement, and the startup restore only discards a persisted file when a live row with a different timestamp has superseded it. Files are read back with awk so a torn write can't glue itself onto the next file and take a valid popup down with it. Co-Authored-By: Claude Fable 5 * Close the remaining cross-generation id collisions in popup persistence Notification ids restart from 1 with every server process, so an id alone never identifies a notification across a shell restart. The first round of fixes guarded row removal, but review (and a live repro) showed the same collision biting everywhere else an id was used on its own: - Dismissing or clicking a restored toast resolved liveRefs by id and could dismiss, or fire the action of, an unrelated fresh notification, and archive its pending row. Restored rows now never resolve to a live object, and pending rows are matched by id plus timestamp. - parsePopupFiles deduped files by id, so a fresh notification reusing a restored critical alert's id got that alert's file deleted as a "stale duplicate" on the next restore. Files are never deduped now: each one is a popup that was on screen, and the rare genuine leftover from a crash re-shows once and cleans itself up. - The restore only skips an entry when a live row matches both id and timestamp (it is that entry); an id-only match shows both toasts rather than guessing which one to drop. - A same-millisecond replaces_id update shares its predecessor's filename; the replacement's file is no longer deleted alongside the replaced row. - A restored popup's reset lifetime is persisted as an absolute deadline, so a second restart judges it by the clock that actually governs its display instead of dropping it while still on screen. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- .../notifications/NotificationLogic.js | 76 +++++++ shell/plugins/notifications/Service.qml | 196 +++++++++++++++++- test/shell.d/notifications-test.sh | 108 ++++++++++ 3 files changed, 370 insertions(+), 10 deletions(-) diff --git a/shell/plugins/notifications/NotificationLogic.js b/shell/plugins/notifications/NotificationLogic.js index 09833e5d..64dcaaef 100644 --- a/shell/plugins/notifications/NotificationLogic.js +++ b/shell/plugins/notifications/NotificationLogic.js @@ -194,6 +194,77 @@ function dumpRows(rows) { return out } +// ---------------------------------------------------- popup persistence +// +// Each on-screen popup is mirrored to its own file under +// ~/.local/state/omarchy/notifications/ so toasts survive shell restarts +// (e.g. the restart `omarchy-update` performs). The file exists exactly as +// long as the popup is on screen: it is written when the toast appears and +// deleted when the toast expires, is dismissed, or its action is invoked. + +function popupEntry(value, normalUrgency) { + var entry = historyEntry(value, normalUrgency) + var expire = Number((value || {}).expireTimeout || 0) + if (!isFinite(expire) || expire < 0) expire = 0 + entry.expireTimeout = expire + // Absolute expiry deadline, set only when a restore resets a surviving + // popup's display lifetime. Kept out of the entry entirely when unset so + // restored rows match the roles of freshly received ones. + var deadline = Number((value || {}).deadline || 0) + if (isFinite(deadline) && deadline > 0) entry.deadline = deadline + return entry +} + +function popupFileName(entry) { + var e = entry || {} + return String(e.timestamp || 0) + "-" + String(e.originalId || 0) + ".json" +} + +function serializePopup(entry, normalUrgency) { + // Compact (single-line) on purpose: restore cats every file together and + // parses line by line, which only works when each file is one line. + return JSON.stringify(popupEntry(entry, normalUrgency)) +} + +// Parse the concatenation of every persisted popup file into entries, +// newest-first. Deliberately NO dedupe by originalId: ids restart from 1 +// with every server process, so two files sharing an id are usually +// different generations — dropping the older one would silently discard a +// restored critical alert the moment a fresh notification reuses its id. +// The one case that leaves a genuine duplicate (a crash between a +// replacement's write and the replaced file's delete) merely re-shows a +// superseded toast, which expires or is dismissed and cleans itself up. +function parsePopupFiles(raw, normalUrgency) { + var lines = String(raw || "").split("\n") + var entries = [] + for (var i = 0; i < lines.length; i++) { + var line = lines[i].trim() + if (!line) continue + try { + var value = JSON.parse(line) + if (value && typeof value === "object") entries.push(popupEntry(value, normalUrgency)) + } catch (e) { + // A torn write from a crash mid-save — skip the line, keep the rest. + } + } + entries.sort(function(a, b) { return (b.timestamp || 0) - (a.timestamp || 0) }) + return entries +} + +// A persisted popup whose lifetime already ran out would have expired on +// screen had the shell kept running, so it is not restored. duration 0 means +// the popup never expires (critical urgency) and always survives restarts. +// A restore-reset deadline outranks the original timestamp: without it, a +// second restart would judge a re-shown toast by a clock that no longer +// governs its display and drop it while it is still on screen. +function popupExpired(entry, duration, now) { + var deadline = Number((entry || {}).deadline || 0) + if (isFinite(deadline) && deadline > 0) return Number(now) >= deadline + var lifetime = Number(duration || 0) + if (!isFinite(lifetime) || lifetime <= 0) return false + return (Number(now) - Number((entry || {}).timestamp || 0)) >= lifetime +} + function popupPlacement(barPosition, barClearance, gapsOut) { var position = String(barPosition || "top") var clearance = Number(barClearance) @@ -236,6 +307,11 @@ if (typeof module !== "undefined") { parseHistory: parseHistory, recentHistoryRows: recentHistoryRows, dumpRows: dumpRows, + popupEntry: popupEntry, + popupFileName: popupFileName, + serializePopup: serializePopup, + parsePopupFiles: parsePopupFiles, + popupExpired: popupExpired, popupPlacement: popupPlacement, imageExtension: imageExtension } diff --git a/shell/plugins/notifications/Service.qml b/shell/plugins/notifications/Service.qml index c12d5eb7..e9cb24ff 100644 --- a/shell/plugins/notifications/Service.qml +++ b/shell/plugins/notifications/Service.qml @@ -24,6 +24,10 @@ Item { // regeneratable cache that a `rm -rf ~/.cache` should wipe. readonly property string stateDir: home + "/.local/state/omarchy/" readonly property string historyPath: stateDir + "notifications.json" + // One file per on-screen popup, so live toasts survive shell restarts. + // A file exists exactly as long as its popup is showing: written when the + // toast appears, deleted when it expires, is dismissed, or is acted upon. + readonly property string popupStateDir: stateDir + "notifications/" // Thumbnails copied from /tmp screenshots are genuinely disposable — if // they vanish the row just renders without an image — so they stay in // ~/.cache where regeneratable artifacts belong. @@ -172,8 +176,9 @@ Item { notification.tracked = false return } + persistPopupFile(snapshot) Qt.callLater(function() { - removeByOriginalId(popupModel, snapshot.originalId) + removePopupsByOriginalId(snapshot.originalId, NotificationLogic.popupFileName(snapshot)) popupModel.insert(0, snapshot) }) return @@ -200,14 +205,43 @@ Item { return } + persistPopupFile(snapshot) // Qt.callLater avoids "QV4::Object::insertMember" crashes when a // Repeater is mid-incubation while we mutate its model. Qt.callLater(function() { - removeByOriginalId(popupModel, snapshot.originalId) + removePopupsByOriginalId(snapshot.originalId, NotificationLogic.popupFileName(snapshot)) popupModel.insert(0, snapshot) }) } + // A restored row carries an id from the previous server generation, and + // the new server hands out ids from 1 again — so a fresh notification + // with the same originalId is a coincidence, not the same notification. + // The timestamp (via the file name) disambiguates: it travels with the + // row through every model and file round-trip. + function isRestoredRow(row) { + return !!row && !!restoredPopups[NotificationLogic.popupFileName(row)] + } + + // Popup-specific variant of removeByOriginalId: a replaced popup + // (freedesktop replaces_id) leaves the screen, so its persisted file has + // to go too — otherwise it resurrects on the next shell restart. + // keepFileName is the replacement's own file: a same-millisecond + // replacement shares the replaced row's filename, and the new write is + // already queued — deleting that path here would erase the replacement's + // only file. + function removePopupsByOriginalId(originalId, keepFileName) { + for (var i = popupModel.count - 1; i >= 0; i--) { + var row = popupModel.get(i) + if (!row || row.originalId !== originalId) continue + // Not a replaces_id match — see isRestoredRow. Removing it here + // would silently kill a restored critical alert on an unrelated ping. + if (isRestoredRow(row)) continue + if (NotificationLogic.popupFileName(row) !== keepFileName) deletePopupFileFor(row) + popupModel.remove(i) + } + } + // Remove every row in `model` whose originalId matches. Chat apps reuse // `replaces_id` per the freedesktop spec to update a single notification // in place — without this, every Discord/Slack ping leaves a fresh row @@ -232,12 +266,15 @@ Item { // Find a pending entry by its libnotify id and move it to pastModel. Called // when a popup naturally dismisses (timer expired or user clicked X / the - // default action) — the user is assumed to have seen it. - function markSeenByOriginalId(originalId) { + // default action) — the user is assumed to have seen it. Matches on + // timestamp too: a popup row and its pending row are born from the same + // snapshot, and id alone can point at an unrelated notification when a + // restored popup's old-generation id has been reused. + function markSeenByOriginalId(originalId, timestamp) { Qt.callLater(function() { for (var i = 0; i < pendingModel.count; i++) { var entry = pendingModel.get(i) - if (!entry || entry.originalId !== originalId) continue + if (!entry || entry.originalId !== originalId || entry.timestamp !== timestamp) continue var snapshot = service.snapshotFromRow(entry) pendingModel.remove(i) pastModel.insert(0, snapshot) @@ -295,7 +332,18 @@ Item { if (index < 0 || index >= popupModel.count) return var entry = popupModel.get(index) var originalId = entry ? entry.originalId : -1 - var ref = originalId >= 0 ? liveRefs[originalId] : null + var timestamp = entry ? entry.timestamp : 0 + // A restored row has no live server object, and its old-generation id + // may meanwhile belong to a fresh notification — resolving liveRefs by + // id would dismiss that unrelated notification at the server. + var restored = isRestoredRow(entry) + var ref = !restored && originalId >= 0 ? liveRefs[originalId] : null + // The popup is leaving the screen — for any reason — so its persisted + // file must not survive to the next shell restart. + if (entry) { + deletePopupFileFor(entry) + if (restored) delete restoredPopups[NotificationLogic.popupFileName(entry)] + } popupModel.remove(index) if (ref) { try { @@ -308,7 +356,7 @@ Item { } } // User (or the lifetime timer) saw the popup — archive it. - if (originalId >= 0) markSeenByOriginalId(originalId) + if (originalId >= 0) markSeenByOriginalId(originalId, timestamp) } function clearPopups() { @@ -397,7 +445,9 @@ Item { function invokePopupDefault(index) { if (index < 0 || index >= popupModel.count) return var entry = popupModel.get(index) - var ref = entry ? liveRefs[entry.originalId] : null + // Restored rows have no live actions, and looking up liveRefs by their + // old-generation id could fire an unrelated fresh notification's action. + var ref = entry && !isRestoredRow(entry) ? liveRefs[entry.originalId] : null var invoked = false try { if (ref && ref.actions) { @@ -511,7 +561,7 @@ Item { Process { id: ensureDirsProc - command: ["mkdir", "-p", service.stateDir, service.imageCacheDir] + command: ["mkdir", "-p", service.stateDir, service.popupStateDir, service.imageCacheDir] running: false } @@ -532,6 +582,123 @@ Item { Process { id: deleteImageProc; running: false } + // ---------------------------------------------------- popup persistence + // + // Mirror every on-screen popup to its own file under popupStateDir so + // toasts survive shell restarts (notably the restart `omarchy-update` + // performs). Writes and deletes go through one serialized queue: a burst + // of replaces_id updates must not race a single reused Process, and + // ordering guarantees a delete issued after a write wins. + + // Popups restored from a previous shell process, keyed by their file + // name (timestamp-originalId) since ids alone repeat across server + // generations. The replaces_id handling and liveRefs lookups must not + // match these rows against fresh notifications. + property var restoredPopups: ({}) + + property var popupFileQueue: [] + + function enqueuePopupFileJob(command) { + popupFileQueue = popupFileQueue.concat([command]) + runNextPopupFileJob() + } + + function runNextPopupFileJob() { + if (popupFileProc.running || popupFileQueue.length === 0) return + popupFileProc.command = popupFileQueue[0] + popupFileQueue = popupFileQueue.slice(1) + popupFileProc.running = true + } + + Process { + id: popupFileProc + running: false + onExited: service.runNextPopupFileJob() + } + + function persistPopupFile(snapshot) { + // The JSON travels as an argument, not through shell interpolation, so + // summaries/bodies with quotes or backticks can't break the command. The + // mkdir guards notifications that arrive before ensureDirsProc has run. + enqueuePopupFileJob(["bash", "-c", + "mkdir -p \"$1\" && printf '%s\\n' \"$2\" > \"$1/$3\"", "--", + popupStateDir, + NotificationLogic.serializePopup(snapshot, NotificationUrgency.Normal), + NotificationLogic.popupFileName(snapshot)]) + } + + function deletePopupFileFor(row) { + if (!row) return + // History replays and the "no recent notifications" placeholder never + // had a file — rm -f on the computed path is a harmless no-op there. + enqueuePopupFileJob(["rm", "-f", popupStateDir + NotificationLogic.popupFileName(row)]) + } + + Process { + id: restorePopupsProc + running: false + stdout: StdioCollector { + waitForEnd: true + onStreamFinished: service.restorePopups(text) + } + } + + function restorePopups(raw) { + var entries = NotificationLogic.parsePopupFiles(raw, NotificationUrgency.Normal) + var now = Date.now() + var live = [] + for (var i = 0; i < entries.length; i++) { + var entry = entries[i] + var duration = durationFor(entry.urgency, entry.expireTimeout) + if (NotificationLogic.popupExpired(entry, duration, now)) { + deletePopupFileFor(entry) + continue + } + // Survivors restart with a full lifetime on purpose: shell restarts + // are rare, and a full look after the restart flicker beats resuming + // a toast with a second left on its clock. The reset is persisted as + // an absolute deadline so a second restart while the toast is still + // on screen judges it by the reset clock, not the original timestamp. + if (duration > 0) { + entry.deadline = now + duration + persistPopupFile(entry) + // deadline is persistence metadata, not a model role — fresh rows + // never carry it, and ListModel roles must stay consistent. + delete entry.deadline + } + live.push(entry) + } + if (live.length === 0) return + + Qt.callLater(function() { + for (var j = 0; j < live.length; j++) { + var restored = live[j] + // A notification received while the restore was reading the dir can + // already occupy this originalId with the same timestamp — then it + // IS this entry, live with its own file, and must be left alone. A + // different timestamp is indistinguishable between a genuine + // cross-restart replaces_id and a new-generation id coincidence, so + // show both: a briefly duplicated toast beats silently dropping a + // restored critical alert. + var duplicate = false + for (var k = 0; k < popupModel.count; k++) { + var row = popupModel.get(k) + if (row && row.originalId === restored.originalId && row.timestamp === restored.timestamp) { + duplicate = true + break + } + } + if (duplicate) continue + // Append (entries are newest-first) so restored toasts stack in + // their original order below anything that just arrived. Restored + // popups have no liveRefs entry — the server object died with the + // old shell — so dismissal and action fallbacks degrade gracefully. + service.restoredPopups[NotificationLogic.popupFileName(restored)] = true + popupModel.append(restored) + } + }) + } + // ---------------------------------------------------- history persistence FileView { @@ -660,7 +827,16 @@ Item { // Once mkdir has had a tick, load the existing history file. FileView // surfaces an empty string when the file doesn't exist; loadHistory // handles that path. - Qt.callLater(function() { historyFile.reload() }) + Qt.callLater(function() { + historyFile.reload() + // Re-show popups that were on screen when the previous shell died. + // The glob-through-bash tolerates a missing/empty dir (first run). + // awk 1 (not cat) so a torn file missing its trailing newline can't + // glue itself onto the next file and take a valid popup down with it. + restorePopupsProc.command = ["bash", "-c", + "awk 1 \"$1\"/*.json 2>/dev/null || true", "--", service.popupStateDir] + restorePopupsProc.running = true + }) } // ---------------------------------------------------- IPC diff --git a/test/shell.d/notifications-test.sh b/test/shell.d/notifications-test.sh index 27651836..9cb439fd 100644 --- a/test/shell.d/notifications-test.sh +++ b/test/shell.d/notifications-test.sh @@ -183,6 +183,78 @@ assertDeepEqual( ) assertEqual(recentRows.length, 5, 'notifications history replay is capped at five rows') +const popup = { + id: 7, + originalId: 7, + app: 'Mail', + appIcon: 'mail', + summary: 'New message', + body: 'Body', + image: '', + glyph: '', + urgency: 2, + expireTimeout: 2500, + timestamp: 1000 +} +assertEqual(notifications.popupFileName(popup), '1000-7.json', 'notifications name popup files by timestamp and id') +assertEqual( + notifications.serializePopup(popup, 1).indexOf('\n'), + -1, + 'notifications serialize popups to a single line' +) +assertEqual( + notifications.popupEntry({ id: 1, timestamp: 5 }, 1).urgency, + 1, + 'notifications default popup urgency to normal' +) +assertEqual( + notifications.popupEntry({ id: 1, timestamp: 5, expireTimeout: 4000 }, 1).expireTimeout, + 4000, + 'notifications preserve popup expire timeouts unlike history rows' +) + +const popupFiles = notifications.parsePopupFiles( + [ + notifications.serializePopup({ id: 1, originalId: 1, summary: 'old-generation', urgency: 2, timestamp: 100 }, 1), + notifications.serializePopup({ id: 1, originalId: 1, summary: 'new-generation', urgency: 1, timestamp: 300 }, 1), + notifications.serializePopup({ id: 2, originalId: 2, summary: 'critical', urgency: 2, timestamp: 200 }, 1), + '{ torn write' + ].join('\n'), + 1 +) +assertDeepEqual( + popupFiles.map(row => row.summary), + ['new-generation', 'critical', 'old-generation'], + 'notifications restore every persisted popup newest-first, never deduping ids across server generations' +) +assertDeepEqual( + notifications.parsePopupFiles('', 1), + [], + 'notifications restore nothing from an empty popup dir' +) + +assert(!notifications.popupExpired({ timestamp: 0 }, 0, 999999), 'critical popups never expire on restore') +assert(!notifications.popupExpired({ timestamp: 1000 }, 8000, 5000), 'popups within their lifetime are restored') +assert(notifications.popupExpired({ timestamp: 1000 }, 8000, 9000), 'popups past their lifetime are not restored') +assert( + !notifications.popupExpired({ timestamp: 1000, deadline: 20000 }, 8000, 15000), + 'a restore-reset deadline outranks the original popup timestamp' +) +assert( + notifications.popupExpired({ timestamp: 1000, deadline: 20000 }, 8000, 20000), + 'popups past their reset deadline are not restored' +) +assertEqual( + notifications.popupEntry(JSON.parse(notifications.serializePopup({ id: 1, originalId: 1, timestamp: 5, deadline: 9000 }, 1)), 1).deadline, + 9000, + 'notifications round-trip reset deadlines through popup files' +) +assertEqual( + 'deadline' in notifications.popupEntry({ id: 1, timestamp: 5 }, 1), + false, + 'notifications omit the deadline field until a restore sets it' +) + assertEqual(notifications.imageExtension('/tmp/screenshot.PNG'), 'png', 'notifications normalize image extensions') assertEqual(notifications.imageExtension('/tmp/no-extension'), 'png', 'notifications default missing image extension') assertEqual(notifications.imageExtension('/tmp/archive.reallylong'), 'png', 'notifications reject suspicious image extensions') @@ -196,4 +268,40 @@ assert( /function showHistory\(\): string \{\s*return service\.showRecentHistory\(\)\s*\}/.test(serviceQml), 'notifications history IPC replays recent notifications' ) +assert( + /readonly property string popupStateDir: stateDir \+ "notifications\/"/.test(serviceQml), + 'notifications service persists popups under the omarchy state dir' +) +assert( + serviceQml.split('persistPopupFile(snapshot)').length === 4, + 'notifications service persists both ephemeral and regular popups' +) +assert( + /if \(entry\) \{\s*\n\s*deletePopupFileFor\(entry\)[\s\S]{0,200}?popupModel\.remove\(index\)/.test(serviceQml), + 'notifications service deletes the popup file when a popup leaves the screen' +) +assert( + /restorePopupsProc\.running = true/.test(serviceQml), + 'notifications service restores persisted popups on startup' +) +assert( + /if \(isRestoredRow\(row\)\) continue/.test(serviceQml), + 'notifications service protects restored popups from new-generation id collisions' +) +assert( + /var ref = !restored && originalId >= 0 \? liveRefs\[originalId\] : null/.test(serviceQml), + 'notifications service never resolves a restored popup to a live server object' +) +assert( + /markSeenByOriginalId\(originalId, timestamp\)/.test(serviceQml), + 'notifications service archives pending rows by id and timestamp' +) +assert( + /popupFileName\(row\) !== keepFileName/.test(serviceQml), + 'notifications service keeps a same-millisecond replacement popup file' +) +assert( + /awk 1 \\"\$1\\"\/\*\.json/.test(serviceQml), + 'notifications service delimits every popup file during restore' +) JS