feat(server): sync thread read status across clients - #5652
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
ApprovabilityVerdict: Needs human review This PR adds a new feature for syncing thread read/unread status across clients via the server. It includes a database migration, new orchestration commands and events, and changes how read state is tracked (from client-local to server-synchronized). The scope and nature of these changes warrant human review. No code changes detected at You can customize Macroscope's approvability policy. Learn more. |
9b9ebb8 to
92bcd1e
Compare
92bcd1e to
1ec6fd1
Compare
1ec6fd1 to
fededda
Compare
d3a0393 to
6f0466d
Compare
6f0466d to
4a75259
Compare
4a75259 to
f3e1ea0
Compare
f3e1ea0 to
f08fd94
Compare
|
Terrible when compared to #5652 |
|
Wrong pr my bad, too many tabs lol |
f08fd94 to
0125c6f
Compare
| threadId: ThreadId, | ||
| }); | ||
|
|
||
| const ThreadMarkViewedCommand = Schema.Struct({ |
There was a problem hiding this comment.
🟡 Medium src/orchestration.ts:674
ThreadMarkViewedCommand.viewedAt is documented as a server-owned boundary copied from thread completion/wake state, but the schema accepts any client-supplied IsoDateTime. Because the decider persists this value directly (only comparing it against the previous one), a client can submit a far-future timestamp and make all subsequent completions and wakes appear already read. The server should derive or validate this boundary against actual thread state rather than trusting the client value.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/contracts/src/orchestration.ts around line 674:
`ThreadMarkViewedCommand.viewedAt` is documented as a server-owned boundary copied from thread completion/wake state, but the schema accepts any client-supplied `IsoDateTime`. Because the decider persists this value directly (only comparing it against the previous one), a client can submit a far-future timestamp and make all subsequent completions and wakes appear already read. The server should derive or validate this boundary against actual thread state rather than trusting the client value.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0125c6f. Configure here.
| .run(registry, { | ||
| environmentId: target.environmentId, | ||
| input: { ...input, expectedLastViewedAt, supersededViewedAt }, | ||
| }) |
There was a problem hiding this comment.
Stale view token rejects open-after-unread
Medium Severity
markViewed snapshots expectedLastViewedAt from the shell before the per-thread serial queue runs. If mark-unread is ahead in that queue or has landed on the server before the shell refreshes, the following open-thread mark-viewed still sends the pre-unread token, fails the decider invariant, and leaves the thread unread while the user is viewing it.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 0125c6f. Configure here.


Thread read and unread state lived in web local storage, so browsers and devices could disagree about which completed threads had been seen.
This moves
lastViewedAtinto the event-sourced server projection and adds server-owned mark-viewed and mark-unread commands. Web and mobile now update the shared state, older servers are capability-gated, and view updates do not touchupdatedAtor reorder threads. Existing threads remain read by default after migration.Verified with focused decider, projection, persistence, command, reducer, sidebar, and UI-state tests (177 passing), plus scoped typechecks for contracts, client-runtime, server, web, and mobile.
Built with GPT-5.6-sol via the Codex harness in T3 Code.
Note
Medium Risk
Touches orchestration commands, DB migration backfill, and cross-client read semantics; migration timestamps could briefly skew unread indicators until users view threads again.
Overview
Thread read/unread is now server-owned instead of per-browser
threadLastVisitedAtByIdinuiStateStore. Threads carrylastViewedAtthrough contracts, projection (projection_threads+ migration 039), snapshot queries, and client reducers; view updates emitthread.view-status-updatedwithout bumpingupdatedAt.New orchestration commands
thread.mark-viewedandthread.mark-unread(with optimistic concurrency on viewed boundaries) are wired through client-runtime atoms (markViewed/markUnread) and advertised via thethreadViewStatuscapability on the server descriptor.Web and mobile call
markThreadViewedwhen opening a thread (and on wake/settle flows where applicable); sidebars, wake banners, and context menus uselastViewedAtandhasUnseenWake, and hide mark-unread when the server lacks support. Existing threads are backfilled on migrate so upgrades do not flood unread state.Reviewed by Cursor Bugbot for commit 0125c6f. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Sync thread read/unread status across clients via server-owned view state
threadLastVisitedAtByIdtimestamps inuiStateStorewith server-persistedlastViewedAt, tracked via newthread.mark-viewedandthread.mark-unreadorchestration commands.threadViewStatuscapability to the server descriptor; clients check this capability before issuing view-state mutations, so older servers are unaffected.thread.mark-viewed(with optimistic concurrency viaexpectedLastViewedAt/supersededViewedAt) andthread.mark-unread(setslastViewedAtto 1ms before the latest turn completion), emittingthread.view-status-updatedevents.last_viewed_attoprojection_threadsand backfills existing rows with the migration timestamp.thread.lastViewedAtand are hidden when the server lacksthreadViewStatussupport.last_viewed_at, so threads that were previously read may briefly reappear as unread after the migration runs.Macroscope summarized 0125c6f.