mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
A CLI split that times out leaves no pane (dark, layout-owner build only) (#30563)
* fix(layout): a pane main closed stays closed when its terminal starts late (dark, layout-owner build only) * fix(layout): main takes back a CLI split pane whose terminal does not start within 10 s (dark, layout-owner build only) * fix(layout): the timeout take-back keeps a pane whose terminal started or is starting (dark, layout-owner build only) * fix(layout): one take-back call keeps the layout-writer count * fix(layout): a split take-back is decided in the write queue, and a spawn for a taken-back pane starts nothing (dark, layout-owner build only) * test(layout): a spawn for a taken-back pane is refused before the provider spawns * test: typed store fake in the closed-pane spawn test, no casts
This commit is contained in:
@@ -0,0 +1,113 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { getDefaultWorkspaceSession } from '../../shared/constants'
|
||||
import { LOCAL_EXECUTION_HOST_ID } from '../../shared/execution-host'
|
||||
import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types'
|
||||
import {
|
||||
_resetClosedPaneFencesForTests,
|
||||
fencedIfLayoutOwner,
|
||||
partitionsFence
|
||||
} from '../persistence/terminal-topology/closed-pane-binding-fence'
|
||||
import { Store } from '../persistence'
|
||||
import { spawnMock } from './pty-ipc-mock-registry'
|
||||
import { setupPtyIpcSuite } from './pty-ipc-test-harness'
|
||||
import { registerPtyHandlers } from './pty/register-handlers'
|
||||
|
||||
vi.mock('electron', () => import('./pty-ipc-mock-registry').then((m) => m.electronModuleMock()))
|
||||
vi.mock('fs', () => import('./pty-ipc-mock-registry').then((m) => m.fsModuleMock()))
|
||||
vi.mock('node-pty', () => import('./pty-ipc-mock-registry').then((m) => m.nodePtyModuleMock()))
|
||||
vi.mock('node:child_process', async (importOriginal) =>
|
||||
(await import('./pty-ipc-mock-registry')).childProcessModuleMock(await importOriginal())
|
||||
)
|
||||
vi.mock('../opencode/hook-service', () =>
|
||||
import('./pty-ipc-mock-registry').then((m) => m.openCodeHookServiceModuleMock())
|
||||
)
|
||||
vi.mock('../mimo/hook-service', () =>
|
||||
import('./pty-ipc-mock-registry').then((m) => m.mimoHookServiceModuleMock())
|
||||
)
|
||||
vi.mock('../agent-hooks/server', () =>
|
||||
import('./pty-ipc-mock-registry').then((m) => m.agentHookServerModuleMock())
|
||||
)
|
||||
vi.mock('../pi/titlebar-extension-service', () =>
|
||||
import('./pty-ipc-mock-registry').then((m) => m.piTitlebarExtensionModuleMock())
|
||||
)
|
||||
vi.mock('../pwsh', () => import('./pty-ipc-mock-registry').then((m) => m.pwshModuleMock()))
|
||||
vi.mock('../wsl', async (importOriginal) =>
|
||||
(await import('./pty-ipc-mock-registry')).wslModuleMock(await importOriginal())
|
||||
)
|
||||
vi.mock('../telemetry/client', () =>
|
||||
import('./pty-ipc-mock-registry').then((m) => m.telemetryClientModuleMock())
|
||||
)
|
||||
vi.mock('../telemetry/classify-error', () =>
|
||||
import('./pty-ipc-mock-registry').then((m) => m.classifyErrorModuleMock())
|
||||
)
|
||||
vi.mock('../cli/linux-terminal-orca-cli-shim', () =>
|
||||
import('./pty-ipc-mock-registry').then((m) => m.linuxCliShimModuleMock())
|
||||
)
|
||||
vi.mock('../memory/pty-registry', () =>
|
||||
import('./pty-ipc-mock-registry').then((m) => m.ptyRegistryModuleMock())
|
||||
)
|
||||
vi.mock('../agent-hooks/migration-unsupported-pty-state', () =>
|
||||
import('./pty-ipc-mock-registry').then((m) => m.migrationUnsupportedPtyModuleMock())
|
||||
)
|
||||
vi.mock('../codex/codex-pane-account-registry', () =>
|
||||
import('./pty-ipc-mock-registry').then((m) => m.codexPaneAccountRegistryModuleMock())
|
||||
)
|
||||
vi.mock('../codex/codex-state-db-backfill-recovery', () =>
|
||||
import('./pty-ipc-mock-registry').then((m) => m.codexBackfillRecoveryModuleMock())
|
||||
)
|
||||
vi.mock('../../shared/layout-owner-switch', () => ({ ORCA_LAYOUT_OWNER_IS_MAIN: true }))
|
||||
|
||||
const TAB = 'tab-1'
|
||||
const KEPT = '11111111-1111-4111-8111-111111111111'
|
||||
const TAKEN = '22222222-2222-4222-8222-222222222222'
|
||||
|
||||
function sessionWith(leafIds: string[]): WorkspaceSessionState {
|
||||
const [first, second] = leafIds
|
||||
return {
|
||||
...getDefaultWorkspaceSession(),
|
||||
terminalLayoutsByTabId: {
|
||||
[TAB]: {
|
||||
root: second
|
||||
? {
|
||||
type: 'split',
|
||||
direction: 'vertical',
|
||||
first: { type: 'leaf', leafId: first! },
|
||||
second: { type: 'leaf', leafId: second }
|
||||
}
|
||||
: { type: 'leaf', leafId: first! },
|
||||
activeLeafId: first!,
|
||||
expandedLeafId: null
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
describe('a window spawn for a pane main took back while it waited', () => {
|
||||
const { handlers, mainWindow } = setupPtyIpcSuite()
|
||||
afterEach(() => _resetClosedPaneFencesForTests())
|
||||
|
||||
it('is refused before the provider spawns anything', async () => {
|
||||
const current = { value: sessionWith([KEPT, TAKEN]) }
|
||||
fencedIfLayoutOwner(
|
||||
() => partitionsFence(['local'], () => current.value, true),
|
||||
() => ((current.value = sessionWith([KEPT])), { value: null })
|
||||
)()
|
||||
const store: Store = Object.create(Store.prototype)
|
||||
vi.spyOn(store, 'getWorkspaceSessionHostIds').mockReturnValue([LOCAL_EXECUTION_HOST_ID])
|
||||
vi.spyOn(store, 'getWorkspaceSession').mockImplementation(() => current.value)
|
||||
handlers.clear()
|
||||
registerPtyHandlers(mainWindow, undefined, undefined, undefined, undefined, store)
|
||||
spawnMock.mockClear()
|
||||
|
||||
await expect(
|
||||
handlers.get('pty:spawn')!(null, {
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
command: 'touch ran',
|
||||
tabId: TAB,
|
||||
leafId: TAKEN
|
||||
})
|
||||
).rejects.toThrow('terminal_pane_closed')
|
||||
expect(spawnMock).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,69 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { getDefaultWorkspaceSession } from '../../../../shared/constants'
|
||||
import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types'
|
||||
import type { Store } from '../../../persistence'
|
||||
import {
|
||||
_resetClosedPaneFencesForTests,
|
||||
fencedIfLayoutOwner,
|
||||
partitionsFence
|
||||
} from '../../../persistence/terminal-topology/closed-pane-binding-fence'
|
||||
import { refuseClosedPaneSpawn } from './closed-pane-spawn-refusal'
|
||||
|
||||
vi.mock('../../../../shared/layout-owner-switch', () => ({ ORCA_LAYOUT_OWNER_IS_MAIN: true }))
|
||||
|
||||
function session(leafIds: string[]): WorkspaceSessionState {
|
||||
const [first, second] = leafIds
|
||||
return {
|
||||
...getDefaultWorkspaceSession(),
|
||||
terminalLayoutsByTabId: {
|
||||
t: {
|
||||
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 storeHolding(current: { value: WorkspaceSessionState }): Store {
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: refuseClosedPaneSpawn reads only these two Store members.
|
||||
return {
|
||||
getWorkspaceSessionHostIds: () => ['local'],
|
||||
getWorkspaceSession: () => current.value
|
||||
} as unknown as Store
|
||||
}
|
||||
|
||||
describe('a spawn for a pane main closed while it waited', () => {
|
||||
afterEach(() => _resetClosedPaneFencesForTests())
|
||||
|
||||
it('starts nothing once main took the pane back', () => {
|
||||
const current = { value: session(['a', 'b']) }
|
||||
fencedIfLayoutOwner(
|
||||
() => partitionsFence(['local'], () => current.value, true),
|
||||
() => ((current.value = session(['a'])), { value: null })
|
||||
)()
|
||||
|
||||
expect(() => refuseClosedPaneSpawn(storeHolding(current), 't', 'b')).toThrow(
|
||||
'terminal_pane_closed'
|
||||
)
|
||||
})
|
||||
|
||||
it('starts a pane main never closed, or one it holds again', () => {
|
||||
const current = { value: session(['a', 'b']) }
|
||||
expect(() => refuseClosedPaneSpawn(storeHolding(current), 't', 'b')).not.toThrow()
|
||||
|
||||
fencedIfLayoutOwner(
|
||||
() => partitionsFence(['local'], () => current.value, true),
|
||||
() => ((current.value = session(['a'])), { value: null })
|
||||
)()
|
||||
current.value = session(['a', 'b'])
|
||||
expect(() => refuseClosedPaneSpawn(storeHolding(current), 't', 'b')).not.toThrow()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,28 @@
|
||||
import { isClosedPaneFenced } from '../../../persistence/terminal-topology/closed-pane-binding-fence'
|
||||
import type { Store } from '../../../persistence'
|
||||
import { layoutContainsLeafId } from '../../../../shared/workspace-layout/terminal-pane-tree'
|
||||
|
||||
/**
|
||||
* Layout-owner build: a spawn for a pane main closed or took back, which main's layout no longer
|
||||
* holds, starts nothing. Its binding would be refused anyway (pty-binding-refusals.ts); refusing
|
||||
* first means its startup command never runs.
|
||||
*/
|
||||
export function refuseClosedPaneSpawn(
|
||||
store: Store | undefined,
|
||||
tabId: string | undefined,
|
||||
leafId: string | null | undefined
|
||||
): void {
|
||||
// The fence's own key: legacy non-UUID leaf ids are matched, never thrown on.
|
||||
if (!tabId || !leafId || !isClosedPaneFenced(`${tabId}:${leafId}`)) {
|
||||
return
|
||||
}
|
||||
const held = (store?.getWorkspaceSessionHostIds() ?? []).some((hostId) =>
|
||||
layoutContainsLeafId(
|
||||
store?.getWorkspaceSession(hostId)?.terminalLayoutsByTabId?.[tabId]?.root ?? null,
|
||||
leafId
|
||||
)
|
||||
)
|
||||
if (!held) {
|
||||
throw new Error('terminal_pane_closed')
|
||||
}
|
||||
}
|
||||
@@ -17,6 +17,8 @@ import { commitPtyIpcSpawn } from './spawn-commit'
|
||||
import { createPtyIpcSpawnState, type PtyIpcSpawnState } from './spawn-state'
|
||||
import { triggerPtySpawnPushTargetMaterialization } from './spawn-push-target-materialization'
|
||||
import type { PtySpawnIpcArgs, PtySpawnIpcDeps } from './spawn-types'
|
||||
import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../shared/layout-owner-switch'
|
||||
import { refuseClosedPaneSpawn } from './closed-pane-spawn-refusal'
|
||||
|
||||
function releaseAbandonedAgentTeamsLeader(ctx: PtyIpcSpawnState): void {
|
||||
if (!ctx.agentTeamsLeaderHandle) {
|
||||
@@ -76,6 +78,10 @@ export async function runPtyIpcSpawn(deps: PtySpawnIpcDeps, args: PtySpawnIpcArg
|
||||
}
|
||||
// Right before the provider spawn, with nothing awaited in between: a closed pane starts nothing.
|
||||
ctx.paneAgentLaunch?.refuseIfClosed()
|
||||
// Switch point: a pane main closed or took back while this spawn waited starts nothing.
|
||||
if (ORCA_LAYOUT_OWNER_IS_MAIN) {
|
||||
refuseClosedPaneSpawn(deps.store, args.tabId, args.leafId)
|
||||
}
|
||||
await executePtyIpcSpawn(ctx)
|
||||
const committed = await commitPtyIpcSpawn(ctx)
|
||||
endPaneAgentLaunch(
|
||||
|
||||
@@ -82,6 +82,17 @@ export function clearPaneSpawnReservation(
|
||||
}
|
||||
}
|
||||
|
||||
/** Whether a window or runtime spawn of this pane has begun and not yet bound or failed. */
|
||||
export function isPaneSpawnInFlight(worktreeId: string, paneKey: string): boolean {
|
||||
for (const ownerKey of paneSpawnReservationsByOwnerKey.keys()) {
|
||||
const parsed: unknown = JSON.parse(ownerKey)
|
||||
if (Array.isArray(parsed) && parsed[1] === worktreeId && parsed[2] === paneKey) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
export function makePaneSpawnReservationKey(
|
||||
worktreeId: string | undefined,
|
||||
connectionId: string | null | undefined,
|
||||
|
||||
@@ -11,6 +11,10 @@ import type { PtyBindingSourceExpectation } from './store'
|
||||
|
||||
import type { StoreRuntimeState } from './store-runtime-state'
|
||||
import type { SessionHostPartitionOperations } from './session-host-partitions'
|
||||
import {
|
||||
withdrawUnstartedPane,
|
||||
type UnstartedPaneKept
|
||||
} from '../terminal-topology/unstarted-pane-withdraw'
|
||||
import { resolveHostId } from './session-host-partitions'
|
||||
import { evaluatePtyBindingFastLane } from './pty-binding-fast-lane'
|
||||
import { ptyBindingIsRefused } from './pty-binding-refusals'
|
||||
@@ -244,12 +248,17 @@ export class PtyBindingPersistenceOperations {
|
||||
)
|
||||
}
|
||||
|
||||
/** With `stillStarting`, a pane whose terminal bound or is starting stays (UnstartedPaneKept). */
|
||||
withdrawTerminalPane(
|
||||
pane: CommandOf<'closePane'>,
|
||||
hostId?: string | null
|
||||
): Promise<LayoutRefusalCode | null> {
|
||||
hostId?: string | null,
|
||||
stillStarting?: () => boolean
|
||||
): Promise<LayoutRefusalCode | UnstartedPaneKept | null> {
|
||||
const resolved = resolveHostId(hostId)
|
||||
return this[ptyBindingPersistenceOperationsContext].runtime.runDurableMutation(
|
||||
withdrawPane(resolveHostId(hostId), pane, this.topology())
|
||||
stillStarting
|
||||
? withdrawUnstartedPane(resolved, pane, this.topology(), stillStarting)
|
||||
: withdrawPane(resolved, pane, this.topology())
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
import type { ExecutionHostId } from '../../../shared/execution-host'
|
||||
import type {
|
||||
CommandOf,
|
||||
LayoutRefusalCode
|
||||
} from '../../../shared/workspace-layout/workspace-layout-command-types'
|
||||
import type { DurableProfileStateMutation } from '../loading-store/store-runtime-state'
|
||||
import { withdrawPane, type TerminalTopologyCommitContext } from './terminal-topology-commit'
|
||||
|
||||
/** Settled with bindings in the write queue: what keeps a pane whose terminal never started. */
|
||||
export type UnstartedPaneKept = 'bound' | 'starting'
|
||||
|
||||
/**
|
||||
* `withdrawPane` for a pane with no terminal: one that bound since ('bound') or whose start is under
|
||||
* way ('starting') stays, and nothing is written.
|
||||
*/
|
||||
export function withdrawUnstartedPane(
|
||||
hostId: ExecutionHostId,
|
||||
pane: CommandOf<'closePane'>,
|
||||
context: TerminalTopologyCommitContext,
|
||||
stillStarting: () => boolean
|
||||
): () => DurableProfileStateMutation<LayoutRefusalCode | UnstartedPaneKept | null> {
|
||||
const withdraw = withdrawPane(hostId, pane, context)
|
||||
return () => {
|
||||
const layout = context.getSession(hostId).terminalLayoutsByTabId?.[pane.tabId]
|
||||
if (layout?.ptyIdsByLeafId?.[pane.leafId]) {
|
||||
return { value: 'bound', persist: false }
|
||||
}
|
||||
return stillStarting() ? { value: 'starting', persist: false } : withdraw()
|
||||
}
|
||||
}
|
||||
@@ -7,7 +7,10 @@ import {
|
||||
type PaneLayoutChange,
|
||||
type TerminalPaneAdmission
|
||||
} from '../persistence/terminal-topology/terminal-pane-admission'
|
||||
import type { CommandOf } from '../../shared/workspace-layout/workspace-layout-command-types'
|
||||
import type {
|
||||
CommandOf,
|
||||
LayoutRefusalCode
|
||||
} from '../../shared/workspace-layout/workspace-layout-command-types'
|
||||
import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types'
|
||||
import { clearReplacedPaneBinding } from '../persistence/loading-store/replaced-pane-binding'
|
||||
|
||||
@@ -102,7 +105,17 @@ export function withRuntimePaneAdmission<
|
||||
return Object.assign(store, {
|
||||
admitTerminalPane: async (admission: TerminalPaneAdmission, hostId?: string) =>
|
||||
commit(hostId, (session) => admitPaneToSession(resolveHostId(hostId), session, admission)),
|
||||
withdrawTerminalPane: async (pane: CommandOf<'closePane'>, hostId?: string) =>
|
||||
commit(hostId, (session) => withdrawPaneFromSession(resolveHostId(hostId), session, pane))
|
||||
withdrawTerminalPane: async (
|
||||
pane: CommandOf<'closePane'>,
|
||||
hostId?: string,
|
||||
stillStarting?: () => boolean
|
||||
): Promise<LayoutRefusalCode | 'bound' | 'starting' | null> =>
|
||||
commit(hostId, (session) =>
|
||||
stillStarting && session.terminalLayoutsByTabId?.[pane.tabId]?.ptyIdsByLeafId?.[pane.leafId]
|
||||
? { value: 'bound' as const, session: null }
|
||||
: stillStarting?.()
|
||||
? { value: 'starting' as const, session: null }
|
||||
: withdrawPaneFromSession(resolveHostId(hostId), session, pane)
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -49,6 +49,7 @@ describe('startInAdmittedPane', () => {
|
||||
).rejects.toThrow('mount timed out')
|
||||
expect(store.withdrawTerminalPane).toHaveBeenCalledWith(
|
||||
{ type: 'closePane', workspace: 'wt-1', tabId: 'tab-1', leafId: 'leaf-new' },
|
||||
undefined,
|
||||
undefined
|
||||
)
|
||||
})
|
||||
|
||||
@@ -3,6 +3,7 @@ import type {
|
||||
TerminalPaneAdmission,
|
||||
TerminalPaneAdmissionOutcome
|
||||
} from '../persistence/terminal-topology/terminal-pane-admission'
|
||||
import type { LayoutRefusalCode } from '../../shared/workspace-layout/workspace-layout-command-types'
|
||||
import type { RuntimePtyController } from './runtime-pty-controller-contract'
|
||||
import type { RuntimeStore } from './runtime-store-contract'
|
||||
|
||||
@@ -66,17 +67,24 @@ export async function withdrawRuntimePane(
|
||||
await withdrawAdmittedPane(store, admissionFor(args), hostIdOf(args))
|
||||
}
|
||||
|
||||
/** Never the user's close: main stops no launch for it, unlike a window's closePane. */
|
||||
async function withdrawAdmittedPane(
|
||||
/**
|
||||
* Never the user's close: main stops no launch for it, unlike a window's closePane. With
|
||||
* `stillStarting`, a pane whose terminal bound or is starting stays ('bound' / 'starting').
|
||||
*/
|
||||
export async function withdrawAdmittedPane(
|
||||
store: RuntimeStore | null | undefined,
|
||||
admission: TerminalPaneAdmission,
|
||||
hostId: string | undefined
|
||||
): Promise<void> {
|
||||
hostId: string | undefined,
|
||||
stillStarting?: () => boolean
|
||||
): Promise<LayoutRefusalCode | 'bound' | 'starting' | null | undefined> {
|
||||
const leafId = admission.type === 'createTerminalTab' ? admission.leafId : admission.newLeafId
|
||||
await store?.withdrawTerminalPane?.(
|
||||
{ type: 'closePane', workspace: admission.workspace, tabId: admission.tabId, leafId },
|
||||
hostId
|
||||
)
|
||||
const pane = {
|
||||
type: 'closePane' as const,
|
||||
workspace: admission.workspace,
|
||||
tabId: admission.tabId,
|
||||
leafId
|
||||
}
|
||||
return store?.withdrawTerminalPane?.(pane, hostId, stillStarting)
|
||||
}
|
||||
|
||||
/** Writes a pane the runtime asks for into main's copy; what fills it is the caller's. */
|
||||
|
||||
@@ -8,6 +8,11 @@ import {
|
||||
} from '../../orca-runtime-test-fixtures.spec'
|
||||
import { createMobileCreateTestNotifier } from '../../orca-runtime-test-scenario-builders.spec'
|
||||
import { collectLayoutLeafIdsInOrder } from '../../../../shared/workspace-layout/terminal-pane-tree'
|
||||
import {
|
||||
clearPaneSpawnReservation,
|
||||
makePaneSpawnReservationKey,
|
||||
reservePaneSpawn
|
||||
} from '../../../ipc/pty/pane/spawn-reservation'
|
||||
|
||||
vi.mock('../../../../shared/layout-owner-switch', () => ({ ORCA_LAYOUT_OWNER_IS_MAIN: true }))
|
||||
|
||||
@@ -35,6 +40,36 @@ function graph(leafIds: string[], unboundLeafIds: string[] = []) {
|
||||
}
|
||||
}
|
||||
|
||||
type RuntimeInternals = {
|
||||
recordPtyWorktree: (
|
||||
ptyId: string,
|
||||
worktreeId: string,
|
||||
opts: { connected: boolean; tabId: string; paneKey: string }
|
||||
) => void
|
||||
}
|
||||
|
||||
function internalsOf(runtime: OrcaRuntimeService): RuntimeInternals {
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: recordPtyWorktree is declared on OrcaRuntimeService's inheritance chain.
|
||||
return runtime as unknown as RuntimeInternals
|
||||
}
|
||||
|
||||
function graphWithSplit(newLeafId: string) {
|
||||
const next = graph([HEADLESS_LEAF_ID, newLeafId])
|
||||
return {
|
||||
...next,
|
||||
tabs: next.tabs.map((tab) => ({
|
||||
...tab,
|
||||
layout: {
|
||||
type: 'split' as const,
|
||||
direction: 'vertical' as const,
|
||||
ratio: 0.5,
|
||||
first: { type: 'leaf' as const, leafId: HEADLESS_LEAF_ID },
|
||||
second: { type: 'leaf' as const, leafId: newLeafId }
|
||||
}
|
||||
}))
|
||||
}
|
||||
}
|
||||
|
||||
describe('a split of a pane the window draws, in a layout-owner build', () => {
|
||||
function setup() {
|
||||
const store = makeRuntimeStoreWithWorkspaceSession(makeWorkspaceSessionWithHeadlessTerminal())
|
||||
@@ -51,7 +86,7 @@ describe('a split of a pane the window draws, in a layout-owner build', () => {
|
||||
written.add(listener)
|
||||
return () => written.delete(listener)
|
||||
})
|
||||
const notifier = createMobileCreateTestNotifier(vi.fn())
|
||||
const notifier = { ...createMobileCreateTestNotifier(vi.fn()), closeTerminalPane: vi.fn() }
|
||||
const runtime = new OrcaRuntimeService(store.runtimeStore)
|
||||
runtime.setNotifier(notifier)
|
||||
runtime.syncWindowGraph(1, graph([HEADLESS_LEAF_ID]))
|
||||
@@ -60,6 +95,7 @@ describe('a split of a pane the window draws, in a layout-owner build', () => {
|
||||
collectLayoutLeafIdsInOrder(store.getSession().terminalLayoutsByTabId[TAB_ID]!.root!)
|
||||
return {
|
||||
...store,
|
||||
notifier,
|
||||
runtime,
|
||||
handle: handle!,
|
||||
splitTerminal: vi.mocked(notifier.splitTerminal),
|
||||
@@ -67,30 +103,92 @@ describe('a split of a pane the window draws, in a layout-owner build', () => {
|
||||
}
|
||||
}
|
||||
|
||||
it('writes the new pane in main before the window mounts it, and keeps it however the start goes', async () => {
|
||||
const { runtime, runtimeStore, handle, splitTerminal, leafIds } = setup()
|
||||
const withdraw = vi.fn(async () => null)
|
||||
Reflect.set(runtimeStore, 'withdrawTerminalPane', withdraw)
|
||||
it('writes the new pane in main before the window mounts it, and takes it back when its terminal never starts', async () => {
|
||||
const { runtime, handle, splitTerminal, leafIds, notifier } = setup()
|
||||
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
const split = runtime.splitTerminal(handle, { direction: 'vertical', command: 'make test' })
|
||||
const rejected = expect(split).rejects.toThrow('Timed out waiting for split pane handle')
|
||||
const rejected = expect(split).rejects.toThrow(
|
||||
/^split_pane_not_started: .*did not start within 10 s.*closed/
|
||||
)
|
||||
await vi.waitFor(() => expect(splitTerminal).toHaveBeenCalledTimes(1))
|
||||
const notice = splitTerminal.mock.calls[0]![2]!
|
||||
expect(notice).toMatchObject({ command: 'make test', sourceLeafId: HEADLESS_LEAF_ID })
|
||||
expect(leafIds()).toEqual([HEADLESS_LEAF_ID, notice.newLeafId])
|
||||
|
||||
// Its terminal never starts: main keeps a dead pane, as any, and takes nothing back.
|
||||
// Its terminal never starts: main takes the pane back, and the window drops it too.
|
||||
await vi.advanceTimersByTimeAsync(11_000)
|
||||
await rejected
|
||||
expect(leafIds()).toEqual([HEADLESS_LEAF_ID, notice.newLeafId])
|
||||
expect(withdraw).not.toHaveBeenCalled()
|
||||
expect(leafIds()).toEqual([HEADLESS_LEAF_ID])
|
||||
expect(notifier.closeTerminalPane).toHaveBeenCalledWith(TAB_ID, notice.newLeafId)
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('keeps a pane whose terminal bound before the wait ran out, though the window reported it late', async () => {
|
||||
const { runtime, runtimeStore, handle, splitTerminal, leafIds, notifier } = setup()
|
||||
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
const startedAt = Date.now()
|
||||
const split = runtime.splitTerminal(handle, { direction: 'vertical' })
|
||||
await vi.waitFor(() => expect(splitTerminal).toHaveBeenCalledTimes(1))
|
||||
const newLeafId = splitTerminal.mock.calls[0]![2]!.newLeafId!
|
||||
await vi.advanceTimersByTimeAsync(9_900 - (Date.now() - startedAt))
|
||||
// Main binds the started terminal at 9.9 s; the busy window's report lands after 10 s.
|
||||
await runtimeStore.persistPtyBinding({
|
||||
worktreeId: TEST_WORKTREE_ID,
|
||||
tabId: TAB_ID,
|
||||
leafId: newLeafId,
|
||||
ptyId: 'pty-1'
|
||||
})
|
||||
await vi.advanceTimersByTimeAsync(600)
|
||||
internalsOf(runtime).recordPtyWorktree('pty-1', TEST_WORKTREE_ID, {
|
||||
connected: true,
|
||||
tabId: TAB_ID,
|
||||
paneKey: makePaneKey(TAB_ID, newLeafId)
|
||||
})
|
||||
runtime.syncWindowGraph(1, graphWithSplit(newLeafId))
|
||||
await vi.advanceTimersByTimeAsync(0)
|
||||
await expect(split).resolves.toMatchObject({ tabId: TAB_ID, leafId: newLeafId })
|
||||
expect(leafIds()).toEqual([HEADLESS_LEAF_ID, newLeafId])
|
||||
expect(notifier.closeTerminalPane).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('keeps the pane of a start still under way when the wait runs out, with the plain timeout', async () => {
|
||||
const { runtime, handle, splitTerminal, leafIds, notifier } = setup()
|
||||
const reservations: (() => void)[] = []
|
||||
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
const split = runtime.splitTerminal(handle, { direction: 'vertical' })
|
||||
const rejected = expect(split).rejects.toThrow(/^Timed out waiting for split pane handle$/)
|
||||
await vi.waitFor(() => expect(splitTerminal).toHaveBeenCalledTimes(1))
|
||||
const newLeafId = splitTerminal.mock.calls[0]![2]!.newLeafId!
|
||||
// A slow (SSH-style) start of the pane's terminal is under way and has not bound yet.
|
||||
const reservationKey = makePaneSpawnReservationKey(
|
||||
TEST_WORKTREE_ID,
|
||||
'ssh-1',
|
||||
makePaneKey(TAB_ID, newLeafId)
|
||||
)!
|
||||
const reservation = reservePaneSpawn(reservationKey)
|
||||
reservations.push(() => clearPaneSpawnReservation(reservationKey, reservation))
|
||||
|
||||
await vi.advanceTimersByTimeAsync(11_000)
|
||||
await rejected
|
||||
expect(leafIds()).toEqual([HEADLESS_LEAF_ID, newLeafId])
|
||||
expect(notifier.closeTerminalPane).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
reservations.forEach((release) => release())
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('closes a CLI split before its terminal starts, and the CLI hears so at once', async () => {
|
||||
const { runtime, handle, splitTerminal, leafIds } = setup()
|
||||
|
||||
@@ -147,7 +245,7 @@ describe('a split of a pane the window draws, in a layout-owner build', () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
const split = runtime.splitTerminal(handle, { direction: 'vertical' })
|
||||
const rejected = expect(split).rejects.toThrow('Timed out waiting for split pane handle')
|
||||
const rejected = expect(split).rejects.toThrow('split_pane_not_started')
|
||||
await vi.waitFor(() => expect(splitTerminal).toHaveBeenCalledTimes(1))
|
||||
// An unrelated write while the pane waits for its terminal.
|
||||
runtimeStore.setWorkspaceSession(runtimeStore.getWorkspaceSession())
|
||||
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
} from '../../claude-agent-teams-shim-env'
|
||||
import { applyClaudeEnvPatch } from '../../../claude-accounts/environment'
|
||||
import { worktreeIdsEqual } from '../../../../shared/worktree/id'
|
||||
import { SPLIT_PANE_WAIT_TIMEOUT_MESSAGE } from './terminal-split-wait-timeout'
|
||||
|
||||
export abstract class OrcaRuntimeWithResolveTerminalSplitSourceAuthority extends OrcaRuntimeWithSplitPtyBackedTerminal {
|
||||
abstract getTerminalHandleForPaneKey(
|
||||
@@ -185,7 +186,7 @@ export abstract class OrcaRuntimeWithResolveTerminalSplitSourceAuthority extends
|
||||
if (idx !== -1) {
|
||||
this.graphSyncCallbacks.splice(idx, 1)
|
||||
}
|
||||
reject(new Error('Timed out waiting for split pane handle'))
|
||||
reject(new Error(SPLIT_PANE_WAIT_TIMEOUT_MESSAGE))
|
||||
}, timeoutMs)
|
||||
|
||||
const check = (): void => {
|
||||
|
||||
@@ -0,0 +1,2 @@
|
||||
/** waitForLeafInTab's timeout; layout-owner splits tell it apart from their own failures. */
|
||||
export const SPLIT_PANE_WAIT_TIMEOUT_MESSAGE = 'Timed out waiting for split pane handle'
|
||||
@@ -6,10 +6,15 @@ import type { RuntimeTerminalSplit } from '../../../../shared/runtime-types'
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { parseExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host'
|
||||
import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../shared/layout-owner-switch'
|
||||
import { admitRuntimePane } from '../../runtime-pane-admission'
|
||||
import { admitRuntimePane, withdrawAdmittedPane } from '../../runtime-pane-admission'
|
||||
import type { RuntimeStore } from '../../runtime-store-contract'
|
||||
import type { Worktree } from '../../../../shared/worktree/types'
|
||||
import { collectLeafIds } from '../../../../shared/terminal-pane-layout-tree'
|
||||
import { makePaneKey } from '../../../../shared/stable-pane-id'
|
||||
import { isPaneSpawnInFlight } from '../../../ipc/pty/pane/spawn-reservation'
|
||||
import { SPLIT_PANE_WAIT_TIMEOUT_MESSAGE } from './terminal-split-wait-timeout'
|
||||
|
||||
const SPLIT_PANE_START_TIMEOUT_MS = 10_000
|
||||
|
||||
export abstract class OrcaRuntimeWithSplitTerminal extends OrcaRuntimeWithStopExplicitlyClosedTabPtys {
|
||||
protected abstract splitPtyBackedTerminal(
|
||||
@@ -52,8 +57,8 @@ export abstract class OrcaRuntimeWithSplitTerminal extends OrcaRuntimeWithStopEx
|
||||
const [hostId] = this.getWorkspaceSessionHostIdsForTab(leaf.worktreeId, leaf.tabId)
|
||||
if (hostId && parseExecutionHostId(hostId)?.kind !== 'runtime') {
|
||||
const newLeafId = randomUUID()
|
||||
// Main keeps the pane however its start goes: one whose terminal never starts is a dead
|
||||
// pane, as any. No pane written, no split: nothing else would put it in main's copy.
|
||||
// No pane written, no split: nothing else would put it in main's copy. A pane whose
|
||||
// terminal never starts is taken back below, so the CLI's failure is the truth.
|
||||
const outcome = await admitRuntimePane(
|
||||
this.store,
|
||||
{
|
||||
@@ -82,8 +87,50 @@ export abstract class OrcaRuntimeWithSplitTerminal extends OrcaRuntimeWithStopEx
|
||||
() => this.getWorkspaceSessionHostIdsForTab(leaf.worktreeId, leaf.tabId),
|
||||
leaf.tabId,
|
||||
newLeafId,
|
||||
this.waitForLeafInTab(leaf.tabId, newLeafId)
|
||||
)
|
||||
this.waitForLeafInTab(leaf.tabId, newLeafId, SPLIT_PANE_START_TIMEOUT_MS)
|
||||
).catch(async (error: unknown) => {
|
||||
if (
|
||||
outcome !== 'admitted' ||
|
||||
!(error instanceof Error) ||
|
||||
error.message !== SPLIT_PANE_WAIT_TIMEOUT_MESSAGE
|
||||
) {
|
||||
throw error
|
||||
}
|
||||
const taken = await withdrawAdmittedPane(
|
||||
this.store,
|
||||
{
|
||||
type: 'splitPane',
|
||||
workspace: leaf.worktreeId,
|
||||
tabId: leaf.tabId,
|
||||
leafId: leaf.leafId,
|
||||
direction,
|
||||
newLeafId
|
||||
},
|
||||
hostId,
|
||||
// Read in the write queue: a start under way (a slow SSH host) keeps its pane.
|
||||
() => isPaneSpawnInFlight(leaf.worktreeId, makePaneKey(leaf.tabId, newLeafId))
|
||||
).catch((withdrawError: unknown) => {
|
||||
console.warn('[layout] an unstarted split pane stayed in main:', withdrawError)
|
||||
return 'threw' as const
|
||||
})
|
||||
if (taken === 'bound') {
|
||||
// Bound just before the take-back: the window's late report brings its handle.
|
||||
return settledByPaneClose(
|
||||
this.store,
|
||||
() => this.getWorkspaceSessionHostIdsForTab(leaf.worktreeId, leaf.tabId),
|
||||
leaf.tabId,
|
||||
newLeafId,
|
||||
this.waitForLeafInTab(leaf.tabId, newLeafId, SPLIT_PANE_START_TIMEOUT_MS)
|
||||
)
|
||||
}
|
||||
if (taken !== null) {
|
||||
throw error
|
||||
}
|
||||
this.notifier?.closeTerminalPane?.(leaf.tabId, newLeafId)
|
||||
throw new Error(
|
||||
`split_pane_not_started: the new pane's terminal did not start within ${SPLIT_PANE_START_TIMEOUT_MS / 1000} s, so the pane was closed${opts.command ? ' and its command was not run' : ''}`
|
||||
)
|
||||
})
|
||||
return {
|
||||
handle: newHandle,
|
||||
tabId: leaf.tabId,
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { execFile } from 'node:child_process'
|
||||
import { existsSync } from 'node:fs'
|
||||
import { existsSync, mkdirSync } from 'node:fs'
|
||||
import path from 'node:path'
|
||||
import { promisify } from 'node:util'
|
||||
import type { Page } from '@stablyai/playwright-test'
|
||||
@@ -138,3 +138,69 @@ test('a CLI split whose tab closes before it mounts answers promptly', async ({
|
||||
expect(outcome.ok ? '' : outcome.error).toContain('split_pane_closed')
|
||||
expect(elapsedMs).toBeLessThan(6_000)
|
||||
})
|
||||
|
||||
// The window is too busy to start the split's terminal within the CLI's 10 s: main takes the
|
||||
// pane back, so the CLI's failure is the truth and the startup command never runs.
|
||||
test('a CLI split whose terminal does not start within 10 s leaves no pane', async ({
|
||||
electronApp,
|
||||
orcaPage
|
||||
}, testInfo) => {
|
||||
test.skip(!LAYOUT_OWNER_BUILD, 'layout-owner build only')
|
||||
test.setTimeout(180_000)
|
||||
await waitForSessionReady(orcaPage)
|
||||
const worktreeId = await waitForActiveWorktree(orcaPage)
|
||||
await ensureTerminalVisible(orcaPage)
|
||||
await waitForActiveTerminalManager(orcaPage, 30_000)
|
||||
const userDataDir = await electronApp.evaluate(({ app }) => app.getPath('userData'))
|
||||
const client = new RuntimeClient(userDataDir, 30_000)
|
||||
const tabId = await createActiveTab(orcaPage, worktreeId)
|
||||
const source = await handleFor(client, worktreeId, tabId)
|
||||
await parkHiddenTabBehindDecoy(orcaPage, worktreeId, tabId, { parkDelayMs: PARKING_DELAY_MS })
|
||||
await orcaPage.evaluate(() => {
|
||||
window.addEventListener(
|
||||
'orca-background-mount-terminal-worktree',
|
||||
() => {
|
||||
const until = performance.now() + 11_000
|
||||
while (performance.now() < until) {
|
||||
// Busy renderer.
|
||||
}
|
||||
},
|
||||
{ once: true, capture: true }
|
||||
)
|
||||
})
|
||||
// A file the command writes: a terminal that ran it and was then killed still leaves it.
|
||||
const marker = testInfo.outputPath(`split-ran-${Date.now()}`)
|
||||
mkdirSync(path.dirname(marker), { recursive: true })
|
||||
const outcome = await runCliSplit(userDataDir, source, `touch '${marker}'`)
|
||||
// Past the stall: the window has handled the split and main's take-back.
|
||||
await new Promise((resolve) => setTimeout(resolve, 5_000))
|
||||
const tabTerminals = await client
|
||||
.call<RuntimeTerminalListResult>('terminal.list', { worktree: `id:${worktreeId}`, limit: 200 })
|
||||
.then((listed) => listed.result.terminals.filter((t) => t.tabId === tabId))
|
||||
const windowLeafIds = await orcaPage.evaluate((id) => {
|
||||
const collect = (node: unknown): string[] => {
|
||||
if (typeof node !== 'object' || node === null) {
|
||||
return []
|
||||
}
|
||||
if ('leafId' in node && typeof node.leafId === 'string') {
|
||||
return [node.leafId]
|
||||
}
|
||||
return [
|
||||
...('first' in node ? collect(node.first) : []),
|
||||
...('second' in node ? collect(node.second) : [])
|
||||
]
|
||||
}
|
||||
return collect(window.__store?.getState().terminalLayoutsByTabId[id]?.root)
|
||||
}, tabId)
|
||||
const report = { outcome, tabTerminals: tabTerminals.map((t) => t.leafId), windowLeafIds }
|
||||
await testInfo.attach('split-taken-back.json', {
|
||||
body: JSON.stringify(report, null, 2),
|
||||
contentType: 'application/json'
|
||||
})
|
||||
expect(outcome.ok ? '' : outcome.error).toContain(
|
||||
"split_pane_not_started: the new pane's terminal did not start within 10 s, so the pane was closed and its command was not run"
|
||||
)
|
||||
expect(tabTerminals).toHaveLength(1)
|
||||
expect(windowLeafIds).toEqual([tabTerminals[0]!.leafId])
|
||||
expect(existsSync(marker)).toBe(false)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user