fix(notifications): preserve prompt owners through lifecycle exits

README.md

@@ -291,6 +291,7 @@ If an AI assistant is helping with install, reinstall, bootstrap, provider setup
 - Unsaved changes guard -- discard/save prompt when closing with unpersisted changes
 - Cron completion alerts -- toast notifications and unread badges scoped to the active profile on the Tasks tab and session sidebar
 - Background agent error alerts -- banner when a non-active session encounters an error
+- Approval and clarification browser alerts notify once per pending owner when its session is not actively viewed. Local approval dismissal hides attention without resolving the prompt; resolution, replacement, and cancellation retire that owner's alert state. Denied or failed notification delivery can retry while the prompt remains pending.
 
 ### Slash commands
 - Type `/` in the composer for autocomplete dropdown

static/boot.js

@@ -93,6 +93,8 @@ async function cancelSessionStream(session){
     else setStatus('');
   }
   if(typeof _clearPendingPromptsForSession==='function') _clearPendingPromptsForSession(sid);
+  if(typeof stopApprovalPollingForSession==='function') stopApprovalPollingForSession(sid);
+  if(typeof stopClarifyPollingForSession==='function') stopClarifyPollingForSession(sid);
   if(typeof _approvalSessionId!=='undefined' && _approvalSessionId===sid){
     stopApprovalPolling();
     hideApprovalCard(true);

static/messages.js

@@ -7421,13 +7421,12 @@ let _approvalDisplayedOwner = null;
 
 const _DISMISSED_APPROVALS_KEY = 'hermes_dismissed_approvals';
 
-// Dismissed approvals are namespaced by session so that two sessions carrying
-// the SAME approval_id (e.g. a gateway/run source that reuses externally
-// supplied IDs across sessions) can't have a dismissal in one session hide the
-// other's still-pending approval. Stored value is "<sid>\u0000<approval_id>".
+// Dismissal uses the same injective full-owner tuple as notifications.
+// Legacy session/ID-only tombstones cannot safely identify a gateway run.
 function _approvalDismissKey(sid, approvalId) {
   if (!approvalId) return '';
-  return String(sid || '') + '\u0000' + String(approvalId);
+  const pending = typeof approvalId === 'object' ? approvalId : {approval_id: approvalId};
+  return _promptNotifyKey('approval', sid, pending);
 }
 
 function _getDismissedApprovals() {
@@ -7469,7 +7468,8 @@ function _approvalPromptBelongsToActiveSession(sid) {
 function activeSessionHasPendingPromptAttention() {
   const sid = _promptActiveSessionId();
   return !!(sid && (
-    _approvalPendingBySession.has(sid) ||
+    (_approvalPendingBySession.has(sid) &&
+      !_isApprovalDismissed(sid, _approvalPendingBySession.get(sid).pending)) ||
     _clarifyPendingBySession.has(sid)
   ));
 }
@@ -7486,10 +7486,6 @@ function _rememberApprovalPending(pending, pendingCount) {
   if (!pending) return null;
   const sid = pending._session_id || _promptActiveSessionId();
   if (!sid) return null;
-  if (pending.approval_id && _isApprovalDismissed(sid, pending.approval_id)) {
-    _clearApprovalPendingForSession(sid);
-    return sid;
-  }
   const prev = _approvalPendingBySession.get(sid);
   // A replacement pending entry DISPLACES the previous prompt for this
   // session: retire the displaced prompt notification-dedupe key so the same
@@ -7499,6 +7495,7 @@ function _rememberApprovalPending(pending, pendingCount) {
   if (prev && prev.pending
       && _promptNotifyKey("approval", sid, prev.pending) !== _promptNotifyKey("approval", sid, pending)) {
     _retirePromptNotifyKey("approval", sid, prev.pending);
+    _unmarkApprovalDismissed(sid, prev.pending);
     _bumpApprovalPromptGeneration(sid);
   } else if (!prev) {
     _bumpApprovalPromptGeneration(sid);
@@ -7512,7 +7509,8 @@ function _clearApprovalPendingForSession(sid) {
   if (sid) {
     const entry = _approvalPendingBySession.get(sid);
     _approvalPendingBySession.delete(sid);
-    if (entry) _bumpApprovalPromptGeneration(sid);
+    _bumpApprovalPromptGeneration(sid);
+    if (entry && entry.pending) _unmarkApprovalDismissed(sid, entry.pending);
     if (entry && entry.pending) _retirePromptNotifyKey('approval', sid, entry.pending);
     if (typeof syncTopbar === 'function') syncTopbar();
   }
@@ -7656,7 +7654,7 @@ function showApprovalForSession(sid, pending, pendingCount) {
 
 function showApprovalCard(pending, pendingCount) {
   const sid = _rememberApprovalPending(pending, pendingCount);
-  if (pending && pending.approval_id && _isApprovalDismissed(sid, pending.approval_id)) return;
+  if (pending && pending.approval_id && _isApprovalDismissed(sid, pending)) return;
   if(typeof _notifyPromptCard==='function') _notifyPromptCard('approval', sid, pending);
   if (!_approvalPromptBelongsToActiveSession(sid)) return;
   _approvalClearedOwner = null;
@@ -7717,10 +7715,11 @@ function showApprovalCard(pending, pendingCount) {
 
 function dismissApprovalCard() {
   const sid = _approvalSessionId;
-  if (_approvalCurrentId) _markApprovalDismissed(sid, _approvalCurrentId);
-  // Dismissal clears local attention while the server remains authoritative.
-  // A later poll of the same dismissed approval stays suppressed below.
-  _clearApprovalPendingForSession(sid);
+  const entry = _approvalPendingBySession.get(sid);
+  if (entry && entry.pending) _markApprovalDismissed(sid, entry.pending);
+  // Keep the unresolved owner for authoritative resolution/replacement.
+  // Local dismissal hides attention, not the server-owned prompt.
+  if (typeof syncTopbar === 'function') syncTopbar();
   hideApprovalCard(true);
 }
 
@@ -8509,7 +8508,7 @@ function _clearClarifyPendingForSession(sid) {
   if (sid) {
     const entry = _clarifyPendingBySession.get(sid);
     _clarifyPendingBySession.delete(sid);
-    if (entry) _bumpClarifyPromptGeneration(sid);
+    _bumpClarifyPromptGeneration(sid);
     if (entry && entry.pending) _retirePromptNotifyKey('clarify', sid, entry.pending);
     if (typeof syncTopbar === 'function') syncTopbar();
   }
@@ -9332,16 +9331,23 @@ function _notifyPromptCard(kind, sid, pending){
   if (typeof _isSessionActivelyViewed === 'function' && _isSessionActivelyViewed(sid)) return;
   if (typeof sendBrowserNotification !== 'function') return;
   if (typeof window !== 'undefined' && !window._notificationsEnabled) return;
-  _promptNotifySeen.set(key, Date.now());
+  const attempt = {};
+  _promptNotifySeen.set(key, attempt);
   // forceHidden: this caller already made the visibility decision (the
   // actively-viewed gate above). sendBrowserNotification's own live gate
   // ("notify only when document.hidden") would otherwise veto the
   // unfocused-but-visible case, which this feature explicitly notifies for.
-  if (kind === 'clarify') {
-    sendBrowserNotification('Clarification needed', p.question || p.description || 'Tool clarification needed', { sid, forceHidden: true });
-  } else {
-    sendBrowserNotification('Approval required', p.description || p.command || 'Tool approval needed', { sid, forceHidden: true });
-  }
+  const rollback = () => {
+    if (_promptNotifySeen.get(key) === attempt) _promptNotifySeen.delete(key);
+  };
+  try {
+    const delivery = kind === 'clarify'
+      ? sendBrowserNotification('Clarification needed', p.question || p.description || 'Tool clarification needed', { sid, forceHidden: true })
+      : sendBrowserNotification('Approval required', p.description || p.command || 'Tool approval needed', { sid, forceHidden: true });
+    Promise.resolve(delivery).then(accepted => {
+      if (accepted !== true) rollback();
+    }, rollback);
+  } catch (_) { rollback(); }
 }
 function sendBrowserNotification(title,body,options={}){
   const force=!!(options&&options.force);
@@ -9355,14 +9361,24 @@ function sendBrowserNotification(title,body,options={}){
   if(!force&&!window._notificationsEnabled) return;
   if(!force&&!forceHidden&&!_isBackgroundedForBrowserNotification()) return;
   if(!('Notification' in window)) return;
-  if(Notification.permission==='granted'){
-    _showPwaNotification(title,body,options).catch(()=>{try{new Notification(title||assistantDisplayName(),_notificationOptions(body,options));}catch(_err){}});
-  }else if(Notification.permission==='denied'){
+  const deliver = async () => {
+    try {
+      await _showPwaNotification(title,body,options);
+      return true;
+    } catch (_) {
+      try {
+        new Notification(title||assistantDisplayName(),_notificationOptions(body,options));
+        return true;
+      } catch (_err) { return false; }
+    }
+  };
+  if(Notification.permission==='granted') return deliver();
+  if(Notification.permission==='denied'){
     // Explicit "Send test" (force) deserves feedback instead of a silent no-op.
     if(force&&typeof showToast==='function') showToast(t('notifications_denied'),3500,'error');
-  }else{
-    requestNotificationPermission().then(p=>{if(p==='granted') _showPwaNotification(title,body,options).catch(()=>{try{new Notification(title||assistantDisplayName(),_notificationOptions(body,options));}catch(_err){}});});
+    return false;
   }
+  return requestNotificationPermission().then(p => p==='granted' ? deliver() : false).catch(() => false);
 }
 
 // ── /btw ephemeral stream ────────────────────────────────────────────────────

tests/test_issue4754_approval_dismiss_persist.py

@@ -53,45 +53,12 @@ def test_dismissed_set_capped_at_100():
 # Guard in showApprovalCard
 # ---------------------------------------------------------------------------
 
-def test_guard_in_show_approval_card():
-    compact = _compact(MESSAGES_JS)
-    # Guard must appear inside showApprovalCard, after _rememberApprovalPending
-    func_start = compact.find("functionshowApprovalCard(")
-    assert func_start != -1
-    # Locate the guard after the function start (dismissals are namespaced by
-    # session, so the guard passes sid + approval_id).
-    guard = "_isApprovalDismissed(sid,pending.approval_id)"
-    guard_idx = compact.find(guard, func_start)
-    assert guard_idx != -1, "guard _isApprovalDismissed must appear in showApprovalCard"
-    # _rememberApprovalPending must appear before the guard
-    remember = "_rememberApprovalPending("
-    remember_idx = compact.find(remember, func_start)
-    assert remember_idx != -1
-    assert remember_idx < guard_idx, "guard must come after _rememberApprovalPending"
 
 
-def test_guard_returns_early():
-    # The guard must be a return statement
-    compact = _compact(MESSAGES_JS)
-    assert "if(pending&&pending.approval_id&&_isApprovalDismissed(sid,pending.approval_id))return;" in compact
-
-
-# ---------------------------------------------------------------------------
-# dismissApprovalCard function
-# ---------------------------------------------------------------------------
-
 def test_dismiss_approval_card_defined():
     assert "function dismissApprovalCard(" in MESSAGES_JS
 
 
-def test_dismiss_approval_card_marks_dismissed():
-    compact = _compact(MESSAGES_JS)
-    func_start = compact.find("functiondismissApprovalCard(")
-    assert func_start != -1
-    body_end = compact.find("}", func_start)
-    body = compact[func_start:body_end + 1]
-    assert "_markApprovalDismissed(sid,_approvalCurrentId)" in body
-
 
 def test_dismiss_approval_card_hides_card():
     compact = _compact(MESSAGES_JS)
@@ -102,19 +69,6 @@ def test_dismiss_approval_card_hides_card():
     assert "hideApprovalCard(true)" in body
 
 
-def test_dismiss_approval_card_clears_pending_attention():
-    # dismissApprovalCard must call _clearApprovalPendingForSession so the tab
-    # indicator stops blinking after dismiss. Without this, _rememberApprovalPending
-    # reinsertes the pending entry on every poll tick and the indicator stays lit.
-    compact = _compact(MESSAGES_JS)
-    func_start = compact.find("functiondismissApprovalCard(")
-    assert func_start != -1
-    body_end = compact.find("}", func_start)
-    body = compact[func_start:body_end + 1]
-    assert "_clearApprovalPendingForSession(sid)" in body, (
-        "dismissApprovalCard must call _clearApprovalPendingForSession(sid)"
-    )
-
 
 def test_dismiss_approval_card_captures_sid_before_hide():
     # The session ID must be captured before hideApprovalCard(true) nulls out
@@ -286,7 +240,7 @@ def test_same_approval_id_in_two_sessions_does_not_collide():
         pytest.skip("node not available")
     helpers = "\n".join(
         _extract_fn(MESSAGES_JS, n)
-        for n in ("_approvalDismissKey", "_getDismissedApprovals",
+        for n in ("_promptNotifyKey", "_approvalDismissKey", "_getDismissedApprovals",
                   "_isApprovalDismissed", "_markApprovalDismissed", "_unmarkApprovalDismissed")
     )
     script = (

tests/test_prompt_notification_lifecycle.py

@@ -0,0 +1,112 @@
+"""Composed prompt-owner and real delivery regressions for #7493."""
+from tests.test_pwa_notification_controls import MESSAGES_JS, _extract_fn, _run_node
+
+
+def run(body, extra=()):
+    names = ['_promptNotifyKey', '_retirePromptNotifyKey', '_notifyPromptCard',
+             '_approvalPromptGeneration', '_bumpApprovalPromptGeneration',
+             '_clarifyPromptGeneration', '_bumpClarifyPromptGeneration',
+             '_rememberApprovalPending', '_clearApprovalPendingForSession',
+             '_clearClarifyPendingForSession', '_approvalDismissKey',
+             '_getDismissedApprovals', '_isApprovalDismissed', '_markApprovalDismissed',
+             '_unmarkApprovalDismissed', 'dismissApprovalCard',
+             '_startApprovalFallbackPoll', 'showApprovalCard',
+             'activeSessionHasPendingPromptAttention']
+    source = '\n'.join(_extract_fn(MESSAGES_JS, n) for n in names + list(extra))
+    return _run_node('''
+const _promptNotifySeen = new Map();
+const _approvalPendingBySession = new Map(), _clarifyPendingBySession = new Map();
+const _approvalPromptGenerationBySession = new Map(), _clarifyPromptGenerationBySession = new Map();
+const _DISMISSED_APPROVALS_KEY = 'dismissed';
+const store = new Map();
+const localStorage = {getItem:k=>store.get(k), setItem:(k,v)=>store.set(k,v)};
+const S = {busy:true,session:{session_id:'s'}};
+const _approvalPromptBelongsToActiveSession=()=>false;
+let _approvalSessionId = 's', _approvalCurrentId = 'id';
+let _approvalFallbackPollInFlight=false, _approvalPollTimer;
+let _clarifyPollingSessionId, _clarifyPollTimer, _clarifyFallbackPollInFlight=false;
+const _promptActiveSessionId=()=> 's';
+const _approvalPollingSessionMissingOrMismatched=()=>false;
+const _hideApprovalCardIfOwner=()=>{};
+const stopApprovalPolling=()=>{}, stopApprovalPollingForSession=()=>{};
+const hideApprovalCard=()=>{};
+const setInterval=()=>1;
+const _isSessionActivelyViewed=()=>false;
+const flush=()=>new Promise(r=>setImmediate(r));
+''' + source + '\n(async()=>{\n' + body + '\n})().catch(e=>{console.error(e);process.exit(1)});')
+
+
+def test_dismiss_poll_resolution_and_new_run_owner():
+    result = run('''
+const sent=[];
+global.sendBrowserNotification=()=>{sent.push(1);return true};
+const a={_session_id:'s',approval_id:'id',run_id:'r1',_gateway_mirror_token:'t1'};
+global.showApprovalForSession=(sid,p)=>showApprovalCard(p,1);
+showApprovalForSession('s',a);
+dismissApprovalCard();
+const dismissed=_isApprovalDismissed('s',a);
+if(activeSessionHasPendingPromptAttention())throw Error('dismissed attention remains');
+showApprovalForSession('s',a);
+const quiet=sent.length;
+const b={...a,run_id:'r2',_gateway_mirror_token:'t2'};
+showApprovalForSession('s',b);
+const replacement=sent.length;
+dismissApprovalCard();
+global.api=async()=>({pending:null});
+_startApprovalFallbackPoll('s');await flush();
+showApprovalForSession('s',b);
+console.log(JSON.stringify({dismissed,quiet,replacement,reused:sent.length}));
+''')
+    assert result == dict(dismissed=True, quiet=1, replacement=2, reused=3)
+
+
+def test_empty_map_terminal_clear_fences_inflight_poll():
+    result = run('''
+let resolve;const shown=[];
+global.api=()=>new Promise(r=>resolve=r);
+global.showApprovalForSession=(sid,p)=>shown.push(p);
+_startApprovalFallbackPoll('s');
+_clearApprovalPendingForSession('s');
+_clearClarifyPendingForSession('s');
+resolve({pending:{approval_id:'stale'}});await flush();
+console.log(JSON.stringify({shown,clarifyGeneration:_clarifyPromptGeneration('s')}));
+''')
+    assert result == dict(shown=[], clarifyGeneration=1)
+
+
+def test_real_delivery_denied_failed_and_inflight_retry():
+    result = run('''
+const window=global.window={_notificationsEnabled:true};
+let attempts=0,fail=false;
+window.Notification=global.Notification=function(){attempts++;if(fail)throw Error('unavailable')};
+Notification.permission='denied';
+global.assistantDisplayName=()=> 'Hermes';
+global._notificationOptions=()=>({});
+Object.defineProperty(global,'navigator',{value:{serviceWorker:{getRegistration:async()=>({active:true,showNotification:async()=>{throw Error('SW failed')}})}}});
+const p={approval_id:'id'};
+_notifyPromptCard('approval','s',p);await flush();
+Notification.permission='granted';fail=true;
+_notifyPromptCard('approval','s',p);await flush();
+const failedConsumed=_promptNotifySeen.size;
+fail=false;
+_notifyPromptCard('approval','s',p);
+_notifyPromptCard('approval','s',p);await flush();
+_notifyPromptCard('approval','s',p);await flush();
+console.log(JSON.stringify({attempts,failedConsumed,consumed:_promptNotifySeen.size}));
+''', ('sendBrowserNotification', '_showPwaNotification'))
+    assert result == dict(attempts=2, failedConsumed=0, consumed=1)
+
+
+def test_empty_clarify_poll_cannot_resurrect_after_terminal_clear():
+    result = run('''
+let _clarifyPollingSessionId, _clarifyPollTimer, _clarifyFallbackPollInFlight=false;
+let resolve;const shown=[];
+global.api=()=>new Promise(r=>resolve=r);
+global.showClarifyForSession=(sid,p)=>shown.push(p);
+_startClarifyFallbackPoll('s');
+_clearClarifyPendingForSession('s');
+resolve({pending:{clarify_id:'stale'}});await flush();
+console.log(JSON.stringify({shown}));
+''', ('_startClarifyFallbackPoll',))
+    assert result == dict(shown=[])
+

tests/test_pwa_notification_controls.py

@@ -43,73 +43,11 @@ def test_notification_payload_uses_completion_session_when_provided():
     assert "assistantText?assistantText.slice(0,100)" not in MESSAGES_JS
 
 
-def test_prompt_notifications_fire_from_card_renderer_chokepoint():
-    """Approval/clarify notifications are owned by the card renderers, so EVERY
-    surfacing path (live SSE, 1.5s fallback poll, post-respond 'next approval'
-    refresh, reload-while-pending) notifies — not just the active-session SSE
-    event. The SSE listeners no longer send notifications directly; the shared
-    _notifyPromptCard gate dedupes per prompt id so repeated poll ticks and
-    re-renders ping exactly once."""
-    # The chokepoint helper exists and both card renderers call it BEFORE the
-    # belongs-to-active-session guard (so a non-active session's prompt still
-    # notifies).
-    assert "function _notifyPromptCard(kind, sid, pending){" in MESSAGES_JS
-    for fn_name, kind in (("showApprovalCard", "approval"), ("showClarifyCard", "clarify")):
-        start = MESSAGES_JS.index(f"function {fn_name}(")
-        body_start = MESSAGES_JS.index("{", start)
-        depth = 0
-        for i in range(body_start, len(MESSAGES_JS)):
-            if MESSAGES_JS[i] == "{":
-                depth += 1
-            elif MESSAGES_JS[i] == "}":
-                depth -= 1
-                if depth == 0:
-                    body = MESSAGES_JS[start : i + 1]
-                    break
-        assert "_notifyPromptCard(" in body, f"{fn_name} must route notifications through _notifyPromptCard"
-    assert "_notifyPromptCard('approval', sid, pending);" in MESSAGES_JS
-    assert "_notifyPromptCard('clarify', sid, pending);" in MESSAGES_JS
-    # Dedupe gate: per logical owner - an INJECTIVE typed-tuple key
-    # (JSON.stringify of [kind, sid, prompt id, gateway run id, mirror token]),
-    # so delimiter-bearing producer strings cannot collide.
-    assert "function _promptNotifyKey(kind, sid, pending){" in MESSAGES_JS
-    assert "return JSON.stringify([kind, String(sid || ''), String(id)," in MESSAGES_JS
-    assert "const key = _promptNotifyKey(kind, sid, p);" in MESSAGES_JS
-    assert "if (_promptNotifySeen.has(key)) return;" in MESSAGES_JS
-    # Entries retire by prompt LIFECYCLE (the pending-clear chokepoints), never
-    # by wall-clock age - a prompt pending >10min must not re-notify.
-    assert "_PROMPT_NOTIFY_TTL_MS" not in MESSAGES_JS
-    assert "function _retirePromptNotifyKey(kind, sid, pending){" in MESSAGES_JS
-    assert "_retirePromptNotifyKey('approval', sid, entry.pending);" in MESSAGES_JS
-    assert "_retirePromptNotifyKey('clarify', sid, entry.pending);" in MESSAGES_JS
-    # Disabled notifications must not consume the notify, so enabling
-    # mid-prompt still re-notifies that owner.
-    assert "!window._notificationsEnabled) return;" in MESSAGES_JS
-    # Visibility gate: suppress only while the prompt's session is actively
-    # viewed (open in pane + tab visible + tab focused). All three of the
-    # user's notification cases fire: non-active session, hidden tab,
-    # unfocused window.
-    assert "_isSessionActivelyViewed(sid)) return;" in MESSAGES_JS
-    # SSE listeners delegate; they must not send directly (would bypass dedupe).
-    approval_listener = _source_between(
-        "source.addEventListener('approval',e=>{",
-        "source.addEventListener('clarify',e=>{",
-    )
-    clarify_listener = _source_between(
-        "source.addEventListener('clarify',e=>{",
-        "source.addEventListener('state_saved',e=>{",
-    )
-    assert "sendBrowserNotification(" not in approval_listener
-    assert "sendBrowserNotification(" not in clarify_listener
-    assert "_notifyPromptCard()" in approval_listener  # ownership comment anchor
-    assert "_notifyPromptCard()" in clarify_listener
-
-
 def _extract_fn(src: str, name: str) -> str:
     marker = f"function {name}"
     start = src.find(marker)
     assert start >= 0, f"{name} not found"
-    brace = src.find("{", start)
+    brace = src.find("{", src.find(")", start))
     depth = 0
     for i in range(brace, len(src)):
         if src[i] == "{":
@@ -389,7 +327,7 @@ const ownerKey = _promptNotifyKey('approval', 'sid-1', {{ approval_id: 'd1' }});
 process.stdout.write(JSON.stringify({{ count: sent.length, bodies: sent.map(s => s.body), consumed: _promptNotifySeen.has(ownerKey) }}));
 """
     result = _run_node(script)
-    assert result["count"] == 2 or result["count"] == 1, f"unexpected ping count (got {result})"
+    assert result["count"] == 1, f"unexpected ping count (got {result})"
     assert result["bodies"][0] == 'after enabling', f"enabling mid-prompt must re-notify the same owner (got {result})"
     assert result["consumed"] is True, "the enabled notify must be recorded (consumed) for the owner"
 
@@ -414,6 +352,7 @@ def test_displaced_pending_prompt_key_is_retired_on_replacement():
     script = f"""
 const _promptNotifySeen = new Map();
 const _isApprovalDismissed = () => false;
+const _unmarkApprovalDismissed = () => {{}};
 const S = {{ session: {{ session_id: "sid-1" }} }};
 {keyfn}
 {retire}