fix(desktop): scope observer feed by channel and add session-boundary dividers#1634
Merged
wpfleger96 merged 10 commits intoJul 8, 2026
Merged
Conversation
9628fe6 to
31e9bfe
Compare
… dividers Two observer-feed defects fixed in a single PR: Slice 1 — channel-scoped archive (index at ingest + idempotent backfill) The archive read path queried all owner_p kind 24200 rows for an identity, then relied on a display-time scopeByChannel filter. When the filter was missing or skipped, frames from other channels leaked into the current channel's observer feed. Fix: add an observer_channel_index table that maps (identity, relay, event_id) to channel_id. Three new Tauri commands drive this: - read_archived_observer_events_for_channel: channel-scoped paginated read via the index (replaces the raw owner_p scan in useLoadArchivedObserverEvents) - index_observer_channel_id: TS calls this after decrypting events to write index entries - read_unindexed_observer_rows: returns all owner_p kind 24200 rows not yet indexed, for the one-shot idempotent backfill Backfill strategy (choice i, backfill-before-read): a React.useEffect in useLoadArchivedObserverEvents runs once on mount, reads all unindexed rows, decrypts each, and batch-writes index entries. After backfill the channel index is authoritative — the scoped read pages it directly with no zero-match raw-page guard needed. Null/decrypt-failed channelId rows stay unscoped and are hidden from every scoped channel view (Will's ruling: option (a) — display decision only, rows stay on disk). Slice 2 — session-boundary rendering Items rendered flat with no session grouping; archived and live frames interleaved without any visual marker of where one agent session ended and another began, causing users to think archived historical context is still live. Fix: split TranscriptItem[] into contiguous session runs before calling buildTranscriptDisplayBlocks. This also fixes the pendingSystemPrompt single-slot clobber (each run gets its own lifecycle). A new session-boundary display block kind is interleaved between runs; SessionBoundaryDivider renders a horizontal rule with session start time. No boundary emitted for single-session transcripts. Liveness — "current session" gating Track latest-live-session-id per (normalized agent pubkey, channelId) in observerRelayStore, set on the handleRelayObserverEvent path, cleared in resetAgentObserverStore. Label a boundary "current session" only when it is both the newest-visible session AND equals that latest-live id; otherwise label it "most recent observed session". Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
31e9bfe to
b50a4cb
Compare
…sionRuns The first-turn wire sequence emitted by the harness is: turn_started(sessionId=null) → session/new(sessionId=null) → session_resolved(sessionId=X) → session/prompt(sessionId=X) The previous splitIntoSessionRuns opened a synthetic 'unknown' run for the leading null-session items, then opened a second run when session_resolved arrived. This caused two bugs: 1. Leaked system prompt: the session/new metadata had no following user prompt in its 'unknown' run so pendingSystemPrompt was never consumed and emitted as a standalone 'System prompt' row. Caught by Desktop Smoke E2E (3), observer-feed-screenshots.spec.ts:726 (expected count 0, got 1). 2. Spurious session boundary: two runs from one session triggered buildTranscriptDisplayBlocks to inject a session-boundary block on a feed that has exactly one session — the opposite of the signal this PR adds. Fix: buffer pre-resolution null-session items and prepend them to the first run that has a non-null sessionId. Only if the entire stream has no resolved session do they form a fallback 'unknown' run. Mid-stream null-session items (after resolution) continue to attribute to the current run as before. Added two regression tests: - firstTurnSequence_noStandaloneSystemPrompt: asserts zero standalone session/new blocks, zero session-boundary blocks, system prompt present inside the turn block (prompt bundle). - genuineSecondSession_boundaryPreserved: asserts the deferral fix does not collapse two genuinely distinct sessions — boundary is still injected. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Both openAgentSession and selectAgentSession were calling setOpenAgentSessionChannelId(channelId ?? null). When the activity-list opener supplies no explicit channelId, the null propagated into sessionChannelId, which caused scopeByChannel to skip filtering entirely (its null fast-path returns all items). Combined with the shared per-pubkey store, this let every channel's live frames appear in any channel's feed. Fix: fall back to activeChannelId in both callbacks so opening from within a channel always resolves a non-null scope key. activeChannelId is already in the useChannelAgentSessions parameter list; add it to both callbacks' dependency arrays. Also fix the archive-load enabled gate: useLoadArchivedObserverEvents was gated on isLive, so an idle agent's channel showed no archived observer history. Change the enable condition to Boolean(sessionChannelId) so archive history loads whenever a channel scope is resolved, regardless of whether the agent is currently running. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…set paging on channel switch F7 — idle agent archived history not renderable (CRITICAL): getAgentObserverSnapshot and getAgentTranscript were early-returning empty when enabled=false, discarding ingested archived events. The enabled flag conflated two concerns: subscribing to the live relay, and reading stored data. Only the relay subscription should gate on isLive/hasObserver. Fix: drop the !enabled early-return in both store readers; keep !agentPubkey as the only guard. The enabled parameter is retained for call-site compatibility (renamed to _enabled) but no longer controls store reads. The relay subscription useEffect in useObserverEvents still gates on enabled so idle agents don't trigger unnecessary relay connections. Also fix the EmptyObserverState gate in ManagedAgentSessionPanel: previously shown whenever !hasObserver regardless of content, now only shown when the agent is idle AND there is nothing to display (transcript.length === 0 && events.length === 0). This allows archived channel history to render for idle agents instead of showing the empty state. F8 — archive paging state not scoped to channel (IMPORTANT): useLoadArchivedObserverEvents held hasOlderArchived, cursorRef, and isFetchingRef in a single hook instance while channelId changed across channel switches in the same AgentSessionThreadPanel lifecycle. Channel A's exhausted state and cursor bled into channel B — B would not page, or B's first read would skip rows newer than A's cursor. Fix: add a useEffect([channelId]) that resets cursorRef to null, isFetchingRef to false, and hasOlderArchived to true on channel change. Backfill state (backfillStatusRef/backfillPromiseRef/backfillResolveRef) is identity-level and deliberately NOT reset — backfill covers all channels and only needs to run once per identity mount. Tests: two new regression cases in ingestArchivedObserverEvents.test.mjs: - test_idle_agent_archived_events_readable_when_enabled_false: idle agent with archived rows for two channels reads both (enabled=false no longer blocks), and scopeByChannel returns only channel-A frames for channel A with zero channel-B contamination. - test_channel_switch_resets_cursor_and_exhaustion: verifies the paging state-machine semantics — channel A exhausted then channel switch resets cursor/hasOlderArchived/isFetching. Plus a spec test confirming backfill state fields are identity-scoped and must not reset on channel switch. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
… regression The two archive-paging-reset tests in ingestArchivedObserverEvents.test.mjs were tautological: they reassigned local let variables and asserted on those same reassignments, not on any production behavior. The tests passed even if the useEffect([channelId]) reset in useLoadArchivedObserverEvents was deleted. Fix: extract the paging state machine into archivePagingState.ts with two pure functions (createArchivePagingState, applyChannelReset). The hook imports and calls these; tests import the same functions directly. Replace the tautological tests with: 1. Three unit tests in ingestArchivedObserverEvents.test.mjs that call createArchivePagingState() and applyChannelReset() — the real production functions, not local copies. These catch behavioral regressions in the state machine itself. 2. Two hook-lifecycle tests in archivePagingReset.test.mjs that mount a React component (via the same DOM shim used in MessageComposerDraftImagePersist.test.mjs), drive channel A to exhaustion, re-render with channel B, and assert B starts fresh via the hook's own reactive state. These tests fail when the useEffect([channelId]) body is deleted — verified before commit: removing applyChannelReset(ps) from the effect causes cursor/hasOlderArchived to remain stale after the channel switch (AssertionError: cursor must reset to null after channel switch (A->B)). The hook's useEffect([channelId]) body is unchanged in behavior: it calls applyChannelReset(ps) which sets cursor=null, isFetching=false, hasOlderArchived=true — identical to the prior inline mutations of cursorRef.current/isFetchingRef.current. Backfill state is not touched by applyChannelReset, preserving the identity-level semantics Paul and Thufir confirmed. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…init Remove archivePagingReset.test.mjs — the HarnessHook lifecycle test was a copy of the production effect testing itself, not the real useLoadArchivedObserverEvents hook. The pure applyChannelReset / createArchivePagingState unit tests in ingestArchivedObserverEvents .test.mjs are sufficient and call the real extracted functions. Fix lazy-init MINOR: useRef(createArchivePagingState()) was building a throwaway ArchivePagingState (including a pending Promise) on every render after the first. Use a nullable ref with an if (!ref.current) guard instead. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…unIndex
Same sessionId can produce two non-contiguous runs when archived history
for session A is followed by session B, then live session A re-resolves.
splitIntoSessionRuns treats each sessionId change as a new run, so session
A appears at runIndex 0 and runIndex 2 — but getDisplayBlockKey keyed on
session-boundary:${sessionId} only, producing two identical React keys.
React responded by duplicating or omitting children, manifesting as the
observable cross-channel contamination symptom (wrong session's blocks
appearing in another channel's feed).
Fix: add runIndex (the session run's position in the run array) to
SessionBoundaryBlock and key on session-boundary:${sessionId}:${runIndex}.
runIndex is always ≥ 1 for boundary blocks and is unique per boundary by
construction, so identical sessionIds on non-contiguous runs now produce
distinct keys.
Strips the [PROBE] console.log instrumentation from observerRelayStore.ts
and AgentSessionThreadPanel.tsx (those were uncommitted diagnostic helpers
added for the #1634 runtime gate; no probe residue remains).
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
… dup-key test
The zero-slides fallback in ProfileLiveActivityEmbed was rendering
ManagedAgentSessionPanel with channelId={null}, causing scopeByChannel to
return all frames unfiltered — every channel's activity, most-recent-wins.
This produced the cross-channel 'most recent wins' leak Will observed.
Fix: thread callerChannelId (the channel from which the profile was opened)
from ChannelPane → UserProfilePanel → ProfileSummaryView →
ProfileInfoTabContent → ProfileLiveActivityEmbed, and pass it as channelId
to the fallback ManagedAgentSessionPanel. Other callers of UserProfilePanel
that have no channel context (HomeView, PulseScreen, AgentsScreen) default
to null, preserving existing behavior.
Also reshapes the regression test for the duplicate-key fix (726c161):
the A→B→A fixture emitted only one sess-A boundary (not two), so it did not
recreate the actual collision. The new B→A→C→A fixture produces two
boundaries both carrying sessionId=sess-A at runIndex=1 and runIndex=3,
matching the observed crash key session-boundary:ses_fe46476c16d031bf.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…usel to caller channel Two remaining leak surfaces on #1634: 1. ChannelPane passed the raw openAgentSessionChannelId to AgentSessionThreadPanel even when activeChannel.id != openAgentSessionChannelId. The channel prop already nulled on mismatch (interrupt guard), but channelId was still stale, so sessionChannelId = channelId (test-3) leaked the wrong channel's content and badge. Fix: when mismatch, pass activeChannelId instead so the panel re-scopes to the current channel. 2. resolveActivityChannelId in ProfileLiveActivityEmbed used feedScope.preferredChannelId (globally most-recently-active) as the preferred resolver, so the carousel opened on whichever channel was most recently active across all sessions — not the channel the profile was opened from. Fix: prefer callerChannelId (already threaded from ChannelPane by the zero-slides fix) so the carousel defaults to the caller's channel when that channel has a slide. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…nelId props Bug 3's fix rebased channelId to activeChannelId on a stale openAgentSessionChannelId mismatch, but the adjacent channel prop still passed null for the same case. This split the panel's effective channel across two values: sessionChannelId (from channelId) resolved to the current channel and correctly scoped the feed and header badge, but handleInterruptTurn early-returned on !channel so an agent actively working in the now-active channel could show an enabled Stop action that silently did nothing. Fix: derive effectiveAgentSessionChannelId once (the same mismatch expression previously inline on channelId) and feed both channelId and channel from it. In the mismatch case effectiveAgentSessionChannelId === activeChannel.id (guaranteed because this IIFE only runs inside the activeChannel && selectedAgent branch), so channel passes activeChannel instead of null. channel and channelId now always express a single consistent scope. Three-case walk: - mismatch: effectiveAgentSessionChannelId = activeChannelId, channel = activeChannel, Stop operates on the current channel - match: effectiveAgentSessionChannelId = openAgentSessionChannelId, channel = activeChannel (same as before, channel id matches) - null open: effectiveAgentSessionChannelId = null, channel falls through to the isAgentInActivityList guard (unchanged) Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes three observer-feed defects introduced when the local archive feature started stitching archived kind 24200 frames into the live observer feed:
Channel contamination — archived frames from other channels appeared in a channel's observer feed. Fixed by indexing
channelIdat ingest and using it as the archive query key (owner_prows backfilled idempotently).Missing session boundaries — archived + live frames rendered flat with no visual marker separating sessions. Fixed by splitting the transcript into session runs in
buildTranscriptDisplayBlocksand injectingsession-boundarydivider blocks between runs. The newest run is labeled "Current session" when it matches the live session id, or "Most recent observed session — not in current context" otherwise; older runs are labeled "Earlier session — not in current context".Duplicate React keys on session boundaries — a session can produce two non-contiguous runs (archived session A, then session B, then live session A re-resolving). Both boundary blocks had identical keys (
session-boundary:${sessionId}), causing React to duplicate/omit children — the visible symptom of the apparent cross-channel leak. Fixed by addingrunIndex(the run's position in the ordered run array) toSessionBoundaryBlockand keying onsession-boundary:${sessionId}:${runIndex}.Changes
observerRelayStore.ts— indexeschannelIdat live-frame ingest; tracks latest-live-session-id per(agent, channel)pairuseObserverEvents.ts/ archive query path — scopes archive reads tochannelIdagentSessionPanelLayout.ts—scopeByChannelfilteragentSessionTranscriptGrouping.ts—splitIntoSessionRuns,buildTranscriptDisplayBlocks,SessionBoundaryBlocktype (addsrunIndex)AgentSessionTranscriptList.tsx—getDisplayBlockKeyusessession-boundary:${sessionId}:${runIndex}agentSessionTranscriptGrouping.test.mjs— tests for session-boundary injection, label states, null-session deferral, and the non-contiguous duplicate-key regression