mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
fix(persistence): a session write cannot bring back a tab its partition recorded as closed (#27125)
* fix(persistence): a session write cannot bring back a tab its partition recorded as closed Main records every terminal close, but only PTY bindings and orphan adoption honoured the record. A renderer save that still listed the tab (when no topology revision fenced it yet), a relay write, or a fast unified-tabs patch could write the closed tab back. Every full write and unified-tabs patch now removes tabs the same partition has a live close record for, scoped to the record's worktree. Rollback stays exempt. * fix(persistence): the close fence accepts host slices that omit terminal maps A host slice with no terminal rows omits tabsByWorktree, and the forced close transform indexed it, so a browser-only save to a partition with a live close record threw and never persisted. Close only tabs actually listed, with the terminal maps supplied. * refactor(persistence): the close fence reuses the shared tab-listing check --------- Co-authored-by: m4air <m4air@m4airs-MacBook-Air.local>
This commit is contained in:
@@ -0,0 +1,186 @@
|
||||
import { mkdtempSync, rmSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { getDefaultWorkspaceSession } from '../../shared/constants'
|
||||
import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types'
|
||||
import { Store } from './loading-store/store'
|
||||
import { closeTestStores, createSqliteTestStore } from '../persistence-test-harness'
|
||||
import { OrcaRuntimeService } from '../runtime/orca-runtime'
|
||||
import {
|
||||
REPO_ID,
|
||||
TAB_ID,
|
||||
WORKTREE_ID,
|
||||
WORKTREE_PATH,
|
||||
makeSession
|
||||
} from '../runtime/__fixtures__/orca-runtime-terminal-close-continuity-fixtures'
|
||||
import { advanceTerminalTopologyRevision } from './terminal-topology/terminal-topology-membership'
|
||||
|
||||
const SSH_HOST_ID = 'ssh:target-1'
|
||||
const SSH_WORKTREE_ID = 'ssh-repo::/srv/app'
|
||||
const OTHER_TAB_ID = '7a1b2c3d-4e5f-4a6b-8c7d-9e0f1a2b3c4d'
|
||||
|
||||
const directories: string[] = []
|
||||
afterEach(async () => {
|
||||
await closeTestStores()
|
||||
for (const directory of directories.splice(0)) {
|
||||
rmSync(directory, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
function createPersistedStore() {
|
||||
const directory = mkdtempSync(join(tmpdir(), 'orca-closed-tab-fence-'))
|
||||
directories.push(directory)
|
||||
const dataFile = join(directory, 'orca-data.json')
|
||||
const store = createSqliteTestStore(Store, { dataFile })
|
||||
store.addRepo({
|
||||
id: REPO_ID,
|
||||
path: WORKTREE_PATH,
|
||||
displayName: 'L',
|
||||
badgeColor: 'gray',
|
||||
addedAt: 1
|
||||
})
|
||||
store.addRepo({
|
||||
id: 'ssh-repo',
|
||||
path: '/srv/app',
|
||||
displayName: 'R',
|
||||
badgeColor: 'gray',
|
||||
addedAt: 1,
|
||||
connectionId: 'target-1'
|
||||
})
|
||||
return {
|
||||
store,
|
||||
runtime: new OrcaRuntimeService(store),
|
||||
reload: async () => {
|
||||
store.flush()
|
||||
store.freezeWrites()
|
||||
await store.waitForPendingWrite()
|
||||
return createSqliteTestStore(Store, { dataFile })
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** A session holding TAB_ID (and OTHER_TAB_ID) for one worktree, with its unified tab and group. */
|
||||
function sessionFor(worktreeId: string): WorkspaceSessionState {
|
||||
const base = makeSession()
|
||||
const rows = base.tabsByWorktree[WORKTREE_ID]!.map((tab) => ({ ...tab, worktreeId }))
|
||||
const unified = (id: string, sortOrder: number) => ({
|
||||
id,
|
||||
entityId: id,
|
||||
groupId: 'group',
|
||||
worktreeId,
|
||||
contentType: 'terminal' as const,
|
||||
label: id,
|
||||
customLabel: null,
|
||||
color: null,
|
||||
sortOrder,
|
||||
createdAt: 1
|
||||
})
|
||||
return {
|
||||
...base,
|
||||
tabsByWorktree: { [worktreeId]: [...rows, { ...rows[0]!, id: OTHER_TAB_ID, sortOrder: 1 }] },
|
||||
unifiedTabs: { [worktreeId]: [unified(TAB_ID, 0), unified(OTHER_TAB_ID, 1)] },
|
||||
tabGroups: {
|
||||
[worktreeId]: [
|
||||
{ id: 'group', worktreeId, activeTabId: TAB_ID, tabOrder: [TAB_ID, OTHER_TAB_ID] }
|
||||
]
|
||||
},
|
||||
activeTabIdByWorktree: { [worktreeId]: TAB_ID }
|
||||
}
|
||||
}
|
||||
|
||||
describe('a write cannot bring back a tab its partition recorded as closed (#12447)', () => {
|
||||
// The close lands before the renderer save that first lists the tab, so no topology revision
|
||||
// fences that save: only the close record can refuse it.
|
||||
it.each([
|
||||
['local', undefined, WORKTREE_ID],
|
||||
['SSH', SSH_HOST_ID, SSH_WORKTREE_ID]
|
||||
] as const)(
|
||||
'drops a closed tab a late %s renderer save still lists',
|
||||
async (_label, hostId, worktreeId) => {
|
||||
const { store, runtime, reload } = createPersistedStore()
|
||||
store.setWorkspaceSession(
|
||||
{ ...getDefaultWorkspaceSession(), tabsByWorktree: { [worktreeId]: [] } },
|
||||
hostId
|
||||
)
|
||||
|
||||
await runtime.closeTerminalSurfaceFromRenderer({
|
||||
worktreeId,
|
||||
target: { kind: 'tab', tabId: TAB_ID }
|
||||
})
|
||||
store.setWorkspaceSession(sessionFor(worktreeId), hostId)
|
||||
|
||||
const after = (await reload()).getWorkspaceSession(hostId)
|
||||
expect(after.tabsByWorktree[worktreeId]?.map((tab) => tab.id)).toEqual([OTHER_TAB_ID])
|
||||
expect(after.terminalLayoutsByTabId[TAB_ID]).toBeUndefined()
|
||||
expect(after.unifiedTabs?.[worktreeId]?.map((tab) => tab.id)).toEqual([OTHER_TAB_ID])
|
||||
expect(after.tabGroups?.[worktreeId]?.[0]?.tabOrder).toEqual([OTHER_TAB_ID])
|
||||
expect(after.closedTerminalTabTombstonesByTabId?.[TAB_ID]).toBeDefined()
|
||||
}
|
||||
)
|
||||
|
||||
it('drops the closed tab from a unified-tabs-only patch', async () => {
|
||||
const { store, runtime } = createPersistedStore()
|
||||
const before = advanceTerminalTopologyRevision(sessionFor(WORKTREE_ID), WORKTREE_ID)
|
||||
store.setWorkspaceSession(before)
|
||||
await runtime.closeTerminalSurfaceFromRenderer({
|
||||
worktreeId: WORKTREE_ID,
|
||||
target: { kind: 'tab', tabId: TAB_ID }
|
||||
})
|
||||
|
||||
store.patchWorkspaceSession({ unifiedTabs: before.unifiedTabs })
|
||||
|
||||
expect(store.getWorkspaceSession().unifiedTabs?.[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([
|
||||
OTHER_TAB_ID
|
||||
])
|
||||
})
|
||||
|
||||
it('keeps a same-id tab under another worktree than the one the record names', async () => {
|
||||
const { store } = createPersistedStore()
|
||||
const session = sessionFor(WORKTREE_ID)
|
||||
store.setWorkspaceSession({
|
||||
...session,
|
||||
closedTerminalTabTombstonesByTabId: {
|
||||
[TAB_ID]: { closedAt: Date.now(), worktreeId: 'repo::/elsewhere', reason: 'user' }
|
||||
}
|
||||
})
|
||||
|
||||
expect(store.getWorkspaceSession().tabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([
|
||||
TAB_ID,
|
||||
OTHER_TAB_ID
|
||||
])
|
||||
})
|
||||
|
||||
// A host slice with no terminal rows omits the terminal maps altogether.
|
||||
it.each(['set', 'stage-before-unload'] as const)(
|
||||
'accepts a browser-only host slice once the partition has a close record (%s)',
|
||||
async (write) => {
|
||||
const { store, runtime } = createPersistedStore()
|
||||
store.setWorkspaceSession(
|
||||
{ ...getDefaultWorkspaceSession(), tabsByWorktree: { [SSH_WORKTREE_ID]: [] } },
|
||||
SSH_HOST_ID
|
||||
)
|
||||
await runtime.closeTerminalSurfaceFromRenderer({
|
||||
worktreeId: SSH_WORKTREE_ID,
|
||||
target: { kind: 'tab', tabId: TAB_ID }
|
||||
})
|
||||
const {
|
||||
tabsByWorktree: _omitted,
|
||||
terminalLayoutsByTabId: _alsoOmitted,
|
||||
...slice
|
||||
} = { ...getDefaultWorkspaceSession(), activeWorktreeId: SSH_WORKTREE_ID }
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: models the renderer's host split, which omits empty terminal maps from its IPC payload.
|
||||
const sparse = slice as WorkspaceSessionState
|
||||
|
||||
if (write === 'set') {
|
||||
store.setWorkspaceSession(sparse, SSH_HOST_ID)
|
||||
} else {
|
||||
store.stageWorkspaceSessionBeforeUnload(sparse, SSH_HOST_ID)
|
||||
}
|
||||
|
||||
const after = store.getWorkspaceSession(SSH_HOST_ID)
|
||||
expect(after.activeWorktreeId).toBe(SSH_WORKTREE_ID)
|
||||
expect(after.closedTerminalTabTombstonesByTabId?.[TAB_ID]).toBeDefined()
|
||||
}
|
||||
)
|
||||
})
|
||||
@@ -0,0 +1,39 @@
|
||||
import { hasClosedTerminalTabRecord } from '../../shared/closed-terminal-tab-tombstones'
|
||||
import {
|
||||
closeTerminalTabInWorkspaceSession,
|
||||
workspaceSessionListsTerminalTab
|
||||
} from '../../shared/workspace-session-terminal-tab-close'
|
||||
import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types'
|
||||
|
||||
/**
|
||||
* Removes every terminal tab this partition's own live close records name, from whichever writer
|
||||
* sent it back (a late renderer save, a relay snapshot, a fast patch). Tab ids are uuids, so a
|
||||
* recorded id is never a new tab; scoped to the record's worktree, and records past the TTL stop
|
||||
* suppressing. Main's rollback path bypasses this: it restores rows a failed close never committed.
|
||||
*/
|
||||
export function dropClosedTerminalTabs(
|
||||
session: WorkspaceSessionState,
|
||||
now = Date.now()
|
||||
): WorkspaceSessionState {
|
||||
const records = session.closedTerminalTabTombstonesByTabId
|
||||
if (!records) {
|
||||
return session
|
||||
}
|
||||
// Why: a host slice omits terminal maps when that host has no terminal rows.
|
||||
const withMaps = {
|
||||
...session,
|
||||
tabsByWorktree: session.tabsByWorktree ?? {},
|
||||
terminalLayoutsByTabId: session.terminalLayoutsByTabId ?? {}
|
||||
}
|
||||
let next: WorkspaceSessionState = withMaps
|
||||
for (const [tabId, { worktreeId }] of Object.entries(records)) {
|
||||
if (
|
||||
hasClosedTerminalTabRecord(records, tabId, undefined, now) &&
|
||||
workspaceSessionListsTerminalTab(next, worktreeId, tabId)
|
||||
) {
|
||||
next = closeTerminalTabInWorkspaceSession(next, worktreeId, tabId, { force: true }).session
|
||||
}
|
||||
}
|
||||
// Leaves a sparse slice as sent when nothing was dropped.
|
||||
return next === withMaps ? session : next
|
||||
}
|
||||
@@ -13,6 +13,7 @@ import { withoutRedundantGlobalFields } from '../../../shared/workspace-session-
|
||||
import { getRepoIdFromWorktreeId } from '../../../shared/worktree/id'
|
||||
import { readTerminalScrollbackSnapshotSync } from '../../terminal-scrollback-snapshots'
|
||||
import { preserveRuntimeAuthoredWorkspaceSessionFields } from '../runtime-authored-workspace-session-fields'
|
||||
import { dropClosedTerminalTabs } from '../closed-terminal-tab-write-fence'
|
||||
import { findWorktreeIdForTab } from '../restoring-sessions/pane-identity-migration'
|
||||
import { invalidateLocalWorktreeMetadataPruneInputs } from '../../local-worktree-metadata-prune-gate'
|
||||
import {
|
||||
@@ -197,7 +198,7 @@ export function setHostWorkspaceSession(
|
||||
owner[sessionHostPartitionOperationsContext].runtime.state.workspaceSessionsByHostId?.[hostId]
|
||||
// Why here and not at the callers: the before-unload stage path writes the renderer's payload
|
||||
// straight through, so a per-caller guard leaves the quit write erasing runtime-authored rows.
|
||||
session = preserveRuntimeAuthoredWorkspaceSessionFields(session, prior)
|
||||
session = dropClosedTerminalTabs(preserveRuntimeAuthoredWorkspaceSessionFields(session, prior))
|
||||
// Why: each partition owns its topology fence; renderer writes omit it and must rebase locally.
|
||||
session = sanitizeWorkspaceSessionTerminalRetirements(session, prior)
|
||||
session = preserveMissingWorkspaceSessionTerminalBindings(
|
||||
|
||||
@@ -18,6 +18,7 @@ import {
|
||||
setHostWorkspaceSession,
|
||||
withRequiredWorkspaceSessionMaps
|
||||
} from './session-host-partitions'
|
||||
import { dropClosedTerminalTabs } from '../closed-terminal-tab-write-fence'
|
||||
import { scheduleSave } from './write-scheduling'
|
||||
|
||||
type SessionSnapshotOperationsRuntime = Pick<
|
||||
@@ -96,6 +97,10 @@ export class SessionSnapshotOperations {
|
||||
if (Object.hasOwn(patch, 'browserUrlHistory')) {
|
||||
next = pruneWorkspaceSessionBrowserHistory(next)
|
||||
}
|
||||
// Why: a unified-tabs patch skips full normalization, so it would bypass the close fence.
|
||||
if (Object.hasOwn(patch, 'unifiedTabs')) {
|
||||
next = dropClosedTerminalTabs(next)
|
||||
}
|
||||
this.publishSession(next, resolved)
|
||||
}
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
migrateWorkspaceSessionTerminalScrollbackSnapshotsAsync
|
||||
} from '../../terminal-scrollback-snapshot-async-migration'
|
||||
import { preserveRuntimeAuthoredWorkspaceSessionFields } from '../runtime-authored-workspace-session-fields'
|
||||
import { dropClosedTerminalTabs } from '../closed-terminal-tab-write-fence'
|
||||
import { registerPersistedPaneKeyAlias } from '../restoring-sessions/pane-alias-normalization'
|
||||
import {
|
||||
normalizeWorkspaceSessionPaneIdentities,
|
||||
@@ -37,7 +38,7 @@ export function setLocalWorkspaceSession(
|
||||
const prior = context.runtime.state.workspaceSession
|
||||
// Why here and not at the callers: the before-unload stage path writes the renderer's payload
|
||||
// straight through, so a per-caller guard leaves the quit write erasing runtime-authored rows.
|
||||
session = preserveRuntimeAuthoredWorkspaceSessionFields(session, prior)
|
||||
session = dropClosedTerminalTabs(preserveRuntimeAuthoredWorkspaceSessionFields(session, prior))
|
||||
session = sanitizeWorkspaceSessionTerminalRetirements(session, prior)
|
||||
session = pruneWorkspaceSessionBrowserHistory(
|
||||
pruneLocalTerminalScrollbackBuffers(session, context.runtime.state.repos)
|
||||
|
||||
@@ -141,25 +141,17 @@ describe('close records', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('lets a close that removes a listed tab replace a record that tab already had', async () => {
|
||||
// A write cannot list a tab its partition recorded as closed, so legacy rows beside a record go.
|
||||
it('drops a listed tab its partition already recorded as closed, and keeps the record', async () => {
|
||||
const earlier = Date.now() - 24 * 60 * 60 * 1000
|
||||
const { store, runtime } = createPersistedRuntime({
|
||||
const record = { closedAt: earlier, worktreeId: WORKTREE_ID, reason: 'cleanup' as const }
|
||||
const { store } = createPersistedRuntime({
|
||||
...makeSession(),
|
||||
closedTerminalTabTombstonesByTabId: {
|
||||
[TAB_ID]: { closedAt: earlier, worktreeId: WORKTREE_ID, reason: 'cleanup' }
|
||||
}
|
||||
closedTerminalTabTombstonesByTabId: { [TAB_ID]: record }
|
||||
})
|
||||
|
||||
await runtime.closeTerminalSurfaceFromRenderer({
|
||||
worktreeId: WORKTREE_ID,
|
||||
target: { kind: 'tab', tabId: TAB_ID },
|
||||
reason: 'user'
|
||||
})
|
||||
|
||||
const record = store.getWorkspaceSession().closedTerminalTabTombstonesByTabId?.[TAB_ID]
|
||||
expect(store.getWorkspaceSession().tabsByWorktree[WORKTREE_ID]).toEqual([])
|
||||
expect(record?.reason).toBe('user')
|
||||
expect(record?.closedAt).toBeGreaterThan(earlier)
|
||||
expect(store.getWorkspaceSession().closedTerminalTabTombstonesByTabId?.[TAB_ID]).toEqual(record)
|
||||
})
|
||||
|
||||
it('admits a late spawn for a tab whose close record is past the TTL', async () => {
|
||||
|
||||
Reference in New Issue
Block a user