From ada8c262ca2beef9e42e00d50e5ebf3b63831cc5 Mon Sep 17 00:00:00 2001 From: allenliang2022 Date: Mon, 29 Jun 2026 03:38:42 +0800 Subject: [PATCH] fix(webui): render server-initiated turns in a hidden tab without a refresh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A turn started SERVER-SIDE (self-wake, cron, restart hook) fans a `server_turn_started` frame onto the per-session live-view SSE channel so an open tab renders it live. But while a tab is HIDDEN the WebUI deliberately does NOT hold that persistent SSE open (connection-pool budget — #3992 / #4151), so `get_session_channel()` returns None at fan-out time and the turn is dropped for that tab. The user only saw the turn after a manual interaction (refresh / send) re-subscribed the channel and replayed it. Bridge the gap with a lightweight poll of `/api/session/status` (one short GET per ~6s tick — NOT a held connection, so the connection-pool budget the hidden guard protects is preserved) that attaches the existing live renderer when it sees a *live* active stream. Backend (`api/session_ops.py`): - `session_status()` now exposes `active_stream_id`, derived through a new `_live_active_stream_id()` helper that only returns the id when the stream is genuinely live (present in STREAMS / ACTIVE_RUNS). A stale id left by a crashed/restarted run surfaces as None, so the poller never attaches a renderer to a dead stream. Additive field — existing consumers ignore it. Frontend (`static/messages.js`): - `_startHiddenActiveStreamPoll` / `_stopHiddenActiveStreamPoll` / `_attachServerInitiatedStream` implement the poll lifecycle. The poll fires one immediate tick on hide (so an in-flight turn is caught without waiting a full interval), attaches mid-flight turns via the reconnecting/replay path, and stops on re-show (the real SSE takes over), session switch, or once a stream is rendering. - Started on BOTH hidden paths: the `visibilitychange` hook's hidden branch (a visible tab going to background) and `startSessionStream`'s hidden early-return skip (a session opened while already hidden). The visible path and `stopSessionStream` clear it. Tests: - `tests/test_hidden_tab_server_initiated_turn.py` — backend live-validation (stale id → None; STREAMS/ACTIVE_RUNS id returned) and frontend lifecycle / both-hidden-paths / replay-attach source locks. - Widened the brittle fixed-width source-window slices in `tests/test_issue3996_sse_visibility.py` (1600/1700 → 2400) so the existing `visibilitychange` / hidden-skip assertions still find their markers after the poll-start lines were inserted into `startSessionStream`. --- api/session_ops.py | 46 +++++++ static/messages.js | 103 ++++++++++++++ .../test_hidden_tab_server_initiated_turn.py | 129 ++++++++++++++++++ tests/test_issue3996_sse_visibility.py | 4 +- 4 files changed, 280 insertions(+), 2 deletions(-) create mode 100644 tests/test_hidden_tab_server_initiated_turn.py diff --git a/api/session_ops.py b/api/session_ops.py index 6b1bef036..d1a3241d3 100644 --- a/api/session_ops.py +++ b/api/session_ops.py @@ -17,6 +17,36 @@ logger = logging.getLogger(__name__) AUTO_TITLE_LABELS = {'untitled', 'new chat'} +def _live_active_stream_id(session) -> str | None: + """Return session.active_stream_id ONLY if that stream is live in THIS + process; else None. + + After a restart/crash the persisted active_stream_id survives in the + session JSON but the in-memory STREAMS / ACTIVE_RUNS that actually drive a + live turn were wiped. Exposing that dead id (e.g. via /api/session/status to + the hidden-tab poller) would make a client attach its renderer to a stream + that never emits — a permanent fake "thinking" state. Liveness test mirrors + routes._clear_stale_stream_state: live iff present in STREAMS (open SSE + channel) or ACTIVE_RUNS (worker bookkeeping). + """ + stream_id = getattr(session, 'active_stream_id', None) + if not stream_id: + return None + try: + from api import config as _cfg + with _cfg.STREAMS_LOCK: + if stream_id in _cfg.STREAMS: + return stream_id + with _cfg.ACTIVE_RUNS_LOCK: + if stream_id in (_cfg.ACTIVE_RUNS or {}): + return stream_id + except Exception: + # On any introspection failure, fail SAFE (report no live stream) rather + # than surfacing a possibly-stale id. + return None + return None + + def session_has_manual_title(session) -> bool: """Return whether adaptive title refresh should leave this title alone.""" return getattr(session, 'manual_title', False) is True @@ -242,6 +272,22 @@ def session_status(session_id: str) -> dict[str, Any]: 'created_at': s.created_at, 'updated_at': s.updated_at, 'agent_running': bool(getattr(s, 'active_stream_id', None)), + # Expose the stream id itself (not just the agent_running bool) so a + # hidden-tab poller can attach the live renderer to a server-initiated + # turn (self-wake / cron / restart hook) without opening the persistent + # per-session SSE while the tab is hidden. See messages.js hidden-tab + # active-stream poll. Additive field — existing consumers ignore it. + # + # CRITICAL: only expose a stream id that is actually LIVE in this + # process. After a restart/crash the persisted active_stream_id is stale + # (the in-memory STREAMS/ACTIVE_RUNS were wiped) — handing that dead id + # to the poller would make it attach a renderer to a stream that never + # produces tokens (a permanent fake "thinking" state). Mirror + # _clear_stale_stream_state's liveness test: a stream counts as live + # only if it's in STREAMS (SSE channel open) or ACTIVE_RUNS (worker + # bookkeeping). Otherwise report None so the poller waits for a REAL + # server_turn_started instead of latching a ghost. + 'active_stream_id': _live_active_stream_id(s), 'input_tokens': inp, 'output_tokens': out, 'total_tokens': inp + out, diff --git a/static/messages.js b/static/messages.js index de17fa888..011fca3d6 100644 --- a/static/messages.js +++ b/static/messages.js @@ -6319,6 +6319,96 @@ let _sessionStreamReconnectTimer = null; // Holds the session id across a hidden-tab close so the visibility handler can // reopen the per-session SSE on re-show (stopSessionStream nulls _sessionStreamSessionId). let _sessionStreamHiddenSid = null; +// Hidden-tab active-stream poll (Defect B continuation): while the tab is +// hidden we do NOT hold the persistent per-session SSE open (connection-pool +// budget — see #3992/#4151). But a server-initiated turn (self-wake / cron / +// restart hook) fans `server_turn_started` onto that channel, so a hidden tab +// would miss it and only reconcile on the next interaction ("过了15秒没弹出来"). +// Bridge the gap with a lightweight poll of /api/session/status (a single +// short-lived GET, NOT a held connection) that attaches the live renderer when +// it sees an active_stream_id. Cleared on re-show (the real SSE takes over) and +// on session switch. +let _sessionStreamHiddenPollTimer = null; +let _sessionStreamHiddenPollSid = null; + +// Attach the existing chat-stream renderer to a server-created stream. Shared +// by the `server_turn_started` SSE handler (visible tab) and the hidden-tab +// active-stream poll. Idempotent per (sid, streamId): bails if this tab is +// already rendering that stream. `recovered` routes through the reconnecting +// (replay) path so the renderer rebuilds from the run journal mid-flight. +function _attachServerInitiatedStream(sid, streamId, recovered) { + try { + streamId = String(streamId || ''); + if (!streamId) return; + const isCurrent = (typeof _isSessionCurrentPane === 'function') + ? _isSessionCurrentPane(sid) + : (S.session && S.session.session_id === sid); + if (!isCurrent) return; + if (S.activeStreamId === streamId) return; + const existingLive = (typeof LIVE_STREAMS !== 'undefined') ? LIVE_STREAMS[sid] : null; + if (existingLive && existingLive.streamId === streamId) return; + S.busy = true; + S.activeStreamId = streamId; + if (S.session && S.session.session_id === sid) { + S.session.active_stream_id = streamId; + if (!S.session.pending_started_at) S.session.pending_started_at = Date.now()/1000; + } + if (typeof ensureLiveWorklogShell === 'function') ensureLiveWorklogShell(); + else if (typeof appendThinking === 'function') appendThinking(); + if (typeof updateSendBtn === 'function') updateSendBtn(); + if (typeof setComposerStatus === 'function') setComposerStatus(''); + if (typeof syncTopbar === 'function') syncTopbar(); + if (typeof startApprovalPolling === 'function') startApprovalPolling(sid); + if (typeof startClarifyPolling === 'function') startClarifyPolling(sid); + if (typeof attachLiveStream === 'function') { + attachLiveStream( + sid, streamId, + (S.session && S.session.pending_attachments) || [], + recovered ? {reconnecting: true} : {}, + ); + } + if (typeof renderSessionList === 'function') void renderSessionList(); + } catch (_) {} +} + +// Poll /api/session/status (~6s) for an active stream while the tab is hidden. +// One short GET per tick — does not consume a persistent connection-pool slot. +// On hit, attach via the reconnecting/replay path (the turn is already +// mid-flight) and stop polling; the renderer owns it from here. +function _startHiddenActiveStreamPoll(sid) { + if (!sid) return; + _stopHiddenActiveStreamPoll(); + _sessionStreamHiddenPollSid = sid; + const tick = () => { + // Stop conditions: tab became visible (real SSE takes over), session + // switched, or we're already rendering a stream. + if (typeof document !== 'undefined' && !document.hidden) { _stopHiddenActiveStreamPoll(); return; } + if (_sessionStreamHiddenPollSid !== sid) { _stopHiddenActiveStreamPoll(); return; } + if (S.activeStreamId) return; // already rendering; wait it out + try { + fetch(_apiUrl('api/session/status?session_id=' + encodeURIComponent(sid)), {credentials: 'same-origin'}) + .then(r => r.ok ? r.json() : null) + .then(d => { + if (!d || _sessionStreamHiddenPollSid !== sid) return; + const streamId = d.active_stream_id; + if (streamId && S.activeStreamId !== String(streamId)) { + // Server-initiated turn in flight while hidden → attach as replay. + _attachServerInitiatedStream(sid, streamId, true); + } + }) + .catch(() => {}); + } catch (_) {} + }; + _sessionStreamHiddenPollTimer = setInterval(tick, 6000); + // Fire one immediately so a turn already running when we go hidden is caught + // without waiting a full interval. + tick(); +} + +function _stopHiddenActiveStreamPoll() { + if (_sessionStreamHiddenPollTimer) { clearInterval(_sessionStreamHiddenPollTimer); _sessionStreamHiddenPollTimer = null; } + _sessionStreamHiddenPollSid = null; +} function startSessionStream(sid) { if (!sid) return; @@ -6336,6 +6426,12 @@ function startSessionStream(sid) { if (document.hidden) { _sessionStreamHiddenSid = _sessionStreamSessionId; stopSessionStream(); + // Tab went to background: don't hold the SSE, but bridge + // server-initiated turns (self-wake / cron / restart) with the + // lightweight status poll so they still render without interaction. + // (stopSessionStream cleared any prior poll; start a fresh one for the + // session we just hid.) + if (_sessionStreamHiddenSid) _startHiddenActiveStreamPoll(_sessionStreamHiddenSid); } else if (_sessionStreamHiddenSid) { const resumeSid = _sessionStreamHiddenSid; _sessionStreamHiddenSid = null; @@ -6349,8 +6445,14 @@ function startSessionStream(sid) { // loaded/restored while the tab is already hidden must still reattach). if (typeof document !== 'undefined' && document.hidden) { _sessionStreamHiddenSid = sid; + // Don't hold the SSE open while hidden, but DO bridge server-initiated + // turns (self-wake / cron / restart) with a lightweight status poll so the + // turn renders without waiting for the user to interact. + _startHiddenActiveStreamPoll(sid); return; } + // Tab is visible — the real SSE owns live-view; ensure no stale hidden poll. + _stopHiddenActiveStreamPoll(); try { const es = new EventSource(_apiUrl('api/session/stream?session_id=' + encodeURIComponent(sid))); _sessionEventSource = es; @@ -6456,6 +6558,7 @@ function startSessionStream(sid) { function stopSessionStream() { if (_sessionStreamReconnectTimer) { clearTimeout(_sessionStreamReconnectTimer); _sessionStreamReconnectTimer = null; } + _stopHiddenActiveStreamPoll(); if (_sessionEventSource) { try { if(_sessionEventSource.readyState!==2)_sessionEventSource.close(); } catch(_){} _sessionEventSource = null; diff --git a/tests/test_hidden_tab_server_initiated_turn.py b/tests/test_hidden_tab_server_initiated_turn.py new file mode 100644 index 000000000..a8657dc13 --- /dev/null +++ b/tests/test_hidden_tab_server_initiated_turn.py @@ -0,0 +1,129 @@ +"""Hidden-tab server-initiated turn render (self-wake / cron / restart). + +A turn started SERVER-SIDE (self-wake, cron, restart hook) fans a +``server_turn_started`` frame onto the per-session live-view SSE channel so an +open tab renders it without a manual refresh. But while a tab is HIDDEN the +WebUI deliberately does NOT hold that persistent SSE open (connection-pool +budget — see issue #3992 / #4151). So a hidden tab missed server-initiated +turns and only reconciled on the next user interaction. + +This bridges the gap with a lightweight poll of ``/api/session/status`` (one +short GET per tick, NOT a held connection) that attaches the existing live +renderer when it sees a *live* ``active_stream_id``. These are source-lock +tests pinning the contract: + +- backend ``session_status`` exposes ``active_stream_id``, but only when the + stream is genuinely live (present in STREAMS / ACTIVE_RUNS) — a stale id left + over from a crashed/restarted run must surface as ``None`` so the poller never + attaches a renderer to a dead stream; +- frontend declares the poll lifecycle (start/stop/attach) and starts it on + BOTH hidden-tab paths: a session opened while already hidden, AND a visible + tab that transitions to hidden via the ``visibilitychange`` hook. +""" + +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +MESSAGES_JS = (REPO_ROOT / "static" / "messages.js").read_text(encoding="utf-8") +SESSION_OPS = (REPO_ROOT / "api" / "session_ops.py").read_text(encoding="utf-8") + + +# ── Backend: session_status exposes a LIVE-validated active_stream_id ─────── + +def test_session_status_exposes_active_stream_id_field(): + """session_status() must return an active_stream_id key for the poller.""" + assert "'active_stream_id'" in SESSION_OPS + # It is derived through the live-validation helper, not the raw attribute, + # so a stale id from a crashed/restarted run is not surfaced. + assert "_live_active_stream_id(" in SESSION_OPS + + +def test_live_active_stream_id_is_stale_safe(): + """The helper only returns an id that is actually live in STREAMS/ACTIVE_RUNS. + + Exercises the real helper: a made-up id (not in either registry) must come + back as None; an id present in STREAMS or ACTIVE_RUNS must be returned. + """ + import sys + sys.path.insert(0, str(REPO_ROOT)) + from types import SimpleNamespace + from api import config as cfg + from api.session_ops import _live_active_stream_id + + assert _live_active_stream_id(SimpleNamespace(active_stream_id=None)) is None + assert _live_active_stream_id(SimpleNamespace(active_stream_id="ghost-not-in-any-registry")) is None + + with cfg.STREAMS_LOCK: + cfg.STREAMS["live-streams-id"] = object() + try: + assert _live_active_stream_id(SimpleNamespace(active_stream_id="live-streams-id")) == "live-streams-id" + finally: + with cfg.STREAMS_LOCK: + cfg.STREAMS.pop("live-streams-id", None) + + with cfg.ACTIVE_RUNS_LOCK: + cfg.ACTIVE_RUNS["live-runs-id"] = object() + try: + assert _live_active_stream_id(SimpleNamespace(active_stream_id="live-runs-id")) == "live-runs-id" + finally: + with cfg.ACTIVE_RUNS_LOCK: + cfg.ACTIVE_RUNS.pop("live-runs-id", None) + + +# ── Frontend: poll lifecycle declared ────────────────────────────────────── + +def test_frontend_declares_hidden_poll_lifecycle(): + """The hidden-tab active-stream poll start/stop/attach functions exist.""" + assert "function _startHiddenActiveStreamPoll(sid)" in MESSAGES_JS + assert "function _stopHiddenActiveStreamPoll()" in MESSAGES_JS + assert "function _attachServerInitiatedStream(sid, streamId, recovered)" in MESSAGES_JS + + +def test_hidden_poll_hits_session_status_and_attaches_as_replay(): + """The poll tick fetches /api/session/status and attaches mid-flight turns. + + A server-initiated turn caught by the poll is already in progress, so it + must attach via the reconnecting/replay path (recovered=true) — the same + path the server_turn_started on-subscribe replay uses — rather than + expecting token 0. + """ + start = MESSAGES_JS.find("function _startHiddenActiveStreamPoll(sid)") + assert start != -1 + body = MESSAGES_JS[start:start + 1400] + assert "api/session/status?session_id=" in body + assert "d.active_stream_id" in body + # attaches as replay (recovered=true) — turn is already mid-flight + assert "_attachServerInitiatedStream(sid, streamId, true)" in body + + +def test_hidden_poll_started_on_both_hidden_paths(): + """The poll must start on BOTH ways a tab ends up hidden with a session. + + (1) visibilitychange → hidden: an already-open visible tab going to the + background still needs the bridge, so the hook's hidden branch starts it. + (2) startSessionStream early-return: a session loaded while the tab is + ALREADY hidden never opens the SSE, so its skip path starts it too. + """ + # Path 1: inside the visibilitychange hook's hidden branch. Anchor on the + # session-stream hook specifically (there are other unrelated + # visibilitychange listeners in the file). + hook_idx = MESSAGES_JS.find("_hermesSessionStreamVisibilityHook") + assert hook_idx != -1 + hook_block = MESSAGES_JS[hook_idx:hook_idx + 900] + assert "_startHiddenActiveStreamPoll(_sessionStreamHiddenSid)" in hook_block + + # Path 2: inside startSessionStream's hidden early-return skip. + start_idx = MESSAGES_JS.find("function startSessionStream(sid)") + block = MESSAGES_JS[start_idx:start_idx + 2400] + skip_idx = block.find("!== 'undefined' && document.hidden) {") + assert skip_idx != -1 + skip_block = block[skip_idx:skip_idx + 400] + assert "_startHiddenActiveStreamPoll(sid)" in skip_block + + +def test_hidden_poll_stops_on_session_teardown(): + """stopSessionStream() must also tear down the hidden poll (session switch).""" + stop_idx = MESSAGES_JS.find("function stopSessionStream()") + assert stop_idx != -1 + block = MESSAGES_JS[stop_idx:stop_idx + 400] + assert "_stopHiddenActiveStreamPoll()" in block diff --git a/tests/test_issue3996_sse_visibility.py b/tests/test_issue3996_sse_visibility.py index 7bddbde24..78e989e86 100644 --- a/tests/test_issue3996_sse_visibility.py +++ b/tests/test_issue3996_sse_visibility.py @@ -41,7 +41,7 @@ def test_session_stream_has_visibility_hook(): assert "_hermesSessionStreamVisibilityHook" in MESSAGES_JS start_idx = MESSAGES_JS.find("function startSessionStream(sid)") assert start_idx != -1 - block = MESSAGES_JS[start_idx:start_idx + 1600] + block = MESSAGES_JS[start_idx:start_idx + 2400] assert "visibilitychange" in block assert "document.hidden" in block # Must skip opening a new EventSource while the tab is hidden. @@ -95,7 +95,7 @@ def test_session_stream_hidden_open_preserves_id_for_reopen(): _sessionStreamHiddenSid = sid before returning. """ start_idx = MESSAGES_JS.find("function startSessionStream(sid)") - block = MESSAGES_JS[start_idx:start_idx + 1700] + block = MESSAGES_JS[start_idx:start_idx + 2400] # Find the hidden-tab early-return skip path (distinct from the in-hook # `if (document.hidden)` branch) and assert it preserves the id. hidden_idx = block.find("!== 'undefined' && document.hidden) {")