diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index 25f7522d..1de6af0b 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -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 = {} - 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 = {} + 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] ?? '') diff --git a/src/renderer/src/components/terminal-pane/layout-serialization.ts b/src/renderer/src/components/terminal-pane/layout-serialization.ts index 10e2fec4..4afa1952 100644 --- a/src/renderer/src/components/terminal-pane/layout-serialization.ts +++ b/src/renderer/src/components/terminal-pane/layout-serialization.ts @@ -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 | undefined, - restoredPaneByLeafId: Map, - options: { shouldPreserveAltScreenForLeaf?: (leafId: string) => boolean } = {} + restoredPaneByLeafId: Map ): 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. diff --git a/src/renderer/src/components/terminal-pane/pty-connection-types.ts b/src/renderer/src/components/terminal-pane/pty-connection-types.ts index c67b2f50..e40d5b5f 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-types.ts @@ -7,11 +7,6 @@ export type PtyConnectionDeps = { startup?: { command: string; env?: Record } | null restoredLeafId?: string | null restoredPtyIdByLeafId?: Record - /** 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 paneTransportsRef: React.RefObject> pendingWritesRef: React.RefObject> isActiveRef: React.RefObject diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 6543dbe8..a414510e 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -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() } } } diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index b5d896ff..336f0c1a 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -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() { diff --git a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts index 7cf46ee0..4e1210bf 100644 --- a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts +++ b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts @@ -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 diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index 6c98774d..27d45d31 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -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 | 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 | 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() } } diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index dd279751..b6eb5494 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -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>> paneTitlesRef: React.RefObject> setRenamingPaneId: React.Dispatch> @@ -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) diff --git a/tests/e2e/cross-group-tab-drag.spec.ts b/tests/e2e/cross-group-tab-drag.spec.ts deleted file mode 100644 index aaf17284..00000000 --- a/tests/e2e/cross-group-tab-drag.spec.ts +++ /dev/null @@ -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 { - 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 { - 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 { - 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 { - 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 { - 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 { - 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!) - } - }) -})