diff --git a/ui/desktop/src/components/chatGroups/ChatGroupsShell.privacy.test.tsx b/ui/desktop/src/components/chatGroups/ChatGroupsShell.privacy.test.tsx index d3d675681..3d0755efe 100644 --- a/ui/desktop/src/components/chatGroups/ChatGroupsShell.privacy.test.tsx +++ b/ui/desktop/src/components/chatGroups/ChatGroupsShell.privacy.test.tsx @@ -28,6 +28,7 @@ import { describe, expect, it, vi, beforeEach } from 'vitest'; const dispatch = vi.fn(); const preloadSessionList = vi.fn(); let cachedList: Array<{ id: string; privacy_tier?: string }> | null = null; +let liveTiers: Record = {}; // The strip is not a child of the shell — it reaches the DOM through BaseChat's // `renderSessionTitle` render prop, so a BaseChat stub that ignores its props @@ -54,6 +55,14 @@ vi.mock('../../utils/sessionListCache', () => ({ preloadSessionList: () => preloadSessionList(), })); +// The LIVE half of the tier map: what the chat stores in this window hold right +// now. Stubbed here so this file stays about the MERGE — the registry's own +// side, a controller's row becoming an entry in that map, is pinned in +// `hooks/chatStreamStore.binding.test.tsx`. +vi.mock('../../hooks/chatStreamStore', () => ({ + useLiveSessionTiers: () => liveTiers, +})); + vi.mock('../../contexts/ChatGroupsContext', () => ({ useChatGroups: () => ({ dispatch, @@ -79,6 +88,7 @@ describe('ChatGroupsShell — the strip warms the list it reads', () => { beforeEach(() => { preloadSessionList.mockClear(); cachedList = null; + liveTiers = {}; lastStripProps = {}; }); @@ -102,3 +112,64 @@ describe('ChatGroupsShell — the strip warms the list it reads', () => { expect(lastStripProps.privacyTiers).toEqual({}); }); }); + +/** + * Finding M8, measured on merged main 2026-09-10: new chat tab, one turn on + * `versa_azure`, and 52.9 s later sqlite read `privacy_tier=private`, the model + * chip said "Private chat", the sidebar row said `data-privacy="private"` — and + * the ACTIVE TAB's own glyph still said `data-privacy="public"`. + * + * The cause is structural rather than a race: a chat CREATED in this window is + * not in the session list at all. `GET /sessions` INNER JOINs `messages`, so a + * row that has recorded none is not listable, and `refreshSessionBinding` + * patches only entries the cache already holds. Waiting longer never fixed it. + * + * The store, meanwhile, had the answer from the reply stream's first frames — + * which is why the chip and the header pill were right. These tests pin that + * the strip now reads the same source, and that the two sources combine in the + * safe direction. + */ +describe('ChatGroupsShell — the tab dot follows the live store, not just the list', () => { + it('marks a chat the store says is private but the list has never carried', () => { + cachedList = [{ id: 'someone-else', privacy_tier: 'public' }]; + liveTiers = { 'sess-1': 'private' }; + render( {}} />); + expect(lastStripProps.privacyTiers).toEqual({ + 'someone-else': 'public', + 'sess-1': 'private', + }); + }); + + it('marks it even when the list has not been fetched at all', () => { + cachedList = null; + liveTiers = { 'sess-1': 'private' }; + render( {}} />); + expect(lastStripProps.privacyTiers).toEqual({ 'sess-1': 'private' }); + }); + + it('lets the live store raise a chat the cache still calls public', () => { + // The ratchet fired during this turn; the cached list was fetched before it. + cachedList = [{ id: 'sess-1', privacy_tier: 'public' }]; + liveTiers = { 'sess-1': 'private' }; + render( {}} />); + expect(lastStripProps.privacyTiers).toEqual({ 'sess-1': 'private' }); + }); + + it('never lets a store that has not caught up LOWER a private the list knows', () => { + // The safe-direction invariant, stated as its own gate. The tier is a + // permanent ratchet server-side, so `private` from either source is a fact + // that still holds — and a surface may only render private-or-unmarked, + // never public over a source that has seen private. + cachedList = [{ id: 'sess-1', privacy_tier: 'private' }]; + liveTiers = { 'sess-1': 'public' }; + render( {}} />); + expect(lastStripProps.privacyTiers).toEqual({ 'sess-1': 'private' }); + }); + + it('still marks nothing when neither source has an opinion', () => { + cachedList = []; + liveTiers = {}; + render( {}} />); + expect(lastStripProps.privacyTiers).toEqual({}); + }); +}); diff --git a/ui/desktop/src/components/chatGroups/ChatGroupsShell.tsx b/ui/desktop/src/components/chatGroups/ChatGroupsShell.tsx index d796cccea..670703d33 100644 --- a/ui/desktop/src/components/chatGroups/ChatGroupsShell.tsx +++ b/ui/desktop/src/components/chatGroups/ChatGroupsShell.tsx @@ -31,6 +31,8 @@ import { preloadSessionList, subscribeSessionList, } from '../../utils/sessionListCache'; +import { useLiveSessionTiers } from '../../hooks/chatStreamStore'; +import { mergeSessionTiers, sessionTiersDiffer } from '../privacy/sessionTier'; import type { SessionClassification } from '../../api'; interface ChatGroupsShellProps { @@ -73,8 +75,51 @@ function renderLayout( /** * Privacy tier per session id, for the tab strips (issue #56, R10). * - * Reads the shared session-list cache and asks it to fill itself. The earlier - * version of this comment claimed AppSidebar warmed the cache "at module + * # Two sources, and the cache is the FALLBACK + * + * A tab whose chat has been opened in this window has a `ChatStreamController` + * holding the row, and that store is the same one the header pill and the + * composer read — `applyTurnBinding` patches the post-ratchet classification + * onto it from the reply stream's FIRST frames. That is the live source, and it + * is read here through `useLiveSessionTiers`. + * + * Only the ACTIVE tab of each pane mounts a `BaseChat`, so a tab never yet + * visited in this window has no store at all. Those get their tier from the + * shared session-list cache, exactly as every tab used to. + * + * # Why the cache alone was wrong (finding M8, measured 2026-09-10) + * + * A chat CREATED in this window is not in that cache and cannot be put there by + * announcing at create time: `GET /sessions` INNER JOINs `messages` + * (`SessionStorage::list_sessions_by_types_maybe_empty`), so a row that has + * recorded no message is not listable. `refreshSessionBinding` patches only an + * entry the cache already holds — deliberately, since list membership belongs + * to the list channel. So: new chat, one turn on a private model, sqlite + * `privacy_tier=private`, the sidebar row private (it reads a freshly-fetched + * list), the model chip private — and the active tab's own dot still + * `data-privacy="public"` 52.9 seconds later. + * + * The comment this replaces claimed the gap was closed "twice over". Both + * mechanisms it named are real and neither reaches this map: the reply stream's + * classification lands on the STORE, and `refreshSessionBinding`'s list patch is + * a no-op for a session the list has never carried. + * + * # The merge is `max`, not "freshest wins" + * + * The tier is a permanent ratchet server-side + * (`crates/biorouter/src/privacy/mod.rs`) — public → private, never back — so a + * `private` from ANY source is a fact that still holds, and a `public` is only + * a lower bound. {@link mergeSessionTiers} folds the two with `max` and + * `undefined` stays unmarked. The invariant, which + * `ChatGroupsShell.privacy.test.tsx` pins: this map may render private-from- + * either-source or unmarked, and can never render public over a source that has + * seen private. There is no failure mode in which it over-marks — no source + * here invents a tier, they only report a row. `ChatTabStrip`'s `privacyTiers` + * prop doc states the same thing, and the two must not drift apart again. + * + * # The cache still has to be warmed here + * + * An earlier version of this comment claimed AppSidebar warmed it "at module * scope"; it does not. `preloadSessionList` lives inside AppSidebar's * `preloadHome()`, wired to `onFocus`/`onPointerEnter` on the Home nav entry, * so it fires only if the user points at Home. What actually warmed the cache @@ -85,40 +130,22 @@ function renderLayout( * non-null and swallows its own errors, costing one fetch on a cold start. * * In jsdom, where the module is mocked or the fetch fails, the cache stays null - * and every tab is simply unmarked — silence, never an assertion of Public. + * and a tab with no store is simply unmarked — silence, never an assertion of + * Public. * * ⚠ What re-emits through `subscribeSessionList` is narrower than it looks. * Any `emitChange` reaches this hook — a completed `refreshSessionList`, the * name-channel patch, `updateCachedSessionList` (SessionListView's rename and - * delete), `clearSessionListCache`. But `notifySessionListChanged`, the signal - * whose own doc-comment says "call after create, diverge, delete or import", - * has exactly ONE production caller: `useDiverge.ts`. Create and import do not - * announce, so this cache learns of them only when some list surface mounts. - * - * ⚠ A stale cache fails in the UNSAFE direction, not the safe one. The tier is - * a permanent ratchet server-side (`crates/biorouter/src/privacy/mod.rs`) — it - * only ever rises public → private — so a cached `public`, or a session the - * cache has never seen, leaves a now-private chat with no dot. There is no - * failure mode in which this over-marks. `ChatTabStrip`'s `privacyTiers` prop - * doc states the same thing, and the two must not drift apart again. - * - * ⚠ This USED to record a known gap, and the gap is closed — the note is kept - * because the shape of the fix is what a future reader needs. It read: the - * header pill is not live either, `privacy_tier` is never re-read on the turn - * path, so a chat that ratchets to Private DURING its life shows no marker on - * either chat-side surface until something reloads it; "closing this needs the - * escalation to announce itself from the provider-bind path". - * - * It does now, twice over. The reply stream states the post-ratchet - * classification in its own first frames, and `ChatStreamController.refresh - * SessionBinding` patches the same four fields onto THIS cache as well as onto - * its own snapshot — so the tab dot, the header pill and the composer read one - * answer. A row rewritten by another process arrives the same way, through - * `utils/sessionMetaSubscription`. History rows and the sidebar rail, which read - * freshly-fetched lists, were always correct and are unchanged. + * delete), `clearSessionListCache`, `notifySessionListChanged`. That last one + * now has three production callers rather than one: `useDiverge`, the import + * handler in `SessionListView`, and `refreshSessionBinding` the first time a + * chat finds itself missing from a list that has been fetched — which is how a + * chat born in this window reaches Home recents and See-all. It is NOT how the + * tab dot gets fixed; the live store above is, and it costs no request. */ function useSessionPrivacyTiers(): Record { - const [tiers, setTiers] = useState>({}); + const [cachedTiers, setCachedTiers] = useState>({}); + const liveTiers = useLiveSessionTiers(); useEffect(() => { const read = () => { @@ -128,13 +155,7 @@ function useSessionPrivacyTiers(): Record { } // Identity-stable when nothing changed, so a list refresh that touched // no tier does not re-render every strip in every pane. - setTiers((prev) => { - const prevKeys = Object.keys(prev); - const same = - prevKeys.length === Object.keys(next).length && - prevKeys.every((id) => prev[id] === next[id]); - return same ? prev : next; - }); + setCachedTiers((prev) => (sessionTiersDiffer(prev, next) ? next : prev)); }; read(); // Subscribe BEFORE asking for the fetch. `preloadSessionList` is async but @@ -146,7 +167,10 @@ function useSessionPrivacyTiers(): Record { return unsubscribe; }, []); - return tiers; + // Memoised on the two inputs, both of which are identity-stable while + // unchanged: the merged object is a prop on every strip in every pane, so a + // fresh one per render would re-render all of them once per streamed token. + return useMemo(() => mergeSessionTiers(cachedTiers, liveTiers), [cachedTiers, liveTiers]); } export function ChatGroupsShell({ onChatChange }: ChatGroupsShellProps) { diff --git a/ui/desktop/src/components/chatGroups/ChatTabStrip.tsx b/ui/desktop/src/components/chatGroups/ChatTabStrip.tsx index a07ea2ad0..a76ec8227 100644 --- a/ui/desktop/src/components/chatGroups/ChatTabStrip.tsx +++ b/ui/desktop/src/components/chatGroups/ChatTabStrip.tsx @@ -99,8 +99,17 @@ export interface ChatTabStripProps { * `ChatTab` is persisted by chatGroupsStorage, and a tier persisted with a tab * is a tier that can be read back stale. The unsafe direction is the likely * one: the tier only ever RISES during a session, so a cached `public` would - * leave a now-private chat unmarked. A session-keyed map recomputed from the - * live session list has no such state to go stale. + * leave a now-private chat unmarked. A session-keyed map recomputed from + * sources with no state of their own has no such thing to go stale. + * + * ⚠ **Recomputed from TWO sources, folded with `max`** — the chat stores this + * window holds (live, and where a ratchet during a turn first appears) over + * the session-list cache (which covers tabs never opened here, and which + * cannot carry a chat created in this window until it has recorded a + * message). `ChatGroupsShell.useSessionPrivacyTiers` builds it, its doc gives + * the measurement, and the two must not drift apart again: reading the list + * cache ALONE is finding M8 — the active tab's dot stuck on public while the + * sidebar, the chip and sqlite all read private. * * Optional with a `{}` default, like `tabAnnotations`, so the four suites that * mount this strip bare keep compiling untouched. An unknown session is diff --git a/ui/desktop/src/components/chatGroups/keyboardResubmitGuard.test.tsx b/ui/desktop/src/components/chatGroups/keyboardResubmitGuard.test.tsx index b2eed99bd..cbf9f6ff0 100644 --- a/ui/desktop/src/components/chatGroups/keyboardResubmitGuard.test.tsx +++ b/ui/desktop/src/components/chatGroups/keyboardResubmitGuard.test.tsx @@ -88,6 +88,11 @@ vi.mock('../ui/sidebar', () => ({ vi.mock('../../hooks/chatStreamStore', () => ({ useRunningChats: () => [], defaultChatStreamRegistry: { peekController: () => undefined }, + // The shell reads the live per-chat privacy tiers from the registry (finding + // M8). Nothing here is about privacy, so an empty map is the whole stub — + // but it has to be PRESENT: this file replaces the module wholesale, and a + // missing export is a render-time throw, not an `undefined`. + useLiveSessionTiers: () => ({}), })); vi.mock('../../utils/sessionNameSync', () => ({ subscribeSessionNameChanges: () => () => undefined, diff --git a/ui/desktop/src/components/privacy/sessionTier.test.ts b/ui/desktop/src/components/privacy/sessionTier.test.ts new file mode 100644 index 000000000..ab0e89ec8 --- /dev/null +++ b/ui/desktop/src/components/privacy/sessionTier.test.ts @@ -0,0 +1,91 @@ +import { describe, expect, it } from 'vitest'; +import { mergeSessionTiers, raiseTier, sessionTiersDiffer } from './sessionTier'; + +/** + * The one rule every surface that draws a tier depends on: two readings of the + * same chat combine with `max`, never with "whichever is fresher". + * + * The safe direction is asserted directly rather than left as a property of the + * caller — a merge that let a stale `public` win is finding M8 in one line, and + * a merge that invented `private` for an unknown chat would be the same lie + * pointing the other way. + */ +describe('raiseTier — max over public < private', () => { + it('raises public to private', () => { + expect(raiseTier('public', 'private')).toBe('private'); + }); + + it('never lowers private back to public, whichever side the public is on', () => { + expect(raiseTier('private', 'public')).toBe('private'); + expect(raiseTier('public', 'private')).toBe('private'); + }); + + it('keeps private when the other side has no opinion', () => { + expect(raiseTier('private', undefined)).toBe('private'); + expect(raiseTier(undefined, 'private')).toBe('private'); + }); + + it('leaves an id nobody has read unmarked, rather than calling it public', () => { + expect(raiseTier(undefined, undefined)).toBeUndefined(); + }); + + it('keeps public when that is all either side has', () => { + expect(raiseTier('public', 'public')).toBe('public'); + expect(raiseTier(undefined, 'public')).toBe('public'); + }); +}); + +describe('mergeSessionTiers — folding a live map over a cached one', () => { + it('takes private from the LIVE source over a stale cached public — finding M8', () => { + const cached = { chat: 'public' } as const; + const live = { chat: 'private' } as const; + expect(mergeSessionTiers(cached, live)).toEqual({ chat: 'private' }); + }); + + it('takes private from the CACHED source when the live store has not seen it', () => { + // A tab never opened in this window has no store, so the list is the only + // source it has — and it must still be marked. + expect(mergeSessionTiers({ chat: 'private' }, {})).toEqual({ chat: 'private' }); + }); + + it('is order-independent, so neither source is privileged', () => { + const cached = { a: 'public', b: 'private' } as const; + const live = { a: 'private', b: 'public' } as const; + expect(mergeSessionTiers(cached, live)).toEqual(mergeSessionTiers(live, cached)); + expect(mergeSessionTiers(cached, live)).toEqual({ a: 'private', b: 'private' }); + }); + + it('omits an id no source has an opinion about', () => { + expect(mergeSessionTiers({}, {})).toEqual({}); + expect(mergeSessionTiers(undefined, null)).toEqual({}); + }); + + it('carries ids that appear in only one source', () => { + expect(mergeSessionTiers({ a: 'public' }, { b: 'private' })).toEqual({ + a: 'public', + b: 'private', + }); + }); +}); + +describe('sessionTiersDiffer — the identity-stability test', () => { + it('is false for equal maps, so nothing re-renders', () => { + expect(sessionTiersDiffer({ a: 'private' }, { a: 'private' })).toBe(false); + expect(sessionTiersDiffer({}, {})).toBe(false); + }); + + it('is true when a tier moved', () => { + expect(sessionTiersDiffer({ a: 'public' }, { a: 'private' })).toBe(true); + }); + + it('is true when the SET of ids changed, both ways', () => { + expect(sessionTiersDiffer({}, { a: 'public' })).toBe(true); + expect(sessionTiersDiffer({ a: 'public' }, {})).toBe(true); + }); + + it('is true for two same-sized maps that name different chats', () => { + // The cheap test — comparing lengths — passes here, so the per-key walk is + // what makes this correct rather than merely fast. + expect(sessionTiersDiffer({ a: 'private' }, { b: 'private' })).toBe(true); + }); +}); diff --git a/ui/desktop/src/components/privacy/sessionTier.ts b/ui/desktop/src/components/privacy/sessionTier.ts new file mode 100644 index 000000000..557e6adb1 --- /dev/null +++ b/ui/desktop/src/components/privacy/sessionTier.ts @@ -0,0 +1,87 @@ +import type { SessionClassification } from '../../api/types.gen'; + +/** + * Combining two readings of a chat's classification (issue #56, R10). + * + * # Why this is arithmetic and not a preference + * + * `SessionClassification` is a two-element lattice — `public < private` — and + * the daemon reduces it with `max` over the life of a session + * (`crates/biorouter/src/privacy/mod.rs`; CLAUDE.md calls it "a permanent + * ratchet"). It only ever rises. That single fact decides every question a + * caller could otherwise get wrong: + * + * - A reading of `private` can never become false. Whoever saw it saw a fact + * about the row that still holds, however old the reading is. + * - A reading of `public` can become false at any moment, and says nothing + * about now. It is a lower bound, not an answer. + * + * So two readings of the same chat are combined with `max`, never with + * "whichever is fresher". Freshness is not the ordering that matters here, and + * a merge that preferred the newer source would let a source which has not yet + * heard about a ratchet overwrite one that has. + * + * # The direction a mistake must fall in + * + * A chat that IS private and is drawn unmarked is the unsafe failure: the user + * is looking at a surface that quietly under-states what the chat holds. A chat + * drawn private when it is not would be merely wrong. `max` makes the first + * impossible from any source that has ever seen the truth, and the second + * impossible outright — because no source in this renderer invents `private`, + * they only ever report a row. + * + * ⚠ **`undefined` is not `public`.** An id no source has an opinion about stays + * absent from the result, and the glyph renders unmarked. Asserting Public for + * a chat nobody has read is the same lie in the other direction. + */ + +/** `max` over `public < private`; `undefined` is "no opinion", not `public`. */ +export function raiseTier( + current: SessionClassification | undefined, + incoming: SessionClassification | undefined +): SessionClassification | undefined { + if (current === 'private' || incoming === 'private') return 'private'; + if (current === 'public' || incoming === 'public') return 'public'; + return undefined; +} + +/** + * Fold any number of per-session tier maps into one, per id, with + * {@link raiseTier}. + * + * Written variadic on purpose: the tab strip merges a LIVE map (what the chat + * stores in this window hold right now) over a CACHED one (the session list), + * and the whole point is that neither is privileged — adding a third source + * must not need a new rule about which of the three wins. + */ +export function mergeSessionTiers( + ...sources: ReadonlyArray> | undefined | null> +): Record { + const merged: Record = {}; + for (const source of sources) { + if (!source) continue; + for (const [sessionId, tier] of Object.entries(source)) { + const raised = raiseTier(merged[sessionId], tier); + if (raised) merged[sessionId] = raised; + } + } + return merged; +} + +/** + * True when `next` says something `previous` does not — the test a caller uses + * to decide whether to publish a new map at all. + * + * Identity-stable output matters more here than it looks: these maps are read + * through `useSyncExternalStore` and passed as a prop to every tab strip in + * every pane, so a new object per notification re-renders all of them once per + * streamed token to discover that nothing moved (#22). + */ +export function sessionTiersDiffer( + previous: Readonly>, + next: Readonly> +): boolean { + const previousIds = Object.keys(previous); + if (previousIds.length !== Object.keys(next).length) return true; + return previousIds.some((id) => previous[id] !== next[id]); +} diff --git a/ui/desktop/src/components/sessions/SessionListView.tsx b/ui/desktop/src/components/sessions/SessionListView.tsx index 6247a5991..083e84ee6 100644 --- a/ui/desktop/src/components/sessions/SessionListView.tsx +++ b/ui/desktop/src/components/sessions/SessionListView.tsx @@ -59,6 +59,7 @@ import { ChatKindIcon } from '../chats/ChatKindIcon'; import { DeclassifySessionDialog } from './DeclassifySessionDialog'; import { getCachedSessionList, + notifySessionListChanged, refreshSessionList, subscribeSessionList, updateCachedSessionList, @@ -1016,6 +1017,20 @@ const SessionListView: React.FC = React.memo(({ onSelectSe title: 'Chat imported', msg: 'The imported chat is now available in chat history.', }); + // ⚠ Announce BEFORE the local reload, not instead of it and not after. + // An import is a membership change, and this pane is not the only surface + // that renders one: the sidebar Recents, Home and any second History pane + // in ANOTHER window all learn of it here, over the list channel. Doing it + // first also means `loadSessions` dedupes onto the request this starts + // (`refreshSessionList` returns the in-flight promise) rather than firing + // a second full list fetch behind it. + // + // An imported chat carries its transcript, so unlike a freshly CREATED + // one it is listable the moment it lands — `GET /sessions` INNER JOINs + // `messages`, which is why create announces from + // `ChatStreamController.refreshSessionBinding` after its first turn + // instead of from `createSession`. + notifySessionListChanged(); await loadSessions(); }, [loadSessions] diff --git a/ui/desktop/src/hooks/chatStreamStore.binding.test.tsx b/ui/desktop/src/hooks/chatStreamStore.binding.test.tsx index 03ab0c65d..5cd8ce1c1 100644 --- a/ui/desktop/src/hooks/chatStreamStore.binding.test.tsx +++ b/ui/desktop/src/hooks/chatStreamStore.binding.test.tsx @@ -57,8 +57,10 @@ vi.mock('../api', async (importOriginal) => { import { ChatStreamRegistry } from './chatStreamStore'; import { announceSessionBinding } from '../utils/sessionBindingSync'; import { + clearSessionListCache, getCachedSessionList, subscribeSessionList, + subscribeSessionListChanges, updateCachedSessionList, } from '../utils/sessionListCache'; @@ -428,6 +430,78 @@ describe('a turn states what it runs on, from its first frames', () => { expect(controller.getSnapshot().pinnedModel?.provider).toBe('versa_azure'); }); + /** + * ⚠ **The frame can land BEFORE the row does, and the tier used to be lost.** + * + * Finding M8's second half, and the ordering is the app's own. A chat CREATED + * by a submit is ATTACHED to — `ChatGroupsContext` observes the turn that is + * already running — while `loadSession` is still fetching the row, and + * `loadSession` blanks `session` for the duration of that fetch. The pin + * survived the window because it has a home of its own; the classification's + * only home is the row, so the updater returned `prev` and the fact was + * dropped. Every chat-side surface then said `public` until the post-turn + * re-read: measured in the running app on session `20260910_4` — snapshot + * `norow` at t+559 ms with the turn already streaming, row lands `public` at + * t+590, and does not move until t+4327, 28 ms after the turn ENDED. + * + * `pinnedModel` is the proof the frame really arrived: it is the half of the + * same frame that was never lost. + */ + it('keeps a classification the observer reported before the row loaded', async () => { + const sid = `turn-start-norow-${++sessionSeq}`; + // Disarm the post-turn re-read, so a `private` row can only be the frame's. + mocks.getSession.mockResolvedValue({ data: null }); + mocks.observeSessionEvents.mockResolvedValue({ stream: streamOf(pinFrame()) }); + mocks.resumeAgent.mockResolvedValue({ data: { session: boundSession(sid) } }); + + const controller = new ChatStreamRegistry().getController(sid); + void controller.observeSession(); + await vi.waitFor(() => + expect(controller.getSnapshot().pinnedModel?.model).toBe('gpt-5.5-2026-04-24') + ); + controller.stopObserving(); + // The frame has been consumed and there is still no row — nothing invented + // in its place. + expect(controller.getSnapshot().session).toBeUndefined(); + + await controller.loadSession(); + + const row = controller.getSnapshot().session; + expect(row?.privacy_tier).toBe('private'); + expect(row?.privacy_reason).toBe('turn:versa_azure'); + }); + + /** + * ⚠ One-shot. A stash that outlived the row it was waiting for could + * re-assert a turn's statement over a row read later — after a + * declassification, the one legitimate private → public move — which is the + * unsafe direction wearing the safe one's clothes. + */ + it('applies a pre-row classification once, and lets the next row overrule it', async () => { + const sid = `turn-start-norow-once-${++sessionSeq}`; + mocks.getSession.mockResolvedValue({ data: null }); + mocks.observeSessionEvents.mockResolvedValue({ stream: streamOf(pinFrame()) }); + mocks.resumeAgent.mockResolvedValue({ data: { session: boundSession(sid) } }); + + const controller = new ChatStreamRegistry().getController(sid); + void controller.observeSession(); + await vi.waitFor(() => + expect(controller.getSnapshot().pinnedModel?.model).toBe('gpt-5.5-2026-04-24') + ); + controller.stopObserving(); + await controller.loadSession(); + expect(controller.getSnapshot().session?.privacy_tier).toBe('private'); + + // A row read AFTERWARDS says public — a declassification — and must stand. + mocks.getSession.mockResolvedValue({ + data: boundSession(sid, { privacy_tier: 'public', privacy_reason: null }), + }); + await controller.refreshSessionBinding(); + + expect(controller.getSnapshot().session?.privacy_tier).toBe('public'); + expect(controller.getSnapshot().session?.privacy_reason).toBeNull(); + }); + /** * ⚠ The regression the widening would otherwise cause, pinned. * @@ -690,3 +764,213 @@ describe('a stale row read cannot overwrite a newer fact', () => { }); }); }); + +/** + * Finding M8 — the chat-tab dot never turned private for a chat created in this + * window, while the sidebar row for the SAME chat did. + * + * The dot read the session-list cache and nothing else. That cache cannot hold + * a chat born in this window: `GET /sessions` INNER JOINs `messages`, so a row + * with none is not listable, and the patch above deliberately touches only an + * entry the cache already holds. The store knew all along — `applyTurnBinding` + * writes the post-ratchet classification from the reply stream's first frames — + * so the fix is to publish what the stores hold rather than to fetch anything. + */ +describe('the registry publishes what its stores say a chat is', () => { + /** + * Notification is batched to one animation frame (#22), and this rides that + * same batch on purpose: the header pill, the composer and the tab dot then + * move together rather than one of the three arriving a frame early. So the + * wait here is a frame, not a network round trip. + */ + const aFrame = () => new Promise((resolve) => setTimeout(resolve, 60)); + + it('reports a private chat that no session list has ever carried', async () => { + const sid = `tier-live-${++sessionSeq}`; + mocks.resumeAgent.mockResolvedValue({ + data: { session: boundSession(sid, { privacy_tier: 'private' }) }, + }); + + const registry = new ChatStreamRegistry(); + expect(registry.getSessionTiersSnapshot()).toEqual({}); + + await registry.getController(sid).loadSession(); + await aFrame(); + + expect(registry.getSessionTiersSnapshot()).toEqual({ [sid]: 'private' }); + }); + + it('follows the ratchet a turn reports, without re-reading the row', async () => { + const sid = `tier-ratchet-${++sessionSeq}`; + mocks.resumeAgent.mockResolvedValue({ data: { session: boundSession(sid) } }); + mocks.reply.mockResolvedValue({ + stream: streamOf( + { + type: 'PrivacyProviderPinned', + provider: 'versa_azure', + model: 'gpt-5.5-2026-04-24', + privacy_tier: 'private', + privacy_reason: 'turn:versa_azure', + } as unknown as MessageEvent, + finishFrame + ), + }); + + const registry = new ChatStreamRegistry(); + const controller = registry.getController(sid); + await controller.loadSession(); + await aFrame(); + expect(registry.getSessionTiersSnapshot()).toEqual({ [sid]: 'public' }); + + await controller.handleSubmit('hi'); + await vi.waitFor(async () => { + await aFrame(); + expect(registry.getSessionTiersSnapshot()).toEqual({ [sid]: 'private' }); + }); + }); + + it('emits to its subscribers only when a tier actually moved', async () => { + const sid = `tier-quiet-${++sessionSeq}`; + mocks.resumeAgent.mockResolvedValue({ + data: { session: boundSession(sid, { privacy_tier: 'private' }) }, + }); + mocks.getSession.mockResolvedValue({ data: boundSession(sid, { privacy_tier: 'private' }) }); + mocks.reply.mockResolvedValue({ stream: streamOf(finishFrame) }); + + const registry = new ChatStreamRegistry(); + const controller = registry.getController(sid); + await controller.loadSession(); + await aFrame(); + + let emits = 0; + const unsubscribe = registry.subscribeSessionTiers(() => { + emits += 1; + }); + const before = registry.getSessionTiersSnapshot(); + + // A whole turn, on a chat whose tier does not move. + await controller.handleSubmit('hi'); + await vi.waitFor(() => expect(mocks.getSession).toHaveBeenCalled()); + await aFrame(); + unsubscribe(); + + expect(emits).toBe(0); + // Identity-stable, so `useSyncExternalStore` re-renders no strip. + expect(registry.getSessionTiersSnapshot()).toBe(before); + }); + + /** + * ⚠ The unsafe direction, and the reason this map is a ratchet of its own. + * A controller can momentarily hold no row — a reload, a rebind — and if the + * published map followed it down, the strip would fall back to whatever the + * list cache last said, which for a chat that just went private is `public`. + * Once private, private. + */ + it('never retracts a private it has already reported', async () => { + const sid = `tier-noretract-${++sessionSeq}`; + mocks.resumeAgent.mockResolvedValue({ + data: { session: boundSession(sid, { privacy_tier: 'private' }) }, + }); + + const registry = new ChatStreamRegistry(); + const controller = registry.getController(sid); + await controller.loadSession(); + await aFrame(); + expect(registry.getSessionTiersSnapshot()).toEqual({ [sid]: 'private' }); + + // A binding announcement is the cheapest real path that writes the snapshot + // without carrying a tier of its own. + announceSessionBinding({ sessionId: sid, provider: 'ollama', model: 'qwen3.6' }); + await aFrame(); + + expect(registry.getSessionTiersSnapshot()).toEqual({ [sid]: 'private' }); + }); +}); + +/** + * The OTHER half of M8, and deliberately not the one that fixes the dot. + * + * Home recents and the See-all view read the session-list cache, and a chat + * created in this window never enters it: `notifySessionListChanged` had exactly + * one production caller (`useDiverge`), so nothing announced a create. Announcing + * from `createSession` cannot work either — the row has no message yet and the + * list endpoint's INNER JOIN omits it — so the announcement is made at the first + * moment the daemon WILL list the chat, which is after its first turn. + */ +describe('a chat born in this window tells the list it exists', () => { + beforeEach(() => { + clearSessionListCache(); + }); + + it('announces once, when a fetched list turns out not to hold it', async () => { + const sid = `bind-announce-${++sessionSeq}`; + // A cache that has been fetched, and does not hold this chat. + updateCachedSessionList([boundSession(`${sid}-neighbour`)]); + let announcements = 0; + const unsubscribe = subscribeSessionListChanges(() => { + announcements += 1; + }); + + mocks.getSession.mockResolvedValue({ data: boundSession(sid, { privacy_tier: 'private' }) }); + mocks.resumeAgent.mockResolvedValue({ data: { session: boundSession(sid) } }); + mocks.reply.mockResolvedValue({ stream: streamOf(finishFrame) }); + + const controller = new ChatStreamRegistry().getController(sid); + await controller.loadSession(); + await controller.handleSubmit('hi'); + await vi.waitFor(() => expect(announcements).toBe(1)); + + // A second turn must not buy another full list fetch. + await controller.handleSubmit('again'); + await vi.waitFor(() => expect(mocks.getSession.mock.calls.length).toBeGreaterThan(1)); + unsubscribe(); + expect(announcements).toBe(1); + }); + + it('stays quiet for a chat the list already holds', async () => { + const sid = `bind-noannounce-${++sessionSeq}`; + updateCachedSessionList([boundSession(sid)]); + let announcements = 0; + const unsubscribe = subscribeSessionListChanges(() => { + announcements += 1; + }); + + mocks.getSession.mockResolvedValue({ data: boundSession(sid, { privacy_tier: 'private' }) }); + mocks.resumeAgent.mockResolvedValue({ data: { session: boundSession(sid) } }); + mocks.reply.mockResolvedValue({ stream: streamOf(finishFrame) }); + + const controller = new ChatStreamRegistry().getController(sid); + await controller.loadSession(); + await controller.handleSubmit('hi'); + await vi.waitFor(() => expect(mocks.getSession).toHaveBeenCalled()); + unsubscribe(); + + expect(announcements).toBe(0); + }); + + /** + * ⚠ A null cache is "nobody has asked yet", not "the chat is missing". + * `ChatGroupsShell` warms it on mount; answering an unfetched cache with a + * full list fetch per turn-end would buy a page of session rows to learn one + * string, on every chat in the window. + */ + it('stays quiet when no list has been fetched at all', async () => { + const sid = `bind-nolist-${++sessionSeq}`; + let announcements = 0; + const unsubscribe = subscribeSessionListChanges(() => { + announcements += 1; + }); + + mocks.getSession.mockResolvedValue({ data: boundSession(sid, { privacy_tier: 'private' }) }); + mocks.resumeAgent.mockResolvedValue({ data: { session: boundSession(sid) } }); + mocks.reply.mockResolvedValue({ stream: streamOf(finishFrame) }); + + const controller = new ChatStreamRegistry().getController(sid); + await controller.loadSession(); + await controller.handleSubmit('hi'); + await vi.waitFor(() => expect(mocks.getSession).toHaveBeenCalled()); + unsubscribe(); + + expect(announcements).toBe(0); + }); +}); diff --git a/ui/desktop/src/hooks/chatStreamStore.tsx b/ui/desktop/src/hooks/chatStreamStore.tsx index 41cedcd3d..20c084a8e 100644 --- a/ui/desktop/src/hooks/chatStreamStore.tsx +++ b/ui/desktop/src/hooks/chatStreamStore.tsx @@ -27,8 +27,13 @@ import { subscribeSessionNameChanges, } from '../utils/sessionNameSync'; import { subscribeSessionBindingChanges } from '../utils/sessionBindingSync'; -import { getCachedSessionList, updateCachedSessionList } from '../utils/sessionListCache'; +import { + getCachedSessionList, + notifySessionListChanged, + updateCachedSessionList, +} from '../utils/sessionListCache'; import { subscribeToSessionMeta } from '../utils/sessionMetaSubscription'; +import { raiseTier } from '../components/privacy/sessionTier'; import { createElicitationResponseMessage, createUserMessage, @@ -813,6 +818,26 @@ class ChatStreamController { private seqTurnId: string | null = null; private lastInteractionTime = Date.now(); private loadPromise: Promise | null = null; + /** + * This chat has already told the session-list cache it exists. + * + * Latched, never cleared: a chat that a fetched list still does not hold + * after being announced is one the list endpoint filters out on purpose (a + * `sub_agent` row under `include_subagents=false`), and re-announcing it once + * per turn forever would buy a full list fetch to be told the same thing. + */ + private announcedListMembership = false; + /** + * A classification this turn reported while the chat still had no row. + * + * Applied by {@link updateSnapshot} to the first snapshot that carries one, + * and cleared there — a one-shot, so it can never re-assert itself over a + * later row. See the note in {@link applyTurnBinding} for the race. + */ + private pendingTurnClassification: { + tier: SessionClassification; + reason: string | null; + } | null = null; /** * R3-01 — synchronous re-entrancy latch for the submit prep window. The * `abortController` guard in `canSubmitMessage` only trips once a turn has @@ -1311,7 +1336,7 @@ class ChatStreamController { } private updateSnapshot(updater: (prev: ChatStreamSnapshot) => ChatStreamSnapshot): void { - const next = updater(this.snapshot); + const next = this.adoptPendingClassification(updater(this.snapshot)); // Several updaters return `prev` to mean "nothing changed" — that must not // wake every subscriber (#22). if (next === this.snapshot) return; @@ -1332,6 +1357,40 @@ class ChatStreamController { this.scheduleNotify(); } + /** + * Put a turn's classification onto the first row that appears, if the frame + * that carried it arrived before there was one (finding M8). + * + * ⚠ **Here, at the one write every snapshot goes through**, rather than at + * each of the several updaters that can set a row — the cached-transcript + * path, the two-phase resume, a diverge. A fix wired to one of them is a fix + * for one road into the same defect. + * + * ⚠ **One-shot.** The stash is cleared the moment a row is seen, whether or + * not it needed changing, so a turn's statement can never re-assert itself + * over a row loaded later — after a declassification, say. Everything after + * that comes from the row itself: the next frame, or the post-turn re-read. + */ + private adoptPendingClassification(candidate: ChatStreamSnapshot): ChatStreamSnapshot { + const pending = this.pendingTurnClassification; + if (!pending || !candidate.session) return candidate; + this.pendingTurnClassification = null; + if ( + candidate.session.privacy_tier === pending.tier && + (candidate.session.privacy_reason ?? null) === pending.reason + ) { + return candidate; + } + return { + ...candidate, + session: { + ...candidate.session, + privacy_tier: pending.tier, + privacy_reason: pending.reason, + }, + }; + } + // `receivedAt` is stamped ONLY by the live stream path. The other callers // (session load, diverge, edit) deliberately omit it: replaying a saved // transcript must never look like a live event, or a historical session @@ -1547,6 +1606,26 @@ class ChatStreamController { if (event.privacy_tier === undefined) return; const tier = event.privacy_tier; const reason = event.privacy_reason ?? null; + // ⚠ **A chat created in this window has no row yet when this frame lands, + // and the classification used to be thrown away** (finding M8). The frame + // is emitted at the top of the turn; for a chat whose session was created + // by the submit that started that turn, `loadSession` is still in flight. + // The pin survived that race because it has a home of its own; the tier's + // only home is the row, so the updater below returned `prev` and the fact + // was lost — the chat then showed public on every chat-side surface until + // the post-turn re-read, four seconds later. Measured 2026-09-10 on session + // `20260910_4`: snapshot `norow` at t+559 ms, row lands `public` at t+590, + // and stays public until t+4327 — 28 ms after the turn ENDED. + // + // So the fact is kept until a row exists to carry it. It is ADOPTED rather + // than ratcheted on arrival, for the same reason this method adopts: the + // frame is the daemon's post-ratchet statement about this turn, and a + // declassified chat (DR-20) is a legitimate private → public move that the + // frame is the first to report. + if (!this.snapshot.session) { + this.pendingTurnClassification = { tier, reason }; + return; + } this.updateSnapshot((prev) => { if (!prev.session) return prev; if (prev.session.privacy_tier === tier && (prev.session.privacy_reason ?? null) === reason) { @@ -2045,6 +2124,27 @@ class ChatStreamController { const cached = getCachedSessionList(); const index = cached?.findIndex((entry) => entry.id === this.sessionId) ?? -1; const entry = index === -1 ? undefined : cached![index]; + // ⚠ A chat CREATED in this window is absent from that list, and announcing + // at `createSession` cannot fix it: `GET /sessions` INNER JOINs `messages` + // (`SessionStorage::list_sessions_by_types_maybe_empty`), so a row with no + // message yet is not listable and a refresh fired at create time comes + // back without it. The first moment the daemon WILL list the chat is after + // its first turn — which is here. So the announcement is made from here + // instead, and it is a refetch rather than an insert for the reason the + // note above gives: membership is the list channel's to own. + // + // ⚠ Once per chat per renderer, and only against a cache that has + // actually been fetched. A null cache means nobody has asked yet + // (`ChatGroupsShell` warms it on mount), and answering that with a full + // list fetch per turn-end would be a page of session rows bought to learn + // one string. The tab dot does not wait for any of this — it reads the + // live store tier through `ChatStreamRegistry.subscribeSessionTiers` — + // which is why this can afford to be the slow, correct path for the + // OTHER list surfaces (Home recents, See-all) rather than the fix for M8. + if (cached !== null && index === -1 && !this.announcedListMembership) { + this.announcedListMembership = true; + notifySessionListChanged(); + } const listDiffers = entry != null && (entry.provider_name !== row.provider_name || @@ -4144,6 +4244,8 @@ export class ChatStreamRegistry { private running = new Map(); private lastRunningSnapshot: RunningChatEntry[] = []; private stopSessionMeta: (() => void) | null = null; + private tierListeners = new Set<() => void>(); + private sessionTiers: Record = {}; /** * Follow session rows this renderer holds, for changes made by ANOTHER @@ -4222,15 +4324,76 @@ export class ChatStreamRegistry { getRunningSnapshot = (): RunningChatEntry[] => this.lastRunningSnapshot; + /** + * The classification of every chat this window holds a STORE for — a live + * reading, not a cached one (issue #56, R10; finding M8). + * + * # What this exists to fix + * + * The chat-tab dot used to read only the session-list cache, and a chat + * CREATED in this window is not in that cache: `GET /sessions` INNER JOINs + * `messages` (`SessionStorage::list_sessions_by_types_maybe_empty`), so a + * brand-new row is not listable until it has recorded one, and + * `refreshSessionBinding` deliberately patches only entries the cache already + * holds. Measured on 2026-09-10: one turn on a new chat, sqlite + * `privacy_tier=private`, the sidebar row and the model chip both private — + * and the ACTIVE TAB's own dot still `data-privacy="public"` 52.9 s later. + * + * The store already knew. `applyTurnBinding` patches the post-ratchet tier + * onto the snapshot from the reply stream's FIRST frames, which is where the + * header pill and the composer read it. This channel publishes that same + * reading to the strip, so the three surfaces cannot disagree — and it needs + * no fetch, because the answer was already in the window. + * + * ⚠ **This map only ever RISES**, mirroring the daemon's own ratchet + * (`privacy::raise`). A controller whose session momentarily goes null — a + * reload, a rebind — must not retract a `private` it has already reported, or + * the strip would fall back to a cached `public` and un-mark a private chat. + * {@link raiseTier} is the whole rule. + * + * ⚠ **O(1) per notification.** `handleControllerActivity` runs on every + * snapshot notification, which during a turn is once per animation frame per + * chat; this compares ONE id's tier and returns, and allocates a new map only + * when a tier actually moved. + */ + subscribeSessionTiers = (listener: () => void): (() => void) => { + this.tierListeners.add(listener); + return () => { + this.tierListeners.delete(listener); + }; + }; + + getSessionTiersSnapshot = (): Record => this.sessionTiers; + resetForTests(): void { this.controllers.clear(); this.running.clear(); this.lastRunningSnapshot = []; + this.sessionTiers = {}; + this.tierListeners.clear(); this.stopSessionMeta?.(); this.stopSessionMeta = null; } + private noteControllerTier(controller: ChatStreamController): void { + const reported = controller.getSnapshot().session?.privacy_tier ?? undefined; + const current = this.sessionTiers[controller.sessionId]; + const raised = raiseTier(current, reported); + if (raised === current) return; + this.sessionTiers = { ...this.sessionTiers }; + if (raised) this.sessionTiers[controller.sessionId] = raised; + else delete this.sessionTiers[controller.sessionId]; + for (const listener of this.tierListeners) listener(); + } + private handleControllerActivity = (controller: ChatStreamController): void => { + // ⚠ FIRST, and outside every early return below. The running-list + // bookkeeping that follows returns without emitting for an idle controller + // with no live entry — which is exactly the shape of a session LOAD, and of + // the `refreshSessionBinding` that runs after a turn has already ended. + // Both carry a tier, and both would be dropped by a tier read placed after + // that guard. + this.noteControllerTier(controller); const current = this.running.get(controller.sessionId); if (controller.isRunning()) { const entry = controller.getRunningEntry(); @@ -4311,3 +4474,20 @@ export function useChatStreamController(sessionId: string): ChatStreamController const registry = useChatStreamRegistry(); return registry.getController(sessionId); } + +/** + * The classification of every chat this window holds a store for, live. + * + * See {@link ChatStreamRegistry.subscribeSessionTiers}. A tab whose chat has + * never been opened in this window is simply absent — the caller merges this + * over the session-list cache with `mergeSessionTiers`, which is where a chat + * nobody has opened gets its tier from. + */ +export function useLiveSessionTiers(): Record { + const registry = useChatStreamRegistry(); + return useSyncExternalStore( + registry.subscribeSessionTiers, + registry.getSessionTiersSnapshot, + registry.getSessionTiersSnapshot + ); +}