From 2646fc1d55ef0262fefeafbaf98e9c92b0f1ccdf Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 1 Sep 2026 03:11:37 -0700 Subject: [PATCH] fix(session): carry parked contested rows through full partition replaces attachHostSessionShadow skipped a parked field when nothing else routed to the co-claimant's slice. That is correct for the patch path (an omitted field leaves the partition untouched) but wrong for persistWorkspaceSessionByHost and the quit snapshots: setHostWorkspaceSession replaces the whole partition, so the omitted field erased the very rows the shadow exists to protect. The attach now takes the write mode and, on a full replace, seeds the missing field with the parked rows. --- .../workspace-session-host-contention.test.ts | 25 +++++++++++++++++++ .../lib/workspace-session-host-contention.ts | 21 ++++++++++++---- .../lib/workspace-session-host-persistence.ts | 15 +++++++---- 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/renderer/src/lib/workspace-session-host-contention.test.ts b/src/renderer/src/lib/workspace-session-host-contention.test.ts index d11be1ec16e..c4e6e3f7579 100644 --- a/src/renderer/src/lib/workspace-session-host-contention.test.ts +++ b/src/renderer/src/lib/workspace-session-host-contention.test.ts @@ -212,6 +212,31 @@ describe('writing a contested workspace id back', () => { expect(tabIds(localWrite, SHARED_ID)).toEqual(['local-tab']) }) + it('carries a parked field the slice never seeded through a full partition replace', async () => { + // Why: api.set swaps the whole partition, so a field with no live entry routing to the + // co-claimant would otherwise be written without its parked rows and erased on disk. + const set: SessionWriteMock = vi.fn(async () => {}) + await persistWorkspaceSessionByHost( + { + set, + get: vi.fn(), + patch: vi.fn(), + setSync: vi.fn(), + flush: vi.fn(async () => {}) + }, + { + ...sessionWithTabs({ [SHARED_ID]: [tab('local-tab')] }), + // Seeds the runtime slice (so its partition IS rewritten) without seeding tabsByWorktree. + lastVisitedAtByWorktreeId: { [RUNTIME_ONLY_ID]: 1 } + }, + runtimeCoClaimantState() + ) + + const runtimeWrite = set.mock.calls.find(([, hostId]) => hostId === RUNTIME_HOST)?.[0] + expect(runtimeWrite).toBeDefined() + expect(tabIds(runtimeWrite, SHARED_ID)).toEqual(['runtime-tab']) + }) + it('restores the parked rows on the debounced patch path too', () => { const patch: SessionPatchMock = vi.fn(async () => {}) patchWorkspaceSessionByHost( diff --git a/src/renderer/src/lib/workspace-session-host-contention.ts b/src/renderer/src/lib/workspace-session-host-contention.ts index 8d2a201f83b..ef1e29b3890 100644 --- a/src/renderer/src/lib/workspace-session-host-contention.ts +++ b/src/renderer/src/lib/workspace-session-host-contention.ts @@ -225,12 +225,16 @@ function hostStillClaimsKey( return !claimed || claimed.has(hostId) } +/** Whether the slices will be applied as a merge-by-field patch or a full partition replace. */ +export type HostSessionWriteMode = 'patch' | 'replace' + /** Write parked entries back into their own host's slice so a write for the primary host cannot * erase a co-claimant's persisted session. Mutates the slices produced by the split. */ export function attachHostSessionShadow( slices: HostSessionSlices, shadow: HostSessionSlices | undefined, - claims: WorktreeHostClaims + claims: WorktreeHostClaims, + mode: HostSessionWriteMode ): void { if (!shadow) { return @@ -245,12 +249,19 @@ export function attachHostSessionShadow( } for (const field of WORKTREE_KEYED_FIELDS) { const parked = shadowSlice[field] - const target = slice[field] - // Why present-only: a patch that omits the field leaves the partition's own copy untouched, - // so nothing needs restoring there. - if (!isWorkspaceSessionRecord(parked) || !isWorkspaceSessionRecord(target)) { + if (!isWorkspaceSessionRecord(parked)) { continue } + let target = slice[field] + if (!isWorkspaceSessionRecord(target)) { + // Why the mode split: a patch that omits the field leaves the partition's own copy + // untouched, but a full set erases omitted fields, so the parked rows must ride along. + if (mode === 'patch') { + continue + } + target = {} + ;(slice as WorkspaceSessionRecord)[field] = target + } for (const [key, entry] of Object.entries(parked)) { if (Object.hasOwn(target, key) || !hostStillClaimsKey(claims, key, hostId)) { continue diff --git a/src/renderer/src/lib/workspace-session-host-persistence.ts b/src/renderer/src/lib/workspace-session-host-persistence.ts index d9d4f4b4b98..26aa6ee714c 100644 --- a/src/renderer/src/lib/workspace-session-host-persistence.ts +++ b/src/renderer/src/lib/workspace-session-host-persistence.ts @@ -15,6 +15,7 @@ import { attachHostSessionShadow, indexWorktreeHostClaims, pickPrimaryHostForClaims, + type HostSessionWriteMode, type WorktreeHostClaims } from './workspace-session-host-contention' import { @@ -169,11 +170,12 @@ export function buildHostIdByWorktreeId(state: HostPersistenceState): HostIdByWo * rows of every host that lost a contested id so this write cannot erase them. */ function splitWorkspaceSessionForWrite( payload: WorkspaceSessionState, - state: HostPersistenceState + state: HostPersistenceState, + mode: HostSessionWriteMode ): HostSessionSlices { const routing = buildHostSessionRouting(state) const slices = splitWorkspaceSessionByHost(payload, routing.hostIdByWorktreeId) - attachHostSessionShadow(slices, state.contestedHostWorkspaceSessions, routing.claims) + attachHostSessionShadow(slices, state.contestedHostWorkspaceSessions, routing.claims, mode) return slices } @@ -185,7 +187,7 @@ export function patchWorkspaceSessionByHost( patch: WorkspaceSessionPatch, state: HostPersistenceState ): Promise { - const slices = splitWorkspaceSessionForWrite(patch as WorkspaceSessionState, state) + const slices = splitWorkspaceSessionForWrite(patch as WorkspaceSessionState, state, 'patch') const local = (slices[LOCAL_EXECUTION_HOST_ID] ?? patch) as WorkspaceSessionPatch const localWrite = api.patch(local) for (const [hostId, slice] of nonLocalHostSessionEntries(slices)) { @@ -205,7 +207,9 @@ export async function persistWorkspaceSessionByHost( payload: WorkspaceSessionState, state: HostPersistenceState ): Promise { - const slices = splitWorkspaceSessionForWrite(payload, state) + // Why 'replace': api.set swaps the whole partition, so parked rows must ride along even for + // fields nothing else routed to this host. + const slices = splitWorkspaceSessionForWrite(payload, state, 'replace') const writes: Promise[] = [api.set(slices[LOCAL_EXECUTION_HOST_ID] ?? payload)] for (const [hostId, slice] of nonLocalHostSessionEntries(slices)) { writes.push(api.set(slice, hostId)) @@ -219,7 +223,8 @@ export function buildWorkspaceSessionHostSnapshots( payload: WorkspaceSessionState, state: HostPersistenceState ): WorkspaceSessionHostSnapshot[] { - const slices = splitWorkspaceSessionForWrite(payload, state) + // Why 'replace': quit snapshots are applied as full partition sets. + const slices = splitWorkspaceSessionForWrite(payload, state, 'replace') return [ { state: slices[LOCAL_EXECUTION_HOST_ID] ?? payload }, ...nonLocalHostSessionEntries(slices).map(([hostId, hostState]) => ({