fix(#5551): refresh sidebar recency after missed session events
A reactivated older conversation wasn't bumped to the top / into Today until a manual refresh when the viewing tab was backgrounded/unfocused: refreshSessionList early-returns on document.hidden unless force=true, and the sidebar SSE closes on blur — but the in-flight/pending coalescing only preserved the refresh REASON string, dropping the opts (force/refreshActive). So a coalesced 'sessions_changed' refresh lost its force flag and got skipped on a hidden tab. Now the pending request preserves merged opts (force/refreshActive OR-merged), and a dedicated resume-refresh path forces a catch-up when the sidebar regains focus, so cross-device recency re-sorts land without a manual refresh. Co-authored-by: rodboev <rodboev@users.noreply.github.com>
This commit is contained in:
1 parent
2cc2eb2b3c
commit
b36bd789ce
4 files changed
+231
-16
No files matched your search
+35
-11
@@ -4821,6 +4821,7 @@ let _activeSessionExternalRefreshInFlight = false;
|
||||
let _deferredActiveSessionExternalRefreshReason = '';
|
||||
let _sessionEventsSSE = null;
|
||||
let _sessionEventsRefreshTimer = 0;
|
||||
let _sessionEventsRefreshPendingRequest = null;
|
||||
let _sessionEventsReconnectTimer = 0;
|
||||
let _sessionEventsNeedsRefreshOnOpen = false;
|
||||
let _sessionEventsReconnectAttempt = 0;
|
||||
@@ -4834,7 +4835,20 @@ function _sessionEventsReconnectDelayMs(){
|
||||
return Math.min(_sessionEventsReconnectMaxMs, Math.floor(base * 0.75) + jitter);
|
||||
}
|
||||
let _sessionListRefreshInFlight = false;
|
||||
let _sessionListRefreshPendingReason = '';
|
||||
let _sessionListRefreshPendingRequest = null;
|
||||
|
||||
function _mergeSessionListRefreshOptions(prev, next){
|
||||
const merged = {...(prev||{}), ...(next||{})};
|
||||
if((prev&&prev.force===true)||(next&&next.force===true)) merged.force = true;
|
||||
if((prev&&prev.refreshActive===true)||(next&&next.refreshActive===true)) merged.refreshActive = true;
|
||||
return merged;
|
||||
}
|
||||
|
||||
function _refreshSessionListAfterSidebarResume(reason){
|
||||
// A direct resume refresh satisfies any pending onopen catch-up from the same close.
|
||||
_sessionEventsNeedsRefreshOnOpen = false;
|
||||
void refreshSessionList(reason, {force:true});
|
||||
}
|
||||
|
||||
function startStreamingPoll(){
|
||||
if(_streamingPollTimer) return;
|
||||
@@ -5025,7 +5039,10 @@ async function refreshSessionList(reason='manual', opts={}){
|
||||
const refreshActive = !!(opts && opts.refreshActive);
|
||||
if(!force && typeof document !== 'undefined' && document.hidden) return;
|
||||
if(_sessionListRefreshInFlight){
|
||||
_sessionListRefreshPendingReason = reason || 'session-list';
|
||||
_sessionListRefreshPendingRequest = {
|
||||
reason: reason || 'session-list',
|
||||
opts:_mergeSessionListRefreshOptions(_sessionListRefreshPendingRequest && _sessionListRefreshPendingRequest.opts, opts),
|
||||
};
|
||||
return;
|
||||
}
|
||||
_sessionListRefreshInFlight = true;
|
||||
@@ -5034,17 +5051,23 @@ async function refreshSessionList(reason='manual', opts={}){
|
||||
if(refreshActive) await refreshActiveSessionIfExternallyUpdated(reason||'session-list');
|
||||
}finally{
|
||||
_sessionListRefreshInFlight = false;
|
||||
const pendingReason = _sessionListRefreshPendingReason;
|
||||
_sessionListRefreshPendingReason = '';
|
||||
if(pendingReason) _scheduleSessionEventsRefresh(pendingReason);
|
||||
const pendingRequest = _sessionListRefreshPendingRequest;
|
||||
_sessionListRefreshPendingRequest = null;
|
||||
if(pendingRequest) _scheduleSessionEventsRefresh(pendingRequest.reason, pendingRequest.opts);
|
||||
}
|
||||
}
|
||||
|
||||
function _scheduleSessionEventsRefresh(reason){
|
||||
function _scheduleSessionEventsRefresh(reason, opts={}){
|
||||
_sessionEventsRefreshPendingRequest = {
|
||||
reason: reason || (_sessionEventsRefreshPendingRequest && _sessionEventsRefreshPendingRequest.reason) || 'event',
|
||||
opts:_mergeSessionListRefreshOptions(_sessionEventsRefreshPendingRequest && _sessionEventsRefreshPendingRequest.opts, opts),
|
||||
};
|
||||
if(_sessionEventsRefreshTimer) return;
|
||||
_sessionEventsRefreshTimer = setTimeout(() => {
|
||||
_sessionEventsRefreshTimer = 0;
|
||||
void refreshSessionList(reason||'event', {refreshActive:true});
|
||||
const request = _sessionEventsRefreshPendingRequest || {reason:'event', opts:{}};
|
||||
_sessionEventsRefreshPendingRequest = null;
|
||||
void refreshSessionList(request.reason||'event', request.opts);
|
||||
}, 300);
|
||||
}
|
||||
|
||||
@@ -5113,7 +5136,7 @@ function _installSidebarSseFocusHook(){
|
||||
// to prevent, in the multi-window scenario this fix targets (#4151).
|
||||
ensureSessionEventsSSE();
|
||||
if(!_gatewaySSE) startGatewaySSE();
|
||||
void refreshSessionList('focus');
|
||||
void _refreshSessionListAfterSidebarResume('focus');
|
||||
});
|
||||
}
|
||||
|
||||
@@ -5121,6 +5144,7 @@ function _closeSessionEventsSSE(){
|
||||
if(_sessionEventsSSE){
|
||||
try{if(_sessionEventsSSE.readyState!==2)_sessionEventsSSE.close();}catch(_){ }
|
||||
_sessionEventsSSE = null;
|
||||
_sessionEventsNeedsRefreshOnOpen = true;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5131,7 +5155,7 @@ function ensureSessionEventsSSE(){
|
||||
_closeSessionEventsSSE();
|
||||
}else{
|
||||
ensureSessionEventsSSE();
|
||||
void refreshSessionList('visible');
|
||||
void _refreshSessionListAfterSidebarResume('visible');
|
||||
}
|
||||
});
|
||||
document._hermesSessionEventsVisibilityHook = true;
|
||||
@@ -5147,7 +5171,7 @@ function ensureSessionEventsSSE(){
|
||||
_sessionEventsReconnectAttempt = 0;
|
||||
if(!_sessionEventsNeedsRefreshOnOpen) return;
|
||||
_sessionEventsNeedsRefreshOnOpen = false;
|
||||
void refreshSessionList('reconnect');
|
||||
void _refreshSessionListAfterSidebarResume('reconnect');
|
||||
};
|
||||
_sessionEventsSSE.addEventListener('sessions_changed', (ev) => {
|
||||
const activeProfile = S.activeProfile || 'default';
|
||||
@@ -5163,7 +5187,7 @@ function ensureSessionEventsSSE(){
|
||||
// Non-JSON payload (or transient malformed event). Keep legacy behavior:
|
||||
// refresh once event was seen.
|
||||
}
|
||||
_scheduleSessionEventsRefresh(eventTargetsActiveSession?'event-active-session':'event');
|
||||
_scheduleSessionEventsRefresh(eventTargetsActiveSession?'event-active-session':'event', {force:true, refreshActive:true});
|
||||
});
|
||||
_sessionEventsSSE.onerror = () => {
|
||||
_sessionEventsNeedsRefreshOnOpen = true;
|
||||
|
||||
@@ -34,4 +34,5 @@ def test_poll_path_skips_non_external_sessions():
|
||||
def test_session_events_refresh_active_session():
|
||||
src = SESSIONS_JS.read_text(encoding="utf-8")
|
||||
body = _function_body(src, "_scheduleSessionEventsRefresh")
|
||||
assert "refreshSessionList(reason||'event', {refreshActive:true})" in body
|
||||
assert "void refreshSessionList(request.reason||'event', request.opts)" in body
|
||||
assert "_scheduleSessionEventsRefresh(eventTargetsActiveSession?'event-active-session':'event', {force:true, refreshActive:true})" in src
|
||||
@@ -61,6 +61,7 @@ def test_focus_hook_closes_both_global_sidebar_streams():
|
||||
assert "window.addEventListener('focus'" in block
|
||||
assert "ensureSessionEventsSSE()" in block
|
||||
assert "startGatewaySSE()" in block
|
||||
assert "_refreshSessionListAfterSidebarResume('focus')" in block
|
||||
|
||||
|
||||
def test_blur_close_is_debounced_and_rechecks_focus_at_fire_time():
|
||||
|
||||
@@ -1,3 +1,7 @@
|
||||
import json
|
||||
import re
|
||||
import subprocess
|
||||
import textwrap
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
@@ -7,6 +11,33 @@ BOOT_JS = Path("static/boot.js").read_text(encoding="utf-8")
|
||||
PANELS_JS = Path("static/panels.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _function_body(src: str, name: str) -> str:
|
||||
# Extracted functions intentionally avoid braces in strings so this stays source-light.
|
||||
m = re.search(rf"(?:async\s+)?function\s+{re.escape(name)}\b", src)
|
||||
assert m, f"{name} function not found"
|
||||
sig_end = src.find(")", m.end())
|
||||
assert sig_end != -1, f"{name} function signature not terminated"
|
||||
brace_start = src.find("{", sig_end)
|
||||
assert brace_start != -1, f"{name} function body not found"
|
||||
depth = 0
|
||||
for idx in range(brace_start, len(src)):
|
||||
ch = src[idx]
|
||||
if ch == "{":
|
||||
depth += 1
|
||||
elif ch == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
return src[m.start():idx + 1]
|
||||
raise AssertionError(f"{name} function body not terminated")
|
||||
|
||||
|
||||
def _run_node(script: str) -> dict:
|
||||
result = subprocess.run(["node", "-e", script], capture_output=True, text=True)
|
||||
if result.returncode != 0:
|
||||
raise AssertionError(f"node failed: {result.stderr}")
|
||||
return json.loads(result.stdout)
|
||||
|
||||
|
||||
def test_load_session_supports_force_reload_for_external_refresh():
|
||||
assert "async function loadSession(sid)" in SESSIONS_JS
|
||||
assert "const opts = arguments[1] || {};" in SESSIONS_JS
|
||||
@@ -93,17 +124,25 @@ def test_session_time_refresh_has_own_visibility_hook():
|
||||
def test_session_list_external_refresh_uses_sse_invalidation_not_polling():
|
||||
"""New sessions should refresh the sidebar from server invalidation events."""
|
||||
assert "async function refreshSessionList(reason='manual', opts={})" in SESSIONS_JS
|
||||
assert "let _sessionListRefreshPendingRequest = null;" in SESSIONS_JS
|
||||
assert "function _mergeSessionListRefreshOptions(prev, next)" in SESSIONS_JS
|
||||
assert "function _refreshSessionListAfterSidebarResume(reason)" in SESSIONS_JS
|
||||
assert "_sessionEventsNeedsRefreshOnOpen = false" in SESSIONS_JS
|
||||
assert "void refreshSessionList(reason, {force:true})" in SESSIONS_JS
|
||||
assert "function ensureSessionEventsSSE()" in SESSIONS_JS
|
||||
assert "new EventSource('api/sessions/events')" in SESSIONS_JS
|
||||
assert "addEventListener('sessions_changed'" in SESSIONS_JS
|
||||
assert "function _scheduleSessionEventsRefresh(reason)" in SESSIONS_JS
|
||||
assert "function _scheduleSessionEventsRefresh(reason, opts={})" in SESSIONS_JS
|
||||
assert "let _sessionEventsRefreshPendingRequest = null;" in SESSIONS_JS
|
||||
assert "_sessionEventsNeedsRefreshOnOpen = true" in SESSIONS_JS
|
||||
assert "void refreshSessionList('reconnect')" in SESSIONS_JS
|
||||
assert "void _refreshSessionListAfterSidebarResume('focus')" in SESSIONS_JS
|
||||
assert "void _refreshSessionListAfterSidebarResume('visible')" in SESSIONS_JS
|
||||
assert "void _refreshSessionListAfterSidebarResume('reconnect')" in SESSIONS_JS
|
||||
assert "renderSessionList({deferWhileInteracting:!force})" in SESSIONS_JS
|
||||
assert "const refreshActive = !!(opts && opts.refreshActive)" in SESSIONS_JS
|
||||
assert "if(refreshActive) await refreshActiveSessionIfExternallyUpdated(reason||'session-list')" in SESSIONS_JS
|
||||
assert "_sessionListRefreshPendingReason = reason || 'session-list'" in SESSIONS_JS
|
||||
assert "if(pendingReason) _scheduleSessionEventsRefresh(pendingReason)" in SESSIONS_JS
|
||||
assert "_sessionListRefreshPendingRequest = {" in SESSIONS_JS
|
||||
assert "_scheduleSessionEventsRefresh(pendingRequest.reason, pendingRequest.opts)" in SESSIONS_JS
|
||||
assert "ensureSessionEventsSSE();" in SESSIONS_JS
|
||||
assert "document._hermesSessionEventsVisibilityHook" in SESSIONS_JS
|
||||
ensure_fn = SESSIONS_JS[SESSIONS_JS.find("function ensureSessionEventsSSE()") :]
|
||||
@@ -120,6 +159,156 @@ def test_session_list_external_refresh_uses_sse_invalidation_not_polling():
|
||||
assert "function _sessionEventTargetsActiveSession(payload)" in SESSIONS_JS
|
||||
assert "typeof payload.session_id === 'string'" in SESSIONS_JS
|
||||
assert "eventTargetsActiveSession?'event-active-session':'event'" in ensure_fn
|
||||
assert "_scheduleSessionEventsRefresh(eventTargetsActiveSession?'event-active-session':'event', {force:true, refreshActive:true})" in ensure_fn
|
||||
|
||||
|
||||
def test_session_events_refresh_forces_hidden_sidebar_render_from_event_path():
|
||||
"""The production sessions_changed path must force the hidden sidebar list render."""
|
||||
merge_body = (
|
||||
_function_body(SESSIONS_JS, "_mergeSessionListRefreshOptions")
|
||||
if "function _mergeSessionListRefreshOptions" in SESSIONS_JS
|
||||
else "function _mergeSessionListRefreshOptions(prev, next){ return {...(prev||{}), ...(next||{})}; }"
|
||||
)
|
||||
functions = "\n\n".join(
|
||||
[
|
||||
merge_body,
|
||||
_function_body(SESSIONS_JS, "refreshSessionList"),
|
||||
_function_body(SESSIONS_JS, "_scheduleSessionEventsRefresh"),
|
||||
]
|
||||
)
|
||||
script = textwrap.dedent(
|
||||
f"""
|
||||
const record = [];
|
||||
global.document = {{hidden: true}};
|
||||
global.renderSessionList = async (opts) => record.push({{kind: 'render', opts}});
|
||||
global.refreshActiveSessionIfExternallyUpdated = async (reason) => record.push({{kind: 'active', reason}});
|
||||
global.setTimeout = (cb) => {{
|
||||
cb();
|
||||
return 1;
|
||||
}};
|
||||
let _sessionListRefreshInFlight = false;
|
||||
let _sessionListRefreshPendingRequest = null;
|
||||
let _sessionEventsRefreshTimer = 0;
|
||||
let _sessionEventsRefreshPendingRequest = null;
|
||||
{functions}
|
||||
(async() => {{
|
||||
_scheduleSessionEventsRefresh('event-active-session', {{force:true, refreshActive:true}});
|
||||
await Promise.resolve();
|
||||
process.stdout.write(JSON.stringify(record));
|
||||
}})().catch((err) => {{
|
||||
process.stderr.write(String(err.stack || err) + '\\n');
|
||||
process.exit(1);
|
||||
}});
|
||||
"""
|
||||
)
|
||||
out = _run_node(script)
|
||||
assert out == [
|
||||
{"kind": "render", "opts": {"deferWhileInteracting": False}},
|
||||
{"kind": "active", "reason": "event-active-session"},
|
||||
]
|
||||
|
||||
|
||||
def test_session_list_external_refresh_forced_resume_survives_hidden_inflight_refresh():
|
||||
"""Queued resume refreshes must keep `force:true` even after a hidden invalidation."""
|
||||
functions = "\n\n".join(
|
||||
_function_body(SESSIONS_JS, name)
|
||||
for name in (
|
||||
"_mergeSessionListRefreshOptions",
|
||||
"refreshSessionList",
|
||||
"_scheduleSessionEventsRefresh",
|
||||
"_refreshSessionListAfterSidebarResume",
|
||||
)
|
||||
)
|
||||
script = textwrap.dedent(
|
||||
f"""
|
||||
const record = [];
|
||||
const state = {{hidden: false}};
|
||||
global.document = state;
|
||||
let renderCount = 0;
|
||||
global.renderSessionList = async (opts) => {{
|
||||
record.push({{kind: 'render', opts}});
|
||||
renderCount += 1;
|
||||
if (renderCount === 1) {{
|
||||
state.hidden = true;
|
||||
await refreshSessionList('focus', {{force:true}});
|
||||
}}
|
||||
}};
|
||||
global.refreshActiveSessionIfExternallyUpdated = async (reason) => {{
|
||||
record.push({{kind: 'active', reason}});
|
||||
}};
|
||||
global.setTimeout = (cb) => {{
|
||||
cb();
|
||||
return 1;
|
||||
}};
|
||||
global.clearTimeout = () => {{}};
|
||||
let _sessionListRefreshInFlight = false;
|
||||
let _sessionListRefreshPendingRequest = null;
|
||||
let _sessionEventsRefreshTimer = 0;
|
||||
let _sessionEventsRefreshPendingRequest = null;
|
||||
{functions}
|
||||
(async() => {{
|
||||
await refreshSessionList('event', {{refreshActive:true}});
|
||||
await refreshSessionList('manual');
|
||||
process.stdout.write(JSON.stringify({{
|
||||
record,
|
||||
resumeSrc: _refreshSessionListAfterSidebarResume.toString(),
|
||||
}}));
|
||||
}})().catch((err) => {{
|
||||
process.stderr.write(String(err.stack || err) + '\\n');
|
||||
process.exit(1);
|
||||
}});
|
||||
"""
|
||||
)
|
||||
out = _run_node(script)
|
||||
assert out["record"] == [
|
||||
{"kind": "render", "opts": {"deferWhileInteracting": True}},
|
||||
{"kind": "active", "reason": "event"},
|
||||
{"kind": "render", "opts": {"deferWhileInteracting": False}},
|
||||
]
|
||||
assert "refreshSessionList(reason, {force:true})" in out["resumeSrc"]
|
||||
assert "refreshActive:true" not in out["resumeSrc"]
|
||||
|
||||
|
||||
def test_session_events_refresh_timer_merges_pending_force_and_active_options():
|
||||
"""The debounce window must not drop force or active-refresh intent."""
|
||||
functions = "\n\n".join(
|
||||
_function_body(SESSIONS_JS, name)
|
||||
for name in (
|
||||
"_mergeSessionListRefreshOptions",
|
||||
"refreshSessionList",
|
||||
"_scheduleSessionEventsRefresh",
|
||||
)
|
||||
)
|
||||
script = textwrap.dedent(
|
||||
f"""
|
||||
const record = [];
|
||||
global.document = {{hidden: true}};
|
||||
global.renderSessionList = async (opts) => record.push({{kind: 'render', opts}});
|
||||
global.refreshActiveSessionIfExternallyUpdated = async (reason) => record.push({{kind: 'active', reason}});
|
||||
let timerCb = null;
|
||||
global.setTimeout = (cb) => {{ timerCb = cb; return 1; }};
|
||||
global.clearTimeout = () => {{}};
|
||||
let _sessionListRefreshInFlight = false;
|
||||
let _sessionListRefreshPendingRequest = null;
|
||||
let _sessionEventsRefreshTimer = 0;
|
||||
let _sessionEventsRefreshPendingRequest = null;
|
||||
{functions}
|
||||
(async() => {{
|
||||
_scheduleSessionEventsRefresh('focus', {{force:true}});
|
||||
_scheduleSessionEventsRefresh('event-active-session', {{refreshActive:true}});
|
||||
timerCb();
|
||||
await Promise.resolve();
|
||||
process.stdout.write(JSON.stringify(record));
|
||||
}})().catch((err) => {{
|
||||
process.stderr.write(String(err.stack || err) + '\\n');
|
||||
process.exit(1);
|
||||
}});
|
||||
"""
|
||||
)
|
||||
assert _run_node(script) == [
|
||||
{"kind": "render", "opts": {"deferWhileInteracting": False}},
|
||||
{"kind": "active", "reason": "event-active-session"},
|
||||
]
|
||||
|
||||
|
||||
def test_session_event_profile_filter_tolerates_default_root_aliases():
|
||||
|
||||
Reference in new issue
Block a user