Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 59 additions & 9 deletions static/sessions.js
Original file line number Diff line number Diff line change
Expand Up @@ -1203,6 +1203,7 @@ async function loadSession(sid){
snapshotLiveTurnHtmlForSession(currentSid);
}
}
const _keepStaleUntilLoaded = !!opts.keepStaleUntilLoaded && sameSessionForceReload;
if (currentSid !== sid || forceReload) {
// #3306: When force-reloading the currently-active session (e.g. external
// poll triggering a refresh), snapshot the existing messages BEFORE we
Expand All @@ -1221,10 +1222,27 @@ async function loadSession(sid){
// instead of collapsing a long session back to the default tail window.
if (sameSessionForceReload) _captureSameSessionForceReloadHint(sid);
else _clearSameSessionForceReloadHint();
S.messages = [];
S.toolCalls = [];
_messagesTruncated = false;
_oldestIdx = 0;
// #5177: keep-stale-until-loaded path — defer the destructive
// S.messages/toolCalls clear so the user does NOT see a transcript-wide
// blank gap during the metadata + messages round-trip. Only the
// visibility / focus recovery callers in refreshActiveSessionIfExternallyUpdated
// request this. The new transcript will be SWAPPED into S.messages by the
// forced _ensureMessagesLoaded(...{force:true}) call below, producing a
// single render frame with old DOM directly replaced by new DOM rather
// than the old → empty → new sequence the default branch produces.
//
// The session-switch branch (currentSid !== sid) MUST continue to clear
// synchronously — leaving a prior session's transcript on screen during a
// navigation is the original bug this clear was written for. We gate
// strictly on sameSessionForceReload (computed above as part of
// _keepStaleUntilLoaded) so cross-session switches keep their existing
// behaviour.
if (!_keepStaleUntilLoaded) {
S.messages = [];
S.toolCalls = [];
_messagesTruncated = false;
_oldestIdx = 0;
}
// Close live SSE streams from the session we're leaving. The error
// handler checks _isSessionActivelyViewed() and won't auto-reconnect
// for a backgrounded session, preventing leaked connections that would
Expand Down Expand Up @@ -1489,7 +1507,7 @@ async function loadSession(sid){
// this session's INFLIGHT snapshot, not leave prior-session rows in place.
if(typeof clearLiveToolCards==='function') clearLiveToolCards();
try {
await _ensureMessagesLoaded(sid);
await _ensureMessagesLoaded(sid, {force:_keepStaleUntilLoaded});
} catch(e) {
S.messages=inflightMessages;
}
Expand Down Expand Up @@ -1582,8 +1600,12 @@ async function loadSession(sid){
}else{
// Phase 2b: Idle session — load full messages lazily for rendering.
// _ensureMessagesLoaded is idempotent; it skips if S.messages already populated.
// #5177: when the caller asked us to keep stale messages until the new ones
// arrive (visibility/focus recovery), force the fetch so the
// "messages already populated" early-return inside _ensureMessagesLoaded
// does NOT skip the swap to the new transcript.
try {
await _ensureMessagesLoaded(sid);
await _ensureMessagesLoaded(sid, {force:_keepStaleUntilLoaded});
} catch (e) {
// Network errors, server failures, or SSE drops (Chrome error codes 4/5)
// can cause _ensureMessagesLoaded to throw. Without a try/catch here the
Expand Down Expand Up @@ -2336,9 +2358,21 @@ function _syncToolCallsForLoadedMessages(messages, sessionToolCalls){
}
}

async function _ensureMessagesLoaded(sid) {
async function _ensureMessagesLoaded(sid, opts) {
// `opts` is an explicit named parameter (vs loadSession's arguments[1]
// pattern) because _ensureMessagesLoaded is a module-private helper: it is
// only called from inside loadSession, so the public signature does not need
// to be preserved. Strict-mode engines optimize named params more reliably
// than arguments-indexing, and a named opts is self-documenting for static
// analysis. Callers pass {force:true} when they need to BYPASS the
// "messages already populated" early-return — currently only the #5177
// keep-stale-until-loaded path, which intentionally leaves the old messages
// in place (to avoid a visible disappear/reappear gap) and relies on
// _ensureMessagesLoaded to fetch and SWAP the new transcript into
// S.messages in a single frame.
opts = opts || {};
// Already have messages? (e.g. from INFLIGHT restore path, already set)
if (S.messages && S.messages.length > 0 && S.messages[0] && S.messages[0].role) {
if (!opts.force && S.messages && S.messages.length > 0 && S.messages[0] && S.messages[0].role) {
_clearSameSessionForceReloadHint(sid);
return;
}
Expand Down Expand Up @@ -4512,7 +4546,23 @@ async function refreshActiveSessionIfExternallyUpdated(reason){
// list metadata, advancing the local last-seen marker so the same metadata
// bump doesn't re-trigger on every subsequent poll.
if(remoteCount !== localCount){
await loadSession(sid, {force:true, externalRefreshReason:reason||'poll'});
// Hidden-tab return / visibility / focus recovery commonly trips
// remoteCount !== localCount when the post-turn bg-review thread or a
// sibling tab persisted messages while the tab was hidden. The default
// loadSession(force) path clears S.messages synchronously and waits for
// the full transcript round-trip before re-rendering, producing the
// user-visible "everything disappears, then reappears after a moment"
// gap that #5061 (metadata-only) and #5122 (SSE 4-probe) DO NOT cover
// (#5177). Pass keepStaleUntilLoaded so the destructive clear is
// deferred to swap-in-place when the new transcript actually arrives.
// Restrict to the recovery reasons that produced the field repro; the
// post-stream idle reconcile and external/imported-session polls keep
// the original behaviour (no DOM is on-screen long enough for the gap
// to matter, and any change there would have to re-verify their own
// tradeoffs).
const _recoveryReasons = {visible:true, focus:true};
const _keepStaleUntilLoaded = !!_recoveryReasons[String(reason||'')];
await loadSession(sid, {force:true, externalRefreshReason:reason||'poll', keepStaleUntilLoaded:_keepStaleUntilLoaded});
if(typeof renderSessionList==='function') void renderSessionList();
return 'reloaded';
}else if(remoteLast > localLast){
Expand Down
2 changes: 1 addition & 1 deletion tests/test_issue3162_ensure_messages_loaded.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def _ensure_messages_loaded_body() -> str:
# Window widened (#3326 added reload-width-hint handling inside this function,
# pushing the carry-forward reassignment further down; #3790 added the
# cold-load expand_renderable param + comment, pushing it further still).
return SESSIONS_JS[start: start + 3000]
return SESSIONS_JS[start: start + 4500]


def test_ensure_messages_loaded_declares_msgs_with_let():
Expand Down
2 changes: 1 addition & 1 deletion tests/test_issue3306_loadsession_carry_forward.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def _load_session_clear_block() -> str:

def _ensure_messages_loaded_body() -> str:
return _function_body(
"async function _ensureMessagesLoaded(sid) {",
"async function _ensureMessagesLoaded(sid",
"function _messageComparableText",
)

Expand Down
2 changes: 1 addition & 1 deletion tests/test_issue4756_session_visit_model_refresh.py
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ def test_load_session_schedules_session_visit_model_refresh_before_message_load(
body = _extract_function_body(_read_static("sessions.js"), "async function loadSession(")

assign_idx = body.index("S.session=data.session")
message_load_idx = body.index("await _ensureMessagesLoaded(sid)", assign_idx)
message_load_idx = body.index("await _ensureMessagesLoaded(sid", assign_idx)
failure_return_idx = body.index("return;", message_load_idx)
model_block_idx = body.index("if(typeof populateModelDropdown==='function')", assign_idx)
guard_helper_idx = body.index("const isActiveModelRefreshSession", model_block_idx)
Expand Down
205 changes: 205 additions & 0 deletions tests/test_issue5177_hidden_tab_blank_gap.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
"""Source-lock for the #5177 keep-stale-until-loaded path.

Symptom this guards against: after a hidden interval during which new messages
were persisted to the active session (post-turn bg-review writes, sibling-tab
writes, the just-finished main turn writing), switching the tab back caused the
ENTIRE transcript to visibly blank for the round-trip and then reappear —
"对话突然消失,重刷才回来". (nesquena/hermes-webui#5177)

Root cause: ``refreshActiveSessionIfExternallyUpdated('visible' | 'focus')`` hit
``remoteCount !== localCount`` and called ``loadSession(sid, {force:true})``,
which synchronously did ``S.messages = []`` before awaiting the metadata +
messages fetches. The inline comment at sessions.js itself warns about exactly
this "disappear/reappear" tradeoff but only short-circuits the metadata-only
(``remoteCount === localCount``) branch (the #5061 fix). #5122 covers a
different path (SSE error mid-stream with ``ready_state=2``) and does not apply
when the SSE error arrives while ``visibility_state='hidden'`` and bottoms out
through ``_deferStreamErrorIfPageHidden``.

Fix: ``refreshActiveSessionIfExternallyUpdated`` passes a new
``keepStaleUntilLoaded`` option to ``loadSession`` for the visibility/focus
recovery reasons. ``loadSession`` honors it by skipping the synchronous
``S.messages = []`` block when ``sameSessionForceReload`` is true, and forces
``_ensureMessagesLoaded`` to bypass its "messages already populated"
early-return so the new transcript is fetched and SWAPPED into ``S.messages``
in a single render frame. The visible result is old DOM → new DOM in one frame
with no intervening empty render.

These are static source assertions (whitespace-stripped substring + simple
brace-matching) so the keep-stale shape and the
recovery-reason → keepStaleUntilLoaded plumbing cannot silently regress.
"""
from pathlib import Path

ROOT = Path(__file__).resolve().parent.parent
SESSIONS_JS = (ROOT / "static" / "sessions.js").read_text(encoding="utf-8")


def _compact(text: str) -> str:
return "".join(text.split())


def _load_session_block(compact: str) -> str:
"""Slice from the start of ``async function loadSession`` to the matching
close brace, using brace counting so future code growth in the function
cannot push assertions out of a fixed window."""
marker = "asyncfunctionloadSession(sid){"
start = compact.find(marker)
assert start != -1, "expected the loadSession definition"
i = start + len(marker) - 1 # position of the opening brace
depth = 0
for j in range(i, len(compact)):
c = compact[j]
if c == "{":
depth += 1
elif c == "}":
depth -= 1
if depth == 0:
return compact[start: j + 1]
raise AssertionError("loadSession braces did not balance")


def _refresh_block(compact: str) -> str:
"""Slice from the start of ``async function refreshActiveSessionIfExternallyUpdated``
to its matching close brace."""
marker = "asyncfunctionrefreshActiveSessionIfExternallyUpdated(reason){"
start = compact.find(marker)
assert start != -1, "expected refreshActiveSessionIfExternallyUpdated definition"
i = start + len(marker) - 1
depth = 0
for j in range(i, len(compact)):
c = compact[j]
if c == "{":
depth += 1
elif c == "}":
depth -= 1
if depth == 0:
return compact[start: j + 1]
raise AssertionError("refreshActiveSessionIfExternallyUpdated braces did not balance")


def test_keep_stale_until_loaded_flag_computed_in_loadsession():
# The flag must be computed by AND-ing the caller's opts with
# sameSessionForceReload — cross-session switches MUST keep clearing
# synchronously, otherwise a stale prior-session transcript stays on
# screen during the navigation.
compact = _compact(SESSIONS_JS)
block = _load_session_block(compact)
assert "const_keepStaleUntilLoaded=!!opts.keepStaleUntilLoaded&&sameSessionForceReload;" in block, (
"loadSession must AND opts.keepStaleUntilLoaded with sameSessionForceReload"
)


def test_loadsession_skips_synchronous_clear_when_keep_stale_until_loaded():
# Inside the (currentSid !== sid || forceReload) block, the four-line clear
# (S.messages=[], S.toolCalls=[], _messagesTruncated=false, _oldestIdx=0)
# must sit inside `if (!_keepStaleUntilLoaded) { ... }`.
block = _load_session_block(_compact(SESSIONS_JS))
guard_idx = block.find("if(!_keepStaleUntilLoaded){")
assert guard_idx != -1, (
"expected the keep-stale guard wrapping the synchronous clear"
)
# The four clear lines must appear inside that guarded scope (between the
# guard's opening { and a `}` of matching depth).
body = block[guard_idx:]
# Limit the slice to the first matching close-brace by depth-counting.
depth = 0
guard_body_end = -1
for j, c in enumerate(body):
if c == "{":
depth += 1
elif c == "}":
depth -= 1
if depth == 0:
guard_body_end = j + 1
break
assert guard_body_end != -1, "guard scope did not close"
guard_body = body[:guard_body_end]
assert "S.messages=[];" in guard_body
assert "S.toolCalls=[];" in guard_body
assert "_messagesTruncated=false;" in guard_body
assert "_oldestIdx=0;" in guard_body


def test_only_one_synchronous_messages_clear_in_loadsession_force_block():
# Guard against a re-introduced unguarded clear sneaking back in. There
# must be exactly ONE `S.messages=[];` site in loadSession, and it must be
# the one nested inside the if(!_keepStaleUntilLoaded) guard above.
block = _load_session_block(_compact(SESSIONS_JS))
assert block.count("S.messages=[];") == 1, (
"loadSession should clear S.messages exactly once, under the keep-stale guard"
)


def test_ensure_messages_loaded_called_with_keep_stale_flag():
# Both _ensureMessagesLoaded call sites inside loadSession must forward
# the keep-stale flag so the early-return inside _ensureMessagesLoaded
# cannot skip the swap when stale messages are still in place.
block = _load_session_block(_compact(SESSIONS_JS))
# Both INFLIGHT and idle paths.
assert block.count("await_ensureMessagesLoaded(sid,{force:_keepStaleUntilLoaded})") == 2


def test_ensure_messages_loaded_supports_force_override():
# The receiving end: _ensureMessagesLoaded must look at opts.force in its
# "messages already populated" early-return so the keep-stale flag
# actually does what it says.
compact = _compact(SESSIONS_JS)
# Accept the function signature with a named `opts` parameter
# (preferred — self-documenting and strict-mode-optimization-friendly,
# greptile P2 r3393… on #5189) and the historical `arguments[1]` shape
# (for callers preserving an existing public signature). The required
# invariant is the EARLY-RETURN being gated on opts.force.
marker_named = "asyncfunction_ensureMessagesLoaded(sid,opts){"
marker_arglist = "asyncfunction_ensureMessagesLoaded(sid){"
start = compact.find(marker_named)
if start == -1:
start = compact.find(marker_arglist)
assert start != -1, "_ensureMessagesLoaded definition not found"
# Take a generously-sized window for the early-return region.
region = compact[start: start + 1000]
# Either explicit named param `opts` (preferred), or arguments[1] fallback,
# both must coerce to an object so opts.force is safe to read.
assert (
"opts=opts||{};" in region
or "constopts=arguments[1]||{};" in region
), "_ensureMessagesLoaded must coerce opts to an object before reading opts.force"
# The early-return MUST be GATED on !opts.force.
assert "if(!opts.force&&S.messages&&S.messages.length>0" in region, (
"_ensureMessagesLoaded's early-return must short-circuit on opts.force"
)


def test_refresh_visibility_path_requests_keep_stale_until_loaded():
# The visibility-recovery callers must opt INTO keepStaleUntilLoaded; the
# post-stream idle reconcile and the poll path stay on the original
# destructive reload behaviour (per the design note in the patch comment).
block = _refresh_block(_compact(SESSIONS_JS))
# The recovery-reason map MUST include 'visible' and 'focus' and EXCLUDE
# 'poll' / 'idle-reconcile'. Strip JS-side whitespace via _compact above.
assert "const_recoveryReasons={visible:true,focus:true};" in block, (
"expected the visibility/focus recovery-reason map"
)
assert "const_keepStaleUntilLoaded=!!_recoveryReasons[String(reason||'')];" in block
# The reloaded-path loadSession call must forward the flag — there is
# exactly one loadSession call in this block (the reloaded branch) and it
# must pass keepStaleUntilLoaded.
assert (
"awaitloadSession(sid,{force:true,externalRefreshReason:reason||'poll',keepStaleUntilLoaded:_keepStaleUntilLoaded});"
in block
)


def test_poll_and_idle_reconcile_do_not_enable_keep_stale():
# Belt-and-suspenders: the recovery-reason map must not list 'poll' or
# 'idle-reconcile'. We assert on the LITERAL set definition so any
# accidental widening is caught by the source lock.
block = _refresh_block(_compact(SESSIONS_JS))
# Just the two keys, exact set.
assert "const_recoveryReasons={visible:true,focus:true};" in block
# Sanity: those exact reason strings must still be the ones the
# visibility/focus listeners use elsewhere in this file. (Failure here
# means a renaming broke our routing.)
compact = _compact(SESSIONS_JS)
assert "refreshActiveSessionIfExternallyUpdated('visible')" in compact
assert "refreshActiveSessionIfExternallyUpdated('focus')" in compact
2 changes: 1 addition & 1 deletion tests/test_regressions.py
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,7 @@ def test_loadSession_inflight_merges_tail_with_persisted_transcript(cleanup_test
assert inflight_idx >= 0, "INFLIGHT branch not found in loadSession"
inflight_block = src[inflight_idx:inflight_idx+1200]

assert "await _ensureMessagesLoaded(sid);" in inflight_block, (
assert "await _ensureMessagesLoaded(sid" in inflight_block, (
"returning to an active stream should load the persisted transcript before adding the live tail"
)
assert "_mergeInflightTailMessages(S.messages,inflightMessages)" in inflight_block, (
Expand Down
4 changes: 2 additions & 2 deletions tests/test_session_channel_option_x.py
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,7 @@ def test_load_session_rearms_stream_on_every_early_return():

# Isolate the loadSession body.
fn_ix = js.index("async function loadSession(")
body = js[fn_ix:fn_ix + 12000]
body = js[fn_ix:fn_ix + 14000]

# The unconditional teardown must still be there (this is what creates the
# dead-stream window the re-arm closes).
Expand Down Expand Up @@ -881,7 +881,7 @@ def test_load_session_rearms_stream_on_every_early_return():
# but guarded against the self-healed-current (404'd) case so it never
# spins the reconnect loop against a dead session_id.
catch_ix = body.index("const _selfHealedCurrent")
catch_src = body[catch_ix:catch_ix + 1400]
catch_src = body[catch_ix:catch_ix + 2200]
assert "!_selfHealedCurrent" in catch_src and "startSessionStream(currentSid)" in catch_src, (
"fetch-error path must restart the on-screen stream, guarded against "
"the self-healed-current (deleted/404) session"
Expand Down
2 changes: 1 addition & 1 deletion tests/test_session_switch_busy_race.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def test_loadSession_clears_busy_before_async_message_load_when_server_idle():
assert "S.busy=false" in idle_block, "idle switch must clear S.busy immediately"
assert "S.activeStreamId=null" in idle_block, "idle switch must clear S.activeStreamId immediately"

ensure_load = body.find("await _ensureMessagesLoaded(sid)")
ensure_load = body.find("await _ensureMessagesLoaded(sid")
assert ensure_load != -1, "loadSession must still lazy-load messages for idle sessions"
assert idle_reset < ensure_load, (
"S.busy must be cleared before _ensureMessagesLoaded so session-list polling "
Expand Down
Loading
Loading