Skip to content
Merged
1 change: 1 addition & 0 deletions packages/cli/src/gemini.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2205,6 +2205,7 @@ describe('startInteractiveUI', () => {
vi.mock('./ui/utils/kittyProtocolDetector.js', () => ({
detectAndEnableKittyProtocol: vi.fn(() => Promise.resolve(true)),
disableKittyProtocol: vi.fn(),
pushKittyProtocolFlags: vi.fn(),
}));

vi.mock('./utils/cleanup.js', () => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,101 @@ describe('<ThinkMessage />', () => {
expect(output).toContain('Line 20');
expect(output).not.toContain('Line 1\n');
});

it('should not shrink the streaming window when the visible tail momentarily drops', () => {
// A long unbroken run pushes the tail window to its full height. When the
// next chunk introduces a newline near the char-budget boundary, the tail
// slice can momentarily collapse to a single line even though the buffer
// only grew. Grow-only height must pad that back up so the block does not
// flicker down and then up again.
const wide = 'X'.repeat(400);
const { lastFrame, rerender } = render(
<ThinkMessage
{...defaultProps}
text={wide}
isPending={true}
expanded={false}
contentWidth={40}
/>,
);
const tallHeight = (lastFrame() ?? '').split('\n').length;

rerender(
<ThinkMessage
{...defaultProps}
text={`${wide}\nY`}
isPending={true}
expanded={false}
contentWidth={40}
/>,
);
const afterFrame = lastFrame() ?? '';
expect(afterFrame).toContain('Y');
// Height is preserved (grow-only), not collapsed to the 1-line natural tail.
expect(afterFrame.split('\n').length).toBe(tallHeight);
});

it('should reset the grow-only window when a new thought replaces the buffer', () => {
const tall = Array.from({ length: 10 }, (_, i) => `Row ${i + 1}`).join(
'\n',
);
const { lastFrame, rerender } = render(
<ThinkMessage
{...defaultProps}
text={tall}
isPending={true}
expanded={false}
contentWidth={40}
/>,
);
const tallHeight = (lastFrame() ?? '').split('\n').length;

// A shorter buffer signals a fresh thought; the window should shrink back.
rerender(
<ThinkMessage
{...defaultProps}
text={'short'}
isPending={true}
expanded={false}
contentWidth={40}
/>,
);
expect((lastFrame() ?? '').split('\n').length).toBeLessThan(tallHeight);
});

it('keeps the streaming window height stable as availableTerminalHeight changes', () => {
// While a thought streams the terminal keeps constrainHeight on, so
// availableTerminalHeight drifts as sibling pending content grows. The
// streaming window must not track it, or the block flickers in height.
const tall = Array.from({ length: 10 }, (_, i) => `Row ${i + 1}`).join(
'\n',
);
const { lastFrame, rerender } = render(
<ThinkMessage
{...defaultProps}
text={tall}
isPending={true}
expanded={false}
contentWidth={40}
availableTerminalHeight={30}
/>,
);
const heightBefore = (lastFrame() ?? '').split('\n').length;

// Same thought, but availableTerminalHeight collapses to a value whose
// old maxLines = floor(6/3) = 2 would have shrunk the window.
rerender(
<ThinkMessage
{...defaultProps}
text={tall}
isPending={true}
expanded={false}
contentWidth={40}
availableTerminalHeight={6}
/>,
);
expect((lastFrame() ?? '').split('\n').length).toBe(heightBefore);
});
});

describe('<ThinkMessageContent />', () => {
Expand Down
45 changes: 32 additions & 13 deletions packages/cli/src/ui/components/messages/ConversationMessages.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import type React from 'react';
import { useRef } from 'react';
import { Box, Text } from 'ink';
import stringWidth from 'string-width';
import {
Expand Down Expand Up @@ -275,7 +276,7 @@ function tailVisualLines(
text: string,
width: number,
maxLines: number,
): string {
): string[] {
const charBudget = maxLines * width * 2;
let sliceStart = Math.max(0, text.length - charBudget);
if (sliceStart > 0) {
Expand All @@ -285,7 +286,7 @@ function tailVisualLines(
}
}
const lines = wrapToVisualLines(text.slice(sliceStart), width);
return lines.slice(-maxLines).join('\n');
return lines.slice(-maxLines);
}

const ThinkBody: React.FC<{
Expand All @@ -295,21 +296,39 @@ const ThinkBody: React.FC<{
availableTerminalHeight?: number;
contentWidth: number;
}> = ({ text, isPending, expanded, availableTerminalHeight, contentWidth }) => {
// Grow-only height tracker for the streaming window: the rendered block never
// shrinks below the tallest it has already reached for this thought, so a
// blank paragraph separator (`\n\n`) transiently entering/leaving the tail
// window can't make the block jump 2→3→5 rows and flicker. Reset when the
// block stops streaming or when the buffer shrinks (a new thought replaced it).
const maxSeenLinesRef = useRef(0);
const prevTextLenRef = useRef(0);
if (!isPending || text.length < prevTextLenRef.current) {
maxSeenLinesRef.current = 0;
}
prevTextLenRef.current = text.length;

if (!isPending && !expanded) return null;

if (isPending && !expanded) {
const innerWidth = Math.max(contentWidth - 2, 20);
const maxLines =
availableTerminalHeight != null
? Math.max(
1,
Math.min(
MAX_STREAMING_THINKING_VISUAL_LINES,
Math.floor(availableTerminalHeight / 3),
),
)
: MAX_STREAMING_THINKING_VISUAL_LINES;
const display = tailVisualLines(text, innerWidth, maxLines);
// Use a constant window height rather than deriving it from
// availableTerminalHeight. While a thought streams the terminal keeps
// constrainHeight on, so availableTerminalHeight (and therefore a derived
// maxLines) drifts up and down as sibling pending content grows — which
// reintroduced the very height flicker this block is meant to remove. The
// window is at most a few lines, so a fixed cap can't meaningfully overflow
// (VP scrolls anyway), and it keeps the height stable.
const maxLines = MAX_STREAMING_THINKING_VISUAL_LINES;
const lines = tailVisualLines(text, innerWidth, maxLines);
const target = Math.max(lines.length, maxSeenLinesRef.current);
maxSeenLinesRef.current = target;
// Pad at the top so the newest line stays pinned to the bottom.
const padded =
lines.length < target
? [...new Array(target - lines.length).fill(''), ...lines]
: lines;
const display = padded.join('\n');
return (
<Box paddingLeft={2}>
<Text dimColor wrap="truncate">
Expand Down
73 changes: 73 additions & 0 deletions packages/cli/src/ui/contexts/KeypressContext.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,79 @@ describe('KeypressContext - Kitty Protocol', () => {
);
});

it('decodes Shift+Enter modifyOtherKeys form without Kitty enabled', () => {
// Ghostty (and other xterm modifyOtherKeys terminals) send Shift+Enter as
// ESC [ 27 ; 2 ; 13 ~ when the Kitty protocol is not negotiated. readline
// shreds this into a partial CSI plus stray "13~" characters; without the
// reassembly path the tail leaks into the input instead of inserting a
// newline. This is the core Shift+Enter fix.
const keyHandler = vi.fn();
const { result } = renderHook(() => useKeypressContext(), {
wrapper: ({ children }) =>
wrapper({ children, kittyProtocolEnabled: false }),
});
act(() => {
result.current.subscribe(keyHandler);
});

act(() => {
stdin.sendKittySequence(`\x1b[27;2;13~`);
});

expect(keyHandler).toHaveBeenCalledWith(
expect.objectContaining({ name: 'return', ctrl: false, shift: true }),
);
// The stray digits/tilde must not leak as literal input.
expect(keyHandler).not.toHaveBeenCalledWith(
expect.objectContaining({ sequence: '~' }),
);
});

it('decodes Ctrl+Enter modifyOtherKeys form without Kitty enabled', () => {
const keyHandler = vi.fn();
const { result } = renderHook(() => useKeypressContext(), {
wrapper: ({ children }) =>
wrapper({ children, kittyProtocolEnabled: false }),
});
act(() => {
result.current.subscribe(keyHandler);
});

// ESC [ 27 ; 5 ; 13 ~ (modifier 5 = Ctrl)
act(() => {
stdin.sendKittySequence(`\x1b[27;5;13~`);
});

expect(keyHandler).toHaveBeenCalledWith(
expect.objectContaining({ name: 'return', ctrl: true, shift: false }),
);
});

it('decodes Shift+Enter modifyOtherKeys form with Kitty enabled (not Escape)', () => {
// With Kitty enabled the same bytes were previously misread as Escape
// (the leading 27 marker mistaken for the Escape key code), which tripped
// the double-Esc rewind prompt. The third parameter is the real key code.
const keyHandler = vi.fn();
const { result } = renderHook(() => useKeypressContext(), {
wrapper: ({ children }) =>
wrapper({ children, kittyProtocolEnabled: true }),
});
act(() => {
result.current.subscribe(keyHandler);
});

act(() => {
stdin.sendKittySequence(`\x1b[27;2;13~`);
});

expect(keyHandler).toHaveBeenCalledWith(
expect.objectContaining({ name: 'return', shift: true }),
);
expect(keyHandler).not.toHaveBeenCalledWith(
expect.objectContaining({ name: 'escape' }),
);
});

it('Ctrl+C escapes a paste mode that never received its paste-end marker', async () => {
// Regression test for the "must restart terminal" lockup reported by
// a user on Ghostty + Sogou pinyin: bracketed-paste-start arrived,
Expand Down
32 changes: 28 additions & 4 deletions packages/cli/src/ui/contexts/KeypressContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -470,11 +470,11 @@ export function KeypressProvider({
// The colon-separated fields (shifted key, base key, event type, text)
// are optional extensions that some terminals send.
const csiUPrefix = new RegExp(
`^${ESC}\\[(\\d+)(?::\\d+)*(?:;(\\d+)(?::\\d+)*)?(?:;\\d+)?([u~])`,
`^${ESC}\\[(\\d+)(?::\\d+)*(?:;(\\d+)(?::\\d+)*)?(?:;(\\d+))?([u~])`,
);
m = buffer.match(csiUPrefix);
if (m) {
const keyCode = parseInt(m[1], 10);
let keyCode = parseInt(m[1], 10);
let modifiers = m[2] ? parseInt(m[2], 10) : KITTY_MODIFIER_BASE;
if (modifiers >= KITTY_MODIFIER_EVENT_TYPES_OFFSET) {
modifiers -= KITTY_MODIFIER_EVENT_TYPES_OFFSET;
Expand All @@ -484,7 +484,17 @@ export function KeypressProvider({
(modifierBits & MODIFIER_SHIFT_BIT) === MODIFIER_SHIFT_BIT;
const alt = (modifierBits & MODIFIER_ALT_BIT) === MODIFIER_ALT_BIT;
const ctrl = (modifierBits & MODIFIER_CTRL_BIT) === MODIFIER_CTRL_BIT;
const terminator = m[3];
const terminator = m[4];

// xterm "modifyOtherKeys" (formatOtherKeys=0): ESC [ 27 ; <mods> ; <key> ~
// Here the leading 27 is a fixed marker, not the key — the real key code
// is the third parameter. Terminals such as Ghostty emit this form for
// modified keys (e.g. Shift+Enter → ESC [ 27 ; 2 ; 13 ~) when the Kitty
// protocol is not active, so decode the third parameter as the key code
// rather than mistaking the marker 27 for Escape.
if (keyCode === CHAR_CODE_ESC && terminator === '~' && m[3]) {
keyCode = parseInt(m[3], 10);
}

// Tilde-coded functional keys (Delete, Insert, PageUp/Down, Home/End)
if (terminator === '~') {
Expand Down Expand Up @@ -937,7 +947,21 @@ export function KeypressProvider({
// Ctrl+C is handled earlier, above the paste-state branches, so
// that it remains an escape hatch even when paste mode is stuck.

if (kittyProtocolEnabled) {
// Reassemble xterm modifyOtherKeys sequences (ESC [ 27 ; … ~) even when
// the Kitty protocol was not negotiated. readline shreds them into a
// partial CSI event plus stray digit/`~` keypresses, so without buffering
// the tail (e.g. "13~") leaks as literal input. Terminals like Ghostty
// emit this form for Shift/Ctrl/Alt+Enter by default, and parseKittyPrefix
// decodes it. Only the 27-marker prefix opts in, so ordinary keys that
// readline already parses cleanly are left untouched.
const isModifyOtherKeysSequence =
key.sequence?.startsWith(`${ESC}[27`) ?? false;

if (
kittyProtocolEnabled ||
kittySequenceBufferRef.current ||
isModifyOtherKeysSequence
) {
if (
kittySequenceBufferRef.current ||
(key.sequence.startsWith(`${ESC}[`) &&
Expand Down
16 changes: 15 additions & 1 deletion packages/cli/src/ui/startInteractiveUI.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ import { VimModeProvider } from './contexts/VimModeContext.js';
import { AgentViewProvider } from './contexts/AgentViewContext.js';
import { BackgroundTaskViewProvider } from './contexts/BackgroundTaskViewContext.js';
import { useKittyKeyboardProtocol } from './hooks/useKittyKeyboardProtocol.js';
import { disableKittyProtocol } from './utils/kittyProtocolDetector.js';
import {
disableKittyProtocol,
pushKittyProtocolFlags,
} from './utils/kittyProtocolDetector.js';
Comment thread
chiga0 marked this conversation as resolved.
import { installTerminalRedrawOptimizer } from './utils/terminalRedrawOptimizer.js';
import { installSynchronizedOutput } from './utils/synchronizedOutput.js';
import { registerCleanup } from '../utils/cleanup.js';
Expand Down Expand Up @@ -206,6 +209,17 @@ export async function startInteractiveUI(
alternateScreen: useVP,
},
);
if (useVP) {
// Ink entered the alternate screen synchronously inside render() above.
// The Kitty keyboard flags were pushed at startup on the main screen, and
// the spec tracks them per screen, so re-push them onto the alternate
// screen now — otherwise Shift+Enter (and other modified keys) arrive
// without their modifier and degrade to a bare Enter or an orphaned Escape.
// The push is ordered after Ink's enter-alternate-screen write, and Ink
// discards the alternate screen (and its flag stack) on unmount, so the
// startup main-screen push remains balanced by disableKittyProtocol() below.
pushKittyProtocolFlags();
}
// Records the moment Ink's `render()` call has returned, which is
// synchronous and happens before React reconciliation actually pushes
// bytes to the terminal. We intentionally keep the legacy name
Expand Down
Loading
Loading