Merge pull request #5925 from nesquena/release/stage-5917

Release exp-v0.52.35: stale sidebar unread-dot on visit (#5917)

CHANGELOG.md

@@ -5,6 +5,8 @@
 
 ### Fixed
 
+- **Stale "unread" dot no longer lingers on a session after you open it.** Visiting a session now clears its sidebar unread indicator across the paths that previously left it stuck — the same-session reselect, the metadata-arrival ack, and the post-message-load re-sync — while a completion that lands in a genuinely hidden/background tab correctly stays unread. It also stops a phantom unread/streaming indicator from appearing when you switch from a busy session to an idle one (the idle session's streaming flags are now reset from its own metadata before the sidebar repaint, instead of inheriting the previous session's busy state). Thanks @neaucode-bot. (#5917, #4946)
+
 - **Plugin-provided model providers route correctly when set as the default.** A model from a plugin-only provider (e.g. a `@plugin:model` route) was surfaced in the catalog but, when that plugin provider was also the configured default, the request dropped the `@plugin:` routing hint and went to the wrong backend. Provider-hint resolution now applies plugin routing before the configured-provider bare-passthrough. Thanks @alexfoxtm. (#5909, #5461)
 
 - **Composer no longer double-sends or re-uploads on a fast second send.** The message text + its attachments are now captured and the textarea cleared immediately on send, before the async upload round-trip, so a re-entrant / interrupt-mode send can't re-read stale composer text and submit the same message twice or inherit the previous send's attachment. A clipboard-copy failure after a successful send no longer shows a false "failed", and a draft typed during the upload window is no longer clobbered. Thanks @harryazj. (#5912, #4750)

static/sessions.js

@@ -603,6 +603,60 @@ function _hasUnreadForSession(s) {
   return s.message_count > Number(counts[s.session_id] || 0);
 }
 
+// Keep the sidebar polling snapshot current for a just-visited session so a
+// deferred /api/sessions list refresh landing across the async message-load gap
+// cannot treat the unchanged, already-open session as a fresh background
+// completion and re-flag a stale unread dot (#4946).
+function _syncSessionListSnapshotOnVisit(sid, messageCount, lastMessageAt) {
+  if (!sid) return;
+  const count = Number(messageCount || 0);
+  const last = Number(lastMessageAt || 0);
+  _sessionListSnapshotById.set(sid, {message_count: count, last_message_at: last});
+  // #5917 gate finding: derive the visited session's streaming state from its
+  // OWN (target-owned) metadata, NOT the global S.busy / S.activeStreamId
+  // flags. When switching from a BUSY session A to an IDLE session B, those
+  // globals can still belong to A at this point in the load, so reading them
+  // here would wrongly record idle B as streaming — a later hidden-tab poll
+  // would then see a streaming->stopped transition and manufacture a phantom
+  // unread completion for B. Only the session object's own is_streaming /
+  // active_stream_id / pending-message fields describe THIS session.
+  const target = (S.session && S.session.session_id === sid) ? S.session : null;
+  const isStreaming = Boolean(
+    target && (
+      target.is_streaming ||
+      target.active_stream_id ||
+      target.pending_user_message ||
+      target.has_pending_user_message
+    )
+  );
+  _sessionStreamingById.set(sid, isStreaming);
+  if (!isStreaming) _forgetObservedStreamingSession(sid);
+}
+
+// Acknowledge that the user actually visited/opened `sid`: clear its viewed
+// count (which also clears any stale completion-unread marker, #3020), sync the
+// polling snapshot so a deferred list poll cannot re-flag it, then repaint from
+// cache. Repainting via renderSessionListFromCache() recomputes each row's
+// aggregated unread state (own + children) authoritatively, so a lineage
+// PARENT keeps its own / other children's unread dot instead of being stripped
+// by ad-hoc DOM surgery (Greptile concern (b) on #4946).
+function _acknowledgeSessionVisit(sid, messageCount = 0, lastMessageAt = 0) {
+  if (!sid) return;
+  _setSessionViewedCount(sid, messageCount);
+  _syncSessionListSnapshotOnVisit(sid, messageCount, lastMessageAt);
+  if (typeof renderSessionListFromCache === 'function') renderSessionListFromCache();
+}
+
+// Does the session currently carry any unread state that a visit should clear?
+// Used by the same-session no-op guard so re-selecting the already-open session
+// still clears a stale dot before short-circuiting.
+function _sessionVisitHasUnreadState(sid) {
+  if (!sid) return false;
+  if (_hasSessionCompletionUnread(sid)) return true;
+  if (!S.session || S.session.session_id !== sid) return false;
+  return _hasUnreadForSession(S.session);
+}
+
 function _isSessionActivelyViewedForList(sid) {
   if (!sid || !S.session || S.session.session_id !== sid) return false;
   if (typeof _loadingSessionId !== 'undefined' && _loadingSessionId && _loadingSessionId !== sid) return false;
@@ -1425,7 +1479,19 @@ async function loadSession(sid){
   // #2971: idempotent re-arm before the no-op guard revives a stream a prior
   // failed loadSession killed; no-ops on real switches.
   _rearmActiveSessionStream();
-  if(currentSid===sid && !forceReload && (!_loadingSessionId || _loadingSessionId===sid)) return;
+  if(currentSid===sid && !forceReload && (!_loadingSessionId || _loadingSessionId===sid)){
+    // Re-selecting the already-open session is a no-op for transcript/scroll, but
+    // it is still a *visit*: clear a stale sidebar unread dot (e.g. one a
+    // background completion left on the open, unfocused pane) before returning.
+    if(_sessionVisitHasUnreadState(sid)){
+      _acknowledgeSessionVisit(
+        sid,
+        Number(S.session.message_count || 0),
+        Number(S.session.last_message_at || S.session.updated_at || 0)
+      );
+    }
+    return;
+  }
   // Mark this session as the in-flight load. Subsequent loadSession() calls
   // will overwrite this; stale awaits use the mismatch to bail out (#1060).
   const _loadGeneration = ++_loadSessionGeneration;
@@ -1711,24 +1777,23 @@ async function loadSession(sid){
   // Sync workspace display immediately so the chip label reflects the new session's workspace
   // before any async message-loading begins (mirrors how model is handled).
   if(typeof syncTopbar==='function') syncTopbar();
-  _setSessionViewedCount(S.session.session_id, Number(data.session.message_count || 0));
-  _clearSessionCompletionUnread(S.session.session_id);
-  try{localStorage.setItem('hermes-webui-session',S.session.session_id);}catch(_){}
-  _setActiveSessionUrl(S.session.session_id);
-  if(typeof startSessionStream==='function') startSessionStream(S.session.session_id);
-
+  // Acknowledge the visit as soon as the session metadata is accepted for the
+  // in-flight load: clears the viewed count + any stale completion-unread marker
   // `let` (not const): re-read below, after the awaited _ensureMessagesLoaded,
   // so a server_turn_started that attaches a live stream MID-RELOAD is honored
   // by the attach/idle decision instead of being clobbered by the stale snapshot.
   let activeStreamId=S.session.active_stream_id||null;
   // If the server says the session is idle, reset browser-side streaming flags
-  // NOW — before the async _ensureMessagesLoaded gap below. Without this,
-  // S.busy can remain true from a still-running stream in the PREVIOUS session
-  // while S.session.session_id has already advanced to the new one.
-  // _isSessionLocallyStreaming() checks (isActive && S.busy), so during the
-  // async window the new session would appear locally-streaming (sidebar spinner,
-  // Stop button, thinking state on an idle chat). Also clears stale INFLIGHT
-  // entries left behind by a crashed/restarted stream.
+  // NOW — BEFORE _acknowledgeSessionVisit() below (whose sidebar repaint would
+  // otherwise inherit the PREVIOUS session's busy/stream state) and before the
+  // async _ensureMessagesLoaded gap. Without this, S.busy can remain true from a
+  // still-running stream in the PREVIOUS session while S.session.session_id has
+  // already advanced to the new one. _isSessionLocallyStreaming() checks
+  // (isActive && S.busy), so the new session would appear locally-streaming
+  // (sidebar spinner, Stop button, thinking state on an idle chat) and the visit
+  // repaint would manufacture a phantom unread. Also clears stale INFLIGHT
+  // entries left behind by a crashed/restarted stream. (#5917 gate: reset must
+  // precede the acknowledge repaint.)
   if(!activeStreamId){
     S.activeStreamId=null;
     S.busy=false;
@@ -1738,6 +1803,18 @@ async function loadSession(sid){
     }
   }
 
+  // and syncs the polling snapshot so a deferred /api/sessions poll landing
+  // during the async message-load gap below cannot re-flag a stale unread dot.
+  _acknowledgeSessionVisit(
+    S.session.session_id,
+    Number(data.session.message_count || 0),
+    Number(data.session.last_message_at || data.session.updated_at || 0)
+  );
+  try{localStorage.setItem('hermes-webui-session',S.session.session_id);}catch(_){}
+  _setActiveSessionUrl(S.session.session_id);
+  if(typeof startSessionStream==='function') startSessionStream(S.session.session_id);
+
+
   function _mergePendingSessionMessage(session,messages){
     if(!Array.isArray(messages)) return false;
     const pendingMsg=typeof getPendingSessionMessage==='function'?getPendingSessionMessage(session,messages):null;
@@ -2053,6 +2130,27 @@ async function loadSession(sid){
   // Clear the in-flight session marker now that this load has completed (#1060).
   if (_isCurrentLoad()) _loadingSessionId = null;
 
+  // Re-acknowledge the visit after the async message-load gap. A deferred
+  // sidebar /api/sessions poll can land while _ensureMessagesLoaded is in
+  // flight and re-mark the open session unread; re-syncing here clears that
+  // sticky dot once the transcript is settled (#4946).
+  //
+  // Gate the final ack on _isSessionActivelyViewedForList(sid): a completion
+  // that lands while _ensureMessagesLoaded() is in flight AND the tab then goes
+  // hidden is correctly marked unread — an UNCONDITIONAL ack here would wrongly
+  // clear that hidden-tab-completion marker. Only clear when the session is
+  // still actively viewed. (#5917 gate finding)
+  if (
+    S.session && S.session.session_id === sid &&
+    (typeof _isSessionActivelyViewedForList !== 'function' || _isSessionActivelyViewedForList(sid))
+  ) {
+    _acknowledgeSessionVisit(
+      sid,
+      Number(S.session.message_count || 0),
+      Number(S.session.last_message_at || S.session.updated_at || 0)
+    );
+  }
+
   if(typeof renderSessionArtifacts==='function') renderSessionArtifacts();
 
   // ── Cross-channel handoff hint ──
@@ -2845,7 +2943,15 @@ async function _ensureMessagesLoaded(sid, opts) {
     if(typeof scheduleTodosRefresh === 'function'){
       scheduleTodosRefresh();
     }
-    _setSessionViewedCount(sid, Number(S.session.message_count || msgs.length));
+    // Only sync the viewed count (which also clears any completion-unread
+    // marker via _setSessionViewedCount -> _clearSessionCompletionUnread)
+    // when the session is STILL actively viewed. A hidden-tab completion that
+    // lands during the awaited message fetch above must NOT be silently marked
+    // read here — mirror the same _isSessionActivelyViewedForList(sid) guard
+    // used on the post-load re-ack in loadSession(). (#5917 gate finding)
+    if(typeof _isSessionActivelyViewedForList !== 'function' || _isSessionActivelyViewedForList(sid)){
+      _setSessionViewedCount(sid, Number(S.session.message_count || msgs.length));
+    }
     if(typeof syncTopbar==='function') syncTopbar();
   }
 }

tests/test_cross_session_message_load_isolation.py

@@ -225,6 +225,13 @@ function createEnvironment() {
   globalThis._clearSameSessionForceReloadHint = () => { clearHintCalls += 1; };
   globalThis._clearStuckSessionOnBoot = () => {};
   globalThis._setSessionViewedCount = () => {};
+  // #4946: loadSession() now routes its viewed-count/unread clear through
+  // _acknowledgeSessionVisit(). This harness exercises cross-session load
+  // ordering + stale-reject, not unread-dot state, so stub it (and its
+  // same-session-guard predicate) to no-ops — mirroring the pre-existing
+  // _setSessionViewedCount / _clearSessionCompletionUnread stubs it replaced.
+  globalThis._acknowledgeSessionVisit = () => {};
+  globalThis._sessionVisitHasUnreadState = () => false;
   globalThis.scheduleTodosRefresh = () => {};
   globalThis.startSessionStream = () => {};
   globalThis.syncTopbar = () => {};

tests/test_gateway_sse_reconnect_dedupe.py

@@ -129,7 +129,7 @@ if(!_isDuplicateGatewaySessionSnapshot([null, {session_id:'web-2', session_sourc
 def test_load_session_persists_only_after_metadata_loads():
     """Do not overwrite the last good localStorage sid before /api/session succeeds."""
     src = _read(SESSIONS_JS)
-    load = _block(src, "async function loadSession(sid)", "activeStreamId=S.session.active_stream_id")
+    load = _block(src, "async function loadSession(sid)", "function _mergePendingSessionMessage")
     api_pos = load.index("data = await api(`/api/session")
     persist_pos = load.index("localStorage.setItem('hermes-webui-session',S.session.session_id)")
 

tests/test_inflight_stream_reuse.py

@@ -166,13 +166,23 @@ def test_load_session_same_sid_noop_does_not_mask_pending_switch_back():
     """
     body = _function_body(SESSIONS_JS, "loadSession")
     compact = re.sub(r"\s+", "", body)
-    guard = "if(currentSid===sid&&!forceReload&&(!_loadingSessionId||_loadingSessionId===sid))return;"
+    # The same-session no-op now opens a block so re-selecting the already-open
+    # session can clear a stale unread dot before returning (#4946). The
+    # protected ownership invariant is unchanged: the guard still requires
+    # (!_loadingSessionId || _loadingSessionId===sid) and still early-returns
+    # before _loadingSessionId=sid.
+    guard = "if(currentSid===sid&&!forceReload&&(!_loadingSessionId||_loadingSessionId===sid)){"
     assert guard in compact, (
         "same-session no-op must be owned by the current load target: "
         "another in-flight sid must not suppress a pending switch-back"
     )
     assert "_loadingSessionId===sid" in guard
-    assert compact.find(guard) < compact.find("_loadingSessionId=sid;")
+    guard_pos = compact.find(guard)
+    assert guard_pos < compact.find("_loadingSessionId=sid;")
+    # The guarded block must still early-return for the same-session no-op,
+    # while now also acknowledging the visit to clear a stale unread dot.
+    assert "_sessionVisitHasUnreadState(sid)" in compact[guard_pos:guard_pos + 600]
+    assert "return;}" in compact[guard_pos:guard_pos + 900]
 
 
 def test_load_session_preserves_existing_worklog_content_without_destructive_fallback():

tests/test_issue856_background_completion_unread.py

@@ -519,17 +519,32 @@ def test_completion_unread_clears_only_when_session_is_opened():
     assert load_idx != -1, "loadSession not found"
     load_block = SESSIONS_JS[load_idx:SESSIONS_JS.find("function _resolveSessionModelForDisplaySoon", load_idx)]
 
-    stale_guard_idx = load_block.find("if (!_isCurrentLoad()) return;")
-    clear_idx = load_block.find("_clearSessionCompletionUnread(S.session.session_id);")
-    set_viewed_idx = load_block.find("_setSessionViewedCount(S.session.session_id")
+    # The metadata-arrival "mark viewed + clear stale completion unread" pair now
+    # flows through _acknowledgeSessionVisit(S.session.session_id, ...), which
+    # calls _setSessionViewedCount() internally (and _setSessionViewedCount clears
+    # any stale completion-unread marker, #3020) (#4946).
+    assign_idx = load_block.find("S.session=data.session;")
+    acknowledge_idx = load_block.find("_acknowledgeSessionVisit(\n    S.session.session_id,", assign_idx)
+    # The last stale-response ownership guard before the visit is acknowledged:
+    # stale loadSession responses must not clear unread markers for sessions the
+    # user did not actually open.
+    stale_guard_idx = load_block.rfind("if (!_isCurrentLoad())", 0, acknowledge_idx)
 
-    assert clear_idx != -1, "loadSession must clear explicit completion unread when the user opens the session"
-    assert stale_guard_idx != -1 and stale_guard_idx < clear_idx, (
-        "stale loadSession responses must not clear unread markers for sessions the user did not actually open"
+    assert assign_idx != -1, "loadSession must assign S.session before acknowledging the visit"
+    assert acknowledge_idx != -1 and assign_idx < acknowledge_idx, (
+        "loadSession must acknowledge the visit only after the session metadata "
+        "response is accepted for the in-flight load"
     )
-    assert set_viewed_idx != -1 and set_viewed_idx < clear_idx, (
-        "completion unread should clear at the same point the session is marked viewed"
+    assert stale_guard_idx != -1 and stale_guard_idx < acknowledge_idx, (
+        "stale loadSession responses must be guarded out before the visit-ack "
+        "clears unread markers for sessions the user did not actually open"
     )
+    # The acknowledge helper is what clears completion unread on visit, via
+    # _setSessionViewedCount (#3020 stale-marker clear).
+    assert "function _acknowledgeSessionVisit(sid, messageCount = 0, lastMessageAt = 0)" in SESSIONS_JS
+    ack_body_start = SESSIONS_JS.find("function _acknowledgeSessionVisit(")
+    ack_body = SESSIONS_JS[ack_body_start:SESSIONS_JS.find("function _sessionVisitHasUnreadState", ack_body_start)]
+    assert "_setSessionViewedCount(sid, messageCount);" in ack_body
 
 
 def test_historical_sessions_are_not_marked_unread_on_list_render():

tests/test_regressions.py

@@ -589,9 +589,12 @@ def test_queue_card_cross_session_helper_used_only_for_session_change(cleanup_te
         "queue-card clear helper must be inside the cross-session branch"
     )
     same_session_idx = load_body.find(
-        "if(currentSid===sid && !forceReload && (!_loadingSessionId || _loadingSessionId===sid)) return;"
+        "if(currentSid===sid && !forceReload && (!_loadingSessionId || _loadingSessionId===sid)){"
+    )
+    assert same_session_idx >= 0, (
+        "same-session no-op guard must still exist (now a block that first clears "
+        "a stale unread dot before returning) and must precede the cross-session branch"
     )
-    assert same_session_idx >= 0
     assert same_session_idx < cross_start
 
 

tests/test_session_channel_option_x.py

@@ -935,9 +935,11 @@ def test_load_session_rearms_stream_on_every_early_return():
         "helper must (re)arm startSessionStream for the currently-shown S.session"
     )
 
-    # Isolate the loadSession body.
+    # Isolate the loadSession body. Widened window: the #4946 visit-ack helpers
+    # added inside loadSession pushed the fetch-error catch's stream restart past
+    # the old 14000-char cutoff.
     fn_ix = js.index("async function loadSession(")
-    body = js[fn_ix:fn_ix + 14000]
+    body = js[fn_ix:fn_ix + 16000]
 
     # The unconditional teardown must still be there (this is what creates the
     # dead-stream window the re-arm closes).

tests/test_session_metadata_fast_path.py

@@ -19,8 +19,16 @@ def test_messages_zero_skips_effective_model_resolution():
 
 def test_full_message_load_updates_viewed_count_after_metadata_fast_path():
     src = (ROOT / "static" / "sessions.js").read_text(encoding="utf-8")
+    compact = re.sub(r"\s+", "", src)
 
-    assert "_setSessionViewedCount(S.session.session_id, Number(data.session.message_count || 0));" in src
+    # The metadata-arrival viewed-count update now flows through
+    # _acknowledgeSessionVisit(), which calls _setSessionViewedCount() internally
+    # (and additionally syncs the polling snapshot + repaints so visiting a
+    # session reliably clears a stale sidebar unread dot) (#4946).
+    assert (
+        "_acknowledgeSessionVisit(S.session.session_id,Number(data.session.message_count||0),"
+        in compact
+    )
     assert "_setSessionViewedCount(sid, Number(S.session.message_count || msgs.length));" in src
 
 

tests/test_session_unread_dot_on_visit.py

@@ -0,0 +1,446 @@
+"""Regression: a stale sidebar unread dot must clear when a session is *visited*.
+
+Salvage of #4946 (originally by @neaucode-bot), rebuilt fresh on master.
+
+The yellow unread dot in the sidebar historically cleared only reliably when the
+sidebar **row** was clicked. Opening/visiting a session through other paths — or
+re-selecting the already-open session — could leave a stale dot, and a deferred
+/api/sessions list poll landing across the async message-load gap could re-flag
+the open session as unread.
+
+The fix introduces `_acknowledgeSessionVisit()` (sync viewed count + polling
+snapshot + repaint) and wires it into loadSession at three points:
+
+  1. the same-session no-op guard (so re-selecting the open session clears a
+     stale dot before returning),
+  2. when the session metadata arrives, and
+  3. again after the async message-load gap (so a deferred poll cannot leave a
+     sticky dot).
+
+Two invariants flagged in review are protected here and MUST NOT regress:
+
+  (a) hidden/background completions must still be marked unread — the visit-ack
+      does NOT loosen the focus gate on the completion paths, so a completion in
+      a non-visible/non-focused tab is still flagged (concern a).
+  (b) cleaning up a visited child's unread state must not strip a lineage
+      PARENT's own unread dot — the visit repaints via
+      renderSessionListFromCache(), which recomputes each row's aggregated
+      unread authoritatively rather than doing ad-hoc DOM surgery (concern b).
+"""
+import json
+import subprocess
+from pathlib import Path
+
+ROOT = Path(__file__).resolve().parents[1]
+SESSIONS_JS = (ROOT / "static" / "sessions.js").read_text(encoding="utf-8")
+
+
+def _load_session_block() -> str:
+    start = SESSIONS_JS.index("async function loadSession(sid")
+    end = SESSIONS_JS.index("function _resolveSessionModelForDisplaySoon", start)
+    return SESSIONS_JS[start:end]
+
+
+def _function_block(name: str, next_marker: str) -> str:
+    start = SESSIONS_JS.index(f"function {name}")
+    end = SESSIONS_JS.index(next_marker, start + 1)
+    return SESSIONS_JS[start:end]
+
+
+# ── Structural anchors ──────────────────────────────────────────────────────
+
+def test_visit_ack_helpers_exist():
+    assert "function _acknowledgeSessionVisit(sid, messageCount = 0, lastMessageAt = 0)" in SESSIONS_JS
+    assert "function _syncSessionListSnapshotOnVisit(sid, messageCount, lastMessageAt)" in SESSIONS_JS
+    assert "function _sessionVisitHasUnreadState(sid)" in SESSIONS_JS
+
+
+def test_acknowledge_visit_syncs_viewed_snapshot_and_repaints():
+    body = _function_block("_acknowledgeSessionVisit", "function _sessionVisitHasUnreadState")
+    # Clears viewed count (which clears the stale completion-unread marker, #3020),
+    # syncs the polling snapshot, and repaints the sidebar from cache.
+    assert "_setSessionViewedCount(sid, messageCount);" in body
+    assert "_syncSessionListSnapshotOnVisit(sid, messageCount, lastMessageAt);" in body
+    assert "renderSessionListFromCache" in body
+
+
+def test_load_session_acknowledges_visit_before_and_after_message_load():
+    block = _load_session_block()
+    # Metadata-arrival acknowledgment.
+    first_ack = block.find("_acknowledgeSessionVisit(\n    S.session.session_id,")
+    loading_clear = block.find("if (_isCurrentLoad()) _loadingSessionId = null;\n\n  // Re-acknowledge")
+    second_ack = block.find("_acknowledgeSessionVisit(", loading_clear)
+
+    assert first_ack != -1, "loadSession must acknowledge the visit when metadata arrives"
+    assert loading_clear != -1, "loadSession must clear the in-flight marker before the final acknowledge"
+    assert second_ack != -1 and first_ack < loading_clear < second_ack, (
+        "loadSession must re-acknowledge after the async message-load gap so a "
+        "deferred sidebar poll cannot leave a sticky unread dot"
+    )
+
+
+def test_post_load_reack_is_guarded_by_active_view():
+    # #5917 gate finding: the post-load re-ack must be gated on
+    # _isSessionActivelyViewedForList(sid). A completion that lands while
+    # _ensureMessagesLoaded() is in flight AND the tab then goes hidden is
+    # correctly marked unread — an UNCONDITIONAL post-load ack would wrongly
+    # clear that hidden-tab-completion marker.
+    block = _load_session_block()
+    loading_clear = block.find("if (_isCurrentLoad()) _loadingSessionId = null;\n\n  // Re-acknowledge")
+    guard = block.find("_isSessionActivelyViewedForList(sid)", loading_clear)
+    second_ack = block.find("_acknowledgeSessionVisit(", loading_clear)
+    assert guard != -1 and guard < second_ack, (
+        "the post-load re-acknowledge must be guarded by "
+        "_isSessionActivelyViewedForList(sid) so a hidden-tab completion stays unread"
+    )
+
+
+def test_same_session_reselect_clears_stale_unread():
+    block = _load_session_block()
+    guard = block.find("if(currentSid===sid && !forceReload && (!_loadingSessionId || _loadingSessionId===sid)){")
+    unread_check = block.find("_sessionVisitHasUnreadState(sid)", guard)
+    acknowledge = block.find("_acknowledgeSessionVisit(", unread_check)
+    ret = block.find("return;", acknowledge)
+
+    assert guard != -1, "same-session no-op guard must still exist"
+    assert unread_check != -1 and guard < unread_check, (
+        "re-selecting the already-open session must check for stale unread state"
+    )
+    assert acknowledge != -1 and unread_check < acknowledge < ret, (
+        "re-selecting the already-open session must acknowledge the visit (clearing "
+        "the stale dot) before returning"
+    )
+
+
+def test_completion_paths_keep_focus_gate_for_hidden_tab_completions():
+    """Concern (a): the visit-ack must NOT loosen the completion paths' focus gate.
+
+    A background completion in a hidden/unfocused tab must still be flagged
+    unread, so the background + polling completion paths must keep using the
+    focus-gated _isSessionActivelyViewedForList, not a focus-independent variant.
+    """
+    background = _function_block("_markSessionCompletionUnreadIfBackground", "function _clearSessionCompletionUnread")
+    assert "_isSessionActivelyViewedForList(sid)" in background, (
+        "background completion must keep the focus-gated read check so a hidden-tab "
+        "completion is not prematurely marked read"
+    )
+
+    polling_start = SESSIONS_JS.index("function _markPollingCompletionUnreadTransitions(sessions)")
+    polling_end = SESSIONS_JS.index("const staleRuntimeStateSids", polling_start)
+    polling = SESSIONS_JS[polling_start:polling_end]
+    assert "!_isSessionActivelyViewedForList(sid)" in polling, (
+        "polling completion must keep the focus-gated read check so a hidden-tab "
+        "completion is not prematurely marked read"
+    )
+
+
+# ── Functional behavior via node ────────────────────────────────────────────
+
+def _extract(name: str) -> str:
+    """Extract a top-level `function name(...) { ... }` definition by brace match."""
+    marker = f"function {name}("
+    start = SESSIONS_JS.index(marker)
+    brace = SESSIONS_JS.index("{", start)
+    depth = 0
+    for i in range(brace, len(SESSIONS_JS)):
+        ch = SESSIONS_JS[i]
+        if ch == "{":
+            depth += 1
+        elif ch == "}":
+            depth -= 1
+            if depth == 0:
+                return SESSIONS_JS[start:i + 1]
+    raise AssertionError(f"could not brace-match {name}")
+
+
+def _run_node(script: str) -> dict:
+    result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
+    return json.loads(result.stdout)
+
+
+def test_acknowledge_visit_clears_completion_unread_marker():
+    """Visiting a session that carries an explicit completion-unread marker must
+    clear it (so the yellow dot disappears) and repaint the sidebar."""
+    ack = _extract("_acknowledgeSessionVisit")
+    sync = _extract("_syncSessionListSnapshotOnVisit")
+    set_viewed = _extract("_setSessionViewedCount")
+    clear_unread = _extract("_clearSessionCompletionUnread")
+    get_unread = _extract("_getSessionCompletionUnread")
+    save_unread = _extract("_saveSessionCompletionUnread")
+    get_counts = _extract("_getSessionViewedCounts")
+    save_counts = _extract("_saveSessionViewedCounts")
+
+    script = f"""
+// Minimal localStorage shim.
+const _store = {{}};
+const localStorage = {{
+  getItem: (k) => (k in _store ? _store[k] : null),
+  setItem: (k, v) => {{ _store[k] = String(v); }},
+}};
+const SESSION_VIEWED_COUNTS_KEY = 'v';
+const SESSION_COMPLETION_UNREAD_KEY = 'u';
+let _sessionViewedCounts = null;
+let _sessionCompletionUnread = null;
+const _sessionListSnapshotById = new Map();
+const _sessionStreamingById = new Map();
+let S = {{ session: {{ session_id: 'open', message_count: 5 }} }};
+let repaints = 0;
+function renderSessionListFromCache() {{ repaints += 1; }}
+function _forgetObservedStreamingSession() {{}}
+{get_counts}
+{save_counts}
+{get_unread}
+{save_unread}
+{clear_unread}
+{set_viewed}
+{sync}
+{ack}
+// Seed a stale completion-unread marker for the open session.
+_getSessionCompletionUnread()['open'] = {{message_count: 5, completed_at: 1}};
+_saveSessionCompletionUnread();
+const before = Object.prototype.hasOwnProperty.call(_getSessionCompletionUnread(), 'open');
+_acknowledgeSessionVisit('open', 5, 10);
+const after = Object.prototype.hasOwnProperty.call(_getSessionCompletionUnread(), 'open');
+const snap = _sessionListSnapshotById.get('open');
+console.log(JSON.stringify({{before, after, repaints, viewed: _getSessionViewedCounts()['open'], snap}}));
+"""
+    out = _run_node(script)
+    assert out["before"] is True, "precondition: marker seeded"
+    assert out["after"] is False, "visiting the session must clear the completion-unread marker"
+    assert out["repaints"] >= 1, "visiting must repaint the sidebar from cache"
+    assert out["viewed"] == 5, "viewed count must be synced to the current message count"
+    assert out["snap"] == {"message_count": 5, "last_message_at": 10}, (
+        "polling snapshot must be synced so a deferred list poll cannot re-flag the session"
+    )
+
+
+def test_visit_snapshot_prevents_deferred_poll_from_reflagging():
+    """A deferred /api/sessions poll running just after a visit must NOT re-mark
+    the open, unchanged session as a fresh background completion.
+
+    This is the sticky-dot race: without the snapshot sync, the poll sees a
+    session that "completed" relative to a stale/absent snapshot and re-flags it.
+    """
+    sync = _extract("_syncSessionListSnapshotOnVisit")
+    set_viewed = _extract("_setSessionViewedCount")
+    clear_unread = _extract("_clearSessionCompletionUnread")
+    get_unread = _extract("_getSessionCompletionUnread")
+    save_unread = _extract("_saveSessionCompletionUnread")
+    get_counts = _extract("_getSessionViewedCounts")
+    save_counts = _extract("_saveSessionViewedCounts")
+    ack = _extract("_acknowledgeSessionVisit")
+    transitions = _extract("_markPollingCompletionUnreadTransitions")
+    effective = _extract("_isSessionEffectivelyStreaming")
+    local = _extract("_isSessionLocallyStreaming")
+
+    script = f"""
+const _store = {{}};
+const localStorage = {{
+  getItem: (k) => (k in _store ? _store[k] : null),
+  setItem: (k, v) => {{ _store[k] = String(v); }},
+}};
+const SESSION_VIEWED_COUNTS_KEY = 'v';
+const SESSION_COMPLETION_UNREAD_KEY = 'u';
+let _sessionViewedCounts = null;
+let _sessionCompletionUnread = null;
+const _sessionListSnapshotById = new Map();
+const _sessionStreamingById = new Map();
+// open + focused/visible so the focus gate treats it as actively viewed too.
+let S = {{ session: {{ session_id: 'open', message_count: 5 }}, busy: false }};
+let repaints = 0;
+function renderSessionListFromCache() {{ repaints += 1; }}
+function _forgetObservedStreamingSession(sid) {{}}
+function _rememberObservedStreamingSession() {{}}
+function _getSessionObservedStreaming() {{ return {{}}; }}
+function _rememberSessionListSource() {{}}
+function _hasPendingUserMessageSignal(s) {{ return !!(s && (s.pending_user_message || s.has_pending_user_message)); }}
+function _markSessionCompletionUnread(sid, count) {{
+  _getSessionCompletionUnread()[sid] = {{message_count: count, completed_at: 1}};
+  _saveSessionCompletionUnread();
+}}
+const document = {{ visibilityState: 'visible', hasFocus: () => true }};
+let _loadingSessionId = null;
+const _allSessionsScope = null;
+const _sessionListSourceById = new Map();
+{get_counts}
+{save_counts}
+{get_unread}
+{save_unread}
+{clear_unread}
+{set_viewed}
+{local}
+{effective}
+{sync}
+{ack}
+function _isSessionActivelyViewedForList(sid) {{
+  if (!sid || !S.session || S.session.session_id !== sid) return false;
+  if (_loadingSessionId && _loadingSessionId !== sid) return false;
+  if (document.visibilityState && document.visibilityState !== 'visible') return false;
+  if (typeof document.hasFocus === 'function' && !document.hasFocus()) return false;
+  return true;
+}}
+{transitions}
+// Simulate: session was streaming, so a snapshot/streaming state exists.
+_sessionStreamingById.set('open', true);
+_sessionListSnapshotById.set('open', {{message_count: 4, last_message_at: 5}});
+// User visits — acknowledge marks it read AND syncs the snapshot to current.
+_acknowledgeSessionVisit('open', 5, 10);
+// Now a deferred /api/sessions poll lands for the SAME (now idle) session.
+_markPollingCompletionUnreadTransitions([
+  {{session_id: 'open', is_streaming: false, active_stream_id: null, message_count: 5, last_message_at: 10, updated_at: 10}}
+]);
+const flagged = Object.prototype.hasOwnProperty.call(_getSessionCompletionUnread(), 'open');
+console.log(JSON.stringify({{flagged}}));
+"""
+    out = _run_node(script)
+    assert out["flagged"] is False, (
+        "a deferred list poll after a visit must not re-flag the open, unchanged "
+        "session as unread — the visit-ack synced snapshot + viewed count"
+    )
+
+
+# ── Functional: hidden-tab completion during message load stays unread ───────
+
+def _extract_async(name: str) -> str:
+    """Like _extract, but preserve an `async` prefix so `await` bodies stay valid."""
+    body = _extract(name)
+    idx = SESSIONS_JS.rindex("async ", 0, SESSIONS_JS.index(body))
+    # Only treat it as async if the `async ` keyword immediately precedes it.
+    if SESSIONS_JS[idx:idx + len("async ")] == "async " and \
+       SESSIONS_JS[idx:].startswith("async function " + name):
+        return "async " + body
+    return body
+
+
+def _hidden_completion_script(*, hidden: bool) -> str:
+    """Build a Node harness that drives the REAL _ensureMessagesLoaded() through a
+    delayed messages fetch, marks a completion mid-fetch, and reports whether the
+    completion-unread marker survives."""
+    ensure = _extract_async("_ensureMessagesLoaded")
+    set_viewed = _extract("_setSessionViewedCount")
+    clear_unread = _extract("_clearSessionCompletionUnread")
+    get_unread = _extract("_getSessionCompletionUnread")
+    save_unread = _extract("_saveSessionCompletionUnread")
+    mark_unread = _extract("_markSessionCompletionUnread")
+    get_counts = _extract("_getSessionViewedCounts")
+    save_counts = _extract("_saveSessionViewedCounts")
+    actively_viewed = _extract("_isSessionActivelyViewedForList")
+
+    visibility = "'hidden'" if hidden else "'visible'"
+    has_focus = "false" if hidden else "true"
+
+    return f"""
+// Minimal localStorage shim.
+const _store = {{}};
+const localStorage = {{
+  getItem: (k) => (k in _store ? _store[k] : null),
+  setItem: (k, v) => {{ _store[k] = String(v); }},
+}};
+const SESSION_VIEWED_COUNTS_KEY = 'v';
+const SESSION_COMPLETION_UNREAD_KEY = 'u';
+let _sessionViewedCounts = null;
+let _sessionCompletionUnread = null;
+let _messagesTruncated = false;
+let _oldestIdx = 0;
+let _messageRenderWindowSize = 0;
+let _pendingCarryForwardSnapshot = null;
+let _loadingSessionId = 'open';
+let _loadSessionGeneration = 0;
+const window = {{}};
+// Tab starts VISIBLE+FOCUSED: the load begins while the user is watching.
+let _visibility = 'visible';
+let _focused = true;
+const document = {{
+  get visibilityState() {{ return _visibility; }},
+  hasFocus: () => _focused,
+}};
+let S = {{ session: {{ session_id: 'open', message_count: 0 }}, messages: [], lastUsage: {{}} }};
+
+// Stubs for the incidental side effects _ensureMessagesLoaded touches.
+function _clearSameSessionForceReloadHint() {{}}
+function _messageReloadLimitForSession() {{ return 0; }}
+function _syncToolCallsForLoadedMessages() {{}}
+function clearLiveToolCards() {{}}
+
+// Delayed messages fetch: resolves only when we release it, simulating a slow
+// /api/session?messages=1 response that spans the tab going hidden.
+let _releaseApi;
+const _apiResult = new Promise((res) => {{ _releaseApi = res; }});
+let _apiCalled = false;
+async function api(url) {{ _apiCalled = true; return _apiResult; }}
+
+{get_counts}
+{save_counts}
+{get_unread}
+{save_unread}
+{clear_unread}
+{mark_unread}
+{set_viewed}
+{actively_viewed}
+{ensure}
+
+function _hasMarker() {{
+  return Object.prototype.hasOwnProperty.call(_getSessionCompletionUnread(), 'open');
+}}
+
+(async () => {{
+  const p = _ensureMessagesLoaded('open', {{}});
+  // Let the synchronous body run up to `await api(...)`.
+  await Promise.resolve();
+  await Promise.resolve();
+  const apiIssued = _apiCalled;
+  // Mid-fetch: the tab's visibility/focus flips to the test scenario and a
+  // background completion lands, marking the session unread.
+  _visibility = {visibility};
+  _focused = {has_focus};
+  _markSessionCompletionUnread('open', 6);
+  const markerBefore = _hasMarker();
+  // The delayed messages response finally arrives and the load finishes.
+  _releaseApi({{ session: {{ session_id: 'open', message_count: 6, messages: [{{ role: 'assistant', content: 'x' }}] }} }});
+  await p;
+  const markerAfter = _hasMarker();
+  const viewed = _getSessionViewedCounts()['open'];
+  console.log(JSON.stringify({{
+    apiIssued,
+    markerBefore,
+    markerAfter,
+    viewed: viewed === undefined ? null : viewed,
+  }}));
+}})();
+"""
+
+
+def test_hidden_tab_completion_during_message_load_survives():
+    """#5917 gate finding (SILENT): a completion that lands while the tab is
+    hidden DURING the awaited message fetch inside _ensureMessagesLoaded() must
+    NOT be silently marked read. The guarded viewed-count clear must skip when
+    the session is no longer actively viewed."""
+    out = _run_node(_hidden_completion_script(hidden=True))
+    assert out["apiIssued"] is True, "precondition: the delayed messages fetch was issued"
+    assert out["markerBefore"] is True, "precondition: the mid-fetch completion marked the session unread"
+    assert out["markerAfter"] is True, (
+        "a hidden-tab completion landing during the awaited message fetch must "
+        "SURVIVE — _ensureMessagesLoaded() must not clear the unread marker via "
+        "an unconditional _setSessionViewedCount when the tab is not actively viewing"
+    )
+    assert out["viewed"] is None, (
+        "the viewed count must NOT be synced for a hidden/background session, "
+        "otherwise the completion-unread marker would be cleared"
+    )
+
+
+def test_visible_tab_message_load_still_syncs_viewed_count():
+    """Control: the guard must not over-block. An ACTIVELY-viewed session still
+    updates its viewed count normally through the message-load path (clearing any
+    marker), so the fix is scoped to hidden/background sessions only."""
+    out = _run_node(_hidden_completion_script(hidden=False))
+    assert out["apiIssued"] is True
+    assert out["markerBefore"] is True
+    assert out["markerAfter"] is False, (
+        "an actively-viewed session must still clear its completion-unread marker "
+        "when the message load finishes"
+    )
+    assert out["viewed"] == 6, (
+        "an actively-viewed session must still sync its viewed count to the "
+        "loaded message count"
+    )

tests/test_stale_stream_cleanup.py

@@ -435,7 +435,7 @@ def test_frontend_drops_inflight_cache_when_server_session_is_idle():
     # (preserved) cache-drop behavior in the now-nested form.
     marker = "If the server says the session is idle, reset browser-side streaming flags"
     marker_pos = SESSIONS_SRC.index(marker)
-    window = SESSIONS_SRC[marker_pos:marker_pos + 900]
+    window = SESSIONS_SRC[marker_pos:marker_pos + 1400]
     assert "if(!activeStreamId){" in window
     assert "S.busy=false" in window
     assert "S.activeStreamId=null" in window