fix: retain durable unread clears under storage pressure

Keep an older persisted unread-clear ordering record when quota rejects its
higher replacement, so reload cannot lose the last durable tombstone. Replace
unread source-shape assertions with Node-driven behavior checks against the real
sessions.js functions.

docs/rfcs/webui-run-state-consistency-contract.md

@@ -210,14 +210,17 @@ writes.
   A client still running the previous revision therefore does not observe clears
   recorded after the migration; that reload boundary is deliberate, because a
   dual write cannot make the shared blob concurrency-safe. Versioned records are
-  pruned by age (7 days) and supersession only — never by session existence,
-  because the sidebar list is filtered by profile, project, and source, so an
-  absent row may simply be hidden. They are never part of the marker map
-  consumers read. A clear order is retained in module memory even when storage
-  quota prevents allocating its versioned key; the client still attempts the
-  smaller write that removes the marker from the existing marker map, so a user
-  can dismiss unread state under storage pressure. Logical clear order and
-  retention time are separate: records compare markers using their order stamp,
+  pruned by age (7 days), or when a newer ordering fact for that session was
+  successfully persisted — never by an in-memory-only superseding clear and never
+  by session existence, because the sidebar list is filtered by profile, project,
+  and source, so an absent row may simply be hidden. They are never part of the
+  marker map consumers read. A clear order is retained in module memory even when
+  storage quota prevents allocating its versioned key; the client still attempts
+  the smaller write that removes the marker from the existing marker map, so a user
+  can dismiss unread state under storage pressure. The failed allocation also
+  leaves any older durable clear record in place so reload does not lose the last
+  persisted ordering fact. Logical clear order and retention time are separate:
+  records compare markers using their order stamp,
   but the 7-day cap uses the wall-clock time at which the record was written, so
   a future logical stamp cannot extend retention indefinitely. The in-memory
   fallback lasts until reload, while successfully persisted records retain the

static/sessions.js

@@ -744,10 +744,10 @@ function _parseSessionCompletionUnreadClearedKey(key) {
   return { sid, version };
 }
 
-// Read the legacy whole-map key plus independent versioned clear records. A
-// clear writes a new key rather than replacing an older operation, so readers
-// can fold the maximum timestamp without a cross-context compare-and-set.
-function _readSessionCompletionUnreadCleared() {
+// Read persisted clear ordering independently from the in-memory fallback. The
+// distinction matters during pruning: a higher clear that failed quota and lives
+// only in memory must not delete the older durable record it supersedes.
+function _readPersistedSessionCompletionUnreadCleared() {
   // Snapshot keys before values. A clear that starts after this operation began
   // is deliberately excluded, so a marker prepared first gets the same logical
   // order and loses the clear-wins tie. A marker started after a completed clear
@@ -757,16 +757,6 @@ function _readSessionCompletionUnreadCleared() {
     keys = Array.from({ length: localStorage.length }, (_, i) => localStorage.key(i));
   } catch (_) {}
   const cleared = _readStoredJsonMap(SESSION_COMPLETION_UNREAD_CLEARED_KEY);
-  const now = Date.now();
-  for (const [sid, memoryRecord] of Object.entries(_sessionCompletionUnreadClearedMemory || {})) {
-    const value = Number(memoryRecord && memoryRecord.order) || 0;
-    const recordedAt = Number(memoryRecord && memoryRecord.recorded_at) || 0;
-    if (recordedAt && now - recordedAt > SESSION_COMPLETION_UNREAD_CLEARED_TTL_MS) {
-      delete _sessionCompletionUnreadClearedMemory[sid];
-      continue;
-    }
-    if (value > (Number(cleared[sid]) || 0)) cleared[sid] = value;
-  }
   try {
     for (const key of keys) {
       const parsed = _parseSessionCompletionUnreadClearedKey(key);
@@ -781,6 +771,23 @@ function _readSessionCompletionUnreadCleared() {
   return cleared;
 }
 
+// Combine durable clear ordering with this client's in-memory fallback. The
+// fallback keeps the current window correct when quota rejects a new key.
+function _readSessionCompletionUnreadCleared() {
+  const cleared = _readPersistedSessionCompletionUnreadCleared();
+  const now = Date.now();
+  for (const [sid, memoryRecord] of Object.entries(_sessionCompletionUnreadClearedMemory || {})) {
+    const value = Number(memoryRecord && memoryRecord.order) || 0;
+    const recordedAt = Number(memoryRecord && memoryRecord.recorded_at) || 0;
+    if (recordedAt && now - recordedAt > SESSION_COMPLETION_UNREAD_CLEARED_TTL_MS) {
+      delete _sessionCompletionUnreadClearedMemory[sid];
+      continue;
+    }
+    if (value > (Number(cleared[sid]) || 0)) cleared[sid] = value;
+  }
+  return cleared;
+}
+
 function _nextSessionCompletionUnreadOrder(sid, cleared = null) {
   const clears = cleared || _readSessionCompletionUnreadCleared();
   const cached = _getSessionCompletionUnread()[sid];
@@ -931,7 +938,7 @@ function _writeSessionCompletionUnreadCleared(sidOrSids, clearedAt) {
     try { localStorage.removeItem(SESSION_COMPLETION_UNREAD_CLEARED_KEY); } catch (_) {}
   }
 
-  const cleared = _readSessionCompletionUnreadCleared();
+  const persistedCleared = _readPersistedSessionCompletionUnreadCleared();
   let keys = [];
   try {
     keys = Array.from({ length: localStorage.length }, (_, i) => localStorage.key(i));
@@ -943,11 +950,10 @@ function _writeSessionCompletionUnreadCleared(sidOrSids, clearedAt) {
       const recordedAt = Number(localStorage.getItem(key)) || 0;
       const version = parsed.version || recordedAt;
       const expired = recordedAt > 0 && Date.now() - recordedAt > SESSION_COMPLETION_UNREAD_CLEARED_TTL_MS;
-      const superseded = version < (Number(cleared[parsed.sid]) || 0);
-      // Records are pruned by age and supersession only. Session existence is
-      // not a pruning signal: the sidebar list is filtered by profile, project,
-      // and source, so a session missing from it may simply be hidden, and
-      // dropping its clear record would re-light a cleared dot.
+      const superseded = version < (Number(persistedCleared[parsed.sid]) || 0);
+      // Prune by age, or when a newer ordering fact is itself durable. A
+      // memory-only clear may keep this window correct under quota pressure, but
+      // it cannot safely replace the older record across reload.
       if (expired || superseded) localStorage.removeItem(key);
     } catch (_) {}
   }

tests/_unread_store_helpers.py

@@ -32,6 +32,7 @@ _HELPER_NAMES = (
     "_nextSessionCompletionUnreadOrder",
     "_sessionCompletionUnreadClearedKey",
     "_parseSessionCompletionUnreadClearedKey",
+    "_readPersistedSessionCompletionUnreadCleared",
     "_readSessionCompletionUnreadCleared",
     "_mergeSessionCompletionUnread",
     "_writeSessionCompletionUnreadCleared",

tests/test_issue856_background_completion_unread.py

@@ -1,12 +1,21 @@
 """Regression checks for #856 background completion unread markers."""
 
 import json
+import shutil
+import subprocess
+import sys
 from pathlib import Path
 
+import pytest
+
 
 REPO = Path(__file__).resolve().parent.parent
 SESSIONS_JS = (REPO / "static" / "sessions.js").read_text(encoding="utf-8")
 MESSAGES_JS = (REPO / "static" / "messages.js").read_text(encoding="utf-8")
+NODE = shutil.which("node")
+
+sys.path.insert(0, str(REPO / "tests"))
+import _unread_store_helpers as unread_store_helpers  # noqa: E402
 
 
 def _done_block() -> str:
@@ -40,25 +49,81 @@ def _function_body(block: str) -> str:
     raise AssertionError("function closing brace not found")
 
 
+def _extract_sessions_function(name: str) -> str:
+    start = SESSIONS_JS.index(f"function {name}(")
+    brace = SESSIONS_JS.index("{", start)
+    depth = 0
+    for i in range(brace, len(SESSIONS_JS)):
+        if SESSIONS_JS[i] == "{":
+            depth += 1
+        elif SESSIONS_JS[i] == "}":
+            depth -= 1
+            if depth == 0:
+                return SESSIONS_JS[start:i + 1]
+    raise AssertionError(f"could not brace-match {name}")
+
+
+def _run_unread_behavior(body: str) -> dict:
+    functions = "\n".join(
+        _extract_sessions_function(name)
+        for name in (
+            "_getSessionViewedCounts",
+            "_saveSessionViewedCounts",
+            "_setSessionViewedCount",
+            "_getSessionCompletionUnread",
+            "_saveSessionCompletionUnread",
+            "_clearSessionCompletionUnread",
+            "_hasSessionCompletionUnread",
+            "_hasUnreadForSession",
+        )
+    )
+    script = f"""
+const store = {{}};
+const localStorage = {{
+  getItem: (key) => Object.prototype.hasOwnProperty.call(store, key) ? store[key] : null,
+  setItem: (key, value) => {{ store[key] = String(value); }},
+  removeItem: (key) => {{ delete store[key]; }},
+  key: (index) => Object.keys(store)[index] ?? null,
+  get length() {{ return Object.keys(store).length; }},
+}};
+const SESSION_VIEWED_COUNTS_KEY = 'hermes-session-viewed-counts';
+const SESSION_COMPLETION_UNREAD_KEY = 'hermes-session-completion-unread';
+let _sessionViewedCounts = null;
+let _sessionCompletionUnread = null;
+let _allSessions = [{{session_id: 'X'}}];
+const _sessionListSnapshotById = new Map();
+const S = {{session: null}};
+{unread_store_helpers.BLOCK}
+{functions}
+{body}
+"""
+    result = subprocess.run(
+        [NODE, "-e", script], capture_output=True, text=True, timeout=60
+    )
+    assert result.returncode == 0, f"node harness failed:\n{result.stderr}"
+    return json.loads(result.stdout)
+
+
+@pytest.mark.skipif(NODE is None, reason="node not on PATH")
 def test_background_completion_unread_uses_explicit_marker_not_message_delta():
     """A background completion must stay unread even when message_count has no delta."""
-    assert "SESSION_COMPLETION_UNREAD_KEY = 'hermes-session-completion-unread'" in SESSIONS_JS
-    assert "function _markSessionCompletionUnread(" in SESSIONS_JS
-    assert "function _clearSessionCompletionUnread(" in SESSIONS_JS
-    assert "function _hasSessionCompletionUnread(" in SESSIONS_JS
-
-    has_unread_idx = SESSIONS_JS.find("function _hasUnreadForSession(s)")
-    assert has_unread_idx != -1, "_hasUnreadForSession not found"
-    has_unread_block = SESSIONS_JS[has_unread_idx:SESSIONS_JS.find("async function newSession", has_unread_idx)]
-
-    marker_idx = has_unread_block.find("_hasSessionCompletionUnread(s.session_id)")
-    count_idx = has_unread_block.find("s.message_count > viewed.message_count")
-    assert marker_idx != -1, "_hasUnreadForSession must check explicit completion unread marker"
-    assert count_idx != -1, "_hasUnreadForSession must keep the existing message_count fallback"
-    assert marker_idx < count_idx, (
-        "explicit completion unread marker must be checked before message_count delta, "
-        "because completed streams can have viewed_count == message_count"
-    )
+    out = _run_unread_behavior("""
+store[SESSION_VIEWED_COUNTS_KEY] = JSON.stringify({
+  X: {message_count: 3, transcript_generation: 0},
+});
+store[SESSION_COMPLETION_UNREAD_KEY] = JSON.stringify({
+  X: {message_count: 3, completed_at: 1000, unread_order: 1000},
+});
+const beforeClear = _hasUnreadForSession({
+  session_id: 'X', message_count: 3, transcript_generation: 0,
+});
+_clearSessionCompletionUnread('X');
+const afterClear = _hasUnreadForSession({
+  session_id: 'X', message_count: 3, transcript_generation: 0,
+});
+console.log(JSON.stringify({beforeClear, afterClear}));
+""")
+    assert out == {"beforeClear": True, "afterClear": False}
 
 
 def test_background_done_sets_marker_when_session_not_actively_viewed():
@@ -550,17 +615,38 @@ def test_completion_unread_clears_only_when_session_is_opened():
     assert "_setSessionViewedCount(sid, messageCount);" in ack_body
 
 
-def test_historical_sessions_are_not_marked_unread_on_list_render():
-    """The explicit unread marker must be event-driven, not initialized by _hasUnreadForSession."""
-    has_unread_idx = SESSIONS_JS.find("function _hasUnreadForSession(s)")
-    assert has_unread_idx != -1
-    has_unread_block = SESSIONS_JS[
-        has_unread_idx:SESSIONS_JS.find("function _isSessionActivelyViewedForList", has_unread_idx)
-    ]
+@pytest.mark.skipif(NODE is None, reason="node not on PATH")
+def test_historical_sessions_are_not_marked_unread_on_first_list_render():
+    """First observation establishes a read baseline instead of inventing unread."""
+    out = _run_unread_behavior("""
+const unread = _hasUnreadForSession({
+  session_id: 'X', message_count: 5, transcript_generation: 3,
+  transcript_generation_baseline: 0,
+});
+const viewed = JSON.parse(store[SESSION_VIEWED_COUNTS_KEY]).X;
+console.log(JSON.stringify({unread, viewed}));
+""")
+    assert out == {
+        "unread": False,
+        "viewed": {"message_count": 5, "transcript_generation": 3},
+    }
 
-    assert "_markSessionCompletionUnread" not in has_unread_block, (
-        "rendering old historical sessions must not create completion-unread markers"
-    )
-    assert "_setSessionViewedCount(s.session_id, Number(s.message_count || 0), generation);" in has_unread_block, (
-        "missing viewed-count baseline should still initialize as read for historical sessions"
-    )
+
+@pytest.mark.skipif(NODE is None, reason="node not on PATH")
+def test_new_transcript_generation_keeps_post_shrink_messages_unread():
+    """A coalesced shrink and growth acknowledges only the retained baseline."""
+    out = _run_unread_behavior("""
+store[SESSION_VIEWED_COUNTS_KEY] = JSON.stringify({
+  X: {message_count: 10, transcript_generation: 0},
+});
+const unread = _hasUnreadForSession({
+  session_id: 'X', message_count: 3, transcript_generation: 1,
+  transcript_generation_baseline: 2,
+});
+const viewed = JSON.parse(store[SESSION_VIEWED_COUNTS_KEY]).X;
+console.log(JSON.stringify({unread, viewed}));
+""")
+    assert out == {
+        "unread": True,
+        "viewed": {"message_count": 2, "transcript_generation": 1},
+    }

tests/test_unread_cross_client_sync.py

@@ -125,6 +125,7 @@ _HELPER_FNS = [
     "_nextSessionCompletionUnreadOrder",
     "_sessionCompletionUnreadClearedKey",
     "_parseSessionCompletionUnreadClearedKey",
+    "_readPersistedSessionCompletionUnreadCleared",
     "_readSessionCompletionUnreadCleared",
     "_mergeSessionCompletionUnread",
     "_writeSessionCompletionUnreadCleared",
@@ -327,6 +328,36 @@ console.log(JSON.stringify({
     assert out == {"before": True, "after": False, "diskHasX": False}
 
 
+def test_quota_failed_clear_keeps_the_older_durable_tombstone_after_reload():
+    """An in-memory-only higher clear must not prune the durable ordering fact."""
+    out = _run_node(_script("""
+listed('X');
+const oldClearKey = `${SESSION_COMPLETION_UNREAD_CLEARED_PREFIX}${encodeURIComponent('X')}:2000`;
+_store[oldClearKey] = '1000';
+const A = makeClient();
+const ordinarySetItem = localStorage.setItem;
+localStorage.setItem = (key, value) => {
+  if (!Object.prototype.hasOwnProperty.call(_store, key)) throw new Error('QuotaExceededError');
+  ordinarySetItem(key, value);
+};
+// This advances X to 2001 in A's memory, but quota rejects that new durable key.
+A.clearUnread('X');
+const durableAfterClear = Object.prototype.hasOwnProperty.call(_store, oldClearKey);
+// Simulate a reload, then a stale client restoring a marker the durable clear
+// already outranks. Module memory is gone; only localStorage may suppress it.
+const B = makeClient();
+setDisk(SESSION_COMPLETION_UNREAD_KEY, {
+  X: {message_count: 3, completed_at: 1500, unread_order: 1500},
+});
+B.handleStorage({ key: oldClearKey });
+console.log(JSON.stringify({
+  durableAfterClear,
+  unreadAfterReload: B.hasUnread('X'),
+}));
+"""))
+    assert out == {"durableAfterClear": True, "unreadAfterReload": False}
+
+
 def test_stale_client_cannot_roll_a_viewed_count_back():
     """Sequential rollback: a client that saw an older list snapshot must not
     lower a count another client already acknowledged."""

tests/test_webui_run_state_consistency_contract_rfc.py

@@ -46,15 +46,6 @@ def test_contract_documents_the_client_side_unread_store_keys():
         )
 
 
-def test_contract_documents_generation_scoped_viewed_counts():
-    text = RFC.read_text(encoding="utf-8")
-    source = _sessions_js()
-
-    assert "transcript_generation" in source
-    assert "transcript_generation" in text
-    assert "monotonic only within" in text
-
-
 def test_documented_retention_window_matches_the_implemented_cap():
     source = _sessions_js()
     match = re.search(