fix(terminal): bound the deferred paste by wall clock, refuse partial replays, and stop hidden panes claiming the chord

Adversarial review of STA-5272:

- A chunked paste re-checks focus between chunks, so `stale-target` can arrive
  with bytes already in the PTY. Deferring that and replaying the whole payload
  duplicated everything written before the cancel. `chunksWritten` is now a
  required conjunct of isDeferrablePasteFocusCancellation.
- Electron leaves backgroundThrottling on, so a hidden renderer's 2s deadline
  timer is aligned into 1s (then 1-minute) buckets. Restoring the window fires
  focusin before the overdue timer, landing the paste minutes later. The wall
  clock now bounds the payload; the timer only notifies.
- A pane can stop being rendered while isActive stays true (closed floating
  panel, display:none background surface). The browser blurs it to <body> with
  no focusin, so paneOwnsFocus stays stale and the document-capture listener
  swallowed the chord for the whole app. Ownership now requires a rendered
  container, and the cheap checks short-circuit first on the keystroke path.
- A deferred payload whose pane was closed is released on the next focusin
  instead of being retained to the deadline.
This commit is contained in:
Brennan Benson
2026-08-28 06:01:42 -07:00
parent 487a0489d4
commit 0f67f57cce
5 changed files with 390 additions and 8 deletions
@@ -2042,6 +2042,7 @@ function TerminalPane(
isDeferrablePasteFocusCancellation({
status: execution.status,
reason: execution.reason,
chunksWritten: execution.chunksWritten,
targetMounted: isPanePasteTargetMounted(pane, transport, ptyId),
focusMovedToOtherPane: isFocusInsideOtherPane({
panes: managerRef.current?.getPanes() ?? [],
@@ -1,6 +1,7 @@
// @vitest-environment happy-dom
import { describe, expect, it, vi, type Mock } from 'vitest'
import { executeTerminalPastePlan, planTerminalPaste } from './terminal-paste-coordinator'
import {
createDeferredPasteFocusInHandler,
createDeferredTerminalPasteQueue,
@@ -144,6 +145,46 @@ describe('deferred terminal paste queue', () => {
)
})
// STA-5272 review: Electron leaves `backgroundThrottling` on, so a hidden
// renderer's timers are aligned to 1s and, after 5 minutes hidden, to 1-minute
// buckets. The deadline timer can therefore still be pending long past 2s while
// restoring the window fires focusin first — landing the paste minutes later.
// The wall clock, not the timer, has to be what bounds the payload.
it('refuses a claim made after the deadline even if the timer has not fired yet', () => {
const timers = createTimerHarness()
const onExpire = vi.fn()
let nowMs = 1_000
const queue = createDeferredTerminalPasteQueue({
onExpire,
now: () => nowMs,
...timers
})
queue.defer(entry())
// The window was hidden; the timer is still queued, unfired.
nowMs += TERMINAL_DEFERRED_PASTE_TIMEOUT_MS + 1
expect(timers.live()).toHaveLength(1)
expect(queue.claim(1, 'leaf-a')).toBe(null)
// and the user is told rather than silently robbed of the paste
expect(onExpire).toHaveBeenCalledTimes(1)
expect(queue.isPending()).toBe(false)
expect(timers.live()).toEqual([])
})
it('still honours a claim made inside the deadline', () => {
const timers = createTimerHarness()
const onExpire = vi.fn()
let nowMs = 1_000
const queue = createDeferredTerminalPasteQueue({ onExpire, now: () => nowMs, ...timers })
queue.defer(entry())
nowMs += TERMINAL_DEFERRED_PASTE_TIMEOUT_MS - 1
expect(queue.claim(1, 'leaf-a')).toEqual(entry())
expect(onExpire).not.toHaveBeenCalled()
})
it('drops the payload on dispose without firing the expiry signal', () => {
const timers = createTimerHarness()
const onExpire = vi.fn()
@@ -162,6 +203,7 @@ describe('isDeferrablePasteFocusCancellation', () => {
const base = {
status: 'cancelled' as const,
reason: 'stale-target' as const,
chunksWritten: 0,
targetMounted: true,
focusMovedToOtherPane: false
}
@@ -179,6 +221,16 @@ describe('isDeferrablePasteFocusCancellation', () => {
expect(isDeferrablePasteFocusCancellation({ ...base, targetMounted: false })).toBe(false)
})
// Ablation gap: every other case here is already excluded by the reason check,
// so the status conjunct needs a reason it would otherwise let through. Today
// the executor only pairs 'rejected' with 'payload-too-large' (see
// terminal-paste-coordinator planning), so this pins the contract, not a
// currently reachable pairing.
it('refuses to defer anything that is not a cancellation, whatever the reason says', () => {
expect(isDeferrablePasteFocusCancellation({ ...base, status: 'rejected' })).toBe(false)
expect(isDeferrablePasteFocusCancellation({ ...base, status: 'pasted' })).toBe(false)
})
it('refuses to defer non-focus cancellations and rejections', () => {
expect(isDeferrablePasteFocusCancellation({ ...base, reason: 'target-disconnected' })).toBe(
false
@@ -410,3 +462,181 @@ describe('deferred paste focusin handler', () => {
expect(scene.onDropped).not.toHaveBeenCalled()
})
})
// STA-5272 review: `stale-target` is not only a pre-write verdict. The chunked
// writer re-checks the target between chunks, so a focus move part-way through a
// large paste cancels with bytes already in the PTY. Deferring that payload and
// replaying it whole duplicates everything written before the cancel.
describe('partially written pastes are not deferrable', () => {
const chunkedPlan = (text: string) =>
planTerminalPaste({
text,
source: 'keyboard',
target: {
kind: 'terminal',
paneId: 1,
leafId: 'leaf-a',
ptyId: 'pty-1',
runtime: { platform: 'darwin', runtimeKey: 'local:darwin', kind: 'local' }
},
maxDirectBytes: 4,
maxChunkBytes: 8
})
it('premise: the real executor cancels mid-write with bytes already in the pty', async () => {
const writes: string[] = []
let focusHeld = true
const execution = await executeTerminalPastePlan(chunkedPlan('abcdefgh12345678zzzzzzzz'), {
pasteText: () => {},
writePty: (data) => {
writes.push(data)
return true
},
// Focus leaves the pane after the first chunk lands, exactly as a
// dictation/overlay handoff does mid-paste.
isTargetCurrent: () => {
if (writes.length >= 1) {
focusHeld = false
}
return focusHeld
}
})
expect(execution.status).toBe('cancelled')
expect(execution.reason).toBe('stale-target')
expect(execution.chunksWritten).toBeGreaterThan(0)
expect(writes.join('')).not.toBe('abcdefgh12345678zzzzzzzz')
})
it('refuses to defer a cancellation that already wrote bytes to the pty', () => {
expect(
isDeferrablePasteFocusCancellation({
status: 'cancelled',
reason: 'stale-target',
chunksWritten: 3,
targetMounted: true,
focusMovedToOtherPane: false
})
).toBe(false)
})
it('still defers a cancellation that wrote nothing', () => {
expect(
isDeferrablePasteFocusCancellation({
status: 'cancelled',
reason: 'stale-target',
chunksWritten: 0,
targetMounted: true,
focusMovedToOtherPane: false
})
).toBe(true)
})
})
// STA-5272 review: the pane the payload was aimed at can be closed while the
// payload waits. Nothing may be written anywhere, and the user has to be told
// now rather than after the full deadline, because the text is still retained.
describe('a deferred paste whose pane is destroyed before focus returns', () => {
const buildScene = (): {
panes: { id: number; leafId: string; container: HTMLElement }[]
ptyWrites: string[]
queue: ReturnType<typeof createDeferredTerminalPasteQueue>
handler: () => void
onDropped: Mock<(deferred: DeferredTerminalPaste) => void>
onExpire: Mock<(deferred: DeferredTerminalPaste) => void>
outside: HTMLInputElement
} => {
document.body.innerHTML = ''
const root = document.createElement('div')
document.body.append(root)
const panes = [0, 1].map((index) => {
const container = document.createElement('div')
const helper = document.createElement('textarea')
helper.className = 'xterm-helper-textarea'
container.append(helper)
root.append(container)
return { id: index + 1, leafId: `leaf-${index}`, container }
})
const outside = document.createElement('input')
document.body.append(outside)
const ptyWrites: string[] = []
const onExpire = vi.fn<(deferred: DeferredTerminalPaste) => void>()
const queue = createDeferredTerminalPasteQueue({ onExpire, ...createTimerHarness() })
const onDropped = vi.fn<(deferred: DeferredTerminalPaste) => void>()
const handler = createDeferredPasteFocusInHandler({
queue,
getPanes: () => panes,
getFocusedElement: () => document.activeElement,
// The only thing delivery does in production is run the paste, which ends
// in a PTY write; a spy that records nothing would hide a real write.
deliver: (_pane, deferred) => ptyWrites.push(deferred.text),
onDropped
})
return { panes, ptyWrites, queue, handler, onDropped, onExpire, outside }
}
it('drops the payload and tells the user when focus lands outside every pane', () => {
const scene = buildScene()
scene.queue.defer(entry({ paneId: 1, leafId: 'leaf-0', text: 'secret token' }))
// The pane is closed: it is gone from the manager and out of the DOM.
scene.panes[0]!.container.remove()
scene.panes.splice(0, 1)
scene.outside.focus()
scene.handler()
expect(scene.ptyWrites).toEqual([])
expect(scene.onDropped).toHaveBeenCalledTimes(1)
expect(scene.onDropped.mock.calls[0]?.[0]?.text).toBe('secret token')
// Retention: the clipboard text is released with the pane, not held to the deadline.
expect(scene.queue.isPending()).toBe(false)
expect(scene.onExpire).not.toHaveBeenCalled()
})
it('drops the payload and tells the user when focus lands in a surviving sibling', () => {
const scene = buildScene()
scene.queue.defer(entry({ paneId: 1, leafId: 'leaf-0' }))
scene.panes[0]!.container.remove()
scene.panes.splice(0, 1)
;(scene.panes[0]!.container.firstElementChild as HTMLElement).focus()
scene.handler()
expect(scene.ptyWrites).toEqual([])
expect(scene.onDropped).toHaveBeenCalledTimes(1)
expect(scene.queue.isPending()).toBe(false)
})
it('leaves the payload alone while the pane manager reports no panes at all', () => {
// A transiently empty manager during a re-render is not evidence the target
// pane was closed; treating it as one would drop a healthy deferred paste.
const scene = buildScene()
scene.queue.defer(entry({ paneId: 1, leafId: 'leaf-0' }))
const panesSnapshot = [...scene.panes]
scene.panes.length = 0
scene.outside.focus()
scene.handler()
expect(scene.onDropped).not.toHaveBeenCalled()
expect(scene.queue.isPending()).toBe(true)
// and it still lands once the manager reports the pane again
scene.panes.push(...panesSnapshot)
;(scene.panes[0]!.container.firstElementChild as HTMLElement).focus()
scene.handler()
expect(scene.ptyWrites).toEqual(['dictated paragraph'])
})
it('keeps a live pane pending so the destroy check cannot swallow a normal defer', () => {
const scene = buildScene()
scene.queue.defer(entry({ paneId: 1, leafId: 'leaf-0' }))
scene.outside.focus()
scene.handler()
expect(scene.onDropped).not.toHaveBeenCalled()
expect(scene.queue.isPending()).toBe(true)
})
})
@@ -25,6 +25,9 @@ export type DeferredTerminalPasteQueue = {
/** Drops the payload without firing the expiry callback; returns what was dropped. */
cancel: () => DeferredTerminalPaste | null
isPending: () => boolean
/** Identity of the pending payload, never its text, so callers can check the
* target is still alive without a second copy of the clipboard escaping. */
pendingTarget: () => { paneId: number; leafId: string } | null
dispose: () => void
}
@@ -33,16 +36,19 @@ type CreateDeferredTerminalPasteQueueArgs = {
timeoutMs?: number
setTimer?: (callback: () => void, ms: number) => number
clearTimer?: (timerId: number) => void
now?: () => number
}
export function createDeferredTerminalPasteQueue({
onExpire,
timeoutMs = TERMINAL_DEFERRED_PASTE_TIMEOUT_MS,
setTimer = (callback, ms) => window.setTimeout(callback, ms),
clearTimer = (timerId) => window.clearTimeout(timerId)
clearTimer = (timerId) => window.clearTimeout(timerId),
now = () => Date.now()
}: CreateDeferredTerminalPasteQueueArgs): DeferredTerminalPasteQueue {
let pending: DeferredTerminalPaste | null = null
let timerId: number | null = null
let deadlineAtMs = 0
const take = (): DeferredTerminalPaste | null => {
const taken = pending
@@ -54,10 +60,25 @@ export function createDeferredTerminalPasteQueue({
return taken
}
// Why: Electron leaves backgroundThrottling on, so a hidden renderer's timers
// are aligned into 1s (then 1-minute) buckets. The wall clock is what bounds
// the payload; the timer is only how the user gets told promptly.
const releaseIfExpired = (): boolean => {
if (!pending || now() < deadlineAtMs) {
return false
}
const expired = take()
if (expired) {
onExpire(expired)
}
return true
}
return {
defer: (entry) => {
take()
pending = entry
deadlineAtMs = now() + timeoutMs
timerId = setTimer(() => {
timerId = null
// Why: release the clipboard text before notifying, so a throwing
@@ -70,13 +91,23 @@ export function createDeferredTerminalPasteQueue({
}, timeoutMs)
},
claim: (paneId, leafId) => {
if (releaseIfExpired()) {
return null
}
if (!pending || pending.paneId !== paneId || pending.leafId !== leafId) {
return null
}
return take()
},
cancel: () => take(),
isPending: () => pending !== null,
isPending: () => {
releaseIfExpired()
return pending !== null
},
pendingTarget: () => {
releaseIfExpired()
return pending ? { paneId: pending.paneId, leafId: pending.leafId } : null
},
dispose: () => {
take()
}
@@ -84,20 +115,29 @@ export function createDeferredTerminalPasteQueue({
}
/** A paste the focus guard stopped is only deferrable while its pane is still the
* live target and no other pane has taken focus — the case the guard exists for. */
* live target and no other pane has taken focus — the case the guard exists for.
* `chunksWritten` is load-bearing: the chunked writer re-checks focus between
* chunks, so a cancel can arrive with bytes already in the PTY, and redelivering
* the whole payload would duplicate everything written before the cancel. */
export function isDeferrablePasteFocusCancellation({
status,
reason,
chunksWritten,
targetMounted,
focusMovedToOtherPane
}: {
status: TerminalPasteExecutionResult['status']
reason: TerminalPasteExecutionReason | undefined
chunksWritten: number
targetMounted: boolean
focusMovedToOtherPane: boolean
}): boolean {
return (
status === 'cancelled' && reason === 'stale-target' && targetMounted && !focusMovedToOtherPane
status === 'cancelled' &&
reason === 'stale-target' &&
chunksWritten === 0 &&
targetMounted &&
!focusMovedToOtherPane
)
}
@@ -110,7 +150,7 @@ type DeferredPasteFocusPane = {
export type DeferredPasteFocusResolution<TPane extends DeferredPasteFocusPane> =
| { action: 'ignore' }
| { action: 'deliver'; pane: TPane; entry: DeferredTerminalPaste }
| { action: 'drop'; pane: TPane; entry: DeferredTerminalPaste | null }
| { action: 'drop'; pane: TPane | null; entry: DeferredTerminalPaste | null }
/** Focus landing back inside the deferred pane delivers it; focus landing in a
* different pane drops it, because that is the wrong-target case the guard protects. */
@@ -123,10 +163,20 @@ export function resolveDeferredPasteFocusIn<TPane extends DeferredPasteFocusPane
focusedElement: Node | null
queue: DeferredTerminalPasteQueue
}): DeferredPasteFocusResolution<TPane> {
if (!queue.isPending() || !focusedElement) {
const target = queue.pendingTarget()
if (!target || !focusedElement) {
return { action: 'ignore' }
}
const pane = panes.find((candidate) => candidate.container.contains(focusedElement))
// Why: the target pane was closed while the payload waited. Release the
// clipboard text now rather than retaining it for a pane that cannot receive
// it; `panes.length` guards a transiently empty manager during a re-render.
const targetGone =
panes.length > 0 &&
!panes.some((candidate) => candidate.id === target.paneId && candidate.leafId === target.leafId)
if (targetGone) {
return { action: 'drop', pane: pane ?? null, entry: queue.cancel() }
}
if (!pane) {
return { action: 'ignore' }
}
@@ -178,6 +178,80 @@ describe('inert-focus paste fallback', () => {
expect(fallback.ownsInertFocus()).toBe(false)
})
// STA-5272 review: the pane can stop being rendered while `isActive` stays
// true (a CSS-hidden floating panel, a display:none background surface). The
// browser blurs its focus to <body> WITHOUT a focusin, so `paneOwnsFocus`
// stays stale. These listeners sit at the document in capture phase, so a
// stale claim swallows the chord (preventDefault + stopPropagation) for the
// whole app and routes the paste into an invisible terminal.
it('does not claim inert focus once the pane is no longer rendered', () => {
const fallback = install()
inside.focus()
inside.blur()
expect(fallback.ownsInertFocus()).toBe(true)
// Exactly how the closed floating terminal panel hides its subtree.
container.style.visibility = 'hidden'
expect(fallback.ownsInertFocus()).toBe(false)
})
it('does not swallow the paste chord for the rest of the app while hidden', () => {
install()
inside.focus()
inside.blur()
container.style.visibility = 'hidden'
pressPasteChordOn(document.body)
document.body.dispatchEvent(new Event('paste', { bubbles: true, cancelable: true }))
expect(onPasteKey).not.toHaveBeenCalled()
expect(onPasteEvent).not.toHaveBeenCalled()
})
it('does not claim inert focus after its container leaves the document', () => {
const fallback = install()
inside.focus()
inside.blur()
container.remove()
expect(fallback.ownsInertFocus()).toBe(false)
pressPasteChordOn(document.body)
expect(onPasteKey).not.toHaveBeenCalled()
})
it('honours the browser visibility check that covers display:none subtrees', () => {
// happy-dom has no checkVisibility; production Chromium does, and it is the
// only check that sees a display:none ancestor. Pin that we call it.
const calls: unknown[] = []
;(container as HTMLElement & { checkVisibility?: (o?: unknown) => boolean }).checkVisibility = (
options
) => {
calls.push(options)
return false
}
const fallback = install()
inside.focus()
inside.blur()
expect(fallback.ownsInertFocus()).toBe(false)
expect(calls.length).toBeGreaterThan(0)
pressPasteChordOn(document.body)
expect(onPasteKey).not.toHaveBeenCalled()
})
it('still recovers when the browser reports the container as visible', () => {
;(container as HTMLElement & { checkVisibility?: (o?: unknown) => boolean }).checkVisibility =
() => true
install()
inside.focus()
inside.blur()
pressPasteChordOn(document.body)
expect(onPasteKey).toHaveBeenCalledTimes(1)
})
it('stops recovering pastes after dispose', () => {
const fallback = install()
inside.focus()
@@ -37,10 +37,16 @@ export function installInertFocusPasteFallback({
paneOwnsFocus = false
}
}
// Order matters: these run at the document on every keystroke in the app, once
// per mounted pane. The two boolean checks short-circuit before any DOM walk,
// and the layout-touching visibility check only runs for the at-most-one pane
// that both owns focus and has focus sitting on <body>.
const ownsInertFocus = (): boolean =>
paneOwnsFocus && isInertDocumentFocus(documentTarget.activeElement)
paneOwnsFocus &&
isInertDocumentFocus(documentTarget.activeElement) &&
isContainerRendered(container)
const shouldRecover = (event: Event): boolean =>
!containsNode(container, event.target) && ownsInertFocus()
ownsInertFocus() && !containsNode(container, event.target)
const onKeyDown = (event: KeyboardEvent): void => {
if (shouldRecover(event)) {
@@ -73,6 +79,27 @@ function containsNode(container: HTMLElement, node: EventTarget | Node | null):
return node instanceof Node && container.contains(node)
}
/** A pane that stopped being rendered (CSS-hidden floating panel, display:none
* background surface) was blurred to `<body>` by the browser with no focusin, so
* `paneOwnsFocus` stays stale. Claiming on that swallows the chord for the whole
* app at the capture phase and routes the paste into an invisible terminal. */
function isContainerRendered(container: HTMLElement): boolean {
if (!container.isConnected) {
return false
}
const checkVisibility = (
container as HTMLElement & {
checkVisibility?: (options?: { visibilityProperty?: boolean }) => boolean
}
).checkVisibility
if (typeof checkVisibility === 'function') {
return checkVisibility.call(container, { visibilityProperty: true })
}
// Fallback for engines without checkVisibility; `visibility` is inherited, so
// reading it off the container also catches a hidden ancestor.
return container.ownerDocument.defaultView?.getComputedStyle(container).visibility !== 'hidden'
}
/** Focus is on <body> but the pane still owns it logically; putting it back before
* the shared paste path runs lets that path's dispatch-element guard see the real target. */
export function restoreInertFocusPasteTarget(