From 4d3080aacc237bd5052ab0b352c21fa9992e313c Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sun, 12 Jul 2026 12:40:25 +0200 Subject: [PATCH] fix(review): gate wheel forwarding by CLI mode/version, stop link click double-fire (PR #144) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - _shouldForwardWheelToApp: claude sessions forward wheel to the TUI only when the banner-parsed cliVersion is known AND >= 2.1.187 (older/unknown Claude Code captures wheel as select-menu navigation → keep local scrollLines); new dependency-free _cliVersionAtLeast semver-ish compare - gemini excluded from wheel forwarding entirely (TUI wheel behavior unverified); codex keeps forwarding (verified); taps/clicks still forwarded for all strip modes - link double-fire: registerFilePathLinkProvider links now track hover state via ILink hover/leave callbacks (_linkHovered) and _handleDesktopTerminalClick bails while a link is hovered, so a link click no longer also sends a synthetic SGR press/release to the TUI - help modal: document Shift+Wheel (scroll local history when mouse passthrough is active) - tests: version gate (2.1.186/unknown/garbage no forward, 2.1.187+ forwards), codex/gemini split, link-hover click suppression Co-Authored-By: Claude Fable 5 --- src/web/public/index.html | 1 + src/web/public/terminal-ui.js | 61 +++++++++++++++++++++++---- test/terminal-touch-tap.test.ts | 73 ++++++++++++++++++++++++++++++++- 3 files changed, 124 insertions(+), 11 deletions(-) diff --git a/src/web/public/index.html b/src/web/public/index.html index 3f6edd2d..56347539 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -560,6 +560,7 @@
Alt/Option+[ / Alt/Option+]
Previous / Next Session
Alt/Option+1-9
Switch to Tab N
Ctrl+L
Clear Terminal
+
Shift+Wheel
Scroll local history (when mouse passthrough is active)
Ctrl++
Increase Font
Ctrl+-
Decrease Font
Ctrl+?
Show Help
diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index af03098d..30437c3f 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -323,12 +323,14 @@ Object.assign(CodemanApp.prototype, { // Register link provider for clickable file paths in Bash tool output this.registerFilePathLinkProvider(); - // Mouse wheel: forward to the TUI for strip-mode sessions, local scrollback - // otherwise. Claude Code (2.1.187+) scrolls its own transcript on SGR wheel - // reports — scrolled-away tool blocks re-render live and stay clickable — - // and its select menus no longer capture wheel as option navigation - // (verified against 2.1.202: /model menu highlight ignores wheel reports), - // so the original reason to keep the wheel local is gone for claude mode. + // Mouse wheel: forward to the TUI only for sessions verified to handle SGR + // wheel reports (codex, and claude 2.1.187+ — see _shouldForwardWheelToApp), + // local scrollback otherwise. Claude Code 2.1.187+ scrolls its own + // transcript on SGR wheel reports — scrolled-away tool blocks re-render + // live and stay clickable — and its select menus no longer capture wheel + // as option navigation (verified against 2.1.202: /model menu highlight + // ignores wheel reports); older versions DO capture wheel as option + // navigation, so they keep the local wheel. // Shift+wheel always scrolls xterm's local scrollback (Codeman's restored // history lives there), and once the viewport left the bottom the wheel // stays local until the user scrolls back down — so both scrollbacks stay @@ -931,6 +933,12 @@ Object.assign(CodemanApp.prototype, { activate(_event, text) { window.open(text, '_blank', 'noopener,noreferrer'); }, + hover() { + self._linkHovered = true; + }, + leave() { + self._linkHovered = false; + }, }); }; @@ -969,6 +977,12 @@ Object.assign(CodemanApp.prototype, { activate(event, text) { self.openLogViewerWindow(text, self.activeSessionId); }, + hover() { + self._linkHovered = true; + }, + leave() { + self._linkHovered = false; + }, }); }; @@ -2146,13 +2160,39 @@ Object.assign(CodemanApp.prototype, { this._sendInputAsync(this.activeSessionId, `\x1b[<0;${pos.col};${pos.row}M\x1b[<0;${pos.col};${pos.row}m`); }, - // Wheel forwarding gate for the container wheel handler: strip-mode session, - // no Shift override, xterm's own encoder dormant, viewport at the bottom. + // True when a parsed CLI version string ('2.1.187' — banner-parsed on the + // server, delivered via session:cliInfo / SessionState.cliVersion) is known + // AND >= the minimum. Unknown or unparseable versions return false so + // callers keep the conservative behavior. + _cliVersionAtLeast(version, minimum) { + if (typeof version !== 'string') return false; + const parts = version.trim().replace(/^v/, '').split('.').map(Number); + if (parts.length !== 3 || parts.some((n) => !Number.isFinite(n))) return false; + const min = minimum.split('.').map(Number); + for (let i = 0; i < 3; i++) { + if (parts[i] !== min[i]) return parts[i] > min[i]; + } + return true; + }, + + // Wheel forwarding gate for the container wheel handler: no Shift override, + // xterm's own encoder dormant, viewport at the bottom, and a TUI VERIFIED to + // scroll its transcript on SGR wheel reports: codex, or claude 2.1.187+ + // (older Claude Code captures wheel as select-menu option navigation; an + // unknown version is treated as older). Gemini is a strip mode too but its + // wheel behavior is unverified, so it keeps the local wheel — taps/clicks + // are still forwarded for it (harmless no-ops at worst). _shouldForwardWheelToApp(ev) { if (ev.shiftKey) return false; const mode = this.terminal?.modes?.mouseTrackingMode; if (mode && mode !== 'none') return false; - if (!this._sessionUsesServerMouseStrip()) return false; + const session = this.sessions?.get(this.activeSessionId); + const sessionMode = session?.mode || 'claude'; + if (sessionMode === 'claude') { + if (!this._cliVersionAtLeast(session?.cliVersion, '2.1.187')) return false; + } else if (sessionMode !== 'codex') { + return false; + } return this._terminalViewportAtBottom(); }, @@ -2189,6 +2229,8 @@ Object.assign(CodemanApp.prototype, { // a meaning elsewhere: synthetic/compat clicks after a touch tap (touchend // reported already), modified clicks (shift keeps xterm's selection // override), double/triple clicks (word/line selection), drag-selections, + // clicks on hovered links (activate() already handles the click — a second + // synthetic SGR press could e.g. dismiss a claude permission dialog), // clicks outside the cell grid, and sessions where xterm's own encoder is // live (it reported the click itself — a second report would double-move). _handleDesktopTerminalClick(ev) { @@ -2199,6 +2241,7 @@ Object.assign(CodemanApp.prototype, { if (mode && mode !== 'none') return; if (!this._sessionUsesServerMouseStrip()) return; if (this.terminal.hasSelection?.()) return; + if (this._linkHovered) return; // link provider hover/leave callbacks (registerFilePathLinkProvider) if (performance.now() <= (this._trustedTapMouseSuppressUntil || 0)) return; if (!ev.target?.closest?.('.xterm-screen')) return; this._sendSyntheticSgrTap(ev.clientX, ev.clientY); diff --git a/test/terminal-touch-tap.test.ts b/test/terminal-touch-tap.test.ts index 4ab9b788..49862e74 100644 --- a/test/terminal-touch-tap.test.ts +++ b/test/terminal-touch-tap.test.ts @@ -219,6 +219,40 @@ describe('terminal touch tap mouse guard', () => { expect(sent).toEqual([]); }); + it('desktop click: skips the click while a terminal link is hovered (activate() handles it)', () => { + const { app } = loadTerminalUiHarness(); + const sent: string[] = []; + app.activeSessionId = 'sess-1'; + app.sessions = new Map([['sess-1', { mode: 'claude' }]]); + app._sendInputAsync = (_id: string, data: string) => sent.push(data); + app.terminal = { + cols: 80, + rows: 24, + modes: { mouseTrackingMode: 'none' }, + hasSelection: () => false, + element: { + querySelector: () => ({ getBoundingClientRect: () => ({ left: 0, top: 0 }) }), + }, + _core: { _renderService: { dimensions: { css: { cell: { width: 8, height: 16 } } } } }, + }; + const click = { + isTrusted: true, + button: 0, + detail: 1, + clientX: 50, + clientY: 50, + target: { closest: (sel: string) => (sel === '.xterm-screen' ? {} : null) }, + }; + + app._linkHovered = true; // link provider hover() fired — this click opens the link + app._handleDesktopTerminalClick(click); + expect(sent).toEqual([]); + + app._linkHovered = false; // leave() fired — plain clicks report again + app._handleDesktopTerminalClick(click); + expect(sent).toEqual(['\x1b[<0;7;4M\x1b[<0;7;4m']); + }); + it('desktop click: skips the compat click that follows a touch tap', () => { const { app, setNow } = loadTerminalUiHarness(); const sent: string[] = []; @@ -277,10 +311,10 @@ describe('terminal touch tap mouse guard', () => { expect(sent).toEqual(['\x1b[<0;7;4M\x1b[<0;7;4m']); }); - it('wheel: forwards to the app only for strip-mode sessions at the buffer bottom without Shift', () => { + it('wheel: forwards to the app only for verified sessions at the buffer bottom without Shift', () => { const { app } = loadTerminalUiHarness(); app.activeSessionId = 'sess-1'; - app.sessions = new Map([['sess-1', { mode: 'claude' }]]); + app.sessions = new Map([['sess-1', { mode: 'claude', cliVersion: '2.1.187' }]]); app.terminal = { modes: { mouseTrackingMode: 'none' }, buffer: { active: { viewportY: 50, baseY: 50 } }, @@ -301,6 +335,41 @@ describe('terminal touch tap mouse guard', () => { expect(app._shouldForwardWheelToApp({ shiftKey: false })).toBe(false); }); + it('wheel: gates claude forwarding on CLI version 2.1.187+ (unknown or older stays local)', () => { + const { app } = loadTerminalUiHarness(); + app.activeSessionId = 'sess-1'; + app.terminal = { + modes: { mouseTrackingMode: 'none' }, + buffer: { active: { viewportY: 50, baseY: 50 } }, + }; + const withVersion = (cliVersion?: string) => { + app.sessions = new Map([['sess-1', { mode: 'claude', cliVersion }]]); + return app._shouldForwardWheelToApp({ shiftKey: false }); + }; + + expect(withVersion(undefined)).toBe(false); // banner not parsed yet → assume older + expect(withVersion('2.1.186')).toBe(false); // last version whose menus capture wheel + expect(withVersion('2.1.187')).toBe(true); // first version verified safe + expect(withVersion('2.2.0')).toBe(true); + expect(withVersion('3.0.0')).toBe(true); + expect(withVersion('garbage')).toBe(false); // unparseable → assume older + }); + + it('wheel: codex forwards without a version; gemini never forwards', () => { + const { app } = loadTerminalUiHarness(); + app.activeSessionId = 'sess-1'; + app.terminal = { + modes: { mouseTrackingMode: 'none' }, + buffer: { active: { viewportY: 50, baseY: 50 } }, + }; + + app.sessions = new Map([['sess-1', { mode: 'codex' }]]); // verified TUI, no version gate + expect(app._shouldForwardWheelToApp({ shiftKey: false })).toBe(true); + + app.sessions = new Map([['sess-1', { mode: 'gemini', cliVersion: '9.9.9' }]]); // unverified TUI + expect(app._shouldForwardWheelToApp({ shiftKey: false })).toBe(false); + }); + it('wheel: encodes SGR 64/65 ticks, caps per event, and coalesces into one flush', () => { const { app } = loadTerminalUiHarness(); const sent: Array<{ id: string; data: string }> = [];