Skip to content

fix(observer): eager archive hydration on panel open + 200-frame pages#2574

Merged
wpfleger96 merged 5 commits into
mainfrom
duncan/observer-feed-hydration
Jul 23, 2026
Merged

fix(observer): eager archive hydration on panel open + 200-frame pages#2574
wpfleger96 merged 5 commits into
mainfrom
duncan/observer-feed-hydration

Conversation

@wpfleger96

@wpfleger96 wpfleger96 commented Jul 23, 2026

Copy link
Copy Markdown
Member

Problem

Opening the Activity panel for an agent/channel showed only the live in-memory window, wiped on every app restart. Archived history required 18+ manual scroll gestures to reach session start for a typical code-review turn (~900 frames). In practice, archived history was invisible.

Root cause: the archive write path is healthy; the read path never loads eagerly.

Changes

A — eager initial hydration (useObserverEvents.ts, archivePagingState.ts): on panel mount (or channel switch), automatically page through up to 10 pages (2,000 frames) before any user gesture.

B — page size 50 → 200: observer frames are small; 200/page means the 10-page budget covers 2,000 frames.

C — post-ingest channel token check: fetchOlderArchived now rechecks requestGeneration === ps.resetGeneration after await ingestArchivedObserverEvents() (async decrypt), not only after the Tauri read. Without this, a stale channel-A call resuming post-ingest could mark channel B exhausted and stop B's loop after one page.

D — generation-aware isFetching clear: the finally block only releases ps.isFetching when requestGeneration === ps.resetGeneration. Stale requests cannot steal the current channel's lock.

E — resetGeneration counter replaces channel-string ownership: ArchivePagingState now carries a monotonically increasing resetGeneration incremented by every applyChannelReset(). Each fetchOlderArchived invocation snapshots the generation at entry and rechecks it after all three async boundaries (backfill, Tauri read, ingest). A→B→A produces gen 1→2→3; old-A (gen=1) is rejected at every check regardless of channel name. activeChannelId is retained as a diagnostic field only.

F — post-backfill isFetching recheck (TOCTOU fix): ps.isFetching was checked only before await ps.backfillPromise. Two same-generation callers could both observe the lock free, both suspend on the pending backfill, then both resume past the post-backfill guard (which previously checked only generation + exhaustion) and both issue concurrent reads from the same cursor. Fix: ps.isFetching is now also included in the post-backfill guard, closing the check-then-await window.

Correctness audit

All async boundaries in fetchOlderArchived are guarded:

  • After await ps.backfillPromise: generation + exhaustion + ps.isFetching
  • After await readArchivedObserverEventsForChannel: generation ✓ (lock held, no concurrent caller)
  • After await ingestArchivedObserverEvents: generation ✓ (lock held, no concurrent caller)
  • finally: generation-gated isFetching release ✓

No further TOCTOU exists: once the lock is acquired at the single acquisition point (after the post-backfill guard), all subsequent awaits are under the lock.

Tests

useLoadArchivedObserverEvents.test.mjs — four mounted-hook regression tests (real useLoadArchivedObserverEvents hook + QueryClientProvider + Tauri IPC interception):

  • (a) exhausted-A → switch-B: B reads from null cursor and ingests. Green at dfb2d03; would be red at the pre-round-1 head (884ed9b).
  • (b) A→B deferred-I/O race: deferred A decrypt + deferred B first read — A cannot mark B exhausted or steal B's lock. Red at dfb2d03 (bCallCount==1). Lock theft independently detected by lock probe.
  • (c) concurrent backfill race: two callers both suspend on pending backfill — exactly one must acquire the lock and issue the Tauri read. Red at 4c92a01 (archiveCallCount==2). Mutation check: removing the post-backfill ps.isFetching recheck → red.
  • (d) A→B→A: old-A's deferred decrypt cannot corrupt fresh-A's exhaustion state or lock. Red when resetGeneration is replaced with channel-string equality.

archivePagingState.ts unit tests cover runHydrationLoop and applyChannelReset as pure functions.

Files changed

  • desktop/src/features/agents/ui/archivePagingState.tsresetGeneration field, runHydrationLoop export, updated applyChannelReset
  • desktop/src/features/agents/ui/useObserverEvents.ts — stable fetchOlderArchived (ref reads), generation token at all async boundaries, post-backfill lock recheck, generation-gated isFetching clear, hydration effect
  • desktop/src/features/agents/ui/useLoadArchivedObserverEvents.test.mjs — four mounted-hook regression tests (new file)
  • desktop/src/features/agents/ingestArchivedObserverEvents.test.mjsrunHydrationLoop unit tests

On panel open the Activity feed showed only the live in-memory window
(wiped on restart) because archived history requires manual scroll-up to
page in — 50 frames at a time. A single code-review turn emits ~900
frames, so reaching session start required 18 scroll gestures nobody
does, making archived history effectively invisible.

Two changes fix this:

A — eager initial hydration: add a hydration loop that fires automatically
when the panel mounts (or channel switches) and calls fetchOlderArchived
up to 10 pages = 2,000 frames before any user gesture. Reuses
fetchOlderArchived's existing lock/cursor/backfill-await machinery; no
parallel state machine. A new ps.initialHydrationDone flag (reset by
applyChannelReset on channel switch) prevents duplicate concurrent loops.

B — page size 50 → 200: observer frames are tiny; 50 was chat-message
tuning. 200 frames/page means the 10-page budget covers 2,000 frames,
more than enough for any single agent turn.

The scroll sentinel in AgentSessionThreadPanel already works correctly
(leave→enter gate, PRELOAD_MARGIN_PX=600) — scroll paging beyond the
initial budget continues unchanged.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96
wpfleger96 requested a review from a team as a code owner July 23, 2026 17:53
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 and others added 4 commits July 23, 2026 14:16
Three issues found in review of 884ed9b:

1. CRITICAL: fetchOlderArchived captured hasOlderArchived from React state
   (closure), not the ref. On an exhausted channel A → switch to B, React
   state was still false while ps.hasOlderArchived had been reset to true
   by applyChannelReset. The hydration loop called fetchOlderArchived 10
   times, each bailing at the !hasOlderArchived guard — leaving B with zero
   reads. Fix: read ps.hasOlderArchived from the ref throughout; remove
   hasOlderArchived from fetchOlderArchived deps so the callback is stable.

2. IMPORTANT: applyChannelReset clears isFetching even when A's Tauri read
   is in flight. When B then starts a read, A's late resolution wrote A's
   cursor into ps.cursor (now scoped to B). Fix: add activeChannelId token
   to ArchivePagingState, set by applyChannelReset. fetchOlderArchived
   captures requestChannelId at call start and discards cursor/exhaustion
   writes if requestChannelId !== ps.activeChannelId at resolution time.

3. IMPORTANT: new tests in ingestArchivedObserverEvents.test.mjs reimplemented
   the hydration loop inline (BUDGET hardcoded, loop logic copied), matching
   the tautological pattern removed in f13cfba. Fix: extract runHydrationLoop
   into archivePagingState.ts as a pure exported function; both the hook and
   the tests import and call it. Tests now fail if the production loop is
   deleted or misdirected.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…ion tests

Finding 1: fetchOlderArchived rechecked the channel token after the Tauri
read but NOT after await ingestArchivedObserverEvents(). A stale A call
resuming post-ingest could write ps.hasOlderArchived=false and clear
ps.isFetching for the new channel B. Fix: add a second token check
immediately after the ingest await, before any exhaustion or React-mirror
write; guard the finally isFetching clear with the same token comparison.

Finding 2: mounted-hook regression tests. Two tests in
useLoadArchivedObserverEvents.test.mjs mount the real production hook
(useLoadArchivedObserverEvents) against a mocked Tauri IPC bridge and a
QueryClientProvider seeded with the identity.

  (a) test_exhausted_channel_A_switch_to_B_hook_reads_B_from_null_cursor:
      exhausted A then switch to B — B reads from null cursor and ingests.
      Verified RED at dfb2d03 (stale-closure bug), GREEN after fix.

  (b) test_deferred_A_ingest_cannot_exhaust_B_or_steal_B_lock: deferred
      A decrypt races a channel switch to B; resolving A must not corrupt
      B's exhaustion state or lock. Verified RED at dfb2d03 (bCallCount=1,
      A's short-page write stopped B's loop), GREEN after post-ingest fix.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…n counter

Channel-string equality fails the A→B→A case: an in-flight old-A request
snapshotting channelId="A" passes every guard after the user returns to A,
allowing it to mark fresh-A exhausted and steal fresh-A's fetch lock.

Replace activeChannelId-as-ownership with a monotonically increasing
resetGeneration counter in ArchivePagingState. applyChannelReset()
increments it on every call. fetchOlderArchived snapshots the counter at
request start and rechecks snapshot === ps.resetGeneration after every
async boundary (backfill, Tauri read, ingest). The finally block clears
ps.isFetching only if the snapshot still matches. A→B→A produces gen
sequences 1→2→3, so old-A (gen=1) is always rejected by fresh-A (gen=3)
regardless of channel name. activeChannelId is retained as a diagnostic
label only — not as an ownership token.

Mounted-hook regressions updated:
- test (a) comment corrected: this test was GREEN at dfb2d03 (stale
  closure was already fixed in round 1, not this round)
- test (b) extended with a lock-probe step: after stale A resolves, call
  fetchOlderArchived() directly while B's first read is in flight and
  assert no new Tauri read starts (lock held). This makes the test fail
  independently when the finally guard is removed — not just when the
  post-ingest exhaustion write is present.
- test (c) new: A→B→A mounted regression. Old-A's decrypt deferred,
  switch A→B→A, fresh-A's first read deferred. Resolves old-A first,
  then lock-probes (concurrent call must be rejected), then resolves
  fresh-A and asserts it reads ≥2 pages.

Mutation verification (run before this commit):
- Remove post-ingest check (line ~342): tests (b) and (c) go RED ✓
- Remove finally guard (line ~357): tests (b) and (c) go RED ✓

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…TOCTOU

fetchOlderArchived checked ps.isFetching only before await ps.backfillPromise.
Two same-generation callers could both observe the lock free, both suspend on
the pending backfill, and both proceed past the post-backfill guard (which
previously checked only generation + exhaustion). Both would set isFetching=true
and issue readArchivedObserverEventsForChannel from the same null cursor.

Fix: add ps.isFetching to the post-backfill guard so the second caller to
resume sees the lock already taken and returns without reading.

Audit confirms no other TOCTOU: after isFetching=true is acquired, both
subsequent awaits (Tauri read, ingest) are already guarded by generation checks
that fire on channel switch. No concurrent same-generation caller can slip
through since all entry paths check isFetching.

New regression test test_two_concurrent_fetches_during_backfill_only_one_proceeds:
- Defers both the backfill response AND the archive response so concurrent
  reads are counted before any response returns (unambiguous witness).
- Verified RED at 4c92a01 (archiveCallCount==2 before fix).
- Mutation check: removing the post-backfill ps.isFetching line causes
  archiveCallCount==2 (RED). GREEN at this head.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96
wpfleger96 merged commit 8cb0502 into main Jul 23, 2026
25 checks passed
@wpfleger96
wpfleger96 deleted the duncan/observer-feed-hydration branch July 23, 2026 19:40
yoamomonstruos added a commit that referenced this pull request Jul 23, 2026
…arch

* origin/main:
  Refine channel lifecycle settings (#2427)
  Fix avatar upload lifecycle edge cases (#2277)
  fix(observer): eager archive hydration on panel open + 200-frame pages (#2574)
  fix(cli): install rustls crypto provider to unbreak WSS publishes in release builds (#2590)
  feat(dev): add `just production` recipe targeting the production relay (#2572)
  chore: Omit the Model control when an optional-model harness has nothing to select (#2262)
  fix(ci): stop moving protected Sprig rolling tag (#2221)
  fix(media): sanitize animated image uploads (#2524)
  fix(desktop): populate team instructions when opening the edit team dialog (#2565)
  feat(desktop): add drag-to-reorder for community rail (#2549)
  fix(channels): strip leading hash prefixes from names (#2250)
  fix(desktop): allow skipping harness setup onboarding (#2360)
  Script cleanup
  feat(relay): make Redis pool size configurable, default 16 (#2521)

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	desktop/src-tauri/src/commands/profile.rs
brow pushed a commit that referenced this pull request Jul 23, 2026
…obile-releasing

* origin/main: (23 commits)
  Gate default relay auto-connect behind release flag (#2589)
  fix(desktop): fast-track relay restart reconnects (#2579)
  fix(sharing): preserve agent/team snapshot tEXt chunks through media sanitization (#2438)
  fix(acp): restrict DM turns to owner and verified siblings (#2591)
  test(desktop): live relay kill/restart reconnect gate (#2583)
  fix(relay): send 1012 restart close to all clients on graceful drain (#2575)
  fix(desktop): retry failed initial relay dials (#2564)
  Refine channel lifecycle settings (#2427)
  Fix avatar upload lifecycle edge cases (#2277)
  fix(observer): eager archive hydration on panel open + 200-frame pages (#2574)
  fix(cli): install rustls crypto provider to unbreak WSS publishes in release builds (#2590)
  feat(dev): add `just production` recipe targeting the production relay (#2572)
  chore: Omit the Model control when an optional-model harness has nothing to select (#2262)
  fix(ci): stop moving protected Sprig rolling tag (#2221)
  fix(media): sanitize animated image uploads (#2524)
  fix(desktop): populate team instructions when opening the edit team dialog (#2565)
  feat(desktop): add drag-to-reorder for community rail (#2549)
  fix(channels): strip leading hash prefixes from names (#2250)
  fix(desktop): allow skipping harness setup onboarding (#2360)
  Script cleanup
  ...
brow pushed a commit that referenced this pull request Jul 23, 2026
…obile-releasing

* origin/main: (23 commits)
  Gate default relay auto-connect behind release flag (#2589)
  fix(desktop): fast-track relay restart reconnects (#2579)
  fix(sharing): preserve agent/team snapshot tEXt chunks through media sanitization (#2438)
  fix(acp): restrict DM turns to owner and verified siblings (#2591)
  test(desktop): live relay kill/restart reconnect gate (#2583)
  fix(relay): send 1012 restart close to all clients on graceful drain (#2575)
  fix(desktop): retry failed initial relay dials (#2564)
  Refine channel lifecycle settings (#2427)
  Fix avatar upload lifecycle edge cases (#2277)
  fix(observer): eager archive hydration on panel open + 200-frame pages (#2574)
  fix(cli): install rustls crypto provider to unbreak WSS publishes in release builds (#2590)
  feat(dev): add `just production` recipe targeting the production relay (#2572)
  chore: Omit the Model control when an optional-model harness has nothing to select (#2262)
  fix(ci): stop moving protected Sprig rolling tag (#2221)
  fix(media): sanitize animated image uploads (#2524)
  fix(desktop): populate team instructions when opening the edit team dialog (#2565)
  feat(desktop): add drag-to-reorder for community rail (#2549)
  fix(channels): strip leading hash prefixes from names (#2250)
  fix(desktop): allow skipping harness setup onboarding (#2360)
  Script cleanup
  ...

Signed-off-by: npub1re830n24qhgstulznk5dxdluccspxkxxdq643lm4q3zwwwjgsuqqmcmdrm <1e4f17cd5505d105f3e29da8d337fcc6201358c6683558ff750444e73a488700@buzz.block.builderlab.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant