mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-07-21 15:10:30 +00:00
fix(webui): render server-initiated turns in a hidden tab without a refresh
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`.
This commit is contained in:
committed by
nesquena-hermes
parent
8cee867f19
commit
ada8c262ca
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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
|
||||
@@ -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) {")
|
||||
|
||||
Reference in New Issue
Block a user