From f2d3a7e3c101bb668c63a18de11a3ef56e6b75f7 Mon Sep 17 00:00:00 2001 From: Rounak Datta Date: Tue, 18 Aug 2026 17:31:54 +0000 Subject: [PATCH 1/5] fix(mobile): links open in a new tab from a tap, in the terminal and the chat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a phone no link was openable, on either surface, for two unrelated reasons. **Terminal.** xterm resolves the link under the pointer on `mousemove` and activates it on `mouseup` over its SCREEN element. A touch tap delivers neither: `touch-action: none` on the terminal subtree plus touchstart's preventDefault for a 'content' tap suppress the browser's compatibility mouse events, `_installMobileTapMouseGuard` drops the trusted ones that still arrive inside the 450ms tap window, and the synthetic mousedown/mouseup pair dispatched for mouse REPORTING goes to the `.xterm` root — an ancestor of the node the linkifier listens on, so it cannot reach it — and carries no mousemove either way. Every URL and file path in the terminal was therefore inert on phones and tablets, Claude Code's own `/login` URL included. The tap path now activates the link itself, through the SAME provider that feeds the hover linkifier (`_terminalLinkAtPoint`), so a tap and a desktop click can never disagree about what is a link or where it ends — containment mirrors xterm's own `_linkAtPosition`. It runs synchronously inside the touchend handler, which is what keeps the user gesture that lets `window.open` past the popup blocker, and before any mouse report, exactly as `_handleDesktopTerminalClick` already skips the SGR tap for a hovered link. Two kinds of row keep their existing meaning: the caret's logical line, where a tap places the cursor and a URL the user typed must stay editable, and TUI-owned rows, where a numbered choice or an expandable readback is answering a dialog and routinely carries the very path the tap would otherwise open. The caret line is the boundary rather than the tap intent, because a plain shell classifies EVERY tap as 'input' and gating on that would leave every URL in shell output inert. **Chat.** `marked` emits a bare `` and the markdown sanitizer's allowlist carries no `target`, so a tap in the response viewer navigated the current tab away: on a phone that unloads the whole dashboard — SSE, terminal buffers, unsent composer text — and there is no middle-click or open-in-new-tab affordance to work around it. `_renderMarkdown` now decorates anchors in the template pass it already makes for code blocks. That pass runs AFTER sanitizing, so it is the only source of both attributes: an agent-authored `target`/`rel` is already stripped, and `rel="noopener noreferrer"` is set on the same element in the same breath, so no page Codeman opens gets a `window.opener` handle back. Fragment links stay in-page; mailto:/tel: are left to the OS rather than stranding an empty tab. Tests: 10 cases in `terminal-touch-tap.test.ts` (URL, file path, log path, scrollback, no-double-report, composer, shell mode, dialog row, no provider) and a new `response-viewer-external-links.test.ts` driving the shipped marked + DOMPurify + app.js. 7 of them fail without the fix. Co-Authored-By: Claude Opus 5 (1M context) --- src/web/public/app.js | 22 +++ src/web/public/terminal-ui.js | 145 +++++++++++++++- test/response-viewer-external-links.test.ts | 177 ++++++++++++++++++++ test/terminal-touch-tap.test.ts | 171 ++++++++++++++++++- 4 files changed, 510 insertions(+), 5 deletions(-) create mode 100644 test/response-viewer-external-links.test.ts diff --git a/src/web/public/app.js b/src/web/public/app.js index 7c8297caa..d7cc0d8ad 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -2053,6 +2053,28 @@ class CodemanApp { wrap.appendChild(actions); wrap.appendChild(pre); }); + // Links open in a NEW tab. + // + // marked emits a bare `` and the sanitizer's allowlist has no + // `target`, so a tap in the chat NAVIGATED THE APP AWAY: on a phone that + // unloads the whole dashboard — SSE, terminal buffers, unsent composer + // text — and the OS back gesture reloads it from scratch, which is what + // "links don't open" reads as on mobile, with no middle-click or + // open-in-new-tab affordance to work around it. + // + // This pass runs AFTER sanitizing, so it is the only source of these two + // attributes: whatever an agent wrote is already gone, and `rel` is set on + // the same element in the same breath, so no page Codeman opens ever gets + // a `window.opener` handle back (reverse tabnabbing). + // + // A fragment link stays in-page, and mailto:/tel: are handed to the OS — + // giving those a target just strands an empty tab. + tmpl.content.querySelectorAll('a[href]').forEach((a) => { + const href = a.getAttribute('href') || ''; + if (!href || href.startsWith('#') || /^(?:mailto|tel):/i.test(href)) return; + a.setAttribute('target', '_blank'); + a.setAttribute('rel', 'noopener noreferrer'); + }); return tmpl.innerHTML; } catch { /* fall through */ } } diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 09bc75858..18eec8a2e 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -1306,7 +1306,7 @@ Object.assign(CodemanApp.prototype, { // Debug: Track if provider is being invoked let lastInvokedLine = -1; - this.terminal.registerLinkProvider({ + const provider = { provideLinks(bufferLineNumber, callback) { // Debug logging - only log if line changed to avoid spam if (bufferLineNumber !== lastInvokedLine) { @@ -1523,11 +1523,124 @@ Object.assign(CodemanApp.prototype, { } callback(links.length > 0 ? links : undefined); }, - }); + }; + + // Keep the provider reachable: on touch devices xterm's linkifier never + // resolves a link (it is driven by mousemove/mouseup, which a tap does not + // produce), so the tap path asks this SAME provider what is under the finger + // rather than growing a second, driftable copy of the patterns. + // See _terminalLinkAtPoint. + this._terminalLinkProvider = provider; + this.terminal.registerLinkProvider(provider); console.log('[LinkProvider] File path link provider registered'); }, + /** + * The terminal link under a viewport point, or null. + * + * Resolved through the provider registered above, so a tap and a desktop click + * can never disagree about what is a link or where it ends. Containment + * mirrors xterm's own `_linkAtPosition` — flattened `y * cols + x`, inclusive + * at both ends — for the same reason. + * + * ⚠️ The provider answers its callback SYNCHRONOUSLY (every path in + * `registerFilePathLinkProvider` does, including the empty ones). xterm's + * ILinkProvider contract permits an async reply, so this reads whatever + * arrived by the time the call returns and answers null otherwise: a tap then + * keeps its normal meaning instead of opening a link late, after the gesture + * that made `window.open` permissible is gone. + */ + _terminalLinkAtPoint(clientX, clientY) { + const provider = this._terminalLinkProvider; + const buffer = this.terminal?.buffer?.active; + if (!provider || !buffer) return null; + const pos = this._clientPointToCell(clientX, clientY); + if (!pos) return null; + // Link ranges are 1-based ABSOLUTE buffer lines (xterm adds ydisp to the + // viewport row before asking), which is what the provider's coordAt() emits. + const y = (buffer.viewportY || 0) + pos.row; + let links = null; + try { + provider.provideLinks(y, (result) => { + links = result || []; + }); + } catch { + return null; + } + if (!links || links.length === 0) return null; + const cols = Math.max(1, this.terminal.cols || 1); + const current = y * cols + pos.col; + return ( + links.find((link) => { + const start = link?.range?.start; + const end = link?.range?.end; + if (!start || !end) return false; + return start.y * cols + start.x <= current && current <= end.y * cols + end.x; + }) || null + ); + }, + + /** + * Is this point on the caret's logical line — the editable composer? + * + * There a tap means "put the cursor here", so a URL the USER typed or pasted + * into a prompt must stay editable rather than opening itself. The caret is the + * signal that works for every CLI: claude's composer row carries it, and in a + * plain shell it sits on the prompt line while output scrolls above, so the + * same test covers both without asking what mode is running (tap + * classification cannot answer this — a shell session classifies EVERY tap as + * 'input', which would leave every URL in shell output inert). + * + * The caret's line is walked out through soft wraps, since a long prompt spans + * rows. + */ + _tapIsOnCaretLine(clientX, clientY) { + const buffer = this.terminal?.buffer?.active; + 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; + }, + + /** + * Activate the terminal link under a touch point. Returns true when one was. + * + * xterm activates a link from a `mousemove` that resolves what is under the + * pointer, followed by a `mouseup` on its SCREEN element — and on a touch + * device it receives neither: `touch-action: none` plus touchstart's + * preventDefault suppress the browser's compatibility mouse events, + * _installMobileTapMouseGuard drops the ones that still arrive, and the + * synthetic pair dispatched for mouse REPORTING goes to the `.xterm` root, + * an ANCESTOR of the node the linkifier listens on (so it cannot reach it) and + * carries no mousemove either way. Every URL and file path in the terminal was + * therefore inert on phones and tablets — Claude Code's own `/login` URL + * included, which is unfinishable from a phone without this. + * + * Activating here, synchronously inside the touchend handler, is what keeps + * the user gesture that lets the URL branch's `window.open` through the popup + * blocker; a later activation (a timer, a promise) is silently swallowed. + */ + _activateTerminalLinkAtPoint(clientX, clientY) { + const link = this._terminalLinkAtPoint(clientX, clientY); + if (!link || typeof link.activate !== 'function') return false; + try { + link.activate(null, link.text); + } catch (err) { + console.warn('[LinkProvider] tap activation failed:', err); + return false; + } + return true; + }, + showWelcome() { // Phones get the session overview instead of the welcome screen: on a small // screen "which session is blocked on me" beats "how do I start one". The @@ -3705,6 +3818,32 @@ Object.assign(CodemanApp.prototype, { // touchstart already classified this exact point; reuse it rather than paying // a second full-viewport scan for the same gesture. const intent = cachedIntent ?? this._classifyMobileTerminalTap(touch.clientX, touch.clientY); + // Computed once and reused by the keyboard decision at the tail of this + // method: both ask the same question, and the pane cannot change in between + // (a mouse report only reaches the PTY; its output lands on a later turn). + const actionable = this._isActionableMobileTerminalTap(touch.clientX, touch.clientY); + + // A tap that lands ON a link activates it, at any scroll position and before + // any mouse report — exactly what a desktop click does, where the provider's + // activate() runs and _handleDesktopTerminalClick deliberately skips the SGR + // tap for a hovered link so the CLI never also sees a click there. + // + // Two kinds of row keep their existing meaning instead: the composer, where a + // tap places the caret in text the USER typed (_tapIsOnCaretLine), and + // TUI-owned rows, where a numbered choice or an expandable readback is + // answering a dialog and routinely carries the very path the tap would + // otherwise open — on a phone the dialog is the only interaction that + // matters, so it wins. + if ( + !actionable && + !this._tapIsOnCaretLine(touch.clientX, touch.clientY) && + this._activateTerminalLinkAtPoint(touch.clientX, touch.clientY) + ) { + // No focus change: a 'content' tap was already blurred by touchstart, and + // popping the keyboard behind a tab that is about to take over is noise. + return 'link'; + } + if (intent === 'history') { // Scrolled up: send NO mouse report — a tap on old output must not be // delivered to the CLI as a click on whatever row now occupies that cell. @@ -3728,7 +3867,7 @@ Object.assign(CodemanApp.prototype, { this._sendSyntheticSgrTap(touch.clientX, touch.clientY); } - if (intent === 'content' && this._isActionableMobileTerminalTap(touch.clientX, touch.clientY)) { + if (intent === 'content' && actionable) { // A synthetic xterm click can focus its helper textarea. Blur after the // report so collapsing a readback never opens or retains the keyboard. this._blurMobileTerminalInput(); diff --git a/test/response-viewer-external-links.test.ts b/test/response-viewer-external-links.test.ts new file mode 100644 index 000000000..a0cfd69d0 --- /dev/null +++ b/test/response-viewer-external-links.test.ts @@ -0,0 +1,177 @@ +/** + * @fileoverview Response-viewer links open in a NEW tab (`CodemanApp._renderMarkdown`). + * + * `marked` emits a bare `` and the markdown sanitizer's allowlist carries no + * `target`, so every link in the chat used to navigate the CURRENT tab. On a phone that + * unloads the whole dashboard — SSE, terminal buffers, unsent composer text — and the OS + * back gesture reloads it from scratch, with no middle-click or open-in-new-tab affordance + * to work around it. That is the "links don't open on mobile" report. + * + * `_renderMarkdown` therefore decorates anchors AFTER sanitizing, which makes it the single + * source of both attributes: whatever an agent wrote is already stripped by then, and `rel` + * is set on the same element in the same pass, so no page Codeman opens can reach back + * through `window.opener` (reverse tabnabbing). + * + * Drives the SHIPPING artifacts — vendored `marked`, vendored DOMPurify + `sanitize-html.js`, + * and `app.js` itself — in a `vm` with a jsdom document injected (the technique from + * markdown-sanitizer.test.ts / response-viewer-file-links.test.ts; a per-file jsdom + * environment would externalize node:fs under vite). + * + * No port / server needed. + */ +import { readFileSync } from 'node:fs'; +import { performance } from 'node:perf_hooks'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { JSDOM } from 'jsdom'; +import { describe, expect, it, vi } from 'vitest'; + +const publicFile = (name: string) => readFileSync(resolve(import.meta.dirname, '../src/web/public', name), 'utf8'); + +const dom = new JSDOM(''); +const jsdomWindow = dom.window as unknown as Window & typeof globalThis; +const { document, NodeFilter } = dom.window; + +/** The shipping sanitizer: vendored DOMPurify bound to our jsdom window + the real config. */ +function loadShippingSanitizer(): (html: string) => string { + const dpModule: { exports: unknown } = { exports: {} }; + // eslint-disable-next-line @typescript-eslint/no-implied-eval, no-new-func + new Function('module', 'exports', publicFile('vendor/dompurify.min.js'))(dpModule, dpModule.exports); + const DOMPurify = (dpModule.exports as (win: unknown) => unknown)(jsdomWindow); + + const sanModule: { exports: { createMarkdownSanitizer?: (dp: unknown) => (html: string) => string } } = { + exports: {}, + }; + // eslint-disable-next-line @typescript-eslint/no-implied-eval, no-new-func + new Function('module', 'exports', publicFile('sanitize-html.js'))(sanModule, sanModule.exports); + const create = sanModule.exports.createMarkdownSanitizer; + if (typeof create !== 'function') throw new Error('createMarkdownSanitizer not exported'); + return create(DOMPurify); +} + +/** The vendored `marked` build the page loads, evaluated as CommonJS. */ +function loadShippingMarked(): { parse: (src: string, opts?: unknown) => string } { + const module: { exports: unknown } = { exports: {} }; + // eslint-disable-next-line @typescript-eslint/no-implied-eval, no-new-func + new Function('module', 'exports', publicFile('vendor/marked.min.js'))(module, module.exports); + return module.exports as { parse: (src: string, opts?: unknown) => string }; +} + +type RenderApp = { _renderMarkdown(text: string): string }; + +function loadCodemanAppClass(): { prototype: RenderApp } { + const context = vm.createContext({ + console, + performance, + setInterval: vi.fn(), + clearInterval: vi.fn(), + setTimeout, + clearTimeout, + requestAnimationFrame: vi.fn(), + HTMLCanvasElement: class HTMLCanvasElement {}, + fetch: vi.fn(), + document, + NodeFilter, + localStorage: { length: 0, key: vi.fn(), getItem: vi.fn(), setItem: vi.fn(), removeItem: vi.fn() }, + // The page wires the sanitizer onto window; _sanitizeHtml fails closed without it, + // and a closed-failing render would make every assertion below vacuous. + window: { addEventListener: vi.fn(), removeEventListener: vi.fn(), sanitizeMarkdownHtml: loadShippingSanitizer() }, + marked: loadShippingMarked(), + MobileDetection: {}, + }); + vm.runInContext( + `${publicFile('constants.js')}\n${publicFile('app.js')}\nglobalThis.__CodemanApp = CodemanApp;`, + context + ); + return (context as { __CodemanApp: { prototype: RenderApp } }).__CodemanApp; +} + +const CodemanApp = loadCodemanAppClass(); + +/** Render markdown the way the response viewer does and return the resulting element. */ +function render(markdown: string): HTMLElement { + const app = Object.create(CodemanApp.prototype) as RenderApp; + const root = document.createElement('div'); + root.className = 'rv-text'; + root.innerHTML = app._renderMarkdown(markdown); + return root as unknown as HTMLElement; +} + +const anchor = (root: HTMLElement, index = 0) => Array.from(root.querySelectorAll('a'))[index]; + +describe('response viewer external links', () => { + it('opens a markdown link in a new tab, with rel set in the same pass', () => { + const root = render('See [the docs](https://example.com/docs?a=1&b=2) for details.'); + + const a = anchor(root); + expect(a, 'the link survived sanitizing').toBeDefined(); + expect(a.getAttribute('href')).toBe('https://example.com/docs?a=1&b=2'); + expect(a.getAttribute('target')).toBe('_blank'); + expect(a.getAttribute('rel')).toBe('noopener noreferrer'); + }); + + it('opens an autolinked bare URL in a new tab too', () => { + // gfm autolinks a bare URL, which is how an agent usually prints one. + const root = render('Login at https://claude.ai/oauth/authorize?code=true&client_id=abc to continue.'); + + const a = anchor(root); + expect(a.getAttribute('href')).toBe('https://claude.ai/oauth/authorize?code=true&client_id=abc'); + expect(a.getAttribute('target')).toBe('_blank'); + expect(a.getAttribute('rel')).toBe('noopener noreferrer'); + }); + + it('sends a same-origin path to a new tab as well — it is still a navigation away', () => { + const root = render('Check [status](/api/status).'); + + expect(anchor(root).getAttribute('target')).toBe('_blank'); + }); + + it('leaves an in-page fragment link alone', () => { + // A target here would open a second copy of the app to scroll it. + const root = render('Jump to [the section](#results).'); + + const a = anchor(root); + expect(a.getAttribute('href')).toBe('#results'); + expect(a.hasAttribute('target')).toBe(false); + expect(a.hasAttribute('rel')).toBe(false); + }); + + it('leaves mailto: and tel: to the OS instead of stranding an empty tab', () => { + const root = render('Mail [me](mailto:a@example.com) or call [now](tel:+15551234).'); + + for (const a of Array.from(root.querySelectorAll('a'))) { + expect(a.hasAttribute('target'), a.getAttribute('href') || '').toBe(false); + } + }); + + it('is the ONLY source of target/rel: an agent cannot ask for an opener', () => { + // The sanitizer's allowlist has neither attribute, so agent-authored ones are gone + // before this pass runs — and the pass sets both together, so `rel` can never end up + // weaker than the target it accompanies. + const root = render('click'); + + const a = anchor(root); + expect(a.getAttribute('target')).toBe('_blank'); + expect(a.getAttribute('rel')).toBe('noopener noreferrer'); + }); + + it('still drops a javascript: link rather than decorating it', () => { + const root = render('[x](javascript:alert(1))'); + + const a = anchor(root); + // DOMPurify strips the unsafe href; whatever is left must not carry a target either, + // which would turn a hollow anchor into a window-opening one. + expect(a?.getAttribute('href') ?? null).toBeNull(); + expect(a?.hasAttribute('target') ?? false).toBe(false); + }); + + it('keeps code blocks and their copy affordance intact', () => { + // The anchor pass shares the one template walk with the code-block wrapper; a mistake + // there would silently drop the toolbar rather than fail loudly. + const root = render('```\nconst a = 1;\n```'); + + expect(root.querySelector('.rv-code-wrap')).not.toBeNull(); + expect(root.querySelector('.rv-copy-btn')).not.toBeNull(); + expect(root.querySelector('pre code')?.textContent).toContain('const a = 1;'); + }); +}); diff --git a/test/terminal-touch-tap.test.ts b/test/terminal-touch-tap.test.ts index be05e9378..4db770353 100644 --- a/test/terminal-touch-tap.test.ts +++ b/test/terminal-touch-tap.test.ts @@ -8,8 +8,11 @@ function loadTerminalUiHarness() { let now = 1_000; let keyboardVisible = false; let activeElement: unknown = null; + // The module hangs its constants off window (CodemanTerminalInput) and the URL branch of + // the link provider opens through window.open, so tests need a handle on the same object. + const windowRef: Record = {}; const context = vm.createContext({ - window: {}, + window: windowRef, document: { body: { classList: { contains: () => false } }, get activeElement() { @@ -18,7 +21,7 @@ function loadTerminalUiHarness() { getElementById: () => null, }, CodemanApp, - console: { warn: vi.fn(), log: vi.fn() }, + console: { warn: vi.fn(), log: vi.fn(), debug: vi.fn() }, _crashDiag: { log: vi.fn() }, performance: { now: () => now }, requestAnimationFrame: (_fn: () => void) => 1, @@ -43,12 +46,18 @@ function loadTerminalUiHarness() { TERMINAL_CHUNK_SIZE: 32 * 1024, }); + // constants.js first: the link provider calls absoluteFilePathPattern() and + // previewsInFileViewer() at scan time, and the SHIPPED definitions are what keep a tap and + // a hover resolving the same links. + const constants = readFileSync(resolve(import.meta.dirname, '../src/web/public/constants.js'), 'utf8'); const code = readFileSync(resolve(import.meta.dirname, '../src/web/public/terminal-ui.js'), 'utf8'); + vm.runInContext(constants, context, { filename: 'constants.js' }); vm.runInContext(code, context, { filename: 'terminal-ui.js' }); const app = new (CodemanApp as any)(); return { app, + windowRef, setNow: (value: number) => { now = value; }, @@ -732,3 +741,161 @@ describe('terminal touch tap mouse guard', () => { expect(event.stopImmediatePropagation).not.toHaveBeenCalled(); }); }); + +describe('terminal link tap', () => { + // xterm resolves a link from mousemove and activates it on mouseup over its SCREEN element. + // A touch tap produces none of those (touch-action:none and touchstart's preventDefault + // suppress the compatibility mouse events, the post-tap guard drops the rest, and the + // synthetic pair this app dispatches for mouse REPORTING lands on the .xterm root, an + // ancestor of the node the linkifier listens on). So the tap path activates the link + // itself, through the same provider, or every URL and path in the terminal stays inert on + // a phone. + // + // Grid geometry from createTerminalGrid: 8×16 cells, screen rect at (0,0), viewportY 0 — + // so 0-based character index i on 0-based row r sits at (i * 8 + 4, r * 16 + 8). + const at = (index: number, row = 0) => ({ clientX: index * 8 + 4, clientY: row * 16 + 8 }); + + /** A claude-mode app with the shipped link provider registered over `lines`. */ + function linkHarness(lines: string[], cursorY = lines.length - 1) { + const harness = loadTerminalUiHarness(); + const { app, windowRef } = harness; + 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 = createTerminalGrid(lines, cursorY); + app.terminal.registerLinkProvider = vi.fn(); + app.openFilePreview = vi.fn(); + app.openLogViewerWindow = vi.fn(); + app._isExternalPreviewPath = () => false; + windowRef.open = vi.fn(); + app.registerFilePathLinkProvider(); + return { app, windowRef, sent }; + } + + it('opens a URL under the finger in a new tab', () => { + const line = 'Login at https://claude.ai/oauth/authorize?code=true&client_id=abc to finish'; + const { app, windowRef } = linkHarness([line, '', '❯ ']); + + expect(app._handleMobileTerminalTap(at(line.indexOf('https')), false, 'content')).toBe('link'); + expect(windowRef.open).toHaveBeenCalledWith( + 'https://claude.ai/oauth/authorize?code=true&client_id=abc', + '_blank', + 'noopener,noreferrer' + ); + }); + + it('sends no mouse report for the tap it just spent on a link', () => { + // The CLI must not also see a click there: that is how a tap on a URL printed inside a + // permission dialog would answer the dialog. Desktop already skips the SGR tap for a + // hovered link (_handleDesktopTerminalClick). + const line = 'see https://example.com/x for more'; + const { app, sent } = linkHarness([line, '', '❯ ']); + + expect(app._sessionUsesServerMouseStrip()).toBe(true); + app._handleMobileTerminalTap(at(line.indexOf('https')), false, 'content'); + + expect(sent).toEqual([]); + }); + + it('leaves a tap beside the link as an ordinary tap', () => { + // Containment is xterm's own rule (flattened cell index), so tap and click agree on + // where a link ends; a tap on the prose around it keeps its mouse report. + const line = 'see https://example.com/x for more'; + const { app, windowRef, sent } = linkHarness([line, '', '❯ ']); + + expect(app._handleMobileTerminalTap(at(line.indexOf('for more') + 3), false, 'content')).toBe('content'); + expect(windowRef.open).not.toHaveBeenCalled(); + expect(sent).toHaveLength(1); + }); + + it('opens a tapped file path in the preview overlay', () => { + const line = 'wrote the chart to /tmp/out/chart.png just now'; + const { app } = linkHarness([line, '', '❯ ']); + + expect(app._handleMobileTerminalTap(at(line.indexOf('/tmp')), false, 'content')).toBe('link'); + expect(app.openFilePreview).toHaveBeenCalledWith('/tmp/out/chart.png', 'sess-1'); + expect(app.openLogViewerWindow).not.toHaveBeenCalled(); + }); + + it('sends a tapped log path to the log viewer', () => { + const line = 'tail -f /var/log/app.log'; + const { app } = linkHarness([line, '', '❯ ']); + + expect(app._handleMobileTerminalTap(at(line.indexOf('/var')), false, 'content')).toBe('link'); + expect(app.openLogViewerWindow).toHaveBeenCalledWith('/var/log/app.log', 'sess-1'); + }); + + it('activates a link in scrollback, where the tap sends no report at all', () => { + // A scrolled-up tap deliberately reports nothing (it would land on whatever row now + // occupies the cell), but reading old output and tapping a URL in it is the common case. + const line = 'docs at https://example.com/guide'; + const { app, windowRef, sent } = linkHarness([line, '', '']); + + expect(app._handleMobileTerminalTap(at(line.indexOf('https')), false, 'history')).toBe('link'); + expect(windowRef.open).toHaveBeenCalledOnce(); + expect(sent).toEqual([]); + }); + + it('never hijacks a TUI-owned choice row that happens to carry a path', () => { + // On a phone the dialog is the only interaction that matters, and its rows routinely + // name the very file a link would open — answering it must keep winning. + // ⚠️ The caret is parked on the QUESTION row, not the choice: with the caret on the + // tapped row this would pass through _tapIsOnCaretLine and pin nothing. + const line = '❯ 1. Yes, edit /home/user/src/app.ts'; + const { app, windowRef, sent } = linkHarness(['Do you want to make this edit?', line, ' 2. No, keep it as is'], 0); + + expect(app._handleMobileTerminalTap(at(line.indexOf('/home'), 1), false, 'content')).toBe('content'); + expect(windowRef.open).not.toHaveBeenCalled(); + expect(app.openFilePreview).not.toHaveBeenCalled(); + expect(sent).toHaveLength(1); // the choice still reaches the CLI + }); + + it('leaves a URL the user typed in the composer editable', () => { + // Tapping your own prompt text means "put the caret here". Opening it instead would + // punish the phone gesture for fixing a typo in a pasted link. + const composer = '❯ summarize https://example.com/guide for me'; + const { app, windowRef, sent } = linkHarness(['earlier output', '', composer], 2); + + expect(app._handleMobileTerminalTap(at(composer.indexOf('https'), 2), true, 'input')).toBe('input'); + expect(windowRef.open).not.toHaveBeenCalled(); + expect(sent).toHaveLength(1); // the tap still positions the caret via the mouse report + }); + + 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 + // in shell output (curl, npm, git remote) inert. The caret line is the real boundary. + const line = 'remote: https://github.com/Ark0N/Codeman.git'; + const harness = loadTerminalUiHarness(); + const { app, windowRef } = harness; + app.activeSessionId = 'sess-1'; + app.sessions = new Map([['sess-1', { mode: 'shell' }]]); + app._sendInputAsync = vi.fn(); + app.terminal = createTerminalGrid([line, '', 'bash-5.3$ '], 2); + app.terminal.registerLinkProvider = vi.fn(); + windowRef.open = vi.fn(); + app.registerFilePathLinkProvider(); + + // No cachedIntent below: real classification runs, and for a shell it answers 'input'. + const point = at(line.indexOf('https')); + expect(app._classifyMobileTerminalTap(point.clientX, point.clientY)).toBe('input'); + expect(app._handleMobileTerminalTap(point, false)).toBe('link'); + expect(windowRef.open).toHaveBeenCalledWith( + 'https://github.com/Ark0N/Codeman.git', + '_blank', + 'noopener,noreferrer' + ); + }); + + it('keeps taps working when no provider was ever registered', () => { + const harness = loadTerminalUiHarness(); + const { app } = harness; + app.activeSessionId = 'sess-1'; + app.sessions = new Map([['sess-1', { mode: 'claude' }]]); + app.terminal = createTerminalGrid(['plain output', '', '❯ '], 2); + + expect(app._terminalLinkAtPoint(4, 8)).toBeNull(); + expect(app._activateTerminalLinkAtPoint(4, 8)).toBe(false); + }); +}); From 756728e55329ccc07f5b8d3b06af1e17a4d0dc36 Mon Sep 17 00:00:00 2001 From: Rounak Datta Date: Wed, 19 Aug 2026 00:07:28 +0000 Subject: [PATCH 2/5] feat(mobile): long-press to select terminal text, tap to extend, Copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There was no way to copy terminal text from a phone at all, and three layers ruled it out independently: `user-select: none` across the whole terminal subtree on touch devices (taps are cursor gestures there, so the OS callout had to go), the WebGL renderer drawing glyphs as pixels with only the accessibility tree behind them, and xterm's own selection being a mouse DRAG while the touch path dispatches a zero-movement mousedown/mouseup pair — a click. `copyTerminal()` exists but is wired to no button and calls `navigator.clipboard` directly, which is undefined on the plain-HTTP LAN install the installer offers. So the gesture drives xterm's `select()` directly: public API, renderer- independent, and the highlight is drawn by xterm itself. Long-press is free real estate — tap and swipe are taken, long-press and double-tap are used by nothing. - **Long-press** (350ms, finger still within the shared tap slop) selects the run of non-whitespace under the finger. Whitespace is the only delimiter on purpose: every punctuation-aware word rule cuts a path, URL or hash in half, which is what you came to copy. - **Drag** while held extends the selection; touchmove diverts from scrolling. - **Tap** while the bar is up extends it too. That is the ergonomic core: picking up a 4px handle with a fingertip is a coin flip, tapping the other end is not. Dismissal stays explicit (✕ or Copy), so no tap is spent leaving a mode the user is still using. - **Copy** goes through the existing `copyTerminalSelection()`, so it inherits the execCommand fallback that is the only route that works on plain HTTP. - **Line** takes the whole logical line, wraps included, trailing pad trimmed. Three guards are what make the gesture survive contact with a real phone, and each fixes a symptom measured on Android Chrome: 1. **The compat mouse pair after touchend.** xterm focuses from its screen-element mousedown and SelectionService resets the model there, so lifting your finger popped the keyboard and dissolved the selection in one go. The tap path already had a guard for those events; the selection path simply never armed it. Armed now, and the touchend is `preventDefault`ed so the synthesis is stopped at the source (that listener is no longer passive). 2. **The platform's own long-press.** Android Chrome runs its handling at ~500ms and focuses the nearest editable element — xterm's helper textarea, parked at the cursor — which no touch handler can preventDefault because it never sees an event. A focus guard blurs the terminal input for the duration of the gesture, whatever focused it, bounded by a self-expiring deadline so a stuck flag can never leave the keyboard unreachable. `contextmenu` is suppressed for the same window, and the threshold sits at 350ms so it lands clear of the platform's. 3. **Copy re-focusing the terminal.** `copyTerminalSelection()` ends with `terminal.focus()`, which is right on a desktop and wrong on a phone: the keyboard covers what was just copied with nothing waiting to be typed. The bar is built in JS because index.html is read once at server start, and its styles live in styles.css rather than mobile.css because the gesture is touch-driven, not width-driven — a touch tablet in landscape gets the gesture and would otherwise have no bar to copy from. 12 tests in `terminal-touch-tap.test.ts` cover the word rule, forward and backward extension, cross-row selection, Line, tap-to-extend, the copy path, and each of the three guards including the focus guard's expiry. Co-Authored-By: Claude Opus 5 (1M context) --- src/web/public/styles.css | 51 +++++ src/web/public/terminal-ui.js | 393 +++++++++++++++++++++++++++++++- test/terminal-touch-tap.test.ts | 197 +++++++++++++++- 3 files changed, 639 insertions(+), 2 deletions(-) diff --git a/src/web/public/styles.css b/src/web/public/styles.css index e0ee73085..f40335821 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -3395,6 +3395,57 @@ body.touch-device .terminal-container .xterm .xterm-helper-textarea { -webkit-touch-callout: none !important; } +/* Touch text-selection bar (long-press → select → Copy). + Lives in styles.css, NOT mobile.css: the gesture is touch-driven, not + width-driven, and mobile.css is media-gated to ≤1023px — a touch tablet in + landscape would get the gesture with no bar to copy from. + Built in JS (index.html is read once at server start, so markup added there + would need a restart to appear). z-index 900 sits above terminal content and + the local-echo overlay (7) and deliberately BELOW floating agent windows + (1000), so it can never cover their controls. */ +.term-select-bar { + position: absolute; + z-index: 900; + display: none; + gap: 2px; + padding: 3px; + background: var(--bg-card); + border: 1px solid var(--border); + border-radius: 8px; + box-shadow: 0 4px 14px rgba(0, 0, 0, 0.45); +} + +.term-select-bar.visible { + display: flex; +} + +.term-select-btn { + min-height: 38px; + min-width: 46px; + padding: 0 0.7rem; + border: none; + border-radius: 6px; + background: transparent; + color: var(--text); + font-family: inherit; + font-size: 0.82rem; + font-weight: 600; + cursor: pointer; + /* The bar is the one place in the terminal subtree a tap must land on a + control rather than a cell, so it opts out of the gesture styles above. */ + touch-action: manipulation; +} + +.term-select-btn:active { + background: var(--bg-hover); +} + +.term-select-btn--close { + min-width: 38px; + padding: 0; + color: var(--text-muted); +} + /* Welcome Overlay */ .welcome-overlay { position: absolute; diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 18eec8a2e..e7400a3d1 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -38,6 +38,19 @@ // a gesture the terminal treats as a scroll but the dismiss handler treats as // a tap would close the keyboard mid-scroll and drop the composer. const MOBILE_KEYBOARD_DISMISS_TAP_SLOP = 8; + // Hold this long, finger still, before a press becomes a text selection. + // + // ⚠️ It must fire well BEFORE the platform's own long-press threshold (~500ms on + // Android), not just under it: the guards this gesture installs are armed when it + // fires, and at 450ms they were still being armed as Chrome ran its own handling + // — which focuses the nearest editable element, so the keyboard shot up over the + // selection the moment it appeared. 350ms is still far above a tap (~100-150ms). + const TOUCH_SELECT_LONG_PRESS_MS = 350; + // How long after a selection gesture the terminal input stays un-focusable. Long + // enough to cover the platform's long-press handling and the compatibility events + // that trail a touchend; short and self-expiring, so a stuck flag can never leave + // the keyboard unreachable. + const TOUCH_SELECT_FOCUS_GUARD_MS = 800; // Regions where a tap must NOT dismiss the on-screen keyboard // (_installMobileKeyboardDismiss). Two groups: anything that is about to take // focus itself, and the accessory bar, which is built to be used while the @@ -206,6 +219,8 @@ TUI_PROMPT_DEFAULT_ROWS_FROM_BOTTOM, MOBILE_KEYBOARD_DISMISS_EXEMPT_SELECTOR, MOBILE_KEYBOARD_DISMISS_TAP_SLOP, + TOUCH_SELECT_LONG_PRESS_MS, + TOUCH_SELECT_FOCUS_GUARD_MS, }; global.CODEMAN_XTERM_THEMES = CODEMAN_XTERM_THEMES; global.codemanCurrentXtermTheme = currentXtermTheme; @@ -270,6 +285,7 @@ Object.assign(CodemanApp.prototype, { const container = document.getElementById('terminalContainer'); this.terminal.open(container); this._installMobileTapMouseGuard(); + this._installTouchSelectionFocusGuard(); // Suppress xterm key handling during CJK IME composition. // Without this, xterm processes raw keyDown events (e.g., "Process" key) @@ -567,6 +583,18 @@ Object.assign(CodemanApp.prototype, { // Register link provider for clickable file paths in Bash tool output this.registerFilePathLinkProvider(); + // Bar visible ⟺ a selection exists. xterm drops the selection on any keypress, + // on reset and on a tab switch, and a Copy button floating over nothing is a + // trap — one that would copy the PREVIOUS session's text if it still worked. + this.terminal.onSelectionChange?.(() => { + if (!this.terminal?.hasSelection?.()) { + this._touchSelecting = false; + this._touchSelectionActive = false; + this._touchSelectionAnchor = null; + this._hideTouchSelectionBar(); + } + }); + // Mouse wheel: forward to the TUI only for sessions verified to handle SGR // wheel reports (claude 2.1.187+ — see _shouldForwardWheelToApp), local // scrollback otherwise. Claude Code 2.1.187+ scrolls its own @@ -686,6 +714,9 @@ Object.assign(CodemanApp.prototype, { let pixelAccum = 0; let didScroll = false; // track whether touchmove fired (tap vs scroll) + let longPressTimer = null; // armed on touchstart, becomes a text selection + let longPressStartX = 0; + let longPressStartY = 0; let touchStartY = 0; let tapStartedWithTerminalFocus = false; let tapStartIntentCache = null; @@ -695,6 +726,13 @@ Object.assign(CodemanApp.prototype, { container.addEventListener( 'touchstart', (ev) => { + // The selection bar is a child of this container: its buttons own their + // own taps and must not arm a gesture on the terminal underneath. + if (ev.target?.closest?.('.term-select-bar')) return; + if (ev.touches.length !== 1) { + clearTimeout(longPressTimer); + longPressTimer = null; + } if (ev.touches.length === 1) { touchLastX = ev.touches[0].clientX; touchLastY = ev.touches[0].clientY; @@ -725,6 +763,15 @@ Object.assign(CodemanApp.prototype, { ev.preventDefault(); this._blurMobileTerminalInput(); } + // Hold still and this press becomes a text selection. Cancelled by any + // travel past the shared tap slop below, so a scroll can never become one. + longPressStartX = touchLastX; + longPressStartY = touchLastY; + clearTimeout(longPressTimer); + longPressTimer = setTimeout(() => { + longPressTimer = null; + this._beginTouchSelection(longPressStartX, longPressStartY); + }, window.CodemanTerminalInput.TOUCH_SELECT_LONG_PRESS_MS); lastTime = 0; if (scrollFrame) { cancelAnimationFrame(scrollFrame); @@ -738,6 +785,24 @@ Object.assign(CodemanApp.prototype, { container.addEventListener( 'touchmove', (ev) => { + // A drag that follows the long press grows the selection instead of + // scrolling; preventDefault keeps the page from taking the gesture back. + if (this._touchSelecting) { + ev.preventDefault(); + const selTouch = ev.touches[0]; + if (selTouch) this._extendTouchSelection(selTouch.clientX, selTouch.clientY); + return; + } + if (longPressTimer && ev.touches.length === 1) { + const t = ev.touches[0]; + if ( + Math.abs(t.clientX - longPressStartX) > TAP_THRESHOLD || + Math.abs(t.clientY - longPressStartY) > TAP_THRESHOLD + ) { + clearTimeout(longPressTimer); + longPressTimer = null; + } + } if (ev.touches.length === 1 && isTouching) { const touchY = ev.touches[0].clientY; if (!didScroll && Math.abs(touchY - touchStartY) >= TAP_THRESHOLD) { @@ -779,7 +844,21 @@ Object.assign(CodemanApp.prototype, { container.addEventListener( 'touchend', (ev) => { + if (ev.target?.closest?.('.term-select-bar')) return; + clearTimeout(longPressTimer); + longPressTimer = null; isTouching = false; + if (this._touchSelecting) { + // Lifting ends the DRAG, not the selection: the bar stays up so the + // range can still be extended by tapping, or copied. preventDefault + // cancels the compatibility mouse events this touchend would otherwise + // synthesize — see _endTouchSelectionGesture. + ev.preventDefault(); + velocity = 0; + this._endTouchSelectionGesture(); + tapStartedWithTerminalFocus = false; + return; + } if (!scrollFrame && Math.abs(velocity) > 0.3) { scrollFrame = requestAnimationFrame(scrollLoop); } @@ -798,13 +877,28 @@ Object.assign(CodemanApp.prototype, { } tapStartedWithTerminalFocus = false; }, - { passive: true } + // NOT passive: the selection branch above must be able to preventDefault + // the compatibility mouse events. Every other path leaves the event alone. + { passive: false } ); + // Android Chrome fires `contextmenu` at its long-press threshold and then runs + // its default long-press behaviour. Suppressed ONLY while a selection gesture + // is in flight — a desktop right-click keeps its menu, since the timer is null + // and no gesture is active there. + container.addEventListener('contextmenu', (ev) => { + if (longPressTimer !== null || this._touchSelecting || this._touchSelectionActive) { + ev.preventDefault(); + } + }); + container.addEventListener( 'touchcancel', () => { + clearTimeout(longPressTimer); + longPressTimer = null; isTouching = false; + this._touchSelecting = false; velocity = 0; pixelAccum = 0; tapStartedWithTerminalFocus = false; @@ -1641,6 +1735,294 @@ Object.assign(CodemanApp.prototype, { return true; }, + // ═══════════════════════════════════════════════════════════════ + // Touch text selection — long-press to select, tap to extend, Copy + // ═══════════════════════════════════════════════════════════════ + // + // There was no way to copy terminal text from a phone at all. Three layers + // ruled it out at once: `user-select: none` on the whole terminal subtree + // (taps are cursor gestures there, so the OS callout had to go), the WebGL + // renderer drawing glyphs as pixels with only the accessibility tree behind + // them, and xterm's own selection being a mouse DRAG — while the tap path + // dispatches a zero-movement mousedown/mouseup pair, i.e. a click. + // + // So the gesture drives xterm's selection API directly (`select`, public and + // renderer-independent, and the highlight is drawn by xterm itself). Long-press + // is free real estate: tap and swipe are taken, long-press and double-tap are + // used by nothing. + + /** + * While a selection gesture is in flight, the terminal input must not hold focus. + * + * ⚠️ This is the guard that actually fixes "the keyboard pops up the moment the + * selection appears". The mouse-event guard cannot: the focus does not arrive + * through a mouse event at all. Android Chrome runs its own long-press handling + * at ~500ms and focuses the nearest editable element — xterm's helper textarea, + * a real