Skip to content

fix(dashboard): let reading a session clear a stuck unread dot - #19779

Open
unstablebrainiac wants to merge 1 commit into
kirodotdev:mainfrom
unstablebrainiac:fix/unread-orphaned-watermark
Open

unstablebrainiac wants to merge 1 commit into
kirodotdev:mainfrom
unstablebrainiac:fix/unread-orphaned-watermark

Conversation

@unstablebrainiac

@unstablebrainiac unstablebrainiac commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Problem / Motivation

Goal: Opening, or marking as read, a session clears its unread dot for good, even when the dot was saved with a time newer than every row the gateway still reports.

An idle session's unread dot goes out when the session is opened, then comes back a moment later. Mark as read does the same, and every reload restores it. The dot was saved by a build before #15014 with the time of a turn's unsaved done row. #19688 traces why no read on main can clear it.

Why it matters

#15014 stopped new dots like this. It left the ones already saved in a browser, to clear on the session's next saved row. A session that gets no new row keeps a dot the user cannot dismiss, short of editing localStorage by hand.

Not a goal

  • How unread watermarks are recorded. fix(dashboard): take slot last_ts and unread watermarks from saved rows #15014 owns that.
  • A read that clears the shared unread record unconditionally. A saved dot goes only when the gateway's own answer shows it names no row; otherwise a read keeps today's guard.
  • Clearing the dot in another open window that never reads the session. It clears there on that window's next read of the session, or its next reload.
  • The crewmate thread view on the Members page. It reads as on main, and once the session is opened or marked read its dot stays out there too.
  • The read the dashboard relays when the user returns to a tab. It is unchanged.

What changed (motivation → approach → change)

An unread dot that came back after every read now stays cleared once the user opens the session or marks it read.

A saved dot clears only with a read at least as new as it, and a read carries the slot's last_ts, the newest row the gateway saved. A dot saved with the time of a row the gateway does not have is newer than any last_ts, so no read cleared it, and each read's own echo lit it again.

The fix asks the gateway, not this window. A read takes the watermark its dot carried at the click. If a page the gateway built after the click has no row as new as it, that watermark names a row the gateway does not have. A real one is always in that page: the gateway adds a row to the slot's window before broadcasting it, and the page includes unsaved rows. A page whose rows carry no time, such as a turn still streaming, proves nothing, so the dot keeps its guard.

A disproved watermark is not deleted from the shared record, which another window may be writing at the same moment. It goes in its own per-slot key, mc-unread-orphaned:<slot>, and every reader of the record treats an entry equal to it as absent. A newer watermark is a different value, so it shows.

Deleting the disproved value instead was rejected. The record's covered-read clear, given the disproved time, deletes any entry at or below it, so it erases a real older watermark another window wrote. An exact-value delete rewrites the whole record, so it can erase a newer watermark written in between. Nor can it tell an orphan from an arrival that left the value in place.

A one-time sweep at boot was rejected too. It would ask the gateway about every saved dot on every window start until a versioned record lands, and it races arrivals exactly as a read does, so it needs the same marker. Fixing a dot on read asks only about the session the user opened. A versioned record would drop every saved dot once, real ones included. The spec section names the removal condition.

Where Change
slotSwitch.ts Opening a session captures the dot's watermark before clearing it and, when last_ts cannot cover it, writes an arrival marker. Once the transcript loads, it retires the watermark if the page has no row as new as it and the marker is unchanged, removes the marker, then relays the read as before.
dashboardSlice.ts, dashboard/readSlot.ts readSlot(key) does the same for Mark as read. Only when last_ts cannot cover the watermark, it first fetches one row from the gateway, bounded by UNREAD_PROOF_TIMEOUT_MS (10 s; a later answer is no proof), then relays the watermark the click captured, so a row that arrives meanwhile stays unread. A fresh marker in mc-unread-arrival:<slot> brackets the read, and every new message removes it, then the tombstone. A new message on a slot holding a tombstone writes the shared record at once instead of joining main's queued write, so the disproved value leaves before its tombstone. A read whose marker changed or could not be written retires nothing, and the retire re-checks the marker after writing its tombstone, so a row any window recorded meanwhile keeps its dot. retireSlotUnread writes the tombstone and, once that write lands, clears this window's dot while it carries that watermark.
dashboard/unread.ts, dashboard/selectors.ts, dashboard/readSlot.ts, unreadArrivalMarker.ts Keep dashboardSlice.ts under the 2,000-line cap; the slice re-exports what callers import. dashboard/unread.ts gains the tombstone keys, forgetUnreadSlots and the proof helpers. dashboard/selectors.ts is new and takes the slice's read-side selectors unchanged. dashboard/readSlot.ts is new and holds readSlot; callers import it from there, because it dispatches the slice's own actions and a re-export from the slice would make an import cycle. unreadArrivalMarker.ts is new and holds the marker.
slotReadRelay.ts emitSlotRead splits into slotReadWatermark, the watermark a read made now carries, and relaySlotReadWatermark, which sends a captured one without adding later rows. Existing callers send what they sent before.
useSessionActions.ts Mark as read dispatches readSlot.
docs/system-specs/modules/app-notifications.md Documents the three key families, their ordering rules and the condition for removing the tombstone and marker.

Backwards compatibility

Compatible: the shared record's format is unchanged, the retire never writes it, and every gesture relays the same slot_read time as before. A disproved watermark keeps one mc-unread-orphaned:<slot> key until the slot's next message, and mc-unread-arrival:<slot> exists only while a read of that slot waits for the gateway. Removing a session, or a live slot list dropping it, removes both. The diff hides tombstoned entries from the record's readers:

  • Reader: website/src/store/dashboard/unread.ts:restoreUnreadSince -- chat -- skips a tombstoned value and restores a newer one (restores a newer watermark written after the retire)
  • Reader: website/src/store/dashboard/unread.ts:restoreUnreadBadges -- chat -- no dot for a tombstoned value; a manual reminder still re-seeds its dot (reload restores neither watermark nor badge while the shared orphan remains)
  • Reader: website/src/store/dashboardSlice.ts:remoteSlotRead -- chat -- a tombstoned value lights no dot, so a read's own echo cannot bring it back (keeps the local dot cleared through its own older slot_read echo)
  • Reader: website/src/store/dashboard/unread.ts:writeSharedUnread -- chat -- a new arrival replaces a tombstoned value even when its ts is older, written at once (a new older arrival replaces a tombstoned shared value and restores its dot) or from the queue (an arrival queued before another window retires the slot replaces the tombstoned value when it lands)
  • Reader: website/src/components/InstancesViewport.tsx:setUnread -- chat -- retireSlotUnread posts the lowered unread count to the hub (posts the lowered unread count to the embedding hub)

The unlisted purge in reconcileSlots deletes the shared entry, tombstone and marker of every listed slot a live-stream list drops, retired ones included. It is gated like eviction, so a stale HTTP reply removes nothing, and held like main's drain, so a list the gateway labels partial keeps a session it has not restored yet. A removed frame clears them whatever the hold, as main's drain does. Readers of what it deletes:

  • Reader: website/src/store/dashboard/unread.ts:restoreUnreadSince -- chat -- a reload restores no watermark for the dropped slot (a retired session another window deletes leaves no saved dot, and a stale reply removes nothing)
  • Reader: website/src/store/dashboard/unread.ts:restoreUnreadBadges -- chat -- a reload lights no dot for the dropped slot (same test)
  • Reader: website/src/store/dashboard/unread.ts:clearSharedUnreadIfCovered -- chat -- finds no entry and clears nothing (a removed frame for a retired session leaves no saved dot)
  • Reader: website/src/store/dashboard/unread.ts:persistSharedUnread -- chat -- the same write rewrites the mc-unread-slots projection, as main's departed-slot drain does (drains unread state for a slot that vanished from the authoritative list)

Tests

website/src/test/unreadOrphanedWatermark.test.ts boots a window from the record an older build left, with a slot row whose last_ts is below the saved dot:

  • a read retires the dot when the gateway's page is older, keeps it out through the window's own slot_read echo, restores nothing on reload, lowers the hub count without counting a held orphan, and relays the click-time watermark only after the gateway answers; a row that arrives while the gateway answers stays out of that relay and keeps its dot across a reload, in this window or another, and an unsaved row received before the click is still in it; an arrival leaves no marker, a read writes its marker before it clears the dot, an arrival while a read waits keeps its dot, including one that lands while the retire writes its tombstone, a read that cannot write its own marker retires nothing, and a finished or failed read leaves no marker behind;
  • the dot stays when the gateway's page has a row as new as it, even though this window's row is stale, when the page holds only a turn still streaming, and when the gateway request fails; no request goes out when this window's last_ts already covers it, including a last_ts a streaming turn moved only in main's mid-turn overlay;
  • the retire makes no write to mc-unread-shared; a newer watermark another window writes before or after it is restored on reload, and so is one this window copied from another window's record before the read;
  • two windows retiring different slots keep both tombstones; a failed tombstone write leaves the dot lit; a window that loaded the dot before the proof keeps it through a relay, so a newer message another window recorded still shows, and drops it on reload or on its own read; a new message removes the tombstone, so its dot shows after a reload even when its write fails or a stale write puts the old value back, including while a read waits, and a queued write still replaces a value another window tombstoned before it landed; removing a session, a slot list or removed frame without it, or a first slot list without an unread session drops its tombstone and marker, and a retired session another window deletes leaves no saved dot while a stale HTTP reply removes nothing; a partial slot list keeps an absent session's dot, tombstone and marker, and a removed frame still clears a retired one's, before the first snapshot too;
  • a manual reminder is never retired, and a retire keeps a manual reminder this window set.

website/src/test/slotReadRelay.test.tsx: opening the session over the dashboard socket keeps the dot out after the gateway echoes the read and restores none on reload, and keeps the saved dot when the fetched page holds its row or has no row time, or when another window records a row during the load, with or without a gap against this tab's cached rows. It writes no marker for a dot last_ts covers, including a last_ts held only in main's mid-turn overlay, retires nothing when it cannot write its own, and leaves none behind when the load fails. The first fails with main's slotSwitch.ts.

website/src/pages/members/MembersPage.openThread.test.tsx: viewing a crewmate thread never asks the gateway about the dot.

Each of the 61 safeguards in the tombstone readers, recordUnreadOrphan, retireSlotUnread, readSlot, the arrival write, the read relay, the arrival marker, watermarkDisprovedBy, orphanedWatermark, newestRowTs, switchSlot, the slot-list cleanup and the Members page view was removed in turn, and a test failed every time. Run locally on this commit: these files, the 61 removals, tsc -p tsconfig.app.json, eslint, the i18n, phantom-class and restyle checks, the production build and the website tests related to the diff (419 files). The whole website test suite (2884 files) ran on an earlier revision of this change.

Manual verification

N/A: unit coverage sufficient. The tests boot a window from the exact record the report describes and drive the dashboard's real socket handler, unread store and read relay.

Screenshots / video

Why no screenshot: the dot looks and lights the same; only when a read clears it changes.

Related Issues

Closes #19688

Follow-up: #21498 (the two cases where the per-session keys still outlive a deleted session).

Pattern harvest

Rule candidate: review-prompt
Pattern: a guard that waits for a later value to cover a stored one never releases when the stored value names a row that no longer exists.

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable)
  • No secrets, credentials, or internal references in the diff

@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Oct 13, 2026
@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Design Review (Opus 5.5, fork) — ✅ PASS

Design-level review of 8ad3def6a6a22b7abbe21416540e40e43a8fb56d via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

Design-Verdict: PASS

Nothing blocks: the change fixes the stuck dot at its source (a saved watermark that names no row), only on the read gestures, and the spec now states when the tombstone and marker can be removed.

Notes

  • This adds two per-slot localStorage key families and a five-step ordering protocol, all to clean up dots saved before fix(dashboard): take slot last_ts and unread watermarks from saved rows #15014, a problem that shrinks over time. You rejected the cheaper option, a versioned record that drops every pre-fix(dashboard): take slot last_ts and unread watermarks from saved rows #15014 entry once, because it would also lose real dots. If that one-time loss turns out to be acceptable, ship the versioned record and remove this machinery, as the documented removal condition already allows.
  • markSlotUnread now writes the shared record immediately (no longer queued) whenever the slot has a tombstone. Any later caller that writes it without going through queueSharedUnread must follow the same order (shared write, then marker, then tombstone), or a disproved value can come back. Rule 1 in the spec states this, but nothing enforces it.

[DESIGN-REVIEWED] 8ad3def

Description read

[DESCRIPTION-READ] 5e048725f5222b00b8a24bee11a121fe268d408d1117df2cedfc2e2aeadfdaa2

That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

UX Review (Opus 5.5, fork) — ✅ PASS

UX-level review of 8ad3def6a6a22b7abbe21416540e40e43a8fb56d via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

UX-Verdict: PASS

Nothing to check. The diff adds no control, label or copy. It only changes store and relay logic, so an unread dot that used to come back now stays cleared when you open a session or mark it read.

[UX-REVIEWED] 8ad3def

Description read

[DESCRIPTION-READ] 4c2e539411d90ebbb702041e868b518d74a79dc112b8c39465cdc076ca280214

That sha256 names everything this verdict read: the PR description, plus the 3 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

First Principles Review (Opus 5.5, fork) — ✅ PASS

Premise-level review of 8ad3def6a6a22b7abbe21416540e40e43a8fb56d via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

First-Principles-Verdict: PASS

Nothing blocks: the fix has a traced cause (#19688) and tests, and changes no default. The tombstone-and-marker machinery is the one thing worth questioning.

Notes

  • Item 4 — oversized: the fix clears legacy data written before fix(dashboard): take slot last_ts and unread watermarks from saved rows #15014 by adding two permanent key families, a tombstone filter in every reader of mc-unread-shared, and three ordering rules, which "stays" until "a versioned record that drops unversioned entries". The description rejects that migration because it "would drop every saved dot once, real ones included". Losing a real dot once is a one-time, reversible cost. The forever cost of the read path is not. Subtraction: replace mc-unread-orphaned:*, mc-unread-arrival:*, readSlot's proof fetch and the reader filters with the one-time versioned-record drop. The spec's own removal condition already names this.

What this change ships

Inventory (8 items) — 7 justified

Intent: let a user dismiss an unread dot that keeps coming back on a session they have opened or marked read. FIX.

  1. Opening a session with a stuck dot clears it for good — justified
  2. Mark as read clears a stuck dot for good — justified
  3. Mark as read on a suspect dot waits up to 10 s for a one-row gateway fetch before relaying — justified
  4. New per-slot tombstone and arrival-marker localStorage keys, filtered by every unread reader — oversized: permanent machinery for a one-time legacy-data problem
  5. A new message on a tombstoned slot writes the shared record at once, not queued — justified
  6. Removed or unlisted slots now purge their shared entry, tombstone and marker — justified
  7. Slot selectors moved to dashboard/selectors.ts — justified
  8. emitSlotRead split into slotReadWatermark + relaySlotReadWatermark — justified

[FIRST-PRINCIPLES-REVIEWED] 8ad3def

Description read

[DESCRIPTION-READ] 221b704030ab7f014d3e8618a483ce2d324b8d8037767ad5c88619d82e486d73

That sha256 names the PR description this verdict read. Recompute it with .github/scripts/pr-description-capture.sh; if it differs, the description changed after this verdict was formed and any finding drawn from the description is unproven.

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

GPT 6.1 Review (fork) — ✅ no blocking findings (all downgraded on adjudication)

Reviewed 8ad3def6a6a22b7abbe21416540e40e43a8fb56d via the fork AI-review pipeline; updated in place on each push.

Opus 5.5 adjudication downgraded every blocking finding. Downgraded: the remedy each one needs is disproportionate to the harm it prevents. Adjudication downgraded all 1 blocking finding(s): the remedy each one requires is disproportionate to the harm it prevents.

Review details

BLOCKING -- website/src/store/dashboardSlice.ts:1657 -- A superseded read deletes another window’s completed retirement
clearUnreadOrphan(slot)
Trigger: Two dashboard windows read the same orphan through readSlot or switchSlot. After A passes retireSlotUnread’s first marker check, B replaces the marker, completes retirement, and removes its marker. A then writes the tombstone, fails its second marker check, and deletes it.
Outcome: B’s successful retirement is undone; the orphan remains in mc-unread-shared, so a subsequent read echo or reload restores the stuck unread dot.
Anchor: defect
Fix: Serialize per-slot marker/tombstone mutations and cleanup across windows so a superseded read cannot delete another read’s successful retirement.

[BLOCK-MERGE-DOWNGRADED] 8ad3def
[GPT-REVIEWED] 8ad3def

Adjudication (Opus 5.5) — is blocking on each finding proportionate?

F1 is downgraded: blocking the merge on it costs more than it would prevent.

  • Harm: LOW. For the failure to happen, window B's marker swap (holdUnreadArrival, website/src/store/unreadArrivalMarker.ts:774-777) has to land after A's first marker check (dashboardSlice.ts:1653). B also has to write its tombstone before A removes it (dashboardSlice.ts:1655-1656). But B only gets to write that tombstone after a gateway request finishes (api.chatSlotDetail, readSlot.ts:258-262), and A's checks run back to back in one synchronous reducer. So B's network round trip would have to fit in the few microseconds between two of A's localStorage calls.

  • If it does happen, the unread dot that was already stuck before this PR stays lit. The user can see it, nothing is corrupted, and the next read in either window takes a fresh marker and retires it again (readSlot.ts:250-262).

  • The real fix costs more than that. It needs a lock shared across windows (Web Locks or an equivalent ordering contract) around the marker, the tombstone and the cleanup in readSlot.ts:266-268. That is a new mechanism serving only this rare path, and it adds async ordering inside a synchronous reducer.

    [ADJUDICATION] 8ad3def total=1 uphold=0 downgrade=1
    DOWNGRADE F1 website/src/store/dashboardSlice.ts:1657 reason=disproportionate-remedy
    [GPT-ADJUDICATED] 8ad3def

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Opus 5.5 Review (fork) — ✅ no blocking findings

Reviewed 8ad3def6a6a22b7abbe21416540e40e43a8fb56d via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 8ad3def

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Oct 13, 2026
@unstablebrainiac
unstablebrainiac force-pushed the fix/unread-orphaned-watermark branch 2 times, most recently from af1f572 to aafcc71 Compare October 13, 2026 10:43
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Oct 13, 2026
@unstablebrainiac
unstablebrainiac force-pushed the fix/unread-orphaned-watermark branch from aafcc71 to e0a07c4 Compare October 13, 2026 16:40
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Oct 13, 2026
@unstablebrainiac
unstablebrainiac force-pushed the fix/unread-orphaned-watermark branch from e0a07c4 to 8052589 Compare October 13, 2026 18:27
@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Oct 13, 2026
@iamwhatever

Copy link
Copy Markdown
Contributor

This PR lets opening a session or Mark as read retire a pre-#15014 saved unread watermark that names no gateway row: the read captures the dot's watermark, asks the gateway for a page built after the click, and when no row is as new records an mc-unread-orphaned:<slot> tombstone that every reader of mc-unread-shared treats as absent, guarded by an mc-unread-arrival:<slot> marker so a message arriving mid-read keeps its dot. I read every production file (dashboardSlice.ts readSlot/retireSlotUnread/watermarkDisprovedBy and the four reader filters, slotSwitch.ts, slotReadRelay.ts, useSessionActions.ts) and the root cause is addressed; I found no defect: a manual sentinel is never retired, a timeless streaming page proves nothing, a failed fetch keeps the guard, the marker is re-checked on both sides of the tombstone write, and a new arrival clears marker then tombstone, so a real newer watermark always shows. The blocker is the hard rule on unanswered AI review: two CONCERNS verdicts on the current head 8052589 have no ai-review-disposition reply. Design Review (target=design) says the tombstone/marker protocol is permanent cost for a legacy-only cause with no stated removal condition, and that the new mc-unread-orphaned/mc-unread-arrival keys and their ordering rules are not written into the owning spec. First Principles (target=first-principles, item 3/4) argues an exact-value delete through persistSharedUnread({}, [slot]) would be enough, since dashboardSlice.ts:183-186 already accepts the last-writer-wins RMW race. Please post one disposition per item (fixed or rebutted with evidence; your description's argument against the exact-value delete is a good basis for the FP rebuttal), and either name the removal condition and document the keys or rebut those too; I will re-check once they are posted. — crew-pr-reviewer (deep re-check)

@unstablebrainiac
unstablebrainiac force-pushed the fix/unread-orphaned-watermark branch from 8052589 to 58081c6 Compare October 14, 2026 00:00
@unstablebrainiac

Copy link
Copy Markdown
Contributor Author

Replies on head 58081c6:

  • Design, no doc for the new keys: fixed. docs/system-specs/modules/app-notifications.md has a new section, "Shared unread record and retired watermarks". It names mc-unread-shared, mc-unread-orphaned:<slot> and mc-unread-arrival:<slot>, and gives the ordering rules. An arrival publishes first, then clears the marker, then the tombstone. A retire checks the marker before and after writing the tombstone. Closing a slot or reconcileSlots' departed branch prunes both keys.
  • Design, permanent cost with no exit: fixed, with a stated removal condition. The same section says the tombstone and marker keys, and the readers' tombstone filter, can be removed once mc-unread-shared is migrated in a way that drops entries saved before fix(dashboard): take slot last_ts and unread watermarks from saved rows #15014, for example a versioned record that discards unversioned entries on first read. Until then a saved orphan can surface on any later read. The cost of an arrival with no read in flight is two removeItem calls.
  • First Principles item 3, exact-value delete: rebutted. The race the record accepts at dashboardSlice.ts:210-217 loses a write, and the next arrival redoes it. An exact-value delete loses a message. The record merges newest-timestamp-wins, and a '' arrival never demotes a stored watermark. So an arrival during the read that publishes '' (no saved row yet) or an older timestamp leaves the stored value equal to the disproved W. Then persistSharedUnread({}, [slot]) behind a stored[slot] === W check deletes a real new message's dot, and nothing relights it until another arrival. The stored value cannot tell "still the orphan" from "absorbed an arrival", so the per-read marker records the arrival instead. The arrival-during-read cases in unreadOrphanedWatermark.test.ts cover this.

@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Oct 14, 2026
@unstablebrainiac

Copy link
Copy Markdown
Contributor Author

Rebased onto main at 0cc75b1. The only conflict was the dashboardSlice actions export list, where #21346 added the pin-order actions; both sets are kept. The patch is otherwise unchanged. tsc and eslint are clean, and the touched suites pass (1037/1037).

Dispositions on 0cc75b1:

  • Design CONCERNS (three permanent key families, nothing schedules removal): needs a maintainer decision. The keys are permanent only if the protocol stays. The spec section "Shared unread record and retired watermarks" names the removal condition: a versioned migration of mc-unread-shared. Which way to go is the migration question below.
  • First Principles CONCERNS (the rejected cheaper fix is the spec's own removal path): partly agreed. The migration is the cheaper code, and it is the removal path. It is not a no-cost swap: one value cannot tell a stuck pre-fix(dashboard): take slot last_ts and unread watermarks from saved rows #15014 dot from a real one, so the migration clears every saved dot once, real ones included. The protocol keeps real dots. That trade is the migration question.
  • UX PASS (restored dot stays out after a reload in two windows): covered by the cross-window reload tests in unreadOrphanedWatermark.test.ts.

Migration question: still open for a maintainer. If the protocol stays, I'll open the removal issue and link it in the description. If the migration is preferred, I'll replace the protocol with it in this PR.

Key leaks: opened #21498 for the last slot deleted during a reconnect and a slot deleted while no window is open. It is linked in the description.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: passed Eligible automated validation passed for the current revision merge conflict Branch has merge conflicts with its base — author must resolve before merge readiness: checking Automated validation is still running labels Oct 17, 2026
@unstablebrainiac
unstablebrainiac force-pushed the fix/unread-orphaned-watermark branch from 0cc75b1 to 9d9969a Compare October 17, 2026 07:23
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Oct 17, 2026
@unstablebrainiac
unstablebrainiac force-pushed the fix/unread-orphaned-watermark branch from 9d9969a to ada3651 Compare October 17, 2026 11:34
@github-actions github-actions Bot removed the readiness: action required A blocking check or review needs attention label Oct 17, 2026
mbriones98
mbriones98 previously approved these changes Oct 18, 2026

@mbriones98 mbriones98 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checks green and readiness passed at e74bdad; no new feature.

@iamwhatever

Copy link
Copy Markdown
Contributor

Holding at e74bdad for a maintainer decision.

The code still looks sound, and our r99 remote-delete finding stays fixed. #21498 tracks the two leak cases. What is still open:

  • Product choice: keep the permanent mc-unread-orphaned / mc-unread-arrival protocol, or replace it with a one-time versioned migration that drops every saved dot once (question). @mbriones98's approval does not rule on this. If the protocol stays, please open the removal issue you offered and link it in the description.
  • Current-head lane replies: Design CONCERNS (removal condition; the 10 s relay wait in readSlot, dashboardSlice.ts#L1838-L1863), First Principles item 4 and the UX check were re-posted for e74bdad. Your dispositions are on 0cc75b1. Re-posting them for this head is enough once the decision is made.

Checks on this head are green.

— crew-pr-reviewer (r110)

@iamwhatever

Copy link
Copy Markdown
Contributor

HOLD at e949e0b.

The code still looks sound and our r99 remote-delete finding stays fixed. A retire checks the arrival marker on both sides of the tombstone write (dashboardSlice.ts L1556-L1570). A live list that drops a slot prunes all three keys for it (L498-L500). #21498 covers the two leak cases. CI on this head is green.

What clears the hold:

  • Maintainer decision. Either keep the permanent mc-unread-orphaned / mc-unread-arrival protocol, or replace it with a one-time versioned migration that drops every saved dot once (question). If the protocol stays, open the removal issue you offered and link it in the description.
  • Current-head lane replies. The Design and First Principles CONCERNS were re-posted for e949e0b, but your dispositions are still on 0cc75b1. Design's second Watch item has no reply on any head: the Reader list should also cover the new unlisted purge in reconcileSlots. One disposition per item on this head is enough.

I read the diff and the test descriptions. I did not run the tests.

— crew-pr-reviewer (r111)

@unstablebrainiac

Copy link
Copy Markdown
Contributor Author

Dispositions on e949e0b:

  • Design Watch 1 (no end date for the compat layer): needs the maintainer decision. The keys are permanent only if the protocol stays. If it stays, I will open the removal issue and link it in the description. If the one-time versioned migration is preferred, I will replace the protocol with it in this PR.
  • Design Watch 2 (readers of the unlisted purge in reconcileSlots): fixed in the description. ## Backwards compatibility now has a second list for that purge: restoreUnreadSince, restoreUnreadBadges, clearSharedUnreadIfCovered and the mc-unread-slots projection written by persistSharedUnread, each with the test that covers it. No code change.
  • First Principles (item 3 oversized; the versioned drop is cheaper): partly agreed, same decision. The drop is less code and is the removal path. It also clears every saved dot once, real ones included, because one value cannot tell a stuck pre-fix(dashboard): take slot last_ts and unread watermarks from saved rows #15014 dot from a real one. The protocol keeps real dots. Which trade to take is the migration question.
  • UX PASS headline (dot stays cleared after Mark as read and a reload, gateway timeout included): covered by the reload and timeout tests in unreadOrphanedWatermark.test.ts. A timeout keeps a possibly real dot by design.

@iamwhatever

Copy link
Copy Markdown
Contributor

HOLD at e949e0b.

Thanks for the dispositions on this head (comment). The second Reader list in Backwards compatibility covers the unlisted purge in reconcileSlots (dashboardSlice.ts L498-L500), and the tests it names exist. That clears Design Watch 2. Every lane item now has a reply on this head, and CI is green.

One thing clears the hold: a maintainer decision on the question. The choice is between keeping the mc-unread-orphaned / mc-unread-arrival protocol and replacing it with a one-time versioned drop of saved dots. Design Watch 1 and First Principles item 3 both depend on it. If the protocol stays, please open the removal issue you offered and link it in the description.

I read the description and test names. I did not run the tests.

— crew-pr-reviewer (r112)

@iamwhatever

Copy link
Copy Markdown
Contributor

HOLD at ec9be94 for a maintainer decision.

This head is a rebase of e949e0b. The only changed hunk is import and signature context from main in useSessionActions.ts. The code still looks sound. #21498 tracks the two key-leak cases. CI is green.

What clears the hold:

— crew-pr-reviewer (r116)

@unstablebrainiac

Copy link
Copy Markdown
Contributor Author

Replies on ec9be94:

  • Maintainer decision and the removal issue (Design note 1 asks for the same issue): unchanged. If the protocol stays, I open the removal issue and link it in the description. If the one-time versioned drop is preferred, I replace the protocol with it in this PR.
  • GPT F1 (a one-row tail page as proof): rebutted. The ordering is real. _flush_segment re-lands a segment's stop card after the finalized reply with its earlier time (segment_flush.py L631-L635), and last_ts is the last durable row by position (slot_projection.py L241-L254), so both read the stop card. On main that makes the reply's dot undismissable: opening the session (slotSwitch.ts L191) and Mark as read both go through markSlotRead, which keeps the shared entry while last_ts cannot cover it (L1454-L1455, L260), so every reload lights it again until a newer durable row lands. readSlot runs only from Mark as read (useSessionActions.ts L425) and can tombstone only held, the watermark lit at that click. A later reply has a different value, and its arrival voids the retire through the marker. So in this ordering the PR clears the dot the user marked read, and a gateway max-timestamp proof would bring main's undismissable dot back.
  • GPT F2 (a stale rollback deletes a committed tombstone): rebutted. The two marker checks and the tombstone write are consecutive statements of one synchronous reducer (L1559-L1563), and the rollback runs only when another window writes the marker between two of them. In the stated order, B's marker arm, its gateway round trip (L1899), its retire and its marker cleanup (L1907) all have to land inside that gap. The worst case is main's state, a stuck dot, which the next Mark as read retires again. A cross-window lock would be new machinery for that gap.
  • Design note 2 (the unlisted purge assumes a full list): no new assumption. Main's reconcileSlots already treats every list as complete. Its drain runs on both writers, with no freshness gate, and deletes the shared entry of every lit slot the list omits (main L448, since fix(dashboard): sync unread badge dismissal across open windows #10004). applySlots takes sidebar membership from the list (main L481). The purge applies the same assumption to a retired slot, which has left unreadSlots, and only on the live-stream path. A caller that sent a partial list would already drop sessions from main's sidebar and wipe their shared dots.

@iamwhatever

Copy link
Copy Markdown
Contributor

HOLD at ec9be94 for a maintainer decision.

Thanks for the replies on this head. They answer the r116 items:

  • GPT F2: the two marker checks and the tombstone write are consecutive statements in one reducer (dashboardSlice.ts#L1556-L1566), and the worst case is main's stuck dot.
  • GPT F1: readSlot runs only from Mark as read and can retire only the watermark lit at that click, so a later reply keeps its dot.
  • Design note 2: main's reconcileSlots already treats every list as complete, so the unlisted purge adds no new assumption.

One thing still clears the hold: a maintainer decision on the open question, keep the mc-unread-orphaned / mc-unread-arrival protocol or replace it with a one-time versioned drop of saved dots. If the protocol stays, open the removal issue you offered and link it in the description. #21498 already tracks the two key-leak cases.

CI is green and the branch merges cleanly with main. No PR code was run locally.

— crew-pr-reviewer (r117)

@iamwhatever

Copy link
Copy Markdown
Contributor

HOLD at 281e923 for a maintainer decision.

This head is a rebase of ec9be94 plus a move of the unread persistence helpers into unreadPersistence.ts. I compared the moved lines with the ones removed from dashboardSlice.ts: the move is faithful, adding only exports and an import. The reconcileSlots call now keys on main's retained rows, so a retained member is not purged. GPT's downgraded finding is the same cross-window rollback you already rebutted. CI is green.

One thing still clears the hold: a maintainer decision on the open question. Either keep the mc-unread-orphaned / mc-unread-arrival protocol, or replace it with a one-time versioned drop of saved dots. If the protocol stays, open the removal issue you offered and link it in the description; the Design note on this head asks for the same follow-up. #21498 already tracks the two key-leak cases.

No PR code was run locally.

— crew-pr-reviewer (r118)

@iamwhatever

Copy link
Copy Markdown
Contributor

HOLD at 46a71b1 for a maintainer decision.

This head is a rebase of 281e923 onto main's split of the unread store. The tombstone helpers now live in store/dashboard/unread.ts, and the read-side selectors move to store/dashboard/selectors.ts; the moved lines match the removed ones apart from imports. The retire logic is unchanged (dashboardSlice.ts L1611-L1620), the code still looks sound, our r99 remote-delete finding stays fixed, and CI is green. GPT's downgraded finding is the same cross-window rollback you already rebutted.

One thing still clears the hold: a maintainer decision on the open question. Either keep the mc-unread-orphaned / mc-unread-arrival protocol, or replace it with a one-time versioned drop of saved dots. If the protocol stays, open the removal issue you offered and link it in the description; the Design and First Principles notes on this head ask for the same follow-up. #21498 already tracks the two key-leak cases.

No PR code was run locally.

— crew-pr-reviewer (r122)

@iamwhatever

Copy link
Copy Markdown
Contributor

HOLD at 284f9ec for a maintainer decision.

This head is a rebase of 46a71b1. The only change is that slotSurfaceKey and slotIsRemoteBound now come from main's slotIdentity.ts. The read and retire logic is unchanged (readSlot, retireSlotUnread), the code still looks sound, and CI is green.

What clears the hold:

  1. A maintainer decision on the open question: keep the mc-unread-orphaned / mc-unread-arrival protocol, or replace it with a one-time versioned drop of saved dots. If the protocol stays, open the removal issue you offered and link it in the description. Unread dot keys can outlive a deleted session in two cases #21498 already tracks the key leaks.
  2. A reply to First Principles item 9 on this head: UNREAD_PROOF_TIMEOUT_MS and newestRowTs are re-exported from the slice (L223) but only tests use that path. Drop them from the re-export, or say why they stay.

No PR code was run locally.

— crew-pr-reviewer (r125)

@unstablebrainiac

Copy link
Copy Markdown
Contributor Author

Replies on cde8bbc:

  • Maintainer decision and the removal issue: unchanged. If the protocol stays, I open the removal issue and link it in the description. If the one-time versioned drop is preferred, I replace the protocol with it in this PR.
  • First Principles item 9: done. dashboardSlice.ts no longer re-exports UNREAD_PROOF_TIMEOUT_MS or newestRowTs (L224). The test imports both from store/dashboard/unread.ts, where they are defined (L38).

A shared unread watermark newer than every row the gateway still reports
can never be covered by a read, so opening the session clears the dot and
the window's own slot_read echo lights it again. Builds before kirodotdev#15014 left
such watermarks from a turn's unsaved done row.

A read now retires the watermark its dot carried when a page the gateway
built after the read began has no row as new as it: the transcript
switchSlot already fetches, or a one-row page for Mark as read. A page
whose rows carry no time, such as a turn still streaming, proves nothing.
Mark as read relays the watermark it captured at the click, so a row that
arrives while its page is in flight stays unread. A read that may retire
first writes a fresh per-slot marker, and retires nothing if it cannot or
if the marker changed while it waited: every new message removes the
marker before the tombstone, and the retire checks the marker before its
write and again after, removing the tombstone if it changed. The read removes its own
marker when it is done. Removing the session, or an authoritative slot
list that drops a session this window listed, removes both per-slot keys.
The retire never rewrites the shared record, which another window may be
writing. It records the disproved value in a key of its own per slot, and
every reader of the record treats a matching entry as absent. The slot's
next message removes that key, so a value that comes back after it shows
a dot. Another window already showing the dot keeps it until that window
reloads or reads the session.
@iamwhatever

Copy link
Copy Markdown
Contributor

HOLD at 8ad3def for a maintainer decision.

This head integrates main's queued shared-unread writes and slotLastTsOf overlay. An arrival on a slot holding a tombstone still writes the shared record at once (dashboardSlice.ts#L1552-L1553), and a queued flush treats a value equal to its tombstone as absent (unread.ts#L134-L150), so the retire ordering still holds. Our r125 item is fixed: the slice no longer re-exports UNREAD_PROOF_TIMEOUT_MS or newestRowTs. Every AI lane passes, GPT's one downgraded finding is the cross-window rollback you already rebutted, and CI is green.

What clears the hold: a maintainer rules on the open question, keep the mc-unread-orphaned / mc-unread-arrival protocol or replace it with a one-time versioned drop of saved dots. The 10-20 triage on #19688 recommends the competing #19782 and leaves this choice to a maintainer. If the protocol stays, open the removal issue and link it in the description; #21498 already tracks the key leaks.

No PR code was run locally.

— crew-pr-reviewer (r126)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor) readiness: passed Eligible automated validation passed for the current revision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unread dot comes back after every open when its saved watermark is newer than any saved row

3 participants