fix(mobile): harden persisted tab selection cleanup

This commit is contained in:
Brennan Benson
2026-07-21 18:41:29 -07:00
parent a26fd44d7e
commit 5e70fec18c
4 changed files with 150 additions and 19 deletions
+52
View File
@@ -9418,6 +9418,7 @@ describe('Store', () => {
})
store.removeProject('r1')
store.flush()
expect(store.getMobileClientTabSelections()['device-a']).toEqual({
'other-repo::/tmp/wt': {
@@ -9426,6 +9427,57 @@ describe('Store', () => {
activeTabIdByGroupId: {}
}
})
const reloaded = await createStore()
expect(reloaded.getMobileClientTabSelections()['device-a']).toEqual({
'other-repo::/tmp/wt': {
activeTabId: 'tab-2',
activeGroupId: null,
activeTabIdByGroupId: {}
}
})
})
it('prunes selections when a folder workspace is removed directly or with its group', async () => {
const store = await createStore()
const directGroup = store.createProjectGroup({
name: 'Direct',
parentPath: '/tmp/direct',
createdFrom: 'manual'
})
const directWorkspace = store.createFolderWorkspace({
projectGroupId: directGroup.id,
name: 'Direct workspace'
})
const cascadeGroup = store.createProjectGroup({
name: 'Cascade',
parentPath: '/tmp/cascade',
createdFrom: 'manual'
})
const cascadeWorkspace = store.createFolderWorkspace({
projectGroupId: cascadeGroup.id,
name: 'Cascade workspace'
})
store.setMobileClientTabSelections({
'device-a': {
[folderWorkspaceKey(directWorkspace.id)]: {
activeTabId: 'tab-direct',
activeGroupId: null,
activeTabIdByGroupId: {}
},
[folderWorkspaceKey(cascadeWorkspace.id)]: {
activeTabId: 'tab-cascade',
activeGroupId: null,
activeTabIdByGroupId: {}
}
}
})
store.removeFolderWorkspace(directWorkspace.id)
store.deleteProjectGroup(cascadeGroup.id)
store.flush()
const reloaded = await createStore()
expect(reloaded.getMobileClientTabSelections()).toEqual({})
})
})
+15 -4
View File
@@ -3885,8 +3885,10 @@ export class Store {
? { ...repo, projectGroupId: null }
: repo
)
const removedFolderWorkspaceKeys = new Set<string>()
for (const workspace of this.state.folderWorkspaces ?? []) {
if (deletedGroupIds.has(workspace.projectGroupId)) {
removedFolderWorkspaceKeys.add(folderWorkspaceKey(workspace.id))
this.state.workspaceSession = removeWorkspaceSessionOwner(
this.state.workspaceSession,
folderWorkspaceKey(workspace.id)
@@ -3897,6 +3899,7 @@ export class Store {
this.state.folderWorkspaces = (this.state.folderWorkspaces ?? []).filter(
(workspace) => !deletedGroupIds.has(workspace.projectGroupId)
)
this.pruneMobileClientTabSelections((worktreeId) => removedFolderWorkspaceKeys.has(worktreeId))
this.scheduleSave()
return true
}
@@ -4046,6 +4049,7 @@ export class Store {
folderWorkspaceKey(id)
)!
this.removeWorkspaceLineageForFolderParent(id)
this.pruneMobileClientTabSelections((worktreeId) => worktreeId === folderWorkspaceKey(id))
this.scheduleSave()
return true
}
@@ -4204,14 +4208,21 @@ export class Store {
delete this.state.workspaceLineageByChildKey[childKey as WorkspaceKey]
}
}
for (const selectionsByWorktree of Object.values(
this.pruneMobileClientTabSelections(belongsToHost)
}
private pruneMobileClientTabSelections(matchesWorktreeId: (worktreeId: string) => boolean): void {
for (const [clientNavigationId, selectionsByWorktree] of Object.entries(
this.state.mobileClientTabSelectionsByDeviceId ?? {}
)) {
for (const key of Object.keys(selectionsByWorktree)) {
if (belongsToHost(key)) {
delete selectionsByWorktree[key]
for (const worktreeId of Object.keys(selectionsByWorktree)) {
if (matchesWorktreeId(worktreeId)) {
delete selectionsByWorktree[worktreeId]
}
}
if (Object.keys(selectionsByWorktree).length === 0) {
delete this.state.mobileClientTabSelectionsByDeviceId?.[clientNavigationId]
}
}
}
@@ -179,6 +179,30 @@ describe('client session-tab selection', () => {
expect(persisted.length).toBe(writes)
})
it('does not persist topology-only projections from unrelated worktrees', () => {
const persisted: PersistedMobileClientTabSelections[] = []
const store = new ClientSessionTabSelectionStore()
store.setPersistListener((state) => persisted.push(state))
store.project({ ...snapshot(), worktree: 'listed-only' }, 'device-a')
store.activate(snapshot(), 'device-a', 'browser-unified')
expect(persisted).toEqual([
{
'device-a': {
'wt-1': {
activeTabId: 'browser-unified',
activeGroupId: 'group-right',
activeTabIdByGroupId: { 'group-right': 'browser-unified' }
}
}
}
])
store.forgetWorktree('listed-only')
expect(persisted).toHaveLength(1)
})
it('does not let an empty snapshot wipe a hydrated selection before tabs arrive', () => {
const store = new ClientSessionTabSelectionStore()
store.hydrate({
@@ -187,8 +211,15 @@ describe('client session-tab selection', () => {
}
})
const empty = { ...snapshot(), tabGroups: [], tabs: [] }
expect(store.project(empty, 'device-a').activeTabId).toBe('terminal-a::leaf-a')
const empty = {
...snapshot(),
activeGroupId: null,
activeTabId: null,
activeTabType: null,
tabGroups: [],
tabs: []
}
expect(store.project(empty, 'device-a').activeTabId).toBeNull()
expect(store.project(snapshot(), 'device-a').activeTabId).toBe('browser-unified')
})
@@ -210,5 +241,19 @@ describe('client session-tab selection', () => {
})
expect(normalizePersistedMobileClientTabSelections(null)).toEqual({})
expect(normalizePersistedMobileClientTabSelections('garbage')).toEqual({})
expect(normalizePersistedMobileClientTabSelections([{ 'wt-1': {} }])).toEqual({})
expect(
normalizePersistedMobileClientTabSelections({
'device-array': [{ activeTabId: 'tab-1' }],
'device-selection-array': { 'wt-1': ['tab-1'] },
'device-group-array': {
'wt-1': { activeTabId: 'tab-1', activeGroupId: null, activeTabIdByGroupId: ['tab-1'] }
}
})
).toEqual({
'device-group-array': {
'wt-1': { activeTabId: 'tab-1', activeGroupId: null, activeTabIdByGroupId: {} }
}
})
})
})
@@ -13,6 +13,8 @@ export type ClientSessionTabSelection = {
type StoredClientSessionTabSelection = {
selection: ClientSessionTabSelection
revision: number
// Why: listAll projects every worktree; only hydrated or user-activated selections belong on disk.
shouldPersist: boolean
}
function emptyClientSessionTabSelection(): ClientSessionTabSelection {
@@ -126,14 +128,18 @@ export function projectClientSessionTabSelection(
}
function normalizeClientSessionTabSelection(raw: unknown): ClientSessionTabSelection | null {
if (typeof raw !== 'object' || raw === null) {
if (typeof raw !== 'object' || raw === null || Array.isArray(raw)) {
return null
}
const candidate = raw as Partial<ClientSessionTabSelection>
const activeTabId = typeof candidate.activeTabId === 'string' ? candidate.activeTabId : null
const activeGroupId = typeof candidate.activeGroupId === 'string' ? candidate.activeGroupId : null
const activeTabIdByGroupId: Record<string, string> = {}
if (typeof candidate.activeTabIdByGroupId === 'object' && candidate.activeTabIdByGroupId) {
if (
typeof candidate.activeTabIdByGroupId === 'object' &&
candidate.activeTabIdByGroupId &&
!Array.isArray(candidate.activeTabIdByGroupId)
) {
for (const [groupId, tabId] of Object.entries(candidate.activeTabIdByGroupId)) {
if (typeof tabId === 'string') {
activeTabIdByGroupId[groupId] = tabId
@@ -151,11 +157,15 @@ export function normalizePersistedMobileClientTabSelections(
raw: unknown
): PersistedMobileClientTabSelections {
const normalized: PersistedMobileClientTabSelections = {}
if (typeof raw !== 'object' || raw === null) {
if (typeof raw !== 'object' || raw === null || Array.isArray(raw)) {
return normalized
}
for (const [clientNavigationId, selectionsByWorktree] of Object.entries(raw)) {
if (typeof selectionsByWorktree !== 'object' || selectionsByWorktree === null) {
if (
typeof selectionsByWorktree !== 'object' ||
selectionsByWorktree === null ||
Array.isArray(selectionsByWorktree)
) {
continue
}
const entries: Record<string, ClientSessionTabSelection> = {}
@@ -183,7 +193,7 @@ export class ClientSessionTabSelectionStore {
)) {
const statesByWorktree = this.getStatesByWorktree(clientNavigationId)
for (const [worktreeId, selection] of Object.entries(selectionsByWorktree)) {
statesByWorktree.set(worktreeId, { selection, revision: 0 })
statesByWorktree.set(worktreeId, { selection, revision: 0, shouldPersist: true })
}
}
}
@@ -197,7 +207,9 @@ export class ClientSessionTabSelectionStore {
for (const [clientNavigationId, statesByWorktree] of this.statesByClient) {
const entries: Record<string, ClientSessionTabSelection> = {}
for (const [worktreeId, state] of statesByWorktree) {
entries[worktreeId] = state.selection
if (state.shouldPersist) {
entries[worktreeId] = state.selection
}
}
if (Object.keys(entries).length > 0) {
persisted[clientNavigationId] = entries
@@ -232,7 +244,8 @@ export class ClientSessionTabSelectionStore {
const state = statesByWorktree.get(snapshot.worktree) ?? {
// Why: host focus is private navigation; a new paired device starts from deterministic topology instead of inheriting it.
selection: emptyClientSessionTabSelection(),
revision: 0
revision: 0,
shouldPersist: false
}
if (snapshot.tabs.length === 0) {
// Why: an empty snapshot has no topology to project; writing it back would wipe a restart-hydrated selection before tabs arrive.
@@ -245,7 +258,8 @@ export class ClientSessionTabSelectionStore {
const projected = projectClientSessionTabSelection(snapshot, state.selection)
statesByWorktree.set(snapshot.worktree, {
selection: projected.selection,
revision: state.revision
revision: state.revision,
shouldPersist: state.shouldPersist
})
return {
...projected.snapshot,
@@ -262,18 +276,25 @@ export class ClientSessionTabSelectionStore {
const statesByWorktree = this.getStatesByWorktree(clientNavigationId)
const state = statesByWorktree.get(snapshot.worktree) ?? {
selection: emptyClientSessionTabSelection(),
revision: 0
revision: 0,
shouldPersist: false
}
const nextSelection = activateClientSessionTabSelection(snapshot, state.selection, activeTabId)
statesByWorktree.set(snapshot.worktree, {
selection: activateClientSessionTabSelection(snapshot, state.selection, activeTabId),
revision: state.revision + 1
selection: nextSelection,
revision: state.revision + 1,
shouldPersist: true
})
this.persistNow()
return this.project(snapshot, clientNavigationId)
}
forgetClient(clientNavigationId: string): void {
if (this.statesByClient.delete(clientNavigationId)) {
const statesByWorktree = this.statesByClient.get(clientNavigationId)
const hadPersistedState = [...(statesByWorktree?.values() ?? [])].some(
(state) => state.shouldPersist
)
if (this.statesByClient.delete(clientNavigationId) && hadPersistedState) {
this.persistNow()
}
}
@@ -281,7 +302,9 @@ export class ClientSessionTabSelectionStore {
forgetWorktree(worktreeId: string): void {
let changed = false
for (const [clientNavigationId, statesByWorktree] of this.statesByClient) {
changed = statesByWorktree.delete(worktreeId) || changed
const state = statesByWorktree.get(worktreeId)
changed = Boolean(state?.shouldPersist) || changed
statesByWorktree.delete(worktreeId)
if (statesByWorktree.size === 0) {
this.statesByClient.delete(clientNavigationId)
}