From ac49d623f58f5a841d3e1d5eab094821d11ba48f Mon Sep 17 00:00:00 2001 From: CharlesMcquade <6466275+CharlesMcquade@users.noreply.github.com> Date: Sun, 13 Sep 2026 18:56:39 -0500 Subject: [PATCH] fix(notifications): close prompt lifecycle races --- static/boot.js | 2 + static/messages.js | 55 ++++++++++++++++--- ...t_issue4771_gateway_approval_failure_ui.py | 12 ++++ tests/test_pwa_notification_controls.py | 10 ++++ 4 files changed, 72 insertions(+), 7 deletions(-) diff --git a/static/boot.js b/static/boot.js index 526caa24..721f2b13 100644 --- a/static/boot.js +++ b/static/boot.js @@ -59,6 +59,7 @@ async function cancelStream(reason){ else setStatus(''); // /api/chat/cancel only exposes `cancelled:bool`, so we cannot // distinguish reasons — keep the toast generic and short. + if(typeof _clearPendingPromptsForSession==='function') _clearPendingPromptsForSession(sid); if(typeof showToast==='function') showToast('Stream is no longer active',2000); } return respOk; @@ -91,6 +92,7 @@ async function cancelSessionStream(session){ if(typeof setComposerStatus==='function') setComposerStatus(''); else setStatus(''); } + if(typeof _clearPendingPromptsForSession==='function') _clearPendingPromptsForSession(sid); if(typeof _approvalSessionId!=='undefined' && _approvalSessionId===sid){ stopApprovalPolling(); hideApprovalCard(true); diff --git a/static/messages.js b/static/messages.js index 24744498..85261739 100644 --- a/static/messages.js +++ b/static/messages.js @@ -7414,6 +7414,7 @@ function hideApprovalCard(force=false) { let _approvalSessionId = null; let _approvalCurrentId = null; // approval_id of the card currently shown let _approvalPendingBySession = new Map(); +const _approvalPromptGenerationBySession = new Map(); let _approvalResponding = null; let _approvalClearedOwner = null; let _approvalDisplayedOwner = null; @@ -7473,6 +7474,14 @@ function activeSessionHasPendingPromptAttention() { )); } +function _approvalPromptGeneration(sid) { + return Number(_approvalPromptGenerationBySession.get(sid) || 0); +} + +function _bumpApprovalPromptGeneration(sid) { + if (sid) _approvalPromptGenerationBySession.set(sid, _approvalPromptGeneration(sid) + 1); +} + function _rememberApprovalPending(pending, pendingCount) { if (!pending) return null; const sid = pending._session_id || _promptActiveSessionId(); @@ -7486,6 +7495,9 @@ function _rememberApprovalPending(pending, pendingCount) { if (prev && prev.pending && _promptNotifyKey("approval", sid, prev.pending) !== _promptNotifyKey("approval", sid, pending)) { _retirePromptNotifyKey("approval", sid, prev.pending); + _bumpApprovalPromptGeneration(sid); + } else if (!prev) { + _bumpApprovalPromptGeneration(sid); } const nextPending = {...pending, _session_id: sid}; _approvalPendingBySession.set(sid, {pending: nextPending, pendingCount: pendingCount || 1}); @@ -7496,6 +7508,7 @@ function _clearApprovalPendingForSession(sid) { if (sid) { const entry = _approvalPendingBySession.get(sid); _approvalPendingBySession.delete(sid); + if (entry) _bumpApprovalPromptGeneration(sid); if (entry && entry.pending) _retirePromptNotifyKey('approval', sid, entry.pending); if (typeof syncTopbar === 'function') syncTopbar(); } @@ -7639,9 +7652,9 @@ 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(typeof _notifyPromptCard==='function') _notifyPromptCard('approval', sid, pending); if (!_approvalPromptBelongsToActiveSession(sid)) return; - if (pending && pending.approval_id && _isApprovalDismissed(sid, pending.approval_id)) return; _approvalClearedOwner = null; const keys = pending.pattern_keys || (pending.pattern_key ? [pending.pattern_key] : []); const desc = (pending.description || "") + (keys.length ? " [" + keys.join(", ") + "]" : ""); @@ -7701,8 +7714,10 @@ function showApprovalCard(pending, pendingCount) { function dismissApprovalCard() { const sid = _approvalSessionId; if (_approvalCurrentId) _markApprovalDismissed(sid, _approvalCurrentId); + // Dismissal is local UI state, not authoritative prompt completion. Keep + // the pending owner and its notification-dedupe key until the server reports + // resolution or a terminal/cancel lifecycle clears it. hideApprovalCard(true); - if (sid) _clearApprovalPendingForSession(sid); } function _syncApprovalCollapseButton(card) { @@ -7894,7 +7909,9 @@ function _startApprovalFallbackPoll(sid) { if (_approvalFallbackPollInFlight) return; _approvalFallbackPollInFlight = true; try { + const generation = _approvalPromptGeneration(sid); const data = await api("/api/approval/pending?session_id=" + encodeURIComponent(sid),{timeoutToast:false}); + if (_approvalPollingSessionMissingOrMismatched(sid) || _approvalPromptGeneration(sid) !== generation) return; if (data.pending) { showApprovalForSession(sid, data.pending, data.pending_count||1); } else if (!_approvalPollingSessionMissingOrMismatched(sid)) { const _resolvedEntry = _approvalPendingBySession.get(sid); @@ -8449,12 +8466,21 @@ let _clarifyMissingEndpointWarned = false; let _clarifyCountdownTimer = null; let _clarifyExpiresAt = 0; let _clarifyPendingBySession = new Map(); +const _clarifyPromptGenerationBySession = new Map(); const CLARIFY_MIN_VISIBLE_MS = 30000; function _clarifyPromptBelongsToActiveSession(sid) { return !!(sid && _promptActiveSessionId() === sid); } +function _clarifyPromptGeneration(sid) { + return Number(_clarifyPromptGenerationBySession.get(sid) || 0); +} + +function _bumpClarifyPromptGeneration(sid) { + if (sid) _clarifyPromptGenerationBySession.set(sid, _clarifyPromptGeneration(sid) + 1); +} + function _rememberClarifyPending(pending) { if (!pending) return null; const sid = pending._session_id || _promptActiveSessionId(); @@ -8466,6 +8492,9 @@ function _rememberClarifyPending(pending) { if (prev && prev.pending && _promptNotifyKey("clarify", sid, prev.pending) !== _promptNotifyKey("clarify", sid, pending)) { _retirePromptNotifyKey("clarify", sid, prev.pending); + _bumpClarifyPromptGeneration(sid); + } else if (!prev) { + _bumpClarifyPromptGeneration(sid); } const nextPending = {...pending, _session_id: sid}; _clarifyPendingBySession.set(sid, {pending: nextPending}); @@ -8476,11 +8505,17 @@ function _clearClarifyPendingForSession(sid) { if (sid) { const entry = _clarifyPendingBySession.get(sid); _clarifyPendingBySession.delete(sid); + if (entry) _bumpClarifyPromptGeneration(sid); if (entry && entry.pending) _retirePromptNotifyKey('clarify', sid, entry.pending); if (typeof syncTopbar === 'function') syncTopbar(); } } +function _clearPendingPromptsForSession(sid) { + _clearApprovalPendingForSession(sid); + _clearClarifyPendingForSession(sid); +} + function _hideClarifyCardIfOwner(sid, force=false, reason="dismissed") { if (!sid || _clarifySessionId === sid) hideClarifyCard(force, reason); } @@ -9015,7 +9050,9 @@ function _startClarifyFallbackPoll(sid) { if (_clarifyFallbackPollInFlight) return; _clarifyFallbackPollInFlight = true; try { + const generation = _clarifyPromptGeneration(sid); const data = await api("/api/clarify/pending?session_id=" + encodeURIComponent(sid),{timeoutToast:false}); + if (!S.session || S.session.session_id !== sid || _clarifyPromptGeneration(sid) !== generation) return; if (data.pending) { showClarifyForSession(sid, data.pending); } else { _clearClarifyPendingForSession(sid); _hideClarifyCardIfOwner(sid, false, 'expired'); } } catch(e) { @@ -9257,7 +9294,11 @@ const _promptNotifySeen = new Map(); // notifications are disabled, so enabling mid-prompt re-notifies that owner. function _promptNotifyKey(kind, sid, pending){ const p = pending || {}; - const id = p.approval_id || p.clarify_id || ''; + // Older clarify producers can omit clarify_id. Their stable prompt payload is + // still a usable lifecycle identity and is retired with the pending entry. + const id = p.approval_id || p.clarify_id || (kind === 'clarify' + ? JSON.stringify([p.question || p.description || '', p.choices_offered || p.choices || [], p.requested_at || '']) + : ''); const runId = String(p.run_id || '').trim(); const mirrorToken = String(p._gateway_mirror_token || '').trim(); return JSON.stringify([kind, String(sid || ''), String(id), @@ -9266,13 +9307,13 @@ function _promptNotifyKey(kind, sid, pending){ } function _retirePromptNotifyKey(kind, sid, pending){ if (!pending) return; - const id = pending.approval_id || pending.clarify_id || ''; - if (!id) return; - _promptNotifySeen.delete(_promptNotifyKey(kind, sid, pending)); + const key = _promptNotifyKey(kind, sid, pending); + if (!key) return; + _promptNotifySeen.delete(key); } function _notifyPromptCard(kind, sid, pending){ const p = pending || {}; - const id = p.approval_id || p.clarify_id || ''; + const id = p.approval_id || p.clarify_id || (kind === 'clarify' ? p.question || p.description || '' : ''); if (!id) return; const key = _promptNotifyKey(kind, sid, p); if (_promptNotifySeen.has(key)) return; diff --git a/tests/test_issue4771_gateway_approval_failure_ui.py b/tests/test_issue4771_gateway_approval_failure_ui.py index 4825f418..d0b0928d 100644 --- a/tests/test_issue4771_gateway_approval_failure_ui.py +++ b/tests/test_issue4771_gateway_approval_failure_ui.py @@ -67,6 +67,10 @@ def _run_failure_case(api_js: str) -> dict: _extract_fn(MESSAGES_JS, "_getDismissedApprovals"), _extract_fn(MESSAGES_JS, "_isApprovalDismissed"), _extract_fn(MESSAGES_JS, "_unmarkApprovalDismissed"), + _extract_fn(MESSAGES_JS, "_promptNotifyKey"), + _extract_fn(MESSAGES_JS, "_retirePromptNotifyKey"), + _extract_fn(MESSAGES_JS, "_approvalPromptGeneration"), + _extract_fn(MESSAGES_JS, "_bumpApprovalPromptGeneration"), _extract_fn(MESSAGES_JS, "_promptActiveSessionId"), _extract_fn(MESSAGES_JS, "_approvalPromptBelongsToActiveSession"), _extract_fn(MESSAGES_JS, "_rememberApprovalPending"), @@ -103,6 +107,7 @@ let renderCalls = 0; let _approvalSessionId = 'sess-1'; let _approvalCurrentId = 'appr-1'; let _approvalPendingBySession = new Map(); +const _approvalPromptGenerationBySession = new Map(); let _loadSessionGeneration = 1; let _approvalResponding = null; let _approvalClearedOwner = null; @@ -111,6 +116,7 @@ let _approvalSignature = ''; let _approvalVisibleSince = 0; let _approvalHideTimer = null; const _clarifyPendingBySession = new Map(); +const _promptNotifySeen = new Map(); const buttons = new Map(); function makeButton(id) {{ return {{ @@ -258,6 +264,10 @@ def test_poll_rerender_keeps_inflight_buttons_disabled_and_blocks_duplicates(): _extract_fn(MESSAGES_JS, "_getDismissedApprovals"), _extract_fn(MESSAGES_JS, "_isApprovalDismissed"), _extract_fn(MESSAGES_JS, "_unmarkApprovalDismissed"), + _extract_fn(MESSAGES_JS, "_promptNotifyKey"), + _extract_fn(MESSAGES_JS, "_retirePromptNotifyKey"), + _extract_fn(MESSAGES_JS, "_approvalPromptGeneration"), + _extract_fn(MESSAGES_JS, "_bumpApprovalPromptGeneration"), _extract_fn(MESSAGES_JS, "_promptActiveSessionId"), _extract_fn(MESSAGES_JS, "_approvalPromptBelongsToActiveSession"), _extract_fn(MESSAGES_JS, "_rememberApprovalPending"), @@ -289,6 +299,7 @@ const _DISMISSED_APPROVALS_KEY = 'hermes_dismissed_approvals'; let _approvalSessionId = 'sess-1'; let _approvalCurrentId = 'appr-1'; let _approvalPendingBySession = new Map(); +const _approvalPromptGenerationBySession = new Map(); let _loadSessionGeneration = 1; let _approvalResponding = null; let _approvalClearedOwner = null; @@ -297,6 +308,7 @@ let _approvalSignature = ''; let _approvalVisibleSince = 0; let _approvalHideTimer = null; const _clarifyPendingBySession = new Map(); +const _promptNotifySeen = new Map(); let resolveApi; let apiCalls = 0; const buttons = new Map(); diff --git a/tests/test_pwa_notification_controls.py b/tests/test_pwa_notification_controls.py index cd7257be..48a72162 100644 --- a/tests/test_pwa_notification_controls.py +++ b/tests/test_pwa_notification_controls.py @@ -407,17 +407,27 @@ def test_displaced_pending_prompt_key_is_retired_on_replacement(): keyfn = _extract_fn(MESSAGES_JS, "_promptNotifyKey") retire = _extract_fn(MESSAGES_JS, "_retirePromptNotifyKey") active = _extract_fn(MESSAGES_JS, "_promptActiveSessionId") + approval_generation = _extract_fn(MESSAGES_JS, "_approvalPromptGeneration") + bump_approval_generation = _extract_fn(MESSAGES_JS, "_bumpApprovalPromptGeneration") + clarify_generation = _extract_fn(MESSAGES_JS, "_clarifyPromptGeneration") + bump_clarify_generation = _extract_fn(MESSAGES_JS, "_bumpClarifyPromptGeneration") script = f""" const _promptNotifySeen = new Map(); const S = {{ session: {{ session_id: "sid-1" }} }}; {keyfn} {retire} {active} +{approval_generation} +{bump_approval_generation} +{clarify_generation} +{bump_clarify_generation} {remember_ap} {remember_cl} {clear_ap} const _approvalPendingBySession = new Map(); const _clarifyPendingBySession = new Map(); +const _approvalPromptGenerationBySession = new Map(); +const _clarifyPromptGenerationBySession = new Map(); // Displaced approval owner: id-1, then a NEW different approval for sid-1. _rememberApprovalPending({{ approval_id: "id-1", description: "first" }}, 1); const key1 = _promptNotifyKey("approval", "sid-1", {{ approval_id: "id-1" }});