From d6d9315fb6264db40cba4b794c16b745c3ce4cd9 Mon Sep 17 00:00:00 2001 From: JD Date: Wed, 14 Oct 2026 21:10:09 -0400 Subject: [PATCH 1/2] fix(mobile): find the caret line from the absolute cursor row on a parked viewport xterm's cursorY is relative to baseY, but _tapIsOnCaretLine read it as a screen row. With the viewport parked above the bottom (a tab switch onto a frame with trailing blank rows) a tap on a URL typed in the prompt opened it, and a URL in scrollback on the row cursorY happened to name stayed inert. --- src/web/public/terminal-ui.js | 20 +++++++++++--------- test/terminal-touch-tap.test.ts | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 9c1ba763..3d5af700 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -2356,15 +2356,17 @@ Object.assign(CodemanApp.prototype, { if (!buffer?.getLine) return false; const pos = this._clientPointToCell(clientX, clientY); if (!pos) return false; - const rows = Math.max(1, this.terminal.rows || 1); - const cursorRow = Math.max(0, Math.min(rows - 1, buffer.cursorY || 0)); - const tappedRow = pos.row - 1; - if (tappedRow === cursorRow) return true; - let start = cursorRow; - while (start > 0 && buffer.getLine(buffer.viewportY + start)?.isWrapped) start--; - let end = cursorRow; - while (end + 1 < rows && buffer.getLine(buffer.viewportY + end + 1)?.isWrapped) end++; - return tappedRow >= start && tappedRow <= end; + // Compare ABSOLUTE buffer lines: cursorY is baseY-relative and the tap is + // viewport-relative, so they only agree while the viewport sits at the bottom + // (a tab switch parks it above, see scrollToLastNonEmptyLine). + const cursorLine = buffer.baseY + (buffer.cursorY || 0); + const tappedLine = buffer.viewportY + pos.row - 1; + let start = cursorLine; + while (start > 0 && buffer.getLine(start)?.isWrapped) start--; + let end = cursorLine; + // Bounded by length: xterm's getLine wraps around its circular list past the end. + while (end + 1 < buffer.length && buffer.getLine(end + 1)?.isWrapped) end++; + return tappedLine >= start && tappedLine <= end; }, /** diff --git a/test/terminal-touch-tap.test.ts b/test/terminal-touch-tap.test.ts index 94005e2c..6460b1a0 100644 --- a/test/terminal-touch-tap.test.ts +++ b/test/terminal-touch-tap.test.ts @@ -1096,6 +1096,39 @@ describe('terminal link tap', () => { expect(sent).toHaveLength(1); // the tap still positions the caret via the mouse report }); + it('finds the caret line from the absolute cursor row while the viewport is parked above the bottom', () => { + // A tab switch parks the viewport above baseY when the frame keeps trailing blank + // rows (scrollToLastNonEmptyLine). cursorY is baseY-relative, so read as a screen + // row it points 2 rows too high here: at the history URL, not the composer. + const history = 'docs at https://example.com/guide'; + const composer = '❯ summarize https://example.com/typed for me'; + const lines = ['', '', '', 'older', 'old', history, 'more', composer, '', '']; + const { app, windowRef } = linkHarness(lines); + Object.assign(app.terminal, { rows: 4 }); + Object.assign(app.terminal.buffer.active, { baseY: 6, viewportY: 4, cursorY: 1 }); + + // Screen row 3 is absolute line 7, the composer: the typed URL stays editable. + expect(app._handleMobileTerminalTap(at(composer.indexOf('https'), 3), true, 'history')).toBe('history'); + expect(windowRef.open).not.toHaveBeenCalled(); + + // Screen row 1 is absolute line 5, scrollback: its URL opens. + expect(app._handleMobileTerminalTap(at(history.indexOf('https'), 1), true, 'history')).toBe('link'); + expect(windowRef.open).toHaveBeenCalledWith('https://example.com/guide', '_blank', 'noopener,noreferrer'); + }); + + it('stops the caret-line wrap walk at the buffer end even when every row is wrapped', () => { + // xterm's getLine indexes its circular list modulo capacity, so past the last row it + // returns earlier rows again; with all of them soft-wrapped an unbounded walk never ends. + const { app } = linkHarness(['❯ a', 'b', 'c'], 2); + let reads = 0; + app.terminal.buffer.active.getLine = (row: number) => { + if (++reads > 1000) throw new Error('wrap walk did not stop'); + return { isWrapped: true, translateToString: () => ['❯ a', 'b', 'c'][((row % 3) + 3) % 3] }; + }; + + expect(app._tapIsOnCaretLine(at(0, 2).clientX, at(0, 2).clientY)).toBe(true); + }); + it('activates a link in a plain shell session, where every tap classifies as input', () => { // A shell has no TUI to own taps, so _classifyMobileTerminalTap short-circuits to // 'input' for the whole screen — gating link taps on the intent would leave every URL From 52557ec558e5acf008e0267d45f0a28e5e329cbc Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sun, 18 Oct 2026 08:10:27 +0200 Subject: [PATCH 2/2] test(#654): pin the wrap walk for a soft-wrapped composer on a parked viewport The parked-viewport test had no wrapped rows and the termination test runs at baseY 0, so nothing pinned the walk of a multi-row composer through absolute lines off the bottom. The new case fails on the pre-fix terminal-ui.js ('link' instead of 'history') and passes with the fix. Co-Authored-By: Claude Opus 5.5 (1M context) --- test/terminal-touch-tap.test.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/terminal-touch-tap.test.ts b/test/terminal-touch-tap.test.ts index 6460b1a0..114ac24d 100644 --- a/test/terminal-touch-tap.test.ts +++ b/test/terminal-touch-tap.test.ts @@ -1116,6 +1116,30 @@ describe('terminal link tap', () => { expect(windowRef.open).toHaveBeenCalledWith('https://example.com/guide', '_blank', 'noopener,noreferrer'); }); + it('walks a soft-wrapped composer through absolute lines while the viewport is parked', () => { + // The composer spans absolute lines 2-3 (3 is its soft-wrapped tail, where the caret + // sits); the viewport is parked one row above the bottom, so screen row 1 is the + // composer's FIRST row and screen row 0 is scrollback. + const history = 'docs at https://example.com/guide'; + const composer = '❯ summarize https://example.com/typed'; + const lines = ['older', history, composer, ' for me', '', '']; + const { app, windowRef } = linkHarness(lines); + Object.assign(app.terminal, { rows: 3 }); + Object.assign(app.terminal.buffer.active, { baseY: 3, viewportY: 1, cursorY: 0 }); + const getLine = app.terminal.buffer.active.getLine; + app.terminal.buffer.active.getLine = (row: number) => { + const line = getLine(row); + return line && row === 3 ? { ...line, isWrapped: true } : line; + }; + + // The composer's first row is on the caret's logical line: its typed URL stays editable. + expect(app._handleMobileTerminalTap(at(composer.indexOf('https'), 1), true, 'history')).toBe('history'); + expect(windowRef.open).not.toHaveBeenCalled(); + // Screen row 0 is absolute line 1, scrollback: its URL opens. + expect(app._handleMobileTerminalTap(at(history.indexOf('https'), 0), true, 'history')).toBe('link'); + expect(windowRef.open).toHaveBeenCalledWith('https://example.com/guide', '_blank', 'noopener,noreferrer'); + }); + it('stops the caret-line wrap walk at the buffer end even when every row is wrapped', () => { // xterm's getLine indexes its circular list modulo capacity, so past the last row it // returns earlier rows again; with all of them soft-wrapped an unbounded walk never ends.