diff --git a/desktop/src/app/App.tsx b/desktop/src/app/App.tsx index 0f311f3a650..4850a6934e7 100644 --- a/desktop/src/app/App.tsx +++ b/desktop/src/app/App.tsx @@ -59,6 +59,7 @@ import { WelcomeSetup } from "@/features/communities/ui/WelcomeSetup"; import { CommunityApplyErrorScreen } from "@/features/communities/ui/CommunityApplyErrorScreen"; import { CommunityChangeOverlay } from "@/features/communities/ui/CommunityChangeOverlay"; import { setAvatarProfileSyncQueryClient } from "@/features/profile/avatarProfileSync"; +import { useBoundedMessageWindows } from "@/features/messages/useBoundedMessageWindows"; import { EncryptedBackupProvider } from "@/features/settings/EncryptedBackupProvider"; import { createBuzzQueryClient } from "@/shared/api/queryClient"; import { isSharedIdentity as isSharedIdentityCmd } from "@/shared/api/tauri"; @@ -213,6 +214,8 @@ function CommunitySwitchGate() { function CommunityQueryProvider({ children }: { children: ReactNode }) { const [queryClient] = useState(createBuzzQueryClient); + useBoundedMessageWindows(queryClient); + useEffect(() => setAvatarProfileSyncQueryClient(queryClient), [queryClient]); useEffect(() => { diff --git a/desktop/src/features/agents/channelActivitySummary.ts b/desktop/src/features/agents/channelActivitySummary.ts new file mode 100644 index 00000000000..a63c6e5ce1e --- /dev/null +++ b/desktop/src/features/agents/channelActivitySummary.ts @@ -0,0 +1,70 @@ +import type { ObserverEvent } from "./ui/agentSessionTypes"; +import { normalizePubkey } from "@/shared/lib/pubkey"; + +// Per-agent channel-activity summary: normalized pubkey → (channelId → latest +// activity ms). One number per distinct channel the agent has been seen active +// in, recorded from every live event as it arrives — so it is NOT subject to +// the UNPINNED_AGENT_EVENT_TAIL cap that bounds `eventsByAgent`. It exists to +// keep the profile activity feed's channel scope intact after an unpinned +// agent's event window is truncated to its newest tail: the tail retains the +// newest events (so the *preferred* channel is safe), but a channel whose last +// event fell out of the tail would vanish from the derived channel set without +// this durable summary. Compact by construction — bounded by the (small) count +// of distinct channels one agent works in, not by event volume. +const channelActivityByAgent = new Map>(); + +const EMPTY_CHANNEL_ACTIVITY: Record = {}; + +/** + * Record one live event into the per-agent channel-activity summary. Keeps the + * latest activity ms per channel so the profile feed's channel scope survives + * the tail-truncation of `eventsByAgent`. A channelId-less event (agent-general + * frame) carries no channel scope and is skipped; a frame with an unparseable + * timestamp cannot advance a channel's recency and is skipped. `key` is already + * a normalized pubkey (the caller normalized it). + */ +export function recordChannelActivity(key: string, event: ObserverEvent): void { + if (!event.channelId) { + return; + } + const millis = Date.parse(event.timestamp); + if (Number.isNaN(millis)) { + return; + } + let byChannel = channelActivityByAgent.get(key); + if (!byChannel) { + byChannel = new Map(); + channelActivityByAgent.set(key, byChannel); + } + const previous = byChannel.get(event.channelId); + if (previous === undefined || millis > previous) { + byChannel.set(event.channelId, millis); + } +} + +/** + * Latest observed activity ms per channel for one agent, from the durable + * channel-activity summary. Unlike `getAgentObserverSnapshot().events`, this is + * NOT truncated to the unpinned tail — it retains one entry per distinct + * channel the agent has been active in for the store's lifetime. The profile + * activity feed folds this into its channel scope so an unpinned agent's older + * channels do not vanish from the switcher after its event window is trimmed. + * Returns a shared empty object (stable identity) for an unknown agent. + */ +export function getAgentChannelActivity( + agentPubkey?: string | null, +): Record { + if (!agentPubkey) { + return EMPTY_CHANNEL_ACTIVITY; + } + const byChannel = channelActivityByAgent.get(normalizePubkey(agentPubkey)); + if (!byChannel || byChannel.size === 0) { + return EMPTY_CHANNEL_ACTIVITY; + } + return Object.fromEntries(byChannel); +} + +/** Clear the summary; called from resetAgentObserverStore. */ +export function clearChannelActivity(): void { + channelActivityByAgent.clear(); +} diff --git a/desktop/src/features/agents/ingestArchivedObserverEvents.test.mjs b/desktop/src/features/agents/ingestArchivedObserverEvents.test.mjs index 343ce241335..69766bd8f84 100644 --- a/desktop/src/features/agents/ingestArchivedObserverEvents.test.mjs +++ b/desktop/src/features/agents/ingestArchivedObserverEvents.test.mjs @@ -1187,3 +1187,134 @@ describe("raw-event-level merge: stateful aggregates across live/archive boundar assert.equal(archived[0].seq, 31); }); }); + +describe("ingestArchivedObserverEvents — atomic commit under staleness gate", () => { + beforeEach(() => { + resetAgentObserverStore(); + _testRegisterKnownAgents(SUB_ID, [AGENT_PUBKEY]); + }); + + /** + * The commit is atomic against a channel switch or panel unmount that lands + * WHILE the page is decrypting. Phase 1 (decrypt into staging) is the only + * async work; phase 2 evaluates `isStale()` once, with no await before the + * appends. Here the caller's gate flips to stale after the page's decrypt is + * released — modeling the user switching channels (A→B, or A→B→A) or the + * panel unmounting mid-decrypt — so the whole page must be dropped and the + * archive must stay empty. Without the gate, a late page resurrects an + * evicted/switched-away channel as most-recently-loaded. + */ + it("test_page_going_stale_during_decrypt_commits_nothing", async () => { + let releaseDecrypt; + const decryptGate = new Promise((resolve) => { + releaseDecrypt = resolve; + }); + let stale = false; + + const ingest = ingestArchivedObserverEvents( + [makeRawEvent(), makeRawEvent({ id: "f".repeat(64) })], + async () => { + await decryptGate; + return makeObserverEvent({ seq: 1, channelId: "chan-1" }); + }, + () => stale, + ); + + // The channel is switched away (or the panel unmounts) while decrypt is in + // flight: the caller's gate now reads stale. + stale = true; + releaseDecrypt(); + await ingest; + + assert.deepEqual( + _testGetArchivedChannelEvents(AGENT_PUBKEY, "chan-1"), + [], + "a page that went stale mid-decrypt must commit nothing", + ); + }); + + /** + * The complement: a page whose gate stays current commits every staged + * frame. Proves the gate is not simply dropping all writes — the same + * multi-frame page that would be dropped when stale lands whole when live. + */ + it("test_page_staying_current_commits_the_whole_page", async () => { + const events = [ + makeObserverEvent({ seq: 1, timestamp: "2026-01-01T00:00:01.000Z" }), + makeObserverEvent({ seq: 2, timestamp: "2026-01-01T00:00:02.000Z" }), + ]; + let idx = 0; + + await ingestArchivedObserverEvents( + [makeRawEvent(), makeRawEvent({ id: "f".repeat(64) })], + () => Promise.resolve(events[idx++]), + () => false, + ); + + const archived = _testGetArchivedChannelEvents(AGENT_PUBKEY, "chan-1"); + assert.equal( + archived.length, + 2, + "a current page must commit all its staged frames", + ); + assert.deepEqual( + archived.map((event) => event.seq).sort(), + [1, 2], + "both frames of the current page are present", + ); + }); + + /** + * The gate is evaluated ONCE, before the first append, against the state at + * commit time — not per frame. A page that was current when it started + * decrypting but is evaluated as stale at commit is dropped WHOLE: the store + * can never hold a torn half-page (frame 1 committed, frame 2 dropped), + * which would leave a resurrected channel with a partial timeline. + */ + it("test_stale_gate_drops_the_whole_page_never_a_partial", async () => { + let released = 0; + const gates = [0, 1].map(() => { + let release; + const promise = new Promise((resolve) => { + release = resolve; + }); + return { promise, release }; + }); + let stale = false; + + const ingest = ingestArchivedObserverEvents( + [makeRawEvent(), makeRawEvent({ id: "f".repeat(64) })], + async () => { + const gate = gates[released++]; + await gate.promise; + return makeObserverEvent({ seq: released, channelId: "chan-1" }); + }, + () => stale, + ); + + // Release the first frame's decrypt and drain microtasks until the ingest + // loop has fully consumed frame 1 (staged it) and re-entered _decryptFn for + // frame 2 — released reaches 2 only after frame 1's decrypt resolved and the + // loop resumed. This is the discriminating setup: frame 1 was current at the + // moment it was decrypted and staged. A torn design that commits frames per + // their decrypt-time currentness would keep frame 1. + gates[0].release(); + while (released < 2) { + await Promise.resolve(); + } + + // The switch lands now — after frame 1 is staged-while-current, before + // frame 2 finishes. The single phase-2 gate is evaluated after ALL decrypts, + // so the whole page — including the frame that was current when staged — + // must be dropped. + stale = true; + gates[1].release(); + await ingest; + + assert.deepEqual( + _testGetArchivedChannelEvents(AGENT_PUBKEY, "chan-1"), + [], + "no torn half-page: a stale gate at commit drops every frame, including ones decrypted while the channel was still current", + ); + }); +}); diff --git a/desktop/src/features/agents/lib/observerArchiveEviction.test.mjs b/desktop/src/features/agents/lib/observerArchiveEviction.test.mjs new file mode 100644 index 00000000000..c08fb94a259 --- /dev/null +++ b/desktop/src/features/agents/lib/observerArchiveEviction.test.mjs @@ -0,0 +1,148 @@ +/** + * Unit tests for selectArchiveEvictionKeys — the pure LRU-with-pins policy that + * bounds the channel-scoped observer archive. + * + * The policy evicts at channel granularity (all agent entries for a channel go + * together) while respecting pins (a pinned channel is never evicted). These + * tests exercise the selection logic directly, with no React/Tauri runtime. + */ + +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; + +import { selectArchiveEvictionKeys } from "./observerArchiveEviction.ts"; + +/** Build one archive entry meta. */ +function meta(agent, channelId, accessSeq) { + return { key: `${agent}:${channelId}`, channelId, accessSeq }; +} + +const NO_PINS = new Set(); + +describe("selectArchiveEvictionKeys", () => { + it("test_empty_entries_returns_no_eviction", () => { + assert.deepEqual(selectArchiveEvictionKeys([], NO_PINS, 8), []); + }); + + it("test_channel_count_below_cap_returns_no_eviction", () => { + const entries = [ + meta("agentA", "chan-1", 1), + meta("agentA", "chan-2", 2), + meta("agentA", "chan-3", 3), + ]; + assert.deepEqual(selectArchiveEvictionKeys(entries, NO_PINS, 8), []); + }); + + it("test_channel_count_exactly_at_cap_returns_no_eviction", () => { + const entries = Array.from({ length: 8 }, (_, i) => + meta("agentA", `chan-${i}`, i), + ); + assert.deepEqual(selectArchiveEvictionKeys(entries, NO_PINS, 8), []); + }); + + it("test_over_cap_evicts_least_recently_accessed_channels", () => { + // 10 channels, cap 8 → evict the 2 lowest-recency (chan-0, chan-1). + const entries = Array.from({ length: 10 }, (_, i) => + meta("agentA", `chan-${i}`, i), + ); + const evicted = selectArchiveEvictionKeys(entries, NO_PINS, 8); + assert.deepEqual(evicted.sort(), ["agentA:chan-0", "agentA:chan-1"].sort()); + }); + + it("test_channel_recency_is_max_access_across_its_agent_entries", () => { + // chan-old has two agent entries; its most-recent access (seq 100) keeps it + // alive even though its other entry (seq 0) is the global minimum. + const entries = [ + meta("agentA", "chan-old", 0), + meta("agentB", "chan-old", 100), + meta("agentA", "chan-1", 1), + meta("agentA", "chan-2", 2), + ]; + // cap 2 → keep the 2 most-recent channels (chan-old@100, chan-2@2), + // evict chan-1@1. + const evicted = selectArchiveEvictionKeys(entries, NO_PINS, 2); + assert.deepEqual(evicted, ["agentA:chan-1"]); + }); + + it("test_evicting_a_channel_removes_all_its_agent_entries", () => { + // chan-evict has 3 agent entries; all three keys must be returned together. + const entries = [ + meta("agentA", "chan-evict", 0), + meta("agentB", "chan-evict", 0), + meta("agentC", "chan-evict", 0), + meta("agentA", "chan-keep", 9), + ]; + const evicted = selectArchiveEvictionKeys(entries, NO_PINS, 1); + assert.deepEqual( + evicted.sort(), + ["agentA:chan-evict", "agentB:chan-evict", "agentC:chan-evict"].sort(), + ); + }); + + it("test_pinned_channel_is_never_evicted_even_when_least_recent", () => { + // chan-pinned is the least-recently accessed but pinned → must survive; + // chan-1 (next lowest) is evicted instead. + const entries = [ + meta("agentA", "chan-pinned", 0), + meta("agentA", "chan-1", 1), + meta("agentA", "chan-2", 2), + meta("agentA", "chan-3", 3), + ]; + const evicted = selectArchiveEvictionKeys( + entries, + new Set(["chan-pinned"]), + 3, + ); + assert.deepEqual(evicted, ["agentA:chan-1"]); + }); + + it("test_pins_exceeding_cap_evict_all_unpinned_but_keep_all_pinned", () => { + // 3 pinned + 2 unpinned, cap 2. Pins alone exceed the cap, so the unpinned + // budget is 0 → both unpinned channels evicted, all pinned retained. + const entries = [ + meta("agentA", "pin-1", 10), + meta("agentA", "pin-2", 11), + meta("agentA", "pin-3", 12), + meta("agentA", "free-1", 1), + meta("agentA", "free-2", 2), + ]; + const evicted = selectArchiveEvictionKeys( + entries, + new Set(["pin-1", "pin-2", "pin-3"]), + 2, + ); + assert.deepEqual(evicted.sort(), ["agentA:free-1", "agentA:free-2"].sort()); + }); + + it("test_pinned_channels_count_against_the_cap_for_unpinned_budget", () => { + // 1 pinned + 3 unpinned, cap 2 → unpinned budget = 2 - 1 = 1. Keep the + // single most-recent unpinned channel (free-3), evict free-1 and free-2. + const entries = [ + meta("agentA", "pin-1", 5), + meta("agentA", "free-1", 1), + meta("agentA", "free-2", 2), + meta("agentA", "free-3", 3), + ]; + const evicted = selectArchiveEvictionKeys(entries, new Set(["pin-1"]), 2); + assert.deepEqual(evicted.sort(), ["agentA:free-1", "agentA:free-2"].sort()); + }); + + it("test_tie_broken_deterministically_by_channel_id", () => { + // Two channels share recency 1 at the eviction boundary. The keep-sort + // breaks ties by ascending channelId, so "chan-a" is retained and the + // higher "chan-b" is evicted — deterministic regardless of input order. + const entries = [ + meta("agentA", "chan-b", 1), + meta("agentA", "chan-a", 1), + meta("agentA", "chan-keep", 9), + ]; + const evicted = selectArchiveEvictionKeys(entries, NO_PINS, 2); + assert.deepEqual(evicted, ["agentA:chan-b"]); + }); + + it("test_zero_cap_evicts_all_unpinned_channels", () => { + const entries = [meta("agentA", "chan-1", 1), meta("agentA", "chan-2", 2)]; + const evicted = selectArchiveEvictionKeys(entries, NO_PINS, 0); + assert.deepEqual(evicted.sort(), ["agentA:chan-1", "agentA:chan-2"].sort()); + }); +}); diff --git a/desktop/src/features/agents/lib/observerArchiveEviction.ts b/desktop/src/features/agents/lib/observerArchiveEviction.ts new file mode 100644 index 00000000000..d38b1761715 --- /dev/null +++ b/desktop/src/features/agents/lib/observerArchiveEviction.ts @@ -0,0 +1,91 @@ +/** + * Pure LRU-with-pins eviction policy for the channel-scoped observer archive. + * + * The archive store (`observerRelayStore`) keys paged history by + * `${normalizedAgentPubkey}:${channelId}`, so a single channel observed for + * several agents occupies several map entries. Memory pressure, however, scales + * with the number of distinct *channels* whose scroll-back is retained — the + * user opens channels, not (agent, channel) pairs. This module therefore evicts + * at channel granularity: when the count of distinct retained channels exceeds + * `cap`, the least-recently-accessed unpinned channels are dropped whole (every + * agent entry for that channel). Eviction is not data loss — the next visit + * re-hydrates the channel from SQLite through the normal paging path. + * + * Pins are inviolable: a pinned channel is never selected for eviction, even + * when pins alone exceed `cap`. A pin marks a channel whose panel is mounted and + * whose eviction would blank a live view, so the cap is enforced only against + * unpinned channels. + * + * Pure and React/Tauri-free so the selection logic is unit-testable in + * isolation, mirroring the `archivePagingState` extraction. + */ + +export interface ArchiveEntryMeta { + /** Composite map key `${normalizedAgentPubkey}:${channelId}`. */ + key: string; + /** Channel the entry belongs to; the unit of eviction. */ + channelId: string; + /** Monotonic access counter; higher = more recently read or written. */ + accessSeq: number; +} + +/** + * Select the composite map keys to evict so that no more than `cap` distinct + * unpinned channels are retained. Returns the keys for every entry belonging to + * an evicted channel; pinned channels never appear in the result. + * + * @param entries One meta per archive map entry (composite key granularity). + * @param pinnedChannels Channel ids whose entries must be retained regardless of cap. + * @param cap Maximum distinct unpinned channels to keep. + */ +export function selectArchiveEvictionKeys( + entries: readonly ArchiveEntryMeta[], + pinnedChannels: ReadonlySet, + cap: number, +): string[] { + // Recency of each channel = the most recent access across all its entries, + // so touching any agent's view of a channel refreshes the whole channel. + const channelRecency = new Map(); + for (const entry of entries) { + const prev = channelRecency.get(entry.channelId); + if (prev === undefined || entry.accessSeq > prev) { + channelRecency.set(entry.channelId, entry.accessSeq); + } + } + + // Only unpinned channels are eviction candidates; pins claim retained slots. + const unpinned: Array<{ channelId: string; recency: number }> = []; + let pinnedPresent = 0; + for (const [channelId, recency] of channelRecency) { + if (pinnedChannels.has(channelId)) { + pinnedPresent += 1; + } else { + unpinned.push({ channelId, recency }); + } + } + + const unpinnedBudget = Math.max(0, cap - pinnedPresent); + if (unpinned.length <= unpinnedBudget) { + return []; + } + + // Keep the most-recent `unpinnedBudget` channels; evict the rest. Sort by + // recency descending, breaking ties by channelId so selection is deterministic. + unpinned.sort( + (a, b) => + b.recency - a.recency || + (a.channelId < b.channelId ? -1 : a.channelId > b.channelId ? 1 : 0), + ); + const evictChannels = new Set( + unpinned.slice(unpinnedBudget).map((channel) => channel.channelId), + ); + + // Expand evicted channels back to every composite map key they cover. + const keys: string[] = []; + for (const entry of entries) { + if (evictChannels.has(entry.channelId)) { + keys.push(entry.key); + } + } + return keys; +} diff --git a/desktop/src/features/agents/observerArchiveEvictionStore.test.mjs b/desktop/src/features/agents/observerArchiveEvictionStore.test.mjs new file mode 100644 index 00000000000..bd485c6e586 --- /dev/null +++ b/desktop/src/features/agents/observerArchiveEvictionStore.test.mjs @@ -0,0 +1,260 @@ +/** + * Store-level eviction tests for the channel-scoped observer archive. + * + * These exercise the REAL ingest path (ingestArchivedObserverEvents with an + * injected decrypt fn) plus the public pin API, asserting the LRU bound and its + * invariants at the store boundary — the layer above the pure policy unit: + * + * - the retained-channel cap is enforced across ingest pages; + * - eviction is least-recently-loaded first; + * - a pinned channel is never evicted (even when least recent); + * - evicting a channel drops every (agent, channel) entry it owns; + * - an evicted channel reads back empty (cache miss → re-hydrate on revisit); + * - resetAgentObserverStore clears the LRU bookkeeping so counts don't leak. + * + * No React/Tauri runtime: security guards are satisfied via _testRegisterKnownAgents + * and decrypt is injected. The cap (MAX_RETAINED_ARCHIVE_CHANNELS = 8) is a + * private constant; tests drive enough distinct channels to cross it and assert + * behaviour relative to that threshold rather than importing the value. + */ + +import assert from "node:assert/strict"; +import { beforeEach, describe, it } from "node:test"; + +import { + ingestArchivedObserverEvents, + resetAgentObserverStore, + pinArchiveChannel, + unpinArchiveChannel, + _testRegisterKnownAgents, + _testGetArchivedChannelEvents, +} from "@/features/agents/observerRelayStore.ts"; + +const CAP = 8; // MAX_RETAINED_ARCHIVE_CHANNELS (private constant, mirrored here) +const SUB_ID = "test-eviction-sub"; + +/** 64-hex agent pubkey from a short label. */ +function agentPubkey(label) { + return label.padEnd(64, "0"); +} + +const AGENT_A = agentPubkey("a"); +const AGENT_B = agentPubkey("b"); + +/** Raw kind-24200 telemetry row for (agent, channel); content carries the event. */ +function makeRawEvent(agent, channelId, seq) { + return { + id: `${seq}`.padStart(64, "e"), + pubkey: agent, + created_at: 1000 + seq, + kind: 24200, + tags: [ + ["p", "d".repeat(64)], + ["agent", agent], + ["frame", "telemetry"], + ], + content: JSON.stringify({ + seq, + timestamp: new Date(1_000_000 + seq * 1000).toISOString(), + kind: "acp_write", + agentIndex: 0, + channelId, + sessionId: "sess-1", + turnId: "turn-1", + payload: {}, + }), + sig: "s".repeat(128), + }; +} + +/** Decrypt fn: content is the JSON-encoded ObserverEvent. */ +const decrypt = (event) => Promise.resolve(JSON.parse(event.content)); + +/** Ingest one telemetry frame for (agent, channel), awaiting the store write. */ +async function ingestOne(agent, channelId, seq) { + await ingestArchivedObserverEvents( + [makeRawEvent(agent, channelId, seq)], + decrypt, + ); +} + +describe("observer archive eviction — store level", () => { + beforeEach(() => { + resetAgentObserverStore(); + _testRegisterKnownAgents(SUB_ID, [AGENT_A, AGENT_B]); + }); + + it("test_channels_up_to_cap_are_all_retained", async () => { + for (let i = 0; i < CAP; i++) { + await ingestOne(AGENT_A, `chan-${i}`, i); + } + for (let i = 0; i < CAP; i++) { + assert.equal( + _testGetArchivedChannelEvents(AGENT_A, `chan-${i}`).length, + 1, + `chan-${i} must be retained at exactly cap`, + ); + } + }); + + it("test_exceeding_cap_evicts_least_recently_loaded_channel", async () => { + // Load CAP+1 distinct channels in ascending order. chan-0 is the oldest + // load and unpinned, so it is the one evicted when the cap is crossed. + for (let i = 0; i <= CAP; i++) { + await ingestOne(AGENT_A, `chan-${i}`, i); + } + assert.equal( + _testGetArchivedChannelEvents(AGENT_A, "chan-0").length, + 0, + "least-recently-loaded channel must be evicted", + ); + // The newest CAP channels survive. + for (let i = 1; i <= CAP; i++) { + assert.equal( + _testGetArchivedChannelEvents(AGENT_A, `chan-${i}`).length, + 1, + `chan-${i} must survive`, + ); + } + }); + + it("test_pinned_channel_survives_even_when_least_recently_loaded", async () => { + // chan-0 is loaded first (oldest) but pinned; loading CAP more channels + // crosses the cap. The eviction must fall on the oldest UNPINNED channel + // (chan-1), never the pinned chan-0. + await ingestOne(AGENT_A, "chan-0", 0); + pinArchiveChannel("chan-0"); + for (let i = 1; i <= CAP; i++) { + await ingestOne(AGENT_A, `chan-${i}`, i); + } + assert.equal( + _testGetArchivedChannelEvents(AGENT_A, "chan-0").length, + 1, + "pinned channel must never be evicted", + ); + assert.equal( + _testGetArchivedChannelEvents(AGENT_A, "chan-1").length, + 0, + "oldest unpinned channel must be evicted instead of the pinned one", + ); + }); + + it("test_unpinning_makes_channel_evictable_again", async () => { + // Pin protects chan-0 across a first over-cap wave; after unpinning, a + // second wave that again exceeds the cap must be free to evict chan-0. + await ingestOne(AGENT_A, "chan-0", 0); + pinArchiveChannel("chan-0"); + for (let i = 1; i <= CAP; i++) { + await ingestOne(AGENT_A, `chan-${i}`, i); + } + assert.equal( + _testGetArchivedChannelEvents(AGENT_A, "chan-0").length, + 1, + "pinned channel survives the first wave", + ); + + unpinArchiveChannel("chan-0"); + // chan-0 is now the least-recently-loaded unpinned channel. Load fresh + // channels to push the retained set past the cap again. + for (let i = CAP + 1; i <= CAP * 2; i++) { + await ingestOne(AGENT_A, `chan-${i}`, i); + } + assert.equal( + _testGetArchivedChannelEvents(AGENT_A, "chan-0").length, + 0, + "unpinned channel becomes evictable and is dropped", + ); + }); + + it("test_refcounted_pin_holds_until_last_release", async () => { + // Two mounted consumers pin chan-0; one unpins. The channel must remain + // protected until the SECOND unpin — refcount, not a boolean flag. + await ingestOne(AGENT_A, "chan-0", 0); + pinArchiveChannel("chan-0"); + pinArchiveChannel("chan-0"); + unpinArchiveChannel("chan-0"); + + for (let i = 1; i <= CAP; i++) { + await ingestOne(AGENT_A, `chan-${i}`, i); + } + assert.equal( + _testGetArchivedChannelEvents(AGENT_A, "chan-0").length, + 1, + "channel stays pinned while one consumer still holds a pin", + ); + + unpinArchiveChannel("chan-0"); + for (let i = CAP + 1; i <= CAP * 2; i++) { + await ingestOne(AGENT_A, `chan-${i}`, i); + } + assert.equal( + _testGetArchivedChannelEvents(AGENT_A, "chan-0").length, + 0, + "channel is evictable only after the last pin is released", + ); + }); + + it("test_eviction_drops_all_agent_entries_for_the_channel", async () => { + // chan-shared is populated for two agents (fan-out: one channel view + // populates multiple (agent, channel) keys). Both keys must be evicted + // together when the channel is dropped. + await ingestOne(AGENT_A, "chan-shared", 0); + await ingestOne(AGENT_B, "chan-shared", 1); + // Push past the cap with unrelated channels; chan-shared is oldest. + for (let i = 0; i < CAP; i++) { + await ingestOne(AGENT_A, `chan-other-${i}`, 100 + i); + } + assert.equal( + _testGetArchivedChannelEvents(AGENT_A, "chan-shared").length, + 0, + "agent-A entry for the evicted channel must be gone", + ); + assert.equal( + _testGetArchivedChannelEvents(AGENT_B, "chan-shared").length, + 0, + "agent-B entry for the same evicted channel must be gone too", + ); + }); + + it("test_evicted_channel_reads_back_empty_for_rehydration", async () => { + // The revisit contract: an evicted channel returns an empty window, which + // the paging hook treats as a cache miss and re-hydrates from SQLite. Here + // we assert the empty read; re-ingesting the same page restores it. + for (let i = 0; i <= CAP; i++) { + await ingestOne(AGENT_A, `chan-${i}`, i); + } + assert.equal( + _testGetArchivedChannelEvents(AGENT_A, "chan-0").length, + 0, + "evicted channel reads back empty", + ); + // Re-hydrate: re-ingesting restores the window (proves eviction is not a + // permanent tombstone — the key is fully reusable). + await ingestOne(AGENT_A, "chan-0", 0); + assert.equal( + _testGetArchivedChannelEvents(AGENT_A, "chan-0").length, + 1, + "re-ingest after eviction restores the channel window", + ); + }); + + it("test_reset_clears_pin_state_so_counts_do_not_leak", async () => { + // A pin outstanding at reset must not survive it — otherwise a phantom pin + // would protect a channel id in the next session. After reset we re-pin + // NOTHING and confirm the same channel id is freely evictable. + await ingestOne(AGENT_A, "chan-0", 0); + pinArchiveChannel("chan-0"); + resetAgentObserverStore(); + _testRegisterKnownAgents(SUB_ID, [AGENT_A, AGENT_B]); + + // If reset leaked the pin, chan-0 would survive this over-cap wave. + for (let i = 0; i <= CAP; i++) { + await ingestOne(AGENT_A, `chan-${i}`, i); + } + assert.equal( + _testGetArchivedChannelEvents(AGENT_A, "chan-0").length, + 0, + "pin state must not survive reset — chan-0 is freely evictable again", + ); + }); +}); diff --git a/desktop/src/features/agents/observerEventOrdering.ts b/desktop/src/features/agents/observerEventOrdering.ts new file mode 100644 index 00000000000..85ff7bf02b1 --- /dev/null +++ b/desktop/src/features/agents/observerEventOrdering.ts @@ -0,0 +1,66 @@ +import type { ObserverEvent } from "./ui/agentSessionTypes"; + +/** + * Total order over observer events: ascending timestamp, then ascending seq on + * a same-millisecond tie. Non-finite timestamps fall through to a pure seq + * comparison. Used to keep the archive window and transcript in one order. + */ +export function compareObserverEvents( + left: ObserverEvent, + right: ObserverEvent, +): number { + const leftTime = Date.parse(left.timestamp); + const rightTime = Date.parse(right.timestamp); + if (Number.isFinite(leftTime) && Number.isFinite(rightTime)) { + const timeDiff = leftTime - rightTime; + if (timeDiff !== 0) { + return timeDiff; + } + } + + return left.seq - right.seq; +} + +/** + * Returns true if `candidate` sorts strictly after `stored` using the same + * two-key ordering as `compareObserverEvents`: later timestamp wins; equal + * timestamp falls back to higher seq. Extracted so latest-live advancement + * cannot drift from transcript ordering. + */ +export function isObserverEventAfter( + candidate: { timestamp: string; seq: number }, + stored: { timestamp: string; seq: number }, +): boolean { + const candidateTime = Date.parse(candidate.timestamp); + const storedTime = Date.parse(stored.timestamp); + if (Number.isFinite(candidateTime) && Number.isFinite(storedTime)) { + if (candidateTime !== storedTime) { + return candidateTime > storedTime; + } + } + return candidate.seq > stored.seq; +} + +// Observer event kind for a batch envelope wrapping multiple events. The ACP +// harness publishes one frame per second; everything that accumulated between +// ticks arrives as `{ kind: "batch", payload: { events: [...] } }` with every +// inner event carrying its own seq/timestamp. Inner events are processed +// exactly as unbatched ones; the envelope itself is never stored. +const OBSERVER_BATCH_KIND = "batch"; + +/** + * Expand a decrypted observer event into its inner events when it is a batch + * envelope; a non-batch event passes through as a single-element array. A + * malformed envelope (no events array) degrades to the envelope itself so a + * harness bug cannot silently blank the session viewer. + */ +export function unwrapObserverBatch(parsed: ObserverEvent): ObserverEvent[] { + if (parsed.kind !== OBSERVER_BATCH_KIND) { + return [parsed]; + } + const payload = parsed.payload as { events?: unknown } | null; + const events = Array.isArray(payload?.events) + ? (payload.events as ObserverEvent[]) + : null; + return events && events.length > 0 ? events : [parsed]; +} diff --git a/desktop/src/features/agents/observerRelayStore.ts b/desktop/src/features/agents/observerRelayStore.ts index 611fdd489d4..c687cce016a 100644 --- a/desktop/src/features/agents/observerRelayStore.ts +++ b/desktop/src/features/agents/observerRelayStore.ts @@ -13,6 +13,20 @@ import { } from "./agentManagement"; import { normalizePubkey } from "@/shared/lib/pubkey"; import { useQueryClient } from "@tanstack/react-query"; +import { + clearChannelActivity, + getAgentChannelActivity, + recordChannelActivity, +} from "./channelActivitySummary"; +import { + compareObserverEvents, + isObserverEventAfter, + unwrapObserverBatch, +} from "./observerEventOrdering"; +// Re-export the channel-activity read and the event-ordering helpers off the +// observer store's public surface; their storage/logic now lives in dedicated +// modules but callers keep importing them from here. +export { compareObserverEvents, getAgentChannelActivity, isObserverEventAfter }; import { agentConfigSurfaceQueryKey } from "@/features/agents/hooks"; import type { ConnectionState, @@ -25,10 +39,30 @@ import { createEmptyTranscriptState, processTranscriptEvent, } from "./ui/agentSessionTranscript"; +import { selectArchiveEvictionKeys } from "./lib/observerArchiveEviction"; const MAX_OBSERVER_EVENTS = 3000; const MAX_PENDING_UNKNOWN_AGENT_FRAMES = 100; +// Live-event window retained in RAM for an agent that has NO mounted session +// viewer. The full MAX_OBSERVER_EVENTS window is kept only while a viewer is +// pinned (see pinObserverAgent); an unpinned agent keeps just the newest tail. +// A long-lived session observes many agents over time and each unpinned agent +// would otherwise hold its full window forever — the accumulator this bounds. +// The tail must stay large enough that the non-viewer consumers keep working: +// preventSleep reads only the newest event, the active-turns bridge processes +// each event once as it arrives (turn state then lives in its own store), and +// the profile activity feed derives a channel set that degrades to the tail. +const UNPINNED_AGENT_EVENT_TAIL = 100; + +// Maximum number of distinct channels whose archive scroll-back is retained in +// RAM. The archive window grows only by explicit paged loads from SQLite, so +// without a bound a session that visits many channels accumulates their full +// history for the lifetime of the app. Eviction drops the least-recently-loaded +// unpinned channels whole; a revisit re-hydrates from SQLite (cache miss, not +// data loss). Channels with a mounted panel are pinned and never evicted. +const MAX_RETAINED_ARCHIVE_CHANNELS = 8; + export type ObserverSnapshot = { connectionState: ConnectionState; errorMessage: string | null; @@ -58,6 +92,131 @@ const snapshotByAgent = new Map(); // TranscriptState once over the combined window. const archiveEventsByChannel = new Map(); +// LRU bookkeeping for `archiveEventsByChannel`. `archiveChannelId` maps a +// composite key back to its channelId (the eviction unit) so the pure policy +// can group agent entries by channel. `archiveAccessSeq` records the value of +// `archiveAccessCounter` at each key's most recent WRITE (a paged load into the +// archive), giving a total order for least-recently-LOADED selection. This is +// deliberately a least-recently-*loaded* policy, not least-recently-accessed: +// reads (`getArchivedChannelEvents`) never touch the sequence, because the read +// hook pins the channel it reads (`useObserverEvents` calls `pinArchiveChannel` +// on the same mount), so a channel being read is always pinned and so never an +// eviction candidate — a read has no reachable state where advancing recency +// would change the outcome. +const archiveChannelId = new Map(); +const archiveAccessSeq = new Map(); +let archiveAccessCounter = 0; + +// Channels whose archive must never be evicted because a panel is mounted on +// them. Refcounted so co-mounted panels (e.g. the channel screen and the +// profile panel viewing the same channel) compose: the channel stays pinned +// until the last panel unmounts. A channel with a positive count is pinned. +const pinnedArchiveChannelCounts = new Map(); + +/** + * Pin a channel's archive against eviction while a panel is mounted on it. + * Refcounted so multiple mounted consumers of the same channel compose; the + * channel is protected until every consumer calls `unpinArchiveChannel`. + * No-op for a null/empty channelId (panels open before a channel resolves). + */ +export function pinArchiveChannel(channelId: string | null | undefined): void { + if (!channelId) return; + pinnedArchiveChannelCounts.set( + channelId, + (pinnedArchiveChannelCounts.get(channelId) ?? 0) + 1, + ); +} + +/** + * Release one pin acquired via `pinArchiveChannel`. When the refcount reaches + * zero the channel becomes eligible for LRU eviction again. No-op for a + * null/empty channelId or a channel with no outstanding pins. + */ +export function unpinArchiveChannel( + channelId: string | null | undefined, +): void { + if (!channelId) return; + const current = pinnedArchiveChannelCounts.get(channelId); + if (current === undefined) return; + if (current <= 1) { + pinnedArchiveChannelCounts.delete(channelId); + } else { + pinnedArchiveChannelCounts.set(channelId, current - 1); + } +} + +// Agents whose full live-event window must be retained because a session viewer +// is mounted on them (the two session panels and the activity bar). Refcounted +// so co-mounted viewers of the same agent compose; the agent keeps its full +// window until the last viewer unmounts. Keyed by normalized pubkey. An agent +// with a positive count is pinned; an unpinned agent's window is bounded to +// UNPINNED_AGENT_EVENT_TAIL. Consumers that read the window WITHOUT displaying +// the transcript (preventSleep, the active-turns bridge, the profile activity +// feed) intentionally do NOT pin — they tolerate the tail by design. +const viewerPinnedAgentCounts = new Map(); + +/** + * Pin an agent's full live-event window against tail-bounding while a session + * viewer is mounted on it. Refcounted so multiple mounted viewers of the same + * agent compose. No-op for a null/empty pubkey (viewers mount before an agent + * resolves). Pinning does not itself grow the window — it lifts the cap so + * subsequent events accumulate up to MAX_OBSERVER_EVENTS. + */ +export function pinObserverAgent(agentPubkey: string | null | undefined): void { + if (!agentPubkey) return; + const key = normalizePubkey(agentPubkey); + viewerPinnedAgentCounts.set(key, (viewerPinnedAgentCounts.get(key) ?? 0) + 1); +} + +/** + * Release one pin acquired via `pinObserverAgent`. When the refcount reaches + * zero the agent's window is immediately bounded back to + * UNPINNED_AGENT_EVENT_TAIL (see `truncateUnpinnedAgentWindow`) so closing a + * viewer reclaims the deep scroll-back it accumulated. No-op for a null/empty + * pubkey or an agent with no outstanding pins. + */ +export function unpinObserverAgent( + agentPubkey: string | null | undefined, +): void { + if (!agentPubkey) return; + const key = normalizePubkey(agentPubkey); + const current = viewerPinnedAgentCounts.get(key); + if (current === undefined) return; + if (current <= 1) { + viewerPinnedAgentCounts.delete(key); + truncateUnpinnedAgentWindow(key); + } else { + viewerPinnedAgentCounts.set(key, current - 1); + } +} + +/** Retention cap for one agent: full window while a viewer is pinned, else the newest-N tail. */ +function agentEventCap(key: string): number { + return viewerPinnedAgentCounts.has(key) + ? MAX_OBSERVER_EVENTS + : UNPINNED_AGENT_EVENT_TAIL; +} + +/** + * Bound a now-unpinned agent's event window to UNPINNED_AGENT_EVENT_TAIL, + * dropping the deep scroll-back its viewer accumulated. Atomic with the derived + * state: the event array, its transcript, and the memoized snapshot are all + * rebuilt from the same truncated window before listeners are notified, so no + * consumer can observe a transcript that references dropped events. No-op when + * the window already fits, so unpinning an agent that never grew is free. + */ +function truncateUnpinnedAgentWindow(key: string): void { + const current = eventsByAgent.get(key); + if (!current || current.length <= UNPINNED_AGENT_EVENT_TAIL) { + return; + } + const trimmed = current.slice(current.length - UNPINNED_AGENT_EVENT_TAIL); + eventsByAgent.set(key, trimmed); + transcriptByAgent.set(key, buildTranscriptState(trimmed)); + invalidateSnapshot(key); + notifyListeners(); +} + // Per-agent, per-channel latest-live-session-id. // Key: `${normalizePubkey(agentPubkey)}:${channelId}`. // Set when a live relay observer event with a sessionId arrives. @@ -195,6 +354,7 @@ function observerTag(event: RelayEvent, tagName: string) { function appendAgentEvent(agentPubkey: string, event: ObserverEvent) { const key = normalizePubkey(agentPubkey); + recordChannelActivity(key, event); const current = eventsByAgent.get(key) ?? []; if ( current.some( @@ -206,10 +366,9 @@ function appendAgentEvent(agentPubkey: string, event: ObserverEvent) { } const sorted = [...current, event].sort(compareObserverEvents); - const trimmed = sorted.length > MAX_OBSERVER_EVENTS; - const final = trimmed - ? sorted.slice(sorted.length - MAX_OBSERVER_EVENTS) - : sorted; + const cap = agentEventCap(key); + const trimmed = sorted.length > cap; + const final = trimmed ? sorted.slice(sorted.length - cap) : sorted; eventsByAgent.set(key, final); // Determine whether the new event landed at the end of the sorted array. @@ -279,9 +438,42 @@ function appendArchivedChannelEvent( // order for consumers that call buildTranscriptState over the window. const sorted = [...current, event].sort(compareObserverEvents); archiveEventsByChannel.set(key, sorted); + // Record LRU bookkeeping: which channel this key belongs to (the eviction + // unit) and its most-recent access, so the policy can pick the least-recently + // loaded unpinned channels when the retained-channel cap is exceeded. + archiveChannelId.set(key, channelId); + archiveAccessSeq.set(key, ++archiveAccessCounter); return true; } +/** + * Evict the least-recently-loaded unpinned channels from the archive window + * when the number of distinct retained channels exceeds + * `MAX_RETAINED_ARCHIVE_CHANNELS`. Pinned channels (a panel is mounted on them) + * are never evicted. Eviction removes every (agent, channel) entry for the + * dropped channels and their LRU bookkeeping; a later revisit re-hydrates the + * channel from SQLite through the normal paging path. Returns true if any key + * was evicted so the caller can decide whether a notify is warranted. + */ +function evictArchiveChannelsIfNeeded(): boolean { + const entries = Array.from(archiveEventsByChannel.keys(), (key) => ({ + key, + channelId: archiveChannelId.get(key) ?? "", + accessSeq: archiveAccessSeq.get(key) ?? 0, + })); + const evictKeys = selectArchiveEvictionKeys( + entries, + new Set(pinnedArchiveChannelCounts.keys()), + MAX_RETAINED_ARCHIVE_CHANNELS, + ); + for (const key of evictKeys) { + archiveEventsByChannel.delete(key); + archiveChannelId.delete(key); + archiveAccessSeq.delete(key); + } + return evictKeys.length > 0; +} + /** * Read the channel-scoped archive raw events for a given (agent, channel) * pair. Returns an empty array when no archive has been loaded yet. @@ -302,64 +494,6 @@ export function getArchivedChannelEvents( ); } -export function compareObserverEvents( - left: ObserverEvent, - right: ObserverEvent, -) { - const leftTime = Date.parse(left.timestamp); - const rightTime = Date.parse(right.timestamp); - if (Number.isFinite(leftTime) && Number.isFinite(rightTime)) { - const timeDiff = leftTime - rightTime; - if (timeDiff !== 0) { - return timeDiff; - } - } - - return left.seq - right.seq; -} - -/** - * Returns true if `candidate` sorts strictly after `stored` using the same - * two-key ordering as `compareObserverEvents`: later timestamp wins; equal - * timestamp falls back to higher seq. Extracted so latest-live advancement - * cannot drift from transcript ordering. - */ -export function isObserverEventAfter( - candidate: { timestamp: string; seq: number }, - stored: { timestamp: string; seq: number }, -): boolean { - const candidateTime = Date.parse(candidate.timestamp); - const storedTime = Date.parse(stored.timestamp); - if (Number.isFinite(candidateTime) && Number.isFinite(storedTime)) { - if (candidateTime !== storedTime) { - return candidateTime > storedTime; - } - } - return candidate.seq > stored.seq; -} - -// Observer event kind for a batch envelope wrapping multiple events. The ACP -// harness publishes one frame per second; everything that accumulated between -// ticks arrives as `{ kind: "batch", payload: { events: [...] } }` with every -// inner event carrying its own seq/timestamp. Inner events are processed -// exactly as unbatched ones; the envelope itself is never stored. -const OBSERVER_BATCH_KIND = "batch"; - -// Expand a decrypted observer event into its inner events when it is a batch -// envelope; a non-batch event passes through as a single-element array. A -// malformed envelope (no events array) degrades to the envelope itself so a -// harness bug cannot silently blank the session viewer. -function unwrapObserverBatch(parsed: ObserverEvent): ObserverEvent[] { - if (parsed.kind !== OBSERVER_BATCH_KIND) { - return [parsed]; - } - const payload = parsed.payload as { events?: unknown } | null; - const events = Array.isArray(payload?.events) - ? (payload.events as ObserverEvent[]) - : null; - return events && events.length > 0 ? events : [parsed]; -} - // Per-event processing shared by every event a live frame carries (one for a // plain frame, many for a batch envelope). function processLiveObserverEvent(agentPubkey: string, parsed: ObserverEvent) { @@ -684,14 +818,28 @@ export function useManagedAgentObserverBridge( * (e.g. an agent that is stopped but has archived history) are dropped. * The caller should ensure the agent is registered before calling. * - * `_decryptFn` is only used by tests to inject a mock decryption function. - * Production callers must always omit it. + * Commit is atomic against channel switches and panel unmounts. Every frame is + * decrypted into a staging buffer FIRST (phase 1, the only async work); the + * whole page is then applied to the store synchronously under a single + * `isStale()` gate (phase 2, no await between the check and the appends). The + * caller passes a gate reading `requestGeneration !== resetGeneration || + * disposed`, so a page whose channel was switched away (including A→B→A) or + * whose panel unmounted while it decrypted is dropped whole — it can never + * commit half a page or resurrect an evicted channel as most-recently-used. + * + * `_decryptFn` is only used by tests to inject a mock decryption function; + * `isStale` defaults to never-stale for direct test calls. Production callers + * must omit `_decryptFn` and always pass `isStale`. */ export async function ingestArchivedObserverEvents( rawEvents: RelayEvent[], _decryptFn: (event: RelayEvent) => Promise = decryptObserverEvent, + isStale: () => boolean = () => false, ): Promise { - let archiveChanged = false; + // Phase 1: decrypt every frame into a staging buffer. All async work happens + // here, before any store write, so no channel switch or unmount can interleave + // between a decrypt and its commit. + const staged: Array<{ agentPubkey: string; event: ObserverEvent }> = []; for (const event of rawEvents) { const agentPubkey = observerTag(event, "agent"); const frame = observerTag(event, "frame"); @@ -707,30 +855,51 @@ export async function ingestArchivedObserverEvents( try { const parsed = (await _decryptFn(event)) as ObserverEvent; for (const inner of unwrapObserverBatch(parsed)) { - // Route archived events to the channel-scoped archive window (no cap) - // rather than the per-agent live-relay store (MAX_OBSERVER_EVENTS cap). - // Events without a channelId fall through to the live store so they - // remain visible in the agent's general transcript. - if (inner.channelId) { - const added = appendArchivedChannelEvent( - agentPubkey, - inner.channelId, - inner, - ); - if (added) archiveChanged = true; - } else { - // Live path already calls notifyListeners() inside appendAgentEvent. - appendAgentEvent(agentPubkey, inner); - } + staged.push({ agentPubkey, event: inner }); } } catch { // Silently drop decrypt failures — same as live path error handling. } } + + // Phase 2: commit the whole page synchronously under one staleness gate. The + // gate is evaluated once, immediately before the first write, with no await + // between the check and the appends — so the request that started this page + // is still current and every staged event belongs to the live channel. A + // stale page is dropped whole rather than committed partially. + if (isStale()) { + return; + } + + let archiveChanged = false; + for (const { agentPubkey, event } of staged) { + // Route archived events to the channel-scoped archive window (no cap) + // rather than the per-agent live-relay store (MAX_OBSERVER_EVENTS cap). + // Events without a channelId fall through to the live store so they remain + // visible in the agent's general transcript. + if (event.channelId) { + const added = appendArchivedChannelEvent( + agentPubkey, + event.channelId, + event, + ); + if (added) archiveChanged = true; + } else { + // Live path already calls notifyListeners() inside appendAgentEvent. + appendAgentEvent(agentPubkey, event); + } + } // Batch-notify once for the whole page of archive events. appendAgentEvent // already notifies individually for live/no-channelId events above, so we // only need one extra notify here for the archive path. - if (archiveChanged) { + // + // Enforce the retained-channel bound after the page lands: the channel just + // ingested has the highest access seq, so it is safe from its own eviction; + // only older unpinned channels are dropped. Fold the eviction result into the + // notify decision so a page that only triggers eviction still refreshes any + // panel reading an evicted channel. + const evicted = evictArchiveChannelsIfNeeded(); + if (archiveChanged || evicted) { notifyListeners(); } } @@ -776,7 +945,13 @@ export function resetAgentObserverStore() { eventsByAgent.clear(); transcriptByAgent.clear(); snapshotByAgent.clear(); + clearChannelActivity(); archiveEventsByChannel.clear(); + archiveChannelId.clear(); + archiveAccessSeq.clear(); + archiveAccessCounter = 0; + pinnedArchiveChannelCounts.clear(); + viewerPinnedAgentCounts.clear(); knownAgentPubkeys.clear(); knownAgentsBySubscription.clear(); pendingUnknownAgentFrames.length = 0; diff --git a/desktop/src/features/agents/observerTieredWindow.test.mjs b/desktop/src/features/agents/observerTieredWindow.test.mjs new file mode 100644 index 00000000000..5316253da74 --- /dev/null +++ b/desktop/src/features/agents/observerTieredWindow.test.mjs @@ -0,0 +1,359 @@ +/** + * Store-level tests for the tiered observer live-event window. + * + * The observer store keeps the full MAX_OBSERVER_EVENTS window only while a + * session viewer is pinned to an agent (pinObserverAgent). An UNPINNED agent — + * one observed only by background consumers, never displayed — is bounded to + * UNPINNED_AGENT_EVENT_TAIL. This is the accumulator Option B bounds: a + * long-lived session observes many agents over time, and without the tail each + * would hold its full window forever. + * + * These exercise the REAL ingestion path (injectObserverEventsForE2E → + * appendAgentEvent → the same trim/transcript rebuild the live relay uses) plus + * the public pin API, asserting: + * + * - an unpinned agent's window truncates to the tail, newest events kept; + * - a pinned agent retains its full window (the cap is lifted, not applied); + * - unpinning the last viewer immediately bounds the window it accumulated; + * - the pin is refcounted — two viewers hold until the last release; + * - the newest event always survives (preventSleep reads only events[last]); + * - active turns survive event-array eviction (the CRITICAL constraint: the + * active-turns bridge processes each event once as it arrives, so a + * turn_started aging out of the tail cannot lose the turn); + * - resetAgentObserverStore clears pins so they don't leak across sessions. + * + * No React/Tauri runtime. The tail (UNPINNED_AGENT_EVENT_TAIL = 100) and the + * full cap (MAX_OBSERVER_EVENTS = 3000) are private constants; tests drive + * counts relative to the tail and stay well under the full cap so a pinned + * agent's retained count proves the cap was lifted rather than merely not hit. + */ + +import assert from "node:assert/strict"; +import { beforeEach, describe, it } from "node:test"; + +import { + getAgentChannelActivity, + getAgentObserverSnapshot, + injectObserverEventsForE2E, + pinObserverAgent, + resetAgentObserverStore, + subscribeAgentObserverStore, + unpinObserverAgent, +} from "@/features/agents/observerRelayStore.ts"; +import { + getActiveTurnsForAgent, + resetActiveAgentTurnsStore, + syncActiveAgentTurnsFromObserver, +} from "@/features/agents/activeAgentTurnsStore.ts"; + +const TAIL = 100; // UNPINNED_AGENT_EVENT_TAIL (private constant, mirrored here) +const OVER_TAIL = TAIL + 50; // enough to force truncation, far under the 3000 cap + +/** 64-hex agent pubkey from a short label. */ +function agentPubkey(label) { + return label.padEnd(64, "0"); +} + +const AGENT = agentPubkey("a"); + +/** + * One live observer event. Monotonic timestamp keyed to seq so the store's + * timestamp-then-seq sort matches insertion order — the newest event is always + * the highest seq. + */ +function makeEvent(seq, overrides = {}) { + return { + seq, + timestamp: new Date(1_000_000 + seq * 1000).toISOString(), + kind: "acp_write", + agentIndex: 0, + channelId: "chan-1", + sessionId: "sess-1", + turnId: "turn-1", + payload: {}, + ...overrides, + }; +} + +/** Inject `count` events (seq 1..count) one at a time, mirroring the live path + * where each relay frame appends and notifies individually. */ +function injectSequential(agent, count, makeAt = makeEvent) { + for (let seq = 1; seq <= count; seq++) { + injectObserverEventsForE2E(agent, [makeAt(seq)]); + } +} + +function snapshotEvents(agent) { + return getAgentObserverSnapshot(agent, true).events; +} + +describe("observer tiered live-event window — store level", () => { + beforeEach(() => { + resetAgentObserverStore(); + resetActiveAgentTurnsStore(); + }); + + it("test_unpinned_agent_window_truncates_to_tail", () => { + injectSequential(AGENT, OVER_TAIL); + + const events = snapshotEvents(AGENT); + assert.equal( + events.length, + TAIL, + "an unpinned agent's window must be bounded to the tail", + ); + assert.equal( + events[events.length - 1].seq, + OVER_TAIL, + "the newest event must be retained", + ); + assert.equal( + events[0].seq, + OVER_TAIL - TAIL + 1, + "the oldest events beyond the tail must be dropped", + ); + }); + + it("test_pinned_agent_retains_full_window", () => { + // Pinning lifts the cap to MAX_OBSERVER_EVENTS. OVER_TAIL is far under it, + // so a pinned agent keeps every event — proving the pin lifted the cap + // rather than the count merely not reaching it. + pinObserverAgent(AGENT); + injectSequential(AGENT, OVER_TAIL); + + const events = snapshotEvents(AGENT); + assert.equal( + events.length, + OVER_TAIL, + "a pinned agent retains its full window, well past the unpinned tail", + ); + assert.equal(events[events.length - 1].seq, OVER_TAIL); + assert.equal(events[0].seq, 1, "the oldest event is still present"); + }); + + it("test_unpin_last_viewer_truncates_existing_window", () => { + // A viewer accumulates a deep window; closing it must immediately reclaim + // the scroll-back down to the tail (not wait for the next append to trim). + pinObserverAgent(AGENT); + injectSequential(AGENT, OVER_TAIL); + assert.equal(snapshotEvents(AGENT).length, OVER_TAIL, "full while pinned"); + + unpinObserverAgent(AGENT); + + const events = snapshotEvents(AGENT); + assert.equal( + events.length, + TAIL, + "unpinning the last viewer bounds the window it accumulated", + ); + assert.equal( + events[events.length - 1].seq, + OVER_TAIL, + "truncation keeps the newest events, drops the oldest", + ); + }); + + it("test_refcount_two_viewers_stays_pinned_until_last_unpin", () => { + // Two mounted viewers of the same agent compose. The window stays full + // until the SECOND unpin — a refcount, not a boolean flag. + pinObserverAgent(AGENT); + pinObserverAgent(AGENT); + injectSequential(AGENT, OVER_TAIL); + + unpinObserverAgent(AGENT); + assert.equal( + snapshotEvents(AGENT).length, + OVER_TAIL, + "still pinned while one viewer remains", + ); + + unpinObserverAgent(AGENT); + assert.equal( + snapshotEvents(AGENT).length, + TAIL, + "window bounded only after the last viewer unpins", + ); + }); + + it("test_newest_event_survives_tail_for_prevent_sleep", () => { + // preventSleepActivity reads ONLY events[events.length - 1]. The tail must + // never drop the newest event, or wake-lock refresh would stall. + injectSequential(AGENT, OVER_TAIL); + + const events = snapshotEvents(AGENT); + const newest = events[events.length - 1]; + assert.equal(newest.seq, OVER_TAIL, "newest event survives the tail"); + assert.equal( + newest.timestamp, + makeEvent(OVER_TAIL).timestamp, + "the retained newest event is the last one appended", + ); + }); + + it("test_active_turns_survive_event_eviction", () => { + // The CRITICAL constraint. The active-turns bridge subscribes to the store + // and drains the snapshot on every notify — so each event is processed once + // as it arrives (as the newest event), advancing the per-(agent,channel) + // watermark. A turn_started can then age out of the tail without losing the + // turn: turn state lives in activeAgentTurnsStore, independent of the event + // array, and the watermark blocks any re-processing that never comes. + const RUNNING = [{ pubkey: AGENT, status: "running" }]; + const unsubscribe = subscribeAgentObserverStore(() => { + syncActiveAgentTurnsFromObserver(RUNNING); + }); + + try { + // A turn starts. The bridge drains this notify and marks the turn active. + injectObserverEventsForE2E(AGENT, [ + makeEvent(1, { kind: "turn_started", channelId: "c1", turnId: "t1" }), + ]); + assert.ok( + getActiveTurnsForAgent(AGENT).some((t) => t.channelId === "c1"), + "turn is active immediately after turn_started", + ); + + // Flood the window past the tail so turn_started ages out. Each event + // notifies, draining the bridge — but the watermark is already past + // turn_started, so it is never reprocessed and the turn is not disturbed. + for (let seq = 2; seq <= OVER_TAIL + 1; seq++) { + injectObserverEventsForE2E(AGENT, [ + makeEvent(seq, { kind: "acp_write", channelId: "c1", turnId: "t1" }), + ]); + } + + // The event array has evicted turn_started... + const events = snapshotEvents(AGENT); + assert.equal(events.length, TAIL, "window is bounded to the tail"); + assert.ok( + !events.some((e) => e.kind === "turn_started"), + "turn_started has aged out of the event array", + ); + + // ...but the turn is still active. Truncation cannot lose turns. + assert.ok( + getActiveTurnsForAgent(AGENT).some((t) => t.channelId === "c1"), + "the active turn survives event-array eviction", + ); + } finally { + unsubscribe(); + } + }); + + it("test_reset_clears_pins", () => { + // A pin outstanding at reset must not survive it — otherwise a phantom pin + // would keep the next session's same-key agent unbounded. + pinObserverAgent(AGENT); + resetAgentObserverStore(); + + // If the pin leaked, the window below would stay full at OVER_TAIL. + injectSequential(AGENT, OVER_TAIL); + assert.equal( + snapshotEvents(AGENT).length, + TAIL, + "pin state must not survive reset — the agent is unpinned again", + ); + }); +}); + +describe("observer channel-activity summary — survives tail truncation", () => { + // Scope note (Thufir pass-3 sharpening #4 — feed-scope parity for archived + // events): the summary is fed ONLY on the live-append path, via + // `recordChannelActivity` inside `appendAgentEvent`. Archive-ingested + // channel events are intentionally OUTSIDE the summary: on main, + // `ingestArchivedObserverEvents` routes channelId-bearing frames to + // `appendArchivedChannelEvent` (the channel-scoped archive window), never to + // the live `eventsByAgent` store, and `deriveProfileActivityFeedScope` draws + // its channel scope only from the live snapshot + transcript. So archive + // events never contributed to feed scope, parity holds, and driving these + // equivalence cases from the live path (`injectObserverEventsForE2E`) is the + // faithful fixture — an archive-fed fixture would test a path the feed scope + // never read. + beforeEach(() => { + resetAgentObserverStore(); + }); + + it("test_channel_dropped_from_tail_survives_in_summary", () => { + // chan-old gets a single early event, then chan-new floods past the tail so + // chan-old's only event ages out of the (unpinned) event window. The + // durable summary must still report chan-old with its original recency — + // this is exactly the profile-feed scope truncation would otherwise drop. + const oldEvent = makeEvent(1, { channelId: "chan-old" }); + injectObserverEventsForE2E(AGENT, [oldEvent]); + for (let seq = 2; seq <= OVER_TAIL + 1; seq++) { + injectObserverEventsForE2E(AGENT, [ + makeEvent(seq, { channelId: "chan-new" }), + ]); + } + + const events = snapshotEvents(AGENT); + assert.ok( + !events.some((e) => e.channelId === "chan-old"), + "chan-old's event has aged out of the truncated window", + ); + + const summary = getAgentChannelActivity(AGENT); + assert.equal( + summary["chan-old"], + Date.parse(oldEvent.timestamp), + "the summary retains chan-old's recency after truncation drops its event", + ); + assert.equal( + summary["chan-new"], + Date.parse(makeEvent(OVER_TAIL + 1).timestamp), + "the summary tracks the newest recency for a still-present channel", + ); + }); + + it("test_summary_keeps_latest_recency_per_channel", () => { + injectObserverEventsForE2E(AGENT, [makeEvent(1, { channelId: "c" })]); + injectObserverEventsForE2E(AGENT, [makeEvent(5, { channelId: "c" })]); + + assert.equal( + getAgentChannelActivity(AGENT)["c"], + Date.parse(makeEvent(5).timestamp), + "a later event advances the channel's recency; an earlier one cannot regress it", + ); + + // A late-arriving older frame for the same channel must not regress it. + injectObserverEventsForE2E(AGENT, [makeEvent(3, { channelId: "c" })]); + assert.equal( + getAgentChannelActivity(AGENT)["c"], + Date.parse(makeEvent(5).timestamp), + "an out-of-order older event does not regress the recency", + ); + }); + + it("test_channelless_and_bad_timestamp_events_do_not_pollute_summary", () => { + injectObserverEventsForE2E(AGENT, [makeEvent(1, { channelId: null })]); + injectObserverEventsForE2E(AGENT, [ + makeEvent(2, { channelId: "bad-ts", timestamp: "not-a-date" }), + ]); + + const summary = getAgentChannelActivity(AGENT); + assert.deepEqual( + summary, + {}, + "a channelless event has no channel scope and an unparseable timestamp has no recency — neither enters the summary", + ); + }); + + it("test_reset_clears_the_summary", () => { + injectObserverEventsForE2E(AGENT, [makeEvent(1, { channelId: "c" })]); + assert.equal(Object.keys(getAgentChannelActivity(AGENT)).length, 1); + + resetAgentObserverStore(); + assert.deepEqual( + getAgentChannelActivity(AGENT), + {}, + "the summary must not survive a store reset", + ); + }); + + it("test_unknown_agent_reads_empty_summary", () => { + assert.deepEqual( + getAgentChannelActivity(agentPubkey("z")), + {}, + "an agent with no observed activity reads an empty summary", + ); + }); +}); diff --git a/desktop/src/features/agents/ui/useLoadArchivedObserverEvents.test.mjs b/desktop/src/features/agents/ui/useLoadArchivedObserverEvents.test.mjs index c3ca1f678b0..49d9b3720ff 100644 --- a/desktop/src/features/agents/ui/useLoadArchivedObserverEvents.test.mjs +++ b/desktop/src/features/agents/ui/useLoadArchivedObserverEvents.test.mjs @@ -934,3 +934,80 @@ describe("useLoadArchivedObserverEvents — mounted hook lifecycle regressions", await unmount(); }); }); + +describe("useLoadArchivedObserverEvents — post-unmount ingest fence", () => { + beforeEach(() => { + resetAgentObserverStore(); + clearIpcHandlers(); + _testRegisterKnownAgents(SUB_ID, [AGENT_PUBKEY]); + }); + + /** + * Post-unmount fence: a page whose Tauri read is still in flight when the + * panel unmounts must NOT write to the archive store. By unmount time the + * panel's pin has been released, so the channel may already be evicted; + * writing the late page would resurrect a dead channel as most-recently-used + * and silently defeat the archive bound. + * + * Sequence: + * 1. Mount on chan-a. Backfill returns immediately (no rows). The archive + * read is DEFERRED so it is still in flight at unmount. + * 2. Unmount the hook while the read is deferred (disposedRef → true). + * 3. Release the archive read. The resolved page reaches the fence check. + * 4. Assert chan-a's archive window is EMPTY — the fence skipped the write. + * + * Without the disposedRef fence, the late page ingests and the channel window + * is non-empty after unmount. + */ + it("test_page_resolving_after_unmount_does_not_write_to_store", async () => { + let resolveArchive; + const archiveDeferred = new Promise((resolve) => { + resolveArchive = resolve; + }); + + setIpcHandler("list_save_subscriptions", async () => + makeOwnerPSubResponse(), + ); + setIpcHandler("read_unindexed_observer_rows", async () => []); + setIpcHandler("index_observer_channel_id", async () => null); + setIpcHandler("read_archived_observer_events_for_channel", async (args) => { + if (args.channelId === "chan-a") { + await archiveDeferred; // hold the read in flight across unmount + return [JSON.stringify(makeArchivedRow(1, "chan-a"))]; + } + return []; + }); + setIpcHandler("decrypt_observer_event", async (args) => { + try { + const event = JSON.parse(args.eventJson); + return JSON.parse(event.content); + } catch { + return { kind: "telemetry", channelId: null }; + } + }); + + const qc = makeQueryClient(); + const { render, unmount } = mountHook("chan-a", qc); + + // Step 1: mount; hydration starts and blocks on the deferred archive read. + await render("chan-a"); + await act(async () => { + await new Promise((r) => setTimeout(r, 20)); + }); + + // Step 2: unmount while the read is still in flight. + await unmount(); + + // Step 3: release the archive read; let the resolved page reach the fence. + resolveArchive(); + await settle(10); + + // Step 4: the late page must NOT have been written to the store. + const archived = _testGetArchivedChannelEvents(AGENT_PUBKEY, "chan-a"); + assert.equal( + archived.length, + 0, + `page resolving after unmount must not write to the store — found ${archived.length} (disposedRef fence missing would leave this 1)`, + ); + }); +}); diff --git a/desktop/src/features/agents/ui/useObserverAgentPin.test.mjs b/desktop/src/features/agents/ui/useObserverAgentPin.test.mjs new file mode 100644 index 00000000000..12408cca0b8 --- /dev/null +++ b/desktop/src/features/agents/ui/useObserverAgentPin.test.mjs @@ -0,0 +1,148 @@ +/** + * Mounted-hook lifecycle test for the observer-agent viewer pin. + * + * The tiered live-event window (observerRelayStore) keeps an agent's full + * MAX_OBSERVER_EVENTS window only while a session viewer is pinned; an unpinned + * agent is bounded to UNPINNED_AGENT_EVENT_TAIL. The pin/unpin lifecycle is + * owned by the viewer hooks: useObserverEvents (session panel) and + * useAgentTranscript (activity bar). This mounts the REAL hooks in a React tree + * and asserts the wiring end to end: + * + * - mounting a viewer pins the agent, so its window accumulates the full + * window (well past the unpinned tail); + * - unmounting the viewer unpins and immediately bounds the window back to + * the tail — reclaiming the deep scroll-back the panel accumulated. + * + * This is a mutation test for the useObserverAgentPin wiring: remove the pin + * from the hook and the mounted-window assertion fails (window would be the + * tail); remove the unpin/truncate and the post-unmount assertion fails + * (window would stay full). + * + * The hooks are mounted with `enabled = false`: the pin is keyed on pubkey and + * is independent of `enabled` (which gates only the live relay subscription), + * so the pin still fires while the relay/Tauri subscription side effect stays + * dormant — no IPC mock needed. + * + * DOM shim: react-dom/client needs a minimal DOM; installDOMShim (from the + * shared observed-unread harness) provides it, matching the convention used by + * the other mounted-hook suites in this package. + */ + +import assert from "node:assert/strict"; +import { beforeEach, describe, it } from "node:test"; + +import { installDOMShim } from "@/features/channels/observedUnreadTestHarness.mjs"; + +installDOMShim(); + +import React from "react"; +import { createRoot } from "react-dom/client"; +import { act } from "react"; + +import { + getAgentObserverSnapshot, + injectObserverEventsForE2E, + resetAgentObserverStore, +} from "@/features/agents/observerRelayStore.ts"; +import { + useAgentTranscript, + useObserverEvents, +} from "@/features/agents/ui/useObserverEvents.ts"; + +const TAIL = 100; // UNPINNED_AGENT_EVENT_TAIL (private constant, mirrored here) +const OVER_TAIL = TAIL + 50; // forces truncation when unpinned; far under 3000 + +const AGENT = "a".repeat(64); + +function makeEvent(seq) { + return { + seq, + timestamp: new Date(1_000_000 + seq * 1000).toISOString(), + kind: "acp_write", + agentIndex: 0, + channelId: "chan-1", + sessionId: "sess-1", + turnId: "turn-1", + payload: {}, + }; +} + +// Injection notifies store listeners, which re-renders the mounted viewer's +// useSyncExternalStore. Wrap in act() so those commits are flushed inside the +// React act scope rather than warning about unbatched updates. +async function injectSequential(agent, count) { + await act(async () => { + for (let seq = 1; seq <= count; seq++) { + injectObserverEventsForE2E(agent, [makeEvent(seq)]); + } + }); +} + +function windowLength(agent) { + return getAgentObserverSnapshot(agent, true).events.length; +} + +/** + * Mount a viewer hook (enabled = false so only the pin fires) and return an + * unmount fn. `useHook` is the viewer hook under test. + */ +async function mountViewer(useHook) { + function Harness() { + useHook(false, AGENT); + return null; + } + const root = createRoot(document.createElement("div")); + await act(async () => { + root.render(React.createElement(Harness)); + }); + return async () => { + await act(async () => { + root.unmount(); + }); + }; +} + +describe("useObserverAgentPin — mounted viewer lifecycle", () => { + beforeEach(() => { + resetAgentObserverStore(); + }); + + it("test_mounting_useObserverEvents_pins_agent_then_unmount_truncates", async () => { + const unmount = await mountViewer(useObserverEvents); + + await injectSequential(AGENT, OVER_TAIL); + assert.equal( + windowLength(AGENT), + OVER_TAIL, + "a mounted session-panel viewer pins the agent — the full window accumulates", + ); + + await unmount(); + assert.equal( + windowLength(AGENT), + TAIL, + "unmounting the viewer unpins and bounds the window back to the tail", + ); + }); + + it("test_mounting_useAgentTranscript_pins_agent_then_unmount_truncates", async () => { + // useAgentTranscript is the activity-bar entry point (BotActivityBar), so + // it must pin too — otherwise a visible activity bar would show a + // tail-truncated transcript. + const unmount = await mountViewer(useAgentTranscript); + + await injectSequential(AGENT, OVER_TAIL); + assert.equal( + windowLength(AGENT), + OVER_TAIL, + "a mounted activity-bar viewer pins the agent — the full window accumulates", + ); + + await unmount(); + assert.equal( + windowLength(AGENT), + TAIL, + "unmounting the activity-bar viewer bounds the window back to the tail", + ); + }); +}); diff --git a/desktop/src/features/agents/ui/useObserverEvents.ts b/desktop/src/features/agents/ui/useObserverEvents.ts index 0c44a64d5f2..93ea11918a3 100644 --- a/desktop/src/features/agents/ui/useObserverEvents.ts +++ b/desktop/src/features/agents/ui/useObserverEvents.ts @@ -6,7 +6,11 @@ import { getAgentTranscript, getArchivedChannelEvents, ingestArchivedObserverEvents, + pinArchiveChannel, + pinObserverAgent, subscribeAgentObserverStore, + unpinArchiveChannel, + unpinObserverAgent, } from "@/features/agents/observerRelayStore"; import { listSaveSubscriptions, @@ -31,10 +35,27 @@ export type { ArchivePagingState } from "./archivePagingState"; const subscribeToStore = (onStoreChange: () => void) => subscribeAgentObserverStore(onStoreChange); +/** + * Pin an agent's full live-event window while a session viewer is mounted, so + * the deep scroll-back it displays is not tail-bounded out from under it. + * Refcounted in the store, so the two session panels and the activity bar + * compose when viewing the same agent. Keyed on pubkey only; `enabled` gates + * the relay subscription, not the display, so an idle-agent viewer still pins. + */ +function useObserverAgentPin(agentPubkey: string | null | undefined): void { + React.useEffect(() => { + if (!agentPubkey) return; + pinObserverAgent(agentPubkey); + return () => unpinObserverAgent(agentPubkey); + }, [agentPubkey]); +} + export function useObserverEvents( enabled: boolean, agentPubkey?: string | null, ) { + useObserverAgentPin(agentPubkey); + const getSnapshot = React.useCallback( () => getAgentObserverSnapshot(agentPubkey, enabled), [agentPubkey, enabled], @@ -55,6 +76,8 @@ export function useAgentTranscript( enabled: boolean, agentPubkey?: string | null, ): TranscriptItem[] { + useObserverAgentPin(agentPubkey); + const getSnapshot = React.useCallback( () => getAgentTranscript(agentPubkey, enabled), [agentPubkey, enabled], @@ -81,6 +104,18 @@ export function useArchivedChannelEvents( agentPubkey: string | null | undefined, channelId: string | null | undefined, ): ObserverEvent[] { + // Pin this channel's archive against LRU eviction while the panel is mounted, + // so the history the user is looking at is never dropped out from under the + // view. Refcounted in the store, so co-mounted consumers of the same channel + // (this read hook plus the loader below, plus a second panel) compose. The + // pin is keyed on channel only — agent identity does not affect eviction, + // which is a per-channel operation. + React.useEffect(() => { + if (!channelId) return; + pinArchiveChannel(channelId); + return () => unpinArchiveChannel(channelId); + }, [channelId]); + const getSnapshot = React.useCallback( () => getArchivedChannelEvents(agentPubkey, channelId), [agentPubkey, channelId], @@ -133,6 +168,19 @@ export function useLoadArchivedObserverEvents( } const ps = pagingStateRef.current; + // Fence for in-flight fetches that resolve after unmount. Set true by the + // cleanup effect below; checked before every store write so a page still + // decrypting when the panel closes cannot repopulate a channel whose pin has + // just been released — which would otherwise defeat the archive bound by + // resurrecting a dead channel as most-recently-used. + const disposedRef = React.useRef(false); + React.useEffect(() => { + disposedRef.current = false; + return () => { + disposedRef.current = true; + }; + }, []); + // React state mirrors the fields callers observe so re-renders fire on change. const [hasSubscription, setHasSubscription] = React.useState( ps.hasSubscription, @@ -335,7 +383,18 @@ export function useLoadArchivedObserverEvents( createdAt: oldestEvent.created_at, id: oldestEvent.id, }; - await ingestArchivedObserverEvents(events); + // Atomic commit gate: ingest decrypts to a staging buffer, then applies + // the whole page synchronously only if this gate reads false at commit + // time. It reads true when a channel switch advanced resetGeneration + // (including A→B→A) or the panel unmounted (disposedRef) while the page + // decrypted — either would otherwise commit stale events onto the live + // channel or resurrect an evicted channel as most-recently-used. The + // cursor advance above is harmless (the ref is discarded on unmount). + await ingestArchivedObserverEvents(events, undefined, () => { + return ( + requestGeneration !== ps.resetGeneration || disposedRef.current + ); + }); } // Re-check generation after ingestArchivedObserverEvents: ingestion diff --git a/desktop/src/features/messages/deferredWriterEviction.test.mjs b/desktop/src/features/messages/deferredWriterEviction.test.mjs new file mode 100644 index 00000000000..8463084e15b --- /dev/null +++ b/desktop/src/features/messages/deferredWriterEviction.test.mjs @@ -0,0 +1,275 @@ +/** + * Mounted-hook eviction-fence tests for the two DEFERRED background writers + * that are React hooks (the reaction- and aux-hydration writers are plain + * async functions, fenced end-to-end in boundMessageWindows.test.mjs): + * + * - useLoadMissingAncestors — its `getEventById` fetch settles after the + * resweep evicts the channel; the deferred `mergeMessages` write must be + * dropped by the generation fence (`updateRetainedMessageUnit`). + * - useDeleteMessageMutation — its `delete_message` mutation resolves after + * the resweep; the deferred `onSuccess` filter must be dropped likewise. + * + * Both mount the REAL production hook (its useEffect / mutation wiring, its + * generation capture, and its guarded commit) so the test fails if the + * generation capture or the `updateRetainedMessageUnit` commit is removed. + * Ordering is Paul's: fetch in flight → resweep evicts the channel → fetch + * settles → deferred write fires → BOTH keys must stay absent (no + * `(current) => …` resurrection of the timeline the cache just bound away). + * + * DOM shim: react-dom/client needs a minimal DOM; installDOMShim (shared + * observed-unread harness) provides it, matching the other mounted-hook suites. + * Tauri IPC is intercepted at globalThis.__TAURI_INTERNALS__.invoke by command + * name (get_event, delete_message), the pattern from + * useLoadArchivedObserverEvents.test.mjs. + */ + +import assert from "node:assert/strict"; +import { beforeEach, describe, it } from "node:test"; + +import { installDOMShim } from "@/features/channels/observedUnreadTestHarness.mjs"; + +installDOMShim(); + +// ── Tauri IPC interceptor (installed before any module importing tauri.ts) ──── + +/** @type {Map Promise>} */ +const ipcHandlers = new Map(); + +globalThis.__TAURI_INTERNALS__ = { + invoke: (cmd, args) => { + const handler = ipcHandlers.get(cmd); + if (handler) return handler(args); + return Promise.reject(new Error(`unmocked Tauri command: ${cmd}`)); + }, + transformCallback: () => Math.random(), +}; + +function setIpcHandler(cmd, fn) { + ipcHandlers.set(cmd, fn); +} +function clearIpcHandlers() { + ipcHandlers.clear(); +} + +// ── Production imports (after shim, after IPC stub) ─────────────────────────── + +import React from "react"; +import { createRoot } from "react-dom/client"; +import { act } from "react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +import { + channelMessagesKey, + channelWindowKey, +} from "./lib/messageQueryKeys.ts"; +import { emptyChannelWindowStore } from "./lib/channelWindowStore.ts"; +import { + enforceMessageWindowBounds, + MAX_RETAINED_MESSAGE_CHANNELS, +} from "./lib/boundMessageWindows.ts"; +import { resetMessageUnitGenerations } from "./lib/messageUnitGuard.ts"; +import { useLoadMissingAncestors } from "./useLoadMissingAncestors.ts"; +import { useDeleteMessageMutation } from "./hooks.ts"; + +const CHAN_CAP = MAX_RETAINED_MESSAGE_CHANNELS; +const EVICTED = "evicted-channel"; + +/** A relay event with the channel `h` tag; `reply`/`root` tags optional. */ +function event(id, createdAt, extraTags = []) { + return { + id: id.padEnd(64, "0"), + pubkey: "a".repeat(64), + created_at: createdAt, + kind: 9, + tags: [["h", EVICTED], ...extraTags], + content: id, + sig: "b".repeat(128), + }; +} + +/** Seed the evicted channel's keys plus CHAN_CAP fresher channels above it. */ +function seedOverCap(client, evictedMessages) { + for (let i = 0; i < CHAN_CAP; i += 1) { + client.setQueryData( + channelWindowKey(`keep-${i}`), + emptyChannelWindowStore(), + { + updatedAt: i + 100, + }, + ); + client.setQueryData(channelMessagesKey(`keep-${i}`), [], { + updatedAt: i + 100, + }); + } + client.setQueryData(channelWindowKey(EVICTED), emptyChannelWindowStore(), { + updatedAt: 0, + }); + client.setQueryData(channelMessagesKey(EVICTED), evictedMessages, { + updatedAt: 0, + }); +} + +function hasEvictedChannel(client) { + return ( + client.getQueryData(channelWindowKey(EVICTED)) !== undefined || + client.getQueryData(channelMessagesKey(EVICTED)) !== undefined + ); +} + +async function settle() { + await act(async () => { + await new Promise((r) => setTimeout(r, 5)); + }); +} + +describe("deferred hook writers — eviction fence", () => { + beforeEach(() => { + resetMessageUnitGenerations(); + clearIpcHandlers(); + }); + + it("test_deferred_ancestor_load_does_not_resurrect_evicted_channel", async () => { + const ancestorId = "c".repeat(64); + // A resolved reply whose `reply` tag points at a missing ancestor — the + // hook fetches that ancestor via getEventById. + const reply = event("reply", 10, [["e", ancestorId, "", "reply"]]); + + let releaseFetch; + setIpcHandler( + "get_event", + () => + new Promise((resolve) => { + // getEventById JSON.parses the returned string. The ancestor carries + // the evicted channel's `h` tag so it passes the hook's channel-match + // guard — proving it is the GENERATION fence, not a channel mismatch, + // that drops the write. + releaseFetch = () => resolve(JSON.stringify(event("ancestor", 5))); + }), + ); + + const client = new QueryClient({ + defaultOptions: { queries: { retry: false, gcTime: Infinity } }, + }); + seedOverCap(client, [reply]); + + const channel = { id: EVICTED, channelType: "channel" }; + function Harness() { + useLoadMissingAncestors(channel, [reply]); + return null; + } + const root = createRoot(document.createElement("div")); + await act(async () => { + root.render( + React.createElement( + QueryClientProvider, + { client }, + React.createElement(Harness), + ), + ); + }); + await settle(); + + // Resweep while the ancestor fetch is still pending: the over-cap channel + // is evicted (generation bumped, both keys removed). + enforceMessageWindowBounds(client); + assert.equal( + hasEvictedChannel(client), + false, + "channel evicted mid-ancestor-fetch", + ); + + // The in-flight fetch settles and the deferred ancestor merge attempts its + // write against the stale captured generation. + await act(async () => { + releaseFetch(); + await new Promise((r) => setTimeout(r, 5)); + }); + + assert.equal( + client.getQueryData(channelMessagesKey(EVICTED)), + undefined, + "ancestor merge must not resurrect the messages key", + ); + assert.equal( + client.getQueryData(channelWindowKey(EVICTED)), + undefined, + "ancestor merge must not resurrect the window key", + ); + + await act(async () => { + root.unmount(); + }); + }); + + it("test_deferred_delete_success_does_not_resurrect_evicted_channel", async () => { + const targetId = "d".repeat(64); + + let releaseDelete; + setIpcHandler( + "delete_message", + () => + new Promise((resolve) => { + releaseDelete = () => resolve(null); + }), + ); + + const client = new QueryClient({ + defaultOptions: { queries: { retry: false, gcTime: Infinity } }, + }); + seedOverCap(client, [event("target", 10)]); + + const channel = { id: EVICTED, channelType: "channel" }; + const mutationRef = { current: null }; + function Harness() { + mutationRef.current = useDeleteMessageMutation(channel); + return null; + } + const root = createRoot(document.createElement("div")); + await act(async () => { + root.render( + React.createElement( + QueryClientProvider, + { client }, + React.createElement(Harness), + ), + ); + }); + + // Fire the delete: onMutate captures the (unevicted) generation, then the + // mutationFn awaits the deferred delete_message IPC. + let mutatePromise; + await act(async () => { + mutatePromise = mutationRef.current.mutateAsync({ eventId: targetId }); + await new Promise((r) => setTimeout(r, 5)); + }); + + // Resweep while the delete is in flight: the channel is evicted. + enforceMessageWindowBounds(client); + assert.equal( + hasEvictedChannel(client), + false, + "channel evicted mid-delete", + ); + + // The delete resolves; onSuccess's filter must be fenced. + await act(async () => { + releaseDelete(); + await mutatePromise; + }); + + assert.equal( + client.getQueryData(channelMessagesKey(EVICTED)), + undefined, + "delete onSuccess must not resurrect the messages key", + ); + assert.equal( + client.getQueryData(channelWindowKey(EVICTED)), + undefined, + "delete onSuccess must not resurrect the window key", + ); + + await act(async () => { + root.unmount(); + }); + }); +}); diff --git a/desktop/src/features/messages/hooks.ts b/desktop/src/features/messages/hooks.ts index 9091121d0bf..fa26ee56d61 100644 --- a/desktop/src/features/messages/hooks.ts +++ b/desktop/src/features/messages/hooks.ts @@ -7,6 +7,11 @@ import { channelWindowKey, threadRepliesKey, } from "@/features/messages/lib/messageQueryKeys"; +import { scheduleMessageWindowSweep } from "@/features/messages/lib/boundMessageWindows"; +import { + messageUnitGeneration, + updateRetainedMessageUnit, +} from "@/features/messages/lib/messageUnitGuard"; import { buildReplyTags, getThreadReference, @@ -672,6 +677,13 @@ export function useSendMessageMutation( }); queryClient.setQueryData(windowKey, next); projectChannelWindowMessages(queryClient, context.channelId); + + // Dropping the optimistic pending send releases this unit's pending-send + // pin. That drain is a manual `setQueryData`, which `useBoundedMessageWindows` + // excludes from its sweep triggers (every live merge is also manual), so + // the release would otherwise go unnoticed until an unrelated `added`. + // Fire the coalesced sweep explicitly here to close that gap. + scheduleMessageWindowSweep(queryClient); }, }); } @@ -708,18 +720,34 @@ export function useToggleReactionMutation() { export function useDeleteMessageMutation(channel: Channel | null) { const queryClient = useQueryClient(); - return useMutation({ + return useMutation< + void, + Error, + { eventId: string }, + { generationAtStart: number } | undefined + >({ mutationFn: async ({ eventId }) => { if (!channel) { throw new Error("No channel selected."); } await deleteMessage(channel.id, eventId); }, - onSuccess: (_data, { eventId }) => { - if (!channel) return; - queryClient.setQueryData( + // Capture the channel unit's generation before the async delete so the + // success write below is dropped if the window is evicted mid-flight (the + // user switched channels and roamed past the cap). Without the guard, the + // `filter` updater would resurrect the evicted key as a torn empty array. + onMutate: () => + channel + ? { generationAtStart: messageUnitGeneration(channel.id) } + : undefined, + onSuccess: (_data, { eventId }, context) => { + if (!channel || !context) return; + updateRetainedMessageUnit( + queryClient, channelMessagesKey(channel.id), - (current = []) => current.filter((message) => message.id !== eventId), + channel.id, + context.generationAtStart, + (current) => current.filter((message) => message.id !== eventId), ); }, onError: (error) => { @@ -741,7 +769,8 @@ export function useEditMessageMutation(channel: Channel | null) { // Pubkeys of mentions *newly added* by this edit, diffed at the composer. // Only these receive a `p` tag so a typo-fix edit re-wakes nobody. mentionPubkeys?: string[]; - } + }, + { generationAtStart: number } | undefined >({ mutationFn: async ({ eventId, content, mediaTags, mentionPubkeys }) => { if (!channel) { @@ -769,8 +798,22 @@ export function useEditMessageMutation(channel: Channel | null) { mentionTags, ); }, - onSuccess: (_data, { eventId, content, mediaTags, mentionPubkeys }) => { - if (!channel) { + // Capture the channel unit's generation before the async edit so the + // success writes below are dropped if the window is evicted mid-flight. + // Both writes go through `updateRetainedMessageUnit`: the window-store + // updater already no-ops on an absent key, but the flattened-array write + // would otherwise resurrect an evicted key, and neither guarded stale + // merges onto an A→B→A re-fetch. + onMutate: () => + channel + ? { generationAtStart: messageUnitGeneration(channel.id) } + : undefined, + onSuccess: ( + _data, + { eventId, content, mediaTags, mentionPubkeys }, + context, + ) => { + if (!channel || !context) { return; } @@ -799,14 +842,19 @@ export function useEditMessageMutation(channel: Channel | null) { // flattened array gets reverted by the next live event (see // mapChannelWindowEvents). Update the store first, then keep the // flattened cache in step for immediate paint. - queryClient.setQueryData( + updateRetainedMessageUnit( + queryClient, channelWindowKey(channel.id), - (current) => - current ? mapChannelWindowEvents(current, applyEdit) : current, + channel.id, + context.generationAtStart, + (current) => mapChannelWindowEvents(current, applyEdit), ); - queryClient.setQueryData( + updateRetainedMessageUnit( + queryClient, channelMessagesKey(channel.id), - (current = []) => current.map(applyEdit), + channel.id, + context.generationAtStart, + (current) => current.map(applyEdit), ); }, }); diff --git a/desktop/src/features/messages/lib/auxBackfill.ts b/desktop/src/features/messages/lib/auxBackfill.ts index 9e27323e14f..697e6695229 100644 --- a/desktop/src/features/messages/lib/auxBackfill.ts +++ b/desktop/src/features/messages/lib/auxBackfill.ts @@ -4,6 +4,10 @@ import { channelMessagesKey, sortMessages, } from "@/features/messages/lib/messageQueryKeys"; +import { + messageUnitGeneration, + updateRetainedMessageUnit, +} from "@/features/messages/lib/messageUnitGuard"; import { relayClient } from "@/shared/api/relayClient"; import { buildChannelStructuralAuxFilter } from "@/shared/api/relayChannelFilters"; import type { RelayEvent } from "@/shared/api/types"; @@ -138,6 +142,13 @@ export async function backfillAuxForMessages( return; } + // Capture the channel unit's generation before the aux fetch. If the window + // is evicted while this backfill is in flight, the generation advances and + // `updateRetainedMessageUnit` drops the write — a deferred aux merge can + // neither resurrect the evicted timeline nor stale-merge onto an A→B→A + // re-fetch. + const generationAtStart = messageUnitGeneration(channelId); + try { const cacheKey = channelMessagesKey(channelId); const cachedEvents = queryClient.getQueryData(cacheKey) ?? []; @@ -157,8 +168,12 @@ export async function backfillAuxForMessages( return; } - queryClient.setQueryData(cacheKey, (current = []) => - sortMessages([...current, ...mergedAuxEvents]), + updateRetainedMessageUnit( + queryClient, + cacheKey, + channelId, + generationAtStart, + (current) => sortMessages([...current, ...mergedAuxEvents]), ); } catch (error) { console.error( diff --git a/desktop/src/features/messages/lib/boundMessageWindows.test.mjs b/desktop/src/features/messages/lib/boundMessageWindows.test.mjs new file mode 100644 index 00000000000..50cc62daeb8 --- /dev/null +++ b/desktop/src/features/messages/lib/boundMessageWindows.test.mjs @@ -0,0 +1,320 @@ +/** + * Store-level tests for enforceMessageWindowBounds against a REAL QueryClient. + * + * These exercise the collect → fold → select → remove path at the cache + * boundary — the layer above the pure policy unit — asserting: + * - channels and threads are bounded independently at their caps; + * - eviction is least-recently-updated first; + * - evicting a channel removes BOTH its window and messages keys together; + * - an active channel (mounted observer) is never evicted, even when stale; + * - a pending send pins its channel — including the cross-key case where the + * optimistic event lands on a non-visible channel-window `liveOverlay`; + * - an evicted channel reads back absent → the freshness seam re-fetches; + * - the REAL deferred background writers (reaction hydration, aux backfill) + * cannot resurrect an evicted channel when their fetch settles after the + * resweep drops the unit — the generation fence, exercised end-to-end + * against the production writers and a real eviction, not a synthetic + * stand-in for the guarded updater. + */ + +import assert from "node:assert/strict"; +import { beforeEach, describe, it, mock } from "node:test"; +import { QueryClient, QueryObserver } from "@tanstack/react-query"; + +import { + channelMessagesKey, + channelWindowKey, + threadRepliesKey, +} from "./messageQueryKeys.ts"; +import { + emptyChannelWindowStore, + mergeLiveChannelWindowEvent, +} from "./channelWindowStore.ts"; +import { shouldRefreshChannelWindowAfterSubscribe } from "./projectChannelWindow.ts"; +import { + enforceMessageWindowBounds, + MAX_RETAINED_MESSAGE_CHANNELS, + MAX_RETAINED_MESSAGE_THREADS, +} from "./boundMessageWindows.ts"; +import { + hydrateRenderScopedReactions, + resetRenderScopedReactionHydration, +} from "./renderScopedReactions.ts"; +import { backfillAuxForMessages } from "./auxBackfill.ts"; +import { resetMessageUnitGenerations } from "./messageUnitGuard.ts"; +import { relayClient } from "@/shared/api/relayClient.ts"; + +const CHAN_CAP = MAX_RETAINED_MESSAGE_CHANNELS; +const THREAD_CAP = MAX_RETAINED_MESSAGE_THREADS; + +let client; + +/** A relay event; `pending` marks an un-acked optimistic send. */ +function event(id, createdAt, extra = {}) { + return { + id: id.padEnd(64, "0"), + pubkey: "a".repeat(64), + created_at: createdAt, + kind: 9, + tags: [["h", "channel"]], + content: id, + sig: "b".repeat(128), + ...extra, + }; +} + +/** + * Seed a channel's window + messages keys at a given recency. `updatedAt` + * drives `dataUpdatedAt` so the LRU order is deterministic in tests. + */ +function seedChannel(channelId, updatedAt, { pendingSend = false } = {}) { + let store = emptyChannelWindowStore(); + if (pendingSend) { + store = mergeLiveChannelWindowEvent( + store, + event(`pending-${channelId}`, updatedAt, { pending: true }), + ); + } + client.setQueryData(channelWindowKey(channelId), store, { updatedAt }); + client.setQueryData(channelMessagesKey(channelId), [], { updatedAt }); +} + +function seedThread(channelId, rootId, updatedAt) { + client.setQueryData(threadRepliesKey(channelId, rootId), [], { updatedAt }); +} + +/** Mount an observer so the query reports `isActive()` — a pinned view. */ +function mountChannel(channelId) { + const windowObserver = new QueryObserver(client, { + queryKey: channelWindowKey(channelId), + enabled: true, + }); + const messagesObserver = new QueryObserver(client, { + queryKey: channelMessagesKey(channelId), + enabled: true, + }); + const unsubWindow = windowObserver.subscribe(() => {}); + const unsubMessages = messagesObserver.subscribe(() => {}); + return () => { + unsubWindow(); + unsubMessages(); + }; +} + +function hasChannel(channelId) { + return ( + client.getQueryData(channelWindowKey(channelId)) !== undefined || + client.getQueryData(channelMessagesKey(channelId)) !== undefined + ); +} + +describe("enforceMessageWindowBounds", () => { + beforeEach(() => { + client = new QueryClient({ + defaultOptions: { queries: { retry: false, gcTime: Infinity } }, + }); + }); + + it("test_channels_up_to_cap_are_all_retained", () => { + for (let i = 0; i < CHAN_CAP; i += 1) seedChannel(`chan-${i}`, i); + enforceMessageWindowBounds(client); + for (let i = 0; i < CHAN_CAP; i += 1) { + assert.ok(hasChannel(`chan-${i}`), `chan-${i} must be retained at cap`); + } + }); + + it("test_exceeding_cap_evicts_least_recently_updated_channel", () => { + for (let i = 0; i <= CHAN_CAP; i += 1) seedChannel(`chan-${i}`, i); + enforceMessageWindowBounds(client); + assert.equal(hasChannel("chan-0"), false, "oldest channel evicted"); + for (let i = 1; i <= CHAN_CAP; i += 1) { + assert.ok(hasChannel(`chan-${i}`), `chan-${i} must survive`); + } + }); + + it("test_evicting_a_channel_removes_both_window_and_messages_keys", () => { + for (let i = 0; i <= CHAN_CAP; i += 1) seedChannel(`chan-${i}`, i); + enforceMessageWindowBounds(client); + assert.equal( + client.getQueryData(channelWindowKey("chan-0")), + undefined, + "window key gone", + ); + assert.equal( + client.getQueryData(channelMessagesKey("chan-0")), + undefined, + "messages key gone", + ); + }); + + it("test_active_channel_is_never_evicted_even_when_least_recent", () => { + // chan-0 is the oldest (recency 0) but mounted → pinned. One extra channel + // over cap forces an eviction; the next-oldest unpinned (chan-1) goes. + for (let i = 0; i <= CHAN_CAP; i += 1) seedChannel(`chan-${i}`, i); + const unmount = mountChannel("chan-0"); + enforceMessageWindowBounds(client); + assert.ok(hasChannel("chan-0"), "active channel pinned"); + assert.equal(hasChannel("chan-1"), false, "next-oldest unpinned evicted"); + unmount(); + }); + + it("test_pending_send_pins_its_channel_via_window_overlay", () => { + // chan-0 is oldest and inactive but holds an optimistic send in its window + // overlay (the cross-key send-from-thread seam) → pinned. chan-1 evicted. + for (let i = 0; i <= CHAN_CAP; i += 1) { + seedChannel(`chan-${i}`, i, { pendingSend: i === 0 }); + } + enforceMessageWindowBounds(client); + assert.ok(hasChannel("chan-0"), "channel with pending send pinned"); + assert.equal(hasChannel("chan-1"), false, "next-oldest unpinned evicted"); + }); + + it("test_threads_are_bounded_independently_from_channels", () => { + // Many channels AND many threads over both caps: each bounded to its own + // cap, neither starving the other. + for (let i = 0; i <= CHAN_CAP; i += 1) seedChannel(`chan-${i}`, i); + for (let i = 0; i <= THREAD_CAP; i += 1) seedThread("c", `root-${i}`, i); + enforceMessageWindowBounds(client); + // Oldest of each evicted; newest survive. + assert.equal(hasChannel("chan-0"), false); + assert.equal( + client.getQueryData(threadRepliesKey("c", "root-0")), + undefined, + "oldest thread evicted", + ); + assert.notEqual( + client.getQueryData(threadRepliesKey("c", `root-${THREAD_CAP}`)), + undefined, + "newest thread survives", + ); + }); + + it("test_evicted_channel_reads_absent_so_revisit_refetches", () => { + for (let i = 0; i <= CHAN_CAP; i += 1) seedChannel(`chan-${i}`, i); + enforceMessageWindowBounds(client); + // Freshness seam: an evicted channel has no messages state, so a resubscribe + // must refresh (returns true) rather than reading a stale window as fresh. + assert.equal( + shouldRefreshChannelWindowAfterSubscribe(client, "chan-0"), + true, + "evicted channel must refetch on revisit", + ); + }); + + it("test_deferred_reaction_hydration_does_not_resurrect_evicted_channel", async () => { + // Fill to cap, then open one more channel over cap whose reaction fetch is + // in flight when the resweep runs. Forces Paul's ordering: the hydrate + // promise settles → the resweep has already evicted the channel → the + // deferred merge fires → the generation fence must drop it, leaving BOTH + // keys absent (no `(current = []) => …` resurrection). + resetMessageUnitGenerations(); + resetRenderScopedReactionHydration(); + const evicted = "reaction-evicted"; + const messageId = "d".repeat(64); + for (let i = 0; i < CHAN_CAP; i += 1) seedChannel(`keep-${i}`, i + 100); + // Seed the target as the least-recently-updated so it is the one evicted. + seedChannel(evicted, 0); + client.setQueryData(channelMessagesKey(evicted), [event(messageId, 1)], { + updatedAt: 0, + }); + + let releaseFetch; + const hydrate = hydrateRenderScopedReactions({ + channelId: evicted, + messageIds: [messageId], + queryClient: client, + deps: { + fetchReactionEventsForMessages: () => + new Promise((resolve) => { + releaseFetch = () => + resolve([ + event("e".repeat(64), 7, { + tags: [ + ["h", "channel"], + ["e", messageId], + ], + }), + ]); + }), + }, + }); + + // Resweep while the fetch is still pending: the over-cap channel is evicted. + enforceMessageWindowBounds(client); + assert.equal(hasChannel(evicted), false, "channel evicted mid-fetch"); + + // Now the in-flight fetch settles and the deferred merge attempts its write. + releaseFetch(); + await hydrate; + + assert.equal( + client.getQueryData(channelMessagesKey(evicted)), + undefined, + "reaction merge must not resurrect the messages key", + ); + assert.equal( + client.getQueryData(channelWindowKey(evicted)), + undefined, + "reaction merge must not resurrect the window key", + ); + }); + + it("test_deferred_aux_backfill_does_not_resurrect_evicted_channel", async () => { + // Same ordering for the structural-aux writer: its aux fetch is in flight + // when the resweep evicts its channel, so its deferred merge must be fenced. + resetMessageUnitGenerations(); + const evicted = "aux-evicted"; + const messageId = "a".repeat(64); + for (let i = 0; i < CHAN_CAP; i += 1) seedChannel(`keep-${i}`, i + 100); + seedChannel(evicted, 0); + client.setQueryData(channelMessagesKey(evicted), [event(messageId, 1)], { + updatedAt: 0, + }); + + let releaseAux; + const auxByRef = mock.method( + relayClient, + "fetchAuxEventsByReference", + () => + new Promise((resolve) => { + releaseAux = () => + resolve([ + event("f".repeat(64), 40003, { + tags: [ + ["h", "channel"], + ["e", messageId], + ], + }), + ]); + }), + ); + const auxDeletions = mock.method( + relayClient, + "fetchAuxDeletionEventsForAuxEvents", + async () => [], + ); + + const backfill = backfillAuxForMessages(client, evicted, [ + event(messageId, 9), + ]); + + enforceMessageWindowBounds(client); + assert.equal(hasChannel(evicted), false, "channel evicted mid-backfill"); + + releaseAux(); + await backfill; + + assert.equal( + client.getQueryData(channelMessagesKey(evicted)), + undefined, + "aux merge must not resurrect the messages key", + ); + assert.equal( + client.getQueryData(channelWindowKey(evicted)), + undefined, + "aux merge must not resurrect the window key", + ); + auxByRef.mock.restore(); + auxDeletions.mock.restore(); + }); +}); diff --git a/desktop/src/features/messages/lib/boundMessageWindows.ts b/desktop/src/features/messages/lib/boundMessageWindows.ts new file mode 100644 index 00000000000..15e13e93cc2 --- /dev/null +++ b/desktop/src/features/messages/lib/boundMessageWindows.ts @@ -0,0 +1,169 @@ +import type { Query, QueryClient } from "@tanstack/react-query"; + +import type { ChannelWindowStore } from "./channelWindowStore"; +import { bumpMessageUnitGeneration } from "./messageUnitGuard"; +import { clearChannelRenderScopedReactionHydration } from "./renderScopedReactions"; +import type { RelayEvent } from "@/shared/api/types"; +import { + selectMessageWindowEvictionUnits, + type MessageWindowUnit, +} from "./messageWindowEviction"; + +/** + * Maximum number of distinct unpinned channel timelines and thread subtrees + * retained in the query cache. Channels and threads are bounded independently + * so a burst of open threads never evicts channel scrollback and vice versa. + * A revisit re-fetches from the relay (cache miss, not data loss). + */ +export const MAX_RETAINED_MESSAGE_CHANNELS = 12; +export const MAX_RETAINED_MESSAGE_THREADS = 24; + +const CHANNEL_WINDOW = "channel-window"; +const CHANNEL_MESSAGES = "channel-messages"; +const THREAD_REPLIES = "thread-replies"; + +/** A cached message query keyed by its parsed unit id, tagged by kind. */ +type ClassifiedQuery = { + query: Query; + unitId: string; + kind: "channel" | "thread"; +}; + +/** + * Classify a cache query as a channel-timeline key, a thread key, or neither. + * Channel units fold both `channel-window` and `channel-messages` under the + * channel id so they evict together; thread units key on channel + root. + */ +function classifyQuery(query: Query): ClassifiedQuery | null { + const key = query.queryKey; + const head = key[0]; + if ( + (head === CHANNEL_WINDOW || head === CHANNEL_MESSAGES) && + typeof key[1] === "string" + ) { + return { query, unitId: key[1], kind: "channel" }; + } + if ( + head === THREAD_REPLIES && + typeof key[1] === "string" && + typeof key[2] === "string" + ) { + return { query, unitId: `${key[1]}\u0000${key[2]}`, kind: "thread" }; + } + return null; +} + +/** Whether a query's cached data holds an optimistic pending send. */ +function holdsPendingSend(query: Query): boolean { + const data = query.state.data; + if (Array.isArray(data)) { + return (data as RelayEvent[]).some((event) => event.pending); + } + // A channel-window store keeps optimistic sends in its live overlay — the + // seam that catches a send-from-thread landing on a non-visible channel. + const overlay = (data as ChannelWindowStore | undefined)?.liveOverlay; + return Array.isArray(overlay) && overlay.some((event) => event.pending); +} + +/** + * Fold classified queries into eviction units. A unit is pinned when ANY of + * its queries has a mounted observer (the view is on screen — the "none" + * placeholder channel is inactive and so never pins), is mid-fetch (its + * authoritative window is still loading), or holds a pending send. Recency is + * the freshest `dataUpdatedAt` across the unit's queries. + */ +function foldUnits(queries: ClassifiedQuery[]): Map { + const units = new Map(); + for (const { query, unitId } of queries) { + const pinned = + query.isActive() || + query.state.fetchStatus === "fetching" || + holdsPendingSend(query); + const recency = query.state.dataUpdatedAt; + const existing = units.get(unitId); + if (existing) { + existing.pinned ||= pinned; + existing.recency = Math.max(existing.recency, recency); + } else { + units.set(unitId, { unitId, pinned, recency }); + } + } + return units; +} + +/** + * Enforce the retained-timeline bound on the query cache. Collects every + * channel and thread query, folds them into pinned/recency units, selects the + * least-recently-updated unpinned units beyond the caps, and removes their + * queries. Channels and threads are bounded independently. + * + * Removal (not invalidation) is deliberate: a removed key reads back as absent, + * so `shouldRefreshChannelWindowAfterSubscribe` re-fetches fresh on revisit. + * Deferred background writers (reaction/aux hydration, ancestor loads, mutation + * success handlers) fire after their own promise settles with no pin, so each + * commits through `updateRetainedMessageUnit`; bumping the evicted unit's + * generation here fences any such write that is already in flight, and clearing + * a channel's reaction-hydration claims lets its revisit re-hydrate from + * scratch instead of treating every id as already hydrated. + */ +export function enforceMessageWindowBounds(queryClient: QueryClient): void { + const channels: ClassifiedQuery[] = []; + const threads: ClassifiedQuery[] = []; + for (const query of queryClient.getQueryCache().getAll()) { + const classified = classifyQuery(query); + if (!classified) continue; + (classified.kind === "channel" ? channels : threads).push(classified); + } + + const evictChannels = new Set( + selectMessageWindowEvictionUnits( + [...foldUnits(channels).values()], + MAX_RETAINED_MESSAGE_CHANNELS, + ), + ); + const evictThreads = new Set( + selectMessageWindowEvictionUnits( + [...foldUnits(threads).values()], + MAX_RETAINED_MESSAGE_THREADS, + ), + ); + if (evictChannels.size === 0 && evictThreads.size === 0) return; + + for (const unitId of evictChannels) { + bumpMessageUnitGeneration(unitId); + clearChannelRenderScopedReactionHydration(unitId); + } + for (const unitId of evictThreads) { + bumpMessageUnitGeneration(unitId); + } + + for (const { query, unitId, kind } of [...channels, ...threads]) { + const evict = kind === "channel" ? evictChannels : evictThreads; + if (evict.has(unitId)) { + queryClient.getQueryCache().remove(query); + } + } +} + +// Per-client coalescing flag for `scheduleMessageWindowSweep`. A WeakSet keyed +// by the client needs no teardown — a client that is garbage-collected drops +// its flag with it, and a pending microtask always runs. +const sweepScheduled = new WeakSet(); + +/** + * Coalesce a bounds sweep to one run per microtask per client. Both the cache + * subscription (a key was added, an observer unmounted, or a fetch settled) and + * the send mutation (its optimistic pending-send pin drained) funnel through + * here, so every distinct "a pin may have been released" signal shares one + * generic resweep — a fetch pin and a mutation pin are not fixed separately. + * A channel open adds two keys at once and both land in the same microtask, so + * the sweep still runs once. + */ +export function scheduleMessageWindowSweep(queryClient: QueryClient): void { + if (sweepScheduled.has(queryClient)) return; + sweepScheduled.add(queryClient); + queueMicrotask(() => { + sweepScheduled.delete(queryClient); + enforceMessageWindowBounds(queryClient); + }); +} diff --git a/desktop/src/features/messages/lib/messageUnitGuard.ts b/desktop/src/features/messages/lib/messageUnitGuard.ts new file mode 100644 index 00000000000..c740e0fa647 --- /dev/null +++ b/desktop/src/features/messages/lib/messageUnitGuard.ts @@ -0,0 +1,69 @@ +import type { QueryClient, QueryKey } from "@tanstack/react-query"; + +/** + * Per-unit generation counter for the bounded message-window cache. A "unit" is + * one retained timeline: a channel (its `channel-window` + `channel-messages` + * keys, keyed by channel id) or a thread subtree (keyed by channel + root). The + * counter is advanced when a unit is evicted (see `enforceMessageWindowBounds`). + * + * The fetch-pin keeps a unit from being evicted *while its query is fetching*, + * but a deferred background writer — reaction/aux hydration, ancestor loads, + * the edit/delete mutation success handlers — fires after its own promise + * settles with no pin and no abort signal. In the window between "the fetch + * settled, the pin dropped, the resweep evicted the unit" and "the deferred + * write lands", an `(current = []) => …` updater would silently resurrect the + * evicted key, or (on an A→B→A revisit) merge stale events onto the re-fetched + * timeline. This generation fence is that backstop: a writer captures the + * unit's generation before its async work and commits through + * `updateRetainedMessageUnit`, which drops the write if the generation moved. + */ +const unitGenerations = new Map(); + +/** Read a unit's current generation; a unit never yet evicted reads 0. */ +export function messageUnitGeneration(unitId: string): number { + return unitGenerations.get(unitId) ?? 0; +} + +/** + * Advance a unit's generation on eviction, invalidating every in-flight + * deferred write that captured an earlier value. + */ +export function bumpMessageUnitGeneration(unitId: string): void { + unitGenerations.set(unitId, messageUnitGeneration(unitId) + 1); +} + +/** Test-only: clear all counters so cases don't leak generations across each other. */ +export function resetMessageUnitGenerations(): void { + unitGenerations.clear(); +} + +/** + * Guarded deferred write into a retained message query. Commits `updater` only + * when BOTH hold: + * - the target key is still present — an evicted key reads back absent, and + * React Query resurrects it from an `(current = []) => …` updater, so a bare + * merge would rebuild the timeline this cache bound just dropped; + * - the unit's generation still equals `generationAtStart` — a mismatch means + * the unit was evicted (and possibly re-fetched under a fresh generation) + * while the caller was awaiting, so the write carries stale events. + * Otherwise the write is dropped. A present-but-dataless query (mid-fetch, no + * data yet) is left untouched; its in-flight fetch produces the authoritative + * window. + */ +export function updateRetainedMessageUnit( + queryClient: QueryClient, + key: QueryKey, + unitId: string, + generationAtStart: number, + updater: (current: T) => T, +): void { + if ( + queryClient.getQueryState(key) === undefined || + messageUnitGeneration(unitId) !== generationAtStart + ) { + return; + } + queryClient.setQueryData(key, (current) => + current === undefined ? current : updater(current), + ); +} diff --git a/desktop/src/features/messages/lib/messageWindowEviction.test.mjs b/desktop/src/features/messages/lib/messageWindowEviction.test.mjs new file mode 100644 index 00000000000..38c5894c8c6 --- /dev/null +++ b/desktop/src/features/messages/lib/messageWindowEviction.test.mjs @@ -0,0 +1,89 @@ +/** + * Unit tests for selectMessageWindowEvictionUnits — the pure LRU-with-pins + * policy that bounds the message query-cache windows. Exercises the selection + * logic directly, with no React/React-Query runtime. + */ + +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; + +import { selectMessageWindowEvictionUnits } from "./messageWindowEviction.ts"; + +/** Build one eviction unit. */ +function unit(unitId, recency, pinned = false) { + return { unitId, recency, pinned }; +} + +describe("selectMessageWindowEvictionUnits", () => { + it("test_empty_units_returns_no_eviction", () => { + assert.deepEqual(selectMessageWindowEvictionUnits([], 8), []); + }); + + it("test_count_below_cap_returns_no_eviction", () => { + const units = [unit("a", 1), unit("b", 2), unit("c", 3)]; + assert.deepEqual(selectMessageWindowEvictionUnits(units, 8), []); + }); + + it("test_count_exactly_at_cap_returns_no_eviction", () => { + const units = Array.from({ length: 8 }, (_, i) => unit(`u-${i}`, i)); + assert.deepEqual(selectMessageWindowEvictionUnits(units, 8), []); + }); + + it("test_over_cap_evicts_least_recently_updated_units", () => { + // 10 units, cap 8 → evict the 2 lowest-recency (u-0, u-1). + const units = Array.from({ length: 10 }, (_, i) => unit(`u-${i}`, i)); + const evicted = selectMessageWindowEvictionUnits(units, 8); + assert.deepEqual(evicted.sort(), ["u-0", "u-1"].sort()); + }); + + it("test_pinned_unit_is_never_evicted_even_when_least_recent", () => { + // u-pin is least recent but pinned → survives; u-1 evicted instead. + const units = [ + unit("u-pin", 0, true), + unit("u-1", 1), + unit("u-2", 2), + unit("u-3", 3), + ]; + const evicted = selectMessageWindowEvictionUnits(units, 3); + assert.deepEqual(evicted, ["u-1"]); + }); + + it("test_pins_exceeding_cap_evict_all_unpinned_but_keep_all_pinned", () => { + // 3 pinned + 2 unpinned, cap 2. Pins alone exceed cap → unpinned budget 0. + const units = [ + unit("pin-1", 10, true), + unit("pin-2", 11, true), + unit("pin-3", 12, true), + unit("free-1", 1), + unit("free-2", 2), + ]; + const evicted = selectMessageWindowEvictionUnits(units, 2); + assert.deepEqual(evicted.sort(), ["free-1", "free-2"].sort()); + }); + + it("test_pinned_units_count_against_cap_for_unpinned_budget", () => { + // 1 pinned + 3 unpinned, cap 2 → unpinned budget = 1. Keep newest unpinned. + const units = [ + unit("pin-1", 5, true), + unit("free-1", 1), + unit("free-2", 2), + unit("free-3", 3), + ]; + const evicted = selectMessageWindowEvictionUnits(units, 2); + assert.deepEqual(evicted.sort(), ["free-1", "free-2"].sort()); + }); + + it("test_tie_broken_deterministically_by_unit_id", () => { + // unit-a and unit-b share recency 1 at the boundary; keep-sort breaks ties + // by ascending unitId, so unit-a is retained and unit-b evicted. + const units = [unit("unit-b", 1), unit("unit-a", 1), unit("unit-keep", 9)]; + const evicted = selectMessageWindowEvictionUnits(units, 2); + assert.deepEqual(evicted, ["unit-b"]); + }); + + it("test_zero_cap_evicts_all_unpinned_units", () => { + const units = [unit("a", 1), unit("b", 2), unit("pin", 3, true)]; + const evicted = selectMessageWindowEvictionUnits(units, 0); + assert.deepEqual(evicted.sort(), ["a", "b"].sort()); + }); +}); diff --git a/desktop/src/features/messages/lib/messageWindowEviction.ts b/desktop/src/features/messages/lib/messageWindowEviction.ts new file mode 100644 index 00000000000..47b44c4f72d --- /dev/null +++ b/desktop/src/features/messages/lib/messageWindowEviction.ts @@ -0,0 +1,65 @@ +/** + * Pure LRU-with-pins eviction policy for the message query-cache windows. + * + * The channel timeline lives in the React Query cache as two keys per channel + * (`channel-window` holding the authoritative window store and + * `channel-messages` holding the flattened render array) plus one + * `thread-replies` key per open thread root. Every visited channel or thread + * leaves its keys in the cache for `gcTime` (one hour) after the view unmounts, + * so a session that visits many channels accumulates every timeline's window + * store and flattened array at once — a renderer memory leak behind the v0.5.9 + * lag. React Query's `gcTime` bounds *how long* an inactive query lingers, not + * *how many* linger concurrently; this policy adds the missing concurrency + * bound. + * + * Eviction is at unit granularity — a channel unit owns both of its keys, a + * thread unit owns its single key — because the two channel keys are one + * timeline: dropping the window store while keeping the flattened array leaves + * a render array that live merges keep re-growing against no backing store. + * + * Pins are inviolable: a unit is never evicted while any of its queries has a + * mounted observer (the view is on screen) or holds an optimistic pending send + * (dropping it would lose an in-flight message the relay has not yet echoed). + * The cap is enforced only against unpinned units, even when pins alone exceed + * it. + * + * Pure and React/React-Query-free so the selection logic is unit-testable in + * isolation, mirroring the Target 1 `observerArchiveEviction` extraction. + */ + +export interface MessageWindowUnit { + /** Stable identifier for the eviction unit (channel id, or channel+root). */ + unitId: string; + /** Most recent `dataUpdatedAt` across the unit's queries; higher = fresher. */ + recency: number; + /** True when the unit must be retained: mounted observer or pending send. */ + pinned: boolean; +} + +/** + * Select the unit ids to evict so that no more than `cap` distinct unpinned + * units are retained. Pinned units never appear in the result. + * + * @param units One entry per eviction unit (channel or thread). + * @param cap Maximum distinct unpinned units to keep. + */ +export function selectMessageWindowEvictionUnits( + units: readonly MessageWindowUnit[], + cap: number, +): string[] { + const unpinned = units.filter((unit) => !unit.pinned); + const pinnedCount = units.length - unpinned.length; + const unpinnedBudget = Math.max(0, cap - pinnedCount); + if (unpinned.length <= unpinnedBudget) { + return []; + } + + // Keep the most-recent `unpinnedBudget` units; evict the rest. Sort by + // recency descending, breaking ties by unitId so selection is deterministic. + const evictable = [...unpinned].sort( + (a, b) => + b.recency - a.recency || + (a.unitId < b.unitId ? -1 : a.unitId > b.unitId ? 1 : 0), + ); + return evictable.slice(unpinnedBudget).map((unit) => unit.unitId); +} diff --git a/desktop/src/features/messages/lib/projectChannelWindow.test.mjs b/desktop/src/features/messages/lib/projectChannelWindow.test.mjs index 2ca2354271e..00ca09af79a 100644 --- a/desktop/src/features/messages/lib/projectChannelWindow.test.mjs +++ b/desktop/src/features/messages/lib/projectChannelWindow.test.mjs @@ -365,3 +365,41 @@ test("test_subscribe_refresh_does_not_duplicate_inflight_initial_fetch", async ( await client.getQueryCache().find({ queryKey })?.promise; unsubscribe(); }); + +test("test_projection_does_not_resurrect_an_evicted_messages_key", () => { + // The deferred `refreshChannelWindowMessages` path projects AFTER awaiting an + // invalidate — long enough for a fetch-settle resweep to evict the channel. + // A bare `(messages = []) => …` updater would resurrect the removed key; the + // presence guard must drop the projection so BOTH keys stay absent. + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + const channelId = "evicted"; + // Only the window store survives (a live overlay merge that raced the + // eviction); the messages key was removed by the resweep. + client.setQueryData( + channelWindowKey(channelId), + replaceNewestChannelWindow( + emptyChannelWindowStore(), + newestPage([event("stale", 100)]), + ), + ); + + projectChannelWindowMessages(client, channelId); + + assert.equal( + client.getQueryData(channelMessagesKey(channelId)), + undefined, + "projection must not resurrect the removed messages key", + ); +}); + +test("test_projection_aligns_an_existing_messages_key", () => { + // The complement: when the timeline cache is still present (every pin-covered + // caller), the projection aligns it from the authoritative window as before — + // the guard is a no-op on a live key. + const harness = createHarness(); + appendLiveEvent(harness, event("live", 130)); + + assert.deepEqual(contents(harness), ["initial", "live"]); +}); diff --git a/desktop/src/features/messages/lib/projectChannelWindow.ts b/desktop/src/features/messages/lib/projectChannelWindow.ts index 2d56c096b6c..d1b593d0432 100644 --- a/desktop/src/features/messages/lib/projectChannelWindow.ts +++ b/desktop/src/features/messages/lib/projectChannelWindow.ts @@ -36,17 +36,33 @@ export function shouldRefreshChannelWindowAfterSubscribe( return now - windowState.dataUpdatedAt >= CHANNEL_WINDOW_FRESH_MS; } -/** Keep the rendered timeline cache aligned with its authoritative window. */ +/** + * Keep the rendered timeline cache aligned with its authoritative window. + * + * Only aligns an EXISTING timeline cache: a removed `channelMessagesKey` reads + * back absent, and a bare `(messages = []) => …` updater would resurrect the + * timeline this cache just bound away. Every pin-covered caller (live append, + * optimistic send, older-page fetch) still holds the key, so the guard is a + * no-op there; it fences only the deferred `refreshChannelWindowMessages` path, + * whose post-`await` projection can otherwise land after a fetch-settle resweep + * evicted the channel. The channel's own messages query owns key CREATION on + * (re)open — projection never needs to create it. An in-flight initial fetch + * has query state (only a never-created/removed key reads `undefined`), so + * first-load population is unaffected. + */ export function projectChannelWindowMessages( queryClient: QueryClient, channelId: string, ) { + const messagesKey = channelMessagesKey(channelId); + if (queryClient.getQueryState(messagesKey) === undefined) { + return; + } const window = queryClient.getQueryData(channelWindowKey(channelId)) ?? emptyChannelWindowStore(); - queryClient.setQueryData( - channelMessagesKey(channelId), - (messages = []) => reconcileChannelWindowMessages(window, messages), + queryClient.setQueryData(messagesKey, (messages = []) => + reconcileChannelWindowMessages(window, messages), ); } diff --git a/desktop/src/features/messages/lib/renderScopedReactions.test.mjs b/desktop/src/features/messages/lib/renderScopedReactions.test.mjs index 6429ecf8116..a8dfe54a21e 100644 --- a/desktop/src/features/messages/lib/renderScopedReactions.test.mjs +++ b/desktop/src/features/messages/lib/renderScopedReactions.test.mjs @@ -42,6 +42,15 @@ function makeQueryClientStub(initialEvents = []) { getQueryData(key) { return store.get(JSON.stringify(key)); }, + // Mirrors React Query: an absent key has no state object, a present one + // reports its cached data. `updateRetainedMessageUnit`'s fence reads this to + // tell an evicted key (skip the write) from a live one. + getQueryState(key) { + const k = JSON.stringify(key); + return store.has(k) + ? { data: store.get(k), status: "success", fetchStatus: "idle" } + : undefined; + }, setQueryData(key, updater) { const k = JSON.stringify(key); const next = diff --git a/desktop/src/features/messages/lib/renderScopedReactions.ts b/desktop/src/features/messages/lib/renderScopedReactions.ts index 2c35cdf79e7..4c06129f626 100644 --- a/desktop/src/features/messages/lib/renderScopedReactions.ts +++ b/desktop/src/features/messages/lib/renderScopedReactions.ts @@ -1,6 +1,10 @@ import type { QueryClient } from "@tanstack/react-query"; import { channelMessagesKey, sortMessages } from "./messageQueryKeys"; +import { + messageUnitGeneration, + updateRetainedMessageUnit, +} from "./messageUnitGuard"; import type { MainTimelineEntry } from "./threadPanel"; import type { TimelineMessage } from "../types"; import { relayClient } from "@/shared/api/relayClient"; @@ -29,6 +33,17 @@ export function resetRenderScopedReactionHydration() { hydratedMessageIdsByChannel.clear(); } +/** + * Drop the hydration claim set for one channel. Called when the channel's + * message window is evicted so a revisit re-hydrates its visible reactions from + * scratch — without this, the claim set would report every id as already + * hydrated and the re-fetched window would render with no reactions until a + * live reaction arrived. + */ +export function clearChannelRenderScopedReactionHydration(channelId: string) { + hydratedMessageIdsByChannel.delete(channelId); +} + function hydratedSetForChannel(channelId: string): Set { let hydrated = hydratedMessageIdsByChannel.get(channelId); if (!hydrated) { @@ -117,6 +132,13 @@ export async function hydrateRenderScopedReactions(input: { return; } + // Capture the channel unit's generation before the fetch. If the window is + // evicted while this reaction fetch is in flight, the generation advances and + // `updateRetainedMessageUnit` drops the write below — so a deferred reaction + // merge can neither resurrect the evicted timeline nor stale-merge onto an + // A→B→A re-fetch. + const generationAtStart = messageUnitGeneration(input.channelId); + try { const reactionEvents = await ( input.deps ?? defaultDeps @@ -125,9 +147,12 @@ export async function hydrateRenderScopedReactions(input: { return; } - input.queryClient.setQueryData( + updateRetainedMessageUnit( + input.queryClient, channelMessagesKey(input.channelId), - (current = []) => sortMessages([...current, ...reactionEvents]), + input.channelId, + generationAtStart, + (current) => sortMessages([...current, ...reactionEvents]), ); } catch (error) { releaseRenderScopedReactionIds(input.channelId, messageIds); diff --git a/desktop/src/features/messages/useBoundedMessageWindows.ts b/desktop/src/features/messages/useBoundedMessageWindows.ts new file mode 100644 index 00000000000..7558997d17a --- /dev/null +++ b/desktop/src/features/messages/useBoundedMessageWindows.ts @@ -0,0 +1,76 @@ +import type { QueryCacheNotifyEvent, QueryClient } from "@tanstack/react-query"; +import * as React from "react"; + +import { scheduleMessageWindowSweep } from "./lib/boundMessageWindows"; + +const BOUNDED_MESSAGE_KEYS = new Set([ + "channel-window", + "channel-messages", + "thread-replies", +]); + +function isBoundedMessageEvent(queryKey: unknown): boolean { + return ( + Array.isArray(queryKey) && + typeof queryKey[0] === "string" && + BOUNDED_MESSAGE_KEYS.has(queryKey[0]) + ); +} + +/** + * Whether a cache event marks the release of a *fetch* pin — an observer + * unmounting (channel switch) or a real fetch settling. A manual `setQueryData` + * (`action.manual`) is excluded: it is the shape a live merge and the + * optimistic pending-send drain both take, and firing a sweep on every live + * merge would reintroduce the per-frame churn this cache bound removes. The + * pending-send pin release is handled explicitly by the send mutation instead. + */ +function isFetchPinRelease(event: QueryCacheNotifyEvent): boolean { + if (event.type === "observerRemoved") return true; + return ( + event.type === "updated" && + (event.action.type === "error" || + (event.action.type === "success" && !event.action.manual)) + ); +} + +/** + * Bound the number of channel timelines and thread subtrees retained in the + * query cache. Every visited channel/thread lingers for `gcTime` (one hour) + * after its view unmounts, so a session that roams many channels accumulates + * every timeline's window store and flattened array at once — the renderer + * memory leak behind the v0.5.9 lag. This mounts one cache subscription per + * QueryClient that evicts the least-recently-updated unpinned timelines past a + * fixed cap (see `enforceMessageWindowBounds`). + * + * Mounted once inside `CommunityQueryProvider`, so the bound applies to every + * window that renders it — the main app, the huddle room, and the companion — + * each on its own client. + * + * Two classes of event can leave a unit newly evictable and so must trigger a + * sweep, both coalesced to one run per microtask via `scheduleMessageWindowSweep`: + * - `added`: a new bounded key can push the count over its cap. A channel + * open adds two keys at once; the coalescer runs the sweep once. + * - a pin release: an observer unmounting (`observerRemoved`, on channel + * switch) or a real fetch settling (`updated` with a non-manual `success`/ + * `error` action — the fetch pin drops) makes a formerly-pinned unit + * evictable. Without this, a unit that went over cap while pinned would + * linger until the next unrelated `added` — the `added`-only gap Thufir + * flagged. The optimistic pending-send pin drains via a manual + * `setQueryData`, indistinguishable here from an ordinary live merge, so + * the send mutation calls `scheduleMessageWindowSweep` itself on success + * rather than being detected here — this keeps per-frame live merges (also + * manual updates) from triggering a sweep on every relay event. + * Removals emit `removed`, never `added` or a pin release, so eviction cannot + * re-trigger itself. The sweep runs synchronously in a single tick — collect + * then remove — so a view mounting mid-sweep cannot be misread as inactive. + */ +export function useBoundedMessageWindows(queryClient: QueryClient): void { + React.useEffect(() => { + return queryClient.getQueryCache().subscribe((event) => { + if (!isBoundedMessageEvent(event.query.queryKey)) return; + if (event.type !== "added" && !isFetchPinRelease(event)) return; + scheduleMessageWindowSweep(queryClient); + }); + }, [queryClient]); +} diff --git a/desktop/src/features/messages/useLoadMissingAncestors.ts b/desktop/src/features/messages/useLoadMissingAncestors.ts index 7928d4632d4..27f37060316 100644 --- a/desktop/src/features/messages/useLoadMissingAncestors.ts +++ b/desktop/src/features/messages/useLoadMissingAncestors.ts @@ -2,6 +2,10 @@ import * as React from "react"; import { useQueryClient } from "@tanstack/react-query"; import { channelMessagesKey } from "@/features/messages/lib/messageQueryKeys"; +import { + messageUnitGeneration, + updateRetainedMessageUnit, +} from "@/features/messages/lib/messageUnitGuard"; import { mergeMessages } from "@/features/messages/hooks"; import { getChannelIdFromTags, @@ -77,6 +81,15 @@ export function useLoadMissingAncestors( let isCancelled = false; + // Capture the channel unit's generation before the ancestor fetches. If the + // window is evicted while a `getEventById` is in flight, the generation + // advances and `updateRetainedMessageUnit` drops the merge below — a + // deferred ancestor write can neither resurrect the evicted timeline nor + // stale-merge onto an A→B→A re-fetch. (`requestedAncestorIdsRef` needs no + // matching clear on eviction: it is a per-hook ref already reset whenever + // `activeChannel.id` changes, so it never carries ids across channels.) + const generationAtStart = messageUnitGeneration(activeChannel.id); + void Promise.all( [...missingAncestorIds].map(async (eventId) => { try { @@ -89,9 +102,12 @@ export function useLoadMissingAncestors( return; } - queryClient.setQueryData( + updateRetainedMessageUnit( + queryClient, channelMessagesKey(activeChannel.id), - (current = []) => mergeMessages(current, event), + activeChannel.id, + generationAtStart, + (current) => mergeMessages(current, event), ); } catch (error) { console.error("Failed to load ancestor event", eventId, error); diff --git a/desktop/src/features/profile/lib/profileActivityFeedScope.test.mjs b/desktop/src/features/profile/lib/profileActivityFeedScope.test.mjs new file mode 100644 index 00000000000..f0bc35b1b07 --- /dev/null +++ b/desktop/src/features/profile/lib/profileActivityFeedScope.test.mjs @@ -0,0 +1,138 @@ +/** + * Fold-contract tests for deriveProfileActivityFeedScope's channelActivity + * input — the durable per-agent channel→latest-activity-ms summary that + * repairs the profile feed's channel scope after the observer store truncates + * an unpinned agent's event window to its newest tail. + * + * The five consumers of the per-agent event window are: + * - auto-restart (reads connectionState — a separate store field, untouched + * by window truncation); + * - working-state (reads activeAgentTurnsStore — a separate store); + * - latest-session (reads latestLiveSessionByAgentChannel — a separate store); + * - preventSleep (reads only events[last] — the newest event, which the tail + * always keeps); + * - profile-feed (reads the FULL event array for its channel set — the one + * consumer truncation degrades). + * The first four are equivalent before and after truncation by construction; + * these tests pin the fifth: with an empty summary the derived scope is + * identical to the events/transcript-only behaviour (equivalence baseline), and + * a non-empty summary re-adds exactly the channels and recency truncation drops. + */ + +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; + +import { deriveProfileActivityFeedScope } from "./profileActivityFeedScope.ts"; + +/** One observer event with a channel and an ISO timestamp derived from `ms`. */ +function event(channelId, ms, seq = 0) { + return { + seq, + timestamp: new Date(ms).toISOString(), + kind: "acp_write", + agentIndex: 0, + channelId, + sessionId: "s", + turnId: "t", + payload: {}, + }; +} + +describe("deriveProfileActivityFeedScope — channelActivity fold", () => { + it("test_empty_summary_is_identical_to_events_only_baseline", () => { + const events = [event("chan-a", 2000), event("chan-b", 3000)]; + const withoutSummary = deriveProfileActivityFeedScope({ + activeTurns: [], + events, + transcript: [], + }); + const withEmptySummary = deriveProfileActivityFeedScope({ + activeTurns: [], + events, + transcript: [], + channelActivity: {}, + }); + assert.deepEqual( + withEmptySummary, + withoutSummary, + "an empty (or omitted) summary must not change the derived scope", + ); + assert.deepEqual(withoutSummary.channelIds, ["chan-a", "chan-b"]); + }); + + it("test_summary_readds_channel_dropped_from_truncated_events", () => { + // The tail kept only chan-b (newest); chan-a fell out of the event window + // but survives in the durable summary — it must reappear in the scope. + const truncatedEvents = [event("chan-b", 3000)]; + const scope = deriveProfileActivityFeedScope({ + activeTurns: [], + events: truncatedEvents, + transcript: [], + channelActivity: { "chan-a": 2000, "chan-b": 3000 }, + }); + assert.deepEqual( + scope.channelIds, + ["chan-a", "chan-b"], + "a channel dropped from the tail is re-added from the summary", + ); + assert.equal(scope.latestActivityAtByChannel["chan-a"], 2000); + assert.equal(scope.latestActivityAtByChannel["chan-b"], 3000); + }); + + it("test_fresher_live_event_wins_over_summary_recency", () => { + // The summary carries a stale recency for chan-b; a fresher live event for + // the same channel is still in the window and must win the max. + const scope = deriveProfileActivityFeedScope({ + activeTurns: [], + events: [event("chan-b", 9000)], + transcript: [], + channelActivity: { "chan-b": 3000 }, + }); + assert.equal( + scope.latestActivityAtByChannel["chan-b"], + 9000, + "a fresher live event beats the summary's older recency", + ); + }); + + it("test_summary_alone_gives_feed_content_and_channels", () => { + // Every event aged out of the tail (empty window) but the summary retains + // the channels: the feed must still report content and its channel set. + const scope = deriveProfileActivityFeedScope({ + activeTurns: [], + events: [], + transcript: [], + channelActivity: { "chan-a": 1000, "chan-c": 2000 }, + }); + assert.equal( + scope.hasFeedContent, + true, + "a non-empty summary means the agent has feed content", + ); + assert.deepEqual(scope.channelIds, ["chan-a", "chan-c"]); + }); + + it("test_live_scope_ignores_summary_for_channel_ids", () => { + // When active turns exist the scope is live: channelIds come from the turns + // (the agent's current work), NOT the historical summary. The summary still + // feeds latestActivityAtByChannel so past channels keep a recency. + const scope = deriveProfileActivityFeedScope({ + activeTurns: [{ channelId: "chan-live", anchorAt: 5000 }], + events: [], + transcript: [], + channelActivity: { "chan-old": 1000 }, + }); + assert.equal(scope.isLive, true); + assert.deepEqual( + scope.channelIds, + ["chan-live"], + "live channelIds are the active turns, not the summary", + ); + assert.equal(scope.preferredChannelId, "chan-live"); + assert.equal( + scope.latestActivityAtByChannel["chan-old"], + 1000, + "the summary still contributes recency for a non-live channel", + ); + }); +}); diff --git a/desktop/src/features/profile/lib/profileActivityFeedScope.ts b/desktop/src/features/profile/lib/profileActivityFeedScope.ts index 596c04ec923..ce0f43f662f 100644 --- a/desktop/src/features/profile/lib/profileActivityFeedScope.ts +++ b/desktop/src/features/profile/lib/profileActivityFeedScope.ts @@ -4,6 +4,7 @@ import type { ActiveTurnSummary } from "@/features/agents/activeAgentTurnsStore" import { subscribeActiveAgentTurns } from "@/features/agents/activeAgentTurnsStore"; import { isManagedAgentActive } from "@/features/agents/lib/managedAgentControlActions"; import { + getAgentChannelActivity, getAgentObserverSnapshot, getAgentTranscript, subscribeAgentObserverStore, @@ -98,6 +99,7 @@ function stableFeedScope( function collectChannelIdsFromFeed( events: readonly ObserverEvent[], transcript: readonly TranscriptItem[], + channelActivity: Record, ): string[] { const channelIds = new Set(); for (const event of events) { @@ -110,6 +112,13 @@ function collectChannelIdsFromFeed( channelIds.add(item.channelId); } } + // Fold in channels from the durable per-agent summary. `events` is truncated + // to the unpinned tail, so a channel whose last event fell out of the tail is + // absent above but still present here — without this union it would vanish + // from the switcher for an idle, unpinned agent. + for (const channelId of Object.keys(channelActivity)) { + channelIds.add(channelId); + } return [...channelIds].sort((left, right) => left.localeCompare(right)); } @@ -143,10 +152,12 @@ function collectLatestActivityAtByChannel({ activeTurns, events, transcript, + channelActivity, }: { activeTurns: readonly ActiveTurnSummary[]; events: readonly ObserverEvent[]; transcript: readonly TranscriptItem[]; + channelActivity: Record; }): Record { const latestActivityAtByChannel: Record = {}; @@ -178,6 +189,14 @@ function collectLatestActivityAtByChannel({ } } + // Fold in the durable per-agent summary last. It carries one recency per + // channel that survives the unpinned-tail truncation of `events`, so a + // channel dropped from the tail keeps its last-known activity here. `record` + // takes the max, so a fresher live event or active turn still wins. + for (const [channelId, timestamp] of Object.entries(channelActivity)) { + record(channelId, timestamp); + } + return latestActivityAtByChannel; } @@ -185,17 +204,29 @@ export function deriveProfileActivityFeedScope({ activeTurns, events, transcript, + channelActivity = {}, }: { activeTurns: readonly ActiveTurnSummary[]; events: readonly ObserverEvent[]; transcript: readonly TranscriptItem[]; + /** + * Durable per-agent channel→latest-activity-ms summary. Survives the + * unpinned-tail truncation of `events`, so it repairs the channel scope that + * truncation would otherwise drop. Defaults to empty for callers that do not + * supply it (behaviour is then identical to reading `events`/`transcript`). + */ + channelActivity?: Record; }): ProfileActivityFeedScope { - const hasFeedContent = events.length > 0 || transcript.length > 0; + const hasFeedContent = + events.length > 0 || + transcript.length > 0 || + Object.keys(channelActivity).length > 0; const isLive = activeTurns.length > 0; const latestActivityAtByChannel = collectLatestActivityAtByChannel({ activeTurns, events, transcript, + channelActivity, }); if (isLive) { @@ -212,7 +243,11 @@ export function deriveProfileActivityFeedScope({ }; } - const feedChannelIds = collectChannelIdsFromFeed(events, transcript); + const feedChannelIds = collectChannelIdsFromFeed( + events, + transcript, + channelActivity, + ); const latestChannelId = deriveLatestChannelId(events, transcript); return { @@ -248,9 +283,15 @@ export function useProfileActivityFeedScope( const { events } = getAgentObserverSnapshot(activityAgent.pubkey, true); const transcript = getAgentTranscript(activityAgent.pubkey, true); + const channelActivity = getAgentChannelActivity(activityAgent.pubkey); return stableFeedScope( agentCacheKey, - deriveProfileActivityFeedScope({ activeTurns, events, transcript }), + deriveProfileActivityFeedScope({ + activeTurns, + events, + transcript, + channelActivity, + }), ); }, [activeTurns, activityAgent, agentCacheKey, hasObserver]); diff --git a/desktop/src/features/profile/ui/UserProfilePopover.tsx b/desktop/src/features/profile/ui/UserProfilePopover.tsx index 06295cc615c..6acbb90f9ce 100644 --- a/desktop/src/features/profile/ui/UserProfilePopover.tsx +++ b/desktop/src/features/profile/ui/UserProfilePopover.tsx @@ -17,6 +17,10 @@ import { useUsersBatchQuery, } from "@/features/profile/hooks"; import { channelMessagesKey } from "@/features/messages/lib/messageQueryKeys"; +import { + messageUnitGeneration, + updateRetainedMessageUnit, +} from "@/features/messages/lib/messageUnitGuard"; import { useRelayAgentsQuery, useManagedAgentsQuery, @@ -451,6 +455,13 @@ export function UserProfilePopover({ mergeTimelineCacheMessages(previousMessages, optimisticMessage), ); + // Capture the DM unit's generation before the send. If the window is + // evicted while `sendChannelMessage` is in flight (the user navigated on + // and the resweep dropped this DM), the generation advances and both the + // success merge and the error rollback below are dropped — neither an + // `(current = []) => …` updater can resurrect the evicted timeline. + const generationAtStart = messageUnitGeneration(dm.id); + try { await goChannel(dm.id); if (isMountedRef.current) { @@ -458,28 +469,38 @@ export function UserProfilePopover({ } const result = await sendChannelMessage(dm.id, content); - queryClient.setQueryData(queryKey, (current = []) => - mergeTimelineCacheMessages(current, { - id: result.eventId, - localKey: optimisticMessage.id, - pubkey: identity.pubkey, - created_at: result.createdAt, - kind: KIND_STREAM_MESSAGE, - tags: [ - ["h", dm.id], - ["p", identity.pubkey], - ], - content: content.trim(), - sig: "", - }), + updateRetainedMessageUnit( + queryClient, + queryKey, + dm.id, + generationAtStart, + (current) => + mergeTimelineCacheMessages(current, { + id: result.eventId, + localKey: optimisticMessage.id, + pubkey: identity.pubkey, + created_at: result.createdAt, + kind: KIND_STREAM_MESSAGE, + tags: [ + ["h", dm.id], + ["p", identity.pubkey], + ], + content: content.trim(), + sig: "", + }), ); } catch (error) { - queryClient.setQueryData(queryKey, (current = []) => - current.filter( - (message) => - message.id !== optimisticMessage.id && - message.localKey !== optimisticMessage.localKey, - ), + updateRetainedMessageUnit( + queryClient, + queryKey, + dm.id, + generationAtStart, + (current) => + current.filter( + (message) => + message.id !== optimisticMessage.id && + message.localKey !== optimisticMessage.localKey, + ), ); throw error; }