Skip to content

fix(terminal): stop idle-Enter keyword highlight flash after #2880 hole #2931

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 5 commits into from
Aug 12, 2026
Merged

fix(terminal): stop idle-Enter keyword highlight flash after #2880 hole #2931

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cd12b04
fix(#2879): automated Cursor CLI fix
netcatty-bot Aug 12, 2026
9cd756f
fix: address Codex review on PR #2931
netcatty-bot Aug 12, 2026
7408e96
fix: address Codex review on PR #2931
netcatty-bot Aug 12, 2026
60820ac
fix: address Codex review on PR #2931
netcatty-bot Aug 12, 2026
d1cd072
fix: address Codex review on PR #2931
netcatty-bot Aug 12, 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
203 changes: 201 additions & 2 deletions components/terminal/keywordHighlight.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1417,7 +1417,9 @@ test("Enter dirty work continues after a queued full refresh", async () => {
handlers.data?.("\r");
setLineText(22, "redrawn without a keyword");
handlers.writeParsed?.();
await new Promise((resolve) => { setTimeout(resolve, 340); });
// Idle Enter suppresses decoration mutation until the Enter guard clears
// (~600ms) so prompt redraw cannot flash still-visible keywords.
await new Promise((resolve) => { setTimeout(resolve, 850); });
raf.flush();

const originalDisposed = originalDecoration.isDisposed;
Expand Down Expand Up @@ -1923,7 +1925,9 @@ test("Enter input still detects redraws away from the cursor", async () => {
handlers.data?.("\r");
setLineText(20, "redrawn without a keyword");
handlers.writeParsed?.();
await new Promise((resolve) => { setTimeout(resolve, 220); });
// Idle Enter defers decoration dispose/apply until the Enter guard clears.
await new Promise((resolve) => { setTimeout(resolve, 850); });
raf.flush();

assert.equal(originalDecoration.isDisposed, true);
highlighter.dispose();
Expand Down Expand Up @@ -2245,6 +2249,201 @@ test("idle Enter scroll before writeParsed does not rescan visible keywords", ()
}
});

test("idle Enter scroll before buffer dims update does not rescan", () => {
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);

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();

// Ubuntu RTT: onScroll can fire while length/baseY/cursor still match the
// last snapshot, so output-driven detection is false. Bottom-pinned Enter
// must still defer — requiring hasOutputDrivenViewportChange reopens flash.
handlers.data?.("\r");
handlers.scroll?.();

assert.equal(
getTranslateCount(),
0,
"Enter-pending bottom scroll without buffer-dim change must not rescan",
);
assert.deepEqual(
refreshCalls,
[],
"Enter-pending bottom scroll without buffer-dim change must not repaint",
);
assert.equal(
existingDecorations.filter(({ isDisposed }) => isDisposed).length,
0,
"Enter-pending bottom scroll must keep existing keyword decorations mounted",
);
highlighter.dispose();
} finally {
raf.restore();
}
});

test("idle Enter prompt redraw does not repaint existing keyword rows", async () => {
const raf = installAnimationFrameQueue();
try {
const {
term,
decorationStates,
handlers,
setLineText,
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,
},
{
id: "prompt",
label: "Prompt",
patterns: ["~", "#"],
color: "#60A5FA",
enabled: true,
},
], true);
raf.flush();
const existingDecorations = [...decorationStates];
assert.ok(existingDecorations.length > 0);
resetRefreshCalls();

handlers.data?.("\r");
term.buffer.active.viewportY += 1;
term.buffer.active.baseY += 1;
term.buffer.active.length += 1;
// New prompt line matches custom ~/# rules — applying those decorations
// makes xterm repaint the full viewport and flashes still-visible keywords.
setLineText(22, "user@host:~# ");
handlers.scroll?.();
handlers.writeParsed?.();
await new Promise((resolve) => { setTimeout(resolve, 220); });
raf.flush();

assert.equal(
existingDecorations.filter(({ isDisposed }) => isDisposed).length,
0,
"idle Enter must keep prior keyword decorations mounted",
);
assert.deepEqual(
refreshCalls,
[],
"idle Enter prompt redraw must not register decorations that force a viewport repaint",
);
highlighter.dispose();
} finally {
raf.restore();
}
});

test("idle Enter keeps suppression across split echo and prompt writes", async () => {
const raf = installAnimationFrameQueue();
try {
const {
term,
decorationStates,
handlers,
setLineText,
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,
},
{
id: "prompt",
label: "Prompt",
patterns: ["~", "#"],
color: "#60A5FA",
enabled: true,
},
], true);
raf.flush();
const existingDecorations = [...decorationStates];
assert.ok(existingDecorations.length > 0);
resetRefreshCalls();

handlers.data?.("\r");
// Batch 1: newline echo advances the buffer.
term.buffer.active.viewportY += 1;
term.buffer.active.baseY += 1;
term.buffer.active.length += 1;
handlers.scroll?.();
handlers.writeParsed?.();
await new Promise((resolve) => { setTimeout(resolve, 40); });
raf.flush();

// Batch 2: prompt redraw for the same idle Enter (custom ~/# matches).
setLineText(22, "user@host:~# ");
handlers.writeParsed?.();
await new Promise((resolve) => { setTimeout(resolve, 220); });
raf.flush();

assert.equal(
existingDecorations.filter(({ isDisposed }) => isDisposed).length,
0,
"split idle-Enter writes must keep prior keyword decorations mounted",
);
assert.deepEqual(
refreshCalls,
[],
"a second writeParsed alone must not register prompt decorations that flash the viewport",
);
highlighter.dispose();
} finally {
raf.restore();
}
});

test("Enter without write clears pending so later user scroll can highlight", async () => {
const raf = installAnimationFrameQueue();
try {
Expand Down
81 changes: 73 additions & 8 deletions components/terminal/keywordHighlight.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,17 @@ export class KeywordHighlighter implements IDisposable {
private enterQueuedWriteCancellationPending = false;
private enterViewportScanInProgress = false;
private enterViewportScanNeedsRepeat = false;
/** True while idle-Enter output should not mutate decorations (xterm full-viewport repaint flash). */
private enterSuppressDecorationMutation = false;
/**
* onWriteParsed count for the current Enter submission. Echo + prompt often
* arrive as two batches; a second callback alone is not sustained output.
*/
private enterWriteParsedCount = 0;
/** Lift idle-Enter decoration mute after this many writeParsed callbacks. */
private static readonly ENTER_SUPPRESS_LIFT_WRITE_COUNT = 3;
/** Viewport browsing state before the latest scroll handler ran. */
private wasBrowsingScrollback = false;
private static readonly DIRTY_SCAN_PADDING = XTERM_PERFORMANCE_CONFIG.highlighting.dirtyScanPadding;
private static readonly INPUT_QUIET_MS = XTERM_PERFORMANCE_CONFIG.highlighting.inputQuietMs;
private static readonly WRITE_BURST_INTERVAL_MS = 28;
Expand Down Expand Up @@ -154,6 +165,11 @@ export class KeywordHighlighter implements IDisposable {
if (data.includes("\r") || data.includes("\n")) {
this.enterInputPending = true;
this.enterQueuedWriteCancellationPending = true;
// First prompt redraw after Enter must not register/dispose decorations:
// xterm repaints the full viewport on decoration mutation and flashes
// still-visible keyword highlights (custom ~/# rules make this obvious).
this.enterSuppressDecorationMutation = true;
this.enterWriteParsedCount = 0;
// Time-bound Enter protection even when no echo/write arrives (echo
// off, stalled PTY). onWriteParsed re-arms this on each write.
this.scheduleEnterInputIdleClear();
Expand Down Expand Up @@ -224,11 +240,27 @@ export class KeywordHighlighter implements IDisposable {
const inputProtectionActive = this.isInputProtectionActive(performance.now());
if (inputProtectionActive || this.enterInputPending) {
if (this.enterInputPending) {
if (this.enterViewportScanInProgress) {
if (this.enterWriteParsedCount > 0) {
this.enterWriteParsedCount += 1;
this.updateWriteBurst();
this.enterViewportScanNeedsRepeat = true;
// Idle Enter often splits echo and prompt across two writeParsed
// batches. Keep decoration mute until sustained output (burst or
// enough follow-up writes) or the Enter idle guard clears.
if (this.shouldLiftEnterDecorationSuppression()) {
this.enterSuppressDecorationMutation = false;
Comment on lines +254 to +255

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 12, 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 Reset burst evidence for each Enter

When Enter follows another write burst within the burst-decay window, recentWriteBurst and lastWriteAt still describe the earlier output because the Enter handler does not reset them. A split echo/prompt can therefore make this predicate true on its second batch, lift suppression, and recreate the prompt-decoration viewport flash. The fresh evidence in this revision is that the new burst-based signal remains global rather than being scoped to writes observed after the current Enter.

Useful? React with 👍 / 👎.

}
if (this.enterViewportScanInProgress) {
this.enterViewportScanNeedsRepeat = true;
} else {
const buffer = this.term.buffer.active;
this.addDirtyRange(buffer.viewportY, buffer.viewportY + this.term.rows - 1);
this.enterViewportScanInProgress = true;
}
} else {
this.enterWriteParsedCount = 1;
this.markDirtyFromWrite({ includeViewportProbe: false });
// Index the viewport while muted so multi-frame Enter continuation
// can finish; decoration mutate stays suppressed for idle prompt.
const buffer = this.term.buffer.active;
this.addDirtyRange(buffer.viewportY, buffer.viewportY + this.term.rows - 1);
this.enterViewportScanInProgress = true;
Expand Down Expand Up @@ -266,6 +298,7 @@ export class KeywordHighlighter implements IDisposable {
})
);
this.lastBufferSnapshot = this.readBufferSnapshot();
this.wasBrowsingScrollback = this.isBrowsingScrollback();
}

public setRules(rules: readonly RuntimeKeywordHighlightRule[], enabled: boolean) {
Expand Down Expand Up @@ -430,6 +463,8 @@ export class KeywordHighlighter implements IDisposable {
this.enterQueuedWriteCancellationPending = false;
this.enterViewportScanInProgress = false;
this.enterViewportScanNeedsRepeat = false;
this.enterSuppressDecorationMutation = false;
this.enterWriteParsedCount = 0;
if (hadDecorations) {
this.term.refresh(0, this.term.rows - 1);
}
Expand Down Expand Up @@ -715,16 +750,23 @@ export class KeywordHighlighter implements IDisposable {

private triggerViewportChangeRefresh() {
const isBrowsingScrollback = this.isBrowsingScrollback();
// End (or other jump-to-bottom) while Enter is pending: we were browsing
// scrollback and just landed on the bottom without waiting for echo.
// Distinguish that from idle-Enter echo still pinned at the bottom.
const returningToBottomFromScrollback =
this.wasBrowsingScrollback && !isBrowsingScrollback;
this.wasBrowsingScrollback = 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.
// Keep real scrollback browsing synchronous; defer bottom-pinned Enter
// echo even when buffer dims have not updated yet (Ubuntu RTT). Do not
// require hasOutputDrivenViewportChange — that hole reopened the flash.
if (
this.enterInputPending
&& !isBrowsingScrollback
&& this.hasOutputDrivenViewportChange()
&& !returningToBottomFromScrollback
) {
if (this.pendingRefreshReason === "scroll") {
this.cancelQueuedRefreshSchedule();
Expand Down Expand Up @@ -1086,13 +1128,24 @@ export class KeywordHighlighter implements IDisposable {
this.enterInputIdleTimer = setTimeout(() => {
this.enterInputIdleTimer = null;
this.enterInputPending = false;
this.enterSuppressDecorationMutation = false;
this.enterWriteParsedCount = 0;
// 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");
}, KeywordHighlighter.ENTER_INPUT_GUARD_MS);
}

/** True when follow-up Enter writes look like real command output, not a split prompt. */
private shouldLiftEnterDecorationSuppression(): boolean {
if (!this.enterSuppressDecorationMutation) return false;
if (this.enterWriteParsedCount >= KeywordHighlighter.ENTER_SUPPRESS_LIFT_WRITE_COUNT) {
return true;
}
return this.isWriteBurstActive(performance.now());

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 12, 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 Keep suppression through all prompt-write batches

When a prompt is emitted in three onWriteParsed batches—for example separate control-sequence, prompt-text, and mode-setting writes—this fixed callback threshold lifts suppression before the final prompt batch is scanned, allowing matching ~/# decorations to trigger the same full-viewport repaint. The fresh evidence in this revision is that the previous second-write trigger was replaced by an equally non-semantic third-write trigger; callback count still does not prove command output, so suppression should remain until an actual output or quiet criterion distinguishes the prompt.

Useful? React with 👍 / 👎.

}

private isBrowsingScrollback(): boolean {
const buffer = this.term.buffer.active;
return buffer.viewportY < buffer.baseY;
Expand Down Expand Up @@ -1519,13 +1572,17 @@ export class KeywordHighlighter implements IDisposable {
for (let lineY = start; lineY <= end; lineY++) {
const line = buffer.getLine(lineY);
if (!line) {
this.disposeLineDecorations(lineY);
if (!this.enterSuppressDecorationMutation) {
this.disposeLineDecorations(lineY);
}
continue;
}

const lineText = line.translateToString(true); // true = trim right whitespace
if (!lineText) {
this.disposeLineDecorations(lineY);
if (!this.enterSuppressDecorationMutation) {
this.disposeLineDecorations(lineY);
}
continue;
}

Expand All @@ -1536,7 +1593,9 @@ export class KeywordHighlighter implements IDisposable {
: this.scanWrappedLine(buffer, lineY, line, lineText, wrappedBlockCache)
: this.getCachedRanges(line, lineText);
if (cachedRanges.length === 0) {
this.disposeLineDecorations(lineY);
if (!this.enterSuppressDecorationMutation) {
this.disposeLineDecorations(lineY);
}
continue;
}

Expand All @@ -1553,6 +1612,12 @@ export class KeywordHighlighter implements IDisposable {
continue;
}

// Idle Enter: keep prior decorations mounted and skip new ones until the
// guard clears or sustained Enter output opts back into mutation.
if (this.enterSuppressDecorationMutation) {
continue;
}
Comment on lines +1638 to +1645

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 12, 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 Apply highlights when users browse scrollback during Enter

When a user presses Enter and immediately scrolls to previously unindexed scrollback, the synchronous scroll refresh reaches this guard and skips every new decoration because Enter suppression is still active. processScrollViewport nevertheless records the range as covered, so matching lines remain visibly unhighlighted until the 600 ms Enter guard plus debounce expires; suppression should preserve existing prompt-area decorations without blocking decoration creation for a real scrollback browse.

Useful? React with 👍 / 👎.


this.disposeLineDecorations(lineY, existing);
this.applyLineDecorations(lineY, cachedRanges, signature, cursorAbsoluteY);
}
Expand Down
Loading