mirror of
https://github.com/stablyai/orca
synced 2026-04-21 14:17:16 +00:00
revert(terminal-pane): drop fragile cross-group PTY reattach machinery
Reverts the renderer-side workarounds that made terminal panes survive cross-group drag when the terminal daemon is off: rAF-retry measurement loop, SIGWINCH width-jitter, alt-screen prefix stripping, per-leaf scrollback capture, eager PTY buffer registry, and related plumbing. Why: the machinery is brittle under load (blank destination pane under some timings) and becomes dead weight once the daemon flips on by default, which provides snapshot-based reattach. Until then, a separate gate in TabGroupDndContext prevents cross-group terminal drops, so this code is unreachable for the flaky case anyway. Also removes the headful e2e spec that exercised the ripped path.
This commit is contained in:
brennanb2025
parent
e62ba1cb4c
commit
2d8a49422f
9 changed files with 102 additions and 926 deletions
|
|
@ -212,95 +212,6 @@ export default function TerminalPane({
|
|||
setTabLayout(tabId, layout)
|
||||
}, [tabId, setTabLayout])
|
||||
|
||||
// Serialize every pane's scrollback + layout into the Zustand snapshot.
|
||||
// Why: called from two distinct sites — the beforeunload handler in App.tsx
|
||||
// (app shutdown) AND the cross-group drag unmount path in the lifecycle hook.
|
||||
// Both need the same serialized buffers so restoreScrollbackBuffers can
|
||||
// repaint the terminal on remount before any fresh PTY output arrives.
|
||||
const captureBuffers = useCallback((): void => {
|
||||
const manager = managerRef.current
|
||||
const container = containerRef.current
|
||||
if (!manager || !container) {
|
||||
return
|
||||
}
|
||||
const panes = manager.getPanes()
|
||||
if (panes.length === 0) {
|
||||
return
|
||||
}
|
||||
// Flush pending background PTY output into terminals before serializing.
|
||||
// terminal.write() is async so some trailing bytes may be lost — best effort.
|
||||
for (const pane of panes) {
|
||||
const pending = pendingWritesRef.current.get(pane.id)
|
||||
if (pending) {
|
||||
pane.terminal.write(pending)
|
||||
pendingWritesRef.current.set(pane.id, '')
|
||||
}
|
||||
}
|
||||
const buffers: Record<string, string> = {}
|
||||
for (const pane of panes) {
|
||||
try {
|
||||
const leafId = paneLeafId(pane.id)
|
||||
let scrollback = pane.terminal.options.scrollback ?? 10_000
|
||||
let serialized = pane.serializeAddon.serialize({ scrollback })
|
||||
// Cap at 512KB — binary search for largest scrollback that fits.
|
||||
if (serialized.length > MAX_BUFFER_BYTES && scrollback > 1) {
|
||||
let lo = 1
|
||||
let hi = scrollback
|
||||
let best = ''
|
||||
while (lo <= hi) {
|
||||
const mid = Math.floor((lo + hi) / 2)
|
||||
const attempt = pane.serializeAddon.serialize({ scrollback: mid })
|
||||
if (attempt.length <= MAX_BUFFER_BYTES) {
|
||||
best = attempt
|
||||
lo = mid + 1
|
||||
} else {
|
||||
hi = mid - 1
|
||||
}
|
||||
}
|
||||
serialized = best
|
||||
}
|
||||
if (serialized.length > 0) {
|
||||
buffers[leafId] = serialized
|
||||
}
|
||||
} catch {
|
||||
// Serialization failure for one pane should not block others.
|
||||
}
|
||||
}
|
||||
const activePaneId = manager.getActivePane()?.id ?? panes[0]?.id ?? null
|
||||
const layout = serializeTerminalLayout(container, activePaneId, expandedPaneIdRef.current)
|
||||
if (Object.keys(buffers).length > 0) {
|
||||
layout.buffersByLeafId = buffers
|
||||
}
|
||||
const ptyEntries = panes
|
||||
.map(
|
||||
(pane) =>
|
||||
[
|
||||
paneLeafId(pane.id),
|
||||
paneTransportsRef.current.get(pane.id)?.getPtyId() ?? null
|
||||
] as const
|
||||
)
|
||||
.filter((entry): entry is readonly [string, string] => entry[1] !== null)
|
||||
if (ptyEntries.length > 0) {
|
||||
layout.ptyIdsByLeafId = Object.fromEntries(ptyEntries)
|
||||
}
|
||||
// Merge pane titles so the shutdown snapshot doesn't silently drop them.
|
||||
// Why: the old early-return on empty buffers skipped this entirely, which
|
||||
// meant titles were lost on restart when the terminal had no scrollback
|
||||
// content (e.g. fresh pane, cleared screen).
|
||||
const titleEntries = panes
|
||||
.filter((p) => paneTitlesRef.current[p.id])
|
||||
.map((p) => [paneLeafId(p.id), paneTitlesRef.current[p.id]] as const)
|
||||
if (titleEntries.length > 0) {
|
||||
layout.titlesByLeafId = Object.fromEntries(titleEntries)
|
||||
}
|
||||
setTabLayout(tabId, layout)
|
||||
}, [tabId, setTabLayout])
|
||||
|
||||
// Ref mirror so the lifecycle hook's unmount cleanup can invoke the latest
|
||||
// captureBuffers closure without participating in the hook's deps.
|
||||
const captureBuffersRef = useRef(captureBuffers)
|
||||
captureBuffersRef.current = captureBuffers
|
||||
|
||||
const syncPanePtyLayoutBinding = useCallback(
|
||||
(paneId: number, ptyId: string | null): void => {
|
||||
const existingLayout = useAppStore.getState().terminalLayoutsByTabId[tabId] ?? EMPTY_LAYOUT
|
||||
|
|
@ -441,7 +352,6 @@ export default function TerminalPane({
|
|||
setExpandedPane,
|
||||
syncExpandedLayout,
|
||||
persistLayoutSnapshot,
|
||||
captureBuffersRef,
|
||||
setPaneTitles,
|
||||
paneTitlesRef,
|
||||
setRenamingPaneId
|
||||
|
|
@ -708,15 +618,92 @@ export default function TerminalPane({
|
|||
}
|
||||
}, [paneTitles, renamingPaneId])
|
||||
|
||||
// Register the capture callback for shutdown. The beforeunload handler in
|
||||
// Register a capture callback for shutdown. The beforeunload handler in
|
||||
// App.tsx calls all registered callbacks to serialize terminal buffers.
|
||||
useEffect(() => {
|
||||
const fn = captureBuffers
|
||||
shutdownBufferCaptures.add(fn)
|
||||
return () => {
|
||||
shutdownBufferCaptures.delete(fn)
|
||||
const captureBuffers = (): void => {
|
||||
const manager = managerRef.current
|
||||
const container = containerRef.current
|
||||
if (!manager || !container) {
|
||||
return
|
||||
}
|
||||
const panes = manager.getPanes()
|
||||
if (panes.length === 0) {
|
||||
return
|
||||
}
|
||||
// Flush pending background PTY output into terminals before serializing.
|
||||
// terminal.write() is async so some trailing bytes may be lost — best effort.
|
||||
for (const pane of panes) {
|
||||
const pending = pendingWritesRef.current.get(pane.id)
|
||||
if (pending) {
|
||||
pane.terminal.write(pending)
|
||||
pendingWritesRef.current.set(pane.id, '')
|
||||
}
|
||||
}
|
||||
const buffers: Record<string, string> = {}
|
||||
for (const pane of panes) {
|
||||
try {
|
||||
const leafId = paneLeafId(pane.id)
|
||||
let scrollback = pane.terminal.options.scrollback ?? 10_000
|
||||
let serialized = pane.serializeAddon.serialize({ scrollback })
|
||||
// Cap at 512KB — binary search for largest scrollback that fits.
|
||||
if (serialized.length > MAX_BUFFER_BYTES && scrollback > 1) {
|
||||
let lo = 1
|
||||
let hi = scrollback
|
||||
let best = ''
|
||||
while (lo <= hi) {
|
||||
const mid = Math.floor((lo + hi) / 2)
|
||||
const attempt = pane.serializeAddon.serialize({ scrollback: mid })
|
||||
if (attempt.length <= MAX_BUFFER_BYTES) {
|
||||
best = attempt
|
||||
lo = mid + 1
|
||||
} else {
|
||||
hi = mid - 1
|
||||
}
|
||||
}
|
||||
serialized = best
|
||||
}
|
||||
if (serialized.length > 0) {
|
||||
buffers[leafId] = serialized
|
||||
}
|
||||
} catch {
|
||||
// Serialization failure for one pane should not block others.
|
||||
}
|
||||
}
|
||||
const activePaneId = manager.getActivePane()?.id ?? panes[0]?.id ?? null
|
||||
const layout = serializeTerminalLayout(container, activePaneId, expandedPaneIdRef.current)
|
||||
if (Object.keys(buffers).length > 0) {
|
||||
layout.buffersByLeafId = buffers
|
||||
}
|
||||
const ptyEntries = panes
|
||||
.map(
|
||||
(pane) =>
|
||||
[
|
||||
paneLeafId(pane.id),
|
||||
paneTransportsRef.current.get(pane.id)?.getPtyId() ?? null
|
||||
] as const
|
||||
)
|
||||
.filter((entry): entry is readonly [string, string] => entry[1] !== null)
|
||||
if (ptyEntries.length > 0) {
|
||||
layout.ptyIdsByLeafId = Object.fromEntries(ptyEntries)
|
||||
}
|
||||
// Merge pane titles so the shutdown snapshot doesn't silently drop them.
|
||||
// Why: the old early-return on empty buffers skipped this entirely, which
|
||||
// meant titles were lost on restart when the terminal had no scrollback
|
||||
// content (e.g. fresh pane, cleared screen).
|
||||
const titleEntries = panes
|
||||
.filter((p) => paneTitlesRef.current[p.id])
|
||||
.map((p) => [paneLeafId(p.id), paneTitlesRef.current[p.id]] as const)
|
||||
if (titleEntries.length > 0) {
|
||||
layout.titlesByLeafId = Object.fromEntries(titleEntries)
|
||||
}
|
||||
setTabLayout(tabId, layout)
|
||||
}
|
||||
}, [captureBuffers])
|
||||
shutdownBufferCaptures.add(captureBuffers)
|
||||
return () => {
|
||||
shutdownBufferCaptures.delete(captureBuffers)
|
||||
}
|
||||
}, [tabId, setTabLayout])
|
||||
|
||||
const handleStartRename = useCallback((paneId: number) => {
|
||||
setRenameValue(paneTitlesRef.current[paneId] ?? '')
|
||||
|
|
|
|||
|
|
@ -194,26 +194,11 @@ function collectLeafIds(
|
|||
* captured while the alternate screen was active (e.g. an agent TUI was
|
||||
* running at shutdown), we exit alt-screen first so the user sees a usable
|
||||
* normal-mode terminal.
|
||||
*
|
||||
* Alt-screen preservation is decided per-leaf via
|
||||
* `shouldPreserveAltScreenForLeaf`. Per-pane granularity matters because a
|
||||
* split tab can have one live PTY and one dead PTY at the same time — e.g.
|
||||
* after an in-session cross-group drag where one pane's process exited
|
||||
* before the drag. A global flag would either (a) preserve alt-screen for
|
||||
* the dead pane, leaving the user stuck in an empty alt-screen buffer with
|
||||
* no TUI process to exit it, or (b) strip alt-screen for the live pane,
|
||||
* clobbering the running TUI's rendering context so its next SIGWINCH
|
||||
* repaint lands in the normal buffer instead of the alt-screen where it
|
||||
* belongs. When the predicate returns true for a leaf we keep the leading
|
||||
* `\x1b[?1049h\x1b[H` prefix so the live TUI's next repaint lands in the
|
||||
* alt-screen; when it returns false (or is omitted) we drop alt-screen
|
||||
* entirely and show the pre-TUI normal scrollback.
|
||||
*/
|
||||
export function restoreScrollbackBuffers(
|
||||
manager: PaneManager,
|
||||
savedBuffers: Record<string, string> | undefined,
|
||||
restoredPaneByLeafId: Map<string, number>,
|
||||
options: { shouldPreserveAltScreenForLeaf?: (leafId: string) => boolean } = {}
|
||||
restoredPaneByLeafId: Map<string, number>
|
||||
): void {
|
||||
if (!savedBuffers) {
|
||||
return
|
||||
|
|
@ -229,41 +214,20 @@ export function restoreScrollbackBuffers(
|
|||
if (!pane) {
|
||||
continue
|
||||
}
|
||||
const preserveAltScreen = options.shouldPreserveAltScreenForLeaf?.(oldLeafId) ?? false
|
||||
try {
|
||||
let buf = buffer
|
||||
// If buffer ends in alt-screen mode (agent TUI was running at
|
||||
// shutdown), exit alt-screen so the user sees a usable terminal.
|
||||
const lastOn = buf.lastIndexOf(ALT_SCREEN_ON)
|
||||
const lastOff = buf.lastIndexOf(ALT_SCREEN_OFF)
|
||||
const endsInAltScreen = lastOn > lastOff
|
||||
// Why: the captured alt-screen content was painted at the source pane's
|
||||
// dimensions, but the freshly-opened xterm here has default 80×24 cell
|
||||
// geometry until the first render cycle completes. Writing the captured
|
||||
// alt-screen content verbatim produces a mangled layout, so we drop the
|
||||
// content and keep only the `\x1b[?1049h\x1b[H` prefix. That leaves
|
||||
// xterm in an empty alt-screen buffer, ready for the live TUI's next
|
||||
// repaint. This matters especially for TUIs (ratatui-based ones like
|
||||
// Codex) that only issue the alt-screen enter once at startup and
|
||||
// assume they stay there — if xterm isn't in alt-screen when their
|
||||
// redraw arrives, the output goes to the normal buffer and is invisible.
|
||||
// Ink-based TUIs (Claude Code) happen to re-emit the alt-screen enter
|
||||
// on every frame, so either approach would work for them.
|
||||
// On app restart the TUI process is dead, so we drop alt-screen entirely
|
||||
// and show the pre-TUI normal scrollback instead — leaving the user in
|
||||
// an empty alt-screen with nothing to exit it would feel broken.
|
||||
if (endsInAltScreen) {
|
||||
buf = preserveAltScreen
|
||||
? `${buf.slice(0, lastOn)}${ALT_SCREEN_ON}\x1b[H`
|
||||
: buf.slice(0, lastOn)
|
||||
if (lastOn > lastOff) {
|
||||
buf = buf.slice(0, lastOn)
|
||||
}
|
||||
if (buf.length > 0) {
|
||||
pane.terminal.write(buf)
|
||||
// Ensure cursor is on a new line so the new shell prompt
|
||||
// doesn't trigger zsh's PROMPT_EOL_MARK (%) indicator. Skip when
|
||||
// we've intentionally kept alt-screen state: appending CRLF there
|
||||
// would scroll the (empty) TUI viewport before it repaints.
|
||||
if (!endsInAltScreen || !preserveAltScreen) {
|
||||
pane.terminal.write('\r\n')
|
||||
}
|
||||
// doesn't trigger zsh's PROMPT_EOL_MARK (%) indicator.
|
||||
pane.terminal.write('\r\n')
|
||||
}
|
||||
} catch {
|
||||
// If restore fails, continue with blank terminal.
|
||||
|
|
|
|||
|
|
@ -7,11 +7,6 @@ export type PtyConnectionDeps = {
|
|||
startup?: { command: string; env?: Record<string, string> } | null
|
||||
restoredLeafId?: string | null
|
||||
restoredPtyIdByLeafId?: Record<string, string>
|
||||
/** Leaf IDs whose scrollback buffer was just written into the xterm via
|
||||
* restoreScrollbackBuffers. The in-session attach path uses this to
|
||||
* skip its default screen-clear, which would otherwise wipe the restored
|
||||
* history during cross-group drag. */
|
||||
leafIdsWithRestoredBuffer?: Set<string>
|
||||
paneTransportsRef: React.RefObject<Map<number, PtyTransport>>
|
||||
pendingWritesRef: React.RefObject<Map<number, string>>
|
||||
isActiveRef: React.RefObject<boolean>
|
||||
|
|
|
|||
|
|
@ -69,14 +69,7 @@ vi.mock('./pty-transport', () => ({
|
|||
throw new Error('No mock transport queued')
|
||||
}
|
||||
return nextTransport
|
||||
}),
|
||||
// Why: connectPanePty consults isLiveInSessionPtyId to tell in-session live
|
||||
// PTYs (split-pane drag) apart from stale daemon sessionIds (cross-session
|
||||
// restart). The connection-level tests don't spin up the real dispatcher,
|
||||
// so a default false keeps existing cross-session routing (deferred
|
||||
// reattach via sessionId) unchanged. Tests exercising the live-PTY branch
|
||||
// override this via mockReturnValue before calling connectPanePty.
|
||||
isLiveInSessionPtyId: vi.fn(() => false)
|
||||
})
|
||||
}))
|
||||
|
||||
function createMockTransport(initialPtyId: string | null = null): MockTransport {
|
||||
|
|
@ -105,13 +98,11 @@ function createPane(paneId: number) {
|
|||
cols: 120,
|
||||
rows: 40,
|
||||
write: vi.fn(),
|
||||
refresh: vi.fn(),
|
||||
onData: vi.fn(() => ({ dispose: vi.fn() })),
|
||||
onResize: vi.fn(() => ({ dispose: vi.fn() }))
|
||||
},
|
||||
fitAddon: {
|
||||
fit: vi.fn(),
|
||||
proposeDimensions: vi.fn(() => ({ cols: 120, rows: 40 }))
|
||||
fit: vi.fn()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ import { isGeminiTerminalTitle, isClaudeAgent } from '@/lib/agent-status'
|
|||
import { scheduleRuntimeGraphSync } from '@/runtime/sync-runtime-graph'
|
||||
import { useAppStore } from '@/store'
|
||||
import type { PtyConnectResult } from './pty-transport'
|
||||
import { createIpcPtyTransport, isLiveInSessionPtyId } from './pty-transport'
|
||||
import { createIpcPtyTransport } from './pty-transport'
|
||||
import { shouldSeedCacheTimerOnInitialTitle } from './cache-timer-seeding'
|
||||
import type { PtyConnectionDeps } from './pty-connection-types'
|
||||
|
||||
|
|
@ -199,50 +199,11 @@ export function connectPanePty(
|
|||
// Defer PTY spawn/attach to next frame so FitAddon has time to calculate
|
||||
// the correct terminal dimensions from the laid-out container.
|
||||
deps.pendingWritesRef.current.set(pane.id, '')
|
||||
// Why: cap the measurement-wait retry so a pane opened into a permanently
|
||||
// zero-sized container (hidden tab, collapsed flex) eventually attaches
|
||||
// with whatever xterm reports rather than rAF-spinning until disposal.
|
||||
// 30 frames ≈ 500 ms at 60 fps, well past any reasonable layout settle.
|
||||
const MAX_MEASURE_FRAMES = 30
|
||||
let measureFramesLeft = MAX_MEASURE_FRAMES
|
||||
const runAttachWork = (): void => {
|
||||
connectFrame = requestAnimationFrame(() => {
|
||||
connectFrame = null
|
||||
if (disposed) {
|
||||
return
|
||||
}
|
||||
// Why: FitAddon.fit() reads _renderService.dimensions.css.cell.{width,height},
|
||||
// which are populated after xterm's first render pass. For panes whose
|
||||
// containers gain layout only after DOM reparenting (bottom pane of a
|
||||
// split created via splitPane()), the renderer hasn't measured cell
|
||||
// geometry yet on the first rAF — proposeDimensions() returns undefined,
|
||||
// fit() is a silent no-op, and we end up reading xterm's default 80×24.
|
||||
// When we then pass those defaults to transport.attach(), the PTY resizes
|
||||
// to 80×24 and the live TUI redraws for 80×24 into an xterm that should
|
||||
// be showing a much larger frame — the rest of the pane stays blank with
|
||||
// only the cursor position responding to keystrokes (classic symptom).
|
||||
// Forcing a refresh(0, rows-1) primes the renderer to measure cell
|
||||
// geometry before fit() runs. If it still can't propose dimensions we
|
||||
// defer one more frame rather than committing to default dims.
|
||||
try {
|
||||
pane.terminal.refresh(0, Math.max(0, pane.terminal.rows - 1))
|
||||
} catch {
|
||||
/* ignore — renderer may still be initialising */
|
||||
}
|
||||
const proposed = pane.fitAddon.proposeDimensions()
|
||||
if (!proposed || proposed.cols === 0 || proposed.rows === 0) {
|
||||
if (measureFramesLeft > 0) {
|
||||
measureFramesLeft -= 1
|
||||
connectFrame = requestAnimationFrame(runAttachWork)
|
||||
return
|
||||
}
|
||||
// Fall through to attach with xterm's current (likely default) dims
|
||||
// so the pane at least connects; a subsequent resize event (tab
|
||||
// becoming visible, container growing) will refit via onResize.
|
||||
deps.onPtyErrorRef?.current?.(
|
||||
pane.id,
|
||||
'Terminal failed to measure cell geometry; connecting at default size.'
|
||||
)
|
||||
}
|
||||
try {
|
||||
pane.fitAddon.fit()
|
||||
} catch {
|
||||
|
|
@ -365,26 +326,14 @@ export function connectPanePty(
|
|||
// tab-level ptyId populated, while daemon-off cold starts clear it during
|
||||
// session hydration.
|
||||
const restoredSessionId = restoredPtyId ?? null
|
||||
// Why (split-pane in-session drag): tab.ptyId only tracks pane 1, so the
|
||||
// old `restoredSessionId === existingPtyId` check always rejected split
|
||||
// panes' restored PTY ids and routed them to fresh-spawn. When the daemon
|
||||
// is disabled, fall back to the dispatcher's exit-handler map
|
||||
// (isLiveInSessionPtyId) to tell "alive right now in this renderer" apart
|
||||
// from "stale daemon sessionId from the previous run" — that lets every
|
||||
// live pane reattach instead of re-spawning a fresh shell. With the
|
||||
// daemon on, keep the original behavior so snapshot/cold-restore still
|
||||
// runs through deferredReattachSessionId at real fitAddon dimensions.
|
||||
const restoredSessionLiveInSession = !daemonEnabled && isLiveInSessionPtyId(restoredSessionId)
|
||||
const detachedLivePtyId =
|
||||
existingPtyId && !hasExistingPaneTransport
|
||||
? restoredSessionId
|
||||
? restoredSessionId === existingPtyId || restoredSessionLiveInSession
|
||||
? restoredSessionId === existingPtyId
|
||||
? restoredSessionId
|
||||
: null
|
||||
: existingPtyId
|
||||
: restoredSessionLiveInSession
|
||||
? restoredSessionId
|
||||
: null
|
||||
: null
|
||||
const deferredReattachSessionId =
|
||||
restoredSessionId && restoredSessionId !== detachedLivePtyId
|
||||
? restoredSessionId
|
||||
|
|
@ -469,19 +418,11 @@ export function connectPanePty(
|
|||
// ptyId must also be cleared from the tab and a fresh spawn kicked
|
||||
// off — otherwise the next remount reads the same dead ptyId from
|
||||
// the store and lands in this branch again in a loop.
|
||||
const hasRestoredBuffer = Boolean(
|
||||
deps.restoredLeafId && deps.leafIdsWithRestoredBuffer?.has(deps.restoredLeafId)
|
||||
)
|
||||
try {
|
||||
transport.attach({
|
||||
existingPtyId: detachedLivePtyId,
|
||||
cols,
|
||||
rows,
|
||||
// Why: restoreScrollbackBuffers has already painted the serialized
|
||||
// scrollback into xterm on this remount. Suppressing the default
|
||||
// 2J/3J clear preserves that history — the live PTY has no fresh
|
||||
// payload to repaint with on this code path.
|
||||
skipClearOnAttach: hasRestoredBuffer,
|
||||
callbacks: {
|
||||
onData: dataCallback,
|
||||
onError: reportError
|
||||
|
|
@ -543,8 +484,7 @@ export function connectPanePty(
|
|||
}
|
||||
}
|
||||
scheduleRuntimeGraphSync()
|
||||
}
|
||||
connectFrame = requestAnimationFrame(runAttachWork)
|
||||
})
|
||||
|
||||
return {
|
||||
dispose() {
|
||||
|
|
|
|||
|
|
@ -68,20 +68,6 @@ export function getEagerPtyBufferHandle(ptyId: string): EagerPtyHandle | undefin
|
|||
return eagerPtyHandles.get(ptyId)
|
||||
}
|
||||
|
||||
/**
|
||||
* Why: in-session cross-group drag detaches a PTY (which removes its data
|
||||
* handler but keeps the exit handler so stale bindings still clear cleanly).
|
||||
* The destination pane needs to tell "live same-session PTY" apart from "stale
|
||||
* daemon session ID from the previous app run" without trusting the tab-level
|
||||
* ptyId (which only tracks the first pane). An exit handler is registered the
|
||||
* moment a transport connects or attaches, and is only removed when the PTY
|
||||
* actually exits — so its presence is a reliable "this PTY is alive right now"
|
||||
* signal for the attach vs. fresh-spawn decision.
|
||||
*/
|
||||
export function isLiveInSessionPtyId(ptyId: string | null | undefined): boolean {
|
||||
return ptyId != null && ptyExitHandlers.has(ptyId)
|
||||
}
|
||||
|
||||
// Why: 512 KB matches the scrollback buffer cap used by TerminalPane's
|
||||
// serialization. Prevents unbounded memory growth if a restored shell
|
||||
// runs a long-lived command (e.g. tail -f) in a worktree the user never opens.
|
||||
|
|
@ -179,12 +165,6 @@ export type PtyTransport = {
|
|||
* Skips the delayed double-resize since a single resize already triggers
|
||||
* a full TUI repaint without content loss. */
|
||||
isAlternateScreen?: boolean
|
||||
/** Suppress the usual 2J/3J/H clear written into the xterm on attach.
|
||||
* Why: during in-session cross-group drag, restoreScrollbackBuffers has
|
||||
* already written the serialized scrollback into the destination pane,
|
||||
* and the live PTY has no fresh output to send — so the default clear
|
||||
* would erase the user's visible history. */
|
||||
skipClearOnAttach?: boolean
|
||||
callbacks: {
|
||||
onConnect?: () => void
|
||||
onDisconnect?: () => void
|
||||
|
|
|
|||
|
|
@ -22,7 +22,6 @@ import { createBellDetector } from './bell-detector'
|
|||
export {
|
||||
ensurePtyDispatcher,
|
||||
getEagerPtyBufferHandle,
|
||||
isLiveInSessionPtyId,
|
||||
registerEagerPtyBuffer,
|
||||
unregisterPtyDataHandlers
|
||||
} from './pty-dispatcher'
|
||||
|
|
@ -58,11 +57,6 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
|
|||
let lastObservedTerminalTitle: string | null = null
|
||||
let openCodeStatus: OpenCodeStatusEvent['status'] | null = null
|
||||
let staleTitleTimer: ReturnType<typeof setTimeout> | null = null
|
||||
// Why: attach() schedules a deferred resize to trigger SIGWINCH for TUI
|
||||
// repaint after cross-group drag. Track the handle so detach/disconnect/
|
||||
// destroy can cancel it before it fires on a torn-down or killed PTY,
|
||||
// which would cause a spurious SIGWINCH or silent IPC error.
|
||||
let attachResizeTimer: ReturnType<typeof setTimeout> | null = null
|
||||
const agentTracker =
|
||||
onAgentBecameIdle || onAgentBecameWorking || onAgentExited
|
||||
? createAgentStatusTracker(
|
||||
|
|
@ -272,14 +266,6 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
|
|||
return
|
||||
}
|
||||
|
||||
// Why: clear any pending deferred resize from a prior attach so we
|
||||
// don't end up with two overlapping timers racing against the new
|
||||
// geometry.
|
||||
if (attachResizeTimer) {
|
||||
clearTimeout(attachResizeTimer)
|
||||
attachResizeTimer = null
|
||||
}
|
||||
|
||||
const id = options.existingPtyId
|
||||
ptyId = id
|
||||
connected = true
|
||||
|
|
@ -310,49 +296,13 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
|
|||
// Why: clear the display before writing the snapshot so restored
|
||||
// content doesn't layer on top of stale output. Skip the clear for
|
||||
// alternate-screen sessions — the snapshot already fills the screen
|
||||
// and clearing would erase it. Also skip when the caller has already
|
||||
// hydrated the xterm with a serialized scrollback buffer (in-session
|
||||
// cross-group drag): clearing would erase the visible history and
|
||||
// there's no fresh snapshot payload on this attach path to replace it.
|
||||
if (!options.isAlternateScreen && !options.skipClearOnAttach) {
|
||||
// and clearing would erase it.
|
||||
if (!options.isAlternateScreen) {
|
||||
storedCallbacks.onData?.('\x1b[2J\x1b[3J\x1b[H')
|
||||
}
|
||||
|
||||
// Why: in-session cross-group drag leaves live TUIs (Codex, Claude Code,
|
||||
// Ink, anything using the alternate screen buffer) pointing at a fresh
|
||||
// xterm whose alt-screen is empty — the old renderer was torn down, and
|
||||
// the captured scrollback deliberately stripped the alt-screen portion
|
||||
// because replaying it into a newly-opened xterm (default 80×24 until
|
||||
// its cell geometry is measured) corrupts the layout. Many TUIs
|
||||
// (notably Node-based ones like Ink) only redraw on an actual
|
||||
// 'resize' event from the OS, which is only raised when the PTY
|
||||
// window dimensions truly change. A plain resize to the same cols/rows
|
||||
// as before is a no-op, so we force a real dim change: resize to
|
||||
// (cols - 1) × rows for one tick, then back to (cols, rows). That
|
||||
// guarantees two SIGWINCH deliveries — the first kicks the TUI into
|
||||
// its resize handler (which typically triggers a full repaint), the
|
||||
// second restores the correct geometry before the user sees a wrong-
|
||||
// width frame. Plain shells are unaffected by width jitter.
|
||||
if (options.cols && options.rows) {
|
||||
if (options.skipClearOnAttach && options.cols > 2) {
|
||||
window.api.pty.resize(id, options.cols - 1, options.rows)
|
||||
// Let the TUI see the width change and schedule its repaint before
|
||||
// we restore the real width; 32 ms covers one 30 Hz frame which
|
||||
// Ink/React TUIs use for their batched rerender scheduler.
|
||||
attachResizeTimer = setTimeout(() => {
|
||||
attachResizeTimer = null
|
||||
// Why: if the transport was torn down or re-attached to a
|
||||
// different PTY during the 32 ms window, issuing the resize
|
||||
// would either SIGWINCH a dead PTY or retarget the wrong
|
||||
// process. Bail when the context has shifted.
|
||||
if (destroyed || ptyId !== id) {
|
||||
return
|
||||
}
|
||||
window.api.pty.resize(id, options.cols!, options.rows!)
|
||||
}, 32)
|
||||
} else {
|
||||
window.api.pty.resize(id, options.cols, options.rows)
|
||||
}
|
||||
window.api.pty.resize(id, options.cols, options.rows)
|
||||
}
|
||||
|
||||
storedCallbacks.onConnect?.()
|
||||
|
|
@ -364,10 +314,6 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
|
|||
clearTimeout(staleTitleTimer)
|
||||
staleTitleTimer = null
|
||||
}
|
||||
if (attachResizeTimer) {
|
||||
clearTimeout(attachResizeTimer)
|
||||
attachResizeTimer = null
|
||||
}
|
||||
openCodeStatus = null
|
||||
if (ptyId) {
|
||||
const id = ptyId
|
||||
|
|
@ -384,10 +330,6 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
|
|||
clearTimeout(staleTitleTimer)
|
||||
staleTitleTimer = null
|
||||
}
|
||||
if (attachResizeTimer) {
|
||||
clearTimeout(attachResizeTimer)
|
||||
attachResizeTimer = null
|
||||
}
|
||||
openCodeStatus = null
|
||||
if (ptyId) {
|
||||
// Why: detach() is used for in-session remounts such as moving a tab
|
||||
|
|
@ -428,13 +370,6 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
|
|||
|
||||
destroy() {
|
||||
destroyed = true
|
||||
// Why: cancel the deferred attach resize explicitly as well as via
|
||||
// disconnect() so the guard holds even if a future refactor changes
|
||||
// disconnect()'s teardown ordering.
|
||||
if (attachResizeTimer) {
|
||||
clearTimeout(attachResizeTimer)
|
||||
attachResizeTimer = null
|
||||
}
|
||||
this.disconnect()
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -26,7 +26,6 @@ import { applyExpandedLayoutTo, restoreExpandedLayoutFrom } from './expand-colla
|
|||
import { applyTerminalAppearance } from './terminal-appearance'
|
||||
import { connectPanePty } from './pty-connection'
|
||||
import type { PtyTransport } from './pty-transport'
|
||||
import { isLiveInSessionPtyId } from './pty-dispatcher'
|
||||
import { fitAndFocusPanes, fitPanes } from './pane-helpers'
|
||||
import { registerRuntimeTerminalTab, scheduleRuntimeGraphSync } from '@/runtime/sync-runtime-graph'
|
||||
import { e2eConfig } from '@/lib/e2e-config'
|
||||
|
|
@ -83,11 +82,6 @@ type UseTerminalPaneLifecycleDeps = {
|
|||
setExpandedPane: (paneId: number | null) => void
|
||||
syncExpandedLayout: () => void
|
||||
persistLayoutSnapshot: () => void
|
||||
/** Called from unmount cleanup on the cross-group drag path (tab survives)
|
||||
* to serialize each pane's scrollback into the store before PTYs detach,
|
||||
* so the remounted TerminalPane can repaint via restoreScrollbackBuffers
|
||||
* before any fresh PTY output arrives. */
|
||||
captureBuffersRef: React.RefObject<() => void>
|
||||
setPaneTitles: React.Dispatch<React.SetStateAction<Record<number, string>>>
|
||||
paneTitlesRef: React.RefObject<Record<number, string>>
|
||||
setRenamingPaneId: React.Dispatch<React.SetStateAction<number | null>>
|
||||
|
|
@ -159,7 +153,6 @@ export function useTerminalPaneLifecycle({
|
|||
setExpandedPane,
|
||||
syncExpandedLayout,
|
||||
persistLayoutSnapshot,
|
||||
captureBuffersRef,
|
||||
setPaneTitles,
|
||||
paneTitlesRef,
|
||||
setRenamingPaneId
|
||||
|
|
@ -238,25 +231,11 @@ export function useTerminalPaneLifecycle({
|
|||
setTabCanExpandPane(tabId, paneCount > 1)
|
||||
}
|
||||
|
||||
// Why: on cross-group drag, the source TerminalPane's unmount cleanup
|
||||
// writes fresh scrollback into the store via captureBuffersRef — but that
|
||||
// cleanup runs *after* this new component rendered, so the render-time
|
||||
// `useRef(savedLayout)` snapshot in initialLayoutRef is already stale.
|
||||
// Re-read the store here so the remounted pane replays the just-captured
|
||||
// buffer + PTY + title state instead of the pre-drag snapshot.
|
||||
const freshLayout = useAppStore.getState().terminalLayoutsByTabId[tabId]
|
||||
if (freshLayout) {
|
||||
initialLayoutRef.current = freshLayout
|
||||
}
|
||||
|
||||
let shouldPersistLayout = false
|
||||
const restoredLeafIdsInCreationOrder = collectLeafIdsInReplayCreationOrder(
|
||||
initialLayoutRef.current.root
|
||||
)
|
||||
let restoredPaneCreateIndex = 0
|
||||
const restoredBufferLeafIds = new Set(
|
||||
Object.keys(initialLayoutRef.current.buffersByLeafId ?? {})
|
||||
)
|
||||
const ptyDeps = {
|
||||
tabId,
|
||||
worktreeId,
|
||||
|
|
@ -278,8 +257,7 @@ export function useTerminalPaneLifecycle({
|
|||
dispatchNotification,
|
||||
setCacheTimerStartedAt,
|
||||
syncPanePtyLayoutBinding,
|
||||
restoredPtyIdByLeafId: initialLayoutRef.current.ptyIdsByLeafId ?? {},
|
||||
leafIdsWithRestoredBuffer: restoredBufferLeafIds
|
||||
restoredPtyIdByLeafId: initialLayoutRef.current.ptyIdsByLeafId ?? {}
|
||||
}
|
||||
|
||||
const unregisterRuntimeTab = registerRuntimeTerminalTab({
|
||||
|
|
@ -476,32 +454,10 @@ export function useTerminalPaneLifecycle({
|
|||
}
|
||||
const restoredPaneByLeafId = replayTerminalLayout(manager, initialLayoutRef.current, isActive)
|
||||
|
||||
// Why: on in-session cross-group drag the underlying PTYs are still alive.
|
||||
// restoreScrollbackBuffers drops the captured alt-screen *content* (it was
|
||||
// painted at the source pane's dimensions and would mis-lay against the
|
||||
// destination pane's different geometry) but keeps xterm in the alt-screen
|
||||
// buffer with an empty canvas — so when transport.attach() kicks a
|
||||
// dim-change SIGWINCH, the live TUI repaints into a fresh alt-screen that
|
||||
// matches its own assumptions. Needed especially for ratatui-based TUIs
|
||||
// (Codex) that only issue `\x1b[?1049h` once at startup; without this
|
||||
// their redraws would land in the normal buffer and be invisible.
|
||||
// On app-restart the TUI is dead, so we strip alt-screen entirely and
|
||||
// show the pre-TUI normal scrollback.
|
||||
// Why: per-leaf preservation — a split tab can have one live PTY and one dead
|
||||
// PTY. Globally preserving alt-screen would leave the dead pane stuck in an
|
||||
// empty alt-screen with no TUI to exit it; globally stripping would clobber
|
||||
// the live pane's TUI state. Evaluate liveness per-leaf instead.
|
||||
const ptyIdsByLeafId = initialLayoutRef.current.ptyIdsByLeafId ?? {}
|
||||
restoreScrollbackBuffers(
|
||||
manager,
|
||||
initialLayoutRef.current.buffersByLeafId,
|
||||
restoredPaneByLeafId,
|
||||
{
|
||||
shouldPreserveAltScreenForLeaf: (leafId) => {
|
||||
const ptyId = ptyIdsByLeafId[leafId]
|
||||
return ptyId != null && isLiveInSessionPtyId(ptyId)
|
||||
}
|
||||
}
|
||||
restoredPaneByLeafId
|
||||
)
|
||||
|
||||
// Seed pane titles from the persisted snapshot using the same
|
||||
|
|
@ -614,16 +570,6 @@ export function useTerminalPaneLifecycle({
|
|||
.getState()
|
||||
.tabsByWorktree[worktreeId]?.find((candidate) => candidate.id === tabId)
|
||||
)
|
||||
// Why: on cross-group drag, the TerminalPane unmounts but the tab (and
|
||||
// its PTYs) survive. Serialize the live scrollback into the store *before*
|
||||
// PaneManager tears down the xterm instances so the remounted pane can
|
||||
// call restoreScrollbackBuffers and repaint what the user was reading —
|
||||
// otherwise the destination group shows a blank terminal until fresh PTY
|
||||
// output redraws the screen. Skipped on full tab close (tabStillExists
|
||||
// false) because there's nothing to restore into.
|
||||
if (tabStillExists) {
|
||||
captureBuffersRef.current?.()
|
||||
}
|
||||
unregisterRuntimeTab()
|
||||
if (resizeRaf !== null) {
|
||||
cancelAnimationFrame(resizeRaf)
|
||||
|
|
|
|||
|
|
@ -1,562 +0,0 @@
|
|||
/* oxlint-disable max-lines */
|
||||
/**
|
||||
* E2E tests for dragging tabs across tab groups.
|
||||
*
|
||||
* User Prompt:
|
||||
* - playwright tests for drag-tabs-across-groups
|
||||
*
|
||||
* Why headful: dnd-kit's PointerSensor needs real pointer events to fire,
|
||||
* including its 5px distance activation constraint. Headless Electron does
|
||||
* not reliably dispatch the pointer sequence needed to start a drag, so
|
||||
* every test here is tagged @headful and runs under the electron-headful
|
||||
* project. The whole suite is serial so Playwright never tries to open
|
||||
* multiple visible Electron windows at once.
|
||||
*/
|
||||
|
||||
import type { Page } from '@stablyai/playwright-test'
|
||||
import { test, expect } from './helpers/orca-app'
|
||||
import {
|
||||
waitForSessionReady,
|
||||
waitForActiveWorktree,
|
||||
getActiveWorktreeId,
|
||||
getWorktreeTabs,
|
||||
ensureTerminalVisible
|
||||
} from './helpers/store'
|
||||
import {
|
||||
splitActiveTerminalPane,
|
||||
waitForActiveTerminalManager,
|
||||
waitForPaneCount
|
||||
} from './helpers/terminal'
|
||||
|
||||
type GroupSnapshot = { id: string; tabOrder: string[] }
|
||||
|
||||
async function createTerminalTab(page: Page, worktreeId: string): Promise<void> {
|
||||
await page.evaluate((targetWorktreeId) => {
|
||||
const store = window.__store
|
||||
if (!store) {
|
||||
return
|
||||
}
|
||||
const state = store.getState()
|
||||
state.createTab(targetWorktreeId)
|
||||
state.setActiveTabType('terminal')
|
||||
// Why: re-read state after createTab — the captured `state` holds the
|
||||
// pre-create snapshot. Concat the new tab at the end of the fresh tab list.
|
||||
const updatedTabs = store.getState().tabsByWorktree[targetWorktreeId] ?? []
|
||||
state.setTabBarOrder(
|
||||
targetWorktreeId,
|
||||
updatedTabs.map((tab) => tab.id)
|
||||
)
|
||||
}, worktreeId)
|
||||
}
|
||||
|
||||
async function getGroupSnapshot(page: Page, worktreeId: string): Promise<GroupSnapshot[]> {
|
||||
return page.evaluate((wt) => {
|
||||
const store = window.__store
|
||||
if (!store) {
|
||||
return []
|
||||
}
|
||||
const groups = store.getState().groupsByWorktree?.[wt] ?? []
|
||||
return groups.map((g: { id: string; tabOrder: string[] }) => ({
|
||||
id: g.id,
|
||||
tabOrder: [...g.tabOrder]
|
||||
}))
|
||||
}, worktreeId)
|
||||
}
|
||||
|
||||
async function splitActiveGroupRight(page: Page, worktreeId: string): Promise<string | null> {
|
||||
return page.evaluate((wt) => {
|
||||
const store = window.__store
|
||||
if (!store) {
|
||||
return null
|
||||
}
|
||||
const state = store.getState()
|
||||
const groups = state.groupsByWorktree?.[wt] ?? []
|
||||
const activeId = state.activeGroupIdByWorktree?.[wt] ?? groups[0]?.id
|
||||
if (!activeId) {
|
||||
return null
|
||||
}
|
||||
return state.createEmptySplitGroup(wt, activeId, 'right') ?? null
|
||||
}, worktreeId)
|
||||
}
|
||||
|
||||
async function moveTabToGroup(
|
||||
page: Page,
|
||||
unifiedTabId: string,
|
||||
targetGroupId: string
|
||||
): Promise<void> {
|
||||
await page.evaluate(
|
||||
([id, group]) => {
|
||||
window.__store?.getState().moveUnifiedTabToGroup(id, group, { activate: false })
|
||||
},
|
||||
[unifiedTabId, targetGroupId] as const
|
||||
)
|
||||
}
|
||||
|
||||
async function getUnifiedTabGroupId(page: Page, unifiedTabId: string): Promise<string | null> {
|
||||
return page.evaluate((id) => {
|
||||
const store = window.__store
|
||||
if (!store) {
|
||||
return null
|
||||
}
|
||||
const byWorktree = store.getState().unifiedTabsByWorktree ?? {}
|
||||
for (const tabs of Object.values(byWorktree) as { id: string; groupId: string }[][]) {
|
||||
const match = tabs.find((t) => t.id === id)
|
||||
if (match) {
|
||||
return match.groupId
|
||||
}
|
||||
}
|
||||
return null
|
||||
}, unifiedTabId)
|
||||
}
|
||||
|
||||
/**
|
||||
* Drive a real pointer drag from one tab element to another.
|
||||
*
|
||||
* Why: dnd-kit's PointerSensor requires (a) a 5px movement after pointerdown
|
||||
* to activate and (b) intermediate pointermoves to fire onDragOver against
|
||||
* the target droppable. The nudge + stepped move satisfies both.
|
||||
*/
|
||||
async function dragTabTo(
|
||||
page: Page,
|
||||
sourceUnifiedTabId: string,
|
||||
targetUnifiedTabId: string
|
||||
): Promise<void> {
|
||||
const sourceLocator = page.locator(`[data-unified-tab-id="${sourceUnifiedTabId}"]`).first()
|
||||
const targetLocator = page.locator(`[data-unified-tab-id="${targetUnifiedTabId}"]`).first()
|
||||
await expect(sourceLocator).toBeVisible({ timeout: 5_000 })
|
||||
await expect(targetLocator).toBeVisible({ timeout: 5_000 })
|
||||
|
||||
const sourceBox = await sourceLocator.boundingBox()
|
||||
const targetBox = await targetLocator.boundingBox()
|
||||
if (!sourceBox || !targetBox) {
|
||||
throw new Error('Could not resolve bounding boxes for drag source/target')
|
||||
}
|
||||
|
||||
const sx = sourceBox.x + sourceBox.width / 2
|
||||
const sy = sourceBox.y + sourceBox.height / 2
|
||||
const tx = targetBox.x + targetBox.width / 2
|
||||
const ty = targetBox.y + targetBox.height / 2
|
||||
|
||||
await page.mouse.move(sx, sy)
|
||||
await page.mouse.down()
|
||||
// Nudge >5px to clear PointerSensor's activationConstraint.distance.
|
||||
await page.mouse.move(sx + 10, sy, { steps: 5 })
|
||||
// Step the pointer across the UI so onDragOver fires on intermediate
|
||||
// droppables and dnd-kit resolves the final insertion index.
|
||||
await page.mouse.move(tx, ty, { steps: 25 })
|
||||
// Why: nudge 1px then back so Playwright fires a fresh pointermove (same-coord
|
||||
// moves are deduplicated). This guarantees one more onDragOver lands after
|
||||
// the stepped move completes, which dnd-kit uses to finalize the insertion
|
||||
// index before drop.
|
||||
await page.mouse.move(tx + 1, ty, { steps: 1 })
|
||||
await page.mouse.move(tx, ty, { steps: 1 })
|
||||
await page.mouse.up()
|
||||
}
|
||||
|
||||
test.describe.configure({ mode: 'serial' })
|
||||
test.describe('Cross-group tab drag', () => {
|
||||
test.beforeEach(async ({ orcaPage }) => {
|
||||
await waitForSessionReady(orcaPage)
|
||||
await waitForActiveWorktree(orcaPage)
|
||||
await ensureTerminalVisible(orcaPage)
|
||||
})
|
||||
|
||||
/**
|
||||
* Drags a tab from one group into another and asserts both groups updated.
|
||||
*
|
||||
* Why two assertions (source shrank, target grew): dnd-kit's drag lifecycle
|
||||
* fires onDragEnd once, but the state transition calls two store actions —
|
||||
* moveUnifiedTabToGroup (updates target) and source-group cleanup. Both
|
||||
* sides must be verified or a regression in either half would pass silently.
|
||||
*/
|
||||
test('@headful drags a tab from one group into another', async ({ orcaPage }) => {
|
||||
const worktreeId = (await getActiveWorktreeId(orcaPage))!
|
||||
|
||||
// Seed three tabs so group A keeps two after we populate group B.
|
||||
// Two in A matters: if we left only one in A after seeding B, the
|
||||
// empty-source-group cleanup path could fire on drop and change the
|
||||
// number of groups — confusing this test's assertions with the
|
||||
// separate cleanup behavior exercised below.
|
||||
await createTerminalTab(orcaPage, worktreeId)
|
||||
await createTerminalTab(orcaPage, worktreeId)
|
||||
await expect
|
||||
.poll(async () => (await getWorktreeTabs(orcaPage, worktreeId)).length, { timeout: 10_000 })
|
||||
.toBeGreaterThanOrEqual(3)
|
||||
|
||||
const targetGroupId = await splitActiveGroupRight(orcaPage, worktreeId)
|
||||
expect(targetGroupId).toBeTruthy()
|
||||
|
||||
const groupsBefore = await getGroupSnapshot(orcaPage, worktreeId)
|
||||
const sourceGroup = groupsBefore.find((g) => g.id !== targetGroupId)!
|
||||
expect(sourceGroup.tabOrder.length).toBeGreaterThanOrEqual(3)
|
||||
const sourceGroupId = sourceGroup.id
|
||||
|
||||
// Take two unified-tab IDs from group A: one we move to B up front
|
||||
// so B's tab bar has a DOM element we can hit with the pointer, and
|
||||
// one we drag across in the real pointer test.
|
||||
const [draggedUnifiedId, seedUnifiedId] = sourceGroup.tabOrder
|
||||
|
||||
await moveTabToGroup(orcaPage, seedUnifiedId, targetGroupId!)
|
||||
await expect
|
||||
.poll(async () => getUnifiedTabGroupId(orcaPage, seedUnifiedId), { timeout: 5_000 })
|
||||
.toBe(targetGroupId)
|
||||
|
||||
// Real pointer drag: drop draggedUnifiedId on top of seedUnifiedId.
|
||||
await dragTabTo(orcaPage, draggedUnifiedId, seedUnifiedId)
|
||||
|
||||
await expect
|
||||
.poll(async () => getUnifiedTabGroupId(orcaPage, draggedUnifiedId), {
|
||||
timeout: 5_000,
|
||||
message: 'Dragged tab did not land in the target group'
|
||||
})
|
||||
.toBe(targetGroupId)
|
||||
|
||||
const groupsAfter = await getGroupSnapshot(orcaPage, worktreeId)
|
||||
const targetAfter = groupsAfter.find((g) => g.id === targetGroupId)
|
||||
const sourceAfter = groupsAfter.find((g) => g.id === sourceGroupId)
|
||||
expect(targetAfter?.tabOrder, 'dragged tab must appear in target group').toContain(
|
||||
draggedUnifiedId
|
||||
)
|
||||
expect(sourceAfter?.tabOrder, 'dragged tab must be removed from source group').not.toContain(
|
||||
draggedUnifiedId
|
||||
)
|
||||
expect(sourceAfter?.tabOrder, 'tabs we did not drag must remain in source').toEqual(
|
||||
sourceGroup.tabOrder.filter((id) => id !== draggedUnifiedId && id !== seedUnifiedId)
|
||||
)
|
||||
})
|
||||
|
||||
/**
|
||||
* User Prompt:
|
||||
* - dragging tabs around to reorder them (via real pointer, not store call)
|
||||
*
|
||||
* Why assert the exact new order (not just "changed"): a reorder bug that
|
||||
* swaps the wrong tabs — or no-ops and returns the prior array in a new
|
||||
* reference — would pass a "not equal to before" check. Asserting the
|
||||
* specific post-drag order catches both classes of regression.
|
||||
*/
|
||||
test('@headful reorders tabs within a group via pointer drag', async ({ orcaPage }) => {
|
||||
const worktreeId = (await getActiveWorktreeId(orcaPage))!
|
||||
|
||||
await createTerminalTab(orcaPage, worktreeId)
|
||||
await expect
|
||||
.poll(async () => (await getWorktreeTabs(orcaPage, worktreeId)).length, { timeout: 10_000 })
|
||||
.toBeGreaterThanOrEqual(2)
|
||||
|
||||
const groupsBefore = await getGroupSnapshot(orcaPage, worktreeId)
|
||||
const group = groupsBefore[0]
|
||||
expect(group.tabOrder.length).toBeGreaterThanOrEqual(2)
|
||||
const [firstUnified, secondUnified] = group.tabOrder
|
||||
const expectedOrder = [secondUnified, firstUnified, ...group.tabOrder.slice(2)]
|
||||
|
||||
// Drag the first tab onto the second tab's center. The dnd-kit insertion
|
||||
// logic uses pointer-vs-midpoint on the hovered tab — landing on its
|
||||
// center + the 25-step move means the final pointer position is past
|
||||
// the midpoint, so the dragged tab is inserted AFTER the second tab
|
||||
// (i.e. the two are swapped).
|
||||
await dragTabTo(orcaPage, firstUnified, secondUnified)
|
||||
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
const after = await getGroupSnapshot(orcaPage, worktreeId)
|
||||
return after.find((g) => g.id === group.id)?.tabOrder ?? []
|
||||
},
|
||||
{ timeout: 5_000, message: 'Tab order did not match expected swap after pointer drag' }
|
||||
)
|
||||
.toEqual(expectedOrder)
|
||||
})
|
||||
|
||||
/**
|
||||
* Exercises the empty-source-group cleanup path: when the last tab is
|
||||
* dragged out of a group, that group is collapsed out of the layout.
|
||||
*
|
||||
* Why this is a distinct test: the cleanup path lives in
|
||||
* maybeCloseEmptySourceGroup inside TabGroupDndContext and runs only
|
||||
* after a cross-group move leaves the source empty. It is separate from
|
||||
* moveUnifiedTabToGroup itself and is easy to regress when either the
|
||||
* DnD context or the store's closeEmptyGroup contract changes.
|
||||
*/
|
||||
test('@headful dragging the last tab out collapses the empty source group', async ({
|
||||
orcaPage
|
||||
}) => {
|
||||
const worktreeId = (await getActiveWorktreeId(orcaPage))!
|
||||
|
||||
// Two tabs total: one stays in group A as the drop target, one gets
|
||||
// moved into group B up front. Then we'll drag the remaining A tab
|
||||
// into B, leaving A empty.
|
||||
await createTerminalTab(orcaPage, worktreeId)
|
||||
await expect
|
||||
.poll(async () => (await getWorktreeTabs(orcaPage, worktreeId)).length, { timeout: 10_000 })
|
||||
.toBeGreaterThanOrEqual(2)
|
||||
|
||||
const targetGroupId = await splitActiveGroupRight(orcaPage, worktreeId)
|
||||
expect(targetGroupId).toBeTruthy()
|
||||
|
||||
const groupsBefore = await getGroupSnapshot(orcaPage, worktreeId)
|
||||
const sourceGroup = groupsBefore.find((g) => g.id !== targetGroupId)!
|
||||
const sourceGroupId = sourceGroup.id
|
||||
expect(sourceGroup.tabOrder.length).toBeGreaterThanOrEqual(2)
|
||||
|
||||
const [lastInSourceUnifiedId, seedUnifiedId] = sourceGroup.tabOrder
|
||||
|
||||
await moveTabToGroup(orcaPage, seedUnifiedId, targetGroupId!)
|
||||
await expect
|
||||
.poll(async () => getUnifiedTabGroupId(orcaPage, seedUnifiedId), { timeout: 5_000 })
|
||||
.toBe(targetGroupId)
|
||||
|
||||
// Confirm group A is now the source's lone-tab state before we drag.
|
||||
const groupsMid = await getGroupSnapshot(orcaPage, worktreeId)
|
||||
expect(
|
||||
groupsMid.find((g) => g.id === sourceGroupId)?.tabOrder,
|
||||
'source must still exist with one tab prior to the final drag'
|
||||
).toEqual([lastInSourceUnifiedId])
|
||||
|
||||
await dragTabTo(orcaPage, lastInSourceUnifiedId, seedUnifiedId)
|
||||
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
const after = await getGroupSnapshot(orcaPage, worktreeId)
|
||||
return after.some((g) => g.id === sourceGroupId)
|
||||
},
|
||||
{
|
||||
timeout: 5_000,
|
||||
message: 'Empty source group was not removed after its last tab was dragged away'
|
||||
}
|
||||
)
|
||||
.toBe(false)
|
||||
|
||||
const groupsAfter = await getGroupSnapshot(orcaPage, worktreeId)
|
||||
expect(groupsAfter).toHaveLength(1)
|
||||
expect(groupsAfter[0].id).toBe(targetGroupId)
|
||||
expect(groupsAfter[0].tabOrder).toContain(lastInSourceUnifiedId)
|
||||
})
|
||||
|
||||
/**
|
||||
* Regression guard: after a tab is dragged across groups, every pane of the
|
||||
* remounted tab must be sized to its container (not the xterm 80×24 default)
|
||||
* and must still show its pre-drag buffer content.
|
||||
*
|
||||
* Why this matters: on remount, use-terminal-pane-lifecycle schedules the
|
||||
* attach work in a rAF. If that callback runs before xterm's renderer
|
||||
* measures cell geometry (common for the non-leftmost pane created via
|
||||
* splitPane after a DOM reparent), fitAddon.proposeDimensions() returns
|
||||
* undefined and fit() is a silent no-op. The PTY then gets resized to the
|
||||
* default 80×24 while its xterm lives inside a much larger container — the
|
||||
* TUI paints into a small subregion and the rest of the pane looks blank.
|
||||
* pty-connection.ts:runAttachWork now retries the rAF until
|
||||
* proposeDimensions() resolves, so cols must exceed 80 and the previously
|
||||
* written sentinel must round-trip through serializeAddon.
|
||||
*
|
||||
* Why both leaves are checked: the original bug only affected the pane that
|
||||
* *was not* the leftmost leaf of the source tree (the one splitPane creates
|
||||
* second). Asserting each leaf individually makes sure the fix covers both
|
||||
* positions — not just the one the rAF happens to win on.
|
||||
*/
|
||||
test('@headful cross-group drag preserves split-pane dimensions and content', async ({
|
||||
orcaPage
|
||||
}) => {
|
||||
const worktreeId = (await getActiveWorktreeId(orcaPage))!
|
||||
|
||||
// Seed three tabs so A keeps at least one after we move the seed into B.
|
||||
// The dragged tab is the one we split and write content into; A still
|
||||
// has the third tab so the source group is not emptied by the drag
|
||||
// (that's a separate regression path).
|
||||
await createTerminalTab(orcaPage, worktreeId)
|
||||
await createTerminalTab(orcaPage, worktreeId)
|
||||
await expect
|
||||
.poll(async () => (await getWorktreeTabs(orcaPage, worktreeId)).length, { timeout: 10_000 })
|
||||
.toBeGreaterThanOrEqual(3)
|
||||
|
||||
const targetGroupId = await splitActiveGroupRight(orcaPage, worktreeId)
|
||||
expect(targetGroupId).toBeTruthy()
|
||||
|
||||
const groupsBefore = await getGroupSnapshot(orcaPage, worktreeId)
|
||||
const sourceGroup = groupsBefore.find((g) => g.id !== targetGroupId)!
|
||||
expect(sourceGroup.tabOrder.length).toBeGreaterThanOrEqual(3)
|
||||
|
||||
const [draggedUnifiedId, seedUnifiedId] = sourceGroup.tabOrder
|
||||
|
||||
// Seed the target group so its tab bar has a drop target.
|
||||
await moveTabToGroup(orcaPage, seedUnifiedId, targetGroupId!)
|
||||
await expect
|
||||
.poll(async () => getUnifiedTabGroupId(orcaPage, seedUnifiedId), { timeout: 5_000 })
|
||||
.toBe(targetGroupId)
|
||||
|
||||
// Resolve the underlying terminal tab id backing the unified tab so we
|
||||
// can address the PaneManager registered in window.__paneManagers.
|
||||
const draggedTerminalTabId = await orcaPage.evaluate((unifiedId) => {
|
||||
const store = window.__store
|
||||
if (!store) {
|
||||
return null
|
||||
}
|
||||
const byWorktree = store.getState().unifiedTabsByWorktree ?? {}
|
||||
for (const tabs of Object.values(byWorktree) as {
|
||||
id: string
|
||||
contentType: string
|
||||
entityId: string
|
||||
}[][]) {
|
||||
const match = tabs.find((t) => t.id === unifiedId && t.contentType === 'terminal')
|
||||
if (match) {
|
||||
return match.entityId
|
||||
}
|
||||
}
|
||||
return null
|
||||
}, draggedUnifiedId)
|
||||
expect(draggedTerminalTabId, 'dragged unified tab must map to a terminal tab').toBeTruthy()
|
||||
|
||||
// Make sure the dragged tab is the active terminal one so its
|
||||
// PaneManager is mounted and we can split it before the drag.
|
||||
await orcaPage.evaluate((tabId) => {
|
||||
const state = window.__store?.getState()
|
||||
state?.setActiveTabType('terminal')
|
||||
state?.setActiveTab(tabId)
|
||||
}, draggedTerminalTabId!)
|
||||
await waitForActiveTerminalManager(orcaPage, 30_000)
|
||||
|
||||
// Split horizontally so we have a top + bottom pane. The bottom pane is
|
||||
// the one most prone to the "default 80×24" bug because splitPane creates
|
||||
// it after the DOM reparent and its flex geometry resolves on a later
|
||||
// tick than the top's.
|
||||
await splitActiveTerminalPane(orcaPage, 'horizontal')
|
||||
await waitForPaneCount(orcaPage, 2, 10_000)
|
||||
|
||||
// Write a sentinel into every pane so we can prove the buffer survives
|
||||
// the remount. `printf` with a newline avoids triggering prompt-eol
|
||||
// quirks; each pane gets a unique token so we can tell them apart.
|
||||
const SENTINEL_PREFIX = `__DRAG_BANNER_${Date.now()}__`
|
||||
const paneSentinels = await orcaPage.evaluate(
|
||||
({ tabId, prefix }) => {
|
||||
const manager = window.__paneManagers?.get(tabId)
|
||||
if (!manager) {
|
||||
throw new Error('no PaneManager for dragged tab')
|
||||
}
|
||||
const panes = manager.getPanes()
|
||||
const assigned: { paneId: number; sentinel: string }[] = []
|
||||
for (const pane of panes) {
|
||||
const sentinel = `${prefix}_P${pane.id}`
|
||||
// Why: write via the pane's xterm directly rather than the PTY.
|
||||
// The regression is "xterm receives repaint data but at the wrong
|
||||
// dimensions"; writing into the xterm buffer isolates the test
|
||||
// from PTY-id routing timing and still forces the scrollback to
|
||||
// round-trip through serializeAddon after remount.
|
||||
pane.terminal.write(`${sentinel}\r\n`)
|
||||
assigned.push({ paneId: pane.id, sentinel })
|
||||
}
|
||||
return assigned
|
||||
},
|
||||
{ tabId: draggedTerminalTabId!, prefix: SENTINEL_PREFIX }
|
||||
)
|
||||
expect(paneSentinels.length).toBe(2)
|
||||
|
||||
// Real pointer drag across groups.
|
||||
await dragTabTo(orcaPage, draggedUnifiedId, seedUnifiedId)
|
||||
await expect
|
||||
.poll(async () => getUnifiedTabGroupId(orcaPage, draggedUnifiedId), {
|
||||
timeout: 5_000,
|
||||
message: 'Dragged tab did not land in the target group'
|
||||
})
|
||||
.toBe(targetGroupId)
|
||||
|
||||
// Reactivate the remounted tab and wait for its new PaneManager to
|
||||
// register — the drag unmounted the old one.
|
||||
await orcaPage.evaluate((tabId) => {
|
||||
const state = window.__store?.getState()
|
||||
state?.setActiveTabType('terminal')
|
||||
state?.setActiveTab(tabId)
|
||||
}, draggedTerminalTabId!)
|
||||
await waitForActiveTerminalManager(orcaPage, 30_000)
|
||||
await waitForPaneCount(orcaPage, 2, 10_000)
|
||||
|
||||
// Poll until every remounted pane has measured geometry (cols > 80
|
||||
// default) and retains its sentinel in the serialized buffer. The
|
||||
// rAF retry loop in runAttachWork spans up to ~500ms, and the
|
||||
// width-jitter in pty-transport.attach fires on a 32ms timeout — use
|
||||
// a generous poll to absorb both before the precise assertions below.
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
return orcaPage.evaluate(
|
||||
({ tabId, sentinelsByPaneId }) => {
|
||||
const manager = window.__paneManagers?.get(tabId)
|
||||
if (!manager) {
|
||||
return false
|
||||
}
|
||||
const panes = manager.getPanes()
|
||||
if (panes.length !== 2) {
|
||||
return false
|
||||
}
|
||||
return panes.every((pane) => {
|
||||
const expected = sentinelsByPaneId[pane.id]
|
||||
if (!expected) {
|
||||
return false
|
||||
}
|
||||
const serialized = pane.serializeAddon?.serialize?.() ?? ''
|
||||
return pane.terminal.cols > 80 && serialized.includes(expected)
|
||||
})
|
||||
},
|
||||
{
|
||||
tabId: draggedTerminalTabId!,
|
||||
sentinelsByPaneId: Object.fromEntries(
|
||||
paneSentinels.map((p) => [p.paneId, p.sentinel])
|
||||
)
|
||||
}
|
||||
)
|
||||
},
|
||||
{
|
||||
timeout: 15_000,
|
||||
message:
|
||||
'Remounted panes never reached measured dimensions with sentinel content after cross-group drag'
|
||||
}
|
||||
)
|
||||
.toBe(true)
|
||||
|
||||
// Now pull the final snapshot and do the precise per-pane assertions.
|
||||
const finalPanes = await orcaPage.evaluate((tabId) => {
|
||||
const manager = window.__paneManagers?.get(tabId)
|
||||
if (!manager) {
|
||||
return []
|
||||
}
|
||||
return manager.getPanes().map((pane) => {
|
||||
const proposed = pane.fitAddon.proposeDimensions?.() ?? null
|
||||
return {
|
||||
paneId: pane.id,
|
||||
cols: pane.terminal.cols,
|
||||
rows: pane.terminal.rows,
|
||||
proposedCols: proposed?.cols ?? null,
|
||||
proposedRows: proposed?.rows ?? null,
|
||||
serialized: pane.serializeAddon?.serialize?.() ?? ''
|
||||
}
|
||||
})
|
||||
}, draggedTerminalTabId!)
|
||||
|
||||
expect(finalPanes).toHaveLength(2)
|
||||
for (const pane of finalPanes) {
|
||||
// Default xterm size is 80x24 — if the attach wrote PTY dimensions
|
||||
// before fitAddon measured cell geometry, we land back at 80 cols and
|
||||
// the TUI paints into the corner. Real terminal windows at E2E
|
||||
// resolution measure much wider than 80 cols.
|
||||
expect(
|
||||
pane.cols,
|
||||
`pane ${pane.paneId} kept xterm default width (likely mis-sized before first render)`
|
||||
).toBeGreaterThan(80)
|
||||
// proposeDimensions is the authoritative "what would fit()" answer
|
||||
// once cell geometry is known. If it disagrees with terminal.cols by
|
||||
// more than 1 (rounding jitter), the fit retry loop never converged.
|
||||
if (pane.proposedCols !== null) {
|
||||
expect(
|
||||
Math.abs(pane.proposedCols - pane.cols),
|
||||
`pane ${pane.paneId} cols (${pane.cols}) disagree with proposeDimensions (${pane.proposedCols})`
|
||||
).toBeLessThanOrEqual(1)
|
||||
}
|
||||
// Every pane must still contain its sentinel. A blank-pane regression
|
||||
// either wipes the buffer on attach or never writes to the correct
|
||||
// xterm region — either failure mode loses the sentinel.
|
||||
const expected = paneSentinels.find((p) => p.paneId === pane.paneId)?.sentinel
|
||||
expect(
|
||||
pane.serialized,
|
||||
`pane ${pane.paneId} lost its own sentinel content after cross-group drag`
|
||||
).toContain(expected!)
|
||||
}
|
||||
})
|
||||
})
|
||||
Loading…
Reference in a new issue