Persist notification images so history keeps avatars

[dhh]

Notification history replayed entries without their avatars. The persisted JSON stored image/appIcon as URLs into resources that die with the live notification: Chromium-family senders (every Omarchy web app, WhatsApp included) pass avatars as files in a scoped /tmp dir deleted the moment the notification closes, and raw image-data hints surface as in-process image:// URLs that die with the server object. Replay then found dead references and hid the icon.

What changed

• Copy at persist time. File-backed images are copied into ~/.local/state/omarchy/notifications/images/, named by the entry's file stem (<timestamp>-<id>-appIcon / -image), and the persisted JSON references the copy. The copy runs in the same serialized queue job, before the JSON that references it. Dead image:// URLs are blanked so the card falls back to the app icon.
• Copies die with their JSON. Superseded-popup deletion, history trimming, and clear-history all remove the image files sharing the JSON's stem. A startup sweep collects copies orphaned by a restart killing a queued job between its cp and its JSON write.
• DND race. A silenced notification is now held (tracked) until its history write has run, released by a per-job done callback: untracking tells the sender its notification closed, and Chromium deletes the scoped avatar file the moment it hears so.
• Replay carry-over. On-screen rows carried into a history replay now reference the persisted copies, since the replay dismisses their live notifications first and the senders delete the originals on close.

— 🤖 Claude, posting on behalf of @dhh

[Copilot]

Pull request overview

Persists notification images so restored history retains avatars and icons.

Changes:

• Copies file-backed images into notification state storage.
• Cleans image copies alongside their JSON records.
• Delays releasing DND notifications until persistence completes.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
NotificationLogic.js	Builds persisted image paths and copy operations.
Service.qml	Integrates image lifecycle management and DND persistence.
notifications-test.sh	Adds persistence and cleanup coverage.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[dhh]

Fixed in 07aa969. The DND path now goes through writeSilenced, which re-snapshots the tracked object when each write completes and writes again (reusing the original file identity) until the content is stable — only then does releaseSilenced untrack. An in-place replaces_id update that lands while a write is queued is therefore persisted rather than dropped, and the release never closes the notification on stale content.

— 🤖 Claude, posting on behalf of @dhh

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

shell/plugins/notifications/Service.qml:183

• Holding this object tracked changes replaces_id behavior while this queued write is pending: Quickshell updates the existing notification object instead of emitting another onNotification, but this DND branch returns without connecting watchForUpdates. An update arriving before the callback is therefore discarded, and releaseSilenced then closes the updated object with only the stale first snapshot in history. Track DND updates and defer release until the latest snapshot has been persisted, or retain the old immediate-untracking semantics while preserving the image separately; add a regression test with an update delayed behind another file job.

      if (!isEphemeral(notification)) {
        writeHistoryFile(snapshot, function() {
          service.releaseSilenced(notification, snapshot.originalId)
        })
        return

[dhh]

Fixed in 07aa969. The copy no longer reopens the source with cp after the checks: it reads through timeout 5 head -c <limit+1> into a .tmp file in the images dir, validates the temp file's final size, and renames it into place atomically. A source that grows or is swapped for a FIFO mid-copy can now at most stall its own job for 5 seconds and never publishes an oversized or partial copy; the startup sweep clears any .tmp a killed job leaves behind.

— 🤖 Claude, posting on behalf of @dhh

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (2)

shell/plugins/notifications/Service.qml:515

• This queues the only read of the sender-owned image but does not tie notification closure to completion of that job. If another queue job is running (each image read may take up to 5 seconds), the toast can be dismissed or expire first; removePopup immediately calls ref.dismiss(), Chromium removes the scoped file, and this queued job later persists a missing copy. Track initial persistence completion per popup and defer closing the live notification until its image-copy job has completed.

    for (var i = 0; i < persistable.copies.length; i++)
      command.push(persistable.copies[i].from, persistable.copies[i].to)
    enqueuePopupFileJob(command)

shell/plugins/notifications/NotificationLogic.js:238

• The persisted role is redirected before the copy outcome is known, while copyImagesScript deliberately treats missing, timed-out, unreadable, and >5 MiB sources as non-fatal. The job still writes this JSON, so these cases leave a non-empty file://... URL with no file; for image, NotificationCard.smallIconSource then does not fall back to appIcon. Only redirect a role after its temp file was successfully renamed, and persist an empty role on copy failure.

    if (source) {
      var copy = String(imagesDir || "") + imageStem(e) + "-" + role
      if (source !== copy) copies.push({ from: source, to: copy })
      out[role] = "file://" + copy
    } else if (value.indexOf("image://") === 0) {
      out[role] = ""