From 53132e3a38ec09bc621223bdae411a4306599aa5 Mon Sep 17 00:00:00 2001 From: arkon Date: Sun, 22 Feb 2026 10:24:03 +0100 Subject: [PATCH] fix: address 7 issues found in xterm-zerolag-input audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Regex global flag safety — strip `g` flag before matching to prevent lastIndex mutation and missing .index on match results 2. state.visible false before activate — null overlay no longer reports visible: true 3. removeChar() returns 'pending' | 'flushed' | false instead of boolean so consumers can distinguish whether to send backspace to PTY 4. removeChar() implements buffer detection cascade (step 3) — detects existing prompt text when both pending and flushed are empty 5. RenderKey includes text content, not just length — prevents stale renders when setFlushed() called with same count but different text 6. Remove dead `import type { Terminal }` from test file 7. Remove internal `FontStyle` from public exports Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/xterm-zerolag-input/README.md | 6 +-- packages/xterm-zerolag-input/src/index.ts | 1 - .../xterm-zerolag-input/src/prompt-finder.ts | 8 +++- .../src/zerolag-input-addon.ts | 45 +++++++++++++----- .../test/prompt-finder.test.ts | 12 +++++ .../test/zerolag-input-addon.test.ts | 46 ++++++++++++++++--- 6 files changed, 94 insertions(+), 24 deletions(-) diff --git a/packages/xterm-zerolag-input/README.md b/packages/xterm-zerolag-input/README.md index 0aaf7a09..a8c582a3 100644 --- a/packages/xterm-zerolag-input/README.md +++ b/packages/xterm-zerolag-input/README.md @@ -40,8 +40,8 @@ terminal.onData((data) => { zerolag.clear(); ws.send(text + '\r'); } else if (data === '\x7f') { - const removed = zerolag.removeChar(); - if (removed) ws.send(data); + const source = zerolag.removeChar(); + if (source === 'flushed') ws.send(data); // only backspace text already in PTY } else if (data.length === 1 && data.charCodeAt(0) >= 32) { zerolag.addChar(data); // Don't send to server yet — wait for Enter @@ -115,7 +115,7 @@ Implements xterm.js `ITerminalAddon`. Load via `terminal.loadAddon(addon)`. |--------|-------------| | `addChar(char)` | Add a single printable character to the overlay | | `appendText(text)` | Append multiple characters (e.g., paste) | -| `removeChar(): boolean` | Remove last char. Returns `false` if nothing to remove | +| `removeChar(): 'pending' \| 'flushed' \| false` | Remove last char. Returns source (`'pending'` = unsent, `'flushed'` = send backspace to PTY) or `false` | | `clear()` | Clear all state and hide overlay | #### Flushed Text Tracking diff --git a/packages/xterm-zerolag-input/src/index.ts b/packages/xterm-zerolag-input/src/index.ts index 97d6acd1..c16416d9 100644 --- a/packages/xterm-zerolag-input/src/index.ts +++ b/packages/xterm-zerolag-input/src/index.ts @@ -7,5 +7,4 @@ export type { PromptFinder, PromptPosition, CellDimensions, - FontStyle, } from './types.js'; diff --git a/packages/xterm-zerolag-input/src/prompt-finder.ts b/packages/xterm-zerolag-input/src/prompt-finder.ts index 736f415f..90ea0cf0 100644 --- a/packages/xterm-zerolag-input/src/prompt-finder.ts +++ b/packages/xterm-zerolag-input/src/prompt-finder.ts @@ -27,11 +27,17 @@ export function findPrompt( } case 'regex': { + // Create a fresh non-global regex to avoid lastIndex mutation + // and ensure .match() returns a single result with .index + const pattern = finder.pattern; + const safePattern = pattern.global + ? new RegExp(pattern.source, pattern.flags.replace('g', '')) + : pattern; for (let row = terminal.rows - 1; row >= 0; row--) { const line = buffer.getLine(viewportTop + row); if (!line) continue; const text = line.translateToString(true); - const match = text.match(finder.pattern); + const match = text.match(safePattern); if (match) { const col = match.index ?? 0; return { row, col }; diff --git a/packages/xterm-zerolag-input/src/zerolag-input-addon.ts b/packages/xterm-zerolag-input/src/zerolag-input-addon.ts index 6b7bab43..283eeec9 100644 --- a/packages/xterm-zerolag-input/src/zerolag-input-addon.ts +++ b/packages/xterm-zerolag-input/src/zerolag-input-addon.ts @@ -48,7 +48,8 @@ const DEFAULT_CURSOR = '#e0e0e0'; * zerolag.clear(); * ws.send(text + '\r'); * } else if (data === '\x7f') { - * if (zerolag.removeChar()) ws.send(data); + * const source = zerolag.removeChar(); + * if (source === 'flushed') ws.send(data); // only send if text was already in PTY * } else if (data.length === 1 && data.charCodeAt(0) >= 32) { * zerolag.addChar(data); * } @@ -194,14 +195,19 @@ export class ZerolagInputAddon implements XtermAddon { * Remove the last character from the overlay. * * Cascade order: - * 1. Remove from `pendingText` if non-empty - * 2. Decrement `flushedOffset` if pending is empty but flushed exists - * 3. Try `detectBufferText()` if both are empty + * 1. Remove from `pendingText` if non-empty → returns `'pending'` + * 2. Decrement `flushedOffset` if pending is empty but flushed exists → returns `'flushed'` + * 3. Try `detectBufferText()` if both are empty, then decrement → returns `'flushed'` * - * @returns `true` if a character was removed, `false` if nothing to remove. - * When `false`, the consumer should NOT send backspace to the PTY. + * @returns The source of the removed character, or `false` if nothing to remove. + * + * - `'pending'`: A character was removed from unsent text. The consumer + * should NOT send backspace to the PTY (the text was never transmitted). + * - `'flushed'`: A character was removed from text already sent to the PTY. + * The consumer SHOULD send backspace to the PTY. + * - `false`: Nothing to remove. The consumer should NOT send backspace. */ - removeChar(): boolean { + removeChar(): 'pending' | 'flushed' | false { if (this._pendingText.length > 0) { this._pendingText = this._pendingText.slice(0, -1); if (this._pendingText.length > 0 || this._flushedOffset > 0) { @@ -209,7 +215,7 @@ export class ZerolagInputAddon implements XtermAddon { } else { this._hide(); } - return true; + return 'pending'; } if (this._flushedOffset > 0) { @@ -220,7 +226,21 @@ export class ZerolagInputAddon implements XtermAddon { } else { this._hide(); } - return true; + return 'flushed'; + } + + // Both empty — try detecting text already on the prompt line + // (handles tab completion, arrow-key edits, etc.) + this._detectBufferText(); + if (this._flushedOffset > 0) { + this._flushedOffset--; + this._flushedText = this._flushedText.slice(0, -1); + if (this._flushedOffset > 0) { + this._render(); + } else { + this._hide(); + } + return 'flushed'; } return false; @@ -369,7 +389,7 @@ export class ZerolagInputAddon implements XtermAddon { pendingText: this._pendingText, flushedLength: this._flushedOffset, flushedText: this._flushedText, - visible: this._overlay?.style.display !== 'none', + visible: this._overlay !== null && this._overlay.style.display !== 'none', promptPosition: this._lastPromptPos ? { ...this._lastPromptPos } : null, }; } @@ -500,8 +520,9 @@ export class ZerolagInputAddon implements XtermAddon { } } - // Skip redundant re-renders - const renderKey = `${displayText.length}:${startCol}:${activePrompt.row}:${activePrompt.col}:${totalCols}:${this._flushedOffset}`; + // Skip redundant re-renders — include text content to detect + // same-length changes (e.g., setFlushed with different text) + const renderKey = `${displayText}:${startCol}:${activePrompt.row}:${activePrompt.col}:${totalCols}:${this._flushedOffset}`; if (renderKey === this._lastRenderKey && this._overlay.style.display !== 'none') return; this._lastRenderKey = renderKey; diff --git a/packages/xterm-zerolag-input/test/prompt-finder.test.ts b/packages/xterm-zerolag-input/test/prompt-finder.test.ts index 08ea8133..8313e377 100644 --- a/packages/xterm-zerolag-input/test/prompt-finder.test.ts +++ b/packages/xterm-zerolag-input/test/prompt-finder.test.ts @@ -88,6 +88,18 @@ describe('findPrompt', () => { expect(pos).toBeNull(); cleanup(); }); + + it('handles global flag safely (strips g to avoid lastIndex)', () => { + const { terminal, cleanup } = term(['user@host:~$ cmd']); + const finder: PromptFinder = { type: 'regex', pattern: /\$/g }; + const pos = findPrompt(terminal as unknown as XtermTerminal, finder); + expect(pos).not.toBeNull(); + expect(pos!.col).toBe(11); + // Call again — should return same result (no lastIndex drift) + const pos2 = findPrompt(terminal as unknown as XtermTerminal, finder); + expect(pos2).toEqual(pos); + cleanup(); + }); }); describe('custom strategy', () => { diff --git a/packages/xterm-zerolag-input/test/zerolag-input-addon.test.ts b/packages/xterm-zerolag-input/test/zerolag-input-addon.test.ts index 43fb9a14..eb63b3c1 100644 --- a/packages/xterm-zerolag-input/test/zerolag-input-addon.test.ts +++ b/packages/xterm-zerolag-input/test/zerolag-input-addon.test.ts @@ -1,7 +1,6 @@ import { describe, it, expect, afterEach } from 'vitest'; import { createMockTerminal } from './helpers.js'; import { ZerolagInputAddon } from '../src/zerolag-input-addon.js'; -import type { Terminal } from '../src/types.js'; function setup(lines: string[] = ['$ '], promptChar = '$') { const mock = createMockTerminal({ buffer: { lines } }); @@ -83,12 +82,12 @@ describe('ZerolagInputAddon', () => { }); describe('removeChar', () => { - it('removes last character from pendingText', () => { + it('returns "pending" when removing from pendingText', () => { const { addon } = tracked(); addon.addChar('a'); addon.addChar('b'); - const removed = addon.removeChar(); - expect(removed).toBe(true); + const source = addon.removeChar(); + expect(source).toBe('pending'); expect(addon.pendingText).toBe('a'); }); @@ -97,10 +96,11 @@ describe('ZerolagInputAddon', () => { expect(addon.removeChar()).toBe(false); }); - it('decrements flushed when pending is empty', () => { + it('returns "flushed" when removing from flushed text', () => { const { addon } = tracked(); addon.setFlushed(3, 'abc'); - expect(addon.removeChar()).toBe(true); + const source = addon.removeChar(); + expect(source).toBe('flushed'); expect(addon.getFlushed().count).toBe(2); expect(addon.getFlushed().text).toBe('ab'); }); @@ -109,7 +109,8 @@ describe('ZerolagInputAddon', () => { const { addon } = tracked(); addon.setFlushed(2, 'ab'); addon.addChar('c'); - expect(addon.removeChar()).toBe(true); + const source = addon.removeChar(); + expect(source).toBe('pending'); expect(addon.pendingText).toBe(''); expect(addon.getFlushed().count).toBe(2); // flushed unchanged }); @@ -120,6 +121,21 @@ describe('ZerolagInputAddon', () => { addon.removeChar(); expect(addon.hasPending).toBe(false); }); + + it('detects buffer text and removes from it when both empty', () => { + const { addon } = tracked(['$ hello']); + // Both pending and flushed are empty, but buffer has text + const source = addon.removeChar(); + expect(source).toBe('flushed'); + // "hello" (5 chars) detected, then one removed = 4 + expect(addon.getFlushed().count).toBe(4); + expect(addon.getFlushed().text).toBe('hell'); + }); + + it('returns false on empty prompt with no buffer text', () => { + const { addon } = tracked(['$ ']); + expect(addon.removeChar()).toBe(false); + }); }); describe('clear', () => { @@ -276,6 +292,22 @@ describe('ZerolagInputAddon', () => { }); }); + describe('state.visible', () => { + it('is false before activate', () => { + const addon = new ZerolagInputAddon(); + expect(addon.state.visible).toBe(false); + // No cleanup needed — never activated + }); + + it('is false after dispose', () => { + const { addon, mock } = tracked(); + addon.addChar('x'); + addon.dispose(); + expect(addon.state.visible).toBe(false); + mock.cleanup(); + }); + }); + describe('rerender / refreshFont', () => { it('rerender does not crash when no text', () => { const { addon } = tracked();