diff --git a/static/sessions.js b/static/sessions.js index 023cf845..04794a6d 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -231,16 +231,38 @@ function _isSessionActivelyViewedForList(sid) { function _isSessionLocallyStreaming(s) { if (!s || !s.session_id) return false; const isActive = S.session && s.session_id === S.session.session_id; - return Boolean( - (isActive && S.busy) - || (typeof INFLIGHT === 'object' && INFLIGHT && INFLIGHT[s.session_id]) - ); + // For the active session, rely on S.busy to indicate an ongoing stream. + // INFLIGHT entries for non-active sessions are artifacts of interrupted + // streams (page refresh, network disconnect, gateway restart) where + // `delete INFLIGHT[sid]` was never reached — they should NOT cause the + // sidebar spinner to appear on completed sessions. (#2066) + return isActive && Boolean(S.busy); } function _isSessionEffectivelyStreaming(s) { return Boolean(s && (s.is_streaming || _isSessionLocallyStreaming(s))); } +function _purgeStaleInflightEntries() { + // Clean up INFLIGHT entries for sessions the server confirms are NOT + // streaming. This prevents the in-memory cache from growing unbounded + // when streams end abnormally. (#2066) + if (typeof INFLIGHT !== 'object' || !INFLIGHT) return; + const sessionsById = new Map(); + if (Array.isArray(_allSessions)) { + for (const s of _allSessions) { + if (s && s.session_id) sessionsById.set(s.session_id, s); + } + } + for (const sid of Object.keys(INFLIGHT)) { + const s = sessionsById.get(sid); + if (s && !s.is_streaming) { + delete INFLIGHT[sid]; + if (typeof clearInflightState === 'function') clearInflightState(sid); + } + } +} + function _rememberRenderedStreamingState(s, isStreaming) { if (!s || !s.session_id || !isStreaming) return; _sessionStreamingById.set(s.session_id, true); @@ -2257,6 +2279,10 @@ function renderSessionListFromCache(){ // Don't re-render while user is actively renaming a session (would destroy the input) if(_renamingSid) return; closeSessionActionMenu(); + // Purge stale INFLIGHT entries for sessions the server confirms are NOT + // streaming. This runs on every list refresh to prevent memory leaks from + // interrupted streams. (#2066) + _purgeStaleInflightEntries(); const q=($('sessionSearch').value||'').toLowerCase(); const activeSidForSidebar=_activeSessionIdForSidebar(); const titleMatches=q?_allSessions.filter(s=>(s.title||'Untitled').toLowerCase().includes(q)):_allSessions; diff --git a/tests/test_issue2066_stale_sidebar_spinner.py b/tests/test_issue2066_stale_sidebar_spinner.py new file mode 100644 index 00000000..f1cf82c6 --- /dev/null +++ b/tests/test_issue2066_stale_sidebar_spinner.py @@ -0,0 +1,71 @@ +"""Regression checks for #2066 stale sidebar spinner state.""" + +import json +import subprocess +from pathlib import Path + + +SESSIONS_JS = (Path(__file__).resolve().parent.parent / "static" / "sessions.js").read_text() + + +def _function_block(name: str, next_name: str) -> str: + start = SESSIONS_JS.find(f"function {name}") + assert start != -1, f"{name} not found in sessions.js" + end = SESSIONS_JS.find(f"function {next_name}", start) + assert end != -1, f"{next_name} not found after {name}" + return SESSIONS_JS[start:end] + + +def test_local_streaming_only_uses_active_session_busy_state(): + block = _function_block("_isSessionLocallyStreaming", "_isSessionEffectivelyStreaming") + + assert "const isActive = S.session && s.session_id === S.session.session_id;" in block + assert "return isActive && Boolean(S.busy);" in block + assert "INFLIGHT[s.session_id]" not in block + assert "INFLIGHT && INFLIGHT[s.session_id]" not in block + + +def test_cache_render_purges_stale_non_streaming_inflight_entries(): + purge_block = _function_block("_purgeStaleInflightEntries", "_rememberRenderedStreamingState") + render_block = _function_block("renderSessionListFromCache", "_showProjectPicker") + + assert "const sessionsById = new Map();" in purge_block + assert "if (s && s.session_id) sessionsById.set(s.session_id, s);" in purge_block + assert "const s = sessionsById.get(sid);" in purge_block + assert "_allSessionsById" not in purge_block + assert "if (s && !s.is_streaming)" in purge_block + assert "delete INFLIGHT[sid];" in purge_block + assert "clearInflightState(sid);" in purge_block + assert "_purgeStaleInflightEntries();" in render_block + + +def test_stale_inflight_purge_executes_without_undeclared_session_map(): + purge_block = _function_block("_purgeStaleInflightEntries", "_rememberRenderedStreamingState") + script = f""" +let _allSessions = [ + {{session_id: 'done-session', is_streaming: false}}, + {{session_id: 'running-session', is_streaming: true}} +]; +let INFLIGHT = {{ + 'done-session': true, + 'running-session': true, + 'unknown-session': true +}}; +let cleared = []; +function clearInflightState(sid) {{ + cleared.push(sid); +}} +{purge_block} +_purgeStaleInflightEntries(); +console.log(JSON.stringify({{inflight: INFLIGHT, cleared}})); +""" + result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True) + payload = json.loads(result.stdout) + + assert payload == { + "inflight": { + "running-session": True, + "unknown-session": True, + }, + "cleared": ["done-session"], + } diff --git a/tests/test_issue856_background_completion_unread.py b/tests/test_issue856_background_completion_unread.py index 47a7da6c..324a535e 100644 --- a/tests/test_issue856_background_completion_unread.py +++ b/tests/test_issue856_background_completion_unread.py @@ -186,8 +186,8 @@ def test_polling_transition_tracks_the_same_effective_streaming_state_as_sidebar assert render_idx != -1, "_renderOneSession not found" render_block = SESSIONS_JS[render_idx:SESSIONS_JS.find("const hasUnread=", render_idx)] - assert "(isActive && S.busy)" in local_block - assert "INFLIGHT && INFLIGHT[s.session_id]" in local_block + assert "isActive && Boolean(S.busy)" in local_block + assert "INFLIGHT && INFLIGHT[s.session_id]" not in local_block assert "s.is_streaming || _isSessionLocallyStreaming(s)" in effective_block assert "const isStreaming=_isSessionEffectivelyStreaming(s);" in render_block, ( "the row spinner and polling completion transition must use the same " diff --git a/tests/test_issue856_pinned_indicator_layout.py b/tests/test_issue856_pinned_indicator_layout.py index 31a30778..93b2d644 100644 --- a/tests/test_issue856_pinned_indicator_layout.py +++ b/tests/test_issue856_pinned_indicator_layout.py @@ -129,8 +129,9 @@ def test_sidebar_uses_local_inflight_state_for_immediate_spinner(): messages_js = (Path(__file__).resolve().parent.parent / "static" / "messages.js").read_text() assert "function _isSessionLocallyStreaming(s)" in SESSIONS_JS - assert "(isActive && S.busy)" in SESSIONS_JS - assert "INFLIGHT[s.session_id]" in SESSIONS_JS + assert "isActive && Boolean(S.busy)" in SESSIONS_JS + assert "function _purgeStaleInflightEntries()" in SESSIONS_JS + assert "delete INFLIGHT[sid];" in SESSIONS_JS assert "function _isSessionEffectivelyStreaming(s)" in SESSIONS_JS assert "const isStreaming=_isSessionEffectivelyStreaming(s);" in SESSIONS_JS assert "if(typeof renderSessionListFromCache==='function') renderSessionListFromCache();" in messages_js