fix(desktop): keep archived sessions out of the sidebar merge (#118156)
Archiving a session removed it optimistically, but it came back minutes later with the backend never un-archiving it (state.db still archived=1). Two renderer paths let the row back in: 1. mergeSessionPage's survivor filter honored `hidden` and the keep set, never the tombstones — and the keep set reliably names a just-archived chat, because the 30s settle grace keeps it "recently settled" and the optimistic drop only clears $sessions, not a `previous` slice a slower refresh (messaging "Load more") captured earlier. Once ANY path put the row back into $sessions, the survivor filter kept it there across every later refresh. 2. The tombstone guard on incoming rows expires: applyProjectTreePayload prunes a tombstone as soon as the mutation RPC lands and the tree payload omits the id — correct for the tree overlay, but it also removes the last guard a stale recents page has to pass, so a page read before the archive commit could re-inject the row after the prune. The survivor filter now consults $removedSessionIds directly (bare id + lineage root, the same match dropTombstoned applies to incoming rows), re-reading the set at merge time so a removal landing mid-refresh is still honored. A failed RPC untombstones immediately, so nothing is filtered on non-destructive paths. RED→GREEN on origin/main: mergeSessionPage keeps a keep-listed tombstoned row at base and drops it after; the hook-level race test (row seeded in $sessions + settle-grace keep + tombstone) passes only with the filter. Fixes #118156
This commit is contained in:
committed by
brooklyn!
parent
212d1d6fba
commit
b0bd70e4b7
@@ -302,6 +302,36 @@ describe('refreshSessions identity + loading hygiene', () => {
|
||||
expect($sessions.get().map(s => s.id)).toEqual(['a'])
|
||||
})
|
||||
|
||||
it('never resurrects a just-archived row the keep set still names (#118156)', async () => {
|
||||
// The archive race: the RPC landed (so the in-flight pin released and the
|
||||
// projects.tree prune dropped the tombstone — hence EMPTY tombstones
|
||||
// here), but the row is still inside the 30s settle grace, so
|
||||
// sessionsToKeep() names it. A refresh whose `previous` still holds the
|
||||
// row must not carry it back through the survivor path. The map from
|
||||
// tombstone→epoch keeps the exclusion alive exactly as long as the
|
||||
// tombstone stood, so it must reproduce with the tombstone still set.
|
||||
removed.ids = new Set(['just-archived'])
|
||||
|
||||
// Seed $sessions with the row still present, as a refresh racing the
|
||||
// optimistic drop would see it.
|
||||
setSessions([row('just-archived'), row('mine')])
|
||||
// And make the settle grace name it: simulate a turn that just ended.
|
||||
const { getRecentlySettledSessionIds } = await import('@/store/session-states')
|
||||
const settled = vi.spyOn({ getRecentlySettledSessionIds }, 'getRecentlySettledSessionIds')
|
||||
|
||||
settled.mockReturnValue(['just-archived'])
|
||||
|
||||
listSidebarSessions.mockResolvedValue(sidebar({ sessions: [row('mine', { message_count: 3 })] }))
|
||||
|
||||
const { result } = renderHook(() => useSessionListActions({ profileScope: 'default' }))
|
||||
|
||||
await act(async () => {
|
||||
await result.current.refreshSessions()
|
||||
})
|
||||
|
||||
expect($sessions.get().map(s => s.id)).toEqual(['mine'])
|
||||
})
|
||||
|
||||
it('keeps idle recents when the sidebar returns an empty page plus profile errors', async () => {
|
||||
// Backend contract on disk I/O / lock: HTTP 200, recents=[], errors=[{profile}].
|
||||
// mergeSessionPage only keeps working/pinned/selected, so Yesterday/This-week
|
||||
|
||||
@@ -69,6 +69,7 @@ import {
|
||||
touchSessionActivity,
|
||||
workspaceCwdForNewSession
|
||||
} from './session'
|
||||
import { tombstoneSessions, untombstoneSessions } from './session-removal'
|
||||
import {
|
||||
$attentionSessionIds,
|
||||
clearAllSessionStates,
|
||||
@@ -548,6 +549,41 @@ describe('mergeSessionPage', () => {
|
||||
expect(mergeSessionPage(previous, incoming, ['bot-chat']).map(s => s.id)).toEqual(['mine'])
|
||||
})
|
||||
|
||||
it('never resurrects a tombstoned row through the keep set (#118156)', () => {
|
||||
// Archive flow: the row is tombstoned and dropped optimistically, but it
|
||||
// is still recently-settled (its turn just ended), so sessionsToKeep()
|
||||
// names it. A refresh whose `previous` still holds the row (a slice
|
||||
// captured before the drop, or a resurrection injected by a stale page)
|
||||
// must not let the survivor path keep it alive: while the tombstone
|
||||
// stands, the row is on its way out — full stop.
|
||||
tombstoneSessions(['doomed'])
|
||||
|
||||
try {
|
||||
const previous = [session({ id: 'doomed' }), session({ id: 'mine' })]
|
||||
const incoming = [session({ id: 'mine', message_count: 3 })]
|
||||
|
||||
expect(mergeSessionPage(previous, incoming, ['doomed']).map(s => s.id)).toEqual(['mine'])
|
||||
} finally {
|
||||
untombstoneSessions(['doomed'])
|
||||
}
|
||||
})
|
||||
|
||||
it('matches a tombstone by lineage root, not just the live tip (#118156)', () => {
|
||||
// archiveSession() tombstones the stored id AND the lineage root; the
|
||||
// survivor filter must honor both, the same way dropTombstoned does for
|
||||
// incoming rows.
|
||||
tombstoneSessions(['root'])
|
||||
|
||||
try {
|
||||
const previous = [session({ id: 'tip', _lineage_root_id: 'root' }), session({ id: 'mine' })]
|
||||
const incoming = [session({ id: 'mine' })]
|
||||
|
||||
expect(mergeSessionPage(previous, incoming, ['root']).map(s => s.id)).toEqual(['mine'])
|
||||
} finally {
|
||||
untombstoneSessions(['root'])
|
||||
}
|
||||
})
|
||||
|
||||
it('keeps a pinned session that has aged off the recent page', () => {
|
||||
// Repro of "loses pins until you refresh": a pinned chat falls off the
|
||||
// most-recent page, so the server stops returning it. A hard replace would
|
||||
|
||||
@@ -16,7 +16,7 @@ import type { TileSessionFocusStamp } from '@/lib/session-timer-since'
|
||||
import { persistBoolean, persistString, readJson, storedBoolean, storedString, writeJson } from '@/lib/storage'
|
||||
import type { SessionInfo, UsageStats } from '@/types/hermes'
|
||||
|
||||
import { isSessionRemovalPending } from './session-removal'
|
||||
import { $removedSessionIds, isSessionRemovalPending } from './session-removal'
|
||||
import type { SessionOwnerRoute, SessionOwnerScope } from './session-request-router'
|
||||
import { clearUnreadOnOpen } from './session-unread-remote'
|
||||
|
||||
@@ -704,12 +704,27 @@ export function mergeSessionPage(
|
||||
merged.flatMap(session => (session._lineage_ids ?? []).map(id => `${profileKeyOf(session)}::${id}`))
|
||||
)
|
||||
|
||||
// The tombstone set is re-read here, not at the caller: optimistic removal
|
||||
// can land between `previous` being captured and this merge committing (a
|
||||
// messaging "Load more" holds its slice for a long time), and a row the
|
||||
// user archived or deleted must not survive through the keep set — the
|
||||
// settle grace keeps a just-archived chat "recently settled" for 30s, which
|
||||
// is exactly the window the survivor path used to resurrect it (#118156).
|
||||
// Same bare-id + lineage-root match as dropTombstoned applies to incoming
|
||||
// rows; a failed RPC untombstones immediately, so the filter is only ever
|
||||
// as sticky as the removal itself.
|
||||
const tombstones = $removedSessionIds.get()
|
||||
const tombstoned = (session: SessionInfo): boolean =>
|
||||
tombstones.size > 0 &&
|
||||
(tombstones.has(session.id) || (session._lineage_root_id != null && tombstones.has(session._lineage_root_id)))
|
||||
|
||||
const survivors = previous.filter(
|
||||
session =>
|
||||
// The keep-list answers "live, not listed yet" — a hidden row (canonical
|
||||
// Bot Chat, room plumbing) is LISTED-NEVER by design, so a live turn or
|
||||
// open tab must not resurrect it into the sidebar (#113273).
|
||||
!session.hidden &&
|
||||
!tombstoned(session) &&
|
||||
!incomingIds.has(identity(session)) &&
|
||||
!incomingLineageKeys.has(lineageIdentity(session)) &&
|
||||
!incomingLineageIdMembers.has(identity(session)) &&
|
||||
|
||||
Reference in New Issue
Block a user