From 94e25ad494747de92f04c21544c0d9c68bc58586 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 23 Jun 2026 17:10:03 -0400 Subject: [PATCH] fix(#4753): notify background desktop tabs without hiding streams --- static/messages.js | 50 ++++- ...ue4753_desktop_background_notifications.py | 187 ++++++++++++++++++ tests/test_pwa_notification_controls.py | 46 ++++- 3 files changed, 272 insertions(+), 11 deletions(-) create mode 100644 tests/test_issue4753_desktop_background_notifications.py diff --git a/static/messages.js b/static/messages.js index 3888c762..3e454851 100644 --- a/static/messages.js +++ b/static/messages.js @@ -56,6 +56,23 @@ function _isDocumentVisibleAndFocused() { return true; } +let _desktopBackgroundedForNotifications=false; +// Desktop shells can background a visible document; keep that signal notification-only. +if(typeof window!=='undefined'){ + window.__hermesSetBackgrounded=(value)=>{ + _desktopBackgroundedForNotifications=!!value; + if(_desktopBackgroundedForNotifications){ + for(const k in _STREAM_NOTIFICATION_BACKGROUND){ + const e=_STREAM_NOTIFICATION_BACKGROUND[k]; + if(e) e.wasBackgrounded=true; + } + } + }; +} +function _isBackgroundedForBrowserNotification(){ + return !!(typeof document!=='undefined'&&document.hidden)||_desktopBackgroundedForNotifications; +} + function _isSessionCurrentPane(sid) { if(!sid || !S.session || S.session.session_id!==sid) return false; // During session switching, S.session still points at the previous row until @@ -1521,6 +1538,7 @@ async function send(){ } const LIVE_STREAMS={}; +const _STREAM_NOTIFICATION_BACKGROUND={}; // #4416: track whether the tab was hidden at ANY point during a live stream, so // the response-complete notification fires for a backgrounded tab even when @@ -1550,6 +1568,22 @@ function _clearStreamHidden(sid, streamId){ if(streamId&&e.streamId&&e.streamId!==streamId) return; delete _STREAM_WAS_HIDDEN[sid]; } +function _clearStreamNotificationBackground(sid, streamId){ + if(!sid) return; + const e=_STREAM_NOTIFICATION_BACKGROUND[sid]; + if(!e) return; + if(streamId&&e.streamId&&e.streamId!==streamId) return; + delete _STREAM_NOTIFICATION_BACKGROUND[sid]; +} +function _shouldForceCompletionNotification(sid, streamId){ + const hiddenEntry=_STREAM_WAS_HIDDEN[sid]; + const backgroundEntry=_STREAM_NOTIFICATION_BACKGROUND[sid]; + const wasHidden=!!(hiddenEntry&&hiddenEntry.wasHidden); + const wasBackgrounded=!!(backgroundEntry&&backgroundEntry.wasBackgrounded); + _clearStreamHidden(sid, streamId); + _clearStreamNotificationBackground(sid, streamId); + return wasHidden||wasBackgrounded; +} function closeLiveStream(sessionId, streamId, source){ const live=LIVE_STREAMS[sessionId]; @@ -1628,6 +1662,11 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ if(!_keep){ _STREAM_WAS_HIDDEN[activeSid]={streamId,wasHidden:(typeof document!=='undefined'&&!!document.hidden)}; } + const _prevBackground=_STREAM_NOTIFICATION_BACKGROUND[activeSid]; + const _keepBackground=reconnecting&&_prevBackground&&_prevBackground.streamId===streamId; + if(!_keepBackground){ + _STREAM_NOTIFICATION_BACKGROUND[activeSid]={streamId,wasBackgrounded:_desktopBackgroundedForNotifications}; + } } if(!INFLIGHT[activeSid]) INFLIGHT[activeSid]={messages:[...S.messages],uploaded:[...uploaded],toolCalls:[]}; else { @@ -1898,6 +1937,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ if(typeof finalizeThinkingCard==='function') finalizeThinkingCard(); _clearOwnerInflightState(); _clearStreamHidden(activeSid, streamId); // #4416: terminal path, drop hidden tracker + _clearStreamNotificationBackground(activeSid, streamId); _flushReasoningToAnchor(); _scheduleAnchorRegistryCleanup(); _clearApprovalForOwner(); @@ -4414,10 +4454,8 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ // delivers late — after the user returns and document.hidden is false). // If the user watched the whole stream, _wasEverHidden stays false and // the notification is suppressed (matches Slack/Discord/Gmail/Claude). - const _hiddenEntry=_STREAM_WAS_HIDDEN[activeSid]; - const _wasEverHidden=!!(_hiddenEntry&&_hiddenEntry.wasHidden); - _clearStreamHidden(activeSid, streamId); - sendBrowserNotification('Response complete',assistantText?assistantText.slice(0,100):'Task finished',{forceHidden:_wasEverHidden,sid:activeSid}); + const _wasEverBackgrounded=_shouldForceCompletionNotification(activeSid, streamId); + sendBrowserNotification('Response complete',assistantText?assistantText.slice(0,100):'Task finished',{forceHidden:_wasEverBackgrounded,sid:activeSid}); }; if(_shouldUseStreamFade()&&assistantBody){ _cancelAnimationFramePendingStreamRender(); @@ -4583,6 +4621,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ try{if(source&&source.readyState!==2)source.close();}catch(_){ } _clearOwnerInflightState(); _clearStreamHidden(activeSid, streamId); // #4416: terminal path, drop hidden tracker + _clearStreamNotificationBackground(activeSid, streamId); _clearApprovalForOwner(); _clearClarifyForOwner('terminal'); let d={}; @@ -4760,6 +4799,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ try{if(source&&source.readyState!==2)source.close();}catch(_){ } _clearOwnerInflightState(); _clearStreamHidden(activeSid, streamId); // #4416: terminal path, drop hidden tracker + _clearStreamNotificationBackground(activeSid, streamId); _clearApprovalForOwner(); _clearClarifyForOwner('cancelled'); let _cancelData={}; @@ -6307,7 +6347,7 @@ function sendBrowserNotification(title,body,options={}){ // explicit "Send test" override); only the visibility gate is bypassed. const forceHidden=!!(options&&options.forceHidden); if(!force&&!window._notificationsEnabled) return; - if(!force&&!forceHidden&&!document.hidden) 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){}}); diff --git a/tests/test_issue4753_desktop_background_notifications.py b/tests/test_issue4753_desktop_background_notifications.py new file mode 100644 index 00000000..c47c1c54 --- /dev/null +++ b/tests/test_issue4753_desktop_background_notifications.py @@ -0,0 +1,187 @@ +"""Behavioral coverage for desktop-backgrounded notification delivery (#4753).""" + +import json +import os +import shutil +import subprocess +import tempfile +from pathlib import Path + +import pytest + + +ROOT = Path(__file__).resolve().parents[1] +MESSAGES_SRC = (ROOT / "static" / "messages.js").read_text(encoding="utf-8") +NODE = shutil.which("node") + +pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH") + + +def _extract_function(source: str, name: str) -> str: + start = source.index(f"function {name}(") + body_start = source.index("){", start) + 1 + depth = 0 + for idx in range(body_start, len(source)): + char = source[idx] + if char == "{": + depth += 1 + elif char == "}": + depth -= 1 + if depth == 0: + return source[start : idx + 1] + raise AssertionError(f"{name} function body did not close") + + +def _notification_contract_source() -> str: + start = MESSAGES_SRC.index("let _desktopBackgroundedForNotifications=false;") + end = MESSAGES_SRC.index("function _isSessionCurrentPane", start) + tracker_start = MESSAGES_SRC.index("const _STREAM_NOTIFICATION_BACKGROUND={};") + tracker_end = MESSAGES_SRC.index("\n", tracker_start) + send = _extract_function(MESSAGES_SRC, "sendBrowserNotification") + return MESSAGES_SRC[start:end] + "\n" + MESSAGES_SRC[tracker_start:tracker_end] + "\n" + send + + +def _background_history_contract_source() -> str: + notification_start = MESSAGES_SRC.index("let _desktopBackgroundedForNotifications=false;") + notification_end = MESSAGES_SRC.index("function _isSessionCurrentPane", notification_start) + tracker_start = MESSAGES_SRC.index("const LIVE_STREAMS={};") + tracker_end = MESSAGES_SRC.index("function closeLiveStream", tracker_start) + return ( + MESSAGES_SRC[notification_start:notification_end] + + "\n" + + MESSAGES_SRC[tracker_start:tracker_end] + ) + + +def _run_notification_case(*, document_hidden: bool, desktop_backgrounded: bool = False, options=None): + payload = { + "documentHidden": document_hidden, + "desktopBackgrounded": desktop_backgrounded, + "options": options or {}, + } + script = ( + "const source = " + json.dumps(_notification_contract_source()) + ";\n" + + "const params = " + json.dumps(payload) + ";\n" + + r""" +const vm = require('vm'); +const shown = []; +const direct = []; + +function Notification(title, options) { + direct.push({ title, options }); +} +Notification.permission = 'granted'; + +const context = { + document: { hidden: params.documentHidden }, + window: { _notificationsEnabled: true }, + Notification, + assistantDisplayName: () => 'Hermes', + _notificationOptions: (body, options) => ({ body, tag: options && options.sid ? options.sid : '' }), + _showPwaNotification: (title, body, options) => { + shown.push({ title, body, options }); + return Promise.resolve(); + }, +}; +context.window.Notification = Notification; + +vm.createContext(context); +vm.runInContext(source, context); +context.window.__hermesSetBackgrounded(params.desktopBackgrounded); +vm.runInContext( + "sendBrowserNotification('Response complete','Task finished'," + JSON.stringify(params.options) + ");", + context +); + +console.log(JSON.stringify({ + shown, + direct, + documentHidden: context.document.hidden, + setterType: typeof context.window.__hermesSetBackgrounded, +})); +""" + ) + temp = tempfile.NamedTemporaryFile(mode="w", suffix=".js", delete=False, encoding="utf-8") + temp.write(script) + temp.close() + try: + result = subprocess.run([NODE, temp.name], capture_output=True, text=True, timeout=30) + if result.returncode != 0: + raise RuntimeError(f"node failed: {result.stderr}") + return json.loads(result.stdout.strip().splitlines()[-1]) + finally: + os.unlink(temp.name) + + +def test_visible_desktop_backgrounded_tab_notifies_without_page_visibility_hidden(): + result = _run_notification_case(document_hidden=False, desktop_backgrounded=True) + + assert result["setterType"] == "function" + assert result["documentHidden"] is False + assert len(result["shown"]) == 1 + assert result["shown"][0]["title"] == "Response complete" + + +def test_visible_foreground_tab_stays_silent_without_force(): + result = _run_notification_case(document_hidden=False, desktop_backgrounded=False) + + assert result["shown"] == [] + assert result["direct"] == [] + + +def test_real_hidden_tab_still_notifies_without_desktop_background_flag(): + result = _run_notification_case(document_hidden=True, desktop_backgrounded=False) + + assert len(result["shown"]) == 1 + assert result["shown"][0]["body"] == "Task finished" + + +def test_force_hidden_still_notifies_visible_documents(): + result = _run_notification_case( + document_hidden=False, + desktop_backgrounded=False, + options={"forceHidden": True}, + ) + + assert len(result["shown"]) == 1 + + +def test_late_done_still_notifies_after_desktop_backgrounded_tab_returns_foreground(): + script = ( + "const source = " + json.dumps(_background_history_contract_source()) + ";\n" + + r""" +const vm = require('vm'); +const context = { + document: { + hidden: false, + addEventListener: () => {}, + }, + window: {}, +}; +vm.createContext(context); +vm.runInContext(source, context); +vm.runInContext( + "_STREAM_WAS_HIDDEN['session-1']={streamId:'stream-1',wasHidden:false};" + + "_STREAM_NOTIFICATION_BACKGROUND['session-1']={streamId:'stream-1',wasBackgrounded:false};", + context +); +context.window.__hermesSetBackgrounded(true); +context.window.__hermesSetBackgrounded(false); +const first = vm.runInContext("_shouldForceCompletionNotification('session-1','stream-1')", context); +const second = vm.runInContext("_shouldForceCompletionNotification('session-1','stream-1')", context); +console.log(JSON.stringify({ first, second })); +""" + ) + temp = tempfile.NamedTemporaryFile(mode="w", suffix=".js", delete=False, encoding="utf-8") + temp.write(script) + temp.close() + try: + result = subprocess.run([NODE, temp.name], capture_output=True, text=True, timeout=30) + if result.returncode != 0: + raise RuntimeError(f"node failed: {result.stderr}") + payload = json.loads(result.stdout.strip().splitlines()[-1]) + finally: + os.unlink(temp.name) + + assert payload["first"] is True + assert payload["second"] is False diff --git a/tests/test_pwa_notification_controls.py b/tests/test_pwa_notification_controls.py index f38b4811..17b8e110 100644 --- a/tests/test_pwa_notification_controls.py +++ b/tests/test_pwa_notification_controls.py @@ -10,6 +10,18 @@ PANELS_JS = (ROOT / "static" / "panels.js").read_text(encoding="utf-8") I18N_JS = (ROOT / "static" / "i18n.js").read_text(encoding="utf-8") CHANGELOG = (ROOT / "CHANGELOG.md").read_text(encoding="utf-8") +DESKTOP_BACKGROUND_NOTIFICATION_NAMES = ( + "_desktopBackgroundedForNotifications", + "__hermesSetBackgrounded", + "_isBackgroundedForBrowserNotification", +) + + +def _source_between(start_marker: str, end_marker: str) -> str: + start = MESSAGES_JS.index(start_marker) + end = MESSAGES_JS.index(end_marker, start) + return MESSAGES_JS[start:end] + def test_browser_notifications_use_service_worker_when_available(): assert "function _showPwaNotification" in MESSAGES_JS @@ -25,7 +37,7 @@ def test_notification_payload_uses_completion_session_when_provided(): assert "_sessionUrlForSid(sid)" in MESSAGES_JS assert "data:{url}" in MESSAGES_JS assert "tag:sid?`hermes-${sid}`" in MESSAGES_JS - assert "sendBrowserNotification('Response complete',assistantText?assistantText.slice(0,100):'Task finished',{forceHidden:_wasEverHidden,sid:activeSid})" in MESSAGES_JS + assert "sendBrowserNotification('Response complete',assistantText?assistantText.slice(0,100):'Task finished',{forceHidden:_wasEverBackgrounded,sid:activeSid})" in MESSAGES_JS assert "sendBrowserNotification('Approval required',d.description||'Tool approval needed',{sid:activeSid})" in MESSAGES_JS assert "sendBrowserNotification('Clarification needed',d.question||'Tool clarification needed',{sid:activeSid})" in MESSAGES_JS @@ -42,16 +54,38 @@ def test_completion_notification_fires_when_tab_was_hidden_during_stream(): assert "function _bindStreamHiddenTracker" in MESSAGES_JS # Entries are stream-owned ({streamId, wasHidden}) so a stale entry from a # non-`done` terminal path can't be mis-attributed to a later same-sid stream. - assert "const _wasEverHidden=!!(_hiddenEntry&&_hiddenEntry.wasHidden);" in MESSAGES_JS + assert "function _shouldForceCompletionNotification(sid, streamId){" in MESSAGES_JS + assert "return wasHidden||wasBackgrounded;" in MESSAGES_JS assert "function _clearStreamHidden" in MESSAGES_JS - # Cleared on the non-done terminal paths too (belt-and-suspenders alongside - # the streamId-ownership guard). - assert MESSAGES_JS.count("_clearStreamHidden(activeSid, streamId)") >= 4 + assert "function _clearStreamNotificationBackground" in MESSAGES_JS + # Done-path cleanup lives inside _shouldForceCompletionNotification(); the + # activeSid call sites are the non-done terminal paths. + assert "_clearStreamHidden(sid, streamId);" in MESSAGES_JS + assert "_clearStreamNotificationBackground(sid, streamId);" in MESSAGES_JS + assert MESSAGES_JS.count("_clearStreamHidden(activeSid, streamId)") >= 3 + assert MESSAGES_JS.count("_clearStreamNotificationBackground(activeSid, streamId)") >= 3 # sendBrowserNotification honors forceHidden but still respects the # notifications-enabled setting (forceHidden is NOT the test-button force). assert "const forceHidden=!!(options&&options.forceHidden);" in MESSAGES_JS assert "if(!force&&!window._notificationsEnabled) return;" in MESSAGES_JS - assert "if(!force&&!forceHidden&&!document.hidden) return;" in MESSAGES_JS + assert "function _isBackgroundedForBrowserNotification(){" in MESSAGES_JS + assert "window.__hermesSetBackgrounded=(value)=>{" in MESSAGES_JS + assert "if(!force&&!forceHidden&&!_isBackgroundedForBrowserNotification()) return;" in MESSAGES_JS + + +def test_desktop_background_notification_signal_stays_out_of_stream_visibility(): + stream_tracker = _source_between( + "const LIVE_STREAMS={};", + "function closeLiveStream(sessionId, streamId, source){", + ) + deferred_recovery = _source_between( + "function _reattachOrRestoreAfterDeferredStreamError(source){", + " // Bug A fix (#631):", + ) + + for name in DESKTOP_BACKGROUND_NOTIFICATION_NAMES: + assert name not in stream_tracker + assert name not in deferred_recovery def test_service_worker_handles_notification_clicks_without_hijacking_other_sessions():