fix(terminal): prevent dead terminal after split via WebGL lifecycle and worktree dedup (#1298)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinwoo Hong
GitHub
parent
be0e3c2fa7
commit
b4f1f0ea6a
|
|
@ -41,6 +41,12 @@ import { toPublicPane } from './pane-public-view'
|
|||
|
||||
export type { PaneManagerOptions, PaneStyleOptions, ManagedPane, DropZone }
|
||||
|
||||
function reattachWebglIfNeeded(pane: ManagedPaneInternal): void {
|
||||
if (pane.gpuRenderingEnabled && !pane.webglAddon && !pane.webglDisabledAfterContextLoss) {
|
||||
attachWebgl(pane)
|
||||
}
|
||||
}
|
||||
|
||||
export class PaneManager {
|
||||
private root: HTMLElement
|
||||
private panes: Map<number, ManagedPaneInternal> = new Map()
|
||||
|
|
@ -60,25 +66,16 @@ export class PaneManager {
|
|||
this.renderingSuspended = options.initialRenderingSuspended === true
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Public API
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
createInitialPane(opts?: { focus?: boolean }): ManagedPane {
|
||||
const pane = this.createPaneInternal()
|
||||
|
||||
// When the pane is the sole child of root (no splits), it must
|
||||
// fill the root container so FitAddon calculates correct dimensions.
|
||||
pane.container.style.width = '100%'
|
||||
pane.container.style.height = '100%'
|
||||
pane.container.style.position = 'relative'
|
||||
pane.container.style.overflow = 'hidden'
|
||||
|
||||
// Place directly into root
|
||||
Object.assign(pane.container.style, {
|
||||
width: '100%',
|
||||
height: '100%',
|
||||
position: 'relative',
|
||||
overflow: 'hidden'
|
||||
})
|
||||
this.root.appendChild(pane.container)
|
||||
|
||||
openTerminal(pane)
|
||||
|
||||
this.activePaneId = pane.id
|
||||
applyPaneOpacity(this.panes.values(), this.activePaneId, this.styleOptions)
|
||||
|
||||
|
|
@ -108,43 +105,40 @@ export class PaneManager {
|
|||
const isVertical = direction === 'vertical'
|
||||
const divider = this.createDividerWrapped(isVertical)
|
||||
|
||||
// Why: wrapInSplit reparents the existing container, which causes the
|
||||
// browser to asynchronously reset scrollTop to 0 during layout. Capture
|
||||
// the scroll state before reparenting so we can restore it after all
|
||||
// layout and reflow have settled.
|
||||
// Why: wrapInSplit reparents the existing container, resetting scrollTop.
|
||||
const scrollState = captureScrollState(existing.terminal)
|
||||
|
||||
// Why: multiple async operations fire after the split (rAFs from
|
||||
// queueResizeAll, WebGL context loss, ResizeObserver 150ms debounce).
|
||||
// Each would independently try to restore scroll, potentially to wrong
|
||||
// positions due to intermediate buffer states. The lock makes safeFit
|
||||
// and fitAllPanesInternal skip their own scroll restoration, leaving
|
||||
// the authoritative restore to the timeout below.
|
||||
// Why: lock prevents safeFit/fitAllPanes from restoring scroll during
|
||||
// the async settle window — scheduleSplitScrollRestore owns the restore.
|
||||
existing.pendingSplitScrollState = scrollState
|
||||
|
||||
// Why: DOM reparenting can silently invalidate a WebGL context without
|
||||
// firing contextlost — Chromium reclaims the oldest context near its
|
||||
// ~8–16 limit. Dispose before the move, reattach in the 200ms timer.
|
||||
const hadWebgl = !!existing.webglAddon
|
||||
disposeWebgl(existing)
|
||||
|
||||
wrapInSplit(existing.container, newPane.container, isVertical, divider, opts)
|
||||
|
||||
openTerminal(newPane)
|
||||
this.activePaneId = newPane.id
|
||||
applyPaneOpacity(this.panes.values(), this.activePaneId, this.styleOptions)
|
||||
this.applyDividerStylesWrapped()
|
||||
applyDividerStyles(this.root, this.styleOptions)
|
||||
newPane.terminal?.focus()
|
||||
updateMultiPaneState(this.getDragCallbacks())
|
||||
// Why: forward the caller's spawn hint so onPaneCreated → connectPanePty
|
||||
// can boot the new PTY in the source pane's live cwd instead of the
|
||||
// worktree root. The hint is synchronous-only: splitPane returns after
|
||||
// onPaneCreated runs, so there is no reason for it to outlive this call.
|
||||
// Why: forward cwd hint so the new PTY spawns in the source pane's cwd.
|
||||
void this.options.onPaneCreated?.(
|
||||
toPublicPane(newPane),
|
||||
opts?.cwd ? { cwd: opts.cwd } : undefined
|
||||
)
|
||||
this.options.onLayoutChanged?.()
|
||||
|
||||
const reattach = hadWebgl ? reattachWebglIfNeeded : undefined
|
||||
scheduleSplitScrollRestore(
|
||||
(id) => this.panes.get(id),
|
||||
existing.id,
|
||||
scrollState,
|
||||
() => this.destroyed
|
||||
() => this.destroyed,
|
||||
reattach
|
||||
)
|
||||
|
||||
return toPublicPane(newPane)
|
||||
|
|
@ -171,13 +165,9 @@ export class PaneManager {
|
|||
paneContainer.remove()
|
||||
}
|
||||
if (this.activePaneId === paneId) {
|
||||
const remaining = Array.from(this.panes.values())
|
||||
if (remaining.length > 0) {
|
||||
this.activePaneId = remaining[0].id
|
||||
remaining[0].terminal.focus()
|
||||
} else {
|
||||
this.activePaneId = null
|
||||
}
|
||||
const next = this.panes.values().next().value as ManagedPaneInternal | undefined
|
||||
this.activePaneId = next?.id ?? null
|
||||
next?.terminal.focus()
|
||||
}
|
||||
applyPaneOpacity(this.panes.values(), this.activePaneId, this.styleOptions)
|
||||
for (const p of this.panes.values()) {
|
||||
|
|
@ -225,14 +215,10 @@ export class PaneManager {
|
|||
setPaneStyleOptions(opts: PaneStyleOptions): void {
|
||||
this.styleOptions = { ...opts }
|
||||
applyPaneOpacity(this.panes.values(), this.activePaneId, this.styleOptions)
|
||||
this.applyDividerStylesWrapped()
|
||||
applyDividerStyles(this.root, this.styleOptions)
|
||||
applyRootBackground(this.root, this.styleOptions)
|
||||
}
|
||||
|
||||
/** Enable or disable programming-ligatures rendering on a single pane.
|
||||
* Called by applyTerminalAppearance whenever the resolved ligatures state
|
||||
* changes, so toggling the setting or switching fonts takes effect on
|
||||
* live panes without restarting. */
|
||||
setPaneLigaturesEnabled(paneId: number, enabled: boolean): void {
|
||||
const pane = this.panes.get(paneId)
|
||||
if (!pane) {
|
||||
|
|
@ -272,25 +258,18 @@ export class PaneManager {
|
|||
this.renderingSuspended = false
|
||||
for (const pane of this.panes.values()) {
|
||||
pane.webglAttachmentDeferred = false
|
||||
if (pane.gpuRenderingEnabled && !pane.webglDisabledAfterContextLoss && !pane.webglAddon) {
|
||||
attachWebgl(pane)
|
||||
// Why: the fitPanes() optimization skips panes whose dimensions are
|
||||
// unchanged (common when a worktree goes hidden→visible at the same
|
||||
// window size). But the fresh WebGL canvas created by attachWebgl()
|
||||
// has no painted content — without an explicit refresh the terminal
|
||||
// appears frozen until something forces a dimension change (e.g. a
|
||||
// split). This mirrors the onContextLoss handler in attachWebgl which
|
||||
// calls the same refresh after falling back to the DOM renderer.
|
||||
reattachWebglIfNeeded(pane)
|
||||
// Why: fresh WebGL canvas has no content — refresh prevents frozen terminal.
|
||||
if (pane.webglAddon) {
|
||||
try {
|
||||
pane.terminal.refresh(0, pane.terminal.rows - 1)
|
||||
} catch {
|
||||
/* ignore — pane may not be fully initialised yet */
|
||||
/* ignore */
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Move a pane from its current position to a new position relative to a target pane. */
|
||||
movePane(sourcePaneId: number, targetPaneId: number, zone: DropZone): void {
|
||||
handlePaneDrop(sourcePaneId, targetPaneId, zone, this.dragState, this.getDragCallbacks())
|
||||
}
|
||||
|
|
@ -305,10 +284,6 @@ export class PaneManager {
|
|||
this.activePaneId = null
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// Internal helpers
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
private createPaneInternal(): ManagedPaneInternal {
|
||||
const id = this.nextPaneId++
|
||||
const pane = createPaneDOM(
|
||||
|
|
@ -316,14 +291,10 @@ export class PaneManager {
|
|||
this.options,
|
||||
this.dragState,
|
||||
this.getDragCallbacks(),
|
||||
// Why: always re-focus even if already active — after splits the
|
||||
// browser's real textarea focus can lag the manager's activePaneId.
|
||||
(paneId) => {
|
||||
if (!this.destroyed) {
|
||||
// Why: split-pane layout/focus callbacks can leave the manager's
|
||||
// activePaneId temporarily in sync while the browser's real focused
|
||||
// xterm textarea is still on a different pane. Clicking a pane must
|
||||
// always re-focus its terminal, even if the manager already thinks
|
||||
// that pane is active; otherwise input can keep going to the wrong
|
||||
// split after vertical/horizontal splits.
|
||||
this.setActivePane(paneId, { focus: true })
|
||||
}
|
||||
},
|
||||
|
|
@ -336,16 +307,6 @@ export class PaneManager {
|
|||
return pane
|
||||
}
|
||||
|
||||
/**
|
||||
* Focus-follows-mouse entry point. Collects gate inputs from the manager
|
||||
* and delegates to the pure gate helper.
|
||||
*
|
||||
* Invariant for future contributors: modal overlays (context menus, close
|
||||
* dialogs, command palette) must be rendered as portals/siblings OUTSIDE
|
||||
* the pane container. If a future overlay is ever rendered inside a .pane
|
||||
* element, mouseenter will still fire on the pane underneath and this
|
||||
* handler will incorrectly switch focus. Keep overlays out of the pane.
|
||||
*/
|
||||
private handlePaneMouseEnter(paneId: number, event: MouseEvent): void {
|
||||
if (
|
||||
shouldFollowMouseFocus({
|
||||
|
|
@ -368,11 +329,6 @@ export class PaneManager {
|
|||
})
|
||||
}
|
||||
|
||||
private applyDividerStylesWrapped(): void {
|
||||
applyDividerStyles(this.root, this.styleOptions)
|
||||
}
|
||||
|
||||
/** Build the callbacks object for drag-reorder functions. */
|
||||
private getDragCallbacks() {
|
||||
return {
|
||||
getPanes: () => this.panes,
|
||||
|
|
@ -382,7 +338,7 @@ export class PaneManager {
|
|||
safeFit: (pane: ManagedPaneInternal) => safeFit(pane),
|
||||
applyPaneOpacity: () =>
|
||||
applyPaneOpacity(this.panes.values(), this.activePaneId, this.styleOptions),
|
||||
applyDividerStyles: () => this.applyDividerStylesWrapped(),
|
||||
applyDividerStyles: () => applyDividerStyles(this.root, this.styleOptions),
|
||||
refitPanesUnder: (el: HTMLElement) => refitPanesUnder(el, this.panes),
|
||||
onLayoutChanged: this.options.onLayoutChanged
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,13 +1,6 @@
|
|||
import type { ManagedPaneInternal, ScrollState } from './pane-manager-types'
|
||||
import { restoreScrollState } from './pane-scroll'
|
||||
|
||||
// Why: wrapInSplit reparents the existing pane's container, which briefly
|
||||
// detaches the WebGL canvas from the DOM. The WebGL renderer's internal
|
||||
// render state can become stale after the re-attachment, leaving the canvas
|
||||
// blank even though the terminal buffer has data. This mirrors the explicit
|
||||
// refresh in resumeRendering() (pane-manager.ts) and the onContextLoss
|
||||
// handler (pane-lifecycle.ts) which address the same "frozen terminal"
|
||||
// symptom for analogous WebGL state transitions.
|
||||
function refreshAfterReparent(pane: ManagedPaneInternal): void {
|
||||
try {
|
||||
pane.terminal.refresh(0, pane.terminal.rows - 1)
|
||||
|
|
@ -16,15 +9,79 @@ function refreshAfterReparent(pane: ManagedPaneInternal): void {
|
|||
}
|
||||
}
|
||||
|
||||
function logPaneHealth(pane: ManagedPaneInternal, phase: string): void {
|
||||
const canvases = pane.container.querySelectorAll('canvas')
|
||||
const canvasInfo = Array.from(canvases).map((c) => {
|
||||
const gl = c.getContext('webgl2') ?? c.getContext('webgl')
|
||||
return {
|
||||
w: c.width,
|
||||
h: c.height,
|
||||
inDOM: c.isConnected,
|
||||
ctxLost: gl ? gl.isContextLost() : 'no-ctx'
|
||||
}
|
||||
})
|
||||
const content = pane.serializeAddon?.serialize?.() ?? ''
|
||||
// oxlint-disable-next-line no-control-regex
|
||||
const stripped = content.replace(/[\s\x00-\x1f]/g, '')
|
||||
const info = {
|
||||
phase,
|
||||
paneId: pane.id,
|
||||
webgl: !!pane.webglAddon,
|
||||
webglDeferred: pane.webglAttachmentDeferred,
|
||||
webglDisabled: pane.webglDisabledAfterContextLoss,
|
||||
canvases: canvasInfo,
|
||||
contentLen: stripped.length,
|
||||
bufferLines: pane.terminal.buffer.active.length
|
||||
}
|
||||
const hasBufferData = pane.terminal.buffer.active.length > pane.terminal.rows
|
||||
if (stripped.length === 0 && hasBufferData) {
|
||||
console.error(
|
||||
'[split-diag] DEAD TERMINAL — pane',
|
||||
pane.id,
|
||||
pane.debugLabel ?? '',
|
||||
'has buffer data but no rendered content at',
|
||||
phase,
|
||||
info
|
||||
)
|
||||
} else if (stripped.length === 0) {
|
||||
console.log(
|
||||
'[split-diag] pane',
|
||||
pane.id,
|
||||
pane.debugLabel ?? '',
|
||||
'no content yet at',
|
||||
phase,
|
||||
'(PTY likely still spawning)'
|
||||
)
|
||||
} else {
|
||||
console.log(
|
||||
'[split-diag] pane',
|
||||
pane.id,
|
||||
pane.debugLabel ?? '',
|
||||
'healthy at',
|
||||
phase,
|
||||
'— content:',
|
||||
stripped.length
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// Why: reparenting a terminal container during split resets the viewport
|
||||
// scroll position (browser clears scrollTop on DOM move). This schedules a
|
||||
// two-phase restore: an early double-rAF (~32ms) to minimise the visible
|
||||
// flash, plus a 200ms authoritative restore that also clears the scroll lock.
|
||||
//
|
||||
// The optional reattachWebgl callback re-creates the WebGL addon after the
|
||||
// DOM has settled. splitPane() disposes WebGL before wrapInSplit() to free
|
||||
// the GPU context slot (Chromium silently kills the oldest context when
|
||||
// approaching its limit without firing contextlost). Reattaching at 200ms
|
||||
// — after all layout and reflow have completed — creates a fresh context on
|
||||
// a stable DOM tree.
|
||||
export function scheduleSplitScrollRestore(
|
||||
getPaneById: (id: number) => ManagedPaneInternal | undefined,
|
||||
paneId: number,
|
||||
scrollState: ScrollState,
|
||||
isDestroyed: () => boolean
|
||||
isDestroyed: () => boolean,
|
||||
reattachWebgl?: (pane: ManagedPaneInternal) => void
|
||||
): void {
|
||||
requestAnimationFrame(() => {
|
||||
requestAnimationFrame(() => {
|
||||
|
|
@ -48,7 +105,21 @@ export function scheduleSplitScrollRestore(
|
|||
return
|
||||
}
|
||||
live.pendingSplitScrollState = null
|
||||
if (reattachWebgl) {
|
||||
reattachWebgl(live)
|
||||
}
|
||||
restoreScrollState(live.terminal, scrollState)
|
||||
refreshAfterReparent(live)
|
||||
}, 200)
|
||||
|
||||
setTimeout(() => {
|
||||
if (isDestroyed()) {
|
||||
return
|
||||
}
|
||||
const live = getPaneById(paneId)
|
||||
// Skip suspended panes — they have no WebGL/content by design.
|
||||
if (live && !live.webglAttachmentDeferred) {
|
||||
logPaneHealth(live, '1s-health-check')
|
||||
}
|
||||
}, 1000)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import type { DropZone, ManagedPaneInternal, PaneStyleOptions } from './pane-manager-types'
|
||||
import { createDivider } from './pane-divider'
|
||||
import { disposeWebgl, attachWebgl } from './pane-lifecycle'
|
||||
|
||||
export { findLineByContent, captureScrollState, restoreScrollState } from './pane-scroll'
|
||||
|
||||
|
|
@ -40,29 +41,6 @@ export function safeFit(pane: ManagedPaneInternal): void {
|
|||
// Why: divider drags fire refits every frame, but most frames do not
|
||||
// cross a cell boundary. Skipping those avoids FitAddon.clear()+refresh()
|
||||
// churn, which was causing visible terminal blinking while resizing.
|
||||
//
|
||||
// Why: wrapInSplit() reparents the pane's container, which can leave
|
||||
// the WebGL canvas stale even when proposed dimensions match current
|
||||
// (the browser detaches and reattaches the canvas during the DOM move).
|
||||
// When pendingSplitScrollState is set we must force a fit + refresh so
|
||||
// the WebGL renderer repaints. Without this, the pane appears blank
|
||||
// until something forces a dimension change.
|
||||
if (pane.pendingSplitScrollState) {
|
||||
console.warn(
|
||||
'[terminal] safeFit forcing fit+refresh during pending split for pane',
|
||||
pane.id,
|
||||
`— dims ${dims.cols}×${dims.rows} match current, webgl:`,
|
||||
!!pane.webglAddon,
|
||||
pane.debugLabel ? `(${pane.debugLabel})` : ''
|
||||
)
|
||||
pane.fitAddon.fit()
|
||||
try {
|
||||
pane.terminal.refresh(0, pane.terminal.rows - 1)
|
||||
} catch {
|
||||
/* ignore — terminal may not be fully initialised */
|
||||
}
|
||||
return
|
||||
}
|
||||
return
|
||||
}
|
||||
pane.fitAddon.fit()
|
||||
|
|
@ -183,6 +161,13 @@ export function insertPaneNextTo(
|
|||
applyPaneFlexStyle(source.container)
|
||||
applyPaneFlexStyle(targetContainer)
|
||||
|
||||
// Why: same pattern as splitPane — dispose WebGL before the DOM reparent
|
||||
// to free GPU context slots, then reattach after layout settles.
|
||||
const sourceHadWebgl = !!source.webglAddon
|
||||
const targetHadWebgl = !!target.webglAddon
|
||||
disposeWebgl(source)
|
||||
disposeWebgl(target)
|
||||
|
||||
// Replace target with the split in the DOM
|
||||
parent.replaceChild(split, targetContainer)
|
||||
|
||||
|
|
@ -197,23 +182,15 @@ export function insertPaneNextTo(
|
|||
split.appendChild(source.container)
|
||||
}
|
||||
|
||||
// Refit both and refresh rendering surfaces — both panes were reparented
|
||||
// into the new split wrapper, which can leave the WebGL canvas in a stale
|
||||
// state (same mechanism as wrapInSplit; see refreshAfterReparent in
|
||||
// pane-split-scroll.ts).
|
||||
requestAnimationFrame(() => {
|
||||
if (sourceHadWebgl && source.gpuRenderingEnabled && !source.webglDisabledAfterContextLoss) {
|
||||
attachWebgl(source)
|
||||
}
|
||||
if (targetHadWebgl && target.gpuRenderingEnabled && !target.webglDisabledAfterContextLoss) {
|
||||
attachWebgl(target)
|
||||
}
|
||||
callbacks.safeFit(source)
|
||||
callbacks.safeFit(target)
|
||||
try {
|
||||
source.terminal.refresh(0, source.terminal.rows - 1)
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
try {
|
||||
target.terminal.refresh(0, target.terminal.rows - 1)
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -23,11 +23,15 @@ function getWorktreeSnapshot(worktreesByRepo: AppState['worktreesByRepo']): Work
|
|||
return cachedSnapshot
|
||||
}
|
||||
|
||||
const allWorktrees = Object.values(worktreesByRepo).flat()
|
||||
// Why: a race between createWorktree (which appends) and fetchWorktrees
|
||||
// (which replaces) can produce duplicate entries for the same worktree ID
|
||||
// within a single repo's array. Deduplicating here prevents React from
|
||||
// seeing duplicate keys, which can corrupt terminal DOM containers.
|
||||
const worktreeMap = new Map<string, Worktree>()
|
||||
for (const worktree of allWorktrees) {
|
||||
for (const worktree of Object.values(worktreesByRepo).flat()) {
|
||||
worktreeMap.set(worktree.id, worktree)
|
||||
}
|
||||
const allWorktrees = Array.from(worktreeMap.values())
|
||||
|
||||
const snapshot = { allWorktrees, worktreeMap }
|
||||
worktreeSnapshotCache.set(worktreesByRepo, snapshot)
|
||||
|
|
|
|||
|
|
@ -105,13 +105,22 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
|
|||
baseBranch,
|
||||
setupDecision
|
||||
})
|
||||
set((s) => ({
|
||||
worktreesByRepo: {
|
||||
...s.worktreesByRepo,
|
||||
[repoId]: [...(s.worktreesByRepo[repoId] ?? []), result.worktree]
|
||||
},
|
||||
sortEpoch: s.sortEpoch + 1
|
||||
}))
|
||||
// Why: a file watcher (worktrees.onChanged) can fire between the
|
||||
// backend creating the worktree and this callback running, causing
|
||||
// fetchWorktrees to add the worktree first. Appending unconditionally
|
||||
// then produces a duplicate entry in worktreesByRepo, which gives
|
||||
// React duplicate keys and can corrupt terminal DOM containers.
|
||||
set((s) => {
|
||||
const current = s.worktreesByRepo[repoId] ?? []
|
||||
const alreadyPresent = current.some((w) => w.id === result.worktree.id)
|
||||
return {
|
||||
worktreesByRepo: {
|
||||
...s.worktreesByRepo,
|
||||
[repoId]: alreadyPresent ? current : [...current, result.worktree]
|
||||
},
|
||||
sortEpoch: s.sortEpoch + 1
|
||||
}
|
||||
})
|
||||
return result
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : String(error)
|
||||
|
|
|
|||
Loading…
Reference in New Issue