diff --git a/apps/desktop/src/app/chat/session-tile-gone.test.ts b/apps/desktop/src/app/chat/session-tile-gone.test.ts deleted file mode 100644 index 70e55ad54b..0000000000 --- a/apps/desktop/src/app/chat/session-tile-gone.test.ts +++ /dev/null @@ -1,195 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' - -import { getSession } from '@/hermes' -import { probeStoredSession } from '@/app/session/hooks/use-session-actions/utils' -import { setApiRequestConnection } from '@/api/client' -import { stashSessionDraft, clearSessionDraft, takeSessionDraft } from '@/store/composer' -import { $gatewaySwitching } from '@/store/gateway-switch' -import { $activeGatewayProfile, $profiles } from '@/store/profile' -import { $connection, $gatewayState, $sessions } from '@/store/session' -import { $sessionTiles, openSessionTile, reopenLastClosedTile, closeSessionTile } from '@/store/session-states' - -import { startUnrestoredTileTitleBackfill } from './session-tile' - -vi.mock('@/hermes', async importActual => ({ - ...(await importActual()), - getSession: vi.fn() -})) - -const get = vi.mocked(getSession) -let stop: (() => void) | undefined - -beforeEach(() => { - $gatewayState.set('idle') - $activeGatewayProfile.set('default') - $profiles.set([{ name: 'default' }, { name: 'writer' }] as never) - $connection.set({ connectionId: 'local', mode: 'local' } as never) - $sessions.set([]) - $sessionTiles.set([]) - get.mockReset() -}) - -afterEach(() => { - stop?.() - $gatewayState.set('idle') - $sessionTiles.set([]) - $sessions.set([]) - $profiles.set([]) - $connection.set(null) - $gatewaySwitching.set(false) - clearSessionDraft('deleted-chat') - window.localStorage.clear() -}) - -describe('restored dead tile backfill', () => { - it('does not resurrect a dead tile ahead of a user-closed tab on reopen', async () => { - openSessionTile('user-closed') - closeSessionTile('user-closed') - openSessionTile('deleted-chat') - get.mockRejectedValue(new Error('404: Session not found')) - stop = startUnrestoredTileTitleBackfill() - $gatewayState.set('open') - await vi.waitFor(() => expect($sessionTiles.get()).toEqual([])) - reopenLastClosedTile() - expect($sessionTiles.get().map(tile => tile.storedSessionId)).toEqual(['user-closed']) - }) - - it('releases scope subscriptions on cancellation even while lookup is pending', async () => { - openSessionTile('deleted-chat') - const released = vi.fn() - const listen = $connection.listen.bind($connection) - const spy = vi.spyOn($connection, 'listen').mockImplementation(listener => { - const off = listen(listener) - return () => { released(); off() } - }) - let resolve!: (result: { status: 'gone' }) => void - const pending = new Promise<{ status: 'gone' }>(yes => { resolve = yes }) - try { - stop = startUnrestoredTileTitleBackfill(() => pending) - $gatewayState.set('open') - expect(spy).toHaveBeenCalledTimes(1) - stop() - expect(released).toHaveBeenCalledTimes(1) - resolve({ status: 'gone' }) - await pending - await new Promise(yes => setTimeout(yes, 0)) - expect(released).toHaveBeenCalledTimes(1) - expect($sessionTiles.get()).toHaveLength(1) - } finally { - resolve({ status: 'gone' }) - spy.mockRestore() - } - }) - - it('persists each dead tile removal without dropping a surviving owned tile', async () => { - openSessionTile('gone-one') - openSessionTile('gone-two') - openSessionTile('survivor', 'right', undefined, undefined, { - workspaceMode: 'sessions', ownerRoute: { connectionId: 'local', profile: 'writer' } - }) - get.mockImplementation(async (id: string) => { - if (id === 'survivor') return { id, title: 'Still here' } as never - throw new Error('404: Session not found') - }) - stop = startUnrestoredTileTitleBackfill() - $gatewayState.set('open') - await vi.waitFor(() => expect($sessionTiles.get().map(tile => tile.storedSessionId)).toEqual(['survivor'])) - const persisted = window.localStorage.getItem('hermes.desktop.sessionTiles.v2') ?? '' - expect(persisted).toContain('survivor') - expect(persisted).not.toContain('gone-one') - expect(persisted).not.toContain('gone-two') - expect($sessionTiles.get()[0].ownerRoute).toEqual({ connectionId: 'local', profile: 'writer' }) - }) - it('uses the real REST scope ladder and preserves caller-selected ownership', async () => { - const actual = await vi.importActual('@/hermes') - const previous = window.hermesDesktop - const api = vi.fn(async (_request: { path: string }) => { throw new Error('404: {"detail":"Session not found"}') }) - window.hermesDesktop = { ...previous, api } as never - setApiRequestConnection('local') - get.mockImplementation(actual.getSession) - try { - expect(await probeStoredSession('deleted-chat')).toEqual({ status: 'gone' }) - expect(api.mock.calls.map(([request]) => (request as { path: string }).path)).toEqual([ - '/api/sessions/deleted-chat?profile=default', '/api/sessions/deleted-chat?profile=writer' - ]) - api.mockClear() - expect(await probeStoredSession('deleted-chat', { connectionId: 'remote', profile: 'alias', targetProfile: 'actual' })).toEqual({ status: 'gone' }) - expect(api).toHaveBeenCalledExactlyOnceWith(expect.objectContaining({ connectionId: 'remote', profile: 'actual', path: '/api/sessions/deleted-chat?profile=actual' })) - } finally { - window.hermesDesktop = previous - setApiRequestConnection(null) - } - }) - - it.each(['500: Session not found', '404: endpoint unavailable', 'network disconnected'])( - 'keeps inconclusive misses: %s', - async message => { - openSessionTile('deleted-chat') - get.mockRejectedValueOnce(new Error(message)).mockRejectedValue(new Error('404: Session not found')) - stop = startUnrestoredTileTitleBackfill() - $gatewayState.set('open') - await vi.waitFor(() => expect(get).toHaveBeenCalledTimes(2)) - expect($sessionTiles.get()).toHaveLength(1) - } - ) - - it('preserves a working cross-profile fallback and fills its title', async () => { - openSessionTile('deleted-chat') - get - .mockRejectedValueOnce(new Error('network disconnected')) - .mockResolvedValueOnce({ id: 'deleted-chat', title: 'Found' } as never) - stop = startUnrestoredTileTitleBackfill() - $gatewayState.set('open') - await vi.waitFor(() => expect($sessions.get()[0]?.title).toBe('Found')) - expect($sessionTiles.get()).toHaveLength(1) - }) - - it.each(['draft', 'switch', 'profile ABA', 'cancel', 'bound', 'no inventory', 'other backend'])( - 'preserves tiles when unsafe: %s', - async reason => { - openSessionTile( - 'deleted-chat', - 'right', - undefined, - undefined, - reason === 'other backend' ? { workspaceMode: 'sessions', ownerRoute: { connectionId: 'remote', profile: 'writer' } } : undefined - ) - if (reason === 'draft') stashSessionDraft('deleted-chat', 'keep my words', []) - if (reason === 'switch') $gatewaySwitching.set(true) - if (reason === 'no inventory') $profiles.set([]) - let reject!: (error: Error) => void - get.mockRejectedValue(new Error('404: Session not found')).mockImplementationOnce( - () => - new Promise((_resolve, no) => { - reject = no - }) - ) - stop = startUnrestoredTileTitleBackfill() - $gatewayState.set('open') - if (reason === 'profile ABA') { - $activeGatewayProfile.set('writer') - $activeGatewayProfile.set('default') - } - if (reason === 'cancel') stop() - if (reason === 'bound') $sessionTiles.set($sessionTiles.get().map(tile => ({ ...tile, runtimeId: 'live' }))) - reject(new Error('404: Session not found')) - await vi.waitFor(() => - expect(get).toHaveBeenCalledTimes(reason === 'other backend' || reason === 'no inventory' ? 1 : 2) - ) - expect($sessionTiles.get()).toHaveLength(1) - if (reason === 'draft') expect(takeSessionDraft('deleted-chat').text).toBe('keep my words') - } - ) - - it('persistently closes an empty tile after every profile confirms the session is gone', async () => { - openSessionTile('deleted-chat') - get.mockRejectedValue(new Error('404: Session not found')) - stop = startUnrestoredTileTitleBackfill() - $gatewayState.set('open') - - await vi.waitFor(() => expect(get).toHaveBeenCalledTimes(2)) - await vi.waitFor(() => expect($sessionTiles.get()).toEqual([])) - expect(window.localStorage.getItem('hermes.desktop.sessionTiles.v2') ?? '').not.toContain('deleted-chat') - expect(get.mock.calls.map(call => call[1])).toEqual(['default', 'writer']) - }) -}) diff --git a/apps/desktop/src/app/chat/session-tile.test.ts b/apps/desktop/src/app/chat/session-tile.test.ts index f10228fac3..225c635598 100644 --- a/apps/desktop/src/app/chat/session-tile.test.ts +++ b/apps/desktop/src/app/chat/session-tile.test.ts @@ -1,8 +1,12 @@ -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { HermesConnection } from '@/global' +import { getSession } from '@/hermes' +import { clearSessionDraft, stashSessionDraft } from '@/store/composer' +import { $gatewaySwitching } from '@/store/gateway-switch' +import { $activeGatewayProfile, $profiles } from '@/store/profile' import { $connection, $gatewayState, $sessions, setSessions } from '@/store/session' -import { $sessionTiles, type SessionTile } from '@/store/session-states' +import { $sessionTiles, openSessionTile, reopenLastClosedTile, type SessionTile } from '@/store/session-states' import { sessionTileResumeFailure, @@ -13,6 +17,11 @@ import { WRONG_BACKEND_TILE_ERROR } from './session-tile' +vi.mock('@/hermes', async importOriginal => ({ + ...(await importOriginal>()), + getSession: vi.fn() +})) + function localConnection(): HermesConnection { return { baseUrl: 'http://127.0.0.1:9119', @@ -175,3 +184,110 @@ describe('startUnrestoredTileTitleBackfill (#94167)', () => { stop() }) }) + +describe('startUnrestoredTileTitleBackfill retires dead tiles (#125678)', () => { + const NOT_FOUND = '404: {"detail":"Session not found"}' + const get = vi.mocked(getSession) + let stop: (() => void) | undefined + + beforeEach(() => { + $gatewayState.set('idle') + $activeGatewayProfile.set('default') + // Connection first: a connection change re-scopes the profile inventory. + $connection.set({ connectionId: 'local', mode: 'local' } as never) + $profiles.set([{ name: 'default' }, { name: 'writer' }] as never) + get.mockReset() + }) + + afterEach(() => { + stop?.() + $gatewayState.set('idle') + $gatewaySwitching.set(false) + $sessionTiles.set([]) + $profiles.set([]) + $connection.set(null) + setSessions([]) + clearSessionDraft('dead-chat') + window.localStorage.clear() + }) + + it('drops a restored tile once every profile answered 404 in calm conditions, off the reopen stack', async () => { + openSessionTile('dead-chat') + get.mockRejectedValue(new Error(NOT_FOUND)) + + stop = startUnrestoredTileTitleBackfill() + $gatewayState.set('open') + + await vi.waitFor(() => expect($sessionTiles.get()).toEqual([])) + expect(get.mock.calls.map(call => call[1])).toEqual(['default', 'writer']) + expect(window.localStorage.getItem('hermes.desktop.sessionTiles.v2') ?? '').not.toContain('dead-chat') + reopenLastClosedTile() + expect($sessionTiles.get()).toEqual([]) + }) + + it.each([ + '500 on one profile', + 'network failure on one profile', + 'gateway switch in flight', + 'profile A→B→A while the probes are out', + 'connection switch while the probes are out', + 'stashed draft text', + 'profile inventory not loaded' + ])('keeps the tile when absence is not conclusive: %s', async reason => { + openSessionTile('dead-chat') + + if (reason === 'gateway switch in flight') { + $gatewaySwitching.set(true) + } + + if (reason === 'stashed draft text') { + stashSessionDraft('dead-chat', 'keep my words', []) + } + + if (reason === 'profile inventory not loaded') { + $profiles.set([]) + } + + let settleFirst!: (error: Error) => void + + const first = new Promise((_resolve, reject) => { + settleFirst = reject + }) + + get.mockRejectedValue(new Error(NOT_FOUND)).mockImplementationOnce(() => first) + + stop = startUnrestoredTileTitleBackfill() + $gatewayState.set('open') + await vi.waitFor(() => expect(get).toHaveBeenCalled()) + + if (reason === 'profile A→B→A while the probes are out') { + $activeGatewayProfile.set('writer') + $activeGatewayProfile.set('default') + } + + if (reason === 'connection switch while the probes are out') { + $connection.set({ connectionId: 'remote', mode: 'remote' } as never) + $connection.set({ connectionId: 'local', mode: 'local' } as never) + } + + settleFirst( + new Error( + reason === '500 on one profile' + ? '500: {"detail":"Session not found"}' + : reason === 'network failure on one profile' + ? 'net::ERR_CONNECTION_REFUSED' + : NOT_FOUND + ) + ) + + // Let the ladder finish (and the retire branch run) before asserting. A + // connection switch re-scopes the inventory, so its ladder stops at one rung. + const rungs = ['profile inventory not loaded', 'connection switch while the probes are out'].includes(reason) + ? 1 + : 2 + + await vi.waitFor(() => expect(get).toHaveBeenCalledTimes(rungs)) + await new Promise(resolve => setTimeout(resolve, 0)) + expect($sessionTiles.get().map(tile => tile.storedSessionId)).toEqual(['dead-chat']) + }) +}) diff --git a/apps/desktop/src/app/chat/session-tile.tsx b/apps/desktop/src/app/chat/session-tile.tsx index 20f14dcafe..8ab87e4aea 100644 --- a/apps/desktop/src/app/chat/session-tile.tsx +++ b/apps/desktop/src/app/chat/session-tile.tsx @@ -624,15 +624,15 @@ export function tileStoredRow(storedSessionId: string): SessionInfo | undefined * A restored background tab has no runtimeId and does not mount its pane, so * the resolution effect above never runs; when its row is outside the recents * page and project tree, `tileTitle()` reads "New session" until first click. - * The shared probe upserts found rows for the tab strip. An empty restored - * tile with authoritative all-profile absence is persistently closed; drafts - * and inconclusive/switching lookups stay recoverable. */ + * The probe upserts a found row into `$sessions`, which the tab strip already + * watches. A tile whose id every profile answered 404 for is retired + * (#125678): left alone it is re-probed on every launch and never heals. */ export function startUnrestoredTileTitleBackfill(lookup = probeStoredSession): () => void { - // Only tiles present at startup can be retired: a newly created unbound - // draft may legitimately have no durable row yet. - const restored = new Set($sessionTiles.get().filter(tile => !tile.runtimeId)) - const pendingCleanups = new Set<() => void>() - let cancelled = false + // Only tiles restored from a previous run can be retired: a draft opened in + // this run before the gateway answers has no durable row yet either. + const restored = new Set($sessionTiles.get().flatMap(tile => (tile.runtimeId ? [] : [tile.storedSessionId]))) + let stopped = false + const run = () => { if ($gatewayState.get() !== 'open') { return @@ -640,60 +640,55 @@ export function startUnrestoredTileTitleBackfill(lookup = probeStoredSession): ( off() - for (const tile of $sessionTiles.get()) { - if (!tile.runtimeId && !tile.workspaceTabTitle && !tileStoredRow(tile.storedSessionId)) { - // Any scope transition invalidates absence evidence, including A→B→A. - let changed = $gatewaySwitching.get() || Boolean($gatewaySwapTarget.get()) - const invalidate = () => { - changed = true - } - const unlisten = [ - $connection.listen(invalidate), - $activeGatewayProfile.listen(invalidate), - $profiles.listen(invalidate), - $gatewayState.listen(invalidate), - $gatewaySwitching.listen(invalidate), - $gatewaySwapTarget.listen(invalidate) - ] - const cleanup = () => { - if (pendingCleanups.delete(cleanup)) unlisten.forEach(off => off()) - } - pendingCleanups.add(cleanup) - const hasInventory = Boolean(tile.ownerRoute) || $profiles.get().length > 0 + // Absence is only authoritative in calm conditions — the same inputs as + // `goneSessionVerdict`, plus any scope change while the probes are out + // (a profile A→B→A lands the 404s on a backend that never owned the id). + let calm = !$gatewaySwitching.get() && !$gatewaySwapTarget.get() - void lookup(tile.storedSessionId, tile.ownerRoute) + const unsettle = () => { + calm = false + } + + const scopes = [$connection, $activeGatewayProfile, $profiles, $gatewayState, $gatewaySwitching, $gatewaySwapTarget] + const offScopes = scopes.map(scope => scope.listen(unsettle)) + + const probes = $sessionTiles + .get() + .filter(tile => !tile.runtimeId && !tile.workspaceTabTitle && !tileStoredRow(tile.storedSessionId)) + .map(tile => + lookup(tile.storedSessionId, tile.ownerRoute) .then(result => { const draft = takeSessionDraft(tile.storedSessionId) + const current = $sessionTiles.get().find(candidate => candidate.storedSessionId === tile.storedSessionId) + if ( - result?.status === 'gone' && - !cancelled && - !changed && - hasInventory && - restored.has(tile) && - $sessionTiles.get().includes(tile) && - !tile.runtimeId && - tile.workspaceMode !== 'bots' && + result.status === 'gone' && + calm && + !stopped && + restored.has(tile.storedSessionId) && + current && + !current.runtimeId && !tileStoredRow(tile.storedSessionId) && !tileBackendIdentityChanged(tile.ownerRoute?.connectionId, $connection.get()) && !draft.text.trim() && draft.attachments.length === 0 ) { + // Not `closeSessionTile`: a dead id must not sit on the reopen stack. discardSessionTile(tile.storedSessionId) } }) .catch(() => undefined) - .finally(cleanup) - } - } + ) + + void Promise.all(probes).finally(() => offScopes.forEach(offScope => offScope())) } const off = $gatewayState.listen(run) run() return () => { - cancelled = true + stopped = true off() - pendingCleanups.forEach(cleanup => cleanup()) } } diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts b/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts index eae78430ca..c4a917844e 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts @@ -1966,16 +1966,22 @@ export async function resolveStoredSession( return result.status === 'found' ? result.session : undefined } -/** Preserve the probe ladder's evidence: only explicit session 404s prove absence. */ +/** `resolveStoredSession` with the ladder's evidence kept: `gone` only when + * every rung answered an explicit session 404 (a 5xx, a network failure or a + * bare 404 from a proxy is `inconclusive`), and — without an owner — only once + * the profile inventory is known, or a single-profile sweep would vouch for + * ids that live on a profile not yet listed (#125678). */ export async function probeStoredSession( storedSessionId: string, ownerRoute?: SessionProfileRoute ): Promise { let allGone = true + const recordFailure = (error: unknown) => { const message = error instanceof Error ? error.message : String(error ?? '') allGone &&= /\b404\b/.test(message) && /session not found/i.test(message) } + // Snapshot BEFORE any await: a resolve that started before an archive/delete // must reject its own stale response (see upsertResolvedSession). const tombstoneGenerationsAtRequestStart = captureSessionTombstoneGenerations() @@ -2008,6 +2014,7 @@ export async function probeStoredSession( // An explicit owner is fail-closed. Probing the ambient or another // profile would turn a stale route into a cross-connection open. recordFailure(error) + return { status: allGone ? 'gone' : 'inconclusive' } } } @@ -2072,7 +2079,7 @@ export async function probeStoredSession( } } - return { status: allGone ? 'gone' : 'inconclusive' } + return { status: allGone && $profiles.get().length > 0 ? 'gone' : 'inconclusive' } } /**