fix(sessions): clear stale sidebar unread indicator on session visit (salvage of #4946)

Salvaged from #4946 by @neaucode-bot. Rebuilt fresh on master (repro-confirmed),
without regressing hidden-tab completion state or lineage-parent unread dots.

Co-authored-by: neaucode-bot <neaucode-bot@users.noreply.github.com>
This commit is contained in:
nesquena-hermes
2026-07-11 03:32:26 +00:00
parent 101421ff67
commit 58af505cc6
7 changed files with 412 additions and 18 deletions
+75 -3
View File
@@ -603,6 +603,47 @@ 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});
const isStreaming = Boolean(
S.session && S.session.session_id === sid
&& (S.busy || S.activeStreamId || (S.session.active_stream_id && S.session.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 +1466,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,8 +1764,15 @@ 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);
// 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
// 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);
@@ -2053,6 +2113,18 @@ 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).
if (S.session && S.session.session_id === 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 ──
+12 -2
View File
@@ -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():
@@ -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():
+5 -2
View File
@@ -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
+4 -2
View File
@@ -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).
+9 -1
View File
@@ -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
+284
View File
@@ -0,0 +1,284 @@
"""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 re
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_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"
)