The run state consistency contract is where sidebar metadata work routes, and its
state-layer table already lists the sidebar layer, but nothing recorded the rules
the two client-side unread stores actually follow: viewed counts converge by a max
merge, completion markers are ordered by per-session tombstones with a bounded
retention window, and the storage listener repairs the store instead of only
dropping the local cache.
Write those rules down where they belong, naming the three localStorage keys and
the retention cap, and state the limitation explicitly: each save is a
read-modify-write across separate localStorage operations with no lock between
them, so the contract is convergence rather than atomicity, and two concurrent
clears can still lose a tombstone.
tests/test_webui_run_state_consistency_contract_rfc.py pins the documented keys
and the documented retention window to the constants in static/sessions.js, so
changing one without the other fails. It fails on the previous revision and passes
with this change.
`_clearSessionViewedCount` returned early when this client's cache did not
hold the session id, so it never consulted the persisted map. A viewed count
another client acknowledged after our cache was loaded therefore survived
session deletion, leaving one entry behind per deleted session — the quota
pressure this prune path exists to avoid — and a later repair merged that
stale entry back into the store.
Read the store on every deletion and drop the key when the store holds it. The
deletion must not cost this client the acknowledgement it holds for another
session, so adopt the store's entries while keeping any higher count already
cached — the merge the repair path uses — instead of replacing the cache with
the store map. The write stays conditional, so a deletion that changes nothing
does not fire a storage event.
Behavioural two-client coverage in tests/test_unread_cross_client_sync.py:
`test_delete_prunes_a_viewed_count_only_the_store_holds` and
`test_delete_keeps_the_higher_count_this_client_holds_for_others`. Both fail on
the previous revision (the store keeps the deleted session's count, and the
repair writes it back) and pass with this change.
Two WebUI clients on the same origin/profile share one localStorage, and each
caches the unread stores in module state. Max-merging viewed counts on save and
dropping the caches on a storage event was not enough: a save read the store,
merged its cache and wrote the result back, so a write landing inside another
client's read-modify-write window was still lost — and the surviving client only
forgot its own cache, so it re-read the value that had already been lost. A
viewed count could roll backwards and re-light a cleared dot. The completion
markers behind the visible dot were worse: that save was a wholesale overwrite
with no merge at all, so a stale client re-serialising its cache re-added a
marker another client had cleared.
Make both stores converge on the later fact rather than on the last writer:
- viewed counts mean "seen at least up to N messages", so they merge with a max:
monotonic per session and additive across clients. A value that exists only in
our cache is written back only for a session the session list still shows
(_allSessions / _sessionListSnapshotById), so a session deleted in another
client, already pruned from the store by _clearSessionViewedCount, is not
resurrected.
- completion markers are add/remove and cannot be ordered by a max, so every
clear records a tombstone (hermes-session-completion-unread-cleared,
sid -> clearedAt) and a marker that does not postdate its clear loses. A
genuine later completion still wins. The tombstone store never enters the
marker map that consumers read, and is pruned by session existence and by a
7-day cap. Both clear paths record it: _clearSessionCompletionUnread (visit /
delete) and _clearCronSessionCompletionUnreadForInactiveProfiles (profile
switch).
- both saves write only when the store does not already hold the merged result,
and the storage listener runs that same merge instead of dropping our cache.
A client that still holds an acknowledgement therefore re-asserts it after the
other client's write, the loser sees a value it cannot beat and stops, and a
repair converges instead of ping-ponging storage events.
tests/test_unread_cross_client_sync.py drives the real functions against a
two-client harness sharing one fake localStorage: an interleaved write inside a
read-modify-write window, a deleted session's count, a stale client's marker
write after a clear, tombstone ordering vs a later re-mark, tombstone pruning,
and the registered storage listener dispatched with synthetic events.
tests/_unread_store_helpers.py gives the neighbouring node harnesses the helpers
those persistence paths call, so they keep driving the real code.
Two WebUI clients on the same origin/profile share one localStorage but each
cache the unread stores in module state and re-serialise that cache wholesale.
With no storage listener for either key, a stale client overwrites the other's
writes: a session's viewed count can roll backwards (re-lighting a cleared
unread dot via message_count > viewed_count) and a cleared completion-unread
marker can be resurrected.
Fix the class:
- max-merge viewed counts on save so a count is monotonic and additive across
clients, never dropping another client's entries or lowering a value;
- keep the in-cache viewed count monotonic on set;
- delete a viewed count directly from the persisted map (removals must still
leave disk, unlike the additive set path);
- invalidate both unread caches on storage events so a concurrent write is
re-read instead of overwritten.
Adds tests/test_unread_cross_client_sync.py (node-driven, observable behavior):
the viewed-count rollback fails before the fix and passes after; the clear path
keeps pruning; the storage-event handler prevents marker resurrection.
Michael Snoyman