Skip to content

fix(terminal): defer keyword scroll refresh while Enter is pending #2880

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
binaricat merged 4 commits into from
Aug 11, 2026
Merged

fix(terminal): defer keyword scroll refresh while Enter is pending #2880

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
315e596
fix(#2879): automated Cursor CLI fix
netcatty-bot Aug 11, 2026
4935066
fix: address Codex review on PR #2880
netcatty-bot Aug 11, 2026
6c4e27c
fix(terminal): keep scrollback responsive during Enter guard
binaricat Aug 11, 2026
fbaa038
fix(terminal): preserve End scroll during Enter guard
binaricat Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 177 additions & 1 deletion components/terminal/keywordHighlight.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1121,7 +1121,13 @@ test("Enter-driven scroll does not dispose nearby keyword decorations", async ()
test("user scroll during Enter keeps prior highlights mounted", async () => {
const raf = installAnimationFrameQueue();
try {
const { term, decorationStates, handlers } = createFakeTerminal("hello DEPLOY world", {
const {
term,
decorationStates,
handlers,
getTranslatedLineIndexes,
resetTranslateCount,
} = createFakeTerminal("hello DEPLOY world", {
lineCount: 80,
});
term.rows = 3;
Expand All @@ -1143,8 +1149,15 @@ test("user scroll during Enter keeps prior highlights mounted", async () => {
handlers.data?.("\r");
handlers.writeParsed?.();
term.buffer.active.viewportY = 10;
resetTranslateCount();
handlers.scroll?.();

assert.ok(
getTranslatedLineIndexes().some((lineY) => lineY >= 10 && lineY < 20),
"scrollback browsing during Enter should synchronously scan newly revealed lines",
);
raf.flush();

assert.equal(
existingDecorations.filter(({ isDisposed }) => isDisposed).length,
0,
Expand All @@ -1156,6 +1169,46 @@ test("user scroll during Enter keeps prior highlights mounted", async () => {
}
});

test("output during Enter does not cancel an active scrollback browse", () => {
const raf = installAnimationFrameQueue();
try {
const { term, handlers } = createFakeTerminal("hello DEPLOY world", { lineCount: 80 });
term.buffer.active.viewportY = 20;
term.buffer.active.baseY = 20;
term.buffer.active.cursorY = 2;
const highlighter = new KeywordHighlighter(term as never);
highlighter.setRules([{
id: "deploy",
label: "Deploy",
patterns: ["DEPLOY"],
color: "#F87171",
enabled: true,
}], true);
raf.flush();

handlers.data?.("\r");
term.buffer.active.viewportY = 10;
const internals = highlighter as unknown as {
pendingRefreshReason: "scroll" | "write" | "full";
};
// Model a scroll refresh that is pending while the user is browsing
// scrollback, then let remote output move the bottom of the buffer.
internals.pendingRefreshReason = "scroll";
term.buffer.active.baseY += 1;
term.buffer.active.length += 1;
handlers.writeParsed?.();

assert.equal(
internals.pendingRefreshReason,
"scroll",
"remote output must not reclassify an active scrollback browse as Enter output",
);
highlighter.dispose();
} finally {
raf.restore();
}
});

test("large output delays keyword highlight scans until output quiets", async () => {
const raf = installAnimationFrameQueue();
try {
Expand Down Expand Up @@ -2083,6 +2136,129 @@ test("pressing Enter does not repaint after keyword markers move", async () => {
}
});

test("idle Enter scroll before writeParsed does not rescan visible keywords", () => {
const raf = installAnimationFrameQueue();
try {
const {
term,
decorationStates,
handlers,
getTranslateCount,
resetTranslateCount,
refreshCalls,
resetRefreshCalls,
} = createFakeTerminal("hello DEPLOY world", { lineCount: 40 });
term.buffer.active.viewportY = 20;
term.buffer.active.baseY = 20;
term.buffer.active.cursorY = 2;
const highlighter = new KeywordHighlighter(term as never);
highlighter.setRules([{
id: "deploy",
label: "Deploy",
patterns: ["DEPLOY"],
color: "#F87171",
enabled: true,
}], true);
raf.flush();
const existingDecorations = [...decorationStates];
assert.ok(existingDecorations.length > 0);

// Ordinary write refreshes clear lastRenderRange. An idle prompt then has no
// scroll coverage hint, so Enter echo onScroll (before writeParsed, Ubuntu RTT)
// would otherwise take the immediate user-scroll path and rescan the viewport.
const internals = highlighter as unknown as {
lastWriteAt: number;
lastRenderRange: { start: number; end: number } | null;
};
internals.lastWriteAt = performance.now() - 10_000;
internals.lastRenderRange = null;
resetTranslateCount();
resetRefreshCalls();

handlers.data?.("\r");
term.buffer.active.viewportY += 1;
term.buffer.active.baseY += 1;
term.buffer.active.length += 1;
handlers.scroll?.();

assert.equal(
getTranslateCount(),
0,
"Enter-pending scroll before writeParsed must not rescan visible keywords",
);
assert.deepEqual(
refreshCalls,
[],
"Enter-pending scroll before writeParsed must not force a keyword repaint",
);
assert.equal(
existingDecorations.filter(({ isDisposed }) => isDisposed).length,
0,
"Enter-pending scroll must keep existing keyword decorations mounted",
);
highlighter.dispose();
} finally {
raf.restore();
}
});

test("Enter without write clears pending so later user scroll can highlight", async () => {
const raf = installAnimationFrameQueue();
try {
const {
term,
handlers,
getTranslateCount,
resetTranslateCount,
} = createFakeTerminal("hello DEPLOY world", { lineCount: 40 });
term.buffer.active.viewportY = 20;
term.buffer.active.baseY = 20;
term.buffer.active.cursorY = 2;
const highlighter = new KeywordHighlighter(term as never);
highlighter.setRules([{
id: "deploy",
label: "Deploy",
patterns: ["DEPLOY"],
color: "#F87171",
enabled: true,
}], true);
raf.flush();

const internals = highlighter as unknown as {
enterInputPending: boolean;
lastWriteAt: number;
lastRenderRange: { start: number; end: number } | null;
};
internals.lastWriteAt = performance.now() - 10_000;
internals.lastRenderRange = null;

// Enter with no echo/writeParsed (echo off / stalled PTY).
handlers.data?.("\r");
assert.equal(internals.enterInputPending, true);

await new Promise((resolve) => { setTimeout(resolve, 700); });
assert.equal(
internals.enterInputPending,
false,
"Enter protection must time out when no write arrives",
);

// User browsing scrollback after the timed-out Enter guard.
resetTranslateCount();
term.buffer.active.viewportY = 10;
handlers.scroll?.();
raf.flush();

assert.ok(
getTranslateCount() > 0,
"user scroll after timed-out Enter must scan newly revealed scrollback",
);
highlighter.dispose();
} finally {
raf.restore();
}
});

test("long-line pressure avoids scanning across a whole soft-wrapped logical line", () => {
const raf = installAnimationFrameQueue();
try {
Expand Down
43 changes: 37 additions & 6 deletions components/terminal/keywordHighlight.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ export class KeywordHighlighter implements IDisposable {
private static readonly WRITE_BURST_DEBOUNCE_MS = 180;
private static readonly WRITE_BURST_IMMEDIATE_MIN_INTERVAL_MS = 48;
private static readonly WRITE_BURST_HIGHLIGHT_PAUSE_MS = 260;
private static readonly ENTER_INPUT_GUARD_MS = 600;
private static readonly WRITE_PRUNE_IDLE_MS = 600;

constructor(term: XTerm) {
Expand All @@ -153,9 +154,15 @@ export class KeywordHighlighter implements IDisposable {
if (data.includes("\r") || data.includes("\n")) {
this.enterInputPending = true;
this.enterQueuedWriteCancellationPending = true;
if (this.enterInputIdleTimer) {
clearTimeout(this.enterInputIdleTimer);
this.enterInputIdleTimer = null;
// Time-bound Enter protection even when no echo/write arrives (echo
// off, stalled PTY). onWriteParsed re-arms this on each write.
this.scheduleEnterInputIdleClear();
// Drop any pending user-scroll refresh so Enter echo cannot finish a
// scroll pass that rescans/repaints still-visible keyword decorations
// before onWriteParsed owns the write path (Ubuntu RTT).
if (this.pendingRefreshReason === "scroll" && !this.isBrowsingScrollback()) {
this.cancelQueuedRefreshSchedule();
this.pendingRefreshReason = "write";
}
}
}),
Expand All @@ -165,8 +172,10 @@ export class KeywordHighlighter implements IDisposable {
if (this.enterInputPending) {
this.scheduleEnterInputIdleClear();
}
const isBrowsingScrollback = this.isBrowsingScrollback();
const outputDrivenPendingScroll =
this.pendingRefreshReason === "scroll"
&& !isBrowsingScrollback
&& (
this.hasOutputPositionChangedSinceLastSnapshot()
|| this.hasDecorationMarkerShiftSinceLastRefresh()
Expand Down Expand Up @@ -706,9 +715,22 @@ export class KeywordHighlighter implements IDisposable {
}

private triggerViewportChangeRefresh() {
const isBrowsingScrollback = this.isBrowsingScrollback();
// Enter echo often emits onScroll before onWriteParsed. After an idle gap
// lastWriteAt looks stale and lastRenderRange is usually null (cleared by
// the previous write refresh), so the output-driven scroll path would
// synchronously rescan the viewport and flash keywords still on screen.
// Keep real scrollback browsing synchronous; only defer the bottom-pinned
// viewport movement that can be caused by the pending Enter echo.
if (this.enterInputPending && !isBrowsingScrollback) {

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve synchronous scans when scrolling back to bottom

When a user reaches the bottom while the Enter guard is active—for example, by pressing End after first scrolling upward—viewportY === baseY makes this branch classify the user action as Enter-driven output. It marks the viewport dirty and returns without scanning it; if no write follows, the newly revealed bottom viewport is not refreshed until the 600 ms guard timer plus the debounce delay expires. Distinguish an actual output-driven viewport change from a user scroll that happens to end at the bottom so the latter retains the synchronous scroll refresh path.

Useful? React with 👍 / 👎.

if (this.pendingRefreshReason === "scroll") {
this.cancelQueuedRefreshSchedule();
this.pendingRefreshReason = "write";
}
this.markVisibleRangeDirty();
return;
Comment on lines +733 to +734

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bound Enter-pending scroll suppression without output

When the remote PTY produces no write after Enter—for example, with echo disabled or a stalled connection—onWriteParsed never runs. enterInputPending is set by onData, but its only clear timer is armed inside onWriteParsed, so this new early return suppresses every subsequent user-scroll refresh indefinitely. Newly revealed scrollback lines therefore remain unscanned and unhighlighted until unrelated output arrives; arm a fallback timer from onData or otherwise time-bound this guard.

Useful? React with 👍 / 👎.

}
const now = performance.now();
const buffer = this.term.buffer.active;
const isBrowsingScrollback = buffer.viewportY < buffer.baseY;
const isOutputDrivenViewportChange =
!isBrowsingScrollback &&
this.lastWriteAt > 0 &&
Expand Down Expand Up @@ -1061,7 +1083,16 @@ export class KeywordHighlighter implements IDisposable {
this.enterInputIdleTimer = setTimeout(() => {
this.enterInputIdleTimer = null;
this.enterInputPending = false;
}, KeywordHighlighter.WRITE_PRUNE_IDLE_MS);
// Catch up any viewport motion deferred while Enter protection blocked
// scroll refresh (e.g. user scrolled during the post-Enter window).
this.markVisibleRangeDirty();
this.triggerRefresh("debounced", "write");

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor bulk-output pressure before Enter catch-up

When Enter launches output that trips large-output pressure while scrollback is saturated, the pressure state intentionally remains active for two largeOutputQuietMs windows (about 960 ms), and scheduleBulkPressureCatchUp() polls until pressure.largeOutput is false before scanning. This new Enter idle timer fires at 600 ms and schedules a normal write refresh anyway; under the configured large-output debounce that can execute at about 880 ms, reintroducing keyword scans/decoration work during the protected bulk window. Please route this catch-up through the bulk-pressure path or skip scheduling while output pressure is still active.

Useful? React with 👍 / 👎.

}, KeywordHighlighter.ENTER_INPUT_GUARD_MS);
}

private isBrowsingScrollback(): boolean {
const buffer = this.term.buffer.active;
return buffer.viewportY < buffer.baseY;
}

private mergeRefreshReason(current: RefreshReason, next: RefreshReason): RefreshReason {
Expand Down