From 1025cb0e663f37bcaf6a71f147c79be7bf6aed3f Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 14 Aug 2026 23:09:07 -0700 Subject: [PATCH] fix(desktop): stop deleted sessions resurrecting through racing list fetches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A session deleted in the sidebar disappeared optimistically but flashed back when any list fetch raced the in-flight DELETE RPC — the backend page still carried the doomed row until the transaction committed (#50928, reproduced with 'Load more' and auto-refresh). The optimistic tombstone ($removedSessionIds) was only honored by the recents slice of refreshSessions; the messaging slice, the per-platform pager, and refreshMessagingSessions ingested backend pages unfiltered. Extract the tombstone filter into dropTombstoned() and apply it at every session-list ingestion point. Tombstones only exist while a delete or archive is in flight (they self-clear on confirmation and are removed immediately on failure), so non-destructive refresh paths are untouched. Fixes #50928 --- .../hooks/use-session-list-actions.test.tsx | 34 +++++++++++++++++++ .../session/hooks/use-session-list-actions.ts | 30 ++++++++++------ 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/apps/desktop/src/app/session/hooks/use-session-list-actions.test.tsx b/apps/desktop/src/app/session/hooks/use-session-list-actions.test.tsx index 8b32a8b85e..f25bfc9d53 100644 --- a/apps/desktop/src/app/session/hooks/use-session-list-actions.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-session-list-actions.test.tsx @@ -171,6 +171,40 @@ describe('refreshSessions identity + loading hygiene', () => { expect($sessions.get().map(s => s.id)).toEqual(['a']) }) + it('drops tombstoned rows from the messaging slice and per-platform paging too (#50928)', async () => { + // The same delete race exists on every ingestion point: the batched + // refresh's messaging slice and the per-platform "load more" pager must + // both honor the tombstone, or a deleted platform thread resurrects. + removed.ids = new Set(['tg-2']) + listSidebarSessions.mockResolvedValue( + sidebar( + { sessions: [] }, + [], + [row('tg-1', { source: 'telegram' }), row('tg-2', { source: 'telegram' })] + ) + ) + + const { result } = renderHook(() => useSessionListActions({ profileScope: 'default' })) + + await act(async () => { + await result.current.refreshSessions() + }) + + expect($messagingSessions.get().map(s => s.id)).toEqual(['tg-1']) + + // Per-platform pager: backend page still lists the doomed row. + listAllProfileSessions.mockResolvedValue({ + sessions: [row('tg-1', { source: 'telegram' }), row('tg-2', { source: 'telegram' }), row('tg-3', { source: 'telegram' })], + total: 3 + }) + + await act(async () => { + await result.current.loadMoreMessagingForPlatform('telegram') + }) + + expect($messagingSessions.get().map(s => s.id)).toEqual(['tg-1', 'tg-3']) + }) + it('still shows loading for the initial (empty-list) fetch', async () => { listSidebarSessions.mockResolvedValue(sidebar({ sessions: [row('a')] })) const { result } = renderHook(() => useSessionListActions({ profileScope: 'default' })) diff --git a/apps/desktop/src/app/session/hooks/use-session-list-actions.ts b/apps/desktop/src/app/session/hooks/use-session-list-actions.ts index bad975a5b3..959edfc5a1 100644 --- a/apps/desktop/src/app/session/hooks/use-session-list-actions.ts +++ b/apps/desktop/src/app/session/hooks/use-session-list-actions.ts @@ -50,6 +50,22 @@ const SIDEBAR_EXCLUDED_SOURCES = ['cron', 'kanban', 'subagent', 'tool', ...MESSA // external-platform conversations remain, then split per platform in the UI. const MESSAGING_EXCLUDED_SOURCES = ['cron', ...LOCAL_SESSION_SOURCE_IDS] +// Drop rows the user just deleted/archived: ANY list fetch (full refresh, +// "Load more" paging, a per-platform messaging page, the cron slice) can race +// an in-flight delete RPC, and the backend page still carries the doomed row +// until the DELETE commits — so it flashed back into the sidebar (#50928). +// Honoring the optimistic tombstone at every ingestion point keeps the removal +// stable; the tombstone self-clears once projects.tree confirms the delete, +// and a failed delete untombstones immediately, so nothing is filtered on the +// non-destructive paths. +function dropTombstoned(sessions: SessionInfo[]): SessionInfo[] { + const tombstones = $removedSessionIds.get() + + return tombstones.size + ? sessions.filter(s => !tombstones.has(s.id) && !(s._lineage_root_id && tombstones.has(s._lineage_root_id))) + : sessions +} + // Rows a session refresh must preserve even if the aggregator omits them: // in-flight first turns (message_count 0), pinned rows aged off the page, the // actively-viewed chat (its "working" flag clears a beat before the aggregator @@ -98,7 +114,7 @@ export function useSessionListActions({ profileScope }: UseSessionListActionsArg // Drop any non-messaging source the broad exclude didn't catch (custom // sources) — those stay in local recents, not a platform section. - const rows = result.sessions.filter(s => isMessagingSource(s.source)) + const rows = dropTombstoned(result.sessions.filter(s => isMessagingSource(s.source))) setMessagingSessions(prev => (sameCronSignature(prev, rows) ? prev : rows)) // Hit the cap → at least one platform may have more on disk than loaded, @@ -120,7 +136,7 @@ export function useSessionListActions({ profileScope }: UseSessionListActionsArg source: platform }) - const incoming = result.sessions.filter(s => normalizeSessionSource(s.source) === platform) + const incoming = dropTombstoned(result.sessions.filter(s => normalizeSessionSource(s.source) === platform)) setMessagingSessions(prev => [ ...prev.filter(s => !inPlatform(s)), @@ -193,13 +209,7 @@ export function useSessionListActions({ profileScope }: UseSessionListActionsArg // in-flight mutation and the backend page still carries the doomed row. // Honoring the optimistic tombstone keeps the removal from flashing back // (the tombstone self-clears once projects.tree confirms the delete). - const tombstones = $removedSessionIds.get() - - const incoming = tombstones.size - ? recents.sessions.filter( - s => !tombstones.has(s.id) && !(s._lineage_root_id && tombstones.has(s._lineage_root_id)) - ) - : recents.sessions + const incoming = dropTombstoned(recents.sessions) // Signature-gate the swap (same pattern as cron/messaging): a refresh // that returns content-identical rows must keep the previous array @@ -244,7 +254,7 @@ export function useSessionListActions({ profileScope }: UseSessionListActionsArg // Messaging sections: drop any non-messaging source the broad exclude // didn't catch (custom sources stay in local recents), then split per // platform in the UI. - const messagingRows = result.messaging.sessions.filter(s => isMessagingSource(s.source)) + const messagingRows = dropTombstoned(result.messaging.sessions.filter(s => isMessagingSource(s.source))) setMessagingSessions(prev => (sameCronSignature(prev, messagingRows) ? prev : messagingRows)) // Hit the cap → at least one platform may have more on disk than loaded.