From 9f62da9ac311b593e1b3b135704d419c159b5664 Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Thu, 22 Oct 2026 17:22:21 -0400 Subject: [PATCH 001/191] feat(terminal): link URLs and paths that wrap across rows or sit in table cells A CLI that word-wraps inside a markdown table cell or an indented paragraph breaks a long URL or path short of the right edge and indents the rest, so nothing is flagged isWrapped and no row is full width. Each row linked only a fragment, and the fragment opened the wrong target. - findTerminalUrls() (constants.js) is the URL grammar for the link provider: a ")" that closes a "(" inside the URL is kept, an unmatched one still ends it, so (https://...) and [text](https://...) stay clean. - terminalUrlContinuationExtent() joins rows the CLI wrapped a URL across, only when the fragment ends on a wrap-break character, the next row is indented no further right than the URL started, and its first token looks like URL material. Table columns after the first fragment are left out. - terminalLinkLine() widens terminalPathLine() to that extent; a URL that spans rows is emitted as one link per row, each opening the whole URL, so the indent and other columns between the pieces are not underlined. - terminalCellWrappedPaths() follows an absolute path down its own table cell (space-separated or box-drawn), so two-column tables link both paths. - A full-width table divider no longer counts as a hard-wrapped line. - OSC 8 hyperlinks open through the same opener, web URLs only. --- src/web/public/constants.js | 307 ++++++++++++++++++++- src/web/public/terminal-tile.js | 2 + src/web/public/terminal-ui.js | 240 ++++++++++------ test/terminal-cell-wrapped-paths.test.ts | 210 ++++++++++++++ test/terminal-url-continuation.test.ts | 337 +++++++++++++++++++++++ test/webview-loopback-links.test.ts | 5 +- 6 files changed, 1018 insertions(+), 83 deletions(-) create mode 100644 test/terminal-cell-wrapped-paths.test.ts create mode 100644 test/terminal-url-continuation.test.ts diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 4cb2f099..6e8d4bff 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2173,9 +2173,9 @@ function findFilePathLinks(text) { // `?img=b.png` belong to the URL link, not to a second, overlapping file link. // (`file:///home/x.png` is not one of them: its path is the file.) const urls = []; - const urlPattern = /\bhttps?:\/\/\S+/gi; + const webUrlPattern = /\bhttps?:\/\/\S+/gi; let match; - while ((match = urlPattern.exec(value)) !== null) urls.push({ index: match.index, path: match[0] }); + while ((match = webUrlPattern.exec(value)) !== null) urls.push({ index: match.index, path: match[0] }); const inUrl = (index, length) => urls.some((u) => index < u.index + u.path.length && u.index < index + length); const absolute = absoluteFilePathPattern(); @@ -2334,7 +2334,12 @@ function terminalLogicalLine(buffer, row, cols, maxRows) { if (r <= 0) return false; if (lineAt(r)?.isWrapped) return true; const prev = lineAt(r - 1); - return !!prev && (prev.translateToString(true) || '').length >= width; + const prevText = prev?.translateToString(true) || ''; + // A full-width table divider is a row boundary, not a hard-wrapped line. + // Treating it as a continuation glues the divider onto the row below and + // hides that row's wrapped URL from the link provider. + if (/^[\s─━═╌╍┄┅┈┉-]+$/u.test(prevText)) return false; + return prevText.length >= width; }; let startRow = row; @@ -2508,6 +2513,293 @@ function terminalPathLine(buffer, row, cols, maxRows) { return { startRow: parts[0].startRow, endRow: parts[parts.length - 1].endRow, text, offsetToCell, cellToOffset }; } +/** + * Web URLs in terminal text, with the trimming the link provider applies. + * + * A single `&` is part of the URL (query separator); `&&` is the shell operator + * and ends it. `)` is kept INSIDE a URL when it closes a `(` that is also inside + * it (`…/Team-Space(QA)Archive/…` used to link only as far as `(QA`), and an + * unmatched `)` ends the URL, which keeps the closing paren of `(https://…)` and + * `[text](https://…)` out of the link. Trailing sentence punctuation is trimmed. + * + * @param {string} text + * @returns {Array<{url: string, startIndex: number, endIndex: number, rawEnd: number}>} + * `endIndex` is exclusive; `rawEnd` is where the match ran to before the + * paren/punctuation trim (the wrap-continuation test needs it). + */ +function findTerminalUrls(text) { + // Named `urlPattern` on purpose: test/link-provider-regex.test.ts extracts it + // by name to prove it stays linear-time on hostile lines. + const urlPattern = /https?:\/\/(?:[^\s"'<>|;&\]\x00-\x1f]|&(?!&))+/g; + const urls = []; + let match; + while ((match = urlPattern.exec(String(text || ''))) !== null) { + const raw = match[0]; + let end = raw.length; + let depth = 0; + for (let i = 0; i < raw.length; i++) { + if (raw[i] === '(') depth++; + else if (raw[i] === ')') { + if (depth === 0) { + end = i; + break; + } + depth--; + } + } + const url = raw.slice(0, end).replace(/[.,;:!?&]+$/, ''); + if (!/:\/\/[^/]/.test(url)) continue; + urls.push({ url, startIndex: match.index, endIndex: match.index + url.length, rawEnd: match.index + raw.length }); + } + return urls; +} + +/** + * The row span a URL covers when the CLI wrapped it with its OWN newlines at a + * point short of the terminal width. + * + * {@link terminalLogicalLine} joins soft wraps and full-width hard wraps. A CLI + * that word-wraps inside a table cell or a paragraph (codex's markdown tables, + * for one) breaks a long URL at `/`, `?`, `-` … well before the right edge and + * indents the rest to the cell's column, so neither rule fires and each row + * linked a fragment. A row continues the URL above it only when ALL hold: + * - the URL fragment ends on a character a wrapper breaks after + * (`/ ? & = - _ % # + ~`); a following table column may be excluded; + * - the next row is indented no further right than the column the URL + * started in (a table cell or list item continuation, not unrelated text); + * - that row's first token looks like URL material (contains one of + * `% & = / ? . - _` or a digit) and is not itself a new `scheme://`. + * The last rule is what stops prose such as "…/files/" + "then run it" from + * being glued into one link. + * + * Bounded by `maxRows`, like the logical line. Pure: reads rows only. + * + * @param {{getLine: (row: number) => any, length: number}} buffer + * @param {number} row 0-based absolute row being examined. + * @param {number} cols Terminal width. + * @param {number} [maxRows] + * @returns {{startRow: number, endRow: number, firstRowUrlEnd?: number} | null} + * `firstRowUrlEnd` is where the URL's cell ends on the first row when + * other table columns follow it; null means the row is not part of a + * URL continued this way. + */ +function terminalUrlContinuationExtent(buffer, row, cols, maxRows) { + const bound = Math.max(1, maxRows || 12); + const here = terminalLogicalLine(buffer, row, cols, bound); + if (!here) return null; + + const chainFrom = (start) => { + const first = terminalLogicalLine(buffer, start, cols, bound); + if (!first || first.startRow !== start) return null; + let text = first.text.replace(/\s+$/, ''); + let endRow = first.endRow; + let urlStartCol = -1; + let firstRowUrlEnd = null; + let extended = false; + while (endRow - start + 1 < bound) { + const urls = findTerminalUrls(text); + const last = urls[urls.length - 1]; + const trailingColumns = + endRow === first.endRow && last && last.rawEnd < text.length && /^ {2,}\S/.test(text.slice(last.rawEnd)); + const urlText = trailingColumns ? text.slice(0, last.rawEnd) : text; + if (!last || last.rawEnd !== urlText.length || !/[/?&=\-_%#+~]$/.test(urlText)) break; + if (urlStartCol < 0) urlStartCol = first.offsetToCell(last.startIndex).col; + const next = buffer.getLine(endRow + 1); + if (!next || next.isWrapped) break; + const nextText = next.translateToString(true) || ''; + const indent = nextText.length - nextText.replace(/^\s+/, '').length; + const token = nextText.slice(indent).split(/\s/, 1)[0]; + if (!token || indent > urlStartCol) break; + // When other table columns follow the URL, the next row must align with + // the start of its cell. A new numbered row must not become URL text. + if ( + trailingColumns && + (!indent || + first.text[indent - 1] !== ' ' || + first.text[indent] === ' ' || + / {2,}/.test(first.text.slice(indent, last.startIndex))) + ) + break; + if (/^[a-z][a-z0-9+.-]*:\/\//i.test(token) || !/^[A-Za-z0-9%_~]/.test(token)) break; + if (!/[%&=/?.\-_0-9]/.test(token)) break; + const nextLogical = terminalLogicalLine(buffer, endRow + 1, cols, bound); + if (!nextLogical) break; + if (trailingColumns) firstRowUrlEnd = last.rawEnd; + text = urlText + nextLogical.text.slice(indent).replace(/\s+$/, ''); + endRow = nextLogical.endRow; + extended = true; + } + return { startRow: start, endRow, extended, firstRowUrlEnd }; + }; + + // Walk up through earlier logical lines: the nearest one whose URL chain + // reaches this row owns it. + let start = here.startRow; + while (start >= 0 && here.startRow - start < bound) { + const chain = chainFrom(start); + if (chain && chain.extended && chain.endRow >= here.startRow) { + return { + startRow: chain.startRow, + endRow: chain.endRow, + ...(chain.firstRowUrlEnd === null ? {} : { firstRowUrlEnd: chain.firstRowUrlEnd }), + }; + } + if (start === 0) break; + const above = terminalLogicalLine(buffer, start - 1, cols, bound); + if (!above) break; + start = above.startRow; + } + return null; +} + +/** + * The text the link provider scans for `row`: {@link terminalPathLine}, widened + * to the rows of a URL the CLI wrapped short of the edge + * ({@link terminalUrlContinuationExtent}). Same shape as terminalPathLine. + * + * Over a widened span every row is read trimmed (except one a soft wrap + * continues from, which fills the width), a HARD continuation's indent is + * dropped (the cell mapping keeps the cells), and the first row stops at the + * URL's cell when other table columns follow it, so those columns never sit + * between the URL's pieces. + * + * @returns Same shape as {@link terminalPathLine}; null when the row does not exist. + */ +function terminalLinkLine(buffer, row, cols, maxRows) { + const base = terminalPathLine(buffer, row, cols, maxRows); + if (!base) return null; + const extent = terminalUrlContinuationExtent(buffer, row, cols, maxRows); + if (!extent || (extent.startRow >= base.startRow && extent.endRow <= base.endRow)) return base; + const startRow = Math.min(extent.startRow, base.startRow); + const endRow = Math.max(extent.endRow, base.endRow); + const segments = []; + let text = ''; + for (let r = startRow; r <= endRow; r++) { + const line = buffer.getLine(r); + if (!line) break; + const next = r < endRow ? buffer.getLine(r + 1) : null; + let rowText = line.translateToString(!(next && next.isWrapped)) || ''; + if (r === extent.startRow && extent.firstRowUrlEnd != null) rowText = rowText.slice(0, extent.firstRowUrlEnd); + let colStart = 0; + if (r > startRow && !line.isWrapped) { + colStart = rowText.length - rowText.replace(/^\s+/, '').length; + rowText = rowText.slice(colStart); + } + segments.push({ row: r, textStart: text.length, colStart }); + text += rowText; + } + const offsetToCell = (offset) => { + for (let i = segments.length - 1; i >= 0; i--) { + const seg = segments[i]; + if (offset >= seg.textStart || i === 0) return { row: seg.row, col: seg.colStart + (offset - seg.textStart) }; + } + return { row: startRow, col: offset }; + }; + const cellToOffset = (targetRow, targetCol) => { + const seg = segments.find((s) => s.row === targetRow); + return seg ? seg.textStart + Math.max(0, targetCol - seg.colStart) : -1; + }; + return { startRow, endRow: segments[segments.length - 1].row, text, offsetToCell, cellToOffset }; +} + +/** + * Absolute file paths the CLI word-wrapped INSIDE a table cell (codex markdown + * tables), returned as one link per path with a segment per row. + * + * Before screenshot (/tmp/x/docs/review/ After screenshot (/tmp/x/review/BLDMESH- + * BLDMESH-243/before-overview.png) 243/after-overview.png) + * + * Neither fragment matches FILE_PATH_LINK_PATTERN alone (the first has no + * extension, the second no absolute root), and a linear stitch of the rows + * cannot serve a two-column table: each column's continuation sits in the same + * next row. So each fragment is followed DOWN ITS OWN CELL: the cell's left + * edge is where its text starts after the previous ≥2-space column gap, and the + * continuation is the token starting at exactly that column on the next row. + * A fragment qualifies only when it is the last thing in its cell on that row + * (row end or a ≥2-space gap follows), and the join is accepted only when the + * whole joined string matches the path pattern from its first character, which + * is what keeps unrelated cell text from being glued on. + * + * Pure: reads rows only. Cells are taken one per string character (xterm rows + * with wide glyphs would shift columns; such rows simply fail the alignment). + * + * @param {{getLine: (row: number) => any, length: number}} buffer + * @param {number} row 0-based absolute row being examined. + * @param {number} [maxRows] Bound on rows one path may span. + * @returns {Array<{path: string, segments: Array<{row: number, startCol: number, endCol: number}>}>} + * Paths with a segment on `row`; `endCol` is exclusive, columns 0-based. + */ +function terminalCellWrappedPaths(buffer, row, maxRows) { + const bound = Math.max(2, maxRows || 4); + const rootPattern = new RegExp(`\\/(?:${FILE_LINK_ROOT_GROUP})\\/`, 'g'); + // Box-drawing verticals are cell borders (Claude Code draws tables with them). + const isPathChar = (ch) => !!ch && !/[\s"'<>|;&\x00-\x1f│┃║]/.test(ch); + const rowText = (r) => { + const line = buffer.getLine(r); + return line ? (line.translateToString(true) || '').replace(/\s+$/, '') : null; + }; + // A fragment is the last text in its cell on this row when the row ends, a + // column gap (≥2 spaces) follows it, or a cell border does. + const endsCell = (text, end) => end >= text.length || /^(?: {2,}| *[│┃║|])/.test(text.slice(end)); + + const fromStart = (startRow) => { + const found = []; + const text = rowText(startRow); + if (!text) return found; + rootPattern.lastIndex = 0; + let m; + while ((m = rootPattern.exec(text)) !== null) { + const start = m.index; + if (start > 0 && !/[\s(\[{"'`<]/.test(text[start - 1])) continue; + let end = start; + while (end < text.length && isPathChar(text[end])) end++; + rootPattern.lastIndex = Math.max(end, start + 1); + if (!endsCell(text, end)) continue; + // Already a complete path on its own row: the ordinary detector owns it. + const alone = absoluteFilePathPattern().exec(text.slice(start, end)); + if (alone && alone.index === 0 && alone[0].length === end - start) continue; + // The cell's left edge: just past the nearest ≥2-space gap or cell border + // (and the padding after it) before the path. + const before = text.slice(0, start); + let cellCol = before.length - before.replace(/^\s+/, '').length; + const edge = /(?: {2,}|[│┃║|] *)(?=\S)/g; + let e; + const upToPath = text.slice(0, start + 1); + while ((e = edge.exec(upToPath)) !== null) cellCol = e.index + e[0].length; + + let joined = text.slice(start, end); + const segments = [{ row: startRow, startCol: start, endCol: end }]; + for (let r = startRow + 1; r < startRow + bound; r++) { + const line = buffer.getLine(r); + if (!line || line.isWrapped) break; + const next = rowText(r); + if (!next || !isPathChar(next[cellCol]) || (cellCol > 0 && next[cellCol - 1] !== ' ')) break; + let tokenEnd = cellCol; + while (tokenEnd < next.length && isPathChar(next[tokenEnd])) tokenEnd++; + const candidate = joined + next.slice(cellCol, tokenEnd); + const match = absoluteFilePathPattern().exec(candidate); + if (match && match.index === 0 && match[0].length > joined.length) { + segments.push({ row: r, startCol: cellCol, endCol: cellCol + match[0].length - joined.length }); + found.push({ path: match[0], segments }); + break; + } + if (!endsCell(next, tokenEnd)) break; + joined = candidate; + segments.push({ row: r, startCol: cellCol, endCol: tokenEnd }); + } + } + return found; + }; + + const results = []; + for (let s = row; s >= 0 && row - s < bound; s--) { + for (const hit of fromStart(s)) { + if (hit.segments.some((seg) => seg.row === row)) results.push(hit); + } + } + return results; +} + // ═══════════════════════════════════════════════════════════════ // Split-Pane Sessions — pure helpers (divider math, picker list) // ═══════════════════════════════════════════════════════════════ @@ -3429,7 +3721,14 @@ if (typeof window !== 'undefined') { rendersAsPage, HTML_PAGE_EXTENSIONS, }; - window.CodemanTerminalLines = { terminalLogicalLine, terminalPathLine }; + window.CodemanTerminalLines = { + terminalLogicalLine, + terminalPathLine, + terminalLinkLine, + terminalUrlContinuationExtent, + findTerminalUrls, + terminalCellWrappedPaths, + }; window.CodemanUrlSession = { sessionIdFromFragment }; window.CodemanSplitPane = { clampDividerPercent, diff --git a/src/web/public/terminal-tile.js b/src/web/public/terminal-tile.js index 1a0c7058..961b56b3 100644 --- a/src/web/public/terminal-tile.js +++ b/src/web/public/terminal-tile.js @@ -244,6 +244,8 @@ scrollback: this.scrollback ?? DEFAULT_SCROLLBACK, allowTransparency: true, allowProposedApi: true, + // OSC 8 hyperlinks open through the primary pane's opener (terminal-ui.js). + linkHandler: global.app?._terminalOsc8LinkHandler?.(), }); this.fitAddon = new FitAddon.FitAddon(); diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index c3778ffe..e4794be6 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -702,6 +702,7 @@ Object.assign(CodemanApp.prototype, { scrollback: scrollback, allowTransparency: true, allowProposedApi: true, + linkHandler: this._terminalOsc8LinkHandler(), }); this.fitAddon = new FitAddon.FitAddon(); @@ -2143,8 +2144,11 @@ Object.assign(CodemanApp.prototype, { // links as the one file it is. // Bounded so a screenful of full-width output (wide tables, box drawing) // cannot make every hover stitch and re-scan the entire viewport. + // `terminalLinkLine` widens that once more to the rows of a URL the CLI + // wrapped short of the edge (a table cell or an indented paragraph), + // where nothing marks the break and no row is full width. const MAX_STITCHED_ROWS = 12; - const logical = window.CodemanTerminalLines?.terminalPathLine( + const logical = window.CodemanTerminalLines?.terminalLinkLine( buffer, bufferLineNumber - 1, terminal.cols, @@ -2162,8 +2166,16 @@ Object.assign(CodemanApp.prototype, { return { x: cell.col + 1, y: cell.row + 1 }; }; + // Paths a CLI word-wrapped inside a table cell (terminalCellWrappedPaths, + // constants.js). They follow their own cell down the rows, which the + // linear stitch above cannot do for a multi-column table, so they are + // found from the buffer directly and claim their cells over the rest. + const cellWrappedPaths = ( + window.CodemanTerminalLines?.terminalCellWrappedPaths?.(buffer, bufferLineNumber - 1, 4) ?? [] + ).filter((hit) => !/(^|\/)\.\.(\/|$)/.test(hit.path)); + // A path needs a `/` or an extension's `.`; a line with neither has no link. - if (!lineText || (!lineText.includes('/') && !lineText.includes('.'))) { + if (!lineText || (!lineText.includes('/') && !lineText.includes('.') && cellWrappedPaths.length === 0)) { callback(undefined); return; } @@ -2175,44 +2187,46 @@ Object.assign(CodemanApp.prototype, { const taken = []; const isTaken = (start, length) => taken.some((t) => start < t.end && t.start < start + length); - // Pattern 0: URLs (https://, http://) — matched first so they take priority - // - // 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 startCol = lineText.indexOf(cleaned, matchIndex); - if (startCol === -1) return; - - const start = coordAt(startCol); - const end = coordAt(startCol + cleaned.length); + // Pattern 0: URLs (https://, http://) — matched first so they take priority. + // The grammar (balanced parens, `&` vs `&&`, trailing punctuation) is + // findTerminalUrls() in constants.js, which the wrap-continuation extent + // above reads URLs with too, so the two cannot disagree. + const addUrlLink = ({ url, startIndex }) => { + const start = coordAt(startIndex); if (links.some((l) => l.range.start.x === start.x && l.range.start.y === start.y)) return; - taken.push({ start: startCol, end: startCol + cleaned.length }); + taken.push({ start: startIndex, end: startIndex + url.length }); - links.push({ - text: cleaned, - range: { start, end }, - decorations: { pointerCursor: true, underline: true }, - activate(_event, text) { - // A `localhost` link tapped from another device can only work - // through the server: route it into a proxied web tab - // (webview-tabs.js). Anything else opens as before. - if (self.openLinkThroughWebTabIfLoopback?.(text)) return; - window.open(text, '_blank', 'noopener,noreferrer'); - }, - hover() { - setHovered(true); - }, - leave() { - setHovered(false); - }, - }); + // One link per terminal row the URL occupies, each opening the WHOLE + // URL. A single range spanning rows would also underline (and make + // clickable) the indent and table columns between the pieces of a + // URL the CLI wrapped. A single-row URL keeps its original range. + const endIndex = startIndex + url.length; + const pieces = []; + let pieceStart = startIndex; + for (let i = startIndex + 1; i < endIndex; i++) { + if (coordAt(i).y !== coordAt(i - 1).y) { + pieces.push([pieceStart, i]); + pieceStart = i; + } + } + pieces.push([pieceStart, endIndex]); + for (let n = 0; n < pieces.length; n++) { + const [from, to] = pieces[n]; + links.push({ + text: url, + range: { start: coordAt(from), end: pieces.length === 1 ? coordAt(to) : coordAt(to - 1) }, + decorations: { pointerCursor: true, underline: true }, + activate() { + self._openTerminalUrl(url); + }, + hover() { + setHovered(true); + }, + leave() { + setHovered(false); + }, + }); + } }; // Pattern 1: Commands with file paths (tail -f, cat, head, grep pattern, etc.) @@ -2235,6 +2249,41 @@ Object.assign(CodemanApp.prototype, { // Pattern 3: Bash() tool output const bashPattern = /Bash\([^)]*?(\/(?:home|tmp|var|etc|opt)[^\s"'<>|;&\)\n\x00-\x1f]+)/g; + const activateFilePath = (text, folder = false) => { + // Tailing a PNG in the log viewer shows binary noise; the file preview + // already renders images, PDFs, documents and media inline — and it + // now reaches files outside the workspace too, which is where an + // agent's screenshots and scratchpad captures actually land. + // + // Text goes to the log viewer, which follows a file that is still + // being written — but ONLY where it can actually read: it spawns + // `tail -f` and allows the workspace, /var/log and ~/logs, so an + // out-of-workspace path there answered "Path must be within + // working directory or allowed log directories" while the SAME + // path clicked in the response viewer previewed fine. The preview + // reads those through the guarded attachment routes, so external + // paths route there and the two surfaces agree. + // + // A relative path is resolved against the session's working + // directory first, and a `~/` path always previews (only the + // server knows that host's home, and the log viewer cannot ask). + const sessionId = getSessionId(); + const target = self._resolveLinkedFilePath(text, sessionId); + // A folder path opens the folder grid (which falls back to the + // preview if the server finds a file there after all). + if (folder) { + self.openFolderGrid(target, sessionId); + return; + } + // An HTML file renders as a page in the preview, never as source + // in the log viewer (markdown is in FILE_PREVIEW_EXTENSIONS). + if (previewsInFileViewer(target) || rendersAsPage(target) || self._isExternalPreviewPath(target, sessionId)) { + self.openFilePreview(target, sessionId); + return; + } + self.openLogViewerWindow(target, sessionId); + }; + const addLink = (filePath, matchIndex, folder = false) => { const startCol = lineText.indexOf(filePath, matchIndex); if (startCol === -1) return; @@ -2253,43 +2302,8 @@ Object.assign(CodemanApp.prototype, { pointerCursor: true, underline: true, }, - activate(event, text) { - // Tailing a PNG in the log viewer shows binary noise; the file preview - // already renders images, PDFs, documents and media inline — and it - // now reaches files outside the workspace too, which is where an - // agent's screenshots and scratchpad captures actually land. - // - // Text goes to the log viewer, which follows a file that is still - // being written — but ONLY where it can actually read: it spawns - // `tail -f` and allows the workspace, /var/log and ~/logs, so an - // out-of-workspace path there answered "Path must be within - // working directory or allowed log directories" while the SAME - // path clicked in the response viewer previewed fine. The preview - // reads those through the guarded attachment routes, so external - // paths route there and the two surfaces agree. - // - // A relative path is resolved against the session's working - // directory first, and a `~/` path always previews (only the - // server knows that host's home, and the log viewer cannot ask). - const sessionId = getSessionId(); - const target = self._resolveLinkedFilePath(text, sessionId); - // A folder path opens the folder grid (which falls back to the - // preview if the server finds a file there after all). - if (folder) { - self.openFolderGrid(target, sessionId); - return; - } - // An HTML file renders as a page in the preview, never as source - // in the log viewer (markdown is in FILE_PREVIEW_EXTENSIONS). - if ( - previewsInFileViewer(target) || - rendersAsPage(target) || - self._isExternalPreviewPath(target, sessionId) - ) { - self.openFilePreview(target, sessionId); - return; - } - self.openLogViewerWindow(target, sessionId); + activate(_event, text) { + activateFilePath(text, folder); }, hover() { setHovered(true); @@ -2303,9 +2317,8 @@ Object.assign(CodemanApp.prototype, { // Match all patterns — URLs first so they take priority let match; - urlPattern.lastIndex = 0; - while ((match = urlPattern.exec(lineText)) !== null) { - addUrlLink(match[0], match.index); + for (const reference of window.CodemanTerminalLines?.findTerminalUrls?.(lineText) ?? []) { + addUrlLink(reference); } cmdPattern.lastIndex = 0; @@ -2322,6 +2335,54 @@ Object.assign(CodemanApp.prototype, { addLink(match[1], match.index); } + // One link per row segment, each opening the WHOLE path (as for a wrapped + // URL). A fragment of the same cells linked by another detector (a + // relative `243/after-overview.png`, say) would open the wrong file, so + // the cell-wrapped path displaces it. + // A path the line stitch already joined whole (a single-column wrap, which + // terminalPathLine handles) keeps its one link. + const cellOnlyPaths = cellWrappedPaths.filter( + (hit) => + !links.some( + (l) => + l.text === hit.path && + l.range.start.y === hit.segments[0].row + 1 && + l.range.start.x === hit.segments[0].startCol + 1 + ) + ); + if (cellOnlyPaths.length > 0) { + const claimed = cellOnlyPaths.flatMap((hit) => hit.segments); + const overlapsClaim = (link) => + claimed.some((seg) => { + const y = seg.row + 1; + if (y < link.range.start.y || y > link.range.end.y) return false; + const from = y === link.range.start.y ? link.range.start.x : 1; + const to = y === link.range.end.y ? link.range.end.x : Number.MAX_SAFE_INTEGER; + return from <= seg.endCol && to >= seg.startCol + 1; + }); + for (let i = links.length - 1; i >= 0; i--) { + if (overlapsClaim(links[i])) links.splice(i, 1); + } + for (const hit of cellOnlyPaths) { + for (const seg of hit.segments) { + links.push({ + text: hit.path, + range: { start: { x: seg.startCol + 1, y: seg.row + 1 }, end: { x: seg.endCol, y: seg.row + 1 } }, + decorations: { pointerCursor: true, underline: true }, + activate() { + activateFilePath(hit.path); + }, + hover() { + setHovered(true); + }, + leave() { + setHovered(false); + }, + }); + } + } + } + if (links.length > 0) { console.debug( '[LinkProvider] Found links:', @@ -2344,6 +2405,29 @@ Object.assign(CodemanApp.prototype, { return provider; }, + /** + * xterm's handler for OSC 8 hyperlinks (a program's own `ESC ]8;;url` links), + * which take precedence over the text link provider. Routed through the same + * opener, and only for web URLs: any other scheme a program sends is ignored. + */ + _terminalOsc8LinkHandler() { + return { + activate: (_event, text) => { + if (!/^https?:\/\/[^\s\x00-\x1f]+$/i.test(text)) return; + this._openTerminalUrl(text); + }, + }; + }, + + /** Open a URL clicked in a terminal (text link or OSC 8). */ + _openTerminalUrl(text) { + // A `localhost` link tapped from another device can only work through the + // server: route it into a proxied web tab (webview-tabs.js). Anything else + // opens in a new tab. + if (this.openLinkThroughWebTabIfLoopback?.(text)) return; + window.open(text, '_blank', 'noopener,noreferrer'); + }, + /** * The terminal link under a viewport point, or null. * diff --git a/test/terminal-cell-wrapped-paths.test.ts b/test/terminal-cell-wrapped-paths.test.ts new file mode 100644 index 00000000..6ed0da6a --- /dev/null +++ b/test/terminal-cell-wrapped-paths.test.ts @@ -0,0 +1,210 @@ +// Port: none (pure functions + the link provider in a vm context — no browser, no server). +// +// Absolute file paths codex word-wrapped inside a markdown table cell were not +// clickable: the first fragment has no extension and the continuation has no +// absolute root, so FILE_PATH_LINK_PATTERN matched neither. In a two-column +// table both continuations sit on the same next row, so each path has to be +// followed down its OWN cell (terminalCellWrappedPaths in constants.js). +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { describe, expect, it, vi } from 'vitest'; + +const CONSTANTS = readFileSync(resolve(import.meta.dirname, '../src/web/public/constants.js'), 'utf8'); +const TERMINAL_UI = readFileSync(resolve(import.meta.dirname, '../src/web/public/terminal-ui.js'), 'utf8'); + +type Row = { text: string; isWrapped?: boolean }; +type Hit = { path: string; segments: Array<{ row: number; startCol: number; endCol: number }> }; +type Link = { + text: string; + range: { start: { x: number; y: number }; end: { x: number; y: number } }; + activate: (event: unknown, text: string) => void; +}; + +const COLS = 200; + +function fakeBuffer(rows: Row[], cols: number) { + const lines = rows.map((row) => ({ + isWrapped: row.isWrapped ?? false, + translateToString(trimRight?: boolean) { + const padded = row.text.padEnd(cols, ' '); + return trimRight ? padded.trimEnd() : padded; + }, + getCell(index: number) { + const padded = row.text.padEnd(cols, ' '); + if (index >= padded.length) return undefined; + return { getChars: () => (padded[index] === ' ' ? '' : padded[index]), getWidth: () => 1 }; + }, + })); + return { length: lines.length, getLine: (i: number) => lines[i] }; +} + +function loadPure() { + const context = vm.createContext({ console, window: undefined }); + vm.runInContext(`${CONSTANTS}\nglobalThis.__cell = terminalCellWrappedPaths;`, context, { + filename: 'constants.js', + }); + return (context as unknown as { __cell: (buffer: unknown, row: number, maxRows?: number) => Hit[] }).__cell; +} + +// Shape copied from a real codex pane (synthetic paths). +const ROW1 = + ' Before screenshot (/tmp/demo-check/docs/review/ After screenshot (/tmp/demo-check/docs/review/BLDMESH-'; +const ROW2 = ' BLDMESH-243/before-overview.png) 243/after-overview.png)'; +const TABLE: Row[] = [ + { text: ' No Vision selected Reprocessed' }, + { text: ROW1 }, + { text: ROW2 }, + { text: '' }, +]; +const BEFORE = '/tmp/demo-check/docs/review/BLDMESH-243/before-overview.png'; +const AFTER = '/tmp/demo-check/docs/review/BLDMESH-243/after-overview.png'; + +describe('terminalCellWrappedPaths', () => { + const find = loadPure(); + + it('joins each column’s path with its own continuation, from either row', () => { + for (const row of [1, 2]) { + const hits = find(fakeBuffer(TABLE, COLS), row); + expect(hits.map((h) => h.path).sort()).toEqual([AFTER, BEFORE]); + } + }); + + it('places the segments on the path cells only', () => { + const hits = find(fakeBuffer(TABLE, COLS), 1); + const before = hits.find((h) => h.path === BEFORE)!; + expect(before.segments).toEqual([ + { row: 1, startCol: ROW1.indexOf('/tmp/'), endCol: ROW1.indexOf('/review/') + '/review/'.length }, + { row: 2, startCol: 3, endCol: 3 + 'BLDMESH-243/before-overview.png'.length }, + ]); + const after = hits.find((h) => h.path === AFTER)!; + const afterCell = ROW1.indexOf('After'); + expect(after.segments[0]).toEqual({ row: 1, startCol: ROW1.lastIndexOf('/tmp/'), endCol: ROW1.length }); + expect(after.segments[1]).toEqual({ + row: 2, + startCol: afterCell, + endCol: afterCell + '243/after-overview.png'.length, + }); + }); + + it('ignores a path already complete on its row', () => { + const rows = [{ text: ' see /tmp/demo/report.md' }, { text: ' next.md' }]; + expect(find(fakeBuffer(rows, COLS), 0)).toEqual([]); + }); + + it('does not glue unrelated text onto a path that is not followed by a matching continuation', () => { + const rows = [{ text: ' saved under /tmp/demo/docs/' }, { text: ' then run the build' }]; + expect(find(fakeBuffer(rows, COLS), 0)).toEqual([]); + expect(find(fakeBuffer(rows, COLS), 1)).toEqual([]); + }); + + it('does not join a continuation that is not aligned to the cell', () => { + const rows = [{ text: ' shot (/tmp/demo/docs/review/' }, { text: ' before.png)' }]; + expect(find(fakeBuffer(rows, COLS), 0)).toEqual([]); + }); + + it('does not join across a soft wrap (the logical line owns that)', () => { + const rows = [{ text: ' shot (/tmp/demo/docs/review/' }, { text: ' before.png', isWrapped: true }]; + expect(find(fakeBuffer(rows, COLS), 0)).toEqual([]); + }); + + it('follows a path across three rows of one cell', () => { + const rows = [{ text: ' a (/tmp/demo/very/' }, { text: ' long/nested/' }, { text: ' file.png)' }]; + const hits = find(fakeBuffer(rows, COLS), 1); + expect(hits.map((h) => h.path)).toEqual(['/tmp/demo/very/long/nested/file.png']); + expect(hits[0].segments.map((s) => s.row)).toEqual([0, 1, 2]); + }); + + it('follows a path down a box-drawn table cell (Claude Code renders tables with │ borders)', () => { + const rows = [ + { text: '│ Before │ /tmp/demo/docs/review/ │ After │ /tmp/demo/docs/review/BLD- │' }, + { text: '│ │ before-overview.png │ │ 243/after-overview.png │' }, + ]; + const hits = find(fakeBuffer(rows, COLS), 1); + expect(hits.map((h) => h.path).sort()).toEqual([ + '/tmp/demo/docs/review/BLD-243/after-overview.png', + '/tmp/demo/docs/review/before-overview.png', + ]); + const before = hits.find((h) => h.path.endsWith('before-overview.png'))!; + expect(before.segments).toEqual([ + { row: 0, startCol: rows[0].text.indexOf('/tmp/'), endCol: rows[0].text.indexOf('/review/') + 8 }, + { row: 1, startCol: rows[1].text.indexOf('before'), endCol: rows[1].text.indexOf('.png') + 4 }, + ]); + }); + + it('does not take a box border or the next cell into a path', () => { + const rows = [{ text: '│ /tmp/demo/docs/ │ x │' }, { text: '│ │ notes.md │ y │' }]; + expect(find(fakeBuffer(rows, COLS), 0)).toEqual([]); + }); +}); + +describe('terminal link provider with cell-wrapped paths', () => { + function harness(rows: Row[]) { + const context = vm.createContext({ + console: { ...console, debug: vi.fn(), log: vi.fn() }, + CodemanApp: class CodemanApp {}, + setInterval: vi.fn(), + clearInterval: vi.fn(), + setTimeout, + clearTimeout, + requestAnimationFrame: vi.fn(), + document: { addEventListener: vi.fn(), getElementById: vi.fn(), documentElement: { dataset: {} } }, + window: { + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + open: vi.fn(), + document: { documentElement: { dataset: {} } }, + }, + }); + vm.runInContext(`${CONSTANTS}\n${TERMINAL_UI}\nglobalThis.__proto = CodemanApp.prototype;`, context); + const proto = (context as unknown as { __proto: Record unknown> }).__proto; + let provider: { provideLinks: (line: number, cb: (links?: Link[]) => void) => void } | undefined; + const app = { + terminal: { + cols: COLS, + rows: 24, + registerLinkProvider(value: typeof provider) { + provider = value; + }, + buffer: { active: { viewportY: 0, ...fakeBuffer(rows, COLS) } }, + }, + activeSessionId: 's1', + sessions: new Map([['s1', { workingDir: '/workspace/project' }]]), + openFilePreview: vi.fn(), + openLogViewerWindow: vi.fn(), + _openTerminalUrl: proto._openTerminalUrl, + _resolveLinkedFilePath: proto._resolveLinkedFilePath, + _isExternalPreviewPath: () => false, + registerFilePathLinkProvider: proto.registerFilePathLinkProvider, + }; + app.registerFilePathLinkProvider.call(app); + const provide = (line: number): Link[] => { + let links: Link[] | undefined; + provider!.provideLinks(line, (value) => { + links = value; + }); + return links ?? []; + }; + return { provide, app }; + } + + it('makes both screenshot paths clickable from either row and opens the whole path', () => { + const { provide, app } = harness(TABLE); + for (const line of [2, 3]) { + const links = provide(line); + for (const target of [BEFORE, AFTER]) { + const onRow = links.find((l) => l.text === target && l.range.start.y === line); + expect(onRow, `${target} on row ${line}`).toBeDefined(); + onRow!.activate(null, onRow!.text); + expect(app.openFilePreview).toHaveBeenLastCalledWith(target, 's1'); + } + } + }); + + it('leaves no fragment link that would open a partial path', () => { + const { provide } = harness(TABLE); + for (const line of [2, 3]) { + for (const link of provide(line)) expect([BEFORE, AFTER]).toContain(link.text); + } + }); +}); diff --git a/test/terminal-url-continuation.test.ts b/test/terminal-url-continuation.test.ts new file mode 100644 index 00000000..9616ac53 --- /dev/null +++ b/test/terminal-url-continuation.test.ts @@ -0,0 +1,337 @@ +// Port: none (pure functions + the link provider in a vm context — no browser, no server). +// +// Terminal URL links that stopped short. +// +// 1. `)` ended every URL, so a path like `…/Team-Space(QA)Archive/…` linked only as +// far as `(QA`. The grammar now keeps a `)` that closes a `(` inside the URL and +// still leaves out the closing paren of `(https://…)` / `[text](https://…)`. +// 2. A CLI that word-wraps inside a table cell (codex) breaks a long URL at `/`, `?`, +// `-` well short of the right edge and indents the rest to the cell's column. +// Nothing is `isWrapped` and no row is full width, so each row linked a fragment. +// +// All URLs here are synthetic (example.com); none come from real output. +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { describe, expect, it, vi } from 'vitest'; + +const CONSTANTS = readFileSync(resolve(import.meta.dirname, '../src/web/public/constants.js'), 'utf8'); +const TERMINAL_UI = readFileSync(resolve(import.meta.dirname, '../src/web/public/terminal-ui.js'), 'utf8'); + +type UrlRef = { url: string; startIndex: number; endIndex: number; rawEnd: number }; +type Extent = { startRow: number; endRow: number } | null; +type Row = { text: string; isWrapped?: boolean }; +type Link = { + text: string; + range: { start: { x: number; y: number }; end: { x: number; y: number } }; + activate: (event: unknown, text: string) => void; +}; + +function loadPure() { + const context = vm.createContext({ console, window: undefined }); + vm.runInContext( + `${CONSTANTS}\nglobalThis.__urls = findTerminalUrls; globalThis.__extent = terminalUrlContinuationExtent;`, + context, + { filename: 'constants.js' } + ); + const ctx = context as unknown as { + __urls: (text: string) => UrlRef[]; + __extent: (buffer: unknown, row: number, cols: number, maxRows?: number) => Extent; + }; + return { findTerminalUrls: ctx.__urls, terminalUrlContinuationExtent: ctx.__extent }; +} + +function fakeBuffer(rows: Row[], cols: number) { + const lines = rows.map((row) => ({ + isWrapped: row.isWrapped ?? false, + translateToString(trimRight?: boolean) { + const padded = row.text.padEnd(cols, ' '); + return trimRight ? padded.trimEnd() : padded; + }, + getCell(index: number) { + const padded = row.text.padEnd(cols, ' '); + if (index >= padded.length) return undefined; + return { getChars: () => (padded[index] === ' ' ? '' : padded[index]), getWidth: () => 1 }; + }, + })); + return { length: lines.length, getLine: (i: number) => lines[i] }; +} + +// A codex-style markdown table row: number, reason, then a `name (url)` cell whose +// URL the CLI wrapped across three rows, each continuation indented to the cell. +const URL = + 'https://docs.example.com/sites/Team-Space(QA)Archive/_layouts/15/Doc.aspx?' + + 'sourcedoc=%7B11111111-2222-3333-4444-555555555555%7D&file=Quarterly%20Plan%20(Draft).xlsx&action=default&' + + 'mobileredirect=true'; +const CELL = 26; +const TABLE: Row[] = [ + { text: ' 11 Unrelated row plain text only' }, + { + text: ' 12 New to trending Quarterly Plan.xlsx (https://docs.example.com/sites/Team-Space(QA)Archive/_layouts/15/Doc.aspx?', + }, + { + text: + ' '.repeat(CELL) + + 'sourcedoc=%7B11111111-2222-3333-4444-555555555555%7D&file=Quarterly%20Plan%20(Draft).xlsx&action=default&', + }, + { text: ' '.repeat(CELL) + 'mobileredirect=true)' }, + { text: ' 13 Next row another cell' }, +]; +const COLS = 160; +const TABLE_CELL_START = TABLE[1].text.indexOf('Quarterly'); +const TABLE_WITH_TRAILING_COLUMNS: Row[] = [ + TABLE[0], + { + text: TABLE[1].text + ' 2026-09-16 18:00:54 Document Owner', + }, + { text: ' '.repeat(TABLE_CELL_START) + TABLE[2].text.slice(CELL) }, + { text: ' '.repeat(TABLE_CELL_START) + TABLE[3].text.slice(CELL) }, + TABLE[4], +]; +const TABLE_COLS = 220; + +describe('findTerminalUrls (grammar)', () => { + const { findTerminalUrls } = loadPure(); + const urls = (text: string) => findTerminalUrls(text).map((u) => u.url); + + it('keeps a paren pair that is part of the URL', () => { + expect(urls('see https://example.com/wiki/Widget_(tool) now')).toEqual(['https://example.com/wiki/Widget_(tool)']); + expect(urls('https://example.com/Team-Space(QA)Archive/x')).toEqual([ + 'https://example.com/Team-Space(QA)Archive/x', + ]); + }); + + it('leaves out the paren that wraps the URL', () => { + expect(urls('(https://example.com/a(b)c)')).toEqual(['https://example.com/a(b)c']); + expect(urls('file (https://example.com/doc?x=1).')).toEqual(['https://example.com/doc?x=1']); + }); + + it('leaves out the closing paren of a markdown link', () => { + expect(urls('[the docs](https://example.com/docs?a=1&b=2) for details')).toEqual([ + 'https://example.com/docs?a=1&b=2', + ]); + }); + + it('still keeps query ampersands and stops at the shell &&', () => { + expect(urls('open https://example.com/p?post=1&action=edit&& echo done')).toEqual([ + 'https://example.com/p?post=1&action=edit', + ]); + }); + + it('trims trailing sentence punctuation', () => { + expect(urls('Go to https://example.com/path.')).toEqual(['https://example.com/path']); + expect(urls('Is it https://example.com/q?')).toEqual(['https://example.com/q']); + }); + + it('reports where the raw match ended before trimming', () => { + const [ref] = findTerminalUrls('x https://example.com/a) y'); + expect(ref.url).toBe('https://example.com/a'); + expect(ref.rawEnd).toBe('x https://example.com/a)'.length); + }); +}); + +describe('terminalUrlContinuationExtent (wrap joining)', () => { + const { terminalUrlContinuationExtent } = loadPure(); + + it('spans all three rows of a URL wrapped inside a table cell, from any of them', () => { + const buffer = fakeBuffer(TABLE, COLS); + for (const row of [1, 2, 3]) { + expect(terminalUrlContinuationExtent(buffer, row, COLS, 12)).toEqual({ startRow: 1, endRow: 3 }); + } + }); + + it('spans a table URL even when date and owner columns follow its first fragment', () => { + const buffer = fakeBuffer(TABLE_WITH_TRAILING_COLUMNS, TABLE_COLS); + for (const row of [1, 2, 3]) { + expect(terminalUrlContinuationExtent(buffer, row, TABLE_COLS, 12)).toMatchObject({ startRow: 1, endRow: 3 }); + } + }); + + it('starts at the URL row when a full-width table divider precedes it', () => { + const rows = [{ text: '─'.repeat(TABLE_COLS) }, ...TABLE_WITH_TRAILING_COLUMNS.slice(1, 4)]; + const buffer = fakeBuffer(rows, TABLE_COLS); + for (const row of [1, 2, 3]) { + expect(terminalUrlContinuationExtent(buffer, row, TABLE_COLS, 12)).toMatchObject({ startRow: 1, endRow: 3 }); + } + }); + + it('does not treat the next numbered table row as a URL continuation', () => { + const rows = [TABLE_WITH_TRAILING_COLUMNS[1], { text: ' 13 New to trending Another document' }]; + const buffer = fakeBuffer(rows, TABLE_COLS); + expect(terminalUrlContinuationExtent(buffer, 0, TABLE_COLS, 12)).toBeNull(); + expect(terminalUrlContinuationExtent(buffer, 1, TABLE_COLS, 12)).toBeNull(); + }); + + it('leaves the neighbouring table rows alone', () => { + const buffer = fakeBuffer(TABLE, COLS); + expect(terminalUrlContinuationExtent(buffer, 0, COLS, 12)).toBeNull(); + expect(terminalUrlContinuationExtent(buffer, 4, COLS, 12)).toBeNull(); + }); + + it('does not glue a following prose line onto a URL that ends a line', () => { + const buffer = fakeBuffer( + [{ text: 'Download from https://example.com/files/' }, { text: 'then run the installer' }], + 80 + ); + expect(terminalUrlContinuationExtent(buffer, 0, 80, 12)).toBeNull(); + expect(terminalUrlContinuationExtent(buffer, 1, 80, 12)).toBeNull(); + }); + + it('does not join a second URL on the next row', () => { + const buffer = fakeBuffer([{ text: '- https://example.com/one/' }, { text: ' https://example.com/two' }], 80); + expect(terminalUrlContinuationExtent(buffer, 0, 80, 12)).toBeNull(); + }); + + it('does not join a row indented to the right of where the URL started', () => { + const buffer = fakeBuffer([{ text: 'x https://example.com/a/b?' }, { text: ' q=1&r=2' }], 80); + expect(terminalUrlContinuationExtent(buffer, 0, 80, 12)).toBeNull(); + }); + + it('does not join when the URL line ends on a character a wrapper would not break after', () => { + const buffer = fakeBuffer([{ text: 'see https://example.com/page' }, { text: 'v2-notes.txt follows' }], 80); + expect(terminalUrlContinuationExtent(buffer, 0, 80, 12)).toBeNull(); + }); + + it('stays within the row bound', () => { + const rows: Row[] = [{ text: 'u https://example.com/' }]; + for (let i = 0; i < 20; i++) rows.push({ text: ` seg${i}-` }); + const extent = terminalUrlContinuationExtent(fakeBuffer(rows, 80), 0, 80, 5); + expect(extent).not.toBeNull(); + expect(extent!.endRow - extent!.startRow + 1).toBeLessThanOrEqual(5); + }); +}); + +describe('native OSC 8 terminal URLs', () => { + it('opens complete HTTP links through the terminal opener and rejects unsafe schemes', () => { + const open = vi.fn(); + const context = vm.createContext({ + console, + CodemanApp: class CodemanApp {}, + document: { addEventListener: vi.fn() }, + window: { addEventListener: vi.fn(), removeEventListener: vi.fn() }, + }); + vm.runInContext(`${CONSTANTS}\n${TERMINAL_UI}\nglobalThis.__proto = CodemanApp.prototype;`, context); + const proto = ( + context as unknown as { + __proto: { _terminalOsc8LinkHandler: () => { activate: (event: unknown, url: string) => void } }; + } + ).__proto; + const handler = proto._terminalOsc8LinkHandler.call({ _openTerminalUrl: open }); + const url = 'https://docs.example.com/Doc.aspx?file=Q3%20Plan&action=default&mobileredirect=true'; + handler.activate(null, url); + expect(open).toHaveBeenCalledExactlyOnceWith(url); + for (const unsafe of ['javascript:alert(1)', 'data:text/html,hello', 'file:///etc/passwd']) { + handler.activate(null, unsafe); + } + expect(open).toHaveBeenCalledTimes(1); + }); +}); + +describe('terminal link provider with wrapped table URLs', () => { + function harness(rows: Row[], cols: number) { + const openSpy = vi.fn(); + const context = vm.createContext({ + console: { ...console, debug: vi.fn(), log: vi.fn() }, + CodemanApp: class CodemanApp {}, + setInterval: vi.fn(), + clearInterval: vi.fn(), + setTimeout, + clearTimeout, + requestAnimationFrame: vi.fn(), + document: { addEventListener: vi.fn(), getElementById: vi.fn(), documentElement: { dataset: {} } }, + window: { + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + open: openSpy, + document: { documentElement: { dataset: {} } }, + }, + }); + vm.runInContext(`${CONSTANTS}\n${TERMINAL_UI}\nglobalThis.__proto = CodemanApp.prototype;`, context); + const proto = (context as unknown as { __proto: Record unknown> }).__proto; + let provider: { provideLinks: (line: number, cb: (links?: Link[]) => void) => void } | undefined; + const app = { + terminal: { + cols, + rows: 24, + registerLinkProvider(value: typeof provider) { + provider = value; + }, + buffer: { active: { viewportY: 0, ...fakeBuffer(rows, cols) } }, + }, + activeSessionId: 's1', + sessions: new Map([['s1', { workingDir: '/workspace/project' }]]), + openFilePreview: vi.fn(), + openLogViewerWindow: vi.fn(), + _openTerminalUrl: proto._openTerminalUrl, + _resolveLinkedFilePath: proto._resolveLinkedFilePath, + _isExternalPreviewPath: () => false, + registerFilePathLinkProvider: proto.registerFilePathLinkProvider, + }; + app.registerFilePathLinkProvider.call(app); + const provide = (line: number): Link[] => { + let links: Link[] | undefined; + provider!.provideLinks(line, (value) => { + links = value; + }); + return links ?? []; + }; + return { provide, openSpy }; + } + + it('opens the whole URL from any of its rows', () => { + const { provide, openSpy } = harness(TABLE, COLS); + for (const line of [2, 3, 4]) { + const urlLinks = provide(line).filter((link) => link.text.startsWith('https://')); + expect(urlLinks.length).toBeGreaterThan(0); + for (const link of urlLinks) expect(link.text).toBe(URL); + const onRow = urlLinks.find((link) => link.range.start.y === line); + expect(onRow).toBeDefined(); + onRow!.activate(null, onRow!.text); + expect(openSpy).toHaveBeenLastCalledWith(URL, '_blank', 'noopener,noreferrer'); + } + }); + + it('opens the whole table URL without including the adjacent date and owner columns', () => { + const { provide, openSpy } = harness(TABLE_WITH_TRAILING_COLUMNS, TABLE_COLS); + for (const line of [2, 3, 4]) { + const links = provide(line).filter((link) => link.text.startsWith('https://')); + expect(links.length).toBeGreaterThan(0); + for (const link of links) expect(link.text).toBe(URL); + links[0].activate(null, links[0].text); + expect(openSpy).toHaveBeenLastCalledWith(URL, '_blank', 'noopener,noreferrer'); + } + const firstRowLink = provide(2).find((link) => link.text === URL && link.range.start.y === 2); + expect(firstRowLink?.range.end.x).toBe(TABLE[1].text.length); + }); + + it('opens every segment after a full-width table divider', () => { + const rows = [{ text: '─'.repeat(TABLE_COLS) }, ...TABLE_WITH_TRAILING_COLUMNS.slice(1, 4)]; + const { provide, openSpy } = harness(rows, TABLE_COLS); + for (const line of [2, 3, 4]) { + const link = provide(line).find((item) => item.text === URL && item.range.start.y === line); + expect(link).toBeDefined(); + link!.activate(null, link!.text); + expect(openSpy).toHaveBeenLastCalledWith(URL, '_blank', 'noopener,noreferrer'); + } + }); + + it('underlines only the URL cells on each row, never the indent between pieces', () => { + const { provide } = harness(TABLE, COLS); + const segments = provide(3).filter((link) => link.text === URL); + expect(segments.map((link) => link.range.start.y)).toEqual([2, 3, 4]); + for (const link of segments) expect(link.range.start.y).toBe(link.range.end.y); + expect(segments[0].range.start.x).toBe(TABLE[1].text.indexOf('https://') + 1); + expect(segments[0].range.end.x).toBe(TABLE[1].text.length); + expect(segments[1].range.start.x).toBe(CELL + 1); + expect(segments[2].range.start.x).toBe(CELL + 1); + // Ends on `true`, not on the closing paren after it. + expect(segments[2].range.end.x).toBe(TABLE[3].text.length - 1); + }); + + it('keeps a single-row URL with a paren pair whole', () => { + const line = 'Open https://example.com/wiki/Widget_(tool) please'; + const { provide } = harness([{ text: line }], 80); + const [link] = provide(1).filter((l) => l.text.startsWith('https://')); + expect(link.text).toBe('https://example.com/wiki/Widget_(tool)'); + }); +}); diff --git a/test/webview-loopback-links.test.ts b/test/webview-loopback-links.test.ts index c19d3086..3d053971 100644 --- a/test/webview-loopback-links.test.ts +++ b/test/webview-loopback-links.test.ts @@ -299,7 +299,10 @@ describe('openLinkThroughWebTabIfLoopback', () => { describe('callers consult the hook first', () => { it('terminal URL links try the web tab before window.open', () => { - const activate = TERMINAL_UI.indexOf('activate(_event, text) {'); + // URL links (and OSC 8 links) open through _openTerminalUrl(), so the desktop + // click and the mobile touch-tap path both consult the hook. Read that function. + expect(TERMINAL_UI).toContain('self._openTerminalUrl(url);'); + const activate = TERMINAL_UI.indexOf('_openTerminalUrl(text) {'); expect(activate).toBeGreaterThan(-1); const body = TERMINAL_UI.slice(activate, TERMINAL_UI.indexOf('},', activate)); expect(body.indexOf('openLinkThroughWebTabIfLoopback?.(text)')).toBeGreaterThan(-1); From 32a1d544ab9c127d673b68252cdae485ce8e9467 Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Thu, 22 Oct 2026 17:22:46 -0400 Subject: [PATCH 002/191] feat(terminal): link file paths that contain spaces as one path Every path pattern ended a path at whitespace, so "/mnt/c/OneDrive - Contoso/a.docx" linked "/mnt/c/OneDrive" plus a relative "Contoso/a.docx", two links to the wrong files, in the terminal and the response viewer alike. findFilePathLinks() now takes a spaced path whole where the line bounds it: the whole line is the path, or it follows a list marker and/or a short "Label:". A relative spaced path needs the marker or label. The candidate must read as one path (no earlier word ends on an extension, starts a new "/" path or is an arrow). A spaced line that is one path but not linkable (unknown root, "." or ".." segment) links nothing instead of a fragment. --- src/web/public/constants.js | 74 ++++++++++++- test/spaced-file-path-links.test.ts | 165 ++++++++++++++++++++++++++++ 2 files changed, 236 insertions(+), 3 deletions(-) create mode 100644 test/spaced-file-path-links.test.ts diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 4cb2f099..3308dfd2 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2178,27 +2178,93 @@ function findFilePathLinks(text) { while ((match = urlPattern.exec(value)) !== null) urls.push({ index: match.index, path: match[0] }); const inUrl = (index, length) => urls.some((u) => index < u.index + u.path.length && u.index < index + length); + // A path with spaces is taken whole, or the span it covers links nothing + // (spacedFilePathOnLine). Either way no detector below may cut a fragment out of it. + const reserved = []; + let lineStart = 0; + for (const line of value.split('\n')) { + const spaced = spacedFilePathOnLine(line); + if (spaced && !inUrl(lineStart + spaced.index, spaced.path.length)) { + const index = lineStart + spaced.index; + reserved.push({ index, path: spaced.path }); + if (spaced.link) found.push({ path: spaced.path, index }); + } + lineStart += line.length + 1; + } + const isReserved = (index, length) => + reserved.some((r) => index < r.index + r.path.length && r.index < index + length); + const absolute = absoluteFilePathPattern(); while ((match = absolute.exec(value)) !== null) { - if (!inUrl(match.index, match[1].length)) found.push({ path: match[1], index: match.index }); + if (!inUrl(match.index, match[1].length) && !isReserved(match.index, match[1].length)) { + found.push({ path: match[1], index: match.index }); + } } const relative = new RegExp(RELATIVE_FILE_PATH_LINK_PATTERN.source, 'g'); while ((match = relative.exec(value)) !== null) { const path = match[2]; const index = match.index + match[1].length; - if (overlaps(index, path.length) || inUrl(index, path.length)) continue; + if (overlaps(index, path.length) || inUrl(index, path.length) || isReserved(index, path.length)) continue; found.push({ path, index }); } const folders = new RegExp(FOLDER_PATH_LINK_PATTERN.source, 'g'); while ((match = folders.exec(value)) !== null) { const path = folderLinkPath(match[2]); const index = match.index + match[1].length; - if (!path || overlaps(index, path.length) || inUrl(index, path.length)) continue; + if (!path || overlaps(index, path.length) || inUrl(index, path.length) || isReserved(index, path.length)) continue; found.push({ path, index, folder: true }); } return found.sort((a, b) => a.index - b.index); } +/** + * A file path WITH SPACES on one line, or null. + * + * Every path pattern ends a path at whitespace, so `/mnt/c/OneDrive - Contoso/a.docx` + * linked `/mnt/c/OneDrive` plus a relative `Contoso/a.docx`, two links to the + * wrong targets. A space is only safe inside a path where the line itself bounds + * the path, so a spaced path is recognised in exactly these shapes: + * + * /mnt/c/My Docs/report.docx the whole line is the path + * - /mnt/c/My Docs/report.docx after a list marker (`-`, `*`, `•`, `1.`, `2)`) + * Saved to: 03 Attachments/plan.pptx after a short label (one to four words + `:`) + * + * An absolute path may stand alone; a relative or `~/` one needs a marker or a + * label, since a bare spaced relative candidate is ordinary prose. One trailing + * sentence mark is left out. The candidate must read as ONE path: it ends on a + * known extension, and no earlier word ends on one, starts a new `/` path, ends + * in `:` or is an arrow, so two paths on a line are never merged. + * + * A candidate that is one path but not a linkable one (an unknown root such as + * `/etc`, a `.`/`..`/empty segment) comes back with `link: false`: its span links + * nothing, rather than the fragment after its last space. + * + * Linear: one bounded prefix match, then string scans. + * + * @param {string} line One line, no newline. + * @returns {{path: string, index: number, link: boolean} | null} + */ +function spacedFilePathOnLine(line) { + const text = String(line || ''); + const prefix = /^[ \t]*((?:\d{1,3}[.)]|[-*•])[ \t]+)?((?:[A-Za-z][\w-]{0,30}(?: [\w-]{1,30}){0,3}):[ \t]+)?/.exec(text); + const index = prefix[0].length; + const path = text.slice(index).replace(/\s+$/, '').replace(/[.,;:!?]$/, ''); + if (!path.includes(' ') || !path.includes('/') || path.length > 1000) return null; + if (!path.startsWith('/') && !prefix[1] && !prefix[2]) return null; + if (!PATH_TOKEN_ENDS_WITH_EXTENSION.test(path)) return null; + if (/[\t"'`<>|;&\x00-\x1f\\]/.test(path) || path.includes('://')) return null; + const words = path.split(' '); + for (const word of words.slice(0, -1)) { + if (word.endsWith(':') || /^(?:—|–|→|=>|->|<-)$/.test(word)) return null; + if (PATH_TOKEN_ENDS_WITH_EXTENSION.test(word.replace(/[.,;:!?)\]}]+$/, ''))) return null; + } + if (words.slice(1).some((word) => word.startsWith('/'))) return null; + const rooted = SPACED_PATH_ROOT.test(path) || path.startsWith('~/'); + const segments = path.replace(/^~?\//, '').split('/'); + const link = (rooted || !path.startsWith('/')) && segments.every((seg) => seg && seg !== '.' && seg !== '..'); + return { path, index, link }; +} + /** * Folder paths, for the folder grid: rooted like {@link FILE_PATH_LINK_PATTERN} * (`/mnt/raid1/neon_getaway/styleframes`), `~/renders`, or relative with a @@ -2385,6 +2451,8 @@ const FILE_LINK_ROOT_GROUP = /^\(\\\/\(\?:([^()]*)\)/.exec(FILE_PATH_LINK_PATTER const PATH_TOKEN_ENDS_WITH_EXTENSION = new RegExp(`\\.${FILE_LINK_EXTENSION_GROUP}$`); const PATH_TOKEN_HAS_EXTENSION = new RegExp(`\\.${FILE_LINK_EXTENSION_GROUP}\\b`); const PATH_TOKEN_STARTS_NEW_PATH = new RegExp(`^(?:~\\/|\\/(?:${FILE_LINK_ROOT_GROUP})\\/)`); +/** A rooted path's start (`/home/`, `/mnt/`, …), for {@link spacedFilePathOnLine}. */ +const SPACED_PATH_ROOT = new RegExp(`^\\/(?:${FILE_LINK_ROOT_GROUP})\\/`); /** A relative head that names its own location (`./a`, `../a`) is an entry of its own. */ const PATH_TOKEN_STARTS_EXPLICIT_RELATIVE = /^\.{1,2}\//; /** First directory of a path, a leading `/` or `./` ignored: `src` for `src/a.ts` and `/src/a.ts`. */ diff --git a/test/spaced-file-path-links.test.ts b/test/spaced-file-path-links.test.ts new file mode 100644 index 00000000..b799a888 --- /dev/null +++ b/test/spaced-file-path-links.test.ts @@ -0,0 +1,165 @@ +// Port: none (a pure function in a vm context — no browser, no server). +// +// File paths with spaces in them (`OneDrive - Contoso/…`, `90 Sensitive/…`) were +// cut at the first space by every path pattern, so the terminal and the response +// viewer linked a fragment such as `/mnt/c/proj/My` plus a bogus relative tail +// `Docs/report.docx`, each opening the wrong target. findFilePathLinks() now takes +// a spaced path WHOLE where the line makes its extent unambiguous: the path is the +// whole line, or follows a list marker and/or a short `Label:`. A spaced line that +// is one path but not a linkable one (unknown root, `..`) links nothing at all, +// rather than a fragment. +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { describe, expect, it } from 'vitest'; + +const SOURCE = readFileSync(resolve(import.meta.dirname, '../src/web/public/constants.js'), 'utf8'); + +function load(): (text: string) => Array<{ path: string; index: number; folder?: true }> { + const context = vm.createContext({ console, window: undefined }); + vm.runInContext(`${SOURCE}\nglobalThis.__fn = findFilePathLinks;`, context, { filename: 'constants.js' }); + return (context as { __fn: never })['__fn'] as never; +} + +const findFilePathLinks = load(); +const paths = (text: string) => findFilePathLinks(text).map((f) => f.path); + +const LONG = + '/mnt/c/Users/someone/OneDrive - Contoso/01_Projects/Other/kb/out/deliverables/fy27-messaging-market-story.docx'; + +describe('spaced file paths', () => { + it('links an own-line absolute path with spaces whole', () => { + expect(paths(LONG)).toEqual([LONG]); + expect(paths('/mnt/c/proj/My Big Docs/report.docx')).toEqual(['/mnt/c/proj/My Big Docs/report.docx']); + }); + + it('reports where the whole path starts', () => { + const line = ' - Path: /mnt/c/proj/My Big Docs/report.docx'; + expect(findFilePathLinks(line)).toEqual([ + { path: '/mnt/c/proj/My Big Docs/report.docx', index: line.indexOf('/mnt') }, + ]); + }); + + it('leaves a single trailing sentence period out', () => { + expect(paths('/mnt/c/proj/My Big Docs/report.docx.')).toEqual(['/mnt/c/proj/My Big Docs/report.docx']); + }); + + it.each(['Path:', 'File:', 'Saved to:'])('links an absolute spaced path after %s', (label) => { + const p = '/mnt/sync/brain/90 Sensitive/team/training/attendees.xlsx'; + expect(paths(`${label} ${p}`)).toEqual([p]); + }); + + it.each(['Path:', 'File:', 'Saved to:'])('links a relative spaced path after %s', (label) => { + const p = '03 Attachments/Build SIPOC Design/Build-SIPOC.pptx'; + expect(paths(`${label} ${p}`)).toEqual([p]); + }); + + it.each(['- ', '* ', '• ', '1. ', '2) '])('links a spaced path after the list marker %j', (marker) => { + const rel = '03 Attachments/Build SIPOC Design/Build-SIPOC.pptx'; + expect(paths(`${marker}${rel}`)).toEqual([rel]); + expect(paths(` ${marker}${LONG}`)).toEqual([LONG]); + }); + + it('works line by line inside multi-line text (the response viewer hands over text nodes)', () => { + const text = `Wrote two files:\n- ${LONG}\n- notes/a b/c.md\n`; + expect(paths(text)).toEqual([LONG, 'notes/a b/c.md']); + }); + + it('links nothing for a spaced path outside the known roots (no partial tail)', () => { + expect(paths('/etc/secret files/passwords.txt')).toEqual([]); + expect(paths('/srvx/team share/notes/passwords.txt')).toEqual([]); + }); + + it('links nothing for a spaced path with a `..` or `.` segment', () => { + expect(paths('/mnt/c/proj/../other place/docs/report.docx')).toEqual([]); + expect(paths('File: 03 Attachments/../private/report.xlsx')).toEqual([]); + expect(paths('- 03 Attachments/./private/report.xlsx')).toEqual([]); + }); + + it('never merges two space-separated paths into one link', () => { + expect(paths('/mnt/c/proj/a.md /mnt/c/proj/b.pdf')).toEqual(['/mnt/c/proj/a.md', '/mnt/c/proj/b.pdf']); + expect(paths('- docs/a.md docs/b.md')).toEqual(['docs/a.md', 'docs/b.md']); + expect(paths('/mnt/c/proj/a b.md -> /mnt/c/proj/c.md')).not.toContain('/mnt/c/proj/a b.md -> /mnt/c/proj/c.md'); + }); + + it('does not take prose into a path', () => { + // No marker or label: a relative spaced candidate is prose, not a path. + expect(paths('See docs/report.docx for details')).toEqual(['docs/report.docx']); + expect(paths('see the docs/a b.md')).not.toContain('see the docs/a b.md'); + // A long description before a colon is a sentence, not a label: only the path links. + const rel = '.worktrees/feature-x/docs/specs/2026-09-05-design.md'; + expect(paths(`• Design committed in the isolated worktree: ${rel}`)).toEqual([rel]); + }); + + it('leaves unspaced paths to the ordinary detectors', () => { + expect(paths('- /mnt/c/proj/docs/report.docx')).toEqual(['/mnt/c/proj/docs/report.docx']); + expect(paths('• committed as /tmp/x/docs/specs/design.md:1 (e7ae1b4).')).toEqual(['/tmp/x/docs/specs/design.md']); + }); + + it('stays linear on a long spaced line', () => { + const line = '- ' + 'a '.repeat(20000) + '/x.md'; + const t0 = performance.now(); + findFilePathLinks(line); + expect(performance.now() - t0).toBeLessThan(500); + }); +}); + +describe('terminal link provider with spaced paths', () => { + type Link = { text: string; activate: (event: unknown, text: string) => void }; + const TERMINAL_UI = readFileSync(resolve(import.meta.dirname, '../src/web/public/terminal-ui.js'), 'utf8'); + + function linksFor(line: string) { + const noop = () => {}; + const context = vm.createContext({ + console: { ...console, debug: noop, log: noop }, + CodemanApp: class CodemanApp {}, + document: { addEventListener: noop, getElementById: noop, documentElement: { dataset: {} } }, + window: { addEventListener: noop, removeEventListener: noop, document: { documentElement: { dataset: {} } } }, + }); + vm.runInContext(`${SOURCE}\n${TERMINAL_UI}\nglobalThis.__proto = CodemanApp.prototype;`, context); + const proto = (context as unknown as { __proto: Record unknown> }).__proto; + let provider: { provideLinks: (row: number, cb: (links?: Link[]) => void) => void } | undefined; + const opened: string[] = []; + const app = { + terminal: { + cols: 200, + registerLinkProvider: (p: typeof provider) => (provider = p), + buffer: { + active: { + length: 1, + getLine: (r: number) => + r === 0 + ? { + isWrapped: false, + translateToString: (trim?: boolean) => (trim === false ? line.padEnd(200) : line), + } + : undefined, + }, + }, + }, + activeSessionId: 's1', + sessions: new Map([['s1', { workingDir: '/workspace/project' }]]), + openFilePreview: (p: string) => opened.push(p), + openLogViewerWindow: (p: string) => opened.push(`log:${p}`), + _isExternalPreviewPath: () => false, + _resolveLinkedFilePath: proto._resolveLinkedFilePath, + registerFilePathLinkProvider: proto.registerFilePathLinkProvider, + }; + app.registerFilePathLinkProvider.call(app); + let links: Link[] = []; + provider!.provideLinks(1, (value) => (links = value ?? [])); + return { links, opened }; + } + + it('links the whole spaced path and opens it, resolved against the workspace', () => { + const rel = '03 Attachments/Build SIPOC Design/Build-SIPOC.pptx'; + const { links, opened } = linksFor(`Saved to: ${rel}`); + expect(links.map((l) => l.text)).toEqual([rel]); + links[0].activate(null, links[0].text); + expect(opened).toEqual([`/workspace/project/${rel}`]); + }); + + it('links nothing for an unlinkable spaced path', () => { + expect(linksFor('/etc/secret files/passwords.txt').links).toEqual([]); + }); +}); From efa647d2578d59d8a4dc721ced1ed9b683b705c5 Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Fri, 23 Oct 2026 07:26:15 -0400 Subject: [PATCH 003/191] fix(terminal): exclude relative prose and trim spaced paths linearly --- CLAUDE.md | 4 +++- docs/architecture-invariants.md | 4 +++- src/web/public/constants.js | 15 ++++++++++----- test/spaced-file-path-links.test.ts | 26 ++++++++++++++++++++++++++ 4 files changed, 42 insertions(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 02704d0f..d341d772 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -222,7 +222,9 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Prompt uploads** (`POST /api/sessions/:id/paste-image`: clipboard paste, drop, the Compose Attach key and the phone toolbar's Attach button; client wiring in `image-input.js` and `keyboard-accessory.js`): a pick is written to `/.codeman-uploads/` and its path lands on the prompt as unsent text. The folder is IN the workspace because that is the only path identical for a local and a container agent, FLAT because `/.codeman/` would BE the data dir when the workspace is the home directory, and self-ignoring through a `.gitignore` of `*` written ONCE with `wx` (a file already there is the user's and is never overwritten; the user's own ignore file is never touched). ⚠️ The directory names live ONLY in `paste-image-gc.ts` (`UPLOAD_DIR_NAMES`): the pre-move `.claude-images/` receives nothing but stays readable for one release, so the GC sweep and the delete cleanup (through `uploadDirs()`) and the image watcher's ignore filter (reading the names) cover BOTH; retire the legacy entry there, never by hand elsewhere. ⚠️ `uploadDirs()` is async and probes the workspace BOUNDED first (`probePathKind()`, the #516 rule: an `unknown` path is skipped, never touched; the sweep keeps the stall cap and the delete passes `pastCap`), then returns only REAL directories (lstat, so a planted symlink is never one), none that is or contains `getDataDir()` (one strictly inside it only holds uploads and is listed), and none for a remote session, since both consumers delete; the check is made at list time, and the same-user race after it is accepted. Images are buffered in memory under `MAX_PASTE_IMAGE_BYTES` (50MB) after a client-side canvas re-encode; a DOCUMENT (pdf, docx, pptx, xlsx, zip and the text family txt/md/csv/tsv/json/log/yaml/yml/xml; the curated `ALLOWED_DOCUMENT_EXTS`, never `SUPPORTED_ATTACHMENT_EXTENSIONS` or `EDITABLE_EXTENSIONS`, which carry sh/py/js/html) is admitted by its name's extension alone, buffered like an image, judged on its bytes by the same fail-closed `imageMagicMatchesExt` (`%PDF-`, `PK\x03\x04` for the OOXML trio and zip, UTF-8 without a NUL byte for text: a SIGNATURE or ENCODING check, never a structure or content check, so generic zip bytes pass as `.docx` and a script renamed `.txt` is text; the sh/py/js/html exclusion is a SCOPE choice by extension, keeping the server from storing what it would serve or run, not a content filter: the agent can already write any of those) and saved as `paste---` with a sanitized stem (`[A-Za-z0-9._-]`, 40 chars, a wholly non-ASCII name leaves no stem; the `paste-` prefix is what the GC matches). A VIDEO is never buffered: `?kind=video` streams it to disk under `MAX_PASTE_VIDEO_BYTES` (1GB, env `CODEMAN_MAX_PASTE_VIDEO_BYTES`), judges the container on the first 12 bytes as they arrive (ISO BMFF / QuickTime atom, EBML for webm, Ogg for ogv; a mislabeled body is drained unwritten from there, never parked on disk first) and unlinks on any failure, a dead transfer included. ⚠️ The flag is the ONLY way onto that path and admits video types alone (the registry's `VIDEO_ATTACHMENT_EXTENSIONS`, single-sourced: mp4/webm/mov/m4v/ogv, so an uploaded video is one the file viewer can also play; 3gp/mkv are out, since a phone's camera roll produces neither and neither previews), so an image can never ride the larger cap through memory; without it the route is byte-identical to the image-only version. Busboy takes its size limit before the part is seen, which is why the choice rides the URL and not the file. ⚠️ A video draws on its OWN per-(IP, session) rate bucket (`consumeVideoToken`, 3/min) beside the image bucket (`consumePasteToken`, 30/min): at 1GB each, 30/min would be 30GB of disk a minute, so the lower rate bounds it to 3GB; the image path keeps the 30/min bucket and is unchanged without the flag. A remote (SSH) session is refused (400) before any disk touch: its `workingDir` is the remote path, so the file would land on THIS host where the agent cannot read it (Docker is fine, the workspace is bind-mounted at the same absolute path). ⚠️ A live session's workspace can still sit on a network mount that stopped answering, so the route probes it BOUNDED (`probePathKind(session.workingDir, { pastCap: true })`, the #516 rule) before the first disk touch and refuses an `unknown` mount through `describeUnknownPath()` (OPERATION_FAILED), the way session create does, so a mount already unreachable cannot freeze a libuv worker on every upload (a mount that dies after the probe is the same residual race create accepts). ⚠️ Dispatch mirrors the client: an `image/*` MIME takes the IMAGE path whatever the name says (a GIF named `animation.txt` stays a GIF), a document extension takes the document branch, and anything else takes the image path exactly as before, where the HEIC sniff still recovers a HEIC under a generic name; a foreign extension with no image MIME gets its `Unsupported file type` 400 only AFTER that sniff fails, never before, so nothing the image path ever accepted by its bytes is refused. A text mismatch is refused as not UTF-8 text (UTF-16 when the file opens with a UTF-16 BOM, as Excel's "Unicode Text" and Notepad's "Unicode" do), telling the user to save it as UTF-8, and its log line names the failure (`utf16-bom`/`nul-byte`/`not-utf8`) and never prints the bytes, since the head of a text file is user content. ⚠️ The client classifies a pick through ONE helper, `_promptAttachKind()` (MIME type first, then the name's extension, because some Android file managers hand over a File with none; returns `image`, `video` or `document`); every pick and paste filter (Attach, drop, the tile grid's drop in `tile-grid.js`, the Compose and paste-dialog pickers, the clipboard handlers), the flag and the re-encode gate read it, never `file.type` directly. Those surfaces ADMIT through `_admitPromptAttachments()` (image-input.js), which calls it and toasts what a pick skipped (how many files of a mixed pick, `Unsupported file type` when none pass, nothing for a text paste); a new surface admits through that helper too, never by filtering on `_promptAttachKind()` itself, or it loses the toasts. ⚠️ A pick that lands while the tile grid owns the terminal goes to the PTY, never the main terminal's overlay: that terminal is parked and hidden then, and a tile's Enter cannot flush it (`_tilesOwnTerminal()` in the `_attachFromToolbar` guard). Tests: `test/toolbar-attach.test.ts`, `test/mobile-prompt-composer.test.ts`, `test/paste-image-gc.test.ts`, `test/paste-image-dir-shared.test.ts`, `test/upload-document-list-parity.test.ts` (the browser's copies of `ALLOWED_DOCUMENT_EXTS`: `_promptAttachKind()`, the three pickers' `accept=` and the two file-path link patterns), the paste-image block of `test/routes/session-routes.test.ts`. → [architecture-invariants#buffers-uploads-and-terminal-history](docs/architecture-invariants.md#buffers-uploads-and-terminal-history) -**File-path links (terminal + chat)**: a path an agent prints is clickable on BOTH surfaces and opens the file-preview overlay. ⚠️ ONE pattern (`FILE_PATH_LINK_PATTERN` / `absoluteFilePathPattern()` in constants.js) feeds the xterm link provider AND `_linkifyFilePaths()`, a fresh instance per call (`lastIndex`). Both call `findFilePathLinks()`, which adds `~/`, relative and folder paths without overlapping a rooted match or a URL; a relative path resolves against `workingDir` (`resolveLinkedFilePath`), `~/` only server-side on the session's host (attachment route), and `terminalPathLine()` joins a path a program hard-wrapped short of the edge. ⚠️ A folder link opens the **folder grid**: `POST /api/sessions/:id/folders` guards + GRANTS the folder (`folder-listing.ts`, own registry, never the 200-capped attachment registry), files come by bare name via `…/folders/:folderId/files/:name/*` through `resolveServableAttachmentPath` plus the same extension/regular-file gate on the symlink's resolved target, so no browser request carries a path; `~/` is refused in a Docker case (the container's home is not this host's). The chat linkifier walks TEXT NODES with DOM APIs, never rebuilds sanitized markup as a string. ⚠️ An out-of-workspace path goes through the ATTACHMENT routes (`POST /api/sessions/:id/attachments` with `notify: false`), never by widening `file-content`/`file-raw` or `file-stream-manager`'s `tail -f` allowlist. ⚠️ `TEXT_ATTACHMENT_EXTENSIONS` IS `EDITABLE_EXTENSIONS` (never a second list), and widening READ must never widen RUN: `html`/`htm`/`svg` stay download-only, other text is inert `text/plain`+`nosniff`. ⚠️ The ONE place HTML renders is `/html-view//...` (`html-view-routes.ts`, `html-view-capabilities.ts`): `POST /api/sessions/:id/html-view` mints a capability for the file's own directory, every response carries `Content-Security-Policy: sandbox` WITHOUT `allow-same-origin` (opaque origin even as a top-level tab, so the page can never read Codeman or drive its API) and with NO fetch directive (open network, owner decision 2026-10-13: never restrict it or add `allow-same-origin` without the owner), and only non-dot, asset-allowlisted files under that directory are served (GET/HEAD only; capabilities revoked with the web-tab ones on logout). That tree is served recursively, so minting is refused for a hidden directory or a broad root (`isBroadHtmlViewRoot`); the preview renders by default and the **Page** pill (or a refused mint) falls through to the source view and its Edit button. Never serve HTML same-origin anywhere else. Tests: `test/routes/html-view-routes.test.ts`. Media extensions are single-sourced in `attachment-registry.ts`. ⚠️ **XLSX previews parse in the BROWSER**, never on the server; the admission limits that keep a hostile workbook from freezing the worker live in `src/web/CLAUDE.md` (Spreadsheet preview). → [architecture-invariants#file-path-links-terminal--response-viewer](docs/architecture-invariants.md#file-path-links-terminal--response-viewer) +**File-path links (terminal + chat)**: a path an agent prints is clickable on BOTH surfaces and opens the file-preview overlay. ⚠️ ONE pattern (`FILE_PATH_LINK_PATTERN` / `absoluteFilePathPattern()` in constants.js) feeds the xterm link provider AND `_linkifyFilePaths()`, a fresh instance per call (`lastIndex`). Both call `findFilePathLinks()`, which reserves spaced-path spans first (even unlinkable ones), then adds rooted, `~/`, relative and folder paths without overlapping a reservation, earlier match or URL; a relative path resolves against `workingDir` (`resolveLinkedFilePath`), `~/` only server-side on the session's host (attachment route), and `terminalPathLine()` joins a path a program hard-wrapped short of the edge. ⚠️ A folder link opens the **folder grid**: `POST /api/sessions/:id/folders` guards + GRANTS the folder (`folder-listing.ts`, own registry, never the 200-capped attachment registry), files come by bare name via `…/folders/:folderId/files/:name/*` through `resolveServableAttachmentPath` plus the same extension/regular-file gate on the symlink's resolved target, so no browser request carries a path; `~/` is refused in a Docker case (the container's home is not this host's). The chat linkifier walks TEXT NODES with DOM APIs, never rebuilds sanitized markup as a string. ⚠️ An out-of-workspace path goes through the ATTACHMENT routes (`POST /api/sessions/:id/attachments` with `notify: false`), never by widening `file-content`/`file-raw` or `file-stream-manager`'s `tail -f` allowlist. ⚠️ `TEXT_ATTACHMENT_EXTENSIONS` IS `EDITABLE_EXTENSIONS` (never a second list), and widening READ must never widen RUN: `html`/`htm`/`svg` stay download-only, other text is inert `text/plain`+`nosniff`. ⚠️ The ONE place HTML renders is `/html-view//...` (`html-view-routes.ts`, `html-view-capabilities.ts`): `POST /api/sessions/:id/html-view` mints a capability for the file's own directory, every response carries `Content-Security-Policy: sandbox` WITHOUT `allow-same-origin` (opaque origin even as a top-level tab, so the page can never read Codeman or drive its API) and with NO fetch directive (open network, owner decision 2026-10-13: never restrict it or add `allow-same-origin` without the owner), and only non-dot, asset-allowlisted files under that directory are served (GET/HEAD only; capabilities revoked with the web-tab ones on logout). That tree is served recursively, so minting is refused for a hidden directory or a broad root (`isBroadHtmlViewRoot`); the preview renders by default and the **Page** pill (or a refused mint) falls through to the source view and its Edit button. Never serve HTML same-origin anywhere else. Tests: `test/routes/html-view-routes.test.ts`. Media extensions are single-sourced in `attachment-registry.ts`. ⚠️ **XLSX previews parse in the BROWSER**, never on the server; the admission limits that keep a hostile workbook from freezing the worker live in `src/web/CLAUDE.md` (Spreadsheet preview). → [architecture-invariants#file-path-links-terminal--response-viewer](docs/architecture-invariants.md#file-path-links-terminal--response-viewer) + +Spaced paths are considered first by `findFilePathLinks()`: accepted candidates reserve their entire span even when unlinkable, so later rooted, relative and folder detectors cannot link fragments. Relative candidates whose first slash-containing word is their last word defer to ordinary detection to exclude list/label prose. This deliberately gives up spaces only in the top directory (`Saved to: 03 Attachments/plan.pptx`); prose shaped like deeper spaced paths (`- Fixed bug in src/My Docs/a.md`, `/home/me/proj contains a.md`) remains ambiguous. Trailing whitespace uses linear `trimEnd()` before the length cap. **Filesystem path picker** (Link Existing "Browse" + the mobile keyboard's `📁 Path` key): lazy one-directory browsing via `GET /api/filesystem/browse`, with `GET /api/filesystem/preview` for the tapped file. Inserts the path **without** Enter, so the prompt is never submitted; the sibling `⌫ All` key clears only the unsent prompt and must never send the agent's `/clear`. ⚠️ This is a **second file-serving surface and inherits neither the attachment confinement nor its ownership scoping** — it allowlists Home, `CASES_DIR`, `/mnt/d` and `CODEMAN_FILE_PICKER_ROOTS`, blocks sensitive trees, and rejects symlink escapes **after** `realpath`. ⚠️ The optional `sessionId` is an ownership boundary that must be `canAccessOwned`-checked by hand (it does not go through `findSessionOrFail`), and in multi-user mode a non-admin gets only their own `userSpacePath` as a root: per-user spaces live INSIDE `homedir()`, so a `Home` root exposes every other user's workspace. Previews go through the same global conversion limiter, and Markdown/TXT/JSON are served as inert `text/plain`. → [architecture-invariants#filesystem-path-picker](docs/architecture-invariants.md#filesystem-path-picker) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index f446f4a1..0a4ac07e 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -407,7 +407,9 @@ A file path an agent prints is a link on both surfaces it can appear on, and cli ⚠️ **The terminal routes an out-of-workspace path to the preview, not the log viewer.** The log viewer spawns `tail -f` and allows only the workspace, `/var/log` and `~/logs`, so an external `.log`/`.json`/code path answered `Path must be within working directory or allowed log directories` while the SAME path clicked in the response viewer previewed fine. `activate()` now checks `_isExternalPreviewPath` alongside `previewsInFileViewer`. In-workspace text keeps the tail viewer, which is the point of it (live follow), except markdown (`.md`/`.markdown`, in `FILE_PREVIEW_EXTENSIONS` since the owner's 2026-10-13 decision, rendered as a document) and `.html`/`.htm` (`rendersAsPage()`, rendered as a page, see below), which always go to the preview; nothing widened `file-stream-manager`'s allowlist, so no `tail -f` is spawned on an arbitrary host path. -**Path forms beyond the rooted one.** `findFilePathLinks(text)` (constants.js) is the single entry point both consumers call; it returns the rooted matches first, then `~/` and relative paths (`RELATIVE_FILE_PATH_LINK_PATTERN`, same extension list, pinned equal by `test/link-provider-regex.test.ts`), then folder paths, each only where it overlaps neither an earlier match nor an `http(s)` URL. A relative match must start at a boundary (line start, whitespace, quote, bracket, `=`, `,`, `>`), so it can never begin inside a word, an absolute path or a URL; brackets, `=` and `,` end it (`(shot.png)`, `OUT=shots/a.png`). `resolveLinkedFilePath()` resolves a relative path against the session's `workingDir` (`.`/`..` collapsed, so `../x.png` lands outside and routes like any external path) and passes absolute and `~/` paths through. ⚠️ `~` is the home of the SESSION'S host, which only the server knows: `~/` paths always take the attachment route, where `registerExternalAttachment` expands them (`homedir()` locally, `$HOME` inside the remote probe script), and the guard runs on the expanded, realpath-resolved path as before. ⚠️ Programs hard-wrap their own output short of the terminal edge (Claude Code breaks tool output at 59-118 columns), so xterm's `isWrapped` never fires. `terminalPathLine()` (link provider only; touch selection keeps `terminalLogicalLine`) joins the next row when the previous row ends in a path token with a `/` and no extension and the join reaches an extension; a row that starts a new path is never joined: rooted or `~/`, `./`/`../`, or a relative one that opens on the same top directory the broken token did (the next entry of `git ls-files`/`find`/`ls -R`: `src/components` + `src/index.ts` are two paths, not `src/componentssrc/index.ts`), while a wrapped path continues INSIDE a name, so `…/captures/2` + `026-10-10/…` still joins; a 3+ row chain is kept only if a later row completes the path, and the broken row must be at least as long as the continuing one (keeps `cd /mnt/foo` + a longer next line apart). Tests: `test/file-path-link-forms.test.ts`. +**Path forms beyond the rooted one.** `findFilePathLinks(text)` (constants.js) is the single entry point both consumers call; it reserves spaced candidates first (including unlinkable spans), then detects rooted matches, then `~/` and relative paths (`RELATIVE_FILE_PATH_LINK_PATTERN`, same extension list, pinned equal by `test/link-provider-regex.test.ts`), then folder paths, each only where it overlaps neither an earlier match nor an `http(s)` URL. A relative match must start at a boundary (line start, whitespace, quote, bracket, `=`, `,`, `>`), so it can never begin inside a word, an absolute path or a URL; brackets, `=` and `,` end it (`(shot.png)`, `OUT=shots/a.png`). `resolveLinkedFilePath()` resolves a relative path against the session's `workingDir` (`.`/`..` collapsed, so `../x.png` lands outside and routes like any external path) and passes absolute and `~/` paths through. ⚠️ `~` is the home of the SESSION'S host, which only the server knows: `~/` paths always take the attachment route, where `registerExternalAttachment` expands them (`homedir()` locally, `$HOME` inside the remote probe script), and the guard runs on the expanded, realpath-resolved path as before. ⚠️ Programs hard-wrap their own output short of the terminal edge (Claude Code breaks tool output at 59-118 columns), so xterm's `isWrapped` never fires. `terminalPathLine()` (link provider only; touch selection keeps `terminalLogicalLine`) joins the next row when the previous row ends in a path token with a `/` and no extension and the join reaches an extension; a row that starts a new path is never joined: rooted or `~/`, `./`/`../`, or a relative one that opens on the same top directory the broken token did (the next entry of `git ls-files`/`find`/`ls -R`: `src/components` + `src/index.ts` are two paths, not `src/componentssrc/index.ts`), while a wrapped path continues INSIDE a name, so `…/captures/2` + `026-10-10/…` still joins; a 3+ row chain is kept only if a later row completes the path, and the broken row must be at least as long as the continuing one (keeps `cd /mnt/foo` + a longer next line apart). Tests: `test/file-path-link-forms.test.ts`. + +Spaced paths are considered first by `findFilePathLinks()`: accepted candidates reserve their entire span even when unlinkable, so later rooted, relative and folder detectors cannot link fragments. Relative candidates whose first slash-containing word is their last word defer to ordinary detection to exclude list/label prose. This deliberately gives up spaces only in the top directory (`Saved to: 03 Attachments/plan.pptx`); prose shaped like deeper spaced paths (`- Fixed bug in src/My Docs/a.md`, `/home/me/proj contains a.md`) remains ambiguous. Trailing whitespace uses linear `trimEnd()` before the length cap. **Folder grid: a folder is granted, then served by bare file name.** A folder path (rooted or `~/` whose last segment has no extension, relative only with a trailing `/`; `FOLDER_PATH_LINK_PATTERN` + `folderLinkPath()`) opens `openFolderGrid()` (panels-ui.js), as does the preview's "Show folder" button. `POST /api/sessions/:id/folders {path}` (`src/folder-listing.ts`) resolves the folder (locally a bounded `probePathKind(…, { pastCap: true })` first, then async `fs.realpath`; remotely one ssh round trip via `remoteListFolder`, through `runWithRemoteSshLimit`), runs the attachment guard on it (blocklist, `confineToWorkspace`, and in multi-user mode `folderAllowedForUser`: a non-admin only inside their own `userSpacePath` or the session's `workingDir`, as the filesystem path picker), locally BEFORE reading it, lists non-hidden subfolders and files of a supported attachment type (each file also blocklist-checked, newest first, capped at `MAX_FOLDER_ENTRIES`), and GRANTS it an id. Files are then fetched as `GET …/folders/:folderId/files/:name/(raw|preview|thumbnail)`: `:name` must be a bare file name of a supported type (no `/`, `\`, NUL, no leading `.`, since the listing hides those), the local file is probed bounded (`unknown` → 503, never 404), and the joined path goes through the SAME `resolveServableAttachmentPath` as an attachment by id (symlink re-resolution, remote re-probe, guard), then the same gate registration applies to what a symlink RESOLVES to (a supported extension on the resolved path and a regular file: locally `fs.stat`, remotely the probe kind; else 400, so `shot.png -> ../secret.bin` is not served and a FIFO or device behind a supported name never reaches `open`), then `folderAllowedForUser` once more on the resolved file, before `serveServableRaw` (the attachment raw route's body, shared) streams it. So browser requests still never carry a path. ⚠️ Grants live in their own registry (`folderGrants`, 32 per session, cleared with the session's attachments), NOT the attachment registry: that one is capped at 200 per session, and paging a folder of 300 renders would evict the session's real attachments. ⚠️ A local folder is never touched unbounded (#516 rule): a folder on a network mount that stopped answering answers 503 with `describeUnknownPath()`, not 404 and not a frozen server. ⚠️ The remote listing script tells "missing" (`E|n`, 404) from "is a file" (`E|f`, 400 `Not a folder`), because the client falls back to the file preview on `Not a folder`; a remote extensionless file otherwise answered "Folder not found". ⚠️ `.folder-grid-overlay` sits at `z-index: 5090`, one step BELOW the preview (5100) and above the response viewer (5000): a tile's preview opens on top and closing it lands back on the grid. ⚠️ ←/→/Esc are handled in a document CAPTURE listener with `stopPropagation`, because the terminal keeps keyboard focus under the overlays and an arrow that reached xterm would be typed into the session; inputs other than xterm's helper textarea keep their keys. ⚠️ A route PARAMETER is capped by the router (Fastify's default is 100 characters, answered as 404 "Route not found"), and a file name goes up to 255, percent-encoded: the server sets `routerOptions.maxParamLength` to `MAX_ROUTE_PARAM_LENGTH` (route-helpers.ts), the route test harness uses the same constant, and a wiring guard pins it. ⚠️ `~/` in a Docker case is refused with 400 (attachment POST and `POST …/folders`): the session is local for these routes, but `~` inside the container is the container's own home, and expanding it on the host would open a different file than the one the agent printed. Tests: `test/routes/folder-grid-routes.test.ts` (incl. the listing script on a real `sh`), `test/folder-grid-ui.test.ts`. diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 3308dfd2..60b8a8bc 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2154,9 +2154,9 @@ const RELATIVE_FILE_PATH_LINK_PATTERN = const FILE_LINK_EXTENSION_GROUP = /\\\.(\(\?:[^()]*\))\)\\b$/.exec(FILE_PATH_LINK_PATTERN.source)[1]; /** - * Every linkable file path in `text`, in order: absolute (rooted) paths first, - * then `~/` and relative ones, then folder paths — each only where it overlaps - * neither an earlier one nor a URL. + * Every linkable file path in `text`, sorted by position. Spaced candidates + * reserve their whole span first, even when unlinkable; rooted, `~/`, relative + * and folder detectors then skip reserved spans, earlier matches and URLs. * * The one entry point for both consumers (terminal link provider, response * viewer), so a path form is clickable in both or in neither. @@ -2227,7 +2227,7 @@ function findFilePathLinks(text) { * * /mnt/c/My Docs/report.docx the whole line is the path * - /mnt/c/My Docs/report.docx after a list marker (`-`, `*`, `•`, `1.`, `2)`) - * Saved to: 03 Attachments/plan.pptx after a short label (one to four words + `:`) + * Saved to: 03 Attachments/Build SIPOC Design/plan.pptx after a short label (one to four words + `:`) * * An absolute path may stand alone; a relative or `~/` one needs a marker or a * label, since a bare spaced relative candidate is ordinary prose. One trailing @@ -2248,7 +2248,7 @@ function spacedFilePathOnLine(line) { const text = String(line || ''); const prefix = /^[ \t]*((?:\d{1,3}[.)]|[-*•])[ \t]+)?((?:[A-Za-z][\w-]{0,30}(?: [\w-]{1,30}){0,3}):[ \t]+)?/.exec(text); const index = prefix[0].length; - const path = text.slice(index).replace(/\s+$/, '').replace(/[.,;:!?]$/, ''); + const path = text.slice(index).trimEnd().replace(/[.,;:!?]$/, ''); if (!path.includes(' ') || !path.includes('/') || path.length > 1000) return null; if (!path.startsWith('/') && !prefix[1] && !prefix[2]) return null; if (!PATH_TOKEN_ENDS_WITH_EXTENSION.test(path)) return null; @@ -2262,6 +2262,11 @@ function spacedFilePathOnLine(line) { const rooted = SPACED_PATH_ROOT.test(path) || path.startsWith('~/'); const segments = path.replace(/^~?\//, '').split('/'); const link = (rooted || !path.startsWith('/')) && segments.every((seg) => seg && seg !== '.' && seg !== '..'); + // A lone slash-containing final word after plain words is usually prose + // ("- Updated src/foo.ts"), not a spaced top-level directory. Let the + // ordinary detectors handle it, including explicit relative paths. + const firstSlash = words.findIndex((word) => word.includes('/')); + if (link && firstSlash === words.length - 1 && !path.startsWith('/')) return null; return { path, index, link }; } diff --git a/test/spaced-file-path-links.test.ts b/test/spaced-file-path-links.test.ts index b799a888..8f49b89c 100644 --- a/test/spaced-file-path-links.test.ts +++ b/test/spaced-file-path-links.test.ts @@ -91,6 +91,32 @@ describe('spaced file paths', () => { expect(paths(`• Design committed in the isolated worktree: ${rel}`)).toEqual([rel]); }); + it.each([ + ['- Updated src/foo.ts', 'src/foo.ts'], + ['1. Added a test in test/foo.test.ts', 'test/foo.test.ts'], + ['• Ran npm test -- test/foo.test.ts', 'test/foo.test.ts'], + ['Error: cannot open src/foo.ts', 'src/foo.ts'], + ['Note: see src/foo.ts', 'src/foo.ts'], + ['- Moved ../a/b.ts', '../a/b.ts'], + ['Saved to: 03 Attachments/plan.pptx', 'Attachments/plan.pptx'], + ])('defers ambiguous relative prose to ordinary detectors: %s', (line, path) => { + expect(paths(line)).toEqual([path]); + }); + + it('pins residual ambiguity when spaces occur below the first directory', () => { + // Without filesystem knowledge these still have the shape of spaced paths. + expect(paths('- Fixed bug in src/My Docs/a.md')).toEqual(['Fixed bug in src/My Docs/a.md']); + expect(paths('/home/me/proj contains a.md')).toEqual(['/home/me/proj contains a.md']); + expect(paths('Saved to: ~/My Docs/plan.md')).toEqual(['~/My Docs/plan.md']); + }); + + it.each([40000, 80000])('stays linear with %i interior spaces', (length) => { + const line = '/home/a' + ' '.repeat(length) + 'x'; + const start = performance.now(); + findFilePathLinks(line); + expect(performance.now() - start).toBeLessThan(500); + }); + it('leaves unspaced paths to the ordinary detectors', () => { expect(paths('- /mnt/c/proj/docs/report.docx')).toEqual(['/mnt/c/proj/docs/report.docx']); expect(paths('• committed as /tmp/x/docs/specs/design.md:1 (e7ae1b4).')).toEqual(['/tmp/x/docs/specs/design.md']); From 40a9528be11b294b22e076d89624d8e9cda0ac38 Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Fri, 23 Oct 2026 07:26:15 -0400 Subject: [PATCH 004/191] fix(terminal): keep unrelated rows out of wrapped links --- CLAUDE.md | 2 ++ docs/architecture-invariants.md | 2 ++ src/web/public/constants.js | 21 ++++++++++++------ src/web/public/terminal-ui.js | 5 +++-- test/link-provider-regex.test.ts | 11 +++++----- test/terminal-cell-wrapped-paths.test.ts | 27 ++++++++++++++++++++++++ test/terminal-logical-line.test.ts | 6 ++++++ test/terminal-url-continuation.test.ts | 19 ++++++++++++++++- 8 files changed, 77 insertions(+), 16 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 02704d0f..f528681b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -224,6 +224,8 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **File-path links (terminal + chat)**: a path an agent prints is clickable on BOTH surfaces and opens the file-preview overlay. ⚠️ ONE pattern (`FILE_PATH_LINK_PATTERN` / `absoluteFilePathPattern()` in constants.js) feeds the xterm link provider AND `_linkifyFilePaths()`, a fresh instance per call (`lastIndex`). Both call `findFilePathLinks()`, which adds `~/`, relative and folder paths without overlapping a rooted match or a URL; a relative path resolves against `workingDir` (`resolveLinkedFilePath`), `~/` only server-side on the session's host (attachment route), and `terminalPathLine()` joins a path a program hard-wrapped short of the edge. ⚠️ A folder link opens the **folder grid**: `POST /api/sessions/:id/folders` guards + GRANTS the folder (`folder-listing.ts`, own registry, never the 200-capped attachment registry), files come by bare name via `…/folders/:folderId/files/:name/*` through `resolveServableAttachmentPath` plus the same extension/regular-file gate on the symlink's resolved target, so no browser request carries a path; `~/` is refused in a Docker case (the container's home is not this host's). The chat linkifier walks TEXT NODES with DOM APIs, never rebuilds sanitized markup as a string. ⚠️ An out-of-workspace path goes through the ATTACHMENT routes (`POST /api/sessions/:id/attachments` with `notify: false`), never by widening `file-content`/`file-raw` or `file-stream-manager`'s `tail -f` allowlist. ⚠️ `TEXT_ATTACHMENT_EXTENSIONS` IS `EDITABLE_EXTENSIONS` (never a second list), and widening READ must never widen RUN: `html`/`htm`/`svg` stay download-only, other text is inert `text/plain`+`nosniff`. ⚠️ The ONE place HTML renders is `/html-view//...` (`html-view-routes.ts`, `html-view-capabilities.ts`): `POST /api/sessions/:id/html-view` mints a capability for the file's own directory, every response carries `Content-Security-Policy: sandbox` WITHOUT `allow-same-origin` (opaque origin even as a top-level tab, so the page can never read Codeman or drive its API) and with NO fetch directive (open network, owner decision 2026-10-13: never restrict it or add `allow-same-origin` without the owner), and only non-dot, asset-allowlisted files under that directory are served (GET/HEAD only; capabilities revoked with the web-tab ones on logout). That tree is served recursively, so minting is refused for a hidden directory or a broad root (`isBroadHtmlViewRoot`); the preview renders by default and the **Page** pill (or a refused mint) falls through to the source view and its Edit button. Never serve HTML same-origin anywhere else. Tests: `test/routes/html-view-routes.test.ts`. Media extensions are single-sourced in `attachment-registry.ts`. ⚠️ **XLSX previews parse in the BROWSER**, never on the server; the admission limits that keep a hostile workbook from freezing the worker live in `src/web/CLAUDE.md` (Spreadsheet preview). → [architecture-invariants#file-path-links-terminal--response-viewer](docs/architecture-invariants.md#file-path-links-terminal--response-viewer) +The terminal provider scans `terminalLinkLine()`, which extends `terminalPathLine()` for URL continuations in indented paragraphs and table cells. List markers, bare numbers and version tokens never continue a URL. `terminalCellWrappedPaths()` follows each table cell independently, gated by `pathContinuation()` and the per-cell row-length rule; accepted paths displace other links on their cells. Full-width divider rows stop `terminalLogicalLine()` stitching for both links and touch selection. OSC 8 links confirm the real target URI before the shared opener runs. + **Filesystem path picker** (Link Existing "Browse" + the mobile keyboard's `📁 Path` key): lazy one-directory browsing via `GET /api/filesystem/browse`, with `GET /api/filesystem/preview` for the tapped file. Inserts the path **without** Enter, so the prompt is never submitted; the sibling `⌫ All` key clears only the unsent prompt and must never send the agent's `/clear`. ⚠️ This is a **second file-serving surface and inherits neither the attachment confinement nor its ownership scoping** — it allowlists Home, `CASES_DIR`, `/mnt/d` and `CODEMAN_FILE_PICKER_ROOTS`, blocks sensitive trees, and rejects symlink escapes **after** `realpath`. ⚠️ The optional `sessionId` is an ownership boundary that must be `canAccessOwned`-checked by hand (it does not go through `findSessionOrFail`), and in multi-user mode a non-admin gets only their own `userSpacePath` as a root: per-user spaces live INSIDE `homedir()`, so a `Home` root exposes every other user's workspace. Previews go through the same global conversion limiter, and Markdown/TXT/JSON are served as inert `text/plain`. → [architecture-invariants#filesystem-path-picker](docs/architecture-invariants.md#filesystem-path-picker) **File Viewer edit mode** (issue #212): the file-preview overlay edits workspace text files in place — `GET .../file-content?edit=1` + `PUT /api/sessions/:id/file-content`, policy in `src/config/file-editing.ts`. This is a **third file surface and the only one that WRITES**: read-path confinement (realpath + workspace + ownership) plus sensitive/blocked/`.git` denies and an extension **allowlist**; writes are `wx`-temp + rename (no `O_CREAT` anywhere = edit-in-place is structural); optimistic concurrency via sha256 `baseHash` → 409. ⚠️ `edit=1` never truncates and the client must never save a plain-preview buffer (the 500-line truncation would silently delete the rest). ⚠️ CRLF/UTF-8 guards: EOL re-applied server-side, non-UTF-8 refused via round-trip compare. → [architecture-invariants#file-viewer-edit-mode](docs/architecture-invariants.md#file-viewer-edit-mode), `docs/file-viewer-edit-plan.md` diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index f446f4a1..6dc61ed7 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -409,6 +409,8 @@ A file path an agent prints is a link on both surfaces it can appear on, and cli **Path forms beyond the rooted one.** `findFilePathLinks(text)` (constants.js) is the single entry point both consumers call; it returns the rooted matches first, then `~/` and relative paths (`RELATIVE_FILE_PATH_LINK_PATTERN`, same extension list, pinned equal by `test/link-provider-regex.test.ts`), then folder paths, each only where it overlaps neither an earlier match nor an `http(s)` URL. A relative match must start at a boundary (line start, whitespace, quote, bracket, `=`, `,`, `>`), so it can never begin inside a word, an absolute path or a URL; brackets, `=` and `,` end it (`(shot.png)`, `OUT=shots/a.png`). `resolveLinkedFilePath()` resolves a relative path against the session's `workingDir` (`.`/`..` collapsed, so `../x.png` lands outside and routes like any external path) and passes absolute and `~/` paths through. ⚠️ `~` is the home of the SESSION'S host, which only the server knows: `~/` paths always take the attachment route, where `registerExternalAttachment` expands them (`homedir()` locally, `$HOME` inside the remote probe script), and the guard runs on the expanded, realpath-resolved path as before. ⚠️ Programs hard-wrap their own output short of the terminal edge (Claude Code breaks tool output at 59-118 columns), so xterm's `isWrapped` never fires. `terminalPathLine()` (link provider only; touch selection keeps `terminalLogicalLine`) joins the next row when the previous row ends in a path token with a `/` and no extension and the join reaches an extension; a row that starts a new path is never joined: rooted or `~/`, `./`/`../`, or a relative one that opens on the same top directory the broken token did (the next entry of `git ls-files`/`find`/`ls -R`: `src/components` + `src/index.ts` are two paths, not `src/componentssrc/index.ts`), while a wrapped path continues INSIDE a name, so `…/captures/2` + `026-10-10/…` still joins; a 3+ row chain is kept only if a later row completes the path, and the broken row must be at least as long as the continuing one (keeps `cd /mnt/foo` + a longer next line apart). Tests: `test/file-path-link-forms.test.ts`. +The terminal provider scans `terminalLinkLine()`, which extends `terminalPathLine()` for URL continuations in indented paragraphs and table cells. List markers, bare numbers and version tokens never continue a URL. `terminalCellWrappedPaths()` follows each table cell independently, gated by `pathContinuation()` and the per-cell row-length rule; accepted paths displace other links on their cells. Full-width divider rows stop `terminalLogicalLine()` stitching for both links and touch selection. OSC 8 links confirm the real target URI before the shared opener runs. + **Folder grid: a folder is granted, then served by bare file name.** A folder path (rooted or `~/` whose last segment has no extension, relative only with a trailing `/`; `FOLDER_PATH_LINK_PATTERN` + `folderLinkPath()`) opens `openFolderGrid()` (panels-ui.js), as does the preview's "Show folder" button. `POST /api/sessions/:id/folders {path}` (`src/folder-listing.ts`) resolves the folder (locally a bounded `probePathKind(…, { pastCap: true })` first, then async `fs.realpath`; remotely one ssh round trip via `remoteListFolder`, through `runWithRemoteSshLimit`), runs the attachment guard on it (blocklist, `confineToWorkspace`, and in multi-user mode `folderAllowedForUser`: a non-admin only inside their own `userSpacePath` or the session's `workingDir`, as the filesystem path picker), locally BEFORE reading it, lists non-hidden subfolders and files of a supported attachment type (each file also blocklist-checked, newest first, capped at `MAX_FOLDER_ENTRIES`), and GRANTS it an id. Files are then fetched as `GET …/folders/:folderId/files/:name/(raw|preview|thumbnail)`: `:name` must be a bare file name of a supported type (no `/`, `\`, NUL, no leading `.`, since the listing hides those), the local file is probed bounded (`unknown` → 503, never 404), and the joined path goes through the SAME `resolveServableAttachmentPath` as an attachment by id (symlink re-resolution, remote re-probe, guard), then the same gate registration applies to what a symlink RESOLVES to (a supported extension on the resolved path and a regular file: locally `fs.stat`, remotely the probe kind; else 400, so `shot.png -> ../secret.bin` is not served and a FIFO or device behind a supported name never reaches `open`), then `folderAllowedForUser` once more on the resolved file, before `serveServableRaw` (the attachment raw route's body, shared) streams it. So browser requests still never carry a path. ⚠️ Grants live in their own registry (`folderGrants`, 32 per session, cleared with the session's attachments), NOT the attachment registry: that one is capped at 200 per session, and paging a folder of 300 renders would evict the session's real attachments. ⚠️ A local folder is never touched unbounded (#516 rule): a folder on a network mount that stopped answering answers 503 with `describeUnknownPath()`, not 404 and not a frozen server. ⚠️ The remote listing script tells "missing" (`E|n`, 404) from "is a file" (`E|f`, 400 `Not a folder`), because the client falls back to the file preview on `Not a folder`; a remote extensionless file otherwise answered "Folder not found". ⚠️ `.folder-grid-overlay` sits at `z-index: 5090`, one step BELOW the preview (5100) and above the response viewer (5000): a tile's preview opens on top and closing it lands back on the grid. ⚠️ ←/→/Esc are handled in a document CAPTURE listener with `stopPropagation`, because the terminal keeps keyboard focus under the overlays and an arrow that reached xterm would be typed into the session; inputs other than xterm's helper textarea keep their keys. ⚠️ A route PARAMETER is capped by the router (Fastify's default is 100 characters, answered as 404 "Route not found"), and a file name goes up to 255, percent-encoded: the server sets `routerOptions.maxParamLength` to `MAX_ROUTE_PARAM_LENGTH` (route-helpers.ts), the route test harness uses the same constant, and a wiring guard pins it. ⚠️ `~/` in a Docker case is refused with 400 (attachment POST and `POST …/folders`): the session is local for these routes, but `~` inside the container is the container's own home, and expanding it on the host would open a different file than the one the agent printed. Tests: `test/routes/folder-grid-routes.test.ts` (incl. the listing script on a real `sh`), `test/folder-grid-ui.test.ts`. **Text reuses the edit-mode allowlist; markup stays download-only.** `TEXT_ATTACHMENT_EXTENSIONS` IS `EDITABLE_EXTENSIONS` (`config/file-editing.ts`) rather than a second curated list that would drift from it: if the viewer would open a file for editing inside the workspace, the same file outside it can be read. The justification for widening is that the agent in the session can already `cat` any of these and the picker already previews them, so the suffix was never the confidentiality gate; the path guard is (sensitive-file blocklist, `/root` and `/etc` trees, realpath first). ⚠️ Two consequences had to be handled at the same time: `~/.codeman*/state.json` joined `isSensitivePath` (it persists `SessionState.envOverrides`, and the env allowlist admits key-shaped names like `GEMINI_API_KEY`, so it can hold a live credential), and `html`/`htm` joined `svg` in `serveRawFile`'s **download-only** branch so that widening what can be READ never widens what can RUN on our own origin. Text with no dedicated MIME entry goes out as inert `text/plain; charset=utf-8` + `nosniff`, matching the picker. ⚠️ **The one place HTML renders is `/html-view//...`** (`html-view-routes.ts`, `html-view-capabilities.ts`), never these routes: `POST /api/sessions/:id/html-view` mints a capability for the HTML file's own directory, and every response carries `Content-Security-Policy: sandbox` WITHOUT `allow-same-origin`, so the page is opaque-origin even as a top-level tab and cannot read Codeman or drive its API. ⚠️ **Network access is open, by the owner's decision (2026-10-13, #547):** `HTML_VIEW_CSP` is the `sandbox` directive alone, which REPLACES Codeman's own CSP, with no fetch directive, so a page loads scripts, styles, fonts, images and media from anywhere and may fetch, XHR, open WebSockets, send beacons and post forms anywhere, like a file opened from disk. It can read its own folder through the capability and nothing else on this server; it cannot use Codeman's origin, cookies or API, or reach other folders. Never add a fetch directive or `allow-same-origin` without the owner; the exact header is pinned in `test/routes/html-view-routes.test.ts`. The directory is served recursively to the page's scripts (non-dot, asset-allowlisted, realpath-confined, blocklist applied, GET/HEAD only; the dot and extension rules run on the requested name AND on the symlink-resolved target, or a `cfg.json -> .git/config` link would serve the git config), which is why minting is refused for an HTML file in a hidden directory (for a file inside the workspace, a hidden directory below the workspace root, so a workspace that itself lives under one still renders) or directly in a broad root (`/`, home, `/tmp`, `/var/tmp` and `$TMPDIR` (named literally, since macOS sets `TMPDIR` in every terminal), the cases and user-space roots, a drive's mount point such as WSL's `/mnt/d`) or in any directory above one of them (`/home`, `/Users`, macOS's `/private`; `isBroadHtmlViewRoot`). A capability minted for a folder outside the workspace records that, and the serving route refuses it per request once `attachmentConfineToWorkspace` is on, the same re-check the attachment serve path makes. The preview renders it by default; the **Page** pill (`htmlRendered`, per-device like `mdRendered`) and any refused mint fall through to the source view with its Edit button. Tests: `test/routes/html-view-routes.test.ts`, the html-view block in `test/webview-auth-exemption.test.ts`, `test/webview-capability-revocation.test.ts`. The by-id text preview is bounded like the workspace one: a `Range` request for the first 512KB (a real partial read, not a discarded 50MB download) plus a 500-line cap, with the footer saying so. diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 6e8d4bff..b89bfdb7 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2568,7 +2568,8 @@ function findTerminalUrls(text) { * - the next row is indented no further right than the column the URL * started in (a table cell or list item continuation, not unrelated text); * - that row's first token looks like URL material (contains one of - * `% & = / ? . - _` or a digit) and is not itself a new `scheme://`. + * `% & = / ? . - _`), is not a bare number/version or list marker, + * and is not itself a new `scheme://`. * The last rule is what stops prose such as "…/files/" + "then run it" from * being glued into one link. * @@ -2609,7 +2610,7 @@ function terminalUrlContinuationExtent(buffer, row, cols, maxRows) { const nextText = next.translateToString(true) || ''; const indent = nextText.length - nextText.replace(/^\s+/, '').length; const token = nextText.slice(indent).split(/\s/, 1)[0]; - if (!token || indent > urlStartCol) break; + if (!token || indent > urlStartCol || /^(?:\d+[.)]|[-*•])(?:\s|$)/.test(nextText.slice(indent))) break; // When other table columns follow the URL, the next row must align with // the start of its cell. A new numbered row must not become URL text. if ( @@ -2621,7 +2622,7 @@ function terminalUrlContinuationExtent(buffer, row, cols, maxRows) { ) break; if (/^[a-z][a-z0-9+.-]*:\/\//i.test(token) || !/^[A-Za-z0-9%_~]/.test(token)) break; - if (!/[%&=/?.\-_0-9]/.test(token)) break; + if (!/[%&=/?.\-_]/.test(token) || /^v?\d+(?:\.\d+)*(?:[-+][\w.-]+)?[.,;:!?)]?$/i.test(token)) break; const nextLogical = terminalLogicalLine(buffer, endRow + 1, cols, bound); if (!nextLogical) break; if (trailingColumns) firstRowUrlEnd = last.rawEnd; @@ -2716,9 +2717,9 @@ function terminalLinkLine(buffer, row, cols, maxRows) { * edge is where its text starts after the previous ≥2-space column gap, and the * continuation is the token starting at exactly that column on the next row. * A fragment qualifies only when it is the last thing in its cell on that row - * (row end or a ≥2-space gap follows), and the join is accepted only when the - * whole joined string matches the path pattern from its first character, which - * is what keeps unrelated cell text from being glued on. + * (row end or a ≥2-space gap follows). Every continuation must pass + * pathContinuation and its cell must be no wider than the preceding cell's + * text, so independent listing rows and longer prose never displace links. * * Pure: reads rows only. Cells are taken one per string character (xterm rows * with wide glyphs would shift columns; such rows simply fail the alignment). @@ -2768,6 +2769,7 @@ function terminalCellWrappedPaths(buffer, row, maxRows) { while ((e = edge.exec(upToPath)) !== null) cellCol = e.index + e[0].length; let joined = text.slice(start, end); + let previousCellLength = end - cellCol; const segments = [{ row: startRow, startCol: start, endCol: end }]; for (let r = startRow + 1; r < startRow + bound; r++) { const line = buffer.getLine(r); @@ -2776,7 +2778,11 @@ function terminalCellWrappedPaths(buffer, row, maxRows) { if (!next || !isPathChar(next[cellCol]) || (cellCol > 0 && next[cellCol - 1] !== ' ')) break; let tokenEnd = cellCol; while (tokenEnd < next.length && isPathChar(next[tokenEnd])) tokenEnd++; - const candidate = joined + next.slice(cellCol, tokenEnd); + const token = next.slice(cellCol, tokenEnd); + // Include prose after the token in this cell, but not another column. + const cellText = next.slice(cellCol).split(/ {2,}|[│┃║|]/, 1)[0].trimEnd(); + if (cellText.length > previousCellLength || !pathContinuation(joined, token)) break; + const candidate = joined + token; const match = absoluteFilePathPattern().exec(candidate); if (match && match.index === 0 && match[0].length > joined.length) { segments.push({ row: r, startCol: cellCol, endCol: cellCol + match[0].length - joined.length }); @@ -2785,6 +2791,7 @@ function terminalCellWrappedPaths(buffer, row, maxRows) { } if (!endsCell(next, tokenEnd)) break; joined = candidate; + previousCellLength = cellText.length; segments.push({ row: r, startCol: cellCol, endCol: tokenEnd }); } } diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index e4794be6..6d6ac4d8 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -2408,13 +2408,14 @@ Object.assign(CodemanApp.prototype, { /** * xterm's handler for OSC 8 hyperlinks (a program's own `ESC ]8;;url` links), * which take precedence over the text link provider. Routed through the same - * opener, and only for web URLs: any other scheme a program sends is ignored. + * opener after confirming the real target (the label may hide it), and only + * for web URLs: any other scheme a program sends is ignored. */ _terminalOsc8LinkHandler() { return { activate: (_event, text) => { if (!/^https?:\/\/[^\s\x00-\x1f]+$/i.test(text)) return; - this._openTerminalUrl(text); + if (window.confirm(`Open this link?\n\n${text}`)) this._openTerminalUrl(text); }, }; }, diff --git a/test/link-provider-regex.test.ts b/test/link-provider-regex.test.ts index 7b896097..7eaeb1f8 100644 --- a/test/link-provider-regex.test.ts +++ b/test/link-provider-regex.test.ts @@ -17,6 +17,7 @@ import { describe, it, expect } from 'vitest'; import { readFileSync } from 'fs'; import { join } from 'path'; +import vm from 'node:vm'; const publicFile = (name: string) => readFileSync(join(__dirname, '..', 'src', 'web', 'public', name), 'utf-8'); @@ -99,8 +100,9 @@ describe('terminal link-provider regexes (shipped source)', () => { // 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 context = vm.createContext({ console, window: undefined }); + vm.runInContext(`${CONSTANTS_SOURCE}\nglobalThis.findUrls = findTerminalUrls;`, context); + const findUrls = context.findUrls as (text: string) => Array<{ url: string }>; const cases: Array<[string, string]> = [ [ 'updated in place: https://bio-hacking.blog/wp-admin/post.php?post=1479&action=edit', @@ -115,10 +117,7 @@ describe('terminal link-provider regexes (shipped source)', () => { ['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); + expect(findUrls(line).map((match) => match.url), line).toEqual([want]); } }); diff --git a/test/terminal-cell-wrapped-paths.test.ts b/test/terminal-cell-wrapped-paths.test.ts index 6ed0da6a..7b25672d 100644 --- a/test/terminal-cell-wrapped-paths.test.ts +++ b/test/terminal-cell-wrapped-paths.test.ts @@ -57,6 +57,14 @@ const TABLE: Row[] = [ { text: ROW2 }, { text: '' }, ]; +const LISTINGS = [ + ['/tmp/x', '/tmp/x/dir', '/tmp/x/dir/a.png'], + [' ⎿ /home/u/proj/src', ' /home/u/proj/src/a.ts'], + ['4.0K /tmp/foo/bar/', '8.0K /tmp/foo/baz.png'], + ['/home/u/proj/docs:', 'guide.md notes.md'], + ['Created /tmp/out', 'report.md written next to it'], +]; + const BEFORE = '/tmp/demo-check/docs/review/BLDMESH-243/before-overview.png'; const AFTER = '/tmp/demo-check/docs/review/BLDMESH-243/after-overview.png'; @@ -70,6 +78,11 @@ describe('terminalCellWrappedPaths', () => { } }); + it.each(LISTINGS.map((lines) => [lines]))('does not join independent listing rows: %j', (lines) => { + const rows = lines.map((text) => ({ text })); + for (let row = 0; row < rows.length; row++) expect(find(fakeBuffer(rows, COLS), row)).toEqual([]); + }); + it('places the segments on the path cells only', () => { const hits = find(fakeBuffer(TABLE, COLS), 1); const before = hits.find((h) => h.path === BEFORE)!; @@ -188,6 +201,20 @@ describe('terminal link provider with cell-wrapped paths', () => { return { provide, app }; } + it.each(LISTINGS.map((lines) => [lines]))('preserves ordinary listing links: %j', (lines) => { + const { provide } = harness(lines.map((text) => ({ text }))); + const expected = [ + ['/tmp/x', '/tmp/x/dir', '/tmp/x/dir/a.png'], + ['/home/u/proj/src', '/home/u/proj/src/a.ts'], + ['/tmp/foo/bar/', '/tmp/foo/baz.png'], + ['/home/u/proj/docs', 'guide.md'], + ['/tmp/out', 'report.md'], + ][LISTINGS.indexOf(lines)]; + for (let row = 0; row < lines.length; row++) { + expect(provide(row + 1).map((link) => link.text)).toContain(expected[row]); + } + }); + it('makes both screenshot paths clickable from either row and opens the whole path', () => { const { provide, app } = harness(TABLE); for (const line of [2, 3]) { diff --git a/test/terminal-logical-line.test.ts b/test/terminal-logical-line.test.ts index 6832216d..9cb3ca9c 100644 --- a/test/terminal-logical-line.test.ts +++ b/test/terminal-logical-line.test.ts @@ -64,6 +64,12 @@ describe('terminalLogicalLine', () => { const ROW1 = ' ackage/thing'; const HARD = [{ text: ROW0 }, { text: ROW1 }]; + it('keeps full-width dividers out of link and touch-selection text', () => { + const rows = buffer([{ text: '─'.repeat(COLS) }, { text: 'https://example.com/docs' }]); + expect(terminalLogicalLine(rows, 0, COLS)).toMatchObject({ startRow: 0, endRow: 0, text: '─'.repeat(COLS) }); + expect(terminalLogicalLine(rows, 1, COLS)).toMatchObject({ startRow: 1, endRow: 1, text: 'https://example.com/docs' }); + }); + it('stitches a hard wrap and drops the indent the continuation carries', () => { const line = terminalLogicalLine(buffer(HARD), 0, COLS)!; diff --git a/test/terminal-url-continuation.test.ts b/test/terminal-url-continuation.test.ts index 9616ac53..6d97ac2f 100644 --- a/test/terminal-url-continuation.test.ts +++ b/test/terminal-url-continuation.test.ts @@ -168,6 +168,16 @@ describe('terminalUrlContinuationExtent (wrap joining)', () => { expect(terminalUrlContinuationExtent(buffer, 4, COLS, 12)).toBeNull(); }); + it.each([ + ['1. https://example.com/one/', '2. https://example.com/two/'], + ['Docs: https://docs.example.com/guide/', '1. Install the package'], + ['Fetching https://registry.example.com/pkg/', 'v1.2.3 is the latest'], + ['GET http://localhost:3000/api/', '200 OK in 12ms'], + ])('keeps list, version and status rows separate after %s', (first, next) => { + const buffer = fakeBuffer([{ text: first }, { text: next }], 100); + for (const row of [0, 1]) expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toBeNull(); + }); + it('does not glue a following prose line onto a URL that ends a line', () => { const buffer = fakeBuffer( [{ text: 'Download from https://example.com/files/' }, { text: 'then run the installer' }], @@ -204,11 +214,12 @@ describe('terminalUrlContinuationExtent (wrap joining)', () => { describe('native OSC 8 terminal URLs', () => { it('opens complete HTTP links through the terminal opener and rejects unsafe schemes', () => { const open = vi.fn(); + const confirm = vi.fn(() => true); const context = vm.createContext({ console, CodemanApp: class CodemanApp {}, document: { addEventListener: vi.fn() }, - window: { addEventListener: vi.fn(), removeEventListener: vi.fn() }, + window: { addEventListener: vi.fn(), removeEventListener: vi.fn(), confirm }, }); vm.runInContext(`${CONSTANTS}\n${TERMINAL_UI}\nglobalThis.__proto = CodemanApp.prototype;`, context); const proto = ( @@ -220,10 +231,16 @@ describe('native OSC 8 terminal URLs', () => { const url = 'https://docs.example.com/Doc.aspx?file=Q3%20Plan&action=default&mobileredirect=true'; handler.activate(null, url); expect(open).toHaveBeenCalledExactlyOnceWith(url); + expect(confirm).toHaveBeenCalledExactlyOnceWith(`Open this link?\n\n${url}`); + confirm.mockReturnValue(false); + handler.activate(null, 'http://localhost:3000/private'); + expect(confirm).toHaveBeenLastCalledWith('Open this link?\n\nhttp://localhost:3000/private'); + expect(open).toHaveBeenCalledTimes(1); for (const unsafe of ['javascript:alert(1)', 'data:text/html,hello', 'file:///etc/passwd']) { handler.activate(null, unsafe); } expect(open).toHaveBeenCalledTimes(1); + expect(confirm).toHaveBeenCalledTimes(2); }); }); From 95e2272f893589fb01fbc198155fa948c8235cbd Mon Sep 17 00:00:00 2001 From: Randalix Date: Fri, 23 Oct 2026 14:00:48 +0200 Subject: [PATCH 005/191] feat(edit-history): edit history panel for local claude sessions Part 1 of #643 (local sessions only; remote follows as its own PR). An opt-in, per-device header button opens a panel with the active session's edits, newest first: every Edit/Write/MultiEdit/NotebookEdit of the main agent and its subagents with its diff, plus images a Bash call rendered or the agent sent. Remote, Docker and non-claude sessions answer `unsupported`. - Reads only on request: `?sync=1` when the panel opens; while it is open on that session it re-reads after an edit-capable `transcript:tool_end` or a `transcript:complete` from the existing transcript watcher. No hook, timer, second watcher or new SSE event; a wiring guard pins it. - Main transcript: the path the transcript watcher follows (hook-reported), else `.jsonl` under the session's own CLAUDE_CONFIG_DIR override or the server's config dir. A config dir set outside Codeman is found with the next hook; until then the panel says so. - Store: dataPath('edit-history')/.json, 0600 via tmp+rename, capped at 500 entries and 4 MiB of diff text per session, deleted with the session, blocked in isSensitivePath, never fed to /api/search. Every route goes through findSessionOrFail with req. - Images only through existing file surfaces: file-raw inside the workspace, the attachment route with notify:false outside it. - textContent rendering, bounded byte-offset reads, marker-class header button, hidden on phones (locked in the mobile policy test). Docs: Working-With-Files, Settings-Reference, architecture invariants, API reference, CLAUDE.md key pattern. Co-Authored-By: Claude Opus 5.5 (1M context) --- CLAUDE.md | 2 + config/test-suites.ts | 1 + docs/api-reference.md | 12 + docs/architecture-invariants.md | 16 + docs/wiki/Settings-Reference.md | 8 +- docs/wiki/Working-With-Files.md | 46 ++ src/edit-history-local-source.ts | 218 ++++++++ src/edit-history-parse.ts | 330 +++++++++++ src/edit-history-store.ts | 451 +++++++++++++++ src/session.ts | 9 +- src/web/edit-history-sync.ts | 307 +++++++++++ src/web/public/app.js | 6 + src/web/public/edit-history-ui.js | 643 ++++++++++++++++++++++ src/web/public/i18n.js | 33 ++ src/web/public/index.html | 25 + src/web/public/mobile.css | 2 + src/web/public/settings-ui.js | 8 +- src/web/public/styles.css | 291 ++++++++++ src/web/routes/edit-history-routes.ts | 130 +++++ src/web/routes/index.ts | 1 + src/web/sensitive-path.ts | 4 + src/web/server.ts | 31 ++ test/edit-history-local-source.test.ts | 134 +++++ test/edit-history-parse.test.ts | 355 ++++++++++++ test/edit-history-store.test.ts | 445 +++++++++++++++ test/edit-history-sync.test.ts | 379 +++++++++++++ test/edit-history.browser.test.ts | 468 ++++++++++++++++ test/mobile-header-buttons-policy.test.ts | 10 +- test/mocks/mock-session.ts | 7 + test/routes/edit-history-routes.test.ts | 261 +++++++++ test/sensitive-path.test.ts | 4 + 31 files changed, 4631 insertions(+), 6 deletions(-) create mode 100644 src/edit-history-local-source.ts create mode 100644 src/edit-history-parse.ts create mode 100644 src/edit-history-store.ts create mode 100644 src/web/edit-history-sync.ts create mode 100644 src/web/public/edit-history-ui.js create mode 100644 src/web/routes/edit-history-routes.ts create mode 100644 test/edit-history-local-source.test.ts create mode 100644 test/edit-history-parse.test.ts create mode 100644 test/edit-history-store.test.ts create mode 100644 test/edit-history-sync.test.ts create mode 100644 test/edit-history.browser.test.ts create mode 100644 test/routes/edit-history-routes.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 02704d0f..15dd8e25 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -232,6 +232,8 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Git status indicator** (`showGitStatus`, per-device, default OFF; `src/git-workspace-status.ts`, `routes/git-status-routes.ts`, `git-status-ui.js`): the bottom-bar indicator and its panel read `GET /api/sessions/:id/git-status` and `/git-diff`, read-only and offline (it never fetches and never writes). ⚠️ git never runs on a repository at or inside a Docker case workspace (walk-up, scan and diff alike, and a cached repository list is re-checked against the CURRENT Docker roots), since a container could plant a clean filter that runs on the host; remote and Docker sessions answer `unsupported`. ⚠️ `git-diff` takes `repo`/`path`/`kind` only as keys matched against the workspace's own repository list (`findWorkspaceRepo()`) and that repository's current status, never as paths. ⚠️ Keep `--no-optional-locks`, `core.fsmonitor=false` and `log.showSignature=false` on every call and `--no-ext-diff --no-textconv` on diffs; clean filters still run, which is why the Docker rule exists. ⚠️ An enclosing repository at `$HOME` or above is ignored (`isUnrelatedAncestor()`), and is identified by one cached `rev-parse` before any full status runs. Everything git supplies renders via `textContent`. Tests: `test/git-workspace-status.test.ts`, `test/routes/git-status-routes.test.ts`, and `test/git-status.browser.test.ts` (browser suite, not in the gate). +**Edit history panel** (`showEditHistory`, per-device, default OFF; `src/edit-history-parse.ts`, `edit-history-store.ts`, `edit-history-local-source.ts`, `web/edit-history-sync.ts`, `routes/edit-history-routes.ts`, `edit-history-ui.js`): which files a LOCAL claude session edited (main agent and subagents, labelled), each edit's diff, and images a Bash call produced. Read from the session's transcripts by byte offset, never from git. `GET /api/sessions/:id/edits[?sync=1]`, `GET …/edits/:entryId` (unified diff), `POST …/edits/older`. ⚠️ **Reads only on request:** the panel sends `?sync=1` when it opens and, while it is open on that session, after an edit-capable `transcript:tool_end` or a `transcript:complete` (debounced). The server never syncs on its own: no hook, no timer, no second watcher; `server.ts` hands the reader to the routes and only calls `remove`/`flushAll` itself (wiring-guard test). The main transcript is the path the session's `TranscriptWatcher` follows (reported by the hooks), else `.jsonl` under the session's `CLAUDE_CONFIG_DIR` override or the server's (`resolveClaudeTranscript`); a config dir set outside Codeman is found with the next hook, until then `unavailable` / `no-transcript`. ⚠️ The store holds every diff, secrets included: `dataPath('edit-history')/.json`, 0600 via tmp + rename, capped per session (500 entries, 4 MiB diff text), deleted with the session, blocked in `isSensitivePath`, never fed to `/api/search`. Images use the existing file surfaces only (workspace `file-raw`; outside it the attachment route with `notify: false` and its extension allowlist); no route here serves a file. A Bash image counts only when the file's mtime lies in the call's window (±2 s) and no entry already covers that file version. Remote, Docker and non-claude sessions answer `unsupported`. Tests: `test/edit-history-*.test.ts`, `test/routes/edit-history-routes.test.ts`, `test/edit-history.browser.test.ts` (browser suite). + **Raw file bodies are streamed and range-aware**: `file-raw`, the attachments `/raw` route, `GET /api/download` and the rendered-HTML route `GET /html-view//...` share `sendFileBody()`, advertise `Accept-Ranges: bytes` and answer `Range` with `206` + `Content-Range` (single-range, parser in `src/web/http-range.ts`); without it `