mirror of
https://github.com/stablyai/orca.git
synced 2026-10-16 16:03:26 +00:00
Prevent active terminal focus on dead panes and ignore stale PTY events
- Redirect active keyboard focus to a live, PTY-backed sibling pane if the focused pane dies, fails to spawn, or is hydrated without a live connection. - Ignore stale data, replay messages, and subscription rejection errors from older PTY sessions after a transport has reconnected. - Avoid persisting stale PTY bindings and dead pane focus inside terminal shutdown layout snapshots.
This commit is contained in:
@@ -349,19 +349,29 @@ function createPane(paneId: number) {
|
||||
}
|
||||
}
|
||||
|
||||
function createManager(paneCount = 1) {
|
||||
function createManager(paneCount = 1, initialActivePaneId: number | null = null) {
|
||||
let activePaneId = initialActivePaneId
|
||||
const panes = Array.from({ length: paneCount }, (_, index) => ({
|
||||
id: index + 1,
|
||||
leafId: leafIdForPane(index + 1)
|
||||
}))
|
||||
return {
|
||||
setPaneGpuRendering: vi.fn(),
|
||||
markPaneHasComplexScriptOutput: vi.fn(),
|
||||
rebuildPaneWebgl: vi.fn(),
|
||||
getPanes: vi.fn(() =>
|
||||
Array.from({ length: paneCount }, (_, index) => ({
|
||||
id: index + 1,
|
||||
leafId: leafIdForPane(index + 1)
|
||||
}))
|
||||
),
|
||||
getPanes: vi.fn(() => panes),
|
||||
closePane: vi.fn(),
|
||||
getActivePane: vi.fn<() => { id: number } | null>(() => null)
|
||||
getActivePane: vi.fn<() => { id: number; leafId?: string } | null>(() =>
|
||||
activePaneId === null
|
||||
? null
|
||||
: (panes.find((candidate) => candidate.id === activePaneId) ?? null)
|
||||
),
|
||||
getNumericIdForLeaf: vi.fn((leafId: string) => {
|
||||
return panes.find((candidate) => candidate.leafId === leafId)?.id ?? null
|
||||
}),
|
||||
setActivePane: vi.fn((paneId: number) => {
|
||||
activePaneId = paneId
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -851,10 +861,30 @@ describe('connectPanePty', () => {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
const transport = createMockTransport('pty-pane-2')
|
||||
transportFactoryQueue.push(transport)
|
||||
const manager = createManager(2)
|
||||
const manager = createManager(2, 2)
|
||||
const deps = createDeps({
|
||||
restoredLeafId: LEAF_2,
|
||||
paneTransportsRef: { current: new Map([[1, createMockTransport('pty-pane-1')]]) }
|
||||
paneTransportsRef: { current: new Map([[1, createMockTransport('pty-pane-1')]]) },
|
||||
clearExitedPanePtyLayoutBinding: vi.fn(() => {
|
||||
mockStoreState = {
|
||||
...mockStoreState,
|
||||
terminalLayoutsByTabId: {
|
||||
...mockStoreState.terminalLayoutsByTabId,
|
||||
'tab-1': {
|
||||
root: {
|
||||
type: 'split',
|
||||
direction: 'horizontal',
|
||||
first: { type: 'leaf', leafId: LEAF_1 },
|
||||
second: { type: 'leaf', leafId: LEAF_2 },
|
||||
ratio: 0.5
|
||||
},
|
||||
activeLeafId: LEAF_1,
|
||||
expandedLeafId: null,
|
||||
ptyIdsByLeafId: { [LEAF_1]: 'pty-pane-1' }
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
connectPanePty(createPane(2) as never, manager as never, deps as never)
|
||||
@@ -867,6 +897,7 @@ describe('connectPanePty', () => {
|
||||
expect(deps.clearTabPtyId).toHaveBeenCalledWith('tab-1', 'pty-pane-2')
|
||||
expect(deps.onPtyExitRef.current).not.toHaveBeenCalled()
|
||||
expect(manager.closePane).not.toHaveBeenCalled()
|
||||
expect(manager.setActivePane).toHaveBeenCalledWith(1, { focus: true })
|
||||
})
|
||||
|
||||
it('closes a split pane when an established PTY exits after output', async () => {
|
||||
|
||||
@@ -1293,6 +1293,35 @@ export function connectPanePty(
|
||||
}
|
||||
})
|
||||
|
||||
const focusSurvivingPtyPaneAfterKeptExit = (): void => {
|
||||
if (manager.getActivePane()?.id !== pane.id) {
|
||||
return
|
||||
}
|
||||
const hasPtyBinding = (paneId: number): boolean =>
|
||||
Boolean(deps.paneTransportsRef.current.get(paneId)?.getPtyId())
|
||||
const repairedActiveLeafId =
|
||||
useAppStore.getState().terminalLayoutsByTabId[deps.tabId]?.activeLeafId ?? null
|
||||
const repairedActivePaneId = repairedActiveLeafId
|
||||
? manager.getNumericIdForLeaf(repairedActiveLeafId)
|
||||
: null
|
||||
const targetPaneId =
|
||||
repairedActivePaneId !== null &&
|
||||
repairedActivePaneId !== pane.id &&
|
||||
hasPtyBinding(repairedActivePaneId)
|
||||
? repairedActivePaneId
|
||||
: (manager
|
||||
.getPanes()
|
||||
.find((candidate) => candidate.id !== pane.id && hasPtyBinding(candidate.id))?.id ??
|
||||
null)
|
||||
if (targetPaneId !== null) {
|
||||
// Why: when a newborn split PTY dies before output/input, the pane stays
|
||||
// mounted for diagnostics; move live focus to the sibling that still owns a PTY.
|
||||
manager.setActivePane(targetPaneId, {
|
||||
focus: deps.isActiveRef.current && deps.isVisibleRef.current
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
const onExit = (ptyId: string): void => {
|
||||
const currentPaneTransport = deps.paneTransportsRef.current.get(pane.id)
|
||||
if (currentPaneTransport && currentPaneTransport !== transport) {
|
||||
@@ -1346,6 +1375,7 @@ export function connectPanePty(
|
||||
) {
|
||||
// Why: a freshly split pane can lose its newborn PTY during setup; keep
|
||||
// the split visible so the failed session does not immediately collapse.
|
||||
focusSurvivingPtyPaneAfterKeptExit()
|
||||
return
|
||||
}
|
||||
manager.closePane(pane.id)
|
||||
|
||||
@@ -343,9 +343,7 @@ describe('createRemoteRuntimePtyTransport', () => {
|
||||
})
|
||||
|
||||
it('ignores stale attach subscription rejection after reattaching a newer remote terminal', async () => {
|
||||
let rejectOldSubscription: (error: Error) => void = () => {
|
||||
throw new Error('old subscription reject was not captured')
|
||||
}
|
||||
let rejectOldSubscription: ((error: Error) => void) | null = null
|
||||
const newStream = {
|
||||
streamId: 2,
|
||||
sendInput: vi.fn(() => true),
|
||||
@@ -389,7 +387,7 @@ describe('createRemoteRuntimePtyTransport', () => {
|
||||
})
|
||||
await vi.waitFor(() => expect(subscribeTerminal).toHaveBeenCalledTimes(2))
|
||||
|
||||
rejectOldSubscription(new Error('terminal_handle_stale'))
|
||||
rejectOldSubscription?.(new Error('terminal_handle_stale'))
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
|
||||
|
||||
@@ -366,4 +366,47 @@ describe('captureTerminalShutdownLayout', () => {
|
||||
expect(layout.activeLeafId).toBe(LEAF_ID_2)
|
||||
expect(layout.ptyIdsByLeafId).toEqual({ [LEAF_ID_2]: 'pty-2' })
|
||||
})
|
||||
|
||||
it('does not preserve a stale prior PTY binding for active shutdown focus', async () => {
|
||||
const { captureTerminalShutdownLayout } = await import('./terminal-shutdown-layout-capture')
|
||||
const paneWithoutPty = {
|
||||
id: 1,
|
||||
leafId: LEAF_ID,
|
||||
stablePaneId: LEAF_ID,
|
||||
terminal: { options: { scrollback: 1_000 } },
|
||||
serializeAddon: {
|
||||
serialize: vi.fn(() => '')
|
||||
}
|
||||
}
|
||||
const paneWithPty = {
|
||||
id: 2,
|
||||
leafId: LEAF_ID_2,
|
||||
stablePaneId: LEAF_ID_2,
|
||||
terminal: { options: { scrollback: 1_000 } },
|
||||
serializeAddon: {
|
||||
serialize: vi.fn(() => '')
|
||||
}
|
||||
}
|
||||
const manager = {
|
||||
getPanes: vi.fn(() => [paneWithoutPty, paneWithPty]),
|
||||
getActivePane: vi.fn(() => paneWithoutPty)
|
||||
}
|
||||
|
||||
const layout = captureTerminalShutdownLayout({
|
||||
manager: manager as never,
|
||||
container: mockRootForSplit(),
|
||||
expandedPaneId: null,
|
||||
paneTransports: new Map([[2, { getPtyId: vi.fn(() => 'pty-2') }]]),
|
||||
paneTitlesByPaneId: {},
|
||||
existingLayout: {
|
||||
root: null,
|
||||
activeLeafId: LEAF_ID,
|
||||
expandedLeafId: null,
|
||||
ptyIdsByLeafId: { [LEAF_ID]: 'stale-pty', [LEAF_ID_2]: 'pty-2' }
|
||||
}
|
||||
})
|
||||
|
||||
expect(layout.activeLeafId).toBe(LEAF_ID_2)
|
||||
expect(layout.ptyIdsByLeafId).toEqual({ [LEAF_ID_2]: 'pty-2' })
|
||||
})
|
||||
})
|
||||
|
||||
@@ -133,7 +133,8 @@ export function captureTerminalShutdownLayout({
|
||||
})
|
||||
const ptyIdsByLeafId = { ...preservedPtyIdsByLeafId, ...livePtyIdsByLeafId }
|
||||
// Why: shutdown snapshots can otherwise persist focus on a mounted pane whose
|
||||
// transport was already cleared during PTY exit/reconnect cleanup.
|
||||
// transport was already cleared during PTY exit/reconnect cleanup. Unlike
|
||||
// scrollback, PTY bindings only preserve prior ids during a live attach gap.
|
||||
layout.activeLeafId = resolveTerminalLayoutActiveLeafId({
|
||||
root: layout.root,
|
||||
activeLeafId: layout.activeLeafId,
|
||||
|
||||
@@ -139,6 +139,58 @@ describe('hydrateWorkspaceSession', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('moves restored active focus from a dead split leaf to a pty-backed sibling', () => {
|
||||
const store = createTestStore()
|
||||
const worktreeId = 'repo1::/wt-1'
|
||||
const liveLeftLeafId = '9ee09218-72a5-4e1c-b075-729e937d4e29'
|
||||
const liveRightLeafId = 'f5fc66b1-ec43-404b-b7b0-a06f0db34940'
|
||||
const deadActiveLeafId = 'fbf63fd9-34d6-4387-9109-562f7c02bc4c'
|
||||
seedStore(store, {
|
||||
worktreesByRepo: {
|
||||
repo1: [makeWorktree({ id: worktreeId, repoId: 'repo1', path: '/wt-1' })]
|
||||
}
|
||||
})
|
||||
|
||||
const session: WorkspaceSessionState = {
|
||||
activeRepoId: 'repo1',
|
||||
activeWorktreeId: worktreeId,
|
||||
activeTabId: 'tab-1',
|
||||
tabsByWorktree: {
|
||||
[worktreeId]: [makeTab({ id: 'tab-1', worktreeId, ptyId: 'old-pty' })]
|
||||
},
|
||||
terminalLayoutsByTabId: {
|
||||
'tab-1': {
|
||||
root: {
|
||||
type: 'split',
|
||||
direction: 'horizontal',
|
||||
first: { type: 'leaf', leafId: liveLeftLeafId },
|
||||
second: {
|
||||
type: 'split',
|
||||
direction: 'vertical',
|
||||
first: { type: 'leaf', leafId: liveRightLeafId },
|
||||
second: { type: 'leaf', leafId: deadActiveLeafId }
|
||||
}
|
||||
},
|
||||
activeLeafId: deadActiveLeafId,
|
||||
expandedLeafId: null,
|
||||
ptyIdsByLeafId: {
|
||||
[liveLeftLeafId]: 'daemon-session-left',
|
||||
[liveRightLeafId]: 'daemon-session-right'
|
||||
},
|
||||
buffersByLeafId: {
|
||||
[deadActiveLeafId]: 'retained scrollback'
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
store.getState().hydrateWorkspaceSession(session)
|
||||
|
||||
// Why: restart can preserve scrollback for an exited pane while live siblings
|
||||
// reattach. Keyboard focus must land on a PTY-backed pane, not the dead leaf.
|
||||
expect(store.getState().terminalLayoutsByTabId['tab-1']?.activeLeafId).toBe(liveLeftLeafId)
|
||||
})
|
||||
|
||||
it('hydrates floating terminal tabs even though they are not repo worktrees', () => {
|
||||
const store = createTestStore()
|
||||
const session: WorkspaceSessionState = {
|
||||
|
||||
@@ -44,7 +44,10 @@ import {
|
||||
restorePtyDataHandlersAfterFailedShutdown,
|
||||
unregisterPtyDataHandlers
|
||||
} from '@/components/terminal-pane/pty-transport'
|
||||
import { normalizeTerminalLayoutSnapshot } from '@/components/terminal-pane/terminal-layout-leaf-ids'
|
||||
import {
|
||||
normalizeTerminalLayoutSnapshot,
|
||||
resolvePtyBoundActiveLeafId
|
||||
} from '@/components/terminal-pane/terminal-layout-leaf-ids'
|
||||
import { shutdownBufferCaptures } from '@/components/terminal-pane/shutdown-buffer-captures'
|
||||
import { callRuntimeRpc } from '@/runtime/runtime-rpc-client'
|
||||
import { parseRemoteRuntimePtyId } from '@/runtime/runtime-terminal-stream'
|
||||
@@ -2780,7 +2783,15 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
|
||||
const tab = Object.values(tabsByWorktree)
|
||||
.flat()
|
||||
.find((entry) => entry.id === tabId)
|
||||
return [tabId, tab ? sanitizeTerminalLayoutPaneTitles(normalized, tab) : normalized]
|
||||
const sanitized = tab ? sanitizeTerminalLayoutPaneTitles(normalized, tab) : normalized
|
||||
const activeLeafId = sanitized.root
|
||||
? resolvePtyBoundActiveLeafId({
|
||||
root: sanitized.root,
|
||||
activeLeafId: sanitized.activeLeafId,
|
||||
ptyIdsByLeafId: sanitized.ptyIdsByLeafId
|
||||
})
|
||||
: sanitized.activeLeafId
|
||||
return [tabId, { ...sanitized, activeLeafId }]
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user