fix(layout): a pane main closed stays closed when its terminal starts late (dark, layout-owner build only) (#30560)

This commit is contained in:
Jinwoo Hong
2026-10-19 03:21:32 -04:00
committed by GitHub
parent f7ce68fb1b
commit f6cc8f67f5
5 changed files with 549 additions and 22 deletions
@@ -0,0 +1,225 @@
import { closeTestStores, createStore, testState } from './persistence-test-harness'
import { mkdtempSync, rmSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { collectLayoutLeafIdsInOrder } from '../shared/workspace-layout/terminal-pane-tree'
import { TEST_LEAF_1, TEST_LEAF_2 } from './persistence-session-fixtures'
import { _resetClosedPaneFencesForTests } from './persistence/terminal-topology/closed-pane-binding-fence'
import { closeLeafOrTab } from './persistence/terminal-topology/terminal-topology-commit'
import { captureTerminalCloseRequest } from './runtime/terminal-surface-close'
import type { TerminalSurfaceCloseTarget } from '../shared/terminal-surface-close-target'
import { LOCAL_EXECUTION_HOST_ID, type ExecutionHostId } from '../shared/execution-host'
vi.mock('electron', () => ({
app: { getPath: () => testState.dir },
safeStorage: { isEncryptionAvailable: () => false }
}))
vi.mock('../shared/layout-owner-switch', () => ({ ORCA_LAYOUT_OWNER_IS_MAIN: true }))
const WORKTREE = 'repo1::/worktree'
const TAB = 'host-tab'
type TestStore = ReturnType<typeof createStore>
/** A tab main wrote and whose terminal started, then a split main wrote whose terminal is starting. */
async function tabWithStartingSplit(store: TestStore): Promise<void> {
expect(
await store.admitTerminalPane({
type: 'createTerminalTab',
workspace: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_1
})
).toBe('admitted')
expect(
await store.persistPtyBinding({
worktreeId: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_1,
ptyId: 'source-pty',
mayCreate: false
})
).toBe(true)
expect(
await store.admitTerminalPane({
type: 'splitPane',
workspace: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_1,
direction: 'vertical',
newLeafId: TEST_LEAF_2
})
).toBe('admitted')
}
/** The window's spawn of the split's terminal committing late: it names no expected state. */
function lateWindowBinding(store: TestStore): Promise<boolean> {
return store.persistPtyBinding({
worktreeId: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_2,
ptyId: 'late-pty',
origin: 'spawn'
})
}
/** Main's own close of a pane or tab (the CLI's, a phone's), through the one close transaction. */
async function closeThroughMain(
store: TestStore,
target: TerminalSurfaceCloseTarget
): Promise<void> {
const hostIds: readonly ExecutionHostId[] = [LOCAL_EXECUTION_HOST_ID]
const refusal = await store.runDurableMutation(
closeLeafOrTab({
worktreeId: WORKTREE,
target,
options: { allowMissing: true },
...captureTerminalCloseRequest(WORKTREE, target, hostIds, store, false),
ownerMatches: () => true,
hostIds: () => hostIds,
getSession: (hostId) => store.getWorkspaceSession(hostId),
setSession: (session, hostId) => store.setWorkspaceSession(session, hostId),
onClosed: () => {}
})
)
expect(refusal).toBeUndefined()
}
function leafIds(store: TestStore): string[] {
const root = store.getWorkspaceSession().terminalLayoutsByTabId[TAB]?.root
return root ? collectLayoutLeafIdsInOrder(root) : []
}
describe('a pane main closed or took back is not brought back by its late terminal binding', () => {
beforeEach(() => {
testState.dir = mkdtempSync(join(tmpdir(), 'orca-closed-pane-fence-'))
})
afterEach(async () => {
vi.useRealTimers()
_resetClosedPaneFencesForTests()
await closeTestStores()
rmSync(testState.dir, { recursive: true, force: true })
})
it('refuses the binding of a split main took back when its terminal did not start in time', async () => {
const store = createStore()
await tabWithStartingSplit(store)
await store.withdrawTerminalPane({
type: 'closePane',
workspace: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_2
})
expect(await lateWindowBinding(store)).toBe(false)
expect(leafIds(store)).toEqual([TEST_LEAF_1])
})
it('refuses the binding of a pane the user closed while its terminal was starting', async () => {
const store = createStore()
await tabWithStartingSplit(store)
const reply = await store.applyLayoutCommand({
type: 'closePane',
workspace: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_2,
userPaneClose: true
})
expect(reply.ok).toBe(true)
expect(await lateWindowBinding(store)).toBe(false)
expect(leafIds(store)).toEqual([TEST_LEAF_1])
})
it('refuses the binding of a pane main closed while its terminal was starting', async () => {
const store = createStore()
await tabWithStartingSplit(store)
await closeThroughMain(store, { kind: 'pane', tabId: TAB, leafId: TEST_LEAF_2 })
expect(await lateWindowBinding(store)).toBe(false)
expect(leafIds(store)).toEqual([TEST_LEAF_1])
})
it('refuses the binding of a pane whose tab the window closed while it was starting', async () => {
const store = createStore()
await tabWithStartingSplit(store)
const reply = await store.applyLayoutCommand({
type: 'closeTabs',
workspace: WORKTREE,
tabIds: [TAB],
force: true
})
expect(reply.ok).toBe(true)
expect(await lateWindowBinding(store)).toBe(false)
expect(store.getWorkspaceSession().terminalLayoutsByTabId[TAB]).toBeUndefined()
})
it('binds a pane the window creates again under the same id', async () => {
const store = createStore()
await tabWithStartingSplit(store)
await store.withdrawTerminalPane({
type: 'closePane',
workspace: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_2
})
const reply = await store.applyLayoutCommand({
type: 'splitPane',
workspace: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_1,
direction: 'vertical',
newLeafId: TEST_LEAF_2
})
expect(reply.ok).toBe(true)
expect(await lateWindowBinding(store)).toBe(true)
})
it('binds a pane a real create wrote again under the same id', async () => {
const store = createStore()
await tabWithStartingSplit(store)
await store.withdrawTerminalPane({
type: 'closePane',
workspace: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_2
})
expect(
await store.admitTerminalPane({
type: 'splitPane',
workspace: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_1,
direction: 'vertical',
newLeafId: TEST_LEAF_2
})
).toBe('admitted')
expect(await lateWindowBinding(store)).toBe(true)
expect(leafIds(store)).toEqual([TEST_LEAF_1, TEST_LEAF_2])
})
it('lets a binding create the pane again once the binding window has passed', async () => {
vi.useFakeTimers({ toFake: ['Date'] })
const store = createStore()
await tabWithStartingSplit(store)
await store.withdrawTerminalPane({
type: 'closePane',
workspace: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_2
})
vi.setSystemTime(Date.now() + 5 * 60_000 + 1_000)
expect(await lateWindowBinding(store)).toBe(true)
expect(leafIds(store)).toEqual([TEST_LEAF_1, TEST_LEAF_2])
})
})
@@ -4,6 +4,8 @@ import type { WorkspaceSessionState } from '../../../shared/workspace-session-st
import type { TerminalSessionPartition } from '../../../shared/workspace-layout/terminal-owner-invariants'
import { layoutContainsLeafId } from '../restoring-sessions/terminal-layout-normalization'
import type { PtyBindingSourceExpectation } from './store'
import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../shared/layout-owner-switch'
import { isClosedPaneFenced } from '../terminal-topology/closed-pane-binding-fence'
export type PtyBindingRefusalRequest = {
tabId: string
@@ -15,7 +17,7 @@ export type PtyBindingRefusalRequest = {
}
/**
* The five fences a binding must clear before anything is mutated, so a refusal leaves nothing
* The fences a binding must clear before anything is mutated, so a refusal leaves nothing
* half-written. Order matters: every `false` here is returned before the write path or the
* fast lane can run, which is what the relay's lease expiry and the stable-owner throw rely on.
*/
@@ -74,6 +76,17 @@ export function ptyBindingIsRefused(
) {
return true
}
// Switch point: a pane main closed or took back is not minted again by a start already under way.
if (
ORCA_LAYOUT_OWNER_IS_MAIN &&
!layoutContainsLeafId(
session.terminalLayoutsByTabId?.[args.tabId]?.root ?? null,
args.leafId
) &&
isClosedPaneFenced(paneKey)
) {
return true
}
// Mirrors the four creating branches of the write path — mint a tab, mint a root leaf, split
// the root and graft a leaf, mint a layout — each of which sets `terminalMembershipChanged`.
if (
@@ -0,0 +1,102 @@
import { afterEach, describe, expect, it, vi } from 'vitest'
import type { ExecutionHostId } from '../../../shared/execution-host'
import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types'
import { getDefaultWorkspaceSession } from '../../../shared/constants'
import {
_resetClosedPaneFencesForTests,
fencedIfLayoutOwner,
isClosedPaneFenced,
partitionsFence
} from './closed-pane-binding-fence'
vi.mock('../../../shared/layout-owner-switch', () => ({ ORCA_LAYOUT_OWNER_IS_MAIN: true }))
const LOCAL: ExecutionHostId = 'local'
const SSH: ExecutionHostId = 'ssh:box'
function sessionWith(tabId: string, leafIds: string[]): WorkspaceSessionState {
const [first, second] = leafIds
return {
...getDefaultWorkspaceSession(),
terminalLayoutsByTabId: first
? {
[tabId]: {
root: second
? {
type: 'split',
direction: 'vertical',
first: { type: 'leaf', leafId: first },
second: { type: 'leaf', leafId: second }
}
: { type: 'leaf', leafId: first },
activeLeafId: first,
expandedLeafId: null
}
}
: {}
}
}
function partitions(initial: Partial<Record<ExecutionHostId, WorkspaceSessionState>>) {
const sessions = new Map(Object.entries(initial))
return {
get: (hostId: ExecutionHostId) => sessions.get(hostId),
set: (hostId: ExecutionHostId, session: WorkspaceSessionState) => sessions.set(hostId, session)
}
}
describe('the closed-pane fence', () => {
afterEach(() => _resetClosedPaneFencesForTests())
it('a write elsewhere that only keeps a fenced pane does not lift its fence', () => {
// An old relay reattach left the tab in both partitions; the SSH copy is taken back.
const store = partitions({
[SSH]: sessionWith('t', ['a', 'b']),
[LOCAL]: sessionWith('t', ['a', 'b'])
})
fencedIfLayoutOwner(
() => partitionsFence([SSH], store.get, true),
() => (store.set(SSH, sessionWith('t', ['a'])), { value: null })
)()
expect(isClosedPaneFenced('t:b')).toBe(true)
fencedIfLayoutOwner(
() => partitionsFence([LOCAL], store.get, false),
() => (store.set(LOCAL, sessionWith('t', ['a', 'b'])), { value: null })
)()
expect(isClosedPaneFenced('t:b')).toBe(true)
})
it('a rolled-back close lifts the fence it wrote, and a rolled-back create restores the one it lifted', () => {
const store = partitions({ [LOCAL]: sessionWith('t', ['a', 'b']) })
const close = fencedIfLayoutOwner(
() => partitionsFence([LOCAL], store.get, true),
() => (store.set(LOCAL, sessionWith('t', ['a'])), { value: null, rollback: () => {} })
)()
expect(isClosedPaneFenced('t:b')).toBe(true)
close.rollback?.()
store.set(LOCAL, sessionWith('t', ['a', 'b']))
expect(isClosedPaneFenced('t:b')).toBe(false)
fencedIfLayoutOwner(
() => partitionsFence([LOCAL], store.get, true),
() => (store.set(LOCAL, sessionWith('t', ['a'])), { value: null })
)()
const create = fencedIfLayoutOwner(
() => partitionsFence([LOCAL], store.get, false),
() => (store.set(LOCAL, sessionWith('t', ['a', 'b'])), { value: null, rollback: () => {} })
)()
expect(isClosedPaneFenced('t:b')).toBe(false)
create.rollback?.()
expect(isClosedPaneFenced('t:b')).toBe(true)
})
it('records nothing for an unfenced command', () => {
const store = partitions({ [LOCAL]: sessionWith('t', ['a', 'b']) })
fencedIfLayoutOwner(
() => null,
() => (store.set(LOCAL, sessionWith('t', ['a'])), { value: null })
)()
expect(isClosedPaneFenced('t:b')).toBe(false)
})
})
@@ -0,0 +1,166 @@
// Panes main closed or took back, kept in memory so a terminal start already under way cannot bring
// one back by its late binding (pty-binding-session-update.ts mints a missing pane). Layout-owner
// build only: every caller is behind ORCA_LAYOUT_OWNER_IS_MAIN.
import type { ExecutionHostId } from '../../../shared/execution-host'
import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../shared/layout-owner-switch'
import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types'
import { collectLayoutLeafIdsInOrder } from '../../../shared/workspace-layout/terminal-pane-tree'
import type { LayoutCommand } from '../../../shared/workspace-layout/workspace-layout-command-types'
import type { DurableProfileStateMutation } from '../loading-store/store-runtime-state'
/**
* Why 5 min: past the longest a start can wait before it binds. An SSH start may first wait on a
* credential prompt (SSH_CREDENTIAL_TIMEOUT_MS, 120 s) and a connect (CONNECT_TIMEOUT_MS, 30 s,
* both ssh-connection-utils.ts), then on its relay request (REQUEST_TIMEOUT_MS, 30 s,
* ssh-channel-multiplexer.ts); the local daemon admits a spawn within 12 s
* (MAX_SPAWN_ADMISSION_WINDOW_MS, daemon-terminal-admission.ts). Pane ids are never reused, so a
* longer fence refuses nothing legitimate; the bound only limits memory.
*/
export const CLOSED_PANE_BINDING_FENCE_MS = 5 * 60_000
const closedAtByPaneKey = new Map<string, number>()
type SessionsRead = {
hostIds: () => readonly ExecutionHostId[]
getSession: (hostId: ExecutionHostId) => WorkspaceSessionState | null | undefined
}
type Fence = { sessions: SessionsRead; fences: boolean }
function paneKeysIn(
hostIds: readonly ExecutionHostId[],
getSession: SessionsRead['getSession']
): Set<string> {
const keys = new Set<string>()
for (const hostId of hostIds) {
const layouts = getSession(hostId)?.terminalLayoutsByTabId ?? {}
for (const [tabId, layout] of Object.entries(layouts)) {
if (layout?.root) {
for (const leafId of collectLayoutLeafIdsInOrder(layout.root)) {
keys.add(`${tabId}:${leafId}`)
}
}
}
}
return keys
}
/**
* Switch point: in a layout-owner build, `mutate` runs through `fencingRemovedPanes` with what `fence`
* names (null: unfenced). Otherwise it is `mutate` itself; `fence` is never called.
*/
export function fencedIfLayoutOwner<V>(
fence: () => Fence | null,
mutate: () => DurableProfileStateMutation<V>
): () => DurableProfileStateMutation<V> {
if (!ORCA_LAYOUT_OWNER_IS_MAIN) {
return mutate
}
return () => {
const plan = fence()
return plan ? fencingRemovedPanes(plan.sessions, plan.fences, mutate) : mutate()
}
}
/** A write to these partitions: `fences` when it is a close or take-back, else it only lifts. */
export function partitionsFence(
hostIds: readonly ExecutionHostId[],
getSession: SessionsRead['getSession'],
fences: boolean
): Fence {
return { sessions: { hostIds: () => hostIds, getSession }, fences }
}
/** A window or client layout command: its closes fence, its terminal creates lift; others neither. */
export function layoutCommandFence(
type: LayoutCommand['type'],
hostIds: readonly ExecutionHostId[],
getSession: SessionsRead['getSession']
): Fence | null {
if (type === 'closePane' || type === 'closeTabs') {
return partitionsFence(hostIds, getSession, true)
}
if (type === 'createTerminalTab' || type === 'splitPane') {
return partitionsFence(hostIds, getSession, false)
}
return null
}
/**
* Runs a write of main's, then fences the panes it removed when `fences` (a close or take-back) and
* lifts the fence of each pane it newly wrote: a real create under the same id binds as any. A
* rolled-back write puts back the fences it changed.
*/
function fencingRemovedPanes<V>(
sessions: SessionsRead,
fences: boolean,
write: () => DurableProfileStateMutation<V>
): DurableProfileStateMutation<V> {
// The partitions as they were before: a tab's close can drop the partition list it came from.
const hostIds = [...sessions.hostIds()]
const before = paneKeysIn(hostIds, sessions.getSession)
const result = write()
const after = paneKeysIn(hostIds, sessions.getSession)
const now = Date.now()
const changed = new Map<string, number | undefined>()
const set = (paneKey: string, closedAt: number | undefined): void => {
if (!changed.has(paneKey)) {
changed.set(paneKey, closedAtByPaneKey.get(paneKey))
}
if (closedAt === undefined) {
closedAtByPaneKey.delete(paneKey)
} else {
closedAtByPaneKey.set(paneKey, closedAt)
}
}
for (const [paneKey, closedAt] of closedAtByPaneKey) {
if (
(after.has(paneKey) && !before.has(paneKey)) ||
now - closedAt >= CLOSED_PANE_BINDING_FENCE_MS
) {
set(paneKey, undefined)
}
}
if (fences) {
for (const paneKey of before) {
if (!after.has(paneKey)) {
set(paneKey, now)
}
}
}
if (changed.size === 0 || !result.rollback) {
return result
}
const rollback = result.rollback
return {
...result,
rollback: () => {
rollback()
for (const [paneKey, closedAt] of changed) {
if (closedAt === undefined) {
closedAtByPaneKey.delete(paneKey)
} else {
closedAtByPaneKey.set(paneKey, closedAt)
}
}
}
}
}
/** Whether main closed or took back this pane within the binding window. */
export function isClosedPaneFenced(paneKey: string): boolean {
const closedAt = closedAtByPaneKey.get(paneKey)
if (closedAt === undefined) {
return false
}
if (Date.now() - closedAt >= CLOSED_PANE_BINDING_FENCE_MS) {
closedAtByPaneKey.delete(paneKey)
return false
}
return true
}
export function _resetClosedPaneFencesForTests(): void {
closedAtByPaneKey.clear()
}
@@ -34,6 +34,11 @@ import { writeRestorable } from './restorable-write'
import { assignWorkspaceSessionPartition } from './terminal-topology-membership'
import { keepWslMoveFrozenSessionsForStore } from '../moving-wsl-projects/wsl-move-session-freeze'
import { resolveWindowPaneDetach, type WindowPaneDetachCommand } from './window-pane-detach'
import {
fencedIfLayoutOwner,
layoutCommandFence,
partitionsFence
} from './closed-pane-binding-fence'
// The commit boundary for terminal layout (tabs, panes, pane-to-PTY bindings). Wraps the close and
// the pane move; the close transform still lives in runtime/ and other writers move here later.
@@ -52,7 +57,11 @@ export function closeLeafOrTab(
): () => DurableProfileStateMutation<Error | undefined> {
return traced(
commit.target.kind === 'pane' ? 'close_leaf' : 'close_tab',
terminalSurfaceCloseMutation(commit),
// A start under way in a pane main closed must not bring it back.
fencedIfLayoutOwner(
() => ({ sessions: commit, fences: true }),
terminalSurfaceCloseMutation(commit)
),
// Refusals are fixed reason codes, never ids.
(refusal) => refusal?.message
)
@@ -81,12 +90,16 @@ export function admitPane(
): () => DurableProfileStateMutation<TerminalPaneAdmissionOutcome> {
return traced(
'admit_pane',
() =>
commitPaneChange(
hostId,
admitPaneToSession(hostId, context.getSession(hostId), admission),
context
),
// A pane main writes again under a closed pane's id binds as any.
fencedIfLayoutOwner(
() => partitionsFence([hostId], context.getSession, false),
() =>
commitPaneChange(
hostId,
admitPaneToSession(hostId, context.getSession(hostId), admission),
context
)
),
(outcome) => (outcome === 'admitted' || outcome === 'exists' ? undefined : outcome)
)
}
@@ -99,12 +112,16 @@ export function withdrawPane(
): () => DurableProfileStateMutation<LayoutRefusalCode | null> {
return traced(
'withdraw_pane',
() =>
commitPaneChange(
hostId,
withdrawPaneFromSession(hostId, context.getSession(hostId), pane),
context
),
// A start under way in a pane main took back must not bring it back.
fencedIfLayoutOwner(
() => partitionsFence([hostId], context.getSession, true),
() =>
commitPaneChange(
hostId,
withdrawPaneFromSession(hostId, context.getSession(hostId), pane),
context
)
),
(code) => code ?? undefined
)
}
@@ -120,14 +137,18 @@ export function applyLayout(
): () => DurableProfileStateMutation<LayoutApplyOutcome> {
return traced(
'apply_layout',
() =>
command.type === 'movePaneToNewTab' && command.windowDetach
? commitWindowPaneDetach(
hostIds,
{ ...command, windowDetach: command.windowDetach },
context
)
: commitLayoutCommand(hostIds, command, context),
// Closes fence the panes they removed; creates lift the fence on the ids they write.
fencedIfLayoutOwner(
() => layoutCommandFence(command.type, hostIds, context.getSession),
() =>
command.type === 'movePaneToNewTab' && command.windowDetach
? commitWindowPaneDetach(
hostIds,
{ ...command, windowDetach: command.windowDetach },
context
)
: commitLayoutCommand(hostIds, command, context)
),
(outcome) => (outcome.ok ? undefined : outcome.code)
)
}