fix: keep terminal input sources explicit (#383)

This commit is contained in:
Adil Shaikh 2026-06-23 04:40:09 +05:30 committed by GitHub
parent 14c2c485be
commit afe0817765
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 59 additions and 17 deletions

View File

@ -10,6 +10,7 @@ const state = vi.hoisted(() => ({
options: Record<string, unknown>; options: Record<string, unknown>;
modes: { bracketedPasteMode: boolean }; modes: { bracketedPasteMode: boolean };
dataListeners: Set<(data: string) => void>; dataListeners: Set<(data: string) => void>;
keyListeners: Set<(event: { key: string }) => void>;
selectionListeners: Set<() => void>; selectionListeners: Set<() => void>;
_core: { _core: {
element: { classList: { add: ReturnType<typeof vi.fn>; remove: ReturnType<typeof vi.fn> } }; element: { classList: { add: ReturnType<typeof vi.fn>; remove: ReturnType<typeof vi.fn> } };
@ -31,6 +32,7 @@ vi.mock("@xterm/xterm", () => ({
wheelHandler?: (event: WheelEvent) => boolean; wheelHandler?: (event: WheelEvent) => boolean;
modes = { bracketedPasteMode: false }; modes = { bracketedPasteMode: false };
dataListeners = new Set<(data: string) => void>(); dataListeners = new Set<(data: string) => void>();
keyListeners = new Set<(event: { key: string }) => void>();
selectionListeners = new Set<() => void>(); selectionListeners = new Set<() => void>();
_core = { _core = {
element: { classList: { add: vi.fn(), remove: vi.fn() } }, element: { classList: { add: vi.fn(), remove: vi.fn() } },
@ -62,8 +64,9 @@ vi.mock("@xterm/xterm", () => ({
onRender() { onRender() {
return { dispose: () => undefined }; return { dispose: () => undefined };
} }
onKey() { onKey(listener: (event: { key: string }) => void) {
return { dispose: () => undefined }; this.keyListeners.add(listener);
return { dispose: () => this.keyListeners.delete(listener) };
} }
onSelectionChange(listener: () => void) { onSelectionChange(listener: () => void) {
this.selectionListeners.add(listener); this.selectionListeners.add(listener);
@ -359,16 +362,25 @@ describe("XtermTerminal", () => {
expect(allowed).toBe(false); expect(allowed).toBe(false);
expect(event.preventDefault).toHaveBeenCalled(); expect(event.preventDefault).toHaveBeenCalled();
expect(event.stopPropagation).toHaveBeenCalled(); expect(event.stopPropagation).toHaveBeenCalled();
expect(onInput).toHaveBeenCalledWith(expected, "terminal"); expect(onInput).toHaveBeenCalledWith(expected, "shortcut");
}); });
it("forwards generated xterm input data such as wheel scroll reports", () => { it("forwards keyboard input from explicit key events", () => {
const onInput = vi.fn(); const onInput = vi.fn();
render(<XtermTerminal theme="dark" onReady={(terminal) => terminal.onUserInput(onInput)} />); render(<XtermTerminal theme="dark" onReady={(terminal) => terminal.onUserInput(onInput)} />);
state.lastTerminal!.dataListeners.forEach((listener) => listener("\x1b[A")); state.lastTerminal!.keyListeners.forEach((listener) => listener({ key: "a" }));
expect(onInput).toHaveBeenCalledWith("\x1b[A", "terminal"); expect(onInput).toHaveBeenCalledWith("a", "keyboard");
});
it("does not forward raw xterm data/control bytes as user input", () => {
const onInput = vi.fn();
render(<XtermTerminal theme="dark" onReady={(terminal) => terminal.onUserInput(onInput)} />);
expect(state.lastTerminal!.dataListeners.size).toBe(0);
state.lastTerminal!.dataListeners.forEach((listener) => listener("\x1b[A"));
expect(onInput).not.toHaveBeenCalled();
}); });
it("translates wheel motion into SGR wheel reports for zellij scrollback", () => { it("translates wheel motion into SGR wheel reports for zellij scrollback", () => {
@ -378,7 +390,7 @@ describe("XtermTerminal", () => {
const suppressed = state.lastTerminal!.wheelHandler!({ deltaY: -50 } as WheelEvent); const suppressed = state.lastTerminal!.wheelHandler!({ deltaY: -50 } as WheelEvent);
expect(suppressed).toBe(false); expect(suppressed).toBe(false);
expect(onInput).toHaveBeenCalledWith("\x1b[<64;1;1M\x1b[<64;1;1M\x1b[<64;1;1M", "terminal"); expect(onInput).toHaveBeenCalledWith("\x1b[<64;1;1M\x1b[<64;1;1M\x1b[<64;1;1M", "wheel");
}); });
it("handles line- and page-mode wheels (Linux/Windows mice), not just pixel deltas", () => { it("handles line- and page-mode wheels (Linux/Windows mice), not just pixel deltas", () => {
@ -387,12 +399,12 @@ describe("XtermTerminal", () => {
// DOM_DELTA_LINE: deltaY is already in lines, so one notch up => one report. // DOM_DELTA_LINE: deltaY is already in lines, so one notch up => one report.
expect(state.lastTerminal!.wheelHandler!({ deltaY: -1, deltaMode: 1 } as WheelEvent)).toBe(false); expect(state.lastTerminal!.wheelHandler!({ deltaY: -1, deltaMode: 1 } as WheelEvent)).toBe(false);
expect(onInput).toHaveBeenLastCalledWith("\x1b[<64;1;1M", "terminal"); expect(onInput).toHaveBeenLastCalledWith("\x1b[<64;1;1M", "wheel");
// DOM_DELTA_PAGE: one page down => rows (24) line reports down. // DOM_DELTA_PAGE: one page down => rows (24) line reports down.
onInput.mockClear(); onInput.mockClear();
expect(state.lastTerminal!.wheelHandler!({ deltaY: 1, deltaMode: 2 } as WheelEvent)).toBe(false); expect(state.lastTerminal!.wheelHandler!({ deltaY: 1, deltaMode: 2 } as WheelEvent)).toBe(false);
expect(onInput).toHaveBeenLastCalledWith("\x1b[<65;1;1M".repeat(24), "terminal"); expect(onInput).toHaveBeenLastCalledWith("\x1b[<65;1;1M".repeat(24), "wheel");
}); });
it("scrolls down on positive wheel delta and leaves zoom (ctrl/meta) wheel alone", () => { it("scrolls down on positive wheel delta and leaves zoom (ctrl/meta) wheel alone", () => {
@ -400,14 +412,14 @@ describe("XtermTerminal", () => {
render(<XtermTerminal theme="dark" onReady={(terminal) => terminal.onUserInput(onInput)} />); render(<XtermTerminal theme="dark" onReady={(terminal) => terminal.onUserInput(onInput)} />);
expect(state.lastTerminal!.wheelHandler!({ deltaY: 20 } as WheelEvent)).toBe(false); expect(state.lastTerminal!.wheelHandler!({ deltaY: 20 } as WheelEvent)).toBe(false);
expect(onInput).toHaveBeenCalledWith("\x1b[<65;1;1M", "terminal"); expect(onInput).toHaveBeenCalledWith("\x1b[<65;1;1M", "wheel");
onInput.mockClear(); onInput.mockClear();
expect(state.lastTerminal!.wheelHandler!({ deltaY: -50, ctrlKey: true } as WheelEvent)).toBe(false); expect(state.lastTerminal!.wheelHandler!({ deltaY: -50, ctrlKey: true } as WheelEvent)).toBe(false);
expect(onInput).not.toHaveBeenCalled(); expect(onInput).not.toHaveBeenCalled();
}); });
it("forces plain drag selection while preserving xterm data forwarding", () => { it("forces plain drag selection without raw xterm data forwarding", () => {
render(<XtermTerminal theme="dark" />); render(<XtermTerminal theme="dark" />);
expect(state.lastTerminal!.options.macOptionClickForcesSelection).toBe(true); expect(state.lastTerminal!.options.macOptionClickForcesSelection).toBe(true);

View File

@ -326,7 +326,7 @@ export function XtermTerminal(props: XtermTerminalProps) {
const normalized = normalizedTerminalShortcut(event); const normalized = normalizedTerminalShortcut(event);
if (!normalized) return true; if (!normalized) return true;
consumeTerminalShortcut(event); consumeTerminalShortcut(event);
emitUserInput(normalized, "terminal"); emitUserInput(normalized, "shortcut");
return false; return false;
}); });
const copyInput = (event: ClipboardEvent) => { const copyInput = (event: ClipboardEvent) => {
@ -430,7 +430,12 @@ export function XtermTerminal(props: XtermTerminalProps) {
// misses them. Listen on window directly as a session-long recovery path. // misses them. Listen on window directly as a session-long recovery path.
window.addEventListener("resize", fitTerminal); window.addEventListener("resize", fitTerminal);
const terminalInput = term.onData((data) => emitUserInput(data, "terminal")); // Do not replace this with term.onData. xterm's raw data stream can include
// terminal-generated control responses during attach/repaint; forwarding
// those bytes through the mux writes dirty input into the real Codex PTY and
// corrupts the TUI. Keyboard is the only safe generic text path here; paste,
// composition, shortcuts, and wheel reports are emitted explicitly below.
const keyInput = term.onKey(({ key }) => emitUserInput(key, "keyboard"));
// Translate wheel motion into SGR wheel reports for zellij (see // Translate wheel motion into SGR wheel reports for zellij (see
// sgrWheelReport), one report per scrolled line. WheelEvent.deltaMode // sgrWheelReport), one report per scrolled line. WheelEvent.deltaMode
@ -457,7 +462,7 @@ export function XtermTerminal(props: XtermTerminalProps) {
} }
if (lines === 0) return false; if (lines === 0) return false;
const button = lines < 0 ? SGR_WHEEL_UP : SGR_WHEEL_DOWN; const button = lines < 0 ? SGR_WHEEL_UP : SGR_WHEEL_DOWN;
emitUserInput(sgrWheelReport(button, Math.abs(lines)), "terminal"); emitUserInput(sgrWheelReport(button, Math.abs(lines)), "wheel");
return false; return false;
}); });
const pasteInput = (event: ClipboardEvent) => { const pasteInput = (event: ClipboardEvent) => {
@ -505,7 +510,7 @@ export function XtermTerminal(props: XtermTerminalProps) {
selectionChange.dispose(); selectionChange.dispose();
host.removeEventListener("paste", pasteInput, true); host.removeEventListener("paste", pasteInput, true);
host.removeEventListener("compositionend", compositionInput, true); host.removeEventListener("compositionend", compositionInput, true);
terminalInput.dispose(); keyInput.dispose();
userInputListeners.clear(); userInputListeners.clear();
try { try {
term.dispose(); term.dispose();

View File

@ -91,6 +91,9 @@ type FakeTerminal = AttachableTerminal & {
clears: number; clears: number;
typeKeys(data: string): void; typeKeys(data: string): void;
paste(data: string): void; paste(data: string): void;
compose(data: string): void;
shortcut(data: string): void;
wheel(data: string): void;
emitResize(cols: number, rows: number): void; emitResize(cols: number, rows: number): void;
}; };
@ -115,8 +118,11 @@ function createFakeTerminal(): FakeTerminal {
resizeListeners.add(listener); resizeListeners.add(listener);
return { dispose: () => resizeListeners.delete(listener) }; return { dispose: () => resizeListeners.delete(listener) };
}, },
typeKeys: (data) => inputListeners.forEach((listener) => listener(data, "terminal")), typeKeys: (data) => inputListeners.forEach((listener) => listener(data, "keyboard")),
paste: (data) => inputListeners.forEach((listener) => listener(data, "paste")), paste: (data) => inputListeners.forEach((listener) => listener(data, "paste")),
compose: (data) => inputListeners.forEach((listener) => listener(data, "composition")),
shortcut: (data) => inputListeners.forEach((listener) => listener(data, "shortcut")),
wheel: (data) => inputListeners.forEach((listener) => listener(data, "wheel")),
emitResize: (cols, rows) => resizeListeners.forEach((listener) => listener({ cols, rows })), emitResize: (cols, rows) => resizeListeners.forEach((listener) => listener({ cols, rows })),
}; };
return terminal; return terminal;
@ -188,6 +194,25 @@ describe("useTerminalSession", () => {
expect(muxes[0].resizes).toContainEqual(["handle-1", 120, 40]); expect(muxes[0].resizes).toContainEqual(["handle-1", 120, 40]);
}); });
it("forwards every explicit input source after the attachment opens", () => {
const { terminal, muxes } = setup();
act(() => muxes[0].emitOpened("handle-1"));
terminal.typeKeys("a");
terminal.paste("paste\r");
terminal.compose("é");
terminal.shortcut("\x1b[1;5D");
terminal.wheel("\x1b[<64;1;1M");
expect(muxes[0].inputs).toEqual([
["handle-1", "a"],
["handle-1", "paste\r"],
["handle-1", "é"],
["handle-1", "\x1b[1;5D"],
["handle-1", "\x1b[<64;1;1M"],
]);
});
it("collapses a drag's burst of grid changes into one trailing PTY resize, then re-asserts it", () => { it("collapses a drag's burst of grid changes into one trailing PTY resize, then re-asserts it", () => {
const { terminal, muxes } = setup(); const { terminal, muxes } = setup();
const initialResizes = muxes[0].resizes.length; // connect() sends the opening size const initialResizes = muxes[0].resizes.length; // connect() sends the opening size

View File

@ -18,7 +18,7 @@ import { workspaceQueryKey } from "./useWorkspaceQuery";
* The slice of xterm's Terminal the attachment needs. Structural, so tests can * The slice of xterm's Terminal the attachment needs. Structural, so tests can
* drive the hook with a tiny fake instead of a real xterm + DOM. * drive the hook with a tiny fake instead of a real xterm + DOM.
*/ */
export type TerminalUserInputSource = "terminal" | "paste" | "composition"; export type TerminalUserInputSource = "keyboard" | "paste" | "composition" | "shortcut" | "wheel";
export type AttachableTerminal = { export type AttachableTerminal = {
cols: number; cols: number;