feat: sync thread unread state across clients - #6662

💭 I'm reviewing the changed web files for UI consistency, noting the diff mainly touches state/hook logic rather than visual components. The environment-scoped actions like markViewed/markUnread properly pass an explicit environmentId, so that rule checks out fine, and now I'm looking at the capability check in ChatView.tsx. I need to verify whether the environmentId used for the config value atom and for supportsThreadViewState actually refers to the thread's own environment rather than whatever environment is currently active.

1. git_tools — environmentId — rejected: repo.url: required parameter is missing or empty;repo: required parameter is missing or empty

2. browse_code — apps/web/src/hooks/useThreadViewState.ts

💭 Environment routing looks explicit here. I want to check ChatView.tsx to see where its environmentId comes from for the config value atom, and confirm Sidebar.tsx's markUnread call also uses an explicit environmentId source.

3. git_tools — const environmentId

4. git_tools — supportsThreadViewState|acknowledgedVisibleThreadRef|markViewed|serverThread?.environmentId

💭 I need to trace where environmentId is defined for supportsThreadViewState in ChatView, since the acknowledger uses serverThread.environmentId for the threadRef while supportsThreadViewState derives from a possibly different environmentId — if they diverge, that's an environment routing mismatch where capability gating and RPC dispatch read from different environments.

5. git_tools — environmentId =|environmentId,$|serverThread =

💭 I'm confirming that serverThread's environmentId matches the route environment used for capability lookup, so the sync is consistent. This diff is really about state-sync logic rather than UI styling or primitives, so it likely falls outside the scope of this review's concerns. Checking LegacySidebar next — verifying that selectedThreadEntries actually exposes a threadRef property and that threadRef is properly scoped where markUnread is called, since that's a compile-correctness question rather than a UI-consistency one.

6. git_tools — selectedThreadEntries|markUnread(threadRef

💭 Since this check only concerns state wiring in the web changes rather than Tailwind or primitives, I'm weighing whether the repeated resolveThreadViewedAt logic across Sidebar, LegacySidebar, ThreadStatusIndicators, and ChatView counts as a relevant consistency issue, though it's more of a code duplication concern than a component-system one. I want to double-check whether the mapped array selector in LegacySidebar's SidebarProjectItem could cause re-render loops if it's not using shallow comparison, so I should look at how it's implemented around line 1220-1250.

7. browse_code — apps/web/src/components/LegacySidebar.tsx

💭 I'm checking the Sidebar.tsx change wrapping the markThreadUnread call in a thread existence check — since that function already no-ops with an undefined completedAt, this shouldn't alter behavior. Now I'm scanning for other places that might still reference the unread computation logic.

8. git_tools — threadLastVisitedAtById

💭 I'm wondering whether other surfaces like Sidebar.logic.ts, ThreadList, notifications, or the command palette still compute unread state purely from local visit data rather than the new server viewedAt, which could create inconsistencies in the Done indicator. I should grep more broadly for lastVisitedAt usages to check.

9. git_tools — lastVisitedAt|LastVisitedAt

💭 I'm checking that all web consumers of threadLastVisitedAtById—Sidebar, LegacySidebar, ThreadStatusIndicators, ChatView—have been migrated, though there may be other surfaces reading unread state via hasUnseenCompletion independently, which isn't relevant here. The mark-viewed and mark-unread hooks look properly migrated too, and the mobile route screen falls outside this diff's scope. I'm noting a minor redundant selector call in ThreadStatusIndicators but nothing that constitutes a real UI consistency issue, so I'm now verifying whether ChatView's environment-scoped config lookup could mismatch if the rendered thread belongs to a different environment than the one passed in. Tracing through, the route thread ref is built from the same environmentId prop, so the server thread's environmentId matches consistently -- no bug there. With no genuine findings surfacing anywhere in this diff, I'm concluding the check should report success, formatted according to the required JSON schema rather than a plain "All clear" string, since the schema requirement takes precedence.