fix(#4753): notify background desktop tabs without hiding streams
This commit is contained in:
1 parent
8a98626967
commit
94e25ad494
3 files changed
+272
-11
No files matched your search
+45
-5
@@ -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){}});
|
||||
|
||||
@@ -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
|
||||
@@ -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():
|
||||
|
||||
Reference in new issue
Block a user