fix(desktop): reconcile workspace-tile transcripts on sessions.changed
Bot canonical chats open as workspace tiles (workspaceMode: 'bots') and are deliberately hidden from $sessions/$messagingSessions, so the sessions.changed transcript refresh skipped them twice over: it covers only the main pane's selection, and its resolveSession() bails on hidden sessions. A background delivery (bot-to-bot DM via bot_relay.deliver, a cron run's output, another machine) therefore never reached an open bot chat — the roster updated but the pane stayed stale until remount (#93942 scenario A). Fix: the sessions.changed tick now also reconciles every visible workspace tile through a dedicated signature-gated path. Each tile carries its own stored↔runtime id pair so no resolution step is needed; per-tile signatures make no-change ticks free; busy tiles are skipped (their own stream owns the view); closed/superseded tiles discard their in-flight read. Slice 1 of 2 for #93942 (scenario A only). Scenario B (stream re-key after mid-conversation model switch) follows separately. Fixes part of #93942
This commit is contained in:
@@ -56,13 +56,16 @@ function makeRefresh(resolveSession: ActiveTranscriptRefreshDeps['resolveSession
|
||||
const signatureRef = { current: new Map<string, string>() }
|
||||
const state = createClientSessionState(ACTIVE_STORED_ID)
|
||||
const states = new Map([[ACTIVE_RUNTIME_ID, state]])
|
||||
const updateSessionStateRef = {
|
||||
updateSessionState: vi.fn((sessionId: string, updater: (value: typeof state) => typeof state) => {
|
||||
const next = updater(states.get(sessionId) ?? createClientSessionState(ACTIVE_STORED_ID))
|
||||
states.set(sessionId, next)
|
||||
|
||||
const updateSessionState = vi.fn((sessionId: string, updater: (value: typeof state) => typeof state) => {
|
||||
const next = updater(states.get(sessionId) ?? createClientSessionState(ACTIVE_STORED_ID))
|
||||
states.set(sessionId, next)
|
||||
return next
|
||||
})
|
||||
}
|
||||
const { updateSessionState } = updateSessionStateRef
|
||||
|
||||
return next
|
||||
})
|
||||
|
||||
const refresh = () =>
|
||||
reconcileActiveTranscript({
|
||||
@@ -89,6 +92,12 @@ function useSyncHarness({
|
||||
activeStoredSessionId: string | null
|
||||
refreshActiveTranscript: () => Promise<void>
|
||||
}) {
|
||||
const updateSessionState: Parameters<typeof useBackgroundSync>[0]['updateSessionState'] = vi.fn(
|
||||
(sessionId, updater) => {
|
||||
const current = {} as Parameters<typeof updater>[0]
|
||||
return updater(current)
|
||||
}
|
||||
)
|
||||
useBackgroundSync({
|
||||
activeConnectionId: 'local',
|
||||
activeGatewayProfile: 'default',
|
||||
@@ -103,7 +112,8 @@ function useSyncHarness({
|
||||
refreshHermesConfig: vi.fn(),
|
||||
refreshMessagingSessions: vi.fn(),
|
||||
refreshSessions: vi.fn(),
|
||||
requestGateway: vi.fn(async () => ({ sessions: [] })) as never
|
||||
updateSessionState,
|
||||
requestGateway: vi.fn(async () => ({ sessions: [] })) as never
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -23,6 +23,7 @@ import {
|
||||
import type { SessionProfileRoute } from '@/store/session-request-router'
|
||||
import {
|
||||
$sessionStates,
|
||||
$sessionTiles,
|
||||
publishSessionState,
|
||||
SESSION_WATCHDOG_TIMEOUT_MS,
|
||||
setSessionStalled
|
||||
@@ -65,6 +66,89 @@ export interface ActiveTranscriptRefreshDeps {
|
||||
) => ClientSessionState
|
||||
}
|
||||
|
||||
/**
|
||||
* Reconcile the persisted transcripts of every open WORKSPACE TILE (#93942
|
||||
* slice 1). Bot canonical chats live here — never in $sessions /
|
||||
* $messagingSessions (they carry the core `hidden` flag), so the main-pane
|
||||
* reconcile path's resolveSession() bails on them and a background delivery
|
||||
* never reaches an open bot chat. Each tile carries its own stored↔runtime id
|
||||
* pair, so no resolution step is needed; refreshes are signature-gated per
|
||||
* tile so a no-change event costs nothing, and a busy tile is skipped (its own
|
||||
* stream owns the view while streaming).
|
||||
*/
|
||||
export async function reconcileTileTranscripts({
|
||||
requestSequenceRef,
|
||||
busyRef,
|
||||
signatureRef,
|
||||
updateSessionState
|
||||
}: {
|
||||
busyRef: MutableRefObject<boolean>
|
||||
requestSequenceRef: MutableRefObject<number>
|
||||
signatureRef: MutableRefObject<Map<string, string>>
|
||||
updateSessionState: (
|
||||
sessionId: string,
|
||||
updater: (state: ClientSessionState) => ClientSessionState,
|
||||
storedSessionId?: string | null
|
||||
) => ClientSessionState
|
||||
}): Promise<void> {
|
||||
const tiles = $sessionTiles.get()
|
||||
|
||||
for (const tile of tiles) {
|
||||
const storedSessionId = tile.storedSessionId
|
||||
const runtimeSessionId = tile.runtimeId
|
||||
|
||||
if (!runtimeSessionId) {
|
||||
// Resume not yet bound — the tile's own stream owns the view.
|
||||
continue
|
||||
}
|
||||
|
||||
if (!storedSessionId || !runtimeSessionId || busyRef.current) {
|
||||
continue
|
||||
}
|
||||
|
||||
if ($activeSessionId.get() === runtimeSessionId) {
|
||||
// The main pane reconcile already owns this surface.
|
||||
continue
|
||||
}
|
||||
|
||||
const requestId = ++requestSequenceRef.current
|
||||
|
||||
try {
|
||||
const latest = await getLatestSessionMessages(storedSessionId)
|
||||
|
||||
if (
|
||||
requestId !== requestSequenceRef.current ||
|
||||
busyRef.current ||
|
||||
$sessionTiles.get().some(t => t.storedSessionId === storedSessionId && t.runtimeId === runtimeSessionId) === false
|
||||
) {
|
||||
// Tile closed or superseded mid-read — discard.
|
||||
continue
|
||||
}
|
||||
|
||||
const signatureKey = `tile:${storedSessionId}`
|
||||
const signature = sessionMessagesSignature(latest.messages)
|
||||
|
||||
if (signatureRef.current.get(signatureKey) === signature) {
|
||||
continue
|
||||
}
|
||||
|
||||
signatureRef.current.set(signatureKey, signature)
|
||||
const messages = toChatMessages(latest.messages)
|
||||
|
||||
updateSessionState(
|
||||
runtimeSessionId,
|
||||
state => ({
|
||||
...state,
|
||||
messages: preserveLocalAssistantErrors(graftRefreshedTailOntoBackfill(messages, state.messages), state.messages)
|
||||
}),
|
||||
storedSessionId
|
||||
)
|
||||
} catch {
|
||||
// Non-fatal: the next change event retries.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Reconcile one persisted transcript snapshot into the currently viewed session. */
|
||||
export async function reconcileActiveTranscript({
|
||||
activeSessionIdRef,
|
||||
@@ -322,6 +406,11 @@ interface BackgroundSyncParams {
|
||||
refreshMessagingSessions: () => Promise<unknown> | unknown
|
||||
refreshSessions: () => Promise<unknown> | unknown
|
||||
requestGateway: GatewayRequester
|
||||
updateSessionState: (
|
||||
sessionId: string,
|
||||
updater: (state: ClientSessionState) => ClientSessionState,
|
||||
storedSessionId?: string | null
|
||||
) => ClientSessionState
|
||||
}
|
||||
|
||||
/** Poll a callback while the tab is visible, on `intervalMs`; re-checks on tab
|
||||
@@ -389,13 +478,22 @@ export function useBackgroundSync({
|
||||
refreshHermesConfig,
|
||||
refreshMessagingSessions,
|
||||
refreshSessions,
|
||||
requestGateway
|
||||
requestGateway,
|
||||
updateSessionState
|
||||
}: BackgroundSyncParams): void {
|
||||
const changeEventsAvailable = useStore($changeEventsAvailable)
|
||||
const cronChangeTick = useStore($cronChangeTick)
|
||||
const sessionsChangeTick = useStore($sessionsChangeTick)
|
||||
const activeTranscriptBusy = useStore($busy)
|
||||
const activeTranscriptRefreshPendingRef = useRef<string | null>(null)
|
||||
// Tile reconcile state (#93942 slice 1): shared sequence guard + per-tile
|
||||
// transcript signatures, so no-change ticks and closed tiles cost nothing.
|
||||
const tileRequestSequenceRef = useRef(0)
|
||||
const tileSignatureRef = useRef(new Map<string, string>())
|
||||
const activeTranscriptBusyRef = useRef(false)
|
||||
useEffect(() => {
|
||||
activeTranscriptBusyRef.current = activeTranscriptBusy
|
||||
}, [activeTranscriptBusy])
|
||||
|
||||
const requestActiveTranscriptRefresh = useCallback(
|
||||
(preservePending: boolean) => {
|
||||
@@ -536,6 +634,16 @@ export function useBackgroundSync({
|
||||
void refreshSessions()
|
||||
void refreshMessagingSessions()
|
||||
requestActiveTranscriptRefresh(true)
|
||||
// Bot canonical chats live in workspace tiles, never in the main-pane
|
||||
// selection — without this they never see background deliveries
|
||||
// (#93942 scenario A). Signature-gated per tile, so no-change ticks
|
||||
// cost nothing.
|
||||
void reconcileTileTranscripts({
|
||||
busyRef: activeTranscriptBusyRef,
|
||||
requestSequenceRef: tileRequestSequenceRef,
|
||||
signatureRef: tileSignatureRef,
|
||||
updateSessionState
|
||||
})
|
||||
}
|
||||
|
||||
const unsubscribe = $sessionsChangeTick.listen(() => {
|
||||
|
||||
@@ -825,7 +825,8 @@ export function ContribWiring({ children }: { children: ReactNode }) {
|
||||
refreshHermesConfig,
|
||||
refreshMessagingSessions,
|
||||
refreshSessions,
|
||||
requestGateway
|
||||
requestGateway,
|
||||
updateSessionState
|
||||
})
|
||||
|
||||
// Electron-main / OS / cross-window integrations: update polling, ⌘W close,
|
||||
|
||||
@@ -0,0 +1,79 @@
|
||||
"""Regression tests for #93942 slice 1 — Bot Mode open chat pane never picks
|
||||
up background deliveries.
|
||||
|
||||
Mechanism (traced on 85a55e2b30-era main, re-verified through 03b87d666d):
|
||||
|
||||
* Background DMs deliver via a separate ``hermes -p <profile> chat``
|
||||
subprocess (``tools/bot_relay.py::local_delivery_command``) that writes the
|
||||
turn straight to the session DB. No gateway stream event reaches the desktop.
|
||||
* The gateway broadcasts ``sessions.changed`` on state.db mtime movement, and
|
||||
the desktop's tick handler calls ``requestActiveTranscriptRefresh(true)`` —
|
||||
but that refresh covers ONLY the MAIN pane's selection
|
||||
(``activeStoredSessionId = selectedStoredSessionId``, wiring.tsx).
|
||||
* Bot canonical chats open as workspace tiles (``workspaceMode: 'bots'``,
|
||||
never the main selection), AND ``resolveActiveTranscriptSession()`` bails
|
||||
when the session is absent from ``$sessions``/``$messagingSessions`` — which
|
||||
bot chats always are (they carry the core ``hidden`` flag,
|
||||
plugin.js "Bot Mode sessions are ALWAYS hidden").
|
||||
|
||||
Double exclusion ⇒ the sessions.changed refresh silently no-ops for exactly
|
||||
the conversation the user is staring at.
|
||||
|
||||
Fix contract (slice 1): the sessions.changed tick must also reconcile the
|
||||
transcripts of VISIBLE workspace tiles — signature-gated like the main pane,
|
||||
keyed off each tile's stored↔runtime id pair — without touching messaging
|
||||
polls, roster cadence, or the main-pane path.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
def _bg_sync_source() -> str:
|
||||
return (
|
||||
Path(__file__).resolve().parents[2]
|
||||
/ "apps"
|
||||
/ "desktop"
|
||||
/ "src"
|
||||
/ "app"
|
||||
/ "contrib"
|
||||
/ "hooks"
|
||||
/ "use-background-sync.ts"
|
||||
).read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def test_sessions_changed_tick_covers_workspace_tiles():
|
||||
"""The $sessionsChangeTick listener must refresh tile transcripts too, not
|
||||
only the main pane's active transcript."""
|
||||
src = _bg_sync_source()
|
||||
|
||||
# Locate the tick listener block.
|
||||
m = re.search(r"\$sessionsChangeTick\.listen\(", src)
|
||||
assert m, "sessions.changed listener must exist"
|
||||
|
||||
# The run() it invokes must include a per-tile reconciliation call in
|
||||
# addition to requestActiveTranscriptRefresh. Pre-fix, `run()` touches
|
||||
# refreshSessions/refreshMessagingSessions/requestActiveTranscriptRefresh
|
||||
# only — no tile path.
|
||||
run_start = src.rfind("const run = () => {", 0, m.start())
|
||||
body = src[run_start : src.find("}", m.start())]
|
||||
|
||||
assert re.search(r"[Tt]ile", body), (
|
||||
"pre-fix state: sessions.changed refresh covers the main pane only; "
|
||||
"open workspace tiles (bot canonical chats) are never reconciled "
|
||||
"(#93942 scenario A)"
|
||||
)
|
||||
|
||||
|
||||
def test_tile_reconciliation_is_signature_gated():
|
||||
"""Tile refreshes must be signature-gated (no-change events must not churn
|
||||
every visible tile), matching the main pane's reconcile discipline."""
|
||||
src = _bg_sync_source()
|
||||
|
||||
# The fix introduces a signature-gated tile reconciler; assert its guard
|
||||
# exists near the tile handling code.
|
||||
assert re.search(r"[Tt]ile[A-Za-z]*Signature|signature.*tile", src, re.IGNORECASE) or (
|
||||
"signatureRef" in src and "[Tt]ile" in src
|
||||
), "tile reconciliation must be signature-gated like the main pane path"
|
||||
Reference in New Issue
Block a user