fix(desktop): trim dead-tile retirement to the salvage bar (#125678, salvage #125706)

Same outcome as the salvaged commits — a restored tab whose id every
profile answered 404 for in calm conditions is dropped instead of being
re-probed on every launch — with less machinery:

- one set of scope listeners per backfill run instead of six per tile;
  retired ids are tracked by storedSessionId, so a tile array rebuilt by
  the backend-identity guard between start and gateway-open cannot make
  the retire silently never fire.
- the "profile inventory known" gate lives in probeStoredSession, where
  the ladder decides what its evidence proves, instead of in the caller.
- the nine-case test file is folded into session-tile.test.ts as two
  invariants: retire on all-profile 404 (red on origin/main), keep on
  5xx / network failure / switch in flight / profile A→B→A / connection
  switch / stashed draft / unknown inventory.
This commit is contained in:
teknium1
2026-09-28 00:48:11 -07:00
committed by Teknium
parent aa492fb76e
commit fc51ca12a2
4 changed files with 164 additions and 241 deletions

View File

@@ -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<typeof import('@/hermes')>()),
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<typeof import('@/hermes')>('@/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'])
})
})

View File

@@ -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<Record<string, unknown>>()),
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<never>((_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'])
})
})

View File

@@ -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())
}
}

View File

@@ -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<StoredSessionProbe> {
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' }
}
/**