Fix two races in the notification popup and history handling

[dhh]

Follow-ups to the notification work in ab57ad6 and cd84583, from Copilot's review of the two PRs those shipped from and a codex pass over this branch. Kept together in one PR.

The replay could read the history mid-write. Popup files are written by a serialized queue of shell jobs, and showHistory read the directory as its own process alongside it. A dismissal issued a moment earlier could still be queued when the read ran, leaving that notification out of the replay it was the newest entry of; a clear issued a moment earlier could still be queued too, replaying entries it was about to remove. The read is now an entry in that queue rather than a process running beside it, which is what makes it a real barrier: it takes its place in line behind the work queued before the request and ahead of everything queued after, so a clear cannot delete files out from under awk mid-glob, no later job can overtake it, and unbroken file traffic cannot postpone it. It releases the queue on process exit rather than on output, so a read that comes back empty or fails cannot park the queue behind it.

An update could arrive before its popup had a row. Watching a notification for in-place updates starts when it is handed over, but the row those updates write to is inserted a tick later — deferred to keep a mid-incubation Repeater from being mutated underneath. A client fast enough to update inside that window found no row, and a property that has already changed does not change again, so the toast and its file stayed on the superseded content indefinitely. The row is now refreshed from the live notification once it exists. A refresh whose content matches the row it would write is dropped, which costs nothing in the common case and also collapses the several signals one multi-property update emits into a single rewrite.

Copilot's finding that the countdown should restart for every refreshed role, not just summary/body/image, I did not apply. The example given was urgency dropping from critical back to normal resuming a nearly-exhausted fraction, but a critical toast never ticks (lifetime is 0, so the timer never runs) and its remaining fraction is still a full 1.0 when it becomes normal. What is left is app name, icon, glyph and expire timeout, none of which change the text the user is reading, and resetting on those would let a client hold a toast on screen indefinitely by re-sending a hint.

Verified against the running shell: dismissAll, showHistory and clear issued back to back replays all four archived notifications and then empties the directory, so the archives landed ahead of the read and the clear behind it; a burst of eight notifications sent immediately after a replay request does not delay or displace it; new notifications still persist and archive after a read, including after one that found no history; and an in-place update still refreshes the toast and its persisted file.

— 🤖 Claude, posting on behalf of @dhh

[Copilot]

Pull request overview

Fixes notification update and history replay races.

Changes:

• Refreshes popups after deferred insertion.
• Skips redundant row/file updates.
• Delays history reads until prior file jobs complete.

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
shell/plugins/notifications/Service.qml	Coordinates popup refreshes and history reads.
shell/plugins/notifications/NotificationLogic.js	Adds popup-role comparison helpers.
test/shell.d/notifications-test.sh	Tests refresh and queue-idle behavior.

---

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

[dhh]

Right — fixed in bb1ff2d. runNextPopupFileJob() now returns early while readHistoryProc.running, so the read is a barrier in both directions, and the queue resumes from the read's onExited rather than from its output so an empty or failed read cannot park the queue behind it.

Verified live: showHistory immediately followed by clear replays all four archived notifications and then empties the directory, and notifications sent afterwards still persist and archive normally.

— 🤖 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 (1)

shell/plugins/notifications/Service.qml:408

• historyReadPending does not actually mark the replay's position in the queue. If a file job is running when showHistory is requested, then a later clear/archive is appended before that job exits; this branch keeps starting queued commands until the entire queue is empty, so the later operation runs before the read (and a continuous stream can starve the read). Insert a replay sentinel/barrier into popupFileQueue when showHistory is called, and start the read when that sentinel reaches the head so commands enqueued afterward remain behind it.

    if (readHistoryProc.running) return
    if (popupFileProc.running || popupFileQueue.length === 0) {
      if (!popupFileProc.running) startHistoryReadWhenIdle()
      return

[Copilot]

Pull request overview

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

Suppressed comments (1)

shell/plugins/notifications/Service.qml:529

• Queueing only the directory read does not preserve the ordering of file jobs created by the replay itself. With a live popup, showHistory followed by clear leaves clear waiting behind this read; when replayHistory() runs, clearPopups() enqueues the live popup's archive after that already-queued clear. The queue then clears history and moves the popup into it again, so the later clear request finishes with non-empty history. Keep replay-generated archive work at the read's queue position (ahead of jobs submitted after showHistory), or avoid archiving post-request rows during replay.

    enqueueHistoryRead()