mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-23 16:36:23 +00:00
fix(desktop): honor optimistic session tombstones on list & tree refresh
A refresh racing an in-flight delete/archive resurrected the just-removed row: refreshSessions repopulated $sessions straight from the backend page ignoring tombstones, and the projects.tree prune dropped a tombstone the moment its id left scoped_session_ids — so the grouped lane un-filtered it too. The row only vanished on a later refresh once the RPC finally landed. refreshSessions now filters tombstoned rows (and their lineage tip) before merging. The prune keeps a tombstone while its mutation is still in flight locally via $sessionMutationsInFlight, then hands it back to the normal scoped-based prune once settled.
This commit is contained in:
parent
66747f154c
commit
4db2f78ec7
4 changed files with 126 additions and 3 deletions
|
|
@ -62,9 +62,18 @@ vi.mock('@/hermes', async importOriginal => ({
|
|||
listSidebarSessions: (...args: unknown[]) => listSidebarSessions(...args)
|
||||
}))
|
||||
|
||||
// The refresh only reads the optimistic tombstone set; stub it so we don't pull
|
||||
// the whole projects store (gateway / fs / git) into this hook's test.
|
||||
const removed = vi.hoisted(() => ({ ids: new Set<string>() }))
|
||||
|
||||
vi.mock('@/store/projects', () => ({
|
||||
$removedSessionIds: { get: () => removed.ids }
|
||||
}))
|
||||
|
||||
beforeEach(() => {
|
||||
listSidebarSessions.mockReset()
|
||||
listAllProfileSessions.mockReset()
|
||||
removed.ids = new Set()
|
||||
setSessions([])
|
||||
setCronSessions([])
|
||||
setMessagingSessions([])
|
||||
|
|
@ -146,6 +155,28 @@ describe('refreshSessions identity + loading hygiene', () => {
|
|||
expect(loadingStates).toEqual([false])
|
||||
})
|
||||
|
||||
it('drops rows the user just deleted, even when the backend page still lists them', async () => {
|
||||
// A delete RPC is in flight: the row is tombstoned optimistically but the
|
||||
// batched refresh still carries it (and a lineage-tip variant). Both must be
|
||||
// filtered so the optimistic removal never flashes back.
|
||||
removed.ids = new Set(['b', 'root-c'])
|
||||
listSidebarSessions.mockResolvedValue(
|
||||
sidebar({
|
||||
sessions: [row('a'), row('b'), row('c', { _lineage_root_id: 'root-c' } as Partial<SessionInfo>)],
|
||||
total: 3,
|
||||
profile_totals: {}
|
||||
})
|
||||
)
|
||||
|
||||
const { result } = renderHook(() => useSessionListActions({ profileScope: 'default' }))
|
||||
|
||||
await act(async () => {
|
||||
await result.current.refreshSessions()
|
||||
})
|
||||
|
||||
expect($sessions.get().map(s => s.id)).toEqual(['a'])
|
||||
})
|
||||
|
||||
it('still shows loading for the initial (empty-list) fetch', async () => {
|
||||
listSidebarSessions.mockResolvedValue(sidebar({ sessions: [row('a')], total: 1, profile_totals: {} }))
|
||||
const { result } = renderHook(() => useSessionListActions({ profileScope: 'default' }))
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ import {
|
|||
import { setCronJobs } from '@/store/cron'
|
||||
import { $pinnedSessionIds, $sessionsLimit, bumpSessionsLimit, SIDEBAR_SESSIONS_PAGE_SIZE } from '@/store/layout'
|
||||
import { ALL_PROFILES, normalizeProfileKey } from '@/store/profile'
|
||||
import { $removedSessionIds } from '@/store/projects'
|
||||
import {
|
||||
$messagingSessions,
|
||||
$selectedStoredSessionId,
|
||||
|
|
@ -180,12 +181,22 @@ export function useSessionListActions({ profileScope }: UseSessionListActionsArg
|
|||
if (refreshSessionsRequestRef.current === requestId) {
|
||||
const recents = result.recents
|
||||
|
||||
// Drop rows the user just deleted/archived: a refresh can race an
|
||||
// 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
|
||||
|
||||
// Signature-gate the swap (same pattern as cron/messaging): a refresh
|
||||
// that returns content-identical rows must keep the previous array
|
||||
// identity, or every sidebar memo keyed on $sessions recomputes and the
|
||||
// whole list re-renders once per turn/broadcast for nothing.
|
||||
setSessions(prev => {
|
||||
const next = mergeSessionPage(prev, recents.sessions, sessionsToKeep())
|
||||
const next = mergeSessionPage(prev, incoming, sessionsToKeep())
|
||||
|
||||
return sameCronSignature(prev, next) ? prev : next
|
||||
})
|
||||
|
|
|
|||
|
|
@ -10,9 +10,13 @@ import {
|
|||
$projectScope,
|
||||
$projectsRpcAvailable,
|
||||
$projectTree,
|
||||
$removedSessionIds,
|
||||
$sessionMutationsInFlight,
|
||||
$worktreeRefreshToken,
|
||||
ALL_PROJECTS,
|
||||
beginSessionMutation,
|
||||
createProject,
|
||||
endSessionMutation,
|
||||
enterProject,
|
||||
exitProjectScope,
|
||||
openProjectCreate,
|
||||
|
|
@ -21,7 +25,8 @@ import {
|
|||
refreshProjects,
|
||||
refreshProjectTree,
|
||||
refreshWorktrees,
|
||||
scanAndRecordRepos
|
||||
scanAndRecordRepos,
|
||||
tombstoneSessions
|
||||
} from './projects'
|
||||
|
||||
vi.mock('@/i18n', () => ({
|
||||
|
|
@ -408,3 +413,49 @@ describe('project tree profile isolation', () => {
|
|||
expect($projectTree.get().map(project => project.id)).toEqual(['profile-b'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('tombstone pruning', () => {
|
||||
const openGatewayReturning = (scopedIds: string[]) => {
|
||||
const gateway = {
|
||||
connectionState: 'open',
|
||||
request: vi.fn().mockResolvedValue({ active_id: null, projects: [], scoped_session_ids: scopedIds })
|
||||
}
|
||||
|
||||
activeGateway.mockImplementation(() => gateway as never)
|
||||
gatewayAtom.set(gateway as never)
|
||||
|
||||
return gateway
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
$removedSessionIds.set(new Set())
|
||||
$sessionMutationsInFlight.set(new Set())
|
||||
})
|
||||
|
||||
it('keeps an in-flight delete tombstone even when the backend snapshot omits it', async () => {
|
||||
// Optimistic delete: hide the row, mark the RPC as in flight.
|
||||
tombstoneSessions(['sess-1'])
|
||||
beginSessionMutation(['sess-1'])
|
||||
|
||||
// A projects.tree refresh races the pending delete: the id is already gone
|
||||
// from scope, but the RPC hasn't landed — the tombstone must survive so the
|
||||
// row doesn't flash back.
|
||||
openGatewayReturning([])
|
||||
await refreshProjectTree()
|
||||
|
||||
expect($removedSessionIds.get().has('sess-1')).toBe(true)
|
||||
})
|
||||
|
||||
it('prunes the tombstone once the mutation settles and scope no longer lists it', async () => {
|
||||
tombstoneSessions(['sess-1'])
|
||||
beginSessionMutation(['sess-1'])
|
||||
openGatewayReturning([])
|
||||
await refreshProjectTree()
|
||||
|
||||
// Delete RPC settled; the next refresh with the id absent from scope drops it.
|
||||
endSessionMutation(['sess-1'])
|
||||
await refreshProjectTree()
|
||||
|
||||
expect($removedSessionIds.get().has('sess-1')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -94,6 +94,32 @@ export function untombstoneSessions(ids: Array<null | string | undefined>): void
|
|||
}
|
||||
}
|
||||
|
||||
// Ids whose delete/archive RPC is still in flight. Their tombstones are pinned
|
||||
// against the projects.tree prune below: a refresh whose snapshot predates the
|
||||
// mutation completing must NOT drop the tombstone, or the row flashes back until
|
||||
// the backend catches up. Keyed by id, so concurrent deletes stay independent.
|
||||
export const $sessionMutationsInFlight = atom<Set<string>>(new Set())
|
||||
|
||||
function mutateInFlight(ids: Array<null | string | undefined>, add: boolean): void {
|
||||
const current = $sessionMutationsInFlight.get()
|
||||
const next = new Set(current)
|
||||
|
||||
for (const id of ids) {
|
||||
const trimmed = id?.trim()
|
||||
|
||||
if (trimmed) {
|
||||
add ? next.add(trimmed) : next.delete(trimmed)
|
||||
}
|
||||
}
|
||||
|
||||
if (next.size !== current.size) {
|
||||
$sessionMutationsInFlight.set(next)
|
||||
}
|
||||
}
|
||||
|
||||
export const beginSessionMutation = (ids: Array<null | string | undefined>): void => mutateInFlight(ids, true)
|
||||
export const endSessionMutation = (ids: Array<null | string | undefined>): void => mutateInFlight(ids, false)
|
||||
|
||||
// True while the disk scan is in flight (drives the "finding repos" hint).
|
||||
export const $reposScanning = atom(false)
|
||||
|
||||
|
|
@ -341,7 +367,11 @@ async function refreshProjectTreeOn(gateway: HermesGateway): Promise<void> {
|
|||
const tombstones = $removedSessionIds.get()
|
||||
|
||||
if (tombstones.size) {
|
||||
const pending = new Set([...tombstones].filter(id => scoped.has(id)))
|
||||
// Keep a tombstone while the backend still lists the id (delete pending on
|
||||
// its side) OR while its mutation is still in flight locally — dropping it
|
||||
// early flashes the row back until the RPC lands.
|
||||
const inFlight = $sessionMutationsInFlight.get()
|
||||
const pending = new Set([...tombstones].filter(id => scoped.has(id) || inFlight.has(id)))
|
||||
|
||||
if (pending.size !== tombstones.size) {
|
||||
$removedSessionIds.set(pending)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue