Skip to content
Merged
35 changes: 35 additions & 0 deletions packages/cli/src/ui/components/MainContent.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { describe, expect, it, vi } from 'vitest';
import { render } from 'ink-testing-library';
import { Text } from 'ink';
import { MainContent } from './MainContent.js';
import { SCROLL_TO_ITEM_END } from './shared/VirtualizedList.js';
import { UIStateContext, type UIState } from '../contexts/UIStateContext.js';
import {
UIActionsContext,
Expand Down Expand Up @@ -769,6 +770,40 @@ describe('<MainContent />', () => {
expect(lastFrame()).toMatch(/VP_ITEM:1[\s\S]*VP_ITEM:2/);
});

it('anchors to the top on a fresh session so the banner is not pushed to the bottom', () => {
scrollableListPropsSpy.mockClear();

// Fresh session: banner + a startup notice, but no conversation turn.
renderMainContent(
createUIState({
useTerminalBuffer: true,
history: [{ id: 1, type: 'info', text: 'Tips: try /help' }],
}),
);

expect(
scrollableListPropsSpy.mock.calls.at(-1)?.[0].initialScrollIndex,
).toBe(0);
});

it('scrolls to the end once a conversation turn exists', () => {
scrollableListPropsSpy.mockClear();

renderMainContent(
createUIState({
useTerminalBuffer: true,
history: [
{ id: 1, type: 'info', text: 'Tips: try /help' },
{ id: 2, type: 'user', text: 'hello' },
],
}),
);

expect(
scrollableListPropsSpy.mock.calls.at(-1)?.[0].initialScrollIndex,
).toBe(SCROLL_TO_ITEM_END);
});

it('keeps ShowMoreLines reachable in VP mode (regression of OverflowProvider misplacement)', () => {
const { lastFrame } = renderMainContent(
createUIState({
Expand Down
17 changes: 14 additions & 3 deletions packages/cli/src/ui/components/MainContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,19 @@ export const MainContent = () => {
uiState.availableTerminalHeight ?? 0,
);

// Anchor the initial view to the top until there is an actual conversation
// turn. On a fresh session the list holds only the banner plus startup
// notices (tips / MOTD / info), which is length > 1 — so keying the anchor
// off length alone made it scroll-to-end and pin the banner to the bottom
// of the viewport, leaving the top half of the VP screen blank. Once the
// user submits (or a response streams in) we resume scroll-to-end so the
// latest output stays in view.
const hasConversationTurn =
pendingHistoryItems.length > 0 ||
visibleHistory.some(
(item) => item.type === 'user' || item.type === 'user_shell',
);
Comment thread
chiga0 marked this conversation as resolved.
Resolved

return (
<OverflowProvider>
<ScrollableList
Expand All @@ -422,9 +435,7 @@ export const MainContent = () => {
renderItem={renderVirtualItem}
estimatedItemHeight={virtualEstimatedItemHeight}
keyExtractor={virtualKeyExtractor}
initialScrollIndex={
allVirtualItems.length <= 1 ? 0 : SCROLL_TO_ITEM_END
}
initialScrollIndex={hasConversationTurn ? SCROLL_TO_ITEM_END : 0}
isStaticItem={virtualIsStaticItem}
containerHeight={scrollContainerHeight}
showScrollbar={showScrollbar}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,67 @@ 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);
});
});

describe('<ThinkMessageContent />', () => {
Expand Down
30 changes: 27 additions & 3 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,6 +296,18 @@ 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) {
Expand All @@ -309,7 +322,18 @@ const ThinkBody: React.FC<{
),
)
: MAX_STREAMING_THINKING_VISUAL_LINES;
const display = tailVisualLines(text, innerWidth, maxLines);
const lines = tailVisualLines(text, innerWidth, maxLines);
const target = Math.min(
maxLines,
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
31 changes: 27 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,20 @@ 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`);
Comment thread
chiga0 marked this conversation as resolved.
Resolved

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.
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