fix(desktop): isolate Inbox session actions from the row resume (#85163)
In card (Inbox) mode the actions cluster holding the ⋮ SessionActionsMenu renders INSIDE SidebarRowBody — the row button whose plain-click onClick resumes. Radix portals the menu content, but React synthetic events still bubble through the logical parent, so an Archive menu click also fired the row's resume: the archive RPC raced a re-open of the chat it was removing, and the queued resume could transiently restore the row. Flat rows are unaffected (their actions render outside the row button via the shell actions column). Two halves, same bug class: 1. stopPropagation (click + pointerdown) on the actions container in card mode. Container-level on purpose: every action owns its gesture; a future child that needs row semantics moves outside the boundary. 2. upsertResolvedSession now refuses to recache a resolved row that raced an archive/delete: while any identity (stored id, row id, lineage root) is tombstoned, when the backend row itself is archived, or when the tombstone lifecycle moved since the request started (ABA-safe — a failed archive rolls the tombstone back while the by-id response is in flight). Tombstone generations in store/session-removal make the ABA cycle detectable; membership alone cannot. The resolved row is still returned, so an explicit resume-by-id of archived history keeps working. Component test drives the REAL Radix menu inside the REAL row: without the stopPropagation, onResume fires twice (the reporter's exact symptom); with it, onArchive fires once and onResume never. Salvage of open PR #85166 by Jakub Wolniewicz, rebased onto current main: tombstone generations live in store/session-removal (their home since the projects/sessions store split), and the upsert guard keeps the hidden-row off-list parking intact. Co-authored-by: Jakub Wolniewicz <4850809+frizikk@users.noreply.github.com>
This commit is contained in:
170
apps/desktop/src/app/chat/sidebar/session-row-actions.test.tsx
Normal file
170
apps/desktop/src/app/chat/sidebar/session-row-actions.test.tsx
Normal file
@@ -0,0 +1,170 @@
|
||||
import { cleanup, fireEvent, render, screen } from '@testing-library/react'
|
||||
import { atom } from 'nanostores'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import type * as HermesModule from '@/hermes'
|
||||
import type { SessionInfo } from '@/hermes'
|
||||
import type * as SessionStore from '@/store/session'
|
||||
import type * as SessionStatesStore from '@/store/session-states'
|
||||
|
||||
import { SidebarSessionRow } from './session-row'
|
||||
|
||||
afterEach(cleanup)
|
||||
|
||||
// Exercises the REAL SessionActionsMenu inside the REAL row (no menu stub, no
|
||||
// DropdownMenu mock) so a menu-item click that bubbles into the card row's
|
||||
// resume onClick fails here — Radix portals the menu content, but React still
|
||||
// propagates synthetic events through the logical parent (#85163).
|
||||
|
||||
vi.mock('@/i18n', () => ({
|
||||
useI18n: () => ({
|
||||
t: {
|
||||
common: { cancel: 'Cancel', close: 'Close', delete: 'Delete', save: 'Save' },
|
||||
assistant: {
|
||||
thread: {
|
||||
today: (time: string) => `Today, ${time}`,
|
||||
yesterday: (time: string) => `Yesterday, ${time}`
|
||||
}
|
||||
},
|
||||
sidebar: {
|
||||
messageCount: (count: number) => `${count} messages`,
|
||||
toolCallCount: (count: number) => `${count} tool calls`,
|
||||
projects: {
|
||||
menuAppearance: 'Appearance',
|
||||
moveFailed: 'Could not move session',
|
||||
moveNoProjects: 'No other projects',
|
||||
movedTo: (name: string) => `Moved to ${name}`,
|
||||
moveToProject: 'Move to project',
|
||||
noColor: 'No color'
|
||||
},
|
||||
row: {
|
||||
ageMin: 'm',
|
||||
ageNow: 'now',
|
||||
archive: 'Archive',
|
||||
backgroundRunning: 'Running in background',
|
||||
branchFrom: 'Branch from here',
|
||||
copyId: 'Copy ID',
|
||||
copyIdFailed: 'Failed to copy ID',
|
||||
deleteDesc: (title: string) => `Delete ${title}?`,
|
||||
deleteTitle: 'Delete session?',
|
||||
deleting: 'Deleting…',
|
||||
deleted: 'Session deleted',
|
||||
export: 'Export',
|
||||
finishedUnread: 'Finished',
|
||||
handoffOrigin: (platform: string) => `Started on ${platform}`,
|
||||
hideTabBar: 'Hide tab bar',
|
||||
messageCount: (count: number) => `${count} messages`,
|
||||
needsInput: 'Needs input',
|
||||
pin: 'Pin',
|
||||
rename: 'Rename',
|
||||
renameDesc: 'Leave empty to clear.',
|
||||
renameFailed: 'Rename failed',
|
||||
renameTitle: 'Rename session',
|
||||
renamed: 'Renamed',
|
||||
sessionActions: 'Session actions',
|
||||
sessionRunning: 'Running',
|
||||
unpin: 'Unpin',
|
||||
untitledPlaceholder: 'Untitled',
|
||||
waitingForAnswer: 'Waiting for answer'
|
||||
}
|
||||
},
|
||||
zones: { closeAll: 'Close all', closeOthers: 'Close others', closeToRight: 'Close to the right' }
|
||||
}
|
||||
})
|
||||
}))
|
||||
vi.mock('@/app/chat/profile-tag', () => ({ ProfileTag: () => null }))
|
||||
vi.mock('@/app/chat/session-drag', () => ({ startSessionDrag: vi.fn() }))
|
||||
vi.mock('@/hermes', async importOriginal => ({
|
||||
...(await importOriginal<typeof HermesModule>()),
|
||||
renameSession: vi.fn(),
|
||||
setSessionUnreadRemote: vi.fn(() => Promise.resolve({ ok: true }))
|
||||
}))
|
||||
vi.mock('@/lib/haptics', () => ({ triggerHaptic: vi.fn() }))
|
||||
vi.mock('@/lib/session-source', () => ({
|
||||
handoffOriginSource: () => null,
|
||||
sessionSourceLabel: () => ''
|
||||
}))
|
||||
vi.mock('@/lib/session-export', () => ({ exportSession: vi.fn() }))
|
||||
vi.mock('@/lib/time', async importOriginal => ({
|
||||
...(await importOriginal<Record<string, unknown>>()),
|
||||
coarseElapsed: () => ({ unit: 'minute' as const, value: 5 })
|
||||
}))
|
||||
vi.mock('@/lib/profile-color', () => ({ PROFILE_SWATCHES: [] }))
|
||||
vi.mock('@/store/gateway', async importOriginal => ({
|
||||
...(await importOriginal<Record<string, unknown>>()),
|
||||
activeGateway: vi.fn(() => null)
|
||||
}))
|
||||
vi.mock('@/store/notifications', () => ({ notify: vi.fn(), notifyError: vi.fn() }))
|
||||
vi.mock('@/store/projects', async importOriginal => ({
|
||||
...(await importOriginal<Record<string, unknown>>()),
|
||||
moveSessionToProject: vi.fn(),
|
||||
projectIdForCwd: vi.fn(() => null),
|
||||
projectRootCwd: vi.fn(() => '')
|
||||
}))
|
||||
vi.mock('@/store/session', async importOriginal => {
|
||||
const actual = await importOriginal<typeof SessionStore>()
|
||||
|
||||
return { ...actual, $unreadFinishedSessionIds: atom<string[]>([]) }
|
||||
})
|
||||
vi.mock('@/store/session-states', async importOriginal => {
|
||||
const actual = await importOriginal<typeof SessionStatesStore>()
|
||||
|
||||
return { ...actual, openSessionTile: vi.fn() }
|
||||
})
|
||||
vi.mock('@/store/windows', async importOriginal => ({
|
||||
...(await importOriginal<Record<string, unknown>>()),
|
||||
canOpenSessionWindow: () => false,
|
||||
openSessionInNewWindow: vi.fn()
|
||||
}))
|
||||
vi.mock('./use-profile-prewarm', () => ({
|
||||
useProfilePrewarm: () => ({ cancelPrewarm: vi.fn(), notePointerMove: vi.fn(), startPrewarm: vi.fn() })
|
||||
}))
|
||||
|
||||
const session = {
|
||||
cwd: '/tmp/project',
|
||||
handoff_platform: null,
|
||||
handoff_state: null,
|
||||
id: 's1',
|
||||
last_active: 0,
|
||||
message_count: 1,
|
||||
profile: 'default',
|
||||
started_at: 0,
|
||||
title: 'Archive me'
|
||||
} as SessionInfo
|
||||
|
||||
describe('SidebarSessionRow actions', () => {
|
||||
it('archives an Inbox card without also resuming it (#85163)', async () => {
|
||||
const onArchive = vi.fn()
|
||||
const onResume = vi.fn()
|
||||
|
||||
render(
|
||||
<SidebarSessionRow
|
||||
card
|
||||
isPinned={false}
|
||||
isSelected={false}
|
||||
onArchive={onArchive}
|
||||
onDelete={vi.fn()}
|
||||
onPin={vi.fn()}
|
||||
onResume={onResume}
|
||||
onToggleUnread={vi.fn()}
|
||||
session={session}
|
||||
unread={false}
|
||||
/>
|
||||
)
|
||||
|
||||
// Full mouse gesture (pointerDown/up + click) — the same sequence Radix
|
||||
// listens for on the trigger and the menu items.
|
||||
const trigger = screen.getByRole('button', { name: 'Session actions' })
|
||||
fireEvent.pointerDown(trigger, { button: 0, pointerType: 'mouse' })
|
||||
fireEvent.pointerUp(trigger, { button: 0, pointerType: 'mouse' })
|
||||
fireEvent.click(trigger)
|
||||
|
||||
const archive = await screen.findByRole('menuitem', { name: 'Archive' })
|
||||
fireEvent.pointerDown(archive, { button: 0, pointerType: 'mouse' })
|
||||
fireEvent.pointerUp(archive, { button: 0, pointerType: 'mouse' })
|
||||
fireEvent.click(archive)
|
||||
|
||||
expect(onArchive).toHaveBeenCalledTimes(1)
|
||||
expect(onResume).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -300,7 +300,21 @@ function SidebarSessionRowImpl({
|
||||
// shell column would span the card's full height and shave every line,
|
||||
// when only the header shares its line with the age and kebab.
|
||||
const actionsNode = (
|
||||
<div className="relative z-2 flex shrink-0 items-center justify-end gap-1" data-row-actions>
|
||||
<div
|
||||
className="relative z-2 flex shrink-0 items-center justify-end gap-1"
|
||||
data-row-actions
|
||||
// Radix renders the menu content in a portal, but React still bubbles its
|
||||
// events through this logical parent (#85163): in card (Inbox) mode this
|
||||
// cluster renders INSIDE the row body whose onClick resumes, so an
|
||||
// Archive menu click also fired the row's resume. This container-level
|
||||
// gate is deliberate: every action owns its gesture instead of inheriting
|
||||
// row resume/drag semantics. A future child that needs row semantics must
|
||||
// move outside this boundary rather than weakening it for every menu
|
||||
// action. Flat rows already achieve this structurally (actions render
|
||||
// outside the row button via the shell's `actions` column).
|
||||
onClick={event => event.stopPropagation()}
|
||||
onPointerDown={event => event.stopPropagation()}
|
||||
>
|
||||
{trailing.map(({ key, node }, index) => (
|
||||
<span
|
||||
className={
|
||||
|
||||
@@ -5,6 +5,7 @@ import { getSession } from '@/hermes'
|
||||
import { $activeGatewayProfile, $profiles } from '@/store/profile'
|
||||
import { $projectTree } from '@/store/projects'
|
||||
import { $cronSessions, $messagingSessions, $sessions, $unlistedSessionOwnerRows } from '@/store/session'
|
||||
import { $removedSessionIds, tombstoneSessions, untombstoneSessions } from '@/store/session-removal'
|
||||
import type { SessionInfo } from '@/types/hermes'
|
||||
|
||||
import { cachedSessionRow, resolveStoredSession } from './utils'
|
||||
@@ -26,6 +27,7 @@ describe('resolveStoredSession profile ownership', () => {
|
||||
$messagingSessions.set([])
|
||||
$sessions.set([])
|
||||
$projectTree.set([])
|
||||
$removedSessionIds.set(new Set())
|
||||
$profiles.set(profiles('default', 'meta'))
|
||||
$activeGatewayProfile.set('meta')
|
||||
$unlistedSessionOwnerRows.set([])
|
||||
@@ -37,6 +39,7 @@ describe('resolveStoredSession profile ownership', () => {
|
||||
$messagingSessions.set([])
|
||||
$sessions.set([])
|
||||
$projectTree.set([])
|
||||
$removedSessionIds.set(new Set())
|
||||
$profiles.set([])
|
||||
$activeGatewayProfile.set('default')
|
||||
$unlistedSessionOwnerRows.set([])
|
||||
@@ -168,6 +171,119 @@ describe('resolveStoredSession profile ownership', () => {
|
||||
// the cached row is owned too — no unowned row is ever re-cached
|
||||
expect($sessions.get().find(s => s.id === 's1')?.profile).toBe('default')
|
||||
})
|
||||
|
||||
it('does not recache a by-id row while its session is tombstoned (#85163)', async () => {
|
||||
// The archive row click's bubbled resume raced the tombstone: the by-id
|
||||
// resolve started just before the archive, and its response must not
|
||||
// re-insert the row the archive optimistically evicted.
|
||||
let resolveRequest!: (value: SessionInfo) => void
|
||||
mockGetSession.mockReturnValueOnce(
|
||||
new Promise<SessionInfo>(resolve => {
|
||||
resolveRequest = resolve
|
||||
})
|
||||
)
|
||||
|
||||
const pending = resolveStoredSession('s1')
|
||||
tombstoneSessions(['s1'])
|
||||
resolveRequest(session({ archived: false, id: 's1' }))
|
||||
|
||||
await expect(pending).resolves.toMatchObject({ id: 's1' })
|
||||
expect($sessions.get()).toEqual([])
|
||||
untombstoneSessions(['s1'])
|
||||
})
|
||||
|
||||
it('does not recache an archived by-id row', async () => {
|
||||
// The direct lookup can also observe the archive itself: the backend row
|
||||
// already carries archived=true while the tombstone is still settling.
|
||||
mockGetSession.mockResolvedValueOnce(session({ archived: true, id: 's1' }))
|
||||
|
||||
const resolved = await resolveStoredSession('s1')
|
||||
|
||||
expect(resolved?.archived).toBe(true)
|
||||
expect($sessions.get()).toEqual([])
|
||||
})
|
||||
|
||||
it('does not recache a stale by-id row when its tombstone clears before the response', async () => {
|
||||
// A failed archive rolls the row back (untombstone) while the by-id
|
||||
// request is still in flight: the response raced the doomed row even
|
||||
// though membership looks untouched.
|
||||
let resolveRequest!: (value: SessionInfo) => void
|
||||
mockGetSession.mockReturnValueOnce(
|
||||
new Promise<SessionInfo>(resolve => {
|
||||
resolveRequest = resolve
|
||||
})
|
||||
)
|
||||
tombstoneSessions(['s1'])
|
||||
|
||||
const pending = resolveStoredSession('s1')
|
||||
untombstoneSessions(['s1'])
|
||||
resolveRequest(session({ archived: false, id: 's1' }))
|
||||
|
||||
await expect(pending).resolves.toMatchObject({ id: 's1' })
|
||||
expect($sessions.get()).toEqual([])
|
||||
})
|
||||
|
||||
it('does not recache a stale by-id row after an in-flight tombstone ABA cycle', async () => {
|
||||
// Tombstone added AND removed while the request was in flight: membership
|
||||
// is back to empty, but the generation moved, so the response is stale.
|
||||
let resolveRequest!: (value: SessionInfo) => void
|
||||
mockGetSession.mockReturnValueOnce(
|
||||
new Promise<SessionInfo>(resolve => {
|
||||
resolveRequest = resolve
|
||||
})
|
||||
)
|
||||
|
||||
const pending = resolveStoredSession('s1')
|
||||
tombstoneSessions(['s1'])
|
||||
untombstoneSessions(['s1'])
|
||||
expect($removedSessionIds.get()).toEqual(new Set())
|
||||
resolveRequest(session({ archived: false, id: 's1' }))
|
||||
|
||||
await expect(pending).resolves.toMatchObject({ id: 's1' })
|
||||
expect($sessions.get()).toEqual([])
|
||||
})
|
||||
|
||||
it('does not recache a stale cross-profile by-id row after an in-flight tombstone cycle', async () => {
|
||||
let resolveProbe!: (value: SessionInfo) => void
|
||||
mockGetSession.mockRejectedValueOnce(new Error('404: Session not found'))
|
||||
mockGetSession.mockReturnValueOnce(
|
||||
new Promise<SessionInfo>(resolve => {
|
||||
resolveProbe = resolve
|
||||
})
|
||||
)
|
||||
|
||||
const pending = resolveStoredSession('s1')
|
||||
await vi.waitFor(() => expect(mockGetSession).toHaveBeenCalledTimes(2))
|
||||
tombstoneSessions(['s1'])
|
||||
untombstoneSessions(['s1'])
|
||||
resolveProbe(session({ archived: false, id: 's1' }))
|
||||
|
||||
await expect(pending).resolves.toMatchObject({ id: 's1', profile: 'default' })
|
||||
expect($sessions.get()).toEqual([])
|
||||
})
|
||||
|
||||
it('does not recache a stale owner-routed by-id row after an in-flight tombstone cycle', async () => {
|
||||
let resolveRequest!: (value: SessionInfo) => void
|
||||
mockGetSession.mockReturnValueOnce(
|
||||
new Promise<SessionInfo>(resolve => {
|
||||
resolveRequest = resolve
|
||||
})
|
||||
)
|
||||
|
||||
const pending = resolveStoredSession(
|
||||
's1',
|
||||
{ connectionId: 'remote-1', profile: 'meta', targetProfile: 'meta' } as never
|
||||
)
|
||||
|
||||
tombstoneSessions(['s1'])
|
||||
untombstoneSessions(['s1'])
|
||||
|
||||
resolveRequest(session({ archived: false, id: 's1' }))
|
||||
|
||||
await expect(pending).resolves.toMatchObject({ connection_id: 'remote-1', id: 's1', profile: 'meta' })
|
||||
expect(mockGetSession).toHaveBeenCalledWith('s1', { connectionId: 'remote-1', profile: 'meta' })
|
||||
expect($sessions.get()).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('cachedSessionRow owner preference', () => {
|
||||
|
||||
@@ -47,6 +47,12 @@ import {
|
||||
setWorkspaceCwdOwner,
|
||||
setYoloActive
|
||||
} from '@/store/session'
|
||||
import {
|
||||
$removedSessionIds,
|
||||
captureSessionTombstoneGenerations,
|
||||
type SessionTombstoneGenerationSnapshot,
|
||||
tombstoneLifecycleChanged
|
||||
} from '@/store/session-removal'
|
||||
import type { SessionProfileRoute } from '@/store/session-request-router'
|
||||
import { runtimeSessionOwner, sessionTileOwnerRoute } from '@/store/session-states'
|
||||
|
||||
@@ -1805,7 +1811,31 @@ export function restoreListedSession(session: SessionInfo, slice?: ListedSession
|
||||
setSessions(prepend)
|
||||
}
|
||||
|
||||
function upsertResolvedSession(session: SessionInfo, storedSessionId: string) {
|
||||
function upsertResolvedSession(
|
||||
session: SessionInfo,
|
||||
storedSessionId: string,
|
||||
tombstoneGenerationsAtRequestStart: SessionTombstoneGenerationSnapshot
|
||||
) {
|
||||
const removed = $removedSessionIds.get()
|
||||
const identities = [storedSessionId, session.id, session._lineage_root_id]
|
||||
|
||||
// A direct by-id resolve may have started just before an archive/delete
|
||||
// (#85163: the archive row click's bubbled resume raced the tombstone).
|
||||
// A stale response must not undo the optimistic eviction while the mutation's
|
||||
// tombstone is active, after the tombstone was already present at request
|
||||
// start, or after an add → remove ABA cycle made membership look unchanged.
|
||||
// Check every identity lineage-aware lookups use. This suppresses only the
|
||||
// sidebar-cache upsert: the resolved row is still returned so an explicit
|
||||
// resume-by-id can open archived history, and a later request after a
|
||||
// settled rollback can publish normally.
|
||||
if (
|
||||
session.archived ||
|
||||
identities.some(id => (id ? removed.has(id) : false)) ||
|
||||
tombstoneLifecycleChanged(tombstoneGenerationsAtRequestStart, identities)
|
||||
) {
|
||||
return
|
||||
}
|
||||
|
||||
const lineage = session._lineage_root_id ?? session.id
|
||||
|
||||
// A hidden row (canonical Bot Chat, room plumbing) is unlisted by design:
|
||||
@@ -1886,6 +1916,10 @@ export async function resolveStoredSession(
|
||||
storedSessionId: string,
|
||||
ownerRoute?: SessionProfileRoute
|
||||
): Promise<SessionInfo | undefined> {
|
||||
// Snapshot BEFORE any await: a resolve that started before an archive/delete
|
||||
// must reject its own stale response (see upsertResolvedSession).
|
||||
const tombstoneGenerationsAtRequestStart = captureSessionTombstoneGenerations()
|
||||
|
||||
const cached = cachedSessionRow(storedSessionId)
|
||||
|
||||
if (ownerRoute) {
|
||||
@@ -1907,7 +1941,7 @@ export async function resolveStoredSession(
|
||||
const session = await getSession(storedSessionId, scope)
|
||||
session.profile = normalizeProfileKey(ownerRoute.profile)
|
||||
session.connection_id = ownerRoute.connectionId
|
||||
upsertResolvedSession(session, storedSessionId)
|
||||
upsertResolvedSession(session, storedSessionId, tombstoneGenerationsAtRequestStart)
|
||||
|
||||
return session
|
||||
} catch {
|
||||
@@ -1941,7 +1975,7 @@ export async function resolveStoredSession(
|
||||
// stamp is preserved for backend compatibility.
|
||||
session.profile ||= activeKey
|
||||
|
||||
upsertResolvedSession(session, storedSessionId)
|
||||
upsertResolvedSession(session, storedSessionId, tombstoneGenerationsAtRequestStart)
|
||||
|
||||
return session
|
||||
} catch {
|
||||
@@ -1967,7 +2001,7 @@ export async function resolveStoredSession(
|
||||
// forwarding, so that backend answers as its own "default").
|
||||
session.profile = profile
|
||||
|
||||
upsertResolvedSession(session, storedSessionId)
|
||||
upsertResolvedSession(session, storedSessionId, tombstoneGenerationsAtRequestStart)
|
||||
|
||||
return session
|
||||
} catch {
|
||||
|
||||
@@ -5,8 +5,10 @@ import {
|
||||
$removedSessionIds,
|
||||
$sessionMutationsInFlight,
|
||||
beginSessionMutation,
|
||||
captureSessionTombstoneGenerations,
|
||||
endSessionMutation,
|
||||
isSessionRemovalPending,
|
||||
tombstoneLifecycleChanged,
|
||||
tombstoneSessions,
|
||||
untombstoneSessions
|
||||
} from './session-removal'
|
||||
@@ -45,6 +47,52 @@ describe('isSessionRemovalPending', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('tombstone generations', () => {
|
||||
it('bumps a per-id generation on add and remove, leaving unrelated ids untouched', () => {
|
||||
const beforeAdd = captureSessionTombstoneGenerations()
|
||||
|
||||
tombstoneSessions(['sess-1'])
|
||||
|
||||
expect(tombstoneLifecycleChanged(beforeAdd, ['sess-1'])).toBe(true)
|
||||
expect(tombstoneLifecycleChanged(beforeAdd, ['unrelated'])).toBe(false)
|
||||
|
||||
const beforeRemove = captureSessionTombstoneGenerations()
|
||||
untombstoneSessions(['sess-1'])
|
||||
|
||||
expect(tombstoneLifecycleChanged(beforeRemove, ['sess-1'])).toBe(true)
|
||||
})
|
||||
|
||||
it('detects an add → remove ABA cycle even though membership is back to unchanged', () => {
|
||||
// The core #85163 race: while a by-id resolve is in flight, the target is
|
||||
// archived AND the archive rolls back. Membership (in vs out) is the same
|
||||
// before and after, but the request raced a doomed row.
|
||||
const before = captureSessionTombstoneGenerations()
|
||||
|
||||
tombstoneSessions(['aba-1'])
|
||||
untombstoneSessions(['aba-1'])
|
||||
|
||||
expect($removedSessionIds.get()).toEqual(new Set())
|
||||
expect(tombstoneLifecycleChanged(before, ['aba-1'])).toBe(true)
|
||||
})
|
||||
|
||||
it('a snapshot taken after the lifecycle settles compares equal again', () => {
|
||||
tombstoneSessions(['settled-1'])
|
||||
untombstoneSessions(['settled-1'])
|
||||
|
||||
const after = captureSessionTombstoneGenerations()
|
||||
|
||||
expect(tombstoneLifecycleChanged(after, ['settled-1'])).toBe(false)
|
||||
})
|
||||
|
||||
it('ignores blank ids rather than inventing generations', () => {
|
||||
const before = captureSessionTombstoneGenerations()
|
||||
|
||||
tombstoneSessions([null, '', ' '])
|
||||
|
||||
expect(tombstoneLifecycleChanged(before, [null, '', ' '])).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('requestSessionResume refuses a doomed session', () => {
|
||||
it('queues a resume for a live session', () => {
|
||||
requestSessionResume('live-1')
|
||||
|
||||
@@ -11,9 +11,73 @@ import { atom } from 'nanostores'
|
||||
// reads `store/session`).
|
||||
export const $removedSessionIds = atom<Set<string>>(new Set())
|
||||
|
||||
/**
|
||||
* Per-id tombstone lifecycle generations (#85163). Membership alone cannot
|
||||
* distinguish "unchanged" from an add → remove ABA cycle while a by-id
|
||||
* resolve is in flight: an archive that begins AND rolls back (failed RPC)
|
||||
* mid-request leaves membership looking untouched, yet the response raced a
|
||||
* doomed row. Immutable snapshots let async publishers reject any response
|
||||
* whose target changed, without blocking unrelated ids or a later explicit
|
||||
* resume that starts after the lifecycle has settled.
|
||||
*/
|
||||
export type SessionTombstoneGenerationSnapshot = ReadonlyMap<string, number>
|
||||
let tombstoneGenerations: SessionTombstoneGenerationSnapshot = new Map()
|
||||
|
||||
function setRemovedSessionIds(next: Set<string>): void {
|
||||
const current = $removedSessionIds.get()
|
||||
const changed = new Set<string>()
|
||||
|
||||
for (const id of current) {
|
||||
if (!next.has(id)) {
|
||||
changed.add(id)
|
||||
}
|
||||
}
|
||||
|
||||
for (const id of next) {
|
||||
if (!current.has(id)) {
|
||||
changed.add(id)
|
||||
}
|
||||
}
|
||||
|
||||
if (!changed.size) {
|
||||
return
|
||||
}
|
||||
|
||||
const generations = new Map(tombstoneGenerations)
|
||||
|
||||
for (const id of changed) {
|
||||
generations.set(id, (generations.get(id) ?? 0) + 1)
|
||||
}
|
||||
|
||||
// Publish the generation FIRST: a subscriber reacting to membership must
|
||||
// already observe the lifecycle change when it starts a by-id lookup.
|
||||
tombstoneGenerations = generations
|
||||
$removedSessionIds.set(next)
|
||||
}
|
||||
|
||||
/** Generation snapshot to compare against later (see `tombstoneLifecycleChanged`). */
|
||||
export function captureSessionTombstoneGenerations(): SessionTombstoneGenerationSnapshot {
|
||||
return tombstoneGenerations
|
||||
}
|
||||
|
||||
/** True when any id's tombstone lifecycle moved since `snapshot` (ABA-safe). */
|
||||
export function tombstoneLifecycleChanged(
|
||||
snapshot: SessionTombstoneGenerationSnapshot,
|
||||
ids: Array<null | string | undefined>
|
||||
): boolean {
|
||||
return ids.some(id => {
|
||||
const target = id?.trim()
|
||||
|
||||
if (!target) {
|
||||
return false
|
||||
}
|
||||
|
||||
return snapshot.get(target) !== tombstoneGenerations.get(target)
|
||||
})
|
||||
}
|
||||
|
||||
export function tombstoneSessions(ids: Array<null | string | undefined>): void {
|
||||
const next = new Set($removedSessionIds.get())
|
||||
const before = next.size
|
||||
|
||||
for (const id of ids) {
|
||||
const trimmed = id?.trim()
|
||||
@@ -23,9 +87,7 @@ export function tombstoneSessions(ids: Array<null | string | undefined>): void {
|
||||
}
|
||||
}
|
||||
|
||||
if (next.size !== before) {
|
||||
$removedSessionIds.set(next)
|
||||
}
|
||||
setRemovedSessionIds(next)
|
||||
}
|
||||
|
||||
export function untombstoneSessions(ids: Array<null | string | undefined>): void {
|
||||
@@ -45,9 +107,7 @@ export function untombstoneSessions(ids: Array<null | string | undefined>): void
|
||||
}
|
||||
}
|
||||
|
||||
if (next.size !== current.size) {
|
||||
$removedSessionIds.set(next)
|
||||
}
|
||||
setRemovedSessionIds(next)
|
||||
}
|
||||
|
||||
// Ids whose delete/archive RPC is still in flight. Their tombstones are pinned
|
||||
|
||||
Reference in New Issue
Block a user