From 346bc8b1737be25f1b63fdaa42013d2574420c6d Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Mon, 27 Jul 2026 22:25:29 +0200 Subject: [PATCH] fix(terminal): link whole URLs and paths instead of truncating them Three separate truncations, each cutting a clickable link short so it opened the wrong target (or nothing at all). 1. A single `&` ended the match. It is a query-parameter separator, so every real query string was cut: a WordPress edit link resolved to `?post=1479` and opened the post list instead of the editor, and Claude Code's own `/login` URL was not usable at all. `&` is now part of a URL; `&&` stays a boundary, since that is the shell operator and never appears inside one. A lone trailing `&` is still trimmed as punctuation. 2. Links longer than the terminal is wide were cut at the row boundary. xterm calls the link provider once per visible ROW and translateToString returns only that row, despite a comment here claiming it handled wrapping. The provider now stitches the continuation rows back into one logical line and maps match offsets back to (x, y), so a link can span rows. Two kinds of continuation exist and handling only the first is not enough. A SOFT wrap is the emulator running out of columns, which flags the next row `isWrapped`. A HARD wrap is the program wrapping its own output and emitting a real newline, which flags nothing: Ink does this, which is why the /login URL was cut at the window edge and why the clickable part grew when the window was widened. A row that fills the full width is now treated as continuing into the next, that being the only trace a hard wrap leaves behind. Bounded to 12 rows so a screenful of wide output cannot make every hover re-scan the viewport. 3. Image and PDF paths were not matched at all. `.claude-images/paste-*.png`, what Codeman writes for a pasted screenshot, rendered as plain text. Those extensions are now linked and open the file preview, which renders images inline, rather than the log viewer, which would show binary noise. Verified in a real terminal: a 450-char /login URL hard-wrapped across 5 rows with zero isWrapped flags in the buffer (so a genuine hard wrap, not the soft case) links intact, as do soft-wrapped URLs and a wrapped attachment path. Regression cases added to link-provider-regex.test.ts, which extracts the patterns from the shipped source so they cannot drift. Its existing ReDoS guard still passes, which matters because this changes a pattern that once froze the tab on hover. Co-Authored-By: Claude Opus 5 (1M context) --- src/web/public/terminal-ui.js | 107 ++++++++++++++++++++++++++----- test/link-provider-regex.test.ts | 55 ++++++++++++++++ 2 files changed, 146 insertions(+), 16 deletions(-) diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index f309a31b..7f502b2a 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -969,8 +969,66 @@ Object.assign(CodemanApp.prototype, { return; } - // Get line text - translateToString handles wrapped lines - const lineText = line.translateToString(true); + // Stitch the LOGICAL line back together. + // + // xterm invokes this provider per visible ROW, and translateToString returns + // that row alone (the old comment here claimed otherwise). A URL or path + // longer than the terminal is wide therefore matched only as far as the row + // boundary, and the link opened a PREFIX of the real target. Walk out to both + // ends of the continuation, match against the joined text, and map offsets + // back to (x, y) so a link can span rows. + // + // Two different kinds of continuation, and handling only the first is not + // enough: + // 1. SOFT wrap: the emulator ran out of columns and flags the next row + // `isWrapped`. + // 2. HARD wrap: the program did its own wrapping and emitted a real + // newline, so nothing is flagged. Ink does this, which is why Claude + // Code's own `/login` URL was cut at the window edge, and why the + // clickable part grew when the window was widened. + // A row that fills the full width is treated as continuing into the next: + // that is the signal a hard wrap leaves behind, and a line that genuinely + // ended would stop short of the last column. + const cols = self.terminal.cols; + const rowAt = (r) => buffer.getLine(r - 1); + const continuesPrevious = (r) => { + if (r <= 1) return false; + if (rowAt(r)?.isWrapped) return true; + const prev = rowAt(r - 1); + return !!prev && prev.translateToString(true).length >= cols; + }; + + // Bounded so a screenful of full-width output (wide tables, box drawing) + // cannot make every hover stitch and re-scan the entire viewport. + const MAX_STITCHED_ROWS = 12; + let startRow = bufferLineNumber; + while (startRow > 1 && bufferLineNumber - startRow < MAX_STITCHED_ROWS && continuesPrevious(startRow)) { + startRow--; + } + let endRow = bufferLineNumber; + while (endRow < buffer.length && endRow - startRow < MAX_STITCHED_ROWS && continuesPrevious(endRow + 1)) { + endRow++; + } + + const rowTexts = []; + for (let r = startRow; r <= endRow; r++) { + const row = rowAt(r); + if (!row) break; + // Only the final row may be trimmed. Continuation rows fill the width by + // definition, and trimming one would shift every later offset. + rowTexts.push(row.translateToString(r === endRow)); + } + const lineText = rowTexts.join(''); + + /** Map an offset in the stitched text back to a 1-based terminal cell. */ + const coordAt = (index) => { + let rest = index; + for (let i = 0; i < rowTexts.length - 1; i++) { + if (rest < rowTexts[i].length) return { x: rest + 1, y: startRow + i }; + rest -= rowTexts[i].length; + } + return { x: rest + 1, y: startRow + rowTexts.length - 1 }; + }; if (!lineText || !lineText.includes('/')) { callback(undefined); @@ -980,22 +1038,27 @@ Object.assign(CodemanApp.prototype, { const links = []; // Pattern 0: URLs (https://, http://) — matched first so they take priority - const urlPattern = /https?:\/\/[^\s"'<>|;&)\]\x00-\x1f]+/g; + // + // A single `&` is PART of the URL: it separates query parameters, so excluding + // it truncated every real query string (`?post=1479&action=edit` linked only + // through `1479`, landing on the wrong page). `&&` is still a boundary, since + // that is the shell operator and never appears inside a URL. A lone trailing + // `&` is trimmed below with the other trailing punctuation. + const urlPattern = /https?:\/\/(?:[^\s"'<>|;&)\]\x00-\x1f]|&(?!&))+/g; const addUrlLink = (url, matchIndex) => { // Strip trailing punctuation that's likely not part of the URL - const cleaned = url.replace(/[.,;:!?)]+$/, ''); + const cleaned = url.replace(/[.,;:!?)&]+$/, ''); const startCol = lineText.indexOf(cleaned, matchIndex); if (startCol === -1) return; - if (links.some((l) => l.range.start.x === startCol + 1)) return; + const start = coordAt(startCol); + const end = coordAt(startCol + cleaned.length); + if (links.some((l) => l.range.start.x === start.x && l.range.start.y === start.y)) return; links.push({ text: cleaned, - range: { - start: { x: startCol + 1, y: bufferLineNumber }, - end: { x: startCol + cleaned.length + 1, y: bufferLineNumber }, - }, + range: { start, end }, decorations: { pointerCursor: true, underline: true }, activate(_event, text) { window.open(text, '_blank', 'noopener,noreferrer'); @@ -1017,31 +1080,43 @@ Object.assign(CodemanApp.prototype, { // the whole tab on hover. Non-empty token + bounded reps is O(n). const cmdPattern = /\b(tail|cat|head|less|grep|watch|vim|nano)\s+(?:[^\s\/]+\s+){0,4}(\/[^\s"'<>|;&\n\x00-\x1f]+)/g; - // Pattern 2: Paths with common extensions + // Pattern 2: Paths with common extensions. + // Image/PDF extensions are included so pasted-attachment paths + // (`.claude-images/paste-*.png`) are clickable; they open the file preview + // rather than the log viewer (see addLink). const extPattern = - /(\/(?:home|tmp|var|etc|opt)[^\s"'<>|;&\n\x00-\x1f]*\.(?:log|txt|json|md|yaml|yml|csv|xml|sh|py|ts|js))\b/g; + /(\/(?:home|tmp|var|etc|opt)[^\s"'<>|;&\n\x00-\x1f]*\.(?:log|txt|json|md|yaml|yml|csv|xml|sh|py|ts|js|png|jpe?g|gif|webp|bmp|svg|pdf))\b/g; // Pattern 3: Bash() tool output const bashPattern = /Bash\([^)]*?(\/(?:home|tmp|var|etc|opt)[^\s"'<>|;&\)\n\x00-\x1f]+)/g; + /** Extensions that should open the image/document preview, not the log viewer. */ + const PREVIEW_EXTS = new Set(['png', 'jpg', 'jpeg', 'gif', 'webp', 'bmp', 'svg', 'pdf']); + const addLink = (filePath, matchIndex) => { const startCol = lineText.indexOf(filePath, matchIndex); if (startCol === -1) return; + const start = coordAt(startCol); + const end = coordAt(startCol + filePath.length); // Skip if already have link at this position - if (links.some((l) => l.range.start.x === startCol + 1)) return; + if (links.some((l) => l.range.start.x === start.x && l.range.start.y === start.y)) return; links.push({ text: filePath, - range: { - start: { x: startCol + 1, y: bufferLineNumber }, // 1-based - end: { x: startCol + filePath.length + 1, y: bufferLineNumber }, - }, + range: { start, end }, // 1-based, may span wrapped rows decorations: { pointerCursor: true, underline: true, }, activate(event, text) { + // Tailing a PNG in the log viewer shows binary noise; the file preview + // already renders images and PDFs inline. + const ext = (text.split('.').pop() || '').toLowerCase(); + if (PREVIEW_EXTS.has(ext)) { + self.openFilePreview(text, self.activeSessionId); + return; + } self.openLogViewerWindow(text, self.activeSessionId); }, hover() { diff --git a/test/link-provider-regex.test.ts b/test/link-provider-regex.test.ts index 23590b10..e51f9f58 100644 --- a/test/link-provider-regex.test.ts +++ b/test/link-provider-regex.test.ts @@ -79,6 +79,61 @@ describe('terminal link-provider regexes (shipped source)', () => { } }); + it('urlPattern keeps query strings whole (a single & is part of the URL)', () => { + // Excluding `&` truncated every real query string: a WordPress edit link + // resolved to `?post=1479` and opened the wrong page, and Claude Code's OAuth + // login URL (many `&` params) was not clickable at all. + const url = shippedPattern('urlPattern'); + const strip = (u: string) => u.replace(/[.,;:!?)&]+$/, ''); + const cases: Array<[string, string]> = [ + [ + 'updated in place: https://bio-hacking.blog/wp-admin/post.php?post=1479&action=edit', + 'https://bio-hacking.blog/wp-admin/post.php?post=1479&action=edit', + ], + [ + 'open https://claude.ai/oauth/authorize?code=true&client_id=abc&scope=user%3Ainference&state=xyz', + 'https://claude.ai/oauth/authorize?code=true&client_id=abc&scope=user%3Ainference&state=xyz', + ], + ['see https://x.com/a?b=1&c=2&d=3 ok', 'https://x.com/a?b=1&c=2&d=3'], + // A lone trailing & is punctuation, not part of the target. + ['trailing https://x.com/a?b=1& next', 'https://x.com/a?b=1'], + ]; + for (const [line, want] of cases) { + url.lastIndex = 0; + const m = url.exec(line); + expect(m, line).not.toBeNull(); + expect(strip(m![0]), line).toBe(want); + } + }); + + it('urlPattern still stops at the shell && operator', () => { + // `&&` never appears inside a URL, so it must remain a boundary or a link + // would swallow the next command. + const url = shippedPattern('urlPattern'); + for (const line of ['curl https://x.com/api && echo done', 'curl https://x.com/api&&echo done']) { + url.lastIndex = 0; + expect(url.exec(line)![0], line).toBe('https://x.com/api'); + } + }); + + it('extPattern links pasted image/PDF attachment paths', () => { + // `.claude-images/paste-*.png` is what Codeman writes for a pasted screenshot; + // without image extensions the path rendered as plain, unclickable text. + const ext = shippedPattern('extPattern'); + const cases = [ + '/home/arkon/default/claudeman/.claude-images/paste-1785164958410-d11eb7d0.png', + '/tmp/shot.jpeg', + '/opt/app/report.pdf', + '/home/a/diagram.svg', + ]; + for (const path of cases) { + ext.lastIndex = 0; + const m = ext.exec(`see ${path} here`); + expect(m, path).not.toBeNull(); + expect(m![1], path).toBe(path); + } + }); + it('cmdPattern arg group cannot match empty tokens (the exponential trigger)', () => { // structural guard: the dangerous construct is an empty-matchable token // inside a repeated group — `[^\s\/]*\s+` repeated. Check the pattern