fix(ime): stop composing keystrokes from driving secondary key handlers

Three surfaces acted on keystrokes an IME still owned:

- the popout preview terminal fed IME-owned key releases to xterm, which
  encodes them as escape sequences once event reporting is on. Scoped to
  keyup: a composing keypress can carry the committed glyph.
- the task page's Mod+F window listener tore down a live preedit by
  focusing and selecting the search field.
- the deferred-newline ceiling emitted Enter into a composition the route
  still positively reported as open, splitting the preedit.

The ceiling now drops the newline and releases the sender's absorb credit,
so the user's next Enter is not swallowed. Dropping it costs one repeatable
keypress; splitting a live preedit does not undo.

Adds a contract test for the previously implicit invariant that every Enter
shape the shortcut guard claims is also one the xterm suppression policy
withholds.

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil
2026-08-02 00:29:03 -07:00
co-authored by Orca
parent 6441c27c71
commit b190b10d4f
10 changed files with 438 additions and 123 deletions
+8 -19
View File
@@ -43,7 +43,10 @@ import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-cl
import { getLocalPreflightContext, localPreflightContextKey } from '@/lib/local-preflight-context'
import { getProviderRuntimeContextKey } from '@/lib/provider-runtime-context'
import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event'
import { resolveTaskPageEscapeAction } from '@/lib/task-page-escape-policy'
import {
resolveTaskPageEscapeAction,
resolveTaskPageSearchShortcut
} from '@/lib/task-page-window-shortcut-policy'
import {
getSettingsFocusedExecutionHostId,
parseExecutionHostId,
@@ -6509,27 +6512,13 @@ export default function TaskPage(): React.JSX.Element {
}
const onKeyDown = (event: KeyboardEvent): void => {
const isMac = navigator.userAgent.includes('Mac')
const modifierPressed = isMac ? event.metaKey : event.ctrlKey
if (!modifierPressed || event.altKey || event.shiftKey || event.key.toLowerCase() !== 'f') {
return
}
const input = taskSearchInputRef.current
if (!input) {
const action = resolveTaskPageSearchShortcut(event, event.target, input, {
isMac: navigator.userAgent.includes('Mac')
})
if (action === 'ignore' || !input) {
return
}
const target = event.target
if (
target instanceof HTMLElement &&
target !== input &&
(target instanceof HTMLInputElement ||
target instanceof HTMLTextAreaElement ||
target.isContentEditable)
) {
return
}
event.preventDefault()
event.stopPropagation()
input.focus()
@@ -129,4 +129,20 @@ describe('preview terminal key handler yields to a composition', () => {
expect(handler.handle(keyup({ key: 'ArrowLeft', code: 'ArrowLeft', keyCode: 37 }))).toBe(true)
handler.dispose()
})
// The release guard must not widen to keypress: a composing keypress can carry the
// committed glyph, and withholding it from xterm drops the text off the live PTY. This is
// the same shape the pane policy deliberately lets through (xterm-bypass-policy.ts).
it('hands a composing keypress to xterm so the committed glyph survives', () => {
const handler = installHandler()
const event = new KeyboardEvent('keypress', {
key: '中',
bubbles: true,
cancelable: true,
isComposing: true
})
expect(handler.handle(event)).toBe(true)
handler.dispose()
})
})
@@ -102,8 +102,9 @@ export function installPreviewTerminalKeyHandler(args: {
}
// Why: xterm encodes accepted releases once Kitty REPORT_EVENT_TYPES is on, so an
// IME-owned keyup reaches the PTY as a release sequence mid-composition. Withhold it
// from xterm without preventDefault — the IME still needs the event.
return !isImeCompositionKeyDown(event)
// from xterm without preventDefault — the IME still needs the event. Scoped to keyup:
// a composing keypress can carry the committed glyph, and dropping it loses text.
return !(event.type === 'keyup' && isImeCompositionKeyDown(event))
}
nativeOnlyShortcutTracker.prepareKeyDown(event)
const resolveAction = (
@@ -196,7 +196,7 @@ describe('sendTerminalInputAfterComposition', () => {
expect(send).toHaveBeenCalledTimes(1)
})
it('sends once at the ceiling when a composition never ends', () => {
it('drops the newline at the ceiling while the composition is still positively pending', () => {
const el = document.createElement('div')
const send = vi.fn()
const transport = { getPtyId: () => 'pty-1' } as unknown as PtyTransport
@@ -210,20 +210,36 @@ describe('sendTerminalInputAfterComposition', () => {
new CustomEvent(XTERM_COMPOSITION_SESSION_START_EVENT, { detail: { id: 1, data: '한' } })
)
sendTerminalInputAfterComposition(el, send, { fallbackMs: 50, maxIdleMs: 200 })
const onAbandon = vi.fn()
sendTerminalInputAfterComposition(el, send, { fallbackMs: 50, maxIdleMs: 200, onAbandon })
vi.advanceTimersByTime(150)
expect(send).not.toHaveBeenCalled()
// Bounded: a composition that stops progressing must not strand the keystroke.
// A user reading candidates is not a stuck composition: emit nothing and settle instead.
vi.advanceTimersByTime(100)
expect(send).toHaveBeenCalledTimes(1)
expect(onAbandon).toHaveBeenCalledTimes(1)
vi.runAllTimers()
expect(send).toHaveBeenCalledTimes(1)
expect(send).not.toHaveBeenCalled()
expect(onAbandon).toHaveBeenCalledTimes(1)
route.dispose()
})
it('still sends at the ceiling once the route no longer reports a pending composition', () => {
const el = document.createElement('div')
const send = vi.fn()
const onAbandon = vi.fn()
// No composition route installed, so nothing positively reports a live preedit.
sendTerminalInputAfterComposition(el, send, { fallbackMs: 50, maxIdleMs: 200, onAbandon })
vi.advanceTimersByTime(50)
vi.runAllTimers()
expect(send).toHaveBeenCalledTimes(1)
expect(onAbandon).not.toHaveBeenCalled()
})
it('still delivers the input on the next macrotask without a terminal element', () => {
const send = vi.fn()
@@ -262,6 +278,32 @@ describe('createTerminalImeDeferredNewlineSender', () => {
expect(sender.absorbRedispatchedEnter(enter(10))).toBe(false)
})
// Abandoning at the ceiling must release the credit too: no commit means no re-dispatch is
// coming, and a surviving credit would swallow the Enter the user presses next.
it('releases its bookkeeping when the ceiling drops the newline', () => {
const el = document.createElement('div')
const send = vi.fn()
const sender = createSender()
const transport = { getPtyId: () => 'pty-1' } as unknown as PtyTransport
const route = installTerminalImeCompositionRoute({
terminalElement: el,
terminal: { input: vi.fn() },
capturedTransport: transport,
getCurrentTransport: () => transport
})
el.dispatchEvent(
new CustomEvent(XTERM_COMPOSITION_SESSION_START_EVENT, { detail: { id: 1, data: '한' } })
)
sender.defer(enter(10), el, send)
vi.runAllTimers()
expect(send).not.toHaveBeenCalled()
expect(sender.absorbRedispatchedEnter(enter(10))).toBe(false)
route.dispose()
})
it('absorbs after the deferred send even if focus moved to another pane', () => {
const el = document.createElement('div')
const send = vi.fn()
@@ -9,12 +9,13 @@ export const TERMINAL_IME_DEFERRED_NEWLINE_FALLBACK_MS = 200
// compositionupdate, not from the start: multi-segment bunsetsu conversion keeps a
// Japanese composition legitimately pending far longer than this, and finishing on a
// total-elapsed ceiling would split it with the newline this deferral exists to hold back.
// At the ceiling the newline is dropped, not sent — see `abandon` below.
export const TERMINAL_IME_DEFERRED_NEWLINE_MAX_IDLE_MS = 2000
export function sendTerminalInputAfterComposition(
terminalElement: HTMLElement | null | undefined,
send: () => void,
options?: { fallbackMs?: number; maxIdleMs?: number }
options?: { fallbackMs?: number; maxIdleMs?: number; onAbandon?: () => void }
): void {
if (!terminalElement) {
window.setTimeout(send, 0)
@@ -29,9 +30,9 @@ export function sendTerminalInputAfterComposition(
let done = false
let idleMs = 0
const finish = (): void => {
const settle = (): boolean => {
if (done) {
return
return false
}
done = true
terminalElement.removeEventListener('compositionend', onCompositionEnd)
@@ -41,8 +42,21 @@ export function sendTerminalInputAfterComposition(
onCompositionSessionEnd
)
window.clearTimeout(fallbackTimer)
// xterm flushes the committed glyph after compositionend.
window.setTimeout(send, 0)
return true
}
const finish = (): void => {
if (settle()) {
// xterm flushes the committed glyph after compositionend.
window.setTimeout(send, 0)
}
}
/** Give up on the newline without emitting it, releasing the caller's bookkeeping. */
const abandon = (): void => {
if (settle()) {
options?.onAbandon?.()
}
}
const finishAfterPendingComposition = (): void => {
@@ -72,10 +86,18 @@ export function sendTerminalInputAfterComposition(
const compositionInProgress =
hasPendingTerminalImeComposition(terminalElement) || sawRecentUpdate
sawRecentUpdate = false
if (!compositionInProgress || idleMs >= maxIdleMs) {
if (!compositionInProgress) {
finish()
return
}
if (idleMs >= maxIdleMs) {
// Why: reaching the ceiling only proves the preedit stopped changing — the route still
// positively reports it live, and a user reading candidates is not a stuck composition.
// Dropping the newline costs one repeatable keypress; splitting a live preedit does not
// undo. Emit nothing while the evidence says a composition is still open.
abandon()
return
}
fallbackTimer = window.setTimeout(onFallbackDeadline, fallbackMs)
}
let fallbackTimer = window.setTimeout(onFallbackDeadline, fallbackMs)
@@ -196,11 +218,23 @@ export function createTerminalImeDeferredNewlineSender(): TerminalImeDeferredNew
state.absorbCredits += 1
statesByTimeStamp.set(enter.timeStamp, state)
statesByEnterCode.set(enter.code, statesByTimeStamp)
sendTerminalInputAfterComposition(terminalElement, () => {
state.inFlightSends -= 1
cleanUpIfSettled(enter, state)
send()
})
sendTerminalInputAfterComposition(
terminalElement,
() => {
state.inFlightSends -= 1
cleanUpIfSettled(enter, state)
send()
},
{
onAbandon: () => {
state.inFlightSends -= 1
// No commit means no redispatched Enter is coming; a surviving credit would
// absorb the user's next one instead.
state.absorbCredits = Math.max(0, state.absorbCredits - 1)
cleanUpIfSettled(enter, state)
}
}
)
},
absorbRedispatchedEnter: (enter) => {
const state = statesByEnterCode.get(enter.code)?.get(enter.timeStamp)
@@ -0,0 +1,114 @@
// @vitest-environment happy-dom
//
// `terminalShortcutIsOwnedByIme` returning true makes the window handler `return` without
// preventDefault, so the keystroke keeps propagating — the IME still needs it. That is only
// safe because the pane's own xterm handler withholds the same event from xterm's `_keyDown`
// (use-terminal-pane-lifecycle.ts calls this policy and returns false on suppression, which
// short-circuits xterm ahead of CompositionHelper and the CR emission).
//
// Nothing in either module states that dependency, so this pins it: every Enter shape the
// guard claims must also be one the bypass policy suppresses. Loosen either side and a
// composing Enter chord reaches the PTY and submits the half-typed command.
import { describe, expect, it } from 'vitest'
import {
shouldSuppressTerminalImeKeyboardEvent,
type XtermBypassEvent
} from './xterm-bypass-policy'
import { terminalShortcutIsOwnedByIme } from './terminal-shortcut-composition-guard'
type Platform = { isMac: boolean; isLinux: boolean }
const MAC: Platform = { isMac: true, isLinux: false }
const WINDOWS: Platform = { isMac: false, isLinux: false }
const LINUX: Platform = { isMac: false, isLinux: true }
// A chord bound to something other than sendInput is the case the guard now claims rather
// than exempting, so it is the case whose propagation has to be provably harmless.
const EXPAND_PANE_CHORD = { type: 'keydown', code: 'Enter', ctrlKey: true, shiftKey: true }
const CLAIMED_ENTER_SHAPES: { name: string; platform: Platform; event: XtermBypassEvent }[] = [
{
name: 'marked-real Enter (isComposing)',
platform: MAC,
event: {
...EXPAND_PANE_CHORD,
key: 'Enter',
keyCode: 13,
isComposing: true,
metaKey: false,
altKey: false
} as XtermBypassEvent
},
{
name: 'Windows Process Enter',
platform: WINDOWS,
event: {
...EXPAND_PANE_CHORD,
key: 'Process',
keyCode: 229,
isComposing: false,
metaKey: false,
altKey: false
} as XtermBypassEvent
},
{
name: 'Linux composing Enter',
platform: LINUX,
event: {
...EXPAND_PANE_CHORD,
key: 'Enter',
keyCode: 13,
isComposing: true,
metaKey: false,
altKey: false
} as XtermBypassEvent
}
]
describe('the shortcut guard only claims Enter shapes xterm never sees', () => {
const expandPane = { type: 'toggleExpandActivePane' } as const
it.each(CLAIMED_ENTER_SHAPES)('$name', ({ platform, event }) => {
const claimed = terminalShortcutIsOwnedByIme(
event as unknown as KeyboardEvent,
() => expandPane,
{ enterIsDeferredToCommit: true }
)
expect(claimed).toBe(true)
expect(
shouldSuppressTerminalImeKeyboardEvent(event, {
compositionActive: true,
candidateKeyGuardActive: false,
pendingCandidateKeyReleaseActive: false,
...platform
})
).toBe(true)
})
it('leaves an unmarked Enter chord to the ordinary dispatch on both sides', () => {
const event = {
...EXPAND_PANE_CHORD,
key: 'Enter',
keyCode: 13,
isComposing: false,
metaKey: false,
altKey: false
} as XtermBypassEvent
expect(
terminalShortcutIsOwnedByIme(event as unknown as KeyboardEvent, () => expandPane, {
enterIsDeferredToCommit: true
})
).toBe(false)
expect(
shouldSuppressTerminalImeKeyboardEvent(event, {
compositionActive: false,
candidateKeyGuardActive: false,
pendingCandidateKeyReleaseActive: false,
...MAC
})
).toBe(false)
})
})
@@ -1,55 +0,0 @@
// @vitest-environment happy-dom
import { describe, expect, it } from 'vitest'
import { resolveTaskPageEscapeAction } from './task-page-escape-policy'
function escape(init: { isComposing?: boolean; keyCode?: number; key?: string } = {}): {
key: string
keyCode: number
isComposing: boolean
} {
return {
key: init.key ?? 'Escape',
keyCode: init.keyCode ?? 27,
isComposing: init.isComposing ?? false
}
}
describe('resolveTaskPageEscapeAction', () => {
it('closes the page when focus is outside a field', () => {
expect(resolveTaskPageEscapeAction(escape(), document.createElement('div'))).toBe('close-page')
})
it.each([
['input', () => document.createElement('input')],
['textarea', () => document.createElement('textarea')],
['select', () => document.createElement('select')]
])('blurs a focused %s instead of closing the page', (_name, create) => {
expect(resolveTaskPageEscapeAction(escape(), create())).toBe('blur-target')
})
it('blurs a contenteditable host', () => {
const target = document.createElement('div')
target.contentEditable = 'true'
Object.defineProperty(target, 'isContentEditable', { value: true })
expect(resolveTaskPageEscapeAction(escape(), target)).toBe('blur-target')
})
// The listener is in the capture phase, so it sees the candidate-window Escape before the
// composing input does. Blurring here would end the composition the user meant to keep.
it.each([
['isComposing', escape({ isComposing: true })],
['keyCode 229', escape({ keyCode: 229 })]
])('ignores an Escape marked by %s while a field has focus', (_marker, event) => {
expect(resolveTaskPageEscapeAction(event, document.createElement('input'))).toBe('ignore')
})
it('ignores a key that is not Escape', () => {
expect(
resolveTaskPageEscapeAction(escape({ key: 'Enter' }), document.createElement('div'))
).toBe('ignore')
})
it('ignores an event with no element target', () => {
expect(resolveTaskPageEscapeAction(escape(), null)).toBe('ignore')
})
})
@@ -1,31 +0,0 @@
import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event'
export type TaskPageEscapeAction = 'ignore' | 'blur-target' | 'close-page'
/**
* What Escape should do at the task page's window-capture listener.
*
* The IME check comes first because the listener runs in the capture phase, ahead of the
* composing element. Escape is how the user dismisses a candidate window, and acting on it
* here would blur the field and take the live composition with it.
*
* Blurring before closing keeps Escape from dismissing the whole page for a user who only
* meant to leave a field.
*/
export function resolveTaskPageEscapeAction(
event: Pick<KeyboardEvent, 'key' | 'keyCode' | 'isComposing'>,
target: EventTarget | null
): TaskPageEscapeAction {
if (isImeCompositionKeyDown(event) || event.key !== 'Escape') {
return 'ignore'
}
if (!(target instanceof HTMLElement)) {
return 'ignore'
}
const focusIsInAField =
target instanceof HTMLInputElement ||
target instanceof HTMLTextAreaElement ||
target instanceof HTMLSelectElement ||
target.isContentEditable
return focusIsInAField ? 'blur-target' : 'close-page'
}
@@ -0,0 +1,132 @@
// @vitest-environment happy-dom
import { describe, expect, it } from 'vitest'
import {
resolveTaskPageEscapeAction,
resolveTaskPageSearchShortcut
} from './task-page-window-shortcut-policy'
function escape(init: { isComposing?: boolean; keyCode?: number; key?: string } = {}): {
key: string
keyCode: number
isComposing: boolean
} {
return {
key: init.key ?? 'Escape',
keyCode: init.keyCode ?? 27,
isComposing: init.isComposing ?? false
}
}
describe('resolveTaskPageEscapeAction', () => {
it('closes the page when focus is outside a field', () => {
expect(resolveTaskPageEscapeAction(escape(), document.createElement('div'))).toBe('close-page')
})
it.each([
['input', () => document.createElement('input')],
['textarea', () => document.createElement('textarea')],
['select', () => document.createElement('select')]
])('blurs a focused %s instead of closing the page', (_name, create) => {
expect(resolveTaskPageEscapeAction(escape(), create())).toBe('blur-target')
})
it('blurs a contenteditable host', () => {
const target = document.createElement('div')
target.contentEditable = 'true'
Object.defineProperty(target, 'isContentEditable', { value: true })
expect(resolveTaskPageEscapeAction(escape(), target)).toBe('blur-target')
})
// The listener is in the capture phase, so it sees the candidate-window Escape before the
// composing input does. Blurring here would end the composition the user meant to keep.
it.each([
['isComposing', escape({ isComposing: true })],
['keyCode 229', escape({ keyCode: 229 })]
])('ignores an Escape marked by %s while a field has focus', (_marker, event) => {
expect(resolveTaskPageEscapeAction(event, document.createElement('input'))).toBe('ignore')
})
it('ignores a key that is not Escape', () => {
expect(
resolveTaskPageEscapeAction(escape({ key: 'Enter' }), document.createElement('div'))
).toBe('ignore')
})
it('ignores an event with no element target', () => {
expect(resolveTaskPageEscapeAction(escape(), null)).toBe('ignore')
})
})
describe('resolveTaskPageSearchShortcut', () => {
const findChord = (
over: Partial<{ isComposing: boolean; keyCode: number; key: string }> = {}
): Parameters<typeof resolveTaskPageSearchShortcut>[0] => ({
key: over.key ?? 'f',
keyCode: over.keyCode ?? 70,
isComposing: over.isComposing ?? false,
metaKey: true,
ctrlKey: false,
altKey: false,
shiftKey: false
})
const search = (): HTMLInputElement => document.createElement('input')
it('focuses the search input on Mod+F from outside a field', () => {
const input = search()
expect(
resolveTaskPageSearchShortcut(findChord(), document.createElement('div'), input, {
isMac: true
})
).toBe('focus-search')
})
it('re-selects when the search input itself already has focus', () => {
const input = search()
expect(resolveTaskPageSearchShortcut(findChord(), input, input, { isMac: true })).toBe(
'focus-search'
)
})
// The search input is exempt from the editable bail-out, so without the IME check a
// composing user pressing the chord has their preedit torn down by focus/select.
it.each([
['isComposing', findChord({ isComposing: true })],
['keyCode 229', findChord({ keyCode: 229 })],
['a Process key', findChord({ key: 'Process', keyCode: 229 })]
])('ignores a find chord marked by %s while composing in the search input', (_marker, event) => {
const input = search()
expect(resolveTaskPageSearchShortcut(event, input, input, { isMac: true })).toBe('ignore')
})
it('ignores the chord while another text field has focus', () => {
expect(
resolveTaskPageSearchShortcut(findChord(), document.createElement('textarea'), search(), {
isMac: true
})
).toBe('ignore')
})
it('ignores the chord when no search input is mounted', () => {
expect(
resolveTaskPageSearchShortcut(findChord(), document.createElement('div'), null, {
isMac: true
})
).toBe('ignore')
})
it('reads Ctrl rather than Meta off Mac', () => {
const input = search()
const target = document.createElement('div')
expect(resolveTaskPageSearchShortcut(findChord(), target, input, { isMac: false })).toBe(
'ignore'
)
expect(
resolveTaskPageSearchShortcut(
{ ...findChord(), metaKey: false, ctrlKey: true },
target,
input,
{ isMac: false }
)
).toBe('focus-search')
})
})
@@ -0,0 +1,73 @@
import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event'
export type TaskPageEscapeAction = 'ignore' | 'blur-target' | 'close-page'
export type TaskPageSearchShortcutAction = 'ignore' | 'focus-search'
/**
* The task page installs its shortcut listeners on `window` in the capture phase, so they
* run ahead of whatever element is composing. Every decision here therefore has to yield to
* a live composition before it looks at key shape.
*/
function targetAcceptsTextEntry(target: EventTarget | null): target is HTMLElement {
return (
target instanceof HTMLElement &&
(target instanceof HTMLInputElement ||
target instanceof HTMLTextAreaElement ||
target.isContentEditable)
)
}
/**
* Escape is how the user dismisses a candidate window, so acting on it here would blur the
* field and take the live composition with it.
*
* Blurring before closing keeps Escape from dismissing the whole page for a user who only
* meant to leave a field.
*/
export function resolveTaskPageEscapeAction(
event: Pick<KeyboardEvent, 'key' | 'keyCode' | 'isComposing'>,
target: EventTarget | null
): TaskPageEscapeAction {
if (isImeCompositionKeyDown(event) || event.key !== 'Escape') {
return 'ignore'
}
if (!(target instanceof HTMLElement)) {
return 'ignore'
}
const focusIsInAField = target instanceof HTMLSelectElement || targetAcceptsTextEntry(target)
return focusIsInAField ? 'blur-target' : 'close-page'
}
/**
* Whether Mod+F should steal focus into the task-search input.
*
* The search input is deliberately exempt from the editable-target bail-out so the chord
* re-selects text while the field already has focus — which is exactly why this needs the
* IME check: composing *inside* that field would otherwise have its preedit torn down by
* the focus/select below.
*/
export function resolveTaskPageSearchShortcut(
event: Pick<
KeyboardEvent,
'key' | 'keyCode' | 'isComposing' | 'metaKey' | 'ctrlKey' | 'altKey' | 'shiftKey'
>,
target: EventTarget | null,
searchInput: HTMLInputElement | null,
{ isMac }: { isMac: boolean }
): TaskPageSearchShortcutAction {
if (isImeCompositionKeyDown(event)) {
return 'ignore'
}
const modifierPressed = isMac ? event.metaKey : event.ctrlKey
if (!modifierPressed || event.altKey || event.shiftKey || event.key.toLowerCase() !== 'f') {
return 'ignore'
}
if (!searchInput) {
return 'ignore'
}
if (target !== searchInput && targetAcceptsTextEntry(target)) {
return 'ignore'
}
return 'focus-search'
}