From 48deb73ebd76268632065833ead30d32e69b5b89 Mon Sep 17 00:00:00 2001 From: joaomarcos Date: Sun, 30 Aug 2026 17:14:45 -0300 Subject: [PATCH] fix(desktop): latch dead runtime recovery across remounts --- .../app/chat/composer/status-stack/index.tsx | 5 - apps/desktop/src/store/composer-status.ts | 9 ++ apps/desktop/src/store/runtime-gone.test.ts | 149 ++++++++++++++++++ apps/desktop/src/store/runtime-gone.ts | 129 +++++++++++++++ 4 files changed, 287 insertions(+), 5 deletions(-) create mode 100644 apps/desktop/src/store/runtime-gone.test.ts create mode 100644 apps/desktop/src/store/runtime-gone.ts diff --git a/apps/desktop/src/app/chat/composer/status-stack/index.tsx b/apps/desktop/src/app/chat/composer/status-stack/index.tsx index 27ed53c342..93f14c8e52 100644 --- a/apps/desktop/src/app/chat/composer/status-stack/index.tsx +++ b/apps/desktop/src/app/chat/composer/status-stack/index.tsx @@ -21,7 +21,6 @@ import { dismissBackgroundProcess, groupStatusItems, refreshBackgroundProcesses, - resetBackgroundPollingGuard, type StatusGroup, stopBackgroundProcess } from '@/store/composer-status' @@ -106,10 +105,6 @@ export function ComposerStatusStack({ queue, sessionId }: ComposerStatusStackPro // process tool completions) live in use-message-stream. useEffect(() => { if (sessionId) { - // Opening/rebinding a session is a fresh runtime binding: clear any - // gone-latch left by a previous runtime under this id so the poll below - // is allowed to run again (see resetBackgroundPollingGuard). - resetBackgroundPollingGuard(sessionId) void refreshBackgroundProcesses(sessionId) void refreshSessionGoal(sessionId) } diff --git a/apps/desktop/src/store/composer-status.ts b/apps/desktop/src/store/composer-status.ts index 8746b2a0b1..159e7c84e7 100644 --- a/apps/desktop/src/store/composer-status.ts +++ b/apps/desktop/src/store/composer-status.ts @@ -10,6 +10,7 @@ import { $goalsBySession, type GoalStatus } from './goals' import { dispatchNativeNotification } from './native-notifications' import { notifyError } from './notifications' import { $sessions, lineageAliases } from './session' +import { markRuntimeGone, noteRuntimeAlive } from './runtime-gone' import { $sessionStates } from './session-states' import { $subagentsBySession, type SubagentProgress } from './subagents' import { $todosBySession } from './todos' @@ -441,11 +442,19 @@ export async function refreshBackgroundProcesses(sid: string): Promise { const result = await gateway.request<{ processes?: GatewayProcessEntry[] }>('process.list', { session_id: sid }) reconcileBackgroundProcesses(sid, result?.processes ?? []) + // The binding answered, so it is healthy: refund the stored session's + // recovery budget (a heal that stuck must not count against the next one). + noteRuntimeAlive(sid) } catch (error) { // A gone session never comes back under this runtime id: stop polling it, // or the 5s timer hammers the gateway with 4001s for the window's lifetime. if (isSessionGoneForBackgroundPolling(error)) { goneSessions.add(sid) + // Latching stops the storm; it does not make the window usable again. + // This poll is the only caller that runs while the user is idle, so it is + // the only one that can carry the gateway's "resume the stored session" + // verdict to the view before the user types into a dead binding. + markRuntimeGone(sid) return } diff --git a/apps/desktop/src/store/runtime-gone.test.ts b/apps/desktop/src/store/runtime-gone.test.ts new file mode 100644 index 0000000000..c7e5280616 --- /dev/null +++ b/apps/desktop/src/store/runtime-gone.test.ts @@ -0,0 +1,149 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { refreshBackgroundProcesses, resetBackgroundPollingGuard } from './composer-status' +import { $gateway } from './gateway' +import { markRuntimeGone, noteRuntimeAlive, resetRuntimeGoneHealing } from './runtime-gone' +import { $activeSessionId, $sessionResumeRequest } from './session' +import { $sessionStates, $sessionTiles } from './session-states' + +const STORED = 'stored-1' +const RUNTIME = 'runtime-dead' + +/** Minimal cached state: only the durable-identity field this module reads. */ +const cachedState = (storedSessionId: string) => ({ messages: [], storedSessionId }) as never + +const tile = (storedSessionId: string, runtimeId?: string) => ({ runtimeId, storedSessionId }) as never + +beforeEach(() => { + resetRuntimeGoneHealing() + $sessionStates.set({}) + $sessionTiles.set([]) + $activeSessionId.set(null) + $sessionResumeRequest.set(null) +}) + +afterEach(() => { + $gateway.set(null as never) + resetBackgroundPollingGuard() + resetRuntimeGoneHealing() + $sessionStates.set({}) + $sessionTiles.set([]) + $activeSessionId.set(null) + $sessionResumeRequest.set(null) +}) + +describe('markRuntimeGone', () => { + it('unbinds the tile holding the dead runtime so its resume effect refires', () => { + $sessionTiles.set([tile(STORED, RUNTIME), tile('stored-2', 'runtime-live')]) + + expect(markRuntimeGone(RUNTIME)).toBe(true) + + const tiles = $sessionTiles.get() + + // The dead binding is cleared — SessionTilePane's resume effect is gated on + // `!runtimeId`, so this is what re-arms it against the intact stored row. + expect(tiles.find(t => t.storedSessionId === STORED)?.runtimeId).toBeUndefined() + // A healthy neighbour must not be disturbed. + expect(tiles.find(t => t.storedSessionId === 'stored-2')?.runtimeId).toBe('runtime-live') + }) + + it('asks the primary chat to resume when the dead runtime is the one on screen', () => { + $sessionStates.set({ [RUNTIME]: cachedState(STORED) }) + $activeSessionId.set(RUNTIME) + + expect(markRuntimeGone(RUNTIME)).toBe(true) + + // useRouteResume skips on `alreadyActive`, which stays true forever against + // a dead id; only an explicit request bypasses it without a reconnect. + expect($sessionResumeRequest.get()?.sessionId).toBe(STORED) + }) + + it('does not navigate the primary chat for a tile-only dead runtime', () => { + $sessionTiles.set([tile(STORED, RUNTIME)]) + $activeSessionId.set('runtime-of-the-focused-chat') + + expect(markRuntimeGone(RUNTIME)).toBe(true) + expect($sessionResumeRequest.get()).toBeNull() + }) + + it('heals a runtime id exactly once, however many pollers report it', () => { + $sessionStates.set({ [RUNTIME]: cachedState(STORED) }) + $activeSessionId.set(RUNTIME) + + expect(markRuntimeGone(RUNTIME)).toBe(true) + + const firstSequence = $sessionResumeRequest.get()?.sequence + + expect(markRuntimeGone(RUNTIME)).toBe(false) + expect(markRuntimeGone(RUNTIME)).toBe(false) + expect($sessionResumeRequest.get()?.sequence).toBe(firstSequence) + }) + + it('latches without resuming when the runtime has no durable identity', () => { + // A never-persisted draft: no cached state, no tile. Nothing to resume. + expect(markRuntimeGone('runtime-orphan')).toBe(false) + expect($sessionResumeRequest.get()).toBeNull() + }) + + it('caps consecutive heals so a reap-on-sight backend cannot become a resume loop', () => { + $activeSessionId.set(null) + + // Each cycle: the view rebinds a fresh runtime id, which is reaped again. + for (let i = 0; i < 3; i += 1) { + $sessionTiles.set([tile(STORED, `runtime-${i}`)]) + expect(markRuntimeGone(`runtime-${i}`)).toBe(true) + } + + $sessionTiles.set([tile(STORED, 'runtime-3')]) + expect(markRuntimeGone('runtime-3')).toBe(false) + expect($sessionTiles.get()[0]?.runtimeId).toBe('runtime-3') + }) + + it('refunds the budget once a binding proves healthy', () => { + for (let i = 0; i < 3; i += 1) { + $sessionTiles.set([tile(STORED, `runtime-${i}`)]) + markRuntimeGone(`runtime-${i}`) + } + + // The next runtime answers a poll, so the earlier deaths are not a streak. + $sessionStates.set({ 'runtime-healthy': cachedState(STORED) }) + noteRuntimeAlive('runtime-healthy') + + $sessionTiles.set([tile(STORED, 'runtime-4')]) + expect(markRuntimeGone('runtime-4')).toBe(true) + }) +}) + +describe('refreshBackgroundProcesses recovery', () => { + it('carries the gateway 4001 verdict to the view instead of only going quiet', async () => { + $sessionTiles.set([tile(STORED, RUNTIME)]) + $sessionStates.set({ [RUNTIME]: cachedState(STORED) }) + $activeSessionId.set(RUNTIME) + $gateway.set({ + request: vi.fn(async () => { + throw new Error('session not found') + }) + } as never) + + await refreshBackgroundProcesses(RUNTIME) + + // Before this fix the poll latched and the window stayed bound to the + // phantom runtime for the rest of its life — quiet, and still unusable. + expect($sessionTiles.get()[0]?.runtimeId).toBeUndefined() + expect($sessionResumeRequest.get()?.sessionId).toBe(STORED) + }) + + it('leaves a transient failure alone — the binding may still be alive', async () => { + $sessionTiles.set([tile(STORED, RUNTIME)]) + $gateway.set({ + request: vi.fn(async () => { + throw new Error('request timed out after 30s: process.list') + }) + } as never) + + await refreshBackgroundProcesses(RUNTIME) + + expect($sessionTiles.get()[0]?.runtimeId).toBe(RUNTIME) + expect($sessionResumeRequest.get()).toBeNull() + }) +}) diff --git a/apps/desktop/src/store/runtime-gone.ts b/apps/desktop/src/store/runtime-gone.ts new file mode 100644 index 0000000000..c9b6ae81d5 --- /dev/null +++ b/apps/desktop/src/store/runtime-gone.ts @@ -0,0 +1,129 @@ +import { $activeSessionId, requestSessionResume } from './session' +import { $sessionStates, $sessionTiles, unbindTileRuntime } from './session-states' + +/** Heal a session view whose bound runtime id the gateway no longer holds. + * + * The desktop learns a runtime is gone through two channels, and only one of + * them ever repaired anything: + * + * - PUSH — `session.reclaimed`. `gateway-event/lifecycle.ts` drops the cached + * state and calls `unbindTileRuntime`, which re-arms SessionTilePane's + * resume effect (gated on `!runtimeId`) so the tile rebinds a fresh runtime + * from the intact stored row. + * - PULL — a session-scoped RPC rejected `4001 "session not found"`. The + * gateway logs "client should resume the stored session" precisely because + * this is the terminal verdict; `_sess_nowait` has no other way to say it. + * + * Every user action already honours the pull verdict: submit, slash, rewind, + * interrupt and the attaches run through `withSessionNotFoundResume`, which + * resumes the stored id and rebinds. The background pollers — the only callers + * that run while the user is NOT acting — did not. They either re-sent the dead + * id forever or (with the gone-latch) went silent against it, and in both cases + * the view stayed bound to a phantom runtime for the rest of its life. + * + * That gap is only reachable through the pull channel. The push channel cannot + * cover a runtime that died with a previous app process (boot-restore), was + * reaped while this client was disconnected, or was reaped by a remote gateway + * this renderer had not yet dialled — the broadcast has no live listener. In + * those cases the 4001 is the *only* notice that ever arrives. + * + * So route it to the same recovery the broadcast drives. Both surfaces that can + * hold a binding get their existing re-arm lever pulled: + * + * - Tiles: `unbindTileRuntime` (SessionTilePane's resume effect refires). + * - The primary chat: `requestSessionResume`, the explicit-request lever. Its + * route-resume effect skips on `alreadyActive` — route === selected and the + * cached runtime === the active one — which stays true forever against a + * dead id, and only `explicitlyRequested` bypasses it without a reconnect. + */ + +/** Runtime ids already healed. A reaped runtime id is dead permanently, and a + * successful heal binds a NEW one, so this never needs clearing: a second heal + * for the same id could only come from a duplicate report of the same death. */ +const healedRuntimes = new Set() + +/** Consecutive heals per stored session id, reset by {@link noteRuntimeAlive}. + * A backend that reaps as fast as we resume would otherwise turn this into the + * very storm it exists to stop — one resume per poll tick, forever. Cap it and + * let the user's next action (which carries its own recovery) take over. */ +const healsByStoredId = new Map() + +/** Enough to ride out a reap that races a resume, low enough that a backend + * reaping on sight cannot be turned into a resume loop. */ +const MAX_CONSECUTIVE_HEALS = 3 + +/** Resolve the durable identity behind a runtime id. The cached session state is + * authoritative; a tile that resumed before the state landed is the fallback. */ +function storedIdForRuntime(runtimeId: string): null | string { + const cached = $sessionStates.get()[runtimeId]?.storedSessionId + + if (cached) { + return cached + } + + return $sessionTiles.get().find(tile => tile.runtimeId === runtimeId)?.storedSessionId ?? null +} + +/** A poll against `runtimeId` succeeded — the binding is healthy, so the stored + * session's heal budget is spent on real deaths only, not on one bad stretch. */ +export function noteRuntimeAlive(runtimeId: string): void { + if (healsByStoredId.size === 0) { + return + } + + const storedId = storedIdForRuntime(runtimeId) + + if (storedId) { + healsByStoredId.delete(storedId) + } +} + +/** Report the gateway's terminal verdict for `runtimeId` and re-arm whichever + * surface holds the binding. Returns true when a recovery was requested. + * + * Safe to call on every 4001: it is idempotent per runtime id, and the levers + * it pulls are themselves bounded (the tile resume effect single-flights + * through `resumingRef` and latches its own terminal errors; the route resume + * backs off over `MAX_RESUME_RETRIES`). */ +export function markRuntimeGone(runtimeId: string): boolean { + if (!runtimeId || healedRuntimes.has(runtimeId)) { + return false + } + + healedRuntimes.add(runtimeId) + + const storedId = storedIdForRuntime(runtimeId) + + if (!storedId) { + // No durable identity to resume from — a never-persisted draft, or a + // runtime whose view is already gone. Latching alone is the whole fix. + return false + } + + const heals = healsByStoredId.get(storedId) ?? 0 + + if (heals >= MAX_CONSECUTIVE_HEALS) { + return false + } + + healsByStoredId.set(storedId, heals + 1) + + // Tiles: clearing the binding re-arms the resume effect. A no-op when no tile + // holds this runtime. + unbindTileRuntime(runtimeId) + + // The primary chat: only an explicit request gets past its `alreadyActive` + // skip. Scoped to the runtime the primary is actually showing, so a tile's + // dead runtime never navigates the main view. + if ($activeSessionId.get() === runtimeId) { + requestSessionResume(storedId) + } + + return true +} + +/** Tests only: forget every heal so cases start from a clean slate. */ +export function resetRuntimeGoneHealing(): void { + healedRuntimes.clear() + healsByStoredId.clear() +}